linux-kernel - Re: [PATCH 1/2] riscv: dts: cv1800b: add pinctrl node for cv1800b

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: <40f003c9-a94b-4ded-b35b-be4985aba3a2@sifive.com>
Date:   Mon, 13 Nov 2023 08:49:27 -0500
From:   Samuel Holland <samuel.holland@...ive.com>
To:     Conor Dooley <conor@...nel.org>, Jisheng Zhang <jszhang@...nel.org>
Cc:     Tony Lindgren <tony@...mide.com>, devicetree@...r.kernel.org,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Chao Wei <chao.wei@...hgo.com>,
        Chen Wang <unicorn_wang@...look.com>
Subject: Re: [PATCH 1/2] riscv: dts: cv1800b: add pinctrl node for cv1800b

Hi Conor,

On 2023-11-13 7:29 AM, Conor Dooley wrote:
> On Mon, Nov 13, 2023 at 09:03:11PM +0800, Jisheng Zhang wrote:
>> On Sun, Nov 12, 2023 at 08:51:20PM -0500, Samuel Holland wrote:
>>> On 2023-11-12 6:57 PM, Jisheng Zhang wrote:
>>>> Add the reset device tree node to cv1800b SoC reusing the
>>>           ^^^^^
>>>           I assume you mean pinctrl here?
>>
>> oops copy and paste the commit msg ;) thanks
>>>
>>>> pinctrl-single driver.
>>>>
>>>> Signed-off-by: Jisheng Zhang <jszhang@...nel.org>
>>>> ---
>>>>  arch/riscv/boot/dts/sophgo/cv-pinctrl.h | 19 +++++++++++++++++++
>>>>  arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 10 ++++++++++
>>>>  2 files changed, 29 insertions(+)
>>>>  create mode 100644 arch/riscv/boot/dts/sophgo/cv-pinctrl.h
>>>>
>>>> diff --git a/arch/riscv/boot/dts/sophgo/cv-pinctrl.h b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h
>>>> new file mode 100644
>>>> index 000000000000..ed78b6fb3142
>>>> --- /dev/null
>>>> +++ b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h
>>>
>>> A couple of questions: Should this go in include/dt-bindings? And is it worth
>>
>> When I cooked this series two weeks ago, I did put it in dt-binding, but
>> then I found commit fe49f2d776f799 ("arm64: dts: ti: Use local header for
>> pinctrl register values"), "These definitions were previously put in the
>> bindings header to avoid code duplication and to provide some context
>> meaning (name), but they do not fit the purpose of bindings." which is
>> suggested and acked by Krzysztof, so I just want to follow the style
>> here.
>>
>>
>>> including macros for the actual function mappings, like in the vendor source[1]?
>>
>> Do you want something as the following?
>>
>> #define UART0_TX	0
>> #define CAM_MCLK1	1
>> ...
>>
>> #define REG_UART0_TX	0x24
>> ...
>>
>> pinctrl-single,pins = <REG_UART0_TX UART0_TX>;
>>
>> Other pinctl-single users just uses the register value directly, I have
>> no preference. But I'd like to get suggestions from DT and pinctl-single
>> maintainers. Hi Rob, Krzysztof, Conor, Tony, what's your opinion?
> 
> Basically, if the definitions map directly to registers and are just
> used to make writing your devicetree easier then they do not belong
> in a binding. This differs from clock or reset indices, where we
> essentially make up a set of indices that may or may not correlate to
> offsets in the hardware as using the register values without any sort of
> abstraction is not defining an ABI.

Right. I should have remembered this policy :)

Regards,
Samuel