diff mbox

[05/33] gpio: add generic single-register fixed-direction GPIO driver

Message ID 20160830091805.GL1041@n2100.armlinux.org.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Russell King (Oracle) Aug. 30, 2016, 9:18 a.m. UTC
On Mon, Aug 29, 2016 at 09:39:54PM +0200, Robert Jarzmik wrote:
> Maybe this one would deserve a doxygen comment ?

Does this solve it?

Comments

Robert Jarzmik Aug. 30, 2016, 4:42 p.m. UTC | #1
Russell King - ARM Linux <linux@armlinux.org.uk> writes:

> On Mon, Aug 29, 2016 at 09:39:54PM +0200, Robert Jarzmik wrote:
>> Maybe this one would deserve a doxygen comment ?
>
> Does this solve it?
Oh yes, that's very nice, especially the "1 = in" for which I have always a
doubt.

Reviewed-by: Robert Jarzmik <robert.jarzmik@free.fr>

I hope to have an "attempt" to test all your series on the lubbock board in the
next days. The "attempt" part is because last time I checked pcmcia support was
broken on lubbock (ie. CONFIG_PCMCIA_SA1111 activated implies an Oops), and I
don't remember having repaired it (and it's disabled on my test farm which is a
sign I was not very confident about this part).

If you happen to have somewhere a branch I can pull, with all your series (at
least "[RFC PATCH 00/33] SA11x0/PXA GPIO rework (Core + PCMCIA only)" and
"[PATCH 0/8] SA11x0/PXA remainder & cleanups"), that would spare me the multiple
git-am to make the test(s).

Cheers.

--
Robert
Russell King (Oracle) Aug. 30, 2016, 6:46 p.m. UTC | #2
On Tue, Aug 30, 2016 at 06:42:03PM +0200, Robert Jarzmik wrote:
> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> 
> > On Mon, Aug 29, 2016 at 09:39:54PM +0200, Robert Jarzmik wrote:
> >> Maybe this one would deserve a doxygen comment ?
> >
> > Does this solve it?
> Oh yes, that's very nice, especially the "1 = in" for which I have always a
> doubt.
> 
> Reviewed-by: Robert Jarzmik <robert.jarzmik@free.fr>
> 
> I hope to have an "attempt" to test all your series on the lubbock board in the
> next days. The "attempt" part is because last time I checked pcmcia support was
> broken on lubbock (ie. CONFIG_PCMCIA_SA1111 activated implies an Oops), and I
> don't remember having repaired it (and it's disabled on my test farm which is a
> sign I was not very confident about this part).
> 
> If you happen to have somewhere a branch I can pull, with all your series (at
> least "[RFC PATCH 00/33] SA11x0/PXA GPIO rework (Core + PCMCIA only)" and
> "[PATCH 0/8] SA11x0/PXA remainder & cleanups"), that would spare me the multiple
> git-am to make the test(s).

If you can wait a day or two, I'll push a branch out for everything in
all these multiple series.

Thanks.
Robert Jarzmik Aug. 30, 2016, 9:32 p.m. UTC | #3
Russell King - ARM Linux <linux@armlinux.org.uk> writes:

> If you can wait a day or two, I'll push a branch out for everything in
> all these multiple series.
Sure, just ping me when you have something.

Cheers.
Russell King (Oracle) Aug. 31, 2016, 8:49 a.m. UTC | #4
On Tue, Aug 30, 2016 at 11:32:16PM +0200, Robert Jarzmik wrote:
> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> 
> > If you can wait a day or two, I'll push a branch out for everything in
> > all these multiple series.
> Sure, just ping me when you have something.

git://git.armlinux.org.uk/~rmk/linux-arm.git sa1100

should get you something suitable to test.  It's based on 4.7-rc3 plus
my fixes branch.

It would be great to have this tested on Lubbock, and get the PCMCIA
issues fixed.  Maybe we can look at converting mainstone as well?
Russell King (Oracle) Aug. 31, 2016, 10:27 a.m. UTC | #5
On Wed, Aug 31, 2016 at 09:49:38AM +0100, Russell King - ARM Linux wrote:
> On Tue, Aug 30, 2016 at 11:32:16PM +0200, Robert Jarzmik wrote:
> > Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> > 
> > > If you can wait a day or two, I'll push a branch out for everything in
> > > all these multiple series.
> > Sure, just ping me when you have something.
> 
> git://git.armlinux.org.uk/~rmk/linux-arm.git sa1100
> 
> should get you something suitable to test.  It's based on 4.7-rc3 plus
> my fixes branch.
> 
> It would be great to have this tested on Lubbock, and get the PCMCIA
> issues fixed.  Maybe we can look at converting mainstone as well?

Yes, looking at mainstone's PCMCIA, it uses a MAX1602 device, which
is supported by the new max1600.c code.

#define MST_PCMCIA_PWR_VPP_0    0x0        /* voltage VPP = 0V */
#define MST_PCMCIA_PWR_VPP_120  0x2        /* voltage VPP = 12V*/
#define MST_PCMCIA_PWR_VPP_VCC  0x1        /* voltage VPP = VCC */
#define MST_PCMCIA_PWR_VCC_0    0x0        /* voltage VCC = 0V */
#define MST_PCMCIA_PWR_VCC_33   0x8        /* voltage VCC = 3.3V */
#define MST_PCMCIA_PWR_VCC_50   0x4        /* voltage VCC = 5.0V */

This follows the Cirrus code (also used by Lubbock.)  So, if we represent
the MST_PCMCIA[01] registers as GPIOs, we can switch pxa2xx_mainstone.c
to use the max1600.c code for power control.

With the other signals in MST_PCMCIA[01] represented as GPIOs (we'd
need to add the VS* to soc_common), we'd then have those read by
generic code, which means mst_pcmcia_socket_state() becomes just the
hack for STSCHG.

With gpio-reg, we can represent these registers easily as GPIOs, eg:

	gc = gpio_reg_init(NULL, (void __iomem *)&MST_PCMCIA0,
			   -1, 11, "mst-pcmcia0", ~MST_PCMCIA_PWR_MASK, 0,
			   mst_pcmcia_names);

There is a slight issue, which is that the interrupts can't be
translated to an interrupt by gpio-reg, which will currently cause
soc_common problems - but that's an easy fix, though leaves us with
more code than I'd desire in pxa2xx_mainstone.c.  Maybe a solution
there would be to have gpio-reg also take an array of interrupt
numbers... not sure yet.


For IrDA, it looks like it has the same transceiver as the assabet, so
I've (already) patches to split out the gpio-based transceiver control
from sa1100_ir - maybe we can re-use that in pxaficp_ir too.
Russell King (Oracle) Sept. 3, 2016, 10:25 a.m. UTC | #6
On Wed, Aug 31, 2016 at 09:49:38AM +0100, Russell King - ARM Linux wrote:
> On Tue, Aug 30, 2016 at 11:32:16PM +0200, Robert Jarzmik wrote:
> > Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> > 
> > > If you can wait a day or two, I'll push a branch out for everything in
> > > all these multiple series.
> > Sure, just ping me when you have something.
> 
> git://git.armlinux.org.uk/~rmk/linux-arm.git sa1100
> 
> should get you something suitable to test.  It's based on 4.7-rc3 plus
> my fixes branch.

Branch updated, hopefully with everything that we've agreed on so far,
all the bug fixes, plus a few extra bits...

> It would be great to have this tested on Lubbock, and get the PCMCIA
> issues fixed.  Maybe we can look at converting mainstone as well?

... like converting mainstone and pxaficp_ir on mainstone.

I'm not going to be around for most of the rest of this weekend.
Robert Jarzmik Sept. 3, 2016, 11:31 a.m. UTC | #7
Russell King - ARM Linux <linux@armlinux.org.uk> writes:

> On Wed, Aug 31, 2016 at 09:49:38AM +0100, Russell King - ARM Linux wrote:
>> On Tue, Aug 30, 2016 at 11:32:16PM +0200, Robert Jarzmik wrote:
>> > Russell King - ARM Linux <linux@armlinux.org.uk> writes:
>> > 
>> > > If you can wait a day or two, I'll push a branch out for everything in
>> > > all these multiple series.
>> > Sure, just ping me when you have something.
>> 
>> git://git.armlinux.org.uk/~rmk/linux-arm.git sa1100
>> 
>> should get you something suitable to test.  It's based on 4.7-rc3 plus
>> my fixes branch.
>
> Branch updated, hopefully with everything that we've agreed on so far,
> all the bug fixes, plus a few extra bits...
Good, I'll retest within the weekend.

>> It would be great to have this tested on Lubbock, and get the PCMCIA
>> issues fixed.  Maybe we can look at converting mainstone as well?
>
> ... like converting mainstone and pxaficp_ir on mainstone.
>
> I'm not going to be around for most of the rest of this weekend.
Okay.

I'll repost on Monday for the latest branch test results and your idea about the
irq_get_chip() workaround.

Good week-end.
Robert Jarzmik Sept. 4, 2016, 7:04 p.m. UTC | #8
Russell King - ARM Linux <linux@armlinux.org.uk> writes:

> On Wed, Aug 31, 2016 at 09:49:38AM +0100, Russell King - ARM Linux wrote:
>> On Tue, Aug 30, 2016 at 11:32:16PM +0200, Robert Jarzmik wrote:
>> > Russell King - ARM Linux <linux@armlinux.org.uk> writes:
>> > 
>> > > If you can wait a day or two, I'll push a branch out for everything in
>> > > all these multiple series.
>> > Sure, just ping me when you have something.
>> 
>> git://git.armlinux.org.uk/~rmk/linux-arm.git sa1100
>> 
>> should get you something suitable to test.  It's based on 4.7-rc3 plus
>> my fixes branch.
>
> Branch updated, hopefully with everything that we've agreed on so far,
> all the bug fixes, plus a few extra bits...

And retested with my 3 patches on top of it. Everything works fine :
 - CF insertion is correctly detected now !
 - AT/2 keyboard interrupts fire, keys are there, etc ...
 - the AT/2 warning is now gone

The pxa_cplds_irqs patch will make it work as before, better than today for
sure. I think edge type interrupts (such as sa1111), if they come close enough,
might be lost with the current implementation, but anyway, that's for another
time.

There is one patch in my serie that I left up to you, as I wasn't very sure
about the accuracy of my commit message, nor been able to write down all the
requirements implied on sa1111 with this additional test.

Cheers.

--
Robert
Russell King (Oracle) Sept. 4, 2016, 8:18 p.m. UTC | #9
On Sun, Sep 04, 2016 at 09:04:59PM +0200, Robert Jarzmik wrote:
> And retested with my 3 patches on top of it. Everything works fine :
>  - CF insertion is correctly detected now !
>  - AT/2 keyboard interrupts fire, keys are there, etc ...
>  - the AT/2 warning is now gone
> 
> The pxa_cplds_irqs patch will make it work as before, better than today for
> sure. I think edge type interrupts (such as sa1111), if they come close enough,
> might be lost with the current implementation, but anyway, that's for another
> time.

Good news, but I suspect CF probably isn't working as it should do (see
my reply to the clock alias patch.)

> There is one patch in my serie that I left up to you, as I wasn't very sure
> about the accuracy of my commit message, nor been able to write down all the
> requirements implied on sa1111 with this additional test.

That's basically what I have here at the moment, but since I made the
suggestion, I've been thinking about moving it into the IRQ
initialisation instead.

Also, I think it's a genirq problem that you can install a chained
handler on an interrupt without a registered chip - and then when an
irq chip comes along later, the IRQ isn't unmasked.  We should either
not allow a chained handler to be installed in that situation, /or/
we should unmask chained IRQs when the chip comes along later.
Linus Walleij Sept. 5, 2016, 9:06 a.m. UTC | #10
On Wed, Aug 31, 2016 at 10:49 AM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:

> git://git.armlinux.org.uk/~rmk/linux-arm.git sa1100
>
> It would be great to have this tested on Lubbock, and get the PCMCIA
> issues fixed.  Maybe we can look at converting mainstone as well?

I couldn't resist testing on the Compaq iPAQ h3600. It works the
same as before so:
Tested-by: Linus Walleij <linus.walleij@linaro.org> [for Compaq iPAQ H3600]

The only news in the bootlog is this:
sa11x0-pcmcia: probe of sa11x0-pcmcia failed with error -2

This device does have a PCMCIA "sleeve" where I slotted in an
ethernet card at one time to see if I could network this thing.
But I never got the PCMCIA working. Now it seems like I could
actually start looking into that as the driver gives its first sign
of life.

I don't think PCMCIA ever worked on this thing upstream,
but I have a copy of the old linux-handheld kernel tree where
it supposedly was working at one point.

Yours,
Linus Walleij
Russell King (Oracle) Sept. 5, 2016, 12:26 p.m. UTC | #11
On Mon, Sep 05, 2016 at 11:06:28AM +0200, Linus Walleij wrote:
> I couldn't resist testing on the Compaq iPAQ h3600. It works the
> same as before so:
> Tested-by: Linus Walleij <linus.walleij@linaro.org> [for Compaq iPAQ H3600]

Great news.  I've been thinking about digging out my h3600, but it's
very old, and hasn't been turned on for many years.  I'm not sure what
state it's in.

I've been hoping to try booting some kernels with qemu-system-arm, but
so far I've completely failed to get qemu-system-arm to do anything
useful - it just sits there doing apparently nothing, irrespective of
which platform I choose or which kernel I give it.

> The only news in the bootlog is this:
> sa11x0-pcmcia: probe of sa11x0-pcmcia failed with error -2

Not so great news - that's -ENOENT.  Did that happen before these
changes?  That could be that the gpiod lookup table isn't found.
However, if that were the case, I'd have expected an error message
along the lines of:

Failed to get GPIO for xxx: -nnn

from soc_pcmcia_request_gpiods().  The other possibility is that
we're not getting to sa11x0_drv_pcmcia_legacy_probe() but instead
trying to initialise it as a generic sa11x0 socket, and
sa11x0_pcmcia_hw_init() is failing as a result.

We should be using the legacy probe on H3600, so sa11x0_pcmcia_hw_init()
should never be reached.

> This device does have a PCMCIA "sleeve" where I slotted in an
> ethernet card at one time to see if I could network this thing.
> But I never got the PCMCIA working. Now it seems like I could
> actually start looking into that as the driver gives its first sign
> of life.
> 
> I don't think PCMCIA ever worked on this thing upstream,
> but I have a copy of the old linux-handheld kernel tree where
> it supposedly was working at one point.

Well, the H3600 PCMCIA driver raises lots of "that can't be right"
questions for me, so I decided to leave most of the GPIO stuff alone
there, and just convert only the bits which made sense to me - the
detect and ready(irq) bits.
Linus Walleij Sept. 8, 2016, 1:21 p.m. UTC | #12
On Mon, Sep 5, 2016 at 2:26 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Mon, Sep 05, 2016 at 11:06:28AM +0200, Linus Walleij wrote:
>> I couldn't resist testing on the Compaq iPAQ h3600. It works the
>> same as before so:
>> Tested-by: Linus Walleij <linus.walleij@linaro.org> [for Compaq iPAQ H3600]
>
> Great news.  I've been thinking about digging out my h3600, but it's
> very old, and hasn't been turned on for many years.  I'm not sure what
> state it's in.
>
> I've been hoping to try booting some kernels with qemu-system-arm, but
> so far I've completely failed to get qemu-system-arm to do anything
> useful - it just sits there doing apparently nothing, irrespective of
> which platform I choose or which kernel I give it.
>
>> The only news in the bootlog is this:
>> sa11x0-pcmcia: probe of sa11x0-pcmcia failed with error -2
>
> Not so great news - that's -ENOENT.  Did that happen before these
> changes?  That could be that the gpiod lookup table isn't found.
> However, if that were the case, I'd have expected an error message
> along the lines of:
>
> Failed to get GPIO for xxx: -nnn
>
> from soc_pcmcia_request_gpiods().  The other possibility is that
> we're not getting to sa11x0_drv_pcmcia_legacy_probe() but instead
> trying to initialise it as a generic sa11x0 socket, and
> sa11x0_pcmcia_hw_init() is failing as a result.
>
> We should be using the legacy probe on H3600, so sa11x0_pcmcia_hw_init()
> should never be reached.

However that is what happens, this is my callstack after
adding some prints:

sa11x0_pcmcia_init
sa11x0_drv_pcmcia_probe
soc_pcmcia_init_one
soc_pcmcia_add_one
soc_pcmcia_hw_init
sa11x0_pcmcia_hw_init
soc_pcmcia_add_one: pcmcia HW init failed
sa11x0-pcmcia: probe of sa11x0-pcmcia failed with error -2

It bails out when trying to get the "reset" gpio.

I'm trying to figure out how this can happen.

On the plus side: this minor snag is all the problems I have.
All other GPIOs work exactly as they should.

Yours,
Linus Walleij
Linus Walleij Sept. 14, 2016, 8:50 a.m. UTC | #13
On Thu, Sep 8, 2016 at 3:21 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Sep 5, 2016 at 2:26 PM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
>> On Mon, Sep 05, 2016 at 11:06:28AM +0200, Linus Walleij wrote:
>>> I couldn't resist testing on the Compaq iPAQ h3600. It works the
>>> same as before so:
>>> Tested-by: Linus Walleij <linus.walleij@linaro.org> [for Compaq iPAQ H3600]
>>
>> Great news.  I've been thinking about digging out my h3600, but it's
>> very old, and hasn't been turned on for many years.  I'm not sure what
>> state it's in.
>>
>> I've been hoping to try booting some kernels with qemu-system-arm, but
>> so far I've completely failed to get qemu-system-arm to do anything
>> useful - it just sits there doing apparently nothing, irrespective of
>> which platform I choose or which kernel I give it.
>>
>>> The only news in the bootlog is this:
>>> sa11x0-pcmcia: probe of sa11x0-pcmcia failed with error -2
>>
>> Not so great news - that's -ENOENT.  Did that happen before these
>> changes?  That could be that the gpiod lookup table isn't found.
>> However, if that were the case, I'd have expected an error message
>> along the lines of:
>>
>> Failed to get GPIO for xxx: -nnn
>>
>> from soc_pcmcia_request_gpiods().  The other possibility is that
>> we're not getting to sa11x0_drv_pcmcia_legacy_probe() but instead
>> trying to initialise it as a generic sa11x0 socket, and
>> sa11x0_pcmcia_hw_init() is failing as a result.
>>
>> We should be using the legacy probe on H3600, so sa11x0_pcmcia_hw_init()
>> should never be reached.
>
> However that is what happens, this is my callstack after
> adding some prints:
>
> sa11x0_pcmcia_init
> sa11x0_drv_pcmcia_probe
> soc_pcmcia_init_one
> soc_pcmcia_add_one
> soc_pcmcia_hw_init
> sa11x0_pcmcia_hw_init
> soc_pcmcia_add_one: pcmcia HW init failed
> sa11x0-pcmcia: probe of sa11x0-pcmcia failed with error -2

Bah I found the cause, just a simple oneliner typo in one of
the patches. I will comment on the patch in question so you can
fix it up on your branch.

Yours,
Linus Walleij
diff mbox

Patch

diff --git a/drivers/gpio/gpio-reg.c b/drivers/gpio/gpio-reg.c
index fc7e0a395f9f..b91488ec122c 100644
--- a/drivers/gpio/gpio-reg.c
+++ b/drivers/gpio/gpio-reg.c
@@ -1,3 +1,12 @@ 
+/*
+ * gpio-reg: single register individually fixed-direction GPIOs
+ *
+ * Copyright (C) 2016 Russell King
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ */
 #include <linux/gpio/driver.h>
 #include <linux/gpio-reg.h>
 #include <linux/io.h>
@@ -87,6 +96,22 @@  static void gpio_reg_set_multiple(struct gpio_chip *gc, unsigned long *mask,
 	spin_unlock_irqrestore(&r->lock, flags);
 }
 
+/**
+ * gpio_reg_init - add a fixed in/out register as gpio
+ * @dev: optional struct device associated with this register
+ * @base: start gpio number, or -1 to allocate
+ * @num: number of GPIOs, maximum 32
+ * @label: GPIO chip label
+ * @direction: bitmask of fixed direction, one per GPIO signal, 1 = in
+ * @def_out: initial GPIO output value
+ * @names: array of %num strings describing each GPIO signal
+ *
+ * Add a single-register GPIO device containing up to 32 GPIO signals,
+ * where each GPIO has a fixed input or output configuration.  Only
+ * input GPIOs are assumed to be readable from the register, and only
+ * then after a double-read.  Output values are assumed not to be
+ * readable.
+ */
 struct gpio_chip *gpio_reg_init(struct device *dev, void __iomem *reg,
 	int base, int num, const char *label, u32 direction, u32 def_out,
 	const char *const *names)