| Message ID | 20190217121513.22965-3-beagleboard@davidjohnsummers.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: dts: rk3288 Tinker Board (S) updates | expand |
A similar configuration has been in use on Armbian images since kernel 4.14, but not in a clean enough format for submission. I will test this, but the only differences I see compared to ours are combination of the mystery "chip_h" power domain/reset with the Wifi one and the inclusion of sd_uhs_ddr50. -Tony On Sun, Feb 17, 2019 at 7:16 AM David Summers <beagleboard@davidjohnsummers.uk> wrote: > > This patch adds the wifi to the ASUS Tinker Board (S) machines. > > Unfortunatly neither the Tinker Board nor the Tinker Board S schematic > indicate how the WiFi is wired up on these devices. The WiFi is > provided by the RTL8723BS device, that has sdio WiFi and UART > Bluetooth. This patch just adds the WiFi interface. > > With no schematic, most of the wiring has been derived from the ASUS > patch to Debian: > > https://github.com/TinkerBoard/debian_kernel/commit/6a3128ade33f758887048578ada61a4b7ab8e678 > > In conjunction with the pin out of the RTL8723BS device: > > http://files.pine64.org/doc/datasheet/pine64/RTL8723BS.pdf > http://cit.odessa.ua/media/pdf/Intel-Compute-Stick/FN-Link_F23BDSM25-W1.pdf > > The only unusual part is that to bring the card up, both the pins > RK_PD3 and RK_PD4 need to be pulled. Why this needs to be done is not > clear, best explaination is that they are connected to the RTL8723BS > pins WL_DIS# and BT_DIS#, which the data sheet vaguely says: > > Shared with GPIO9 This Pin Can Externally Shutdown the RTL8723BS WLAN > function when BT_DISn is Pulled Low. When this pin deasserted, SDIO > interface will be disabled. This pin can also support the WLAN Ra > dio-off function with host interface remaining connected. > > Anyway extensive testing the TheSaint on ArchLinux Arm Forum > > https://archlinuxarm.org/forum/viewtopic.php?f=44&t=13064&start=120#p60548 > > Signed-off-by: David Summers <beagleboard@davidjohnsummers.uk> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > --- > arch/arm/boot/dts/rk3288-tinker.dtsi | 39 +++++++++++++++++++++++++--- > 1 file changed, 36 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/boot/dts/rk3288-tinker.dtsi b/arch/arm/boot/dts/rk3288-tinker.dtsi > index fceaeed44e34..e1796f340eef 100644 > --- a/arch/arm/boot/dts/rk3288-tinker.dtsi > +++ b/arch/arm/boot/dts/rk3288-tinker.dtsi > @@ -3,8 +3,9 @@ > * Copyright (c) 2017 Fuzhou Rockchip Electronics Co., Ltd. > */ > > #include "rk3288.dtsi" > #include <dt-bindings/input/input.h> > +#include <dt-bindings/clock/rockchip,rk808.h> > > / { > chosen { > @@ -98,6 +97,15 @@ > startup-delay-us = <100000>; > vin-supply = <&vcc_io>; > }; > + > + sdio_pwrseq: sdio-pwrseq { > + compatible = "mmc-pwrseq-simple"; > + clocks = <&rk808 RK808_CLKOUT1>; > + clock-names = "ext_clock"; > + pinctrl-names = "default"; > + pinctrl-0 = <&wifi_enable>; > + reset-gpios = <&gpio4 RK_PD3 GPIO_ACTIVE_LOW>, <&gpio4 RK_PD4 GPIO_ACTIVE_LOW>; > + }; > }; > > &cpu0 { > @@ -337,8 +345,8 @@ > > &io_domains { > status = "okay"; > - > sdcard-supply = <&vccio_sd>; > + wifi-supply = <&vcc_18>; > }; > > &pinctrl { > @@ -417,6 +425,12 @@ > rockchip,pins = <7 8 RK_FUNC_GPIO &pcfg_pull_none>; > }; > }; > + > + sdio { > + wifi_enable: wifi-enable { > + rockchip,pins = <4 RK_PD3 RK_FUNC_GPIO &pcfg_pull_none>, <4 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>; > + }; > + }; > }; > > &pwm0 { > @@ -440,6 +454,25 @@ > vqmmc-supply = <&vccio_sd>; > }; > > +&sdio0 { > + bus-width = <4>; > + cap-sd-highspeed; > + cap-sdio-irq; > + keep-power-in-suspend; > + mmc-pwrseq = <&sdio_pwrseq>; > + non-removable; > + pinctrl-names = "default"; > + pinctrl-0 = <&sdio0_bus4>, <&sdio0_cmd>, <&sdio0_clk>, <&sdio0_int>; > + max-frequency = <50000000>; > + sd-uhs-sdr12; > + sd-uhs-sdr25; > + sd-uhs-sdr50; > + sd-uhs-ddr50; > + vmmc-supply = <&vcc_io>; > + vqmmc-supply = <&vcc_18>; > + status = "okay"; > +}; > + > &tsadc { > rockchip,hw-tshut-mode = <1>; /* tshut mode 0:CRU 1:GPIO */ > rockchip,hw-tshut-polarity = <1>; /* tshut polarity 0:LOW 1:HIGH */ > -- > beagleboard@davidjohnsummers.uk > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
On 17/02/2019 14:32, Tony McKahan wrote: > A similar configuration has been in use on Armbian images since kernel > 4.14, but not in a clean enough format for submission. I will test > this, but the only differences I see compared to ours are combination > of the mystery "chip_h" power domain/reset with the Wifi one and the > inclusion of sd_uhs_ddr50. > > -Tony > > On Sun, Feb 17, 2019 at 7:16 AM David Summers > <beagleboard@davidjohnsummers.uk> wrote: >> This patch adds the wifi to the ASUS Tinker Board (S) machines. >> Tony, thanks for the interest in this and the Bluetooth RFC. To my mind it would be good if we could mainline an agreed solution. As much as anything doesn't make sense to me for Armbian and ArchLinux Arm to have independent solutions. In the ArchLinux Arm camp I'm not a fan of applying many patches to support a board, as much as anything it makes that board non generic - and so has to have its own hand crafted kernel. This to me isn't a good solution, so if possible prefer the pain of getting it mainlined. Stefan Wahren has come up with a minimal patch to hci_h5 for bluetooth, that says the relevant properties can be read from the device tree. This means that acpi is no longer necessary. Thats currently out in testing. Anyway would be good if Arch and Armbian could merge our efforts ... Regards, David.
Hello David, I agree, of course coordination is not always so simple, I did not know Arch didn't have this configured yet. As for patching, I agree, but the Tinker has some... peculiarities, I am a hardware guy, software is a passtime, so my code is not always perhaps up to standard. And I wasn't 100% on submitting changes to the device tree based on a staging driver. Most of my work has been absorbed into https://github.com/Miouyouyou/RockMyy . As for your approach, as I said, the wifi is the same as I have except for the reset/enables and the DDR50, which I didn't know the rtl8723bs supported. BT I have no in-kernel support for other than enabling the RTS pin to the device tree, I handle it in user space for now. I've been watching the hci_h5 conversation, I'll look into testing these changes/options tonight perhaps once I make sure our rockchip-dev recipes are still valid with the newest RC, and can have them committed so others can test. Thank you, -Tony On Sun, Feb 17, 2019 at 10:55 AM David Summers <beagleboard@davidjohnsummers.uk> wrote: > > On 17/02/2019 14:32, Tony McKahan wrote: > > A similar configuration has been in use on Armbian images since kernel > > 4.14, but not in a clean enough format for submission. I will test > > this, but the only differences I see compared to ours are combination > > of the mystery "chip_h" power domain/reset with the Wifi one and the > > inclusion of sd_uhs_ddr50. > > > > -Tony > > > > On Sun, Feb 17, 2019 at 7:16 AM David Summers > > <beagleboard@davidjohnsummers.uk> wrote: > >> This patch adds the wifi to the ASUS Tinker Board (S) machines. > >> > Tony, thanks for the interest in this and the Bluetooth RFC. To my mind > it would be good if we could mainline an agreed solution. As much as > anything doesn't make sense to me for Armbian and ArchLinux Arm to have > independent solutions. > > In the ArchLinux Arm camp I'm not a fan of applying many patches to > support a board, as much as anything it makes that board non generic - > and so has to have its own hand crafted kernel. This to me isn't a good > solution, so if possible prefer the pain of getting it mainlined. > > Stefan Wahren has come up with a minimal patch to hci_h5 for bluetooth, > that says the relevant properties can be read from the device tree. This > means that acpi is no longer necessary. Thats currently out in testing. > > Anyway would be good if Arch and Armbian could merge our efforts ... > > Regards, > > David. >
Oh yes - didn't reply to the "chip_h", in asus patch called "chip_enable_h". This is the <&gpio4 RK_PD3 GPIO_ACTIVE_LOW> IIRC. We could see that two pins were pulled. RK_PD3 and RK_PD4; but not knowing exactly why the pins were pulled, question is should they be separate? We went the direction of putting both pins under wifi_enable. Its not clear to me which is the least confusing, should wifi_enable need pulling two pins, or should there be an extra service "chip_enable_h" for the extra pin? Think our problem is we didn't see what "chip_enable_h" meant - the description doesn't help. Hence why we removed, and merged with wifi_enable ... On 17/02/2019 14:32, Tony McKahan wrote: > A similar configuration has been in use on Armbian images since kernel > 4.14, but not in a clean enough format for submission. I will test > this, but the only differences I see compared to ours are combination > of the mystery "chip_h" power domain/reset with the Wifi one and the > inclusion of sd_uhs_ddr50. > > -Tony > > On Sun, Feb 17, 2019 at 7:16 AM David Summers > <beagleboard@davidjohnsummers.uk> wrote: >> This patch adds the wifi to the ASUS Tinker Board (S) machines. >> >> Unfortunatly neither the Tinker Board nor the Tinker Board S schematic >> indicate how the WiFi is wired up on these devices. The WiFi is >> provided by the RTL8723BS device, that has sdio WiFi and UART >> Bluetooth. This patch just adds the WiFi interface. >> >> With no schematic, most of the wiring has been derived from the ASUS >> patch to Debian: >> >> https://github.com/TinkerBoard/debian_kernel/commit/6a3128ade33f758887048578ada61a4b7ab8e678 >> >> In conjunction with the pin out of the RTL8723BS device: >> >> http://files.pine64.org/doc/datasheet/pine64/RTL8723BS.pdf >> http://cit.odessa.ua/media/pdf/Intel-Compute-Stick/FN-Link_F23BDSM25-W1.pdf >> >> The only unusual part is that to bring the card up, both the pins >> RK_PD3 and RK_PD4 need to be pulled. Why this needs to be done is not >> clear, best explaination is that they are connected to the RTL8723BS >> pins WL_DIS# and BT_DIS#, which the data sheet vaguely says: >> >> Shared with GPIO9 This Pin Can Externally Shutdown the RTL8723BS WLAN >> function when BT_DISn is Pulled Low. When this pin deasserted, SDIO >> interface will be disabled. This pin can also support the WLAN Ra >> dio-off function with host interface remaining connected. >> >> Anyway extensive testing the TheSaint on ArchLinux Arm Forum >> >> https://archlinuxarm.org/forum/viewtopic.php?f=44&t=13064&start=120#p60548 >> >> Signed-off-by: David Summers <beagleboard@davidjohnsummers.uk> >> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> >> >> + >> + sdio_pwrseq: sdio-pwrseq { >> + compatible = "mmc-pwrseq-simple"; >> + clocks = <&rk808 RK808_CLKOUT1>; >> + clock-names = "ext_clock"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&wifi_enable>; >> + reset-gpios = <&gpio4 RK_PD3 GPIO_ACTIVE_LOW>, <&gpio4 RK_PD4 GPIO_ACTIVE_LOW>; >> + }; >> >> + >> + sdio { >> + wifi_enable: wifi-enable { >> + rockchip,pins = <4 RK_PD3 RK_FUNC_GPIO &pcfg_pull_none>, <4 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>; >> + }; >> + };
Hi David, > David Summers <beagleboard@davidjohnsummers.uk> hat am 17. Februar 2019 um 13:15 geschrieben: > > > This patch adds the wifi to the ASUS Tinker Board (S) machines. > > Unfortunatly neither the Tinker Board nor the Tinker Board S schematic > indicate how the WiFi is wired up on these devices. The WiFi is > provided by the RTL8723BS device, that has sdio WiFi and UART > Bluetooth. This patch just adds the WiFi interface. > > With no schematic, most of the wiring has been derived from the ASUS > patch to Debian: > > https://github.com/TinkerBoard/debian_kernel/commit/6a3128ade33f758887048578ada61a4b7ab8e678 > > In conjunction with the pin out of the RTL8723BS device: > > http://files.pine64.org/doc/datasheet/pine64/RTL8723BS.pdf > http://cit.odessa.ua/media/pdf/Intel-Compute-Stick/FN-Link_F23BDSM25-W1.pdf > > The only unusual part is that to bring the card up, both the pins > RK_PD3 and RK_PD4 need to be pulled. Why this needs to be done is not > clear, best explaination is that they are connected to the RTL8723BS > pins WL_DIS# and BT_DIS#, which the data sheet vaguely says: > > Shared with GPIO9 This Pin Can Externally Shutdown the RTL8723BS WLAN > function when BT_DISn is Pulled Low. When this pin deasserted, SDIO > interface will be disabled. This pin can also support the WLAN Ra > dio-off function with host interface remaining connected. > > Anyway extensive testing the TheSaint on ArchLinux Arm Forum > > https://archlinuxarm.org/forum/viewtopic.php?f=44&t=13064&start=120#p60548 > > Signed-off-by: David Summers <beagleboard@davidjohnsummers.uk> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > --- > arch/arm/boot/dts/rk3288-tinker.dtsi | 39 +++++++++++++++++++++++++--- > 1 file changed, 36 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/boot/dts/rk3288-tinker.dtsi b/arch/arm/boot/dts/rk3288-tinker.dtsi > index fceaeed44e34..e1796f340eef 100644 > --- a/arch/arm/boot/dts/rk3288-tinker.dtsi > +++ b/arch/arm/boot/dts/rk3288-tinker.dtsi > @@ -3,8 +3,9 @@ > * Copyright (c) 2017 Fuzhou Rockchip Electronics Co., Ltd. > */ > > #include "rk3288.dtsi" > #include <dt-bindings/input/input.h> > +#include <dt-bindings/clock/rockchip,rk808.h> > > / { > chosen { > @@ -98,6 +97,15 @@ > startup-delay-us = <100000>; > vin-supply = <&vcc_io>; > }; > + > + sdio_pwrseq: sdio-pwrseq { > + compatible = "mmc-pwrseq-simple"; > + clocks = <&rk808 RK808_CLKOUT1>; > + clock-names = "ext_clock"; > + pinctrl-names = "default"; > + pinctrl-0 = <&wifi_enable>; As i wrote at the Arch board it would be nice to keep the Asus comment here: /* * On the module itself this is one of these (depending * on the actual card populated): * - SDIO_RESET_L_WL_REG_ON * - PDN (power down when low) */ > + reset-gpios = <&gpio4 RK_PD3 GPIO_ACTIVE_LOW>, <&gpio4 RK_PD4 GPIO_ACTIVE_LOW>; > + }; > }; > > &cpu0 { > @@ -337,8 +345,8 @@ > > &io_domains { > status = "okay"; > - Please drop this whitespace change > sdcard-supply = <&vccio_sd>; > + wifi-supply = <&vcc_18>; > }; > > &pinctrl { > @@ -417,6 +425,12 @@ > rockchip,pins = <7 8 RK_FUNC_GPIO &pcfg_pull_none>; > }; > }; > + > + sdio { > + wifi_enable: wifi-enable { > + rockchip,pins = <4 RK_PD3 RK_FUNC_GPIO &pcfg_pull_none>, <4 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>; Please make a line break after the first pin definition > + }; > + }; > }; > > &pwm0 { > @@ -440,6 +454,25 @@ > vqmmc-supply = <&vccio_sd>; > }; > > +&sdio0 { > + bus-width = <4>; > + cap-sd-highspeed; > + cap-sdio-irq; > + keep-power-in-suspend; > + mmc-pwrseq = <&sdio_pwrseq>; > + non-removable; > + pinctrl-names = "default"; > + pinctrl-0 = <&sdio0_bus4>, <&sdio0_cmd>, <&sdio0_clk>, <&sdio0_int>; > + max-frequency = <50000000>; > + sd-uhs-sdr12; > + sd-uhs-sdr25; > + sd-uhs-sdr50; > + sd-uhs-ddr50; I'm okay with Tony's suggestion to remove sd-ush-ddr50. > + vmmc-supply = <&vcc_io>; > + vqmmc-supply = <&vcc_18>; > + status = "okay"; > +}; > + > &tsadc { > rockchip,hw-tshut-mode = <1>; /* tshut mode 0:CRU 1:GPIO */ > rockchip,hw-tshut-polarity = <1>; /* tshut polarity 0:LOW 1:HIGH */ > -- > beagleboard@davidjohnsummers.uk > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi, > David Summers <beagleboard@davidjohnsummers.uk> hat am 17. Februar 2019 um 16:55 geschrieben: > > > Stefan Wahren has come up with a minimal patch to hci_h5 for bluetooth, > that says the relevant properties can be read from the device tree. This > means that acpi is no longer necessary. Thats currently out in testing. > this patch is more a quick hack in order to test bluetooth and don't have all features ( support for baud limitation, host-wake-up ). Also the patch [1] has only been compile tested (i don't have a Tinker Board), so please be aware. I created my patch before this series has been submitted, but Patch #3 doesn't work with this and needs changes. Stefan [1] - https://gist.github.com/lategoodbye/79bac99d4f1158a719a48ea3c45eb7f1 > Regards, > > David. > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 17/02/2019 20:43, Stefan Wahren wrote: > Hi David, > >> David Summers <beagleboard@davidjohnsummers.uk> hat am 17. Februar 2019 um 13:15 geschrieben: >> >> >> This patch adds the wifi to the ASUS Tinker Board (S) machines. >> >> Unfortunatly neither the Tinker Board nor the Tinker Board S schematic >> indicate how the WiFi is wired up on these devices. The WiFi is >> provided by the RTL8723BS device, that has sdio WiFi and UART >> Bluetooth. This patch just adds the WiFi interface. >> >> With no schematic, most of the wiring has been derived from the ASUS >> patch to Debian: >> >> https://github.com/TinkerBoard/debian_kernel/commit/6a3128ade33f758887048578ada61a4b7ab8e678 >> >> In conjunction with the pin out of the RTL8723BS device: >> >> http://files.pine64.org/doc/datasheet/pine64/RTL8723BS.pdf >> http://cit.odessa.ua/media/pdf/Intel-Compute-Stick/FN-Link_F23BDSM25-W1.pdf >> >> The only unusual part is that to bring the card up, both the pins >> RK_PD3 and RK_PD4 need to be pulled. Why this needs to be done is not >> clear, best explaination is that they are connected to the RTL8723BS >> pins WL_DIS# and BT_DIS#, which the data sheet vaguely says: >> >> Shared with GPIO9 This Pin Can Externally Shutdown the RTL8723BS WLAN >> function when BT_DISn is Pulled Low. When this pin deasserted, SDIO >> interface will be disabled. This pin can also support the WLAN Ra >> dio-off function with host interface remaining connected. >> >> Anyway extensive testing the TheSaint on ArchLinux Arm Forum >> >> https://archlinuxarm.org/forum/viewtopic.php?f=44&t=13064&start=120#p60548 >> >> Signed-off-by: David Summers <beagleboard@davidjohnsummers.uk> >> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> >> --- >> arch/arm/boot/dts/rk3288-tinker.dtsi | 39 +++++++++++++++++++++++++--- >> 1 file changed, 36 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm/boot/dts/rk3288-tinker.dtsi b/arch/arm/boot/dts/rk3288-tinker.dtsi >> index fceaeed44e34..e1796f340eef 100644 >> --- a/arch/arm/boot/dts/rk3288-tinker.dtsi >> +++ b/arch/arm/boot/dts/rk3288-tinker.dtsi >> @@ -3,8 +3,9 @@ >> * Copyright (c) 2017 Fuzhou Rockchip Electronics Co., Ltd. >> */ >> >> #include "rk3288.dtsi" >> #include <dt-bindings/input/input.h> >> +#include <dt-bindings/clock/rockchip,rk808.h> >> >> / { >> chosen { >> @@ -98,6 +97,15 @@ >> startup-delay-us = <100000>; >> vin-supply = <&vcc_io>; >> }; >> + >> + sdio_pwrseq: sdio-pwrseq { >> + compatible = "mmc-pwrseq-simple"; >> + clocks = <&rk808 RK808_CLKOUT1>; >> + clock-names = "ext_clock"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&wifi_enable>; > > As i wrote at the Arch board it would be nice to keep the Asus comment here: > > /* > * On the module itself this is one of these (depending > * on the actual card populated): > * - SDIO_RESET_L_WL_REG_ON > * - PDN (power down when low) > */ Note that that comment (like many others) is merely copy-pasted from the Rockchip evaluation board DT, so is of dubious relevance here. If anything it becomes actively misleading when we already know that "this" is two pins on a module where nothing matches either of those names. Robin. >> + reset-gpios = <&gpio4 RK_PD3 GPIO_ACTIVE_LOW>, <&gpio4 RK_PD4 GPIO_ACTIVE_LOW>; >> + }; >> }; >> >> &cpu0 { >> @@ -337,8 +345,8 @@ >> >> &io_domains { >> status = "okay"; >> - > > Please drop this whitespace change > >> sdcard-supply = <&vccio_sd>; >> + wifi-supply = <&vcc_18>; >> }; >> >> &pinctrl { >> @@ -417,6 +425,12 @@ >> rockchip,pins = <7 8 RK_FUNC_GPIO &pcfg_pull_none>; >> }; >> }; >> + >> + sdio { >> + wifi_enable: wifi-enable { >> + rockchip,pins = <4 RK_PD3 RK_FUNC_GPIO &pcfg_pull_none>, <4 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>; > > Please make a line break after the first pin definition > >> + }; >> + }; >> }; >> >> &pwm0 { >> @@ -440,6 +454,25 @@ >> vqmmc-supply = <&vccio_sd>; >> }; >> >> +&sdio0 { >> + bus-width = <4>; >> + cap-sd-highspeed; >> + cap-sdio-irq; >> + keep-power-in-suspend; >> + mmc-pwrseq = <&sdio_pwrseq>; >> + non-removable; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&sdio0_bus4>, <&sdio0_cmd>, <&sdio0_clk>, <&sdio0_int>; >> + max-frequency = <50000000>; >> + sd-uhs-sdr12; >> + sd-uhs-sdr25; >> + sd-uhs-sdr50; >> + sd-uhs-ddr50; > > I'm okay with Tony's suggestion to remove sd-ush-ddr50. > >> + vmmc-supply = <&vcc_io>; >> + vqmmc-supply = <&vcc_18>; >> + status = "okay"; >> +}; >> + >> &tsadc { >> rockchip,hw-tshut-mode = <1>; /* tshut mode 0:CRU 1:GPIO */ >> rockchip,hw-tshut-polarity = <1>; /* tshut polarity 0:LOW 1:HIGH */ >> -- >> beagleboard@davidjohnsummers.uk >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip >
On 17/02/2019 20:43, Stefan Wahren wrote: > Hi David, > >> David Summers <beagleboard@davidjohnsummers.uk> hat am 17. Februar 2019 um 13:15 geschrieben: >> >> >> + >> + sdio_pwrseq: sdio-pwrseq { >> + compatible = "mmc-pwrseq-simple"; >> + clocks = <&rk808 RK808_CLKOUT1>; >> + clock-names = "ext_clock"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&wifi_enable>; > As i wrote at the Arch board it would be nice to keep the Asus comment here: > > /* > * On the module itself this is one of these (depending > * on the actual card populated): > * - SDIO_RESET_L_WL_REG_ON > * - PDN (power down when low) > */ > Sorry Stefan, I'm with Robin on this one. Three reasons: 1) When you look at where ASUS added these comments: https://github.com/TinkerBoard/debian_kernel/commit/6a3128ade33f758887048578ada61a4b7ab8e678 They has only added RK_PD4 = 28 So at best to only applies to that change. 2) If you look at datasheet: http://files.pine64.org/doc/datasheet/pine64/RTL8723BS.pdf http://cit.odessa.ua/media/pdf/Intel-Compute-Stick/FN-Link_F23BDSM25-W1.pdf Then neither of the two pins are mentioned, and the comment is not helpful 3) Should dts have comments in? Over time I'm got to like git, and that you click on blame. That is what showed in post one that the comment only applied to one pin. To me blame point to the patch - and that usually gives more details. Its why I explain that we don't know what these two pins are attached to - this doesn't sound good, but actually its all we know. So it explains things. So I now think that dts should have very few comments in, and only where they *really* add something. In this case we don't know what the comment means .. >> + reset-gpios = <&gpio4 RK_PD3 GPIO_ACTIVE_LOW>, <&gpio4 RK_PD4 GPIO_ACTIVE_LOW>; >> + }; >> }; >> >> &cpu0 { >> @@ -337,8 +345,8 @@ >> >> &io_domains { >> status = "okay"; >> - > Please drop this whitespace change Most {} in dts don't have white space line in - only between {} {}. So a white space line in the middle of a single {} is unusual. I can't see what its needed for, or what it makes clearer. So to me removing makes it more in line with everything else. If people are unhappy though I'll add the white line back in again. >> sdcard-supply = <&vccio_sd>; >> + wifi-supply = <&vcc_18>; >> }; >> >> &pinctrl { >> @@ -417,6 +425,12 @@ >> rockchip,pins = <7 8 RK_FUNC_GPIO &pcfg_pull_none>; >> }; >> }; >> + >> + sdio { >> + wifi_enable: wifi-enable { >> + rockchip,pins = <4 RK_PD3 RK_FUNC_GPIO &pcfg_pull_none>, <4 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>; > Please make a line break after the first pin definition Yes that makes sense David
Hi, > David Summers <beagleboard@davidjohnsummers.uk> hat am 18. Februar 2019 um 21:26 geschrieben: > > > On 17/02/2019 20:43, Stefan Wahren wrote: > > Hi David, > > > >> David Summers <beagleboard@davidjohnsummers.uk> hat am 17. Februar 2019 um 13:15 geschrieben: > >> > >> > >> + > >> + sdio_pwrseq: sdio-pwrseq { > >> + compatible = "mmc-pwrseq-simple"; > >> + clocks = <&rk808 RK808_CLKOUT1>; > >> + clock-names = "ext_clock"; > >> + pinctrl-names = "default"; > >> + pinctrl-0 = <&wifi_enable>; > > As i wrote at the Arch board it would be nice to keep the Asus comment here: > > > > /* > > * On the module itself this is one of these (depending > > * on the actual card populated): > > * - SDIO_RESET_L_WL_REG_ON > > * - PDN (power down when low) > > */ > > > Sorry Stefan, > > I'm with Robin on this one. i don't known anything about the reference design. So please ignore my comment. > > > >> + reset-gpios = <&gpio4 RK_PD3 GPIO_ACTIVE_LOW>, <&gpio4 RK_PD4 GPIO_ACTIVE_LOW>; > >> + }; > >> }; > >> > >> &cpu0 { > >> @@ -337,8 +345,8 @@ > >> > >> &io_domains { > >> status = "okay"; > >> - > > Please drop this whitespace change > Most {} in dts don't have white space line in - only between {} {}. So a > white space line in the middle of a single {} is unusual. I can't see > what its needed for, or what it makes clearer. So to me removing makes > it more in line with everything else. If people are unhappy though I'll > add the white line back in again. I don't have a problem with dropping the empy line in general. But it's unrelated to this change. So please make it a separate patch or drop it completely.
And so synthesis on where we are on the WiFi patch for the Thinker Boards. There have been very few comments, only really Tony McKahan from Armbian - where they apply a similar patch. Tony was going to test the patch, but we have heard nothing back. The patch has been tested on ArchLinux Arm, and found to work. So with no further comments, I propose this is accepted as is. Heiko, let me know if you want me to respin this patch to apply cleanly after the sd card changes. David. On 17/02/2019 12:15, David Summers wrote: > This patch adds the wifi to the ASUS Tinker Board (S) machines. > > Unfortunatly neither the Tinker Board nor the Tinker Board S schematic > indicate how the WiFi is wired up on these devices. The WiFi is > provided by the RTL8723BS device, that has sdio WiFi and UART > Bluetooth. This patch just adds the WiFi interface. > > With no schematic, most of the wiring has been derived from the ASUS > patch to Debian: > > https://github.com/TinkerBoard/debian_kernel/commit/6a3128ade33f758887048578ada61a4b7ab8e678 > > In conjunction with the pin out of the RTL8723BS device: > > http://files.pine64.org/doc/datasheet/pine64/RTL8723BS.pdf > http://cit.odessa.ua/media/pdf/Intel-Compute-Stick/FN-Link_F23BDSM25-W1.pdf > > The only unusual part is that to bring the card up, both the pins > RK_PD3 and RK_PD4 need to be pulled. Why this needs to be done is not > clear, best explaination is that they are connected to the RTL8723BS > pins WL_DIS# and BT_DIS#, which the data sheet vaguely says: > > Shared with GPIO9 This Pin Can Externally Shutdown the RTL8723BS WLAN > function when BT_DISn is Pulled Low. When this pin deasserted, SDIO > interface will be disabled. This pin can also support the WLAN Ra > dio-off function with host interface remaining connected. > > Anyway extensive testing the TheSaint on ArchLinux Arm Forum > > https://archlinuxarm.org/forum/viewtopic.php?f=44&t=13064&start=120#p60548 > > Signed-off-by: David Summers <beagleboard@davidjohnsummers.uk> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > --- > arch/arm/boot/dts/rk3288-tinker.dtsi | 39 +++++++++++++++++++++++++--- > 1 file changed, 36 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/boot/dts/rk3288-tinker.dtsi b/arch/arm/boot/dts/rk3288-tinker.dtsi > index fceaeed44e34..e1796f340eef 100644 > --- a/arch/arm/boot/dts/rk3288-tinker.dtsi > +++ b/arch/arm/boot/dts/rk3288-tinker.dtsi > @@ -3,8 +3,9 @@ > * Copyright (c) 2017 Fuzhou Rockchip Electronics Co., Ltd. > */ > > #include "rk3288.dtsi" > #include <dt-bindings/input/input.h> > +#include <dt-bindings/clock/rockchip,rk808.h> > > / { > chosen { > @@ -98,6 +97,15 @@ > startup-delay-us = <100000>; > vin-supply = <&vcc_io>; > }; > + > + sdio_pwrseq: sdio-pwrseq { > + compatible = "mmc-pwrseq-simple"; > + clocks = <&rk808 RK808_CLKOUT1>; > + clock-names = "ext_clock"; > + pinctrl-names = "default"; > + pinctrl-0 = <&wifi_enable>; > + reset-gpios = <&gpio4 RK_PD3 GPIO_ACTIVE_LOW>, <&gpio4 RK_PD4 GPIO_ACTIVE_LOW>; > + }; > }; > > &cpu0 { > @@ -337,8 +345,8 @@ > > &io_domains { > status = "okay"; > - > sdcard-supply = <&vccio_sd>; > + wifi-supply = <&vcc_18>; > }; > > &pinctrl { > @@ -417,6 +425,12 @@ > rockchip,pins = <7 8 RK_FUNC_GPIO &pcfg_pull_none>; > }; > }; > + > + sdio { > + wifi_enable: wifi-enable { > + rockchip,pins = <4 RK_PD3 RK_FUNC_GPIO &pcfg_pull_none>, <4 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>; > + }; > + }; > }; > > &pwm0 { > @@ -440,6 +454,25 @@ > vqmmc-supply = <&vccio_sd>; > }; > > +&sdio0 { > + bus-width = <4>; > + cap-sd-highspeed; > + cap-sdio-irq; > + keep-power-in-suspend; > + mmc-pwrseq = <&sdio_pwrseq>; > + non-removable; > + pinctrl-names = "default"; > + pinctrl-0 = <&sdio0_bus4>, <&sdio0_cmd>, <&sdio0_clk>, <&sdio0_int>; > + max-frequency = <50000000>; > + sd-uhs-sdr12; > + sd-uhs-sdr25; > + sd-uhs-sdr50; > + sd-uhs-ddr50; > + vmmc-supply = <&vcc_io>; > + vqmmc-supply = <&vcc_18>; > + status = "okay"; > +}; > + > &tsadc { > rockchip,hw-tshut-mode = <1>; /* tshut mode 0:CRU 1:GPIO */ > rockchip,hw-tshut-polarity = <1>; /* tshut polarity 0:LOW 1:HIGH */
On Sun, Mar 3, 2019 at 2:40 PM David Summers <beagleboard@davidjohnsummers.uk> wrote: > > And so synthesis on where we are on the WiFi patch for the Thinker Boards. > > There have been very few comments, only really Tony McKahan from Armbian > - where they apply a similar patch. Tony was going to test the patch, > but we have heard nothing back. The patch has been tested on ArchLinux > Arm, and found to work. Apologies, I've got too many things going at once, and it sounded like there would/should be a V2 of this patch. I tested it against 5.0 rc8 with sd-uhs-ddr50 removed as discussed previously, no issues encountered. Tony > > So with no further comments, I propose this is accepted as is. Heiko, > let me know if you want me to respin this patch to apply cleanly after > the sd card changes. > > David. > > > > On 17/02/2019 12:15, David Summers wrote: > > This patch adds the wifi to the ASUS Tinker Board (S) machines. > > > > Unfortunatly neither the Tinker Board nor the Tinker Board S schematic > > indicate how the WiFi is wired up on these devices. The WiFi is > > provided by the RTL8723BS device, that has sdio WiFi and UART > > Bluetooth. This patch just adds the WiFi interface. > > > > With no schematic, most of the wiring has been derived from the ASUS > > patch to Debian: > > > > https://github.com/TinkerBoard/debian_kernel/commit/6a3128ade33f758887048578ada61a4b7ab8e678 > > > > In conjunction with the pin out of the RTL8723BS device: > > > > http://files.pine64.org/doc/datasheet/pine64/RTL8723BS.pdf > > http://cit.odessa.ua/media/pdf/Intel-Compute-Stick/FN-Link_F23BDSM25-W1.pdf > > > > The only unusual part is that to bring the card up, both the pins > > RK_PD3 and RK_PD4 need to be pulled. Why this needs to be done is not > > clear, best explaination is that they are connected to the RTL8723BS > > pins WL_DIS# and BT_DIS#, which the data sheet vaguely says: > > > > Shared with GPIO9 This Pin Can Externally Shutdown the RTL8723BS WLAN > > function when BT_DISn is Pulled Low. When this pin deasserted, SDIO > > interface will be disabled. This pin can also support the WLAN Ra > > dio-off function with host interface remaining connected. > > > > Anyway extensive testing the TheSaint on ArchLinux Arm Forum > > > > https://archlinuxarm.org/forum/viewtopic.php?f=44&t=13064&start=120#p60548 > > > > Signed-off-by: David Summers <beagleboard@davidjohnsummers.uk> > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > > --- > > arch/arm/boot/dts/rk3288-tinker.dtsi | 39 +++++++++++++++++++++++++--- > > 1 file changed, 36 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm/boot/dts/rk3288-tinker.dtsi b/arch/arm/boot/dts/rk3288-tinker.dtsi > > index fceaeed44e34..e1796f340eef 100644 > > --- a/arch/arm/boot/dts/rk3288-tinker.dtsi > > +++ b/arch/arm/boot/dts/rk3288-tinker.dtsi > > @@ -3,8 +3,9 @@ > > * Copyright (c) 2017 Fuzhou Rockchip Electronics Co., Ltd. > > */ > > > > #include "rk3288.dtsi" > > #include <dt-bindings/input/input.h> > > +#include <dt-bindings/clock/rockchip,rk808.h> > > > > / { > > chosen { > > @@ -98,6 +97,15 @@ > > startup-delay-us = <100000>; > > vin-supply = <&vcc_io>; > > }; > > + > > + sdio_pwrseq: sdio-pwrseq { > > + compatible = "mmc-pwrseq-simple"; > > + clocks = <&rk808 RK808_CLKOUT1>; > > + clock-names = "ext_clock"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&wifi_enable>; > > + reset-gpios = <&gpio4 RK_PD3 GPIO_ACTIVE_LOW>, <&gpio4 RK_PD4 GPIO_ACTIVE_LOW>; > > + }; > > }; > > > > &cpu0 { > > @@ -337,8 +345,8 @@ > > > > &io_domains { > > status = "okay"; > > - > > sdcard-supply = <&vccio_sd>; > > + wifi-supply = <&vcc_18>; > > }; > > > > &pinctrl { > > @@ -417,6 +425,12 @@ > > rockchip,pins = <7 8 RK_FUNC_GPIO &pcfg_pull_none>; > > }; > > }; > > + > > + sdio { > > + wifi_enable: wifi-enable { > > + rockchip,pins = <4 RK_PD3 RK_FUNC_GPIO &pcfg_pull_none>, <4 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>; > > + }; > > + }; > > }; > > > > &pwm0 { > > @@ -440,6 +454,25 @@ > > vqmmc-supply = <&vccio_sd>; > > }; > > > > +&sdio0 { > > + bus-width = <4>; > > + cap-sd-highspeed; > > + cap-sdio-irq; > > + keep-power-in-suspend; > > + mmc-pwrseq = <&sdio_pwrseq>; > > + non-removable; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&sdio0_bus4>, <&sdio0_cmd>, <&sdio0_clk>, <&sdio0_int>; > > + max-frequency = <50000000>; > > + sd-uhs-sdr12; > > + sd-uhs-sdr25; > > + sd-uhs-sdr50; > > + sd-uhs-ddr50; > > + vmmc-supply = <&vcc_io>; > > + vqmmc-supply = <&vcc_18>; > > + status = "okay"; > > +}; > > + > > &tsadc { > > rockchip,hw-tshut-mode = <1>; /* tshut mode 0:CRU 1:GPIO */ > > rockchip,hw-tshut-polarity = <1>; /* tshut polarity 0:LOW 1:HIGH */ > >
diff --git a/arch/arm/boot/dts/rk3288-tinker.dtsi b/arch/arm/boot/dts/rk3288-tinker.dtsi index fceaeed44e34..e1796f340eef 100644 --- a/arch/arm/boot/dts/rk3288-tinker.dtsi +++ b/arch/arm/boot/dts/rk3288-tinker.dtsi @@ -3,8 +3,9 @@ * Copyright (c) 2017 Fuzhou Rockchip Electronics Co., Ltd. */ #include "rk3288.dtsi" #include <dt-bindings/input/input.h> +#include <dt-bindings/clock/rockchip,rk808.h> / { chosen { @@ -98,6 +97,15 @@ startup-delay-us = <100000>; vin-supply = <&vcc_io>; }; + + sdio_pwrseq: sdio-pwrseq { + compatible = "mmc-pwrseq-simple"; + clocks = <&rk808 RK808_CLKOUT1>; + clock-names = "ext_clock"; + pinctrl-names = "default"; + pinctrl-0 = <&wifi_enable>; + reset-gpios = <&gpio4 RK_PD3 GPIO_ACTIVE_LOW>, <&gpio4 RK_PD4 GPIO_ACTIVE_LOW>; + }; }; &cpu0 { @@ -337,8 +345,8 @@ &io_domains { status = "okay"; - sdcard-supply = <&vccio_sd>; + wifi-supply = <&vcc_18>; }; &pinctrl { @@ -417,6 +425,12 @@ rockchip,pins = <7 8 RK_FUNC_GPIO &pcfg_pull_none>; }; }; + + sdio { + wifi_enable: wifi-enable { + rockchip,pins = <4 RK_PD3 RK_FUNC_GPIO &pcfg_pull_none>, <4 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>; + }; + }; }; &pwm0 { @@ -440,6 +454,25 @@ vqmmc-supply = <&vccio_sd>; }; +&sdio0 { + bus-width = <4>; + cap-sd-highspeed; + cap-sdio-irq; + keep-power-in-suspend; + mmc-pwrseq = <&sdio_pwrseq>; + non-removable; + pinctrl-names = "default"; + pinctrl-0 = <&sdio0_bus4>, <&sdio0_cmd>, <&sdio0_clk>, <&sdio0_int>; + max-frequency = <50000000>; + sd-uhs-sdr12; + sd-uhs-sdr25; + sd-uhs-sdr50; + sd-uhs-ddr50; + vmmc-supply = <&vcc_io>; + vqmmc-supply = <&vcc_18>; + status = "okay"; +}; + &tsadc { rockchip,hw-tshut-mode = <1>; /* tshut mode 0:CRU 1:GPIO */ rockchip,hw-tshut-polarity = <1>; /* tshut polarity 0:LOW 1:HIGH */