linux-kernel - Re: [PATCH v6 2/2] 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] [day] [month] [year] [list]

Date:   Wed, 11 Oct 2017 18:38:48 +0900
From:   Masahiro Yamada <yamada.masahiro@...ionext.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Rob Herring <robh@...nel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Jassi Brar <jaswinder.singh@...aro.org>,
        Mauro Carvalho Chehab <mchehab@...nel.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 v6 2/2] gpio: uniphier: add UniPhier GPIO controller driver

2017-10-11 17:40 GMT+09:00 Linus Walleij <linus.walleij@...aro.org>:

>
>> +++ b/include/dt-bindings/gpio/uniphier-gpio.h
>> @@ -0,0 +1,18 @@
>> +/*
>> + * Copyright (C) 2017 Socionext Inc.
>> + *   Author: Masahiro Yamada <yamada.masahiro@...ionext.com>
>> + */
>> +
>> +#ifndef _DT_BINDINGS_GPIO_UNIPHIER_H
>> +#define _DT_BINDINGS_GPIO_UNIPHIER_H
>> +
>> +#define UNIPHIER_GPIO_LINES_PER_BANK   8
>> +
>> +#define UNIPHIER_GPIO_IRQ_OFFSET       ((UNIPHIER_GPIO_LINES_PER_BANK) * 15)
>> +
>> +#define UNIPHIER_GPIO_PORT(bank, line) \
>> +                       ((UNIPHIER_GPIO_LINES_PER_BANK) * (bank) + (line))
>> +
>> +#define UNIPHIER_GPIO_IRQ(n)           ((UNIPHIER_GPIO_IRQ_OFFSET) + (n))
>> +
>> +#endif /* _DT_BINDINGS_GPIO_UNIPHIER_H */
>
> I do not understand what some of these things are doing in the
> device tree header file.
> It just looks creepingly similar to some of the magic I've seen
> in board files.

Because this hardware design is crazy crap.

Only the one's place of the GPIO labels is octal.
The other places are decimal.



> It makes much more sense that the device trees either:
>
> - Use the interrupts as a flat array 0...N across all banks

I think I did so.

But, please note only bank 15, 16, 17 can support irq.
(and bank 15, 16, 17 share the same irq control register)

No irq support for bank 0-14, 18-31.

No symmetry in the hardware structure.


> - Model each bank as a separate GPIO chip

No, no, never.
I had already tried per-bank splitting approach,
but it was a horrible disaster.

Please note there would end up 31 banks if it were split.
(31 nodes in DT)

What is worse, registers from different banks interleave,
and, in some places, share the same registers.



> This is somewhere inbetween, you are modeling it as a single
> gpiochip but still not, becuase the device tree author still need
> to address banking, that is confusing.

This is the best solution I found.



Unfortunately, the hardware specification adopts weird GPIO pin labeling.
The ports are named as
  PORT00,  PORT01,  PORT02,  ..., PORT07,
  PORT10,  PORT11,  PORT12,  ..., PORT17,
  PORT20,  PORT21,  PORT22,  ..., PORT27,
    ...
  PORT90,  PORT91,  PORT92,  ..., PORT97,
  PORT100, PORT101, PORT102, ..., PORT107,
   ...


No PORT08, PORT09, ...
As I said above, only the one's place is octal.




-- 
Best Regards
Masahiro Yamada