| Message ID | 20160830091805.GL1041@n2100.armlinux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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.
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.
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?
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.
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.
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.
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
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.
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
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.
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
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 --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)
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?