Message-ID: <868085b4-427a-e785-e4eb-942da53a2537@samsung.com>
Date:   Wed, 22 Feb 2023 08:15:28 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0)
        Gecko/20100101 Thunderbird/102.7.2
Subject: Re: [RFC v1 0/4] Simplify regulator supply resolution code by
 offloading to driver core
Content-Language: en-US
To:     Saravana Kannan <saravanak@google.com>
Cc:     Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Sudeep Holla <sudeep.holla@arm.com>,
        Tony Lindgren <tony@atomide.com>,
        Doug Anderson <dianders@chromium.org>,
        Guenter Roeck <linux@roeck-us.net>,
        Luca Weiss <luca.weiss@fairphone.com>, kernel-team@android.com,
        linux-kernel@vger.kernel.org
From:   Marek Szyprowski <m.szyprowski@samsung.com>
In-Reply-To: <CAGETcx-UcVnDw-FJAPeA1mLpPno4OE3AAv4WsfP852zOdKqPCw@mail.gmail.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <CGME20230218083300eucas1p28c7c584877b8914a3b88904690be82f6@eucas1p2.samsung.com>
        <20230218083252.2044423-1-saravanak@google.com>
        <e3814c81-c74d-7087-e87d-12dcb49e6444@samsung.com>
        <CAGETcx-UcVnDw-FJAPeA1mLpPno4OE3AAv4WsfP852zOdKqPCw@mail.gmail.com>
Precedence: bulk

On 21.02.2023 23:36, Saravana Kannan wrote:
> On Mon, Feb 20, 2023 at 1:02 AM Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> On 18.02.2023 09:32, Saravana Kannan wrote:
>>> This series is just an RFC to see if you agree with where this is going.
>>> Please point out bugs, but don't bother with a proper code review.
>>>
>>> The high level idea is to not reimplement what driver core can already
>>> handle for us and use it to do some of the work. Instead of trying to
>>> resolve supplies from all different code paths and bits and pieces of
>>> the tree, we just build it from the root to the leaves by using deferred
>>> probing to sequence things in the right order.
>>>
>>> The last patch is the main one. Rest of them are just setting up for it.
>>>
>>> I believe there's room for further simplification but this is what I
>>> could whip up as a quick first draft that shows the high level idea.
>>> I'll probably need some help with getting a better understanding of why
>>> things are done in a specific order in regulator_register() before I
>>> could attempt simplifying things further.
>>>
>>> Ideally, regulator_register() would just have DT parsing, init data
>>> struct sanity checks and adding the regulator device and then we move
>>> everything else to into the probe function that's guaranteed to run only
>>> after the supply has been resolved/ready to resolve.
>>>
>>> fw_devlink/device links should further optimize the flow and also allow
>>> us to simplify some of the guarantees and address some of the existing
>>> FIXMEs. But this patch series is NOT dependent on fw_devlink or device
>>> links.
>>>
>>> Any thoughts on where this is going?
>>>
>>> I've tested this on one hardware I have and it works and nothing is
>>> broken. But the regulator tree in my hardware isn't that complicated or
>>> deep. The regulators are also added mostly in the right order (due to
>>> existing fw_devlink). So if you agree with the idea, the next step is to
>>> ask people to give it a test.
>>>
>>> Also, it's based on driver-core-next since that's what I had synced up
>>> and had a working baseline. I'll rebase it on the regulator tree when I
>>> go from RFC -> PATCH.
>> I've applied this patchset on top of linux next-20230220 and gave it a
>> try on my test farm, as it revealed a few issues related to regulator
>> initialization in the past. It looks that handling of some corner cases
>> is missing, because this patchset introduced a regression on Samsung
>> Snow/Peach-Pit/Peach-Pi Chromebooks, as well as Hardkernel's Odroid-M1
>> board. It looks that the issue is common - PHY devices don't probe
>> properly. This is an output from Odroid-M1 board
>> (arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts):
>>
>> # cat /sys/kernel/debug/devices_deferred 2>/dev/null
>> fd8c0000.usb    platform: wait for supplier host-port
>> fe830000.phy
>> fe8a0000.usb2phy        rockchip-usb2phy: failed to create phy
>> fe8b0000.usb2phy        rockchip-usb2phy: failed to create phy
>> 3c0800000.pcie  rockchip-dw-pcie: failed to get vpcie3v3 regulator
>> fcc00000.usb    platform: wait for supplier otg-port
>> fd000000.usb    platform: wait for supplier host-port
>> fd800000.usb    platform: wait for supplier otg-port
>> fd840000.usb    platform: wait for supplier otg-port
>> fd880000.usb    platform: wait for supplier host-port
>> fe820000.phy
>>
>> If you need any additional tests on the mentioned boards, let me know.
> Thanks for testing it Marek! I don't want people to spend more time
> testing this before I hear Mark/Liam's thoughts. So, let's hold off
> for now.
>
> I took a peek at the dts and the logs above. If you go into
> /sys/bus/regulator/devices/, I'd expect all of them to have probed
> (they'll have a "driver" symlink in their folder). Or at least the
> regulator tree used by the phys.

Nope, something is wrong there with those PHY regulators:

# cd /sys/bus/regulator/devices

# ls -l regulator.4
lrwxrwxrwx 1 root root 0 Feb 14  2019 regulator.4 -> 
../../../devices/platform/vcc5v0-usb-host-regulator/regulator.4

# ls -l regulator.4/driver
ls: cannot access 'regulator.4/driver': No such file or directory

# ls -l regulator.4/..
total 0
lrwxrwxrwx 1 root root    0 Feb 22 07:06 consumer:platform:fe820000.phy 
-> 
../../virtual/devlink/platform:vcc5v0-usb-host-regulator--platform:fe820000.phy
lrwxrwxrwx 1 root root    0 Feb 22 07:06 
consumer:platform:fe8a0000.usb2phy -> 
../../virtual/devlink/platform:vcc5v0-usb-host-regulator--platform:fe8a0000.usb2phy
lrwxrwxrwx 1 root root    0 Feb 22 07:06 
consumer:platform:fe8b0000.usb2phy -> 
../../virtual/devlink/platform:vcc5v0-usb-host-regulator--platform:fe8b0000.usb2phy
lrwxrwxrwx 1 root root    0 Feb 22 07:06 driver -> 
../../../bus/platform/drivers/reg-fixed-voltage
-rw-r--r-- 1 root root 4096 Feb 22 07:06 driver_override
-r--r--r-- 1 root root 4096 Feb 22 07:06 modalias
lrwxrwxrwx 1 root root    0 Feb 22 07:06 of_node -> 
../../../firmware/devicetree/base/vcc5v0-usb-host-regulator
drwxr-xr-x 2 root root    0 Feb 22 07:06 power
drwxr-xr-x 3 root root    0 Feb 14  2019 regulator.4
lrwxrwxrwx 1 root root    0 Feb 14  2019 subsystem -> ../../../bus/platform
lrwxrwxrwx 1 root root    0 Feb 22 07:06 supplier:platform:fdd60000.gpio 
-> 
../../virtual/devlink/platform:fdd60000.gpio--platform:vcc5v0-usb-host-regulator
lrwxrwxrwx 1 root root    0 Feb 22 07:06 supplier:platform:pinctrl -> 
../../virtual/devlink/platform:pinctrl--platform:vcc5v0-usb-host-regulator
lrwxrwxrwx 1 root root    0 Feb 22 07:06 
supplier:platform:vcc5v0-sys-regulator -> 
../../virtual/devlink/platform:vcc5v0-sys-regulator--platform:vcc5v0-usb-host-regulator
-rw-r--r-- 1 root root 4096 Feb 14  2019 uevent

# ls -l regulator.5
lrwxrwxrwx 1 root root 0 Feb 14  2019 regulator.5 -> 
../../../devices/platform/vcc5v0-usb-otg-regulator/regulator.5

# ls -l regulator.5/driver
ls: cannot access 'regulator.5/driver': No such file or directory

# ls -l regulator.5/..
total 0
lrwxrwxrwx 1 root root    0 Feb 22 07:09 consumer:platform:fe830000.phy 
-> 
../../virtual/devlink/platform:vcc5v0-usb-otg-regulator--platform:fe830000.phy
lrwxrwxrwx 1 root root    0 Feb 22 07:09 
consumer:platform:fe8a0000.usb2phy -> 
../../virtual/devlink/platform:vcc5v0-usb-otg-regulator--platform:fe8a0000.usb2phy
lrwxrwxrwx 1 root root    0 Feb 22 07:07 driver -> 
../../../bus/platform/drivers/reg-fixed-voltage
-rw-r--r-- 1 root root 4096 Feb 22 07:09 driver_override
-r--r--r-- 1 root root 4096 Feb 22 07:09 modalias
lrwxrwxrwx 1 root root    0 Feb 22 07:09 of_node -> 
../../../firmware/devicetree/base/vcc5v0-usb-otg-regulator
drwxr-xr-x 2 root root    0 Feb 22 07:09 power
drwxr-xr-x 3 root root    0 Feb 14  2019 regulator.5
lrwxrwxrwx 1 root root    0 Feb 14  2019 subsystem -> ../../../bus/platform
lrwxrwxrwx 1 root root    0 Feb 22 07:09 supplier:platform:fdd60000.gpio 
-> 
../../virtual/devlink/platform:fdd60000.gpio--platform:vcc5v0-usb-otg-regulator
lrwxrwxrwx 1 root root    0 Feb 22 07:09 supplier:platform:pinctrl -> 
../../virtual/devlink/platform:pinctrl--platform:vcc5v0-usb-otg-regulator
lrwxrwxrwx 1 root root    0 Feb 22 07:09 
supplier:platform:vcc5v0-sys-regulator -> 
../../virtual/devlink/platform:vcc5v0-sys-regulator--platform:vcc5v0-usb-otg-regulator
-rw-r--r-- 1 root root 4096 Feb 14  2019 uevent

# ls -l regulator.23
lrwxrwxrwx 1 root root 0 Feb 14  2019 regulator.23 -> 
../../../devices/platform/vcc3v3-pcie-regulator/regulator.23

# ls -l regulator.23/driver
ls: cannot access 'regulator.23/driver': No such file or directory

# ls -l regulator.23/..
total 0
lrwxrwxrwx 1 root root    0 Feb 22 07:10 
consumer:platform:3c0800000.pcie -> 
../../virtual/devlink/platform:vcc3v3-pcie-regulator--platform:3c0800000.pcie
lrwxrwxrwx 1 root root    0 Feb 22 07:07 driver -> 
../../../bus/platform/drivers/reg-fixed-voltage
-rw-r--r-- 1 root root 4096 Feb 22 07:10 driver_override
-r--r--r-- 1 root root 4096 Feb 22 07:10 modalias
lrwxrwxrwx 1 root root    0 Feb 22 07:10 of_node -> 
../../../firmware/devicetree/base/vcc3v3-pcie-regulator
drwxr-xr-x 2 root root    0 Feb 22 07:10 power
drwxr-xr-x 3 root root    0 Feb 14  2019 regulator.23
lrwxrwxrwx 1 root root    0 Feb 14  2019 subsystem -> ../../../bus/platform
lrwxrwxrwx 1 root root    0 Feb 22 07:10 supplier:platform:fe770000.gpio 
-> 
../../virtual/devlink/platform:fe770000.gpio--platform:vcc3v3-pcie-regulator
lrwxrwxrwx 1 root root    0 Feb 22 07:10 supplier:platform:pinctrl -> 
../../virtual/devlink/platform:pinctrl--platform:vcc3v3-pcie-regulator
lrwxrwxrwx 1 root root    0 Feb 22 07:10 
supplier:platform:vcc3v3-sys-regulator -> 
../../virtual/devlink/platform:vcc3v3-sys-regulator--platform:vcc3v3-pcie-regulator
-rw-r--r-- 1 root root 4096 Feb 14  2019 uevent


However other fixed regulator devices have been probed properly:

# ls -l regulator.3/driver
lrwxrwxrwx 1 root root 0 Feb 22 07:10 regulator.3/driver -> 
../../../../bus/regulator/drivers/regulator_drv
# ls -l regulator.3/..
total 0
lrwxrwxrwx 1 root root    0 Feb 22 07:10 
consumer:platform:vcc5v0-usb-host-regulator -> 
../../virtual/devlink/platform:vcc5v0-sys-regulator--platform:vcc5v0-usb-host-regulator
lrwxrwxrwx 1 root root    0 Feb 22 07:10 
consumer:platform:vcc5v0-usb-otg-regulator -> 
../../virtual/devlink/platform:vcc5v0-sys-regulator--platform:vcc5v0-usb-otg-regulator
lrwxrwxrwx 1 root root    0 Feb 22 07:10 driver -> 
../../../bus/platform/drivers/reg-fixed-voltage
-rw-r--r-- 1 root root 4096 Feb 22 07:10 driver_override
-r--r--r-- 1 root root 4096 Feb 22 07:10 modalias
lrwxrwxrwx 1 root root    0 Feb 22 07:10 of_node -> 
../../../firmware/devicetree/base/vcc5v0-sys-regulator
drwxr-xr-x 2 root root    0 Feb 22 07:10 power
drwxr-xr-x 3 root root    0 Feb 14  2019 regulator.3
lrwxrwxrwx 1 root root    0 Feb 14  2019 subsystem -> ../../../bus/platform
lrwxrwxrwx 1 root root    0 Feb 22 07:10 
supplier:platform:dc-12v-regulator -> 
../../virtual/devlink/platform:dc-12v-regulator--platform:vcc5v0-sys-regulator
-rw-r--r-- 1 root root 4096 Feb 14  2019 uevent

# ls -l regulator.2/driver
lrwxrwxrwx 1 root root 0 Feb 22 07:13 regulator.2/driver -> 
../../../../bus/regulator/drivers/regulator_drv
# ls -l regulator.2/..
total 0
lrwxrwxrwx 1 root root    0 Feb 22 07:13 consumer:i2c:3-001c -> 
../../virtual/devlink/platform:vcc3v3-sys-regulator--i2c:3-001c
lrwxrwxrwx 1 root root    0 Feb 22 07:13 consumer:i2c:3-0020 -> 
../../virtual/devlink/platform:vcc3v3-sys-regulator--i2c:3-0020
lrwxrwxrwx 1 root root    0 Feb 22 07:13 
consumer:platform:fe2a0000.ethernet -> 
../../virtual/devlink/platform:vcc3v3-sys-regulator--platform:fe2a0000.ethernet
lrwxrwxrwx 1 root root    0 Feb 22 07:13 
consumer:platform:vcc3v3-pcie-regulator -> 
../../virtual/devlink/platform:vcc3v3-sys-regulator--platform:vcc3v3-pcie-regulator
lrwxrwxrwx 1 root root    0 Feb 22 07:13 driver -> 
../../../bus/platform/drivers/reg-fixed-voltage
-rw-r--r-- 1 root root 4096 Feb 22 07:13 driver_override
-r--r--r-- 1 root root 4096 Feb 22 07:13 modalias
lrwxrwxrwx 1 root root    0 Feb 22 07:13 of_node -> 
../../../firmware/devicetree/base/vcc3v3-sys-regulator
drwxr-xr-x 2 root root    0 Feb 22 07:13 power
drwxr-xr-x 3 root root    0 Feb 14  2019 regulator.2
lrwxrwxrwx 1 root root    0 Feb 14  2019 subsystem -> ../../../bus/platform
lrwxrwxrwx 1 root root    0 Feb 22 07:13 
supplier:platform:dc-12v-regulator -> 
../../virtual/devlink/platform:dc-12v-regulator--platform:vcc3v3-sys-regulator
-rw-r--r-- 1 root root 4096 Feb 14  2019 uevent

# ls -l regulator.1/driver
lrwxrwxrwx 1 root root 0 Feb 22 07:14 regulator.1/driver -> 
../../../../bus/regulator/drivers/regulator_drv
# ls -l regulator.1/..
total 0
lrwxrwxrwx 1 root root    0 Feb 22 07:14 
consumer:platform:vcc3v3-sys-regulator -> 
../../virtual/devlink/platform:dc-12v-regulator--platform:vcc3v3-sys-regulator
lrwxrwxrwx 1 root root    0 Feb 22 07:14 
consumer:platform:vcc5v0-sys-regulator -> 
../../virtual/devlink/platform:dc-12v-regulator--platform:vcc5v0-sys-regulator
lrwxrwxrwx 1 root root    0 Feb 22 07:14 driver -> 
../../../bus/platform/drivers/reg-fixed-voltage
-rw-r--r-- 1 root root 4096 Feb 22 07:14 driver_override
-r--r--r-- 1 root root 4096 Feb 22 07:14 modalias
lrwxrwxrwx 1 root root    0 Feb 22 07:14 of_node -> 
../../../firmware/devicetree/base/dc-12v-regulator
drwxr-xr-x 2 root root    0 Feb 22 07:14 power
drwxr-xr-x 3 root root    0 Feb 14  2019 regulator.1
lrwxrwxrwx 1 root root    0 Feb 14  2019 subsystem -> ../../../bus/platform
-rw-r--r-- 1 root root 4096 Feb 14  2019 uevent


I hope the above logs will help identifying the issue.


Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland