[PATCH] ARM: dts: bcm63138: drop invalid "nand" interrupt name

[PATCH] ARM: dts: bcm63138: drop invalid "nand" interrupt name

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

String "nand" was never a valid interrupt name. It was never documented
and never used in Linux or U-Boot driver. It most likely comes from a
copy & paste mistake ("nand" is used in "reg-names").

The whole "interrupt-names" property is optional and can be skipped.

Fixes: b5762cacc411 ("ARM: bcm63138: add NAND DT support")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 arch/arm/boot/dts/bcm63138.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/bcm63138.dtsi b/arch/arm/boot/dts/bcm63138.dtsi
index b774a8d63813..7c1f656e3278 100644
--- a/arch/arm/boot/dts/bcm63138.dtsi
+++ b/arch/arm/boot/dts/bcm63138.dtsi
@@ -211,7 +211,6 @@ nand_controller: nand-controller@2000 {
 			reg-names = "nand", "nand-int-base";
 			status = "disabled";
 			interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
-			interrupt-names = "nand";
 		};
 
 		bootlut: bootlut@8000 {
-- 
2.34.1

[PATCH] ARM: dts: bcm63138: drop invalid "nand" interrupt name

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

String "nand" was never a valid interrupt name. It was never documented
and never used in Linux or U-Boot driver. It most likely comes from a
copy & paste mistake ("nand" is used in "reg-names").

The whole "interrupt-names" property is optional and can be skipped.

Fixes: b5762cacc411 ("ARM: bcm63138: add NAND DT support")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 arch/arm/boot/dts/bcm63138.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/bcm63138.dtsi b/arch/arm/boot/dts/bcm63138.dtsi
index b774a8d63813..7c1f656e3278 100644
--- a/arch/arm/boot/dts/bcm63138.dtsi
+++ b/arch/arm/boot/dts/bcm63138.dtsi
@@ -211,7 +211,6 @@ nand_controller: nand-controller@2000 {
 			reg-names = "nand", "nand-int-base";
 			status = "disabled";
 			interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
-			interrupt-names = "nand";
 		};
 
 		bootlut: bootlut@8000 {
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: dts: bcm63138: drop invalid "nand" interrupt name

[Florian Fainelli]

On 11/3/2022 3:24 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> String "nand" was never a valid interrupt name. It was never documented
> and never used in Linux or U-Boot driver. It most likely comes from a
> copy & paste mistake ("nand" is used in "reg-names").
> 
> The whole "interrupt-names" property is optional and can be skipped.

How about we just fix the binding document instead? Deciding the fate of 
a property based upon client programs of the DTS using it is a weak 
argument IMHO.
-- 
Florian

Re: [PATCH] ARM: dts: bcm63138: drop invalid "nand" interrupt name

[Florian Fainelli]

On 11/3/2022 3:24 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> String "nand" was never a valid interrupt name. It was never documented
> and never used in Linux or U-Boot driver. It most likely comes from a
> copy & paste mistake ("nand" is used in "reg-names").
> 
> The whole "interrupt-names" property is optional and can be skipped.

How about we just fix the binding document instead? Deciding the fate of 
a property based upon client programs of the DTS using it is a weak 
argument IMHO.
-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: dts: bcm63138: drop invalid "nand" interrupt name

[Krzysztof Kozlowski]

On 03/11/2022 11:31, Florian Fainelli wrote:
> 
> 
> On 11/3/2022 3:24 AM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> String "nand" was never a valid interrupt name. It was never documented
>> and never used in Linux or U-Boot driver. It most likely comes from a
>> copy & paste mistake ("nand" is used in "reg-names").
>>
>> The whole "interrupt-names" property is optional and can be skipped.
> 
> How about we just fix the binding document instead? Deciding the fate of 
> a property based upon client programs of the DTS using it is a weak 
> argument IMHO.

Having a "names" property with only one item and the name matching the
device name is also not useful.

For new bindings the recommendation would be: drop the interrupt-names.
For existing bindings - choose something reasonable, e.g. less changes
needed.

Best regards,
Krzysztof

Re: [PATCH] ARM: dts: bcm63138: drop invalid "nand" interrupt name

[Krzysztof Kozlowski]

On 03/11/2022 11:31, Florian Fainelli wrote:
> 
> 
> On 11/3/2022 3:24 AM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> String "nand" was never a valid interrupt name. It was never documented
>> and never used in Linux or U-Boot driver. It most likely comes from a
>> copy & paste mistake ("nand" is used in "reg-names").
>>
>> The whole "interrupt-names" property is optional and can be skipped.
> 
> How about we just fix the binding document instead? Deciding the fate of 
> a property based upon client programs of the DTS using it is a weak 
> argument IMHO.

Having a "names" property with only one item and the name matching the
device name is also not useful.

For new bindings the recommendation would be: drop the interrupt-names.
For existing bindings - choose something reasonable, e.g. less changes
needed.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: dts: bcm63138: drop invalid "nand" interrupt name

[Rafał Miłecki]

On 3.11.2022 16:31, Florian Fainelli wrote:
> On 11/3/2022 3:24 AM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> String "nand" was never a valid interrupt name. It was never documented
>> and never used in Linux or U-Boot driver. It most likely comes from a
>> copy & paste mistake ("nand" is used in "reg-names").
>>
>> The whole "interrupt-names" property is optional and can be skipped.
> 
> How about we just fix the binding document instead? Deciding the fate of a property based upon client programs of the DTS using it is a weak argument IMHO.

It's not a matter of client programs.


Binding clearly says that the first interrupt is "NAND CTLRDY interrupt".
Please check: Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml

Other interrupts are described as well. It's just "interrupts-names" that
are optional.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: dts: bcm63138: drop invalid "nand" interrupt name

[Rafał Miłecki]

On 3.11.2022 16:31, Florian Fainelli wrote:
> On 11/3/2022 3:24 AM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> String "nand" was never a valid interrupt name. It was never documented
>> and never used in Linux or U-Boot driver. It most likely comes from a
>> copy & paste mistake ("nand" is used in "reg-names").
>>
>> The whole "interrupt-names" property is optional and can be skipped.
> 
> How about we just fix the binding document instead? Deciding the fate of a property based upon client programs of the DTS using it is a weak argument IMHO.

It's not a matter of client programs.


Binding clearly says that the first interrupt is "NAND CTLRDY interrupt".
Please check: Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml

Other interrupts are described as well. It's just "interrupts-names" that
are optional.

Re: [PATCH] ARM: dts: bcm63138: drop invalid "nand" interrupt name

[Florian Fainelli]

On 11/3/2022 9:02 AM, Rafał Miłecki wrote:
> On 3.11.2022 16:31, Florian Fainelli wrote:
>> On 11/3/2022 3:24 AM, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> String "nand" was never a valid interrupt name. It was never documented
>>> and never used in Linux or U-Boot driver. It most likely comes from a
>>> copy & paste mistake ("nand" is used in "reg-names").
>>>
>>> The whole "interrupt-names" property is optional and can be skipped.
>>
>> How about we just fix the binding document instead? Deciding the fate 
>> of a property based upon client programs of the DTS using it is a weak 
>> argument IMHO.
> 
> It's not a matter of client programs.
> 
> 
> Binding clearly says that the first interrupt is "NAND CTLRDY interrupt".
> Please check: Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml

The binding is trying to capture all of the existing conditions that are 
present in Linux's tree but it is not actually specific enough for 
instance flash_dma_done and flash_edu_done are mutually exclusive and 
depend upon the controller's generation. Something that ought to be 
fixed as a separate patch.

> 
> Other interrupts are described as well. It's just "interrupts-names" that
> are optional.

How about we just rename the interrupt-names to "nand_ctlrdy" then, and 
also drop the Fixes: tag because we are not fixing anything functional here.
-- 
Florian

Re: [PATCH] ARM: dts: bcm63138: drop invalid "nand" interrupt name

[Florian Fainelli]

On 11/3/2022 9:02 AM, Rafał Miłecki wrote:
> On 3.11.2022 16:31, Florian Fainelli wrote:
>> On 11/3/2022 3:24 AM, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> String "nand" was never a valid interrupt name. It was never documented
>>> and never used in Linux or U-Boot driver. It most likely comes from a
>>> copy & paste mistake ("nand" is used in "reg-names").
>>>
>>> The whole "interrupt-names" property is optional and can be skipped.
>>
>> How about we just fix the binding document instead? Deciding the fate 
>> of a property based upon client programs of the DTS using it is a weak 
>> argument IMHO.
> 
> It's not a matter of client programs.
> 
> 
> Binding clearly says that the first interrupt is "NAND CTLRDY interrupt".
> Please check: Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml

The binding is trying to capture all of the existing conditions that are 
present in Linux's tree but it is not actually specific enough for 
instance flash_dma_done and flash_edu_done are mutually exclusive and 
depend upon the controller's generation. Something that ought to be 
fixed as a separate patch.

> 
> Other interrupts are described as well. It's just "interrupts-names" that
> are optional.

How about we just rename the interrupt-names to "nand_ctlrdy" then, and 
also drop the Fixes: tag because we are not fixing anything functional here.
-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: dts: bcm63138: drop invalid "nand" interrupt name

[Rafał Miłecki]

On 3.11.2022 17:12, Florian Fainelli wrote:
> On 11/3/2022 9:02 AM, Rafał Miłecki wrote:
>> On 3.11.2022 16:31, Florian Fainelli wrote:
>>> On 11/3/2022 3:24 AM, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>
>>>> String "nand" was never a valid interrupt name. It was never documented
>>>> and never used in Linux or U-Boot driver. It most likely comes from a
>>>> copy & paste mistake ("nand" is used in "reg-names").
>>>>
>>>> The whole "interrupt-names" property is optional and can be skipped.
>>>
>>> How about we just fix the binding document instead? Deciding the fate of a property based upon client programs of the DTS using it is a weak argument IMHO.
>>
>> It's not a matter of client programs.
>>
>>
>> Binding clearly says that the first interrupt is "NAND CTLRDY interrupt".
>> Please check: Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> 
> The binding is trying to capture all of the existing conditions that are present in Linux's tree but it is not actually specific enough for instance flash_dma_done and flash_edu_done are mutually exclusive and depend upon the controller's generation. Something that ought to be fixed as a separate patch.

In that case binding it wrong and has to be fixed first.

In this case I'll work on the binding first before cleaning DTS files.

Please drop this patch for now.

Re: [PATCH] ARM: dts: bcm63138: drop invalid "nand" interrupt name

[Rafał Miłecki]

On 3.11.2022 17:12, Florian Fainelli wrote:
> On 11/3/2022 9:02 AM, Rafał Miłecki wrote:
>> On 3.11.2022 16:31, Florian Fainelli wrote:
>>> On 11/3/2022 3:24 AM, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>
>>>> String "nand" was never a valid interrupt name. It was never documented
>>>> and never used in Linux or U-Boot driver. It most likely comes from a
>>>> copy & paste mistake ("nand" is used in "reg-names").
>>>>
>>>> The whole "interrupt-names" property is optional and can be skipped.
>>>
>>> How about we just fix the binding document instead? Deciding the fate of a property based upon client programs of the DTS using it is a weak argument IMHO.
>>
>> It's not a matter of client programs.
>>
>>
>> Binding clearly says that the first interrupt is "NAND CTLRDY interrupt".
>> Please check: Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> 
> The binding is trying to capture all of the existing conditions that are present in Linux's tree but it is not actually specific enough for instance flash_dma_done and flash_edu_done are mutually exclusive and depend upon the controller's generation. Something that ought to be fixed as a separate patch.

In that case binding it wrong and has to be fixed first.

In this case I'll work on the binding first before cleaning DTS files.

Please drop this patch for now.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel