linux-kernel - Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions

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]

Message-ID: <3db802d6-114f-097a-6c69-e7b40e4d2764@starfivetech.com>
Date:   Tue, 29 Nov 2022 22:46:54 +0800
From:   Jianlong Huang <jianlong.huang@...rfivetech.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Hal Feng <hal.feng@...rfivetech.com>,
        <linux-riscv@...ts.infradead.org>, <devicetree@...r.kernel.org>,
        <linux-gpio@...r.kernel.org>
CC:     Conor Dooley <conor@...nel.org>,
        Palmer Dabbelt <palmer@...belt.com>,
        "Rob Herring" <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Emil Renner Berthing <emil.renner.berthing@...onical.com>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl
 definitions

On Tue, 29 Nov 2022 08:49:49 +0100, Krzysztof Kozlowski wrote:
> On 29/11/2022 02:47, Jianlong Huang wrote:
>> On Mon, 28 Nov 2022 09:32:45 +0100, Krzysztof Kozlowski wrote:
>>> On 28/11/2022 01:48, Jianlong Huang wrote:
>>>
>>>>>>> +/* aon_iomux doen */
>>>>>>> +#define GPOEN_AON_PTC0_OE_N_4			2
>>>>>>> +#define GPOEN_AON_PTC0_OE_N_5			3
>>>>>>> +#define GPOEN_AON_PTC0_OE_N_6			4
>>>>>>> +#define GPOEN_AON_PTC0_OE_N_7			5
>>>>>>> +
>>>>>>
>>>>>> It looks like you add register constants to the bindings. Why? The
>>>>>> bindings are not the place to represent hardware programming model. Not
>>>>>> mentioning that there is no benefit in this.
>>>>>
>>>>> Also: this entire file should be dropped, but if it stays, you have to
>>>>> name it matching bindings or compatible (vendor,device.h).
>>>>
>>>> Thanks your comments.
>>>> These macros are used to configure pinctrl in dts, so the file should stay,
>>>
>>> Why they should stay? What's the reason? If it is not a constant used by
>>> driver, then register values should not be placed in the bindings, so
>>> drop it.
>>>
>> 
>> Thanks.
>> 
>> These macros in binding header(example, DOUT, DOEN etc) will be used in DTS,
>> and driver will parse the DT for pinctrl configuration.
>> 
>> Example in dts:
>> uart0_pins: uart0-0 {
>> 	tx-pins {
>> 		pinmux = <GPIOMUX(5, GPOUT_SYS_UART0_TX, GPOEN_ENABLE, GPI_NONE)>;
> 
> This is usage in DTS and is not an argument to store register
> addresses/offsets as bindings. What is the usage (of define, not value)
> in the driver?
> 

The existing implementation reuse the macros for DTS and driver.
Do you mean we need to separate the macros, one for DTS and one for driver usage?
Or you have any better suggestion?

These macros are the value of register, not register addresses/offsets,
except for with prefix of GPI.

Drivers rarely reference macros directly, mostly parsing dts and writing them to registers.
 
Best regards,
Jianlong Huang