[PATCHv6 0/2] add DT endianness binding support for regmap

[PATCHv6 0/2] add DT endianness binding support for regmap

[Xiubo Li]

Changes in V6:
- there is some mistake for V5 patch series, resend it.

Changes in V5:
- remove the register's dt endianness support for it will always be BE
  for now.
- only support the register values buffers dt endianness support.



Xiubo Li (2):
  regmap: add DT endianness binding support.
  dt/bindings: Add the DT binding documentation for endianness

 .../devicetree/bindings/regmap/regmap.txt          |  48 +++++++++
 drivers/base/regmap/regmap-i2c.c                   |   2 +
 drivers/base/regmap/regmap-spi.c                   |   2 +
 drivers/base/regmap/regmap.c                       | 117 +++++++++++++++++++--
 4 files changed, 158 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regmap/regmap.txt

-- 
1.8.5

[PATCHv6 1/2] regmap: add DT endianness binding support.

[Xiubo Li]

For many drivers which will support rich endianness of Devices
need define DT properties by itself with the binding support.

The endianness using regmap:
Index      Device     Properties if needs bytes-swap,
                      or just ignore it
-------------------------------------------------------------
1          BE         'big-endian'
2          LE         'little-endian'

The properties include all the register values and the buffers.
And these properties are very usful for the MMIO devices:

Such as: a memory-mapped device, on one SoC is in BE mode, while
in another SoC will be in LE mode, and the CPU will always in LE
mode.

For the first case, we must use cpu_to_be32/be32_to_cpu for
32-bit registers accessing, so the 'big-endian' property is needed.

For the second case, we can just ignore the bytes-swap
functions like cpu_to_le32/le32_to_cpu, so the 'little-endian'
property could be abscent.

And vice versa...

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 drivers/base/regmap/regmap-i2c.c |   2 +
 drivers/base/regmap/regmap-spi.c |   2 +
 drivers/base/regmap/regmap.c     | 117 +++++++++++++++++++++++++++++++++++----
 3 files changed, 110 insertions(+), 11 deletions(-)

diff --git a/drivers/base/regmap/regmap-i2c.c b/drivers/base/regmap/regmap-i2c.c
index ca193d1..053150a 100644
--- a/drivers/base/regmap/regmap-i2c.c
+++ b/drivers/base/regmap/regmap-i2c.c
@@ -168,6 +168,8 @@ static struct regmap_bus regmap_i2c = {
 	.write = regmap_i2c_write,
 	.gather_write = regmap_i2c_gather_write,
 	.read = regmap_i2c_read,
+	.reg_format_endian_default = REGMAP_ENDIAN_BIG,
+	.val_format_endian_default = REGMAP_ENDIAN_BIG,
 };
 
 static const struct regmap_bus *regmap_get_i2c_bus(struct i2c_client *i2c,
diff --git a/drivers/base/regmap/regmap-spi.c b/drivers/base/regmap/regmap-spi.c
index 0eb3097..53d1148 100644
--- a/drivers/base/regmap/regmap-spi.c
+++ b/drivers/base/regmap/regmap-spi.c
@@ -109,6 +109,8 @@ static struct regmap_bus regmap_spi = {
 	.async_alloc = regmap_spi_async_alloc,
 	.read = regmap_spi_read,
 	.read_flag_mask = 0x80,
+	.reg_format_endian_default = REGMAP_ENDIAN_BIG,
+	.val_format_endian_default = REGMAP_ENDIAN_BIG,
 };
 
 /**
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 74d8c06..c033766 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -15,6 +15,7 @@
 #include <linux/export.h>
 #include <linux/mutex.h>
 #include <linux/err.h>
+#include <linux/of.h>
 #include <linux/rbtree.h>
 #include <linux/sched.h>
 
@@ -448,6 +449,102 @@ int regmap_attach_dev(struct device *dev, struct regmap *map,
 }
 EXPORT_SYMBOL_GPL(regmap_attach_dev);
 
+enum regmap_endian_type {
+	REGMAP_ENDIAN_REG,
+	REGMAP_ENDIAN_VAL,
+};
+
+static int of_regmap_get_endian(struct device *dev,
+				const struct regmap_bus *bus,
+				const struct regmap_config *config,
+				enum regmap_endian_type type,
+				enum regmap_endian *endian)
+{
+	struct device_node *np = dev->of_node;
+
+	if (!endian || !config)
+		return -EINVAL;
+
+	/*
+	 * Firstly, try to parse the endianness from driver's config,
+	 * this is to be compatible with the none DT or the old drivers.
+	 * From the driver's config the endianness value maybe:
+	 *   REGMAP_ENDIAN_BIG,
+	 *   REGMAP_ENDIAN_LITTLE,
+	 *   REGMAP_ENDIAN_NATIVE,
+	 *   REGMAP_ENDIAN_DEFAULT.
+	 */
+	switch (type) {
+	case REGMAP_ENDIAN_REG:
+		*endian = config->reg_format_endian;
+		break;
+	case REGMAP_ENDIAN_VAL:
+		*endian = config->val_format_endian;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/*
+	 * If the endianness parsed from driver config is
+	 * REGMAP_ENDIAN_DEFAULT, that means maybe we are using the DT
+	 * node to specify the endianness information.
+	 */
+	if (*endian != REGMAP_ENDIAN_DEFAULT)
+		return 0;
+
+	/*
+	 * Secondly, try to parse the endianness from DT node if the
+	 * driver config does not specify it.
+	 * From the DT node the endianness value maybe:
+	 *   REGMAP_ENDIAN_BIG,
+	 *   REGMAP_ENDIAN_LITTLE,
+	 *   REGMAP_ENDIAN_NATIVE,
+	 */
+	switch (type) {
+	case REGMAP_ENDIAN_VAL:
+		if (of_property_read_bool(np, "big-endian"))
+			*endian = REGMAP_ENDIAN_BIG;
+		else if (of_property_read_bool(np, "little-endian"))
+			*endian = REGMAP_ENDIAN_LITTLE;
+		else
+			*endian = REGMAP_ENDIAN_NATIVE;
+		break;
+	case REGMAP_ENDIAN_REG:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/*
+	 * If the endianness parsed from DT node is REGMAP_ENDIAN_NATIVE, that
+	 * maybe means the DT does not care the endianness or it should use
+	 * the regmap bus's default endianness, then we should try to check
+	 * whether the regmap bus has specified the default endianness.
+	 */
+	if (*endian != REGMAP_ENDIAN_NATIVE)
+		return 0;
+
+	/*
+	 * Finally, try to parse the endianness from regmap bus config
+	 * if in device's DT node the endianness property is absent.
+	 */
+	switch (type) {
+	case REGMAP_ENDIAN_REG:
+		if (bus && bus->reg_format_endian_default)
+			*endian = bus->reg_format_endian_default;
+		break;
+	case REGMAP_ENDIAN_VAL:
+		if (bus && bus->val_format_endian_default)
+			*endian = bus->val_format_endian_default;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 /**
  * regmap_init(): Initialise register map
  *
@@ -551,17 +648,15 @@ struct regmap *regmap_init(struct device *dev,
 		map->reg_read  = _regmap_bus_read;
 	}
 
-	reg_endian = config->reg_format_endian;
-	if (reg_endian == REGMAP_ENDIAN_DEFAULT)
-		reg_endian = bus->reg_format_endian_default;
-	if (reg_endian == REGMAP_ENDIAN_DEFAULT)
-		reg_endian = REGMAP_ENDIAN_BIG;
-
-	val_endian = config->val_format_endian;
-	if (val_endian == REGMAP_ENDIAN_DEFAULT)
-		val_endian = bus->val_format_endian_default;
-	if (val_endian == REGMAP_ENDIAN_DEFAULT)
-		val_endian = REGMAP_ENDIAN_BIG;
+	ret = of_regmap_get_endian(dev, bus, config, REGMAP_ENDIAN_REG,
+				   &reg_endian);
+	if (ret)
+		return ERR_PTR(ret);
+
+	ret = of_regmap_get_endian(dev, bus, config, REGMAP_ENDIAN_VAL,
+				   &val_endian);
+	if (ret)
+		return ERR_PTR(ret);
 
 	switch (config->reg_bits + map->reg_shift) {
 	case 2:
-- 
1.8.5

[PATCHv6 2/2] dt/bindings: Add the DT binding documentation for endianness

[Xiubo Li]

Device-Tree binding for device endianness
Index     Device     Endianness properties
---------------------------------------------------
1         BE         'big-endian'
2         LE         'little-endian'

For one device driver, which will run in different scenarios above
on different SoCs using the devicetree, we need one way to simplify
this.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 .../devicetree/bindings/regmap/regmap.txt          | 47 ++++++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regmap/regmap.txt

diff --git a/Documentation/devicetree/bindings/regmap/regmap.txt b/Documentation/devicetree/bindings/regmap/regmap.txt
new file mode 100644
index 0000000..b494f8b
--- /dev/null
+++ b/Documentation/devicetree/bindings/regmap/regmap.txt
@@ -0,0 +1,47 @@
+Device-Tree binding for regmap
+
+The endianness mode of CPU & Device scenarios:
+Index     Device     Endianness properties
+---------------------------------------------------
+1         BE         'big-endian'
+2         LE         'little-endian'
+
+For one device driver, which will run in different scenarios above
+on different SoCs using the devicetree, we need one way to simplify
+this.
+
+Required properties:
+- {big,little}-endian: these are boolean properties, if absent
+  meaning that the CPU and the Device are in the same endianness mode,
+  these properties are for register values and all the buffers only.
+
+Examples:
+Scenario 1 : CPU in LE mode & device in LE mode.
+dev: dev@40031000 {
+	      compatible = "name";
+	      reg = <0x40031000 0x1000>;
+	      ...
+};
+
+Scenario 2 : CPU in LE mode & device in BE mode.
+dev: dev@40031000 {
+	      compatible = "name";
+	      reg = <0x40031000 0x1000>;
+	      ...
+	      big-endian;
+};
+
+Scenario 3 : CPU in BE mode & device in BE mode.
+dev: dev@40031000 {
+	      compatible = "name";
+	      reg = <0x40031000 0x1000>;
+	      ...
+};
+
+Scenario 4 : CPU in BE mode & device in LE mode.
+dev: dev@40031000 {
+	      compatible = "name";
+	      reg = <0x40031000 0x1000>;
+	      ...
+	      little-endian;
+};
-- 
1.8.5

Re: [PATCHv6 2/2] dt/bindings: Add the DT binding documentation for endianness

[Varka Bhadram]

On 07/15/2014 09:53 AM, Xiubo Li wrote:
> Device-Tree binding for device endianness
> Index     Device     Endianness properties
> ---------------------------------------------------
> 1         BE         'big-endian'
> 2         LE         'little-endian'
>
> For one device driver, which will run in different scenarios above
> on different SoCs using the devicetree, we need one way to simplify
> this.
>
> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> ---
>   .../devicetree/bindings/regmap/regmap.txt          | 47 ++++++++++++++++++++++
>   1 file changed, 47 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/regmap/regmap.txt
>
> diff --git a/Documentation/devicetree/bindings/regmap/regmap.txt b/Documentation/devicetree/bindings/regmap/regmap.txt
> new file mode 100644
> index 0000000..b494f8b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regmap/regmap.txt
> @@ -0,0 +1,47 @@
> +Device-Tree binding for regmap
> +
> +The endianness mode of CPU & Device scenarios:
> +Index     Device     Endianness properties
> +---------------------------------------------------
> +1         BE         'big-endian'
> +2         LE         'little-endian'
> +
> +For one device driver, which will run in different scenarios above
> +on different SoCs using the devicetree, we need one way to simplify
> +this.
> +
> +Required properties:
> +- {big,little}-endian: these are boolean properties, if absent
> +  meaning that the CPU and the Device are in the same endianness mode,
> +  these properties are for register values and all the buffers only.
> +

It would be more readable if its like this:
Required properties:
- {big,little}-endian:	These are boolean properties, if absent meaning
			that the CPU and the Device are in the same
			endianness mode, these propperties are for
			values and all the buffers only.

> +Examples:
> +Scenario 1 : CPU in LE mode & device in LE mode.
> +dev: dev@40031000 {
> +	      compatible = "name";
> +	      reg = <0x40031000 0x1000>;
> +	      ...
> +};
> +
> +Scenario 2 : CPU in LE mode & device in BE mode.
> +dev: dev@40031000 {
> +	      compatible = "name";
> +	      reg = <0x40031000 0x1000>;
> +	      ...
> +	      big-endian;
> +};
> +
> +Scenario 3 : CPU in BE mode & device in BE mode.
> +dev: dev@40031000 {
> +	      compatible = "name";
> +	      reg = <0x40031000 0x1000>;
> +	      ...
> +};
> +
> +Scenario 4 : CPU in BE mode & device in LE mode.
> +dev: dev@40031000 {
> +	      compatible = "name";
> +	      reg = <0x40031000 0x1000>;
> +	      ...
> +	      little-endian;
> +};


-- 
Regards,
Varka Bhadram.

RE: [PATCHv6 2/2] dt/bindings: Add the DT binding documentation for endianness

[Li.Xiubo]

> > +For one device driver, which will run in different scenarios above
> > +on different SoCs using the devicetree, we need one way to simplify
> > +this.
> > +
> > +Required properties:
> > +- {big,little}-endian: these are boolean properties, if absent
> > +  meaning that the CPU and the Device are in the same endianness mode,
> > +  these properties are for register values and all the buffers only.
> > +
> 
> It would be more readable if its like this:
> Required properties:
> - {big,little}-endian:	These are boolean properties, if absent meaning
> 			that the CPU and the Device are in the same
> 			endianness mode, these propperties are for
> 			values and all the buffers only.
> 

Yes, it is.

See the next verison.

Thanks,

BRs
Xiubo



> > +Examples:
> > +Scenario 1 : CPU in LE mode & device in LE mode.
> > +dev: dev@40031000 {
> > +	      compatible = "name";
> > +	      reg = <0x40031000 0x1000>;
> > +	      ...
> > +};
> > +
> > +Scenario 2 : CPU in LE mode & device in BE mode.
> > +dev: dev@40031000 {
> > +	      compatible = "name";
> > +	      reg = <0x40031000 0x1000>;
> > +	      ...
> > +	      big-endian;
> > +};
> > +
> > +Scenario 3 : CPU in BE mode & device in BE mode.
> > +dev: dev@40031000 {
> > +	      compatible = "name";
> > +	      reg = <0x40031000 0x1000>;
> > +	      ...
> > +};
> > +
> > +Scenario 4 : CPU in BE mode & device in LE mode.
> > +dev: dev@40031000 {
> > +	      compatible = "name";
> > +	      reg = <0x40031000 0x1000>;
> > +	      ...
> > +	      little-endian;
> > +};
> 
> 
> --
> Regards,
> Varka Bhadram.

Re: [PATCHv6 2/2] dt/bindings: Add the DT binding documentation for endianness

[Thierry Reding]

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

On Tue, Jul 15, 2014 at 12:23:03PM +0800, Xiubo Li wrote:
> Device-Tree binding for device endianness
> Index     Device     Endianness properties
> ---------------------------------------------------
> 1         BE         'big-endian'
> 2         LE         'little-endian'
> 
> For one device driver, which will run in different scenarios above
> on different SoCs using the devicetree, we need one way to simplify
> this.
> 
> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> ---
>  .../devicetree/bindings/regmap/regmap.txt          | 47 ++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regmap/regmap.txt

Sorry for jumping in so late. I think this binding documentation needs
to be moved somewhere else. regmap is a Linux kernel specific
implementation detail and therefore has no place in device tree binding
descriptions. Furthermore the endianness properties described herein do
apply to drivers that don't use regmap.

Perhaps we need some top-level file that describes generic properties
such as this?

Thierry

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

RE: [PATCHv6 2/2] dt/bindings: Add the DT binding documentation for endianness

[Li.Xiubo]

> Subject: Re: [PATCHv6 2/2] dt/bindings: Add the DT binding documentation for
> endianness
> 
> On Tue, Jul 15, 2014 at 12:23:03PM +0800, Xiubo Li wrote:
> > Device-Tree binding for device endianness
> > Index     Device     Endianness properties
> > ---------------------------------------------------
> > 1         BE         'big-endian'
> > 2         LE         'little-endian'
> >
> > For one device driver, which will run in different scenarios above
> > on different SoCs using the devicetree, we need one way to simplify
> > this.
> >
> > Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> > ---
> >  .../devicetree/bindings/regmap/regmap.txt          | 47
> ++++++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/regmap/regmap.txt
> 
> Sorry for jumping in so late. I think this binding documentation needs
> to be moved somewhere else. regmap is a Linux kernel specific
> implementation detail and therefore has no place in device tree binding
> descriptions. Furthermore the endianness properties described herein do
> apply to drivers that don't use regmap.
> 
> Perhaps we need some top-level file that describes generic properties
> such as this?
> 
> Thierry

Yes, I do agree.
And there are many devices that have the endian issue without using the
Regmap in our LS* SoCs.

And I also think one generic binding description of this will be much
better.

Where should we place it? In Documentation/devicetree/bindings/, or...?


Thanks,

BRs
Xiubo

Re: [PATCHv6 2/2] dt/bindings: Add the DT binding documentation for endianness

[Thierry Reding]

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

On Wed, Aug 20, 2014 at 09:14:02AM +0000, Li.Xiubo@freescale.com wrote:
> > Subject: Re: [PATCHv6 2/2] dt/bindings: Add the DT binding documentation for
> > endianness
> > 
> > On Tue, Jul 15, 2014 at 12:23:03PM +0800, Xiubo Li wrote:
> > > Device-Tree binding for device endianness
> > > Index     Device     Endianness properties
> > > ---------------------------------------------------
> > > 1         BE         'big-endian'
> > > 2         LE         'little-endian'
> > >
> > > For one device driver, which will run in different scenarios above
> > > on different SoCs using the devicetree, we need one way to simplify
> > > this.
> > >
> > > Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> > > ---
> > >  .../devicetree/bindings/regmap/regmap.txt          | 47
> > ++++++++++++++++++++++
> > >  1 file changed, 47 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/regmap/regmap.txt
> > 
> > Sorry for jumping in so late. I think this binding documentation needs
> > to be moved somewhere else. regmap is a Linux kernel specific
> > implementation detail and therefore has no place in device tree binding
> > descriptions. Furthermore the endianness properties described herein do
> > apply to drivers that don't use regmap.
> > 
> > Perhaps we need some top-level file that describes generic properties
> > such as this?
> > 
> > Thierry
> 
> Yes, I do agree.
> And there are many devices that have the endian issue without using the
> Regmap in our LS* SoCs.
> 
> And I also think one generic binding description of this will be much
> better.
> 
> Where should we place it? In Documentation/devicetree/bindings/, or...?

Documentation/devicetree/bindings/standard-properties.txt perhaps?

Thierry

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

RE: [PATCHv6 0/2] add DT endianness binding support for regmap

[Li.Xiubo]

Hi,

I'd like to know the status about this patch series, there are many patches
Based the Regmap still waiting for these.

Thanks,
BRs
Xiubo


> -----Original Message-----
> From: Xiubo Li [mailto:Li.Xiubo@freescale.com]
> Sent: Tuesday, July 15, 2014 12:23 PM
> To: broonie@kernel.org; mark.rutland@arm.com
> Cc: robh+dt@kernel.org; pawel.moll@arm.com; ijc+devicetree@hellion.org.uk;
> galak@codeaurora.org; gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; Xiubo Li-B47053
> Subject: [PATCHv6 0/2] add DT endianness binding support for regmap
> 
> Changes in V6:
> - there is some mistake for V5 patch series, resend it.
> 
> Changes in V5:
> - remove the register's dt endianness support for it will always be BE
>   for now.
> - only support the register values buffers dt endianness support.
> 
> 
> 
> Xiubo Li (2):
>   regmap: add DT endianness binding support.
>   dt/bindings: Add the DT binding documentation for endianness
> 
>  .../devicetree/bindings/regmap/regmap.txt          |  48 +++++++++
>  drivers/base/regmap/regmap-i2c.c                   |   2 +
>  drivers/base/regmap/regmap-spi.c                   |   2 +
>  drivers/base/regmap/regmap.c                       | 117 +++++++++++++++++++-
> -
>  4 files changed, 158 insertions(+), 11 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/regmap/regmap.txt
> 
> --
> 1.8.5

Re: [PATCHv6 0/2] add DT endianness binding support for regmap

[Mark Brown]

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

On Tue, Jul 15, 2014 at 12:23:01PM +0800, Xiubo Li wrote:
> Changes in V6:
> - there is some mistake for V5 patch series, resend it.

Applied both, thanks.

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