[PATCH v1 1/3] i2c: designware-pci: Make bus number allocation robust

[PATCH v1 1/3] i2c: designware-pci: Make bus number allocation robust

[Andy Shevchenko]

On some platforms, such as Intel Medfield, the I2C slave devices are enumerated
through SFI tables where bus numbering is expected to be defined in the OS.
Make the bus number allocation robust for such platforms.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 84 ++++++++++++------------------
 1 file changed, 34 insertions(+), 50 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 7368be0..0f1fc48 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -41,13 +41,7 @@
 #define DRIVER_NAME "i2c-designware-pci"
 
 enum dw_pci_ctl_id_t {
-	medfield_0,
-	medfield_1,
-	medfield_2,
-	medfield_3,
-	medfield_4,
-	medfield_5,
-
+	medfield,
 	baytrail,
 	haswell,
 };
@@ -68,6 +62,7 @@ struct dw_pci_controller {
 	u32 clk_khz;
 	u32 functionality;
 	struct dw_scl_sda_cfg *scl_sda_cfg;
+	int (*setup)(struct pci_dev *pdev, struct dw_pci_controller *c);
 };
 
 #define INTEL_MID_STD_CFG  (DW_IC_CON_MASTER |			\
@@ -98,48 +93,31 @@ static struct dw_scl_sda_cfg hsw_config = {
 	.sda_hold = 0x9,
 };
 
+static int mfld_setup(struct pci_dev *pdev, struct dw_pci_controller *c)
+{
+	switch (pdev->device) {
+	case 0x0817:
+	case 0x0818:
+	case 0x0819:
+		c->bus_num = pdev->device - 0x817 + 3;
+		return 0;
+	case 0x082C:
+	case 0x082D:
+	case 0x082E:
+		c->bus_num = pdev->device - 0x82C + 0;
+		return 0;
+	}
+	return -ENODEV;
+}
+
 static struct dw_pci_controller dw_pci_controllers[] = {
-	[medfield_0] = {
-		.bus_num     = 0,
-		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_FAST,
-		.tx_fifo_depth = 32,
-		.rx_fifo_depth = 32,
-		.clk_khz      = 25000,
-	},
-	[medfield_1] = {
-		.bus_num     = 1,
-		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_FAST,
-		.tx_fifo_depth = 32,
-		.rx_fifo_depth = 32,
-		.clk_khz      = 25000,
-	},
-	[medfield_2] = {
-		.bus_num     = 2,
-		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_FAST,
-		.tx_fifo_depth = 32,
-		.rx_fifo_depth = 32,
-		.clk_khz      = 25000,
-	},
-	[medfield_3] = {
-		.bus_num     = 3,
-		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_STD,
-		.tx_fifo_depth = 32,
-		.rx_fifo_depth = 32,
-		.clk_khz      = 25000,
-	},
-	[medfield_4] = {
-		.bus_num     = 4,
-		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_FAST,
-		.tx_fifo_depth = 32,
-		.rx_fifo_depth = 32,
-		.clk_khz      = 25000,
-	},
-	[medfield_5] = {
-		.bus_num     = 5,
+	[medfield] = {
+		.bus_num = -1,
 		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_FAST,
 		.tx_fifo_depth = 32,
 		.rx_fifo_depth = 32,
 		.clk_khz      = 25000,
+		.setup = mfld_setup,
 	},
 	[baytrail] = {
 		.bus_num = -1,
@@ -242,6 +220,12 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	dev->tx_fifo_depth = controller->tx_fifo_depth;
 	dev->rx_fifo_depth = controller->rx_fifo_depth;
 
+	if (controller->setup) {
+		r = controller->setup(pdev, controller);
+		if (r)
+			return r;
+	}
+
 	adap = &dev->adapter;
 	adap->owner = THIS_MODULE;
 	adap->class = 0;
@@ -276,12 +260,12 @@ MODULE_ALIAS("i2c_designware-pci");
 
 static const struct pci_device_id i2_designware_pci_ids[] = {
 	/* Medfield */
-	{ PCI_VDEVICE(INTEL, 0x0817), medfield_3 },
-	{ PCI_VDEVICE(INTEL, 0x0818), medfield_4 },
-	{ PCI_VDEVICE(INTEL, 0x0819), medfield_5 },
-	{ PCI_VDEVICE(INTEL, 0x082C), medfield_0 },
-	{ PCI_VDEVICE(INTEL, 0x082D), medfield_1 },
-	{ PCI_VDEVICE(INTEL, 0x082E), medfield_2 },
+	{ PCI_VDEVICE(INTEL, 0x0817), medfield },
+	{ PCI_VDEVICE(INTEL, 0x0818), medfield },
+	{ PCI_VDEVICE(INTEL, 0x0819), medfield },
+	{ PCI_VDEVICE(INTEL, 0x082C), medfield },
+	{ PCI_VDEVICE(INTEL, 0x082D), medfield },
+	{ PCI_VDEVICE(INTEL, 0x082E), medfield },
 	/* Baytrail */
 	{ PCI_VDEVICE(INTEL, 0x0F41), baytrail },
 	{ PCI_VDEVICE(INTEL, 0x0F42), baytrail },
-- 
2.8.1

[PATCH v1 2/3] i2c: designware-pci: Introduce Merrifield support

[Andy Shevchenko]

This patch enables I2C controllers found on Intel Edison board.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 33 ++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 0f1fc48..a51f6e0 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -42,6 +42,7 @@
 
 enum dw_pci_ctl_id_t {
 	medfield,
+	merrifield,
 	baytrail,
 	haswell,
 };
@@ -75,6 +76,14 @@ struct dw_pci_controller {
 					I2C_FUNC_SMBUS_WORD_DATA |	\
 					I2C_FUNC_SMBUS_I2C_BLOCK)
 
+/* Merrifield HCNT/LCNT/SDA hold time */
+static struct dw_scl_sda_cfg mrfld_config = {
+	.ss_hcnt = 0x2f8,
+	.fs_hcnt = 0x87,
+	.ss_lcnt = 0x37b,
+	.fs_lcnt = 0x10a,
+};
+
 /* BayTrail HCNT/LCNT/SDA hold time */
 static struct dw_scl_sda_cfg byt_config = {
 	.ss_hcnt = 0x200,
@@ -110,6 +119,19 @@ static int mfld_setup(struct pci_dev *pdev, struct dw_pci_controller *c)
 	return -ENODEV;
 }
 
+static int mrfld_setup(struct pci_dev *pdev, struct dw_pci_controller *c)
+{
+	switch (PCI_SLOT(pdev->devfn)) {
+	case 8:
+		c->bus_num = PCI_FUNC(pdev->devfn) + 0 + 1;
+		return 0;
+	case 9:
+		c->bus_num = PCI_FUNC(pdev->devfn) + 4 + 1;
+		return 0;
+	}
+	return -ENODEV;
+}
+
 static struct dw_pci_controller dw_pci_controllers[] = {
 	[medfield] = {
 		.bus_num = -1,
@@ -119,6 +141,14 @@ static struct dw_pci_controller dw_pci_controllers[] = {
 		.clk_khz      = 25000,
 		.setup = mfld_setup,
 	},
+	[merrifield] = {
+		.bus_num = -1,
+		.bus_cfg = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_FAST,
+		.tx_fifo_depth = 64,
+		.rx_fifo_depth = 64,
+		.scl_sda_cfg = &mrfld_config,
+		.setup = mrfld_setup,
+	},
 	[baytrail] = {
 		.bus_num = -1,
 		.bus_cfg = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_FAST,
@@ -266,6 +296,9 @@ static const struct pci_device_id i2_designware_pci_ids[] = {
 	{ PCI_VDEVICE(INTEL, 0x082C), medfield },
 	{ PCI_VDEVICE(INTEL, 0x082D), medfield },
 	{ PCI_VDEVICE(INTEL, 0x082E), medfield },
+	/* Merrifield */
+	{ PCI_VDEVICE(INTEL, 0x1195), merrifield },
+	{ PCI_VDEVICE(INTEL, 0x1196), merrifield },
 	/* Baytrail */
 	{ PCI_VDEVICE(INTEL, 0x0F41), baytrail },
 	{ PCI_VDEVICE(INTEL, 0x0F42), baytrail },
-- 
2.8.1

Re: [PATCH v1 2/3] i2c: designware-pci: Introduce Merrifield support

[Jarkko Nikula]

On 06/15/2016 01:55 AM, Andy Shevchenko wrote:
> This patch enables I2C controllers found on Intel Edison board.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/i2c/busses/i2c-designware-pcidrv.c | 33 ++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
> index 0f1fc48..a51f6e0 100644
> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> @@ -42,6 +42,7 @@
>
>  enum dw_pci_ctl_id_t {
>  	medfield,
> +	merrifield,
>  	baytrail,
>  	haswell,
>  };
> @@ -75,6 +76,14 @@ struct dw_pci_controller {
>  					I2C_FUNC_SMBUS_WORD_DATA |	\
>  					I2C_FUNC_SMBUS_I2C_BLOCK)
>
> +/* Merrifield HCNT/LCNT/SDA hold time */
> +static struct dw_scl_sda_cfg mrfld_config = {
> +	.ss_hcnt = 0x2f8,
> +	.fs_hcnt = 0x87,
> +	.ss_lcnt = 0x37b,
> +	.fs_lcnt = 0x10a,
> +};
> +
>  /* BayTrail HCNT/LCNT/SDA hold time */
>  static struct dw_scl_sda_cfg byt_config = {
>  	.ss_hcnt = 0x200,
> @@ -110,6 +119,19 @@ static int mfld_setup(struct pci_dev *pdev, struct dw_pci_controller *c)
>  	return -ENODEV;
>  }
>
> +static int mrfld_setup(struct pci_dev *pdev, struct dw_pci_controller *c)
> +{
> +	switch (PCI_SLOT(pdev->devfn)) {
> +	case 8:
> +		c->bus_num = PCI_FUNC(pdev->devfn) + 0 + 1;
> +		return 0;
> +	case 9:
> +		c->bus_num = PCI_FUNC(pdev->devfn) + 4 + 1;
> +		return 0;
> +	}
> +	return -ENODEV;
> +}
> +

What kind of bus numbers we are expected to see here and what offsets 
these magic numbers represent?

-- 
Jarkko

Re: [PATCH v1 2/3] i2c: designware-pci: Introduce Merrifield support

[Andy Shevchenko]

On Wed, 2016-06-15 at 16:48 +0300, Jarkko Nikula wrote:
> On 06/15/2016 01:55 AM, Andy Shevchenko wrote:
> > This patch enables I2C controllers found on Intel Edison board.

> > +static int mrfld_setup(struct pci_dev *pdev, struct
> > dw_pci_controller *c)
> > +{
> > +	switch (PCI_SLOT(pdev->devfn)) {
> > +	case 8:
> > +		c->bus_num = PCI_FUNC(pdev->devfn) + 0 + 1;
> > +		return 0;
> > +	case 9:
> > +		c->bus_num = PCI_FUNC(pdev->devfn) + 4 + 1;
> > +		return 0;
> > +	}
> > +	return -ENODEV;
> > +}
> > +
> 
> What kind of bus numbers we are expected to see here and what offsets 
> these magic numbers represent?

Official bus numbers are [1..7]. Thus, this + 1 at the end. First PCI
device provides 4 functions, that's why +0 and +4.
Do I need to add a comment there?

-- 

Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[PATCH v1 3/3] i2c: designware-pci: Sort header block alphabetically

[Andy Shevchenko]

Simply sort header block alphabetically.

While here fix an indentation in one place and update a copyright line for
Intel.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index a51f6e0..f3c9e0d 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -6,7 +6,7 @@
  * Copyright (C) 2006 Texas Instruments.
  * Copyright (C) 2007 MontaVista Software Inc.
  * Copyright (C) 2009 Provigent Ltd.
- * Copyright (C) 2011, 2015 Intel Corporation.
+ * Copyright (C) 2011, 2015, 2016 Intel Corporation.
  *
  * ----------------------------------------------------------------------------
  *
@@ -23,19 +23,20 @@
  *
  */
 
-#include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/acpi.h>
 #include <linux/delay.h>
-#include <linux/i2c.h>
-#include <linux/errno.h>
-#include <linux/sched.h>
 #include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
-#include <linux/slab.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/pm_runtime.h>
-#include <linux/acpi.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+
 #include "i2c-designware-core.h"
 
 #define DRIVER_NAME "i2c-designware-pci"
@@ -198,7 +199,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	struct dw_i2c_dev *dev;
 	struct i2c_adapter *adap;
 	int r;
-	struct  dw_pci_controller *controller;
+	struct dw_pci_controller *controller;
 	struct dw_scl_sda_cfg *cfg;
 
 	if (id->driver_data >= ARRAY_SIZE(dw_pci_controllers)) {
-- 
2.8.1

Re: [PATCH v1 3/3] i2c: designware-pci: Sort header block alphabetically

[Jarkko Nikula]

On 06/15/2016 01:56 AM, Andy Shevchenko wrote:
> Simply sort header block alphabetically.
>
> While here fix an indentation in one place and update a copyright line for
> Intel.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/i2c/busses/i2c-designware-pcidrv.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Re: [PATCH v1 1/3] i2c: designware-pci: Make bus number allocation robust

[Jarkko Nikula]

On 06/15/2016 01:55 AM, Andy Shevchenko wrote:
> On some platforms, such as Intel Medfield, the I2C slave devices are enumerated
> through SFI tables where bus numbering is expected to be defined in the OS.
> Make the bus number allocation robust for such platforms.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/i2c/busses/i2c-designware-pcidrv.c | 84 ++++++++++++------------------
>  1 file changed, 34 insertions(+), 50 deletions(-)
...
> -	[medfield_3] = {
> -		.bus_num     = 3,
> -		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_STD,
> -		.tx_fifo_depth = 32,
> -		.rx_fifo_depth = 32,
> -		.clk_khz      = 25000,
> -	},
> -	[medfield_4] = {
> -		.bus_num     = 4,
> -		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_FAST,
> -		.tx_fifo_depth = 32,
> -		.rx_fifo_depth = 32,
> -		.clk_khz      = 25000,
> -	},
> -	[medfield_5] = {
> -		.bus_num     = 5,
> +	[medfield] = {
> +		.bus_num = -1,
>  		.bus_cfg   = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_FAST,
>  		.tx_fifo_depth = 32,
>  		.rx_fifo_depth = 32,
>  		.clk_khz      = 25000,
> +		.setup = mfld_setup,

Now bus 3 speed is pumped up to fast speed. Can it cause problems?

-- 
Jarkko

Re: [PATCH v1 1/3] i2c: designware-pci: Make bus number allocation robust

[Andy Shevchenko]

On Wed, 2016-06-15 at 16:42 +0300, Jarkko Nikula wrote:
> On 06/15/2016 01:55 AM, Andy Shevchenko wrote:
> > On some platforms, such as Intel Medfield, the I2C slave devices are
> > enumerated
> > through SFI tables where bus numbering is expected to be defined in
> > the OS.
> > Make the bus number allocation robust for such platforms.

> > ...
> > -	[medfield_3] = {
> > -		.bus_num     = 3,
> > -		.bus_cfg   = INTEL_MID_STD_CFG |
> > DW_IC_CON_SPEED_STD,

> > +	[medfield] = {
> > +		.bus_num = -1,
> >  		.bus_cfg   = INTEL_MID_STD_CFG |
> > DW_IC_CON_SPEED_FAST,

> Now bus 3 speed is pumped up to fast speed. Can it cause problems?

Oh, I have no idea, so, I would leave it at lower speed.

While we are here, what are your recommendations to put ->setup() call
in the ->probe()? Currently it is located somewhere in the middle of
assignments. I think the workflow has to be the following:
1. Take static controller data from platform based on ID's
2. Call controller->setup() if defined
3. Re-assign i2c device parameters with data in controller

-- 

Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy