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

From: Andy Shevchenko
Date: Sat Jul 08 2017 - 17:48:22 EST


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