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

Groups > linux.kernel > #1366591 > unrolled thread

[PATCH 2/2] regmap: mmio: Parse endianness definitions from DT

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/2] regmap: mmio: Parse endianness definitions from DT Mark Brown <broonie@kernel.org> - 2016-03-29 21:50 +0200
    Re: [PATCH 2/2] regmap: mmio: Parse endianness definitions from DT Alexander Stein <alexander.stein@systec-electronic.com> - 2016-03-30 12:40 +0200
    Re: [PATCH 2/2] regmap: mmio: Parse endianness definitions from DT Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-03-31 08:10 +0200
      Re: [PATCH 2/2] regmap: mmio: Parse endianness definitions from DT Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-03-31 08:30 +0200
      Re: [PATCH 2/2] regmap: mmio: Parse endianness definitions from DT Mark Brown <broonie@kernel.org> - 2016-03-31 19:20 +0200

#1366591 — [PATCH 2/2] regmap: mmio: Parse endianness definitions from DT

Since we changed to do formatting in the bus we now skip all the format
parsing that the core does for its data marshalling code.  This means
that we skip the DT parsing it does which breaks some systems, we need
to add an explict call in the MMIO code to do this.

Reported-by: Alexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---

Completely untested, I had been hoping you'd be able to write and test
something yourself.

 drivers/base/regmap/regmap-mmio.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regmap-mmio.c b/drivers/base/regmap/regmap-mmio.c
index b27573c69af7..7132a662c80d 100644
--- a/drivers/base/regmap/regmap-mmio.c
+++ b/drivers/base/regmap/regmap-mmio.c
@@ -23,6 +23,8 @@
 #include <linux/regmap.h>
 #include <linux/slab.h>
 
+#include "internal.h"
+
 struct regmap_mmio_context {
 	void __iomem *regs;
 	unsigned val_bytes;
@@ -245,7 +247,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->val_format_endian) {
+	switch (regmap_get_val_endian(dev, &regmap_mmio, config)) {
 	case REGMAP_ENDIAN_DEFAULT:
 	case REGMAP_ENDIAN_LITTLE:
 #ifdef __LITTLE_ENDIAN
-- 
2.8.0.rc3

[toc] | [next] | [standalone]

#1367089

On Tuesday 29 March 2016 12:41:55, Mark Brown wrote:
> Since we changed to do formatting in the bus we now skip all the format
> parsing that the core does for its data marshalling code.  This means
> that we skip the DT parsing it does which breaks some systems, we need
> to add an explict call in the MMIO code to do this.
> 
> Reported-by: Alexander Stein <alexander.stein@systec-electronic.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> 
> Completely untested, I had been hoping you'd be able to write and test
> something yourself.
> 
>  drivers/base/regmap/regmap-mmio.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

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

Best rehards,
Alexander

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

#1367830

On Wed, Mar 30, 2016 at 4:41 AM, Mark Brown <broonie@kernel.org> wrote:
> Since we changed to do formatting in the bus we now skip all the format
> parsing that the core does for its data marshalling code.  This means
> that we skip the DT parsing it does which breaks some systems, we need
> to add an explict call in the MMIO code to do this.
>
> Reported-by: Alexander Stein <alexander.stein@systec-electronic.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>

This breaks my syscon reboot handler (Exynos4412, Trats2):
###############
-sh-4.1# reboot
Rebooting.
[   37.056210] reboot: Restarting system
[   38.058649] Unable to restart system
[   39.060987] Reboot failed -- System halted
###############

I added a printk before switch and:
config->val_format_endian: REGMAP_ENDIAN_DEFAULT
regmap_get_val_endian(): REGMAP_ENDIAN_BIG,

so this not entirely backward compatible... but of course maybe my DT
config needs some fixes...

Below bisect status:
# bad: [3fdbce4cc3e9f3f3c59046b96d3f5ac72b37ba22] Add linux-next
specific files for 20160331
# good: [62f444e0548eb503b42c8447675b468f5cf40c69] Merge branch
'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
git bisect start 'HEAD' 'next/stable'
# good: [5db649ad8967952294445dc9006078a305f9ac88] Merge
remote-tracking branch 'nand/nand/next'
git bisect good 5db649ad8967952294445dc9006078a305f9ac88
# bad: [7936113e7237d075597aac5171b11dfe4f6c49a3] Merge
remote-tracking branch 'usb-chipidea-next/ci-for-usb-next'
git bisect bad 7936113e7237d075597aac5171b11dfe4f6c49a3
# bad: [6d1f8fb2e0ed9f85e5121b021d19b9e135fbea54] Merge
remote-tracking branch 'sound/for-next'
git bisect bad 6d1f8fb2e0ed9f85e5121b021d19b9e135fbea54
# good: [432f856d8b4963f2f1d95844794266d7c9a9ca97] drm/i915: Split out
load time interface registration
git bisect good 432f856d8b4963f2f1d95844794266d7c9a9ca97
# good: [a16b7658f4e0d4aec9bc3e75a5f0cc3f7a3a0422] drm/i915: Call
intel_dp_mst_resume() before resuming displays
git bisect good a16b7658f4e0d4aec9bc3e75a5f0cc3f7a3a0422
# good: [0b7c2ce88d7c26623df086a46afb966805992662] Merge
remote-tracking branch 'hdlcd/for-upstream/hdlcd'
git bisect good 0b7c2ce88d7c26623df086a46afb966805992662
# bad: [d5f9fa5b25846a620ce1893e659e137b48325bef] Merge
remote-tracking branch 'regmap/for-next'
git bisect bad d5f9fa5b25846a620ce1893e659e137b48325bef
# good: [2468fc97cc4555c37fa491379c98561b30c739f3] Merge branch
'kbuild/kbuild' into kbuild/for-next
git bisect good 2468fc97cc4555c37fa491379c98561b30c739f3
# bad: [51a1944f6b46b61bd0e2a58410ff91a5f6c9361c] Merge
remote-tracking branches 'regmap/fix/be' and 'regmap/fix/doc' into
regmap-linus
git bisect bad 51a1944f6b46b61bd0e2a58410ff91a5f6c9361c
# good: [4f7d6dd4df8b388e2056c89b528254cdd79dea2a] regmap: Fix
implicit inclusion of device.h
git bisect good 4f7d6dd4df8b388e2056c89b528254cdd79dea2a
# good: [2596e07a3ed5a5f4d8b89be316c2b704d6f5dc5f] regmap: fix
documentation to match code
git bisect good 2596e07a3ed5a5f4d8b89be316c2b704d6f5dc5f
# bad: [0dbdb76c0ca8e7caf27c9a210f64c4359e2974a4] regmap: mmio: Parse
endianness definitions from DT
git bisect bad 0dbdb76c0ca8e7caf27c9a210f64c4359e2974a4
# first bad commit: [0dbdb76c0ca8e7caf27c9a210f64c4359e2974a4] regmap:
mmio: Parse endianness definitions from DT

Best regards,
Krzysztof

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

#1367841

On Thu, Mar 31, 2016 at 3:09 PM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> On Wed, Mar 30, 2016 at 4:41 AM, Mark Brown <broonie@kernel.org> wrote:
>> Since we changed to do formatting in the bus we now skip all the format
>> parsing that the core does for its data marshalling code.  This means
>> that we skip the DT parsing it does which breaks some systems, we need
>> to add an explict call in the MMIO code to do this.
>>
>> Reported-by: Alexander Stein <alexander.stein@systec-electronic.com>
>> Signed-off-by: Mark Brown <broonie@kernel.org>
>
> This breaks my syscon reboot handler (Exynos4412, Trats2):
> ###############
> -sh-4.1# reboot
> Rebooting.
> [   37.056210] reboot: Restarting system
> [   38.058649] Unable to restart system
> [   39.060987] Reboot failed -- System halted
> ###############
>
> I added a printk before switch and:
> config->val_format_endian: REGMAP_ENDIAN_DEFAULT
> regmap_get_val_endian(): REGMAP_ENDIAN_BIG,
>

... and the big-endian is coming from the last return in
regmap_get_val_endian() (/* Use this if no other value was found */).
I don't have endian property in DTS for the syscon device so it always
ended with default which in the regmap_mmio_gen_context() was mapped
to little endian. Now default is big endian.

I guess the big/little endian property should be required in that case.

Best regards,
Krzysztof

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

#1368461

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

On Thu, Mar 31, 2016 at 03:09:16PM +0900, Krzysztof Kozlowski wrote:

> This breaks my syscon reboot handler (Exynos4412, Trats2):

Please point people to device trees, not everyone is going to be
familiar with your particular board.

> I added a printk before switch and:
> config->val_format_endian: REGMAP_ENDIAN_DEFAULT
> regmap_get_val_endian(): REGMAP_ENDIAN_BIG,

I've pushed up a fix which makes little endian the default for MMIO,
please check again.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web