[PATCH v1 0/3] dt-bindings: riscv: sophgo: add mfd RTC and power for CV1800

[PATCH v1 0/3] dt-bindings: riscv: sophgo: add mfd RTC and power for CV1800

[Jingbao Qiu]

This patch add Documentation for mfd and its child node.This hardware
block is an independently powered module within the CV1800 SoC. This
hardware block provides a register map for RTC, Power-On-Reset/POR.

The drivers corresponding to the hardware will be added next version.

Jingbao Qiu (3):
  dt-bindings: rtc: sophgo: add RTC for Sophgo CV1800 series SoC.
  dt-bindings: power: sophgo: add Power-On-Reset/POR for Sophgo CV1800
    series SoC.
  dt-bindings: mfd: sophgo: add misc MFD for Sophgo CV1800 series SoC.

 .../bindings/mfd/sophgo,cv1800-misc.yaml      | 69 +++++++++++++++++++
 .../bindings/power/sophgo,cv1800-por.yaml     | 29 ++++++++
 .../bindings/rtc/sophgo,cv1800-rtc.yaml       | 40 +++++++++++
 3 files changed, 138 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/sophgo,cv1800-misc.yaml
 create mode 100644 Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml
 create mode 100644 Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml


base-commit: 2726b01362a5132f18b978711d84aebae286facb
-- 
2.25.1

[PATCH v1 1/3] dt-bindings: rtc: sophgo: add RTC for Sophgo CV1800 series SoC.

[Jingbao Qiu]

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

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

 .../bindings/rtc/sophgo,cv1800-rtc.yaml       | 40 +++++++++++++++++++
 1 file changed, 40 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..595d3289b183
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml
@@ -0,0 +1,40 @@
+# 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
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - interrupts
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    rtc {
+      compatible = "sophgo,cv1800-rtc";
+      interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
+      clocks = <&clk 15>;
+    };
-- 
2.25.1

Re: [PATCH v1 1/3] dt-bindings: rtc: sophgo: add RTC for Sophgo CV1800 series SoC.

[Krzysztof Kozlowski]

On 29/12/2023 10:06, Jingbao Qiu wrote:
> Add devicetree binding to describe the RTC for Sophgo CV1800 SoC.
> 
> Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com>
> ---
> This patch depends on the clk driver
> Clk driver link:
> https://lore.kernel.org/all/IA1PR20MB49539CDAD9A268CBF6CA184BBB9FA@IA1PR20MB4953.namprd20.prod.outlook.com/

I don't understand how binding can depend on a driver. This is very
confusing and suggests you write binding for the driver, which is not
what we want.

What's more, I really do not see the dependency here, so your message is
incorrect?

> 
>  .../bindings/rtc/sophgo,cv1800-rtc.yaml       | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml
> 

You have dependencies between patches, but I do not see this explained
at all. How people can figure out merging strategy if they are not aware
there is dependency?


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v1 1/3] dt-bindings: rtc: sophgo: add RTC for Sophgo CV1800 series SoC.

[Jingbao Qiu]

On Thu, Jan 4, 2024 at 4:32 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 29/12/2023 10:06, Jingbao Qiu wrote:
> > Add devicetree binding to describe the RTC for Sophgo CV1800 SoC.
> >
> > Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com>
> > ---
> > This patch depends on the clk driver
> > Clk driver link:
> > https://lore.kernel.org/all/IA1PR20MB49539CDAD9A268CBF6CA184BBB9FA@IA1PR20MB4953.namprd20.prod.outlook.com/
>
> I don't understand how binding can depend on a driver. This is very
> confusing and suggests you write binding for the driver, which is not
> what we want.
>
> What's more, I really do not see the dependency here, so your message is
> incorrect?
>

What I mean is that clk is used in the following example. In the clk
patch, there
is a macro definition that can be used to replace this number.

> >
> >  .../bindings/rtc/sophgo,cv1800-rtc.yaml       | 40 +++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml
> >
>
> You have dependencies between patches, but I do not see this explained
> at all. How people can figure out merging strategy if they are not aware
> there is dependency?

I'm sorry for that, I will add an explanation in the description.

Happy new year!

Best regards,
Jingbao Qiu

>
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> Best regards,
> Krzysztof
>

Re: [PATCH v1 1/3] dt-bindings: rtc: sophgo: add RTC for Sophgo CV1800 series SoC.

[Krzysztof Kozlowski]

On 04/01/2024 11:34, Jingbao Qiu wrote:
> On Thu, Jan 4, 2024 at 4:32 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 29/12/2023 10:06, Jingbao Qiu wrote:
>>> Add devicetree binding to describe the RTC for Sophgo CV1800 SoC.
>>>
>>> Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com>
>>> ---
>>> This patch depends on the clk driver
>>> Clk driver link:
>>> https://lore.kernel.org/all/IA1PR20MB49539CDAD9A268CBF6CA184BBB9FA@IA1PR20MB4953.namprd20.prod.outlook.com/
>>
>> I don't understand how binding can depend on a driver. This is very
>> confusing and suggests you write binding for the driver, which is not
>> what we want.
>>
>> What's more, I really do not see the dependency here, so your message is
>> incorrect?
>>
> 
> What I mean is that clk is used in the following example. In the clk
> patch, there
> is a macro definition that can be used to replace this number.

And how is this a dependency?


Best regards,
Krzysztof

Re: [PATCH v1 1/3] dt-bindings: rtc: sophgo: add RTC for Sophgo CV1800 series SoC.

[Jingbao Qiu]

On Thu, Jan 4, 2024 at 6:44 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 04/01/2024 11:34, Jingbao Qiu wrote:
> > On Thu, Jan 4, 2024 at 4:32 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 29/12/2023 10:06, Jingbao Qiu wrote:
> >>> Add devicetree binding to describe the RTC for Sophgo CV1800 SoC.
> >>>
> >>> Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com>
> >>> ---
> >>> This patch depends on the clk driver
> >>> Clk driver link:
> >>> https://lore.kernel.org/all/IA1PR20MB49539CDAD9A268CBF6CA184BBB9FA@IA1PR20MB4953.namprd20.prod.outlook.com/
> >>
> >> I don't understand how binding can depend on a driver. This is very
> >> confusing and suggests you write binding for the driver, which is not
> >> what we want.
> >>
> >> What's more, I really do not see the dependency here, so your message is
> >> incorrect?
> >>
> >
> > What I mean is that clk is used in the following example. In the clk
> > patch, there
> > is a macro definition that can be used to replace this number.
>
> And how is this a dependency?

Thanks. I will drop it.

Best regards,
Jingbao Qiu

>
>
> Best regards,
> Krzysztof
>

[PATCH v1 2/3] dt-bindings: power: sophgo: add Power-On-Reset/POR for Sophgo CV1800 series SoC.

[Jingbao Qiu]

Add devicetree binding to describe the Power-On-Reset/POR for Sophgo CV1800 SoC.

Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com>
---
 .../bindings/power/sophgo,cv1800-por.yaml     | 29 +++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml

diff --git a/Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml b/Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml
new file mode 100644
index 000000000000..8706230a1cbc
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml
@@ -0,0 +1,29 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/sophgo,cv1800-por.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Power-On-Reset/POR of the Sophgo CV1800 SoC
+
+maintainers:
+  - Jingbao Qiu <qiujingbao.dlmu@gmail.com>
+
+description:
+  This hardware provides triggering and timing control
+  for chip power on, off, and reset.
+
+properties:
+  compatible:
+    const: sophgo,cv1800-por
+
+required:
+  - compatible
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    por {
+      compatible = "sophgo,cv1800-por";
+    };
-- 
2.25.1

Re: [PATCH v1 2/3] dt-bindings: power: sophgo: add Power-On-Reset/POR for Sophgo CV1800 series SoC.

[Krzysztof Kozlowski]

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

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

Subject: Make it concise. It's way over the limit. Also, unnecessary
full stop.

This all applies to all your patches.

> 

> Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com>
> ---

What changed here? Where is the changelog? Did you just ignore entire
feedback from v3?

>  .../bindings/power/sophgo,cv1800-por.yaml     | 29 +++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml b/Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml
> new file mode 100644
> index 000000000000..8706230a1cbc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml
> @@ -0,0 +1,29 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/sophgo,cv1800-por.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Power-On-Reset/POR of the Sophgo CV1800 SoC
> +
> +maintainers:
> +  - Jingbao Qiu <qiujingbao.dlmu@gmail.com>
> +
> +description:
> +  This hardware provides triggering and timing control
> +  for chip power on, off, and reset.
> +
> +properties:
> +  compatible:
> +    const: sophgo,cv1800-por
> +

Empty schema, drop entire file. You do not need it.

Best regards,
Krzysztof

Re: [PATCH v1 2/3] dt-bindings: power: sophgo: add Power-On-Reset/POR for Sophgo CV1800 series SoC.

[Jingbao Qiu]

On Thu, Jan 4, 2024 at 4:37 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 29/12/2023 10:06, Jingbao Qiu wrote:
> > Add devicetree binding to describe the Power-On-Reset/POR for Sophgo CV1800 SoC.
>
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>
> Subject: Make it concise. It's way over the limit. Also, unnecessary
> full stop.
>
> This all applies to all your patches.
>

I will do that.

> >
>
> > Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com>
> > ---
>
> What changed here? Where is the changelog? Did you just ignore entire
> feedback from v3?

Actually, I separated the previous patches because there were issues
in various places.
I want to solve it bit by bit. Should I continue with the current
patch changes or return to
the previous patch?

>
> >  .../bindings/power/sophgo,cv1800-por.yaml     | 29 +++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml b/Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml
> > new file mode 100644
> > index 000000000000..8706230a1cbc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/sophgo,cv1800-por.yaml
> > @@ -0,0 +1,29 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/power/sophgo,cv1800-por.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Power-On-Reset/POR of the Sophgo CV1800 SoC
> > +
> > +maintainers:
> > +  - Jingbao Qiu <qiujingbao.dlmu@gmail.com>
> > +
> > +description:
> > +  This hardware provides triggering and timing control
> > +  for chip power on, off, and reset.
> > +
> > +properties:
> > +  compatible:
> > +    const: sophgo,cv1800-por
> > +
>
> Empty schema, drop entire file. You do not need it.
>

I will drop it.

Best regards,
Jingbao Qiu

> Best regards,
> Krzysztof
>

Re: [PATCH v1 2/3] dt-bindings: power: sophgo: add Power-On-Reset/POR for Sophgo CV1800 series SoC.

[Krzysztof Kozlowski]

On 04/01/2024 11:39, Jingbao Qiu wrote:
> On Thu, Jan 4, 2024 at 4:37 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 29/12/2023 10:06, Jingbao Qiu wrote:
>>> Add devicetree binding to describe the Power-On-Reset/POR for Sophgo CV1800 SoC.
>>
>> Please wrap commit message according to Linux coding style / submission
>> process (neither too early nor over the limit):
>> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>>
>> Subject: Make it concise. It's way over the limit. Also, unnecessary
>> full stop.
>>
>> This all applies to all your patches.
>>
> 
> I will do that.
> 
>>>
>>
>>> Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com>
>>> ---
>>
>> What changed here? Where is the changelog? Did you just ignore entire
>> feedback from v3?
> 
> Actually, I separated the previous patches because there were issues
> in various places.
> I want to solve it bit by bit. Should I continue with the current
> patch changes or return to
> the previous patch?

You need to keep consistent versioning and changelogs.

Best regards,
Krzysztof

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

[Jingbao Qiu]

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

Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com>
---
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 <qiujingbao.dlmu@gmail.com>
+
+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

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

[Krzysztof Kozlowski]

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 <qiujingbao.dlmu@gmail.com>
> ---
> 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 <qiujingbao.dlmu@gmail.com>
> +
> +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

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

[Jingbao Qiu]

On Thu, Jan 4, 2024 at 4:36 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> 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 <qiujingbao.dlmu@gmail.com>
> > ---
> > 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 <qiujingbao.dlmu@gmail.com>
> > +
> > +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
>

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

[Krzysztof Kozlowski]

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

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

[Jingbao Qiu]

On Thu, Jan 4, 2024 at 7:16 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> 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
>

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

[Krzysztof Kozlowski]

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

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

[Jingbao Qiu]

On Thu, Jan 4, 2024 at 8:03 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> 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