arm64: dts: qcom: sc8280xp-pmics: Specify interrupt parent explicitly

  Nodes like pwrkey, resin, iadc, adc-tm, temp-alarm which are the grand
children of spmi_bus node represent the interrupt generating devices but
don't have "interrupt-parent" property.

As per the devicetree spec v0.3, section 2.4:

"The physical wiring of an interrupt source to an interrupt controller is
represented in the devicetree with the interrupt-parent property. Nodes
that represent interrupt-generating devices contain an interrupt-parent
property which has a phandle value that points to the device to which the
device’s interrupts are routed, typically an interrupt controller. If an
interrupt-generating device does not have an interrupt-parent property,
its interrupt parent is assumed to be its devicetree parent."

This clearly says that if the "interrupt-parent" property is absent, then
the immediate devicetree parent will be assumed as the interrupt parent.
But the immediate parents of these nodes are not interrupt controllers
themselves.

This may lead to failure while wiring the interrupt for these nodes by an
operating system. But a few operating systems like Linux, workaround this
issue by walking up the parent nodes until it finds the "interrupt-cells"
property. Then the node that has the "interrupt-cells" property will be
used as the interrupt parent.

But this workaround is not as per the DT spec and is not being implemented
by other operating systems such as OpenBSD.

Hence, fix this issue by adding the "interrupts-extended" property that
explicitly specifies the spmi_bus node as the interrupt parent. Note that
the "interrupts-extended" property is chosen over "interrupt-parent" as it
allows specifying both interrupt parent phandle and interrupt specifiers in
a single property.

Reported-by: Patrick Wildt <patrick@blueri.se>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---

I just fixed one dtsi since wanted to make sure that everyone agrees to
this implementation, otherwise I'll end up wasting too much time fixing
every other DTs making use of these nodes.

Next step would be to fix the bindings of these nodes to include interrupt-
parent or interrupts-extended properties and affected DTs.

 arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
  

On Mon, Feb 13, 2023 at 02:31:18PM +0530, Manivannan Sadhasivam wrote:
> Nodes like pwrkey, resin, iadc, adc-tm, temp-alarm which are the grand
> children of spmi_bus node represent the interrupt generating devices but
> don't have "interrupt-parent" property.
> 
> As per the devicetree spec v0.3, section 2.4:
> 
> "The physical wiring of an interrupt source to an interrupt controller is
> represented in the devicetree with the interrupt-parent property. Nodes
> that represent interrupt-generating devices contain an interrupt-parent
> property which has a phandle value that points to the device to which the
> device’s interrupts are routed, typically an interrupt controller. If an
> interrupt-generating device does not have an interrupt-parent property,
> its interrupt parent is assumed to be its devicetree parent."
> 
> This clearly says that if the "interrupt-parent" property is absent, then
> the immediate devicetree parent will be assumed as the interrupt parent.
> But the immediate parents of these nodes are not interrupt controllers
> themselves.
> 
> This may lead to failure while wiring the interrupt for these nodes by an
> operating system. But a few operating systems like Linux, workaround this
> issue by walking up the parent nodes until it finds the "interrupt-cells"
> property. Then the node that has the "interrupt-cells" property will be
> used as the interrupt parent.
> 
> But this workaround is not as per the DT spec and is not being implemented
> by other operating systems such as OpenBSD.
> 
> Hence, fix this issue by adding the "interrupts-extended" property that
> explicitly specifies the spmi_bus node as the interrupt parent. Note that
> the "interrupts-extended" property is chosen over "interrupt-parent" as it
> allows specifying both interrupt parent phandle and interrupt specifiers in
> a single property.
> 
> Reported-by: Patrick Wildt <patrick@blueri.se>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks Mani, I had forgotten about this issue again.

Reviewed-by: Bjorn Andersson <andersson@kernel.org>

Regards,
Bjorn

> ---
> 
> I just fixed one dtsi since wanted to make sure that everyone agrees to
> this implementation, otherwise I'll end up wasting too much time fixing
> every other DTs making use of these nodes.
> 
> Next step would be to fix the bindings of these nodes to include interrupt-
> parent or interrupts-extended properties and affected DTs.
> 
>  arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> index f2c0b71b5d8e..df7d28f7ae60 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> @@ -64,14 +64,14 @@ pmk8280_pon: pon@1300 {
>  
>  			pmk8280_pon_pwrkey: pwrkey {
>  				compatible = "qcom,pmk8350-pwrkey";
> -				interrupts = <0x0 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
> +				interrupts-extended = <&spmi_bus 0x0 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
>  				linux,code = <KEY_POWER>;
>  				status = "disabled";
>  			};
>  
>  			pmk8280_pon_resin: resin {
>  				compatible = "qcom,pmk8350-resin";
> -				interrupts = <0x0 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
> +				interrupts-extended = <&spmi_bus 0x0 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
>  				status = "disabled";
>  			};
>  		};
> @@ -79,7 +79,7 @@ pmk8280_pon_resin: resin {
>  		pmk8280_vadc: adc@3100 {
>  			compatible = "qcom,spmi-adc7";
>  			reg = <0x3100>;
> -			interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
> +			interrupts-extended = <&spmi_bus 0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			#io-channel-cells = <1>;
> @@ -89,7 +89,7 @@ pmk8280_vadc: adc@3100 {
>  		pmk8280_adc_tm: adc-tm@3400 {
>  			compatible = "qcom,spmi-adc-tm5-gen2";
>  			reg = <0x3400>;
> -			interrupts = <0x0 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
> +			interrupts-extended = <&spmi_bus 0x0 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			#thermal-sensor-cells = <1>;
> @@ -106,7 +106,7 @@ pmc8280_1: pmic@1 {
>  		pm8280_1_temp_alarm: temp-alarm@a00 {
>  			compatible = "qcom,spmi-temp-alarm";
>  			reg = <0xa00>;
> -			interrupts = <0x1 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
> +			interrupts-extended = <&spmi_bus 0x1 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
>  			#thermal-sensor-cells = <0>;
>  		};
>  
> @@ -158,7 +158,7 @@ pmc8280_2: pmic@3 {
>  		pm8280_2_temp_alarm: temp-alarm@a00 {
>  			compatible = "qcom,spmi-temp-alarm";
>  			reg = <0xa00>;
> -			interrupts = <0x2 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
> +			interrupts-extended = <&spmi_bus 0x2 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
>  			#thermal-sensor-cells = <0>;
>  		};
>  
> -- 
> 2.25.1
>
  

On Mon, 13 Feb 2023 14:31:18 +0530, Manivannan Sadhasivam wrote:
> Nodes like pwrkey, resin, iadc, adc-tm, temp-alarm which are the grand
> children of spmi_bus node represent the interrupt generating devices but
> don't have "interrupt-parent" property.
> 
> As per the devicetree spec v0.3, section 2.4:
> 
> "The physical wiring of an interrupt source to an interrupt controller is
> represented in the devicetree with the interrupt-parent property. Nodes
> that represent interrupt-generating devices contain an interrupt-parent
> property which has a phandle value that points to the device to which the
> device’s interrupts are routed, typically an interrupt controller. If an
> interrupt-generating device does not have an interrupt-parent property,
> its interrupt parent is assumed to be its devicetree parent."
> 
> [...]

Applied, thanks!

[1/1] arm64: dts: qcom: sc8280xp-pmics: Specify interrupt parent explicitly
      commit: 2d5cab9232ba6bac734186f3e74fb106793bc738

Best regards,