| Message ID | 20190116082110.5604-1-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| 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
On Wed, Apr 03, 2019 at 10:34:58AM +0200, Geert Uytterhoeven 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. > I agree configuring the system like this is a mistake by the integrator, > to be detected during integration testing. Yeah, it's kind of unfortunate that the flags are set at the controller level rather than at the chip select level as especially in this case it's likely that capabilities will diverge.
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.) 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. Yours, Linus Walleij
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
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);