[PATCH 0/2] Setting GPIOs simultaneously

[PATCH 0/2] Setting GPIOs simultaneously

[Anton Vorontsov]

Hi all,

I've been sitting on these patches for some time, but now it appears
that the set_sync() feature is needed elsewhere. So here are the
patches.

Joakim, I think this is what you need.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

[PATCH 1/2] gpiolib: Implement gpio_set_values_sync()

[Anton Vorontsov]

Sometimes it's necessary to set GPIO values synchronously, e.g.
when CPU's GPIOs go to a FPGA and the FPGA has no latch-enable line,
so intermediate values on GPIO lines may trigger unnecessary actions
by the FPGA.

Another usage is chip-select muxes, e.g. using X GPIOs as address lines
to mux 2^X chip-selects, and setting some amount of GPIOs at once may
be a huge win.

Note that it's not always possible to set up GPIOs synchronously, so
gpio_can_set_values_sync() call is implemented. Currently it checks
for two things:

1. Whether gpio_chip has .set_sync() callback;
2. Whether all passed GPIOs belong to one particular gpio_chip.

Someday we may want to implement chip-specific .can_set_sync() check,
but currently this isn't needed.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/gpio/gpiolib.c     |   57 ++++++++++++++++++++++++++++++++++++++++++++
 include/asm-generic/gpio.h |    8 ++++++
 include/linux/gpio.h       |   14 ++++++++++
 3 files changed, 79 insertions(+), 0 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 51a8d41..cf714ad 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1067,6 +1067,53 @@ void __gpio_set_value(unsigned gpio, int value)
 EXPORT_SYMBOL_GPL(__gpio_set_value);
 
 /**
+ * __gpio_can_set_values_sync() - check if gpios can be set synchronously
+ * @num: amount of gpios
+ * @gpios: gpios in question
+ * Context: any
+ *
+ * This is used directly or indirectly to implement gpio_can_set_values_sync().
+ * It returns nonzero if an array of gpios can be set simultaneously.
+ */
+int __gpio_can_set_values_sync(unsigned num, unsigned *gpios)
+{
+	struct gpio_chip *chip;
+	unsigned i;
+
+	chip = gpio_to_chip(gpios[0]);
+	if (!chip->set_sync)
+		return 0;
+
+	for (i = 1; i < num; i++) {
+		if (chip != gpio_to_chip(gpios[i]))
+			return 0;
+	}
+
+	return 1;
+}
+EXPORT_SYMBOL_GPL(__gpio_can_set_values_sync);
+
+/**
+ * __gpio_set_values_sync() - assign gpios' values synchronously
+ * @num: amount of gpios
+ * @gpios: gpios whose value will be assigned
+ * @values: value to assign
+ * Context: any
+ *
+ * This is used directly or indirectly to implement gpio_set_values_sync().
+ * It invokes the associated gpio_chip.set_sync() method.
+ */
+void __gpio_set_values_sync(unsigned num, unsigned *gpios, int *values)
+{
+	struct gpio_chip *chip;
+
+	chip = gpio_to_chip(gpios[0]);
+	WARN_ON(extra_checks && chip->can_sleep);
+	chip->set_sync(chip, num, gpios, values);
+}
+EXPORT_SYMBOL_GPL(__gpio_set_values_sync);
+
+/**
  * __gpio_cansleep() - report whether gpio value access will sleep
  * @gpio: gpio in question
  * Context: any
@@ -1129,6 +1176,16 @@ void gpio_set_value_cansleep(unsigned gpio, int value)
 }
 EXPORT_SYMBOL_GPL(gpio_set_value_cansleep);
 
+void gpio_set_values_sync_cansleep(unsigned num, unsigned *gpios, int *values)
+{
+	struct gpio_chip	*chip;
+
+	might_sleep_if(extra_checks);
+	chip = gpio_to_chip(gpios[0]);
+	chip->set_sync(chip, num, gpios, values);
+}
+EXPORT_SYMBOL_GPL(gpio_set_values_sync_cansleep);
+
 
 #ifdef CONFIG_DEBUG_FS
 
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index d6c379d..c181623 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -88,6 +88,10 @@ struct gpio_chip {
 						unsigned offset, int value);
 	void			(*set)(struct gpio_chip *chip,
 						unsigned offset, int value);
+	/* NOTE: _sync() version accepts gpios, not offsets. */
+	void			(*set_sync)(struct gpio_chip *chip,
+						unsigned num, unsigned *gpios,
+						int *values);
 
 	int			(*to_irq)(struct gpio_chip *chip,
 						unsigned offset);
@@ -121,6 +125,8 @@ extern int gpio_direction_output(unsigned gpio, int value);
 
 extern int gpio_get_value_cansleep(unsigned gpio);
 extern void gpio_set_value_cansleep(unsigned gpio, int value);
+extern void gpio_set_values_sync_cansleep(unsigned num, unsigned *gpios,
+					  int *values);
 
 
 /* A platform's <asm/gpio.h> code may want to inline the I/O calls when
@@ -129,6 +135,8 @@ extern void gpio_set_value_cansleep(unsigned gpio, int value);
  */
 extern int __gpio_get_value(unsigned gpio);
 extern void __gpio_set_value(unsigned gpio, int value);
+extern int __gpio_can_set_values_sync(unsigned num, unsigned *gpios);
+extern void __gpio_set_values_sync(unsigned num, unsigned *gpios, int *values);
 
 extern int __gpio_cansleep(unsigned gpio);
 
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index e10c49a..ff62c1e 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -62,6 +62,20 @@ static inline void gpio_set_value(unsigned gpio, int value)
 	WARN_ON(1);
 }
 
+static inline int gpio_can_set_values_sync(unsigned num, unsigned *gpios)
+{
+	/* GPIO can never have been requested or set as output */
+	WARN_ON(1);
+	return 0;
+}
+
+static inline void gpio_set_values_sync(unsigned num, unsigned *gpios,
+					int *values)
+{
+	/* GPIO can never have been requested or set as output */
+	WARN_ON(1);
+}
+
 static inline int gpio_cansleep(unsigned gpio)
 {
 	/* GPIO can never have been requested or set as {in,out}put */
-- 
1.6.3.3

[PATCH 2/2] powerpc/qe: Implement set_sync() callback for QE GPIOs

[Anton Vorontsov]

This is needed to set GPIO's values synchronously.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/include/asm/gpio.h   |   11 +++++++++++
 arch/powerpc/sysdev/qe_lib/gpio.c |   27 +++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/gpio.h b/arch/powerpc/include/asm/gpio.h
index ea04632..899f365 100644
--- a/arch/powerpc/include/asm/gpio.h
+++ b/arch/powerpc/include/asm/gpio.h
@@ -33,6 +33,17 @@ static inline void gpio_set_value(unsigned int gpio, int value)
 	__gpio_set_value(gpio, value);
 }
 
+static inline int gpio_can_set_values_sync(unsigned num, unsigned *gpios)
+{
+	return __gpio_can_set_values_sync(num, gpios);
+}
+
+static inline void gpio_set_values_sync(unsigned num, unsigned *gpios,
+					int *values)
+{
+	__gpio_set_values_sync(num, gpios, values);
+}
+
 static inline int gpio_cansleep(unsigned int gpio)
 {
 	return __gpio_cansleep(gpio);
diff --git a/arch/powerpc/sysdev/qe_lib/gpio.c b/arch/powerpc/sysdev/qe_lib/gpio.c
index 3485288..6cfe784 100644
--- a/arch/powerpc/sysdev/qe_lib/gpio.c
+++ b/arch/powerpc/sysdev/qe_lib/gpio.c
@@ -84,6 +84,32 @@ static void qe_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 	spin_unlock_irqrestore(&qe_gc->lock, flags);
 }
 
+static void qe_gpio_set_sync(struct gpio_chip *gc, unsigned int num,
+			     unsigned int *gpios, int *vals)
+{
+	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct qe_gpio_chip *qe_gc = to_qe_gpio_chip(mm_gc);
+	struct qe_pio_regs __iomem *regs = mm_gc->regs;
+	unsigned long flags;
+	int i;
+
+	spin_lock_irqsave(&qe_gc->lock, flags);
+
+	for (i = 0; i < num; i++) {
+		unsigned int gpio = gpios[i] - gc->base;
+		u32 pin_mask = 1 << (QE_PIO_PINS - 1 - gpio);
+
+		if (vals[i])
+			qe_gc->cpdata |= pin_mask;
+		else
+			qe_gc->cpdata &= ~pin_mask;
+	}
+
+	out_be32(&regs->cpdata, qe_gc->cpdata);
+
+	spin_unlock_irqrestore(&qe_gc->lock, flags);
+}
+
 static int qe_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
 {
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
@@ -328,6 +354,7 @@ static int __init qe_add_gpiochips(void)
 		gc->direction_output = qe_gpio_dir_out;
 		gc->get = qe_gpio_get;
 		gc->set = qe_gpio_set;
+		gc->set_sync = qe_gpio_set_sync;
 
 		ret = of_mm_gpiochip_add(np, mm_gc);
 		if (ret)
-- 
1.6.3.3

Re: [PATCH 0/2] Setting GPIOs simultaneously

[Joakim Tjernlund]

Anton Vorontsov <avorontsov@ru.mvista.com> wrote on 13/07/2009 17:19:11:
>
> Hi all,
>
> I've been sitting on these patches for some time, but now it appears
> that the set_sync() feature is needed elsewhere. So here are the
> patches.
>
> Joakim, I think this is what you need.

Yes, it sure looks so :) I will have to look closer later as
I will be traveling the next few days.

Question though, have you considered using a bitmask instead of
an array:
static void qe_gpio_set_sync(struct gpio_chip *gc, unsigned int num,
	 		      unsigned int gpio_mask, unsigned int vals)
If you want to set bit 0, 3 and 8 you would set positions 0, 3 and 8 in gpio_mask
to ones. Similarly in vals, set bit positions 0, 3 and 8 to requested value.

While being at it, the reason for me needing this is that the spi_mpc83xx driver
was recently converted to a OF only driver so I have no way of defining my own
CS function anymore. While OF is good I don't feel that OF drivers should block the native
method, OF should be a layer on top of the native methods.

 Jocke

Re: [PATCH 0/2] Setting GPIOs simultaneously

[Anton Vorontsov]

On Mon, Jul 13, 2009 at 06:01:02PM +0200, Joakim Tjernlund wrote:
> 
> Anton Vorontsov <avorontsov@ru.mvista.com> wrote on 13/07/2009 17:19:11:
> >
> > Hi all,
> >
> > I've been sitting on these patches for some time, but now it appears
> > that the set_sync() feature is needed elsewhere. So here are the
> > patches.
> >
> > Joakim, I think this is what you need.
> 
> Yes, it sure looks so :) I will have to look closer later as
> I will be traveling the next few days.
> 
> Question though, have you considered using a bitmask instead of
> an array:
> static void qe_gpio_set_sync(struct gpio_chip *gc, unsigned int num,
> 	 		      unsigned int gpio_mask, unsigned int vals)
> If you want to set bit 0, 3 and 8 you would set positions 0, 3 and 8 in gpio_mask
> to ones. Similarly in vals, set bit positions 0, 3 and 8 to requested value.

Yeah, I thought about it. We could do the u64 masks (to handle up
to 64 bits parallel IO buses).

It's all easy with dumb memory-mapped GPIO controllers, because
we have a 8/16/32/64 bits registers with linear bit<->gpio mapping.

But some gpio controllers aren't that easy. I know at least one
(FPGA-based) gpio controller that won't change any GPIO lines
for real unless changes are "commited". The controller has several
banks (registers) of PIOs (total count > 64 bits), but you can commit
all the changes to the banks at once (synchronously). This isn't
because the controller is uber-cool, it's just the controller has
sequential IO. So with masks approach you won't able to use _sync()
calls that easily for all GPIOs range.

But OK, if we throw away the special cases, I can't imagine any
clear api for this approach, all I can think of is something
along these lines:

int num = 3;
u32 gpios[3];
u64 shifts[3];

/* this implies checks whether we can use _sync() */
if (!gpio_get_shifts(num, gpios, shifts))
	return -EINVAL;

gpio_set_values_sync(chip, 1 << shifts[0] | 1 << shifts[1],
		     val0 << shifts[0] | val1 << shifts[1]).

We can implement it, if that's acceptable. But that's a bit
ugly, I think.

> While being at it, the reason for me needing this is that the spi_mpc83xx driver
> was recently converted to a OF only driver so I have no way of defining my own
> CS function anymore. While OF is good I don't feel that OF drivers should block the native
> method, OF should be a layer on top of the native methods.

Um, I don't get it. You have a mux, which is a sort of GPIO controller.
All you need to do is to write "of-gpio-mux" driver, that will get all
the needed gpios from the underlaying GPIO controller.

In the device tree it'll look like this:

muxed_gpio: gpio-controller {
	#gpio-cells = <2>;
	compatible = "board-gpio-mux", "generic-gpio-mux";
	gpios = <&qe_pio_d  2 0   /* AD0 */
		 &qe_pio_d 17 0   /* AD1 */
		 &qe_pio_d  5 0>; /* AD2 */
	gpio-controller;
};

spi-controller {
	gpios = <&muxed_gpio 0 0
		 &muxed_gpio 1 0
		 &muxed_gpio 2 0
		 &muxed_gpio 3 0
		 &muxed_gpio 4 0
		 &muxed_gpio 5 0
		 &muxed_gpio 6 0
		 &muxed_gpio 7 0>;

	spi-device@0 {
		reg = <0>;
	};
	...
	spi-device@7 {
		reg = <0>;
	};
};

So you don't have to modify the spi driver.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 0/2] Setting GPIOs simultaneously

[Joakim Tjernlund]

Anton Vorontsov <avorontsov@ru.mvista.com> wrote on 13/07/2009 19:34:55:
>
> On Mon, Jul 13, 2009 at 06:01:02PM +0200, Joakim Tjernlund wrote:
> >
> > Anton Vorontsov <avorontsov@ru.mvista.com> wrote on 13/07/2009 17:19:11:
> > >
> > > Hi all,
> > >
> > > I've been sitting on these patches for some time, but now it appears
> > > that the set_sync() feature is needed elsewhere. So here are the
> > > patches.
> > >
> > > Joakim, I think this is what you need.
> >
> > Yes, it sure looks so :) I will have to look closer later as
> > I will be traveling the next few days.
> >
> > Question though, have you considered using a bitmask instead of
> > an array:
> > static void qe_gpio_set_sync(struct gpio_chip *gc, unsigned int num,
> >                 unsigned int gpio_mask, unsigned int vals)
> > If you want to set bit 0, 3 and 8 you would set positions 0, 3 and 8 in gpio_mask
> > to ones. Similarly in vals, set bit positions 0, 3 and 8 to requested value.
>
> Yeah, I thought about it. We could do the u64 masks (to handle up
> to 64 bits parallel IO buses).
>
> It's all easy with dumb memory-mapped GPIO controllers, because
> we have a 8/16/32/64 bits registers with linear bit<->gpio mapping.

Yes, this is gpio ..

>
> But some gpio controllers aren't that easy. I know at least one
> (FPGA-based) gpio controller that won't change any GPIO lines
> for real unless changes are "commited". The controller has several
> banks (registers) of PIOs (total count > 64 bits), but you can commit
> all the changes to the banks at once (synchronously). This isn't
> because the controller is uber-cool, it's just the controller has
> sequential IO. So with masks approach you won't able to use _sync()
> calls that easily for all GPIOs range.

.. but this one I am not sure qualify as gpio. Feels more like
its own device that should have its own driver.

>
> But OK, if we throw away the special cases, I can't imagine any
> clear api for this approach, all I can think of is something
> along these lines:
>
> int num = 3;
> u32 gpios[3];
> u64 shifts[3];
>
> /* this implies checks whether we can use _sync() */
> if (!gpio_get_shifts(num, gpios, shifts))
>    return -EINVAL;
>
> gpio_set_values_sync(chip, 1 << shifts[0] | 1 << shifts[1],
>            val0 << shifts[0] | val1 << shifts[1]).
>
> We can implement it, if that's acceptable. But that's a bit
> ugly, I think.

I see, that doesn't look good that either.

>
> > While being at it, the reason for me needing this is that the spi_mpc83xx driver
> > was recently converted to a OF only driver so I have no way of defining my own
> > CS function anymore. While OF is good I don't feel that OF drivers should
> block the native
> > method, OF should be a layer on top of the native methods.
>
> Um, I don't get it. You have a mux, which is a sort of GPIO controller.
> All you need to do is to write "of-gpio-mux" driver, that will get all
> the needed gpios from the underlaying GPIO controller.

Well, I already have a mux controller that is using the native spi methods. I
don't want to write a new one, far more complicated than what I got now.
While the OF system is very powerful and flexible I still think that
one should be able to use native SPI methods too. Why can they not
co-exist?

>
> In the device tree it'll look like this:

I must admit that I just started to look at the GPIO subsystem, but
I do wonder if mpc83xx_spi_cs_control() can do this muxing
without any change.

Very good example BTW.

      Jocke

Re: [PATCH 0/2] Setting GPIOs simultaneously

[Anton Vorontsov]

On Mon, Jul 13, 2009 at 09:59:54PM +0200, Joakim Tjernlund wrote:
[...]
> > > While being at it, the reason for me needing this is that the spi_mpc83xx driver
> > > was recently converted to a OF only driver so I have no way of defining my own
> > > CS function anymore. While OF is good I don't feel that OF drivers should
> > block the native
> > > method, OF should be a layer on top of the native methods.
> >
> > Um, I don't get it. You have a mux, which is a sort of GPIO controller.
> > All you need to do is to write "of-gpio-mux" driver, that will get all
> > the needed gpios from the underlaying GPIO controller.
> 
> Well, I already have a mux controller that is using the native spi methods. I
> don't want to write a new one, far more complicated than what I got now.
> While the OF system is very powerful and flexible I still think that
> one should be able to use native SPI methods too. Why can they not
> co-exist?

I strongly believe that they can. You just need to post patches
so that we could see them, and maybe discuss how to do things
more generic. But if making things generic will be too hard (see
below), I don't think anybody will object applying a non-generic
solution (even permitting "legacy" bindings in the spi_8xxx driver).

But any users of the legacy bindings should be in-tree.

> > In the device tree it'll look like this:
> 
> I must admit that I just started to look at the GPIO subsystem, but
> I do wonder if mpc83xx_spi_cs_control() can do this muxing
> without any change.
> 
> Very good example BTW.

Well, there is one caveat: the "GPIOs" aren't independent,
so thinking about it more, I believe we should now discuss
"chip-select framework", not gpio controllers (it could be
that using gpio controllers for this purpose wasn't that
good idea).

http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg34738.html
^^^ I'm dreaming about this framework. I.e. true addressing
    for chip-selects. :-)

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Ang: Re: [PATCH 0/2] Setting GPIOs simultaneously

[Joakim Tjernlund]

-----Anton Vorontsov <avorontsov@ru.mvista.com> skrev: -----
>Till: Joakim Tjernlund <joakim.tjernlund@transmode.se>
>Fr=E5n: Anton Vorontsov <avorontsov@ru.mvista.com>
>Datum: 07/14/2009 00:20
>Kopia: David Brownell <dbrownell@users.sourceforge.net>,
>linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org
>=C4rende: Re: [PATCH 0/2] Setting GPIOs simultaneously
>
>
>On Mon, Jul 13, 2009 at 09:59:54PM +0200, Joakim Tjernlund wrote:
>[...]
>> > > While being at it, the reason for me needing this is that the sp=
i_mpc83xx
>driver
>> > > was recently converted to a OF only driver so I have no way of d=
efining
>my own
>> > > CS function anymore. While OF is good I don't feel that OF drive=
rs
>should
>> > block the native
>> > > method, OF should be a layer on top of the native methods.
>> >
>> > Um, I don't get it. You have a mux, which is a sort of GPIO contro=
ller.
>> > All you need to do is to write "of-gpio-mux" driver, that will get=
 all
>> > the needed gpios from the underlaying GPIO controller.
>>
>> Well, I already have a mux controller that is using the native spi m=
ethods.
>I
>> don't want to write a new one, far more complicated than what I got =
now.
>> While the OF system is very powerful and flexible I still think that=

>> one should be able to use native SPI methods too. Why can they not
>> co-exist?
>
>I strongly believe that they can. You just need to post patches
>so that we could see them, and maybe discuss how to do things
>more generic. But if making things generic will be too hard (see
>below), I don't think anybody will object applying a non-generic
>solution (even permitting "legacy" bindings in the spi_8xxx driver).
>
>But any users of the legacy bindings should be in-tree.

ehh, it was working until you made it OF only. Why do call the native
way legacy?  It is the method all non OF arch uses.

>
>> > In the device tree it'll look like this:
>>
>> I must admit that I just started to look at the GPIO subsystem, but
>> I do wonder if mpc83xx_spi_cs_control() can do this muxing
>> without any change.
>>
>> Very good example BTW.
>
>Well, there is one caveat: the "GPIOs" aren't independent,
>so thinking about it more, I believe we should now discuss
>"chip-select framework", not gpio controllers (it could be
>that using gpio controllers for this purpose wasn't that
>good idea).
>
>http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg34738.htm=
l
>^^^ I'm dreaming about this framework. I.e. true addressing
>    for chip-selects. :-)

This is probably needed to support most SPI users out there, but until
such framework is in place I think the native methods need to stay, rig=
ht?
As is now, SPI has regressed w.r.t earlier releases.

 Jocke
BTW, I will be offline for a few days as of now.

>
>--
>Anton Vorontsov
>email: cbouatmailru@gmail.com
>irc://irc.freenode.net/bd2=

Re: Ang: Re: [PATCH 0/2] Setting GPIOs simultaneously

[Anton Vorontsov]

On Tue, Jul 14, 2009 at 11:20:13PM +0200, Joakim Tjernlund wrote:
[...]
> >But any users of the legacy bindings should be in-tree.
> 
> ehh, it was working until you made it OF only. Why do call the native
> way legacy?  It is the method all non OF arch uses.

It's legacy because there are no in-tree users anymore. Nowadays
we're trying to pass all needed information via OF, and we're
trying to avoid ugly platform-dependant hacks. Your SPI scheme
can be easily described via OF, but sure, it's hard to implement
it with the current SPI/OF subsystem.

[...]
> >http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg34738.html
> >^^^ I'm dreaming about this framework. I.e. true addressing
> >    for chip-selects. :-)
> 
> This is probably needed to support most SPI users out there, but until
> such framework is in place I think the native methods need to stay, right?

I'm not the right person to ask. I can only express my opinions.
The maintainer make final decision.

But if you ask for my opinion, I don't think that they should stay
unless we'll see a user in the mainline.

> As is now, SPI has regressed w.r.t earlier releases.

Yes and no. Yes, it has "regressed" for out-of-tree code, and no,
I don't feel sorry about that. :-)

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: Ang: Re: [PATCH 0/2] Setting GPIOs simultaneously

[Joakim Tjernlund]

Anton Vorontsov <avorontsov@ru.mvista.com> wrote on 15/07/2009 00:09:31:
>
> On Tue, Jul 14, 2009 at 11:20:13PM +0200, Joakim Tjernlund wrote:
> [...]
> > >But any users of the legacy bindings should be in-tree.
> >
> > ehh, it was working until you made it OF only. Why do call the native
> > way legacy?  It is the method all non OF arch uses.
>
> It's legacy because there are no in-tree users anymore. Nowadays
> we're trying to pass all needed information via OF, and we're
> trying to avoid ugly platform-dependant hacks. Your SPI scheme
> can be easily described via OF, but sure, it's hard to implement
> it with the current SPI/OF subsystem.

Sorry for the long delay, I have been on vaction and been busy with other stuff.

Tell me, how does this new OF SPI device scheme work out if you want
to use the bitbanged SPI driver?

>
> [...]
> > >http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg34738.html
> > >^^^ I'm dreaming about this framework. I.e. true addressing
> > >    for chip-selects. :-)
> >
> > This is probably needed to support most SPI users out there, but until
> > such framework is in place I think the native methods need to stay, right?
>
> I'm not the right person to ask. I can only express my opinions.
> The maintainer make final decision.
>
> But if you ask for my opinion, I don't think that they should stay
> unless we'll see a user in the mainline.
>
> > As is now, SPI has regressed w.r.t earlier releases.
>
> Yes and no. Yes, it has "regressed" for out-of-tree code, and no,
> I don't feel sorry about that. :-)

That is a Yes and some bla bla so the answer is yes.

   Jocke

Re: Ang: Re: [PATCH 0/2] Setting GPIOs simultaneously

[Anton Vorontsov]

On Tue, Aug 04, 2009 at 03:38:40PM +0200, Joakim Tjernlund wrote:
> Anton Vorontsov <avorontsov@ru.mvista.com> wrote on 15/07/2009 00:09:31:
> >
> > On Tue, Jul 14, 2009 at 11:20:13PM +0200, Joakim Tjernlund wrote:
> > [...]
> > > >But any users of the legacy bindings should be in-tree.
> > >
> > > ehh, it was working until you made it OF only. Why do call the native
> > > way legacy?  It is the method all non OF arch uses.
> >
> > It's legacy because there are no in-tree users anymore. Nowadays
> > we're trying to pass all needed information via OF, and we're
> > trying to avoid ugly platform-dependant hacks. Your SPI scheme
> > can be easily described via OF, but sure, it's hard to implement
> > it with the current SPI/OF subsystem.
> 
> Sorry for the long delay, I have been on vaction and been busy with other stuff.
> 
> Tell me, how does this new OF SPI device scheme work out if you want
> to use the bitbanged SPI driver?

spi-controller {
	...
	compatible = "generic-bitbang-spi";
	gpios = <GPIO_CLK GPIO_MOSI GPIO_MISO>;
	spi,chip-select-machines = <&gpio_csm &board_csm>;

	gpio_csm: chip-select-machine {
		#address-cells = <1>;
		#size-cells = <0>;
		compatible = "generic-gpio-csm";
		gpios = <GPIO_CS1 GPIO_CS2>;

		touchscreen@0 {
			reg = <0>; /* using GPIO CS1 */
		};

		eeprom@1 {
			reg = <1>; /* using GPIO CS2 */
		};
	};
};

localbus {
	...
	board_csm: chip-select-machine {
		#address-cells = <1>;
		#size-cells = <0>;
		compatibe = "board-specific-csm";

		light-sensor@0 {
			reg = <0>; /* using board-specific "CS3" */
		};

		battery@1 {
			reg = <1>; /* using board-specific "CS4" */
		};
	};
};

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: Ang: Re: [PATCH 0/2] Setting GPIOs simultaneously

[Joakim Tjernlund]

Anton Vorontsov <avorontsov@ru.mvista.com> wrote on 04/08/2009 16:22:50:
>
> On Tue, Aug 04, 2009 at 03:38:40PM +0200, Joakim Tjernlund wrote:
> > Anton Vorontsov <avorontsov@ru.mvista.com> wrote on 15/07/2009 00:09:31:
> > >
> > > On Tue, Jul 14, 2009 at 11:20:13PM +0200, Joakim Tjernlund wrote:
> > > [...]
> > > > >But any users of the legacy bindings should be in-tree.
> > > >
> > > > ehh, it was working until you made it OF only. Why do call the native
> > > > way legacy?  It is the method all non OF arch uses.
> > >
> > > It's legacy because there are no in-tree users anymore. Nowadays
> > > we're trying to pass all needed information via OF, and we're
> > > trying to avoid ugly platform-dependant hacks. Your SPI scheme
> > > can be easily described via OF, but sure, it's hard to implement
> > > it with the current SPI/OF subsystem.
> >
> > Sorry for the long delay, I have been on vaction and been busy with other stuff.
> >
> > Tell me, how does this new OF SPI device scheme work out if you want
> > to use the bitbanged SPI driver?
>
> spi-controller {
>    ...
>    compatible = "generic-bitbang-spi";
>    gpios = <GPIO_CLK GPIO_MOSI GPIO_MISO>;
>    spi,chip-select-machines = <&gpio_csm &board_csm>;
>
>    gpio_csm: chip-select-machine {
>       #address-cells = <1>;
>       #size-cells = <0>;
>       compatible = "generic-gpio-csm";
>       gpios = <GPIO_CS1 GPIO_CS2>;
>
>       touchscreen@0 {
>          reg = <0>; /* using GPIO CS1 */
>       };
>
>       eeprom@1 {
>          reg = <1>; /* using GPIO CS2 */
>       };
>    };
> };
>
> localbus {
>    ...
>    board_csm: chip-select-machine {
>       #address-cells = <1>;
>       #size-cells = <0>;
>       compatibe = "board-specific-csm";
>
>       light-sensor@0 {
>          reg = <0>; /* using board-specific "CS3" */
>       };
>
>       battery@1 {
>          reg = <1>; /* using board-specific "CS4" */
>       };
>    };
> };

hmm, this looks promising. Can I write my own board-specific-csm
and attach it to the SPI CS engine? I don't quite
se how, the 8xxx SPI driver only looks for gpio functions.
Perhaps this is something that you are working on?

       Jocke

Re: Ang: Re: [PATCH 0/2] Setting GPIOs simultaneously

[Anton Vorontsov]

On Tue, Aug 04, 2009 at 04:59:13PM +0200, Joakim Tjernlund wrote:
[...]
> hmm, this looks promising. Can I write my own board-specific-csm
> and attach it to the SPI CS engine?

Yes, you'd able to do it, but there is no such thing as SPI CS
engine, yet.

See the two options of how we can implement it:
http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg34738.html

Grant said that he like option "2.", but also said that we might
want to modify SPI requests (messages) before sending the data to
the outside world. I think that the option "1." fits better for
this task. Which means that implementation should be quite
trivial -- just another spi master driver.

> [...] the 8xxx SPI driver only looks for gpio functions.

Yes, and we should consider removing the gpio stuff too.

> Perhaps this is something that you are working on?

No, I don't need the feature right now (i.e. no board that I'm
working on needs this). But I know how to implement it (see the
link above).

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 0/2] Setting GPIOs simultaneously

[Joakim Tjernlund]

Joakim Tjernlund/Transmode wrote on 13/07/2009 18:01:02:
>
> Anton Vorontsov <avorontsov@ru.mvista.com> wrote on 13/07/2009 17:19:11:
> >
> > Hi all,
> >
> > I've been sitting on these patches for some time, but now it appears
> > that the set_sync() feature is needed elsewhere. So here are the
> > patches.
> >
> > Joakim, I think this is what you need.

> Yes, it sure looks so :) I will have to look closer later as
> I will be traveling the next few days.

hmm, one must define a new OF syntax (and update the spi_mpc83xx driver) to
use this(I think). Have you given this any thought?

 Jocke

Re: [PATCH 0/2] Setting GPIOs simultaneously

[Kumar Gala]

On Jul 13, 2009, at 10:19 AM, Anton Vorontsov wrote:

> Hi all,
>
> I've been sitting on these patches for some time, but now it appears
> that the set_sync() feature is needed elsewhere. So here are the
> patches.
>
> Joakim, I think this is what you need.
>
> Thanks,

What this resolved or dropped?

- k

Re: [PATCH 0/2] Setting GPIOs simultaneously

[Anton Vorontsov]

On Thu, Nov 05, 2009 at 08:49:38AM -0600, Kumar Gala wrote:
> 
> On Jul 13, 2009, at 10:19 AM, Anton Vorontsov wrote:
> 
> >Hi all,
> >
> >I've been sitting on these patches for some time, but now it appears
> >that the set_sync() feature is needed elsewhere. So here are the
> >patches.
> >
> >Joakim, I think this is what you need.
> >
> >Thanks,
> 
> What this resolved or dropped?

That was something like an RFC. There are no mainline users for
this support (yet), so we can drop it for now.

Thanks,

p.s.

As David didn't comment on the patches, I'd assume there are no
strong objections against this feature if we'll want to mainline
the support in future. ;-)

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2