[PATCH v2 0/2] i2c: added driver for Mellanox BlueField SoC

[PATCH v2 0/2] i2c: added driver for Mellanox BlueField SoC

[Khalil Blaiech]

Added I2C SMBus driver and device tree bindings documentation.
This patch set depends on ARCH_MLNX_BLUEFIELD. There is separate
effort that aims to add support for the Mellanox BlueField SoC
to the kernel tree.

Changes since v1:

1) Device bindings documentation:

   - Enumerate the device resources.
   - Edit the 'compatible' property to make it less generic.
   - Remove the 'bus' index property and replace i with a
     standard approach to read the bus identifier.
   - Get rid of the 'profile' property.
   - Use 'clock-frequency' property instead of 'bus-freq'.
   - Convert the clock frequency from KHz to Hz.
   - Remove useless examples.

2) Device driver source:

   - Update the '.compatible' field of of_device_id.
   - Update mlx_i2c_of_probe() call to extract the bus id
     from the pre-defined aliases.
   - Don't check the profile property.
   - Read the 'clock-frequency' property to get the timing
     parameters. 
   - Convert the timing configuration from KHz to Hz.

Khalil Blaiech (2):
  i2c: i2c-mlx: I2C SMBus driver for Mellanox BlueField SoC
  dt-bindings: i2c: I2C binding for Mellanox BlueField SoC

 .../devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt |   67 +
 drivers/i2c/busses/Kconfig                         |   13 +
 drivers/i2c/busses/Makefile                        |    1 +
 drivers/i2c/busses/i2c-mlx.c                       | 2341 ++++++++++++++++++++
 4 files changed, 2422 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt
 create mode 100644 drivers/i2c/busses/i2c-mlx.c

-- 
2.1.2

[PATCH v2 2/2] dt-bindings: i2c: I2C binding for Mellanox BlueField SoC

[Khalil Blaiech]

Added device tree bindings documentation for Mellanox BlueField
I2C SMBus controller.

Reviewed-by: David Woods <dwoods@mellanox.com>
Signed-off-by: Khalil Blaiech <kblaiech@mellanox.com>
---
 .../devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt | 67 ++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt

diff --git a/Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt b/Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt
new file mode 100644
index 0000000..61a8037
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt
@@ -0,0 +1,67 @@
+Device tree configuration for the Mellanox I2C SMBus on BlueField SoCs
+
+Required Properties:
+- reg        :	address offset and length of the device registers. The
+		registers consists of a set of dedicated and shared
+	       	resources:
+
+			1: Smbus block registers (dedicated)
+			2: Cause master registers (dedicated)
+			3: Cause slave registers (dedicated)
+			4: Cause coalesce registers (shared)
+			5: GPIO 0 registers (shared)
+			6: CorePLL registers (shared)
+
+		The BlueField SoCs includes three I2C bus controllers;
+		the set of resources <address length> must be defined
+		as follow:
+
+		i2c bus 0:
+		    <0x02804000 0x800>	/* Smbus[0]        */
+		    <0x02801200 0x020>	/* Cause Master[0] */
+		    <0x02801260 0x020>	/* Cause Slave[0]  */
+		    <0x02801300 0x010>	/* Cause Coalesce  */
+		    <0x02802000 0x100>	/* GPIO 0          */
+		    <0x02800358 0x008>	/* CorePll         */
+
+		i2c bus 1:
+		    <0x02804800 0x800>	/* Smbus[1]        */
+		    <0x02801220 0x020>	/* Cause Master[1] */
+		    <0x02801280 0x020>	/* Cause Slave[1]  */
+		    <0x02801300 0x010>	/* Cause Coalesce  */
+		    <0x02802000 0x100>	/* GPIO 0          */
+		    <0x02800358 0x008>	/* CorePll         */
+
+		i2c bus 2:
+		    <0x02805000 0x800>	/* Smbus[2]        */
+		    <0x02801240 0x020>	/* Cause Master[2] */
+		    <0x028012a0 0x020>	/* Cause Slave[2]  */
+		    <0x02801300 0x010>	/* Cause Coalesce  */
+		    <0x02802000 0x100>	/* GPIO 0          */
+		    <0x02800358 0x008>	/* CorePll         */
+
+- compatible : 	should be "mellanox,i2c-mlxbf".
+- interrupts : 	interrupt number.
+
+Optional Properties:
+- clock-frequency :	bus frequency used to configure timing registers;
+  		    	allowed values are 100000, 400000 and 1000000;
+		    	those are expressed in Hz.
+
+Example:
+
+aliases {
+	i2c0 = &i2c_0
+};
+
+i2c_0: i2c {
+	compatible = "mellanox,i2c-mlxbf";
+	reg = <0x02804000 0x800>,
+	      <0x02801200 0x020>,
+	      <0x02801260 0x020>,
+	      <0x02801300 0x010>,
+	      <0x02802000 0x100>,
+	      <0x02800358 0x008>;
+	interrupts = <57>;
+	clock-frequency = <100000>;
+};
-- 
2.1.2

Re: [PATCH v2 2/2] dt-bindings: i2c: I2C binding for Mellanox BlueField SoC

[Rob Herring]

On Tue, Dec 04, 2018 at 12:40:58PM -0500, Khalil Blaiech wrote:
> Added device tree bindings documentation for Mellanox BlueField
> I2C SMBus controller.
> 
> Reviewed-by: David Woods <dwoods@mellanox.com>
> Signed-off-by: Khalil Blaiech <kblaiech@mellanox.com>
> ---
>  .../devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt | 67 ++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt b/Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt
> new file mode 100644
> index 0000000..61a8037
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt
> @@ -0,0 +1,67 @@
> +Device tree configuration for the Mellanox I2C SMBus on BlueField SoCs
> +
> +Required Properties:
> +- reg        :	address offset and length of the device registers. The
> +		registers consists of a set of dedicated and shared
> +	       	resources:

mixed tab and spaces.

> +
> +			1: Smbus block registers (dedicated)
> +			2: Cause master registers (dedicated)
> +			3: Cause slave registers (dedicated)
> +			4: Cause coalesce registers (shared)
> +			5: GPIO 0 registers (shared)
> +			6: CorePLL registers (shared)
> +
> +		The BlueField SoCs includes three I2C bus controllers;
> +		the set of resources <address length> must be defined
> +		as follow:
> +
> +		i2c bus 0:
> +		    <0x02804000 0x800>	/* Smbus[0]        */
> +		    <0x02801200 0x020>	/* Cause Master[0] */
> +		    <0x02801260 0x020>	/* Cause Slave[0]  */
> +		    <0x02801300 0x010>	/* Cause Coalesce  */
> +		    <0x02802000 0x100>	/* GPIO 0          */
> +		    <0x02800358 0x008>	/* CorePll         */
> +
> +		i2c bus 1:
> +		    <0x02804800 0x800>	/* Smbus[1]        */
> +		    <0x02801220 0x020>	/* Cause Master[1] */
> +		    <0x02801280 0x020>	/* Cause Slave[1]  */

> +		    <0x02801300 0x010>	/* Cause Coalesce  */
> +		    <0x02802000 0x100>	/* GPIO 0          */
> +		    <0x02800358 0x008>	/* CorePll         */

You should not have overlapping register addresses in DT. These should 
be separate nodes.

Rob

RE: [PATCH v2 2/2] dt-bindings: i2c: I2C binding for Mellanox BlueField SoC

[Khalil Blaiech]

Rob,

Apologies for the late reply. It wasn't that easy to address your comments
and revisit the design of the driver. Please note that I applied the necessary
changes and submitted a V3.

> On Tue, Dec 04, 2018 at 12:40:58PM -0500, Khalil Blaiech wrote:
> > Added device tree bindings documentation for Mellanox BlueField I2C
> > SMBus controller.
> >
> > Reviewed-by: David Woods <dwoods@mellanox.com>
> > Signed-off-by: Khalil Blaiech <kblaiech@mellanox.com>
> > ---
> >  .../devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt | 67
> > ++++++++++++++++++++++
> >  1 file changed, 67 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt
> > b/Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt
> > new file mode 100644
> > index 0000000..61a8037
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.txt
> > @@ -0,0 +1,67 @@
> > +Device tree configuration for the Mellanox I2C SMBus on BlueField
> > +SoCs
> > +
> > +Required Properties:
> > +- reg        :	address offset and length of the device registers. The
> > +		registers consists of a set of dedicated and shared
> > +	       	resources:
> 
> mixed tab and spaces.

Removed the spaces and used tabs instead (sorry about that).

> 
> > +
> > +			1: Smbus block registers (dedicated)
> > +			2: Cause master registers (dedicated)
> > +			3: Cause slave registers (dedicated)
> > +			4: Cause coalesce registers (shared)
> > +			5: GPIO 0 registers (shared)
> > +			6: CorePLL registers (shared)
> > +
> > +		The BlueField SoCs includes three I2C bus controllers;
> > +		the set of resources <address length> must be defined
> > +		as follow:
> > +
> > +		i2c bus 0:
> > +		    <0x02804000 0x800>	/* Smbus[0]        */
> > +		    <0x02801200 0x020>	/* Cause Master[0] */
> > +		    <0x02801260 0x020>	/* Cause Slave[0]  */
> > +		    <0x02801300 0x010>	/* Cause Coalesce  */
> > +		    <0x02802000 0x100>	/* GPIO 0          */
> > +		    <0x02800358 0x008>	/* CorePll         */
> > +
> > +		i2c bus 1:
> > +		    <0x02804800 0x800>	/* Smbus[1]        */
> > +		    <0x02801220 0x020>	/* Cause Master[1] */
> > +		    <0x02801280 0x020>	/* Cause Slave[1]  */
> 
> > +		    <0x02801300 0x010>	/* Cause Coalesce  */
> > +		    <0x02802000 0x100>	/* GPIO 0          */
> > +		    <0x02800358 0x008>	/* CorePll         */
> 
> You should not have overlapping register addresses in DT. These should be
> separate nodes.

>From our perspective, a separate node would make things more complicated
because this would imply a sophisticated probe routine for such simple device;
this may also require a deferral of the probe of the i2c controllers...
That being said, I believe the intuitive approach would be to implement a logic
within the driver code that maps the device memory and serialize the access to
the common registers. The only convenient of such approach is to keep the
common register definitions consistent across our platforms...

By the way, these definitions were initially based on the design of our hardware.

> 
> Rob

Regards,
Khalil