| Message ID | 20190116082110.5604-1-linus.walleij@linaro.org |
|---|---|
| State | Accepted |
| Commit | 2df201e0067d84db5955d07cc0d7ccc3b7295aef |
| Headers | show |
| Series | [1/4,v3] spi: Support high CS when using descriptors | expand |
Hi Linus, On Wed, Jan 16, 2019 at 7:27 PM Linus Walleij <linus.walleij@linaro.org> wrote: > All controllers using GPIO descriptors can by definition > support high CS connections, so just enforce this when > registering an SPI controller. But that is guaranteed to be true only for chip selects handled by a GPIO, right? Native chip selects may still not support SPI_CS_HIGH, depending on the controller. Before, the bad_bits check in spi_setup() would detect this, and return an error. After, this will fail silently. I agree configuring the system like this is a mistake by the integrator, to be detected during integration testing. > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -2336,6 +2336,11 @@ int spi_register_controller(struct spi_controller *ctlr) > status = spi_get_gpio_descs(ctlr); > if (status) > return status; > + /* > + * A controller using GPIO descriptors always > + * supports SPI_CS_HIGH if need be. > + */ > + ctlr->mode_bits |= SPI_CS_HIGH; > } else { > /* Legacy code path for GPIOs from DT */ > status = of_spi_register_master(ctlr); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Linus, On Wed, Apr 3, 2019 at 6:24 PM Linus Walleij <linus.walleij@linaro.org> wrote: > On Wed, Apr 3, 2019 at 3:35 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Wed, Jan 16, 2019 at 7:27 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > > All controllers using GPIO descriptors can by definition > > > support high CS connections, so just enforce this when > > > registering an SPI controller. > > > > But that is guaranteed to be true only for chip selects handled by a GPIO, > > right? > > Native chip selects may still not support SPI_CS_HIGH, depending > > on the controller. > > Before, the bad_bits check in spi_setup() would detect this, and return > > an error. After, this will fail silently. > > Yes but only for systems that use descriptors all the way. So dealing > with this is part of the process of converting to using descriptors. > (Thus the other patches in this series.) Well, if the it worked before (no error), it should work after the conversion. The error is handy for new (future) users. > Do we have some hardware that supports only active low native > CS but also want to use GPIOs? Because then maybe I should > take a stab at that in particular. If I keep converting drivers to this > then I will hit my head into it sooner or later I suppose. There may be hardware like that. The integrator should take care of it[*]. [*] Until we manage to describe functional relations in DT instead of explicit GPIO/native <whatever>/IRQn relations. After that - drivers can switch from native to GPIO chip select automatically, when some native feature is missing, - the system can choose to use spi-gpio or i2c-gpio if no driver is available for the hardware SPI or I2C controller on the same pins, - the system can pinmux between a GPIO with interrupt functionality or a real interrupt controller pin, depending on availability. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 06b9139664a3..31696f2fc8d5 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -2336,6 +2336,11 @@ int spi_register_controller(struct spi_controller *ctlr) status = spi_get_gpio_descs(ctlr); if (status) return status; + /* + * A controller using GPIO descriptors always + * supports SPI_CS_HIGH if need be. + */ + ctlr->mode_bits |= SPI_CS_HIGH; } else { /* Legacy code path for GPIOs from DT */ status = of_spi_register_master(ctlr);