2017-11-08 07:48:02

by Baolin Wang

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: rtc: Add Spreadtrum SC27xx RTC documentation

Hi Alexandre,

On 8 November 2017 at 11:15, Alexandre Belloni
<[email protected]> wrote:
> Hi,
>
> On 07/11/2017 at 19:34:07 +0800, Baolin Wang wrote:
>> This patch adds the binding documentation for Spreadtrum SC27xx series
>> RTC device.
>>
>> Signed-off-by: Baolin Wang <[email protected]>
>> ---
>> .../devicetree/bindings/rtc/sprd,sc27xx-rtc.txt | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/rtc/sprd,sc27xx-rtc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/rtc/sprd,sc27xx-rtc.txt b/Documentation/devicetree/bindings/rtc/sprd,sc27xx-rtc.txt
>> new file mode 100644
>> index 0000000..971d3a2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/rtc/sprd,sc27xx-rtc.txt
>> @@ -0,0 +1,16 @@
>> +Spreadtrum SC27xx Real Time Clock
>> +
>> +Required properties:
>> +- compatible: should be "sprd,sc27xx-rtc".
>
> Don't use wildcards in a compatible, use a specific chip model. later
> chips may or may not be compatible with that one.

Our Spreadtrum SC27xx series PMICs contain SC2720, SC2721, SC2723,
SC2730 and SC2731. They all integrate the same RTC IP, so I think it
will be better to use "sc27xx" string. Our PMIC driver also used the
"sprd,sc27xx-rtc" sting which has been merged into Lee's MFD next
tree.

>
>> +- reg: address offset of rtc register.
>> +- interrupt-parent: phandle for the interrupt controller.
>> +- interrupts: rtc alarm interrupt.
>> +
>> +Example:
>> +
>> + rtc: rtc@280 {
>
> The rtc: alias is probably useless

Yes, I will remove it.

>
>> + compatible = "sprd,sc27xx-rtc";
>> + reg = <0x280>;
>> + interrupt-parent = <&pmic>;
>> + interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
>> + };
>
> I would include the parent MFD node too.

OK. Thanks for your comments.

--
Baolin Wang
Best Regards

From 1583483166224618069@xxx Wed Nov 08 07:47:06 +0000 2017
X-GM-THRID: 1583412423125923291
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread


2017-11-08 07:47:06

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: rtc: Add Spreadtrum SC27xx RTC documentation

On 08/11/2017 at 13:02:59 +0800, Baolin Wang wrote:
> Hi Alexandre,
>
> On 8 November 2017 at 11:15, Alexandre Belloni
> <[email protected]> wrote:
> > Hi,
> >
> > On 07/11/2017 at 19:34:07 +0800, Baolin Wang wrote:
> >> This patch adds the binding documentation for Spreadtrum SC27xx series
> >> RTC device.
> >>
> >> Signed-off-by: Baolin Wang <[email protected]>
> >> ---
> >> .../devicetree/bindings/rtc/sprd,sc27xx-rtc.txt | 16 ++++++++++++++++
> >> 1 file changed, 16 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/rtc/sprd,sc27xx-rtc.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/rtc/sprd,sc27xx-rtc.txt b/Documentation/devicetree/bindings/rtc/sprd,sc27xx-rtc.txt
> >> new file mode 100644
> >> index 0000000..971d3a2
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/rtc/sprd,sc27xx-rtc.txt
> >> @@ -0,0 +1,16 @@
> >> +Spreadtrum SC27xx Real Time Clock
> >> +
> >> +Required properties:
> >> +- compatible: should be "sprd,sc27xx-rtc".
> >
> > Don't use wildcards in a compatible, use a specific chip model. later
> > chips may or may not be compatible with that one.
>
> Our Spreadtrum SC27xx series PMICs contain SC2720, SC2721, SC2723,
> SC2730 and SC2731. They all integrate the same RTC IP, so I think it
> will be better to use "sc27xx" string. Our PMIC driver also used the
> "sprd,sc27xx-rtc" sting which has been merged into Lee's MFD next
> tree.
>

And at some point it time, you'll have sc2750 or 2770 with a different
rtc and everything will get confusing.

I should have been copied on the mfd series if you wanted to avoid that.
Anyway, I'm pretty sure Rob will have the same opinion.

--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

From 1583470522057379011@xxx Wed Nov 08 04:26:07 +0000 2017
X-GM-THRID: 1583412423125923291
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread