| Message ID | 1455298217-15744-1-git-send-email-sboyd@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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
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.
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
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?
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.
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
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.
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.
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)
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(-)