Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1363419 > unrolled thread

[PATCH] regmap: mmio: Fix value endianness selection

Back to article view | Back to linux.kernel

Contents

  [PATCH] regmap: mmio: Fix value endianness selection Mark Brown <broonie@kernel.org> - 2016-03-23 14:20 +0100
    Re: [PATCH] regmap: mmio: Fix value endianness selection Alexander Stein <alexander.stein@systec-electronic.com> - 2016-03-23 14:40 +0100
    Applied "regmap: mmio: Fix value endianness selection" to the regmap tree Mark Brown <broonie@kernel.org> - 2016-03-23 14:50 +0100
    Re: [PATCH] regmap: mmio: Fix value endianness selection Alexander Stein <alexander.stein@systec-electronic.com> - 2016-03-23 15:30 +0100
      Re: [PATCH] regmap: mmio: Fix value endianness selection Mark Brown <broonie@kernel.org> - 2016-03-25 12:30 +0100
        Re: [PATCH] regmap: mmio: Fix value endianness selection Alexander Stein <alexander.stein@systec-electronic.com> - 2016-03-29 08:20 +0200
          Re: [PATCH] regmap: mmio: Fix value endianness selection Stefan Agner <stefan@agner.ch> - 2016-03-29 08:20 +0200

#1363419 — [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 <alexander.stein@systec-electronic.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 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

[toc] | [next] | [standalone]

#1363422

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 <alexander.stein@systec-electronic.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---

Tested-by: Alexander Stein <alexander.stein@systec-electronic.com>

Thanks and best regards,
Alexander

[toc] | [prev] | [next] | [standalone]

#1363433 — 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 <broonie@kernel.org>
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 <alexander.stein@systec-electronic.com>
Tested-by: Alexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 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

[toc] | [prev] | [next] | [standalone]

#1363459

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 <alexander.stein@systec-electronic.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>

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

[toc] | [prev] | [next] | [standalone]

#1364635

[Multipart message — attachments visible in raw view] — view raw

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.

[toc] | [prev] | [next] | [standalone]

#1365888

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

[toc] | [prev] | [next] | [standalone]

#1365895

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web