[PATCH] wilc1000: Convert module-global "isinit" to device-specific variable

[PATCH] wilc1000: Convert module-global "isinit" to device-specific variable

[David Mosberger-Tang]

Signed-off-by: David Mosberger-Tang <davidm@egauge.net>
---
 drivers/net/wireless/microchip/wilc1000/spi.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/spi.c b/drivers/net/wireless/microchip/wilc1000/spi.c
index 8b180c29d682..dd481dc0b5ce 100644
--- a/drivers/net/wireless/microchip/wilc1000/spi.c
+++ b/drivers/net/wireless/microchip/wilc1000/spi.c
@@ -39,6 +39,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 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 */
@@ -908,15 +909,15 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
 	struct wilc_spi *spi_priv = wilc->bus_data;
 	u32 reg;
 	u32 chipid;
-	static int isinit;
 	int ret, i;
 
-	if (isinit) {
+	if (spi_priv->isinit) {
+		/* Confirm we can read chipid register without error: */
 		ret = wilc_spi_read_reg(wilc, WILC_CHIPID, &chipid);
-		if (ret)
-			dev_err(&spi->dev, "Fail cmd read chip id...\n");
+		if (ret == 0)
+			return 0;
 
-		return ret;
+		dev_err(&spi->dev, "Fail cmd read chip id...\n");
 	}
 
 	/*
@@ -974,7 +975,7 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
 	spi_priv->probing_crc = false;
 
 	/*
-	 * make sure can read back chip id correctly
+	 * make sure can read chip id without protocol error
 	 */
 	ret = wilc_spi_read_reg(wilc, WILC_CHIPID, &chipid);
 	if (ret) {
@@ -982,7 +983,7 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
 		return ret;
 	}
 
-	isinit = 1;
+	spi_priv->isinit = true;
 
 	return 0;
 }
-- 
2.25.1

Re: [PATCH] wilc1000: Convert module-global "isinit" to device-specific variable

[Ajay.Kathat]

Hi David,


On 27/08/21 1:42 am, David Mosberger-Tang wrote:


The code changes look okay but the commit description is missing. Please 
add the appropriate commit description.

Btw, I too had submitted one patch[1] to avoid use of the static 
variable(isinit) in patch series [2] but I still have to resubmit the 
series after some rework. As your patch has landed first so I will 
realign my V2 series on top of your patch.


Regards,

Ajay


[1] 
https://patchwork.kernel.org/project/linux-wireless/patch/20210423182925.5865-10-ajay.kathat@microchip.com/

[2]. https://www.spinics.net/lists/linux-wireless/msg210278.html


> Signed-off-by: David Mosberger-Tang <davidm@egauge.net>
> ---
>   drivers/net/wireless/microchip/wilc1000/spi.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/microchip/wilc1000/spi.c b/drivers/net/wireless/microchip/wilc1000/spi.c
> index 8b180c29d682..dd481dc0b5ce 100644
> --- a/drivers/net/wireless/microchip/wilc1000/spi.c
> +++ b/drivers/net/wireless/microchip/wilc1000/spi.c
> @@ -39,6 +39,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 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 */
> @@ -908,15 +909,15 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
>          struct wilc_spi *spi_priv = wilc->bus_data;
>          u32 reg;
>          u32 chipid;
> -       static int isinit;
>          int ret, i;
>
> -       if (isinit) {
> +       if (spi_priv->isinit) {
> +               /* Confirm we can read chipid register without error: */
>                  ret = wilc_spi_read_reg(wilc, WILC_CHIPID, &chipid);
> -               if (ret)
> -                       dev_err(&spi->dev, "Fail cmd read chip id...\n");
> +               if (ret == 0)
> +                       return 0;
>
> -               return ret;
> +               dev_err(&spi->dev, "Fail cmd read chip id...\n");
>          }
>
>          /*
> @@ -974,7 +975,7 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
>          spi_priv->probing_crc = false;
>
>          /*
> -        * make sure can read back chip id correctly
> +        * make sure can read chip id without protocol error
>           */
>          ret = wilc_spi_read_reg(wilc, WILC_CHIPID, &chipid);
>          if (ret) {
> @@ -982,7 +983,7 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
>                  return ret;
>          }
>
> -       isinit = 1;
> +       spi_priv->isinit = true;
>
>          return 0;
>   }
> --
> 2.25.1
>