Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1570781 > unrolled thread

Re: [PATCH v3 04/14] GPIO: Add gpio-ingenic driver

Started byLinus Walleij <linus.walleij@linaro.org>
First post2017-01-31 15:30 +0100
Last post2017-02-03 15:00 +0100
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v3 04/14] GPIO: Add gpio-ingenic driver Linus Walleij <linus.walleij@linaro.org> - 2017-01-31 15:30 +0100
    Re: [PATCH v3 04/14] GPIO: Add gpio-ingenic driver Paul Cercueil <paul@crapouillou.net> - 2017-01-31 16:40 +0100
      Re: [PATCH v3 04/14] GPIO: Add gpio-ingenic driver Linus Walleij <linus.walleij@linaro.org> - 2017-02-03 15:00 +0100

#1570781 — Re: [PATCH v3 04/14] GPIO: Add gpio-ingenic driver

FromLinus Walleij <linus.walleij@linaro.org>
Date2017-01-31 15:30 +0100
SubjectRe: [PATCH v3 04/14] GPIO: Add gpio-ingenic driver
Message-ID<t5HBv-1Ll-7@gated-at.bofh.it>
Forgot to mention this:

On Wed, Jan 25, 2017 at 7:51 PM, Paul Cercueil <paul@crapouillou.net> wrote:
> This driver handles the GPIOs of all the Ingenic JZ47xx SoCs
> currently supported by the upsteam Linux kernel.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
(...)
> +       jzgc->gc.base = -1;

Nice

> +       of_property_read_u32(dev->of_node, "base", &jzgc->gc.base);

Remove this. Dynamic allocation should be fine, if you're using the
new userspace ABI like tools/gpio/* or libgpiod and only that and in-kernel
consumers, dynamic numbers are just fine.

If you have old sysfs userspace that you need to support using the
global GPIO numberspace, please look into ways to phase that out.

Yours,
Linus Walleij

[toc] | [next] | [standalone]


#1570833

FromPaul Cercueil <paul@crapouillou.net>
Date2017-01-31 16:40 +0100
Message-ID<t5IHf-2n8-17@gated-at.bofh.it>
In reply to#1570781
Le 2017-01-31 15:20, Linus Walleij a écrit :

>> + of_property_read_u32(dev->of_node, "base", &jzgc->gc.base);
> 
> Remove this. Dynamic allocation should be fine, if you're using the
> new userspace ABI like tools/gpio/* or libgpiod and only that and 
> in-kernel
> consumers, dynamic numbers are just fine.

The problem is that the QI_LB60 board code still have a lot of 
references
to global GPIO numbers. Just grep for JZ_GPIO_PORT in
arch/mips/jz4740/board-qi_lb60.c to see what I mean...

-Paul

[toc] | [prev] | [next] | [standalone]


#1573107

FromLinus Walleij <linus.walleij@linaro.org>
Date2017-02-03 15:00 +0100
Message-ID<t6Mz8-2gr-19@gated-at.bofh.it>
In reply to#1570833
On Tue, Jan 31, 2017 at 4:29 PM, Paul Cercueil <paul@crapouillou.net> wrote:
> Le 2017-01-31 15:20, Linus Walleij a écrit :
>
>>> + of_property_read_u32(dev->of_node, "base", &jzgc->gc.base);
>>
>>
>> Remove this. Dynamic allocation should be fine, if you're using the
>> new userspace ABI like tools/gpio/* or libgpiod and only that and
>> in-kernel
>> consumers, dynamic numbers are just fine.
>
>
> The problem is that the QI_LB60 board code still have a lot of references
> to global GPIO numbers. Just grep for JZ_GPIO_PORT in
> arch/mips/jz4740/board-qi_lb60.c to see what I mean...

OK I understand we might need a compromise here to get the code moving.

But we need to keep it out of the device tree.

I think it's better to put a base table relative to the memory base in
the driver in that case:

unsigned int gpio_global_base;

switch (memory_base_address) {
case 0x41000000:
    gpio_global_base = 0x00;
    break;
case 0x42000000:
    gpio_global_base = 0x20;

(...)

etc. (Those are not your base addresses but you get the idea).

Include a few comments like:

/*
 * DO NOT EXPAND THIS: FOR BACKWARD GPIO NUMBERSPACE
 * COMPATIBIBILITY ONLY: WORK TO TRANSITION CONSUMERS TO
 * USE THE GPIO DESCRIPTOR API IN <linux/gpio/consumer.h> INSTEAD.
 */

Then I'll be happy :)

Yours,
Linus Walleij

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web