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

2016-09-14 Thread Linus Walleij
On Thu, Sep 8, 2016 at 3:21 PM, Linus Walleij  wrote:
> On Mon, Sep 5, 2016 at 2:26 PM, Russell King - ARM Linux
>  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  [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

___
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

2016-09-08 Thread Linus Walleij
On Mon, Sep 5, 2016 at 2:26 PM, Russell King - ARM Linux
 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  [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

___
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

2016-09-05 Thread Russell King - ARM Linux
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  [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.

-- 
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

2016-09-05 Thread Linus Walleij
On Wed, Aug 31, 2016 at 10:49 AM, Russell King - ARM Linux
 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  [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

___
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

2016-09-04 Thread Linus Walleij
On Fri, Sep 2, 2016 at 7:00 PM, Russell King - ARM Linux
 wrote:

> Linus,
>
> There's a change I'd like to merge into patch 5 - overall it looks
> like the below, and allows us to use gpio-reg with the PXA mainstone
> MST_PCMCIA[01] registers.  Some of these GPIO signals have hardware
> interrupts associated with them, but not all.
>
> Do you approve?

Yes of course.
Acked-by: Linus Walleij 

Yours,
Linus Walleij

___
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

2016-09-04 Thread Russell King - ARM Linux
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 sa), 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 sa 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.

-- 
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

2016-09-04 Thread Robert Jarzmik
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.
>
> 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 sa), 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 sa with this additional test.

Cheers.

--
Robert

___
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

2016-09-03 Thread Robert Jarzmik
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.
>
> 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

___
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

2016-09-03 Thread Russell King - ARM Linux
On Fri, Sep 02, 2016 at 11:21:12PM +0200, Robert Jarzmik wrote:
> It looks that I have an ordering problem :
>  - I want gpio-pxa.probe() to be called at device initcall time
>  - pxa_cplds_irqs.probe() cannot complete before gpio-pxa.probe() because it
>needs GPIO0 as its interrupt source
>=> it might need to honor -EPROBE_DEFER
>  - sa.sa_probe() cannot complete before pxa_cplds_irqs.probe() because
>it needs IRQ305 as its source
>=> it might need to honor -EPROBE_DEFER

I wonder if we can work around that by adding this to sa_probe():

if (!irq_get_chip(irq))
return -EPROBE_DEFER;

It's not particularly nice, but it should at least ensure that things
happen in the right order.

-- 
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

2016-09-03 Thread Russell King - ARM Linux
On Thu, Sep 01, 2016 at 09:19:13AM +0200, Robert Jarzmik wrote:
> >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().

Patch applied, with some fixups for the error code propagation changes
and edited the patch description a little.

Thanks.

-- 
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

2016-09-02 Thread Russell King - ARM Linux
On Fri, Sep 02, 2016 at 07:50:35PM +0200, Robert Jarzmik wrote:
> Russell King - ARM Linux  writes:
> >> 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?
> I see no other interrupts at all.
> Actually I see no interrupt claimed in /proc/interrupts, where I would have
> expected interrupt 305.
>   cat /proc/interrupts
>  CPU0   
>24:   1419SC   8 Edge  gpio-0
>25:  0SC   9 Edge  gpio-1
>26:  0SC  10 Edge  gpio-mux
>38:118SC  22 Edge  UART1
>41:  0SC  25 Edge  DMA
>42:  40224SC  26 Edge  ost0
>   112:   1419  GPIO   0 Edge  pxa_cplds_irqs
>   307:   1419  pxa_cplds   3 Edge  eth0
>   387:  0  SA-h Edge  SA PCMCIA card detect
>   388:  0  SA-h Edge  SA CF card detect
>   389:  0  SA-h Edge  SA PCMCIA BVD1
>   390:  0  SA-h Edge  SA CF BVD1
>   Err:  0
> 
> Actually this leads me to think that this interrupt 305 is not "requested" nor
> activated. I see in sa.c:506 :
>   "irq_set_chained_handler_and_data(sachip->irq, sa_irq_handler, ..."
> This puts in the handler and data, but I don't this is "enables" the 
> interrupt,
> right ?

It should enable the interrupt - the end of that does:

if (handle != handle_bad_irq && is_chained) {
irq_settings_set_noprobe(desc);
irq_settings_set_norequest(desc);
irq_settings_set_nothread(desc);
desc->action = _action;
irq_startup(desc, true);
}

and indeed, I see that it gets enabled on Assabet.  That irq_startup()
should result in the irqchip's irq_startup, irq_enable, or irq_unmask
method being called.

So, that should result in the IRQ to which the sa is connected
being unmasked.

Chained interrupts don't appear in /proc/interrupts.

> I traced the code in the interrupt controller (pxa_cplds_irqs), the SA
> physical line is not set (even with an AT/2 keyboard connected, and I don't
> think anybody tried this AT/2 on a lubbock for a long time).

Hmm.  Looking at pxa_cplds_irqs, that's trying to support the CPLD
interrupts using a very very old and inefficient technique.  The
whole point of the chained interrupt system is to avoid several
overheads in the system...  I'm curious why PXA moved away from that.

One of the problems is that we end up nesting irq_entry()/irq_exit()
which contains a lot of code, eg trying to run softirqs.  All that
stuff is pure overhead because we'll never get to run anything like
that in this path.

I'm also very concerned that the conversion is wrong.  The old code
has this comment in the irq_unmask function:

-   /* the irq can be acknowledged only if deasserted, so it's done here */
-   LUB_IRQ_SET_CLR &= ~(1 << lubbock_irq);

In other words, we "acknowledge" (really clear the latched status) of
the interrupt _after_ we've serviced the peripheral, not before.
The new code tries to clear down the interrupt when masking it - in
other words, before the peripheral handler has had a chance to clear
down its interrupt.

I suspect you're seeing about twice as many interrupt counts on your
ethernet interface because of this - you'll be entering the handler
once for the real interrupt, but because the clear-down was ineffective,
you end up re-entering it a second time when hopefully the peripheral
is no longer asserting the interrupt.

> The interrupt is for sure masked, and therefore the SA interrupts are not
> fired. Even if they would have been enabled, the "pending interrupts register"
> doesn't show any sa interrupt pending, so there is something else.

Masked where though - in the SA or FPGA?

> As I don't know if "enable_irq()" in sa.c would be an option as I have no
> sight on sa.c requirements, I would take any advice here.

It shouldn't be required, as I say above, irq_set_chained_handler_and_data()
deals with unmasking the IRQ already.

> > Hmm, on Neponset with a CF card inserted, I can cat that, and it reports
> > the CIS.  Any error messages in the kernel log?
> None.
> I have an ever more suprising thing.
> I tried again this morning, without changing a single line of code (excepting 
> in
> pxa_cplds_irqs.c) nor touching the CF card, and now it really rocks !!! :
> hexdump -C /sys/class/pcmcia_socket/pcmcia_socket1/cis
> hexdump -C /sys/class/pcmcia_socket/pcmcia_socket1/cis
>   01 04 df 79 01 ff 1c 04  02 db 01 ff 18 02 df 01  |...y|
> 0010  20 04 45 00 01 04 15 0b  

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

2016-09-02 Thread Robert Jarzmik
Russell King - ARM Linux  writes:

>> 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.)
Ah, I see. Let's drop the patch then, I won't be able to know what to return in
this function if 2 initialisations fail, nor if the first failure should be
fatal or not, nor if a rollback is possible after 1 success (badge4) and 1
failure (neponset).

> 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.
Ok.

>> 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?
I see no other interrupts at all.
Actually I see no interrupt claimed in /proc/interrupts, where I would have
expected interrupt 305.
cat /proc/interrupts
   CPU0   
 24:   1419SC   8 Edge  gpio-0
 25:  0SC   9 Edge  gpio-1
 26:  0SC  10 Edge  gpio-mux
 38:118SC  22 Edge  UART1
 41:  0SC  25 Edge  DMA
 42:  40224SC  26 Edge  ost0
112:   1419  GPIO   0 Edge  pxa_cplds_irqs
307:   1419  pxa_cplds   3 Edge  eth0
387:  0  SA-h Edge  SA PCMCIA card detect
388:  0  SA-h Edge  SA CF card detect
389:  0  SA-h Edge  SA PCMCIA BVD1
390:  0  SA-h Edge  SA CF BVD1
Err:  0

Actually this leads me to think that this interrupt 305 is not "requested" nor
activated. I see in sa.c:506 :
  "irq_set_chained_handler_and_data(sachip->irq, sa_irq_handler, ..."
This puts in the handler and data, but I don't this is "enables" the interrupt,
right ?

I traced the code in the interrupt controller (pxa_cplds_irqs), the SA
physical line is not set (even with an AT/2 keyboard connected, and I don't
think anybody tried this AT/2 on a lubbock for a long time).

The interrupt is for sure masked, and therefore the SA interrupts are not
fired. Even if they would have been enabled, the "pending interrupts register"
doesn't show any sa interrupt pending, so there is something else.

As I don't know if "enable_irq()" in sa.c would be an option as I have no
sight on sa.c requirements, I would take any advice here.

> Hmm, on Neponset with a CF card inserted, I can cat that, and it reports
> the CIS.  Any error messages in the kernel log?
None.
I have an ever more suprising thing.
I tried again this morning, without changing a single line of code (excepting in
pxa_cplds_irqs.c) nor touching the CF card, and now it really rocks !!! :
hexdump -C /sys/class/pcmcia_socket/pcmcia_socket1/cis
hexdump -C /sys/class/pcmcia_socket/pcmcia_socket1/cis
  01 04 df 79 01 ff 1c 04  02 db 01 ff 18 02 df 01  |...y|
0010  20 04 45 00 01 04 15 0b  04 01 00 43 46 43 41 52  | .ECFCAR|
0020  44 00 ff 21 02 04 01 22  02 01 01 22 03 02 0c 0f  |D..!..."..."|
0030  1a 05 01 03 00 02 0f 1b  08 c0 40 a1 01 55 08 00  |..@..U..|
0040  20 1b 06 00 01 21 b5 1e  4d 1b 0a c1 41 99 01 55  | !..M...A..U|
0050  64 f0 ff ff 20 1b 06 01  01 21 b5 1e 4d 1b 0f c2  |d... !..M...|
0060  41 99 01 55 ea 61 f0 01  07 f6 03 01 ee 20 1b 06  |A..U.a... ..|
0070  02 01 21 b5 1e 4d 1b 0f  c3 41 99 01 55 ea 61 70  |..!..M...A..U.ap|
0080  01 07 76 03 01 ee 20 1b  06 03 01 21 b5 1e 4d 14  |..v... !..M.|
0090  00|.|

I think this proves that your patches related to lubbock pcmcia conversion to
max1602 is fully functional !

Only the interrupt part to fight a bit, and then I'll try to make a couple of
tests on the IRDA part.

Cheers.

-- 
Robert

___
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

2016-09-02 Thread Russell King - ARM Linux
Linus,

There's a change I'd like to merge into patch 5 - overall it looks
like the below, and allows us to use gpio-reg with the PXA mainstone
MST_PCMCIA[01] registers.  Some of these GPIO signals have hardware
interrupts associated with them, but not all.

Do you approve?

Thanks.

8<
From: Russell King 
Subject: [PATCH] gpio: gpio-reg: add irq mapping for gpio-reg users

Add support for mapping gpio-reg gpios to interrupts.  This may be a
non-linear mapping - some gpios in the register may not even have
corresponding interrupts associated with them, so we need to pass an
array.

Signed-off-by: Russell King 
---
 arch/arm/mach-pxa/lubbock.c |  2 +-
 arch/arm/mach-sa1100/assabet.c  |  2 +-
 arch/arm/mach-sa1100/neponset.c |  2 +-
 drivers/gpio/gpio-reg.c | 25 +++--
 include/linux/gpio/gpio-reg.h   |  3 ++-
 5 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-pxa/lubbock.c b/arch/arm/mach-pxa/lubbock.c
index 1f7bfabfa4eb..cd401546cea8 100644
--- a/arch/arm/mach-pxa/lubbock.c
+++ b/arch/arm/mach-pxa/lubbock.c
@@ -477,7 +477,7 @@ static void __init lubbock_init(void)
 
lubbock_misc_wr_gc = gpio_reg_init(NULL, (void *)_MISC_WR,
   -1, 16, "lubbock", 0, LUB_MISC_WR,
-  NULL);
+  NULL, NULL, NULL);
if (IS_ERR(lubbock_misc_wr_gc)) {
pr_err("Lubbock: unable to register lubbock GPIOs: %ld\n",
   PTR_ERR(lubbock_misc_wr_gc));
diff --git a/arch/arm/mach-sa1100/assabet.c b/arch/arm/mach-sa1100/assabet.c
index 51950454afac..e086c609551c 100644
--- a/arch/arm/mach-sa1100/assabet.c
+++ b/arch/arm/mach-sa1100/assabet.c
@@ -120,7 +120,7 @@ static int __init assabet_init_gpio(void __iomem *reg, u32 
def_val)
writel_relaxed(def_val, reg);
 
gc = gpio_reg_init(NULL, reg, -1, 32, "assabet", 0xff00, def_val,
-  assabet_names);
+  assabet_names, NULL, NULL);
 
if (IS_ERR(gc))
return PTR_ERR(gc);
diff --git a/arch/arm/mach-sa1100/neponset.c b/arch/arm/mach-sa1100/neponset.c
index 5abc04fb196d..b16de1f0d5d0 100644
--- a/arch/arm/mach-sa1100/neponset.c
+++ b/arch/arm/mach-sa1100/neponset.c
@@ -240,7 +240,7 @@ static int neponset_init_gpio(struct gpio_chip **gcp,
struct gpio_chip *gc;
 
gc = gpio_reg_init(dev, reg, -1, num, label, in ? 0x : 0,
-  readl_relaxed(reg), names);
+  readl_relaxed(reg), names, NULL, NULL);
if (IS_ERR(gc))
return PTR_ERR(gc);
 
diff --git a/drivers/gpio/gpio-reg.c b/drivers/gpio/gpio-reg.c
index 561e28b4d3aa..39175d373871 100644
--- a/drivers/gpio/gpio-reg.c
+++ b/drivers/gpio/gpio-reg.c
@@ -19,6 +19,8 @@ struct gpio_reg {
u32 direction;
u32 out;
void __iomem *reg;
+   struct irq_domain *irqdomain;
+   const int *irqs;
 };
 
 #define to_gpio_reg(x) container_of(x, struct gpio_reg, gc)
@@ -96,6 +98,17 @@ static void gpio_reg_set_multiple(struct gpio_chip *gc, 
unsigned long *mask,
spin_unlock_irqrestore(>lock, flags);
 }
 
+static int gpio_reg_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+   struct gpio_reg *r = to_gpio_reg(gc);
+   int irq = r->irqs[offset];
+
+   if (irq >= 0 && r->irqdomain)
+   irq = irq_find_mapping(r->irqdomain, irq);
+
+   return irq;
+}
+
 /**
  * gpio_reg_init - add a fixed in/out register as gpio
  * @dev: optional struct device associated with this register
@@ -104,7 +117,12 @@ static void gpio_reg_set_multiple(struct gpio_chip *gc, 
unsigned long *mask,
  * @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
+ * @names: array of %num strings describing each GPIO signal or %NULL
+ * @irqdom: irq domain or %NULL
+ * @irqs: array of %num ints describing the interrupt mapping for each
+ *GPIO signal, or %NULL.  If @irqdom is %NULL, then this
+ *describes the Linux interrupt number, otherwise it describes
+ *the hardware interrupt number in the specified irq domain.
  *
  * Add a single-register GPIO device containing up to 32 GPIO signals,
  * where each GPIO has a fixed input or output configuration.  Only
@@ -114,7 +132,7 @@ static void gpio_reg_set_multiple(struct gpio_chip *gc, 
unsigned long *mask,
  */
 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)
+   const char *const *names, struct irq_domain *irqdom, const int *irqs)
 {
struct gpio_reg *r;
int ret;
@@ -136,12 +154,15 @@ struct gpio_chip *gpio_reg_init(struct device 

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

2016-09-01 Thread Russell King - ARM Linux
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

2016-08-31 Thread Russell King - ARM Linux
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.  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_00x0/* 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_00x0/* 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 *)_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.

-- 
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

2016-08-31 Thread Russell King - ARM Linux
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.  Maybe we can look at converting mainstone as well?

-- 
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

2016-08-30 Thread Linus Walleij
On Tue, Aug 30, 2016 at 11:42 PM, Russell King - ARM Linux
 wrote:
> On Tue, Aug 30, 2016 at 11:25:19PM +0200, Linus Walleij wrote:

>> > +#define to_gpio_reg(x) container_of(x, struct gpio_reg, gc)
>>
>> You don't need that trickery anymore, just:
>>
>> > +static int gpio_reg_get_direction(struct gpio_chip *gc, unsigned offset)
>> > +{
>> > +   struct gpio_reg *r = to_gpio_reg(gc);
>>
>> struct gpio_reg *r = gpiochip_get_data(gc);
>>
>> (applied everywhere)
>
> I prefer my method by a long shot

Sure it's no strong preference. Keep it like this if you like.
I'm very happy with the series either way!

Yours,
Linus Walleij

___
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

2016-08-30 Thread Russell King - ARM Linux
On Tue, Aug 30, 2016 at 11:25:19PM +0200, Linus Walleij wrote:
> On Mon, Aug 29, 2016 at 12:24 PM, Russell King
>  wrote:
> 
> > Add a simple, generic, single register fixed-direction GPIO driver.
> > This is able to support a single register where a fixed number of
> > bits are used for input and a fixed number of bits used for output.
> >
> > Signed-off-by: Russell King 
> 
> Clever, I like it!
> 
> >  include/linux/gpio-reg.h |  12 
> 
> Can we put this in include/linux/gpio/gpio-reg.h?
> 
> I try to do some scopeing to  there.

Sure, I'll just have to hunt through all the patches to find an occurance
of the include to fix them all up...

> > +#define to_gpio_reg(x) container_of(x, struct gpio_reg, gc)
> 
> You don't need that trickery anymore, just:
> 
> > +static int gpio_reg_get_direction(struct gpio_chip *gc, unsigned offset)
> > +{
> > +   struct gpio_reg *r = to_gpio_reg(gc);
> 
> struct gpio_reg *r = gpiochip_get_data(gc);
> 
> (applied everywhere)

I prefer my method by a long shot - it's always going to work because
the gpiochip is embedded within the gpio_reg structure, and the compiler
is inteligent enough to keep a single pointer around.  With your
suggestion, the compiler has no idea that 'r' and 'gc' are actually
the same pointer, but a different type, and we end up having to carry
around identical pointers in two registers rather than just one.

It makes more sense to use gpiochip_get_data() if gpio_chip were a const
data structure that was never embedded, but the way *gpiochip_add*()
writes to the structure and the presence of members like "base" prevents
that.

> 
> > +   if (dev)
> > +   ret = devm_gpiochip_add_data(dev, >gc, r);
> > +   else
> > +   ret = gpiochip_add_data(>gc, r);
> 
> Aha both device and device-less, I see.

Yes, to avoid problems with the transition to it - the legacy APIs
(such as ASSABET_BCR_frob(), etc) can be called really early, so we
need the gpiochip available early as well so we can keep the legacy
APIs working until they can be killed off.  There's some corner
cases in the assabet code which make that difficult at the moment.

This whole patch set is still very much a work-in-progress - there's
more that needs doing, but I wanted to get _this_ out there so that
people can reviewing it, and hopefully get it queued for the next
merge window.

> Apart from that it looks nice, any other questionmarks were
> fixed in the other replies.
> 
> Yours,
> Linus Walleij

-- 
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

2016-08-30 Thread Robert Jarzmik
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.

Cheers.

-- 
Robert

___
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

2016-08-30 Thread Linus Walleij
On Mon, Aug 29, 2016 at 12:24 PM, Russell King
 wrote:

> Add a simple, generic, single register fixed-direction GPIO driver.
> This is able to support a single register where a fixed number of
> bits are used for input and a fixed number of bits used for output.
>
> Signed-off-by: Russell King 

Clever, I like it!

>  include/linux/gpio-reg.h |  12 

Can we put this in include/linux/gpio/gpio-reg.h?

I try to do some scopeing to  there.

> +#define to_gpio_reg(x) container_of(x, struct gpio_reg, gc)

You don't need that trickery anymore, just:

> +static int gpio_reg_get_direction(struct gpio_chip *gc, unsigned offset)
> +{
> +   struct gpio_reg *r = to_gpio_reg(gc);

struct gpio_reg *r = gpiochip_get_data(gc);

(applied everywhere)

> +   if (dev)
> +   ret = devm_gpiochip_add_data(dev, >gc, r);
> +   else
> +   ret = gpiochip_add_data(>gc, r);

Aha both device and device-less, I see.

Apart from that it looks nice, any other questionmarks were
fixed in the other replies.

Yours,
Linus Walleij

___
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

2016-08-30 Thread Russell King - ARM Linux
On Tue, Aug 30, 2016 at 06:42:03PM +0200, Robert Jarzmik wrote:
> Russell King - ARM Linux  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 
> 
> 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_SA 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.

-- 
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

2016-08-30 Thread Robert Jarzmik
Russell King - ARM Linux  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 

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_SA 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

___
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

2016-08-30 Thread Russell King - ARM Linux
On Tue, Aug 30, 2016 at 09:08:03AM +0300, Alexander Shiyan wrote:
> >Вторник, 30 августа 2016, 2:12 +03:00 от Russell King - ARM Linux 
> >:
> >
> >On Mon, Aug 29, 2016 at 09:39:54PM +0200, Robert Jarzmik wrote:
> >> Hi Russell,
> >> 
> >> Russell King < rmk+ker...@armlinux.org.uk > writes:
> >> 
> >> > Add a simple, generic, single register fixed-direction GPIO driver.
> >> > This is able to support a single register where a fixed number of
> >> > bits are used for input and a fixed number of bits used for output.
> >> >
> >> > Signed-off-by: Russell King < rmk+ker...@armlinux.org.uk >
> 
> There is a GPIO driver which already performs these tasks.
> Plaease take a look on the gpio-74xx-mmio driver.

I did, and no it doesn't, because:

1. It is either all-in or all-out, it doesn't support a mixture of
   fixed-directions for a single register.
2. It is DT-only, I need it for legacy platforms.
3. It uses the bgpio stuff, which is unsuitable for reasons already
   covered in a previous reply.

So, gpio-74xx-mmio is unsuitable.

-- 
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

2016-08-30 Thread Alexander Shiyan
>Вторник, 30 августа 2016, 2:12 +03:00 от Russell King - ARM Linux 
>:
>
>On Mon, Aug 29, 2016 at 09:39:54PM +0200, Robert Jarzmik wrote:
>> Hi Russell,
>> 
>> Russell King < rmk+ker...@armlinux.org.uk > writes:
>> 
>> > Add a simple, generic, single register fixed-direction GPIO driver.
>> > This is able to support a single register where a fixed number of
>> > bits are used for input and a fixed number of bits used for output.
>> >
>> > Signed-off-by: Russell King < rmk+ker...@armlinux.org.uk >

There is a GPIO driver which already performs these tasks.
Plaease take a look on the gpio-74xx-mmio driver.

---

___
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

2016-08-29 Thread Russell King - ARM Linux
On Mon, Aug 29, 2016 at 09:39:54PM +0200, Robert Jarzmik wrote:
> Hi Russell,
> 
> Russell King  writes:
> 
> > Add a simple, generic, single register fixed-direction GPIO driver.
> > This is able to support a single register where a fixed number of
> > bits are used for input and a fixed number of bits used for output.
> >
> > Signed-off-by: Russell King 
> > ---
> >  drivers/gpio/Kconfig |   6 ++
> >  drivers/gpio/Makefile|   1 +
> >  drivers/gpio/gpio-reg.c  | 139 
> > +++
> >  include/linux/gpio-reg.h |  12 
> >  4 files changed, 158 insertions(+)
> >  create mode 100644 drivers/gpio/gpio-reg.c
> >  create mode 100644 include/linux/gpio-reg.h
> >
> > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> > index 98dd47a30fc7..49bd8b89712e 100644
> > --- a/drivers/gpio/Kconfig
> > +++ b/drivers/gpio/Kconfig
> > @@ -365,6 +365,12 @@ config GPIO_RCAR
> > help
> >   Say yes here to support GPIO on Renesas R-Car SoCs.
> >  
> > +config GPIO_REG
> > +   bool
> So I suppose it is on purpose you forbid it to be a module. Is there a way to
> write either in the commit message or in the Kconfig this purpose, so that
> nobody on "purpose" changes this bool to tristate ?

The only forbidding is that it's used from code early at boot, so it has
to be available to the kernel for some of these platforms to function.
Even if it's changed to tristate, the select from these platforms would
force it to be built-in.

> > +   help
> > + A 32-bit single register GPIO fixed in/out implementation.  This
> > + can be used to represent any register as a set of GPIO signals.
> Another question I was asking myself was how it differenciated itself from
> gpio-mmio, ie. what brought the need for this driver that isn't available with
> gpio-mmio ?

gpio-mmio doesn't allow the fixed direction - it assumes that you always
have some form of direction register.  It also doesn't do the double-read
that's necessary for some platforms to get the "current" state of inputs.
It also doesn't do the "use 32-bit accessors even for smaller numbers of
GPIOs".  Lastly, it assumes that the current output state can be read
from the registers - this is not always the case (and is not the case for
Assabet.)

> I seem to understand that this is mainly for platform code, hence the
> "builtin only" necessity, and if I'm right a part of your cover letter
> could very well fit within this patch's commit message.

Apart from the missing MODULE_* tags and symbol exports, there's nothing
whic prohibits it becoming a module.

> > +static void gpio_reg_set_multiple(struct gpio_chip *gc, unsigned long 
> > *mask,
> > +   unsigned long *bits)
> > +{
> > +   struct gpio_reg *r = to_gpio_reg(gc);
> > +   unsigned long flags;
> > +
> > +   spin_lock_irqsave(>lock, flags);
> > +   r->out = (r->out & ~*mask) | *bits;
> Shouldn't this be :
>   r->out = (r->out & ~*mask) | (*bits & *mask);

Possibly, but the latter is redundant when used through gpiolib.

> > diff --git a/include/linux/gpio-reg.h b/include/linux/gpio-reg.h
> > new file mode 100644
> > index ..0352bec7319a
> > --- /dev/null
> > +++ b/include/linux/gpio-reg.h
> > @@ -0,0 +1,12 @@
> > +#ifndef GPIO_REG_H
> > +#define GPIO_REG_H
> > +
> > +struct device;
> > +
> > +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);
> Maybe this one would deserve a doxygen comment ?

Maybe, but I've been told not to put such comments in header files.
I've already spent the last two weeks on this stuff (at the expense
of reading mail), I've not got around to thinking about any kind of
documentation.

-- 
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

2016-08-29 Thread Robert Jarzmik
Hi Russell,

Russell King  writes:

> Add a simple, generic, single register fixed-direction GPIO driver.
> This is able to support a single register where a fixed number of
> bits are used for input and a fixed number of bits used for output.
>
> Signed-off-by: Russell King 
> ---
>  drivers/gpio/Kconfig |   6 ++
>  drivers/gpio/Makefile|   1 +
>  drivers/gpio/gpio-reg.c  | 139 
> +++
>  include/linux/gpio-reg.h |  12 
>  4 files changed, 158 insertions(+)
>  create mode 100644 drivers/gpio/gpio-reg.c
>  create mode 100644 include/linux/gpio-reg.h
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 98dd47a30fc7..49bd8b89712e 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -365,6 +365,12 @@ config GPIO_RCAR
>   help
> Say yes here to support GPIO on Renesas R-Car SoCs.
>  
> +config GPIO_REG
> + bool
So I suppose it is on purpose you forbid it to be a module. Is there a way to
write either in the commit message or in the Kconfig this purpose, so that
nobody on "purpose" changes this bool to tristate ?

> + help
> +   A 32-bit single register GPIO fixed in/out implementation.  This
> +   can be used to represent any register as a set of GPIO signals.
Another question I was asking myself was how it differenciated itself from
gpio-mmio, ie. what brought the need for this driver that isn't available with
gpio-mmio ?

I seem to understand that this is mainly for platform code, hence the "builtin
only" necessity, and if I'm right a part of your cover letter could very well
fit within this patch's commit message.

> diff --git a/drivers/gpio/gpio-reg.c b/drivers/gpio/gpio-reg.c
> new file mode 100644
> index ..fc7e0a395f9f
> --- /dev/null
> +++ b/drivers/gpio/gpio-reg.c

...zip...
> +static void gpio_reg_set_multiple(struct gpio_chip *gc, unsigned long *mask,
> + unsigned long *bits)
> +{
> + struct gpio_reg *r = to_gpio_reg(gc);
> + unsigned long flags;
> +
> + spin_lock_irqsave(>lock, flags);
> + r->out = (r->out & ~*mask) | *bits;
Shouldn't this be :
r->out = (r->out & ~*mask) | (*bits & *mask);

> diff --git a/include/linux/gpio-reg.h b/include/linux/gpio-reg.h
> new file mode 100644
> index ..0352bec7319a
> --- /dev/null
> +++ b/include/linux/gpio-reg.h
> @@ -0,0 +1,12 @@
> +#ifndef GPIO_REG_H
> +#define GPIO_REG_H
> +
> +struct device;
> +
> +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);
Maybe this one would deserve a doxygen comment ?

Cheers.

-- 
Robert

___
Linux PCMCIA reimplementation list
http://lists.infradead.org/mailman/listinfo/linux-pcmcia