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;
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
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
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;
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;
- }
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 v3: - Fixed the memcpy indentation issue - Removed the extra blank line in the commit message
drivers/staging/greybus/spilib.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c index e4d1ae8308aa..39d5bf234c61 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 + break; + default: return -EINVAL; + }
spi_board.mode = le16_to_cpu(response.mode); spi_board.bus_num = ctlr->bus_num;