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

[Tom Fitzhenry <tom@tom-fitzhenry.me.uk>]

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