Message-id: <1722883.ZQ0ZDYoSn5@amdc3058>
Date:   Wed, 30 May 2018 15:44:08 +0200
From:   Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Mark Brown <broonie@...nel.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Sangbeom Kim <sbkim73@...sung.com>,
        Chanwoo Choi <cw00.choi@...sung.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>
Subject: Re: [PATCH 10/19 v3] regulator: s2mps11: Pass descriptor instead of
 GPIO number

On Monday, May 28, 2018 01:29:07 PM Bartlomiej Zolnierkiewicz wrote:
> 
> Hi Linus,
> 
> On Monday, May 28, 2018 10:41:31 AM Linus Walleij wrote:
> > On Sat, May 26, 2018 at 12:02 PM, Mark Brown <broonie@...nel.org> wrote:
> > > On Mon, May 14, 2018 at 10:06:31AM +0200, Linus Walleij wrote:
> > >> Instead of passing a global GPIO number for the enable GPIO, pass
> > >> a descriptor looked up with the standard devm_gpiod_get_optional()
> > >> call.
> > >>
> > >> This regulator supports passing platform data, but enable/sleep
> > >> regulators are looked up from the device tree exclusively, so
> > >> we can need not touch other files.
> > >
> > > This seems to have broken the boot on Odroid XU3 so I'm going to revert
> > > it:
> > >
> > > https://storage.kernelci.org/next/master/v4.17-rc6-9523-g47b9cef0672d/arm/multi_v7_defconfig/lab-baylibre-seattle/boot-exynos5422-odroidxu3.html
> > 
> > How annoying. I will check with a colleague who might have this
> > board so I can test it on hardware.
> 
> I've reproduced the problem on TM2e board and the patch below fixes
> it (old code always initialized the structure, new code does it only
> in case GPIO properties are provided).
> 
> I've also tested the new code (with fixup) on Artik5 board (which
> actually uses GPIO properties) and discovered the other problem,
> the GPIO core code doesn't handle shared GPIOs which are used by
> many platforms.
> 
> Old code:
> [    1.094950] s2mps11-pmic s2mps14-regulator: Using GPIO 21 for ext-control over 10/LDO11
> [    1.095210] s2mps11-pmic s2mps14-regulator: Using GPIO 21 for ext-control over 11/LDO12
> 
> New code (with fixup):
> [    1.114288] s2mps11-pmic s2mps14-regulator: Using GPIO for ext-control over 10/LDO11
> [    1.143209] s2mps11-pmic s2mps14-regulator: Failed to get control GPIO for 11/LDO12
> 
> [ It fails with -EBUSY on gpiod_request() in gpiod_get_from_of_node(). ]
> 
> Therefore it seems that more work is needed before s2mps11 driver
> can be converted to use GPIO descriptors.

The similar issue with shared GPIOs seems to be present in commit
c89c00e2b8f0 ("regulator: max77686: Pass descriptor instead of GPIO
number") and Midas board (arch/arm/boot/dts/exynos4412-midas.dtsi).
I don't have the board itself to test it but from the code audit it
seems that commit c89c00e2b8f0 should also be reverted.

Marek has also discovered separate problem on Trats2 board with
commit 3c6b38d45fa5 ("regulator: wm8994: Pass descriptor instead of
GPIO number"):

[    2.278330] wm8994 4-001a: Failed to get supply 'DBVDD1': -517
[    2.282773] wm8994 4-001a: Failed to get supplies: -517
[    2.291919] ------------[ cut here ]------------
[    2.295210] WARNING: CPU: 3 PID: 1 at drivers/gpio/gpiolib.c:2399 
release_nodes+0x178/0x1fc
[    2.303562] Modules linked in:
[    2.306483] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 
4.17.0-rc7-next-20180530-00001-g06c3288f5beb #811
[    2.315920] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[    2.321975] [<c011313c>] (unwind_backtrace) from [<c010ed20>] 
(show_stack+0x10/0x14)
[    2.329715] [<c010ed20>] (show_stack) from [<c0a3c070>] 
(dump_stack+0x98/0xc4)
[    2.336913] [<c0a3c070>] (dump_stack) from [<c0127a50>] 
(__warn+0x10c/0x124)
[    2.343935] [<c0127a50>] (__warn) from [<c0127b7c>] 
(warn_slowpath_null+0x40/0x48)
[    2.351495] [<c0127b7c>] (warn_slowpath_null) from [<c057e564>] 
(release_nodes+0x178/0x1fc)
[    2.359838] [<c057e564>] (release_nodes) from [<c0579dd8>] 
(driver_probe_device+0xe8/0x4a4)
[    2.368169] [<c0579dd8>] (driver_probe_device) from [<c057a298>] 
(__driver_attach+0x104/0x120)
[    2.376765] [<c057a298>] (__driver_attach) from [<c0577ef8>] 
(bus_for_each_dev+0x68/0xb4)
[    2.384920] [<c0577ef8>] (bus_for_each_dev) from [<c057915c>] 
(bus_add_driver+0x1a8/0x268)
[    2.393166] [<c057915c>] (bus_add_driver) from [<c057b3ec>] 
(driver_register+0x78/0x10c)
[    2.401243] [<c057b3ec>] (driver_register) from [<c06ce0b4>] 
(i2c_register_driver+0x3c/0xa8)
[    2.409664] [<c06ce0b4>] (i2c_register_driver) from [<c0103188>] 
(do_one_initcall+0x8c/0x4b0)
[    2.418172] [<c0103188>] (do_one_initcall) from [<c0f01268>] 
(kernel_init_freeable+0x3e4/0x570)
[    2.426854] [<c0f01268>] (kernel_init_freeable) from [<c0a52418>] 
(kernel_init+0x8/0x10c)
[    2.435003] [<c0a52418>] (kernel_init) from [<c01010b4>] 
(ret_from_fork+0x14/0x20)
[    2.442542] Exception stack(0xef0bbfb0 to 0xef0bbff8)
[    2.447549] bfa0: 00000000 00000000 00000000 00000000
[    2.455745] bfc0: 00000000 00000000 00000000 00000000 00000000 
00000000 00000000 00000000
[    2.463903] bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    2.470568] irq event stamp: 331409
[    2.474034] hardirqs last  enabled at (331493): [<c01935a4>] 
console_unlock+0x4a4/0x660
[    2.481962] hardirqs last disabled at (331500): [<c01931cc>] 
console_unlock+0xcc/0x660
[    2.489923] softirqs last  enabled at (331530): [<c01023e0>] 
__do_softirq+0x2b8/0x650
[    2.497733] softirqs last disabled at (331541): [<c0130068>] 
irq_exit+0x158/0x164
[    2.505190] ---[ end trace 54f70738f2594ecf ]---

[ Reverting the commit in question fixes the issue. ]

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Powered by blists - more mailing lists