The current version of the wilc1000 driver has a probe function that simply
assumes the chip is present. It is only later, in wilc_spi_init(), that the
driver verifies that it can actually communicate with the chip. The result of
this is that the net device (typically wlan0) is created and remains in place as
long as the wilc1000-spi driver is loaded, even if the WILC1000 chip is not
present or not working.
Is there any reason not to detect the chip's present in wilc_bus_probe()? The
patch below (relative to 5.15.147) works for me, but perhaps I'm missing
something? Would it make sense to merge something along these lines into
mainline?
--david
diff --git a/drivers/net/wireless/microchip/wilc1000/spi.c
b/drivers/net/wireless/microchip/wilc1000/spi.c
index 6bac52527e38..c7ab816d65bc 100644
--- a/drivers/net/wireless/microchip/wilc1000/spi.c
+++ b/drivers/net/wireless/microchip/wilc1000/spi.c
@@ -42,7 +42,7 @@ MODULE_PARM_DESC(enable_crc16,
#define WILC_SPI_RSP_HDR_EXTRA_DATA 8
struct wilc_spi {
- bool isinit; /* true if SPI protocol has been configured */
+ bool isinit; /* true if wilc_spi_init was successful */
bool probing_crc; /* true if we're probing chip's CRC config */
bool crc7_enabled; /* true if crc7 is currently enabled */
bool crc16_enabled; /* true if crc16 is currently enabled */
@@ -55,6 +55,7 @@ struct wilc_spi {
static const struct wilc_hif_func wilc_hif_spi;
static int wilc_spi_reset(struct wilc *wilc);
+static int wilc_spi_configure_bus_protocol(struct wilc *);
/********************************************
*
@@ -232,6 +233,22 @@ static int wilc_bus_probe(struct spi_device *spi)
}
clk_prepare_enable(wilc->rtc_clk);
+ dev_info(&spi->dev, "crc7=%sabled crc16=%sabled",
+ enable_crc7 ? "en" : "dis", enable_crc16 ? "en" : "dis");
+
+ /* we need power to configure the bus protocol and to read the chip id: */
+
+ wilc_wlan_power(wilc, true);
+
+ ret = wilc_spi_configure_bus_protocol(wilc);
+
+ wilc_wlan_power(wilc, false);
+
+ if (ret) {
+ ret = -ENODEV;
+ goto netdev_cleanup;
+ }
+
return 0;
netdev_cleanup:
@@ -1108,7 +1125,7 @@ static int wilc_spi_deinit(struct wilc *wilc)
return 0;
}
-static int wilc_spi_init(struct wilc *wilc, bool resume)
+static int wilc_spi_configure_bus_protocol (struct wilc *wilc)
{
struct spi_device *spi = to_spi_device(wilc->dev);
struct wilc_spi *spi_priv = wilc->bus_data;
@@ -1116,21 +1133,6 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
u32 chipid;
int ret, i;
- if (spi_priv->isinit) {
- /* Confirm we can read chipid register without error: */
- ret = wilc_spi_read_reg(wilc, WILC_CHIPID, &chipid);
- if (ret == 0)
- return 0;
-
- dev_err(&spi->dev, "Fail cmd read chip id...\n");
- }
-
- wilc_wlan_power(wilc, true);
-
- /*
- * configure protocol
- */
-
/*
* Infer the CRC settings that are currently in effect. This
* is necessary because we can't be sure that the chip has
@@ -1147,7 +1149,7 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
}
if (ret) {
dev_err(&spi->dev, "Failed with CRC7 on and off.\n");
- goto fail;
+ return ret;
}
/* set up the desired CRC configuration: */
@@ -1170,7 +1172,7 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
dev_err(&spi->dev,
"[wilc spi %d]: Failed internal write reg\n",
__LINE__);
- goto fail;
+ return ret;
}
/* update our state to match new protocol settings: */
spi_priv->crc7_enabled = enable_crc7;
@@ -1187,16 +1189,38 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
ret = wilc_spi_read_reg(wilc, WILC_CHIPID, &chipid);
if (ret) {
dev_err(&spi->dev, "Fail cmd read chip id...\n");
- goto fail;
+ return ret;
+ }
+ return 0;
+}
+
+static int wilc_spi_init(struct wilc *wilc, bool resume)
+{
+ struct spi_device *spi = to_spi_device(wilc->dev);
+ struct wilc_spi *spi_priv = wilc->bus_data;
+ u32 chipid;
+ int ret;
+
+ if (spi_priv->isinit) {
+ /* Confirm we can read chipid register without error: */
+ ret = wilc_spi_read_reg(wilc, WILC_CHIPID, &chipid);
+ if (ret == 0)
+ return 0;
+
+ dev_err(&spi->dev, "Fail cmd read chip id...\n");
+ }
+
+ wilc_wlan_power(wilc, true);
+
+ ret = wilc_spi_configure_bus_protocol(wilc);
+ if (ret) {
+ wilc_wlan_power(wilc, false);
+ return ret;
}
spi_priv->isinit = true;
return 0;
-
- fail:
- wilc_wlan_power(wilc, false);
- return ret;
}
static int wilc_spi_read_size(struct wilc *wilc, u32 *size)
