[PATCH] staging: greybus: spilib: Handle SPI device types with a switch statement

[PATCH] staging: greybus: spilib: Handle SPI device types with a switch statement

[singh.supreet14]

From: Supreet <singh.supreet14@gmail.com>

Replaced the if/else chain with switch statement. makes it easier to
extend support for additional device types.

Remove the temporary spidev variable since it is only used to test the
return value of spi_new_device(). Call spi_new_device() directly in
the conditional instead.

checkpatch was run, had no warnings and errors.

Signed-off-by: Supreet <singh.supreet14@gmail.com>
---
 drivers/staging/greybus/spilib.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c
index e4d1ae8308aa..53fc0c124754 100644
--- a/drivers/staging/greybus/spilib.c
+++ b/drivers/staging/greybus/spilib.c
@@ -444,7 +444,6 @@ static int gb_spi_setup_device(struct gb_spilib *spi, u8 cs)
 	struct gb_spi_device_config_request request;
 	struct gb_spi_device_config_response response;
 	struct spi_board_info spi_board = { {0} };
-	struct spi_device *spidev;
 	int ret;
 	u8 dev_type;
 
@@ -458,25 +457,29 @@ static int gb_spi_setup_device(struct gb_spilib *spi, u8 cs)
 
 	dev_type = response.device_type;
 
-	if (dev_type == GB_SPI_SPI_DEV)
+	switch (dev_type) {
+	case GB_SPI_SPI_DEV:
 		strscpy(spi_board.modalias, "spidev",
 			sizeof(spi_board.modalias));
-	else if (dev_type == GB_SPI_SPI_NOR)
+		break;
+	case GB_SPI_SPI_NOR:
 		strscpy(spi_board.modalias, "spi-nor",
 			sizeof(spi_board.modalias));
-	else if (dev_type == GB_SPI_SPI_MODALIAS)
+		break;
+	case GB_SPI_SPI_MODALIAS:
 		memcpy(spi_board.modalias, response.name,
-		       sizeof(spi_board.modalias));
-	else
+		sizeof(spi_board.modalias));
+		break;
+	default:
 		return -EINVAL;
+	}
 
 	spi_board.mode		= le16_to_cpu(response.mode);
 	spi_board.bus_num	= ctlr->bus_num;
 	spi_board.chip_select	= cs;
 	spi_board.max_speed_hz	= le32_to_cpu(response.max_speed_hz);
 
-	spidev = spi_new_device(ctlr, &spi_board);
-	if (!spidev)
+	if (!spi_new_device(ctlr, &spi_board))
 		return -EINVAL;
 
 	return 0;
-- 
2.55.0

Re: [PATCH] staging: greybus: spilib: Handle SPI device types with a switch statement

[Greg Kroah-Hartman]

On Wed, Aug 05, 2026 at 04:49:57PM +0530, singh.supreet14@gmail.com wrote:
> From: Supreet <singh.supreet14@gmail.com>

We need a full name please.


> 
> Replaced the if/else chain with switch statement. makes it easier to
> extend support for additional device types.
> 
> Remove the temporary spidev variable since it is only used to test the
> return value of spi_new_device(). Call spi_new_device() directly in
> the conditional instead.
> 
> checkpatch was run, had no warnings and errors.
> 
> Signed-off-by: Supreet <singh.supreet14@gmail.com>
> ---
>  drivers/staging/greybus/spilib.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c
> index e4d1ae8308aa..53fc0c124754 100644
> --- a/drivers/staging/greybus/spilib.c
> +++ b/drivers/staging/greybus/spilib.c
> @@ -444,7 +444,6 @@ static int gb_spi_setup_device(struct gb_spilib *spi, u8 cs)
>  	struct gb_spi_device_config_request request;
>  	struct gb_spi_device_config_response response;
>  	struct spi_board_info spi_board = { {0} };
> -	struct spi_device *spidev;
>  	int ret;
>  	u8 dev_type;
>  
> @@ -458,25 +457,29 @@ static int gb_spi_setup_device(struct gb_spilib *spi, u8 cs)
>  
>  	dev_type = response.device_type;
>  
> -	if (dev_type == GB_SPI_SPI_DEV)
> +	switch (dev_type) {
> +	case GB_SPI_SPI_DEV:
>  		strscpy(spi_board.modalias, "spidev",
>  			sizeof(spi_board.modalias));
> -	else if (dev_type == GB_SPI_SPI_NOR)
> +		break;
> +	case GB_SPI_SPI_NOR:
>  		strscpy(spi_board.modalias, "spi-nor",
>  			sizeof(spi_board.modalias));
> -	else if (dev_type == GB_SPI_SPI_MODALIAS)
> +		break;
> +	case GB_SPI_SPI_MODALIAS:
>  		memcpy(spi_board.modalias, response.name,
> -		       sizeof(spi_board.modalias));
> -	else
> +		sizeof(spi_board.modalias));
> +		break;
> +	default:
>  		return -EINVAL;
> +	}
>  
>  	spi_board.mode		= le16_to_cpu(response.mode);
>  	spi_board.bus_num	= ctlr->bus_num;
>  	spi_board.chip_select	= cs;
>  	spi_board.max_speed_hz	= le32_to_cpu(response.max_speed_hz);
>  
> -	spidev = spi_new_device(ctlr, &spi_board);
> -	if (!spidev)
> +	if (!spi_new_device(ctlr, &spi_board))

Why was this change made?  It's a different one than the switch
statement, right?

thanks,

greg k-h

Re: [PATCH] staging: greybus: spilib: Handle SPI device types with a switch statement

[Supreet Singh]

On Wed, Aug 05, 2026 at 01:28:47PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Aug 05, 2026 at 04:49:57PM +0530, singh.supreet14@gmail.com wrote:
> > From: Supreet <singh.supreet14@gmail.com>
> 
> We need a full name please.
> 
> 
> > 
> > Replaced the if/else chain with switch statement. makes it easier to
> > extend support for additional device types.
> > 
> > Remove the temporary spidev variable since it is only used to test the
> > return value of spi_new_device(). Call spi_new_device() directly in
> > the conditional instead.
> > 
> > checkpatch was run, had no warnings and errors.
> > 
> > Signed-off-by: Supreet <singh.supreet14@gmail.com>
> > ---
> >  drivers/staging/greybus/spilib.c | 19 +++++++++++--------
> >  1 file changed, 11 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c
> > index e4d1ae8308aa..53fc0c124754 100644
> > --- a/drivers/staging/greybus/spilib.c
> > +++ b/drivers/staging/greybus/spilib.c
> > @@ -444,7 +444,6 @@ static int gb_spi_setup_device(struct gb_spilib *spi, u8 cs)
> >  	struct gb_spi_device_config_request request;
> >  	struct gb_spi_device_config_response response;
> >  	struct spi_board_info spi_board = { {0} };
> > -	struct spi_device *spidev;
> >  	int ret;
> >  	u8 dev_type;
> >  
> > @@ -458,25 +457,29 @@ static int gb_spi_setup_device(struct gb_spilib *spi, u8 cs)
> >  
> >  	dev_type = response.device_type;
> >  
> > -	if (dev_type == GB_SPI_SPI_DEV)
> > +	switch (dev_type) {
> > +	case GB_SPI_SPI_DEV:
> >  		strscpy(spi_board.modalias, "spidev",
> >  			sizeof(spi_board.modalias));
> > -	else if (dev_type == GB_SPI_SPI_NOR)
> > +		break;
> > +	case GB_SPI_SPI_NOR:
> >  		strscpy(spi_board.modalias, "spi-nor",
> >  			sizeof(spi_board.modalias));
> > -	else if (dev_type == GB_SPI_SPI_MODALIAS)
> > +		break;
> > +	case GB_SPI_SPI_MODALIAS:
> >  		memcpy(spi_board.modalias, response.name,
> > -		       sizeof(spi_board.modalias));
> > -	else
> > +		sizeof(spi_board.modalias));
> > +		break;
> > +	default:
> >  		return -EINVAL;
> > +	}
> >  
> >  	spi_board.mode		= le16_to_cpu(response.mode);
> >  	spi_board.bus_num	= ctlr->bus_num;
> >  	spi_board.chip_select	= cs;
> >  	spi_board.max_speed_hz	= le32_to_cpu(response.max_speed_hz);
> >  
> > -	spidev = spi_new_device(ctlr, &spi_board);
> > -	if (!spidev)
> > +	if (!spi_new_device(ctlr, &spi_board))
> 
> Why was this change made?  It's a different one than the switch
> statement, right?
> 
> thanks,
> 
> greg k-h

Hi Greg

Yes, you are right, the `spidev` change is independent of the switch
statement. My intention was to remove the temporary variable as it was
only used to check the return value of the `spi_new_device()`.

I will drop that change, and send a v2 focused only on the switch
statement. If appropriate I can send that cleanup as a separate patch.

Apologies for the last email, first time using mutt.

Thanks
Supreet Singh

[PATCH v2] staging: greybus: spilib: Handle SPI device types with a switch statement

[singh.supreet14]

From: Supreet Singh <singh.supreet14@gmail.com>

Replace the if/else chain that selects the SPI device type with a switch
statement. This makes the code easier to extend when additional device
types are introduced and improves readability.


Signed-off-by: Supreet Singh <singh.supreet14@gmail.com>
---
Changes in v2:
- Restore the temporary variable spidev
- Use the contributor's full name in the From and Signed-off-by lines.

 drivers/staging/greybus/spilib.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c
index e4d1ae8308aa..d1b7ca9c4ee3 100644
--- a/drivers/staging/greybus/spilib.c
+++ b/drivers/staging/greybus/spilib.c
@@ -458,17 +458,22 @@ static int gb_spi_setup_device(struct gb_spilib *spi, u8 cs)
 
 	dev_type = response.device_type;
 
-	if (dev_type == GB_SPI_SPI_DEV)
+	switch (dev_type) {
+	case GB_SPI_SPI_DEV:
 		strscpy(spi_board.modalias, "spidev",
 			sizeof(spi_board.modalias));
-	else if (dev_type == GB_SPI_SPI_NOR)
+		break;
+	case GB_SPI_SPI_NOR:
 		strscpy(spi_board.modalias, "spi-nor",
 			sizeof(spi_board.modalias));
-	else if (dev_type == GB_SPI_SPI_MODALIAS)
+		break;
+	case GB_SPI_SPI_MODALIAS:
 		memcpy(spi_board.modalias, response.name,
-		       sizeof(spi_board.modalias));
-	else
+		sizeof(spi_board.modalias));
+		break;
+	default:
 		return -EINVAL;
+	}
 
 	spi_board.mode		= le16_to_cpu(response.mode);
 	spi_board.bus_num	= ctlr->bus_num;
-- 
2.55.0

Re: [PATCH v2] staging: greybus: spilib: Handle SPI device types with a switch statement

[Dan Carpenter]

On Thu, Aug 06, 2026 at 03:41:18PM +0530, singh.supreet14@gmail.com wrote:
> From: Supreet Singh <singh.supreet14@gmail.com>
> 
> Replace the if/else chain that selects the SPI device type with a switch
> statement. This makes the code easier to extend when additional device
> types are introduced and improves readability.
> 
> 

Delete the extra blank line.

> Signed-off-by: Supreet Singh <singh.supreet14@gmail.com>
> ---
> Changes in v2:
> - Restore the temporary variable spidev
> - Use the contributor's full name in the From and Signed-off-by lines.
> 
>  drivers/staging/greybus/spilib.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c
> index e4d1ae8308aa..d1b7ca9c4ee3 100644
> --- a/drivers/staging/greybus/spilib.c
> +++ b/drivers/staging/greybus/spilib.c
> @@ -458,17 +458,22 @@ static int gb_spi_setup_device(struct gb_spilib *spi, u8 cs)
>  
>  	dev_type = response.device_type;
>  
> -	if (dev_type == GB_SPI_SPI_DEV)
> +	switch (dev_type) {
> +	case GB_SPI_SPI_DEV:
>  		strscpy(spi_board.modalias, "spidev",
>  			sizeof(spi_board.modalias));
> -	else if (dev_type == GB_SPI_SPI_NOR)
> +		break;
> +	case GB_SPI_SPI_NOR:
>  		strscpy(spi_board.modalias, "spi-nor",
>  			sizeof(spi_board.modalias));
> -	else if (dev_type == GB_SPI_SPI_MODALIAS)
> +		break;
> +	case GB_SPI_SPI_MODALIAS:
>  		memcpy(spi_board.modalias, response.name,
> -		       sizeof(spi_board.modalias));
> -	else
> +		sizeof(spi_board.modalias));

This line is indented badly now.

regards,
dan carpenter

> +		break;
> +	default:
>  		return -EINVAL;
> +	}