Re: [PATCH 6/6] wifi: wilc1000: read MAC address from fuse at probe
From: Heiko Thiery
Date: Tue Apr 23 2024 - 04:03:30 EST
Hi Alexis, All
Am Mi., 17. Apr. 2024 um 11:34 Uhr schrieb Alexis Lothoré
<alexis.lothore@xxxxxxxxxxx>:
>
> From: Ajay Singh <ajay.kathat@xxxxxxxxxxxxx>
>
> 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@xxxxxxxxx>
> Closes: https://lore.kernel.org/netdev/CAEyMn7aV-B4OEhHR4Ad0LM3sKCz1-nDqSb9uZNmRWR-hMZ=z+A@xxxxxxxxxxxxxx/T/
> Signed-off-by: Ajay Singh <ajay.kathat@xxxxxxxxxxxxx>
> Co-developed-by: Alexis Lothoré <alexis.lothore@xxxxxxxxxxx>
> Signed-off-by: Alexis Lothoré <alexis.lothore@xxxxxxxxxxx>
I applied the series on next and tested it on my env. It looks good.
Tested-by: Heiko Thiery <heiko.thiery@xxxxxxxxx>
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
>