On 4/25/22 11:28 AM, Jernej Škrabec wrote:
> 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@xxxxxxxxx>
>>>
>>> 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.
Yes, agreed, except for VCC-PL, which (as the comments in several devicetrees
note) will cause a circular dependency when loading drivers.
>> 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 .
All of the ports without a separate VCC-Px input are powered by VCC-IO, which in
this case is supplied from DCDC1.
Regards,
Samuel
> 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@xxxxxxxxxxxx>
>>>
>>>> ---
>>>> 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 = <®_dcdc1>;
>>>> + vcc-pg-supply = <®_aldo2>;
>>>> +};
>>>> +
>>>> &pwm {
>>>> status = "okay";
>>>> };
>>>>
>>
>>
>
>
