linux-kernel - Re: [PATCH v1 5/5] riscv: dts: microchip: add a device tree for Discovery Kit

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: <e8553f7e-0d86-4236-a071-bb3a1a70140d@microchip.com>
Date: Tue, 2 Sep 2025 08:39:53 +0000
From: <Valentina.FernandezAlanis@...rochip.com>
To: <krzk@...nel.org>, <Conor.Dooley@...rochip.com>,
	<Daire.McNamara@...rochip.com>, <paul.walmsley@...ive.com>,
	<palmer@...belt.com>, <robh@...nel.org>, <krzk+dt@...nel.org>,
	<aou@...s.berkeley.edu>, <alex@...ti.fr>
CC: <linux-riscv@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
	<devicetree@...r.kernel.org>
Subject: Re: [PATCH v1 5/5] riscv: dts: microchip: add a device tree for
 Discovery Kit

On 02/09/2025 07:22, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 01/09/2025 17:28, Valentina.FernandezAlanis@...rochip.com wrote:
>> On 28/08/2025 18:46, Krzysztof Kozlowski wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 25/08/2025 18:19, Valentina Fernandez wrote:
>>>> +++ b/arch/riscv/boot/dts/microchip/mpfs-disco-kit-fabric.dtsi
>>>> @@ -0,0 +1,58 @@
>>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>>> +/* Copyright (c) 2020-2025 Microchip Technology Inc */
>>>> +
>>>> +/ {
>>>> +     core_pwm0: pwm@...00000 {
>>>> +             compatible = "microchip,corepwm-rtl-v4";
>>>> +             reg = <0x0 0x40000000 0x0 0xF0>;
>>>> +             microchip,sync-update-mask = /bits/ 32 <0>;
>>>> +             #pwm-cells = <3>;
>>>> +             clocks = <&ccc_sw CLK_CCC_PLL0_OUT3>;
>>>> +             status = "disabled";
>>>> +     };
>>>> +
>>>> +     i2c2: i2c@...00200 {
>>>> +             compatible = "microchip,corei2c-rtl-v7";
>>>> +             reg = <0x0 0x40000200 0x0 0x100>;
>>>> +             #address-cells = <1>;
>>>> +             #size-cells = <0>;
>>>> +             clocks = <&ccc_sw CLK_CCC_PLL0_OUT3>;
>>>> +             interrupt-parent = <&plic>;
>>>> +             interrupts = <122>;
>>>> +             clock-frequency = <100000>;
>>>> +             status = "disabled";
>>>> +     };
>>>> +
>>>> +     ihc: mailbox {
>>>> +             compatible = "microchip,sbi-ipc";
>>>> +             interrupt-parent = <&plic>;
>>>> +             interrupts = <180>, <179>, <178>, <177>;
>>>> +             interrupt-names = "hart-1", "hart-2", "hart-3", "hart-4";
>>>> +             #mbox-cells = <1>;
>>>> +             status = "disabled";
>>>> +     };
>>>> +
>>>> +     mailbox@...00000 {
>>>> +             compatible = "microchip,miv-ihc-rtl-v2";
>>>> +             microchip,ihc-chan-disabled-mask = /bits/ 16 <0>;
>>>
>>> Does not look like following DTS coding style - order of properties.
>>>
>>>> +             reg = <0x0 0x50000000 0x0 0x1c000>;
>>>> +             interrupt-parent = <&plic>;
>>>> +             interrupts = <180>, <179>, <178>, <177>;
>>>> +             interrupt-names = "hart-1", "hart-2", "hart-3", "hart-4";
>>>> +             #mbox-cells = <1>;
>>>> +             status = "disabled";
>>>> +     };
>>>> +
>>>> +     refclk_ccc: cccrefclk {
>>>
>>> Please use name for all fixed clocks which matches current format
>>> recommendation: 'clock-<freq>' (see also the pattern in the binding for
>>> any other options).
>>>
>>> https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/fixed-clock.yaml
>> The fabric dtsi describes elements configured by the FPGA bitstream.
>> This node is named as such because the Clock Conditioner Circuit CCC's
>> reference clock source is set by the FPGA bitstream, while its frequency
>> is determined by an on-board oscillator.
>>
>> Hope this clarifies the rationale behind the node name.
> No, because there is no style naming clocks like this. Neither proper
> suffix, nor prefix. Use standard naming.
> 
> And all other comments you ignored?
I sent a v2 with the rest of the comments addressed. I didn't notice you 
were still not happy with the clock node name, please ignore the v2.

> 
> Best regards,
> Krzysztof