Re: [PATCH 05/33] gpio: add generic single-register fixed-direction GPIO driver
On Thu, Sep 01, 2016 at 11:58:28PM +0200, Robert Jarzmik wrote: > Russell King - ARM Linux writes: > > On Thu, Sep 01, 2016 at 09:19:13AM +0200, Robert Jarzmik wrote: > > It looks like: > > > > (a) pcmcia_probe() in drivers/pcmcia/sa_generic.c doesn't check the > > return value from the platform specific init functions, meaning if > > they fail, the driver still binds. (note: they return -ENODEV to > > indicate that they should skip to the next platform.) > You're right, I submitted a patch for that, and I confirm it actually happens > on > lubbock. That'll work fine for lubbock, but not the others (we can have several of the others enabled on sa11x0 platforms - eg, badge4 with neponset.) > > (b) there is no clock provided for the sa pcmcia device (aka "1800"). > > This should be the same clock as pxa2xx-pcmcia. > Again right in the spot. > I added temporarily a clock until I have a more complete understanding in > lubbock.c : > + clk_add_alias(NULL, "1800", "SA_CLK", NULL); > > With this, things look way better : > [1.507480] pcmcia_socket pcmcia_socket1: pccard: PCMCIA card inserted > into slot 1 Yay! > I'm still investigating the new message errors: > [0.479157] genirq: Setting trigger mode 3 for irq 387 failed > (sa_type_highirq+0x0/0x6c) > [0.488213] genirq: Setting trigger mode 3 for irq 389 failed > (sa_type_highirq+0x0/0x6c) > [0.507449] genirq: Setting trigger mode 3 for irq 388 failed > (sa_type_highirq+0x0/0x6c) > [0.516492] genirq: Setting trigger mode 3 for irq 390 failed > (sa_type_highirq+0x0/0x6c) Ignore those for now - the old ARM IRQ stuff was silent on that, but genirq is more noisy. I should probably make the sa irqchip handle the both- edge case itself. > Moreover, I have a bit of homework as I also see : > - no SA interrupts at all, especially nothing when I insert/remove my >CompactFlash card >This might be an effect of pxa_cplds_irqs.c I created, I must have a look. Do you get other SA interrupts (eg, the PS/2 interrupts) delivered? > - cat /sys/class/pcmcia_socket/pcmcia_socket1/cis >cat: read error: Input/output error >That will cost me a review of the memory timings registers MCIO1/MECR/xxx, >the power lines, etc ... Hmm, on Neponset with a CF card inserted, I can cat that, and it reports the CIS. Any error messages in the kernel log? > - cat /sys/class/pcmcia_socket/pcmcia_socket1/status > slot : 1 > status : SS_READY SS_DETECT SS_POWERON SS_3VCARD > csc_mask : SS_DETECT > cs_flags : SS_OUTPUT_ENA > Vcc : 33 > Vpp : 33 > IRQ : 0 (386) That looks hopeful, but the IRQ hasn't been properly assigned (probably because no driver has bound to the card.) > >> As your gpios are contiguous (0 .. 31), why an array instead of a simple > >> offset > >> so that your translation is only a linear irq = gpio + offset ? > > > > There isn't a linear translation here: > ...zip... > > MST_PCMCIA_nCD => MAINSTONE_S0_CD_IRQ or MAINSTONE_S1_CD_IRQ > > MST_PCMCIA_nSTSCHG_BVD1 => MAINSTONE_S0_STSCHG_IRQ or > > MAINSTONE_S1_STSCHG_IRQ > > MST_PCMCIA_nIRQ => MAINSTONE_S0_IRQ or MAINSTONE_S1_IRQ > > > > So they aren't linear, and every "gpio" doesn't have a corresponding > > interrupt. > Ah yes, too bad, it would have been so much simpler. Indeed, but a tabular approach isn't that painful, especially if we also insist on knowing an irqdomain as well, which relieves us of having to know absolute interrupt numbers, which may end up being dynamically allocated eventually. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ Linux PCMCIA reimplementation list http://lists.infradead.org/mailman/listinfo/linux-pcmcia
Re: [PATCH 05/33] gpio: add generic single-register fixed-direction GPIO driver
Russell King - ARM Linux writes: > On Thu, Sep 01, 2016 at 09:19:13AM +0200, Robert Jarzmik wrote: > It looks like: > > (a) pcmcia_probe() in drivers/pcmcia/sa_generic.c doesn't check the > return value from the platform specific init functions, meaning if > they fail, the driver still binds. (note: they return -ENODEV to > indicate that they should skip to the next platform.) You're right, I submitted a patch for that, and I confirm it actually happens on lubbock. > (b) there is no clock provided for the sa pcmcia device (aka "1800"). > This should be the same clock as pxa2xx-pcmcia. Again right in the spot. I added temporarily a clock until I have a more complete understanding in lubbock.c : + clk_add_alias(NULL, "1800", "SA_CLK", NULL); With this, things look way better : [1.507480] pcmcia_socket pcmcia_socket1: pccard: PCMCIA card inserted into slot 1 I'm still investigating the new message errors: [0.479157] genirq: Setting trigger mode 3 for irq 387 failed (sa_type_highirq+0x0/0x6c) [0.488213] genirq: Setting trigger mode 3 for irq 389 failed (sa_type_highirq+0x0/0x6c) [0.507449] genirq: Setting trigger mode 3 for irq 388 failed (sa_type_highirq+0x0/0x6c) [0.516492] genirq: Setting trigger mode 3 for irq 390 failed (sa_type_highirq+0x0/0x6c) Moreover, I have a bit of homework as I also see : - no SA interrupts at all, especially nothing when I insert/remove my CompactFlash card This might be an effect of pxa_cplds_irqs.c I created, I must have a look. - cat /sys/class/pcmcia_socket/pcmcia_socket1/cis cat: read error: Input/output error That will cost me a review of the memory timings registers MCIO1/MECR/xxx, the power lines, etc ... - cat /sys/class/pcmcia_socket/pcmcia_socket1/status slot : 1 status : SS_READY SS_DETECT SS_POWERON SS_3VCARD csc_mask : SS_DETECT cs_flags : SS_OUTPUT_ENA Vcc : 33 Vpp : 33 IRQ : 0 (386) >> As your gpios are contiguous (0 .. 31), why an array instead of a simple >> offset >> so that your translation is only a linear irq = gpio + offset ? > > There isn't a linear translation here: ...zip... > MST_PCMCIA_nCD=> MAINSTONE_S0_CD_IRQ or MAINSTONE_S1_CD_IRQ > MST_PCMCIA_nSTSCHG_BVD1 => MAINSTONE_S0_STSCHG_IRQ or MAINSTONE_S1_STSCHG_IRQ > MST_PCMCIA_nIRQ => MAINSTONE_S0_IRQ or MAINSTONE_S1_IRQ > > So they aren't linear, and every "gpio" doesn't have a corresponding > interrupt. Ah yes, too bad, it would have been so much simpler. Cheers. -- Robert ___ Linux PCMCIA reimplementation list http://lists.infradead.org/mailman/listinfo/linux-pcmcia
[PATCH] pcmcia: sa1111: propagate error code
Propagate board initialization return code upwards, so that pcmcia_probe() can report a failure if occurs down the call chain. Signed-off-by: Robert Jarzmik --- drivers/pcmcia/sa_generic.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/pcmcia/sa_generic.c b/drivers/pcmcia/sa_generic.c index a1531feb8460..16b382256ace 100644 --- a/drivers/pcmcia/sa_generic.c +++ b/drivers/pcmcia/sa_generic.c @@ -204,18 +204,18 @@ static int pcmcia_probe(struct sa_dev *dev) sa_writel(PCCR_S0_FLT | PCCR_S1_FLT, base + PCCR); #ifdef CONFIG_SA1100_BADGE4 - pcmcia_badge4_init(dev); + ret = pcmcia_badge4_init(dev); #endif #ifdef CONFIG_SA1100_JORNADA720 - pcmcia_jornada720_init(dev); + ret = pcmcia_jornada720_init(dev); #endif #ifdef CONFIG_ARCH_LUBBOCK - pcmcia_lubbock_init(dev); + ret = pcmcia_lubbock_init(dev); #endif #ifdef CONFIG_ASSABET_NEPONSET - pcmcia_neponset_init(dev); + ret = pcmcia_neponset_init(dev); #endif - return 0; + return ret; } static int pcmcia_remove(struct sa_dev *dev) -- 2.1.4 ___ Linux PCMCIA reimplementation list http://lists.infradead.org/mailman/listinfo/linux-pcmcia
Re: [RFC PATCH 00/33] SA11x0/PXA GPIO rework (Core + PCMCIA only)
On Tue, Aug 30, 2016 at 11:31:58PM +0200, Linus Walleij wrote: > For all the GPIO patches: > Acked-by: Linus Walleij > > With the smallish changes needed to patch 5/33 that one is acked too. > > In fact I would ACK it anyway, because the net total cleanup is so > nice on the kernel at large... SA11x0 has never been prettier than > after this series. Thanks, ack applied to all the gpio patches. > I suspect you want to keep the series together and queue it in the ARM > tree? Else tell me what to apply in the GPIO tree. Yes, I need to keep the series together because of the dependencies - trying to split it up will lead to conflicts and breakage from missing include files, and when it's not from missing include files, it'll be non-functional. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ Linux PCMCIA reimplementation list http://lists.infradead.org/mailman/listinfo/linux-pcmcia
Re: [PATCH 05/33] gpio: add generic single-register fixed-direction GPIO driver
On Thu, Sep 01, 2016 at 09:19:13AM +0200, Robert Jarzmik wrote: > The result today is that my PCMCIA card inserted in Lubbock's slot is not > detected, but that's not really a surprise. As a matter of fact, I didn't > look much into it, the only data points I have are : > - PCMCIA is probed now >- this is based on dmesg in [3] >- this is also based on ls /sys/bus/sa-rab/devices/1800 > lrwxrwxrwx1 root root 0 Jan 1 00:15 driver -> > ../../../../bus/sa-rab/drivers/sa-pcmcia > drwxr-xr-x2 root root 0 Jan 1 00:15 power > lrwxrwxrwx1 root root 0 Jan 1 00:15 subsystem -> > ../../../../bus/sa-rab > -rw-r--r--1 root root 4096 Jan 1 00:15 uevent > > - PCMCIA sockets are not there >- this is based on ls /sys/class/pcmcia_socket/ where there is nothing > > - MAX16xx gpio are not claimed, which is surprising >- this is based on cat /sys/kernel/debug/gpio > gpiochip0: GPIOs 0-84, gpio-pxa: > gpiochip2: GPIOs 478-495, parent: platform/sa, sa: > gpiochip1: GPIOs 496-511, lubbock: It looks like: (a) pcmcia_probe() in drivers/pcmcia/sa_generic.c doesn't check the return value from the platform specific init functions, meaning if they fail, the driver still binds. (note: they return -ENODEV to indicate that they should skip to the next platform.) (b) there is no clock provided for the sa pcmcia device (aka "1800"). This should be the same clock as pxa2xx-pcmcia. > > 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. > > Normally these interrupts are already dealt with by > arch/arm/mach-pxa/pxa_cplds_irq.c, which adds an irqdomain for them. The > missing > part is the "gpio to interrupt" translation part, right ? This is where your > array comes into play if I get your right, to be the input of the to_irq() > function of the gpiochip. Yes. > As your gpios are contiguous (0 .. 31), why an array instead of a simple > offset > so that your translation is only a linear irq = gpio + offset ? There isn't a linear translation here: #define MST_PCMCIA_nIRQ (1 << 10) /* IRQ / ready signal */ #define MST_PCMCIA_nSPKR_BVD2 (1 << 9) /* VDD sense / digital speaker */ #define MST_PCMCIA_nSTSCHG_BVD1 (1 << 8) /* VDD sense / card status changed */#define MST_PCMCIA_nVS2 (1 << 7) /* VSS voltage sense */ #define MST_PCMCIA_nVS1 (1 << 6) /* VSS voltage sense */ #define MST_PCMCIA_nCD (1 << 5) /* Card detection signal */ #define MST_PCMCIA_RESET(1 << 4) /* Card reset signal */ #define MST_PCMCIA_PWR_MASK (0x000f) /* MAX1602 power-supply controls */ #define MAINSTONE_S0_CD_IRQ MAINSTONE_IRQ(9) #define MAINSTONE_S0_STSCHG_IRQ MAINSTONE_IRQ(10) #define MAINSTONE_S0_IRQMAINSTONE_IRQ(11) #define MAINSTONE_S1_CD_IRQ MAINSTONE_IRQ(13) #define MAINSTONE_S1_STSCHG_IRQ MAINSTONE_IRQ(14) #define MAINSTONE_S1_IRQMAINSTONE_IRQ(15) MST_PCMCIA_nCD => MAINSTONE_S0_CD_IRQ or MAINSTONE_S1_CD_IRQ MST_PCMCIA_nSTSCHG_BVD1 => MAINSTONE_S0_STSCHG_IRQ or MAINSTONE_S1_STSCHG_IRQ MST_PCMCIA_nIRQ => MAINSTONE_S0_IRQ or MAINSTONE_S1_IRQ So they aren't linear, and every "gpio" doesn't have a corresponding interrupt. > [1] Typo fix > diff --git a/arch/arm/mach-pxa/lubbock.c b/arch/arm/mach-pxa/lubbock.c > index f034928b99a1..81a1de6fb46f 100644 > --- a/arch/arm/mach-pxa/lubbock.c > +++ b/arch/arm/mach-pxa/lubbock.c > @@ -13,7 +13,7 @@ > */ > #include > #include > -#include > +#include > #include > #include > #include Fixed. > [2] Kind-of fix for lubbock pcmcia probe > >From 977c16201a752aac8a8fb2da1f4271795f0b2122 Mon Sep 17 00:00:00 2001 > From: Robert Jarzmik > Date: Thu, 1 Sep 2016 08:31:08 +0200 > Subject: [PATCH] pcmcia: lubbock: fix sockets configuration > > On lubbock board, the probe of the driver crashes by dereferencing very > early a platform_data structure which is not set, in > pxa2xx_configure_sockets(). > > This patch blindly fixes it without any analysis as to know if it's the > right fix or even if the fix doesn't break in suspend/resume. > > The stack fixed is : > [0.244353] SA Microprocessor Companion Chip: silicon revision 1, > metal revision 1 > [0.256321] sa sa: Providing IRQ336-390 > [0.340899] clocksource: Switched to clocksource oscr0 > [0.472263] Unable to handle kernel NULL pointer dereference at virtual > address 0004 > [0.480469] pgd = c0004000 > [0.483432] [0004] *pgd= > [0.487105] Internal error: Oops: f5 [#1] ARM > [0.491497] Modules lin
Re: [PATCH 05/33] gpio: add generic single-register fixed-direction GPIO driver
Russell King - ARM Linux 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 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. Ok Russell, I run a first mini-test. I used : - one trivial typo fix of your patches in [1] - one "make it better fix" in [2] to pass the pcmcia probe, without much thinking about it, just to have the probe finished The result today is that my PCMCIA card inserted in Lubbock's slot is not detected, but that's not really a surprise. As a matter of fact, I didn't look much into it, the only data points I have are : - PCMCIA is probed now - this is based on dmesg in [3] - this is also based on ls /sys/bus/sa-rab/devices/1800 lrwxrwxrwx1 root root 0 Jan 1 00:15 driver -> ../../../../bus/sa-rab/drivers/sa-pcmcia drwxr-xr-x2 root root 0 Jan 1 00:15 power lrwxrwxrwx1 root root 0 Jan 1 00:15 subsystem -> ../../../../bus/sa-rab -rw-r--r--1 root root 4096 Jan 1 00:15 uevent - PCMCIA sockets are not there - this is based on ls /sys/class/pcmcia_socket/ where there is nothing - MAX16xx gpio are not claimed, which is surprising - this is based on cat /sys/kernel/debug/gpio gpiochip0: GPIOs 0-84, gpio-pxa: gpiochip2: GPIOs 478-495, parent: platform/sa, sa: gpiochip1: GPIOs 496-511, lubbock: I'm not familiar with drivers/pcmcia part of the kernel tree so I won't be fast to go down to the problem, but I'll try in the next days. Maybe if I activate debug logs in pcmcia related parts I'll find out more quickly what is happening. > Maybe we can look at converting mainstone as well? Yep. > 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. Yes. > 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. Normally these interrupts are already dealt with by arch/arm/mach-pxa/pxa_cplds_irq.c, which adds an irqdomain for them. The missing part is the "gpio to interrupt" translation part, right ? This is where your array comes into play if I get your right, to be the input of the to_irq() function of the gpiochip. As your gpios are contiguous (0 .. 31), why an array instead of a simple offset so that your translation is only a linear irq = gpio + offset ? > 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. Once PCMCIA is over, sure. Cheers. -- Robert [1] Typo fix diff --git a/arch/arm/mach-pxa/lubbock.c b/arch/arm/mach-pxa/lubbock.c index f034928b99a1..81a1de6fb46f 100644 --- a/arch/arm/mach-pxa/lubbock.c +++ b/arch/arm/mach-pxa/lubbock.c @@ -13,7 +13,7 @@ */ #include #include -#include +#include #include #include #include [2] Kind-of fix for lubbock pcmcia probe From 977c16201a752aac8a8fb2da1f4271795f0b2122 Mon Sep 17 00:00:00 2001 From: Robert Jarzmik Date: Thu, 1 Sep 2016 08:31:08 +0200 Subject: [PATCH] pcmcia: lubbock: fix sockets configuration On lubbock board, the probe of the driver crashes by dereferencing very early a platform_data structure which is not set, in pxa2xx_configure_sockets(). This patch blindly fixes it without any analysis as to know if it's the right fix or even if the fix doesn't break in suspend/resume. The stack fixed is : [0.244353] SA Microprocessor Companion Chip: silicon revision 1, metal revision 1 [0.256321] sa sa: Providing IRQ336-390 [0.340899] clocksource: Switched to clocksource oscr0 [0.472263] Unable to handle kernel NULL pointer dereference at virtual address 0004 [0.480469] pgd = c0004000 [0.483432] [0004] *pgd= [0.487105] Internal error: Oops: f5 [#1] ARM [0.491497] Modules linked in: [0.494650] CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-rc3-00080-g1aaa68426f0c-dirty #2068 [0.503229] Hardware name: Intel DBPXA250 Development Platform (aka Lubbock) [0.510344] task: c3e42000 task.stack: c3
