RE: [EXT] Re: [PATCH 1/2] dt-bindings: power: Add regulator-pd yaml file

From: Shenwei Wang
Date: Tue Aug 22 2023 - 11:50:30 EST




> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Sent: Tuesday, August 22, 2023 10:26 AM
> To: Shenwei Wang <shenwei.wang@xxxxxxx>; Rob Herring
> <robh+dt@xxxxxxxxxx>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>; Conor Dooley
> <conor+dt@xxxxxxxxxx>; Ulf Hansson <ulf.hansson@xxxxxxxxxx>; Liam Girdwood
> <lgirdwood@xxxxxxxxx>; Mark Brown <broonie@xxxxxxxxxx>;
> imx@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> dl-linux-imx <linux-imx@xxxxxxx>
> Subject: [EXT] Re: [PATCH 1/2] dt-bindings: power: Add regulator-pd yaml file
> >
> > No offend. :) Sorry for my poor word. To provide more context, a
> > common use case example is using a GPIO pin as a power switch. The
> > current implementation operates as a fixed regulator, which makes it
> > difficult to control the on/off timing without modifying its driver.
>
> So it is a problem of a driver?
>

That is arguable too. The driver may assume its power is on when probed, which
aligns with how the PD behaves.

> > It also lacks power management support.
>
> Which is not related to bindings but implementation in given driver.
>

For those simple drivers, the default power management logic can handle
power correctly without requiring any specialized implementation in the
driver code.

> >
> >> The detail that power-domains get handled automatically is an
> >> implementation detail in the kernel (currently). That could easily
> >> change and you'd be in the same position as with regulator supplies.
> >
> > The proposed regulator-pd driver follows the standard PD driver
> > framework, so it for sure relies on certain kernel implementation
> > details. If those underlying implementation details change in the
> > future, this driver as well as other PD drivers built on the same framework
> would need to be updated accordingly.
>
> We talk about bindings which you would not be allowed to change. Thus your
> case would stop working...
>

As a new driver, it has to involve some new bindings especially the compatible
string.

> >
> >> We could just as easily decide to make the driver core turn on all
> >> supplies in a node. That would give you the same "feature". Why would
> >> you design your DT around implementation decisions of the OS?
> >>
> >
> > This DT properties are proposed solely for this specific driver, not
> > to hack the OS. This is no different than other PD drivers like gpc/scu-
> pd/imx93-pd.
>
> I am not sure if you got Rob's point, I have feelings that not. Argument that
> some OS implements something some way, is not an argument for a new
> binding, barely hardware related.
>

Thank you for the clarification. The issue is that this driver is purely a software layer
that wraps the regulators as a power domain. The bindings make the implementation
clean and easy to understand. I don't think we should add extra complex logic inside
the driver solely to avoid introducing new bindings.

Thanks,
Shenwei

> Best regards,
> Krzysztof