Re: [PATCH] arm64: dts: rockchip: add BT/wifi nodes to Pinephone Pro

[Tom Fitzhenry]

Hi Ondřej,

Thanks for the review.

On 6/9/22 23:35, Ondřej Jirman wrote:
> > +	/* Power sequence for SDIO WiFi module */
> > +	sdio_pwrseq: sdio-pwrseq {
> > +		compatible = "mmc-pwrseq-simple";
> > +		clocks = <&rk818 1>;
> > +		clock-names = "ext_clock";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&wifi_enable_h_pin>;
> > +		post-power-on-delay-ms = <100>;
> > +		power-off-delay-us = <500000>;
>
> Do we really need such long delays? Almost no boards in rockchip/ use such
> delays at all, and if they do they don't usually use power off delay.

I have checked the datasheet, and updated the delays accordingly with 
explanatory comments. This is applied in v2.

> >   &sdmmc {
>
> see below
>
> > @@ -380,6 +414,20 @@ &sdmmc {
> >   	status = "okay";
> >   };
> >   
> > +&sdio0 {
>
> sd'i'o0 comes before 'm' in the alphabet.

Done. :)

> > +	bus-width = <4>;
> > +	cap-sd-highspeed;
> > +	cap-sdio-irq;
> > +	disable-wp;
> > +	keep-power-in-suspend;
> > +	mmc-pwrseq = <&sdio_pwrseq>;
> > +	non-removable;
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&sdio0_bus4 &sdio0_cmd &sdio0_clk>;
> > +	sd-uhs-sdr104;
> > +	status = "okay";
>
> It might also be good to add the wifi node, and hookup the interrupt line and
> pinctrls, so that WoW works, while you're at it.
>
> See eg. https://elixir.bootlin.com/linux/v5.19.7/source/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4b-plus.dts#L30
>
> Looks like WIFI_HOST_WAKE_L is hooked to GPIO4_D0/PCIE_CLKREQnB_u according
> to the schematic. Let's hope GPIO4_D will consider 1.8V as high, because SoC
> GPIO4_D is in 3.0V domain and VDDIO of wifi chip is 1.8V.

As discussed off-list but included here for posterity, I'll leave this 
out of the DT for now, until we know the GPIO that the firmware is 
expecting.

Regards,
Tom