[PATCH 0/2] i2c-designware-pci: Add Haswell ULT device support

[PATCH 0/2] i2c-designware-pci: Add Haswell ULT device support

[Benson Leung]

Add the Haswell ULT device IDs for i2c-designware-pci. These are used,
for example, in Haswell generation Chromebooks, the HP Chromebook 14
and the Acer C720.

[PATCH 1/2] i2c-designware-pci: Add Haswell ULT device IDs
[PATCH 2/2] i2c-designware-pci: Index Haswell ULT bus names from 0

[PATCH 0/2] i2c-designware-pci: Add Haswell ULT device support

[Benson Leung]

Add the Haswell ULT device IDs for i2c-designware-pci. These are used,
for example, in Haswell generation Chromebooks, the HP Chromebook 14
and the Acer C720.

[PATCH 1/2] i2c-designware-pci: Add Haswell ULT device IDs
[PATCH 2/2] i2c-designware-pci: Index Haswell ULT bus names from 0

[PATCH 1/2] i2c-designware-pci: Add Haswell ULT device IDs

[Benson Leung]

From: Duncan Laurie <dlaurie@chromium.org>

Add the necessary PCI Device IDs to use the Haswell ULT
I2C controller in PCI mode.

Set the bus numbers to -1 so it will use dynamic assignment
rather than hardcoded.

Signed-off-by: Duncan Laurie <dlaurie@chromium.org>
Signed-off-by: Benson Leung <bleung@chromium.org>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index f6ed06c..e4cbbdf 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -54,6 +54,9 @@ enum dw_pci_ctl_id_t {
 	medfield_3,
 	medfield_4,
 	medfield_5,
+
+	haswell_0,
+	haswell_1,
 };
 
 struct dw_pci_controller {
@@ -132,6 +135,20 @@ static struct  dw_pci_controller  dw_pci_controllers[] = {
 		.rx_fifo_depth = 32,
 		.clk_khz      = 25000,
 	},
+	[haswell_0] = {
+		.bus_num     = -1,
+		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_STD,
+		.tx_fifo_depth = 32,
+		.rx_fifo_depth = 32,
+		.clk_khz      = 25000,
+	},
+	[haswell_1] = {
+		.bus_num     = -1,
+		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_STD,
+		.tx_fifo_depth = 32,
+		.rx_fifo_depth = 32,
+		.clk_khz      = 25000,
+	},
 };
 static struct i2c_algorithm i2c_dw_algo = {
 	.master_xfer	= i2c_dw_xfer,
@@ -321,6 +338,9 @@ static DEFINE_PCI_DEVICE_TABLE(i2_designware_pci_ids) = {
 	{ PCI_VDEVICE(INTEL, 0x082C), medfield_0 },
 	{ PCI_VDEVICE(INTEL, 0x082D), medfield_1 },
 	{ PCI_VDEVICE(INTEL, 0x082E), medfield_2 },
+	/* Haswell ULT */
+	{ PCI_VDEVICE(INTEL, 0x9c61), haswell_0 },
+	{ PCI_VDEVICE(INTEL, 0x9c62), haswell_1 },
 	{ 0,}
 };
 MODULE_DEVICE_TABLE(pci, i2_designware_pci_ids);
-- 
1.8.3.2

Re: [PATCH 1/2] i2c-designware-pci: Add Haswell ULT device IDs

[Mika Westerberg]

On Sun, Oct 20, 2013 at 08:26:49PM -0700, Benson Leung wrote:
> From: Duncan Laurie <dlaurie@chromium.org>
> 
> Add the necessary PCI Device IDs to use the Haswell ULT
> I2C controller in PCI mode.
> 
> Set the bus numbers to -1 so it will use dynamic assignment
> rather than hardcoded.
> 
> Signed-off-by: Duncan Laurie <dlaurie@chromium.org>
> Signed-off-by: Benson Leung <bleung@chromium.org>

Looks good to me, except one thing...

> ---
>  drivers/i2c/busses/i2c-designware-pcidrv.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
> index f6ed06c..e4cbbdf 100644
> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> @@ -54,6 +54,9 @@ enum dw_pci_ctl_id_t {
>  	medfield_3,
>  	medfield_4,
>  	medfield_5,
> +
> +	haswell_0,
> +	haswell_1,
>  };
>  
>  struct dw_pci_controller {
> @@ -132,6 +135,20 @@ static struct  dw_pci_controller  dw_pci_controllers[] = {
>  		.rx_fifo_depth = 32,
>  		.clk_khz      = 25000,
>  	},
> +	[haswell_0] = {
> +		.bus_num     = -1,
> +		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_STD,
> +		.tx_fifo_depth = 32,
> +		.rx_fifo_depth = 32,
> +		.clk_khz      = 25000,

The input clock for I2C in Haswell is 100MHz, not 25MHz.

> +	},
> +	[haswell_1] = {
> +		.bus_num     = -1,
> +		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_STD,
> +		.tx_fifo_depth = 32,
> +		.rx_fifo_depth = 32,
> +		.clk_khz      = 25000,

Ditto.

> +	},
>  };

Re: [PATCH 1/2] i2c-designware-pci: Add Haswell ULT device IDs

[Mika Westerberg]

On Sun, Oct 20, 2013 at 08:26:49PM -0700, Benson Leung wrote:
> From: Duncan Laurie <dlaurie-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> Add the necessary PCI Device IDs to use the Haswell ULT
> I2C controller in PCI mode.
> 
> Set the bus numbers to -1 so it will use dynamic assignment
> rather than hardcoded.
> 
> Signed-off-by: Duncan Laurie <dlaurie-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Benson Leung <bleung-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Looks good to me, except one thing...

> ---
>  drivers/i2c/busses/i2c-designware-pcidrv.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
> index f6ed06c..e4cbbdf 100644
> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> @@ -54,6 +54,9 @@ enum dw_pci_ctl_id_t {
>  	medfield_3,
>  	medfield_4,
>  	medfield_5,
> +
> +	haswell_0,
> +	haswell_1,
>  };
>  
>  struct dw_pci_controller {
> @@ -132,6 +135,20 @@ static struct  dw_pci_controller  dw_pci_controllers[] = {
>  		.rx_fifo_depth = 32,
>  		.clk_khz      = 25000,
>  	},
> +	[haswell_0] = {
> +		.bus_num     = -1,
> +		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_STD,
> +		.tx_fifo_depth = 32,
> +		.rx_fifo_depth = 32,
> +		.clk_khz      = 25000,

The input clock for I2C in Haswell is 100MHz, not 25MHz.

> +	},
> +	[haswell_1] = {
> +		.bus_num     = -1,
> +		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_STD,
> +		.tx_fifo_depth = 32,
> +		.rx_fifo_depth = 32,
> +		.clk_khz      = 25000,

Ditto.

> +	},
>  };

Re: [PATCH 1/2] i2c-designware-pci: Add Haswell ULT device IDs

[Benson Leung]

On Sun, Oct 20, 2013 at 11:53 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>> +     [haswell_0] = {
>> +             .bus_num     = -1,
>> +             .bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_STD,
>> +             .tx_fifo_depth = 32,
>> +             .rx_fifo_depth = 32,
>> +             .clk_khz      = 25000,
>
> The input clock for I2C in Haswell is 100MHz, not 25MHz.


Thanks for catching that. I will fix it.

-- 
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org

Re: [PATCH 1/2] i2c-designware-pci: Add Haswell ULT device IDs

[Benson Leung]

On Sun, Oct 20, 2013 at 11:53 PM, Mika Westerberg
<mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>> +     [haswell_0] = {
>> +             .bus_num     = -1,
>> +             .bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_STD,
>> +             .tx_fifo_depth = 32,
>> +             .rx_fifo_depth = 32,
>> +             .clk_khz      = 25000,
>
> The input clock for I2C in Haswell is 100MHz, not 25MHz.


Thanks for catching that. I will fix it.

-- 
Benson Leung
Software Engineer, Chrom* OS
bleung-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org

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

[Benson Leung]

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(-)

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);
-- 
1.8.3.2

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

[Benson Leung]

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-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

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);
-- 
1.8.3.2

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

[Mika Westerberg]

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...

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

[Mika Westerberg]

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...

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

[Benson Leung]

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.

-- 
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org

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

[Benson Leung]

On Sun, Oct 20, 2013 at 11:58 PM, Mika Westerberg
<mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 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.

-- 
Benson Leung
Software Engineer, Chrom* OS
bleung-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org

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

[Mika Westerberg]

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

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

[Mika Westerberg]

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-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 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

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 253 bytes --]


> 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?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 253 bytes --]


> 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?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Benson Leung]

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.


-- 
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1059 bytes --]

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


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1085 bytes --]

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-z923LK4zBo2bacvFa/9K2g@public.gmane.org> 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


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1196 bytes --]

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?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1222 bytes --]

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-z923LK4zBo2bacvFa/9K2g@public.gmane.org> 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?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Benson Leung]

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?



-- 
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org

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

[Benson Leung]

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-z923LK4zBo2bacvFa/9K2g@public.gmane.org> 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?



-- 
Benson Leung
Software Engineer, Chrom* OS
bleung-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org

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

[Jean Delvare]

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.

-- 
Jean Delvare

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

[Jean Delvare]

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.

-- 
Jean Delvare

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 157 bytes --]


> 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.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 157 bytes --]


> 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.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Benson Leung]

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.


-- 
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org

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

[Benson Leung]

Hi Wolfram,

On Thu, Jan 16, 2014 at 11:51 AM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> 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.


-- 
Benson Leung
Software Engineer, Chrom* OS
bleung-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org

[PATCH v2 0/2] i2c-designware-pci: Add Haswell ULT device support

[Benson Leung]

Fixed 25Mhz clock instead of 100Mhz clock in 1/2 and a warning in 2/2.

[PATCH v2 1/2] i2c-designware-pci: Add Haswell ULT device IDs
[PATCH v2 2/2] i2c-designware-pci: Index Haswell ULT bus names from 0

[PATCH v2 0/2] i2c-designware-pci: Add Haswell ULT device support

[Benson Leung]

Fixed 25Mhz clock instead of 100Mhz clock in 1/2 and a warning in 2/2.

[PATCH v2 1/2] i2c-designware-pci: Add Haswell ULT device IDs
[PATCH v2 2/2] i2c-designware-pci: Index Haswell ULT bus names from 0

[PATCH v2 1/2] i2c-designware-pci: Add Haswell ULT device IDs

[Benson Leung]

From: Duncan Laurie <dlaurie@chromium.org>

Add the necessary PCI Device IDs to use the Haswell ULT
I2C controller in PCI mode.

Set the bus numbers to -1 so it will use dynamic assignment
rather than hardcoded.

Signed-off-by: Duncan Laurie <dlaurie@chromium.org>
Signed-off-by: Benson Leung <bleung@chromium.org>
---
v2: Changed Haswell bus_clk to 100Mhz, thanks Mika Westerberg.
v1: Initial
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index f6ed06c..816cbd1 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -54,6 +54,9 @@ enum dw_pci_ctl_id_t {
 	medfield_3,
 	medfield_4,
 	medfield_5,
+
+	haswell_0,
+	haswell_1,
 };
 
 struct dw_pci_controller {
@@ -132,6 +135,20 @@ static struct  dw_pci_controller  dw_pci_controllers[] = {
 		.rx_fifo_depth = 32,
 		.clk_khz      = 25000,
 	},
+	[haswell_0] = {
+		.bus_num     = -1,
+		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_STD,
+		.tx_fifo_depth = 32,
+		.rx_fifo_depth = 32,
+		.clk_khz      = 100000,
+	},
+	[haswell_1] = {
+		.bus_num     = -1,
+		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_STD,
+		.tx_fifo_depth = 32,
+		.rx_fifo_depth = 32,
+		.clk_khz      = 100000,
+	},
 };
 static struct i2c_algorithm i2c_dw_algo = {
 	.master_xfer	= i2c_dw_xfer,
@@ -321,6 +338,9 @@ static DEFINE_PCI_DEVICE_TABLE(i2_designware_pci_ids) = {
 	{ PCI_VDEVICE(INTEL, 0x082C), medfield_0 },
 	{ PCI_VDEVICE(INTEL, 0x082D), medfield_1 },
 	{ PCI_VDEVICE(INTEL, 0x082E), medfield_2 },
+	/* Haswell ULT */
+	{ PCI_VDEVICE(INTEL, 0x9c61), haswell_0 },
+	{ PCI_VDEVICE(INTEL, 0x9c62), haswell_1 },
 	{ 0,}
 };
 MODULE_DEVICE_TABLE(pci, i2_designware_pci_ids);
-- 
1.8.3.2

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

[Benson Leung]

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>
---
v2: Fixed warning.
v1: Initial
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 816cbd1..0c6771d 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;
+
+	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-%d",
-		adap->nr);
+		 adapter_num);
 
 	r = devm_request_irq(&pdev->dev, pdev->irq, i2c_dw_isr, IRQF_SHARED,
 			adap->name, dev);
-- 
1.8.3.2

Re: [PATCH v2 0/2] i2c-designware-pci: Add Haswell ULT device support

[Mika Westerberg]

On Mon, Oct 21, 2013 at 08:05:42AM -0700, Benson Leung wrote:
> Fixed 25Mhz clock instead of 100Mhz clock in 1/2 and a warning in 2/2.
> 
> [PATCH v2 1/2] i2c-designware-pci: Add Haswell ULT device IDs
> [PATCH v2 2/2] i2c-designware-pci: Index Haswell ULT bus names from 0

Both patches,

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thanks.

Re: [PATCH v2 0/2] i2c-designware-pci: Add Haswell ULT device support

[Mika Westerberg]

On Mon, Oct 21, 2013 at 08:05:42AM -0700, Benson Leung wrote:
> Fixed 25Mhz clock instead of 100Mhz clock in 1/2 and a warning in 2/2.
> 
> [PATCH v2 1/2] i2c-designware-pci: Add Haswell ULT device IDs
> [PATCH v2 2/2] i2c-designware-pci: Index Haswell ULT bus names from 0

Both patches,

Reviewed-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Thanks.

Re: [PATCH v2 0/2] i2c-designware-pci: Add Haswell ULT device support

[Kirill A. Shutemov]

On Mon, Oct 21, 2013 at 07:14:28PM +0300, Mika Westerberg wrote:
> On Mon, Oct 21, 2013 at 08:05:42AM -0700, Benson Leung wrote:
> > Fixed 25Mhz clock instead of 100Mhz clock in 1/2 and a warning in 2/2.
> > 
> > [PATCH v2 1/2] i2c-designware-pci: Add Haswell ULT device IDs
> > [PATCH v2 2/2] i2c-designware-pci: Index Haswell ULT bus names from 0
> 
> Both patches,
> 
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Guys, any idea why these patches have not upstreamed yet?

-- 
 Kirill A. Shutemov

Re: [PATCH v2 0/2] i2c-designware-pci: Add Haswell ULT device support

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 687 bytes --]

On Sun, Apr 06, 2014 at 04:54:24PM +0300, Kirill A. Shutemov wrote:
> On Mon, Oct 21, 2013 at 07:14:28PM +0300, Mika Westerberg wrote:
> > On Mon, Oct 21, 2013 at 08:05:42AM -0700, Benson Leung wrote:
> > > Fixed 25Mhz clock instead of 100Mhz clock in 1/2 and a warning in 2/2.
> > > 
> > > [PATCH v2 1/2] i2c-designware-pci: Add Haswell ULT device IDs
> > > [PATCH v2 2/2] i2c-designware-pci: Index Haswell ULT bus names from 0
> > 
> > Both patches,
> > 
> > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> Guys, any idea why these patches have not upstreamed yet?

Yes, read the discussion of V1 of these patches. AFAIU the patches are
obsolete.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 0/2] i2c-designware-pci: Add Haswell ULT device support

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 711 bytes --]

On Sun, Apr 06, 2014 at 04:54:24PM +0300, Kirill A. Shutemov wrote:
> On Mon, Oct 21, 2013 at 07:14:28PM +0300, Mika Westerberg wrote:
> > On Mon, Oct 21, 2013 at 08:05:42AM -0700, Benson Leung wrote:
> > > Fixed 25Mhz clock instead of 100Mhz clock in 1/2 and a warning in 2/2.
> > > 
> > > [PATCH v2 1/2] i2c-designware-pci: Add Haswell ULT device IDs
> > > [PATCH v2 2/2] i2c-designware-pci: Index Haswell ULT bus names from 0
> > 
> > Both patches,
> > 
> > Reviewed-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> 
> Guys, any idea why these patches have not upstreamed yet?

Yes, read the discussion of V1 of these patches. AFAIU the patches are
obsolete.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 0/2] i2c-designware-pci: Add Haswell ULT device support

[Mika Westerberg]

On Sun, Apr 06, 2014 at 05:31:04PM +0200, Wolfram Sang wrote:
> On Sun, Apr 06, 2014 at 04:54:24PM +0300, Kirill A. Shutemov wrote:
> > On Mon, Oct 21, 2013 at 07:14:28PM +0300, Mika Westerberg wrote:
> > > On Mon, Oct 21, 2013 at 08:05:42AM -0700, Benson Leung wrote:
> > > > Fixed 25Mhz clock instead of 100Mhz clock in 1/2 and a warning in 2/2.
> > > > 
> > > > [PATCH v2 1/2] i2c-designware-pci: Add Haswell ULT device IDs
> > > > [PATCH v2 2/2] i2c-designware-pci: Index Haswell ULT bus names from 0
> > > 
> > > Both patches,
> > > 
> > > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > 
> > Guys, any idea why these patches have not upstreamed yet?
> 
> Yes, read the discussion of V1 of these patches. AFAIU the patches are
> obsolete.

Well, we still don't have HSW PCI Ids in the driver.

Benson, are you still planning to send a patch to add those? I can add them
as well, if you are fine with that.

Re: [PATCH v2 0/2] i2c-designware-pci: Add Haswell ULT device support

[Mika Westerberg]

On Sun, Apr 06, 2014 at 05:31:04PM +0200, Wolfram Sang wrote:
> On Sun, Apr 06, 2014 at 04:54:24PM +0300, Kirill A. Shutemov wrote:
> > On Mon, Oct 21, 2013 at 07:14:28PM +0300, Mika Westerberg wrote:
> > > On Mon, Oct 21, 2013 at 08:05:42AM -0700, Benson Leung wrote:
> > > > Fixed 25Mhz clock instead of 100Mhz clock in 1/2 and a warning in 2/2.
> > > > 
> > > > [PATCH v2 1/2] i2c-designware-pci: Add Haswell ULT device IDs
> > > > [PATCH v2 2/2] i2c-designware-pci: Index Haswell ULT bus names from 0
> > > 
> > > Both patches,
> > > 
> > > Reviewed-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > 
> > Guys, any idea why these patches have not upstreamed yet?
> 
> Yes, read the discussion of V1 of these patches. AFAIU the patches are
> obsolete.

Well, we still don't have HSW PCI Ids in the driver.

Benson, are you still planning to send a patch to add those? I can add them
as well, if you are fine with that.