[8/8] arm64: dts: rockchip: RockPro64: enable wifi module at sdio0
diff mbox series

Message ID 20191209223822.27236-8-smoch@web.de
State New
Headers show
Series
  • Untitled series #213955
Related show

Commit Message

Soeren Moch Dec. 9, 2019, 10:38 p.m. UTC
RockPro64 supports an Ampak AP6359SA based wifi/bt combo module.
The BCM4359/9 wifi controller in this module is connected to sdio0,
enable this interface.

Signed-off-by: Soeren Moch <smoch@web.de>
---
Not sure where to place exactly the sdio0 node in the dts because
existing sd nodes are not sorted alphabetically.

This last patch in this brcmfmac patch series probably should be picked
up by Heiko independently of the rest of this series. It was sent together
to show how this brcmfmac extension for 4359-sdio support with RSDB is
used and tested.

Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: linux-wireless@vger.kernel.org
Cc: brcm80211-dev-list.pdl@broadcom.com
Cc: brcm80211-dev-list@cypress.com
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-rockchip@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 .../boot/dts/rockchip/rk3399-rockpro64.dts    | 21 ++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

--
2.17.1

Comments

Heiko Stuebner Dec. 9, 2019, 11:08 p.m. UTC | #1
Hi Soeren,

Am Montag, 9. Dezember 2019, 23:38:22 CET schrieb Soeren Moch:
> RockPro64 supports an Ampak AP6359SA based wifi/bt combo module.
> The BCM4359/9 wifi controller in this module is connected to sdio0,
> enable this interface.
> 
> Signed-off-by: Soeren Moch <smoch@web.de>
> ---
> Not sure where to place exactly the sdio0 node in the dts because
> existing sd nodes are not sorted alphabetically.
> 
> This last patch in this brcmfmac patch series probably should be picked
> up by Heiko independently of the rest of this series. It was sent together
> to show how this brcmfmac extension for 4359-sdio support with RSDB is
> used and tested.

node placement looks good so I can apply it, just a general questions
I only got patch 8/8 are patches 1-7 relevant for this one and what are they?

Thanks
Heiko


> 
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: linux-wireless@vger.kernel.org
> Cc: brcm80211-dev-list.pdl@broadcom.com
> Cc: brcm80211-dev-list@cypress.com
> Cc: netdev@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-rockchip@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  .../boot/dts/rockchip/rk3399-rockpro64.dts    | 21 ++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
> index 7f4b2eba31d4..9fa92790d6e0 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
> @@ -71,13 +71,6 @@
>  		clock-names = "ext_clock";
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&wifi_enable_h>;
> -
> -		/*
> -		 * 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 = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
>  	};
> 
> @@ -650,6 +643,20 @@
>  	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";
> +};
> +
>  &sdmmc {
>  	bus-width = <4>;
>  	cap-sd-highspeed;
> --
> 2.17.1
>
Soeren Moch Dec. 9, 2019, 11:29 p.m. UTC | #2
Hi Heiko,

On 10.12.19 00:08, Heiko Stübner wrote:
> Hi Soeren,
>
> Am Montag, 9. Dezember 2019, 23:38:22 CET schrieb Soeren Moch:
>> RockPro64 supports an Ampak AP6359SA based wifi/bt combo module.
>> The BCM4359/9 wifi controller in this module is connected to sdio0,
>> enable this interface.
>>
>> Signed-off-by: Soeren Moch <smoch@web.de>
>> ---
>> Not sure where to place exactly the sdio0 node in the dts because
>> existing sd nodes are not sorted alphabetically.
>>
>> This last patch in this brcmfmac patch series probably should be picked
>> up by Heiko independently of the rest of this series. It was sent together
>> to show how this brcmfmac extension for 4359-sdio support with RSDB is
>> used and tested.
> node placement looks good so I can apply it, just a general questions
> I only got patch 8/8 are patches 1-7 relevant for this one and what are they?
Patches 1-7 are the patches to support the BCM4359 chipset with SDIO
interface in the linux brcmfmac net-wireless driver, see [1].

So this patch series has 2 parts:
patches 1-7: add support for the wifi chipset in the wireless driver,
this has to go through net-wireless
patch 8: enable the wifi module with this chipset on RockPro64, this patch

If this was confusing, what would be the ideal way to post such series?

Thanks,
Soeren

[1] https://patchwork.kernel.org/project/linux-wireless/list/?series=213951
>
> Thanks
> Heiko
>
>
>> Cc: Heiko Stuebner <heiko@sntech.de>
>> Cc: Kalle Valo <kvalo@codeaurora.org>
>> Cc: linux-wireless@vger.kernel.org
>> Cc: brcm80211-dev-list.pdl@broadcom.com
>> Cc: brcm80211-dev-list@cypress.com
>> Cc: netdev@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-rockchip@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>  .../boot/dts/rockchip/rk3399-rockpro64.dts    | 21 ++++++++++++-------
>>  1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>> index 7f4b2eba31d4..9fa92790d6e0 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>> @@ -71,13 +71,6 @@
>>  		clock-names = "ext_clock";
>>  		pinctrl-names = "default";
>>  		pinctrl-0 = <&wifi_enable_h>;
>> -
>> -		/*
>> -		 * 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 = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
>>  	};
>>
>> @@ -650,6 +643,20 @@
>>  	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";
>> +};
>> +
>>  &sdmmc {
>>  	bus-width = <4>;
>>  	cap-sd-highspeed;
>> --
>> 2.17.1
>>
>
>
>
Heiko Stuebner Dec. 10, 2019, 1:18 a.m. UTC | #3
Hi Soeren,

Am Dienstag, 10. Dezember 2019, 00:29:21 CET schrieb Soeren Moch:
> On 10.12.19 00:08, Heiko Stübner wrote:
> > Am Montag, 9. Dezember 2019, 23:38:22 CET schrieb Soeren Moch:
> >> RockPro64 supports an Ampak AP6359SA based wifi/bt combo module.
> >> The BCM4359/9 wifi controller in this module is connected to sdio0,
> >> enable this interface.
> >>
> >> Signed-off-by: Soeren Moch <smoch@web.de>
> >> ---
> >> Not sure where to place exactly the sdio0 node in the dts because
> >> existing sd nodes are not sorted alphabetically.
> >>
> >> This last patch in this brcmfmac patch series probably should be picked
> >> up by Heiko independently of the rest of this series. It was sent together
> >> to show how this brcmfmac extension for 4359-sdio support with RSDB is
> >> used and tested.
> > node placement looks good so I can apply it, just a general questions
> > I only got patch 8/8 are patches 1-7 relevant for this one and what are they?
> Patches 1-7 are the patches to support the BCM4359 chipset with SDIO
> interface in the linux brcmfmac net-wireless driver, see [1].
> 
> So this patch series has 2 parts:
> patches 1-7: add support for the wifi chipset in the wireless driver,
> this has to go through net-wireless
> patch 8: enable the wifi module with this chipset on RockPro64, this patch

Thanks for the clarification :-) .

As patch 8 "only" does the core sdio node, it doesn't really depend on the
earlier ones and you can submit any uart-hooks for bluetooth once the
other patches land I guess.


> If this was confusing, what would be the ideal way to post such series?

I think every maintainer has some slightly different perspective on this,
but personally I like getting the whole series to follow the discussion but
also to just see when the driver-side changes get merged, as the dts-parts
need to wait for that in a lot of cases.

Heiko


> [1] https://patchwork.kernel.org/project/linux-wireless/list/?series=213951
> >
> > Thanks
> > Heiko
> >
> >
> >> Cc: Heiko Stuebner <heiko@sntech.de>
> >> Cc: Kalle Valo <kvalo@codeaurora.org>
> >> Cc: linux-wireless@vger.kernel.org
> >> Cc: brcm80211-dev-list.pdl@broadcom.com
> >> Cc: brcm80211-dev-list@cypress.com
> >> Cc: netdev@vger.kernel.org
> >> Cc: linux-arm-kernel@lists.infradead.org
> >> Cc: linux-rockchip@lists.infradead.org
> >> Cc: linux-kernel@vger.kernel.org
> >> ---
> >>  .../boot/dts/rockchip/rk3399-rockpro64.dts    | 21 ++++++++++++-------
> >>  1 file changed, 14 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
> >> index 7f4b2eba31d4..9fa92790d6e0 100644
> >> --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
> >> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
> >> @@ -71,13 +71,6 @@
> >>  		clock-names = "ext_clock";
> >>  		pinctrl-names = "default";
> >>  		pinctrl-0 = <&wifi_enable_h>;
> >> -
> >> -		/*
> >> -		 * 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 = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
> >>  	};
> >>
> >> @@ -650,6 +643,20 @@
> >>  	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";
> >> +};
> >> +
> >>  &sdmmc {
> >>  	bus-width = <4>;
> >>  	cap-sd-highspeed;
> >> --
> >> 2.17.1
> >>
> >
> >
> >
> 
>
Kalle Valo Dec. 10, 2019, 9:14 a.m. UTC | #4
Heiko Stübner <heiko@sntech.de> writes:

> Hi Soeren,
>
> Am Dienstag, 10. Dezember 2019, 00:29:21 CET schrieb Soeren Moch:
>> On 10.12.19 00:08, Heiko Stübner wrote:
>> > Am Montag, 9. Dezember 2019, 23:38:22 CET schrieb Soeren Moch:
>> >> RockPro64 supports an Ampak AP6359SA based wifi/bt combo module.
>> >> The BCM4359/9 wifi controller in this module is connected to sdio0,
>> >> enable this interface.
>> >>
>> >> Signed-off-by: Soeren Moch <smoch@web.de>
>> >> ---
>> >> Not sure where to place exactly the sdio0 node in the dts because
>> >> existing sd nodes are not sorted alphabetically.
>> >>
>> >> This last patch in this brcmfmac patch series probably should be picked
>> >> up by Heiko independently of the rest of this series. It was sent together
>> >> to show how this brcmfmac extension for 4359-sdio support with RSDB is
>> >> used and tested.
>> > node placement looks good so I can apply it, just a general questions
>> > I only got patch 8/8 are patches 1-7 relevant for this one and what are they?
>> Patches 1-7 are the patches to support the BCM4359 chipset with SDIO
>> interface in the linux brcmfmac net-wireless driver, see [1].
>> 
>> So this patch series has 2 parts:
>> patches 1-7: add support for the wifi chipset in the wireless driver,
>> this has to go through net-wireless
>> patch 8: enable the wifi module with this chipset on RockPro64, this patch
>
> Thanks for the clarification :-) .
>
> As patch 8 "only" does the core sdio node, it doesn't really depend on the
> earlier ones and you can submit any uart-hooks for bluetooth once the
> other patches land I guess.
>
>
>> If this was confusing, what would be the ideal way to post such series?
>
> I think every maintainer has some slightly different perspective on this,
> but personally I like getting the whole series to follow the discussion but
> also to just see when the driver-side changes get merged, as the dts-parts
> need to wait for that in a lot of cases.

FWIW I prefer the same as Heiko. If I don't see all the patches in the
patchset I start worrying if patchwork lost them, or something, and then
it takes more time from me to investigate what happened. So I strongly
recommend sending the whole series to everyone as it saves time.
Soeren Moch Dec. 10, 2019, 10:08 a.m. UTC | #5
Hi Heiko,

On 10.12.19 02:18, Heiko Stübner wrote:
> Hi Soeren,
>
> Am Dienstag, 10. Dezember 2019, 00:29:21 CET schrieb Soeren Moch:
>> On 10.12.19 00:08, Heiko Stübner wrote:
>>> Am Montag, 9. Dezember 2019, 23:38:22 CET schrieb Soeren Moch:
>>>> RockPro64 supports an Ampak AP6359SA based wifi/bt combo module.
>>>> The BCM4359/9 wifi controller in this module is connected to sdio0,
>>>> enable this interface.
>>>>
>>>> Signed-off-by: Soeren Moch <smoch@web.de>
>>>> ---
>>>> Not sure where to place exactly the sdio0 node in the dts because
>>>> existing sd nodes are not sorted alphabetically.
>>>>
>>>> This last patch in this brcmfmac patch series probably should be picked
>>>> up by Heiko independently of the rest of this series. It was sent together
>>>> to show how this brcmfmac extension for 4359-sdio support with RSDB is
>>>> used and tested.
>>> node placement looks good so I can apply it, just a general questions
>>> I only got patch 8/8 are patches 1-7 relevant for this one and what are they?
>> Patches 1-7 are the patches to support the BCM4359 chipset with SDIO
>> interface in the linux brcmfmac net-wireless driver, see [1].
>>
>> So this patch series has 2 parts:
>> patches 1-7: add support for the wifi chipset in the wireless driver,
>> this has to go through net-wireless
>> patch 8: enable the wifi module with this chipset on RockPro64, this patch
> Thanks for the clarification :-) .
>
> As patch 8 "only" does the core sdio node, it doesn't really depend on the
> earlier ones and you can submit any uart-hooks for bluetooth once the
> other patches land I guess.
The uart part for bluetooth already is in: uart0.
However, I haven't tested if it really works.
>> If this was confusing, what would be the ideal way to post such series?
> I think every maintainer has some slightly different perspective on this,
> but personally I like getting the whole series to follow the discussion but
> also to just see when the driver-side changes get merged, as the dts-parts
> need to wait for that in a lot of cases.
OK, thanks.
I will add you for the whole series when sending a v2.

Soeren
>
> Heiko
>
>
>> [1] https://patchwork.kernel.org/project/linux-wireless/list/?series=213951
>>> Thanks
>>> Heiko
>>>
>>>
>>>> Cc: Heiko Stuebner <heiko@sntech.de>
>>>> Cc: Kalle Valo <kvalo@codeaurora.org>
>>>> Cc: linux-wireless@vger.kernel.org
>>>> Cc: brcm80211-dev-list.pdl@broadcom.com
>>>> Cc: brcm80211-dev-list@cypress.com
>>>> Cc: netdev@vger.kernel.org
>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>> Cc: linux-rockchip@lists.infradead.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> ---
>>>>  .../boot/dts/rockchip/rk3399-rockpro64.dts    | 21 ++++++++++++-------
>>>>  1 file changed, 14 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>>>> index 7f4b2eba31d4..9fa92790d6e0 100644
>>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>>>> @@ -71,13 +71,6 @@
>>>>  		clock-names = "ext_clock";
>>>>  		pinctrl-names = "default";
>>>>  		pinctrl-0 = <&wifi_enable_h>;
>>>> -
>>>> -		/*
>>>> -		 * 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 = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
>>>>  	};
>>>>
>>>> @@ -650,6 +643,20 @@
>>>>  	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";
>>>> +};
>>>> +
>>>>  &sdmmc {
>>>>  	bus-width = <4>;
>>>>  	cap-sd-highspeed;
>>>> --
>>>> 2.17.1
>>>>
>>>
>>>
>>
>
>
>
Heiko Stuebner Dec. 10, 2019, 10:13 a.m. UTC | #6
Am Dienstag, 10. Dezember 2019, 11:08:18 CET schrieb Soeren Moch:
> Hi Heiko,
> 
> On 10.12.19 02:18, Heiko Stübner wrote:
> > Hi Soeren,
> >
> > Am Dienstag, 10. Dezember 2019, 00:29:21 CET schrieb Soeren Moch:
> >> On 10.12.19 00:08, Heiko Stübner wrote:
> >>> Am Montag, 9. Dezember 2019, 23:38:22 CET schrieb Soeren Moch:
> >>>> RockPro64 supports an Ampak AP6359SA based wifi/bt combo module.
> >>>> The BCM4359/9 wifi controller in this module is connected to sdio0,
> >>>> enable this interface.
> >>>>
> >>>> Signed-off-by: Soeren Moch <smoch@web.de>
> >>>> ---
> >>>> Not sure where to place exactly the sdio0 node in the dts because
> >>>> existing sd nodes are not sorted alphabetically.
> >>>>
> >>>> This last patch in this brcmfmac patch series probably should be picked
> >>>> up by Heiko independently of the rest of this series. It was sent together
> >>>> to show how this brcmfmac extension for 4359-sdio support with RSDB is
> >>>> used and tested.
> >>> node placement looks good so I can apply it, just a general questions
> >>> I only got patch 8/8 are patches 1-7 relevant for this one and what are they?
> >> Patches 1-7 are the patches to support the BCM4359 chipset with SDIO
> >> interface in the linux brcmfmac net-wireless driver, see [1].
> >>
> >> So this patch series has 2 parts:
> >> patches 1-7: add support for the wifi chipset in the wireless driver,
> >> this has to go through net-wireless
> >> patch 8: enable the wifi module with this chipset on RockPro64, this patch
> > Thanks for the clarification :-) .
> >
> > As patch 8 "only" does the core sdio node, it doesn't really depend on the
> > earlier ones and you can submit any uart-hooks for bluetooth once the
> > other patches land I guess.
> The uart part for bluetooth already is in: uart0.
> However, I haven't tested if it really works.

In the new world there is now also a way to actually hook the bt-uart to
the wifi driver without userspace intervention, and you might want to hook
up the interrupt as well for sdio? For example look at the rock960:

sdio-interrupt: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi#n510
uart-magic: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi#n557


Heiko

> >> If this was confusing, what would be the ideal way to post such series?
> > I think every maintainer has some slightly different perspective on this,
> > but personally I like getting the whole series to follow the discussion but
> > also to just see when the driver-side changes get merged, as the dts-parts
> > need to wait for that in a lot of cases.
> OK, thanks.
> I will add you for the whole series when sending a v2.
> 
> Soeren
> >
> > Heiko
> >
> >
> >> [1] https://patchwork.kernel.org/project/linux-wireless/list/?series=213951
> >>> Thanks
> >>> Heiko
> >>>
> >>>
> >>>> Cc: Heiko Stuebner <heiko@sntech.de>
> >>>> Cc: Kalle Valo <kvalo@codeaurora.org>
> >>>> Cc: linux-wireless@vger.kernel.org
> >>>> Cc: brcm80211-dev-list.pdl@broadcom.com
> >>>> Cc: brcm80211-dev-list@cypress.com
> >>>> Cc: netdev@vger.kernel.org
> >>>> Cc: linux-arm-kernel@lists.infradead.org
> >>>> Cc: linux-rockchip@lists.infradead.org
> >>>> Cc: linux-kernel@vger.kernel.org
> >>>> ---
> >>>>  .../boot/dts/rockchip/rk3399-rockpro64.dts    | 21 ++++++++++++-------
> >>>>  1 file changed, 14 insertions(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
> >>>> index 7f4b2eba31d4..9fa92790d6e0 100644
> >>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
> >>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
> >>>> @@ -71,13 +71,6 @@
> >>>>  		clock-names = "ext_clock";
> >>>>  		pinctrl-names = "default";
> >>>>  		pinctrl-0 = <&wifi_enable_h>;
> >>>> -
> >>>> -		/*
> >>>> -		 * 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 = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
> >>>>  	};
> >>>>
> >>>> @@ -650,6 +643,20 @@
> >>>>  	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";
> >>>> +};
> >>>> +
> >>>>  &sdmmc {
> >>>>  	bus-width = <4>;
> >>>>  	cap-sd-highspeed;
> >>>> --
> >>>> 2.17.1
> >>>>
> >>>
> >>>
> >>
> >
> >
> >
> 
>
Soeren Moch Dec. 10, 2019, 10:15 a.m. UTC | #7
On 10.12.19 10:14, Kalle Valo wrote:
> Heiko Stübner <heiko@sntech.de> writes:
>
>> Hi Soeren,
>>
>> Am Dienstag, 10. Dezember 2019, 00:29:21 CET schrieb Soeren Moch:
>>> On 10.12.19 00:08, Heiko Stübner wrote:
>>>> Am Montag, 9. Dezember 2019, 23:38:22 CET schrieb Soeren Moch:
>>>>> RockPro64 supports an Ampak AP6359SA based wifi/bt combo module.
>>>>> The BCM4359/9 wifi controller in this module is connected to sdio0,
>>>>> enable this interface.
>>>>>
>>>>> Signed-off-by: Soeren Moch <smoch@web.de>
>>>>> ---
>>>>> Not sure where to place exactly the sdio0 node in the dts because
>>>>> existing sd nodes are not sorted alphabetically.
>>>>>
>>>>> This last patch in this brcmfmac patch series probably should be picked
>>>>> up by Heiko independently of the rest of this series. It was sent together
>>>>> to show how this brcmfmac extension for 4359-sdio support with RSDB is
>>>>> used and tested.
>>>> node placement looks good so I can apply it, just a general questions
>>>> I only got patch 8/8 are patches 1-7 relevant for this one and what are they?
>>> Patches 1-7 are the patches to support the BCM4359 chipset with SDIO
>>> interface in the linux brcmfmac net-wireless driver, see [1].
>>>
>>> So this patch series has 2 parts:
>>> patches 1-7: add support for the wifi chipset in the wireless driver,
>>> this has to go through net-wireless
>>> patch 8: enable the wifi module with this chipset on RockPro64, this patch
>> Thanks for the clarification :-) .
>>
>> As patch 8 "only" does the core sdio node, it doesn't really depend on the
>> earlier ones and you can submit any uart-hooks for bluetooth once the
>> other patches land I guess.
>>
>>
>>> If this was confusing, what would be the ideal way to post such series?
>> I think every maintainer has some slightly different perspective on this,
>> but personally I like getting the whole series to follow the discussion but
>> also to just see when the driver-side changes get merged, as the dts-parts
>> need to wait for that in a lot of cases.
> FWIW I prefer the same as Heiko. If I don't see all the patches in the
> patchset I start worrying if patchwork lost them, or something, and then
> it takes more time from me to investigate what happened. So I strongly
> recommend sending the whole series to everyone as it saves time.
>
Thanks for your explanation.
I will keep this in mind for future submissions.

Soeren
Soeren Moch Dec. 11, 2019, 3:50 p.m. UTC | #8
re-send as plain text for mailing lists, sorry.

Soeren

On 11.12.19 16:43, Soeren Moch wrote:
>
>
> On 10.12.19 11:13, Heiko Stübner wrote:
>> Am Dienstag, 10. Dezember 2019, 11:08:18 CET schrieb Soeren Moch:
>>> Hi Heiko,
>>>
>>> On 10.12.19 02:18, Heiko Stübner wrote:
>>>> Hi Soeren,
>>>>
>>>> Am Dienstag, 10. Dezember 2019, 00:29:21 CET schrieb Soeren Moch:
>>>>> On 10.12.19 00:08, Heiko Stübner wrote:
>>>>>> Am Montag, 9. Dezember 2019, 23:38:22 CET schrieb Soeren Moch:
>>>>>>> RockPro64 supports an Ampak AP6359SA based wifi/bt combo module.
>>>>>>> The BCM4359/9 wifi controller in this module is connected to sdio0,
>>>>>>> enable this interface.
>>>>>>>
>>>>>>> Signed-off-by: Soeren Moch <smoch@web.de>
>>>>>>> ---
>>>>>>> Not sure where to place exactly the sdio0 node in the dts because
>>>>>>> existing sd nodes are not sorted alphabetically.
>>>>>>>
>>>>>>> This last patch in this brcmfmac patch series probably should be picked
>>>>>>> up by Heiko independently of the rest of this series. It was sent together
>>>>>>> to show how this brcmfmac extension for 4359-sdio support with RSDB is
>>>>>>> used and tested.
>>>>>> node placement looks good so I can apply it, just a general questions
>>>>>> I only got patch 8/8 are patches 1-7 relevant for this one and what are they?
>>>>> Patches 1-7 are the patches to support the BCM4359 chipset with SDIO
>>>>> interface in the linux brcmfmac net-wireless driver, see [1].
>>>>>
>>>>> So this patch series has 2 parts:
>>>>> patches 1-7: add support for the wifi chipset in the wireless driver,
>>>>> this has to go through net-wireless
>>>>> patch 8: enable the wifi module with this chipset on RockPro64, this patch
>>>> Thanks for the clarification :-) .
>>>>
>>>> As patch 8 "only" does the core sdio node, it doesn't really depend on the
>>>> earlier ones and you can submit any uart-hooks for bluetooth once the
>>>> other patches land I guess.
>>> The uart part for bluetooth already is in: uart0.
>>> However, I haven't tested if it really works.
>> In the new world there is now also a way to actually hook the bt-uart to
>> the wifi driver without userspace intervention, and you might want to hook
>> up the interrupt as well for sdio?
>>  For example look at the rock960:
> Thanks for the examples.
>> sdio-interrupt: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi#n510
> The signal name wifi_host_wake_l suggests that this is an active-low
> wake-up signal, probably used for wake-on-wifi. But in fact this is
> the active-high out-of-band sdio interrupt and can also be used as
> such on RockPro64 when following your example.
> However, with this external interrupt enabled  wifi runs unstable,
> maybe because board designers (probably confused by their own naming
> style) mixed in the PCI-Express-WAKE# signal to route this to the same
> GPIO.
>
> So I want to use the in-band sdio interrupt instead on RockPro64,
> which works perfectly fine.
> ||||
>> uart-magic: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399-rock960.dtsi#n557
> OK, people probably like to see Bluetooth support. I will add it.
>
> Soeren
>> Heiko
>>
>>>>> If this was confusing, what would be the ideal way to post such series?
>>>> I think every maintainer has some slightly different perspective on this,
>>>> but personally I like getting the whole series to follow the discussion but
>>>> also to just see when the driver-side changes get merged, as the dts-parts
>>>> need to wait for that in a lot of cases.
>>> OK, thanks.
>>> I will add you for the whole series when sending a v2.
>>>
>>> Soeren
>>>> Heiko
>>>>
>>>>
>>>>> [1] https://patchwork.kernel.org/project/linux-wireless/list/?series=213951
>>>>>> Thanks
>>>>>> Heiko
>>>>>>
>>>>>>
>>>>>>> Cc: Heiko Stuebner <heiko@sntech.de>
>>>>>>> Cc: Kalle Valo <kvalo@codeaurora.org>
>>>>>>> Cc: linux-wireless@vger.kernel.org
>>>>>>> Cc: brcm80211-dev-list.pdl@broadcom.com
>>>>>>> Cc: brcm80211-dev-list@cypress.com
>>>>>>> Cc: netdev@vger.kernel.org
>>>>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>>>>> Cc: linux-rockchip@lists.infradead.org
>>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>>> ---
>>>>>>>  .../boot/dts/rockchip/rk3399-rockpro64.dts    | 21 ++++++++++++-------
>>>>>>>  1 file changed, 14 insertions(+), 7 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>>>>>>> index 7f4b2eba31d4..9fa92790d6e0 100644
>>>>>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>>>>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
>>>>>>> @@ -71,13 +71,6 @@
>>>>>>>  		clock-names = "ext_clock";
>>>>>>>  		pinctrl-names = "default";
>>>>>>>  		pinctrl-0 = <&wifi_enable_h>;
>>>>>>> -
>>>>>>> -		/*
>>>>>>> -		 * 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 = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
>>>>>>>  	};
>>>>>>>
>>>>>>> @@ -650,6 +643,20 @@
>>>>>>>  	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";
>>>>>>> +};
>>>>>>> +
>>>>>>>  &sdmmc {
>>>>>>>  	bus-width = <4>;
>>>>>>>  	cap-sd-highspeed;
>>>>>>> --
>>>>>>> 2.17.1
>>>>>>>
>>
>
Johan Jonker Dec. 16, 2019, 1:16 p.m. UTC | #9
Hi Soeren,

> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
> index 7f4b2eba31d4..9fa92790d6e0 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts

> +&sdio0 {
> +	bus-width = <4>;
> +	cap-sd-highspeed;
> +	cap-sdio-irq;

> +	disable-wp;

The mmc-controller.yaml didn't explicitly say disable-wp is for SD card
slot only, but that is what it was designed for in the first place.
Remove all disable-wp from emmc or sdio controllers.

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

Patch
diff mbox series

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
index 7f4b2eba31d4..9fa92790d6e0 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
@@ -71,13 +71,6 @@ 
 		clock-names = "ext_clock";
 		pinctrl-names = "default";
 		pinctrl-0 = <&wifi_enable_h>;
-
-		/*
-		 * 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 = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
 	};

@@ -650,6 +643,20 @@ 
 	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";
+};
+
 &sdmmc {
 	bus-width = <4>;
 	cap-sd-highspeed;