| Message ID | 1437999076-693-2-git-send-email-sifan.naeem@imgtec.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sifan, On Mon, Jul 27, 2015 at 5:11 AM, Sifan Naeem <sifan.naeem@imgtec.com> wrote: > spfi_setup may be called many times bye the spi framework, but > gpio_request_one can only be called once without freeing, repeatedly > calling gpio_request_one will cause an error to be thrown, which causes > the request to spi_setup to be marked as failed. > > We can simply use gpio_direction_output to set the direction of the > gpio instead of gpio_request_one to put the gpio in to initial state, > after which the spi framework can control the chipselect line via gpio > using gpio_set_value. I don't think we want to leave the CS GPIOs un-requested. Instead, we could either request them all at probe() time (and then set direction in setup()) or have a per-spi_device flag that indicates whether or not the GPIO has been requested. -Andrew -- 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 27, 2015 at 09:34:41AM -0700, Andrew Bresticker wrote: > On Mon, Jul 27, 2015 at 5:11 AM, Sifan Naeem <sifan.naeem@imgtec.com> wrote: > > spfi_setup may be called many times bye the spi framework, but > > gpio_request_one can only be called once without freeing, repeatedly > > calling gpio_request_one will cause an error to be thrown, which causes > > the request to spi_setup to be marked as failed. > > We can simply use gpio_direction_output to set the direction of the > > gpio instead of gpio_request_one to put the gpio in to initial state, > > after which the spi framework can control the chipselect line via gpio > > using gpio_set_value. > I don't think we want to leave the CS GPIOs un-requested. Instead, we > could either request them all at probe() time (and then set direction > in setup()) or have a per-spi_device flag that indicates whether or > not the GPIO has been requested. Not requesting the GPIOs at all would just be a straight up bug.
Thanks Mark, Andrew. I'll rework this and submit a new patch. Thanks, Sifan > -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: 27 July 2015 17:40 > To: Andrew Bresticker > Cc: Sifan Naeem; linux-spi@vger.kernel.org; Stable kernel (v4.1) > Subject: Re: [PATCH 2/2] spi: img-spfi: fix spfi_setup by removing > gpio_request_one > > On Mon, Jul 27, 2015 at 09:34:41AM -0700, Andrew Bresticker wrote: > > On Mon, Jul 27, 2015 at 5:11 AM, Sifan Naeem <sifan.naeem@imgtec.com> > wrote: > > > > spfi_setup may be called many times bye the spi framework, but > > > gpio_request_one can only be called once without freeing, repeatedly > > > calling gpio_request_one will cause an error to be thrown, which > > > causes the request to spi_setup to be marked as failed. > > > > We can simply use gpio_direction_output to set the direction of the > > > gpio instead of gpio_request_one to put the gpio in to initial > > > state, after which the spi framework can control the chipselect line > > > via gpio using gpio_set_value. > > > I don't think we want to leave the CS GPIOs un-requested. Instead, we > > could either request them all at probe() time (and then set direction > > in setup()) or have a per-spi_device flag that indicates whether or > > not the GPIO has been requested. > > Not requesting the GPIOs at all would just be a straight up bug. -- 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
Please find the new patch in http://marc.info/?l=linux-spi&m=143816718611912&w=2 Thanks, Sifan > -----Original Message----- > From: Sifan Naeem > Sent: 27 July 2015 17:54 > To: 'Mark Brown'; Andrew Bresticker > Cc: linux-spi@vger.kernel.org; Stable kernel (v4.1) > Subject: RE: [PATCH 2/2] spi: img-spfi: fix spfi_setup by removing > gpio_request_one > > Thanks Mark, Andrew. > > I'll rework this and submit a new patch. > > Thanks, > Sifan > > > -----Original Message----- > > From: Mark Brown [mailto:broonie@kernel.org] > > Sent: 27 July 2015 17:40 > > To: Andrew Bresticker > > Cc: Sifan Naeem; linux-spi@vger.kernel.org; Stable kernel (v4.1) > > Subject: Re: [PATCH 2/2] spi: img-spfi: fix spfi_setup by removing > > gpio_request_one > > > > On Mon, Jul 27, 2015 at 09:34:41AM -0700, Andrew Bresticker wrote: > > > On Mon, Jul 27, 2015 at 5:11 AM, Sifan Naeem > > > <sifan.naeem@imgtec.com> > > wrote: > > > > > > spfi_setup may be called many times bye the spi framework, but > > > > gpio_request_one can only be called once without freeing, > > > > repeatedly calling gpio_request_one will cause an error to be > > > > thrown, which causes the request to spi_setup to be marked as failed. > > > > > > We can simply use gpio_direction_output to set the direction of > > > > the gpio instead of gpio_request_one to put the gpio in to initial > > > > state, after which the spi framework can control the chipselect > > > > line via gpio using gpio_set_value. > > > > > I don't think we want to leave the CS GPIOs un-requested. Instead, > > > we could either request them all at probe() time (and then set > > > direction in setup()) or have a per-spi_device flag that indicates > > > whether or not the GPIO has been requested. > > > > Not requesting the GPIOs at all would just be a straight up bug. -- 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 Wed, Jul 29, 2015 at 11:05:05AM +0000, Sifan Naeem wrote: > Please find the new patch in http://marc.info/?l=linux-spi&m=143816718611912&w=2 > > Thanks, > Sifan > > > -----Original Message----- Please don't top post and please submit patches using the normal process documented in SubmittingPatches.
Hi Mark, > On Wed, Jul 29, 2015 at 11:05:05AM +0000, Sifan Naeem wrote: > > Please find the new patch in > > http://marc.info/?l=linux-spi&m=143816718611912&w=2 > > > > Thanks, > > Sifan > > > > > -----Original Message----- > > Please don't top post and please submit patches using the normal process > documented in SubmittingPatches. Noted, I did submit the modified patch following the normal process, but as the patch title changed I mentioned it here. [PATCH v2] spi: img-spfi: fix multiple calls to request gpio Thanks, Sifan -- 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-img-spfi.c b/drivers/spi/spi-img-spfi.c index f27d9d5..a7f3d6a 100644 --- a/drivers/spi/spi-img-spfi.c +++ b/drivers/spi/spi-img-spfi.c @@ -442,13 +442,15 @@ static int img_spfi_setup(struct spi_device *spi) { int ret; - ret = gpio_request_one(spi->cs_gpio, (spi->mode & SPI_CS_HIGH) ? - GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH, - dev_name(&spi->dev)); - if (ret) - dev_err(&spi->dev, "can't request chipselect gpio %d\n", - spi->cs_gpio); - + if (gpio_is_valid(spi->cs_gpio)) { + int mode = ((spi->mode & SPI_CS_HIGH) ? + GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH); + + ret = gpio_direction_output(spi->cs_gpio, mode); + if (ret) + dev_err(&spi->dev, "chipselect gpio %d setup failed (%d)\n", + spi->cs_gpio, ret); + } return ret; }
spfi_setup may be called many times bye the spi framework, but gpio_request_one can only be called once without freeing, repeatedly calling gpio_request_one will cause an error to be thrown, which causes the request to spi_setup to be marked as failed. We can simply use gpio_direction_output to set the direction of the gpio instead of gpio_request_one to put the gpio in to initial state, after which the spi framework can control the chipselect line via gpio using gpio_set_value. Fixes: 8c2c8c0 ("spi: img-spfi: Control CS lines with GPIO") Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com> Cc: Stable kernel (v4.1) <stable@vger.kernel.org> --- drivers/spi/spi-img-spfi.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)