2023-12-29 09:07:46

by Jingbao Qiu

[permalink] [raw]
Subject: [PATCH v1 3/3] dt-bindings: mfd: sophgo: add misc MFD for Sophgo CV1800 series SoC.

Add devicetree binding to describe the MFD for Sophgo CV1800 SoC.

Signed-off-by: Jingbao Qiu <[email protected]>
---
This patch depends on the clk driver
Clk driver link:
https://lore.kernel.org/all/IA1PR20MB49539CDAD9A268CBF6CA184BBB9FA@IA1PR20MB4953.namprd20.prod.outlook.com/

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

diff --git a/Documentation/devicetree/bindings/mfd/sophgo,cv1800-misc.yaml b/Documentation/devicetree/bindings/mfd/sophgo,cv1800-misc.yaml
new file mode 100644
index 000000000000..6fd574a2a945
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/sophgo,cv1800-misc.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/sophgo,cv1800-misc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo CV1800 SoC MISC hardware block
+
+maintainers:
+ - Jingbao Qiu <[email protected]>
+
+description:
+ This hardware block is an independently powered module within
+ the CV1800 SoC. This hardware block contains RTC, Power-On-Reset/POR.
+
+properties:
+ compatible:
+ items:
+ - const: sophgo,cv1800-misc
+ - const: syscon
+ - const: simple-mfd
+
+ reg:
+ maxItems: 1
+
+ rtc:
+ # Child node
+ $ref: /schemas/rtc/sophgo,cv1800-rtc.yaml
+ type: object
+ description:
+ RTC for the SoC. This child node definition
+ should follow the bindings specified in
+ Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml.
+
+ por:
+ # Child node
+ $ref: /schemas/power/sophgo,cv1800-por.yaml
+ type: object
+ description:
+ Power-On-Reset/POR for the SoC. This child node definition
+ should follow the bindings specified in
+ Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml.
+
+required:
+ - compatible
+ - reg
+ - rtc
+ - por
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ misc@5025000 {
+ compatible = "sophgo,cv1800-misc", "syscon", "simple-mfd";
+ reg = <0x05025000 0x2000>;
+
+ rtc {
+ compatible = "sophgo,cv1800-rtc";
+ interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk 15>;
+ };
+
+ por {
+ compatible = "sophgo,cv1800-por";
+ };
+ };
--
2.25.1



2024-01-04 08:36:17

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v1 3/3] dt-bindings: mfd: sophgo: add misc MFD for Sophgo CV1800 series SoC.

On 29/12/2023 10:06, Jingbao Qiu wrote:
> Add devicetree binding to describe the MFD for Sophgo CV1800 SoC.

SoC does not have MFDs. We already talked about this.

>
> Signed-off-by: Jingbao Qiu <[email protected]>
> ---
> This patch depends on the clk driver
> Clk driver link:
> https://lore.kernel.org/all/IA1PR20MB49539CDAD9A268CBF6CA184BBB9FA@IA1PR20MB4953.namprd20.prod.outlook.com/

How? Really, explain me how this depends on driver? Because if it does,
then it is clear NAK as binding cannot depend on driver.

Wait, this is v4, not v1!

Include full changelog.

So you just ignored all the comments? NAK.

>
> .../bindings/mfd/sophgo,cv1800-misc.yaml | 69 +++++++++++++++++++
> 1 file changed, 69 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/sophgo,cv1800-misc.yaml
>
> diff --git a/Documentation/devicetree/bindings/mfd/sophgo,cv1800-misc.yaml b/Documentation/devicetree/bindings/mfd/sophgo,cv1800-misc.yaml
> new file mode 100644
> index 000000000000..6fd574a2a945
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/sophgo,cv1800-misc.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/sophgo,cv1800-misc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sophgo CV1800 SoC MISC hardware block
> +
> +maintainers:
> + - Jingbao Qiu <[email protected]>
> +
> +description:
> + This hardware block is an independently powered module within
> + the CV1800 SoC. This hardware block contains RTC, Power-On-Reset/POR.
> +
> +properties:
> + compatible:
> + items:
> + - const: sophgo,cv1800-misc
> + - const: syscon
> + - const: simple-mfd
> +
> + reg:
> + maxItems: 1
> +
> + rtc:
> + # Child node

Drop, it's obvious.

> + $ref: /schemas/rtc/sophgo,cv1800-rtc.yaml
> + type: object
> + description:
> + RTC for the SoC. This child node definition
> + should follow the bindings specified in
> + Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml.
> +
> + por:
> + # Child node

Drop

> + $ref: /schemas/power/sophgo,cv1800-por.yaml
> + type: object
> + description:
> + Power-On-Reset/POR for the SoC. This child node definition
> + should follow the bindings specified in
> + Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml.

You do not have any resources, so no need for this child.

> +
> +required:
> + - compatible
> + - reg
> + - rtc
> + - por
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + misc@5025000 {


misc can be anything.

> + compatible = "sophgo,cv1800-misc", "syscon", "simple-mfd";
> + reg = <0x05025000 0x2000>;
> +
> + rtc {
> + compatible = "sophgo,cv1800-rtc";
> + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk 15>;
> + };
> +
> + por {
> + compatible = "sophgo,cv1800-por";
> + };
> + };

Best regards,
Krzysztof


2024-01-04 11:06:06

by Jingbao Qiu

[permalink] [raw]
Subject: Re: [PATCH v1 3/3] dt-bindings: mfd: sophgo: add misc MFD for Sophgo CV1800 series SoC.

On Thu, Jan 4, 2024 at 4:36 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 29/12/2023 10:06, Jingbao Qiu wrote:
> > Add devicetree binding to describe the MFD for Sophgo CV1800 SoC.
>
> SoC does not have MFDs. We already talked about this.

I'm sorry for that. I will fix the comment.

>
> >
> > Signed-off-by: Jingbao Qiu <[email protected]>
> > ---
> > This patch depends on the clk driver
> > Clk driver link:
> > https://lore.kernel.org/all/IA1PR20MB49539CDAD9A268CBF6CA184BBB9FA@IA1PR20MB4953.namprd20.prod.outlook.com/
>
> How? Really, explain me how this depends on driver? Because if it does,
> then it is clear NAK as binding cannot depend on driver.

I will fix it.

>
> Wait, this is v4, not v1!
>
> Include full changelog.
>
> So you just ignored all the comments? NAK.

Thanks,I will keep consistent versioning and changelogs.

>
> >
> > .../bindings/mfd/sophgo,cv1800-misc.yaml | 69 +++++++++++++++++++
> > 1 file changed, 69 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/mfd/sophgo,cv1800-misc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/sophgo,cv1800-misc.yaml b/Documentation/devicetree/bindings/mfd/sophgo,cv1800-misc.yaml
> > new file mode 100644
> > index 000000000000..6fd574a2a945
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/sophgo,cv1800-misc.yaml
> > @@ -0,0 +1,69 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mfd/sophgo,cv1800-misc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Sophgo CV1800 SoC MISC hardware block
> > +
> > +maintainers:
> > + - Jingbao Qiu <[email protected]>
> > +
> > +description:
> > + This hardware block is an independently powered module within
> > + the CV1800 SoC. This hardware block contains RTC, Power-On-Reset/POR.
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - const: sophgo,cv1800-misc
> > + - const: syscon
> > + - const: simple-mfd
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + rtc:
> > + # Child node
>
> Drop, it's obvious.

I will do that.

>
> > + $ref: /schemas/rtc/sophgo,cv1800-rtc.yaml
> > + type: object
> > + description:
> > + RTC for the SoC. This child node definition
> > + should follow the bindings specified in
> > + Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml.
> > +
> > + por:
> > + # Child node
>
> Drop

I will do that.

>
> > + $ref: /schemas/power/sophgo,cv1800-por.yaml
> > + type: object
> > + description:
> > + Power-On-Reset/POR for the SoC. This child node definition
> > + should follow the bindings specified in
> > + Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml.
>
> You do not have any resources, so no need for this child.

I will do that.

>
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - rtc
> > + - por
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > + misc@5025000 {
>
>
> misc can be anything.

Actually, there are RTC and (Power On Reset/POR) here. I can't find a suitable
word to describe him. Can you give me some advice?

Best regards,
Jingbao Qiu
>
> > + compatible = "sophgo,cv1800-misc", "syscon", "simple-mfd";
> > + reg = <0x05025000 0x2000>;
> > +
> > + rtc {
> > + compatible = "sophgo,cv1800-rtc";
> > + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clk 15>;
> > + };
> > +
> > + por {
> > + compatible = "sophgo,cv1800-por";
> > + };
> > + };
>
> Best regards,
> Krzysztof
>

2024-01-04 11:17:21

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v1 3/3] dt-bindings: mfd: sophgo: add misc MFD for Sophgo CV1800 series SoC.

On 04/01/2024 12:05, Jingbao Qiu wrote:
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - rtc
>>> + - por
>>> +
>>> +unevaluatedProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + #include <dt-bindings/interrupt-controller/irq.h>
>>> +
>>> + misc@5025000 {
>>
>>
>> misc can be anything.
>
> Actually, there are RTC and (Power On Reset/POR) here. I can't find a suitable
> word to describe him. Can you give me some advice?

Then maybe just rtc? If there is nothing else, why RTC is separate subnode?

Best regards,
Krzysztof


2024-01-04 11:43:18

by Jingbao Qiu

[permalink] [raw]
Subject: Re: [PATCH v1 3/3] dt-bindings: mfd: sophgo: add misc MFD for Sophgo CV1800 series SoC.

On Thu, Jan 4, 2024 at 7:16 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 04/01/2024 12:05, Jingbao Qiu wrote:
> >>> +
> >>> +required:
> >>> + - compatible
> >>> + - reg
> >>> + - rtc
> >>> + - por
> >>> +
> >>> +unevaluatedProperties: false
> >>> +
> >>> +examples:
> >>> + - |
> >>> + #include <dt-bindings/interrupt-controller/irq.h>
> >>> +
> >>> + misc@5025000 {
> >>
> >>
> >> misc can be anything.
> >
> > Actually, there are RTC and (Power On Reset/POR) here. I can't find a suitable
> > word to describe him. Can you give me some advice?
>
> Then maybe just rtc? If there is nothing else, why RTC is separate subnode?
>

There is also a por submodule used to provide power off and restart functions.
Do you mean to use RTC as the parent node like this.
rtc{
//something
por{
}
}

Best regards,
Jingbao Qiu


> Best regards,
> Krzysztof
>

2024-01-04 12:03:16

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v1 3/3] dt-bindings: mfd: sophgo: add misc MFD for Sophgo CV1800 series SoC.

On 04/01/2024 12:42, Jingbao Qiu wrote:
ties: false
>>>>> +
>>>>> +examples:
>>>>> + - |
>>>>> + #include <dt-bindings/interrupt-controller/irq.h>
>>>>> +
>>>>> + misc@5025000 {
>>>>
>>>>
>>>> misc can be anything.
>>>
>>> Actually, there are RTC and (Power On Reset/POR) here. I can't find a suitable
>>> word to describe him. Can you give me some advice?
>>
>> Then maybe just rtc? If there is nothing else, why RTC is separate subnode?
>>
>
> There is also a por submodule used to provide power off and restart functions.
> Do you mean to use RTC as the parent node like this.
> rtc{
> //something
> por{
> }

por is empty in your binding, so there is little point in having it as
subnode.

Best regards,
Krzysztof


2024-01-05 09:54:28

by Jingbao Qiu

[permalink] [raw]
Subject: Re: [PATCH v1 3/3] dt-bindings: mfd: sophgo: add misc MFD for Sophgo CV1800 series SoC.

On Thu, Jan 4, 2024 at 8:03 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 04/01/2024 12:42, Jingbao Qiu wrote:
> ties: false
> >>>>> +
> >>>>> +examples:
> >>>>> + - |
> >>>>> + #include <dt-bindings/interrupt-controller/irq.h>
> >>>>> +
> >>>>> + misc@5025000 {
> >>>>
> >>>>
> >>>> misc can be anything.
> >>>
> >>> Actually, there are RTC and (Power On Reset/POR) here. I can't find a suitable
> >>> word to describe him. Can you give me some advice?
> >>
> >> Then maybe just rtc? If there is nothing else, why RTC is separate subnode?
> >>
> >
> > There is also a por submodule used to provide power off and restart functions.
> > Do you mean to use RTC as the parent node like this.
> > rtc{
> > //something
> > por{
> > }
>
> por is empty in your binding, so there is little point in having it as
> subnode.

Thanks for your patient reply. I will remove POR from RTC and
clearly state their relationship in the description.

Best regards,
Jingbao Qiu