| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CABjd4YzxvG6u8g8OjCBSOe6Zddk6Fe-sv+M5-8Si2-=Vw8qHBg@mail.gmail.com>
Date: Wed, 4 Jun 2025 23:48:42 +0400
From: Alexey Charkov <alchark@...il.com>
To: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Heiko Stuebner <heiko@...ech.de>, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] arm64: dts: rockchip: enable wifi on ArmSoM Sige5
On Wed, Jun 4, 2025 at 11:01 PM Nicolas Frattaroli
<nicolas.frattaroli@...labora.com> wrote:
>
> On Tuesday, 3 June 2025 19:01:15 Central European Summer Time Alexey Charkov wrote:
> > ArmSoM Sige5 uses a soldered-on SDIO connected WiFi module. Namely,
> > board v1.1 uses a Realtek based BL-M8852BS2, while v1.2 uses a Broadcom
> > based BW3752-50B1.
>
> Okay, so there's two board revisions, which makes this patch and the
> following that adds bluetooth a problem. This patch here doesn't
> seem to actually add the SDIO Wi-Fi module node where it'll differ,
> so skirts around the issue AFAIU. It might also be that it's not
> needed for you and I just don't notice because I have v1.1 where the
> SDIO driver for that module doesn't exist yet iirc.
The WiFi module actually probes just fine without explicitly listing
it in the device tree, because SDIO is enumerable (unlike
UART/serdev). Pin configs and the lpo clock are pulled in by the
mmc-pwrseq part, and with that it just works as long as the drivers
are in place (which is the case with the Broadcom module, but perhaps
not yet with the Realtek one).
I haven't tested the wake-host functionality, given that there is no
suspend support anyway (no Rockchip SiP communication support in
mainline, and nothing to ask ATF to suspend the main CPU like
downstream rockchip_pm_config.c does). Guess it doesn't work, because
nothing binds the respective GPIO to the WiFi driver right now. But I
do get a WiFi connection without it.
> But what we should think about is first doing
> - probably add a new compatible for armsom,sige5-v1.2
> - git mv arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts \
> arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dtsi
> - commit here like Sebastian Reichel had to do for [1]
> - create a new arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts
> that includes the dtsi and moves the model and compatible property
> there. Make sure the model includes v1.1 in the name, compatible
> should remain the same
> - commit here that will be a squash commit like Sebastian Reichel
> had to do in [2]
I believe another way to do this is with --break-rewrites, as Dragan did in [1]
[1] https://lore.kernel.org/all/9ffedc0e2ca7f167d9d795b2a8f43cb9f56a653b.1717923308.git.dsimic@manjaro.org/
> - create a new arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5-v1.2.dts,
> could be adding the Wi-Fi node here to cut down on how many patches we
> do. I assume you only have the v1.2 board and can't test the older Wi-Fi,
> which is fine.
> compatible should probably be something like
> compatible = "armsom,sige5-v1.2", "armsom,sige5", "rockchip,rk3576";
> Don't forget to change the model property as well.
Given that the user would have to manually feed an appropriate DTB for
their board to the bootloader, and given that the WiFi/BT module used
is the only difference between v1.1 vs. v1.2, this doesn't seem to be
much easier for users vs. applying a DTBO... But more churn in the
kernel just for that bluetooth function. With a DTBO we could keep the
same compatible and just move the Bluetooth node into an overlay.
Happy to go either way. Wonder what Heiko prefers?
> I know this will be a royal PITA, especially considering they'll likely
> never produce v1.1 again and it now hogs the name, but keeping the
> current dts as 1.1 may be the best way forward, as it keeps compat (though
> the Wi-Fi changes won't affect anything already in there) and will stop
> people from accidentally picking the higher number better DTS like what
> happens with rockpro64 all the time where the non-numbered DTS is 2.1 and
> the numbered one is 2.0 and people keep picking 2.0.
Quite a PITA indeed :)
> > Add required device tree nodes in the SoC .dtsi for the SDIO controller
> > and pinctrl / clock wiring in the board .dts for the module itself.
> >
> > Signed-off-by: Alexey Charkov <alchark@...il.com>
> > ---
> > .../boot/dts/rockchip/rk3576-armsom-sige5.dts | 36 ++++++++++++++++++++++
> > arch/arm64/boot/dts/rockchip/rk3576.dtsi | 16 ++++++++++
> > 2 files changed, 52 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts b/arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts
> > index 7ce1fb1380b0863c902fdd9cbc7454ee6011cf92..dcd033859398312f7693bebb7f080ee4f2ecaa32 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts
> > @@ -219,6 +219,15 @@ vcc_5v0_host: regulator-vcc-5v0-host {
> > pinctrl-names = "default";
> > pinctrl-0 = <&usb_host_pwren>;
> > };
> > +
> > + sdio_pwrseq: sdio-pwrseq {
> > + compatible = "mmc-pwrseq-simple";
> > + clocks = <&hym8563>;
> > + clock-names = "ext_clock";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&wifi_reg_on>;
> > + reset-gpios = <&gpio1 RK_PC6 GPIO_ACTIVE_LOW>;
> > + };
> > };
> >
> > &combphy1_psu {
> > @@ -781,6 +790,16 @@ usb_host_pwren: usb-host-pwren {
> > rockchip,pins = <4 RK_PA4 RK_FUNC_GPIO &pcfg_pull_none>;
> > };
> > };
> > +
> > + wireless-wlan {
> > + wifi_wake_host: wifi-wake-host {
> > + rockchip,pins = <0 RK_PB0 RK_FUNC_GPIO &pcfg_pull_down>;
> > + };
> > +
> > + wifi_reg_on: wifi-reg-on {
> > + rockchip,pins = <1 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
> > + };
> > + };
> > };
> >
> > &sai1 {
> > @@ -808,6 +827,23 @@ &sdhci {
> > status = "okay";
> > };
> >
> > +&sdio {
> > + bus-width = <4>;
> > + cap-sdio-irq;
> > + disable-wp;
> > + keep-power-in-suspend;
> > + mmc-pwrseq = <&sdio_pwrseq>;
> > + no-sd;
> > + no-mmc;
> > + non-removable;
> > + sd-uhs-sdr50;
> > + sd-uhs-sdr104;
> > + vmmc-supply = <&vcc_3v3_s3>;
> > + vqmmc-supply = <&vcc_1v8_s3>;
> > + wakeup-source;
> > + status = "okay";
> > +};
> > +
> > &sdmmc {
> > bus-width = <4>;
> > cap-mmc-highspeed;
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
>
> Okay, I think adding the sdio node to the SoC dtsi should be a separate
> patch before this one.
Fair enough, will split it out, thanks!
> > index 1086482f04792325dc4c22fb8ceeb27eef59afe4..a09582470bb7f654b711308da1e51fa8571ca1e8 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> > @@ -1695,6 +1695,22 @@ sdmmc: mmc@...10000 {
> > status = "disabled";
> > };
> >
> > + sdio: mmc@...20000 {
> > + compatible = "rockchip,rk3576-dw-mshc", "rockchip,rk3288-dw-mshc";
> > + reg = <0x0 0x2a320000 0x0 0x4000>;
> > + clocks = <&cru HCLK_SDIO>, <&cru CCLK_SRC_SDIO>;
> > + clock-names = "biu", "ciu";
> > + fifo-depth = <0x100>;
> > + interrupts = <GIC_SPI 252 IRQ_TYPE_LEVEL_HIGH>;
> > + max-frequency = <200000000>;
> > + pinctrl-0 = <&sdmmc1m0_clk &sdmmc1m0_cmd &sdmmc1m0_bus4>;
> > + pinctrl-names = "default";
> > + power-domains = <&power RK3576_PD_SDGMAC>;
> > + resets = <&cru SRST_H_SDIO>;
> > + reset-names = "reset";
> > + status = "disabled";
> > + };
> > +
> > sdhci: mmc@...30000 {
> > compatible = "rockchip,rk3576-dwcmshc", "rockchip,rk3588-dwcmshc";
> > reg = <0x0 0x2a330000 0x0 0x10000>;
> >
> >
>
> So in conclusion:
> - bindings patch adding v1.2 compatible
> - SoC dtsi patch for the sdio node
> - git mv patch
> - rk3576-armsom-sige5.dts patch, ask maintainer to squash it into the
> previous patch, make it obvious by giving it the same subject or something
> - rk3576-armsom-sige5-v1.2.dts patch
>
> Sorry to drop all this on you, it's a little unpleasant and in-the-woods
> with regards to preparing a patch series. Basically, the reason why I've
> held off on this for the moment is that I don't have a v1.2 board and my
> v1.1 board's Wi-Fi module doesn't seem to be supported (yet).
No worries, thanks for delving into all this stuff with me - much appreciated!
Best regards,
Alexey
Powered by blists - more mailing lists