[2/2] dt-bindings: interrupt-controller: add yaml for LoongArch CPU interrupt controller

Message ID 20221107023404.26730-2-liupeibao@loongson.cn
State New
Headers
Series [1/2] irqchip: loongarch-cpu: add DT support |

Commit Message

Liu Peibao Nov. 7, 2022, 2:34 a.m. UTC
  Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
---
 .../loongarch,cpu-interrupt-controller.yaml   | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml
  

Comments

Krzysztof Kozlowski Nov. 7, 2022, 8:28 a.m. UTC | #1
On 07/11/2022 03:34, Liu Peibao wrote:

Add commit msg explaining what you are doing here (e.g. the hardware).

> Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
> ---
>  .../loongarch,cpu-interrupt-controller.yaml   | 42 +++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml
> new file mode 100644
> index 000000000000..30b742661a3f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/loongarch,cpu-interrupt-controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LoongArch CPU Interrupt Controller
> +
> +description: >
> +   On LoongArch the loongarch_cpu_irq_of_init() helper can be used to initialize
> +   the 14 CPU IRQs from a devicetree file and create a irq_domain for this IRQ
> +   controller.
> +
> +   With the irq_domain in place we can describe how the 14 IRQs are wired to the
> +   platforms internal interrupt controller cascade.

This should be the description of hardware, not Linux drivers.

> +
> +maintainers:
> +  - Liu Peibao <liupeibao@loongson.cn>
> +
> +properties:
> +  compatible:
> +    const: loongarch,cpu-interrupt-controller

You have exactly one and only one type of CPU interrupt controller for
all your Loongarch designs? All current and all future? All?

> +
> +  '#interrupt-cells':

Best regards,
Krzysztof
  
Liu Peibao Nov. 7, 2022, 9:21 a.m. UTC | #2
On 11/7/22 4:28 PM, Krzysztof Kozlowski wrote:
> On 07/11/2022 03:34, Liu Peibao wrote:
> 
> Add commit msg explaining what you are doing here (e.g. the hardware).
> 

I just add this yaml for what I did in patch 1/2 and the header seems enough
to describe what I want to, so I did not add the commit log.

>> Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
>> ---
>>  .../loongarch,cpu-interrupt-controller.yaml   | 42 +++++++++++++++++++
>>  1 file changed, 42 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml
>> new file mode 100644
>> index 000000000000..30b742661a3f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml
>> @@ -0,0 +1,42 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/interrupt-controller/loongarch,cpu-interrupt-controller.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: LoongArch CPU Interrupt Controller
>> +
>> +description: >
>> +   On LoongArch the loongarch_cpu_irq_of_init() helper can be used to initialize
>> +   the 14 CPU IRQs from a devicetree file and create a irq_domain for this IRQ
>> +   controller.
>> +
>> +   With the irq_domain in place we can describe how the 14 IRQs are wired to the
>> +   platforms internal interrupt controller cascade.
> 
> This should be the description of hardware, not Linux drivers.
> 

OK, I will remove this in the next version of this patch.

>> +
>> +maintainers:
>> +  - Liu Peibao <liupeibao@loongson.cn>
>> +
>> +properties:
>> +  compatible:
>> +    const: loongarch,cpu-interrupt-controller
> 
> You have exactly one and only one type of CPU interrupt controller for
> all your Loongarch designs? All current and all future? All?
> 

It is sure of that "all current and recent designs". It is really hard to limit the
design in the distant future.

And if there is updating, maybe I will add additional things like this:
"loongarch,cpu-interrupt-controller-2.0".

BR,
Peibao
  
Krzysztof Kozlowski Nov. 7, 2022, 9:55 a.m. UTC | #3
On 07/11/2022 10:21, Liu Peibao wrote:
> On 11/7/22 4:28 PM, Krzysztof Kozlowski wrote:
>> On 07/11/2022 03:34, Liu Peibao wrote:
>>
>> Add commit msg explaining what you are doing here (e.g. the hardware).
>>
> 
> I just add this yaml for what I did in patch 1/2 and the header seems enough
> to describe what I want to, so I did not add the commit log.

This should instead describe briefly the hardware here.

> 
>>> Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
>>> ---
>>>  .../loongarch,cpu-interrupt-controller.yaml   | 42 +++++++++++++++++++
>>>  1 file changed, 42 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml
>>> new file mode 100644
>>> index 000000000000..30b742661a3f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml
>>> @@ -0,0 +1,42 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/interrupt-controller/loongarch,cpu-interrupt-controller.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: LoongArch CPU Interrupt Controller
>>> +
>>> +description: >
>>> +   On LoongArch the loongarch_cpu_irq_of_init() helper can be used to initialize
>>> +   the 14 CPU IRQs from a devicetree file and create a irq_domain for this IRQ
>>> +   controller.
>>> +
>>> +   With the irq_domain in place we can describe how the 14 IRQs are wired to the
>>> +   platforms internal interrupt controller cascade.
>>
>> This should be the description of hardware, not Linux drivers.
>>
> 
> OK, I will remove this in the next version of this patch.
> 
>>> +
>>> +maintainers:
>>> +  - Liu Peibao <liupeibao@loongson.cn>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: loongarch,cpu-interrupt-controller
>>
>> You have exactly one and only one type of CPU interrupt controller for
>> all your Loongarch designs? All current and all future? All?
>>
> 
> It is sure of that "all current and recent designs". It is really hard to limit the
> design in the distant future.
> 
> And if there is updating, maybe I will add additional things like this:
> "loongarch,cpu-interrupt-controller-2.0".

Unless you have a clear versioning of your hardware, adding 2.0 won't be
correct. Don't you have this for specific SoC?

Best regards,
Krzysztof
  
Liu Peibao Nov. 7, 2022, 11:20 a.m. UTC | #4
On 11/7/22 5:55 PM, Krzysztof Kozlowski wrote:
> On 07/11/2022 10:21, Liu Peibao wrote:
>> On 11/7/22 4:28 PM, Krzysztof Kozlowski wrote:
>>> On 07/11/2022 03:34, Liu Peibao wrote:
>>>
>>> Add commit msg explaining what you are doing here (e.g. the hardware).
>>>
>>
>> I just add this yaml for what I did in patch 1/2 and the header seems enough
>> to describe what I want to, so I did not add the commit log.
> 
> This should instead describe briefly the hardware here.
> 

How about I add the following comments:

"Current LoongArch compatible CPUs support 14 CPU IRQs. We can describe how
the 14 IRQs are wired to the platforms internal interrupt controller cascade
by devicetree."


>>
>>>> Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
>>>> ---
>>>>  .../loongarch,cpu-interrupt-controller.yaml   | 42 +++++++++++++++++++
>>>>  1 file changed, 42 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml
>>>> new file mode 100644
>>>> index 000000000000..30b742661a3f
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml
>>>> @@ -0,0 +1,42 @@
>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/interrupt-controller/loongarch,cpu-interrupt-controller.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: LoongArch CPU Interrupt Controller
>>>> +
>>>> +description: >
>>>> +   On LoongArch the loongarch_cpu_irq_of_init() helper can be used to initialize
>>>> +   the 14 CPU IRQs from a devicetree file and create a irq_domain for this IRQ
>>>> +   controller.
>>>> +
>>>> +   With the irq_domain in place we can describe how the 14 IRQs are wired to the
>>>> +   platforms internal interrupt controller cascade.
>>>
>>> This should be the description of hardware, not Linux drivers.
>>>
>>
>> OK, I will remove this in the next version of this patch.
>>
>>>> +
>>>> +maintainers:
>>>> +  - Liu Peibao <liupeibao@loongson.cn>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: loongarch,cpu-interrupt-controller
>>>
>>> You have exactly one and only one type of CPU interrupt controller for
>>> all your Loongarch designs? All current and all future? All?
>>>
>>
>> It is sure of that "all current and recent designs". It is really hard to limit the
>> design in the distant future.
>>
>> And if there is updating, maybe I will add additional things like this:
>> "loongarch,cpu-interrupt-controller-2.0".
> 
> Unless you have a clear versioning of your hardware, adding 2.0 won't be
> correct. Don't you have this for specific SoC?
> 

The "loongarch,cpu-interrupt-controller" now is compatible for all the LoongArch
compatible CPUs, not specific for one chip. And we may keep this CPU interrupt
controller for a long time.

BR,
Peibao
  
Krzysztof Kozlowski Nov. 7, 2022, 11:33 a.m. UTC | #5
On 07/11/2022 12:20, Liu Peibao wrote:
> On 11/7/22 5:55 PM, Krzysztof Kozlowski wrote:
>> On 07/11/2022 10:21, Liu Peibao wrote:
>>> On 11/7/22 4:28 PM, Krzysztof Kozlowski wrote:
>>>> On 07/11/2022 03:34, Liu Peibao wrote:
>>>>
>>>> Add commit msg explaining what you are doing here (e.g. the hardware).
>>>>
>>>
>>> I just add this yaml for what I did in patch 1/2 and the header seems enough
>>> to describe what I want to, so I did not add the commit log.
>>
>> This should instead describe briefly the hardware here.
>>
> 
> How about I add the following comments:
> 
> "Current LoongArch compatible CPUs support 14 CPU IRQs. We can describe how
> the 14 IRQs are wired to the platforms internal interrupt controller cascade
> by devicetree."

Sure.

>>>>> +    const: loongarch,cpu-interrupt-controller
>>>>
>>>> You have exactly one and only one type of CPU interrupt controller for
>>>> all your Loongarch designs? All current and all future? All?
>>>>
>>>
>>> It is sure of that "all current and recent designs". It is really hard to limit the
>>> design in the distant future.
>>>
>>> And if there is updating, maybe I will add additional things like this:
>>> "loongarch,cpu-interrupt-controller-2.0".
>>
>> Unless you have a clear versioning of your hardware, adding 2.0 won't be
>> correct. Don't you have this for specific SoC?
>>
> 
> The "loongarch,cpu-interrupt-controller" now is compatible for all the LoongArch
> compatible CPUs, not specific for one chip. And we may keep this CPU interrupt
> controller for a long time.

Still specific compatibles (as fallbacks) are used for such cases, so
why is this different? Hardware compatible with several other devices
still gets specific compatible, right?

You cannot have "-2.0" suffix in the future just because "you want", so
be sure that your choice is reasonable.

Best regards,
Krzysztof
  
Liu Peibao Nov. 7, 2022, 12:12 p.m. UTC | #6
On 11/7/22 7:33 PM, Krzysztof Kozlowski wrote:
>>>>>> +    const: loongarch,cpu-interrupt-controller
>>>>>
>>>>> You have exactly one and only one type of CPU interrupt controller for
>>>>> all your Loongarch designs? All current and all future? All?
>>>>>
>>>>
>>>> It is sure of that "all current and recent designs". It is really hard to limit the
>>>> design in the distant future.
>>>>
>>>> And if there is updating, maybe I will add additional things like this:
>>>> "loongarch,cpu-interrupt-controller-2.0".
>>>
>>> Unless you have a clear versioning of your hardware, adding 2.0 won't be
>>> correct. Don't you have this for specific SoC?
>>>
>>
>> The "loongarch,cpu-interrupt-controller" now is compatible for all the LoongArch
>> compatible CPUs, not specific for one chip. And we may keep this CPU interrupt
>> controller for a long time.
> 
> Still specific compatibles (as fallbacks) are used for such cases, so
> why is this different? Hardware compatible with several other devices
> still gets specific compatible, right?
>

I don't really agree with that. This is a specified higher level abstract of all
our designed hardware. We could do this as we have unified this in hardware. So
this compatible could be simple.
 
> You cannot have "-2.0" suffix in the future just because "you want", so
> be sure that your choice is reasonable.
> 

It was an example and the CPUs IRQs hardware updating is not on our schedule.
If I do some thing like "-2.0" in the future, I will find a proper way and
be reasonable.

BR,
Peibao
  

Patch

diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml
new file mode 100644
index 000000000000..30b742661a3f
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/loongarch,cpu-interrupt-controller.yaml
@@ -0,0 +1,42 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/loongarch,cpu-interrupt-controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LoongArch CPU Interrupt Controller
+
+description: >
+   On LoongArch the loongarch_cpu_irq_of_init() helper can be used to initialize
+   the 14 CPU IRQs from a devicetree file and create a irq_domain for this IRQ
+   controller.
+
+   With the irq_domain in place we can describe how the 14 IRQs are wired to the
+   platforms internal interrupt controller cascade.
+
+maintainers:
+  - Liu Peibao <liupeibao@loongson.cn>
+
+properties:
+  compatible:
+    const: loongarch,cpu-interrupt-controller
+
+  '#interrupt-cells':
+    const: 1
+
+  interrupt-controller: true
+
+additionalProperties: false
+
+required:
+  - compatible
+  - '#interrupt-cells'
+  - interrupt-controller
+
+examples:
+  - |
+    interrupt-controller {
+      compatible = "loongarch,cpu-interrupt-controller";
+      #interrupt-cells = <1>;
+      interrupt-controller;
+    };