[PATCH v2 0/6] Add support for MSCC Ocelot i2c

[PATCH v2 0/6] Add support for MSCC Ocelot i2c

[Alexandre Belloni]

Hi,

Because the designware IP was not able to handle the SDA hold time before
version 1.11a, MSCC has its own implementation. Add support for it and then add
i2c on ocelot boards.

I would expect patches 1 to 4 to go through the i2c tree and 5-6 through
the mips tree once patch 4 has been reviewed by the DT maintainers.

Changes in v2:
 - removed first patch as a similar one is in i2c-next
 - rebase on top of i2c-next
 - Added two patches to implement ideas from Andy

Alexandre Belloni (6):
  i2c: designware: use generic table matching
  i2c: designware: move #ifdef CONFIG_OF to the top
  i2c: designware: allow IP specific sda_hold_time
  i2c: designware: add MSCC Ocelot support
  mips: dts: mscc: Add i2c on ocelot
  mips: dts: mscc: enable i2c on ocelot_pcb123

 .../bindings/i2c/i2c-designware.txt           |  9 ++-
 arch/mips/boot/dts/mscc/ocelot.dtsi           | 18 ++++++
 arch/mips/boot/dts/mscc/ocelot_pcb123.dts     |  6 ++
 drivers/i2c/busses/i2c-designware-common.c    |  2 +
 drivers/i2c/busses/i2c-designware-core.h      |  4 ++
 drivers/i2c/busses/i2c-designware-platdrv.c   | 62 +++++++++++++++----
 6 files changed, 87 insertions(+), 14 deletions(-)

-- 
2.18.0

[PATCH v2 1/6] i2c: designware: use generic table matching

[Alexandre Belloni]

Switch to device_get_match_data in probe to match the device specific data
instead of using the acpi specific function.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index ddf13527aaee..00bf62f77c47 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -119,10 +119,6 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev)
 		break;
 	}
 
-	id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev);
-	if (id && id->driver_data)
-		dev->flags |= (u32)id->driver_data;
-
 	if (acpi_bus_get_device(handle, &adev))
 		return -ENODEV;
 
@@ -269,6 +265,8 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	else
 		i2c_parse_fw_timings(&pdev->dev, t, false);
 
+	dev->flags |= (u32)device_get_match_data(&pdev->dev);
+
 	acpi_speed = i2c_acpi_find_bus_speed(&pdev->dev);
 	/*
 	 * Some DSTDs use a non standard speed, round down to the lowest
-- 
2.18.0

Re: [PATCH v2 1/6] i2c: designware: use generic table matching

[Andy Shevchenko]

On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote:
> Switch to device_get_match_data in probe to match the device specific
> data
> instead of using the acpi specific function.
> 

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
>  drivers/i2c/busses/i2c-designware-platdrv.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> b/drivers/i2c/busses/i2c-designware-platdrv.c
> index ddf13527aaee..00bf62f77c47 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -119,10 +119,6 @@ static int dw_i2c_acpi_configure(struct
> platform_device *pdev)
>  		break;
>  	}
>  
> -	id = acpi_match_device(pdev->dev.driver->acpi_match_table,
> &pdev->dev);
> -	if (id && id->driver_data)
> -		dev->flags |= (u32)id->driver_data;
> -
>  	if (acpi_bus_get_device(handle, &adev))
>  		return -ENODEV;
>  
> @@ -269,6 +265,8 @@ static int dw_i2c_plat_probe(struct
> platform_device *pdev)
>  	else
>  		i2c_parse_fw_timings(&pdev->dev, t, false);
>  
> +	dev->flags |= (u32)device_get_match_data(&pdev->dev);
> +
>  	acpi_speed = i2c_acpi_find_bus_speed(&pdev->dev);
>  	/*
>  	 * Some DSTDs use a non standard speed, round down to the
> lowest

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

Re: [PATCH v2 1/6] i2c: designware: use generic table matching

[Andy Shevchenko]

On Tue, 2018-07-31 at 17:02 +0300, Andy Shevchenko wrote:
> On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote:
> > Switch to device_get_match_data in probe to match the device
> > specific
> > data
> > instead of using the acpi specific function.
> > 
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Oops. See below.

> > -	id = acpi_match_device(pdev->dev.driver->acpi_match_table,
> > &pdev->dev);
> > -	if (id && id->driver_data)
> > -		dev->flags |= (u32)id->driver_data;

It seems you forgot to remove unused variable.

> > -
> >  	if (acpi_bus_get_device(handle, &adev))
> >  		return -ENODEV;
> >  
> > @@ -269,6 +265,8 @@ static int dw_i2c_plat_probe(struct
> > platform_device *pdev)
> >  	else
> >  		i2c_parse_fw_timings(&pdev->dev, t, false);
> >  
> > +	dev->flags |= (u32)device_get_match_data(&pdev->dev);
> > +

And since it would be changed anyway, can you actually move this line
closet to  if (has_acpi_companion()) one ?

> >  	acpi_speed = i2c_acpi_find_bus_speed(&pdev->dev);
> >  	/*
> >  	 * Some DSTDs use a non standard speed, round down to the
> > lowest
> 
> 

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

Re: [PATCH v2 1/6] i2c: designware: use generic table matching

[Alexandre Belloni]

On 31/07/2018 17:23:04+0300, Andy Shevchenko wrote:
> On Tue, 2018-07-31 at 17:02 +0300, Andy Shevchenko wrote:
> > On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote:
> > > Switch to device_get_match_data in probe to match the device
> > > specific
> > > data
> > > instead of using the acpi specific function.
> > > 
> > 
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Oops. See below.
> 
> > > -	id = acpi_match_device(pdev->dev.driver->acpi_match_table,
> > > &pdev->dev);
> > > -	if (id && id->driver_data)
> > > -		dev->flags |= (u32)id->driver_data;
> 
> It seems you forgot to remove unused variable.
> 
> > > -
> > >  	if (acpi_bus_get_device(handle, &adev))
> > >  		return -ENODEV;
> > >  
> > > @@ -269,6 +265,8 @@ static int dw_i2c_plat_probe(struct
> > > platform_device *pdev)
> > >  	else
> > >  		i2c_parse_fw_timings(&pdev->dev, t, false);
> > >  
> > > +	dev->flags |= (u32)device_get_match_data(&pdev->dev);
> > > +
> 
> And since it would be changed anyway, can you actually move this line
> closet to  if (has_acpi_companion()) one ?
> 

I need that value to be set before calling of_configure though.

> > >  	acpi_speed = i2c_acpi_find_bus_speed(&pdev->dev);
> > >  	/*
> > >  	 * Some DSTDs use a non standard speed, round down to the
> > > lowest
> > 
> > 
> 
> -- 
> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Intel Finland Oy

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 1/6] i2c: designware: use generic table matching

[Andy Shevchenko]

On Tue, 2018-07-31 at 16:30 +0200, Alexandre Belloni wrote:
> On 31/07/2018 17:23:04+0300, Andy Shevchenko wrote:
> > On Tue, 2018-07-31 at 17:02 +0300, Andy Shevchenko wrote:
> > > On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote:
 
> > > > +	dev->flags |= (u32)device_get_match_data(&pdev->dev);
> > > > +
> > 
> > And since it would be changed anyway, can you actually move this
> > line
> > closet to  if (has_acpi_companion()) one ?
> > 
> 
> I need that value to be set before calling of_configure though.

AFAICS, you added those lines later, so, it would be something like

dev->flags |= ...

if (of_node)
 of_configure()
if (has_acpi_companion())
 acpi_configure()

Would it work for you?

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

Re: [PATCH v2 1/6] i2c: designware: use generic table matching

[Alexandre Belloni]

On 31/07/2018 17:53:17+0300, Andy Shevchenko wrote:
> On Tue, 2018-07-31 at 16:30 +0200, Alexandre Belloni wrote:
> > On 31/07/2018 17:23:04+0300, Andy Shevchenko wrote:
> > > On Tue, 2018-07-31 at 17:02 +0300, Andy Shevchenko wrote:
> > > > On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote:
>  
> > > > > +	dev->flags |= (u32)device_get_match_data(&pdev->dev);
> > > > > +
> > > 
> > > And since it would be changed anyway, can you actually move this
> > > line
> > > closet to  if (has_acpi_companion()) one ?
> > > 
> > 
> > I need that value to be set before calling of_configure though.
> 
> AFAICS, you added those lines later, so, it would be something like
> 
> dev->flags |= ...
> 
> if (of_node)
>  of_configure()
> if (has_acpi_companion())
>  acpi_configure()
> 
> Would it work for you?
> 

Works for me, I'll send v3 soon.

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 1/6] i2c: designware: use generic table matching

[Alexandre Belloni]

On 31/07/2018 17:23:04+0300, Andy Shevchenko wrote:
> On Tue, 2018-07-31 at 17:02 +0300, Andy Shevchenko wrote:
> > On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote:
> > > Switch to device_get_match_data in probe to match the device
> > > specific
> > > data
> > > instead of using the acpi specific function.
> > > 
> > 
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Oops. See below.
> 
> > > -	id = acpi_match_device(pdev->dev.driver->acpi_match_table,
> > > &pdev->dev);
> > > -	if (id && id->driver_data)
> > > -		dev->flags |= (u32)id->driver_data;
> 

I'll change that cast to (uintptr_t). Else gcc complains about the size
mismatch on 64 bit platform. I'm letting kbuild play with my branch a
bit before sending v3.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v2 2/6] i2c: designware: move #ifdef CONFIG_OF to the top

[Alexandre Belloni]

Move the #ifdef CONFIG_OF section to the top of the file, after the ACPI
section so functions defined there can be used in dw_i2c_plat_probe.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 00bf62f77c47..ba142d7c0e05 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -157,6 +157,14 @@ static inline int dw_i2c_acpi_configure(struct platform_device *pdev)
 }
 #endif
 
+#ifdef CONFIG_OF
+static const struct of_device_id dw_i2c_of_match[] = {
+	{ .compatible = "snps,designware-i2c", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
+#endif
+
 static void i2c_dw_configure_master(struct dw_i2c_dev *dev)
 {
 	struct i2c_timings *t = &dev->timings;
@@ -391,14 +399,6 @@ static int dw_i2c_plat_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_OF
-static const struct of_device_id dw_i2c_of_match[] = {
-	{ .compatible = "snps,designware-i2c", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
-#endif
-
 #ifdef CONFIG_PM_SLEEP
 static int dw_i2c_plat_prepare(struct device *dev)
 {
-- 
2.18.0

Re: [PATCH v2 2/6] i2c: designware: move #ifdef CONFIG_OF to the top

[Andy Shevchenko]

On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote:
> Move the #ifdef CONFIG_OF section to the top of the file, after the
> ACPI
> section so functions defined there can be used in dw_i2c_plat_probe.
> 

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
>  drivers/i2c/busses/i2c-designware-platdrv.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 00bf62f77c47..ba142d7c0e05 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -157,6 +157,14 @@ static inline int dw_i2c_acpi_configure(struct
> platform_device *pdev)
>  }
>  #endif
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id dw_i2c_of_match[] = {
> +	{ .compatible = "snps,designware-i2c", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
> +#endif
> +
>  static void i2c_dw_configure_master(struct dw_i2c_dev *dev)
>  {
>  	struct i2c_timings *t = &dev->timings;
> @@ -391,14 +399,6 @@ static int dw_i2c_plat_remove(struct
> platform_device *pdev)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_OF
> -static const struct of_device_id dw_i2c_of_match[] = {
> -	{ .compatible = "snps,designware-i2c", },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
> -#endif
> -
>  #ifdef CONFIG_PM_SLEEP
>  static int dw_i2c_plat_prepare(struct device *dev)
>  {

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

[PATCH v2 3/6] i2c: designware: allow IP specific sda_hold_time

[Alexandre Belloni]

Because some old designware IPs were not supporting setting an SDA hold
time, vendors developed their own solution. Add a way for the final driver
to provide its own SDA hold time handling.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---

Changes in v2:
 - rebased on i2c-next
 - moved the if (dev->set_sda_hold_time) after the version test

 drivers/i2c/busses/i2c-designware-common.c | 2 ++
 drivers/i2c/busses/i2c-designware-core.h   | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index e67d6c7d552c..463906a02c7f 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -201,6 +201,8 @@ int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev)
 		dev_dbg(dev->dev, "SDA Hold Time TX:RX = %d:%d\n",
 			dev->sda_hold_time & ~(u32)DW_IC_SDA_HOLD_RX_MASK,
 			dev->sda_hold_time >> DW_IC_SDA_HOLD_RX_SHIFT);
+	} else if (dev->set_sda_hold_time) {
+		dev->set_sda_hold_time(dev);
 	} else if (dev->sda_hold_time) {
 		dev_warn(dev->dev,
 			"Hardware too old to adjust SDA hold time.\n");
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 70c188d6fb6b..870444bbbcc4 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -269,6 +269,7 @@ struct dw_i2c_dev {
 	void			(*disable)(struct dw_i2c_dev *dev);
 	void			(*disable_int)(struct dw_i2c_dev *dev);
 	int			(*init)(struct dw_i2c_dev *dev);
+	int			(*set_sda_hold_time)(struct dw_i2c_dev *dev);
 	int			mode;
 	struct i2c_bus_recovery_info rinfo;
 };
-- 
2.18.0

[PATCH v2 4/6] i2c: designware: add MSCC Ocelot support

[Alexandre Belloni]

The Microsemi Ocelot I2C controller is a designware IP. It also has a
second set of registers to allow tweaking SDA hold time and spike
filtering.

Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
Changes in v2:
 - improved binding doc
 - changed the model handling as suggested by Andy

 .../bindings/i2c/i2c-designware.txt           |  9 ++++-
 drivers/i2c/busses/i2c-designware-core.h      |  3 ++
 drivers/i2c/busses/i2c-designware-platdrv.c   | 40 +++++++++++++++++++
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
index fbb0a6d8b964..0fb17387c735 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
@@ -2,7 +2,8 @@
 
 Required properties :
 
- - compatible : should be "snps,designware-i2c"
+ - compatible : should be "snps,designware-i2c" or "mscc,ocelot-i2c" followed by
+   "snps,designware-i2c" for fallback
  - reg : Offset and length of the register set for the device
  - interrupts : <IRQ> where IRQ is the interrupt number.
 
@@ -11,8 +12,12 @@ Recommended properties :
  - clock-frequency : desired I2C bus clock frequency in Hz.
 
 Optional properties :
+ - reg : for "mscc,ocelot-i2c", a second register set to configure the SDA hold
+   time, named ICPU_CFG:TWI_DELAY in the datasheet.
+
  - i2c-sda-hold-time-ns : should contain the SDA hold time in nanoseconds.
-   This option is only supported in hardware blocks version 1.11a or newer.
+   This option is only supported in hardware blocks version 1.11a or newer and
+   on Microsemi SoCs ("mscc,ocelot-i2c" compatible).wtf
 
  - i2c-scl-falling-time-ns : should contain the SCL falling time in nanoseconds.
    This value which is by default 300ns is used to compute the tLOW period.
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 870444bbbcc4..5e240ce9968e 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -225,6 +225,7 @@
 struct dw_i2c_dev {
 	struct device		*dev;
 	void __iomem		*base;
+	void __iomem		*ext;
 	struct completion	cmd_complete;
 	struct clk		*clk;
 	struct reset_control	*rst;
@@ -279,6 +280,8 @@ struct dw_i2c_dev {
 #define ACCESS_INTR_MASK	0x00000004
 
 #define MODEL_CHERRYTRAIL	0x00000100
+#define MODEL_MSCC_OCELOT	0x00000200
+#define MODEL_MASK		0x00000f00
 
 u32 dw_readl(struct dw_i2c_dev *dev, int offset);
 void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index ba142d7c0e05..68619b270b4c 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -158,11 +158,48 @@ static inline int dw_i2c_acpi_configure(struct platform_device *pdev)
 #endif
 
 #ifdef CONFIG_OF
+#define MSCC_ICPU_CFG_TWI_DELAY		0x0
+#define MSCC_ICPU_CFG_TWI_DELAY_ENABLE	BIT(0)
+#define MSCC_ICPU_CFG_TWI_SPIKE_FILTER	0x4
+
+static int mscc_twi_set_sda_hold_time(struct dw_i2c_dev *dev)
+{
+	writel((dev->sda_hold_time << 1) | MSCC_ICPU_CFG_TWI_DELAY_ENABLE,
+	       dev->ext + MSCC_ICPU_CFG_TWI_DELAY);
+
+	return 0;
+}
+
+int dw_i2c_of_configure(struct platform_device *pdev)
+{
+	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
+	struct resource *mem;
+
+	switch (dev->flags & MODEL_MASK) {
+	case MODEL_MSCC_OCELOT:
+		mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+		dev->ext = devm_ioremap_resource(&pdev->dev, mem);
+		if (!IS_ERR(dev->ext))
+			dev->set_sda_hold_time = mscc_twi_set_sda_hold_time;
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
 static const struct of_device_id dw_i2c_of_match[] = {
 	{ .compatible = "snps,designware-i2c", },
+	{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
 	{},
 };
 MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
+#else
+static inline int dw_i2c_of_configure(struct platform_device *pdev)
+{
+	return -ENODEV;
+}
 #endif
 
 static void i2c_dw_configure_master(struct dw_i2c_dev *dev)
@@ -275,6 +312,9 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 
 	dev->flags |= (u32)device_get_match_data(&pdev->dev);
 
+	if (pdev->dev.of_node)
+		dw_i2c_of_configure(pdev);
+
 	acpi_speed = i2c_acpi_find_bus_speed(&pdev->dev);
 	/*
 	 * Some DSTDs use a non standard speed, round down to the lowest
-- 
2.18.0

Re: [PATCH v2 4/6] i2c: designware: add MSCC Ocelot support

[Andy Shevchenko]

On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote:
> The Microsemi Ocelot I2C controller is a designware IP. It also has a
> second set of registers to allow tweaking SDA hold time and spike
> filtering.
> 

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

(except the bindings, not my area)

> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> Changes in v2:
>  - improved binding doc
>  - changed the model handling as suggested by Andy
> 
>  .../bindings/i2c/i2c-designware.txt           |  9 ++++-
>  drivers/i2c/busses/i2c-designware-core.h      |  3 ++
>  drivers/i2c/busses/i2c-designware-platdrv.c   | 40
> +++++++++++++++++++
>  3 files changed, 50 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> index fbb0a6d8b964..0fb17387c735 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> @@ -2,7 +2,8 @@
>  
>  Required properties :
>  
> - - compatible : should be "snps,designware-i2c"
> + - compatible : should be "snps,designware-i2c" or "mscc,ocelot-i2c"
> followed by
> +   "snps,designware-i2c" for fallback
>   - reg : Offset and length of the register set for the device
>   - interrupts : <IRQ> where IRQ is the interrupt number.
>  
> @@ -11,8 +12,12 @@ Recommended properties :
>   - clock-frequency : desired I2C bus clock frequency in Hz.
>  
>  Optional properties :
> + - reg : for "mscc,ocelot-i2c", a second register set to configure
> the SDA hold
> +   time, named ICPU_CFG:TWI_DELAY in the datasheet.
> +
>   - i2c-sda-hold-time-ns : should contain the SDA hold time in
> nanoseconds.
> -   This option is only supported in hardware blocks version 1.11a or
> newer.
> +   This option is only supported in hardware blocks version 1.11a or
> newer and

> +   on Microsemi SoCs ("mscc,ocelot-i2c" compatible).wtf

wtf?

>  
>   - i2c-scl-falling-time-ns : should contain the SCL falling time in
> nanoseconds.
>     This value which is by default 300ns is used to compute the tLOW
> period.
> diff --git a/drivers/i2c/busses/i2c-designware-core.h
> b/drivers/i2c/busses/i2c-designware-core.h
> index 870444bbbcc4..5e240ce9968e 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -225,6 +225,7 @@
>  struct dw_i2c_dev {
>  	struct device		*dev;
>  	void __iomem		*base;
> +	void __iomem		*ext;
>  	struct completion	cmd_complete;
>  	struct clk		*clk;
>  	struct reset_control	*rst;
> @@ -279,6 +280,8 @@ struct dw_i2c_dev {
>  #define ACCESS_INTR_MASK	0x00000004
>  
>  #define MODEL_CHERRYTRAIL	0x00000100
> +#define MODEL_MSCC_OCELOT	0x00000200
> +#define MODEL_MASK		0x00000f00
>  
>  u32 dw_readl(struct dw_i2c_dev *dev, int offset);
>  void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset);
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> b/drivers/i2c/busses/i2c-designware-platdrv.c
> index ba142d7c0e05..68619b270b4c 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -158,11 +158,48 @@ static inline int dw_i2c_acpi_configure(struct
> platform_device *pdev)
>  #endif
>  
>  #ifdef CONFIG_OF
> +#define MSCC_ICPU_CFG_TWI_DELAY		0x0
> +#define MSCC_ICPU_CFG_TWI_DELAY_ENABLE	BIT(0)
> +#define MSCC_ICPU_CFG_TWI_SPIKE_FILTER	0x4
> +
> +static int mscc_twi_set_sda_hold_time(struct dw_i2c_dev *dev)
> +{
> +	writel((dev->sda_hold_time << 1) |
> MSCC_ICPU_CFG_TWI_DELAY_ENABLE,
> +	       dev->ext + MSCC_ICPU_CFG_TWI_DELAY);
> +
> +	return 0;
> +}
> +
> +int dw_i2c_of_configure(struct platform_device *pdev)
> +{
> +	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
> +	struct resource *mem;
> +
> +	switch (dev->flags & MODEL_MASK) {
> +	case MODEL_MSCC_OCELOT:
> +		mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +		dev->ext = devm_ioremap_resource(&pdev->dev, mem);
> +		if (!IS_ERR(dev->ext))
> +			dev->set_sda_hold_time =
> mscc_twi_set_sda_hold_time;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct of_device_id dw_i2c_of_match[] = {
>  	{ .compatible = "snps,designware-i2c", },
> +	{ .compatible = "mscc,ocelot-i2c", .data = (void
> *)MODEL_MSCC_OCELOT },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
> +#else
> +static inline int dw_i2c_of_configure(struct platform_device *pdev)
> +{
> +	return -ENODEV;
> +}
>  #endif
>  
>  static void i2c_dw_configure_master(struct dw_i2c_dev *dev)
> @@ -275,6 +312,9 @@ static int dw_i2c_plat_probe(struct
> platform_device *pdev)
>  
>  	dev->flags |= (u32)device_get_match_data(&pdev->dev);
>  
> +	if (pdev->dev.of_node)
> +		dw_i2c_of_configure(pdev);
> +
>  	acpi_speed = i2c_acpi_find_bus_speed(&pdev->dev);
>  	/*
>  	 * Some DSTDs use a non standard speed, round down to the
> lowest

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

[PATCH v2 5/6] mips: dts: mscc: Add i2c on ocelot

[Alexandre Belloni]

Ocelot has an i2c controller, add it. There is only one possible pinmux
configuration so add it as well.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 arch/mips/boot/dts/mscc/ocelot.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/mips/boot/dts/mscc/ocelot.dtsi b/arch/mips/boot/dts/mscc/ocelot.dtsi
index 4f33dbc67348..34093acebc0d 100644
--- a/arch/mips/boot/dts/mscc/ocelot.dtsi
+++ b/arch/mips/boot/dts/mscc/ocelot.dtsi
@@ -78,6 +78,19 @@
 			status = "disabled";
 		};
 
+		i2c: i2c@100400 {
+			compatible = "mscc,ocelot-i2c", "snps,designware-i2c";
+			pinctrl-0 = <&i2c_pins>;
+			pinctrl-names = "default";
+			reg = <0x100400 0x100>, <0x198 0x8>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <8>;
+			clocks = <&ahb_clk>;
+
+			status = "disabled";
+		};
+
 		uart2: serial@100800 {
 			pinctrl-0 = <&uart2_pins>;
 			pinctrl-names = "default";
@@ -178,6 +191,11 @@
 				pins = "GPIO_12", "GPIO_13";
 				function = "uart2";
 			};
+
+			i2c_pins: i2c-pins {
+				pins = "GPIO_16", "GPIO_17";
+				function = "twi";
+			};
 		};
 
 		mdio0: mdio@107009c {
-- 
2.18.0

[PATCH v2 6/6] mips: dts: mscc: enable i2c on ocelot_pcb123

[Alexandre Belloni]

Enable the i2c controller on ocelot PCB123. While there are no i2c devices
on the board itself, it can be used to control the SFP transceivers.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 arch/mips/boot/dts/mscc/ocelot_pcb123.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb123.dts b/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
index 4ccd65379059..336c859a9bbe 100644
--- a/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
+++ b/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
@@ -26,6 +26,12 @@
 	status = "okay";
 };
 
+&i2c {
+	clock-frequency = <100000>;
+	i2c-sda-hold-time-ns = <300>;
+	status = "okay";
+};
+
 &mdio0 {
 	status = "okay";
 };
-- 
2.18.0

Re: [PATCH v2 0/6] Add support for MSCC Ocelot i2c

[Andy Shevchenko]

On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote:
> Hi,
> 
> Because the designware IP was not able to handle the SDA hold time
> before
> version 1.11a, MSCC has its own implementation. Add support for it and
> then add
> i2c on ocelot boards.
> 
> I would expect patches 1 to 4 to go through the i2c tree and 5-6
> through
> the mips tree once patch 4 has been reviewed by the DT maintainers.
> 

Thanks for an update, looks quite nice!
I gave my tags wherever it is appropriate.

> Changes in v2:
>  - removed first patch as a similar one is in i2c-next

>  - rebase on top of i2c-next

It seems you also took my patches which are not yet in i2c/for-next.

>  - Added two patches to implement ideas from Andy
> 
> Alexandre Belloni (6):
>   i2c: designware: use generic table matching
>   i2c: designware: move #ifdef CONFIG_OF to the top
>   i2c: designware: allow IP specific sda_hold_time
>   i2c: designware: add MSCC Ocelot support
>   mips: dts: mscc: Add i2c on ocelot
>   mips: dts: mscc: enable i2c on ocelot_pcb123
> 
>  .../bindings/i2c/i2c-designware.txt           |  9 ++-
>  arch/mips/boot/dts/mscc/ocelot.dtsi           | 18 ++++++
>  arch/mips/boot/dts/mscc/ocelot_pcb123.dts     |  6 ++
>  drivers/i2c/busses/i2c-designware-common.c    |  2 +
>  drivers/i2c/busses/i2c-designware-core.h      |  4 ++
>  drivers/i2c/busses/i2c-designware-platdrv.c   | 62 +++++++++++++++---
> -
>  6 files changed, 87 insertions(+), 14 deletions(-)
> 

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

Re: [PATCH v2 0/6] Add support for MSCC Ocelot i2c

[Alexandre Belloni]

On 31/07/2018 17:17:47+0300, Andy Shevchenko wrote:
> On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote:
> > Hi,
> > 
> > Because the designware IP was not able to handle the SDA hold time
> > before
> > version 1.11a, MSCC has its own implementation. Add support for it and
> > then add
> > i2c on ocelot boards.
> > 
> > I would expect patches 1 to 4 to go through the i2c tree and 5-6
> > through
> > the mips tree once patch 4 has been reviewed by the DT maintainers.
> > 
> 
> Thanks for an update, looks quite nice!
> I gave my tags wherever it is appropriate.
> 
> > Changes in v2:
> >  - removed first patch as a similar one is in i2c-next
> 
> >  - rebase on top of i2c-next
> 
> It seems you also took my patches which are not yet in i2c/for-next.
> 

That is right, I also based on https://patchwork.ozlabs.org/project/linux-i2c/list/?series=57551
but it is not a hard dependency.

> >  - Added two patches to implement ideas from Andy
> > 
> > Alexandre Belloni (6):
> >   i2c: designware: use generic table matching
> >   i2c: designware: move #ifdef CONFIG_OF to the top
> >   i2c: designware: allow IP specific sda_hold_time
> >   i2c: designware: add MSCC Ocelot support
> >   mips: dts: mscc: Add i2c on ocelot
> >   mips: dts: mscc: enable i2c on ocelot_pcb123
> > 
> >  .../bindings/i2c/i2c-designware.txt           |  9 ++-
> >  arch/mips/boot/dts/mscc/ocelot.dtsi           | 18 ++++++
> >  arch/mips/boot/dts/mscc/ocelot_pcb123.dts     |  6 ++
> >  drivers/i2c/busses/i2c-designware-common.c    |  2 +
> >  drivers/i2c/busses/i2c-designware-core.h      |  4 ++
> >  drivers/i2c/busses/i2c-designware-platdrv.c   | 62 +++++++++++++++---
> > -
> >  6 files changed, 87 insertions(+), 14 deletions(-)
> > 
> 
> -- 
> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Intel Finland Oy

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com