[PATCH v3 2/2] arm64: dts: allwinner: a64: Add WiFi/BT header on SoPine

[Chen-Yu Tsai]

On Sun, Apr 20, 2025 at 10:13 AM Dragan Simic <dsimic at manjaro.org> wrote:
>
> Hello Chen-Yu,
>
> On 2025-04-20 03:59, Chen-Yu Tsai wrote:
> > On Sun, Apr 20, 2025 at 3:57 AM Dragan Simic <dsimic at manjaro.org>
> > wrote:
> >>
> >> On 2025-04-19 21:26, Peter Robinson wrote:
> >> > On Sat, 19 Apr 2025 at 19:25, Dragan Simic <dsimic at manjaro.org> wrote:
> >> >> On 2025-04-19 18:00, Peter Robinson wrote:
> >> >>> This adds all the pin mappings on the WiFi/BT header on
> >> >>> the SoPine baseboard/A64-LTS. They're disabled by default
> >> >>> as the modules don't ship by default. This includes, where
> >> >>> they haven't been already, UART1 for BT and mmc1 for WiFi.
> >> >>
> >> >> The patch subject should be improved a bit, to include
> >> >> "Baseboard" as well.  Having just "SoPine" is a bit too
> >> >> vague, and it should actually be written as "SOPINE"
> >> >> at all places in the prose.
> >> >>
> >> >>> Signed-off-by: Peter Robinson <pbrobinson at gmail.com>
> >> >>> ---
> >> >>> .../allwinner/sun50i-a64-sopine-baseboard.dts | 25
> >> >> +++++++++++++++++++
> >> >>> 1 file changed, 25 insertions(+)
> >> >>>
> >> >>> diff --git
> >> >>> a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> >> >>> b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> >> >>> index be2347c8f267..64d696f110ee 100644
> >> >>> ---
> >> >> a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> >> >>> +++
> >> >> b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> >> >>> @@ -42,6 +42,11 @@ reg_vcc1v8: vcc1v8 {
> >> >>> regulator-min-microvolt = <1800000>;
> >> >>> regulator-max-microvolt = <1800000>;
> >> >>> };
> >> >>> +
> >> >>> +     wifi_pwrseq: pwrseq {
> >> >>> +             compatible = "mmc-pwrseq-simple";
> >> >>> +             reset-gpios = <&r_pio 0 2 GPIO_ACTIVE_LOW>; /* PL2
> >> >> */
> >> >>> +     };
> >> >>
> >> >> Is there a reason why the status of this node isn't set to
> >> >> "disabled"?  Or even better, why don't we move this node
> >> >> entirely into the proposed DT overlay?
> >> >>
> >> >> The required reset procedure actually depends on what's
> >> >> found on the add-on module, so it should belong to the DT
> >> >> overlay that defines the add-on module.
> >> >
> >> > But the pin itself is pinned out through the header on the device.
> >>
> >> Good point.  On second thought, the right approach should be
> >> to reserve the reset pin through pinctrl additions to the
> >> board dts files, but to move the "mmc-pwrseq-simple" nodes
> >> to the proposed add-on overlays.
>
> Actually, s/overlays/overlay/
>
> >>
> >> You can have a look at rk3399-pinebook-pro.dts for an example
> >> of such a pinctrl definition and its use.
> >
> > For sunxi, we don't add pinctrl nodes for GPIO usage.
>
> I see, thanks for the note.  In that case, I'd still suggest
> that the "mmc-pwrseq-simple" nodes are moved to the proposed
> add-on overlay, because that's where they belong to.
>
> In theory, some add-on module may actually not need resetting,
> so defining that should belong to what defines the specific
> add-on module, which is the add-on overlay.

I agree.

> >> >>> };
> >> >>>
> >> >>> &ac_power_supply {
> >> >>> @@ -103,6 +108,18 @@ ext_rgmii_phy: ethernet-phy at 1 {
> >> >>> };
> >> >>> };
> >> >>>
> >> >>> +/* On Wifi/BT connector */
> >> >>> +&mmc1 {
> >> >>> +     pinctrl-names = "default";
> >> >>> +     pinctrl-0 = <&mmc1_pins>;
> >> >>> +     vmmc-supply = <&reg_dldo4>;
> >> >>> +     vqmmc-supply = <&reg_eldo1>;
> >> >>> +     mmc-pwrseq = <&wifi_pwrseq>;
> >> >>
> >> >> Of course, the "mmc-pwrseq" property would then also be moved
> >> >> to the DT overlay that defines the add-on module.
> >> >>
> >> >>> +     bus-width = <4>;
> >> >>> +     non-removable;
> >> >>> +     status = "disabled";
> >> >>> +};
> >> >>> +
> >> >>> &mmc2 {
> >> >>> pinctrl-names = "default";
> >> >>> pinctrl-0 = <&mmc2_pins>;
> >> >>> @@ -175,6 +192,14 @@ &uart0 {
> >> >>> status = "okay";
> >> >>> };
> >> >>>
> >> >>> +/* On Wifi/BT connector, with RTS/CTS */
> >> >>> +&uart1 {
> >> >>> +     pinctrl-names = "default";
> >> >>> +     pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
> >> >>> +     uart-has-rtscts;
> >> >>> +     status = "disabled";
> >> >>> +};
> >> >>> +
> >> >>> /* On Pi-2 connector */
> >> >>> &uart2 {
> >> >>> pinctrl-names = "default";
>