public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Marek Szyprowski <m.szyprowski@samsung.com>
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
Subject: Re: [RFC v1 0/4] Simplify regulator supply resolution code by offloading to driver core
Date: Wed, 22 Feb 2023 08:15:28 +0100	[thread overview]
Message-ID: <868085b4-427a-e785-e4eb-942da53a2537@samsung.com> (raw)
In-Reply-To: <CAGETcx-UcVnDw-FJAPeA1mLpPno4OE3AAv4WsfP852zOdKqPCw@mail.gmail.com>

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


      parent reply	other threads:[~2023-02-22  7:15 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20230218083300eucas1p28c7c584877b8914a3b88904690be82f6@eucas1p2.samsung.com>
2023-02-18  8:32 ` [RFC v1 0/4] Simplify regulator supply resolution code by offloading to driver core Saravana Kannan
2023-02-18  8:32   ` [RFC v1 1/4] regulator: core: Add regulator devices to bus instead of class Saravana Kannan
2023-02-18  8:32   ` [RFC v1 2/4] regulator: core: Add sysfs class backward compatibility Saravana Kannan
2023-02-22 17:47     ` Mark Brown
2023-02-18  8:32   ` [RFC v1 3/4] regulator: core: Probe regulator devices Saravana Kannan
2023-02-22 17:50     ` Mark Brown
2023-02-18  8:32   ` [RFC v1 4/4] regulator: core: Move regulator supply resolving to the probe function Saravana Kannan
2023-02-22 22:51     ` Mark Brown
2023-02-18  8:36   ` [RFC v1 0/4] Simplify regulator supply resolution code by offloading to driver core Saravana Kannan
2023-02-18  8:54   ` Greg Kroah-Hartman
2023-02-20  9:01   ` Marek Szyprowski
2023-02-21 22:36     ` Saravana Kannan
2023-02-21 22:52       ` Mark Brown
2023-02-22  3:13         ` Saravana Kannan
2023-02-22 14:54           ` Mark Brown
2023-02-22  7:15       ` Marek Szyprowski [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=868085b4-427a-e785-e4eb-942da53a2537@samsung.com \
    --to=m.szyprowski@samsung.com \
    --cc=andersson@kernel.org \
    --cc=broonie@kernel.org \
    --cc=dianders@chromium.org \
    --cc=geert@linux-m68k.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel-team@android.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=luca.weiss@fairphone.com \
    --cc=saravanak@google.com \
    --cc=sudeep.holla@arm.com \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox