diff mbox series

[6/6] wifi: wilc1000: read MAC address from fuse at probe

Message ID 20240417-mac_addr_at_probe-v1-6-67d6c9b3bc2b@bootlin.com (mailing list archive)
State Deferred
Delegated to: Kalle Valo
Headers show
Series wifi: wilc1000: fix default mac address | expand

Commit Message

Alexis Lothoré April 17, 2024, 9:34 a.m. UTC
From: Ajay Singh <ajay.kathat@microchip.com>

The default netdev interface exposed by WILC1000 is registered at probe,
but the chip mac address is not known until ndo_open, which will load and
start chip firmware and then retrieve stored MAC address from it. As a
consequence, the interface has uninitialized value (00:00:00:00:00) until a
user brings up the interface.

Fix MAC address at probe by setting the following steps:
- at probe, read MAC address directly from fuse
- whenever a new netdevice is created, apply saved mac address (which can
  be a user-provided address, or the eFuse Mac address if no address has
  been passed by user)
- whenever an interface is brought up for the first time (and so the
  firmware is loaded and started), enforce netdevice mac address to the
  chip (in case user has changed it)

Reported-by: Heiko Thiery <heiko.thiery@gmail.com>
Closes: https://lore.kernel.org/netdev/CAEyMn7aV-B4OEhHR4Ad0LM3sKCz1-nDqSb9uZNmRWR-hMZ=z+A@mail.gmail.com/T/
Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
Co-developed-by: Alexis Lothoré <alexis.lothore@bootlin.com>
Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 drivers/net/wireless/microchip/wilc1000/netdev.c | 38 ++++++++++++++----------
 drivers/net/wireless/microchip/wilc1000/sdio.c   | 14 +++++++++
 drivers/net/wireless/microchip/wilc1000/spi.c    |  6 ++++
 3 files changed, 42 insertions(+), 16 deletions(-)

Comments

Heiko Thiery April 23, 2024, 8 a.m. UTC | #1
Hi Alexis, All

Am Mi., 17. Apr. 2024 um 11:34 Uhr schrieb Alexis Lothoré
<alexis.lothore@bootlin.com>:
>
> From: Ajay Singh <ajay.kathat@microchip.com>
>
> The default netdev interface exposed by WILC1000 is registered at probe,
> but the chip mac address is not known until ndo_open, which will load and
> start chip firmware and then retrieve stored MAC address from it. As a
> consequence, the interface has uninitialized value (00:00:00:00:00) until a
> user brings up the interface.
>
> Fix MAC address at probe by setting the following steps:
> - at probe, read MAC address directly from fuse
> - whenever a new netdevice is created, apply saved mac address (which can
>   be a user-provided address, or the eFuse Mac address if no address has
>   been passed by user)
> - whenever an interface is brought up for the first time (and so the
>   firmware is loaded and started), enforce netdevice mac address to the
>   chip (in case user has changed it)
>
> Reported-by: Heiko Thiery <heiko.thiery@gmail.com>
> Closes: https://lore.kernel.org/netdev/CAEyMn7aV-B4OEhHR4Ad0LM3sKCz1-nDqSb9uZNmRWR-hMZ=z+A@mail.gmail.com/T/
> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> Co-developed-by: Alexis Lothoré <alexis.lothore@bootlin.com>
> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>

I applied the series on next and tested it on my env. It looks good.

Tested-by: Heiko Thiery <heiko.thiery@gmail.com>

Thank you!
Heiko

> ---
>  drivers/net/wireless/microchip/wilc1000/netdev.c | 38 ++++++++++++++----------
>  drivers/net/wireless/microchip/wilc1000/sdio.c   | 14 +++++++++
>  drivers/net/wireless/microchip/wilc1000/spi.c    |  6 ++++
>  3 files changed, 42 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
> index 5ab448d0b643..f1fbc6e8a82a 100644
> --- a/drivers/net/wireless/microchip/wilc1000/netdev.c
> +++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
> @@ -588,7 +588,6 @@ static int wilc_mac_open(struct net_device *ndev)
>         struct wilc *wl = vif->wilc;
>         int ret = 0;
>         struct mgmt_frame_regs mgmt_regs = {};
> -       u8 addr[ETH_ALEN] __aligned(2);
>
>         if (!wl || !wl->dev) {
>                 netdev_err(ndev, "device not ready\n");
> @@ -607,25 +606,19 @@ static int wilc_mac_open(struct net_device *ndev)
>                 return ret;
>         }
>
> -       wilc_set_operation_mode(vif, wilc_get_vif_idx(vif), vif->iftype,
> -                               vif->idx);
> -
> -       if (is_valid_ether_addr(ndev->dev_addr)) {
> -               ether_addr_copy(addr, ndev->dev_addr);
> -               wilc_set_mac_address(vif, addr);
> -       } else {
> -               wilc_get_mac_address(vif, addr);
> -               eth_hw_addr_set(ndev, addr);
> -       }
>         netdev_dbg(ndev, "Mac address: %pM\n", ndev->dev_addr);
> -
> -       if (!is_valid_ether_addr(ndev->dev_addr)) {
> -               netdev_err(ndev, "Wrong MAC address\n");
> +       ret = wilc_set_mac_address(vif, ndev->dev_addr);
> +       if (ret) {
> +               netdev_err(ndev, "Failed to enforce MAC address in chip");
>                 wilc_deinit_host_int(ndev);
> -               wilc_wlan_deinitialize(ndev);
> -               return -EINVAL;
> +               if (!wl->open_ifcs)
> +                       wilc_wlan_deinitialize(ndev);
> +               return ret;
>         }
>
> +       wilc_set_operation_mode(vif, wilc_get_vif_idx(vif), vif->iftype,
> +                               vif->idx);
> +
>         mgmt_regs.interface_stypes = vif->mgmt_reg_stypes;
>         /* so we detect a change */
>         vif->mgmt_reg_stypes = 0;
> @@ -941,6 +934,7 @@ struct wilc_vif *wilc_netdev_ifc_init(struct wilc *wl, const char *name,
>                                       int vif_type, enum nl80211_iftype type,
>                                       bool rtnl_locked)
>  {
> +       u8 mac_address[ETH_ALEN];
>         struct net_device *ndev;
>         struct wilc_vif *vif;
>         int ret;
> @@ -969,6 +963,18 @@ struct wilc_vif *wilc_netdev_ifc_init(struct wilc *wl, const char *name,
>         vif->iftype = vif_type;
>         vif->idx = wilc_get_available_idx(wl);
>         vif->mac_opened = 0;
> +
> +       memcpy(mac_address, wl->nv_mac_address, ETH_ALEN);
> +       /* WILC firmware uses locally administered MAC address for the
> +        * second virtual interface (bit 1 of first byte set), but
> +        * since it is possibly not loaded/running yet, reproduce this behavior
> +        * in the driver during interface creation.
> +        */
> +       if (vif->idx)
> +               mac_address[0] |= 0x2;
> +
> +       eth_hw_addr_set(vif->ndev, mac_address);
> +
>         mutex_lock(&wl->vif_mutex);
>         list_add_tail_rcu(&vif->list, &wl->vif_list);
>         wl->vif_num += 1;
> diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c b/drivers/net/wireless/microchip/wilc1000/sdio.c
> index 04d6565df2cb..e6e20c86b791 100644
> --- a/drivers/net/wireless/microchip/wilc1000/sdio.c
> +++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
> @@ -24,6 +24,9 @@ MODULE_DEVICE_TABLE(sdio, wilc_sdio_ids);
>
>  #define WILC_SDIO_BLOCK_SIZE 512
>
> +static int wilc_sdio_init(struct wilc *wilc, bool resume);
> +static int wilc_sdio_deinit(struct wilc *wilc);
> +
>  struct wilc_sdio {
>         bool irq_gpio;
>         u32 block_size;
> @@ -178,6 +181,16 @@ static int wilc_sdio_probe(struct sdio_func *func,
>         }
>         clk_prepare_enable(wilc->rtc_clk);
>
> +       wilc_sdio_init(wilc, false);
> +
> +       ret = wilc_load_mac_from_nv(wilc);
> +       if (ret) {
> +               pr_err("Can not retrieve MAC address from chip\n");
> +               goto clk_disable_unprepare;
> +       }
> +
> +       wilc_sdio_deinit(wilc);
> +
>         vif = wilc_netdev_ifc_init(wilc, "wlan%d", WILC_STATION_MODE,
>                                    NL80211_IFTYPE_STATION, false);
>         if (IS_ERR(vif)) {
> @@ -187,6 +200,7 @@ static int wilc_sdio_probe(struct sdio_func *func,
>
>         dev_info(&func->dev, "Driver Initializing success\n");
>         return 0;
> +
>  clk_disable_unprepare:
>         clk_disable_unprepare(wilc->rtc_clk);
>  dispose_irq:
> diff --git a/drivers/net/wireless/microchip/wilc1000/spi.c b/drivers/net/wireless/microchip/wilc1000/spi.c
> index add0e70a09ea..5ff940c53ad9 100644
> --- a/drivers/net/wireless/microchip/wilc1000/spi.c
> +++ b/drivers/net/wireless/microchip/wilc1000/spi.c
> @@ -250,6 +250,12 @@ static int wilc_bus_probe(struct spi_device *spi)
>         if (ret)
>                 goto power_down;
>
> +       ret = wilc_load_mac_from_nv(wilc);
> +       if (ret) {
> +               pr_err("Can not retrieve MAC address from chip\n");
> +               goto power_down;
> +       }
> +
>         wilc_wlan_power(wilc, false);
>         vif = wilc_netdev_ifc_init(wilc, "wlan%d", WILC_STATION_MODE,
>                                    NL80211_IFTYPE_STATION, false);
>
> --
> 2.44.0
>
diff mbox series

Patch

diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
index 5ab448d0b643..f1fbc6e8a82a 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.c
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
@@ -588,7 +588,6 @@  static int wilc_mac_open(struct net_device *ndev)
 	struct wilc *wl = vif->wilc;
 	int ret = 0;
 	struct mgmt_frame_regs mgmt_regs = {};
-	u8 addr[ETH_ALEN] __aligned(2);
 
 	if (!wl || !wl->dev) {
 		netdev_err(ndev, "device not ready\n");
@@ -607,25 +606,19 @@  static int wilc_mac_open(struct net_device *ndev)
 		return ret;
 	}
 
-	wilc_set_operation_mode(vif, wilc_get_vif_idx(vif), vif->iftype,
-				vif->idx);
-
-	if (is_valid_ether_addr(ndev->dev_addr)) {
-		ether_addr_copy(addr, ndev->dev_addr);
-		wilc_set_mac_address(vif, addr);
-	} else {
-		wilc_get_mac_address(vif, addr);
-		eth_hw_addr_set(ndev, addr);
-	}
 	netdev_dbg(ndev, "Mac address: %pM\n", ndev->dev_addr);
-
-	if (!is_valid_ether_addr(ndev->dev_addr)) {
-		netdev_err(ndev, "Wrong MAC address\n");
+	ret = wilc_set_mac_address(vif, ndev->dev_addr);
+	if (ret) {
+		netdev_err(ndev, "Failed to enforce MAC address in chip");
 		wilc_deinit_host_int(ndev);
-		wilc_wlan_deinitialize(ndev);
-		return -EINVAL;
+		if (!wl->open_ifcs)
+			wilc_wlan_deinitialize(ndev);
+		return ret;
 	}
 
+	wilc_set_operation_mode(vif, wilc_get_vif_idx(vif), vif->iftype,
+				vif->idx);
+
 	mgmt_regs.interface_stypes = vif->mgmt_reg_stypes;
 	/* so we detect a change */
 	vif->mgmt_reg_stypes = 0;
@@ -941,6 +934,7 @@  struct wilc_vif *wilc_netdev_ifc_init(struct wilc *wl, const char *name,
 				      int vif_type, enum nl80211_iftype type,
 				      bool rtnl_locked)
 {
+	u8 mac_address[ETH_ALEN];
 	struct net_device *ndev;
 	struct wilc_vif *vif;
 	int ret;
@@ -969,6 +963,18 @@  struct wilc_vif *wilc_netdev_ifc_init(struct wilc *wl, const char *name,
 	vif->iftype = vif_type;
 	vif->idx = wilc_get_available_idx(wl);
 	vif->mac_opened = 0;
+
+	memcpy(mac_address, wl->nv_mac_address, ETH_ALEN);
+	/* WILC firmware uses locally administered MAC address for the
+	 * second virtual interface (bit 1 of first byte set), but
+	 * since it is possibly not loaded/running yet, reproduce this behavior
+	 * in the driver during interface creation.
+	 */
+	if (vif->idx)
+		mac_address[0] |= 0x2;
+
+	eth_hw_addr_set(vif->ndev, mac_address);
+
 	mutex_lock(&wl->vif_mutex);
 	list_add_tail_rcu(&vif->list, &wl->vif_list);
 	wl->vif_num += 1;
diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c b/drivers/net/wireless/microchip/wilc1000/sdio.c
index 04d6565df2cb..e6e20c86b791 100644
--- a/drivers/net/wireless/microchip/wilc1000/sdio.c
+++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
@@ -24,6 +24,9 @@  MODULE_DEVICE_TABLE(sdio, wilc_sdio_ids);
 
 #define WILC_SDIO_BLOCK_SIZE 512
 
+static int wilc_sdio_init(struct wilc *wilc, bool resume);
+static int wilc_sdio_deinit(struct wilc *wilc);
+
 struct wilc_sdio {
 	bool irq_gpio;
 	u32 block_size;
@@ -178,6 +181,16 @@  static int wilc_sdio_probe(struct sdio_func *func,
 	}
 	clk_prepare_enable(wilc->rtc_clk);
 
+	wilc_sdio_init(wilc, false);
+
+	ret = wilc_load_mac_from_nv(wilc);
+	if (ret) {
+		pr_err("Can not retrieve MAC address from chip\n");
+		goto clk_disable_unprepare;
+	}
+
+	wilc_sdio_deinit(wilc);
+
 	vif = wilc_netdev_ifc_init(wilc, "wlan%d", WILC_STATION_MODE,
 				   NL80211_IFTYPE_STATION, false);
 	if (IS_ERR(vif)) {
@@ -187,6 +200,7 @@  static int wilc_sdio_probe(struct sdio_func *func,
 
 	dev_info(&func->dev, "Driver Initializing success\n");
 	return 0;
+
 clk_disable_unprepare:
 	clk_disable_unprepare(wilc->rtc_clk);
 dispose_irq:
diff --git a/drivers/net/wireless/microchip/wilc1000/spi.c b/drivers/net/wireless/microchip/wilc1000/spi.c
index add0e70a09ea..5ff940c53ad9 100644
--- a/drivers/net/wireless/microchip/wilc1000/spi.c
+++ b/drivers/net/wireless/microchip/wilc1000/spi.c
@@ -250,6 +250,12 @@  static int wilc_bus_probe(struct spi_device *spi)
 	if (ret)
 		goto power_down;
 
+	ret = wilc_load_mac_from_nv(wilc);
+	if (ret) {
+		pr_err("Can not retrieve MAC address from chip\n");
+		goto power_down;
+	}
+
 	wilc_wlan_power(wilc, false);
 	vif = wilc_netdev_ifc_init(wilc, "wlan%d", WILC_STATION_MODE,
 				   NL80211_IFTYPE_STATION, false);