diff mbox series

[v3,4/4] dt-binding: perf: Add Amlogic DDR PMU

Message ID 20220801060049.1655177-4-jiucheng.xu@amlogic.com (mailing list archive)
State Superseded
Headers show
Series [v3,1/4] perf/amlogic: Add support for Amlogic meson G12 SoC DDR PMU driver | expand

Commit Message

Jiucheng Xu Aug. 1, 2022, 6 a.m. UTC 
Add binding documentation for the Amlogic G12 series DDR
performance monitor unit.

Signed-off-by: Jiucheng Xu <jiucheng.xu@amlogic.com>
---
Changes v2 -> v3:
  - Remove oneOf
  - Add descriptions
  - Fix compiling warning

Changes v1 -> v2:
  - Rename file, from aml_ddr_pmu.yaml to amlogic,g12_ddr_pmu.yaml
  - Delete "model", "dmc_nr", "chann_nr" new properties
  - Fix compiling error
---
 .../bindings/perf/amlogic,g12_ddr_pmu.yaml    | 51 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml

Comments

Rob Herring Aug. 1, 2022, 1:43 p.m. UTC | #1 
On Mon, 01 Aug 2022 14:00:49 +0800, Jiucheng Xu wrote:
> Add binding documentation for the Amlogic G12 series DDR
> performance monitor unit.
> 
> Signed-off-by: Jiucheng Xu <jiucheng.xu@amlogic.com>
> ---
> Changes v2 -> v3:
>   - Remove oneOf
>   - Add descriptions
>   - Fix compiling warning
> 
> Changes v1 -> v2:
>   - Rename file, from aml_ddr_pmu.yaml to amlogic,g12_ddr_pmu.yaml
>   - Delete "model", "dmc_nr", "chann_nr" new properties
>   - Fix compiling error
> ---
>  .../bindings/perf/amlogic,g12_ddr_pmu.yaml    | 51 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 52 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/perf/amlogic,g12_ddr_pmu.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Krzysztof Kozlowski Aug. 2, 2022, 8:04 a.m. UTC | #2 
On 01/08/2022 08:00, Jiucheng Xu wrote:
> Add binding documentation for the Amlogic G12 series DDR
> performance monitor unit.
> 
> Signed-off-by: Jiucheng Xu <jiucheng.xu@amlogic.com>
> ---
> Changes v2 -> v3:
>   - Remove oneOf
>   - Add descriptions
>   - Fix compiling warning
> 
> Changes v1 -> v2:
>   - Rename file, from aml_ddr_pmu.yaml to amlogic,g12_ddr_pmu.yaml
>   - Delete "model", "dmc_nr", "chann_nr" new properties
>   - Fix compiling error
> ---
>  .../bindings/perf/amlogic,g12_ddr_pmu.yaml    | 51 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 52 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml
> 
> diff --git a/Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml b/Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml
> new file mode 100644
> index 000000000000..961656d4db6e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/perf/amlogic,g12-ddr-pmu.yaml#

You still did not test the bindings...

You received such comment (with instructions how to do it) and still
decided to send untested bindings.

That's not how submission procces should look like.

NAK, till you send something which you actually test.


Best regards,
Krzysztof
Jiucheng Xu Aug. 2, 2022, 9:25 a.m. UTC | #3 
On 2022/8/2 16:04, Krzysztof Kozlowski wrote:
> [ EXTERNAL EMAIL ]
>
> On 01/08/2022 08:00, Jiucheng Xu wrote:
>> Add binding documentation for the Amlogic G12 series DDR
>> performance monitor unit.
>>
>> Signed-off-by: Jiucheng Xu <jiucheng.xu@amlogic.com>
>> ---
>> Changes v2 -> v3:
>>    - Remove oneOf
>>    - Add descriptions
>>    - Fix compiling warning
>>
>> Changes v1 -> v2:
>>    - Rename file, from aml_ddr_pmu.yaml to amlogic,g12_ddr_pmu.yaml
>>    - Delete "model", "dmc_nr", "chann_nr" new properties
>>    - Fix compiling error
>> ---
>>   .../bindings/perf/amlogic,g12_ddr_pmu.yaml    | 51 +++++++++++++++++++
>>   MAINTAINERS                                   |  1 +
>>   2 files changed, 52 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml b/Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml
>> new file mode 100644
>> index 000000000000..961656d4db6e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml
>> @@ -0,0 +1,51 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/perf/amlogic,g12-ddr-pmu.yaml#
> You still did not test the bindings...
>
> You received such comment (with instructions how to do it) and still
> decided to send untested bindings.
>
> That's not how submission procces should look like.
>
> NAK, till you send something which you actually test.
>
>
> Best regards,
> Krzysztof

Hi Krzysztof,


Thanks for your time. I think I got a wrong understanding.

As the binding doc says, I run the following command to check:

# make dt_binding_check DT_SCHEMA_FILES=amlogic,g12_ddr_pmu.yaml ARCH=arm64

I saw the warning/errors in v2 patch, and thanks to your comments let 
them gone.

But I didn't see any warning/errors keywords printing in v3 except the 
message as below:

"$id: relative path/filename doesn't match actual path or filename".


I admit I didn't understand this prompt. I found the doc in example says 
that:

# $id is a unique identifier based on the filename. There may or may not 
be a
# file present at the URL.

So I thought that was not warning/errors prompt.

Today, I find the root cause of the warning/errors prompt.

I think I have fixed it and the building log is so quiet.


This is my first upstream patch, TBH I even didn't know what is binding 
before.

In order to upstream my driver, I try to learn how to write binding scheme.

It looks like I still did terribly.

I think my stupid misunderstanding get you angry. I'm sorry for wasting 
your time and comments.

I will re-submit v4 to fix it.
Krzysztof Kozlowski Aug. 2, 2022, 12:13 p.m. UTC | #4 
On 02/08/2022 11:25, Jiucheng Xu wrote:
> 
> On 2022/8/2 16:04, Krzysztof Kozlowski wrote:
>> [ EXTERNAL EMAIL ]
>>
>> On 01/08/2022 08:00, Jiucheng Xu wrote:
>>> Add binding documentation for the Amlogic G12 series DDR
>>> performance monitor unit.
>>>
>>> Signed-off-by: Jiucheng Xu <jiucheng.xu@amlogic.com>
>>> ---
>>> Changes v2 -> v3:
>>>    - Remove oneOf
>>>    - Add descriptions
>>>    - Fix compiling warning
>>>
>>> Changes v1 -> v2:
>>>    - Rename file, from aml_ddr_pmu.yaml to amlogic,g12_ddr_pmu.yaml
>>>    - Delete "model", "dmc_nr", "chann_nr" new properties
>>>    - Fix compiling error
>>> ---
>>>   .../bindings/perf/amlogic,g12_ddr_pmu.yaml    | 51 +++++++++++++++++++
>>>   MAINTAINERS                                   |  1 +
>>>   2 files changed, 52 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml b/Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml
>>> new file mode 100644
>>> index 000000000000..961656d4db6e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml
>>> @@ -0,0 +1,51 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/perf/amlogic,g12-ddr-pmu.yaml#
>> You still did not test the bindings...
>>
>> You received such comment (with instructions how to do it) and still
>> decided to send untested bindings.
>>
>> That's not how submission procces should look like.
>>
>> NAK, till you send something which you actually test.
>>
>>
>> Best regards,
>> Krzysztof
> 
> Hi Krzysztof,
> 
> 
> Thanks for your time. I think I got a wrong understanding.
> 
> As the binding doc says, I run the following command to check:
> 
> # make dt_binding_check DT_SCHEMA_FILES=amlogic,g12_ddr_pmu.yaml ARCH=arm64

Ah, cool, thanks.

> 
> I saw the warning/errors in v2 patch, and thanks to your comments let 
> them gone.
> 
> But I didn't see any warning/errors keywords printing in v3 except the 
> message as below:
> 
> "$id: relative path/filename doesn't match actual path or filename".
> 
> 
> I admit I didn't understand this prompt. I found the doc in example says 
> that:
> 
> # $id is a unique identifier based on the filename. There may or may not 
> be a
> # file present at the URL.
> 
> So I thought that was not warning/errors prompt.

Unfortunately, it is and warning message. The final part of this ID is
directory and filename - these must match real file path.


> 
> Today, I find the root cause of the warning/errors prompt.
> 
> I think I have fixed it and the building log is so quiet.
> 
> 
> This is my first upstream patch, TBH I even didn't know what is binding 
> before.
> 
> In order to upstream my driver, I try to learn how to write binding scheme.
> 
> It looks like I still did terribly.
> 
> I think my stupid misunderstanding get you angry. I'm sorry for wasting 
> your time and comments.

Apologies for being harsh. We all make mistakes and hopefully learn from
them.

> 
> I will re-submit v4 to fix it.
> 

Awesome, thank you!

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml b/Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml
new file mode 100644
index 000000000000..961656d4db6e
--- /dev/null
+++ b/Documentation/devicetree/bindings/perf/amlogic,g12_ddr_pmu.yaml
@@ -0,0 +1,51 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/perf/amlogic,g12-ddr-pmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Amlogic G12 DDR performance monitor
+
+maintainers:
+  - Jiucheng Xu <jiucheng.xu@amlogic.com>
+
+description: |
+  Amlogic G12 series SoC integrate DDR bandwidth monitor.
+  A timer is inside and can generate interrupt when timeout.
+  The bandwidth is counted in the timer ISR.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - amlogic,g12b-ddr-pmu
+          - amlogic,g12a-ddr-pmu
+          - amlogic,sm1-ddr-pmu
+
+  reg:
+    items:
+      - description: Physical address of DMC bandwidth register
+          and size of the configuration address space.
+      - description: Physical address of DMC PLL register and
+          size of the configuration address space.
+
+  interrupts:
+    items:
+      - description: The IRQ of the inside timer timeout.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    ddr_pmu@ff638000 {
+        compatible = "amlogic,g12a-ddr-pmu";
+        reg = <0xff638000 0x100
+               0xff638c00 0x100>;
+        interrupts = <GIC_SPI 52 IRQ_TYPE_EDGE_RISING>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 8ee68e699e6d..67c2c9e8c4ea 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1055,6 +1055,7 @@  M:	Jiucheng Xu <jiucheng.xu@amlogic.com>
 S:	Supported
 W:	http://www.amlogic.com
 F:	Documentation/admin-guide/perf/meson-ddr-pmu.rst
+F:	Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml
 F:	drivers/perf/amlogic/
 F:	include/soc/amlogic/