2022-11-07 02:36:48

by Liu Peibao

[permalink] [raw]
Subject: [PATCH 1/2] irqchip: loongarch-cpu: add DT support

LoongArch is coming to support booting with FDT, so DT
support of this driver is desired.

Signed-off-by: Liu Peibao <[email protected]>
---
drivers/irqchip/irq-loongarch-cpu.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/drivers/irqchip/irq-loongarch-cpu.c b/drivers/irqchip/irq-loongarch-cpu.c
index 741612ba6a52..a28b7c549654 100644
--- a/drivers/irqchip/irq-loongarch-cpu.c
+++ b/drivers/irqchip/irq-loongarch-cpu.c
@@ -92,6 +92,25 @@ static const struct irq_domain_ops loongarch_cpu_intc_irq_domain_ops = {
.xlate = irq_domain_xlate_onecell,
};

+#ifdef CONFIG_OF
+int __init loongarch_cpu_irq_of_init(struct device_node *of_node,
+ struct device_node *parent)
+{
+ cpuintc_handle = of_node_to_fwnode(of_node);
+
+ irq_domain = irq_domain_create_linear(cpuintc_handle, EXCCODE_INT_NUM,
+ &loongarch_cpu_intc_irq_domain_ops, NULL);
+ if (!irq_domain)
+ panic("Failed to add irqdomain for loongarch CPU");
+
+ set_handle_irq(&handle_cpu_irq);
+
+ return 0;
+}
+IRQCHIP_DECLARE(cpu_intc, "loongson,cpu-interrupt-controller",
+ loongarch_cpu_irq_of_init);
+#endif
+
static int __init
liointc_parse_madt(union acpi_subtable_headers *header,
const unsigned long end)
--
2.20.1



2022-11-07 03:13:25

by Liu Peibao

[permalink] [raw]
Subject: [PATCH 2/2] dt-bindings: interrupt-controller: add yaml for LoongArch CPU interrupt controller

Signed-off-by: Liu Peibao <[email protected]>
---
.../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.
+
+maintainers:
+ - Liu Peibao <[email protected]>
+
+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;
+ };
--
2.20.1


2022-11-07 07:15:52

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/2] irqchip: loongarch-cpu: add DT support

Hi Liu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tip/irq/core]
[also build test WARNING on robh/for-next linus/master v6.1-rc4 next-20221104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Liu-Peibao/irqchip-loongarch-cpu-add-DT-support/20221107-103606
patch link: https://lore.kernel.org/r/20221107023404.26730-1-liupeibao%40loongson.cn
patch subject: [PATCH 1/2] irqchip: loongarch-cpu: add DT support
config: loongarch-allyesconfig
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/10a355fa4bf4ae52f3889b1d4bec5be1143dd4e2
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Liu-Peibao/irqchip-loongarch-cpu-add-DT-support/20221107-103606
git checkout 10a355fa4bf4ae52f3889b1d4bec5be1143dd4e2
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> drivers/irqchip/irq-loongarch-cpu.c:96:12: warning: no previous prototype for 'loongarch_cpu_irq_of_init' [-Wmissing-prototypes]
96 | int __init loongarch_cpu_irq_of_init(struct device_node *of_node,
| ^~~~~~~~~~~~~~~~~~~~~~~~~


vim +/loongarch_cpu_irq_of_init +96 drivers/irqchip/irq-loongarch-cpu.c

94
95 #ifdef CONFIG_OF
> 96 int __init loongarch_cpu_irq_of_init(struct device_node *of_node,
97 struct device_node *parent)
98 {
99 cpuintc_handle = of_node_to_fwnode(of_node);
100
101 irq_domain = irq_domain_create_linear(cpuintc_handle, EXCCODE_INT_NUM,
102 &loongarch_cpu_intc_irq_domain_ops, NULL);
103 if (!irq_domain)
104 panic("Failed to add irqdomain for loongarch CPU");
105
106 set_handle_irq(&handle_cpu_irq);
107
108 return 0;
109 }
110 IRQCHIP_DECLARE(cpu_intc, "loongson,cpu-interrupt-controller",
111 loongarch_cpu_irq_of_init);
112 #endif
113

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (2.66 kB)
config (329.32 kB)
Download all attachments

2022-11-07 09:05:40

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] dt-bindings: interrupt-controller: add yaml for LoongArch CPU interrupt controller

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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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


2022-11-07 09:38:33

by Liu Peibao

[permalink] [raw]
Subject: Re: [PATCH 2/2] dt-bindings: interrupt-controller: add yaml for LoongArch CPU interrupt controller

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 <[email protected]>
>> ---
>> .../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 <[email protected]>
>> +
>> +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


2022-11-07 10:10:10

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] dt-bindings: interrupt-controller: add yaml for LoongArch CPU interrupt controller

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 <[email protected]>
>>> ---
>>> .../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 <[email protected]>
>>> +
>>> +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


2022-11-07 11:45:25

by Liu Peibao

[permalink] [raw]
Subject: Re: [PATCH 2/2] dt-bindings: interrupt-controller: add yaml for LoongArch CPU interrupt controller

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 <[email protected]>
>>>> ---
>>>> .../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 <[email protected]>
>>>> +
>>>> +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


2022-11-07 12:59:26

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] dt-bindings: interrupt-controller: add yaml for LoongArch CPU interrupt controller

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


2022-11-07 13:00:18

by Liu Peibao

[permalink] [raw]
Subject: Re: [PATCH 2/2] dt-bindings: interrupt-controller: add yaml for LoongArch CPU interrupt controller

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