RE: [PATCH] wifi: rtw89: 8922a: set random mac if efuse contains zeroes

[Ping-Ke Shih <pkshih@realtek.com>]

Jose Ignacio Tornos Martinez <jtornosm@redhat.com> wrote:
> I have some rtl8952ae devices with no permanent mac stored in efuse.

I think rtl8952ae is NOT a typo, but it is surprising me that this driver can
bring it up. Does it really work well? And where did you get the hardware?

> 
> It could be properly saved and/or configured from user tools like
> NetworkManager, but it would be desirable to be able to initialize it
> somehow to get the device working by default.
> 
> So, in the same way as with other devices, if the mac address read from
> efuse contains zeros, a random mac address is assigned to at least allow
> operation, and the user is warned about this in case any action needs to
> be considered.
> 
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
> ---
>  drivers/net/wireless/realtek/rtw89/rtw8922a.c | 22 +++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw89/rtw8922a.c
> b/drivers/net/wireless/realtek/rtw89/rtw8922a.c
> index 6aa19ad259ac..050307fa16d2 100644
> --- a/drivers/net/wireless/realtek/rtw89/rtw8922a.c
> +++ b/drivers/net/wireless/realtek/rtw89/rtw8922a.c
> @@ -636,16 +636,30 @@ static int rtw8922a_read_efuse_rf(struct rtw89_dev *rtwdev, u8 *log_map)
>  static int rtw8922a_read_efuse(struct rtw89_dev *rtwdev, u8 *log_map,
>                                enum rtw89_efuse_block block)
>  {
> +       struct rtw89_efuse *efuse = &rtwdev->efuse;
> +       int ret;
> +
>         switch (block) {
>         case RTW89_EFUSE_BLOCK_HCI_DIG_PCIE_SDIO:
> -               return rtw8922a_read_efuse_pci_sdio(rtwdev, log_map);
> +               ret = rtw8922a_read_efuse_pci_sdio(rtwdev, log_map);
> +               break;
>         case RTW89_EFUSE_BLOCK_HCI_DIG_USB:
> -               return rtw8922a_read_efuse_usb(rtwdev, log_map);
> +               ret = rtw8922a_read_efuse_usb(rtwdev, log_map);
> +               break;
>         case RTW89_EFUSE_BLOCK_RF:
> -               return rtw8922a_read_efuse_rf(rtwdev, log_map);
> +               ret = rtw8922a_read_efuse_rf(rtwdev, log_map);
> +               break;
>         default:
> -               return 0;
> +               ret = 0;
> +               break;
> +       }
> +
> +       if (!ret && is_zero_ether_addr(efuse->addr)) {
> +               rtw89_warn(rtwdev,"efuse mac address is zero, using random mac");

I think rtw89_info() is more suitable.

Need a '\n' at end of the message, and you missed a space after comma. 

> +               eth_random_addr(efuse->addr);
>         }
> +
> +       return ret;
>  }
> 
>  #define THM_TRIM_POSITIVE_MASK BIT(6)
> --
> 2.51.1