[PATCH] dspi: config dspi master regmap with right mode depend on BE or LE

[PATCH] dspi: config dspi master regmap with right mode depend on BE or LE

[Po Liu]

When kernel config with big endian mode, spi master need
to config regmap data value to be little endian mode. Or else,
the kernel boot will hang.

Signed-off-by: Po Liu <po.liu@nxp.com>
---
 drivers/spi/spi-fsl-dspi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 39412c9..a1d893c 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -637,6 +637,9 @@ static const struct regmap_config dspi_regmap_config = {
 	.val_bits = 32,
 	.reg_stride = 4,
 	.max_register = 0x88,
+#ifdef CONFIG_CPU_BIG_ENDIAN
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+#endif
 };
 
 static int dspi_probe(struct platform_device *pdev)
-- 
2.1.0.27.g96db324

[PATCH] dspi: config dspi master regmap with right mode depend on BE or LE

[Po Liu]

When kernel config with big endian mode, spi master need
to config regmap data value to be little endian mode. Or else,
the kernel boot will hang.

Signed-off-by: Po Liu <po.liu-3arQi8VN3Tc@public.gmane.org>
---
 drivers/spi/spi-fsl-dspi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 39412c9..a1d893c 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -637,6 +637,9 @@ static const struct regmap_config dspi_regmap_config = {
 	.val_bits = 32,
 	.reg_stride = 4,
 	.max_register = 0x88,
+#ifdef CONFIG_CPU_BIG_ENDIAN
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+#endif
 };
 
 static int dspi_probe(struct platform_device *pdev)
-- 
2.1.0.27.g96db324

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dspi: config dspi master regmap with right mode depend on BE or LE

[Mark Brown]

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

On Wed, Apr 13, 2016 at 07:03:22PM +0800, Po Liu wrote:
> When kernel config with big endian mode, spi master need
> to config regmap data value to be little endian mode. Or else,
> the kernel boot will hang.

> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +	.val_format_endian = REGMAP_ENDIAN_LITTLE,
> +#endif

This should be unconditionally set to native endian if the IP changes
endianness along with the CPU.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] dspi: config dspi master regmap with right mode depend on BE or LE

[Mark Brown]

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

On Wed, Apr 13, 2016 at 07:03:22PM +0800, Po Liu wrote:
> When kernel config with big endian mode, spi master need
> to config regmap data value to be little endian mode. Or else,
> the kernel boot will hang.

> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +	.val_format_endian = REGMAP_ENDIAN_LITTLE,
> +#endif

This should be unconditionally set to native endian if the IP changes
endianness along with the CPU.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] dspi: config dspi master regmap with right mode depend on BE or LE

[Scott Wood]

On 04/13/2016 06:12 AM, Po Liu wrote:
> When kernel config with big endian mode, spi master need
> to config regmap data value to be little endian mode. Or else,
> the kernel boot will hang.
> 
> Signed-off-by: Po Liu <po.liu@nxp.com>
> ---
>  drivers/spi/spi-fsl-dspi.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
> index 39412c9..a1d893c 100644
> --- a/drivers/spi/spi-fsl-dspi.c
> +++ b/drivers/spi/spi-fsl-dspi.c
> @@ -637,6 +637,9 @@ static const struct regmap_config dspi_regmap_config = {
>  	.val_bits = 32,
>  	.reg_stride = 4,
>  	.max_register = 0x88,
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +	.val_format_endian = REGMAP_ENDIAN_LITTLE,
> +#endif

Drop this ifdef.  It's little endian regardless of what mode the CPU is in.

Are you sure this is correct on all platforms?  E.g. ls1021a?

Looking more closely, the binding has a big-endian property.  It says
the default if that property is absent is native endian, which is
insane.  Fix the binding to say that little endian is the default (this
change shouldn't break any existing trees), and make sure that's what
the code implements.  I think you need val_format_endian_default, not
val_format_endian.

-Scott

Re: [PATCH] dspi: config dspi master regmap with right mode depend on BE or LE

[Mark Brown]

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

On Sun, Apr 17, 2016 at 02:41:40AM +0000, Scott Wood wrote:

> Looking more closely, the binding has a big-endian property.  It says
> the default if that property is absent is native endian, which is
> insane.  Fix the binding to say that little endian is the default (this
> change shouldn't break any existing trees), and make sure that's what
> the code implements.  I think you need val_format_endian_default, not
> val_format_endian.

The binding defaults to little endian and (in implmentation terms)
always has done though that used to be unintentional.  There *are* a
reasonable number of devices out there which are native endian, for
example most of the MIPS chips switch the endianness of the entire chip
rather than just the CPU, but most of the current usage has been on ARM
devices which only switch the core.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] dspi: config dspi master regmap with right mode depend on BE or LE

[Mark Brown]

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

On Sun, Apr 17, 2016 at 02:41:40AM +0000, Scott Wood wrote:

> Looking more closely, the binding has a big-endian property.  It says
> the default if that property is absent is native endian, which is
> insane.  Fix the binding to say that little endian is the default (this
> change shouldn't break any existing trees), and make sure that's what
> the code implements.  I think you need val_format_endian_default, not
> val_format_endian.

The binding defaults to little endian and (in implmentation terms)
always has done though that used to be unintentional.  There *are* a
reasonable number of devices out there which are native endian, for
example most of the MIPS chips switch the endianness of the entire chip
rather than just the CPU, but most of the current usage has been on ARM
devices which only switch the core.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

RE: [PATCH] dspi: config dspi master regmap with right mode depend on BE or LE

[Po Liu]

Hi Mark and Scott,

Thank you very much both!
The default dspi in the .dtsi setting to be Big endian as default. But it will hang at cpu setting big endian mode. After setting _format_endian = REGMAP_ENDIAN_LITTLE will fix it only in big endian mode.

We'll check it more about the val_format_endian_default.


Best regards,
Liu Po


Best regards,
Liu Po

EXT:8038


-----Original Message-----
From: Mark Brown [mailto:broonie@kernel.org] 
Sent: Monday, April 18, 2016 6:11 PM
To: Scott Wood
Cc: Po Liu; linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] dspi: config dspi master regmap with right mode depend on BE or LE

On Sun, Apr 17, 2016 at 02:41:40AM +0000, Scott Wood wrote:

> Looking more closely, the binding has a big-endian property.  It says 
> the default if that property is absent is native endian, which is 
> insane.  Fix the binding to say that little endian is the default 
> (this change shouldn't break any existing trees), and make sure that's 
> what the code implements.  I think you need val_format_endian_default, 
> not val_format_endian.

The binding defaults to little endian and (in implmentation terms) always has done though that used to be unintentional.  There *are* a reasonable number of devices out there which are native endian, for example most of the MIPS chips switch the endianness of the entire chip rather than just the CPU, but most of the current usage has been on ARM devices which only switch the core.

RE: [PATCH] dspi: config dspi master regmap with right mode depend on BE or LE

[Po Liu]

Hi Mark and Scott,

Thank you very much both!
The default dspi in the .dtsi setting to be Big endian as default. But it will hang at cpu setting big endian mode. After setting _format_endian = REGMAP_ENDIAN_LITTLE will fix it only in big endian mode.

We'll check it more about the val_format_endian_default.


Best regards,
Liu Po


Best regards,
Liu Po

EXT:8038


-----Original Message-----
From: Mark Brown [mailto:broonie@kernel.org] 
Sent: Monday, April 18, 2016 6:11 PM
To: Scott Wood
Cc: Po Liu; linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] dspi: config dspi master regmap with right mode depend on BE or LE

On Sun, Apr 17, 2016 at 02:41:40AM +0000, Scott Wood wrote:

> Looking more closely, the binding has a big-endian property.  It says 
> the default if that property is absent is native endian, which is 
> insane.  Fix the binding to say that little endian is the default 
> (this change shouldn't break any existing trees), and make sure that's 
> what the code implements.  I think you need val_format_endian_default, 
> not val_format_endian.

The binding defaults to little endian and (in implmentation terms) always has done though that used to be unintentional.  There *are* a reasonable number of devices out there which are native endian, for example most of the MIPS chips switch the endianness of the entire chip rather than just the CPU, but most of the current usage has been on ARM devices which only switch the core.