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@xxxxxxxxx>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
Thanks Mani, I had forgotten about this issue again.
Reviewed-by: Bjorn Andersson <andersson@xxxxxxxxxx>
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
>
