linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drivers: spi-gpio: add support for controllers without MISO or MOSI pin
@ 2010-04-01 10:35 Marek Szyprowski
       [not found] ` <1270118151-13286-1-git-send-email-m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Marek Szyprowski @ 2010-04-01 10:35 UTC (permalink / raw)
  To: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	'David Brownell'
  Cc: kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ,
	m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ

There are some boards that do not strictly follow SPI standard and use
only 3 wires (SCLK, MOSI or MISO, SS) for connecting some simple auxiliary
chips and controls them with GPIO based 'spi controller'. In this
configuration the MISO or MOSI line is missing (it is not required if the
chip does not transfer any data back to host or host only reads data from
chip).

This patch adds support for such non-standard configuration in GPIO-based
SPI controller. It has been tested in configuration without MISO pin.

Reviewed-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/spi/spi_gpio.c       |   55 +++++++++++++++++++++++++++++------------
 include/linux/spi/spi_gpio.h |    5 ++++
 2 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/spi_gpio.c b/drivers/spi/spi_gpio.c
index 26bd03e..2db2f6f 100644
--- a/drivers/spi/spi_gpio.c
+++ b/drivers/spi/spi_gpio.c
@@ -109,12 +109,16 @@ static inline void setsck(const struct spi_device *spi, int is_on)
 
 static inline void setmosi(const struct spi_device *spi, int is_on)
 {
-	gpio_set_value(SPI_MOSI_GPIO, is_on);
+	if (SPI_MOSI_GPIO != SPI_GPIO_NO_MOSI)
+		gpio_set_value(SPI_MOSI_GPIO, is_on);
 }
 
 static inline int getmiso(const struct spi_device *spi)
 {
-	return !!gpio_get_value(SPI_MISO_GPIO);
+	if (SPI_MISO_GPIO != SPI_GPIO_NO_MISO)
+		return !!gpio_get_value(SPI_MISO_GPIO);
+	else
+		return 0;
 }
 
 #undef pdata
@@ -233,19 +237,30 @@ static int __init spi_gpio_alloc(unsigned pin, const char *label, bool is_in)
 }
 
 static int __init
-spi_gpio_request(struct spi_gpio_platform_data *pdata, const char *label)
+spi_gpio_request(struct spi_gpio_platform_data *pdata, const char *label,
+	u16 *res_flags)
 {
 	int value;
 
 	/* NOTE:  SPI_*_GPIO symbols may reference "pdata" */
 
-	value = spi_gpio_alloc(SPI_MOSI_GPIO, label, false);
-	if (value)
-		goto done;
+	if (SPI_MOSI_GPIO != SPI_GPIO_NO_MOSI) {
+		value = spi_gpio_alloc(SPI_MOSI_GPIO, label, false);
+		if (value)
+			goto done;
+	} else {
+		/* HW configuration without MOSI pin */
+		*res_flags |= SPI_MASTER_NO_TX;
+	}
 
-	value = spi_gpio_alloc(SPI_MISO_GPIO, label, true);
-	if (value)
-		goto free_mosi;
+	if (SPI_MISO_GPIO != SPI_GPIO_NO_MISO) {
+		value = spi_gpio_alloc(SPI_MISO_GPIO, label, true);
+		if (value)
+			goto free_mosi;
+	} else {
+		/* HW configuration without MISO pin */
+		*res_flags |= SPI_MASTER_NO_RX;
+	}
 
 	value = spi_gpio_alloc(SPI_SCK_GPIO, label, false);
 	if (value)
@@ -254,9 +269,11 @@ spi_gpio_request(struct spi_gpio_platform_data *pdata, const char *label)
 	goto done;
 
 free_miso:
-	gpio_free(SPI_MISO_GPIO);
+	if (SPI_MISO_GPIO != SPI_GPIO_NO_MISO)
+		gpio_free(SPI_MISO_GPIO);
 free_mosi:
-	gpio_free(SPI_MOSI_GPIO);
+	if (SPI_MOSI_GPIO != SPI_GPIO_NO_MOSI)
+		gpio_free(SPI_MOSI_GPIO);
 done:
 	return value;
 }
@@ -267,6 +284,7 @@ static int __init spi_gpio_probe(struct platform_device *pdev)
 	struct spi_master		*master;
 	struct spi_gpio			*spi_gpio;
 	struct spi_gpio_platform_data	*pdata;
+	u16 master_flags = 0;
 
 	pdata = pdev->dev.platform_data;
 #ifdef GENERIC_BITBANG
@@ -274,7 +292,7 @@ static int __init spi_gpio_probe(struct platform_device *pdev)
 		return -ENODEV;
 #endif
 
-	status = spi_gpio_request(pdata, dev_name(&pdev->dev));
+	status = spi_gpio_request(pdata, dev_name(&pdev->dev), &master_flags);
 	if (status < 0)
 		return status;
 
@@ -290,6 +308,7 @@ static int __init spi_gpio_probe(struct platform_device *pdev)
 	if (pdata)
 		spi_gpio->pdata = *pdata;
 
+	master->flags = master_flags;
 	master->bus_num = pdev->id;
 	master->num_chipselect = SPI_N_CHIPSEL;
 	master->setup = spi_gpio_setup;
@@ -308,8 +327,10 @@ 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);
-		gpio_free(SPI_MOSI_GPIO);
+		if (SPI_MISO_GPIO != SPI_GPIO_NO_MISO)
+			gpio_free(SPI_MISO_GPIO);
+		if (SPI_MOSI_GPIO != SPI_GPIO_NO_MOSI)
+			gpio_free(SPI_MOSI_GPIO);
 		gpio_free(SPI_SCK_GPIO);
 		spi_master_put(master);
 	}
@@ -332,8 +353,10 @@ static int __exit spi_gpio_remove(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, NULL);
 
-	gpio_free(SPI_MISO_GPIO);
-	gpio_free(SPI_MOSI_GPIO);
+	if (SPI_MISO_GPIO != SPI_GPIO_NO_MISO)
+		gpio_free(SPI_MISO_GPIO);
+	if (SPI_MOSI_GPIO != SPI_GPIO_NO_MOSI)
+		gpio_free(SPI_MOSI_GPIO);
 	gpio_free(SPI_SCK_GPIO);
 
 	return status;
diff --git a/include/linux/spi/spi_gpio.h b/include/linux/spi/spi_gpio.h
index ca6782e..369b3d7 100644
--- a/include/linux/spi/spi_gpio.h
+++ b/include/linux/spi/spi_gpio.h
@@ -29,11 +29,16 @@
  * SPI_GPIO_NO_CHIPSELECT to the controller_data:
  *		.controller_data = (void *) SPI_GPIO_NO_CHIPSELECT;
  *
+ * If the MISO or MOSI pin is not available then it should be set to
+ * SPI_GPIO_NO_MISO or SPI_GPIO_NO_MOSI.
+ *
  * If the bitbanged bus is later switched to a "native" controller,
  * that platform_device and controller_data should be removed.
  */
 
 #define SPI_GPIO_NO_CHIPSELECT		((unsigned long)-1l)
+#define SPI_GPIO_NO_MISO		((unsigned long)-1l)
+#define SPI_GPIO_NO_MOSI		((unsigned long)-1l)
 
 /**
  * struct spi_gpio_platform_data - parameter for bitbanged SPI master
-- 
1.6.4


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] drivers: spi-gpio: add support for controllers without MISO or MOSI pin
       [not found] ` <1270118151-13286-1-git-send-email-m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
@ 2010-04-01 11:17   ` jassi brar
       [not found]     ` <i2r1b68c6791004010417q1aaa996au22f526afbedb9635-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2010-06-01 11:48   ` Marek Szyprowski
  1 sibling, 1 reply; 10+ messages in thread
From: jassi brar @ 2010-04-01 11:17 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ, David Brownell

On Thu, Apr 1, 2010 at 7:35 PM, Marek Szyprowski
<m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> There are some boards that do not strictly follow SPI standard and use
> only 3 wires (SCLK, MOSI or MISO, SS) for connecting some simple auxiliary
> chips and controls them with GPIO based 'spi controller'. In this
> configuration the MISO or MOSI line is missing (it is not required if the
> chip does not transfer any data back to host or host only reads data from
> chip).
>
> This patch adds support for such non-standard configuration in GPIO-based
> SPI controller. It has been tested in configuration without MISO pin.
Though not very clear atm, but wouldn't having some ineffective
virtual GPIO assigned
to this non-existing MISO/MOSI do the trick?

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drivers: spi-gpio: add support for controllers without MISO or MOSI pin
       [not found]     ` <i2r1b68c6791004010417q1aaa996au22f526afbedb9635-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-04-01 13:10       ` Marek Szyprowski
  0 siblings, 0 replies; 10+ messages in thread
From: Marek Szyprowski @ 2010-04-01 13:10 UTC (permalink / raw)
  To: 'jassi brar'
  Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ, 'David Brownell'

Hello,

On Thursday, April 01, 2010 1:17 PM jassi brar wrote:

> > There are some boards that do not strictly follow SPI standard and use
> > only 3 wires (SCLK, MOSI or MISO, SS) for connecting some simple auxiliary
> > chips and controls them with GPIO based 'spi controller'. In this
> > configuration the MISO or MOSI line is missing (it is not required if the
> > chip does not transfer any data back to host or host only reads data from
> > chip).
> >
> > This patch adds support for such non-standard configuration in GPIO-based
> > SPI controller. It has been tested in configuration without MISO pin.
> Though not very clear atm, but wouldn't having some ineffective
> virtual GPIO assigned
> to this non-existing MISO/MOSI do the trick?

This will be very hacky, also the platform would need to have some virtual
GPIO only for this purpose.

The proposed patch does it in the right way, especially because all required
SPI master flags (SPI_MASTER_NO_TX and SPI_MASTER_NO_RX) are already merged
to SPI core.

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center



------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drivers: spi-gpio: add support for controllers without MISO or MOSI pin
       [not found] ` <1270118151-13286-1-git-send-email-m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
  2010-04-01 11:17   ` jassi brar
@ 2010-06-01 11:48   ` Marek Szyprowski
       [not found]     ` <001701cb0180$60f0b320$22d21960$%szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
  1 sibling, 1 reply; 10+ messages in thread
From: Marek Szyprowski @ 2010-06-01 11:48 UTC (permalink / raw)
  To: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	'David Brownell'
  Cc: kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ, Marek Szyprowski

Hello,

On Thursday, April 01, 2010 12:36 PM Marek Szyprowski wrote:

> There are some boards that do not strictly follow SPI standard and use
> only 3 wires (SCLK, MOSI or MISO, SS) for connecting some simple auxiliary
> chips and controls them with GPIO based 'spi controller'. In this
> configuration the MISO or MOSI line is missing (it is not required if the
> chip does not transfer any data back to host or host only reads data from
> chip).
> 
> This patch adds support for such non-standard configuration in GPIO-based
> SPI controller. It has been tested in configuration without MISO pin.
> 
> Reviewed-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Is there any progress on merging this patch? It hangs on the patchwork for
2 months still marked as 'new'. I'm a bit confused. The idea behind the patch
was already accepted (see commit 568d0697f42771425ae9f1e9a3db769fef7e10b6)
but then the patch is still waiting... My fault I didn't asked for it on the
beginning of the merge window.

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center



------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drivers: spi-gpio: add support for controllers without MISO or MOSI pin
       [not found]     ` <001701cb0180$60f0b320$22d21960$%szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
@ 2010-06-01 14:59       ` Grant Likely
       [not found]         ` <AANLkTilQ5yD16LAq4xEmXKAmrNqO8eWFh0BmHPd2cR0q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Grant Likely @ 2010-06-01 14:59 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ, David Brownell

On Tue, Jun 1, 2010 at 5:48 AM, Marek Szyprowski
<m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> Hello,
>
> On Thursday, April 01, 2010 12:36 PM Marek Szyprowski wrote:
>
>> There are some boards that do not strictly follow SPI standard and use
>> only 3 wires (SCLK, MOSI or MISO, SS) for connecting some simple auxiliary
>> chips and controls them with GPIO based 'spi controller'. In this
>> configuration the MISO or MOSI line is missing (it is not required if the
>> chip does not transfer any data back to host or host only reads data from
>> chip).
>>
>> This patch adds support for such non-standard configuration in GPIO-based
>> SPI controller. It has been tested in configuration without MISO pin.
>>
>> Reviewed-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>
> Is there any progress on merging this patch? It hangs on the patchwork for
> 2 months still marked as 'new'. I'm a bit confused. The idea behind the patch
> was already accepted (see commit 568d0697f42771425ae9f1e9a3db769fef7e10b6)
> but then the patch is still waiting... My fault I didn't asked for it on the
> beginning of the merge window.

I'm not convinced this is the best approach.  I did look at it, but
decided not to merge it yet.  It adds code to the hottest path in the
spi-gpio driver (not that this driver will ever be fast, but on
bit-banged gpio, every instruction counts).

I was delayed it getting on the SPI backlog this merge cycle due to
travel, so I've deferred making a decision on this patch.  I'll be
taking another look in the next few weeks and I'll let you know
whether or not I'll pick it up for 2.6.36

g.

------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drivers: spi-gpio: add support for controllers without MISO or MOSI pin
       [not found]         ` <AANLkTilQ5yD16LAq4xEmXKAmrNqO8eWFh0BmHPd2cR0q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-06-02  6:00           ` Marek Szyprowski
       [not found]             ` <002b01cb0218$ddab68e0$99023aa0$%szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Marek Szyprowski @ 2010-06-02  6:00 UTC (permalink / raw)
  To: 'Grant Likely'
  Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ, 'David Brownell',
	Marek Szyprowski

Hello,

On Tuesday, June 01, 2010 5:00 PM Grant Likely wrote:

> On Tue, Jun 1, 2010 at 5:48 AM, Marek Szyprowski
> <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> > Hello,
> >
> > On Thursday, April 01, 2010 12:36 PM Marek Szyprowski wrote:
> >
> >> There are some boards that do not strictly follow SPI standard and use
> >> only 3 wires (SCLK, MOSI or MISO, SS) for connecting some simple
> auxiliary
> >> chips and controls them with GPIO based 'spi controller'. In this
> >> configuration the MISO or MOSI line is missing (it is not required if
> the
> >> chip does not transfer any data back to host or host only reads data
> from
> >> chip).
> >>
> >> This patch adds support for such non-standard configuration in GPIO-
> based
> >> SPI controller. It has been tested in configuration without MISO pin.
> >>
> >> Reviewed-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >> Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >
> > Is there any progress on merging this patch? It hangs on the patchwork
> for
> > 2 months still marked as 'new'. I'm a bit confused. The idea behind the
> patch
> > was already accepted (see commit 568d0697f42771425ae9f1e9a3db769fef7e10b6)
> > but then the patch is still waiting... My fault I didn't asked for it on
> the
> > beginning of the merge window.
> 
> I'm not convinced this is the best approach.  I did look at it, but
> decided not to merge it yet.  It adds code to the hottest path in the
> spi-gpio driver (not that this driver will ever be fast, but on
> bit-banged gpio, every instruction counts).

If this is really a problem I can remove the 'if (SPI_{MOSI,MISO}_GPIO !=
SPI_GPIO_NO_{MOSI,MISO})' checks. If we assume that the upper level of the
spi framework works correctly, these functions shouldn't be called if
the pin is set to NO_GPIO (the master device has SPI_MASTER_NO_{TX,RX} flags
set).

> I was delayed it getting on the SPI backlog this merge cycle due to
> travel, so I've deferred making a decision on this patch.  I'll be
> taking another look in the next few weeks and I'll let you know
> whether or not I'll pick it up for 2.6.36

Ok.

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center


------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drivers: spi-gpio: add support for controllers without MISO or MOSI pin
       [not found]             ` <002b01cb0218$ddab68e0$99023aa0$%szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
@ 2010-06-02  9:00               ` Jassi Brar
       [not found]                 ` <AANLkTim-yZb55jAyOaQJ_pOnp-LcXh-wdzw4TKqAyfea-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Jassi Brar @ 2010-06-02  9:00 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ, David Brownell

On Wed, Jun 2, 2010 at 3:00 PM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> On Tuesday, June 01, 2010 5:00 PM Grant Likely wrote:
>> On Tue, Jun 1, 2010 at 5:48 AM, Marek Szyprowski
>> <m.szyprowski@samsung.com> wrote:
>> I'm not convinced this is the best approach.  I did look at it, but
>> decided not to merge it yet.  It adds code to the hottest path in the
>> spi-gpio driver (not that this driver will ever be fast, but on
>> bit-banged gpio, every instruction counts).
>
> If this is really a problem I can remove the 'if (SPI_{MOSI,MISO}_GPIO !=
> SPI_GPIO_NO_{MOSI,MISO})' checks. If we assume that the upper level of the
> spi framework works correctly, these functions shouldn't be called if
> the pin is set to NO_GPIO (the master device has SPI_MASTER_NO_{TX,RX} flags
> set).
As I pointed in the last thread, such peculiar platforms had better
assign some virtual gpio
for such non-existent miso/mosi, rather than imposing these extra
checks on most of the
normal platforms.

------------------------------------------------------------------------------

_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drivers: spi-gpio: add support for controllers without MISO or MOSI pin
       [not found]                 ` <AANLkTim-yZb55jAyOaQJ_pOnp-LcXh-wdzw4TKqAyfea-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-06-09  7:14                   ` Marek Szyprowski
       [not found]                     ` <000301cb07a3$679c6ec0$36d54c40$%szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Marek Szyprowski @ 2010-06-09  7:14 UTC (permalink / raw)
  To: 'Jassi Brar'
  Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ, 'David Brownell',
	'Marek Szyprowski'

Hello,

On Wednesday, June 02, 2010 11:00 AM Jassi Brar wrote:

> On Wed, Jun 2, 2010 at 3:00 PM, Marek Szyprowski
> <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> > On Tuesday, June 01, 2010 5:00 PM Grant Likely wrote:
> >> On Tue, Jun 1, 2010 at 5:48 AM, Marek Szyprowski
> >> <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> >> I'm not convinced this is the best approach.  I did look at it, but
> >> decided not to merge it yet.  It adds code to the hottest path in the
> >> spi-gpio driver (not that this driver will ever be fast, but on
> >> bit-banged gpio, every instruction counts).
> >
> > If this is really a problem I can remove the 'if (SPI_{MOSI,MISO}_GPIO !=
> > SPI_GPIO_NO_{MOSI,MISO})' checks. If we assume that the upper level of
> the
> > spi framework works correctly, these functions shouldn't be called if
> > the pin is set to NO_GPIO (the master device has SPI_MASTER_NO_{TX,RX}
> flags
> > set).

Ok. I've did a research and that SPI_{MOSI,MISO}_GPIO != SPI_GPIO_NO_{MOSI,MISO}
checks cannot be easily removed, because in each loop both getmiso() and
setmosi() are called by the bitbang_txrx_be_* functions. I also understand that
this part of code is really hot and should be optimized as much as possible.

> As I pointed in the last thread, such peculiar platforms had better
> assign some virtual gpio
> for such non-existent miso/mosi, rather than imposing these extra
> checks on most of the normal platforms.

Creating a virtual gpio bank only for this case is also an overhead imho.
GPIO SPI masters without RX line will be used by more than one machine, so
some kind of a generic driver is imho needed.

Notice that the spi_s3c24xx_gpio.c driver already silently handles such
NO_RX transfers and machines that use it depends on such behavior. I expect
that this driver will be soon removed in favor of generic spi_gpio driver,
once plat-s3c24xx will be finally converted to use generic gpio lib.

What do you think about duplicating spi_gpio driver with my proposed changes
as spi_gpio_adv? The original spi_gpio driver won't be modified at all, so no
performance lose will happen for machines that use it. For new machines the
developers will be able to select weather they need a high speed transfers or
a driver that supports some rarely used features.

The other possibility would be to create a new version of the bitbang_txrx_be_*
functions for NO_TX and NO_RX cases. Then the proper function can be assigned
in spi_gpio_probe(). However this would increase the size of spi_gpio driver
significantly.

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center



------------------------------------------------------------------------------
ThinkGeek and WIRED's GeekDad team up for the Ultimate 
GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the 
lucky parental unit.  See the prize list and enter to win: 
http://p.sf.net/sfu/thinkgeek-promo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drivers: spi-gpio: add support for controllers without MISO or MOSI pin
       [not found]                     ` <000301cb07a3$679c6ec0$36d54c40$%szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
@ 2010-06-16 19:52                       ` Grant Likely
       [not found]                         ` <AANLkTimT_IHfv4dKXuvTpa8M4L_MDqIvjrDPBLgzKUWn-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Grant Likely @ 2010-06-16 19:52 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ, David Brownell

On Wed, Jun 9, 2010 at 1:14 AM, Marek Szyprowski
<m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> Hello,
>
> On Wednesday, June 02, 2010 11:00 AM Jassi Brar wrote:
>
>> On Wed, Jun 2, 2010 at 3:00 PM, Marek Szyprowski
>> <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>> > On Tuesday, June 01, 2010 5:00 PM Grant Likely wrote:
>> >> On Tue, Jun 1, 2010 at 5:48 AM, Marek Szyprowski
>> >> <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>> >> I'm not convinced this is the best approach.  I did look at it, but
>> >> decided not to merge it yet.  It adds code to the hottest path in the
>> >> spi-gpio driver (not that this driver will ever be fast, but on
>> >> bit-banged gpio, every instruction counts).
>> >
>> > If this is really a problem I can remove the 'if (SPI_{MOSI,MISO}_GPIO !=
>> > SPI_GPIO_NO_{MOSI,MISO})' checks. If we assume that the upper level of
>> the
>> > spi framework works correctly, these functions shouldn't be called if
>> > the pin is set to NO_GPIO (the master device has SPI_MASTER_NO_{TX,RX}
>> flags
>> > set).
>
> Ok. I've did a research and that SPI_{MOSI,MISO}_GPIO != SPI_GPIO_NO_{MOSI,MISO}
> checks cannot be easily removed, because in each loop both getmiso() and
> setmosi() are called by the bitbang_txrx_be_* functions. I also understand that
> this part of code is really hot and should be optimized as much as possible.
>
>> As I pointed in the last thread, such peculiar platforms had better
>> assign some virtual gpio
>> for such non-existent miso/mosi, rather than imposing these extra
>> checks on most of the normal platforms.
>
> Creating a virtual gpio bank only for this case is also an overhead imho.
> GPIO SPI masters without RX line will be used by more than one machine, so
> some kind of a generic driver is imho needed.

It is overhead, but it is overhead in the right place.  It costs
nothing when it isn't called, and it is trivial to create a do-nothing
gpio number for simple gpio drivers.  Systems using gpiolib can even
share the same no-op gpio controller driver.

> Notice that the spi_s3c24xx_gpio.c driver already silently handles such
> NO_RX transfers and machines that use it depends on such behavior. I expect
> that this driver will be soon removed in favor of generic spi_gpio driver,
> once plat-s3c24xx will be finally converted to use generic gpio lib.
>
> What do you think about duplicating spi_gpio driver with my proposed changes
> as spi_gpio_adv? The original spi_gpio driver won't be modified at all, so no
> performance lose will happen for machines that use it. For new machines the
> developers will be able to select weather they need a high speed transfers or
> a driver that supports some rarely used features.

I say no.  I don't think it warrants a duplicated driver, and the
dummy GPIO number is trivial to implement.  In gpiolib it would simply
mean setting aside one of the entries in the gpio_desc table; probably
at the top of the GPIO range so that existing users don't get broken.

>
> The other possibility would be to create a new version of the bitbang_txrx_be_*
> functions for NO_TX and NO_RX cases. Then the proper function can be assigned
> in spi_gpio_probe(). However this would increase the size of spi_gpio driver
> significantly.

Not that significantly, and it would keep all the relevant code
together.  I'm more in favor of this approach.

g.

------------------------------------------------------------------------------
ThinkGeek and WIRED's GeekDad team up for the Ultimate 
GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the 
lucky parental unit.  See the prize list and enter to win: 
http://p.sf.net/sfu/thinkgeek-promo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drivers: spi-gpio: add support for controllers without MISO or MOSI pin
       [not found]                         ` <AANLkTimT_IHfv4dKXuvTpa8M4L_MDqIvjrDPBLgzKUWn-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-06-17  6:54                           ` Marek Szyprowski
  0 siblings, 0 replies; 10+ messages in thread
From: Marek Szyprowski @ 2010-06-17  6:54 UTC (permalink / raw)
  To: 'Grant Likely'
  Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ, 'David Brownell',
	'Marek Szyprowski'

Hello,

On Wednesday, June 16, 2010 9:52 PM Grant Likely:

> > The other possibility would be to create a new version of the
> bitbang_txrx_be_*
> > functions for NO_TX and NO_RX cases. Then the proper function can be
> assigned
> > in spi_gpio_probe(). However this would increase the size of spi_gpio
> driver
> > significantly.
> 
> Not that significantly, and it would keep all the relevant code
> together.  I'm more in favor of this approach.

Ok, I will prepare a new patch then.

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center



------------------------------------------------------------------------------
ThinkGeek and WIRED's GeekDad team up for the Ultimate 
GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the 
lucky parental unit.  See the prize list and enter to win: 
http://p.sf.net/sfu/thinkgeek-promo

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2010-06-17  6:54 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-01 10:35 [PATCH] drivers: spi-gpio: add support for controllers without MISO or MOSI pin Marek Szyprowski
     [not found] ` <1270118151-13286-1-git-send-email-m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2010-04-01 11:17   ` jassi brar
     [not found]     ` <i2r1b68c6791004010417q1aaa996au22f526afbedb9635-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-04-01 13:10       ` Marek Szyprowski
2010-06-01 11:48   ` Marek Szyprowski
     [not found]     ` <001701cb0180$60f0b320$22d21960$%szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2010-06-01 14:59       ` Grant Likely
     [not found]         ` <AANLkTilQ5yD16LAq4xEmXKAmrNqO8eWFh0BmHPd2cR0q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-06-02  6:00           ` Marek Szyprowski
     [not found]             ` <002b01cb0218$ddab68e0$99023aa0$%szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2010-06-02  9:00               ` Jassi Brar
     [not found]                 ` <AANLkTim-yZb55jAyOaQJ_pOnp-LcXh-wdzw4TKqAyfea-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-06-09  7:14                   ` Marek Szyprowski
     [not found]                     ` <000301cb07a3$679c6ec0$36d54c40$%szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2010-06-16 19:52                       ` Grant Likely
     [not found]                         ` <AANLkTimT_IHfv4dKXuvTpa8M4L_MDqIvjrDPBLgzKUWn-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-06-17  6:54                           ` Marek Szyprowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).