Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH 0/2] Setting GPIOs simultaneously

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


avorontsov at ru

Jul 13, 2009, 8:19 AM

Post #1 of 8 (290 views)
Permalink
[PATCH 0/2] Setting GPIOs simultaneously

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 [at] gmail
irc://irc.freenode.net/bd2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


joakim.tjernlund at transmode

Jul 13, 2009, 9:01 AM

Post #2 of 8 (266 views)
Permalink
Re: [PATCH 0/2] Setting GPIOs simultaneously [In reply to]

Anton Vorontsov <avorontsov [at] ru> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


joakim.tjernlund at transmode

Jul 13, 2009, 10:17 AM

Post #3 of 8 (277 views)
Permalink
Re: [PATCH 0/2] Setting GPIOs simultaneously [In reply to]

Joakim Tjernlund/Transmode wrote on 13/07/2009 18:01:02:
>
> Anton Vorontsov <avorontsov [at] ru> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


avorontsov at ru

Jul 13, 2009, 10:34 AM

Post #4 of 8 (265 views)
Permalink
Re: [PATCH 0/2] Setting GPIOs simultaneously [In reply to]

On Mon, Jul 13, 2009 at 06:01:02PM +0200, Joakim Tjernlund wrote:
>
> Anton Vorontsov <avorontsov [at] ru> 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 [at] gmail
irc://irc.freenode.net/bd2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


joakim.tjernlund at transmode

Jul 13, 2009, 12:59 PM

Post #5 of 8 (265 views)
Permalink
Re: [PATCH 0/2] Setting GPIOs simultaneously [In reply to]

Anton Vorontsov <avorontsov [at] ru> wrote on 13/07/2009 19:34:55:
>
> On Mon, Jul 13, 2009 at 06:01:02PM +0200, Joakim Tjernlund wrote:
> >
> > Anton Vorontsov <avorontsov [at] ru> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


avorontsov at ru

Jul 13, 2009, 3:20 PM

Post #6 of 8 (261 views)
Permalink
Re: [PATCH 0/2] Setting GPIOs simultaneously [In reply to]

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 [at] lists/msg34738.html
^^^ I'm dreaming about this framework. I.e. true addressing
for chip-selects. :-)

--
Anton Vorontsov
email: cbouatmailru [at] gmail
irc://irc.freenode.net/bd2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


galak at kernel

Nov 5, 2009, 6:49 AM

Post #7 of 8 (180 views)
Permalink
Re: [PATCH 0/2] Setting GPIOs simultaneously [In reply to]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


avorontsov at ru

Nov 5, 2009, 6:58 AM

Post #8 of 8 (182 views)
Permalink
Re: [PATCH 0/2] Setting GPIOs simultaneously [In reply to]

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 [at] gmail
irc://irc.freenode.net/bd2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.