[PATCH RESEND 0/4] I2C Ocores updates

[PATCH RESEND 0/4] I2C Ocores updates

[Jayachandran C]

[Sending this out again - the changes are straight-forward and I 
 do not see any reason for not merging them.  Comments welcome]

While trying to add reg-io-width property to i2c-ocores, we noticed
a few things that needs to fixed up in i2c-ocores device tree code.

The changes are to: 
 * use the standard 'reg-shift' property instead of 'regstep'
 * fix the fallout of the about change in drivers/mfd/timberdale.c
 * move bindings documentation to under Documentation/, 
 * fix up formatting, and add \n to a few dev_* messages,
 * and finally to add reg-io-width optional property.

Regards,
JC.

Ganesan Ramalingam (2):
  i2c: i2c-ocores: Use reg-shift property
  i2c: i2c-ocores: support for 16bit and 32bit IO

Jayachandran C (2):
  i2c: i2c-ocores - DT bindings and minor fixes.
  V4L/DVB: mfd: use reg_shift instead of regstep

 .../devicetree/bindings/i2c/i2c-ocores.txt         |   33 +++++++
 drivers/i2c/busses/i2c-ocores.c                    |   96 +++++++++-----------
 drivers/mfd/timberdale.c                           |    2 +-
 include/linux/i2c-ocores.h                         |    7 +-
 4 files changed, 83 insertions(+), 55 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-ocores.txt

-- 
1.7.9.5

[PATCH 1/4] i2c: i2c-ocores - DT bindings and minor fixes.

[Jayachandran C]

Cleanups to i2c-cores, no change in logic, changes are:
* Move i2c-ocores device tree documentation from source file to
  Documentation/devicetree/bindings/i2c/i2c-ocores.txt.
* Add \n to dev_warn and dev_err messages where missing
* Minor updates to the text and formatting fixes.

Signed-off-by: Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org>
---
 .../devicetree/bindings/i2c/i2c-ocores.txt         |   27 ++++++++++++
 drivers/i2c/busses/i2c-ocores.c                    |   45 +++-----------------
 2 files changed, 34 insertions(+), 38 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-ocores.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
new file mode 100644
index 0000000..bfec894
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
@@ -0,0 +1,27 @@
+Device tree configuration for i2c-ocores
+
+Required properties:
+- compatible      : "opencores,i2c-ocores"
+- reg             : bus address start and address range size of device
+- interrupts      : interrupt number
+- regstep         : size of device registers in bytes
+- clock-frequency : frequency of bus clock in Hz
+- #address-cells  : should be <1>
+- #size-cells     : should be <0>
+
+Example:
+
+	i2c0: ocores@a0000000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "opencores,i2c-ocores";
+		reg = <0xa0000000 0x8>;
+		interrupts = <10>;
+		regstep = <1>;
+		clock-frequency = <20000000>;
+
+		dummy@60 {
+			compatible = "dummy";
+			reg = <0x60>;
+		};
+	};
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 75194c5..e8159db 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -10,40 +10,9 @@
  */
 
 /*
- * Device tree configuration:
- *
- * Required properties:
- * - compatible      : "opencores,i2c-ocores"
- * - reg             : bus address start and address range size of device
- * - interrupts      : interrupt number
- * - regstep         : size of device registers in bytes
- * - clock-frequency : frequency of bus clock in Hz
- * 
- * Example:
- *
- *  i2c0: ocores@a0000000 {
- *              compatible = "opencores,i2c-ocores";
- *              reg = <0xa0000000 0x8>;
- *              interrupts = <10>;
- *
- *              regstep = <1>;
- *              clock-frequency = <20000000>;
- *
- * -- Devices connected on this I2C bus get
- * -- defined here; address- and size-cells
- * -- apply to these child devices
- *
- *              #address-cells = <1>;
- *              #size-cells = <0>;
- *
- *              dummy@60 {
- *                     compatible = "dummy";
- *                     reg = <60>;
- *              };
- *  };
- *
+ * This driver can be used from the device tree, see
+ *     Documentation/devicetree/bindings/i2c/ocore-i2c.txt
  */
-
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -247,14 +216,14 @@ static struct i2c_adapter ocores_adapter = {
 };
 
 #ifdef CONFIG_OF
-static int ocores_i2c_of_probe(struct platform_device* pdev,
-				struct ocores_i2c* i2c)
+static int ocores_i2c_of_probe(struct platform_device *pdev,
+				struct ocores_i2c *i2c)
 {
 	const __be32* val;
 
 	val = of_get_property(pdev->dev.of_node, "regstep", NULL);
 	if (!val) {
-		dev_err(&pdev->dev, "Missing required parameter 'regstep'");
+		dev_err(&pdev->dev, "Missing required parameter 'regstep'\n");
 		return -ENODEV;
 	}
 	i2c->regstep = be32_to_cpup(val);
@@ -262,7 +231,7 @@ static int ocores_i2c_of_probe(struct platform_device* pdev,
 	val = of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
 	if (!val) {
 		dev_err(&pdev->dev,
-			"Missing required parameter 'clock-frequency'");
+			"Missing required parameter 'clock-frequency'\n");
 		return -ENODEV;
 	}
 	i2c->clock_khz = be32_to_cpup(val) / 1000;
@@ -351,7 +320,7 @@ static int __devinit ocores_i2c_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int __devexit ocores_i2c_remove(struct platform_device* pdev)
+static int __devexit ocores_i2c_remove(struct platform_device *pdev)
 {
 	struct ocores_i2c *i2c = platform_get_drvdata(pdev);
 
-- 
1.7.9.5

[PATCH 2/4] i2c: i2c-ocores: Use reg-shift property

[Jayachandran C]

From: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

Deprecate 'regstep' property and use the standard 'reg-shift' property
for register offset shifts. 'regstep' will still be supported as an
optional property, but will give a warning when used.

Signed-off-by: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org>
---
 .../devicetree/bindings/i2c/i2c-ocores.txt         |    8 +++--
 drivers/i2c/busses/i2c-ocores.c                    |   36 ++++++++++++--------
 include/linux/i2c-ocores.h                         |    6 ++--
 3 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
index bfec894..1c9334b 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
@@ -4,11 +4,14 @@ Required properties:
 - compatible      : "opencores,i2c-ocores"
 - reg             : bus address start and address range size of device
 - interrupts      : interrupt number
-- regstep         : size of device registers in bytes
 - clock-frequency : frequency of bus clock in Hz
 - #address-cells  : should be <1>
 - #size-cells     : should be <0>
 
+Optional properties:
+- reg-shift       : device register offsets are shifted by this value
+- regstep         : deprecated, use reg-shift above
+
 Example:
 
 	i2c0: ocores@a0000000 {
@@ -17,9 +20,10 @@ Example:
 		compatible = "opencores,i2c-ocores";
 		reg = <0xa0000000 0x8>;
 		interrupts = <10>;
-		regstep = <1>;
 		clock-frequency = <20000000>;
 
+		reg-shift = <0>;	/* 8 bit registers */
+
 		dummy@60 {
 			compatible = "dummy";
 			reg = <0x60>;
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index e8159db..9617ec1 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -25,10 +25,11 @@
 #include <linux/slab.h>
 #include <linux/io.h>
 #include <linux/of_i2c.h>
+#include <linux/log2.h>
 
 struct ocores_i2c {
 	void __iomem *base;
-	int regstep;
+	int reg_shift;
 	wait_queue_head_t wait;
 	struct i2c_adapter adap;
 	struct i2c_msg *msg;
@@ -71,12 +72,12 @@ struct ocores_i2c {
 
 static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8 value)
 {
-	iowrite8(value, i2c->base + reg * i2c->regstep);
+	iowrite8(value, i2c->base + (reg << i2c->reg_shift));
 }
 
 static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
 {
-	return ioread8(i2c->base + reg * i2c->regstep);
+	return ioread8(i2c->base + (reg << i2c->reg_shift));
 }
 
 static void ocores_process(struct ocores_i2c *i2c)
@@ -219,22 +220,29 @@ static struct i2c_adapter ocores_adapter = {
 static int ocores_i2c_of_probe(struct platform_device *pdev,
 				struct ocores_i2c *i2c)
 {
-	const __be32* val;
-
-	val = of_get_property(pdev->dev.of_node, "regstep", NULL);
-	if (!val) {
-		dev_err(&pdev->dev, "Missing required parameter 'regstep'\n");
-		return -ENODEV;
+	struct device_node *np = pdev->dev.of_node;
+	u32 val;
+
+	if (of_property_read_u32(np, "reg-shift", &i2c->reg_shift)) {
+		/* no 'reg-shift', check for deprecated 'regstep' */
+		if (!of_property_read_u32(np, "regstep", &val)) {
+			if (!is_power_of_2(val)) {
+				dev_err(&pdev->dev, "invalid regstep %d\n",
+					val);
+				return -EINVAL;
+			}
+			i2c->reg_shift = ilog2(val);
+			dev_warn(&pdev->dev,
+				"regstep property deprecated, use reg-shift\n");
+		}
 	}
-	i2c->regstep = be32_to_cpup(val);
 
-	val = of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
-	if (!val) {
+	if (of_property_read_u32(np, "clock-frequency", &val)) {
 		dev_err(&pdev->dev,
 			"Missing required parameter 'clock-frequency'\n");
 		return -ENODEV;
 	}
-	i2c->clock_khz = be32_to_cpup(val) / 1000;
+	i2c->clock_khz = val / 1000;
 
 	return 0;
 }
@@ -277,7 +285,7 @@ static int __devinit ocores_i2c_probe(struct platform_device *pdev)
 
 	pdata = pdev->dev.platform_data;
 	if (pdata) {
-		i2c->regstep = pdata->regstep;
+		i2c->reg_shift = pdata->reg_shift;
 		i2c->clock_khz = pdata->clock_khz;
 	} else {
 		ret = ocores_i2c_of_probe(pdev, i2c);
diff --git a/include/linux/i2c-ocores.h b/include/linux/i2c-ocores.h
index 4d5e57f..bb58c7d 100644
--- a/include/linux/i2c-ocores.h
+++ b/include/linux/i2c-ocores.h
@@ -12,9 +12,9 @@
 #define _LINUX_I2C_OCORES_H
 
 struct ocores_i2c_platform_data {
-	u32 regstep;   /* distance between registers */
-	u32 clock_khz; /* input clock in kHz */
-	u8 num_devices; /* number of devices in the devices list */
+	u32 reg_shift;		/* register offset shift value */
+	u32 clock_khz;		/* input clock in kHz */
+	u8 num_devices;		/* number of devices in the devices list */
 	struct i2c_board_info const *devices; /* devices connected to the bus */
 };
 
-- 
1.7.9.5

[PATCH 3/4] V4L/DVB: mfd: use reg_shift instead of regstep

[Jayachandran C]

Update for change in i2c-ocores.h which uses reg_shift to
specify the register offset shifts instead of regstep.

Signed-off-by: Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org>
---
 drivers/mfd/timberdale.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/timberdale.c b/drivers/mfd/timberdale.c
index 0ba26fb..a447f4e 100644
--- a/drivers/mfd/timberdale.c
+++ b/drivers/mfd/timberdale.c
@@ -83,7 +83,7 @@ timberdale_xiic_platform_data = {
 
 static __devinitdata struct ocores_i2c_platform_data
 timberdale_ocores_platform_data = {
-	.regstep = 4,
+	.reg_shift = 2,
 	.clock_khz = 62500,
 	.devices = timberdale_i2c_board_info,
 	.num_devices = ARRAY_SIZE(timberdale_i2c_board_info)
-- 
1.7.9.5

[PATCH 4/4] i2c: i2c-ocores: support for 16bit and 32bit IO

[Jayachandran C]

From: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

Some architectures supports only 16-bit or 32-bit read/write access to
their IO space. Add a 'reg-io-width' platform and OF parameter which
specifies the IO width to support these platforms.

reg-io-width can be specified as 1, 2 or 4, and has a default value
of 1 if it is unspecified.

Signed-off-by: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org>
---
 .../devicetree/bindings/i2c/i2c-ocores.txt         |    2 ++
 drivers/i2c/busses/i2c-ocores.c                    |   21 ++++++++++++++++++--
 include/linux/i2c-ocores.h                         |    1 +
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
index 1c9334b..c15781f 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
@@ -10,6 +10,7 @@ Required properties:
 
 Optional properties:
 - reg-shift       : device register offsets are shifted by this value
+- reg-io-width    : io register width in bytes (1, 2 or 4)
 - regstep         : deprecated, use reg-shift above
 
 Example:
@@ -23,6 +24,7 @@ Example:
 		clock-frequency = <20000000>;
 
 		reg-shift = <0>;	/* 8 bit registers */
+		reg-io-width = <1>;	/* 8 bit read/write */
 
 		dummy@60 {
 			compatible = "dummy";
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 9617ec1..034d1d5 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -30,6 +30,7 @@
 struct ocores_i2c {
 	void __iomem *base;
 	int reg_shift;
+	int reg_io_width;
 	wait_queue_head_t wait;
 	struct i2c_adapter adap;
 	struct i2c_msg *msg;
@@ -72,12 +73,22 @@ struct ocores_i2c {
 
 static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8 value)
 {
-	iowrite8(value, i2c->base + (reg << i2c->reg_shift));
+	if (i2c->reg_io_width == 4)
+		iowrite32(value, i2c->base + (reg << i2c->reg_shift));
+	else if (i2c->reg_io_width == 2)
+		iowrite16(value, i2c->base + (reg << i2c->reg_shift));
+	else
+		iowrite8(value, i2c->base + (reg << i2c->reg_shift));
 }
 
 static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
 {
-	return ioread8(i2c->base + (reg << i2c->reg_shift));
+	if (i2c->reg_io_width == 4)
+		return ioread32(i2c->base + (reg << i2c->reg_shift));
+	else if (i2c->reg_io_width == 2)
+		return ioread16(i2c->base + (reg << i2c->reg_shift));
+	else
+		return ioread8(i2c->base + (reg << i2c->reg_shift));
 }
 
 static void ocores_process(struct ocores_i2c *i2c)
@@ -244,6 +255,8 @@ static int ocores_i2c_of_probe(struct platform_device *pdev,
 	}
 	i2c->clock_khz = val / 1000;
 
+	of_property_read_u32(pdev->dev.of_node, "reg-io-width",
+				&i2c->reg_io_width);
 	return 0;
 }
 #else
@@ -286,6 +299,7 @@ static int __devinit ocores_i2c_probe(struct platform_device *pdev)
 	pdata = pdev->dev.platform_data;
 	if (pdata) {
 		i2c->reg_shift = pdata->reg_shift;
+		i2c->reg_io_width = pdata->reg_io_width;
 		i2c->clock_khz = pdata->clock_khz;
 	} else {
 		ret = ocores_i2c_of_probe(pdev, i2c);
@@ -293,6 +307,9 @@ static int __devinit ocores_i2c_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	if (i2c->reg_io_width == 0)
+		i2c->reg_io_width = 1; /* Set to default value */
+
 	ocores_init(i2c);
 
 	init_waitqueue_head(&i2c->wait);
diff --git a/include/linux/i2c-ocores.h b/include/linux/i2c-ocores.h
index bb58c7d..1d99ebd 100644
--- a/include/linux/i2c-ocores.h
+++ b/include/linux/i2c-ocores.h
@@ -13,6 +13,7 @@
 
 struct ocores_i2c_platform_data {
 	u32 reg_shift;		/* register offset shift value */
+	u32 reg_io_width;	/* register io read/write width */
 	u32 clock_khz;		/* input clock in kHz */
 	u8 num_devices;		/* number of devices in the devices list */
 	struct i2c_board_info const *devices; /* devices connected to the bus */
-- 
1.7.9.5

Re: [PATCH 3/4] V4L/DVB: mfd: use reg_shift instead of regstep

[Richard Röjfors]

On Tue, 2012-07-10 at 11:40 +0530, Jayachandran C wrote:
> Update for change in i2c-ocores.h which uses reg_shift to
> specify the register offset shifts instead of regstep.
> 
> Signed-off-by: Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org>

Looks good to me.

Acked-by: Richard Röjfors <richard.rojfors-gfIc91nka+FZroRs9YW3xA@public.gmane.org>

> ---
>  drivers/mfd/timberdale.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/timberdale.c b/drivers/mfd/timberdale.c
> index 0ba26fb..a447f4e 100644
> --- a/drivers/mfd/timberdale.c
> +++ b/drivers/mfd/timberdale.c
> @@ -83,7 +83,7 @@ timberdale_xiic_platform_data = {
>  
>  static __devinitdata struct ocores_i2c_platform_data
>  timberdale_ocores_platform_data = {
> -	.regstep = 4,
> +	.reg_shift = 2,
>  	.clock_khz = 62500,
>  	.devices = timberdale_i2c_board_info,
>  	.num_devices = ARRAY_SIZE(timberdale_i2c_board_info)

Re: [PATCH 2/4] i2c: i2c-ocores: Use reg-shift property

[Wolfram Sang]

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

On Tue, Jul 10, 2012 at 11:40:56AM +0530, Jayachandran C wrote:
> From: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> 
> Deprecate 'regstep' property and use the standard 'reg-shift' property
> for register offset shifts. 'regstep' will still be supported as an
> optional property, but will give a warning when used.
> 
> Signed-off-by: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org>
> ---
>  .../devicetree/bindings/i2c/i2c-ocores.txt         |    8 +++--
>  drivers/i2c/busses/i2c-ocores.c                    |   36 ++++++++++++--------
>  include/linux/i2c-ocores.h                         |    6 ++--
>  3 files changed, 31 insertions(+), 19 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
> index bfec894..1c9334b 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
> @@ -4,11 +4,14 @@ Required properties:
>  - compatible      : "opencores,i2c-ocores"
>  - reg             : bus address start and address range size of device
>  - interrupts      : interrupt number
> -- regstep         : size of device registers in bytes
>  - clock-frequency : frequency of bus clock in Hz
>  - #address-cells  : should be <1>
>  - #size-cells     : should be <0>
>  
> +Optional properties:
> +- reg-shift       : device register offsets are shifted by this value
> +- regstep         : deprecated, use reg-shift above
> +
>  Example:
>  
>  	i2c0: ocores@a0000000 {
> @@ -17,9 +20,10 @@ Example:
>  		compatible = "opencores,i2c-ocores";
>  		reg = <0xa0000000 0x8>;
>  		interrupts = <10>;
> -		regstep = <1>;
>  		clock-frequency = <20000000>;
>  
> +		reg-shift = <0>;	/* 8 bit registers */
> +
>  		dummy@60 {
>  			compatible = "dummy";
>  			reg = <0x60>;
> diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
> index e8159db..9617ec1 100644
> --- a/drivers/i2c/busses/i2c-ocores.c
> +++ b/drivers/i2c/busses/i2c-ocores.c
> @@ -25,10 +25,11 @@
>  #include <linux/slab.h>
>  #include <linux/io.h>
>  #include <linux/of_i2c.h>
> +#include <linux/log2.h>
>  
>  struct ocores_i2c {
>  	void __iomem *base;
> -	int regstep;
> +	int reg_shift;

Should be u32 since you use of_property_read_u32(). sparse tells you
that.

>  	wait_queue_head_t wait;
>  	struct i2c_adapter adap;
>  	struct i2c_msg *msg;
> @@ -71,12 +72,12 @@ struct ocores_i2c {
>  
>  static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8 value)
>  {
> -	iowrite8(value, i2c->base + reg * i2c->regstep);
> +	iowrite8(value, i2c->base + (reg << i2c->reg_shift));
>  }
>  
>  static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
>  {
> -	return ioread8(i2c->base + reg * i2c->regstep);
> +	return ioread8(i2c->base + (reg << i2c->reg_shift));
>  }
>  
>  static void ocores_process(struct ocores_i2c *i2c)
> @@ -219,22 +220,29 @@ static struct i2c_adapter ocores_adapter = {
>  static int ocores_i2c_of_probe(struct platform_device *pdev,
>  				struct ocores_i2c *i2c)
>  {
> -	const __be32* val;
> -
> -	val = of_get_property(pdev->dev.of_node, "regstep", NULL);
> -	if (!val) {
> -		dev_err(&pdev->dev, "Missing required parameter 'regstep'\n");
> -		return -ENODEV;
> +	struct device_node *np = pdev->dev.of_node;
> +	u32 val;
> +
> +	if (of_property_read_u32(np, "reg-shift", &i2c->reg_shift)) {
> +		/* no 'reg-shift', check for deprecated 'regstep' */
> +		if (!of_property_read_u32(np, "regstep", &val)) {
> +			if (!is_power_of_2(val)) {
> +				dev_err(&pdev->dev, "invalid regstep %d\n",
> +					val);
> +				return -EINVAL;
> +			}
> +			i2c->reg_shift = ilog2(val);
> +			dev_warn(&pdev->dev,
> +				"regstep property deprecated, use reg-shift\n");
> +		}
>  	}
> -	i2c->regstep = be32_to_cpup(val);
>  
> -	val = of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
> -	if (!val) {
> +	if (of_property_read_u32(np, "clock-frequency", &val)) {
>  		dev_err(&pdev->dev,
>  			"Missing required parameter 'clock-frequency'\n");
>  		return -ENODEV;
>  	}
> -	i2c->clock_khz = be32_to_cpup(val) / 1000;
> +	i2c->clock_khz = val / 1000;
>  
>  	return 0;
>  }
> @@ -277,7 +285,7 @@ static int __devinit ocores_i2c_probe(struct platform_device *pdev)
>  
>  	pdata = pdev->dev.platform_data;
>  	if (pdata) {
> -		i2c->regstep = pdata->regstep;
> +		i2c->reg_shift = pdata->reg_shift;
>  		i2c->clock_khz = pdata->clock_khz;
>  	} else {
>  		ret = ocores_i2c_of_probe(pdev, i2c);
> diff --git a/include/linux/i2c-ocores.h b/include/linux/i2c-ocores.h
> index 4d5e57f..bb58c7d 100644
> --- a/include/linux/i2c-ocores.h
> +++ b/include/linux/i2c-ocores.h
> @@ -12,9 +12,9 @@
>  #define _LINUX_I2C_OCORES_H
>  
>  struct ocores_i2c_platform_data {
> -	u32 regstep;   /* distance between registers */
> -	u32 clock_khz; /* input clock in kHz */
> -	u8 num_devices; /* number of devices in the devices list */
> +	u32 reg_shift;		/* register offset shift value */
> +	u32 clock_khz;		/* input clock in kHz */
> +	u8 num_devices;		/* number of devices in the devices list */

Minor: I'd like to keep the changes here minimal, so 'git blame' will be more
useful. Given the line below, indentation is not consisten anyhow.

>  	struct i2c_board_info const *devices; /* devices connected to the bus */
>  };

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

Re: [PATCH 3/4] V4L/DVB: mfd: use reg_shift instead of regstep

[Wolfram Sang]

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

On Tue, Jul 10, 2012 at 11:40:57AM +0530, Jayachandran C wrote:
> Update for change in i2c-ocores.h which uses reg_shift to
> specify the register offset shifts instead of regstep.
> 
> Signed-off-by: Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org>

I'd like to take this via I2C tree since because of dependencies.
So MFD acks are very welcome, but I think not enough lists are on CC.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

Re: [PATCH 4/4] i2c: i2c-ocores: support for 16bit and 32bit IO

[Wolfram Sang]

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

On Tue, Jul 10, 2012 at 11:40:58AM +0530, Jayachandran C wrote:
> From: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> 
> Some architectures supports only 16-bit or 32-bit read/write access to
> their IO space. Add a 'reg-io-width' platform and OF parameter which
> specifies the IO width to support these platforms.
> 
> reg-io-width can be specified as 1, 2 or 4, and has a default value
> of 1 if it is unspecified.
> 
> Signed-off-by: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org>
> ---
>  .../devicetree/bindings/i2c/i2c-ocores.txt         |    2 ++
>  drivers/i2c/busses/i2c-ocores.c                    |   21 ++++++++++++++++++--
>  include/linux/i2c-ocores.h                         |    1 +
>  3 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
> index 1c9334b..c15781f 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
> @@ -10,6 +10,7 @@ Required properties:
>  
>  Optional properties:
>  - reg-shift       : device register offsets are shifted by this value
> +- reg-io-width    : io register width in bytes (1, 2 or 4)
>  - regstep         : deprecated, use reg-shift above
>  
>  Example:
> @@ -23,6 +24,7 @@ Example:
>  		clock-frequency = <20000000>;
>  
>  		reg-shift = <0>;	/* 8 bit registers */
> +		reg-io-width = <1>;	/* 8 bit read/write */
>  
>  		dummy@60 {
>  			compatible = "dummy";
> diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
> index 9617ec1..034d1d5 100644
> --- a/drivers/i2c/busses/i2c-ocores.c
> +++ b/drivers/i2c/busses/i2c-ocores.c
> @@ -30,6 +30,7 @@
>  struct ocores_i2c {
>  	void __iomem *base;
>  	int reg_shift;
> +	int reg_io_width;

u32 again.

>  	wait_queue_head_t wait;
>  	struct i2c_adapter adap;
>  	struct i2c_msg *msg;
> @@ -72,12 +73,22 @@ struct ocores_i2c {
>  
>  static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8 value)
>  {
> -	iowrite8(value, i2c->base + (reg << i2c->reg_shift));
> +	if (i2c->reg_io_width == 4)
> +		iowrite32(value, i2c->base + (reg << i2c->reg_shift));
> +	else if (i2c->reg_io_width == 2)
> +		iowrite16(value, i2c->base + (reg << i2c->reg_shift));
> +	else
> +		iowrite8(value, i2c->base + (reg << i2c->reg_shift));

Hmm, this could be a helper function, probably. But nothing you'd need
to do right now...

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

Re: [PATCH 2/4] i2c: i2c-ocores: Use reg-shift property

[Jayachandran C.]

On Thu, Jul 12, 2012 at 03:31:57PM +0200, Wolfram Sang wrote:
> On Tue, Jul 10, 2012 at 11:40:56AM +0530, Jayachandran C wrote:
> > From: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> > 
> > Deprecate 'regstep' property and use the standard 'reg-shift' property
> > for register offset shifts. 'regstep' will still be supported as an
> > optional property, but will give a warning when used.
> > 
> > Signed-off-by: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org>
> > ---
> >  .../devicetree/bindings/i2c/i2c-ocores.txt         |    8 +++--
> >  drivers/i2c/busses/i2c-ocores.c                    |   36 ++++++++++++--------
> >  include/linux/i2c-ocores.h                         |    6 ++--
> >  3 files changed, 31 insertions(+), 19 deletions(-)
> > 
[...]
> > diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
> > index e8159db..9617ec1 100644
> > --- a/drivers/i2c/busses/i2c-ocores.c
> > +++ b/drivers/i2c/busses/i2c-ocores.c
> > @@ -25,10 +25,11 @@
> >  #include <linux/slab.h>
> >  #include <linux/io.h>
> >  #include <linux/of_i2c.h>
> > +#include <linux/log2.h>
> >  
> >  struct ocores_i2c {
> >  	void __iomem *base;
> > -	int regstep;
> > +	int reg_shift;
> 
> Should be u32 since you use of_property_read_u32(). sparse tells you
> that.

We can keep reg_shift as int (fixing the width for reg_shift to 32
is unnecessary), and use a u32 to read the property, see below

> > +	struct device_node *np = pdev->dev.of_node;
> > +	u32 val;
> > +
> > +	if (of_property_read_u32(np, "reg-shift", &i2c->reg_shift)) {
> > +		/* no 'reg-shift', check for deprecated 'regstep' */
> > +		if (!of_property_read_u32(np, "regstep", &val)) {
> > +			if (!is_power_of_2(val)) {
> > +				dev_err(&pdev->dev, "invalid regstep %d\n",
> > +					val);
> > +				return -EINVAL;
> > +			}
> > +			i2c->reg_shift = ilog2(val);
> > +			dev_warn(&pdev->dev,
> > +				"regstep property deprecated, use reg-shift\n");
> > +		}
> >  	}

Will change this to 
	if (of_property_read_u32(np, "reg-shift", &val)) {
		....
	} else
		i2c->reg_shift = val;

[...]
> > diff --git a/include/linux/i2c-ocores.h b/include/linux/i2c-ocores.h
> > index 4d5e57f..bb58c7d 100644
> > --- a/include/linux/i2c-ocores.h
> > +++ b/include/linux/i2c-ocores.h
> > @@ -12,9 +12,9 @@
> >  #define _LINUX_I2C_OCORES_H
> >  
> >  struct ocores_i2c_platform_data {
> > -	u32 regstep;   /* distance between registers */
> > -	u32 clock_khz; /* input clock in kHz */
> > -	u8 num_devices; /* number of devices in the devices list */
> > +	u32 reg_shift;		/* register offset shift value */
> > +	u32 clock_khz;		/* input clock in kHz */
> > +	u8 num_devices;		/* number of devices in the devices list */
> 
> Minor: I'd like to keep the changes here minimal, so 'git blame' will be more
> useful. Given the line below, indentation is not consisten anyhow.

Ok, I will post an updated patchset with the changes.

Regards,
JC.

Re: [PATCH 2/4] i2c: i2c-ocores: Use reg-shift property

[Jayachandran C.]

On Fri, Jul 13, 2012 at 11:17:31AM +0200, Wolfram Sang wrote:
> On Fri, Jul 13, 2012 at 12:09:01PM +0530, Jayachandran C wrote:
> > From: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> > 
> > Deprecate 'regstep' property and use the standard 'reg-shift' property
> > for register offset shifts. 'regstep' will still be supported as an
> > optional property, but will give a warning when used.
> > 
> > Signed-off-by: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org>
> > ---
[...]
> > +		}
> > +	} else
> > +		i2c->reg_shift = val;
> 
> Now you are assigning an u32 to an int. Regstep will never be really a
> huge number, but still...
> 
> Also, braces around else-block according to coding style.
> 
> >  
> > -	val = of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
> > -	if (!val) {
> > +	if (of_property_read_u32(np, "clock-frequency", &val)) {
> >  		dev_err(&pdev->dev,
> >  			"Missing required parameter 'clock-frequency'\n");
> >  		return -ENODEV;
> >  	}
> > -	i2c->clock_khz = be32_to_cpup(val) / 1000;
> > +	i2c->clock_khz = val / 1000;
> 
> Okay, "/ 1000" will guarantee that the divided value will fit into an
> int. Yet, what do we gain by not using u32?

I will send out a new version of this patch using u32. In this case I
felt that there is no need to use a fixed width type when an int would
do.

Regards,
JC.

Re: [PATCH 2/4] i2c: i2c-ocores: Use reg-shift property

[Wolfram Sang]

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

On Fri, Jul 13, 2012 at 12:09:01PM +0530, Jayachandran C wrote:
> From: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> 
> Deprecate 'regstep' property and use the standard 'reg-shift' property
> for register offset shifts. 'regstep' will still be supported as an
> optional property, but will give a warning when used.
> 
> Signed-off-by: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org>
> ---
>  .../devicetree/bindings/i2c/i2c-ocores.txt         |    8 +++-
>  drivers/i2c/busses/i2c-ocores.c                    |   39 ++++++++++++--------
>  include/linux/i2c-ocores.h                         |    2 +-
>  3 files changed, 31 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
> index bfec894..1c9334b 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
> @@ -4,11 +4,14 @@ Required properties:
>  - compatible      : "opencores,i2c-ocores"
>  - reg             : bus address start and address range size of device
>  - interrupts      : interrupt number
> -- regstep         : size of device registers in bytes
>  - clock-frequency : frequency of bus clock in Hz
>  - #address-cells  : should be <1>
>  - #size-cells     : should be <0>
>  
> +Optional properties:
> +- reg-shift       : device register offsets are shifted by this value
> +- regstep         : deprecated, use reg-shift above
> +
>  Example:
>  
>  	i2c0: ocores@a0000000 {
> @@ -17,9 +20,10 @@ Example:
>  		compatible = "opencores,i2c-ocores";
>  		reg = <0xa0000000 0x8>;
>  		interrupts = <10>;
> -		regstep = <1>;
>  		clock-frequency = <20000000>;
>  
> +		reg-shift = <0>;	/* 8 bit registers */
> +
>  		dummy@60 {
>  			compatible = "dummy";
>  			reg = <0x60>;
> diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
> index e8159db..fb8db30 100644
> --- a/drivers/i2c/busses/i2c-ocores.c
> +++ b/drivers/i2c/busses/i2c-ocores.c
> @@ -25,10 +25,11 @@
>  #include <linux/slab.h>
>  #include <linux/io.h>
>  #include <linux/of_i2c.h>
> +#include <linux/log2.h>
>  
>  struct ocores_i2c {
>  	void __iomem *base;
> -	int regstep;
> +	int reg_shift;
>  	wait_queue_head_t wait;
>  	struct i2c_adapter adap;
>  	struct i2c_msg *msg;
> @@ -71,12 +72,12 @@ struct ocores_i2c {
>  
>  static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8 value)
>  {
> -	iowrite8(value, i2c->base + reg * i2c->regstep);
> +	iowrite8(value, i2c->base + (reg << i2c->reg_shift));
>  }
>  
>  static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
>  {
> -	return ioread8(i2c->base + reg * i2c->regstep);
> +	return ioread8(i2c->base + (reg << i2c->reg_shift));
>  }
>  
>  static void ocores_process(struct ocores_i2c *i2c)
> @@ -219,22 +220,30 @@ static struct i2c_adapter ocores_adapter = {
>  static int ocores_i2c_of_probe(struct platform_device *pdev,
>  				struct ocores_i2c *i2c)
>  {
> -	const __be32* val;
> -
> -	val = of_get_property(pdev->dev.of_node, "regstep", NULL);
> -	if (!val) {
> -		dev_err(&pdev->dev, "Missing required parameter 'regstep'\n");
> -		return -ENODEV;
> -	}
> -	i2c->regstep = be32_to_cpup(val);
> +	struct device_node *np = pdev->dev.of_node;
> +	u32 val;
> +
> +	if (of_property_read_u32(np, "reg-shift", &val)) {
> +		/* no 'reg-shift', check for deprecated 'regstep' */
> +		if (!of_property_read_u32(np, "regstep", &val)) {
> +			if (!is_power_of_2(val)) {
> +				dev_err(&pdev->dev, "invalid regstep %d\n",
> +					val);
> +				return -EINVAL;
> +			}
> +			i2c->reg_shift = ilog2(val);
> +			dev_warn(&pdev->dev,
> +				"regstep property deprecated, use reg-shift\n");
> +		}
> +	} else
> +		i2c->reg_shift = val;

Now you are assigning an u32 to an int. Regstep will never be really a
huge number, but still...

Also, braces around else-block according to coding style.

>  
> -	val = of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
> -	if (!val) {
> +	if (of_property_read_u32(np, "clock-frequency", &val)) {
>  		dev_err(&pdev->dev,
>  			"Missing required parameter 'clock-frequency'\n");
>  		return -ENODEV;
>  	}
> -	i2c->clock_khz = be32_to_cpup(val) / 1000;
> +	i2c->clock_khz = val / 1000;

Okay, "/ 1000" will guarantee that the divided value will fit into an
int. Yet, what do we gain by not using u32?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

[PATCH 2/4] i2c: i2c-ocores: Use reg-shift property

[Jayachandran C]

From: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

Deprecate 'regstep' property and use the standard 'reg-shift' property
for register offset shifts. 'regstep' will still be supported as an
optional property, but will give a warning when used.

Signed-off-by: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org>
---
 .../devicetree/bindings/i2c/i2c-ocores.txt         |    8 +++-
 drivers/i2c/busses/i2c-ocores.c                    |   39 ++++++++++++--------
 include/linux/i2c-ocores.h                         |    2 +-
 3 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
index bfec894..1c9334b 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
@@ -4,11 +4,14 @@ Required properties:
 - compatible      : "opencores,i2c-ocores"
 - reg             : bus address start and address range size of device
 - interrupts      : interrupt number
-- regstep         : size of device registers in bytes
 - clock-frequency : frequency of bus clock in Hz
 - #address-cells  : should be <1>
 - #size-cells     : should be <0>
 
+Optional properties:
+- reg-shift       : device register offsets are shifted by this value
+- regstep         : deprecated, use reg-shift above
+
 Example:
 
 	i2c0: ocores@a0000000 {
@@ -17,9 +20,10 @@ Example:
 		compatible = "opencores,i2c-ocores";
 		reg = <0xa0000000 0x8>;
 		interrupts = <10>;
-		regstep = <1>;
 		clock-frequency = <20000000>;
 
+		reg-shift = <0>;	/* 8 bit registers */
+
 		dummy@60 {
 			compatible = "dummy";
 			reg = <0x60>;
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index e8159db..fb8db30 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -25,10 +25,11 @@
 #include <linux/slab.h>
 #include <linux/io.h>
 #include <linux/of_i2c.h>
+#include <linux/log2.h>
 
 struct ocores_i2c {
 	void __iomem *base;
-	int regstep;
+	int reg_shift;
 	wait_queue_head_t wait;
 	struct i2c_adapter adap;
 	struct i2c_msg *msg;
@@ -71,12 +72,12 @@ struct ocores_i2c {
 
 static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8 value)
 {
-	iowrite8(value, i2c->base + reg * i2c->regstep);
+	iowrite8(value, i2c->base + (reg << i2c->reg_shift));
 }
 
 static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
 {
-	return ioread8(i2c->base + reg * i2c->regstep);
+	return ioread8(i2c->base + (reg << i2c->reg_shift));
 }
 
 static void ocores_process(struct ocores_i2c *i2c)
@@ -219,22 +220,30 @@ static struct i2c_adapter ocores_adapter = {
 static int ocores_i2c_of_probe(struct platform_device *pdev,
 				struct ocores_i2c *i2c)
 {
-	const __be32* val;
-
-	val = of_get_property(pdev->dev.of_node, "regstep", NULL);
-	if (!val) {
-		dev_err(&pdev->dev, "Missing required parameter 'regstep'\n");
-		return -ENODEV;
-	}
-	i2c->regstep = be32_to_cpup(val);
+	struct device_node *np = pdev->dev.of_node;
+	u32 val;
+
+	if (of_property_read_u32(np, "reg-shift", &val)) {
+		/* no 'reg-shift', check for deprecated 'regstep' */
+		if (!of_property_read_u32(np, "regstep", &val)) {
+			if (!is_power_of_2(val)) {
+				dev_err(&pdev->dev, "invalid regstep %d\n",
+					val);
+				return -EINVAL;
+			}
+			i2c->reg_shift = ilog2(val);
+			dev_warn(&pdev->dev,
+				"regstep property deprecated, use reg-shift\n");
+		}
+	} else
+		i2c->reg_shift = val;
 
-	val = of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
-	if (!val) {
+	if (of_property_read_u32(np, "clock-frequency", &val)) {
 		dev_err(&pdev->dev,
 			"Missing required parameter 'clock-frequency'\n");
 		return -ENODEV;
 	}
-	i2c->clock_khz = be32_to_cpup(val) / 1000;
+	i2c->clock_khz = val / 1000;
 
 	return 0;
 }
@@ -277,7 +286,7 @@ static int __devinit ocores_i2c_probe(struct platform_device *pdev)
 
 	pdata = pdev->dev.platform_data;
 	if (pdata) {
-		i2c->regstep = pdata->regstep;
+		i2c->reg_shift = pdata->reg_shift;
 		i2c->clock_khz = pdata->clock_khz;
 	} else {
 		ret = ocores_i2c_of_probe(pdev, i2c);
diff --git a/include/linux/i2c-ocores.h b/include/linux/i2c-ocores.h
index 4d5e57f..75d7767 100644
--- a/include/linux/i2c-ocores.h
+++ b/include/linux/i2c-ocores.h
@@ -12,7 +12,7 @@
 #define _LINUX_I2C_OCORES_H
 
 struct ocores_i2c_platform_data {
-	u32 regstep;   /* distance between registers */
+	int reg_shift; /* register offset shift value */
 	u32 clock_khz; /* input clock in kHz */
 	u8 num_devices; /* number of devices in the devices list */
 	struct i2c_board_info const *devices; /* devices connected to the bus */
-- 
1.7.9.5

Re: [PATCH 2/4] i2c: i2c-ocores: Use reg-shift property

[Jayachandran C.]

On Fri, Jun 08, 2012 at 05:56:51PM +0530, Jayachandran C. wrote:
> On Fri, Jun 08, 2012 at 01:41:36PM +0200, Peter Korsgaard wrote:
> > >>>>> "J" == Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org> writes:
> > 
> >  J> From: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> >  J> Deprecate 'regstep' property and use the standard 'reg-shift' property
> >  J> for register offset shifts. 'regstep' will still be supported as an
> >  J> optional property, but will give a warning when used.
> > 
> > ..
> >  
> >  J>  struct ocores_i2c_platform_data {
> >  J> -	u32 regstep;   /* distance between registers */
> >  J> -	u32 clock_khz; /* input clock in kHz */
> >  J> -	u8 num_devices; /* number of devices in the devices list */
> >  J> +	u32 reg_shift;		/* register offset shift value */
> >  J> +	u32 clock_khz;		/* input clock in kHz */
> >  J> +	u8 num_devices;		/* number of devices in the devices list */
> >  J>  	struct i2c_board_info const *devices; /* devices connected to the bus */
> >  J>  };
> > 
> > Why not just keep this change to the dt bindings, instead of risking
> > breaking stuff for platform drivers as well? There's no conceptual
> > reason why reg_shift is any better than regstep.
> 
> This is to keep the names and meanings of platform property and DT
> property same. Having two ways (setting regstep in platform code or
> setting 'reg-shift' in DT) of specifying the same parameter is not
> a nice.
> 
> There is only one user of this API in the whole kernel tree, which
> is fixed as part of the patchset.
> 
> Also we make sure that we do not break existing DTBs by still accepting
> 'regstep' property.

Any further comments on this patchset? If the changes are fine, an Acked-by
would be really aeppreciated.

Thanks,
JC.

Re: [PATCH 2/4] i2c: i2c-ocores: Use reg-shift property

[Jayachandran C.]

On Fri, Jun 08, 2012 at 01:41:36PM +0200, Peter Korsgaard wrote:
> >>>>> "J" == Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org> writes:
> 
>  J> From: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>  J> Deprecate 'regstep' property and use the standard 'reg-shift' property
>  J> for register offset shifts. 'regstep' will still be supported as an
>  J> optional property, but will give a warning when used.
> 
> ..
>  
>  J>  struct ocores_i2c_platform_data {
>  J> -	u32 regstep;   /* distance between registers */
>  J> -	u32 clock_khz; /* input clock in kHz */
>  J> -	u8 num_devices; /* number of devices in the devices list */
>  J> +	u32 reg_shift;		/* register offset shift value */
>  J> +	u32 clock_khz;		/* input clock in kHz */
>  J> +	u8 num_devices;		/* number of devices in the devices list */
>  J>  	struct i2c_board_info const *devices; /* devices connected to the bus */
>  J>  };
> 
> Why not just keep this change to the dt bindings, instead of risking
> breaking stuff for platform drivers as well? There's no conceptual
> reason why reg_shift is any better than regstep.

This is to keep the names and meanings of platform property and DT
property same. Having two ways (setting regstep in platform code or
setting 'reg-shift' in DT) of specifying the same parameter is not
a nice.

There is only one user of this API in the whole kernel tree, which
is fixed as part of the patchset.

Also we make sure that we do not break existing DTBs by still accepting
'regstep' property.

Regards,
JC.

Re: [PATCH 2/4] i2c: i2c-ocores: Use reg-shift property

[Peter Korsgaard]

>>>>> "J" == Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org> writes:

 J> From: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
 J> Deprecate 'regstep' property and use the standard 'reg-shift' property
 J> for register offset shifts. 'regstep' will still be supported as an
 J> optional property, but will give a warning when used.

..
 
 J>  struct ocores_i2c_platform_data {
 J> -	u32 regstep;   /* distance between registers */
 J> -	u32 clock_khz; /* input clock in kHz */
 J> -	u8 num_devices; /* number of devices in the devices list */
 J> +	u32 reg_shift;		/* register offset shift value */
 J> +	u32 clock_khz;		/* input clock in kHz */
 J> +	u8 num_devices;		/* number of devices in the devices list */
 J>  	struct i2c_board_info const *devices; /* devices connected to the bus */
 J>  };

Why not just keep this change to the dt bindings, instead of risking
breaking stuff for platform drivers as well? There's no conceptual
reason why reg_shift is any better than regstep.

-- 
Bye, Peter Korsgaard

[PATCH 2/4] i2c: i2c-ocores: Use reg-shift property

[Jayachandran C]

From: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

Deprecate 'regstep' property and use the standard 'reg-shift' property
for register offset shifts. 'regstep' will still be supported as an
optional property, but will give a warning when used.

Signed-off-by: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org>
---
 .../devicetree/bindings/i2c/i2c-ocores.txt         |    8 +++--
 drivers/i2c/busses/i2c-ocores.c                    |   36 ++++++++++++--------
 include/linux/i2c-ocores.h                         |    6 ++--
 3 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
index bfec894..1c9334b 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
@@ -4,11 +4,14 @@ Required properties:
 - compatible      : "opencores,i2c-ocores"
 - reg             : bus address start and address range size of device
 - interrupts      : interrupt number
-- regstep         : size of device registers in bytes
 - clock-frequency : frequency of bus clock in Hz
 - #address-cells  : should be <1>
 - #size-cells     : should be <0>
 
+Optional properties:
+- reg-shift       : device register offsets are shifted by this value
+- regstep         : deprecated, use reg-shift above
+
 Example:
 
 	i2c0: ocores@a0000000 {
@@ -17,9 +20,10 @@ Example:
 		compatible = "opencores,i2c-ocores";
 		reg = <0xa0000000 0x8>;
 		interrupts = <10>;
-		regstep = <1>;
 		clock-frequency = <20000000>;
 
+		reg-shift = <0>;	/* 8 bit registers */
+
 		dummy@60 {
 			compatible = "dummy";
 			reg = <0x60>;
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index e8159db..9617ec1 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -25,10 +25,11 @@
 #include <linux/slab.h>
 #include <linux/io.h>
 #include <linux/of_i2c.h>
+#include <linux/log2.h>
 
 struct ocores_i2c {
 	void __iomem *base;
-	int regstep;
+	int reg_shift;
 	wait_queue_head_t wait;
 	struct i2c_adapter adap;
 	struct i2c_msg *msg;
@@ -71,12 +72,12 @@ struct ocores_i2c {
 
 static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8 value)
 {
-	iowrite8(value, i2c->base + reg * i2c->regstep);
+	iowrite8(value, i2c->base + (reg << i2c->reg_shift));
 }
 
 static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
 {
-	return ioread8(i2c->base + reg * i2c->regstep);
+	return ioread8(i2c->base + (reg << i2c->reg_shift));
 }
 
 static void ocores_process(struct ocores_i2c *i2c)
@@ -219,22 +220,29 @@ static struct i2c_adapter ocores_adapter = {
 static int ocores_i2c_of_probe(struct platform_device *pdev,
 				struct ocores_i2c *i2c)
 {
-	const __be32* val;
-
-	val = of_get_property(pdev->dev.of_node, "regstep", NULL);
-	if (!val) {
-		dev_err(&pdev->dev, "Missing required parameter 'regstep'\n");
-		return -ENODEV;
+	struct device_node *np = pdev->dev.of_node;
+	u32 val;
+
+	if (of_property_read_u32(np, "reg-shift", &i2c->reg_shift)) {
+		/* no 'reg-shift', check for deprecated 'regstep' */
+		if (!of_property_read_u32(np, "regstep", &val)) {
+			if (!is_power_of_2(val)) {
+				dev_err(&pdev->dev, "invalid regstep %d\n",
+					val);
+				return -EINVAL;
+			}
+			i2c->reg_shift = ilog2(val);
+			dev_warn(&pdev->dev,
+				"regstep property deprecated, use reg-shift\n");
+		}
 	}
-	i2c->regstep = be32_to_cpup(val);
 
-	val = of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
-	if (!val) {
+	if (of_property_read_u32(np, "clock-frequency", &val)) {
 		dev_err(&pdev->dev,
 			"Missing required parameter 'clock-frequency'\n");
 		return -ENODEV;
 	}
-	i2c->clock_khz = be32_to_cpup(val) / 1000;
+	i2c->clock_khz = val / 1000;
 
 	return 0;
 }
@@ -277,7 +285,7 @@ static int __devinit ocores_i2c_probe(struct platform_device *pdev)
 
 	pdata = pdev->dev.platform_data;
 	if (pdata) {
-		i2c->regstep = pdata->regstep;
+		i2c->reg_shift = pdata->reg_shift;
 		i2c->clock_khz = pdata->clock_khz;
 	} else {
 		ret = ocores_i2c_of_probe(pdev, i2c);
diff --git a/include/linux/i2c-ocores.h b/include/linux/i2c-ocores.h
index 4d5e57f..bb58c7d 100644
--- a/include/linux/i2c-ocores.h
+++ b/include/linux/i2c-ocores.h
@@ -12,9 +12,9 @@
 #define _LINUX_I2C_OCORES_H
 
 struct ocores_i2c_platform_data {
-	u32 regstep;   /* distance between registers */
-	u32 clock_khz; /* input clock in kHz */
-	u8 num_devices; /* number of devices in the devices list */
+	u32 reg_shift;		/* register offset shift value */
+	u32 clock_khz;		/* input clock in kHz */
+	u8 num_devices;		/* number of devices in the devices list */
 	struct i2c_board_info const *devices; /* devices connected to the bus */
 };
 
-- 
1.7.9.5