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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 1 Sep 2017 00:36:27 +0900
From:   Masahiro Yamada <yamada.masahiro@...ionext.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Marc Zyngier <marc.zyngier@....com>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Jassi Brar <jaswinder.singh@...aro.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Randy Dunlap <rdunlap@...radead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Rob Herring <robh+dt@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Mark Rutland <mark.rutland@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3] gpio: uniphier: add UniPhier GPIO controller driver

Hi Linus,


2017-08-31 22:41 GMT+09:00 Linus Walleij <linus.walleij@...aro.org>:
> On Tue, Aug 22, 2017 at 6:16 PM, Masahiro Yamada
> <yamada.masahiro@...ionext.com> wrote:
>
>> This GPIO controller device is used on UniPhier SoCs.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@...ionext.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