Node names for the channel are supposed to be adc-chan@REG.
Use this format and at the same time sort the nodes by reg value.
Signed-off-by: Luca Weiss <[email protected]>
---
arch/arm/boot/dts/qcom-pm8941.dtsi | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi b/arch/arm/boot/dts/qcom-pm8941.dtsi
index 9cd49deb9fa7..3c15eecf2f21 100644
--- a/arch/arm/boot/dts/qcom-pm8941.dtsi
+++ b/arch/arm/boot/dts/qcom-pm8941.dtsi
@@ -101,26 +101,33 @@ pm8941_vadc: adc@3100 {
#size-cells = <0>;
#io-channel-cells = <1>;
- bat_temp {
- reg = <VADC_LR_MUX1_BAT_THERM>;
+
+ adc-chan@6 {
+ reg = <VADC_VBAT_SNS>;
};
- die_temp {
+
+ adc-chan@8 {
reg = <VADC_DIE_TEMP>;
};
- ref_625mv {
+
+ adc-chan@9 {
reg = <VADC_REF_625MV>;
};
- ref_1250v {
+
+ adc-chan@10 {
reg = <VADC_REF_1250MV>;
};
- ref_gnd {
+
+ adc-chan@14 {
reg = <VADC_GND_REF>;
};
- ref_vdd {
+
+ adc-chan@15 {
reg = <VADC_VDD_VADC>;
};
- vbat_sns {
- reg = <VADC_VBAT_SNS>;
+
+ adc-chan@48 {
+ reg = <VADC_LR_MUX1_BAT_THERM>;
};
};
--
2.37.3
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 <[email protected]>
---
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>;
--
2.37.3
On 25/09/2022 18:18, Luca Weiss wrote:
> Node names for the channel are supposed to be adc-chan@REG.
>
> Use this format and at the same time sort the nodes by reg value.
>
> Signed-off-by: Luca Weiss <[email protected]>
> ---
> arch/arm/boot/dts/qcom-pm8941.dtsi | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi b/arch/arm/boot/dts/qcom-pm8941.dtsi
> index 9cd49deb9fa7..3c15eecf2f21 100644
> --- a/arch/arm/boot/dts/qcom-pm8941.dtsi
> +++ b/arch/arm/boot/dts/qcom-pm8941.dtsi
> @@ -101,26 +101,33 @@ pm8941_vadc: adc@3100 {
> #size-cells = <0>;
> #io-channel-cells = <1>;
>
> - bat_temp {
> - reg = <VADC_LR_MUX1_BAT_THERM>;
> +
> + adc-chan@6 {
> + reg = <VADC_VBAT_SNS>;
> };
> - die_temp {
> +
> + adc-chan@8 {
> reg = <VADC_DIE_TEMP>;
> };
> - ref_625mv {
> +
> + adc-chan@9 {
> reg = <VADC_REF_625MV>;
> };
> - ref_1250v {
> +
> + adc-chan@10 {
> reg = <VADC_REF_1250MV>;
> };
> - ref_gnd {
> +
> + adc-chan@14 {
> reg = <VADC_GND_REF>;
> };
> - ref_vdd {
> +
> + adc-chan@15 {
> reg = <VADC_VDD_VADC>;
> };
> - vbat_sns {
> - reg = <VADC_VBAT_SNS>;
> +
> + adc-chan@48 {
> + reg = <VADC_LR_MUX1_BAT_THERM>;
> };
> };
>
Here aswell, I don't see where this is required, bindings doesn't mandate this naming:
patternProperties:
"^.*@[0-9a-f]+$":
Neil
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 <[email protected]>
> ---
> 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
On 25/09/2022 18:18, Luca Weiss wrote:
> Node names for the channel are supposed to be adc-chan@REG.
Why exactly?
>
> Use this format and at the same time sort the nodes by reg value.
>
> Signed-off-by: Luca Weiss <[email protected]>
> ---
> arch/arm/boot/dts/qcom-pm8941.dtsi | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
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 <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
> > ---
> >
> > 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:
> Node names for the channel are supposed to be adc-chan@REG.
>
> Use this format and at the same time sort the nodes by reg value.
>
> Signed-off-by: Luca Weiss <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
> arch/arm/boot/dts/qcom-pm8941.dtsi | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi b/arch/arm/boot/dts/qcom-pm8941.dtsi
> index 9cd49deb9fa7..3c15eecf2f21 100644
> --- a/arch/arm/boot/dts/qcom-pm8941.dtsi
> +++ b/arch/arm/boot/dts/qcom-pm8941.dtsi
> @@ -101,26 +101,33 @@ pm8941_vadc: adc@3100 {
> #size-cells = <0>;
> #io-channel-cells = <1>;
>
> - bat_temp {
> - reg = <VADC_LR_MUX1_BAT_THERM>;
> +
> + adc-chan@6 {
> + reg = <VADC_VBAT_SNS>;
> };
> - die_temp {
> +
> + adc-chan@8 {
> reg = <VADC_DIE_TEMP>;
> };
> - ref_625mv {
> +
> + adc-chan@9 {
> reg = <VADC_REF_625MV>;
> };
> - ref_1250v {
> +
> + adc-chan@10 {
> reg = <VADC_REF_1250MV>;
> };
> - ref_gnd {
> +
> + adc-chan@14 {
> reg = <VADC_GND_REF>;
> };
> - ref_vdd {
> +
> + adc-chan@15 {
> reg = <VADC_VDD_VADC>;
> };
> - vbat_sns {
> - reg = <VADC_VBAT_SNS>;
> +
> + adc-chan@48 {
> + reg = <VADC_LR_MUX1_BAT_THERM>;
> };
> };
>
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 <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>
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 <[email protected]>
>>> ---
>>>
>>> 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