| Message ID | 20240824-b4-fix-nanopineoplus2-pio-regs-v1-1-7c5f7da445af@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | arm64: dts: sunxi: nanopi-neo-plus2: Add pio regulators | expand |
On 24/08/2024 09:09, Kryštof Černý wrote: > The board does not have a dedicated regulator for pio and r_pio, > but this fixes the kernel warning about dummy regulators being used. > Tested on the actual board. > Judging by commit msg these are not correct regulators. Please do not add incorrect hardware description to silence some warnings coming from OS. Either you need proper (correct) hardware description or fix the problem other way, assuming there is anything to fix in the first place. Best regards, Krzysztof
I am sorry if the message is wrong, this is my first patch ever sent to the Linux kernel. I have checked the schematic of the board and it shares the same power line with mmc0, so I assumed I can use the same regulator. Thanks for your feedback and I would be glad for your further response. Dne 24. 08. 24 v 9:40 Krzysztof Kozlowski napsal(a): > On 24/08/2024 09:09, Kryštof Černý wrote: >> The board does not have a dedicated regulator for pio and r_pio, >> but this fixes the kernel warning about dummy regulators being used. >> Tested on the actual board. >> > Judging by commit msg these are not correct regulators. Please do not > add incorrect hardware description to silence some warnings coming from > OS. Either you need proper (correct) hardware description or fix the > problem other way, assuming there is anything to fix in the first place. > > Best regards, > Krzysztof >
Hi, On Sat, Aug 24, 2024 at 5:08 PM Kryštof Černý <cleverline1mc@gmail.com> wrote: > > I am sorry if the message is wrong, this is my first patch ever sent to > the Linux kernel. I have checked the schematic of the board and it > shares the same power line with mmc0, so I assumed I can use the same > regulator. Thanks for your feedback and I would be glad for your further > response. So some of the pin groups do have dedicated supplies, and should thus be described, but not all of them. The schematic only shows dedicated supplies for PD and PG pingroups. So just add those. PD supply is from 2.5V ethernet PHY I/O regulator supply, so you would need to add that as well. The datasheet also mentions a separate supply pin for PC pingroup, but it is not shown in the schematic. I would just omit that. And as Krzysztof mentioned, device tree changes should be to model the hardware, not to work around some operating system warning. At least most of the time that is. So your commit message should also be about fixing the description or providing more detail, and not about the operating system. ChenYu > Dne 24. 08. 24 v 9:40 Krzysztof Kozlowski napsal(a): > > On 24/08/2024 09:09, Kryštof Černý wrote: > >> The board does not have a dedicated regulator for pio and r_pio, > >> but this fixes the kernel warning about dummy regulators being used. > >> Tested on the actual board. > >> > > Judging by commit msg these are not correct regulators. Please do not > > add incorrect hardware description to silence some warnings coming from > > OS. Either you need proper (correct) hardware description or fix the > > problem other way, assuming there is anything to fix in the first place. > > > > Best regards, > > Krzysztof > >
Yes, you are right with vcc-pd, I misunderstood it, thank you. Let me explain and ask about the rest: VDD_SYS_3.3V = reg_vcc3v3 Groups PA, PE, PF are powered from vcc-io, according to Allwinner H5 datasheet, Vcc-io is connected to VDD_SYS_3.3V, just like mmc0. Should those be set or omitted? vcc-pc (ball G15), which is labeled as vcc_io2 (probably wrong), which is also connected to VDD_SYS_3.3V. vcc-pd (ball J15) is bonded to VDD_PHY_2.5V, which is always on and made from VDD_SYS_3.3V, so I should make a new fixed regulator of name "reg_gmac_2v5"? Mainline eth driver does not implement this pin, so it would be used only for the pio. vcc-pg (ball H7) is also VDD_SYS_3.3V. While PL is supplied from vcc-rtc (vcc_rtc, ball K6), it is connected directly to the VDD_SYS_3.3V too, should this be the same regulator or a new one or omitted too? Do you agree with my findings? I hope they are 100% now. If so, I will try to make V2 with a new fixed regulator of 2.5V for the PD. Many thanks for your replies, I will do better next time. Dne 24. 08. 24 v 14:34 Chen-Yu Tsai napsal(a): > Hi, > > On Sat, Aug 24, 2024 at 5:08 PM Kryštof Černý <cleverline1mc@gmail.com> wrote: >> I am sorry if the message is wrong, this is my first patch ever sent to >> the Linux kernel. I have checked the schematic of the board and it >> shares the same power line with mmc0, so I assumed I can use the same >> regulator. Thanks for your feedback and I would be glad for your further >> response. > So some of the pin groups do have dedicated supplies, and should thus be > described, but not all of them. The schematic only shows dedicated > supplies for PD and PG pingroups. So just add those. PD supply is from > 2.5V ethernet PHY I/O regulator supply, so you would need to add that > as well. > > The datasheet also mentions a separate supply pin for PC pingroup, but > it is not shown in the schematic. I would just omit that. > > And as Krzysztof mentioned, device tree changes should be to model > the hardware, not to work around some operating system warning. At > least most of the time that is. So your commit message should also > be about fixing the description or providing more detail, and not > about the operating system. > > > ChenYu > >> Dne 24. 08. 24 v 9:40 Krzysztof Kozlowski napsal(a): >>> On 24/08/2024 09:09, Kryštof Černý wrote: >>>> The board does not have a dedicated regulator for pio and r_pio, >>>> but this fixes the kernel warning about dummy regulators being used. >>>> Tested on the actual board. >>>> >>> Judging by commit msg these are not correct regulators. Please do not >>> add incorrect hardware description to silence some warnings coming from >>> OS. Either you need proper (correct) hardware description or fix the >>> problem other way, assuming there is anything to fix in the first place. >>> >>> Best regards, >>> Krzysztof >>>
On Sat, 24 Aug 2024 16:24:41 +0200 Kryštof Černý <cleverline1mc@gmail.com> wrote: Hi Kryštof, many thanks for taking care of those warnings and sending a patch, that's the right way of fixing things and I wish more people would actually do that! I also checked the schematics and the H5 datasheet, so: > Yes, you are right with vcc-pd, I misunderstood it, thank you. Let me > explain and ask about the rest: > VDD_SYS_3.3V = reg_vcc3v3 > Groups PA, PE, PF are powered from vcc-io, according to Allwinner H5 > datasheet, Vcc-io is connected to VDD_SYS_3.3V, just like mmc0. Should > those be set or omitted? Yes, you can set those supplies to reg_vcc3v3. It seems like most of the boards's I/O (expect PD) is actually 3.3V, driven from that one discrete regulator, which is correctly described as reg_vcc3v3. This isn't very clear in the DT (it looks like a "dummy" regulator), which is what probably triggered Krzysztof's comment. To make this more obvious, please change the regulator description as seen in sun50i-h618-longanpi-3h.dts: There should be one 5V regulator, as the external power input, from the MicroUSB port. Every other regulator should be "chained" to that, via the vin-supply property. And add a comment mentioning that it's a discrete regulator, maybe even mentioning the chip name (SY8089A). > vcc-pc (ball G15), which is labeled as vcc_io2 (probably wrong), which > is also connected to VDD_SYS_3.3V. As Chen-Yu mentioned, VCC-PC is not mentioned in the schematics, but it must be 3.3V: there is a pull-up for PC7, to 3.3V, also there is no 1.8V regulator, so the eMMC must work with 3.3V, supported by the missing vqmmc-supply property for mmc2. > vcc-pd (ball J15) is bonded to VDD_PHY_2.5V, which is always on and made > from VDD_SYS_3.3V, so I should make a new fixed regulator of name > "reg_gmac_2v5"? Mainline eth driver does not implement this pin, so it > would be used only for the pio. Yes, please have another "regulator-fixed", feeding of reg_vcc3v3, and mention it's a discrete RT9193. > vcc-pg (ball H7) is also VDD_SYS_3.3V. Yes, that's the same reg_vcc3v3. > While PL is supplied from vcc-rtc (vcc_rtc, ball K6), it is connected > directly to the VDD_SYS_3.3V too, should this be the same regulator or a > new one or omitted too? Since they are directly wired together, it's indeed the same reg_vcc3v3 regulator. > Do you agree with my findings? I hope they are 100% now. If so, I will > try to make V2 with a new fixed regulator of 2.5V for the PD. > > Many thanks for your replies, I will do better next time. No worries, except for the VCC-PD at 2.5V this was actually all correct. So well done, especially for a first try! Cheers, Andre > > Dne 24. 08. 24 v 14:34 Chen-Yu Tsai napsal(a): > > Hi, > > > > On Sat, Aug 24, 2024 at 5:08 PM Kryštof Černý <cleverline1mc@gmail.com> wrote: > >> I am sorry if the message is wrong, this is my first patch ever sent to > >> the Linux kernel. I have checked the schematic of the board and it > >> shares the same power line with mmc0, so I assumed I can use the same > >> regulator. Thanks for your feedback and I would be glad for your further > >> response. > > So some of the pin groups do have dedicated supplies, and should thus be > > described, but not all of them. The schematic only shows dedicated > > supplies for PD and PG pingroups. So just add those. PD supply is from > > 2.5V ethernet PHY I/O regulator supply, so you would need to add that > > as well. > > > > The datasheet also mentions a separate supply pin for PC pingroup, but > > it is not shown in the schematic. I would just omit that. > > > > And as Krzysztof mentioned, device tree changes should be to model > > the hardware, not to work around some operating system warning. At > > least most of the time that is. So your commit message should also > > be about fixing the description or providing more detail, and not > > about the operating system. > > > > > > ChenYu > > > >> Dne 24. 08. 24 v 9:40 Krzysztof Kozlowski napsal(a): > >>> On 24/08/2024 09:09, Kryštof Černý wrote: > >>>> The board does not have a dedicated regulator for pio and r_pio, > >>>> but this fixes the kernel warning about dummy regulators being used. > >>>> Tested on the actual board. > >>>> > >>> Judging by commit msg these are not correct regulators. Please do not > >>> add incorrect hardware description to silence some warnings coming from > >>> OS. Either you need proper (correct) hardware description or fix the > >>> problem other way, assuming there is anything to fix in the first place. > >>> > >>> Best regards, > >>> Krzysztof > >>> >
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dts index b69032c44557..2841c9a8aa50 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dts @@ -146,6 +146,18 @@ &ohci3 { status = "okay"; }; +&pio { + vcc-pa-supply = <®_vcc3v3>; + vcc-pc-supply = <®_vcc3v3>; + vcc-pd-supply = <®_vcc3v3>; + vcc-pf-supply = <®_vcc3v3>; + vcc-pg-supply = <®_vcc3v3>; +}; + +&r_pio { + vcc-pl-supply = <®_vcc3v3>; +}; + &uart0 { pinctrl-names = "default"; pinctrl-0 = <&uart0_pa_pins>;
The board does not have a dedicated regulator for pio and r_pio, but this fixes the kernel warning about dummy regulators being used. Tested on the actual board. Signed-off-by: Kryštof Černý <cleverline1mc@gmail.com> --- arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dts | 12 ++++++++++++ 1 file changed, 12 insertions(+) --- base-commit: c3f38fa61af77b49866b006939479069cd451173 change-id: 20240824-b4-fix-nanopineoplus2-pio-regs-b54335d8a9e4 Best regards,