linux-kernel - Re: [PATCH 1/9] ARM: dts: exynos: Add macros for GPIO configuration

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:   Wed, 31 Aug 2016 15:07:00 +0200
From:   Krzysztof Kozlowski <k.kozlowski@...sung.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     linux-arm-kernel@...ts.infradead.org,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Kukjin Kim <kgene@...nel.org>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        devicetree@...r.kernel.org, linux-samsung-soc@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Javier Martinez Canillas <javier@....samsung.com>,
        Tomasz Figa <tomasz.figa@...il.com>,
        Sylwester Nawrocki <s.nawrocki@...sung.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Subject: Re: [PATCH 1/9] ARM: dts: exynos: Add macros for GPIO configuration

On 08/31/2016 02:58 PM, Arnd Bergmann wrote:
> On Wednesday, August 31, 2016 2:53:02 PM CEST Krzysztof Kozlowski wrote:
>> On 08/31/2016 02:42 PM, Arnd Bergmann wrote:
>>> On Wednesday, August 31, 2016 2:13:25 PM CEST Krzysztof Kozlowski wrote:
>>>>
>>>> +#define PIN_PULL_NONE          0
>>>> +#define PIN_PULL_DOWN          1
>>>> +#define PIN_PULL_UP            3
>>>> +
>>>> +#define PIN_DRV_LV1            0
>>>> +#define PIN_DRV_LV2            2
>>>> +#define PIN_DRV_LV3            1
>>>> +#define PIN_DRV_LV4            3
>>>> +
>>>> +#define PIN_FUNC_INPUT         0
>>>> +#define PIN_FUNC_OUTPUT                1
>>>> +#define PIN_FUNC_SPC_2         2
>>>> +#define PIN_FUNC_SPC_3         3
>>>> +#define PIN_FUNC_SPC_4         4
>>>> +#define PIN_FUNC_SPC_5         5
>>>> +#define PIN_FUNC_SPC_F         0xf
>>>
>>> Any reason for having a copy in each of those files instead of one
>>> that is shared across all of them?
>>
>> The drive strengths differ between some of them. There are three groups
>> of drive strengths:
>> 1. Exynos3250, Exynos4 (all) and Exynos5250,
>> 2. Exynos5260,
>> 3. Exynos5410, 542x and 5800.
> 
> I see. That sounds like an even stronger reason to not duplicate
> the definitions, as this is very confusing.

Okay.

> 
>> Rest (functions and pull up/down) is the same so sharing the defines is
>> possible but not that obvious. Solution would be for example adding a
>> SoC-family prefix for PIN_DRV_LVx. Not that good...
>>
>> I could put it into three DTSI:
>>  - exynos3-pinctrl.dtsi (new file)
>>  - exynos5260-pinctrl.dtsi (like it is now)
>>  - exynos54xx-pinctrl.dtsi (new file)
>>
>> which would reduce the duplication. Other ideas?
> 
> I think having the soc-family prefix is better, as it avoids
> defining the same symbol to a different value. Better make this
> as explicit as possible.

Ok, sounds reasonable. I want to convert also older platforms S3C (drive
strengths and pull up/down differ) and arm64 SoC: Exynos7. For the
latter the problem is there is no common place for sharing DTS, except
the headers. However this does not really belong to headers. I guess
some level of duplication might be still exist.


> I think overall, a better solution would have been to define the
> constants globally (shared with non-exynos) to start with,
> and have the driver translate generic numbers into vendor
> specific ones. Obviously it's too late for that now.

We could extend driver by adding new bindings accepting generic numbers
(and still backward compatible) but this looks like an overkill.

Best regards,
Krzysztof