2016-03-23 13:18:19

by Mark Brown

[permalink] [raw]
Subject: [PATCH] regmap: mmio: Fix value endianness selection

Currently when selecting value endianness we check the register
endiannes, not the value endianness.

Reported-by: Alexander Stein <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---
drivers/base/regmap/regmap-mmio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regmap-mmio.c b/drivers/base/regmap/regmap-mmio.c
index 7526906ca080..b27573c69af7 100644
--- a/drivers/base/regmap/regmap-mmio.c
+++ b/drivers/base/regmap/regmap-mmio.c
@@ -245,7 +245,7 @@ static struct regmap_mmio_context *regmap_mmio_gen_context(struct device *dev,
ctx->val_bytes = config->val_bits / 8;
ctx->clk = ERR_PTR(-ENODEV);

- switch (config->reg_format_endian) {
+ switch (config->val_format_endian) {
case REGMAP_ENDIAN_DEFAULT:
case REGMAP_ENDIAN_LITTLE:
#ifdef __LITTLE_ENDIAN
--
2.7.0


2016-03-23 13:38:53

by Alexander Stein

[permalink] [raw]
Subject: Re: [PATCH] regmap: mmio: Fix value endianness selection

On Wednesday 23 March 2016 13:17:59, Mark Brown wrote:
> Currently when selecting value endianness we check the register
> endiannes, not the value endianness.
>
> Reported-by: Alexander Stein <[email protected]>
> Signed-off-by: Mark Brown <[email protected]>
> ---

Tested-by: Alexander Stein <[email protected]>

Thanks and best regards,
Alexander

2016-03-23 13:43:03

by Mark Brown

[permalink] [raw]
Subject: Applied "regmap: mmio: Fix value endianness selection" to the regmap tree

The patch

regmap: mmio: Fix value endianness selection

has been applied to the regmap tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 9f9f8b863ad130ec0c25f378bdbad64ba71291de Mon Sep 17 00:00:00 2001
From: Mark Brown <[email protected]>
Date: Wed, 23 Mar 2016 12:13:12 +0000
Subject: [PATCH] regmap: mmio: Fix value endianness selection

Currently when selecting value endianness we check the register
endiannes, not the value endianness.

Reported-by: Alexander Stein <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---
drivers/base/regmap/regmap-mmio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regmap-mmio.c b/drivers/base/regmap/regmap-mmio.c
index 7526906ca080..b27573c69af7 100644
--- a/drivers/base/regmap/regmap-mmio.c
+++ b/drivers/base/regmap/regmap-mmio.c
@@ -245,7 +245,7 @@ static struct regmap_mmio_context *regmap_mmio_gen_context(struct device *dev,
ctx->val_bytes = config->val_bits / 8;
ctx->clk = ERR_PTR(-ENODEV);

- switch (config->reg_format_endian) {
+ switch (config->val_format_endian) {
case REGMAP_ENDIAN_DEFAULT:
case REGMAP_ENDIAN_LITTLE:
#ifdef __LITTLE_ENDIAN
--
2.7.0

2016-03-23 14:20:58

by Alexander Stein

[permalink] [raw]
Subject: Re: [PATCH] regmap: mmio: Fix value endianness selection

On Wednesday 23 March 2016 13:17:59, Mark Brown wrote:
> Currently when selecting value endianness we check the register
> endiannes, not the value endianness.
>
> Reported-by: Alexander Stein <[email protected]>
> Signed-off-by: Mark Brown <[email protected]>

Mh, while this _does_ fix the problem regarding accessing SCFG peripheral
using syscon, it _does not_ fix the access for spi-fsl-dspi, this is still
using little-endian.
The difference in those drivers is that syscon manually sets
config.val_format_endian before calling regmap_init_mmio.
spi-fsl-dspi does not. I guess this driver relies on this configuration being
done in regmap_get_val_endian. But this is never reached because after setting
map->reg_read this code is skipped due to "goto skip_format_initialization;"
IMHO a call to regmap_get_val_endian should be added to
regmap_mmio_gen_context.

Best regards,
Alexander

2016-03-25 11:25:13

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] regmap: mmio: Fix value endianness selection

On Wed, Mar 23, 2016 at 03:20:46PM +0100, Alexander Stein wrote:

> The difference in those drivers is that syscon manually sets
> config.val_format_endian before calling regmap_init_mmio.
> spi-fsl-dspi does not. I guess this driver relies on this configuration being
> done in regmap_get_val_endian. But this is never reached because after setting
Does this IP exist in configurations where it is anything other than big
endian? If not then this probably shouldn't be in DT.

> map->reg_read this code is skipped due to "goto skip_format_initialization;"
> IMHO a call to regmap_get_val_endian should be added to
> regmap_mmio_gen_context.

That sounds reasonable.


Attachments:
(No filename) (672.00 B)
signature.asc (473.00 B)
Download all attachments

2016-03-29 06:10:36

by Alexander Stein

[permalink] [raw]
Subject: Re: [PATCH] regmap: mmio: Fix value endianness selection

On Friday 25 March 2016 11:24:59, Mark Brown wrote:
> On Wed, Mar 23, 2016 at 03:20:46PM +0100, Alexander Stein wrote:
> > The difference in those drivers is that syscon manually sets
> > config.val_format_endian before calling regmap_init_mmio.
> > spi-fsl-dspi does not. I guess this driver relies on this configuration
> > being done in regmap_get_val_endian. But this is never reached because
> > after setting
> Does this IP exist in configurations where it is anything other than big
> endian? If not then this probably shouldn't be in DT.

AFAIK it is included once or twice in the VFxxx series. CC'ed Stefan Agner for
confirmation.
Stefan: Is the DCU on VFxxx attached little-endian, e.g. no byte swapping
needed when accessing the periphery?

Best regards,
Alexander

2016-03-29 06:14:20

by Stefan Agner

[permalink] [raw]
Subject: Re: [PATCH] regmap: mmio: Fix value endianness selection

On 2016-03-28 23:10, Alexander Stein wrote:
> On Friday 25 March 2016 11:24:59, Mark Brown wrote:
>> On Wed, Mar 23, 2016 at 03:20:46PM +0100, Alexander Stein wrote:
>> > The difference in those drivers is that syscon manually sets
>> > config.val_format_endian before calling regmap_init_mmio.
>> > spi-fsl-dspi does not. I guess this driver relies on this configuration
>> > being done in regmap_get_val_endian. But this is never reached because
>> > after setting
>> Does this IP exist in configurations where it is anything other than big
>> endian? If not then this probably shouldn't be in DT.
>
> AFAIK it is included once or twice in the VFxxx series. CC'ed Stefan Agner for
> confirmation.
> Stefan: Is the DCU on VFxxx attached little-endian, e.g. no byte swapping
> needed when accessing the periphery?

Yes, DCU as well as DSPI is attached little endian on Vybrid (aka.
vf610).

--
Stefan