Re: Re: [PATCH] arm64: dts: allwinner: teres-i: Add GPIO port regulators

["Jernej Škrabec" <jernej.skrabec@gmail.com>]

Hi Harald!

Dne ponedeljek, 25. april 2022 ob 13:01:54 CEST je Harald Geyer napisal(a):
> On 24.04.2022 03:56, Samuel Holland wrote:
> > On 4/15/22 11:56 AM, Harald Geyer wrote:
> >> Allwinner A64 SoC has separate supplies for PC, PD, PE, PG and PL.
> >>
> >> Usually supplies are linked via the 'regulator-name' property of
> >> regulator nodes. However when regulators are shared we need to
> >> declare the additional links in the pinctrl node.
> >>
> >> Signed-off-by: Harald Geyer <harald@ccbib.org>
> >
> > I'm curious if this solved an issue for you, or if this is just for 
> > accuracy.
> > Both of these regulators have the regulator-always-on property, so
> > they should have been enabled already.
> 
> You are right, there shouldn't be any change in functionality. It is 
> mostly
> for extra correctness. However the pincontrol driver started spewing 
> lot's
> of warnings about missing regulator nodes a few versions back. The 
> visible
> effect of this change is to silence those warnings. Also make the DTS 
> more
> future proof in case the driver is made even more picky in the future.
> 
> > If it's the latter reason, why not add the other
> > ports? Regardless:
> 
> PD, PE and PL have dedicated regulators, that can be matched via the
> 'regulator-name' property. I didn't want to specify the same 
> information
> in two places.

"regulator-name" is only a label, while phandle is actual regulator reference 
that can be used by the driver. While DT files reside in Linux kernel source, 
they are used by other OSes and bootloaders, so you can't really assume what 
is good or not just by judging based on Linux behaviour. So please add PD and 
PL regulators too.

> 
> For the PF supply, I couldn't find any connection information in the
> board schematic. I could have added a dummy regulator. But since there 
> is
> only one warning about pf-supply during driver initialization and not 
> the
> dozens of warnings I see about PC and PG, I figured, I'd rather not add
> information of dubious use or qualiy.

You mean PE right? There is no PF supply on A64. Anyway, if it's not on 
schematic, it can be assumed unconnected and thus you shouldn't define that 
property. Messages like "using dummy regulator" are fine in such cases .

There is no issue of "dubious quality" if schematic is clear. Also don't worry 
about usefulness. DT files are hardware description files. They should reflect 
hardware configuration, no matter how useful information seems.

FYI, information in this case is useful to the driver. If you check sunxi 
pinctrl driver, you can see that port bias is set according to regulator 
voltage.

Best regards,
Jernej

> 
> best regards,
> Harald
> 
> 
> > Reviewed-by: Samuel Holland <samuel@sholland.org>
> >
> >> ---
> >>  arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts 
> >> b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> >> index aff0660b899c..cc316ef2e2d6 100644
> >> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> >> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> >> @@ -197,6 +197,11 @@ &ohci1 {
> >>  	status = "okay";
> >>  };
> >>
> >> +&pio {
> >> +	vcc-pc-supply = <&reg_dcdc1>;
> >> +	vcc-pg-supply = <&reg_aldo2>;
> >> +};
> >> +
> >>  &pwm {
> >>  	status = "okay";
> >>  };
> >>
> 
>