| Message ID | 20220925161821.78030-2-luca@z3ntu.xyz (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] ARM: dts: qcom: pm8941: fix vadc channel node names | expand |
On 25/09/2022 18:18, Luca Weiss wrote: > The iadc node name is supposed to be just 'adc' and the compatible is > only supposed to be qcom,spmi-iadc according to the bindings. > > Adjust the node to match that. > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > --- > arch/arm/boot/dts/qcom-pm8941.dtsi | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi b/arch/arm/boot/dts/qcom-pm8941.dtsi > index 3c15eecf2f21..33517cccee01 100644 > --- a/arch/arm/boot/dts/qcom-pm8941.dtsi > +++ b/arch/arm/boot/dts/qcom-pm8941.dtsi > @@ -131,8 +131,8 @@ adc-chan@48 { > }; > }; > > - pm8941_iadc: iadc@3600 { > - compatible = "qcom,pm8941-iadc", "qcom,spmi-iadc"; > + pm8941_iadc: adc@3600 { > + compatible = "qcom,spmi-iadc"; > reg = <0x3600>; > interrupts = <0x0 0x36 0x0 IRQ_TYPE_EDGE_RISING>; > qcom,external-resistor-micro-ohms = <10000>; Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
On 25/09/2022 18:18, Luca Weiss wrote: > The iadc node name is supposed to be just 'adc' and the compatible is > only supposed to be qcom,spmi-iadc according to the bindings. > > Adjust the node to match that. > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > --- > arch/arm/boot/dts/qcom-pm8941.dtsi | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi b/arch/arm/boot/dts/qcom-pm8941.dtsi > index 3c15eecf2f21..33517cccee01 100644 > --- a/arch/arm/boot/dts/qcom-pm8941.dtsi > +++ b/arch/arm/boot/dts/qcom-pm8941.dtsi > @@ -131,8 +131,8 @@ adc-chan@48 { > }; > }; > > - pm8941_iadc: iadc@3600 { > - compatible = "qcom,pm8941-iadc", "qcom,spmi-iadc"; > + pm8941_iadc: adc@3600 { > + compatible = "qcom,spmi-iadc"; I am not sure this is correct. Usually specific compatibles are encouraged. Best regards, Krzysztof
Hi Krzysztof, On Montag, 26. September 2022 10:54:23 CEST Krzysztof Kozlowski wrote: > On 25/09/2022 18:18, Luca Weiss wrote: > > The iadc node name is supposed to be just 'adc' and the compatible is > > only supposed to be qcom,spmi-iadc according to the bindings. > > > > Adjust the node to match that. > > > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > > --- > > > > arch/arm/boot/dts/qcom-pm8941.dtsi | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi > > b/arch/arm/boot/dts/qcom-pm8941.dtsi index 3c15eecf2f21..33517cccee01 > > 100644 > > --- a/arch/arm/boot/dts/qcom-pm8941.dtsi > > +++ b/arch/arm/boot/dts/qcom-pm8941.dtsi > > @@ -131,8 +131,8 @@ adc-chan@48 { > > > > }; > > > > }; > > > > - pm8941_iadc: iadc@3600 { > > - compatible = "qcom,pm8941-iadc", "qcom,spmi-iadc"; > > + pm8941_iadc: adc@3600 { > > + compatible = "qcom,spmi-iadc"; > > I am not sure this is correct. Usually specific compatibles are encouraged. I'm happy to change it the other way also. But the sibling of this compatible, qcom,spmi-vadc also only has that single compatible so it'd align it with that. Let me know what you think. Regards Luca > > Best regards, > Krzysztof
On 25.09.2022 18:18, Luca Weiss wrote: > The iadc node name is supposed to be just 'adc' and the compatible is > only supposed to be qcom,spmi-iadc according to the bindings. > > Adjust the node to match that. > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org> Konrad > arch/arm/boot/dts/qcom-pm8941.dtsi | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi b/arch/arm/boot/dts/qcom-pm8941.dtsi > index 3c15eecf2f21..33517cccee01 100644 > --- a/arch/arm/boot/dts/qcom-pm8941.dtsi > +++ b/arch/arm/boot/dts/qcom-pm8941.dtsi > @@ -131,8 +131,8 @@ adc-chan@48 { > }; > }; > > - pm8941_iadc: iadc@3600 { > - compatible = "qcom,pm8941-iadc", "qcom,spmi-iadc"; > + pm8941_iadc: adc@3600 { > + compatible = "qcom,spmi-iadc"; > reg = <0x3600>; > interrupts = <0x0 0x36 0x0 IRQ_TYPE_EDGE_RISING>; > qcom,external-resistor-micro-ohms = <10000>;
On 26/09/2022 17:05, Luca Weiss wrote: > Hi Krzysztof, > > On Montag, 26. September 2022 10:54:23 CEST Krzysztof Kozlowski wrote: >> On 25/09/2022 18:18, Luca Weiss wrote: >>> The iadc node name is supposed to be just 'adc' and the compatible is >>> only supposed to be qcom,spmi-iadc according to the bindings. >>> >>> Adjust the node to match that. >>> >>> Signed-off-by: Luca Weiss <luca@z3ntu.xyz> >>> --- >>> >>> arch/arm/boot/dts/qcom-pm8941.dtsi | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi >>> b/arch/arm/boot/dts/qcom-pm8941.dtsi index 3c15eecf2f21..33517cccee01 >>> 100644 >>> --- a/arch/arm/boot/dts/qcom-pm8941.dtsi >>> +++ b/arch/arm/boot/dts/qcom-pm8941.dtsi >>> @@ -131,8 +131,8 @@ adc-chan@48 { >>> >>> }; >>> >>> }; >>> >>> - pm8941_iadc: iadc@3600 { >>> - compatible = "qcom,pm8941-iadc", > "qcom,spmi-iadc"; >>> + pm8941_iadc: adc@3600 { >>> + compatible = "qcom,spmi-iadc"; >> >> I am not sure this is correct. Usually specific compatibles are encouraged. > > I'm happy to change it the other way also. > > But the sibling of this compatible, qcom,spmi-vadc also only has that single > compatible so it'd align it with that. Ugh, there is a mess around them. Some other ADCs have specific compatibles, some not, so there is no consistency. I propose to have device specific compatible with qcom,spmi-iadc fallback, so basically document the DTS in bindings. Maybe other IADC will need some quirks, so specific compatible helps here. Best regards, Krzysztof
diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi b/arch/arm/boot/dts/qcom-pm8941.dtsi index 3c15eecf2f21..33517cccee01 100644 --- a/arch/arm/boot/dts/qcom-pm8941.dtsi +++ b/arch/arm/boot/dts/qcom-pm8941.dtsi @@ -131,8 +131,8 @@ adc-chan@48 { }; }; - pm8941_iadc: iadc@3600 { - compatible = "qcom,pm8941-iadc", "qcom,spmi-iadc"; + pm8941_iadc: adc@3600 { + compatible = "qcom,spmi-iadc"; reg = <0x3600>; interrupts = <0x0 0x36 0x0 IRQ_TYPE_EDGE_RISING>; qcom,external-resistor-micro-ohms = <10000>;
The iadc node name is supposed to be just 'adc' and the compatible is only supposed to be qcom,spmi-iadc according to the bindings. Adjust the node to match that. Signed-off-by: Luca Weiss <luca@z3ntu.xyz> --- arch/arm/boot/dts/qcom-pm8941.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)