| Message ID | 20240108072253.30183-2-qiujingbao.dlmu@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | dt-bindings: riscv: sophgo: add RTC for CV1800 | expand |
On 08/01/2024 08:22, Jingbao Qiu wrote: > Add RTC devicetree binding for Sophgo CV1800 SoC. > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com> > --- > .../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? > + > +maintainers: > + - Jingbao Qiu <qiujingbao.dlmu@gmail.com> > + > +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. > + > + reg: > + maxItems: 1 Best regards, Krzysztof
On Mon, Jan 8, 2024 at 4:04 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 08/01/2024 08:22, Jingbao Qiu wrote: > > Add RTC devicetree binding for Sophgo CV1800 SoC. > > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com> > > --- > > .../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. > > > + > > +maintainers: > > + - Jingbao Qiu <qiujingbao.dlmu@gmail.com> > > + > > +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. So I wrote the following in the changelog. - add syscon attribute to share registers with POR Best regards, Jingbao Qiu
On 08/01/2024 10:10, Jingbao Qiu wrote: > On Mon, Jan 8, 2024 at 4:04 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 08/01/2024 08:22, Jingbao Qiu wrote: >>> Add RTC devicetree binding for Sophgo CV1800 SoC. >>> >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>> Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com> >>> --- >>> .../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@gmail.com> >>> + >>> +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
On Mon, Jan 8, 2024 at 5:28 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 08/01/2024 10:10, Jingbao Qiu wrote: > > On Mon, Jan 8, 2024 at 4:04 PM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 08/01/2024 08:22, Jingbao Qiu wrote: > >>> Add RTC devicetree binding for Sophgo CV1800 SoC. > >>> > >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >>> Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com> > >>> --- > >>> .../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. > Thank you for your reply. I will adjust their order. > > > >> > >>> + > >>> +maintainers: > >>> + - Jingbao Qiu <qiujingbao.dlmu@gmail.com> > >>> + > >>> +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. I will explain in the next version of DTS. Thank you again for your patient reply. Best regards, Jingbao Qiu
On 08/01/2024 14:47, Jingbao Qiu wrote: >>> 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. > > I will explain in the next version of DTS. > Thank you again for your patient reply. You added some syscon attribute. What is this? Best regards, Krzysztof
On Mon, Jan 8, 2024 at 11:24 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 08/01/2024 14:47, Jingbao Qiu wrote: > >>> 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. > > > > I will explain in the next version of DTS. > > Thank you again for your patient reply. > > You added some syscon attribute. What is this? > This RTC device has a POR submodule, which is explained in the description. The corresponding driver of the POR submodule provides power off restart function. The driver of the POR submodule just uses reg to work.As you mentioned in your last comment.POR is empty, so there is little point in having it as subnode. we need share the reg to POR. RTC driver and POR driver will access this address simultaneously. so,I added this syscon attribute. Best regards, Jingbao Qiu
On 09/01/2024 03:26, Jingbao Qiu wrote: > On Mon, Jan 8, 2024 at 11:24 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 08/01/2024 14:47, Jingbao Qiu wrote: >>>>> 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. >>> >>> I will explain in the next version of DTS. >>> Thank you again for your patient reply. >> >> You added some syscon attribute. What is this? >> > > This RTC device has a POR submodule, which is explained in the description. > The corresponding driver of the POR submodule provides power off > restart function. > The driver of the POR submodule just uses reg to work.As you mentioned in your > last comment.POR is empty, so there is little point in having it as > subnode. we need > share the reg to POR. RTC driver and POR driver will access this > address simultaneously. > so,I added this syscon attribute. Nothing from above explains what is "syscon attribute", but if you cannot explain it, at least point me to where did you add this syscon attribute? Changelog said you added it. Where? Best regards, Krzysztof
On Tue, Jan 9, 2024 at 4:02 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 09/01/2024 03:26, Jingbao Qiu wrote: > > On Mon, Jan 8, 2024 at 11:24 PM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 08/01/2024 14:47, Jingbao Qiu wrote: > >>>>> 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. > >>> > >>> I will explain in the next version of DTS. > >>> Thank you again for your patient reply. > >> > >> You added some syscon attribute. What is this? > >> > > > > This RTC device has a POR submodule, which is explained in the description. > > The corresponding driver of the POR submodule provides power off > > restart function. > > The driver of the POR submodule just uses reg to work.As you mentioned in your > > last comment.POR is empty, so there is little point in having it as > > subnode. we need > > share the reg to POR. RTC driver and POR driver will access this > > address simultaneously. > > so,I added this syscon attribute. > > Nothing from above explains what is "syscon attribute", but if you > cannot explain it, at least point me to where did you add this syscon > attribute? Changelog said you added it. Where? > Thank you for your comment. I will add it in the next version. Best regards, Jingbao Qiu
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# + +maintainers: + - Jingbao Qiu <qiujingbao.dlmu@gmail.com> + +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 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + maxItems: 1 + +required: + - compatible + - reg + - interrupts + - clocks + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + rtc@5025000 { + compatible = "sophgo,cv1800-rtc", "syscon"; + reg = <0x5025000 0x2000>; + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk 15>; + }; +...