* [PATCH v5 0/5] dt-bindings: interrupt-controller: Fix some loongson,liointc warnings
@ 2023-11-20 9:06 Binbin Zhou
2023-11-20 9:06 ` [PATCH v5 1/5] dt-bindings: interrupt-controller: loongson,liointc: Standardize the naming of 'loongson,parent-int-map' Binbin Zhou
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Binbin Zhou @ 2023-11-20 9:06 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Huacai Chen, loongson-kernel, devicetree, Thomas Bogendoerfer,
Jiaxun Yang, linux-mips, lvjianmin, WANG Xuerui, loongarch,
linux-kernel, Binbin Zhou
Hi all:
Some liointc-related DTBS_CHECK warnings were found when trying to
introduce the Loongson-2K DTS{I} for LoongArch.
This patch series attempts to fix those warnings, as well as fixing
non-standard property naming.
Of course, these fixes also apply to MIPS Loongson-2K1000.
Thanks.
-----
V5:
- Add Reviewed-by tag;
patch(1/5):
- Just drop 'loongson,parent_int_map' instead of marking it as
deprecated.
Link to V4:
https://lore.kernel.org/all/cover.1699521866.git.zhoubinbin@loongson.cn/
V4:
- Add Acked-by tag;
patch(2/5):
- Just add 'maxitem 2' instead of duplicating the list;
patch(3/5):
- Rewite commit message for 'interrupt-names'.
Link to V3:
https://lore.kernel.org/all/cover.1698717154.git.zhoubinbin@loongson.cn/
V3:
patch(1/5):
- new patch, 'loongson,parent_int_map' renamed to 'loongson,parent-int-map';
patch(2/5)(3/5):
- Separate the change points into separate patches;
patch(4/5):
- new patch, make sure both parent map forms can be parsed;
patch(5/5):
- new patch, fix 'loongson,parent_int_map' references in mips loongson
dts{i}.
Link to V2:
https://lore.kernel.org/all/20230821061315.3416836-1-zhoubinbin@loongson.cn/
V2:
1. Update commit message;
2. "interruprt-names" should be "required", the driver gets the parent
interrupts through it;
3. Add more descriptions to explain the rationale for multiple nodes;
4. Rewrite if-else statements.
Link to V1:
https://lore.kernel.org/all/20230815084713.1627520-1-zhoubinbin@loongson.cn/
Binbin Zhou (5):
dt-bindings: interrupt-controller: loongson,liointc: Standardize the
naming of 'loongson,parent-int-map'
dt-bindings: interrupt-controller: loongson,liointc: Fix dtbs_check
warning for reg-names
dt-bindings: interrupt-controller: loongson,liointc: Fix dtbs_check
warning for interrupt-names
irqchip/loongson-liointc: Fix 'loongson,parent_int_map' parse
MIPS: Loongson64: DTS: Fix 'loongson,parent_int_map' references
.../loongson,liointc.yaml | 26 +++++++++++--------
.../boot/dts/loongson/loongson64-2k1000.dtsi | 4 +--
.../dts/loongson/loongson64c-package.dtsi | 2 +-
.../dts/loongson/loongson64g-package.dtsi | 2 +-
.../dts/loongson/loongson64v_4core_virtio.dts | 2 +-
drivers/irqchip/irq-loongson-liointc.c | 7 ++++-
6 files changed, 26 insertions(+), 17 deletions(-)
--
2.39.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 1/5] dt-bindings: interrupt-controller: loongson,liointc: Standardize the naming of 'loongson,parent-int-map'
2023-11-20 9:06 [PATCH v5 0/5] dt-bindings: interrupt-controller: Fix some loongson,liointc warnings Binbin Zhou
@ 2023-11-20 9:06 ` Binbin Zhou
2023-11-27 18:28 ` Rob Herring
2023-11-20 9:06 ` [PATCH v5 2/5] dt-bindings: interrupt-controller: loongson,liointc: Fix dtbs_check warning for reg-names Binbin Zhou
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Binbin Zhou @ 2023-11-20 9:06 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Huacai Chen, loongson-kernel, devicetree, Thomas Bogendoerfer,
Jiaxun Yang, linux-mips, lvjianmin, WANG Xuerui, loongarch,
linux-kernel, Binbin Zhou
Since the 'loongson,parent_int_map' attribute naming is non-standard, we
should use 'loongson,parent-int-map' instead.
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Acked-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
.../bindings/interrupt-controller/loongson,liointc.yaml | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
index 00b570c82903..70c125bf8095 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
@@ -54,7 +54,7 @@ properties:
'#interrupt-cells':
const: 2
- loongson,parent_int_map:
+ loongson,parent-int-map:
description: |
This property points how the children interrupts will be mapped into CPU
interrupt lines. Each cell refers to a parent interrupt line from 0 to 3
@@ -71,8 +71,7 @@ required:
- interrupts
- interrupt-controller
- '#interrupt-cells'
- - loongson,parent_int_map
-
+ - loongson,parent-int-map
unevaluatedProperties: false
@@ -109,11 +108,10 @@ examples:
interrupts = <2>, <3>;
interrupt-names = "int0", "int1";
- loongson,parent_int_map = <0xf0ffffff>, /* int0 */
+ loongson,parent-int-map = <0xf0ffffff>, /* int0 */
<0x0f000000>, /* int1 */
<0x00000000>, /* int2 */
<0x00000000>; /* int3 */
-
};
...
--
2.39.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 2/5] dt-bindings: interrupt-controller: loongson,liointc: Fix dtbs_check warning for reg-names
2023-11-20 9:06 [PATCH v5 0/5] dt-bindings: interrupt-controller: Fix some loongson,liointc warnings Binbin Zhou
2023-11-20 9:06 ` [PATCH v5 1/5] dt-bindings: interrupt-controller: loongson,liointc: Standardize the naming of 'loongson,parent-int-map' Binbin Zhou
@ 2023-11-20 9:06 ` Binbin Zhou
2023-11-20 9:06 ` [PATCH v5 3/5] dt-bindings: interrupt-controller: loongson,liointc: Fix dtbs_check warning for interrupt-names Binbin Zhou
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Binbin Zhou @ 2023-11-20 9:06 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Huacai Chen, loongson-kernel, devicetree, Thomas Bogendoerfer,
Jiaxun Yang, linux-mips, lvjianmin, WANG Xuerui, loongarch,
linux-kernel, Binbin Zhou, Rob Herring
As we know, the Loongson-2K0500 is a single-core CPU, and the
core1-related register (isr1) does not exist. So "reg" and "reg-names"
should be set to "minItems 2"(main nad isr0).
This fixes dtbs_check warning:
DTC_CHK arch/loongarch/boot/dts/loongson-2k0500-ref.dtb
arch/loongarch/boot/dts/loongson-2k0500-ref.dtb: interrupt-controller@1fe11400: reg-names: ['main', 'isr0'] is too short
From schema: Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
arch/loongarch/boot/dts/loongson-2k0500-ref.dtb: interrupt-controller@1fe11400: Unevaluated properties are not allowed ('reg-names' was unexpected)
From schema: Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
arch/loongarch/boot/dts/loongson-2k0500-ref.dtb: interrupt-controller@1fe11400: reg: [[0, 534844416, 0, 64], [0, 534843456, 0, 8]] is too short
From schema: Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
arch/loongarch/boot/dts/loongson-2k0500-ref.dtb: interrupt-controller@1fe11440: reg-names: ['main', 'isr0'] is too short
From schema: Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Acked-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../interrupt-controller/loongson,liointc.yaml | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
index 70c125bf8095..976cd2df1e62 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
@@ -11,8 +11,13 @@ maintainers:
description: |
This interrupt controller is found in the Loongson-3 family of chips and
- Loongson-2K1000 chip, as the primary package interrupt controller which
+ Loongson-2K series chips, as the primary package interrupt controller which
can route local I/O interrupt to interrupt lines of cores.
+ Be aware of the following points.
+ 1.The Loongson-2K0500 is a single core CPU;
+ 2.The Loongson-2K0500/2K1000 has 64 device interrupt sources as inputs, so we
+ need to define two nodes in dts{i} to describe the "0-31" and "32-61" interrupt
+ sources respectively.
allOf:
- $ref: /schemas/interrupt-controller.yaml#
@@ -33,6 +38,7 @@ properties:
- const: main
- const: isr0
- const: isr1
+ minItems: 2
interrupt-controller: true
@@ -85,7 +91,8 @@ if:
then:
properties:
reg:
- minItems: 3
+ minItems: 2
+ maxItems: 3
required:
- reg-names
--
2.39.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 3/5] dt-bindings: interrupt-controller: loongson,liointc: Fix dtbs_check warning for interrupt-names
2023-11-20 9:06 [PATCH v5 0/5] dt-bindings: interrupt-controller: Fix some loongson,liointc warnings Binbin Zhou
2023-11-20 9:06 ` [PATCH v5 1/5] dt-bindings: interrupt-controller: loongson,liointc: Standardize the naming of 'loongson,parent-int-map' Binbin Zhou
2023-11-20 9:06 ` [PATCH v5 2/5] dt-bindings: interrupt-controller: loongson,liointc: Fix dtbs_check warning for reg-names Binbin Zhou
@ 2023-11-20 9:06 ` Binbin Zhou
2023-11-20 9:06 ` [PATCH v5 4/5] irqchip/loongson-liointc: Fix 'loongson,parent_int_map' parse Binbin Zhou
2023-11-20 9:06 ` [PATCH v5 5/5] MIPS: Loongson64: DTS: Fix 'loongson,parent_int_map' references Binbin Zhou
4 siblings, 0 replies; 10+ messages in thread
From: Binbin Zhou @ 2023-11-20 9:06 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Huacai Chen, loongson-kernel, devicetree, Thomas Bogendoerfer,
Jiaxun Yang, linux-mips, lvjianmin, WANG Xuerui, loongarch,
linux-kernel, Binbin Zhou, Rob Herring
The Loongson-2K0500/2K1000 CPUs have 64 interrupt sources as inputs, and
a route-mapped node handles up to 32 interrupt sources, so two liointc
nodes are defined in dts{i}.
Of course, we have to make sure that the routing outputs ("intx") of the
two nodes do not conflict, i.e. "int0" can only be used as a routing
output for one of them. Therefore, "interrupt-names" should be defined
as "pattern".
In addition, since "interrupt-names" and "interrupts" are one-to-one
correspondence, we pass it to get the corresponding interrupt number in
the driver. Setting it to "required" does not break ABI, because it
is already logically represented as "required".
This fixes dtbs_check warning:
DTC_CHK arch/loongarch/boot/dts/loongson-2k0500-ref.dtb
arch/loongarch/boot/dts/loongson-2k0500-ref.dtb: interrupt-controller@1fe11440: interrupt-names:0: 'int0' was expected
From schema: Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
arch/loongarch/boot/dts/loongson-2k0500-ref.dtb: interrupt-controller@1fe11440: Unevaluated properties are not allowed ('interrupt-names' was unexpected)
From schema: Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
DTC_CHK arch/loongarch/boot/dts/loongson-2k1000-ref.dtb
arch/loongarch/boot/dts/loongson-2k1000-ref.dtb: interrupt-controller@1fe01440: interrupt-names:0: 'int0' was expected
From schema: Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
arch/loongarch/boot/dts/loongson-2k1000-ref.dtb: interrupt-controller@1fe01440: Unevaluated properties are not allowed ('interrupt-names' was unexpected)
From schema: Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Acked-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../bindings/interrupt-controller/loongson,liointc.yaml | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
index 976cd2df1e62..d078309d67c6 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
@@ -51,11 +51,9 @@ properties:
interrupt-names:
description: List of names for the parent interrupts.
items:
- - const: int0
- - const: int1
- - const: int2
- - const: int3
+ pattern: int[0-3]
minItems: 1
+ maxItems: 4
'#interrupt-cells':
const: 2
@@ -75,6 +73,7 @@ required:
- compatible
- reg
- interrupts
+ - interrupt-names
- interrupt-controller
- '#interrupt-cells'
- loongson,parent-int-map
--
2.39.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 4/5] irqchip/loongson-liointc: Fix 'loongson,parent_int_map' parse
2023-11-20 9:06 [PATCH v5 0/5] dt-bindings: interrupt-controller: Fix some loongson,liointc warnings Binbin Zhou
` (2 preceding siblings ...)
2023-11-20 9:06 ` [PATCH v5 3/5] dt-bindings: interrupt-controller: loongson,liointc: Fix dtbs_check warning for interrupt-names Binbin Zhou
@ 2023-11-20 9:06 ` Binbin Zhou
2023-12-08 14:20 ` Thomas Gleixner
2023-11-20 9:06 ` [PATCH v5 5/5] MIPS: Loongson64: DTS: Fix 'loongson,parent_int_map' references Binbin Zhou
4 siblings, 1 reply; 10+ messages in thread
From: Binbin Zhou @ 2023-11-20 9:06 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Huacai Chen, loongson-kernel, devicetree, Thomas Bogendoerfer,
Jiaxun Yang, linux-mips, lvjianmin, WANG Xuerui, loongarch,
linux-kernel, Binbin Zhou
In keeping with naming standards, 'loongson,parent_int_map' is renamed
to 'loongson,parent-int-map'. But for the driver, we need to make sure
that both forms can be parsed.
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Acked-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
---
drivers/irqchip/irq-loongson-liointc.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c
index e4b33aed1c97..add2e0a955b8 100644
--- a/drivers/irqchip/irq-loongson-liointc.c
+++ b/drivers/irqchip/irq-loongson-liointc.c
@@ -330,6 +330,7 @@ static int __init liointc_of_init(struct device_node *node,
bool have_parent = FALSE;
int sz, i, index, revision, err = 0;
struct resource res;
+ const char *prop_name = "loongson,parent-int-map";
if (!of_device_is_compatible(node, "loongson,liointc-2.0")) {
index = 0;
@@ -350,8 +351,12 @@ static int __init liointc_of_init(struct device_node *node,
if (!have_parent)
return -ENODEV;
+ if (!of_find_property(node, prop_name, &i))
+ /* Fallback to 'loongson,parent_int_map'. */
+ prop_name = "loongson,parent_int_map";
+
sz = of_property_read_variable_u32_array(node,
- "loongson,parent_int_map",
+ prop_name,
&parent_int_map[0],
LIOINTC_NUM_PARENT,
LIOINTC_NUM_PARENT);
--
2.39.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 5/5] MIPS: Loongson64: DTS: Fix 'loongson,parent_int_map' references
2023-11-20 9:06 [PATCH v5 0/5] dt-bindings: interrupt-controller: Fix some loongson,liointc warnings Binbin Zhou
` (3 preceding siblings ...)
2023-11-20 9:06 ` [PATCH v5 4/5] irqchip/loongson-liointc: Fix 'loongson,parent_int_map' parse Binbin Zhou
@ 2023-11-20 9:06 ` Binbin Zhou
4 siblings, 0 replies; 10+ messages in thread
From: Binbin Zhou @ 2023-11-20 9:06 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Huacai Chen, loongson-kernel, devicetree, Thomas Bogendoerfer,
Jiaxun Yang, linux-mips, lvjianmin, WANG Xuerui, loongarch,
linux-kernel, Binbin Zhou
Since 'loongson,parent_int_map' has been dropped, replace all
relevant references in the MIPS loongson dts{i} with
'loongson,parent-int-map'.
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Acked-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
---
arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi | 4 ++--
arch/mips/boot/dts/loongson/loongson64c-package.dtsi | 2 +-
arch/mips/boot/dts/loongson/loongson64g-package.dtsi | 2 +-
arch/mips/boot/dts/loongson/loongson64v_4core_virtio.dts | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
index f878f47e4501..36f499a3772e 100644
--- a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
+++ b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
@@ -71,7 +71,7 @@ liointc0: interrupt-controller@1fe11400 {
interrupts = <2>;
interrupt-names = "int0";
- loongson,parent_int_map = <0xffffffff>, /* int0 */
+ loongson,parent-int-map = <0xffffffff>, /* int0 */
<0x00000000>, /* int1 */
<0x00000000>, /* int2 */
<0x00000000>; /* int3 */
@@ -91,7 +91,7 @@ liointc1: interrupt-controller@1fe11440 {
interrupts = <3>;
interrupt-names = "int1";
- loongson,parent_int_map = <0x00000000>, /* int0 */
+ loongson,parent-int-map = <0x00000000>, /* int0 */
<0xffffffff>, /* int1 */
<0x00000000>, /* int2 */
<0x00000000>; /* int3 */
diff --git a/arch/mips/boot/dts/loongson/loongson64c-package.dtsi b/arch/mips/boot/dts/loongson/loongson64c-package.dtsi
index 5bb876a4de52..38de0108e804 100644
--- a/arch/mips/boot/dts/loongson/loongson64c-package.dtsi
+++ b/arch/mips/boot/dts/loongson/loongson64c-package.dtsi
@@ -35,7 +35,7 @@ liointc: interrupt-controller@3ff01400 {
interrupts = <2>, <3>;
interrupt-names = "int0", "int1";
- loongson,parent_int_map = <0xf0ffffff>, /* int0 */
+ loongson,parent-int-map = <0xf0ffffff>, /* int0 */
<0x0f000000>, /* int1 */
<0x00000000>, /* int2 */
<0x00000000>; /* int3 */
diff --git a/arch/mips/boot/dts/loongson/loongson64g-package.dtsi b/arch/mips/boot/dts/loongson/loongson64g-package.dtsi
index d4314f62ccc2..8972adcb83d6 100644
--- a/arch/mips/boot/dts/loongson/loongson64g-package.dtsi
+++ b/arch/mips/boot/dts/loongson/loongson64g-package.dtsi
@@ -32,7 +32,7 @@ liointc: interrupt-controller@3ff01400 {
interrupts = <2>, <3>;
interrupt-names = "int0", "int1";
- loongson,parent_int_map = <0x00ffffff>, /* int0 */
+ loongson,parent-int-map = <0x00ffffff>, /* int0 */
<0xff000000>, /* int1 */
<0x00000000>, /* int2 */
<0x00000000>; /* int3 */
diff --git a/arch/mips/boot/dts/loongson/loongson64v_4core_virtio.dts b/arch/mips/boot/dts/loongson/loongson64v_4core_virtio.dts
index d0588d81e0c2..88642fee1bbd 100644
--- a/arch/mips/boot/dts/loongson/loongson64v_4core_virtio.dts
+++ b/arch/mips/boot/dts/loongson/loongson64v_4core_virtio.dts
@@ -34,7 +34,7 @@ liointc: interrupt-controller@3ff01400 {
interrupts = <2>, <3>;
interrupt-names = "int0", "int1";
- loongson,parent_int_map = <0x00000001>, /* int0 */
+ loongson,parent-int-map = <0x00000001>, /* int0 */
<0xfffffffe>, /* int1 */
<0x00000000>, /* int2 */
<0x00000000>; /* int3 */
--
2.39.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v5 1/5] dt-bindings: interrupt-controller: loongson,liointc: Standardize the naming of 'loongson,parent-int-map'
2023-11-20 9:06 ` [PATCH v5 1/5] dt-bindings: interrupt-controller: loongson,liointc: Standardize the naming of 'loongson,parent-int-map' Binbin Zhou
@ 2023-11-27 18:28 ` Rob Herring
2023-11-30 13:02 ` Binbin Zhou
0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2023-11-27 18:28 UTC (permalink / raw)
To: Binbin Zhou
Cc: Binbin Zhou, Huacai Chen, Thomas Gleixner, Krzysztof Kozlowski,
Conor Dooley, Huacai Chen, loongson-kernel, devicetree,
Thomas Bogendoerfer, Jiaxun Yang, linux-mips, lvjianmin,
WANG Xuerui, loongarch, linux-kernel
On Mon, Nov 20, 2023 at 05:06:23PM +0800, Binbin Zhou wrote:
> Since the 'loongson,parent_int_map' attribute naming is non-standard, we
> should use 'loongson,parent-int-map' instead.
>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> Acked-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> .../bindings/interrupt-controller/loongson,liointc.yaml | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
> index 00b570c82903..70c125bf8095 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
> @@ -54,7 +54,7 @@ properties:
> '#interrupt-cells':
> const: 2
>
> - loongson,parent_int_map:
> + loongson,parent-int-map:
Not what I said to do. Now you just break the ABI instead of maintaining
both names.
Just use loongson,parent_int_map *forever*. Drop this patch.
Rob
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 1/5] dt-bindings: interrupt-controller: loongson,liointc: Standardize the naming of 'loongson,parent-int-map'
2023-11-27 18:28 ` Rob Herring
@ 2023-11-30 13:02 ` Binbin Zhou
0 siblings, 0 replies; 10+ messages in thread
From: Binbin Zhou @ 2023-11-30 13:02 UTC (permalink / raw)
To: Rob Herring
Cc: Binbin Zhou, Huacai Chen, Thomas Gleixner, Krzysztof Kozlowski,
Conor Dooley, Huacai Chen, loongson-kernel, devicetree,
Thomas Bogendoerfer, Jiaxun Yang, linux-mips, lvjianmin,
WANG Xuerui, loongarch, linux-kernel
On Tue, Nov 28, 2023 at 12:28 AM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Nov 20, 2023 at 05:06:23PM +0800, Binbin Zhou wrote:
> > Since the 'loongson,parent_int_map' attribute naming is non-standard, we
> > should use 'loongson,parent-int-map' instead.
> >
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > Acked-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> > ---
> > .../bindings/interrupt-controller/loongson,liointc.yaml | 8 +++-----
> > 1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
> > index 00b570c82903..70c125bf8095 100644
> > --- a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml
> > @@ -54,7 +54,7 @@ properties:
> > '#interrupt-cells':
> > const: 2
> >
> > - loongson,parent_int_map:
> > + loongson,parent-int-map:
>
> Not what I said to do. Now you just break the ABI instead of maintaining
> both names.
>
> Just use loongson,parent_int_map *forever*. Drop this patch.
Hi Rob:
Thanks for your reply, and I am very sorry that I may have missed your
previous thought, but at the same time I'm confused about how to
handle the 'parent_int_map' attribute.
During the V2 patchset, krzysztof noticed the non-standard naming of
this property and suggested that we rename 'parent_int_map' in the
binding and label it as "deprecated". But you don't think it's worth
doing that.
My understanding is that it doesn't make sense to keep
'parent_int_map' for the new binding, so I'm just going to rename the
property in this version.
It's true that this will result in an ABI break, but at the same time
corresponding changes have been made to the driver as well as in the
existing DTS{i}:
Patch 4: Handles attribute names in both naming styles;
Patch 5: Replace all 'parent_int_map' in the MIPS DTS{i}.
Do you think this is a suitable way to handle this? Or just keep the "_" naming?
Thanks.
Binbin
>
> Rob
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 4/5] irqchip/loongson-liointc: Fix 'loongson,parent_int_map' parse
2023-11-20 9:06 ` [PATCH v5 4/5] irqchip/loongson-liointc: Fix 'loongson,parent_int_map' parse Binbin Zhou
@ 2023-12-08 14:20 ` Thomas Gleixner
2023-12-11 2:37 ` Binbin Zhou
0 siblings, 1 reply; 10+ messages in thread
From: Thomas Gleixner @ 2023-12-08 14:20 UTC (permalink / raw)
To: Binbin Zhou, Binbin Zhou, Huacai Chen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Huacai Chen, loongson-kernel, devicetree, Thomas Bogendoerfer,
Jiaxun Yang, linux-mips, lvjianmin, WANG Xuerui, loongarch,
linux-kernel, Binbin Zhou
On Mon, Nov 20 2023 at 17:06, Binbin Zhou wrote:
$Subject: s/parse/parsing/
> In keeping with naming standards, 'loongson,parent_int_map' is renamed
> to 'loongson,parent-int-map'. But for the driver, we need to make sure
> that both forms can be parsed.
Please keep changelogs in neutral or imperative tone:
For backwards compatibility it is required to parse the original
string too.
Makes it entirely clear what this is about without 'we'. See also:
https://www.kernel.org/doc/html/latest/process/submitting-patches.rst
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> Acked-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
> drivers/irqchip/irq-loongson-liointc.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c
> index e4b33aed1c97..add2e0a955b8 100644
> --- a/drivers/irqchip/irq-loongson-liointc.c
> +++ b/drivers/irqchip/irq-loongson-liointc.c
> @@ -330,6 +330,7 @@ static int __init liointc_of_init(struct device_node *node,
> bool have_parent = FALSE;
> int sz, i, index, revision, err = 0;
> struct resource res;
> + const char *prop_name = "loongson,parent-int-map";
Please don't glue variables randomly into the declaration section:
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations
> if (!of_device_is_compatible(node, "loongson,liointc-2.0")) {
> index = 0;
> @@ -350,8 +351,12 @@ static int __init liointc_of_init(struct device_node *node,
> if (!have_parent)
> return -ENODEV;
>
> + if (!of_find_property(node, prop_name, &i))
> + /* Fallback to 'loongson,parent_int_map'. */
> + prop_name = "loongson,parent_int_map";
This lacks curly brackets:
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#bracket-rules
> sz = of_property_read_variable_u32_array(node,
> - "loongson,parent_int_map",
> + prop_name,
> &parent_int_map[0],
> LIOINTC_NUM_PARENT,
> LIOINTC_NUM_PARENT);
Thanks,
tglx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 4/5] irqchip/loongson-liointc: Fix 'loongson,parent_int_map' parse
2023-12-08 14:20 ` Thomas Gleixner
@ 2023-12-11 2:37 ` Binbin Zhou
0 siblings, 0 replies; 10+ messages in thread
From: Binbin Zhou @ 2023-12-11 2:37 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Binbin Zhou, Huacai Chen, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Huacai Chen, loongson-kernel, devicetree,
Thomas Bogendoerfer, Jiaxun Yang, linux-mips, lvjianmin,
WANG Xuerui, loongarch, linux-kernel
On Fri, Dec 8, 2023 at 10:20 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Mon, Nov 20 2023 at 17:06, Binbin Zhou wrote:
>
> $Subject: s/parse/parsing/
>
> > In keeping with naming standards, 'loongson,parent_int_map' is renamed
> > to 'loongson,parent-int-map'. But for the driver, we need to make sure
> > that both forms can be parsed.
>
> Please keep changelogs in neutral or imperative tone:
>
> For backwards compatibility it is required to parse the original
> string too.
>
> Makes it entirely clear what this is about without 'we'. See also:
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.rst
>
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > Acked-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> > Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> > drivers/irqchip/irq-loongson-liointc.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c
> > index e4b33aed1c97..add2e0a955b8 100644
> > --- a/drivers/irqchip/irq-loongson-liointc.c
> > +++ b/drivers/irqchip/irq-loongson-liointc.c
> > @@ -330,6 +330,7 @@ static int __init liointc_of_init(struct device_node *node,
> > bool have_parent = FALSE;
> > int sz, i, index, revision, err = 0;
> > struct resource res;
> > + const char *prop_name = "loongson,parent-int-map";
>
> Please don't glue variables randomly into the declaration section:
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations
>
> > if (!of_device_is_compatible(node, "loongson,liointc-2.0")) {
> > index = 0;
> > @@ -350,8 +351,12 @@ static int __init liointc_of_init(struct device_node *node,
> > if (!have_parent)
> > return -ENODEV;
> >
> > + if (!of_find_property(node, prop_name, &i))
> > + /* Fallback to 'loongson,parent_int_map'. */
> > + prop_name = "loongson,parent_int_map";
>
> This lacks curly brackets:
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#bracket-rules
Hi Thomas:
Thanks for the detailed review.
Rob suggested in the V5 patchset to remove the
'loongson,parent-int-map' renaming operation as an ABI breakage[1]. I
had tried to explain that the driver would be compatible with the
parsing of both naming styles[2], but unfortunately did not get a
response from Rob.
As a result, I removed the renaming-related patches in the V6
patchset, including this one[3].
However, how the 'loongson,parent-int-map' renaming operation is
finally going to be handled needs to be decided together, and if it
remains needed, I will fix the above issue and submit it as part of
the V7 patchset.
[1]: https://lore.kernel.org/all/20231127182836.GA2150516-robh@kernel.org/
[2]: https://lore.kernel.org/all/CAMpQs4LSTV6PgZSuyQx2Nq+87OHxSa=-Wz5nbhFVsmmvHubQFQ@mail.gmail.com/
[3]: https://lore.kernel.org/all/cover.1701933946.git.zhoubinbin@loongson.cn/
Thanks.
Binbin
>
> > sz = of_property_read_variable_u32_array(node,
> > - "loongson,parent_int_map",
> > + prop_name,
> > &parent_int_map[0],
> > LIOINTC_NUM_PARENT,
> > LIOINTC_NUM_PARENT);
>
> Thanks,
>
> tglx
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-12-11 2:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-20 9:06 [PATCH v5 0/5] dt-bindings: interrupt-controller: Fix some loongson,liointc warnings Binbin Zhou
2023-11-20 9:06 ` [PATCH v5 1/5] dt-bindings: interrupt-controller: loongson,liointc: Standardize the naming of 'loongson,parent-int-map' Binbin Zhou
2023-11-27 18:28 ` Rob Herring
2023-11-30 13:02 ` Binbin Zhou
2023-11-20 9:06 ` [PATCH v5 2/5] dt-bindings: interrupt-controller: loongson,liointc: Fix dtbs_check warning for reg-names Binbin Zhou
2023-11-20 9:06 ` [PATCH v5 3/5] dt-bindings: interrupt-controller: loongson,liointc: Fix dtbs_check warning for interrupt-names Binbin Zhou
2023-11-20 9:06 ` [PATCH v5 4/5] irqchip/loongson-liointc: Fix 'loongson,parent_int_map' parse Binbin Zhou
2023-12-08 14:20 ` Thomas Gleixner
2023-12-11 2:37 ` Binbin Zhou
2023-11-20 9:06 ` [PATCH v5 5/5] MIPS: Loongson64: DTS: Fix 'loongson,parent_int_map' references Binbin Zhou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).