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

[Jan Kiszka]

On 2017-07-08 23:48, Andy Shevchenko wrote:
> 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?

-ENOPARSE

> 
>>
>> 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
> 
> 
>

Attachment: signature.asc
Description: OpenPGP digital signature