2016-05-18 01:15:12

by Daniel Lenski

[permalink] [raw]
Subject: [PATCH 0/2] rtl8xxxu: increase polling timeout for firmware startup

Here are patches patch to increase the polling timeout for rtl8xxxu firmware
startup, and to make it configurable.

Resubmitting with changes per Julian Calaby's comments.


Dan Lenski (2):
rtl8xxxu: Increase default polling timeout for firmware startup
rtl8xxxu: Make polling timeout for firmware configurable

drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c | 11 +++++++----
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 2 +-
2 files changed, 8 insertions(+), 5 deletions(-)

--
2.8.2



2016-05-18 14:57:27

by Jes Sorensen

[permalink] [raw]
Subject: Re: [PATCH 1/2] rtl8xxxu: Increase default polling timeout for firmware startup

Daniel Lenski <[email protected]> writes:
> On Tue, May 17, 2016 at 8:35 PM, Jes Sorensen <[email protected]> wrote:
>> Dan Lenski <[email protected]> writes:
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> index f2a1bac..39c6ce7 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> @@ -49,7 +49,7 @@
>>> #define TX_PAGE_NUM_NORM_PQ 0x02
>>>
>>> #define RTL_FW_PAGE_SIZE 4096
>>> -#define RTL8XXXU_FIRMWARE_POLL_MAX 1000
>>> +#define RTL8XXXU_FIRMWARE_POLL_MAX 5000
>>
>> This is a long delay - how about 2000?
>
> I tried 2000 first, and it worked for me on only 1 out of 3 cold boots.

OK, lets go for 5000 then to be safe.

Cheers,
Jes

2016-05-18 04:04:31

by Daniel Lenski

[permalink] [raw]
Subject: Re: [PATCH 1/2] rtl8xxxu: Increase default polling timeout for firmware startup

On Tue, May 17, 2016 at 8:35 PM, Jes Sorensen <[email protected]> wrote:
> Dan Lenski <[email protected]> writes:
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> index f2a1bac..39c6ce7 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> @@ -49,7 +49,7 @@
>> #define TX_PAGE_NUM_NORM_PQ 0x02
>>
>> #define RTL_FW_PAGE_SIZE 4096
>> -#define RTL8XXXU_FIRMWARE_POLL_MAX 1000
>> +#define RTL8XXXU_FIRMWARE_POLL_MAX 5000
>
> This is a long delay - how about 2000?

I tried 2000 first, and it worked for me on only 1 out of 3 cold boots.

Dan

2016-05-18 01:20:27

by Julian Calaby

[permalink] [raw]
Subject: Re: [PATCH 1/2] rtl8xxxu: Increase default polling timeout for firmware startup

Hi,

On Wed, May 18, 2016 at 11:14 AM, Dan Lenski <[email protected]> wrote:
> This patch increases the default value for the maximum number of polling
> loops to wait for the rtl8xxxu MCU to start after the firmware is loaded
> (from 1000 to 5000).
>
> With RTL8723AU chipset, I frequently encounter "Firmware failed to start"
> errors from rtl8xxxu after a cold boot.
>
> It appears that other chipsets supported by the driver have the same
> problem. Here are a couple of relevant bug reports:
> - http://ubuntuforums.org/showthread.php?t=2321756
> - https://www.mail-archive.com/[email protected]/msg4942468.html
>
> This issue seems to occur because RTL8XXXU_FIRMWARE_POLL_MAX (1000) is
> too short, and the MCU fails to start up as quickly as expected.
>
> With a longer value (5000), the driver starts up consistently and
> successfully after cold-boot.
>
> Signed-off-by: Dan Lenski <[email protected]>

This looks good to me.

Reviewed-by: Julian Calaby <[email protected]>

> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index f2a1bac..39c6ce7 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -49,7 +49,7 @@
> #define TX_PAGE_NUM_NORM_PQ 0x02
>
> #define RTL_FW_PAGE_SIZE 4096
> -#define RTL8XXXU_FIRMWARE_POLL_MAX 1000
> +#define RTL8XXXU_FIRMWARE_POLL_MAX 5000
>
> #define RTL8723A_CHANNEL_GROUPS 3
> #define RTL8723A_MAX_RF_PATHS 2
> --
> 2.8.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/

2016-05-18 01:15:29

by Daniel Lenski

[permalink] [raw]
Subject: [PATCH 2/2] rtl8xxxu: Make polling timeout for firmware configurable

This patch makes RTL8XXXU_FIRMWARE_POLL_MAX into a configurable module
parameter, firmware_poll_max.

Signed-off-by: Dan Lenski <[email protected]>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
index 6aed923..a1efb2c 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
@@ -44,6 +44,7 @@

static int rtl8xxxu_debug;
static bool rtl8xxxu_ht40_2g;
+static int rtl8xxxu_firmware_poll_max = RTL8XXXU_FIRMWARE_POLL_MAX;

MODULE_AUTHOR("Jes Sorensen <[email protected]>");
MODULE_DESCRIPTION("RTL8XXXu USB mac80211 Wireless LAN Driver");
@@ -59,6 +60,8 @@ module_param_named(debug, rtl8xxxu_debug, int, 0600);
MODULE_PARM_DESC(debug, "Set debug mask");
module_param_named(ht40_2g, rtl8xxxu_ht40_2g, bool, 0600);
MODULE_PARM_DESC(ht40_2g, "Enable HT40 support on the 2.4GHz band");
+module_param_named(firmware_poll_max, rtl8xxxu_firmware_poll_max, int, 0600);
+MODULE_PARM_DESC(firmware_poll_max, "Maximum polling count for firmware startup (increase if firmware fails to start)");

#define USB_VENDOR_ID_REALTEK 0x0bda
/* Minimum IEEE80211_MAX_FRAME_LEN */
@@ -2050,13 +2053,13 @@ static int rtl8xxxu_start_firmware(struct rtl8xxxu_priv *priv)
u32 val32;

/* Poll checksum report */
- for (i = 0; i < RTL8XXXU_FIRMWARE_POLL_MAX; i++) {
+ for (i = 0; i < rtl8xxxu_firmware_poll_max; i++) {
val32 = rtl8xxxu_read32(priv, REG_MCU_FW_DL);
if (val32 & MCU_FW_DL_CSUM_REPORT)
break;
}

- if (i == RTL8XXXU_FIRMWARE_POLL_MAX) {
+ if (i == rtl8xxxu_firmware_poll_max) {
dev_warn(dev, "Firmware checksum poll timed out\n");
ret = -EAGAIN;
goto exit;
@@ -2068,7 +2071,7 @@ static int rtl8xxxu_start_firmware(struct rtl8xxxu_priv *priv)
rtl8xxxu_write32(priv, REG_MCU_FW_DL, val32);

/* Wait for firmware to become ready */
- for (i = 0; i < RTL8XXXU_FIRMWARE_POLL_MAX; i++) {
+ for (i = 0; i < rtl8xxxu_firmware_poll_max; i++) {
val32 = rtl8xxxu_read32(priv, REG_MCU_FW_DL);
if (val32 & MCU_WINT_INIT_READY)
break;
@@ -2076,7 +2079,7 @@ static int rtl8xxxu_start_firmware(struct rtl8xxxu_priv *priv)
udelay(100);
}

- if (i == RTL8XXXU_FIRMWARE_POLL_MAX) {
+ if (i == rtl8xxxu_firmware_poll_max) {
dev_warn(dev, "Firmware failed to start\n");
ret = -EAGAIN;
goto exit;
--
2.8.2


2016-05-18 03:36:50

by Jes Sorensen

[permalink] [raw]
Subject: Re: [PATCH 2/2] rtl8xxxu: Make polling timeout for firmware configurable

Dan Lenski <[email protected]> writes:
> This patch makes RTL8XXXU_FIRMWARE_POLL_MAX into a configurable module
> parameter, firmware_poll_max.
>
> Signed-off-by: Dan Lenski <[email protected]>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)

I really see no use for this one - it's an extremely rare case, and not
something that I'd expect anyone ever using.

Again, please submit patches against the correct git tree.

Jes

2016-05-18 01:15:24

by Daniel Lenski

[permalink] [raw]
Subject: [PATCH 1/2] rtl8xxxu: Increase default polling timeout for firmware startup

This patch increases the default value for the maximum number of polling
loops to wait for the rtl8xxxu MCU to start after the firmware is loaded
(from 1000 to 5000).

With RTL8723AU chipset, I frequently encounter "Firmware failed to start"
errors from rtl8xxxu after a cold boot.

It appears that other chipsets supported by the driver have the same
problem. Here are a couple of relevant bug reports:
- http://ubuntuforums.org/showthread.php?t=2321756
- https://www.mail-archive.com/[email protected]/msg4942468.html

This issue seems to occur because RTL8XXXU_FIRMWARE_POLL_MAX (1000) is
too short, and the MCU fails to start up as quickly as expected.

With a longer value (5000), the driver starts up consistently and
successfully after cold-boot.

Signed-off-by: Dan Lenski <[email protected]>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index f2a1bac..39c6ce7 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -49,7 +49,7 @@
#define TX_PAGE_NUM_NORM_PQ 0x02

#define RTL_FW_PAGE_SIZE 4096
-#define RTL8XXXU_FIRMWARE_POLL_MAX 1000
+#define RTL8XXXU_FIRMWARE_POLL_MAX 5000

#define RTL8723A_CHANNEL_GROUPS 3
#define RTL8723A_MAX_RF_PATHS 2
--
2.8.2


2016-05-18 03:35:45

by Jes Sorensen

[permalink] [raw]
Subject: Re: [PATCH 1/2] rtl8xxxu: Increase default polling timeout for firmware startup

Dan Lenski <[email protected]> writes:
> This patch increases the default value for the maximum number of polling
> loops to wait for the rtl8xxxu MCU to start after the firmware is loaded
> (from 1000 to 5000).
>
> With RTL8723AU chipset, I frequently encounter "Firmware failed to start"
> errors from rtl8xxxu after a cold boot.
>
> It appears that other chipsets supported by the driver have the same
> problem. Here are a couple of relevant bug reports:
> - http://ubuntuforums.org/showthread.php?t=2321756
> - https://www.mail-archive.com/[email protected]/msg4942468.html
>
> This issue seems to occur because RTL8XXXU_FIRMWARE_POLL_MAX (1000) is
> too short, and the MCU fails to start up as quickly as expected.
>
> With a longer value (5000), the driver starts up consistently and
> successfully after cold-boot.
>
> Signed-off-by: Dan Lenski <[email protected]>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

As previously pointed out, please provide some details about the system
where this goes wrong.

> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index f2a1bac..39c6ce7 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -49,7 +49,7 @@
> #define TX_PAGE_NUM_NORM_PQ 0x02
>
> #define RTL_FW_PAGE_SIZE 4096
> -#define RTL8XXXU_FIRMWARE_POLL_MAX 1000
> +#define RTL8XXXU_FIRMWARE_POLL_MAX 5000

This is a long delay - how about 2000?

Jes

2016-05-18 01:21:21

by Julian Calaby

[permalink] [raw]
Subject: Re: [PATCH 2/2] rtl8xxxu: Make polling timeout for firmware configurable

Hi,

On Wed, May 18, 2016 at 11:14 AM, Dan Lenski <[email protected]> wrote:
> This patch makes RTL8XXXU_FIRMWARE_POLL_MAX into a configurable module
> parameter, firmware_poll_max.
>
> Signed-off-by: Dan Lenski <[email protected]>

This looks good to me.

Reviewed-by: Julian Calaby <[email protected]>

> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> index 6aed923..a1efb2c 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> @@ -44,6 +44,7 @@
>
> static int rtl8xxxu_debug;
> static bool rtl8xxxu_ht40_2g;
> +static int rtl8xxxu_firmware_poll_max = RTL8XXXU_FIRMWARE_POLL_MAX;
>
> MODULE_AUTHOR("Jes Sorensen <[email protected]>");
> MODULE_DESCRIPTION("RTL8XXXu USB mac80211 Wireless LAN Driver");
> @@ -59,6 +60,8 @@ module_param_named(debug, rtl8xxxu_debug, int, 0600);
> MODULE_PARM_DESC(debug, "Set debug mask");
> module_param_named(ht40_2g, rtl8xxxu_ht40_2g, bool, 0600);
> MODULE_PARM_DESC(ht40_2g, "Enable HT40 support on the 2.4GHz band");
> +module_param_named(firmware_poll_max, rtl8xxxu_firmware_poll_max, int, 0600);
> +MODULE_PARM_DESC(firmware_poll_max, "Maximum polling count for firmware startup (increase if firmware fails to start)");
>
> #define USB_VENDOR_ID_REALTEK 0x0bda
> /* Minimum IEEE80211_MAX_FRAME_LEN */
> @@ -2050,13 +2053,13 @@ static int rtl8xxxu_start_firmware(struct rtl8xxxu_priv *priv)
> u32 val32;
>
> /* Poll checksum report */
> - for (i = 0; i < RTL8XXXU_FIRMWARE_POLL_MAX; i++) {
> + for (i = 0; i < rtl8xxxu_firmware_poll_max; i++) {
> val32 = rtl8xxxu_read32(priv, REG_MCU_FW_DL);
> if (val32 & MCU_FW_DL_CSUM_REPORT)
> break;
> }
>
> - if (i == RTL8XXXU_FIRMWARE_POLL_MAX) {
> + if (i == rtl8xxxu_firmware_poll_max) {
> dev_warn(dev, "Firmware checksum poll timed out\n");
> ret = -EAGAIN;
> goto exit;
> @@ -2068,7 +2071,7 @@ static int rtl8xxxu_start_firmware(struct rtl8xxxu_priv *priv)
> rtl8xxxu_write32(priv, REG_MCU_FW_DL, val32);
>
> /* Wait for firmware to become ready */
> - for (i = 0; i < RTL8XXXU_FIRMWARE_POLL_MAX; i++) {
> + for (i = 0; i < rtl8xxxu_firmware_poll_max; i++) {
> val32 = rtl8xxxu_read32(priv, REG_MCU_FW_DL);
> if (val32 & MCU_WINT_INIT_READY)
> break;
> @@ -2076,7 +2079,7 @@ static int rtl8xxxu_start_firmware(struct rtl8xxxu_priv *priv)
> udelay(100);
> }
>
> - if (i == RTL8XXXU_FIRMWARE_POLL_MAX) {
> + if (i == rtl8xxxu_firmware_poll_max) {
> dev_warn(dev, "Firmware failed to start\n");
> ret = -EAGAIN;
> goto exit;
> --
> 2.8.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/