Patchwork [2/2] i2c-designware-pci: Index Haswell ULT bus names from 0

login
register
mail settings
Submitter Benson Leung
Date Oct. 21, 2013, 3:26 a.m.
Message ID <1382326010-4554-3-git-send-email-bleung@chromium.org>
Download mbox | patch
Permalink /patch/285104/
State Superseded
Headers show

Comments

Benson Leung - Oct. 21, 2013, 3:26 a.m.
Rather than having the bus names be "i2c-designware-pci--1" because
we have set the .bus_num to -1 to force dynamic allocation, lets have
the busses named "i2c-designware-pci-0" and "i2c-designware-pci-1"
to correspond to the correct names of these busses.

The adapter number will still be dynamically assigned.

Signed-off-by: Benson Leung <bleung@chromium.org>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
Mika Westerberg - Oct. 21, 2013, 6:58 a.m.
On Sun, Oct 20, 2013 at 08:26:50PM -0700, Benson Leung wrote:
> Rather than having the bus names be "i2c-designware-pci--1" because
> we have set the .bus_num to -1 to force dynamic allocation, lets have
> the busses named "i2c-designware-pci-0" and "i2c-designware-pci-1"
> to correspond to the correct names of these busses.
> 
> The adapter number will still be dynamically assigned.

Is there any real value in having names like "i2c-designware-pci-0"
available? I would just drop the whole naming dance instead...
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benson Leung - Oct. 21, 2013, 2:20 p.m.
On Sun, Oct 20, 2013 at 11:58 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> Is there any real value in having names like "i2c-designware-pci-0"
> available? I would just drop the whole naming dance instead...

I'd like some way of distinguishing between the two busses by name. It
seems sensible to name them 0 and 1 since that's how they are referred
to on schematics.

In the chromeos_laptop driver, I do by-name matching of i2c busses to
find busses and instantiate devices, so there is value to have each
named something predictable.
Mika Westerberg - Oct. 21, 2013, 4:12 p.m.
On Mon, Oct 21, 2013 at 07:20:33AM -0700, Benson Leung wrote:
> On Sun, Oct 20, 2013 at 11:58 PM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > Is there any real value in having names like "i2c-designware-pci-0"
> > available? I would just drop the whole naming dance instead...
> 
> I'd like some way of distinguishing between the two busses by name. It
> seems sensible to name them 0 and 1 since that's how they are referred
> to on schematics.
> 
> In the chromeos_laptop driver, I do by-name matching of i2c busses to
> find busses and instantiate devices, so there is value to have each
> named something predictable.

OK
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang - Nov. 14, 2013, 6:05 p.m.
> In the chromeos_laptop driver, I do by-name matching of i2c busses to
> find busses and instantiate devices, so there is value to have each
> named something predictable.

Any why don't you use fixed bus numbers which you can attach the devices
to?
Benson Leung - Nov. 20, 2013, 2:14 a.m.
Hi Wolfram,

On Thu, Nov 14, 2013 at 10:05 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> In the chromeos_laptop driver, I do by-name matching of i2c busses to
>> find busses and instantiate devices, so there is value to have each
>> named something predictable.
>
> Any why don't you use fixed bus numbers which you can attach the devices
> to?

On this particular set of systems, there are two other classes of i2c
adapters that use dynamically assigned bus numbers, specifically the
i915 gmbus adapters, and the i801_smbus adapter. This is why
chromeos_laptop uses the name matching, as some of the boards that it
supports have devices on those dynamic busses.

I can't guarantee that these other busses will always have the same
bus numbers, nor can I guarantee that this driver
(i2c_designware_pcidrv) is loaded before or after the others, so it
seemed the right thing to do to to make sure that the names of each of
the adapters correspond to how they are referred to on various
schematics and datasheets.
Wolfram Sang - Nov. 26, 2013, 1:09 p.m.
On Tue, Nov 19, 2013 at 06:14:18PM -0800, Benson Leung wrote:
> Hi Wolfram,
> 
> On Thu, Nov 14, 2013 at 10:05 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >> In the chromeos_laptop driver, I do by-name matching of i2c busses to
> >> find busses and instantiate devices, so there is value to have each
> >> named something predictable.
> >
> > Any why don't you use fixed bus numbers which you can attach the devices
> > to?
> 
> On this particular set of systems, there are two other classes of i2c
> adapters that use dynamically assigned bus numbers, specifically the
> i915 gmbus adapters, and the i801_smbus adapter. This is why
> chromeos_laptop uses the name matching, as some of the boards that it
> supports have devices on those dynamic busses.

I am not sure I get the problem. If you use i2c_register_board_info() to
register the known devices on the designware busses the dynamically
assigned numbers are guaranteed to be enumarated higer than the static
ones. Check drivers/i2c/i2c-boardinfo.c.

Regards,

   Wolfram
Wolfram Sang - Jan. 3, 2014, 3:52 p.m.
On Tue, Nov 26, 2013 at 02:09:59PM +0100, Wolfram Sang wrote:
> On Tue, Nov 19, 2013 at 06:14:18PM -0800, Benson Leung wrote:
> > Hi Wolfram,
> > 
> > On Thu, Nov 14, 2013 at 10:05 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> > >> In the chromeos_laptop driver, I do by-name matching of i2c busses to
> > >> find busses and instantiate devices, so there is value to have each
> > >> named something predictable.
> > >
> > > Any why don't you use fixed bus numbers which you can attach the devices
> > > to?
> > 
> > On this particular set of systems, there are two other classes of i2c
> > adapters that use dynamically assigned bus numbers, specifically the
> > i915 gmbus adapters, and the i801_smbus adapter. This is why
> > chromeos_laptop uses the name matching, as some of the boards that it
> > supports have devices on those dynamic busses.
> 
> I am not sure I get the problem. If you use i2c_register_board_info() to
> register the known devices on the designware busses the dynamically
> assigned numbers are guaranteed to be enumarated higer than the static
> ones. Check drivers/i2c/i2c-boardinfo.c.

Ping. Was this helpful or do you still have the issue?
Benson Leung - Jan. 10, 2014, 12:12 a.m.
Hi Wolfram,

Thank you for the advice. Sorry for the delay in my response.
(sorry for the duplicated message. I neglected to set plain text in my
email editor).

On Fri, Jan 3, 2014 at 7:52 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> > I am not sure I get the problem. If you use i2c_register_board_info() to
> > register the known devices on the designware busses the dynamically
> > assigned numbers are guaranteed to be enumarated higer than the static
> > ones. Check drivers/i2c/i2c-boardinfo.c.
>
> Ping. Was this helpful or do you still have the issue?

Our devices and our platforms have some other requirements which
turned me away from using i2c_register_board_info.

i2c_register_board_info looks to create predeclarations for a specific
i2c bus... However, right now, the chromeos_laptop driver is
structured to do explicit declaration (using i2c_new_probed_device)
*after* the busses have come up.

Specifically, we have a class of atmel_mxt i2c touchpad/touchscreen
devices that may appear at different addresses depending on whether
the touch device is in bootloader mode or operational mode.

For that reason, the chromeos_laptop driver uses i2c_new_probed_device
with a list of possible addresses when dealing with the atmel touch
device.

You can see the driver here :
drivers/platform/chrome/chromeos_laptop.c

Is there some way of getting the "probe" behavior while using
i2c_register_board_info?
Jean Delvare - Jan. 10, 2014, 7:59 a.m.
On Thu, 9 Jan 2014 16:12:14 -0800, Benson Leung wrote:
> Our devices and our platforms have some other requirements which
> turned me away from using i2c_register_board_info.
> 
> i2c_register_board_info looks to create predeclarations for a specific
> i2c bus... However, right now, the chromeos_laptop driver is
> structured to do explicit declaration (using i2c_new_probed_device)
> *after* the busses have come up.
> 
> Specifically, we have a class of atmel_mxt i2c touchpad/touchscreen
> devices that may appear at different addresses depending on whether
> the touch device is in bootloader mode or operational mode.
> 
> For that reason, the chromeos_laptop driver uses i2c_new_probed_device
> with a list of possible addresses when dealing with the atmel touch
> device.
> 
> You can see the driver here :
> drivers/platform/chrome/chromeos_laptop.c
> 
> Is there some way of getting the "probe" behavior while using
> i2c_register_board_info?

No, i2c_register_board_info works only for devices which are always
present at a known address.
Wolfram Sang - Jan. 16, 2014, 7:51 p.m.
> Our devices and our platforms have some other requirements which
> turned me away from using i2c_register_board_info.

Okay, so I'll drop these patches.
Benson Leung - Jan. 16, 2014, 8:14 p.m.
Hi Wolfram,

On Thu, Jan 16, 2014 at 11:51 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
> > Our devices and our platforms have some other requirements which
> > turned me away from using i2c_register_board_info.
>
> Okay, so I'll drop these patches.


Sorry if I was unclear, but I am not able to use
i2c_register_board_info for these devices because of the requirement
to use "probe."

Currently, I am doing explicit instantiation of devices using
i2c_new_probed_device.

Patch

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index e4cbbdf..d08b2d9 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -229,7 +229,7 @@  static int i2c_dw_pci_probe(struct pci_dev *pdev,
 {
 	struct dw_i2c_dev *dev;
 	struct i2c_adapter *adap;
-	int r;
+	int r, adapter_num;
 	struct  dw_pci_controller *controller;
 
 	if (id->driver_data >= ARRAY_SIZE(dw_pci_controllers)) {
@@ -287,8 +287,18 @@  static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	adap->algo = &i2c_dw_algo;
 	adap->dev.parent = &pdev->dev;
 	adap->nr = controller->bus_num;
-	snprintf(adap->name, sizeof(adap->name), "i2c-designware-pci-%d",
-		adap->nr);
+
+	switch (id->driver_data) {
+	case haswell_0:
+	case haswell_1:
+		adapter_num = id->driver_data - haswell_0;
+		break;
+	default:
+		adapter_num = adap->nr;
+		break;
+	}
+	snprintf(adap->name, sizeof(adap->name), "i2c-designware-pci-%ld",
+		 adapter_num);
 
 	r = devm_request_irq(&pdev->dev, pdev->irq, i2c_dw_isr, IRQF_SHARED,
 			adap->name, dev);