2024-01-08 07:23:05

by Jingbao Qiu

[permalink] [raw]
Subject: [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800

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.

Power On Reset/POR submodule only using register resources
so it should be empty. The 32KHz oscillator only provides
pulses for RTC in hardware.


Changes since v4:
- remove POR dt-bindings because it empty
- remove MFD dt-bindings because SoC does
not have MFDs
- add syscon attribute to share registers
with POR

v4: https://lore.kernel.org/all/[email protected]/

Changes since v3:
- temporarily not submitting RTC driver code
waiting for communication with IC designer
- add MFD dt-bindings
- add POR dt-bindings

v3: https://lore.kernel.org/all/[email protected]/

Changes since v2:
- add mfd support for CV1800
- add rtc to mfd
- using regmap replace iomap
- merge register address in dts

v2: https://lore.kernel.org/lkml/[email protected]/

Changes since v1
- fix duplicate names in subject
- using RTC replace RTC controller
- improve the properties of dt-bindings
- using `unevaluatedProperties` replace `additionalProperties`
- dt-bindings passed the test
- using `devm_platform_ioremap_resource()` replace
`platform_get_resource()` and `devm_ioremap_resource()`
- fix random order of the code
- fix wrong wrapping of the `devm_request_irq()` and map the flag with dts
- using devm_clk_get_enabled replace `devm_clk_get()` and
`clk_prepare_enable()`
- fix return style
- add rtc clock calibration function
- use spinlock when write register on read/set time

v1: https://lore.kernel.org/lkml/[email protected]/

Jingbao Qiu (1):
dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

.../bindings/rtc/sophgo,cv1800-rtc.yaml | 56 +++++++++++++++++++
1 file changed, 56 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml


base-commit: 92c255c7157a07614f3e1df4eb63fbd49bc738e0
--
2.43.0



2024-01-08 07:23:20

by Jingbao Qiu

[permalink] [raw]
Subject: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

Add RTC devicetree binding for Sophgo CV1800 SoC.

Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Jingbao Qiu <[email protected]>
---
.../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#
+
+maintainers:
+ - Jingbao Qiu <[email protected]>
+
+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>;
+ };
+...
--
2.43.0


2024-01-08 08:02:46

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800

On 08/01/2024 08:22, Jingbao Qiu wrote:
> 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.
>
> Power On Reset/POR submodule only using register resources
> so it should be empty. The 32KHz oscillator only provides
> pulses for RTC in hardware.
>
>
> Changes since v4:
> - remove POR dt-bindings because it empty
> - remove MFD dt-bindings because SoC does
> not have MFDs
> - add syscon attribute to share registers
> with POR
>
> v4: https://lore.kernel.org/all/[email protected]/
>
> Changes since v3:
> - temporarily not submitting RTC driver code
> waiting for communication with IC designer

Hm, why?

We do not need bindings if nothing matches to them. If this binding is
for other upstream open-source project, please provide references.

See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L61

Best regards,
Krzysztof


2024-01-08 08:05:00

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

On 08/01/2024 08:22, Jingbao Qiu wrote:
> Add RTC devicetree binding for Sophgo CV1800 SoC.
>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> Signed-off-by: Jingbao Qiu <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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


2024-01-08 09:02:48

by Jingbao Qiu

[permalink] [raw]
Subject: Re: [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800

On Mon, Jan 8, 2024 at 4:02 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 08/01/2024 08:22, Jingbao Qiu wrote:
> > 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.
> >
> > Power On Reset/POR submodule only using register resources
> > so it should be empty. The 32KHz oscillator only provides
> > pulses for RTC in hardware.
> >
> >
> > Changes since v4:
> > - remove POR dt-bindings because it empty
> > - remove MFD dt-bindings because SoC does
> > not have MFDs
> > - add syscon attribute to share registers
> > with POR
> >
> > v4: https://lore.kernel.org/all/[email protected]/
> >
> > Changes since v3:
> > - temporarily not submitting RTC driver code
> > waiting for communication with IC designer
>
> Hm, why?
>
> We do not need bindings if nothing matches to them. If this binding is
> for other upstream open-source project, please provide references.
>
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L61
>

Hi!

There is a function in the RTC driver code used to calibrate the
clock, which is define in the datasheet.
However, Alexandre Belloni raised concerns that clock calibration
should be done using GPS or similar
methods, rather than using other clock sources. I think what he said
makes sense. So it is necessary
to communicate with IC designers.

link: https://lore.kernel.org/all/[email protected]/

Best regards,
Jingbao Qiu

2024-01-08 09:07:20

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800

On 08/01/2024 10:00, Jingbao Qiu wrote:
> On Mon, Jan 8, 2024 at 4:02 PM Krzysztof Kozlowski
> <[email protected]> wrote:
>>
>> On 08/01/2024 08:22, Jingbao Qiu wrote:
>>> 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.
>>>
>>> Power On Reset/POR submodule only using register resources
>>> so it should be empty. The 32KHz oscillator only provides
>>> pulses for RTC in hardware.
>>>
>>>
>>> Changes since v4:
>>> - remove POR dt-bindings because it empty
>>> - remove MFD dt-bindings because SoC does
>>> not have MFDs
>>> - add syscon attribute to share registers
>>> with POR
>>>
>>> v4: https://lore.kernel.org/all/[email protected]/
>>>
>>> Changes since v3:
>>> - temporarily not submitting RTC driver code
>>> waiting for communication with IC designer
>>
>> Hm, why?
>>
>> We do not need bindings if nothing matches to them. If this binding is
>> for other upstream open-source project, please provide references.
>>
>> See also:
>> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L61
>>
>
> Hi!
>
> There is a function in the RTC driver code used to calibrate the
> clock, which is define in the datasheet.
> However, Alexandre Belloni raised concerns that clock calibration
> should be done using GPS or similar
> methods, rather than using other clock sources. I think what he said
> makes sense. So it is necessary
> to communicate with IC designers.
>
> link: https://lore.kernel.org/all/[email protected]/

Sure, this I get, but why sending bindings alone? There is no user of them.

Best regards,
Krzysztof


2024-01-08 09:11:11

by Jingbao Qiu

[permalink] [raw]
Subject: Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

On Mon, Jan 8, 2024 at 4:04 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 08/01/2024 08:22, Jingbao Qiu wrote:
> > Add RTC devicetree binding for Sophgo CV1800 SoC.
> >
> > Reviewed-by: Krzysztof Kozlowski <[email protected]>
> > Signed-off-by: Jingbao Qiu <[email protected]>
> > ---
> > .../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 <[email protected]>
> > +
> > +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

2024-01-08 09:12:16

by Jingbao Qiu

[permalink] [raw]
Subject: Re: [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800

On Mon, Jan 8, 2024 at 5:06 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 08/01/2024 10:00, Jingbao Qiu wrote:
> > On Mon, Jan 8, 2024 at 4:02 PM Krzysztof Kozlowski
> > <[email protected]> wrote:
> >>
> >> On 08/01/2024 08:22, Jingbao Qiu wrote:
> >>> 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.
> >>>
> >>> Power On Reset/POR submodule only using register resources
> >>> so it should be empty. The 32KHz oscillator only provides
> >>> pulses for RTC in hardware.
> >>>
> >>>
> >>> Changes since v4:
> >>> - remove POR dt-bindings because it empty
> >>> - remove MFD dt-bindings because SoC does
> >>> not have MFDs
> >>> - add syscon attribute to share registers
> >>> with POR
> >>>
> >>> v4: https://lore.kernel.org/all/[email protected]/
> >>>
> >>> Changes since v3:
> >>> - temporarily not submitting RTC driver code
> >>> waiting for communication with IC designer
> >>
> >> Hm, why?
> >>
> >> We do not need bindings if nothing matches to them. If this binding is
> >> for other upstream open-source project, please provide references.
> >>
> >> See also:
> >> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L61
> >>
> >
> > Hi!
> >
> > There is a function in the RTC driver code used to calibrate the
> > clock, which is define in the datasheet.
> > However, Alexandre Belloni raised concerns that clock calibration
> > should be done using GPS or similar
> > methods, rather than using other clock sources. I think what he said
> > makes sense. So it is necessary
> > to communicate with IC designers.
> >
> > link: https://lore.kernel.org/all/[email protected]/
>
> Sure, this I get, but why sending bindings alone? There is no user of them.
>

Thank you for your patient reply.
May I ask if this user refers to driver code or DTS?

Best regards,
Jingbao Qiu

2024-01-08 09:28:50

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

On 08/01/2024 10:10, Jingbao Qiu wrote:
> On Mon, Jan 8, 2024 at 4:04 PM Krzysztof Kozlowski
> <[email protected]> wrote:
>>
>> On 08/01/2024 08:22, Jingbao Qiu wrote:
>>> Add RTC devicetree binding for Sophgo CV1800 SoC.
>>>
>>> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>>> Signed-off-by: Jingbao Qiu <[email protected]>
>>> ---
>>> .../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 <[email protected]>
>>> +
>>> +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


2024-01-08 09:29:05

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800

On 08/01/2024 10:11, Jingbao Qiu wrote:
> On Mon, Jan 8, 2024 at 5:06 PM Krzysztof Kozlowski
> <[email protected]> wrote:
>>
>> On 08/01/2024 10:00, Jingbao Qiu wrote:
>>> On Mon, Jan 8, 2024 at 4:02 PM Krzysztof Kozlowski
>>> <[email protected]> wrote:
>>>>
>>>> On 08/01/2024 08:22, Jingbao Qiu wrote:
>>>>> 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.
>>>>>
>>>>> Power On Reset/POR submodule only using register resources
>>>>> so it should be empty. The 32KHz oscillator only provides
>>>>> pulses for RTC in hardware.
>>>>>
>>>>>
>>>>> Changes since v4:
>>>>> - remove POR dt-bindings because it empty
>>>>> - remove MFD dt-bindings because SoC does
>>>>> not have MFDs
>>>>> - add syscon attribute to share registers
>>>>> with POR
>>>>>
>>>>> v4: https://lore.kernel.org/all/[email protected]/
>>>>>
>>>>> Changes since v3:
>>>>> - temporarily not submitting RTC driver code
>>>>> waiting for communication with IC designer
>>>>
>>>> Hm, why?
>>>>
>>>> We do not need bindings if nothing matches to them. If this binding is
>>>> for other upstream open-source project, please provide references.
>>>>
>>>> See also:
>>>> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L61
>>>>
>>>
>>> Hi!
>>>
>>> There is a function in the RTC driver code used to calibrate the
>>> clock, which is define in the datasheet.
>>> However, Alexandre Belloni raised concerns that clock calibration
>>> should be done using GPS or similar
>>> methods, rather than using other clock sources. I think what he said
>>> makes sense. So it is necessary
>>> to communicate with IC designers.
>>>
>>> link: https://lore.kernel.org/all/[email protected]/
>>
>> Sure, this I get, but why sending bindings alone? There is no user of them.
>>
>
> Thank you for your patient reply.
> May I ask if this user refers to driver code or DTS?

Anything. Any user.

Best regards,
Krzysztof


2024-01-08 13:43:08

by Jingbao Qiu

[permalink] [raw]
Subject: Re: [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800

On Mon, Jan 8, 2024 at 5:28 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 08/01/2024 10:11, Jingbao Qiu wrote:
> > On Mon, Jan 8, 2024 at 5:06 PM Krzysztof Kozlowski
> > <[email protected]> wrote:
> >>
> >> On 08/01/2024 10:00, Jingbao Qiu wrote:
> >>> On Mon, Jan 8, 2024 at 4:02 PM Krzysztof Kozlowski
> >>> <[email protected]> wrote:
> >>>>
> >>>> On 08/01/2024 08:22, Jingbao Qiu wrote:
> >>>>> 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.
> >>>>>
> >>>>> Power On Reset/POR submodule only using register resources
> >>>>> so it should be empty. The 32KHz oscillator only provides
> >>>>> pulses for RTC in hardware.
> >>>>>
> >>>>>
> >>>>> Changes since v4:
> >>>>> - remove POR dt-bindings because it empty
> >>>>> - remove MFD dt-bindings because SoC does
> >>>>> not have MFDs
> >>>>> - add syscon attribute to share registers
> >>>>> with POR
> >>>>>
> >>>>> v4: https://lore.kernel.org/all/[email protected]/
> >>>>>
> >>>>> Changes since v3:
> >>>>> - temporarily not submitting RTC driver code
> >>>>> waiting for communication with IC designer
> >>>>
> >>>> Hm, why?
> >>>>
> >>>> We do not need bindings if nothing matches to them. If this binding is
> >>>> for other upstream open-source project, please provide references.
> >>>>
> >>>> See also:
> >>>> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L61
> >>>>
> >>>
> >>> Hi!
> >>>
> >>> There is a function in the RTC driver code used to calibrate the
> >>> clock, which is define in the datasheet.
> >>> However, Alexandre Belloni raised concerns that clock calibration
> >>> should be done using GPS or similar
> >>> methods, rather than using other clock sources. I think what he said
> >>> makes sense. So it is necessary
> >>> to communicate with IC designers.
> >>>
> >>> link: https://lore.kernel.org/all/[email protected]/
> >>
> >> Sure, this I get, but why sending bindings alone? There is no user of them.
> >>
> >
> > Thank you for your patient reply.
> > May I ask if this user refers to driver code or DTS?
>
> Anything. Any user.
>

Thank you for your suggestion. I will include DTS or all of it in the
next version.

Best regards,
Jingbao Qiu

2024-01-08 13:48:14

by Jingbao Qiu

[permalink] [raw]
Subject: Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

On Mon, Jan 8, 2024 at 5:28 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 08/01/2024 10:10, Jingbao Qiu wrote:
> > On Mon, Jan 8, 2024 at 4:04 PM Krzysztof Kozlowski
> > <[email protected]> wrote:
> >>
> >> On 08/01/2024 08:22, Jingbao Qiu wrote:
> >>> Add RTC devicetree binding for Sophgo CV1800 SoC.
> >>>
> >>> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> >>> Signed-off-by: Jingbao Qiu <[email protected]>
> >>> ---
> >>> .../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 <[email protected]>
> >>> +
> >>> +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

2024-01-08 15:24:17

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

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


2024-01-09 02:27:23

by Jingbao Qiu

[permalink] [raw]
Subject: Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

On Mon, Jan 8, 2024 at 11:24 PM Krzysztof Kozlowski
<[email protected]> 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

2024-01-09 08:02:53

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

On 09/01/2024 03:26, Jingbao Qiu wrote:
> On Mon, Jan 8, 2024 at 11:24 PM Krzysztof Kozlowski
> <[email protected]> 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


2024-01-09 10:11:59

by Jingbao Qiu

[permalink] [raw]
Subject: Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

On Tue, Jan 9, 2024 at 4:02 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 09/01/2024 03:26, Jingbao Qiu wrote:
> > On Mon, Jan 8, 2024 at 11:24 PM Krzysztof Kozlowski
> > <[email protected]> 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