On 08/01/2024 10:10, Jingbao Qiu wrote: > On Mon, Jan 8, 2024 at 4:04 PM Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: >> >> On 08/01/2024 08:22, Jingbao Qiu wrote: >>> Add RTC devicetree binding for Sophgo CV1800 SoC. >>> >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> >>> Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@xxxxxxxxx> >>> --- >>> .../bindings/rtc/sophgo,cv1800-rtc.yaml | 56 +++++++++++++++++++ >>> 1 file changed, 56 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml b/Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml >>> new file mode 100644 >>> index 000000000000..01a926cb5c81 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml >>> @@ -0,0 +1,56 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/rtc/sophgo,cv1800-rtc.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Real Time Clock of the Sophgo CV1800 SoC >>> + >>> +allOf: >>> + - $ref: rtc.yaml# >> >> Why the allOf has moved? > > Hi, > Do you mean allof should be under maintainers? Or other meanings. Yes and the patch which got my review had it correctly placed. > >> >>> + >>> +maintainers: >>> + - Jingbao Qiu <qiujingbao.dlmu@xxxxxxxxx> >>> + >>> +description: >>> + Real Time Clock (RTC) is an independently powered module >>> + within the chip, which includes a 32KHz oscillator and a >>> + Power On Reset/POR submodule. It can be used for time display >>> + and timed alarm generation. In addition, the hardware state >>> + machine provides triggering and timing control for chip >>> + power on, off, and reset. >>> + >>> +properties: >>> + compatible: >>> + items: >>> + - const: sophgo,cv1800-rtc >>> + - const: syscon >> >> Why is this syscon? Description does not explain this. > > Because the driver of the submodule POR in RTC only requires register > address and range to work, according to what you said, it is only a compatible > attribute and does not need to be a child node. What child node has anything to do with syscon? Anyway I don't understand that. > > So I wrote the following in the changelog. > > - add syscon attribute to share registers > with POR Where is this syscon attribute? Please point me to specific line in DTS and in the driver. Best regards, Krzysztof