Re: [PATCH 6/7] arm64: dts: broadcom: bcm2712: Add second SDHCI controller node

["Ivan T. Ivanov" <iivanov@suse.de>]

Hi,

I am sorry so long delayed answer. Shifting priorities.

On 08-02 21:01, Stefan Wahren wrote:
> 
> Hi,
> 
> [add Arend and Raspberry Pi devs]
> 
> Am 31.07.24 um 08:28 schrieb Ivan T. Ivanov:
> > Add SDIO2 node. On RPi5 it is connected to WiFi chip.
> > Add related pin, gpio and regulator definitions and
> > add WiFi node. With this and firmware already provided by
> > distributions, at least on openSUSE Tumbleweed, this is
> > sufficient to make WiFi operational on RPi5 \o/.
> > 
> > Signed-off-by: Ivan T. Ivanov <iivanov@suse.de>
> > ---
> >   .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     | 55 +++++++++++++++++++
> >   arch/arm64/boot/dts/broadcom/bcm2712.dtsi     | 13 +++++
> >   2 files changed, 68 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
> > index 06e926af16b7..b6bfe0abb774 100644
> > --- a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
> > +++ b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
> > @@ -46,6 +46,20 @@ sd_vcc_reg: sd-vcc-reg {
> >   		gpios = <&gio_aon 4 GPIO_ACTIVE_HIGH>;
> >   	};
> > 
> > +	wl_on_reg: wl-on-reg {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "wl-on-regulator";
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +		pinctrl-0 = <&wl_on_pins>;
> > +		pinctrl-names = "default";
> > +
> > +		gpio = <&gio 28 GPIO_ACTIVE_HIGH>;
> > +
> > +		startup-delay-us = <150000>;
> > +		enable-active-high;
> > +	};
> I don't think this GPIO is a regulator from hardware perspective. I
> guess it's the same reset pin we have on the older Raspberry Pis. Please
> look at bcm283x-rpi-wifi-bt.dtsi for the wifi power sequence.

FWICS, "mmc-pwrseq-simple" is more about proper reset sequence while
here we need power to be applied to the connected device.

And this is not a precedent. Just grep for "vmmc-supply = <&wlan"

> > +
> >   	pwr-button {
> >   		compatible = "gpio-keys";
> > 
> > @@ -80,6 +94,25 @@ &sdio1 {
> >   	cd-gpios = <&gio_aon 5 GPIO_ACTIVE_LOW>;
> >   };
> > 
> > +/* SDIO2 drives the WLAN interface */
> > +&sdio2 {
> > +	pinctrl-0 = <&sdio2_30_pins>;
> > +	pinctrl-names = "default";
> > +	bus-width = <4>;
> > +	vmmc-supply = <&wl_on_reg>;
> > +	sd-uhs-ddr50;
> > +	non-removable;
> > +	status = "okay";
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +
> > +	wifi: wifi@1 {
> > +		reg = <1>;
> > +		compatible = "brcm,bcm4329-fmac";
> > +		local-mac-address = [00 00 00 00 00 00];
> I think we can drop this?

Sure.

Regards,
Ivan