| Message ID | 20170724095327.18602-1-mnhu@prevas.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
+Cc: Mika, Jarkko On Mon, Jul 24, 2017 at 12:53 PM, Martin Hundebøll <mnhu@prevas.dk> wrote: > Some gpio controllers requires lines to be configured as outputs to make > subsequent calls to `gpio_set_value()` have any effect. > > Add a call to `gpio_request_one()` for configured gpio chip-selects to > make it work on (at least) mvebu gpio controllers. > More and more hacks around plain GPIO integers... :-( Perhaps we need to discuss how to convert SPI core to use GPIO descriptors everywhere. > Signed-off-by: Martin Hundebøll <mnhu@prevas.dk> > --- > drivers/spi/spi.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index 0725c78b0de6..b7bfc52525aa 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -2010,8 +2010,13 @@ static int of_spi_register_master(struct spi_controller *ctlr) > for (i = 0; i < ctlr->num_chipselect; i++) > cs[i] = -ENOENT; > > - for (i = 0; i < nb; i++) > + for (i = 0; i < nb; i++) { > cs[i] = of_get_named_gpio(np, "cs-gpios", i); > + if (gpio_is_valid(cs[i]) && > + gpio_request_one(cs[i], 0, "spi-cs") < 0) > + dev_warn(&master->dev, > + "gpio_request_one failed for chipselect %i\n", i); > + } > > return 0; > } > -- > 2.13.3 > > -- > 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
On Mon, Jul 24, 2017 at 01:49:14PM +0300, Andy Shevchenko wrote: > On Mon, Jul 24, 2017 at 12:53 PM, Martin Hundebøll <mnhu@prevas.dk> wrote: > > Some gpio controllers requires lines to be configured as outputs to make > > subsequent calls to `gpio_set_value()` have any effect. > > Add a call to `gpio_request_one()` for configured gpio chip-selects to > > make it work on (at least) mvebu gpio controllers. > More and more hacks around plain GPIO integers... :-( > Perhaps we need to discuss how to convert SPI core to use GPIO > descriptors everywhere. The best way would be to test the work that Chris Packham submitted in May to do this, it'll need a refresh now and it ought to get some testing on ep93xx but sadly nobody working on ep93xx has responded - I think at this point I'd probably just apply and let people test -next.
On Mon, Jul 24, 2017 at 01:49:14PM +0300, Andy Shevchenko wrote:
> More and more hacks around plain GPIO integers... :-(
I have to say now I've looked at the patch I'm also struggling to see
how this is a hack around anything...
On Mon, Jul 24, 2017 at 3:02 PM, Mark Brown <broonie@kernel.org> wrote: > On Mon, Jul 24, 2017 at 01:49:14PM +0300, Andy Shevchenko wrote: > >> More and more hacks around plain GPIO integers... :-( > > I have to say now I've looked at the patch I'm also struggling to see > how this is a hack around anything... In case we switch to GPIO descriptors we can do a generic CS allocation / request. The part in the patch does something witch would be used only in OF case, while ACPI or built-in device properties are missing.
On 25/07/17 00:42, Mark Brown wrote: > On Mon, Jul 24, 2017 at 01:49:14PM +0300, Andy Shevchenko wrote: >> On Mon, Jul 24, 2017 at 12:53 PM, Martin Hundebøll <mnhu@prevas.dk> wrote: >>> Some gpio controllers requires lines to be configured as outputs to make >>> subsequent calls to `gpio_set_value()` have any effect. > >>> Add a call to `gpio_request_one()` for configured gpio chip-selects to >>> make it work on (at least) mvebu gpio controllers. > >> More and more hacks around plain GPIO integers... :-( > >> Perhaps we need to discuss how to convert SPI core to use GPIO >> descriptors everywhere. > > The best way would be to test the work that Chris Packham submitted in > May to do this, it'll need a refresh now and it ought to get some > testing on ep93xx but sadly nobody working on ep93xx has responded - I > think at this point I'd probably just apply and let people test -next. > My patchset kind of stalled. I can do a refresh if there's interest. But I really do need some help from people with access to hardware (I can cover spi-orion and spi-fsl-espi but that's about it). After the initial RFC series I posted in May I found that iMX platforms use the integer cs_gpio field for both native chipselects and gpio based ones (values < 0 indicate native CS) so again I'd be coding blind trying to disentangle that. -- 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
On July 24, 2017 11:16:17 PM GMT+02:00, Chris Packham <Chris.Packham@alliedtelesis.co.nz> wrote: >On 25/07/17 00:42, Mark Brown wrote: >> On Mon, Jul 24, 2017 at 01:49:14PM +0300, Andy Shevchenko wrote: >>> On Mon, Jul 24, 2017 at 12:53 PM, Martin Hundebøll <mnhu@prevas.dk> >wrote: >>>> Some gpio controllers requires lines to be configured as outputs to >make >>>> subsequent calls to `gpio_set_value()` have any effect. >> >>>> Add a call to `gpio_request_one()` for configured gpio chip-selects >to >>>> make it work on (at least) mvebu gpio controllers. >> >>> More and more hacks around plain GPIO integers... :-( >> >>> Perhaps we need to discuss how to convert SPI core to use GPIO >>> descriptors everywhere. >> >> The best way would be to test the work that Chris Packham submitted >in >> May to do this, it'll need a refresh now and it ought to get some >> testing on ep93xx but sadly nobody working on ep93xx has responded - >I >> think at this point I'd probably just apply and let people test >-next. >> > >My patchset kind of stalled. I can do a refresh if there's interest. >But >I really do need some help from people with access to hardware (I can >cover spi-orion and spi-fsl-espi but that's about it). After the >initial >RFC series I posted in May I found that iMX platforms use the integer >cs_gpio field for both native chipselects and gpio based ones (values < > >0 indicate native CS) so again I'd be coding blind trying to >disentangle >that. My submission was more or less a fly-by patch, but I'd be happy to test/debug patches on spi-imx if you CC me on the series. Any other drivers with special gpio-cs handling? // Martin
On Mon, Jul 24, 2017 at 09:16:17PM +0000, Chris Packham wrote: > On 25/07/17 00:42, Mark Brown wrote: > > The best way would be to test the work that Chris Packham submitted in > > May to do this, it'll need a refresh now and it ought to get some > > testing on ep93xx but sadly nobody working on ep93xx has responded - I > > think at this point I'd probably just apply and let people test -next. > My patchset kind of stalled. I can do a refresh if there's interest. But > I really do need some help from people with access to hardware (I can > cover spi-orion and spi-fsl-espi but that's about it). After the initial It's definitely something that'd be really helpful. If we get it merged near the start of a development cycle that should give plenty of opportunities for people to test. > RFC series I posted in May I found that iMX platforms use the integer > cs_gpio field for both native chipselects and gpio based ones (values < > 0 indicate native CS) so again I'd be coding blind trying to disentangle > that. i.MX is actively developed (even the SPI driver in particular) so that should be no problem at least.
On 25/07/17 09:28, Martin Hundebøll wrote: >> My patchset kind of stalled. I can do a refresh if there's interest. >> But I really do need some help from people with access to hardware (I can >> cover spi-orion and spi-fsl-espi but that's about it). After the >> initial RFC series I posted in May I found that iMX platforms use the integer >> cs_gpio field for both native chipselects and gpio based ones (values < >> 0 indicate native CS) so again I'd be coding blind trying to >> disentangle that. > > My submission was more or less a fly-by patch, but I'd be happy to > test/debug patches on spi-imx if you CC me on the series. Thanks for the offer. Now I just need to get a few round tuits. > > Any other drivers with special gpio-cs handling? > Based on a quick grep spi-{atmel,davinci,ep93xx,gpio,imx,mt65xx,stm32} all do stuff with cs_gpios outside of the spi core. I think at least some of those a trivially handled if we let the core code do the gpio management. -- 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
On Tue, Jul 25, 2017 at 12:44 AM, Chris Packham <Chris.Packham@alliedtelesis.co.nz> wrote: > On 25/07/17 09:28, Martin Hundebøll wrote: >>> My patchset kind of stalled. I can do a refresh if there's interest. >>> But I really do need some help from people with access to hardware (I can >>> cover spi-orion and spi-fsl-espi but that's about it). After the >>> initial RFC series I posted in May I found that iMX platforms use the integer >>> cs_gpio field for both native chipselects and gpio based ones (values < >>> 0 indicate native CS) so again I'd be coding blind trying to >>> disentangle that. > > >> My submission was more or less a fly-by patch, but I'd be happy to >> test/debug patches on spi-imx if you CC me on the series. > > Thanks for the offer. Now I just need to get a few round tuits. Can you please, resend the latest you have with Cc'ing me? >> Any other drivers with special gpio-cs handling? > > > > Based on a quick grep spi-{atmel,davinci,ep93xx,gpio,imx,mt65xx,stm32} > all do stuff with cs_gpios outside of the spi core. I think at least > some of those a trivially handled if we let the core code do the gpio > management. I have got ep93xx board from Mika to test, so, I can perhaps perform tests later, after I got pxa2xx working.
On 27/07/17 21:47, Andy Shevchenko wrote: > On Tue, Jul 25, 2017 at 12:44 AM, Chris Packham > <Chris.Packham@alliedtelesis.co.nz> wrote: >> On 25/07/17 09:28, Martin Hundebøll wrote: >>>> My patchset kind of stalled. I can do a refresh if there's interest. >>>> But I really do need some help from people with access to hardware (I can >>>> cover spi-orion and spi-fsl-espi but that's about it). After the >>>> initial RFC series I posted in May I found that iMX platforms use the integer >>>> cs_gpio field for both native chipselects and gpio based ones (values < >>>> 0 indicate native CS) so again I'd be coding blind trying to >>>> disentangle that. >> > >>> My submission was more or less a fly-by patch, but I'd be happy to >>> test/debug patches on spi-imx if you CC me on the series. >> >> Thanks for the offer. Now I just need to get a few round tuits. > > Can you please, resend the latest you have with Cc'ing me? > >>> Any other drivers with special gpio-cs handling? >> > >> >> Based on a quick grep spi-{atmel,davinci,ep93xx,gpio,imx,mt65xx,stm32} >> all do stuff with cs_gpios outside of the spi core. I think at least >> some of those a trivially handled if we let the core code do the gpio >> management. > > I have got ep93xx board from Mika to test, so, I can perhaps perform > tests later, after I got pxa2xx working. > Sure I'll rebase the series against linux-next#master I notice there is a rename of some variables in areas I'm touching. Should come through shortly. -- 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
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 0725c78b0de6..b7bfc52525aa 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -2010,8 +2010,13 @@ static int of_spi_register_master(struct spi_controller *ctlr) for (i = 0; i < ctlr->num_chipselect; i++) cs[i] = -ENOENT; - for (i = 0; i < nb; i++) + for (i = 0; i < nb; i++) { cs[i] = of_get_named_gpio(np, "cs-gpios", i); + if (gpio_is_valid(cs[i]) && + gpio_request_one(cs[i], 0, "spi-cs") < 0) + dev_warn(&master->dev, + "gpio_request_one failed for chipselect %i\n", i); + } return 0; }
Some gpio controllers requires lines to be configured as outputs to make subsequent calls to `gpio_set_value()` have any effect. Add a call to `gpio_request_one()` for configured gpio chip-selects to make it work on (at least) mvebu gpio controllers. Signed-off-by: Martin Hundebøll <mnhu@prevas.dk> --- drivers/spi/spi.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)