[PATCH v2 0/7] staging: mt7621-gpio: last cleanups

[NeilBrown]

On Mon, Jun 11 2018, Sergio Paracuellos wrote:

> On Mon, Jun 11, 2018 at 06:33:44PM +1000, NeilBrown wrote:
>> On Mon, Jun 11 2018, Sergio Paracuellos wrote:
>> 
>> > After submiting this driver to try to get mainlined and get
>> > out of staging some new cleanups seems to be necessary.
>> > According to this main of Linus Walleij:
>> >
>> > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-June/121742.html
>> >
>> > this series tries to fix all of the issues in order to send
>> > v2 and give it a new try. Because I don't have to hardware to
>> > test my changes I send new cleanups first in staging to make
>> > easier to NeilBrown test it and get a feedback about them.
>> >
>> > Changes in v2:
>> >     - Patch where GPIOLIB_IRQCHIP was used avoiding
>> >       the use of a custom irq domain has been dropped to
>> >       be sure after this changes all is working properly.
>> >       (This was PATCH 7 in previous series)
>> >     - PATCH 1: 
>> >          * avoid introducing new macros and use 'bank'
>> >            field of mtk_gc with register offset.
>> >          * Make correct use of bgpio_init passing new
>> >            void __iomem pointers instead of use the
>> >            macros. 
>> >     - Previous series PATCH 8 now is PATCH 7. Avoid the
>> >       use of a switch-case statement which was wrong and
>> >       distinc if we have RISSING AND FALLING EDGE interrupt
>> >       or HIGH LOW level ones. This last two are exclusive and
>> >       cannot be generated at the same time.
>> >
>> > Also, I think is we finally avoid to use a new irq_domain the
>> > need for the new functions introduced for request and release
>> > resources dissapears. I was diving down the other drivers code
>> > and I see that these two only are used in drivers which use its
>> > own irq_domain. Correct me if I am wrong, please.
>> >
>> > Hope this helps.
>> >
>> > Thanks in advance.
>> 
>> Thanks a lot.
>> This series appears to work, though I sent a separate comment on one
>> piece of code.
>
> Thanks for testing, review and feedback.
>
> I have resent a complete v3 of this series taking into account
> your comments there.
>
>> However the gpio are numbers
>> 480-511
>> 448-479
>> 416-447
>> 
>> instead of
>> 0-31
>> 32-63
>> 64-95
>> 
>> which would be more normal.
>> Maybe when you resubmit I'll raid it with Linus Walleij and see if he
>> can explain why I can't have 0-95.
>
> This change is because the chip.base property changed to be -1 for
> dynamic enumeration of the gpio's. In this mail there is some explanation
> about it:
>
> http://lists.infradead.org/pipermail/linux-rpi-kernel/2014-October/001045.html

Interesting - thanks for the link.

It seems that the goal is to focus more on names than on numbers, and
that probably makes sense.
It means that we need to make sure that the names are useful.

Currently the driver defines 3 gpiochips, one for each bank.

# grep . /sys/class/gpio/gpiochip4*/label
/sys/class/gpio/gpiochip416/label:1e000600.gpio
/sys/class/gpio/gpiochip448/label:1e000600.gpio
/sys/class/gpio/gpiochip480/label:1e000600.gpio

Unfortunately they all have the same label :-(
It would be good if there was a way to add 0, 1, and 2 to the labels, or
something like that.

In /proc/interrupts we now have:

 17:          0          0          0          0  MIPS GIC  19  mt7621, mt7621, mt7621

which is the interrupt from the GPIO controller.
It is a little weird that all three banks are named "mt7621" here.
We also have:

 26:          0          0          0          0      GPIO  18  reset

which is the interrupt from GPIO which provides the "reset" button.
I suspect that if I had interrupts form two different banks they would
both be called "GPIO" which would be a little confusing.
We could declare three different 'struct irq_chip' with three different
names, but that would be ugly.  Hopefully there is a better way.

Thanks,
NeilBrown
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/attachments/20180612/b6dcd4d0/attachment.asc>