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

Groups > linux.kernel > #1332958 > unrolled thread

[PATCH] Revert "ASoC: qcom: Specify LE device endianness"

Back to article view | Back to linux.kernel

Contents

  [PATCH] Revert "ASoC: qcom: Specify LE device endianness" Stephen Boyd <sboyd@codeaurora.org> - 2016-02-12 18:40 +0100
    Re: [PATCH] Revert "ASoC: qcom: Specify LE device endianness" Mark Brown <broonie@kernel.org> - 2016-02-12 20:40 +0100
    Re: [PATCH] Revert "ASoC: qcom: Specify LE device endianness" Arnd Bergmann <arnd@arndb.de> - 2016-02-12 21:00 +0100
      Re: [PATCH] Revert "ASoC: qcom: Specify LE device endianness" Mark Brown <broonie@kernel.org> - 2016-02-12 21:10 +0100
        Re: [PATCH] Revert "ASoC: qcom: Specify LE device endianness" Arnd Bergmann <arnd@arndb.de> - 2016-02-12 21:40 +0100
          Re: [PATCH] Revert "ASoC: qcom: Specify LE device endianness" Mark Brown <broonie@kernel.org> - 2016-02-12 22:30 +0100
      Re: [PATCH] Revert "ASoC: qcom: Specify LE device endianness" Stephen Boyd <sboyd@codeaurora.org> - 2016-02-12 23:10 +0100
    Re: [PATCH] Revert "ASoC: qcom: Specify LE device endianness" Mark Brown <broonie@kernel.org> - 2016-02-12 22:30 +0100
      Re: [PATCH] Revert "ASoC: qcom: Specify LE device endianness" Stephen Boyd <sboyd@codeaurora.org> - 2016-02-12 23:10 +0100
        Re: [PATCH] Revert "ASoC: qcom: Specify LE device endianness" Mark Brown <broonie@kernel.org> - 2016-02-12 23:30 +0100

#1332958 — [PATCH] Revert "ASoC: qcom: Specify LE device endianness"

This reverts commit 18560a4e3b07438113b50589e78532d95f907029.

The commit that caused us to specify LE device endianness here,
29bb45f25ff3 (regmap-mmio: Use native endianness for read/write,
2015-10-29), has been reverted in mainline so now when we specify
LE it actively breaks big endian kernels because the byte
swapping in regmap-mmio is incorrect. Let's revert this change
because it will 1) fix the big endian kernels and 2) be redundant
to specify LE because that will become the default soon.

Cc: Kenneth Westfield <kwestfie@codeaurora.org>
Cc: Kevin Hilman <khilman@linaro.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 sound/soc/qcom/lpass-cpu.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index 00b6c9d039cf..e5101e0d2d37 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -355,7 +355,6 @@ static struct regmap_config lpass_cpu_regmap_config = {
 	.readable_reg = lpass_cpu_regmap_readable,
 	.volatile_reg = lpass_cpu_regmap_volatile,
 	.cache_type = REGCACHE_FLAT,
-	.val_format_endian = REGMAP_ENDIAN_LITTLE,
 };
 
 int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [next] | [standalone]

#1333062

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

On Fri, Feb 12, 2016 at 09:30:17AM -0800, Stephen Boyd wrote:
> This reverts commit 18560a4e3b07438113b50589e78532d95f907029.

Please use subject lines matching the style for the subsystem and 
please include human readable descriptions of things like commits and
issues being discussed in e-mail in your mails, this makes them much
easier for humans to read especially when they have no internet access.
I do frequently catch up on my mail on flights or while otherwise
travelling so this is even more pressing for me than just being about
making things a bit easier to read.

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

#1333072

On Friday 12 February 2016 09:30:17 Stephen Boyd wrote:
> This reverts commit 18560a4e3b07438113b50589e78532d95f907029.
> 
> The commit that caused us to specify LE device endianness here,
> 29bb45f25ff3 (regmap-mmio: Use native endianness for read/write,
> 2015-10-29), has been reverted in mainline so now when we specify
> LE it actively breaks big endian kernels because the byte
> swapping in regmap-mmio is incorrect. Let's revert this change
> because it will 1) fix the big endian kernels and 2) be redundant
> to specify LE because that will become the default soon.
> 
> Cc: Kenneth Westfield <kwestfie@codeaurora.org>
> Cc: Kevin Hilman <khilman@linaro.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>

Ah, too bad to have to revert a correct change until the infrastructure
is fixed properly, but I guess it's the easier way out here.

What about the other uses of REGMAP_ENDIAN_LITTLE in the kernel?
In particular I see 

drivers/clk/nxp/clk-lpc32xx.c:  .val_format_endian = REGMAP_ENDIAN_LITTLE,

drivers/clk/qcom/gcc-apq8084.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
drivers/clk/qcom/gcc-ipq806x.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
drivers/clk/qcom/gcc-msm8660.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
drivers/clk/qcom/gcc-msm8916.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
drivers/clk/qcom/gcc-msm8960.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
drivers/clk/qcom/gcc-msm8960.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
drivers/clk/qcom/gcc-msm8974.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
drivers/clk/qcom/lcc-ipq806x.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
drivers/clk/qcom/lcc-msm8960.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
drivers/clk/qcom/mmcc-apq8084.c:        .val_format_endian = REGMAP_ENDIAN_LITTLE,
drivers/clk/qcom/mmcc-msm8960.c:        .val_format_endian = REGMAP_ENDIAN_LITTLE,
drivers/clk/qcom/mmcc-msm8960.c:        .val_format_endian = REGMAP_ENDIAN_LITTLE,
drivers/clk/qcom/mmcc-msm8974.c:        .val_format_endian = REGMAP_ENDIAN_LITTLE,

drivers/nvmem/qfprom.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,

and of course

drivers/mfd/syscon.c:   syscon_config.val_format_endian = REGMAP_ENDIAN_LITTLE;

which all look like they are regmap_mmio users as well. Do they 
suffer from the same problem?

	Arnd

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

#1333076

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

On Fri, Feb 12, 2016 at 08:48:58PM +0100, Arnd Bergmann wrote:

> which all look like they are regmap_mmio users as well. Do they 
> suffer from the same problem?

Probably, yes.  It's only a problem if they're in systems that might
otherwise run big endian successfully.

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

#1333088

On Friday 12 February 2016 20:03:11 Mark Brown wrote:
> On Fri, Feb 12, 2016 at 08:48:58PM +0100, Arnd Bergmann wrote:
> 
> > which all look like they are regmap_mmio users as well. Do they 
> > suffer from the same problem?
> 
> Probably, yes.  It's only a problem if they're in systems that might
> otherwise run big endian successfully.
> 

The affected clock drivers seem to include all the qcom platforms,
so I assumed that whatever platform uses sound/soc/qcom/lpass-cpu.c
has to use one of those as well. Same thing for drivers/nvmem/qfprom.c.

lpc32xx probably won't support big-endian.

syscon is probably fine because it only sets that flag based on the
"little-endian" DT property and I don't see anything setting that
(except the one platform that did so incorrectly).

	Arnd

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

#1333140

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

On Fri, Feb 12, 2016 at 09:35:26PM +0100, Arnd Bergmann wrote:

> syscon is probably fine because it only sets that flag based on the
> "little-endian" DT property and I don't see anything setting that
> (except the one platform that did so incorrectly).

Though it should be able to remove that as the regmap code will also
parse the property.  Probably worth double checking that works as
intended though.

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

#1333168

On 02/12, Arnd Bergmann wrote:
> 
> drivers/clk/qcom/gcc-apq8084.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
> drivers/clk/qcom/gcc-ipq806x.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
> drivers/clk/qcom/gcc-msm8660.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
> drivers/clk/qcom/gcc-msm8916.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
> drivers/clk/qcom/gcc-msm8960.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
> drivers/clk/qcom/gcc-msm8960.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
> drivers/clk/qcom/gcc-msm8974.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
> drivers/clk/qcom/lcc-ipq806x.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
> drivers/clk/qcom/lcc-msm8960.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,
> drivers/clk/qcom/mmcc-apq8084.c:        .val_format_endian = REGMAP_ENDIAN_LITTLE,
> drivers/clk/qcom/mmcc-msm8960.c:        .val_format_endian = REGMAP_ENDIAN_LITTLE,
> drivers/clk/qcom/mmcc-msm8960.c:        .val_format_endian = REGMAP_ENDIAN_LITTLE,
> drivers/clk/qcom/mmcc-msm8974.c:        .val_format_endian = REGMAP_ENDIAN_LITTLE,

These are taken care of.

> 
> drivers/nvmem/qfprom.c: .val_format_endian = REGMAP_ENDIAN_LITTLE,

I don't plan on reverting this one because it's only in -next. It
is redundant, but it's not actively breaking anything when the
driver tree and the regmap tree are merged together.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

#1333139

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

On Fri, Feb 12, 2016 at 09:30:17AM -0800, Stephen Boyd wrote:
> This reverts commit 18560a4e3b07438113b50589e78532d95f907029.
> 
> The commit that caused us to specify LE device endianness here,
> 29bb45f25ff3 (regmap-mmio: Use native endianness for read/write,
> 2015-10-29), has been reverted in mainline so now when we specify

So, I applied this to -next as that's where it applies but it seems that
you're trying to revert a commit that's in Linus' tree so should go as a
fix to him.  Can you send a fix against Linus' tree too please?

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

#1333167

On 02/12, Mark Brown wrote:
> On Fri, Feb 12, 2016 at 09:30:17AM -0800, Stephen Boyd wrote:
> > This reverts commit 18560a4e3b07438113b50589e78532d95f907029.
> > 
> > The commit that caused us to specify LE device endianness here,
> > 29bb45f25ff3 (regmap-mmio: Use native endianness for read/write,
> > 2015-10-29), has been reverted in mainline so now when we specify
> 
> So, I applied this to -next as that's where it applies but it seems that
> you're trying to revert a commit that's in Linus' tree so should go as a
> fix to him.  Can you send a fix against Linus' tree too please?

Did you want me to send the fix directly to Linus? The patch
applies to both -next and Linus' tree, although you're right I
generated this patch against -next. If I apply it to Linus' tree
and then git format-patch it's exactly the same except for the
commit hash:

$ diff linus next
1c1
< From 4f7c654bfcd159b3068b01e3b522e8af88069cb9 Mon Sep 17 00:00:00 2001
---
> From 798212b523668d5a37a977c660554827aa0d89ed Mon Sep 17 00:00:00 2001

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

#1333177

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

On Fri, Feb 12, 2016 at 02:04:54PM -0800, Stephen Boyd wrote:
> On 02/12, Mark Brown wrote:

> > So, I applied this to -next as that's where it applies but it seems that
> > you're trying to revert a commit that's in Linus' tree so should go as a
> > fix to him.  Can you send a fix against Linus' tree too please?

> Did you want me to send the fix directly to Linus? The patch
> applies to both -next and Linus' tree, although you're right I
> generated this patch against -next. If I apply it to Linus' tree
> and then git format-patch it's exactly the same except for the
> commit hash:

No, of course not!  Send it to me.  What I'm looking for is something
that I can apply - it doesn't apply to my current Qualcomm fixes branch.  
Looking again I see that the issue is that that branch is based on
v4.4, I pulled it over now.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web