There are some boards that do not strictly follow SPI standard and use only 3 wires
(SCLK, MOSI, SS) for connecting some simple auxiliary chips and controls them with GPIO
based 'spi controller'. In this configuration the MISO line is missing (it is not
required if the chip does not transfer any data back to host). The example of such
board is a NCP ARM S3C64XX based machine. This patch adds support for such non-standard
configuration in GPIO-based SPI controller.
Reviewed-by: Kyungmin Park <[email protected]>
Signed-off-by: Marek Szyprowski <[email protected]>
---
diff --git a/drivers/spi/spi_gpio.c b/drivers/spi/spi_gpio.c
index 26bd03e..16f74fd 100644
--- a/drivers/spi/spi_gpio.c
+++ b/drivers/spi/spi_gpio.c
@@ -114,7 +114,10 @@ static inline void setmosi(const struct spi_device *spi, int is_on)
static inline int getmiso(const struct spi_device *spi)
{
- return !!gpio_get_value(SPI_MISO_GPIO);
+ if (gpio_is_valid(SPI_MISO_GPIO))
+ return !!gpio_get_value(SPI_MISO_GPIO);
+ else
+ return 0;
}
#undef pdata
@@ -243,9 +246,11 @@ spi_gpio_request(struct spi_gpio_platform_data *pdata, const char *label)
if (value)
goto done;
- value = spi_gpio_alloc(SPI_MISO_GPIO, label, true);
- if (value)
- goto free_mosi;
+ if (gpio_is_valid(SPI_MISO_GPIO)) {
+ value = spi_gpio_alloc(SPI_MISO_GPIO, label, true);
+ if (value)
+ goto free_mosi;
+ }
value = spi_gpio_alloc(SPI_SCK_GPIO, label, false);
if (value)
@@ -254,7 +259,8 @@ spi_gpio_request(struct spi_gpio_platform_data *pdata, const char *label)
goto done;
free_miso:
- gpio_free(SPI_MISO_GPIO);
+ if (gpio_is_valid(SPI_MISO_GPIO))
+ gpio_free(SPI_MISO_GPIO);
free_mosi:
gpio_free(SPI_MOSI_GPIO);
done:
@@ -308,7 +314,8 @@ static int __init spi_gpio_probe(struct platform_device *pdev)
if (status < 0) {
spi_master_put(spi_gpio->bitbang.master);
gpio_free:
- gpio_free(SPI_MISO_GPIO);
+ if (gpio_is_valid(SPI_MISO_GPIO))
+ gpio_free(SPI_MISO_GPIO);
gpio_free(SPI_MOSI_GPIO);
gpio_free(SPI_SCK_GPIO);
spi_master_put(master);
@@ -332,7 +339,8 @@ static int __exit spi_gpio_remove(struct platform_device *pdev)
platform_set_drvdata(pdev, NULL);
- gpio_free(SPI_MISO_GPIO);
+ if (gpio_is_valid(SPI_MISO_GPIO))
+ gpio_free(SPI_MISO_GPIO);
gpio_free(SPI_MOSI_GPIO);
gpio_free(SPI_SCK_GPIO);
On Tuesday 23 June 2009, Marek Szyprowski wrote:
> There are some boards that do not strictly follow SPI standard and use only 3 wires
> (SCLK, MOSI, SS) for connecting some simple auxiliary chips and controls them with GPIO
> based 'spi controller'. In this configuration the MISO line is missing (it is not
> required if the chip does not transfer any data back to host). The example of such
> board is a NCP ARM S3C64XX based machine. This patch adds support for such non-standard
> configuration in GPIO-based SPI controller.
>
> Reviewed-by: Kyungmin Park <[email protected]>
> Signed-off-by: Marek Szyprowski <[email protected]>
This is missing one integrity feature: it should reject all
requests that provide a read buffer, since obviously it can't
satisfy any such requests.
Also, please add a comment in spi_gpio_request() explaining
this point: that output-only configs exist, and you're
supporting them by letting MISO be invalid.
Address those two points and this will be almost OK.
Thing is, this raises two related issues: (a) there's the
analagous input-only case where MOSI isn't used, e.g. for
some kinds of sensor; and (b) there's also "real 3-wire SPI"
(spi->mode & SPI_3WIRE) where interactions are limited to
half duplex and one pin switches roles between MOSI and MISO.
Clearly this "output-only" case is a subset of SPI_3WIRE (the
MOMI/SISO pin can't switch direction) so one more change I
want to see is requiring that spi->mode flag be set in all
SPI devices registered when this mode is used.
If you have time, it would be good to generaize this patch
to cover all of those 3-wire modes ... accept all half
duplex calls, use gpio_direction_*() to switch direction.
If not -- e.g. nothing to test that with! -- then I can
understand, but please make your changes with that model
in mind, and leave an appropriate FIXME in place.
- Dave
p.s. I'll CC you on one more related patch. Briefly,
there's a new spi_master->flags field that should
set SPI_MASTER_HALF_DUPLEX. Arguably, this specific
case should also advertise something like NO_RX.
>
> ---
>
> diff --git a/drivers/spi/spi_gpio.c b/drivers/spi/spi_gpio.c
> index 26bd03e..16f74fd 100644
> --- a/drivers/spi/spi_gpio.c
> +++ b/drivers/spi/spi_gpio.c
> @@ -114,7 +114,10 @@ static inline void setmosi(const struct spi_device *spi, int is_on)
>
> static inline int getmiso(const struct spi_device *spi)
> {
> - return !!gpio_get_value(SPI_MISO_GPIO);
> + if (gpio_is_valid(SPI_MISO_GPIO))
> + return !!gpio_get_value(SPI_MISO_GPIO);
> + else
> + return 0;
> }
>
> #undef pdata
> @@ -243,9 +246,11 @@ spi_gpio_request(struct spi_gpio_platform_data *pdata, const char *label)
> if (value)
> goto done;
>
> - value = spi_gpio_alloc(SPI_MISO_GPIO, label, true);
> - if (value)
> - goto free_mosi;
> + if (gpio_is_valid(SPI_MISO_GPIO)) {
> + value = spi_gpio_alloc(SPI_MISO_GPIO, label, true);
> + if (value)
> + goto free_mosi;
> + }
>
> value = spi_gpio_alloc(SPI_SCK_GPIO, label, false);
> if (value)
> @@ -254,7 +259,8 @@ spi_gpio_request(struct spi_gpio_platform_data *pdata, const char *label)
> goto done;
>
> free_miso:
> - gpio_free(SPI_MISO_GPIO);
> + if (gpio_is_valid(SPI_MISO_GPIO))
> + gpio_free(SPI_MISO_GPIO);
> free_mosi:
> gpio_free(SPI_MOSI_GPIO);
> done:
> @@ -308,7 +314,8 @@ static int __init spi_gpio_probe(struct platform_device *pdev)
> if (status < 0) {
> spi_master_put(spi_gpio->bitbang.master);
> gpio_free:
> - gpio_free(SPI_MISO_GPIO);
> + if (gpio_is_valid(SPI_MISO_GPIO))
> + gpio_free(SPI_MISO_GPIO);
> gpio_free(SPI_MOSI_GPIO);
> gpio_free(SPI_SCK_GPIO);
> spi_master_put(master);
> @@ -332,7 +339,8 @@ static int __exit spi_gpio_remove(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, NULL);
>
> - gpio_free(SPI_MISO_GPIO);
> + if (gpio_is_valid(SPI_MISO_GPIO))
> + gpio_free(SPI_MISO_GPIO);
> gpio_free(SPI_MOSI_GPIO);
> gpio_free(SPI_SCK_GPIO);
>
>
>
Hello,
On Tuesday, June 23, 2009 8:55 PM, David Brownell wrote:
> > There are some boards that do not strictly follow SPI standard and use only 3 wires
> > (SCLK, MOSI, SS) for connecting some simple auxiliary chips and controls them with GPIO
> > based 'spi controller'. In this configuration the MISO line is missing (it is not
> > required if the chip does not transfer any data back to host). The example of such
> > board is a NCP ARM S3C64XX based machine. This patch adds support for such non-standard
> > configuration in GPIO-based SPI controller.
> >
> > Reviewed-by: Kyungmin Park <[email protected]>
> > Signed-off-by: Marek Szyprowski <[email protected]>
>
> This is missing one integrity feature: it should reject all
> requests that provide a read buffer, since obviously it can't
> satisfy any such requests.
>
> Also, please add a comment in spi_gpio_request() explaining
> this point: that output-only configs exist, and you're
> supporting them by letting MISO be invalid.
>
> Address those two points and this will be almost OK.
Thanks for your review. I will post an updated patch soon.
> Thing is, this raises two related issues: (a) there's the
> analagous input-only case where MOSI isn't used, e.g. for
> some kinds of sensor; and (b) there's also "real 3-wire SPI"
> (spi->mode & SPI_3WIRE) where interactions are limited to
> half duplex and one pin switches roles between MOSI and MISO.
>
> Clearly this "output-only" case is a subset of SPI_3WIRE (the
> MOMI/SISO pin can't switch direction) so one more change I
> want to see is requiring that spi->mode flag be set in all
> SPI devices registered when this mode is used.
>
> If you have time, it would be good to generaize this patch
> to cover all of those 3-wire modes ... accept all half
> duplex calls, use gpio_direction_*() to switch direction.
>
> If not -- e.g. nothing to test that with! -- then I can
> understand, but please make your changes with that model
> in mind, and leave an appropriate FIXME in place.
I'm sorry, but I have only a hw board with missing MISO pin so I won't
be able to properly check/debug any other configurations. However I
will add a support for missing MOSI pin as well (simple code
reusage).
> p.s. I'll CC you on one more related patch. Briefly,
> there's a new spi_master->flags field that should
> set SPI_MASTER_HALF_DUPLEX. Arguably, this specific
> case should also advertise something like NO_RX.
Thanks. I will add SPI_MASTER_NO_TX/NO_RX flags there.
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
On Thursday 25 June 2009, Marek Szyprowski wrote:
> > Thing is, this raises two related issues: ?(a) there's the
> > analagous input-only case where MOSI isn't used, e.g. for
> > some kinds of sensor; and (b) there's also "real 3-wire SPI"
> > (spi->mode & SPI_3WIRE) where interactions are limited to
> > half duplex and one pin switches roles between MOSI and MISO.
> >
> > Clearly this "output-only" case is a subset of SPI_3WIRE (the
> > MOMI/SISO pin can't switch direction) so one more change I
> > want to see is requiring that spi->mode flag be set in all
> > SPI devices registered when this mode is used.
Don't forget that change ...
> > If you have time, it would be good to generalize this patch
> > to cover all of those 3-wire modes ... accept all half
> > duplex calls, use gpio_direction_*() to switch direction.
> >
> > If not -- e.g. nothing to test that with! -- then I can
> > understand, but please make your changes with that model
> > in mind, and leave an appropriate FIXME in place.
>
> I'm sorry, but I have only a hw board with missing MISO pin so I won't
> be able to properly check/debug any other configurations.
OK, then just stick to what you can test, and comment the issue.