2016-03-29 19:42:11

by Mark Brown

[permalink] [raw]
Subject: [PATCH 1/2] regmap: Fix implicit inclusion of device.h

internal.h is using dev_name() but doesn't include device.h which
defines it. Add an explicit include to avoid build problems due to
this.

Signed-off-by: Mark Brown <[email protected]>
---
drivers/base/regmap/internal.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 5c79526245c2..a0380338946a 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -13,6 +13,7 @@
#ifndef _REGMAP_INTERNAL_H
#define _REGMAP_INTERNAL_H

+#include <linux/device.h>
#include <linux/regmap.h>
#include <linux/fs.h>
#include <linux/list.h>
--
2.8.0.rc3


2016-03-29 19:42:12

by Mark Brown

[permalink] [raw]
Subject: [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 <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---

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

2016-03-30 10:35:19

by Alexander Stein

[permalink] [raw]
Subject: Re: [PATCH 1/2] regmap: Fix implicit inclusion of device.h

On Tuesday 29 March 2016 12:41:54, Mark Brown wrote:
> internal.h is using dev_name() but doesn't include device.h which
> defines it. Add an explicit include to avoid build problems due to
> this.
>
> Signed-off-by: Mark Brown <[email protected]>
> ---
> drivers/base/regmap/internal.h | 1 +
> 1 file changed, 1 insertion(+)

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

Best regards,
Alexander

2016-03-30 10:36:32

by Alexander Stein

[permalink] [raw]
Subject: Re: [PATCH 2/2] regmap: mmio: Parse endianness definitions from DT

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 <[email protected]>
> Signed-off-by: Mark Brown <[email protected]>
> ---
>
> 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 <[email protected]>

Best rehards,
Alexander

2016-03-31 06:09:18

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] regmap: mmio: Parse endianness definitions from DT

On Wed, Mar 30, 2016 at 4:41 AM, Mark Brown <[email protected]> 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 <[email protected]>
> Signed-off-by: Mark Brown <[email protected]>

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

2016-03-31 06:23:46

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] regmap: mmio: Parse endianness definitions from DT

On Thu, Mar 31, 2016 at 3:09 PM, Krzysztof Kozlowski
<[email protected]> wrote:
> On Wed, Mar 30, 2016 at 4:41 AM, Mark Brown <[email protected]> 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 <[email protected]>
>> Signed-off-by: Mark Brown <[email protected]>
>
> 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

2016-03-31 17:19:46

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 2/2] regmap: mmio: Parse endianness definitions from DT

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.


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