2023-06-18 13:24:29

by Rabara, Niravkumar L

[permalink] [raw]
Subject: [PATCH 2/4] dt-bindings: clock: Add Intel Agilex5 clocks and resets

From: Niravkumar L Rabara <[email protected]>

Add clock and reset ID definitions for Intel Agilex5 SoCFPGA

Co-developed-by: Teh Wen Ping <[email protected]>
Signed-off-by: Teh Wen Ping <[email protected]>
Signed-off-by: Niravkumar L Rabara <[email protected]>
---
.../bindings/clock/intel,agilex5.yaml | 42 ++++++++
include/dt-bindings/clock/agilex5-clock.h | 100 ++++++++++++++++++
.../dt-bindings/reset/altr,rst-mgr-agilex5.h | 79 ++++++++++++++
3 files changed, 221 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/intel,agilex5.yaml
create mode 100644 include/dt-bindings/clock/agilex5-clock.h
create mode 100644 include/dt-bindings/reset/altr,rst-mgr-agilex5.h

diff --git a/Documentation/devicetree/bindings/clock/intel,agilex5.yaml b/Documentation/devicetree/bindings/clock/intel,agilex5.yaml
new file mode 100644
index 000000000000..e408c52deefa
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/intel,agilex5.yaml
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/intel,agilex5.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel SoCFPGA Agilex5 platform clock controller binding
+
+maintainers:
+ - Teh Wen Ping <[email protected]>
+ - Niravkumar L Rabara <[email protected]>
+
+description:
+ The Intel Agilex5 Clock controller is an integrated clock controller, which
+ generates and supplies to all modules.
+
+properties:
+ compatible:
+ const: intel,agilex5-clkmgr
+
+ '#clock-cells':
+ const: 1
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+ # Clock controller node:
+ - |
+ clkmgr: clock-controller@10d10000 {
+ compatible = "intel,agilex5-clkmgr";
+ reg = <0x10d10000 0x1000>;
+ #clock-cells = <1>;
+ };
+...
diff --git a/include/dt-bindings/clock/agilex5-clock.h b/include/dt-bindings/clock/agilex5-clock.h
new file mode 100644
index 000000000000..4505b352cd83
--- /dev/null
+++ b/include/dt-bindings/clock/agilex5-clock.h
@@ -0,0 +1,100 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * Copyright (C) 2022, Intel Corporation
+ */
+
+#ifndef __AGILEX5_CLOCK_H
+#define __AGILEX5_CLOCK_H
+
+/* fixed rate clocks */
+#define AGILEX5_OSC1 0
+#define AGILEX5_CB_INTOSC_HS_DIV2_CLK 1
+#define AGILEX5_CB_INTOSC_LS_CLK 2
+#define AGILEX5_F2S_FREE_CLK 3
+
+/* PLL clocks */
+#define AGILEX5_MAIN_PLL_CLK 4
+#define AGILEX5_MAIN_PLL_C0_CLK 5
+#define AGILEX5_MAIN_PLL_C1_CLK 6
+#define AGILEX5_MAIN_PLL_C2_CLK 7
+#define AGILEX5_MAIN_PLL_C3_CLK 8
+#define AGILEX5_PERIPH_PLL_CLK 9
+#define AGILEX5_PERIPH_PLL_C0_CLK 10
+#define AGILEX5_PERIPH_PLL_C1_CLK 11
+#define AGILEX5_PERIPH_PLL_C2_CLK 12
+#define AGILEX5_PERIPH_PLL_C3_CLK 13
+#define AGILEX5_CORE0_FREE_CLK 14
+#define AGILEX5_CORE1_FREE_CLK 15
+#define AGILEX5_CORE2_FREE_CLK 16
+#define AGILEX5_CORE3_FREE_CLK 17
+#define AGILEX5_DSU_FREE_CLK 18
+#define AGILEX5_BOOT_CLK 19
+
+/* fixed factor clocks */
+#define AGILEX5_L3_MAIN_FREE_CLK 20
+#define AGILEX5_NOC_FREE_CLK 21
+#define AGILEX5_S2F_USR0_CLK 22
+#define AGILEX5_NOC_CLK 23
+#define AGILEX5_EMAC_A_FREE_CLK 24
+#define AGILEX5_EMAC_B_FREE_CLK 25
+#define AGILEX5_EMAC_PTP_FREE_CLK 26
+#define AGILEX5_GPIO_DB_FREE_CLK 27
+#define AGILEX5_S2F_USER0_FREE_CLK 28
+#define AGILEX5_S2F_USER1_FREE_CLK 29
+#define AGILEX5_PSI_REF_FREE_CLK 30
+#define AGILEX5_USB31_FREE_CLK 31
+
+/* Gate clocks */
+#define AGILEX5_CORE0_CLK 32
+#define AGILEX5_CORE1_CLK 33
+#define AGILEX5_CORE2_CLK 34
+#define AGILEX5_CORE3_CLK 35
+#define AGILEX5_MPU_CLK 36
+#define AGILEX5_MPU_PERIPH_CLK 37
+#define AGILEX5_MPU_CCU_CLK 38
+#define AGILEX5_L4_MAIN_CLK 39
+#define AGILEX5_L4_MP_CLK 40
+#define AGILEX5_L4_SYS_FREE_CLK 41
+#define AGILEX5_L4_SP_CLK 42
+#define AGILEX5_CS_AT_CLK 43
+#define AGILEX5_CS_TRACE_CLK 44
+#define AGILEX5_CS_PDBG_CLK 45
+#define AGILEX5_EMAC1_CLK 47
+#define AGILEX5_EMAC2_CLK 48
+#define AGILEX5_EMAC_PTP_CLK 49
+#define AGILEX5_GPIO_DB_CLK 50
+#define AGILEX5_S2F_USER0_CLK 51
+#define AGILEX5_S2F_USER1_CLK 52
+#define AGILEX5_PSI_REF_CLK 53
+#define AGILEX5_USB31_SUSPEND_CLK 54
+#define AGILEX5_EMAC0_CLK 46
+#define AGILEX5_USB31_BUS_CLK_EARLY 55
+#define AGILEX5_USB2OTG_HCLK 56
+#define AGILEX5_SPIM_0_CLK 57
+#define AGILEX5_SPIM_1_CLK 58
+#define AGILEX5_SPIS_0_CLK 59
+#define AGILEX5_SPIS_1_CLK 60
+#define AGILEX5_DMA_CORE_CLK 61
+#define AGILEX5_DMA_HS_CLK 62
+#define AGILEX5_I3C_0_CORE_CLK 63
+#define AGILEX5_I3C_1_CORE_CLK 64
+#define AGILEX5_I2C_0_PCLK 65
+#define AGILEX5_I2C_1_PCLK 66
+#define AGILEX5_I2C_EMAC0_PCLK 67
+#define AGILEX5_I2C_EMAC1_PCLK 68
+#define AGILEX5_I2C_EMAC2_PCLK 69
+#define AGILEX5_UART_0_PCLK 70
+#define AGILEX5_UART_1_PCLK 71
+#define AGILEX5_SPTIMER_0_PCLK 72
+#define AGILEX5_SPTIMER_1_PCLK 73
+#define AGILEX5_DFI_CLK 74
+#define AGILEX5_NAND_NF_CLK 75
+#define AGILEX5_NAND_BCH_CLK 76
+#define AGILEX5_SDMMC_SDPHY_REG_CLK 77
+#define AGILEX5_SDMCLK 78
+#define AGILEX5_SOFTPHY_REG_PCLK 79
+#define AGILEX5_SOFTPHY_PHY_CLK 80
+#define AGILEX5_SOFTPHY_CTRL_CLK 81
+#define AGILEX5_NUM_CLKS 82
+
+#endif /* __AGILEX5_CLOCK_H */
diff --git a/include/dt-bindings/reset/altr,rst-mgr-agilex5.h b/include/dt-bindings/reset/altr,rst-mgr-agilex5.h
new file mode 100644
index 000000000000..81e5e8c89893
--- /dev/null
+++ b/include/dt-bindings/reset/altr,rst-mgr-agilex5.h
@@ -0,0 +1,79 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * Copyright (C) 2023 Intel Corporation. All rights reserved
+ *
+ */
+
+#ifndef _DT_BINDINGS_RESET_ALTR_RST_MGR_AGILEX5_H
+#define _DT_BINDINGS_RESET_ALTR_RST_MGR_AGILEX5_H
+
+/* PER0MODRST */
+#define EMAC0_RESET 32
+#define EMAC1_RESET 33
+#define EMAC2_RESET 34
+#define USB0_RESET 35
+#define USB1_RESET 36
+#define NAND_RESET 37
+#define SOFT_PHY_RESET 38
+#define SDMMC_RESET 39
+#define EMAC0_OCP_RESET 40
+#define EMAC1_OCP_RESET 41
+#define EMAC2_OCP_RESET 42
+#define USB0_OCP_RESET 43
+#define USB1_OCP_RESET 44
+#define NAND_OCP_RESET 45
+/* 46 is empty */
+#define SDMMC_OCP_RESET 47
+#define DMA_RESET 48
+#define SPIM0_RESET 49
+#define SPIM1_RESET 50
+#define SPIS0_RESET 51
+#define SPIS1_RESET 52
+#define DMA_OCP_RESET 53
+#define EMAC_PTP_RESET 54
+/* 55 is empty*/
+#define DMAIF0_RESET 56
+#define DMAIF1_RESET 57
+#define DMAIF2_RESET 58
+#define DMAIF3_RESET 59
+#define DMAIF4_RESET 60
+#define DMAIF5_RESET 61
+#define DMAIF6_RESET 62
+#define DMAIF7_RESET 63
+
+/* PER1MODRST */
+#define WATCHDOG0_RESET 64
+#define WATCHDOG1_RESET 65
+#define WATCHDOG2_RESET 66
+#define WATCHDOG3_RESET 67
+#define L4SYSTIMER0_RESET 68
+#define L4SYSTIMER1_RESET 69
+#define SPTIMER0_RESET 70
+#define SPTIMER1_RESET 71
+#define I2C0_RESET 72
+#define I2C1_RESET 73
+#define I2C2_RESET 74
+#define I2C3_RESET 75
+#define I2C4_RESET 76
+#define I3C0_RESET 77
+#define I3C1_RESET 78
+/* 79 is empty */
+#define UART0_RESET 80
+#define UART1_RESET 81
+/* 82-87 is empty */
+#define GPIO0_RESET 88
+#define GPIO1_RESET 89
+#define WATCHDOG4_RESET 90
+
+/* BRGMODRST */
+#define SOC2FPGA_RESET 96
+#define LWHPS2FPGA_RESET 97
+#define FPGA2SOC_RESET 98
+#define F2SSDRAM0_RESET 99
+/* 100-101 is empty */
+#define MPFE_RESET 102
+
+/* DBGMODRST */
+#define DBG_RESET 128
+
+#endif
--
2.25.1



2023-06-18 19:18:27

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/4] dt-bindings: clock: Add Intel Agilex5 clocks and resets

On 18/06/2023 15:22, [email protected] wrote:
> From: Niravkumar L Rabara <[email protected]>
>
> Add clock and reset ID definitions for Intel Agilex5 SoCFPGA
>
> Co-developed-by: Teh Wen Ping <[email protected]>
> Signed-off-by: Teh Wen Ping <[email protected]>
> Signed-off-by: Niravkumar L Rabara <[email protected]>
> ---
> .../bindings/clock/intel,agilex5.yaml | 42 ++++++++
> include/dt-bindings/clock/agilex5-clock.h | 100 ++++++++++++++++++
> .../dt-bindings/reset/altr,rst-mgr-agilex5.h | 79 ++++++++++++++
> 3 files changed, 221 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/intel,agilex5.yaml
> create mode 100644 include/dt-bindings/clock/agilex5-clock.h
> create mode 100644 include/dt-bindings/reset/altr,rst-mgr-agilex5.h
>
> diff --git a/Documentation/devicetree/bindings/clock/intel,agilex5.yaml b/Documentation/devicetree/bindings/clock/intel,agilex5.yaml
> new file mode 100644
> index 000000000000..e408c52deefa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/intel,agilex5.yaml

Filename matching compatible, so missing "clk"

> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/intel,agilex5.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Intel SoCFPGA Agilex5 platform clock controller binding

Drop "binding"

> +
> +maintainers:
> + - Teh Wen Ping <[email protected]>
> + - Niravkumar L Rabara <[email protected]>
> +
> +description:
> + The Intel Agilex5 Clock controller is an integrated clock controller, which
> + generates and supplies to all modules.

"generates and supplies" what?

> +
> +properties:
> + compatible:
> + const: intel,agilex5-clkmgr


Why "clkmgr", not "clk"? You did not call it Clock manager anywhere in
the description or title.

> +
> + '#clock-cells':
> + const: 1
> +
> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - '#clock-cells'

Keep the same order as in properties:

> +
> +additionalProperties: false
> +
> +examples:
> + # Clock controller node:
> + - |
> + clkmgr: clock-controller@10d10000 {
> + compatible = "intel,agilex5-clkmgr";
> + reg = <0x10d10000 0x1000>;
> + #clock-cells = <1>;
> + };
> +...
> diff --git a/include/dt-bindings/clock/agilex5-clock.h b/include/dt-bindings/clock/agilex5-clock.h
> new file mode 100644
> index 000000000000..4505b352cd83
> --- /dev/null
> +++ b/include/dt-bindings/clock/agilex5-clock.h

Filename the same as binding. Missing vendor prefix, entirely different
device name.

> @@ -0,0 +1,100 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * Copyright (C) 2022, Intel Corporation
> + */

...

> +
> +#endif /* __AGILEX5_CLOCK_H */
> diff --git a/include/dt-bindings/reset/altr,rst-mgr-agilex5.h b/include/dt-bindings/reset/altr,rst-mgr-agilex5.h
> new file mode 100644
> index 000000000000..81e5e8c89893
> --- /dev/null
> +++ b/include/dt-bindings/reset/altr,rst-mgr-agilex5.h

Same filename as binding.

But why do you need this file? Your device is not a reset controller.

Best regards,
Krzysztof


2023-06-19 02:29:40

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 2/4] dt-bindings: clock: Add Intel Agilex5 clocks and resets


On Sun, 18 Jun 2023 21:22:33 +0800, [email protected] wrote:
> From: Niravkumar L Rabara <[email protected]>
>
> Add clock and reset ID definitions for Intel Agilex5 SoCFPGA
>
> Co-developed-by: Teh Wen Ping <[email protected]>
> Signed-off-by: Teh Wen Ping <[email protected]>
> Signed-off-by: Niravkumar L Rabara <[email protected]>
> ---
> .../bindings/clock/intel,agilex5.yaml | 42 ++++++++
> include/dt-bindings/clock/agilex5-clock.h | 100 ++++++++++++++++++
> .../dt-bindings/reset/altr,rst-mgr-agilex5.h | 79 ++++++++++++++
> 3 files changed, 221 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/intel,agilex5.yaml
> create mode 100644 include/dt-bindings/clock/agilex5-clock.h
> create mode 100644 include/dt-bindings/reset/altr,rst-mgr-agilex5.h
>

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:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/intel,agilex5.yaml: title: 'Intel SoCFPGA Agilex5 platform clock controller binding' should not be valid under {'pattern': '([Bb]inding| [Ss]chema)'}
hint: Everything is a binding/schema, no need to say it. Describe what hardware the binding is for.
from schema $id: http://devicetree.org/meta-schemas/core.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


2023-06-20 10:58:46

by wen.ping.teh

[permalink] [raw]
Subject: Re: [PATCH 2/4] dt-bindings: clock: Add Intel Agilex5 clocks and resets

>From: Krzysztof Kozlowski <[email protected]>
>> From: Niravkumar L Rabara <[email protected]>
>>
>> Add clock and reset ID definitions for Intel Agilex5 SoCFPGA
>>
>> Co-developed-by: Teh Wen Ping <[email protected]>
>> Signed-off-by: Teh Wen Ping <[email protected]>
>> Signed-off-by: Niravkumar L Rabara <[email protected]>
>> ---
>> .../bindings/clock/intel,agilex5.yaml | 42 ++++++++
>> include/dt-bindings/clock/agilex5-clock.h | 100 ++++++++++++++++++
>> .../dt-bindings/reset/altr,rst-mgr-agilex5.h | 79 ++++++++++++++
>> 3 files changed, 221 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/clock/intel,agilex5.yaml
>> create mode 100644 include/dt-bindings/clock/agilex5-clock.h
>> create mode 100644 include/dt-bindings/reset/altr,rst-mgr-agilex5.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/intel,agilex5.yaml b/Documentation/devicetree/bindings/clock/intel,agilex5.yaml
>> new file mode 100644
>> index 000000000000..e408c52deefa
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/intel,agilex5.yaml
>
>Filename matching compatible, so missing "clk"
>

Will update in V2 patch, rename file to intel,agilex5-clk.yaml

>> @@ -0,0 +1,42 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/intel,agilex5.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Intel SoCFPGA Agilex5 platform clock controller binding
>
>Drop "binding"
>

Will update in V2 patch.

>> +
>> +maintainers:
>> + - Teh Wen Ping <[email protected]>
>> + - Niravkumar L Rabara <[email protected]>
>> +
>> +description:
>> + The Intel Agilex5 Clock controller is an integrated clock controller, which
>> + generates and supplies to all modules.
>
>"generates and supplies" what?

Will change to "generates and supplies clock" in V2 patch.

>
>> +
>> +properties:
>> + compatible:
>> + const: intel,agilex5-clkmgr
>
>
>Why "clkmgr", not "clk"? You did not call it Clock manager anywhere in
>the description or title.
>

The register in Agilex5 handling the clock is named clock_mgr.
Previous IntelSocFPGA, Agilex and Stratix10, are also named clkmgr.

>> +
>> + '#clock-cells':
>> + const: 1
>> +
>> + reg:
>> + maxItems: 1
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - '#clock-cells'
>
>Keep the same order as in properties:
>

Will update in V2 patch.

>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + # Clock controller node:
>> + - |
>> + clkmgr: clock-controller@10d10000 {
>> + compatible = "intel,agilex5-clkmgr";
>> + reg = <0x10d10000 0x1000>;
>> + #clock-cells = <1>;
>> + };
>> +...
>> diff --git a/include/dt-bindings/clock/agilex5-clock.h b/include/dt-bindings/clock/agilex5-clock.h
>> new file mode 100644
>> index 000000000000..4505b352cd83
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/agilex5-clock.h
>
>Filename the same as binding. Missing vendor prefix, entirely different
>device name.
>

Will change filename to intel,agilex5-clock.h in V2.

>> @@ -0,0 +1,100 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
>> +/*
>> + * Copyright (C) 2022, Intel Corporation
>> + */
>
>...
>
>> +
>> +#endif /* __AGILEX5_CLOCK_H */
>> diff --git a/include/dt-bindings/reset/altr,rst-mgr-agilex5.h b/include/dt-bindings/reset/altr,rst-mgr-agilex5.h
>> new file mode 100644
>> index 000000000000..81e5e8c89893
>> --- /dev/null
>> +++ b/include/dt-bindings/reset/altr,rst-mgr-agilex5.h
>
>Same filename as binding.
>
>But why do you need this file? Your device is not a reset controller.

Because Agilex5 device tree uses the reset definition from this file.

Best Regards,
Wen Ping

2023-06-20 11:16:07

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/4] dt-bindings: clock: Add Intel Agilex5 clocks and resets

On 20/06/2023 12:39, [email protected] wrote:
>>
>>> +
>>> +properties:
>>> + compatible:
>>> + const: intel,agilex5-clkmgr
>>
>>
>> Why "clkmgr", not "clk"? You did not call it Clock manager anywhere in
>> the description or title.
>>
>
> The register in Agilex5 handling the clock is named clock_mgr.
> Previous IntelSocFPGA, Agilex and Stratix10, are also named clkmgr.

So use it in description.

>
>>> +
>>> + '#clock-cells':
>>> + const: 1
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - '#clock-cells'
>>
>> Keep the same order as in properties:
>>
>
> Will update in V2 patch.
>
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> + # Clock controller node:
>>> + - |
>>> + clkmgr: clock-controller@10d10000 {
>>> + compatible = "intel,agilex5-clkmgr";
>>> + reg = <0x10d10000 0x1000>;
>>> + #clock-cells = <1>;
>>> + };
>>> +...
>>> diff --git a/include/dt-bindings/clock/agilex5-clock.h b/include/dt-bindings/clock/agilex5-clock.h
>>> new file mode 100644
>>> index 000000000000..4505b352cd83
>>> --- /dev/null
>>> +++ b/include/dt-bindings/clock/agilex5-clock.h
>>
>> Filename the same as binding. Missing vendor prefix, entirely different
>> device name.
>>
>
> Will change filename to intel,agilex5-clock.h in V2.

Read the comment - same as binding. You did not call binding that way...
unless you rename the binding.

>>
>>> +
>>> +#endif /* __AGILEX5_CLOCK_H */
>>> diff --git a/include/dt-bindings/reset/altr,rst-mgr-agilex5.h b/include/dt-bindings/reset/altr,rst-mgr-agilex5.h
>>> new file mode 100644
>>> index 000000000000..81e5e8c89893
>>> --- /dev/null
>>> +++ b/include/dt-bindings/reset/altr,rst-mgr-agilex5.h
>>
>> Same filename as binding.
>>
>> But why do you need this file? Your device is not a reset controller.
>
> Because Agilex5 device tree uses the reset definition from this file.

That's not the correct reason. The binding header has nothing to do with
this device. You miss another patch adding support for your device
(compatible) with this header.

Best regards,
Krzysztof


2023-06-21 11:45:23

by wen.ping.teh

[permalink] [raw]
Subject: Re: [PATCH 2/4] dt-bindings: clock: Add Intel Agilex5 clocks and resets

>From: Krzysztof Kozlowski @ 2023-06-20 11:06 UTC (permalink / raw)
>>>
>>>> +
>>>> +properties:
>>>> + compatible:
>>>> + const: intel,agilex5-clkmgr
>>>
>>>
>>> Why "clkmgr", not "clk"? You did not call it Clock manager anywhere in
>>> the description or title.
>>>
>>
>> The register in Agilex5 handling the clock is named clock_mgr.
>> Previous IntelSocFPGA, Agilex and Stratix10, are also named clkmgr.
>
>So use it in description.

Noted. Will update the description in V2.

>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> + # Clock controller node:
>>>> + - |
>>>> + clkmgr: clock-controller@10d10000 {
>>>> + compatible = "intel,agilex5-clkmgr";
>>>> + reg = <0x10d10000 0x1000>;
>>>> + #clock-cells = <1>;
>>>> + };
>>>> +...
>>>> diff --git a/include/dt-bindings/clock/agilex5-clock.h b/include/dt-bindings/clock/agilex5-clock.h
>>>> new file mode 100644
>>>> index 000000000000..4505b352cd83
>>>> --- /dev/null
>>>> +++ b/include/dt-bindings/clock/agilex5-clock.h
>>>
>>> Filename the same as binding. Missing vendor prefix, entirely different
>>> device name.
>>>
>>
>> Will change filename to intel,agilex5-clock.h in V2.
>
>Read the comment - same as binding. You did not call binding that way...
>unless you rename the binding.

Just to confirm, the binding name you are referring to is "intel,agilex5-clkmgr"?
I will change the filename to intel,agilex5-clkmgr.h in V2.

Best Regards,
Wen Ping