[PATCH v1 0/3] spidev: A few cleanups

[PATCH v1 0/3] spidev: A few cleanups

[Andy Shevchenko]

A few cleanups to the spidev.c to utilize existing APIs and make it
use less amount of Lines of Code. No functional change intended.

Andy Shevchenko (3):
  spidev: Decrease indentation level in spidev_ioctl() SPI_IOC_RD_MODE*
  spidev: Switch to use spi_get_csgpiod()
  spidev: Simplify SPI_IOC_RD_MODE* cases in spidev_ioctl()

 drivers/spi/spidev.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 1/3] spidev: Decrease indentation level in spidev_ioctl() SPI_IOC_RD_MODE*

[Andy Shevchenko]

Instead of defining a local controller variable inside an indented
block, move the definition to the top of spidev_ioctl() and reuse
it in the SPI_IOC_RD_MODE* and SPI_IOC_WR_MODE* cases.

This drops unneeded indentation and reduces amount of LoCs.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spidev.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index d13dc15cc191..dc516f0ca71f 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -357,6 +357,7 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	int			retval = 0;
 	struct spidev_data	*spidev;
 	struct spi_device	*spi;
+	struct spi_controller	*ctlr;
 	u32			tmp;
 	unsigned		n_ioc;
 	struct spi_ioc_transfer	*ioc;
@@ -376,6 +377,8 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		return -ESHUTDOWN;
 	}
 
+	ctlr = spi->controller;
+
 	/* use the buffer lock here for triple duty:
 	 *  - prevent I/O (from us) so calling spi_setup() is safe;
 	 *  - prevent concurrent SPI_IOC_WR_* from morphing
@@ -390,13 +393,9 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	case SPI_IOC_RD_MODE32:
 		tmp = spi->mode;
 
-		{
-			struct spi_controller *ctlr = spi->controller;
-
-			if (ctlr->use_gpio_descriptors && ctlr->cs_gpiods &&
-			    ctlr->cs_gpiods[spi_get_chipselect(spi, 0)])
-				tmp &= ~SPI_CS_HIGH;
-		}
+		if (ctlr->use_gpio_descriptors && ctlr->cs_gpiods &&
+		    ctlr->cs_gpiods[spi_get_chipselect(spi, 0)])
+			tmp &= ~SPI_CS_HIGH;
 
 		if (cmd == SPI_IOC_RD_MODE)
 			retval = put_user(tmp & SPI_MODE_MASK,
@@ -424,7 +423,6 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		else
 			retval = get_user(tmp, (u32 __user *)arg);
 		if (retval == 0) {
-			struct spi_controller *ctlr = spi->controller;
 			u32	save = spi->mode;
 
 			if (tmp & ~SPI_MODE_MASK) {
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 2/3] spidev: Switch to use spi_get_csgpiod()

[Andy Shevchenko]

spidev_ioctl() checks if there is an SPI chip select is driven by GPIO.
Instead of current code, we can call spi_get_csgpiod().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spidev.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index dc516f0ca71f..e324b42c658c 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -393,8 +393,7 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	case SPI_IOC_RD_MODE32:
 		tmp = spi->mode;
 
-		if (ctlr->use_gpio_descriptors && ctlr->cs_gpiods &&
-		    ctlr->cs_gpiods[spi_get_chipselect(spi, 0)])
+		if (ctlr->use_gpio_descriptors && spi_get_csgpiod(spi, 0))
 			tmp &= ~SPI_CS_HIGH;
 
 		if (cmd == SPI_IOC_RD_MODE)
@@ -430,8 +429,7 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 				break;
 			}
 
-			if (ctlr->use_gpio_descriptors && ctlr->cs_gpiods &&
-			    ctlr->cs_gpiods[spi_get_chipselect(spi, 0)])
+			if (ctlr->use_gpio_descriptors && spi_get_csgpiod(spi, 0))
 				tmp |= SPI_CS_HIGH;
 
 			tmp |= spi->mode & ~SPI_MODE_MASK;
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 3/3] spidev: Simplify SPI_IOC_RD_MODE* cases in spidev_ioctl()

[Andy Shevchenko]

The temporary variable tmp is not used outside of the
SPI_IOC_RD_MODE* cases, hence we can optimize its use.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/spi/spidev.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index e324b42c658c..c5450217528b 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -391,17 +391,15 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	/* read requests */
 	case SPI_IOC_RD_MODE:
 	case SPI_IOC_RD_MODE32:
-		tmp = spi->mode;
+		tmp = spi->mode & SPI_MODE_MASK;
 
 		if (ctlr->use_gpio_descriptors && spi_get_csgpiod(spi, 0))
 			tmp &= ~SPI_CS_HIGH;
 
 		if (cmd == SPI_IOC_RD_MODE)
-			retval = put_user(tmp & SPI_MODE_MASK,
-					  (__u8 __user *)arg);
+			retval = put_user(tmp, (__u8 __user *)arg);
 		else
-			retval = put_user(tmp & SPI_MODE_MASK,
-					  (__u32 __user *)arg);
+			retval = put_user(tmp, (__u32 __user *)arg);
 		break;
 	case SPI_IOC_RD_LSB_FIRST:
 		retval = put_user((spi->mode & SPI_LSB_FIRST) ?  1 : 0,
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v1 1/3] spidev: Decrease indentation level in spidev_ioctl() SPI_IOC_RD_MODE*

[Sverdlin, Alexander]

Hi!

On Thu, 2023-08-24 at 19:22 +0300, Andy Shevchenko wrote:
> Instead of defining a local controller variable inside an indented
> block, move the definition to the top of spidev_ioctl() and reuse
> it in the SPI_IOC_RD_MODE* and SPI_IOC_WR_MODE* cases.
> 
> This drops unneeded indentation and reduces amount of LoCs.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

> ---
>  drivers/spi/spidev.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
> index d13dc15cc191..dc516f0ca71f 100644
> --- a/drivers/spi/spidev.c
> +++ b/drivers/spi/spidev.c
> @@ -357,6 +357,7 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>         int                     retval = 0;
>         struct spidev_data      *spidev;
>         struct spi_device       *spi;
> +       struct spi_controller   *ctlr;
>         u32                     tmp;
>         unsigned                n_ioc;
>         struct spi_ioc_transfer *ioc;
> @@ -376,6 +377,8 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>                 return -ESHUTDOWN;
>         }
>  
> +       ctlr = spi->controller;
> +
>         /* use the buffer lock here for triple duty:
>          *  - prevent I/O (from us) so calling spi_setup() is safe;
>          *  - prevent concurrent SPI_IOC_WR_* from morphing
> @@ -390,13 +393,9 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>         case SPI_IOC_RD_MODE32:
>                 tmp = spi->mode;
>  
> -               {
> -                       struct spi_controller *ctlr = spi->controller;
> -
> -                       if (ctlr->use_gpio_descriptors && ctlr->cs_gpiods &&
> -                           ctlr->cs_gpiods[spi_get_chipselect(spi, 0)])
> -                               tmp &= ~SPI_CS_HIGH;
> -               }
> +               if (ctlr->use_gpio_descriptors && ctlr->cs_gpiods &&
> +                   ctlr->cs_gpiods[spi_get_chipselect(spi, 0)])
> +                       tmp &= ~SPI_CS_HIGH;
>  
>                 if (cmd == SPI_IOC_RD_MODE)
>                         retval = put_user(tmp & SPI_MODE_MASK,
> @@ -424,7 +423,6 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>                 else
>                         retval = get_user(tmp, (u32 __user *)arg);
>                 if (retval == 0) {
> -                       struct spi_controller *ctlr = spi->controller;
>                         u32     save = spi->mode;
>  
>                         if (tmp & ~SPI_MODE_MASK) {

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

Re: [PATCH v1 2/3] spidev: Switch to use spi_get_csgpiod()

[Sverdlin, Alexander]

Hi!

On Thu, 2023-08-24 at 19:22 +0300, Andy Shevchenko wrote:
> spidev_ioctl() checks if there is an SPI chip select is driven by GPIO.
                                                     ^^^ ?

> Instead of current code, we can call spi_get_csgpiod().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

> ---
>  drivers/spi/spidev.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
> index dc516f0ca71f..e324b42c658c 100644
> --- a/drivers/spi/spidev.c
> +++ b/drivers/spi/spidev.c
> @@ -393,8 +393,7 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>         case SPI_IOC_RD_MODE32:
>                 tmp = spi->mode;
>  
> -               if (ctlr->use_gpio_descriptors && ctlr->cs_gpiods &&
> -                   ctlr->cs_gpiods[spi_get_chipselect(spi, 0)])
> +               if (ctlr->use_gpio_descriptors && spi_get_csgpiod(spi, 0))
>                         tmp &= ~SPI_CS_HIGH;
>  
>                 if (cmd == SPI_IOC_RD_MODE)
> @@ -430,8 +429,7 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>                                 break;
>                         }
>  
> -                       if (ctlr->use_gpio_descriptors && ctlr->cs_gpiods &&
> -                           ctlr->cs_gpiods[spi_get_chipselect(spi, 0)])
> +                       if (ctlr->use_gpio_descriptors && spi_get_csgpiod(spi, 0))
>                                 tmp |= SPI_CS_HIGH;
>  
>                         tmp |= spi->mode & ~SPI_MODE_MASK;

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

Re: [PATCH v1 3/3] spidev: Simplify SPI_IOC_RD_MODE* cases in spidev_ioctl()

[Sverdlin, Alexander]

Hi!

On Thu, 2023-08-24 at 19:22 +0300, Andy Shevchenko wrote:
> The temporary variable tmp is not used outside of the
> SPI_IOC_RD_MODE* cases, hence we can optimize its use.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Lukas Wunner <lukas@wunner.de>

Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

> ---
>  drivers/spi/spidev.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
> index e324b42c658c..c5450217528b 100644
> --- a/drivers/spi/spidev.c
> +++ b/drivers/spi/spidev.c
> @@ -391,17 +391,15 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>         /* read requests */
>         case SPI_IOC_RD_MODE:
>         case SPI_IOC_RD_MODE32:
> -               tmp = spi->mode;
> +               tmp = spi->mode & SPI_MODE_MASK;
>  
>                 if (ctlr->use_gpio_descriptors && spi_get_csgpiod(spi, 0))
>                         tmp &= ~SPI_CS_HIGH;
>  
>                 if (cmd == SPI_IOC_RD_MODE)
> -                       retval = put_user(tmp & SPI_MODE_MASK,
> -                                         (__u8 __user *)arg);
> +                       retval = put_user(tmp, (__u8 __user *)arg);
>                 else
> -                       retval = put_user(tmp & SPI_MODE_MASK,
> -                                         (__u32 __user *)arg);
> +                       retval = put_user(tmp, (__u32 __user *)arg);
>                 break;
>         case SPI_IOC_RD_LSB_FIRST:
>                 retval = put_user((spi->mode & SPI_LSB_FIRST) ?  1 : 0,

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

Re: [PATCH v1 0/3] spidev: A few cleanups

[Mark Brown]

On Thu, 24 Aug 2023 19:22:06 +0300, Andy Shevchenko wrote:
> A few cleanups to the spidev.c to utilize existing APIs and make it
> use less amount of Lines of Code. No functional change intended.
> 
> Andy Shevchenko (3):
>   spidev: Decrease indentation level in spidev_ioctl() SPI_IOC_RD_MODE*
>   spidev: Switch to use spi_get_csgpiod()
>   spidev: Simplify SPI_IOC_RD_MODE* cases in spidev_ioctl()
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/3] spidev: Decrease indentation level in spidev_ioctl() SPI_IOC_RD_MODE*
      commit: 12c8d7a76cd6100a2f35b9ef4b87d11128b9105b
[2/3] spidev: Switch to use spi_get_csgpiod()
      commit: 193a7f9e1a78f69c913bb26ca4500f6edad1e8ff
[3/3] spidev: Simplify SPI_IOC_RD_MODE* cases in spidev_ioctl()
      commit: 764246c7feda01f46b1a243cfa15ad5627874ef9

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark