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

Groups > linux.kernel > #1724239 > unrolled thread

Re: [PATCH v3] gpio: uniphier: add UniPhier GPIO controller driver

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] gpio: uniphier: add UniPhier GPIO controller driver Linus Walleij <linus.walleij@linaro.org> - 2017-08-31 15:50 +0200
    Re: [PATCH v3] gpio: uniphier: add UniPhier GPIO controller driver Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-08-31 17:40 +0200
    Re: [PATCH v3] gpio: uniphier: add UniPhier GPIO controller driver Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-09-07 03:20 +0200

#1724239 — Re: [PATCH v3] gpio: uniphier: add UniPhier GPIO controller driver

On Tue, Aug 22, 2017 at 6:16 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:

> This GPIO controller device is used on UniPhier SoCs.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> Changes in v3:
>   - Add .irq_set_affinity() hook
>   - Use irq_domain_create_hierarchy() instead of legacy
>     irq_domain_add_hierarchy().

This is getting better and better.

> +static const u32 uniphier_gpio_irq_parent_hwirqs[] = {
> +       48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
> +       154, 155, 156, 157, 158, 217, 218, 219,
> +};

This array which is used when allocating the hierarchical domain
looks like configuration data. Why is it not in the device tree?

Yours,
Linus Walleij

[toc] | [next] | [standalone]

#1724324

Hi Linus,


2017-08-31 22:41 GMT+09:00 Linus Walleij <linus.walleij@linaro.org>:
> On Tue, Aug 22, 2017 at 6:16 PM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>
>> This GPIO controller device is used on UniPhier SoCs.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>> Changes in v3:
>>   - Add .irq_set_affinity() hook
>>   - Use irq_domain_create_hierarchy() instead of legacy
>>     irq_domain_add_hierarchy().
>
> This is getting better and better.
>
>> +static const u32 uniphier_gpio_irq_parent_hwirqs[] = {
>> +       48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
>> +       154, 155, 156, 157, 158, 217, 218, 219,
>> +};
>
> This array which is used when allocating the hierarchical domain
> looks like configuration data. Why is it not in the device tree?


The same comment as in v1.

There is no systematic way to map hwirqs to the parent
for irqdomain hierarchy:
https://lkml.org/lkml/2017/7/6/758



If you see irqchip drivers using domain hierarchy,
many drivers hard-code

    parent_fwspec.param_count = 3;

without parsing #interrupt-cells property of the parent.

It is hard-coded configuration data, isn't it?



The array of parent hwirqs is more or less SoC specific data.


This is just a matter of taste, but
if you like, I can move the array to the SoC data associated
with compatible.


.ngpio is already associated with compatible,
so adding more data would not hurt...


static const u32 uniphier_pxs2_gpio_irqs[] = {
        48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
        154, 155, 156, 157, 158, 217, 218, 219,
};

static const struct uniphier_gpio_socdata uniphier_pxs2_gpio_data = {
        .ngpio = 232,
        .irqs = uniphier_pxs2_gpio_irqs,
        .nirqs = ARRAY_SIZE(uniphier_pxs2_gpio_irqs),
};





-- 
Best Regards
Masahiro Yamada

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

#1727862

Hi Linus,


2017-08-31 22:41 GMT+09:00 Linus Walleij <linus.walleij@linaro.org>:
> On Tue, Aug 22, 2017 at 6:16 PM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>
>> This GPIO controller device is used on UniPhier SoCs.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>> Changes in v3:
>>   - Add .irq_set_affinity() hook
>>   - Use irq_domain_create_hierarchy() instead of legacy
>>     irq_domain_add_hierarchy().
>
> This is getting better and better.
>
>> +static const u32 uniphier_gpio_irq_parent_hwirqs[] = {
>> +       48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
>> +       154, 155, 156, 157, 158, 217, 218, 219,
>> +};
>
> This array which is used when allocating the hierarchical domain
> looks like configuration data. Why is it not in the device tree?
>


Another solution might be
to use the brand-new irq_domain_push_irq().


With a quick look at it, the hierarchy IRQ domain can work with
#interrupts property by inserting the outer-most irq_data with
irq_domain_push_irq().


If you are unhappy with hard-coded configuration parameters,
I will switch to generic compatible + DT properties approach.


-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web