During "wlan-up", we are programming the FW into the WiFi-chip. However,
re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
is made in-between the programmings.
To conform to this requirement and to fix the regression in a simple way,
let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
(runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
current code is to treat this scenario as an error, but unfortunate this
doesn't work as expected, so let's fix this.
The other part is to guarantee that a power cycle of the SDIO card has been
completed when wl12xx_sdio_power_on() returns, as to allow the FW
programming to succeed. However, relying solely on runtime PM to deal with
this isn't sufficient. For example, userspace may prevent runtime suspend
via sysfs for the device that represents the SDIO card, leading to that the
mmc core also keeps it powered on. For this reason, let's instead do a
brute force power cycle in wl12xx_sdio_power_on().
Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
Signed-off-by: Ulf Hansson <[email protected]>
---
Changes in v2:
- Keep the SDIO host claimed when calling mmc_hw_reset().
- Add a fixes tag.
---
drivers/net/wireless/ti/wlcore/sdio.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
index bd10165d7eec..4d4b07701149 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -164,6 +164,12 @@ static int wl12xx_sdio_power_on(struct wl12xx_sdio_glue *glue)
}
sdio_claim_host(func);
+ /*
+ * To guarantee that the SDIO card is power cycled, as required to make
+ * the FW programming to succeed, let's do a brute force HW reset.
+ */
+ mmc_hw_reset(card->host);
+
sdio_enable_func(func);
sdio_release_host(func);
@@ -174,20 +180,13 @@ static int wl12xx_sdio_power_off(struct wl12xx_sdio_glue *glue)
{
struct sdio_func *func = dev_to_sdio_func(glue->dev);
struct mmc_card *card = func->card;
- int error;
sdio_claim_host(func);
sdio_disable_func(func);
sdio_release_host(func);
/* Let runtime PM know the card is powered off */
- error = pm_runtime_put(&card->dev);
- if (error < 0 && error != -EBUSY) {
- dev_err(&card->dev, "%s failed: %i\n", __func__, error);
-
- return error;
- }
-
+ pm_runtime_put(&card->dev);
return 0;
}
--
2.17.1
* Ulf Hansson <[email protected]> [190116 11:37]:
> During "wlan-up", we are programming the FW into the WiFi-chip. However,
> re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
> is made in-between the programmings.
>
> To conform to this requirement and to fix the regression in a simple way,
> let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
> (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
> current code is to treat this scenario as an error, but unfortunate this
> doesn't work as expected, so let's fix this.
>
> The other part is to guarantee that a power cycle of the SDIO card has been
> completed when wl12xx_sdio_power_on() returns, as to allow the FW
> programming to succeed. However, relying solely on runtime PM to deal with
> this isn't sufficient. For example, userspace may prevent runtime suspend
> via sysfs for the device that represents the SDIO card, leading to that the
> mmc core also keeps it powered on. For this reason, let's instead do a
> brute force power cycle in wl12xx_sdio_power_on().
>
> Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
> Signed-off-by: Ulf Hansson <[email protected]>
> ---
>
> Changes in v2:
> - Keep the SDIO host claimed when calling mmc_hw_reset().
> - Add a fixes tag.
This v2 version works for me as tested with:
# while [ 1 ]; do ifconfig wlan0 down; ifconfig wlan0 up; done
[ 181.364990] wlcore: down
[ 182.116424] wlcore: firmware booted (Rev 6.3.10.0.141)
[ 182.151641] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
[ 182.166778] wlcore: down
[ 182.773132] wlcore: firmware booted (Rev 6.3.10.0.141)
[ 182.811096] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
...
Thanks for fixing this issue properly, and feel free to add:
Tested-by: Tony Lindgren <[email protected]>
Ulf Hansson <[email protected]> writes:
> On Wed, 16 Jan 2019 at 16:43, Tony Lindgren <[email protected]> wrote:
>>
>> * Ulf Hansson <[email protected]> [190116 11:37]:
>> > During "wlan-up", we are programming the FW into the WiFi-chip. However,
>> > re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
>> > is made in-between the programmings.
>> >
>> > To conform to this requirement and to fix the regression in a simple way,
>> > let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
>> > (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
>> > current code is to treat this scenario as an error, but unfortunate this
>> > doesn't work as expected, so let's fix this.
>> >
>> > The other part is to guarantee that a power cycle of the SDIO card has been
>> > completed when wl12xx_sdio_power_on() returns, as to allow the FW
>> > programming to succeed. However, relying solely on runtime PM to deal with
>> > this isn't sufficient. For example, userspace may prevent runtime suspend
>> > via sysfs for the device that represents the SDIO card, leading to that the
>> > mmc core also keeps it powered on. For this reason, let's instead do a
>> > brute force power cycle in wl12xx_sdio_power_on().
>> >
>> > Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
>> > Signed-off-by: Ulf Hansson <[email protected]>
>> > ---
>> >
>> > Changes in v2:
>> > - Keep the SDIO host claimed when calling mmc_hw_reset().
>> > - Add a fixes tag.
>>
>> This v2 version works for me as tested with:
>>
>> # while [ 1 ]; do ifconfig wlan0 down; ifconfig wlan0 up; done
>> [ 181.364990] wlcore: down
>> [ 182.116424] wlcore: firmware booted (Rev 6.3.10.0.141)
>> [ 182.151641] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
>> [ 182.166778] wlcore: down
>> [ 182.773132] wlcore: firmware booted (Rev 6.3.10.0.141)
>> [ 182.811096] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
>> ...
>>
>> Thanks for fixing this issue properly, and feel free to add:
>>
>> Tested-by: Tony Lindgren <[email protected]>
>
> Thanks for testing, great news!
>
> Before queuing this as fix, let's also see what the Hikey folkz thinks
> of this. I have pinged Anders and he is currently running a test
> suite.
>
> Kalle, can you please tag this for stable as well?
Ok, I'll queue this for 5.0 (but wait for testing feedback at least a
day) and add:
Cc: <[email protected]>
--
Kalle Valo
On 16.01.19 12:37, Ulf Hansson wrote:
> During "wlan-up", we are programming the FW into the WiFi-chip. However,
> re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
> is made in-between the programmings.
>
> To conform to this requirement and to fix the regression in a simple way,
> let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
> (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
> current code is to treat this scenario as an error, but unfortunate this
> doesn't work as expected, so let's fix this.
>
> The other part is to guarantee that a power cycle of the SDIO card has been
> completed when wl12xx_sdio_power_on() returns, as to allow the FW
> programming to succeed. However, relying solely on runtime PM to deal with
> this isn't sufficient. For example, userspace may prevent runtime suspend
> via sysfs for the device that represents the SDIO card, leading to that the
> mmc core also keeps it powered on. For this reason, let's instead do a
> brute force power cycle in wl12xx_sdio_power_on().
>
> Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
> Signed-off-by: Ulf Hansson <[email protected]>
> ---
>
> Changes in v2:
> - Keep the SDIO host claimed when calling mmc_hw_reset().
> - Add a fixes tag.
> ---
> drivers/net/wireless/ti/wlcore/sdio.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
> index bd10165d7eec..4d4b07701149 100644
> --- a/drivers/net/wireless/ti/wlcore/sdio.c
> +++ b/drivers/net/wireless/ti/wlcore/sdio.c
> @@ -164,6 +164,12 @@ static int wl12xx_sdio_power_on(struct wl12xx_sdio_glue *glue)
> }
>
> sdio_claim_host(func);
> + /*
> + * To guarantee that the SDIO card is power cycled, as required to make
> + * the FW programming to succeed, let's do a brute force HW reset.
> + */
> + mmc_hw_reset(card->host);
> +
> sdio_enable_func(func);
> sdio_release_host(func);
>
> @@ -174,20 +180,13 @@ static int wl12xx_sdio_power_off(struct wl12xx_sdio_glue *glue)
> {
> struct sdio_func *func = dev_to_sdio_func(glue->dev);
> struct mmc_card *card = func->card;
> - int error;
>
> sdio_claim_host(func);
> sdio_disable_func(func);
> sdio_release_host(func);
>
> /* Let runtime PM know the card is powered off */
> - error = pm_runtime_put(&card->dev);
> - if (error < 0 && error != -EBUSY) {
> - dev_err(&card->dev, "%s failed: %i\n", __func__, error);
> -
> - return error;
> - }
> -
> + pm_runtime_put(&card->dev);
> return 0;
> }
>
>
Just tested on both HiKey (620) and Ultra96 but it fails to fix the issue on
both. I'm getting
wl1271_sdio: probe of mmc2:0001:1 failed with error -16
during boot again, and the interface is not available.
Jan
On Wed, 16 Jan 2019 at 16:43, Tony Lindgren <[email protected]> wrote:
>
> * Ulf Hansson <[email protected]> [190116 11:37]:
> > During "wlan-up", we are programming the FW into the WiFi-chip. However,
> > re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
> > is made in-between the programmings.
> >
> > To conform to this requirement and to fix the regression in a simple way,
> > let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
> > (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
> > current code is to treat this scenario as an error, but unfortunate this
> > doesn't work as expected, so let's fix this.
> >
> > The other part is to guarantee that a power cycle of the SDIO card has been
> > completed when wl12xx_sdio_power_on() returns, as to allow the FW
> > programming to succeed. However, relying solely on runtime PM to deal with
> > this isn't sufficient. For example, userspace may prevent runtime suspend
> > via sysfs for the device that represents the SDIO card, leading to that the
> > mmc core also keeps it powered on. For this reason, let's instead do a
> > brute force power cycle in wl12xx_sdio_power_on().
> >
> > Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
> > Signed-off-by: Ulf Hansson <[email protected]>
> > ---
> >
> > Changes in v2:
> > - Keep the SDIO host claimed when calling mmc_hw_reset().
> > - Add a fixes tag.
>
> This v2 version works for me as tested with:
>
> # while [ 1 ]; do ifconfig wlan0 down; ifconfig wlan0 up; done
> [ 181.364990] wlcore: down
> [ 182.116424] wlcore: firmware booted (Rev 6.3.10.0.141)
> [ 182.151641] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
> [ 182.166778] wlcore: down
> [ 182.773132] wlcore: firmware booted (Rev 6.3.10.0.141)
> [ 182.811096] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
> ...
>
> Thanks for fixing this issue properly, and feel free to add:
>
> Tested-by: Tony Lindgren <[email protected]>
Thanks for testing, great news!
Before queuing this as fix, let's also see what the Hikey folkz thinks
of this. I have pinged Anders and he is currently running a test
suite.
Kalle, can you please tag this for stable as well?
Kind regards
Uffe
On Wed, 16 Jan 2019 at 21:26, Jan Kiszka <[email protected]> wrote:
>
> On 16.01.19 12:37, Ulf Hansson wrote:
> > During "wlan-up", we are programming the FW into the WiFi-chip. However,
> > re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
> > is made in-between the programmings.
> >
> > To conform to this requirement and to fix the regression in a simple way,
> > let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
> > (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
> > current code is to treat this scenario as an error, but unfortunate this
> > doesn't work as expected, so let's fix this.
> >
> > The other part is to guarantee that a power cycle of the SDIO card has been
> > completed when wl12xx_sdio_power_on() returns, as to allow the FW
> > programming to succeed. However, relying solely on runtime PM to deal with
> > this isn't sufficient. For example, userspace may prevent runtime suspend
> > via sysfs for the device that represents the SDIO card, leading to that the
> > mmc core also keeps it powered on. For this reason, let's instead do a
> > brute force power cycle in wl12xx_sdio_power_on().
> >
> > Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
> > Signed-off-by: Ulf Hansson <[email protected]>
> > ---
> >
> > Changes in v2:
> > - Keep the SDIO host claimed when calling mmc_hw_reset().
> > - Add a fixes tag.
> > ---
> > drivers/net/wireless/ti/wlcore/sdio.c | 15 +++++++--------
> > 1 file changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
> > index bd10165d7eec..4d4b07701149 100644
> > --- a/drivers/net/wireless/ti/wlcore/sdio.c
> > +++ b/drivers/net/wireless/ti/wlcore/sdio.c
> > @@ -164,6 +164,12 @@ static int wl12xx_sdio_power_on(struct wl12xx_sdio_glue *glue)
> > }
> >
> > sdio_claim_host(func);
> > + /*
> > + * To guarantee that the SDIO card is power cycled, as required to make
> > + * the FW programming to succeed, let's do a brute force HW reset.
> > + */
> > + mmc_hw_reset(card->host);
> > +
> > sdio_enable_func(func);
> > sdio_release_host(func);
> >
> > @@ -174,20 +180,13 @@ static int wl12xx_sdio_power_off(struct wl12xx_sdio_glue *glue)
> > {
> > struct sdio_func *func = dev_to_sdio_func(glue->dev);
> > struct mmc_card *card = func->card;
> > - int error;
> >
> > sdio_claim_host(func);
> > sdio_disable_func(func);
> > sdio_release_host(func);
> >
> > /* Let runtime PM know the card is powered off */
> > - error = pm_runtime_put(&card->dev);
> > - if (error < 0 && error != -EBUSY) {
> > - dev_err(&card->dev, "%s failed: %i\n", __func__, error);
> > -
> > - return error;
> > - }
> > -
> > + pm_runtime_put(&card->dev);
> > return 0;
> > }
> >
> >
>
> Just tested on both HiKey (620) and Ultra96 but it fails to fix the issue on
> both. I'm getting
>
> wl1271_sdio: probe of mmc2:0001:1 failed with error -16
>
> during boot again, and the interface is not available.
Okay, sounds like this may be a different problem then. Can you share
the complete log and the kernel config?
I can prepare a debug patch as well, if you are willing to re-run the test?
Adding a post-power-on-delay-ms of 1 ms as you suggested [1], doesn't
sounds like the correct solution to me, unless I am overlooking some
things. The point is, since the mmc core succeeds to detect and
initialize the SDIO card, the power sequence seems to be correct.
Kind regards
Uffe
[1]
https://patchwork.kernel.org/patch/10745075/
On 17.01.19 10:54, Ulf Hansson wrote:
> On Wed, 16 Jan 2019 at 21:26, Jan Kiszka <[email protected]> wrote:
>>
>> On 16.01.19 12:37, Ulf Hansson wrote:
>>> During "wlan-up", we are programming the FW into the WiFi-chip. However,
>>> re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
>>> is made in-between the programmings.
>>>
>>> To conform to this requirement and to fix the regression in a simple way,
>>> let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
>>> (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
>>> current code is to treat this scenario as an error, but unfortunate this
>>> doesn't work as expected, so let's fix this.
>>>
>>> The other part is to guarantee that a power cycle of the SDIO card has been
>>> completed when wl12xx_sdio_power_on() returns, as to allow the FW
>>> programming to succeed. However, relying solely on runtime PM to deal with
>>> this isn't sufficient. For example, userspace may prevent runtime suspend
>>> via sysfs for the device that represents the SDIO card, leading to that the
>>> mmc core also keeps it powered on. For this reason, let's instead do a
>>> brute force power cycle in wl12xx_sdio_power_on().
>>>
>>> Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
>>> Signed-off-by: Ulf Hansson <[email protected]>
>>> ---
>>>
>>> Changes in v2:
>>> - Keep the SDIO host claimed when calling mmc_hw_reset().
>>> - Add a fixes tag.
>>> ---
>>> drivers/net/wireless/ti/wlcore/sdio.c | 15 +++++++--------
>>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
>>> index bd10165d7eec..4d4b07701149 100644
>>> --- a/drivers/net/wireless/ti/wlcore/sdio.c
>>> +++ b/drivers/net/wireless/ti/wlcore/sdio.c
>>> @@ -164,6 +164,12 @@ static int wl12xx_sdio_power_on(struct wl12xx_sdio_glue *glue)
>>> }
>>>
>>> sdio_claim_host(func);
>>> + /*
>>> + * To guarantee that the SDIO card is power cycled, as required to make
>>> + * the FW programming to succeed, let's do a brute force HW reset.
>>> + */
>>> + mmc_hw_reset(card->host);
>>> +
>>> sdio_enable_func(func);
>>> sdio_release_host(func);
>>>
>>> @@ -174,20 +180,13 @@ static int wl12xx_sdio_power_off(struct wl12xx_sdio_glue *glue)
>>> {
>>> struct sdio_func *func = dev_to_sdio_func(glue->dev);
>>> struct mmc_card *card = func->card;
>>> - int error;
>>>
>>> sdio_claim_host(func);
>>> sdio_disable_func(func);
>>> sdio_release_host(func);
>>>
>>> /* Let runtime PM know the card is powered off */
>>> - error = pm_runtime_put(&card->dev);
>>> - if (error < 0 && error != -EBUSY) {
>>> - dev_err(&card->dev, "%s failed: %i\n", __func__, error);
>>> -
>>> - return error;
>>> - }
>>> -
>>> + pm_runtime_put(&card->dev);
>>> return 0;
>>> }
>>>
>>>
>>
>> Just tested on both HiKey (620) and Ultra96 but it fails to fix the issue on
>> both. I'm getting
>>
>> wl1271_sdio: probe of mmc2:0001:1 failed with error -16
>>
>> during boot again, and the interface is not available.
>
> Okay, sounds like this may be a different problem then. Can you share
> the complete log and the kernel config?
You can find the config here [1], log from the HiKey boot attached.
> I can prepare a debug patch as well, if you are willing to re-run the test?
Sure, send it over, I can run it.
>
> Adding a post-power-on-delay-ms of 1 ms as you suggested [1], doesn't
> sounds like the correct solution to me, unless I am overlooking some
> things. The point is, since the mmc core succeeds to detect and
> initialize the SDIO card, the power sequence seems to be correct.
Yeah, I'm not claiming at all I know what I'm doing there, just that it happens
to work.
Jan
[1]
https://github.com/siemens/jailhouse-images/blob/next/recipes-kernel/linux/files/arm64_defconfig_4.19
On Wed, 16 Jan 2019 at 16:43, Tony Lindgren <[email protected]> wrote:
>
> * Ulf Hansson <[email protected]> [190116 11:37]:
> > During "wlan-up", we are programming the FW into the WiFi-chip. However,
> > re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
> > is made in-between the programmings.
> >
> > To conform to this requirement and to fix the regression in a simple way,
> > let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
> > (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
> > current code is to treat this scenario as an error, but unfortunate this
> > doesn't work as expected, so let's fix this.
> >
> > The other part is to guarantee that a power cycle of the SDIO card has been
> > completed when wl12xx_sdio_power_on() returns, as to allow the FW
> > programming to succeed. However, relying solely on runtime PM to deal with
> > this isn't sufficient. For example, userspace may prevent runtime suspend
> > via sysfs for the device that represents the SDIO card, leading to that the
> > mmc core also keeps it powered on. For this reason, let's instead do a
> > brute force power cycle in wl12xx_sdio_power_on().
> >
> > Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
> > Signed-off-by: Ulf Hansson <[email protected]>
> > ---
> >
> > Changes in v2:
> > - Keep the SDIO host claimed when calling mmc_hw_reset().
> > - Add a fixes tag.
>
> This v2 version works for me as tested with:
>
> # while [ 1 ]; do ifconfig wlan0 down; ifconfig wlan0 up; done
> [ 181.364990] wlcore: down
> [ 182.116424] wlcore: firmware booted (Rev 6.3.10.0.141)
> [ 182.151641] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
> [ 182.166778] wlcore: down
> [ 182.773132] wlcore: firmware booted (Rev 6.3.10.0.141)
> [ 182.811096] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
> ...
>
> Thanks for fixing this issue properly, and feel free to add:
>
> Tested-by: Tony Lindgren <[email protected]>
Tested-by: Anders Roxell <[email protected]>
I tested it on a hikey-6220, and it worked.
Cheers,
Anders
On Fri, 18 Jan 2019 at 13:09, Jan Kiszka <[email protected]> wrote:
>
> On 17.01.19 10:54, Ulf Hansson wrote:
> > On Wed, 16 Jan 2019 at 21:26, Jan Kiszka <[email protected]> wrote:
> >>
> >> On 16.01.19 12:37, Ulf Hansson wrote:
> >>> During "wlan-up", we are programming the FW into the WiFi-chip. However,
> >>> re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
> >>> is made in-between the programmings.
> >>>
> >>> To conform to this requirement and to fix the regression in a simple way,
> >>> let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
> >>> (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
> >>> current code is to treat this scenario as an error, but unfortunate this
> >>> doesn't work as expected, so let's fix this.
> >>>
> >>> The other part is to guarantee that a power cycle of the SDIO card has been
> >>> completed when wl12xx_sdio_power_on() returns, as to allow the FW
> >>> programming to succeed. However, relying solely on runtime PM to deal with
> >>> this isn't sufficient. For example, userspace may prevent runtime suspend
> >>> via sysfs for the device that represents the SDIO card, leading to that the
> >>> mmc core also keeps it powered on. For this reason, let's instead do a
> >>> brute force power cycle in wl12xx_sdio_power_on().
> >>>
> >>> Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
> >>> Signed-off-by: Ulf Hansson <[email protected]>
> >>> ---
> >>>
> >>> Changes in v2:
> >>> - Keep the SDIO host claimed when calling mmc_hw_reset().
> >>> - Add a fixes tag.
> >>> ---
> >>> drivers/net/wireless/ti/wlcore/sdio.c | 15 +++++++--------
> >>> 1 file changed, 7 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
> >>> index bd10165d7eec..4d4b07701149 100644
> >>> --- a/drivers/net/wireless/ti/wlcore/sdio.c
> >>> +++ b/drivers/net/wireless/ti/wlcore/sdio.c
> >>> @@ -164,6 +164,12 @@ static int wl12xx_sdio_power_on(struct wl12xx_sdio_glue *glue)
> >>> }
> >>>
> >>> sdio_claim_host(func);
> >>> + /*
> >>> + * To guarantee that the SDIO card is power cycled, as required to make
> >>> + * the FW programming to succeed, let's do a brute force HW reset.
> >>> + */
> >>> + mmc_hw_reset(card->host);
> >>> +
> >>> sdio_enable_func(func);
> >>> sdio_release_host(func);
> >>>
> >>> @@ -174,20 +180,13 @@ static int wl12xx_sdio_power_off(struct wl12xx_sdio_glue *glue)
> >>> {
> >>> struct sdio_func *func = dev_to_sdio_func(glue->dev);
> >>> struct mmc_card *card = func->card;
> >>> - int error;
> >>>
> >>> sdio_claim_host(func);
> >>> sdio_disable_func(func);
> >>> sdio_release_host(func);
> >>>
> >>> /* Let runtime PM know the card is powered off */
> >>> - error = pm_runtime_put(&card->dev);
> >>> - if (error < 0 && error != -EBUSY) {
> >>> - dev_err(&card->dev, "%s failed: %i\n", __func__, error);
> >>> -
> >>> - return error;
> >>> - }
> >>> -
> >>> + pm_runtime_put(&card->dev);
> >>> return 0;
> >>> }
> >>>
> >>>
> >>
> >> Just tested on both HiKey (620) and Ultra96 but it fails to fix the issue on
> >> both. I'm getting
> >>
> >> wl1271_sdio: probe of mmc2:0001:1 failed with error -16
> >>
> >> during boot again, and the interface is not available.
> >
> > Okay, sounds like this may be a different problem then. Can you share
> > the complete log and the kernel config?
>
> You can find the config here [1], log from the HiKey boot attached.
>
> > I can prepare a debug patch as well, if you are willing to re-run the test?
>
> Sure, send it over, I can run it.
Alright, sounds great. However, I need to defer that to Monday/Tuesday
next week.
>
> >
> > Adding a post-power-on-delay-ms of 1 ms as you suggested [1], doesn't
> > sounds like the correct solution to me, unless I am overlooking some
> > things. The point is, since the mmc core succeeds to detect and
> > initialize the SDIO card, the power sequence seems to be correct.
>
> Yeah, I'm not claiming at all I know what I'm doing there, just that it happens
> to work.
I see. Good to know, thanks!
>
> Jan
>
> [1]
> https://github.com/siemens/jailhouse-images/blob/next/recipes-kernel/linux/files/arm64_defconfig_4.19
I have looked through the log and the defconfig. No obvious things
found at this point. Thanks for sharing them!
Kind regards
Uffe
* Ulf Hansson <[email protected]> [190118 15:09]:
> On Fri, 18 Jan 2019 at 13:09, Jan Kiszka <[email protected]> wrote:
> > Yeah, I'm not claiming at all I know what I'm doing there, just that it happens
> > to work.
>
> I see. Good to know, thanks!
This seems like some separate issue. I wonder if adding
another reset just before pm_runtime_put() clears this
one instead of using the msleep? Just guessing..
Regards,
Tony
Anders Roxell <[email protected]> writes:
> On Wed, 16 Jan 2019 at 16:43, Tony Lindgren <[email protected]> wrote:
>>
>> * Ulf Hansson <[email protected]> [190116 11:37]:
>> > During "wlan-up", we are programming the FW into the WiFi-chip. However,
>> > re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
>> > is made in-between the programmings.
>> >
>> > To conform to this requirement and to fix the regression in a simple way,
>> > let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
>> > (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
>> > current code is to treat this scenario as an error, but unfortunate this
>> > doesn't work as expected, so let's fix this.
>> >
>> > The other part is to guarantee that a power cycle of the SDIO card has been
>> > completed when wl12xx_sdio_power_on() returns, as to allow the FW
>> > programming to succeed. However, relying solely on runtime PM to deal with
>> > this isn't sufficient. For example, userspace may prevent runtime suspend
>> > via sysfs for the device that represents the SDIO card, leading to that the
>> > mmc core also keeps it powered on. For this reason, let's instead do a
>> > brute force power cycle in wl12xx_sdio_power_on().
>> >
>> > Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
>> > Signed-off-by: Ulf Hansson <[email protected]>
>> > ---
>> >
>> > Changes in v2:
>> > - Keep the SDIO host claimed when calling mmc_hw_reset().
>> > - Add a fixes tag.
>>
>> This v2 version works for me as tested with:
>>
>> # while [ 1 ]; do ifconfig wlan0 down; ifconfig wlan0 up; done
>> [ 181.364990] wlcore: down
>> [ 182.116424] wlcore: firmware booted (Rev 6.3.10.0.141)
>> [ 182.151641] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
>> [ 182.166778] wlcore: down
>> [ 182.773132] wlcore: firmware booted (Rev 6.3.10.0.141)
>> [ 182.811096] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
>> ...
>>
>> Thanks for fixing this issue properly, and feel free to add:
>>
>> Tested-by: Tony Lindgren <[email protected]>
>
> Tested-by: Anders Roxell <[email protected]>
>
> I tested it on a hikey-6220, and it worked.
So what's the conclusion, can I take this patch? I see that this didn't
help with Jan but as Tony and Anders provided positive test results I'm
inclined to take this now and Jan's problem can be fixed with another
patch. Do everyone agree?
--
Kalle Valo
* Kalle Valo <[email protected]> [190118 15:37]:
> Anders Roxell <[email protected]> writes:
>
> > On Wed, 16 Jan 2019 at 16:43, Tony Lindgren <[email protected]> wrote:
> >>
> >> * Ulf Hansson <[email protected]> [190116 11:37]:
> >> > During "wlan-up", we are programming the FW into the WiFi-chip. However,
> >> > re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
> >> > is made in-between the programmings.
> >> >
> >> > To conform to this requirement and to fix the regression in a simple way,
> >> > let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
> >> > (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
> >> > current code is to treat this scenario as an error, but unfortunate this
> >> > doesn't work as expected, so let's fix this.
> >> >
> >> > The other part is to guarantee that a power cycle of the SDIO card has been
> >> > completed when wl12xx_sdio_power_on() returns, as to allow the FW
> >> > programming to succeed. However, relying solely on runtime PM to deal with
> >> > this isn't sufficient. For example, userspace may prevent runtime suspend
> >> > via sysfs for the device that represents the SDIO card, leading to that the
> >> > mmc core also keeps it powered on. For this reason, let's instead do a
> >> > brute force power cycle in wl12xx_sdio_power_on().
> >> >
> >> > Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
> >> > Signed-off-by: Ulf Hansson <[email protected]>
> >> > ---
> >> >
> >> > Changes in v2:
> >> > - Keep the SDIO host claimed when calling mmc_hw_reset().
> >> > - Add a fixes tag.
> >>
> >> This v2 version works for me as tested with:
> >>
> >> # while [ 1 ]; do ifconfig wlan0 down; ifconfig wlan0 up; done
> >> [ 181.364990] wlcore: down
> >> [ 182.116424] wlcore: firmware booted (Rev 6.3.10.0.141)
> >> [ 182.151641] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
> >> [ 182.166778] wlcore: down
> >> [ 182.773132] wlcore: firmware booted (Rev 6.3.10.0.141)
> >> [ 182.811096] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
> >> ...
> >>
> >> Thanks for fixing this issue properly, and feel free to add:
> >>
> >> Tested-by: Tony Lindgren <[email protected]>
> >
> > Tested-by: Anders Roxell <[email protected]>
> >
> > I tested it on a hikey-6220, and it worked.
>
> So what's the conclusion, can I take this patch? I see that this didn't
> help with Jan but as Tony and Anders provided positive test results I'm
> inclined to take this now and Jan's problem can be fixed with another
> patch. Do everyone agree?
Sounds good to me, seems like there is another separate issue
lurking around somewhere.
Regards,
Tony
On Fri, 18 Jan 2019 at 16:09, Ulf Hansson <[email protected]> wrote:
>
> On Fri, 18 Jan 2019 at 13:09, Jan Kiszka <[email protected]> wrote:
> >
> > On 17.01.19 10:54, Ulf Hansson wrote:
> > > On Wed, 16 Jan 2019 at 21:26, Jan Kiszka <[email protected]> wrote:
> > >>
> > >> On 16.01.19 12:37, Ulf Hansson wrote:
> > >>> During "wlan-up", we are programming the FW into the WiFi-chip. However,
> > >>> re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
> > >>> is made in-between the programmings.
> > >>>
> > >>> To conform to this requirement and to fix the regression in a simple way,
> > >>> let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
> > >>> (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
> > >>> current code is to treat this scenario as an error, but unfortunate this
> > >>> doesn't work as expected, so let's fix this.
> > >>>
> > >>> The other part is to guarantee that a power cycle of the SDIO card has been
> > >>> completed when wl12xx_sdio_power_on() returns, as to allow the FW
> > >>> programming to succeed. However, relying solely on runtime PM to deal with
> > >>> this isn't sufficient. For example, userspace may prevent runtime suspend
> > >>> via sysfs for the device that represents the SDIO card, leading to that the
> > >>> mmc core also keeps it powered on. For this reason, let's instead do a
> > >>> brute force power cycle in wl12xx_sdio_power_on().
> > >>>
> > >>> Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
> > >>> Signed-off-by: Ulf Hansson <[email protected]>
> > >>> ---
> > >>>
> > >>> Changes in v2:
> > >>> - Keep the SDIO host claimed when calling mmc_hw_reset().
> > >>> - Add a fixes tag.
> > >>> ---
> > >>> drivers/net/wireless/ti/wlcore/sdio.c | 15 +++++++--------
> > >>> 1 file changed, 7 insertions(+), 8 deletions(-)
> > >>>
> > >>> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
> > >>> index bd10165d7eec..4d4b07701149 100644
> > >>> --- a/drivers/net/wireless/ti/wlcore/sdio.c
> > >>> +++ b/drivers/net/wireless/ti/wlcore/sdio.c
> > >>> @@ -164,6 +164,12 @@ static int wl12xx_sdio_power_on(struct wl12xx_sdio_glue *glue)
> > >>> }
> > >>>
> > >>> sdio_claim_host(func);
> > >>> + /*
> > >>> + * To guarantee that the SDIO card is power cycled, as required to make
> > >>> + * the FW programming to succeed, let's do a brute force HW reset.
> > >>> + */
> > >>> + mmc_hw_reset(card->host);
> > >>> +
> > >>> sdio_enable_func(func);
> > >>> sdio_release_host(func);
> > >>>
> > >>> @@ -174,20 +180,13 @@ static int wl12xx_sdio_power_off(struct wl12xx_sdio_glue *glue)
> > >>> {
> > >>> struct sdio_func *func = dev_to_sdio_func(glue->dev);
> > >>> struct mmc_card *card = func->card;
> > >>> - int error;
> > >>>
> > >>> sdio_claim_host(func);
> > >>> sdio_disable_func(func);
> > >>> sdio_release_host(func);
> > >>>
> > >>> /* Let runtime PM know the card is powered off */
> > >>> - error = pm_runtime_put(&card->dev);
> > >>> - if (error < 0 && error != -EBUSY) {
> > >>> - dev_err(&card->dev, "%s failed: %i\n", __func__, error);
> > >>> -
> > >>> - return error;
> > >>> - }
> > >>> -
> > >>> + pm_runtime_put(&card->dev);
> > >>> return 0;
> > >>> }
> > >>>
> > >>>
> > >>
> > >> Just tested on both HiKey (620) and Ultra96 but it fails to fix the issue on
> > >> both. I'm getting
> > >>
> > >> wl1271_sdio: probe of mmc2:0001:1 failed with error -16
> > >>
> > >> during boot again, and the interface is not available.
> > >
> > > Okay, sounds like this may be a different problem then. Can you share
> > > the complete log and the kernel config?
> >
> > You can find the config here [1], log from the HiKey boot attached.
> >
> > > I can prepare a debug patch as well, if you are willing to re-run the test?
> >
> > Sure, send it over, I can run it.
>
> Alright, sounds great. However, I need to defer that to Monday/Tuesday
> next week.
>
> >
> > >
> > > Adding a post-power-on-delay-ms of 1 ms as you suggested [1], doesn't
> > > sounds like the correct solution to me, unless I am overlooking some
> > > things. The point is, since the mmc core succeeds to detect and
> > > initialize the SDIO card, the power sequence seems to be correct.
> >
> > Yeah, I'm not claiming at all I know what I'm doing there, just that it happens
> > to work.
>
> I see. Good to know, thanks!
>
> >
> > Jan
> >
> > [1]
> > https://github.com/siemens/jailhouse-images/blob/next/recipes-kernel/linux/files/arm64_defconfig_4.19
>
> I have looked through the log and the defconfig. No obvious things
> found at this point. Thanks for sharing them!
>
So, I have put together a debug patch, mostly to verify that things
seems to be correct in regards to runtime PM. It should produce some
prints to the log, particular during power on/off of the SDIO card and
during probe of the wifi driver. Please re-run the test on top of the
v2 version of the $subject patch.
Kind regards
Uffe
* Ulf Hansson <[email protected]> [190121 06:41]:
> So, I have put together a debug patch, mostly to verify that things
> seems to be correct in regards to runtime PM. It should produce some
> prints to the log, particular during power on/off of the SDIO card and
> during probe of the wifi driver. Please re-run the test on top of the
> v2 version of the $subject patch.
For reference, here's what I'm seeing on omap4 with your v2 fix
and the debug patch for:
# ifconfig wlan0 up; ifconfig wlan0 down
Note that this is without pwrseq, so maybe that's the difference.
Regards,
Tony
8< -----------------
[ 148.458282] wl1271_sdio mmc4:0001:2: wl12xx_sdio_power_on: STEP1
[ 148.464691] mmc mmc4:0001: mmc_sdio_runtime_resume
[ 148.641021] mmc mmc4:0001: mmc_sdio_runtime_resume - ret=0 DONE
[ 148.647613] wl1271_sdio mmc4:0001:2: wl12xx_sdio_power_on: STEP2
[ 148.653961] wl1271_sdio mmc4:0001:2: wl12xx_sdio_power_on: STEP3
[ 148.660430] mmc mmc4:0001: mmc_sdio_hw_reset
[ 148.828735] mmc mmc4:0001: mmc_sdio_hw_reset - ret=0 DONE
[ 148.834503] wl1271_sdio mmc4:0001:2: wl12xx_sdio_power_on: STEP4
[ 148.840789] wl1271_sdio mmc4:0001:2: wl12xx_sdio_power_on: STEP5
[ 149.194671] wlcore: firmware booted (Rev 7.3.10.0.141)
[ 149.200134] wl1271_sdio mmc4:0001:2: sdio_bus_runtime_resume
[ 149.233276] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
[ 149.248352] wlcore: down
[ 149.253082] wl1271_sdio mmc4:0001:2: wl12xx_sdio_power_off: STEP1
[ 149.259613] wl1271_sdio mmc4:0001:2: wl12xx_sdio_power_off: STEP2
[ 149.265869] wl1271_sdio mmc4:0001:2: wl12xx_sdio_power_off: STEP3
[ 149.272155] wl1271_sdio mmc4:0001:2: wl12xx_sdio_power_off: STEP4
[ 149.307952] wl1271_sdio mmc4:0001:2: sdio_bus_runtime_suspend
[ 149.314147] mmc mmc4:0001: mmc_sdio_runtime_suspend
[ 149.321136] mmc mmc4:0001: mmc_sdio_runtime_suspend - DONE
On 21.01.19 15:40, Ulf Hansson wrote:
> On Fri, 18 Jan 2019 at 16:09, Ulf Hansson <[email protected]> wrote:
>>
>> On Fri, 18 Jan 2019 at 13:09, Jan Kiszka <[email protected]> wrote:
>>>
>>> On 17.01.19 10:54, Ulf Hansson wrote:
>>>> On Wed, 16 Jan 2019 at 21:26, Jan Kiszka <[email protected]> wrote:
>>>>>
>>>>> On 16.01.19 12:37, Ulf Hansson wrote:
>>>>>> During "wlan-up", we are programming the FW into the WiFi-chip. However,
>>>>>> re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
>>>>>> is made in-between the programmings.
>>>>>>
>>>>>> To conform to this requirement and to fix the regression in a simple way,
>>>>>> let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
>>>>>> (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
>>>>>> current code is to treat this scenario as an error, but unfortunate this
>>>>>> doesn't work as expected, so let's fix this.
>>>>>>
>>>>>> The other part is to guarantee that a power cycle of the SDIO card has been
>>>>>> completed when wl12xx_sdio_power_on() returns, as to allow the FW
>>>>>> programming to succeed. However, relying solely on runtime PM to deal with
>>>>>> this isn't sufficient. For example, userspace may prevent runtime suspend
>>>>>> via sysfs for the device that represents the SDIO card, leading to that the
>>>>>> mmc core also keeps it powered on. For this reason, let's instead do a
>>>>>> brute force power cycle in wl12xx_sdio_power_on().
>>>>>>
>>>>>> Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
>>>>>> Signed-off-by: Ulf Hansson <[email protected]>
>>>>>> ---
>>>>>>
>>>>>> Changes in v2:
>>>>>> - Keep the SDIO host claimed when calling mmc_hw_reset().
>>>>>> - Add a fixes tag.
>>>>>> ---
>>>>>> drivers/net/wireless/ti/wlcore/sdio.c | 15 +++++++--------
>>>>>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
>>>>>> index bd10165d7eec..4d4b07701149 100644
>>>>>> --- a/drivers/net/wireless/ti/wlcore/sdio.c
>>>>>> +++ b/drivers/net/wireless/ti/wlcore/sdio.c
>>>>>> @@ -164,6 +164,12 @@ static int wl12xx_sdio_power_on(struct wl12xx_sdio_glue *glue)
>>>>>> }
>>>>>>
>>>>>> sdio_claim_host(func);
>>>>>> + /*
>>>>>> + * To guarantee that the SDIO card is power cycled, as required to make
>>>>>> + * the FW programming to succeed, let's do a brute force HW reset.
>>>>>> + */
>>>>>> + mmc_hw_reset(card->host);
>>>>>> +
>>>>>> sdio_enable_func(func);
>>>>>> sdio_release_host(func);
>>>>>>
>>>>>> @@ -174,20 +180,13 @@ static int wl12xx_sdio_power_off(struct wl12xx_sdio_glue *glue)
>>>>>> {
>>>>>> struct sdio_func *func = dev_to_sdio_func(glue->dev);
>>>>>> struct mmc_card *card = func->card;
>>>>>> - int error;
>>>>>>
>>>>>> sdio_claim_host(func);
>>>>>> sdio_disable_func(func);
>>>>>> sdio_release_host(func);
>>>>>>
>>>>>> /* Let runtime PM know the card is powered off */
>>>>>> - error = pm_runtime_put(&card->dev);
>>>>>> - if (error < 0 && error != -EBUSY) {
>>>>>> - dev_err(&card->dev, "%s failed: %i\n", __func__, error);
>>>>>> -
>>>>>> - return error;
>>>>>> - }
>>>>>> -
>>>>>> + pm_runtime_put(&card->dev);
>>>>>> return 0;
>>>>>> }
>>>>>>
>>>>>>
>>>>>
>>>>> Just tested on both HiKey (620) and Ultra96 but it fails to fix the issue on
>>>>> both. I'm getting
>>>>>
>>>>> wl1271_sdio: probe of mmc2:0001:1 failed with error -16
>>>>>
>>>>> during boot again, and the interface is not available.
>>>>
>>>> Okay, sounds like this may be a different problem then. Can you share
>>>> the complete log and the kernel config?
>>>
>>> You can find the config here [1], log from the HiKey boot attached.
>>>
>>>> I can prepare a debug patch as well, if you are willing to re-run the test?
>>>
>>> Sure, send it over, I can run it.
>>
>> Alright, sounds great. However, I need to defer that to Monday/Tuesday
>> next week.
>>
>>>
>>>>
>>>> Adding a post-power-on-delay-ms of 1 ms as you suggested [1], doesn't
>>>> sounds like the correct solution to me, unless I am overlooking some
>>>> things. The point is, since the mmc core succeeds to detect and
>>>> initialize the SDIO card, the power sequence seems to be correct.
>>>
>>> Yeah, I'm not claiming at all I know what I'm doing there, just that it happens
>>> to work.
>>
>> I see. Good to know, thanks!
>>
>>>
>>> Jan
>>>
>>> [1]
>>> https://github.com/siemens/jailhouse-images/blob/next/recipes-kernel/linux/files/arm64_defconfig_4.19
>>
>> I have looked through the log and the defconfig. No obvious things
>> found at this point. Thanks for sharing them!
>>
>
> So, I have put together a debug patch, mostly to verify that things
> seems to be correct in regards to runtime PM. It should produce some
> prints to the log, particular during power on/off of the SDIO card and
> during probe of the wifi driver. Please re-run the test on top of the
> v2 version of the $subject patch.
>
Log attached.
Jan
Ulf Hansson <[email protected]> wrote:
> During "wlan-up", we are programming the FW into the WiFi-chip. However,
> re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
> is made in-between the programmings.
>
> To conform to this requirement and to fix the regression in a simple way,
> let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
> (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
> current code is to treat this scenario as an error, but unfortunate this
> doesn't work as expected, so let's fix this.
>
> The other part is to guarantee that a power cycle of the SDIO card has been
> completed when wl12xx_sdio_power_on() returns, as to allow the FW
> programming to succeed. However, relying solely on runtime PM to deal with
> this isn't sufficient. For example, userspace may prevent runtime suspend
> via sysfs for the device that represents the SDIO card, leading to that the
> mmc core also keeps it powered on. For this reason, let's instead do a
> brute force power cycle in wl12xx_sdio_power_on().
>
> Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
> Signed-off-by: Ulf Hansson <[email protected]>
> Tested-by: Tony Lindgren <[email protected]>
> Tested-by: Anders Roxell <[email protected]>
> Signed-off-by: Ulf Hansson <[email protected]>
Patch applied to wireless-drivers.git, thanks.
13e62626c578 wlcore: sdio: Fixup power on/off sequence
--
https://patchwork.kernel.org/patch/10765781/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Tue, 22 Jan 2019 at 17:08, Jan Kiszka <[email protected]> wrote:
>
> On 21.01.19 15:40, Ulf Hansson wrote:
> > On Fri, 18 Jan 2019 at 16:09, Ulf Hansson <[email protected]> wrote:
> >>
> >> On Fri, 18 Jan 2019 at 13:09, Jan Kiszka <[email protected]> wrote:
> >>>
> >>> On 17.01.19 10:54, Ulf Hansson wrote:
> >>>> On Wed, 16 Jan 2019 at 21:26, Jan Kiszka <[email protected]> wrote:
> >>>>>
> >>>>> On 16.01.19 12:37, Ulf Hansson wrote:
> >>>>>> During "wlan-up", we are programming the FW into the WiFi-chip. However,
> >>>>>> re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
> >>>>>> is made in-between the programmings.
> >>>>>>
> >>>>>> To conform to this requirement and to fix the regression in a simple way,
> >>>>>> let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
> >>>>>> (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
> >>>>>> current code is to treat this scenario as an error, but unfortunate this
> >>>>>> doesn't work as expected, so let's fix this.
> >>>>>>
> >>>>>> The other part is to guarantee that a power cycle of the SDIO card has been
> >>>>>> completed when wl12xx_sdio_power_on() returns, as to allow the FW
> >>>>>> programming to succeed. However, relying solely on runtime PM to deal with
> >>>>>> this isn't sufficient. For example, userspace may prevent runtime suspend
> >>>>>> via sysfs for the device that represents the SDIO card, leading to that the
> >>>>>> mmc core also keeps it powered on. For this reason, let's instead do a
> >>>>>> brute force power cycle in wl12xx_sdio_power_on().
> >>>>>>
> >>>>>> Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
> >>>>>> Signed-off-by: Ulf Hansson <[email protected]>
> >>>>>> ---
> >>>>>>
> >>>>>> Changes in v2:
> >>>>>> - Keep the SDIO host claimed when calling mmc_hw_reset().
> >>>>>> - Add a fixes tag.
> >>>>>> ---
> >>>>>> drivers/net/wireless/ti/wlcore/sdio.c | 15 +++++++--------
> >>>>>> 1 file changed, 7 insertions(+), 8 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
> >>>>>> index bd10165d7eec..4d4b07701149 100644
> >>>>>> --- a/drivers/net/wireless/ti/wlcore/sdio.c
> >>>>>> +++ b/drivers/net/wireless/ti/wlcore/sdio.c
> >>>>>> @@ -164,6 +164,12 @@ static int wl12xx_sdio_power_on(struct wl12xx_sdio_glue *glue)
> >>>>>> }
> >>>>>>
> >>>>>> sdio_claim_host(func);
> >>>>>> + /*
> >>>>>> + * To guarantee that the SDIO card is power cycled, as required to make
> >>>>>> + * the FW programming to succeed, let's do a brute force HW reset.
> >>>>>> + */
> >>>>>> + mmc_hw_reset(card->host);
> >>>>>> +
> >>>>>> sdio_enable_func(func);
> >>>>>> sdio_release_host(func);
> >>>>>>
> >>>>>> @@ -174,20 +180,13 @@ static int wl12xx_sdio_power_off(struct wl12xx_sdio_glue *glue)
> >>>>>> {
> >>>>>> struct sdio_func *func = dev_to_sdio_func(glue->dev);
> >>>>>> struct mmc_card *card = func->card;
> >>>>>> - int error;
> >>>>>>
> >>>>>> sdio_claim_host(func);
> >>>>>> sdio_disable_func(func);
> >>>>>> sdio_release_host(func);
> >>>>>>
> >>>>>> /* Let runtime PM know the card is powered off */
> >>>>>> - error = pm_runtime_put(&card->dev);
> >>>>>> - if (error < 0 && error != -EBUSY) {
> >>>>>> - dev_err(&card->dev, "%s failed: %i\n", __func__, error);
> >>>>>> -
> >>>>>> - return error;
> >>>>>> - }
> >>>>>> -
> >>>>>> + pm_runtime_put(&card->dev);
> >>>>>> return 0;
> >>>>>> }
> >>>>>>
> >>>>>>
> >>>>>
> >>>>> Just tested on both HiKey (620) and Ultra96 but it fails to fix the issue on
> >>>>> both. I'm getting
> >>>>>
> >>>>> wl1271_sdio: probe of mmc2:0001:1 failed with error -16
> >>>>>
> >>>>> during boot again, and the interface is not available.
> >>>>
> >>>> Okay, sounds like this may be a different problem then. Can you share
> >>>> the complete log and the kernel config?
> >>>
> >>> You can find the config here [1], log from the HiKey boot attached.
> >>>
> >>>> I can prepare a debug patch as well, if you are willing to re-run the test?
> >>>
> >>> Sure, send it over, I can run it.
> >>
> >> Alright, sounds great. However, I need to defer that to Monday/Tuesday
> >> next week.
> >>
> >>>
> >>>>
> >>>> Adding a post-power-on-delay-ms of 1 ms as you suggested [1], doesn't
> >>>> sounds like the correct solution to me, unless I am overlooking some
> >>>> things. The point is, since the mmc core succeeds to detect and
> >>>> initialize the SDIO card, the power sequence seems to be correct.
> >>>
> >>> Yeah, I'm not claiming at all I know what I'm doing there, just that it happens
> >>> to work.
> >>
> >> I see. Good to know, thanks!
> >>
> >>>
> >>> Jan
> >>>
> >>> [1]
> >>> https://github.com/siemens/jailhouse-images/blob/next/recipes-kernel/linux/files/arm64_defconfig_4.19
> >>
> >> I have looked through the log and the defconfig. No obvious things
> >> found at this point. Thanks for sharing them!
> >>
> >
> > So, I have put together a debug patch, mostly to verify that things
> > seems to be correct in regards to runtime PM. It should produce some
> > prints to the log, particular during power on/off of the SDIO card and
> > during probe of the wifi driver. Please re-run the test on top of the
> > v2 version of the $subject patch.
> >
>
> Log attached.
Thanks! Okay, so the re-initialization of the SDIO card is failing,
that's very valuable information.
I noticed one difference while comparing your log with the one I
received (offlist) from Anders... In your case the initialization
frequency that works the first time is 300KHz, while in Anders case
it's 100KHz. This sounds a bit fishy to me, so maybe there are some
problems with the pwrseq after all.
Let me think a bit and see what I can come up with as a possible solution.
In the meantime, can you re-run the test with same debug patch, but
change the post-power-on-delay-ms to let's say 10 ms in the DTS? I am
going to ask Anders to do the same test on his side, as to see if we
get different values of the found initialization frequency.
Kind regards
Uffe
On Tue, 22 Jan 2019 at 17:24, Kalle Valo <[email protected]> wrote:
>
> Ulf Hansson <[email protected]> wrote:
>
> > During "wlan-up", we are programming the FW into the WiFi-chip. However,
> > re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
> > is made in-between the programmings.
> >
> > To conform to this requirement and to fix the regression in a simple way,
> > let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
> > (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
> > current code is to treat this scenario as an error, but unfortunate this
> > doesn't work as expected, so let's fix this.
> >
> > The other part is to guarantee that a power cycle of the SDIO card has been
> > completed when wl12xx_sdio_power_on() returns, as to allow the FW
> > programming to succeed. However, relying solely on runtime PM to deal with
> > this isn't sufficient. For example, userspace may prevent runtime suspend
> > via sysfs for the device that represents the SDIO card, leading to that the
> > mmc core also keeps it powered on. For this reason, let's instead do a
> > brute force power cycle in wl12xx_sdio_power_on().
> >
> > Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
> > Signed-off-by: Ulf Hansson <[email protected]>
> > Tested-by: Tony Lindgren <[email protected]>
> > Tested-by: Anders Roxell <[email protected]>
> > Signed-off-by: Ulf Hansson <[email protected]>
>
> Patch applied to wireless-drivers.git, thanks.
>
> 13e62626c578 wlcore: sdio: Fixup power on/off sequence
Great, thanks! Let's see how we can fix Jan's problem on top.
Kind regards
Uffe
On 23.01.19 09:50, Ulf Hansson wrote:
> On Tue, 22 Jan 2019 at 17:08, Jan Kiszka <[email protected]> wrote:
>>
>> On 21.01.19 15:40, Ulf Hansson wrote:
>>> On Fri, 18 Jan 2019 at 16:09, Ulf Hansson <[email protected]> wrote:
>>>>
>>>> On Fri, 18 Jan 2019 at 13:09, Jan Kiszka <[email protected]> wrote:
>>>>>
>>>>> On 17.01.19 10:54, Ulf Hansson wrote:
>>>>>> On Wed, 16 Jan 2019 at 21:26, Jan Kiszka <[email protected]> wrote:
>>>>>>>
>>>>>>> On 16.01.19 12:37, Ulf Hansson wrote:
>>>>>>>> During "wlan-up", we are programming the FW into the WiFi-chip. However,
>>>>>>>> re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
>>>>>>>> is made in-between the programmings.
>>>>>>>>
>>>>>>>> To conform to this requirement and to fix the regression in a simple way,
>>>>>>>> let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
>>>>>>>> (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
>>>>>>>> current code is to treat this scenario as an error, but unfortunate this
>>>>>>>> doesn't work as expected, so let's fix this.
>>>>>>>>
>>>>>>>> The other part is to guarantee that a power cycle of the SDIO card has been
>>>>>>>> completed when wl12xx_sdio_power_on() returns, as to allow the FW
>>>>>>>> programming to succeed. However, relying solely on runtime PM to deal with
>>>>>>>> this isn't sufficient. For example, userspace may prevent runtime suspend
>>>>>>>> via sysfs for the device that represents the SDIO card, leading to that the
>>>>>>>> mmc core also keeps it powered on. For this reason, let's instead do a
>>>>>>>> brute force power cycle in wl12xx_sdio_power_on().
>>>>>>>>
>>>>>>>> Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
>>>>>>>> Signed-off-by: Ulf Hansson <[email protected]>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> Changes in v2:
>>>>>>>> - Keep the SDIO host claimed when calling mmc_hw_reset().
>>>>>>>> - Add a fixes tag.
>>>>>>>> ---
>>>>>>>> drivers/net/wireless/ti/wlcore/sdio.c | 15 +++++++--------
>>>>>>>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
>>>>>>>> index bd10165d7eec..4d4b07701149 100644
>>>>>>>> --- a/drivers/net/wireless/ti/wlcore/sdio.c
>>>>>>>> +++ b/drivers/net/wireless/ti/wlcore/sdio.c
>>>>>>>> @@ -164,6 +164,12 @@ static int wl12xx_sdio_power_on(struct wl12xx_sdio_glue *glue)
>>>>>>>> }
>>>>>>>>
>>>>>>>> sdio_claim_host(func);
>>>>>>>> + /*
>>>>>>>> + * To guarantee that the SDIO card is power cycled, as required to make
>>>>>>>> + * the FW programming to succeed, let's do a brute force HW reset.
>>>>>>>> + */
>>>>>>>> + mmc_hw_reset(card->host);
>>>>>>>> +
>>>>>>>> sdio_enable_func(func);
>>>>>>>> sdio_release_host(func);
>>>>>>>>
>>>>>>>> @@ -174,20 +180,13 @@ static int wl12xx_sdio_power_off(struct wl12xx_sdio_glue *glue)
>>>>>>>> {
>>>>>>>> struct sdio_func *func = dev_to_sdio_func(glue->dev);
>>>>>>>> struct mmc_card *card = func->card;
>>>>>>>> - int error;
>>>>>>>>
>>>>>>>> sdio_claim_host(func);
>>>>>>>> sdio_disable_func(func);
>>>>>>>> sdio_release_host(func);
>>>>>>>>
>>>>>>>> /* Let runtime PM know the card is powered off */
>>>>>>>> - error = pm_runtime_put(&card->dev);
>>>>>>>> - if (error < 0 && error != -EBUSY) {
>>>>>>>> - dev_err(&card->dev, "%s failed: %i\n", __func__, error);
>>>>>>>> -
>>>>>>>> - return error;
>>>>>>>> - }
>>>>>>>> -
>>>>>>>> + pm_runtime_put(&card->dev);
>>>>>>>> return 0;
>>>>>>>> }
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> Just tested on both HiKey (620) and Ultra96 but it fails to fix the issue on
>>>>>>> both. I'm getting
>>>>>>>
>>>>>>> wl1271_sdio: probe of mmc2:0001:1 failed with error -16
>>>>>>>
>>>>>>> during boot again, and the interface is not available.
>>>>>>
>>>>>> Okay, sounds like this may be a different problem then. Can you share
>>>>>> the complete log and the kernel config?
>>>>>
>>>>> You can find the config here [1], log from the HiKey boot attached.
>>>>>
>>>>>> I can prepare a debug patch as well, if you are willing to re-run the test?
>>>>>
>>>>> Sure, send it over, I can run it.
>>>>
>>>> Alright, sounds great. However, I need to defer that to Monday/Tuesday
>>>> next week.
>>>>
>>>>>
>>>>>>
>>>>>> Adding a post-power-on-delay-ms of 1 ms as you suggested [1], doesn't
>>>>>> sounds like the correct solution to me, unless I am overlooking some
>>>>>> things. The point is, since the mmc core succeeds to detect and
>>>>>> initialize the SDIO card, the power sequence seems to be correct.
>>>>>
>>>>> Yeah, I'm not claiming at all I know what I'm doing there, just that it happens
>>>>> to work.
>>>>
>>>> I see. Good to know, thanks!
>>>>
>>>>>
>>>>> Jan
>>>>>
>>>>> [1]
>>>>> https://github.com/siemens/jailhouse-images/blob/next/recipes-kernel/linux/files/arm64_defconfig_4.19
>>>>
>>>> I have looked through the log and the defconfig. No obvious things
>>>> found at this point. Thanks for sharing them!
>>>>
>>>
>>> So, I have put together a debug patch, mostly to verify that things
>>> seems to be correct in regards to runtime PM. It should produce some
>>> prints to the log, particular during power on/off of the SDIO card and
>>> during probe of the wifi driver. Please re-run the test on top of the
>>> v2 version of the $subject patch.
>>>
>>
>> Log attached.
>
> Thanks! Okay, so the re-initialization of the SDIO card is failing,
> that's very valuable information.
>
> I noticed one difference while comparing your log with the one I
> received (offlist) from Anders... In your case the initialization
> frequency that works the first time is 300KHz, while in Anders case
> it's 100KHz. This sounds a bit fishy to me, so maybe there are some
> problems with the pwrseq after all.
>
> Let me think a bit and see what I can come up with as a possible solution.
>
> In the meantime, can you re-run the test with same debug patch, but
> change the post-power-on-delay-ms to let's say 10 ms in the DTS? I am
> going to ask Anders to do the same test on his side, as to see if we
> get different values of the found initialization frequency.
Here is a log with 10 ms power-on-delay.
Jan
On Wed, 23 Jan 2019 at 21:17, Jan Kiszka <[email protected]> wrote:
>
> On 23.01.19 09:50, Ulf Hansson wrote:
> > On Tue, 22 Jan 2019 at 17:08, Jan Kiszka <[email protected]> wrote:
> >>
> >> On 21.01.19 15:40, Ulf Hansson wrote:
> >>> On Fri, 18 Jan 2019 at 16:09, Ulf Hansson <[email protected]> wrote:
> >>>>
> >>>> On Fri, 18 Jan 2019 at 13:09, Jan Kiszka <[email protected]> wrote:
> >>>>>
> >>>>> On 17.01.19 10:54, Ulf Hansson wrote:
> >>>>>> On Wed, 16 Jan 2019 at 21:26, Jan Kiszka <[email protected]> wrote:
> >>>>>>>
> >>>>>>> On 16.01.19 12:37, Ulf Hansson wrote:
> >>>>>>>> During "wlan-up", we are programming the FW into the WiFi-chip. However,
> >>>>>>>> re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
> >>>>>>>> is made in-between the programmings.
> >>>>>>>>
> >>>>>>>> To conform to this requirement and to fix the regression in a simple way,
> >>>>>>>> let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
> >>>>>>>> (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
> >>>>>>>> current code is to treat this scenario as an error, but unfortunate this
> >>>>>>>> doesn't work as expected, so let's fix this.
> >>>>>>>>
> >>>>>>>> The other part is to guarantee that a power cycle of the SDIO card has been
> >>>>>>>> completed when wl12xx_sdio_power_on() returns, as to allow the FW
> >>>>>>>> programming to succeed. However, relying solely on runtime PM to deal with
> >>>>>>>> this isn't sufficient. For example, userspace may prevent runtime suspend
> >>>>>>>> via sysfs for the device that represents the SDIO card, leading to that the
> >>>>>>>> mmc core also keeps it powered on. For this reason, let's instead do a
> >>>>>>>> brute force power cycle in wl12xx_sdio_power_on().
> >>>>>>>>
> >>>>>>>> Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
> >>>>>>>> Signed-off-by: Ulf Hansson <[email protected]>
> >>>>>>>> ---
> >>>>>>>>
> >>>>>>>> Changes in v2:
> >>>>>>>> - Keep the SDIO host claimed when calling mmc_hw_reset().
> >>>>>>>> - Add a fixes tag.
> >>>>>>>> ---
> >>>>>>>> drivers/net/wireless/ti/wlcore/sdio.c | 15 +++++++--------
> >>>>>>>> 1 file changed, 7 insertions(+), 8 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
> >>>>>>>> index bd10165d7eec..4d4b07701149 100644
> >>>>>>>> --- a/drivers/net/wireless/ti/wlcore/sdio.c
> >>>>>>>> +++ b/drivers/net/wireless/ti/wlcore/sdio.c
> >>>>>>>> @@ -164,6 +164,12 @@ static int wl12xx_sdio_power_on(struct wl12xx_sdio_glue *glue)
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> sdio_claim_host(func);
> >>>>>>>> + /*
> >>>>>>>> + * To guarantee that the SDIO card is power cycled, as required to make
> >>>>>>>> + * the FW programming to succeed, let's do a brute force HW reset.
> >>>>>>>> + */
> >>>>>>>> + mmc_hw_reset(card->host);
> >>>>>>>> +
> >>>>>>>> sdio_enable_func(func);
> >>>>>>>> sdio_release_host(func);
> >>>>>>>>
> >>>>>>>> @@ -174,20 +180,13 @@ static int wl12xx_sdio_power_off(struct wl12xx_sdio_glue *glue)
> >>>>>>>> {
> >>>>>>>> struct sdio_func *func = dev_to_sdio_func(glue->dev);
> >>>>>>>> struct mmc_card *card = func->card;
> >>>>>>>> - int error;
> >>>>>>>>
> >>>>>>>> sdio_claim_host(func);
> >>>>>>>> sdio_disable_func(func);
> >>>>>>>> sdio_release_host(func);
> >>>>>>>>
> >>>>>>>> /* Let runtime PM know the card is powered off */
> >>>>>>>> - error = pm_runtime_put(&card->dev);
> >>>>>>>> - if (error < 0 && error != -EBUSY) {
> >>>>>>>> - dev_err(&card->dev, "%s failed: %i\n", __func__, error);
> >>>>>>>> -
> >>>>>>>> - return error;
> >>>>>>>> - }
> >>>>>>>> -
> >>>>>>>> + pm_runtime_put(&card->dev);
> >>>>>>>> return 0;
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>> Just tested on both HiKey (620) and Ultra96 but it fails to fix the issue on
> >>>>>>> both. I'm getting
> >>>>>>>
> >>>>>>> wl1271_sdio: probe of mmc2:0001:1 failed with error -16
> >>>>>>>
> >>>>>>> during boot again, and the interface is not available.
> >>>>>>
> >>>>>> Okay, sounds like this may be a different problem then. Can you share
> >>>>>> the complete log and the kernel config?
> >>>>>
> >>>>> You can find the config here [1], log from the HiKey boot attached.
> >>>>>
> >>>>>> I can prepare a debug patch as well, if you are willing to re-run the test?
> >>>>>
> >>>>> Sure, send it over, I can run it.
> >>>>
> >>>> Alright, sounds great. However, I need to defer that to Monday/Tuesday
> >>>> next week.
> >>>>
> >>>>>
> >>>>>>
> >>>>>> Adding a post-power-on-delay-ms of 1 ms as you suggested [1], doesn't
> >>>>>> sounds like the correct solution to me, unless I am overlooking some
> >>>>>> things. The point is, since the mmc core succeeds to detect and
> >>>>>> initialize the SDIO card, the power sequence seems to be correct.
> >>>>>
> >>>>> Yeah, I'm not claiming at all I know what I'm doing there, just that it happens
> >>>>> to work.
> >>>>
> >>>> I see. Good to know, thanks!
> >>>>
> >>>>>
> >>>>> Jan
> >>>>>
> >>>>> [1]
> >>>>> https://github.com/siemens/jailhouse-images/blob/next/recipes-kernel/linux/files/arm64_defconfig_4.19
> >>>>
> >>>> I have looked through the log and the defconfig. No obvious things
> >>>> found at this point. Thanks for sharing them!
> >>>>
> >>>
> >>> So, I have put together a debug patch, mostly to verify that things
> >>> seems to be correct in regards to runtime PM. It should produce some
> >>> prints to the log, particular during power on/off of the SDIO card and
> >>> during probe of the wifi driver. Please re-run the test on top of the
> >>> v2 version of the $subject patch.
> >>>
> >>
> >> Log attached.
> >
> > Thanks! Okay, so the re-initialization of the SDIO card is failing,
> > that's very valuable information.
> >
> > I noticed one difference while comparing your log with the one I
> > received (offlist) from Anders... In your case the initialization
> > frequency that works the first time is 300KHz, while in Anders case
> > it's 100KHz. This sounds a bit fishy to me, so maybe there are some
> > problems with the pwrseq after all.
> >
> > Let me think a bit and see what I can come up with as a possible solution.
> >
> > In the meantime, can you re-run the test with same debug patch, but
> > change the post-power-on-delay-ms to let's say 10 ms in the DTS? I am
> > going to ask Anders to do the same test on his side, as to see if we
> > get different values of the found initialization frequency.
>
> Here is a log with 10 ms power-on-delay.
Thanks!
According to the log, we are now using 400KHz as the init frequency,
because we succeeds to initialize the SDIO card already at the first
attempt. The same thing happens to Anders' new tests.
Conclusion from my side: We need the delay, probably because the
internals of the WiFi chip isn't ready as soon as the WiFi spec
states. In either case, I suggest you to re-spin your patch [1] and
extend to delay to 10ms (instead of 1ms, just to be safe). Let it have
also have the below fixes tag (as it seems like an original problem
with the pwrseq) and also tag it for stable.
Fixes: ea452678734e ("arm64: dts: hikey: Fix WiFi support")
Finally, feel free to add my ack for it.
Kind regards
Uffe
[1]
https://patchwork.kernel.org/patch/10745075/