| 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
| ||
|
Message-ID: <4b82e393-093f-4477-bf0f-ee559a3b97c1@oss.nxp.com>
Date: Fri, 1 Nov 2024 11:29:24 +0200
From: Ciprian Marian Costea <ciprianmarian.costea@....nxp.com>
To: Frank Li <Frank.li@....com>
Cc: Alexandre Belloni <alexandre.belloni@...tlin.com>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Catalin Marinas
<catalin.marinas@....com>, Will Deacon <will@...nel.org>,
linux-rtc@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
NXP S32 Linux Team <s32@....com>, Christophe Lizzi <clizzi@...hat.com>,
Alberto Ruiz <aruizrui@...hat.com>, Enric Balletbo <eballetb@...hat.com>,
Bogdan-Gabriel Roman <bogdan-gabriel.roman@....com>,
Ghennadi Procopciuc <ghennadi.procopciuc@....com>,
IMX Review List <imx@...ts.linux.dev>
Subject: Re: [PATCH v3 1/4] dt-bindings: rtc: add schema for NXP S32G2/S32G3
SoCs
On 10/31/2024 6:39 PM, Frank Li wrote:
Hello Frank,
Thank you for your review!
> On Thu, Oct 31, 2024 at 10:35:54AM +0200, Ciprian Costea wrote:
>> From: Ciprian Marian Costea <ciprianmarian.costea@....nxp.com>
>>
>> This patch adds the dt-bindings for NXP S32G2/S32G3 SoCs RTC driver.
>>
>> Co-developed-by: Bogdan-Gabriel Roman <bogdan-gabriel.roman@....com>
>> Signed-off-by: Bogdan-Gabriel Roman <bogdan-gabriel.roman@....com>
>> Co-developed-by: Ghennadi Procopciuc <ghennadi.procopciuc@....com>
>> Signed-off-by: Ghennadi Procopciuc <ghennadi.procopciuc@....com>
>> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@....nxp.com>
>> ---
>
> next time you can cc imx@...ts.linux.dev
Thanks for your suggestion, I will start adding this list.
>
>> .../devicetree/bindings/rtc/nxp,s32g-rtc.yaml | 99 +++++++++++++++++++
>> 1 file changed, 99 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml b/Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml
>> new file mode 100644
>> index 000000000000..3694af883dc7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml
>> @@ -0,0 +1,99 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/rtc/nxp,s32g-rtc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: NXP S32G2/S32G3 Real Time Clock (RTC)
>> +
>> +maintainers:
>> + - Bogdan Hamciuc <bogdan.hamciuc@....com>
>> + - Ciprian Marian Costea <ciprianmarian.costea@....com>
>> +
>> +properties:
>> + compatible:
>> + oneOf:
>> + - enum:
>> + - nxp,s32g2-rtc
>> + - items:
>> + - const: nxp,s32g3-rtc
>> + - const: nxp,s32g2-rtc
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + "#clock-cells":
>> + const: 1
>
> Does your RTC is clock provider? why need #clock-cells
>
RTC clocking on S32G2/S32G3 has a clock mux for selecting between up to
4 different clock sources (parents).
Now, as suggested in previous reviews, I've switched to using the CCF in
order to implement this hardware particularity. In the end I've resorted
to implementing 'assigned-*' approach which while not mandatory as per
proposed bindings I find it quite scalable in selecting clock sources
for the RTC module compared to the first iteration (V1) of this patchset.
>> +
>> + clocks:
>> + items:
>> + - description: ipg clock drives the access to the
>> + RTC iomapped registers
>> +
>> + clock-names:
>> + items:
>> + - const: ipg
>> +
>> + assigned-clocks:
>> + minItems: 1
>> + items:
>> + - description: Runtime clock source. It must be a clock
>> + source for the RTC module. It will be disabled by hardware
>> + during Standby/Suspend.
>> + - description: Standby/Suspend clock source. It is optional
>> + and can be used in case the RTC will continue ticking during
>> + platform/system suspend. RTC hardware module contains a
>> + hardware mux for clock source selection.
>> +
>> + assigned-clock-parents:
>> + description: List of phandles to each parent clock.
>> +
>> + assigned-clock-rates:
>> + description: List of frequencies for RTC clock sources.
>> + RTC module contains 2 hardware divisors which can be
>> + enabled or not. Hence, available frequencies are the following
>> + parent_freq, parent_freq / 512, parent_freq / 32 or
>> + parent_freq / (512 * 32)
>
> Needn't assigned-*
>
'assigned-*' entries are not required, but based on my previous answer I
would prefer to document them in order to instruct further S32 SoC based
boards which may use this driver.
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> + - "#clock-cells"
>> + - clocks
>> + - clock-names
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> + rtc0: rtc@...60000 {
>
> needn't label
Thanks. I will remove the label in V4.
>
>> + compatible = "nxp,s32g3-rtc",
>> + "nxp,s32g2-rtc";
>> + reg = <0x40060000 0x1000>;
>> + interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
>> + #clock-cells = <1>;
>> + clocks = <&clks 54>;
>> + clock-names = "ipg";
>> + /*
>> + * Configuration of default parent clocks.
>> + * 'assigned-clocks' 0-3 IDs are Runtime clock sources
>> + * 4-7 IDs are Suspend/Standby clock sources.
>> + */
>> + assigned-clocks = <&rtc0 2>, <&rtc0 4>;
>> + assigned-clock-parents = <&clks 56>, <&clks 55>;
>> + /*
>> + * Clock frequency can be divided by value
>> + * 512 or 32 (or both) via hardware divisors.
>> + * Below configuration:
>> + * Runtime clock source: FIRC (51 MHz) / 512 (DIV512)
>> + * Suspend/Standby clock source: SIRC (32 KHz)
>> + */
>> + assigned-clock-rates = <99609>, <32000>;
>> + };
>> --
>> 2.45.2
>>
Powered by blists - more mailing lists