| Message ID | 20170523040322.10433-2-chris.packham@alliedtelesis.co.nz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 > > > -- 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
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
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.
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)
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(-)