diff mbox series

arm64: dts: rockchip: add BT/wifi nodes to Pinephone Pro

Message ID 20220906124713.1683587-1-tom@tom-fitzhenry.me.uk (mailing list archive)
State New, archived
Headers show
Series arm64: dts: rockchip: add BT/wifi nodes to Pinephone Pro | expand

Commit Message

Tom Fitzhenry Sept. 6, 2022, 12:47 p.m. UTC
Pinephone Pro includes a AzureWave AW-CM256SM wifi (sdio0) and
bt (uart0) combo module, which is based on Cypress
CYP43455 (BCM43455).

Signed-off-by: Tom Fitzhenry <tom@tom-fitzhenry.me.uk>
---
 .../dts/rockchip/rk3399-pinephone-pro.dts     | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)

Comments

Ondřej Jirman Sept. 6, 2022, 1:35 p.m. UTC | #1
Hello Tom,

On Tue, Sep 06, 2022 at 10:47:13PM +1000, Tom Fitzhenry wrote:
> Pinephone Pro includes a AzureWave AW-CM256SM wifi (sdio0) and
> bt (uart0) combo module, which is based on Cypress
> CYP43455 (BCM43455).
> 
> Signed-off-by: Tom Fitzhenry <tom@tom-fitzhenry.me.uk>
> ---
>  .../dts/rockchip/rk3399-pinephone-pro.dts     | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> index 2e058c3150256..096238126e4c1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> @@ -43,6 +43,20 @@ key-power {
>  		};
>  	};
>  
> +	/* 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.

> +		/* WL_REG_ON on module */
> +		reset-gpios = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
> +	};
> +
>  	vcc_sys: vcc-sys-regulator {
>  		compatible = "regulator-fixed";
>  		regulator-name = "vcc_sys";
> @@ -360,11 +374,31 @@ vsel2_pin: vsel2-pin {
>  		};
>  	};
>  
> +	sdio-pwrseq {
> +		wifi_enable_h_pin: wifi-enable-h-pin {
> +			rockchip,pins = <0 RK_PB2 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +	};
> +
>  	sound {
>  		vcc1v8_codec_en: vcc1v8-codec-en {
>  			rockchip,pins = <3 RK_PA4 RK_FUNC_GPIO &pcfg_pull_down>;
>  		};
>  	};
> +
> +	wireless-bluetooth {
> +		bt_wake_pin: bt-wake-pin {
> +			rockchip,pins = <2 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +
> +		bt_host_wake_pin: bt-host-wake-pin {
> +			rockchip,pins = <0 RK_PA4 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +
> +		bt_reset_pin: bt-reset-pin {
> +			rockchip,pins = <0 RK_PB1 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +	};
>  };
>  
>  &sdmmc {

see below

> @@ -380,6 +414,20 @@ &sdmmc {
>  	status = "okay";
>  };
>  
> +&sdio0 {

sd'i'o0 comes before 'm' in the alphabet.

> +	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.

Other than that,

Reviewed-by: Ondřej Jirman <megi@xff.cz>

Thank you and kind regards,
	o.
Caleb Connolly Sept. 6, 2022, 5:38 p.m. UTC | #2
On 06/09/2022 13:47, Tom Fitzhenry wrote:
> Pinephone Pro includes a AzureWave AW-CM256SM wifi (sdio0) and
> bt (uart0) combo module, which is based on Cypress
> CYP43455 (BCM43455).
> 
> Signed-off-by: Tom Fitzhenry <tom@tom-fitzhenry.me.uk>
> ---
>   .../dts/rockchip/rk3399-pinephone-pro.dts     | 69 +++++++++++++++++++
>   1 file changed, 69 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> index 2e058c3150256..096238126e4c1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> @@ -43,6 +43,20 @@ key-power {
>   		};
>   	};
> 
> +	/* Power sequence for SDIO WiFi module */

This comment isn't needed, instead give the node a better name/label
> +	sdio_pwrseq: sdio-pwrseq {

	wifi_pwrseq: sdio-pwrseq-wifi {
> +		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>;
> +
> +		/* WL_REG_ON on module */
> +		reset-gpios = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
> +	};
> +
>   	vcc_sys: vcc-sys-regulator {
>   		compatible = "regulator-fixed";
>   		regulator-name = "vcc_sys";
> @@ -360,11 +374,31 @@ vsel2_pin: vsel2-pin {
>   		};
>   	};
> 
> +	sdio-pwrseq {
> +		wifi_enable_h_pin: wifi-enable-h-pin {
> +			rockchip,pins = <0 RK_PB2 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +	};
> +
>   	sound {
>   		vcc1v8_codec_en: vcc1v8-codec-en {
>   			rockchip,pins = <3 RK_PA4 RK_FUNC_GPIO &pcfg_pull_down>;
>   		};
>   	};
> +
> +	wireless-bluetooth {
> +		bt_wake_pin: bt-wake-pin {
> +			rockchip,pins = <2 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +
> +		bt_host_wake_pin: bt-host-wake-pin {
> +			rockchip,pins = <0 RK_PA4 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +
> +		bt_reset_pin: bt-reset-pin {
> +			rockchip,pins = <0 RK_PB1 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +	};
>   };
> 
>   &sdmmc {
> @@ -380,6 +414,20 @@ &sdmmc {
>   	status = "okay";
>   };
> 
> +&sdio0 {
> +	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";
> +};
> +
>   &sdhci {
>   	bus-width = <8>;
>   	mmc-hs200-1_8v;
> @@ -393,6 +441,27 @@ &tsadc {
>   	status = "okay";
>   };
> 
> +&uart0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart0_xfer &uart0_cts &uart0_rts>;
> +	uart-has-rtscts;
> +	status = "okay";
> +
> +	bluetooth {
> +		compatible = "brcm,bcm4345c5";
> +		clocks = <&rk818 1>;
> +		clock-names = "lpo";
> +		device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
> +		host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
> +		max-speed = <1500000>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&bt_host_wake_pin &bt_wake_pin &bt_reset_pin>;
> +		shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
> +		vbat-supply = <&vcc3v3_sys>;
> +		vddio-supply = <&vcc_1v8>;
> +	};
> +};
> +
>   &uart2 {
>   	status = "okay";
>   };
> --
> 2.37.1
>
Heiko Stübner Sept. 7, 2022, 8:26 a.m. UTC | #3
Am Dienstag, 6. September 2022, 19:38:38 CEST schrieb Caleb Connolly:
> 
> On 06/09/2022 13:47, Tom Fitzhenry wrote:
> > Pinephone Pro includes a AzureWave AW-CM256SM wifi (sdio0) and
> > bt (uart0) combo module, which is based on Cypress
> > CYP43455 (BCM43455).
> > 
> > Signed-off-by: Tom Fitzhenry <tom@tom-fitzhenry.me.uk>
> > ---
> >   .../dts/rockchip/rk3399-pinephone-pro.dts     | 69 +++++++++++++++++++
> >   1 file changed, 69 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > index 2e058c3150256..096238126e4c1 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > @@ -43,6 +43,20 @@ key-power {
> >   		};
> >   	};
> > 
> > +	/* Power sequence for SDIO WiFi module */
> 
> This comment isn't needed, instead give the node a better name/label
> > +	sdio_pwrseq: sdio-pwrseq {
> 
> 	wifi_pwrseq: sdio-pwrseq-wifi {

I guess, I'd move the components around a tiny bit and go with

	wifi_pwrseq: sdio-wifi-pwrseq {

So far everywhere the "-pwrseq" is at the end and while I don't
think that this is enforced (yet), keeping some sort of consistency
might be nice :-) 


Heiko


> > +		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>;
> > +
> > +		/* WL_REG_ON on module */
> > +		reset-gpios = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
> > +	};
> > +
> >   	vcc_sys: vcc-sys-regulator {
> >   		compatible = "regulator-fixed";
> >   		regulator-name = "vcc_sys";
> > @@ -360,11 +374,31 @@ vsel2_pin: vsel2-pin {
> >   		};
> >   	};
> > 
> > +	sdio-pwrseq {
> > +		wifi_enable_h_pin: wifi-enable-h-pin {
> > +			rockchip,pins = <0 RK_PB2 RK_FUNC_GPIO &pcfg_pull_none>;
> > +		};
> > +	};
> > +
> >   	sound {
> >   		vcc1v8_codec_en: vcc1v8-codec-en {
> >   			rockchip,pins = <3 RK_PA4 RK_FUNC_GPIO &pcfg_pull_down>;
> >   		};
> >   	};
> > +
> > +	wireless-bluetooth {
> > +		bt_wake_pin: bt-wake-pin {
> > +			rockchip,pins = <2 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;
> > +		};
> > +
> > +		bt_host_wake_pin: bt-host-wake-pin {
> > +			rockchip,pins = <0 RK_PA4 RK_FUNC_GPIO &pcfg_pull_none>;
> > +		};
> > +
> > +		bt_reset_pin: bt-reset-pin {
> > +			rockchip,pins = <0 RK_PB1 RK_FUNC_GPIO &pcfg_pull_none>;
> > +		};
> > +	};
> >   };
> > 
> >   &sdmmc {
> > @@ -380,6 +414,20 @@ &sdmmc {
> >   	status = "okay";
> >   };
> > 
> > +&sdio0 {
> > +	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";
> > +};
> > +
> >   &sdhci {
> >   	bus-width = <8>;
> >   	mmc-hs200-1_8v;
> > @@ -393,6 +441,27 @@ &tsadc {
> >   	status = "okay";
> >   };
> > 
> > +&uart0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart0_xfer &uart0_cts &uart0_rts>;
> > +	uart-has-rtscts;
> > +	status = "okay";
> > +
> > +	bluetooth {
> > +		compatible = "brcm,bcm4345c5";
> > +		clocks = <&rk818 1>;
> > +		clock-names = "lpo";
> > +		device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
> > +		host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
> > +		max-speed = <1500000>;
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&bt_host_wake_pin &bt_wake_pin &bt_reset_pin>;
> > +		shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
> > +		vbat-supply = <&vcc3v3_sys>;
> > +		vddio-supply = <&vcc_1v8>;
> > +	};
> > +};
> > +
> >   &uart2 {
> >   	status = "okay";
> >   };
> > --
> > 2.37.1
> > 
>
Tom Fitzhenry Oct. 2, 2022, 9:33 a.m. UTC | #4
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
Tom Fitzhenry Oct. 2, 2022, 9:35 a.m. UTC | #5
Hi Caleb and Heiko,

Thank you for your reviews.

On 7/9/22 18:26, Heiko Stübner wrote:
>>> +	/* Power sequence for SDIO WiFi module */
>>
>> This comment isn't needed, instead give the node a better name/label
>>> +	sdio_pwrseq: sdio-pwrseq {
>>
>> 	wifi_pwrseq: sdio-pwrseq-wifi {
> 
> I guess, I'd move the components around a tiny bit and go with
> 
> 	wifi_pwrseq: sdio-wifi-pwrseq {
> 
> So far everywhere the "-pwrseq" is at the end and while I don't
> think that this is enforced (yet), keeping some sort of consistency
> might be nice :-)

Done. I have applied this in v2.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
index 2e058c3150256..096238126e4c1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
@@ -43,6 +43,20 @@  key-power {
 		};
 	};
 
+	/* 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>;
+
+		/* WL_REG_ON on module */
+		reset-gpios = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
+	};
+
 	vcc_sys: vcc-sys-regulator {
 		compatible = "regulator-fixed";
 		regulator-name = "vcc_sys";
@@ -360,11 +374,31 @@  vsel2_pin: vsel2-pin {
 		};
 	};
 
+	sdio-pwrseq {
+		wifi_enable_h_pin: wifi-enable-h-pin {
+			rockchip,pins = <0 RK_PB2 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
 	sound {
 		vcc1v8_codec_en: vcc1v8-codec-en {
 			rockchip,pins = <3 RK_PA4 RK_FUNC_GPIO &pcfg_pull_down>;
 		};
 	};
+
+	wireless-bluetooth {
+		bt_wake_pin: bt-wake-pin {
+			rockchip,pins = <2 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+
+		bt_host_wake_pin: bt-host-wake-pin {
+			rockchip,pins = <0 RK_PA4 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+
+		bt_reset_pin: bt-reset-pin {
+			rockchip,pins = <0 RK_PB1 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
 };
 
 &sdmmc {
@@ -380,6 +414,20 @@  &sdmmc {
 	status = "okay";
 };
 
+&sdio0 {
+	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";
+};
+
 &sdhci {
 	bus-width = <8>;
 	mmc-hs200-1_8v;
@@ -393,6 +441,27 @@  &tsadc {
 	status = "okay";
 };
 
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_xfer &uart0_cts &uart0_rts>;
+	uart-has-rtscts;
+	status = "okay";
+
+	bluetooth {
+		compatible = "brcm,bcm4345c5";
+		clocks = <&rk818 1>;
+		clock-names = "lpo";
+		device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
+		host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
+		max-speed = <1500000>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&bt_host_wake_pin &bt_wake_pin &bt_reset_pin>;
+		shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
+		vbat-supply = <&vcc3v3_sys>;
+		vddio-supply = <&vcc_1v8>;
+	};
+};
+
 &uart2 {
 	status = "okay";
 };