| Message ID | 20231217110952.78784-2-qiujingbao.dlmu@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | riscv: rtc: sophgo: add rtc support for CV1800 | expand |
On Sun, Dec 17, 2023 at 07:09:50PM +0800, Jingbao Qiu wrote: > + reg: > + items: > + - description: data register > + - description: control register > + rtc@5025000{ > + compatible = "sophgo,cv1800-rtc"; > + reg = <0x5025000 0x1000>, <0x5026000 0x1000>; Why are these two regions rather than just one, given they are located next to one another? Are they separate on one of the other devices in this family? Thanks, Conor. > + clocks = <&osc>; > + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>; > + };
On Sun, Dec 17, 2023 at 8:26 PM Conor Dooley <conor@kernel.org> wrote: > > On Sun, Dec 17, 2023 at 07:09:50PM +0800, Jingbao Qiu wrote: > > > + reg: > > + items: > > + - description: data register > > + - description: control register > > > + rtc@5025000{ > > + compatible = "sophgo,cv1800-rtc"; > > + reg = <0x5025000 0x1000>, <0x5026000 0x1000>; > > Why are these two regions rather than just one, given they are located > next to one another? > Are they separate on one of the other devices in this family? > > Thanks, > Conor. > I think there are two reasons, the first one is to distinguish different logical , REG_ CTRL (base on 0x5025000) controls clock calibration, sleep,and other functions, RTC_ CORE (base on 0x5026000) has basic RTC functionality, The second is the maximum address used by RTC_CTRL (base on 0x5025000) is 0x0ac,which is much smaller than 0x1000. Therefore, the datasheet divides it into two parts for introduction, and I also divide it into two parts based on this introduction.So do you suggest that I merge them together？ Best regards, Jingbao Qiu > > + clocks = <&osc>; > > + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>; > > + }; >
On Sun, Dec 17, 2023 at 09:16:39PM +0800, jingbao qiu wrote: > On Sun, Dec 17, 2023 at 8:26 PM Conor Dooley <conor@kernel.org> wrote: > > > > On Sun, Dec 17, 2023 at 07:09:50PM +0800, Jingbao Qiu wrote: > > > > > + reg: > > > + items: > > > + - description: data register > > > + - description: control register > > > > > + rtc@5025000{ > > > + compatible = "sophgo,cv1800-rtc"; > > > + reg = <0x5025000 0x1000>, <0x5026000 0x1000>; > > > > Why are these two regions rather than just one, given they are located > > next to one another? > > Are they separate on one of the other devices in this family? > > > > Thanks, > > Conor. > > > > I think there are two reasons, the first one is to distinguish > different logical , > REG_ CTRL (base on 0x5025000) controls clock calibration, sleep,and other > functions, RTC_ CORE (base on 0x5026000) has basic RTC functionality, > The second is the maximum address used by RTC_CTRL (base on 0x5025000) > is 0x0ac,which is much smaller than 0x1000. Therefore, the datasheet divides > it into two parts for introduction, and I also divide it into two > parts based on this > introduction.So do you suggest that I merge them together？ If all of the cv1800 series devices have them sequentially, I would just make them one region.
On Mon, Dec 18, 2023 at 4:47 AM Conor Dooley <conor@kernel.org> wrote: > > On Sun, Dec 17, 2023 at 09:16:39PM +0800, jingbao qiu wrote: > > On Sun, Dec 17, 2023 at 8:26 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > On Sun, Dec 17, 2023 at 07:09:50PM +0800, Jingbao Qiu wrote: > > > > > > > + reg: > > > > + items: > > > > + - description: data register > > > > + - description: control register > > > > > > > + rtc@5025000{ > > > > + compatible = "sophgo,cv1800-rtc"; > > > > + reg = <0x5025000 0x1000>, <0x5026000 0x1000>; > > > > > > Why are these two regions rather than just one, given they are located > > > next to one another? > > > Are they separate on one of the other devices in this family? > > > > > > Thanks, > > > Conor. > > > > > > > I think there are two reasons, the first one is to distinguish > > different logical , > > REG_ CTRL (base on 0x5025000) controls clock calibration, sleep,and other > > functions, RTC_ CORE (base on 0x5026000) has basic RTC functionality, > > The second is the maximum address used by RTC_CTRL (base on 0x5025000) > > is 0x0ac,which is much smaller than 0x1000. Therefore, the datasheet divides > > it into two parts for introduction, and I also divide it into two > > parts based on this > > introduction.So do you suggest that I merge them together？ > > If all of the cv1800 series devices have them sequentially, I would just > make them one region. Thanks, I will fix it. Best regards, Jingbao Qiu
>On Sun, Dec 17, 2023 at 09:16:39PM +0800, jingbao qiu wrote: >> On Sun, Dec 17, 2023 at 8:26=E2=80=AFPM Conor Dooley <conor@kernel.org> w= >rote: >> > >> > On Sun, Dec 17, 2023 at 07:09:50PM +0800, Jingbao Qiu wrote: >> > >> > > + reg: >> > > + items: >> > > + - description: data register >> > > + - description: control register >> > >> > > + rtc@5025000{ >> > > + compatible =3D "sophgo,cv1800-rtc"; >> > > + reg =3D <0x5025000 0x1000>, <0x5026000 0x1000>; >> > >> > Why are these two regions rather than just one, given they are located >> > next to one another? >> > Are they separate on one of the other devices in this family? >> > >> > Thanks, >> > Conor. >> > >>=20 >> I think there are two reasons, the first one is to distinguish >> different logical , >> REG_ CTRL (base on 0x5025000) controls clock calibration, sleep,and other >> functions, RTC_ CORE (base on 0x5026000) has basic RTC functionality, >> The second is the maximum address used by RTC_CTRL (base on 0x5025000) >> is 0x0ac,which is much smaller than 0x1000. Therefore, the datasheet divi= >des >> it into two parts for introduction, and I also divide it into two >> parts based on this >> introduction.So do you suggest that I merge them together=EF=BC=9F > >If all of the cv1800 series devices have them sequentially, I would just >make them one region. > I agree with using one region. The ctrl and core region are highly releated. Moreover, I suggest using syscon to describe this region, the reboot device is also in this region.
On Mon, Dec 18, 2023 at 11:41:52AM +0800, Inochi Amaoto wrote: > >On Sun, Dec 17, 2023 at 09:16:39PM +0800, jingbao qiu wrote: > >> On Sun, Dec 17, 2023 at 8:26=E2=80=AFPM Conor Dooley <conor@kernel.org> w= > >rote: > >> > > >> > On Sun, Dec 17, 2023 at 07:09:50PM +0800, Jingbao Qiu wrote: > >> > > >> > > + reg: > >> > > + items: > >> > > + - description: data register > >> > > + - description: control register > >> > > >> > > + rtc@5025000{ > >> > > + compatible =3D "sophgo,cv1800-rtc"; > >> > > + reg =3D <0x5025000 0x1000>, <0x5026000 0x1000>; > >> > > >> > Why are these two regions rather than just one, given they are located > >> > next to one another? > >> > Are they separate on one of the other devices in this family? > >> > > >> > Thanks, > >> > Conor. > >> > > >>=20 > >> I think there are two reasons, the first one is to distinguish > >> different logical , > >> REG_ CTRL (base on 0x5025000) controls clock calibration, sleep,and other > >> functions, RTC_ CORE (base on 0x5026000) has basic RTC functionality, > >> The second is the maximum address used by RTC_CTRL (base on 0x5025000) > >> is 0x0ac,which is much smaller than 0x1000. Therefore, the datasheet divi= > >des > >> it into two parts for introduction, and I also divide it into two > >> parts based on this > >> introduction.So do you suggest that I merge them together=EF=BC=9F > > > >If all of the cv1800 series devices have them sequentially, I would just > >make them one region. > > > > I agree with using one region. The ctrl and core region are highly > releated. > > Moreover, I suggest using syscon to describe this region, the reboot > device is also in this region. Then the description of the device is incomplete. Please describe the whole block/device. Rob
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..a9e1dcc2a5be --- /dev/null +++ b/Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml @@ -0,0 +1,47 @@ +# 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 + +maintainers: + - Jingbao Qiu <qiujingbao.dlmu@gmail.com> + +allOf: + - $ref: rtc.yaml# + +properties: + compatible: + const: sophgo,cv1800-rtc + + reg: + items: + - description: data register + - description: control register + + clocks: + maxItems: 1 + + interrupts: + maxItems: 1 + +required: + - compatible + - reg + - clocks + - interrupts + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + rtc@5025000{ + compatible = "sophgo,cv1800-rtc"; + reg = <0x5025000 0x1000>, <0x5026000 0x1000>; + clocks = <&osc>; + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>; + };
Add devicetree binding for Sophgo CV1800 SoC. Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com> --- .../bindings/rtc/sophgo,cv1800-rtc.yaml | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml