linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rtl8xxxu: Fix device info for RTL8192EU devices
@ 2021-03-23 19:36 Pascal Terjan
  2021-04-18  5:57 ` Kalle Valo
  2021-04-19 11:53 ` Jes Sorensen
  0 siblings, 2 replies; 6+ messages in thread
From: Pascal Terjan @ 2021-03-23 19:36 UTC (permalink / raw)
  To: Jes Sorensen, linux-wireless, linux-kernel; +Cc: Pascal Terjan

Based on 2001:3319 and 2357:0109 which I used to test the fix and
0bda:818b and 2357:0108 for which I found efuse dumps online.

== 2357:0109 ==
=== Before ===
Vendor: Realtek
Product: \x03802.11n NI
Serial:
=== After ===
Vendor: Realtek
Product: 802.11n NIC
Serial not available.

== 2001:3319 ==
=== Before ===
Vendor: Realtek
Product: Wireless N
Serial: no USB Adap
=== After ===
Vendor: Realtek
Product: Wireless N Nano USB Adapter
Serial not available.

Signed-off-by: Pascal Terjan <pterjan@google.com>
---
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  | 11 ++--
 .../realtek/rtl8xxxu/rtl8xxxu_8192e.c         | 53 ++++++++++++++++---
 2 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index d6d1be4169e5..acb6b0cd3667 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -853,15 +853,10 @@ struct rtl8192eu_efuse {
 	u8 usb_optional_function;
 	u8 res9[2];
 	u8 mac_addr[ETH_ALEN];		/* 0xd7 */
-	u8 res10[2];
-	u8 vendor_name[7];
-	u8 res11[2];
-	u8 device_name[0x0b];		/* 0xe8 */
-	u8 res12[2];
-	u8 serial[0x0b];		/* 0xf5 */
-	u8 res13[0x30];
+	u8 device_info[80];
+	u8 res11[3];
 	u8 unknown[0x0d];		/* 0x130 */
-	u8 res14[0xc3];
+	u8 res12[0xc3];
 };
 
 struct rtl8xxxu_reg8val {
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index cfe2dfdae928..9c5fad49ed2a 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -554,9 +554,39 @@ rtl8192e_set_tx_power(struct rtl8xxxu_priv *priv, int channel, bool ht40)
 	}
 }
 
+static void rtl8192eu_log_device_info(struct rtl8xxxu_priv *priv,
+				      char *record_name,
+				      char **record)
+{
+	/* A record is [ total length | 0x03 | value ] */
+	unsigned char l = (*record)[0];
+
+	/* The whole section seems to be 80 characters so a record should not
+	 * be able to be that large.
+	 */
+	if (l > 80) {
+		dev_warn(&priv->udev->dev,
+			 "invalid record length %d while parsing \"%s\".\n",
+			 l, record_name);
+		return;
+	}
+
+	if (l >= 2) {
+		char value[80];
+
+		memcpy(value, &(*record)[2], l - 2);
+		value[l - 2] = '\0';
+		dev_info(&priv->udev->dev, "%s: %s\n", record_name, value);
+		*record = *record + l;
+	} else {
+		dev_info(&priv->udev->dev, "%s not available.\n", record_name);
+	}
+}
+
 static int rtl8192eu_parse_efuse(struct rtl8xxxu_priv *priv)
 {
 	struct rtl8192eu_efuse *efuse = &priv->efuse_wifi.efuse8192eu;
+	char *record = efuse->device_info;
 	int i;
 
 	if (efuse->rtl_id != cpu_to_le16(0x8129))
@@ -604,12 +634,23 @@ static int rtl8192eu_parse_efuse(struct rtl8xxxu_priv *priv)
 	priv->has_xtalk = 1;
 	priv->xtalk = priv->efuse_wifi.efuse8192eu.xtal_k & 0x3f;
 
-	dev_info(&priv->udev->dev, "Vendor: %.7s\n", efuse->vendor_name);
-	dev_info(&priv->udev->dev, "Product: %.11s\n", efuse->device_name);
-	if (memchr_inv(efuse->serial, 0xff, 11))
-		dev_info(&priv->udev->dev, "Serial: %.11s\n", efuse->serial);
-	else
-		dev_info(&priv->udev->dev, "Serial not available.\n");
+	/* device_info section seems to be laid out as records
+	 * [ total length | 0x03 | value ] so:
+	 * - vendor length + 2
+	 * - 0x03
+	 * - vendor string (not null terminated)
+	 * - product length + 2
+	 * - 0x03
+	 * - product string (not null terminated)
+	 * Then there is one or 2 0x00 on all the 4 devices I own or found
+	 * dumped online.
+	 * As previous version of the code handled an optional serial
+	 * string, I now assume there may be a third record if the
+	 * length is not 0.
+	 */
+	rtl8192eu_log_device_info(priv, "Vendor", &record);
+	rtl8192eu_log_device_info(priv, "Product", &record);
+	rtl8192eu_log_device_info(priv, "Serial", &record);
 
 	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_EFUSE) {
 		unsigned char *raw = priv->efuse_wifi.raw;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] rtl8xxxu: Fix device info for RTL8192EU devices
  2021-03-23 19:36 [PATCH] rtl8xxxu: Fix device info for RTL8192EU devices Pascal Terjan
@ 2021-04-18  5:57 ` Kalle Valo
  2021-04-19 11:53 ` Jes Sorensen
  1 sibling, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2021-04-18  5:57 UTC (permalink / raw)
  To: Pascal Terjan; +Cc: Jes Sorensen, linux-wireless, linux-kernel, Pascal Terjan

Pascal Terjan <pterjan@google.com> wrote:

> Based on 2001:3319 and 2357:0109 which I used to test the fix and
> 0bda:818b and 2357:0108 for which I found efuse dumps online.
> 
> == 2357:0109 ==
> === Before ===
> Vendor: Realtek
> Product: \x03802.11n NI
> Serial:
> === After ===
> Vendor: Realtek
> Product: 802.11n NIC
> Serial not available.
> 
> == 2001:3319 ==
> === Before ===
> Vendor: Realtek
> Product: Wireless N
> Serial: no USB Adap
> === After ===
> Vendor: Realtek
> Product: Wireless N Nano USB Adapter
> Serial not available.
> 
> Signed-off-by: Pascal Terjan <pterjan@google.com>

Can someone review this, please?

Patch set to Deferred.

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20210323193617.3748164-1-pterjan@google.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] rtl8xxxu: Fix device info for RTL8192EU devices
  2021-03-23 19:36 [PATCH] rtl8xxxu: Fix device info for RTL8192EU devices Pascal Terjan
  2021-04-18  5:57 ` Kalle Valo
@ 2021-04-19 11:53 ` Jes Sorensen
  2021-04-19 15:55   ` Pascal Terjan
  2021-04-24 17:29   ` [PATCH v2] " Pascal Terjan
  1 sibling, 2 replies; 6+ messages in thread
From: Jes Sorensen @ 2021-04-19 11:53 UTC (permalink / raw)
  To: Pascal Terjan, linux-wireless, linux-kernel

On 3/23/21 3:36 PM, Pascal Terjan wrote:
> Based on 2001:3319 and 2357:0109 which I used to test the fix and
> 0bda:818b and 2357:0108 for which I found efuse dumps online.
> 
> == 2357:0109 ==
> === Before ===
> Vendor: Realtek
> Product: \x03802.11n NI
> Serial:
> === After ===
> Vendor: Realtek
> Product: 802.11n NIC
> Serial not available.
> 
> == 2001:3319 ==
> === Before ===
> Vendor: Realtek
> Product: Wireless N
> Serial: no USB Adap
> === After ===
> Vendor: Realtek
> Product: Wireless N Nano USB Adapter
> Serial not available.
> 
> Signed-off-by: Pascal Terjan <pterjan@google.com>
> ---
>  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  | 11 ++--
>  .../realtek/rtl8xxxu/rtl8xxxu_8192e.c         | 53 ++++++++++++++++---
>  2 files changed, 50 insertions(+), 14 deletions(-)

This makes sense, you may want to account for the total length of the
record though, see below.

Some cosmetic nits too.

> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index d6d1be4169e5..acb6b0cd3667 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -853,15 +853,10 @@ struct rtl8192eu_efuse {
>  	u8 usb_optional_function;
>  	u8 res9[2];
>  	u8 mac_addr[ETH_ALEN];		/* 0xd7 */
> -	u8 res10[2];
> -	u8 vendor_name[7];
> -	u8 res11[2];
> -	u8 device_name[0x0b];		/* 0xe8 */
> -	u8 res12[2];
> -	u8 serial[0x0b];		/* 0xf5 */
> -	u8 res13[0x30];
> +	u8 device_info[80];
> +	u8 res11[3];
>  	u8 unknown[0x0d];		/* 0x130 */
> -	u8 res14[0xc3];
> +	u8 res12[0xc3];
>  };
>  
>  struct rtl8xxxu_reg8val {
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
> index cfe2dfdae928..9c5fad49ed2a 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
> @@ -554,9 +554,39 @@ rtl8192e_set_tx_power(struct rtl8xxxu_priv *priv, int channel, bool ht40)
>  	}
>  }
>  
> +static void rtl8192eu_log_device_info(struct rtl8xxxu_priv *priv,
> +				      char *record_name,
> +				      char **record)
> +{
> +	/* A record is [ total length | 0x03 | value ] */
> +	unsigned char l = (*record)[0];

These parenthesis make no sense.

> +
> +	/* The whole section seems to be 80 characters so a record should not
> +	 * be able to be that large.
> +	 */

Please respect the comment formatting of the driver, ie
/*
 * Foo
 */


> +	if (l > 80) {
> +		dev_warn(&priv->udev->dev,
> +			 "invalid record length %d while parsing \"%s\".\n",
> +			 l, record_name);
> +		return;
> +	}

The 80 check is only valid for the first entry, consecutive entries are
already advanced. Maybe switch it over to use an index to address into
the record keep an index and just pass in efuse->device_info instead.

> +
> +	if (l >= 2) {
> +		char value[80];
> +
> +		memcpy(value, &(*record)[2], l - 2);
> +		value[l - 2] = '\0';
> +		dev_info(&priv->udev->dev, "%s: %s\n", record_name, value);
> +		*record = *record + l;
> +	} else {
> +		dev_info(&priv->udev->dev, "%s not available.\n", record_name);
> +	}
> +}
> +
>  static int rtl8192eu_parse_efuse(struct rtl8xxxu_priv *priv)
>  {
>  	struct rtl8192eu_efuse *efuse = &priv->efuse_wifi.efuse8192eu;
> +	char *record = efuse->device_info;
>  	int i;
>  
>  	if (efuse->rtl_id != cpu_to_le16(0x8129))
> @@ -604,12 +634,23 @@ static int rtl8192eu_parse_efuse(struct rtl8xxxu_priv *priv)
>  	priv->has_xtalk = 1;
>  	priv->xtalk = priv->efuse_wifi.efuse8192eu.xtal_k & 0x3f;
>  
> -	dev_info(&priv->udev->dev, "Vendor: %.7s\n", efuse->vendor_name);
> -	dev_info(&priv->udev->dev, "Product: %.11s\n", efuse->device_name);
> -	if (memchr_inv(efuse->serial, 0xff, 11))
> -		dev_info(&priv->udev->dev, "Serial: %.11s\n", efuse->serial);
> -	else
> -		dev_info(&priv->udev->dev, "Serial not available.\n");
> +	/* device_info section seems to be laid out as records
> +	 * [ total length | 0x03 | value ] so:
> +	 * - vendor length + 2
> +	 * - 0x03
> +	 * - vendor string (not null terminated)
> +	 * - product length + 2
> +	 * - 0x03
> +	 * - product string (not null terminated)
> +	 * Then there is one or 2 0x00 on all the 4 devices I own or found
> +	 * dumped online.
> +	 * As previous version of the code handled an optional serial
> +	 * string, I now assume there may be a third record if the
> +	 * length is not 0.
> +	 */
> +	rtl8192eu_log_device_info(priv, "Vendor", &record);
> +	rtl8192eu_log_device_info(priv, "Product", &record);
> +	rtl8192eu_log_device_info(priv, "Serial", &record);
>  
>  	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_EFUSE) {
>  		unsigned char *raw = priv->efuse_wifi.raw;
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] rtl8xxxu: Fix device info for RTL8192EU devices
  2021-04-19 11:53 ` Jes Sorensen
@ 2021-04-19 15:55   ` Pascal Terjan
  2021-04-24 17:29   ` [PATCH v2] " Pascal Terjan
  1 sibling, 0 replies; 6+ messages in thread
From: Pascal Terjan @ 2021-04-19 15:55 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: linux-wireless, linux-kernel

On Mon, 19 Apr 2021 at 12:53, Jes Sorensen <jes.sorensen@gmail.com> wrote:
>
> On 3/23/21 3:36 PM, Pascal Terjan wrote:
> > Based on 2001:3319 and 2357:0109 which I used to test the fix and
> > 0bda:818b and 2357:0108 for which I found efuse dumps online.
> >
> > == 2357:0109 ==
> > === Before ===
> > Vendor: Realtek
> > Product: \x03802.11n NI
> > Serial:
> > === After ===
> > Vendor: Realtek
> > Product: 802.11n NIC
> > Serial not available.
> >
> > == 2001:3319 ==
> > === Before ===
> > Vendor: Realtek
> > Product: Wireless N
> > Serial: no USB Adap
> > === After ===
> > Vendor: Realtek
> > Product: Wireless N Nano USB Adapter
> > Serial not available.
> >
> > Signed-off-by: Pascal Terjan <pterjan@google.com>
> > ---
> >  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  | 11 ++--
> >  .../realtek/rtl8xxxu/rtl8xxxu_8192e.c         | 53 ++++++++++++++++---
> >  2 files changed, 50 insertions(+), 14 deletions(-)
>
> This makes sense, you may want to account for the total length of the
> record though, see below.
>
> Some cosmetic nits too.

Thanks for the review, I'll send a v2

> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > index d6d1be4169e5..acb6b0cd3667 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > @@ -853,15 +853,10 @@ struct rtl8192eu_efuse {
> >       u8 usb_optional_function;
> >       u8 res9[2];
> >       u8 mac_addr[ETH_ALEN];          /* 0xd7 */
> > -     u8 res10[2];
> > -     u8 vendor_name[7];
> > -     u8 res11[2];
> > -     u8 device_name[0x0b];           /* 0xe8 */
> > -     u8 res12[2];
> > -     u8 serial[0x0b];                /* 0xf5 */
> > -     u8 res13[0x30];
> > +     u8 device_info[80];
> > +     u8 res11[3];
> >       u8 unknown[0x0d];               /* 0x130 */
> > -     u8 res14[0xc3];
> > +     u8 res12[0xc3];
> >  };
> >
> >  struct rtl8xxxu_reg8val {
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
> > index cfe2dfdae928..9c5fad49ed2a 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
> > @@ -554,9 +554,39 @@ rtl8192e_set_tx_power(struct rtl8xxxu_priv *priv, int channel, bool ht40)
> >       }
> >  }
> >
> > +static void rtl8192eu_log_device_info(struct rtl8xxxu_priv *priv,
> > +                                   char *record_name,
> > +                                   char **record)
> > +{
> > +     /* A record is [ total length | 0x03 | value ] */
> > +     unsigned char l = (*record)[0];
>
> These parenthesis make no sense.
>
> > +
> > +     /* The whole section seems to be 80 characters so a record should not
> > +      * be able to be that large.
> > +      */
>
> Please respect the comment formatting of the driver, ie
> /*
>  * Foo
>  */

I blame checkpatch telling me "WARNING: networking block comments
don't use an empty /* line, use /* Comment..." (and myself for not
checking the driver again when fixing it :) )

> > +     if (l > 80) {
> > +             dev_warn(&priv->udev->dev,
> > +                      "invalid record length %d while parsing \"%s\".\n",
> > +                      l, record_name);
> > +             return;
> > +     }
>
> The 80 check is only valid for the first entry, consecutive entries are
> already advanced. Maybe switch it over to use an index to address into
> the record keep an index and just pass in efuse->device_info instead.
>
> > +
> > +     if (l >= 2) {
> > +             char value[80];
> > +
> > +             memcpy(value, &(*record)[2], l - 2);
> > +             value[l - 2] = '\0';
> > +             dev_info(&priv->udev->dev, "%s: %s\n", record_name, value);
> > +             *record = *record + l;
> > +     } else {
> > +             dev_info(&priv->udev->dev, "%s not available.\n", record_name);
> > +     }
> > +}
> > +
> >  static int rtl8192eu_parse_efuse(struct rtl8xxxu_priv *priv)
> >  {
> >       struct rtl8192eu_efuse *efuse = &priv->efuse_wifi.efuse8192eu;
> > +     char *record = efuse->device_info;
> >       int i;
> >
> >       if (efuse->rtl_id != cpu_to_le16(0x8129))
> > @@ -604,12 +634,23 @@ static int rtl8192eu_parse_efuse(struct rtl8xxxu_priv *priv)
> >       priv->has_xtalk = 1;
> >       priv->xtalk = priv->efuse_wifi.efuse8192eu.xtal_k & 0x3f;
> >
> > -     dev_info(&priv->udev->dev, "Vendor: %.7s\n", efuse->vendor_name);
> > -     dev_info(&priv->udev->dev, "Product: %.11s\n", efuse->device_name);
> > -     if (memchr_inv(efuse->serial, 0xff, 11))
> > -             dev_info(&priv->udev->dev, "Serial: %.11s\n", efuse->serial);
> > -     else
> > -             dev_info(&priv->udev->dev, "Serial not available.\n");
> > +     /* device_info section seems to be laid out as records
> > +      * [ total length | 0x03 | value ] so:
> > +      * - vendor length + 2
> > +      * - 0x03
> > +      * - vendor string (not null terminated)
> > +      * - product length + 2
> > +      * - 0x03
> > +      * - product string (not null terminated)
> > +      * Then there is one or 2 0x00 on all the 4 devices I own or found
> > +      * dumped online.
> > +      * As previous version of the code handled an optional serial
> > +      * string, I now assume there may be a third record if the
> > +      * length is not 0.
> > +      */
> > +     rtl8192eu_log_device_info(priv, "Vendor", &record);
> > +     rtl8192eu_log_device_info(priv, "Product", &record);
> > +     rtl8192eu_log_device_info(priv, "Serial", &record);
> >
> >       if (rtl8xxxu_debug & RTL8XXXU_DEBUG_EFUSE) {
> >               unsigned char *raw = priv->efuse_wifi.raw;
> >
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v2] rtl8xxxu: Fix device info for RTL8192EU devices
  2021-04-19 11:53 ` Jes Sorensen
  2021-04-19 15:55   ` Pascal Terjan
@ 2021-04-24 17:29   ` Pascal Terjan
  2021-06-19  9:05     ` Kalle Valo
  1 sibling, 1 reply; 6+ messages in thread
From: Pascal Terjan @ 2021-04-24 17:29 UTC (permalink / raw)
  To: linux-wireless; +Cc: Jes Sorensen, Pascal Terjan

Based on 2001:3319 and 2357:0109 which I used to test the fix and
0bda:818b and 2357:0108 for which I found efuse dumps online.

== 2357:0109 ==
=== Before ===
Vendor: Realtek
Product: \x03802.11n NI
Serial:
=== After ===
Vendor: Realtek
Product: 802.11n NIC
Serial not available.

== 2001:3319 ==
=== Before ===
Vendor: Realtek
Product: Wireless N
Serial: no USB Adap
=== After ===
Vendor: Realtek
Product: Wireless N Nano USB Adapter
Serial not available.

Signed-off-by: Pascal Terjan <pterjan@google.com>
---
v2: properly check we don't get out of the 80 bytes section and cosmetic fixes

 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  | 11 +---
 .../realtek/rtl8xxxu/rtl8xxxu_8192e.c         | 59 +++++++++++++++++--
 2 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index d6d1be4169e5..acb6b0cd3667 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -853,15 +853,10 @@ struct rtl8192eu_efuse {
 	u8 usb_optional_function;
 	u8 res9[2];
 	u8 mac_addr[ETH_ALEN];		/* 0xd7 */
-	u8 res10[2];
-	u8 vendor_name[7];
-	u8 res11[2];
-	u8 device_name[0x0b];		/* 0xe8 */
-	u8 res12[2];
-	u8 serial[0x0b];		/* 0xf5 */
-	u8 res13[0x30];
+	u8 device_info[80];
+	u8 res11[3];
 	u8 unknown[0x0d];		/* 0x130 */
-	u8 res14[0xc3];
+	u8 res12[0xc3];
 };
 
 struct rtl8xxxu_reg8val {
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index cfe2dfdae928..b06508d0cdf8 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -554,9 +554,43 @@ rtl8192e_set_tx_power(struct rtl8xxxu_priv *priv, int channel, bool ht40)
 	}
 }
 
+static void rtl8192eu_log_next_device_info(struct rtl8xxxu_priv *priv,
+					   char *record_name,
+					   char *device_info,
+					   unsigned int *record_offset)
+{
+	char *record = device_info + *record_offset;
+
+	/* A record is [ total length | 0x03 | value ] */
+	unsigned char l = record[0];
+
+	/*
+	 * The whole device info section seems to be 80 characters, make sure
+	 * we don't read further.
+	 */
+	if (*record_offset + l > 80) {
+		dev_warn(&priv->udev->dev,
+			 "invalid record length %d while parsing \"%s\" at offset %u.\n",
+			 l, record_name, *record_offset);
+		return;
+	}
+
+	if (l >= 2) {
+		char value[80];
+
+		memcpy(value, &record[2], l - 2);
+		value[l - 2] = '\0';
+		dev_info(&priv->udev->dev, "%s: %s\n", record_name, value);
+		*record_offset = *record_offset + l;
+	} else {
+		dev_info(&priv->udev->dev, "%s not available.\n", record_name);
+	}
+}
+
 static int rtl8192eu_parse_efuse(struct rtl8xxxu_priv *priv)
 {
 	struct rtl8192eu_efuse *efuse = &priv->efuse_wifi.efuse8192eu;
+	unsigned int record_offset;
 	int i;
 
 	if (efuse->rtl_id != cpu_to_le16(0x8129))
@@ -604,12 +638,25 @@ static int rtl8192eu_parse_efuse(struct rtl8xxxu_priv *priv)
 	priv->has_xtalk = 1;
 	priv->xtalk = priv->efuse_wifi.efuse8192eu.xtal_k & 0x3f;
 
-	dev_info(&priv->udev->dev, "Vendor: %.7s\n", efuse->vendor_name);
-	dev_info(&priv->udev->dev, "Product: %.11s\n", efuse->device_name);
-	if (memchr_inv(efuse->serial, 0xff, 11))
-		dev_info(&priv->udev->dev, "Serial: %.11s\n", efuse->serial);
-	else
-		dev_info(&priv->udev->dev, "Serial not available.\n");
+	/*
+	 * device_info section seems to be laid out as records
+	 * [ total length | 0x03 | value ] so:
+	 * - vendor length + 2
+	 * - 0x03
+	 * - vendor string (not null terminated)
+	 * - product length + 2
+	 * - 0x03
+	 * - product string (not null terminated)
+	 * Then there is one or 2 0x00 on all the 4 devices I own or found
+	 * dumped online.
+	 * As previous version of the code handled an optional serial
+	 * string, I now assume there may be a third record if the
+	 * length is not 0.
+	 */
+	record_offset = 0;
+	rtl8192eu_log_next_device_info(priv, "Vendor", efuse->device_info, &record_offset);
+	rtl8192eu_log_next_device_info(priv, "Product", efuse->device_info, &record_offset);
+	rtl8192eu_log_next_device_info(priv, "Serial", efuse->device_info, &record_offset);
 
 	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_EFUSE) {
 		unsigned char *raw = priv->efuse_wifi.raw;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] rtl8xxxu: Fix device info for RTL8192EU devices
  2021-04-24 17:29   ` [PATCH v2] " Pascal Terjan
@ 2021-06-19  9:05     ` Kalle Valo
  0 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2021-06-19  9:05 UTC (permalink / raw)
  To: Pascal Terjan; +Cc: linux-wireless, Jes Sorensen, Pascal Terjan

Pascal Terjan <pterjan@google.com> wrote:

> Based on 2001:3319 and 2357:0109 which I used to test the fix and
> 0bda:818b and 2357:0108 for which I found efuse dumps online.
> 
> == 2357:0109 ==
> === Before ===
> Vendor: Realtek
> Product: \x03802.11n NI
> Serial:
> === After ===
> Vendor: Realtek
> Product: 802.11n NIC
> Serial not available.
> 
> == 2001:3319 ==
> === Before ===
> Vendor: Realtek
> Product: Wireless N
> Serial: no USB Adap
> === After ===
> Vendor: Realtek
> Product: Wireless N Nano USB Adapter
> Serial not available.
> 
> Signed-off-by: Pascal Terjan <pterjan@google.com>

Patch applied to wireless-drivers-next.git, thanks.

c240b044edef rtl8xxxu: Fix device info for RTL8192EU devices

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20210424172959.1559890-1-pterjan@google.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-06-19  9:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-23 19:36 [PATCH] rtl8xxxu: Fix device info for RTL8192EU devices Pascal Terjan
2021-04-18  5:57 ` Kalle Valo
2021-04-19 11:53 ` Jes Sorensen
2021-04-19 15:55   ` Pascal Terjan
2021-04-24 17:29   ` [PATCH v2] " Pascal Terjan
2021-06-19  9:05     ` Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).