linux-kernel - Re: [PATCH 0/5] pinctrl_gpio_get

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]

Message-Id: <1531351332.2021.2@smtp.crapouillou.net>
Date:   Thu, 12 Jul 2018 01:22:12 +0200
From:   Paul Cercueil <paul@...pouillou.net>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Andy Shevchenko <andy.shevchenko@...il.com>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes

Hi Linus,


Le lun. 9 juil. 2018 à 14:09, Linus Walleij <linus.walleij@...aro.org> 
a écrit :
> Hi folks,
> 
> On Wed, Jun 27, 2018 at 7:18 PM Andy Shevchenko
> <andy.shevchenko@...il.com> wrote:
> 
>>  Even if GPIO and pin muxing has only one set of buffers to indicate
>>  input or output (same registers in use) it's a GPIO driver business 
>> to
>>  get direction from GPIO part of IP.
>> 
>>  Looking into the existing code I would rather say that
>>  pinctrl-ingenic.c should incorporate gpio-ingenic.c as they are
>>  (partially) sharing same registers.
> 
> Usually we only split the functionality into two drivers if the two 
> features
> pin control and GPIO are explicitly in different hardware blocks,
> and typically not sharing the same memory range.
> 
> If these registers are intermingled and the hardware actually
> just one piece of silicon, I would suggest to try to merge the
> two drivers into a combined pin control and GPIO driver
> inside drivers/pinctrl/pinctrl-ingenic.c.
> 
> We have a few drivers like that already, good textbook
> examples of how to do this include
> drivers/pinctrl/pinctrl-sx150x.c where the two blocks are
> handled in one driver using both APIs.
> 
> Paul could you have a look at if we can simply merge these
> two into one big driver? It is much more natural to write
> into the same set of registers when we do that.

Well I wish you had told me that when I submitted the ingenic 
pinctrl/gpio
patchset :)

I won't have much time before 4.19-rc1, but I can have a look after 
that.

> If you still prefer to proceed with the GPIO/pinctrl as separate
> drivers we need to look into this patch set, which I am
> a bit ambivalent about, because it makes sense but at the
> same time I want to keep GPIO and pin control business
> separate because separation of concerns is just nice.

Well I can still implement the get_direction() function in the GPIO
driver by reading the registers instead of calling into pinctrl.

I just thought it felt illogic as set_direction() does that.

> Yours,
> Linus Walleij

Thanks,
-Paul