mbox series

[v5,0/1] dt-bindings: riscv: sophgo: add RTC for CV1800

Message ID 20240108072253.30183-1-qiujingbao.dlmu@gmail.com
Headers show
Series dt-bindings: riscv: sophgo: add RTC for CV1800 | expand

Message

Jingbao Qiu Jan. 8, 2024, 7:22 a.m. UTC
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

Comments

Krzysztof Kozlowski Jan. 8, 2024, 8:02 a.m. UTC | #1
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
Jingbao Qiu Jan. 8, 2024, 9 a.m. UTC | #2
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
Krzysztof Kozlowski Jan. 8, 2024, 9:06 a.m. UTC | #3
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
Jingbao Qiu Jan. 8, 2024, 9:11 a.m. UTC | #4
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
Krzysztof Kozlowski Jan. 8, 2024, 9:28 a.m. UTC | #5
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
Jingbao Qiu Jan. 8, 2024, 1:42 p.m. UTC | #6
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