| 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 |
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.
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 >
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 > > >
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
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 --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"; };
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(+)