Re: [PATCH] spi: pxa2xx: Only claim CS GPIOs when the slave device is created

[Andy Shevchenko]

On Sat, Jul 8, 2017 at 11:41 AM, Jan Kiszka <jan.kiszka@xxxxxx> wrote:
> From: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
>
> Avoid hogging chip select GPIOs just because they are listed for the
> master. They might be mulitplexed and, if no slave device is attached,
> used for different purposes. Moreover, this strategy avoids having to
> allocate a cs_gpiods structure.
>
> Tested on the IOT2000 where the second SPI bus is connected to an
> Arduino-compatible connector and multiplexed between SPI, GPIO and PWM
> usage.

Can we first switch the driver to use GPIO descriptors instead of
plain integers?

>
> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
> ---
>  drivers/spi/spi-pxa2xx.c | 59 +++++++++++++++++-------------------------------
>  1 file changed, 21 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
> index 38d053682892..be991266a6ce 100644
> --- a/drivers/spi/spi-pxa2xx.c
> +++ b/drivers/spi/spi-pxa2xx.c
> @@ -1213,21 +1213,33 @@ static int setup_cs(struct spi_device *spi, struct chip_data *chip,
>                     struct pxa2xx_spi_chip *chip_info)
>  {
>         struct driver_data *drv_data = spi_master_get_devdata(spi->master);
> +       struct device *pdev = &drv_data->pdev->dev;
> +       struct gpio_desc *gpiod;
>         int err = 0;
> +       int count;
>
>         if (chip == NULL)
>                 return 0;
>
> -       if (drv_data->cs_gpiods) {
> -               struct gpio_desc *gpiod;
> +       count = gpiod_count(pdev, "cs");
> +       if (count > 0) {
> +               if (spi->chip_select >= count)
> +                       return -EINVAL;
> +
> +               gpiod = gpiod_get_index(pdev, "cs", spi->chip_select,
> +                                       GPIOD_OUT_HIGH);
> +               if (IS_ERR(gpiod)) {
> +                       /* Means use native chip select */
> +                       if (PTR_ERR(gpiod) == -ENOENT)
> +                               return 0;
>
> -               gpiod = drv_data->cs_gpiods[spi->chip_select];
> -               if (gpiod) {
> -                       chip->gpio_cs = desc_to_gpio(gpiod);
> -                       chip->gpio_cs_inverted = spi->mode & SPI_CS_HIGH;
> -                       gpiod_set_value(gpiod, chip->gpio_cs_inverted);
> +                       return PTR_ERR(gpiod);
>                 }
>
> +               chip->gpio_cs = desc_to_gpio(gpiod);
> +               chip->gpio_cs_inverted = spi->mode & SPI_CS_HIGH;
> +               gpiod_set_value(gpiod, chip->gpio_cs_inverted);
> +
>                 return 0;
>         }
>
> @@ -1415,8 +1427,7 @@ static void cleanup(struct spi_device *spi)
>         if (!chip)
>                 return;
>
> -       if (drv_data->ssp_type != CE4100_SSP && !drv_data->cs_gpiods &&
> -           gpio_is_valid(chip->gpio_cs))
> +       if (drv_data->ssp_type != CE4100_SSP && gpio_is_valid(chip->gpio_cs))
>                 gpio_free(chip->gpio_cs);
>
>         kfree(chip);
> @@ -1752,38 +1763,10 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
>         master->num_chipselect = platform_info->num_chipselect;
>
>         count = gpiod_count(&pdev->dev, "cs");
> -       if (count > 0) {
> -               int i;
> -
> +       if (count > 0)
>                 master->num_chipselect = max_t(int, count,
>                         master->num_chipselect);
>
> -               drv_data->cs_gpiods = devm_kcalloc(&pdev->dev,
> -                       master->num_chipselect, sizeof(struct gpio_desc *),
> -                       GFP_KERNEL);
> -               if (!drv_data->cs_gpiods) {
> -                       status = -ENOMEM;
> -                       goto out_error_clock_enabled;
> -               }
> -
> -               for (i = 0; i < master->num_chipselect; i++) {
> -                       struct gpio_desc *gpiod;
> -
> -                       gpiod = devm_gpiod_get_index(dev, "cs", i,
> -                                                    GPIOD_OUT_HIGH);
> -                       if (IS_ERR(gpiod)) {
> -                               /* Means use native chip select */
> -                               if (PTR_ERR(gpiod) == -ENOENT)
> -                                       continue;
> -
> -                               status = (int)PTR_ERR(gpiod);
> -                               goto out_error_clock_enabled;
> -                       } else {
> -                               drv_data->cs_gpiods[i] = gpiod;
> -                       }
> -               }
> -       }
> -
>         tasklet_init(&drv_data->pump_transfers, pump_transfers,
>                      (unsigned long)drv_data);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
With Best Regards,
Andy Shevchenko