[PATCH 1/2] spi: orion: Handle GPIO chip-selects

[PATCH 1/2] spi: orion: Handle GPIO chip-selects

[Chris Packham]

Some hardware designs use GPIOs to add (or supplement) the SPI
chip-select so that more than one SPI slave device can be used.

For this to work with the spi-orion driver the SPI_MASTER_GPIO_SS flag
needs to be set (because the other outputs are gated internally by the
CS) and the correct chip-select (in this case CS0) needs to be driven by
the controller.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 drivers/spi/spi-orion.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-orion.c b/drivers/spi/spi-orion.c
index be2e87ee8b31..28fc9f161b9d 100644
--- a/drivers/spi/spi-orion.c
+++ b/drivers/spi/spi-orion.c
@@ -22,6 +22,7 @@
 #include <linux/of_device.h>
 #include <linux/clk.h>
 #include <linux/sizes.h>
+#include <linux/gpio.h>
 #include <asm/unaligned.h>
 
 #define DRIVER_NAME			"orion_spi"
@@ -320,12 +321,18 @@ orion_spi_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
 static void orion_spi_set_cs(struct spi_device *spi, bool enable)
 {
 	struct orion_spi *orion_spi;
+	int cs;
+
+	if (gpio_is_valid(spi->cs_gpio))
+		cs = 0;
+	else
+		cs = spi->chip_select;
 
 	orion_spi = spi_master_get_devdata(spi->master);
 
 	orion_spi_clrbits(orion_spi, ORION_SPI_IF_CTRL_REG, ORION_SPI_CS_MASK);
 	orion_spi_setbits(orion_spi, ORION_SPI_IF_CTRL_REG,
-				ORION_SPI_CS(spi->chip_select));
+				ORION_SPI_CS(cs));
 
 	/* Chip select logic is inverted from spi_set_cs */
 	if (!enable)
@@ -606,6 +613,7 @@ static int orion_spi_probe(struct platform_device *pdev)
 	master->setup = orion_spi_setup;
 	master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(16);
 	master->auto_runtime_pm = true;
+	master->flags = SPI_MASTER_GPIO_SS;
 
 	platform_set_drvdata(pdev, master);
 
-- 
2.13.0

[PATCH 1/2] spi: orion: Handle GPIO chip-selects

[Chris Packham]

Some hardware designs use GPIOs to add (or supplement) the SPI
chip-select so that more than one SPI slave device can be used.

For this to work with the spi-orion driver the SPI_MASTER_GPIO_SS flag
needs to be set (because the other outputs are gated internally by the
CS) and the correct chip-select (in this case CS0) needs to be driven by
the controller.

Signed-off-by: Chris Packham <chris.packham-6g8wRflRTwXFdCa3tKVlE6U/zSkkHjvu@public.gmane.org>
---
 drivers/spi/spi-orion.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-orion.c b/drivers/spi/spi-orion.c
index be2e87ee8b31..28fc9f161b9d 100644
--- a/drivers/spi/spi-orion.c
+++ b/drivers/spi/spi-orion.c
@@ -22,6 +22,7 @@
 #include <linux/of_device.h>
 #include <linux/clk.h>
 #include <linux/sizes.h>
+#include <linux/gpio.h>
 #include <asm/unaligned.h>
 
 #define DRIVER_NAME			"orion_spi"
@@ -320,12 +321,18 @@ orion_spi_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
 static void orion_spi_set_cs(struct spi_device *spi, bool enable)
 {
 	struct orion_spi *orion_spi;
+	int cs;
+
+	if (gpio_is_valid(spi->cs_gpio))
+		cs = 0;
+	else
+		cs = spi->chip_select;
 
 	orion_spi = spi_master_get_devdata(spi->master);
 
 	orion_spi_clrbits(orion_spi, ORION_SPI_IF_CTRL_REG, ORION_SPI_CS_MASK);
 	orion_spi_setbits(orion_spi, ORION_SPI_IF_CTRL_REG,
-				ORION_SPI_CS(spi->chip_select));
+				ORION_SPI_CS(cs));
 
 	/* Chip select logic is inverted from spi_set_cs */
 	if (!enable)
@@ -606,6 +613,7 @@ static int orion_spi_probe(struct platform_device *pdev)
 	master->setup = orion_spi_setup;
 	master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(16);
 	master->auto_runtime_pm = true;
+	master->flags = SPI_MASTER_GPIO_SS;
 
 	platform_set_drvdata(pdev, master);
 
-- 
2.13.0

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] spi: use gpio_desc instead of numeric gpio

[Chris Packham]

By using a gpio_desc and gpiod_set_value() instead of a numeric gpio and
gpio_set_value() the gpio flags are taken into account. This is useful
when using a gpio chip-select to supplement a controllers native
chip-select.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    My specific use-case is I have a board that uses the spi-orion driver but
    only has one CS pin available. In order to access two spi slave devices the
    board has a 1-of-2 decoder/demultiplexer which is driven via a gpio.
    
    The problem is that for one of the 2 slave devices the gpio level required
    is opposite to the chip-select so I can't simply specify "spi-cs-high".
    With this change I can flag the gpio as active low and the gpio subsystem
    takes care of the additional inversion required.

 drivers/spi/spi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 6f87fec409b5..b39c0f9956dd 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -725,7 +725,10 @@ static void spi_set_cs(struct spi_device *spi, bool enable)
 		enable = !enable;
 
 	if (gpio_is_valid(spi->cs_gpio)) {
-		gpio_set_value(spi->cs_gpio, !enable);
+		struct gpio_desc *gpio = gpio_to_desc(spi->cs_gpio);
+
+		if (gpio)
+			gpiod_set_value(gpio, !enable);
 		/* Some SPI masters need both GPIO CS & slave_select */
 		if ((spi->master->flags & SPI_MASTER_GPIO_SS) &&
 		    spi->master->set_cs)
-- 
2.13.0

Re: [PATCH 2/2] spi: use gpio_desc instead of numeric gpio

[Andy Shevchenko]

On Tue, May 23, 2017 at 7:03 AM, Chris Packham
<chris.packham@alliedtelesis.co.nz> wrote:
> By using a gpio_desc and gpiod_set_value() instead of a numeric gpio and
> gpio_set_value() the gpio flags are taken into account. This is useful
> when using a gpio chip-select to supplement a controllers native
> chip-select.

I think would be better to move everything in SPI core to GPIO descriptors.

>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>
> Notes:
>     My specific use-case is I have a board that uses the spi-orion driver but
>     only has one CS pin available. In order to access two spi slave devices the
>     board has a 1-of-2 decoder/demultiplexer which is driven via a gpio.
>
>     The problem is that for one of the 2 slave devices the gpio level required
>     is opposite to the chip-select so I can't simply specify "spi-cs-high".
>     With this change I can flag the gpio as active low and the gpio subsystem
>     takes care of the additional inversion required.
>
>  drivers/spi/spi.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 6f87fec409b5..b39c0f9956dd 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -725,7 +725,10 @@ static void spi_set_cs(struct spi_device *spi, bool enable)
>                 enable = !enable;
>
>         if (gpio_is_valid(spi->cs_gpio)) {
> -               gpio_set_value(spi->cs_gpio, !enable);
> +               struct gpio_desc *gpio = gpio_to_desc(spi->cs_gpio);
> +
> +               if (gpio)
> +                       gpiod_set_value(gpio, !enable);
>                 /* Some SPI masters need both GPIO CS & slave_select */
>                 if ((spi->master->flags & SPI_MASTER_GPIO_SS) &&
>                     spi->master->set_cs)
> --
> 2.13.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] spi: use gpio_desc instead of numeric gpio

[Chris Packham]

Hi Andy,

On 24/05/17 06:28, Andy Shevchenko wrote:
> On Tue, May 23, 2017 at 7:03 AM, Chris Packham
> <chris.packham@alliedtelesis.co.nz> wrote:
>> By using a gpio_desc and gpiod_set_value() instead of a numeric gpio and
>> gpio_set_value() the gpio flags are taken into account. This is useful
>> when using a gpio chip-select to supplement a controllers native
>> chip-select.
>
> I think would be better to move everything in SPI core to GPIO descriptors.

I did consider it but it's a big change and I don't have access a lot of 
gear to test on (maybe 2 or 3 SoCs with a SPI host controller and the 
same SPI-NOR chips).

I can give it a try. Perhaps converting the spi core structures over and 
leaving the slaves using numeric gpios. Then later converting the slaves.

>
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>
>> Notes:
>>      My specific use-case is I have a board that uses the spi-orion driver but
>>      only has one CS pin available. In order to access two spi slave devices the
>>      board has a 1-of-2 decoder/demultiplexer which is driven via a gpio.
>>
>>      The problem is that for one of the 2 slave devices the gpio level required
>>      is opposite to the chip-select so I can't simply specify "spi-cs-high".
>>      With this change I can flag the gpio as active low and the gpio subsystem
>>      takes care of the additional inversion required.
>>
>>   drivers/spi/spi.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>> index 6f87fec409b5..b39c0f9956dd 100644
>> --- a/drivers/spi/spi.c
>> +++ b/drivers/spi/spi.c
>> @@ -725,7 +725,10 @@ static void spi_set_cs(struct spi_device *spi, bool enable)
>>                  enable = !enable;
>>
>>          if (gpio_is_valid(spi->cs_gpio)) {
>> -               gpio_set_value(spi->cs_gpio, !enable);
>> +               struct gpio_desc *gpio = gpio_to_desc(spi->cs_gpio);
>> +
>> +               if (gpio)
>> +                       gpiod_set_value(gpio, !enable);
>>                  /* Some SPI masters need both GPIO CS & slave_select */
>>                  if ((spi->master->flags & SPI_MASTER_GPIO_SS) &&
>>                      spi->master->set_cs)
>> --
>> 2.13.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>

Re: [PATCH 2/2] spi: use gpio_desc instead of numeric gpio

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2074 bytes --]

Hi Chris,

[auto build test ERROR on spi/for-next]
[also build test ERROR on v4.12-rc2 next-20170523]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chris-Packham/spi-orion-Handle-GPIO-chip-selects/20170524-074032
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: sparc-defconfig (attached as .config)
compiler: sparc-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc 

All error/warnings (new ones prefixed by >>):

   drivers/spi/spi.c: In function 'spi_set_cs':
>> drivers/spi/spi.c:728:28: error: implicit declaration of function 'gpio_to_desc' [-Werror=implicit-function-declaration]
      struct gpio_desc *gpio = gpio_to_desc(spi->cs_gpio);
                               ^~~~~~~~~~~~
>> drivers/spi/spi.c:728:28: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
>> drivers/spi/spi.c:731:4: error: implicit declaration of function 'gpiod_set_value' [-Werror=implicit-function-declaration]
       gpiod_set_value(gpio, !enable);
       ^~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/gpio_to_desc +728 drivers/spi/spi.c

   722	static void spi_set_cs(struct spi_device *spi, bool enable)
   723	{
   724		if (spi->mode & SPI_CS_HIGH)
   725			enable = !enable;
   726	
   727		if (gpio_is_valid(spi->cs_gpio)) {
 > 728			struct gpio_desc *gpio = gpio_to_desc(spi->cs_gpio);
   729	
   730			if (gpio)
 > 731				gpiod_set_value(gpio, !enable);
   732			/* Some SPI masters need both GPIO CS & slave_select */
   733			if ((spi->master->flags & SPI_MASTER_GPIO_SS) &&
   734			    spi->master->set_cs)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 12031 bytes --]

Re: [PATCH 2/2] spi: use gpio_desc instead of numeric gpio

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2074 bytes --]

Hi Chris,

[auto build test ERROR on spi/for-next]
[also build test ERROR on v4.12-rc2 next-20170523]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chris-Packham/spi-orion-Handle-GPIO-chip-selects/20170524-074032
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: sparc-defconfig (attached as .config)
compiler: sparc-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc 

All error/warnings (new ones prefixed by >>):

   drivers/spi/spi.c: In function 'spi_set_cs':
>> drivers/spi/spi.c:728:28: error: implicit declaration of function 'gpio_to_desc' [-Werror=implicit-function-declaration]
      struct gpio_desc *gpio = gpio_to_desc(spi->cs_gpio);
                               ^~~~~~~~~~~~
>> drivers/spi/spi.c:728:28: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
>> drivers/spi/spi.c:731:4: error: implicit declaration of function 'gpiod_set_value' [-Werror=implicit-function-declaration]
       gpiod_set_value(gpio, !enable);
       ^~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/gpio_to_desc +728 drivers/spi/spi.c

   722	static void spi_set_cs(struct spi_device *spi, bool enable)
   723	{
   724		if (spi->mode & SPI_CS_HIGH)
   725			enable = !enable;
   726	
   727		if (gpio_is_valid(spi->cs_gpio)) {
 > 728			struct gpio_desc *gpio = gpio_to_desc(spi->cs_gpio);
   729	
   730			if (gpio)
 > 731				gpiod_set_value(gpio, !enable);
   732			/* Some SPI masters need both GPIO CS & slave_select */
   733			if ((spi->master->flags & SPI_MASTER_GPIO_SS) &&
   734			    spi->master->set_cs)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 12031 bytes --]

Re: [PATCH 2/2] spi: use gpio_desc instead of numeric gpio

[Chris Packham]

On 24/05/17 08:43, Chris Packham wrote:
> Hi Andy,
> 
> On 24/05/17 06:28, Andy Shevchenko wrote:
>> On Tue, May 23, 2017 at 7:03 AM, Chris Packham
>> <chris.packham@alliedtelesis.co.nz> wrote:
>>> By using a gpio_desc and gpiod_set_value() instead of a numeric gpio and
>>> gpio_set_value() the gpio flags are taken into account. This is useful
>>> when using a gpio chip-select to supplement a controllers native
>>> chip-select.
>>
>> I think would be better to move everything in SPI core to GPIO descriptors.
> 
> I did consider it but it's a big change and I don't have access a lot of
> gear to test on (maybe 2 or 3 SoCs with a SPI host controller and the
> same SPI-NOR chips).
> 
> I can give it a try. Perhaps converting the spi core structures over and
> leaving the slaves using numeric gpios. Then later converting the slaves.
> 

OK so I've had a look at this. The changes to change struct spi_master 
to use gpio_desc in drivers/spi/spi-gpio.c are pretty straight forward 
(in-line patch below, apologies for MUA wrapping).

There are a few SPI host drivers that deal with cs_gpios directly. 
spi-mt65xx.c and spi-davinci.c would be trivial to update. spi-ep93xx.c 
and spi-imx.c are harder because they handle non-dt platforms.

Changing struct spi_device to use gpio_desc would be wider reaching but 
probably automatable s/gpio_set_value/gpiod_set_value/.

I'm a little too nervous about messing up spi-ep93xx.c and spi-ep93xx.c 
to push ahead with changing all of SPI core. I'd be happy to help out 
someone with access to platforms using those.

-- 8< --
index b39c0f9956dd..80a96d3daa45 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -40,6 +40,7 @@
  #include <linux/ioport.h>
  #include <linux/acpi.h>
  #include <linux/highmem.h>
+#include <linux/gpio/consumer.h>

  #define CREATE_TRACE_POINTS
  #include <trace/events/spi.h>
@@ -531,8 +532,8 @@ int spi_add_device(struct spi_device *spi)
                 goto done;
         }

-       if (master->cs_gpios)
-               spi->cs_gpio = master->cs_gpios[spi->chip_select];
+       if (master->cs_gpios[spi->chip_select])
+               spi->cs_gpio = 
desc_to_gpio(master->cs_gpios[spi->chip_select]);

         /* Drivers may modify this initial i/o setup, but will
          * normally rely on the device being setup.  Devices
@@ -1878,7 +1879,8 @@ EXPORT_SYMBOL_GPL(spi_alloc_master);
  #ifdef CONFIG_OF
  static int of_spi_register_master(struct spi_master *master)
  {
-       int nb, i, *cs;
+       int nb, i;
+       struct gpio_desc **cs;
         struct device_node *np = master->dev.of_node;

         if (!np)
@@ -1894,7 +1896,7 @@ static int of_spi_register_master(struct 
spi_master *master)
                 return nb;

         cs = devm_kzalloc(&master->dev,
-                         sizeof(int) * master->num_chipselect,
+                         sizeof(*master->cs_gpios) * 
master->num_chipselect,
                           GFP_KERNEL);
         master->cs_gpios = cs;

@@ -1902,10 +1904,16 @@ static int of_spi_register_master(struct 
spi_master *master)
                 return -ENOMEM;

         for (i = 0; i < master->num_chipselect; i++)
-               cs[i] = -ENOENT;
+               cs[i] = NULL;

-       for (i = 0; i < nb; i++)
-               cs[i] = of_get_named_gpio(np, "cs-gpios", i);
+       for (i = 0; i < nb; i++) {
+               struct gpio_desc *gpio;
+
+               gpio = devm_gpiod_get_index(&master->dev, "cs", i,
+                                           GPIOD_OUT_HIGH);
+               if (!IS_ERR(gpio))
+                       cs[i] = gpio;
+       }

         return 0;
  }
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 935bd2854ff1..46960ca62d5b 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -556,7 +556,7 @@ struct spi_master {
                            struct spi_message *message);

         /* gpio chip select */
-       int                     *cs_gpios;
+       struct gpio_desc        **cs_gpios;

         /* statistics */
         struct spi_statistics   statistics;
-- >8 --

>>
>>>
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>> ---
>>>
>>> Notes:
>>>       My specific use-case is I have a board that uses the spi-orion driver but
>>>       only has one CS pin available. In order to access two spi slave devices the
>>>       board has a 1-of-2 decoder/demultiplexer which is driven via a gpio.
>>>
>>>       The problem is that for one of the 2 slave devices the gpio level required
>>>       is opposite to the chip-select so I can't simply specify "spi-cs-high".
>>>       With this change I can flag the gpio as active low and the gpio subsystem
>>>       takes care of the additional inversion required.
>>>
>>>    drivers/spi/spi.c | 5 ++++-
>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>>> index 6f87fec409b5..b39c0f9956dd 100644
>>> --- a/drivers/spi/spi.c
>>> +++ b/drivers/spi/spi.c
>>> @@ -725,7 +725,10 @@ static void spi_set_cs(struct spi_device *spi, bool enable)
>>>                   enable = !enable;
>>>
>>>           if (gpio_is_valid(spi->cs_gpio)) {
>>> -               gpio_set_value(spi->cs_gpio, !enable);
>>> +               struct gpio_desc *gpio = gpio_to_desc(spi->cs_gpio);
>>> +
>>> +               if (gpio)
>>> +                       gpiod_set_value(gpio, !enable);
>>>                   /* Some SPI masters need both GPIO CS & slave_select */
>>>                   if ((spi->master->flags & SPI_MASTER_GPIO_SS) &&
>>>                       spi->master->set_cs)
>>> --
>>> 2.13.0
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
> 
> 
> 

Re: [PATCH 2/2] spi: use gpio_desc instead of numeric gpio

[Chris Packham]

On 24/05/17 08:43, Chris Packham wrote:
> Hi Andy,
> 
> On 24/05/17 06:28, Andy Shevchenko wrote:
>> On Tue, May 23, 2017 at 7:03 AM, Chris Packham
>> <chris.packham-6g8wRflRTwXFdCa3tKVlE6U/zSkkHjvu@public.gmane.org> wrote:
>>> By using a gpio_desc and gpiod_set_value() instead of a numeric gpio and
>>> gpio_set_value() the gpio flags are taken into account. This is useful
>>> when using a gpio chip-select to supplement a controllers native
>>> chip-select.
>>
>> I think would be better to move everything in SPI core to GPIO descriptors.
> 
> I did consider it but it's a big change and I don't have access a lot of
> gear to test on (maybe 2 or 3 SoCs with a SPI host controller and the
> same SPI-NOR chips).
> 
> I can give it a try. Perhaps converting the spi core structures over and
> leaving the slaves using numeric gpios. Then later converting the slaves.
> 

OK so I've had a look at this. The changes to change struct spi_master 
to use gpio_desc in drivers/spi/spi-gpio.c are pretty straight forward 
(in-line patch below, apologies for MUA wrapping).

There are a few SPI host drivers that deal with cs_gpios directly. 
spi-mt65xx.c and spi-davinci.c would be trivial to update. spi-ep93xx.c 
and spi-imx.c are harder because they handle non-dt platforms.

Changing struct spi_device to use gpio_desc would be wider reaching but 
probably automatable s/gpio_set_value/gpiod_set_value/.

I'm a little too nervous about messing up spi-ep93xx.c and spi-ep93xx.c 
to push ahead with changing all of SPI core. I'd be happy to help out 
someone with access to platforms using those.

-- 8< --
index b39c0f9956dd..80a96d3daa45 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -40,6 +40,7 @@
  #include <linux/ioport.h>
  #include <linux/acpi.h>
  #include <linux/highmem.h>
+#include <linux/gpio/consumer.h>

  #define CREATE_TRACE_POINTS
  #include <trace/events/spi.h>
@@ -531,8 +532,8 @@ int spi_add_device(struct spi_device *spi)
                 goto done;
         }

-       if (master->cs_gpios)
-               spi->cs_gpio = master->cs_gpios[spi->chip_select];
+       if (master->cs_gpios[spi->chip_select])
+               spi->cs_gpio = 
desc_to_gpio(master->cs_gpios[spi->chip_select]);

         /* Drivers may modify this initial i/o setup, but will
          * normally rely on the device being setup.  Devices
@@ -1878,7 +1879,8 @@ EXPORT_SYMBOL_GPL(spi_alloc_master);
  #ifdef CONFIG_OF
  static int of_spi_register_master(struct spi_master *master)
  {
-       int nb, i, *cs;
+       int nb, i;
+       struct gpio_desc **cs;
         struct device_node *np = master->dev.of_node;

         if (!np)
@@ -1894,7 +1896,7 @@ static int of_spi_register_master(struct 
spi_master *master)
                 return nb;

         cs = devm_kzalloc(&master->dev,
-                         sizeof(int) * master->num_chipselect,
+                         sizeof(*master->cs_gpios) * 
master->num_chipselect,
                           GFP_KERNEL);
         master->cs_gpios = cs;

@@ -1902,10 +1904,16 @@ static int of_spi_register_master(struct 
spi_master *master)
                 return -ENOMEM;

         for (i = 0; i < master->num_chipselect; i++)
-               cs[i] = -ENOENT;
+               cs[i] = NULL;

-       for (i = 0; i < nb; i++)
-               cs[i] = of_get_named_gpio(np, "cs-gpios", i);
+       for (i = 0; i < nb; i++) {
+               struct gpio_desc *gpio;
+
+               gpio = devm_gpiod_get_index(&master->dev, "cs", i,
+                                           GPIOD_OUT_HIGH);
+               if (!IS_ERR(gpio))
+                       cs[i] = gpio;
+       }

         return 0;
  }
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 935bd2854ff1..46960ca62d5b 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -556,7 +556,7 @@ struct spi_master {
                            struct spi_message *message);

         /* gpio chip select */
-       int                     *cs_gpios;
+       struct gpio_desc        **cs_gpios;

         /* statistics */
         struct spi_statistics   statistics;
-- >8 --

>>
>>>
>>> Signed-off-by: Chris Packham <chris.packham-6g8wRflRTwXFdCa3tKVlE6U/zSkkHjvu@public.gmane.org>
>>> ---
>>>
>>> Notes:
>>>       My specific use-case is I have a board that uses the spi-orion driver but
>>>       only has one CS pin available. In order to access two spi slave devices the
>>>       board has a 1-of-2 decoder/demultiplexer which is driven via a gpio.
>>>
>>>       The problem is that for one of the 2 slave devices the gpio level required
>>>       is opposite to the chip-select so I can't simply specify "spi-cs-high".
>>>       With this change I can flag the gpio as active low and the gpio subsystem
>>>       takes care of the additional inversion required.
>>>
>>>    drivers/spi/spi.c | 5 ++++-
>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>>> index 6f87fec409b5..b39c0f9956dd 100644
>>> --- a/drivers/spi/spi.c
>>> +++ b/drivers/spi/spi.c
>>> @@ -725,7 +725,10 @@ static void spi_set_cs(struct spi_device *spi, bool enable)
>>>                   enable = !enable;
>>>
>>>           if (gpio_is_valid(spi->cs_gpio)) {
>>> -               gpio_set_value(spi->cs_gpio, !enable);
>>> +               struct gpio_desc *gpio = gpio_to_desc(spi->cs_gpio);
>>> +
>>> +               if (gpio)
>>> +                       gpiod_set_value(gpio, !enable);
>>>                   /* Some SPI masters need both GPIO CS & slave_select */
>>>                   if ((spi->master->flags & SPI_MASTER_GPIO_SS) &&
>>>                       spi->master->set_cs)
>>> --
>>> 2.13.0
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
>>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] spi: use gpio_desc instead of numeric gpio

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 534 bytes --]

On Tue, May 23, 2017 at 09:28:22PM +0300, Andy Shevchenko wrote:
> On Tue, May 23, 2017 at 7:03 AM, Chris Packham

> > By using a gpio_desc and gpiod_set_value() instead of a numeric gpio and
> > gpio_set_value() the gpio flags are taken into account. This is useful
> > when using a gpio chip-select to supplement a controllers native
> > chip-select.

> I think would be better to move everything in SPI core to GPIO descriptors.

Yes, this sort of bodge is just far too ugly and I'd not trust it to
continue to work.  

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] spi: use gpio_desc instead of numeric gpio

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 534 bytes --]

On Tue, May 23, 2017 at 09:28:22PM +0300, Andy Shevchenko wrote:
> On Tue, May 23, 2017 at 7:03 AM, Chris Packham

> > By using a gpio_desc and gpiod_set_value() instead of a numeric gpio and
> > gpio_set_value() the gpio flags are taken into account. This is useful
> > when using a gpio chip-select to supplement a controllers native
> > chip-select.

> I think would be better to move everything in SPI core to GPIO descriptors.

Yes, this sort of bodge is just far too ugly and I'd not trust it to
continue to work.  

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] spi: use gpio_desc instead of numeric gpio

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 363 bytes --]

On Wed, May 24, 2017 at 05:42:38AM +0000, Chris Packham wrote:

> I'm a little too nervous about messing up spi-ep93xx.c and spi-ep93xx.c 
> to push ahead with changing all of SPI core. I'd be happy to help out 
> someone with access to platforms using those.

Well, if you try writing the patch blind we can hopefully find someone
to test it separately?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] spi: use gpio_desc instead of numeric gpio

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 363 bytes --]

On Wed, May 24, 2017 at 05:42:38AM +0000, Chris Packham wrote:

> I'm a little too nervous about messing up spi-ep93xx.c and spi-ep93xx.c 
> to push ahead with changing all of SPI core. I'd be happy to help out 
> someone with access to platforms using those.

Well, if you try writing the patch blind we can hopefully find someone
to test it separately?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]