linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800
@ 2024-01-08  7:22 Jingbao Qiu
  2024-01-08  7:22 ` [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC Jingbao Qiu
  2024-01-08  8:02 ` [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800 Krzysztof Kozlowski
  0 siblings, 2 replies; 16+ messages in thread
From: Jingbao Qiu @ 2024-01-08  7:22 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, paul.walmsley, palmer, aou
  Cc: linux-rtc, devicetree, linux-kernel

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/20231229090643.116575-1-qiujingbao.dlmu@gmail.com/

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/20231226100431.331616-1-qiujingbao.dlmu@gmail.com/

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/20231217110952.78784-1-qiujingbao.dlmu@gmail.com/

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/20231121094642.2973795-1-qiujingbao.dlmu@gmail.com/

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC
  2024-01-08  7:22 [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800 Jingbao Qiu
@ 2024-01-08  7:22 ` Jingbao Qiu
  2024-01-08  8:04   ` Krzysztof Kozlowski
  2024-01-08  8:02 ` [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800 Krzysztof Kozlowski
  1 sibling, 1 reply; 16+ messages in thread
From: Jingbao Qiu @ 2024-01-08  7:22 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, paul.walmsley, palmer, aou
  Cc: linux-rtc, devicetree, linux-kernel

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#
+
+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>;
+    };
+...
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800
  2024-01-08  7:22 [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800 Jingbao Qiu
  2024-01-08  7:22 ` [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC Jingbao Qiu
@ 2024-01-08  8:02 ` Krzysztof Kozlowski
  2024-01-08  9:00   ` Jingbao Qiu
  1 sibling, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-08  8:02 UTC (permalink / raw)
  To: Jingbao Qiu, a.zummo, alexandre.belloni, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, paul.walmsley, palmer, aou
  Cc: linux-rtc, devicetree, linux-kernel

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/20231229090643.116575-1-qiujingbao.dlmu@gmail.com/
> 
> 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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC
  2024-01-08  7:22 ` [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC Jingbao Qiu
@ 2024-01-08  8:04   ` Krzysztof Kozlowski
  2024-01-08  9:10     ` Jingbao Qiu
  0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-08  8:04 UTC (permalink / raw)
  To: Jingbao Qiu, a.zummo, alexandre.belloni, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, paul.walmsley, palmer, aou
  Cc: linux-rtc, devicetree, linux-kernel

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800
  2024-01-08  8:02 ` [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800 Krzysztof Kozlowski
@ 2024-01-08  9:00   ` Jingbao Qiu
  2024-01-08  9:06     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 16+ messages in thread
From: Jingbao Qiu @ 2024-01-08  9:00 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, paul.walmsley, palmer, aou, linux-rtc, devicetree,
	linux-kernel

On Mon, Jan 8, 2024 at 4:02 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> 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/20231229090643.116575-1-qiujingbao.dlmu@gmail.com/
> >
> > 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/202312271350242a208426@mail.local/

Best regards,
Jingbao Qiu

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800
  2024-01-08  9:00   ` Jingbao Qiu
@ 2024-01-08  9:06     ` Krzysztof Kozlowski
  2024-01-08  9:11       ` Jingbao Qiu
  0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-08  9:06 UTC (permalink / raw)
  To: Jingbao Qiu
  Cc: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, paul.walmsley, palmer, aou, linux-rtc, devicetree,
	linux-kernel

On 08/01/2024 10:00, Jingbao Qiu wrote:
> On Mon, Jan 8, 2024 at 4:02 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> 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/20231229090643.116575-1-qiujingbao.dlmu@gmail.com/
>>>
>>> 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/202312271350242a208426@mail.local/

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

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC
  2024-01-08  8:04   ` Krzysztof Kozlowski
@ 2024-01-08  9:10     ` Jingbao Qiu
  2024-01-08  9:28       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 16+ messages in thread
From: Jingbao Qiu @ 2024-01-08  9:10 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, paul.walmsley, palmer, aou, linux-rtc, devicetree,
	linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800
  2024-01-08  9:06     ` Krzysztof Kozlowski
@ 2024-01-08  9:11       ` Jingbao Qiu
  2024-01-08  9:28         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 16+ messages in thread
From: Jingbao Qiu @ 2024-01-08  9:11 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, paul.walmsley, palmer, aou, linux-rtc, devicetree,
	linux-kernel

On Mon, Jan 8, 2024 at 5:06 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 08/01/2024 10:00, Jingbao Qiu wrote:
> > On Mon, Jan 8, 2024 at 4:02 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> 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/20231229090643.116575-1-qiujingbao.dlmu@gmail.com/
> >>>
> >>> 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/202312271350242a208426@mail.local/
>
> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC
  2024-01-08  9:10     ` Jingbao Qiu
@ 2024-01-08  9:28       ` Krzysztof Kozlowski
  2024-01-08 13:47         ` Jingbao Qiu
  0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-08  9:28 UTC (permalink / raw)
  To: Jingbao Qiu
  Cc: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, paul.walmsley, palmer, aou, linux-rtc, devicetree,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800
  2024-01-08  9:11       ` Jingbao Qiu
@ 2024-01-08  9:28         ` Krzysztof Kozlowski
  2024-01-08 13:42           ` Jingbao Qiu
  0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-08  9:28 UTC (permalink / raw)
  To: Jingbao Qiu
  Cc: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, paul.walmsley, palmer, aou, linux-rtc, devicetree,
	linux-kernel

On 08/01/2024 10:11, Jingbao Qiu wrote:
> On Mon, Jan 8, 2024 at 5:06 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 08/01/2024 10:00, Jingbao Qiu wrote:
>>> On Mon, Jan 8, 2024 at 4:02 PM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> 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/20231229090643.116575-1-qiujingbao.dlmu@gmail.com/
>>>>>
>>>>> 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/202312271350242a208426@mail.local/
>>
>> 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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800
  2024-01-08  9:28         ` Krzysztof Kozlowski
@ 2024-01-08 13:42           ` Jingbao Qiu
  0 siblings, 0 replies; 16+ messages in thread
From: Jingbao Qiu @ 2024-01-08 13:42 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, paul.walmsley, palmer, aou, linux-rtc, devicetree,
	linux-kernel

On Mon, Jan 8, 2024 at 5:28 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 08/01/2024 10:11, Jingbao Qiu wrote:
> > On Mon, Jan 8, 2024 at 5:06 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 08/01/2024 10:00, Jingbao Qiu wrote:
> >>> On Mon, Jan 8, 2024 at 4:02 PM Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@linaro.org> 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/20231229090643.116575-1-qiujingbao.dlmu@gmail.com/
> >>>>>
> >>>>> 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/202312271350242a208426@mail.local/
> >>
> >> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC
  2024-01-08  9:28       ` Krzysztof Kozlowski
@ 2024-01-08 13:47         ` Jingbao Qiu
  2024-01-08 15:24           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 16+ messages in thread
From: Jingbao Qiu @ 2024-01-08 13:47 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, paul.walmsley, palmer, aou, linux-rtc, devicetree,
	linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC
  2024-01-08 13:47         ` Jingbao Qiu
@ 2024-01-08 15:24           ` Krzysztof Kozlowski
  2024-01-09  2:26             ` Jingbao Qiu
  0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-08 15:24 UTC (permalink / raw)
  To: Jingbao Qiu
  Cc: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, paul.walmsley, palmer, aou, linux-rtc, devicetree,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC
  2024-01-08 15:24           ` Krzysztof Kozlowski
@ 2024-01-09  2:26             ` Jingbao Qiu
  2024-01-09  8:02               ` Krzysztof Kozlowski
  0 siblings, 1 reply; 16+ messages in thread
From: Jingbao Qiu @ 2024-01-09  2:26 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, paul.walmsley, palmer, aou, linux-rtc, devicetree,
	linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC
  2024-01-09  2:26             ` Jingbao Qiu
@ 2024-01-09  8:02               ` Krzysztof Kozlowski
  2024-01-09  9:51                 ` Jingbao Qiu
  0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-09  8:02 UTC (permalink / raw)
  To: Jingbao Qiu
  Cc: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, paul.walmsley, palmer, aou, linux-rtc, devicetree,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC
  2024-01-09  8:02               ` Krzysztof Kozlowski
@ 2024-01-09  9:51                 ` Jingbao Qiu
  0 siblings, 0 replies; 16+ messages in thread
From: Jingbao Qiu @ 2024-01-09  9:51 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, paul.walmsley, palmer, aou, linux-rtc, devicetree,
	linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-01-09  9:52 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-08  7:22 [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800 Jingbao Qiu
2024-01-08  7:22 ` [PATCH v5 1/1] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC Jingbao Qiu
2024-01-08  8:04   ` Krzysztof Kozlowski
2024-01-08  9:10     ` Jingbao Qiu
2024-01-08  9:28       ` Krzysztof Kozlowski
2024-01-08 13:47         ` Jingbao Qiu
2024-01-08 15:24           ` Krzysztof Kozlowski
2024-01-09  2:26             ` Jingbao Qiu
2024-01-09  8:02               ` Krzysztof Kozlowski
2024-01-09  9:51                 ` Jingbao Qiu
2024-01-08  8:02 ` [PATCH v5 0/1] dt-bindings: riscv: sophgo: add RTC for CV1800 Krzysztof Kozlowski
2024-01-08  9:00   ` Jingbao Qiu
2024-01-08  9:06     ` Krzysztof Kozlowski
2024-01-08  9:11       ` Jingbao Qiu
2024-01-08  9:28         ` Krzysztof Kozlowski
2024-01-08 13:42           ` Jingbao Qiu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).