diff mbox series

[v3,2/3] dt-bindings: thermal: qcom-spmi-adc-tm5: Use generic ADC node name

Message ID 20230201204447.542385-3-marijn.suijten@somainline.org
State New
Headers show
Series None | expand

Commit Message

Marijn Suijten Feb. 1, 2023, 8:44 p.m. UTC
Update the example to reflect a future requirement for the generic
adc-chan node name on ADC channel nodes, while conveying the board name
of the channel in a label instead.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 .../devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml   | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Rob Herring (Arm) Feb. 3, 2023, 9:25 p.m. UTC | #1
On Wed, Feb 01, 2023 at 09:44:46PM +0100, Marijn Suijten wrote:
> Update the example to reflect a future requirement for the generic
> adc-chan node name on ADC channel nodes, while conveying the board name
> of the channel in a label instead.

I don't think we've defined 'adc-chan' as THE generic name. Looks like 
we have:

adc-chan
adc-channel
channel

'channel' is the most common (except for QCom).

Rob
Jonathan Cameron Feb. 5, 2023, 3:06 p.m. UTC | #2
On Fri, 3 Feb 2023 15:25:01 -0600
Rob Herring <robh@kernel.org> wrote:

> On Wed, Feb 01, 2023 at 09:44:46PM +0100, Marijn Suijten wrote:
> > Update the example to reflect a future requirement for the generic
> > adc-chan node name on ADC channel nodes, while conveying the board name
> > of the channel in a label instead.  
> 
> I don't think we've defined 'adc-chan' as THE generic name. Looks like 
> we have:
> 
> adc-chan
> adc-channel
> channel
> 
> 'channel' is the most common (except for QCom).
Good spot.

We also have that defined as the channel name in 
bindings/iio/adc.yaml

Now this particular binding doesn't use anything from that
generic binding (other than trivial use of reg) but better to be
consistent with it than not!

Thanks,

Jonathan


> 
> Rob
Marijn Suijten March 16, 2023, 12:43 p.m. UTC | #3
On 2023-02-05 15:06:45, Jonathan Cameron wrote:
> On Fri, 3 Feb 2023 15:25:01 -0600
> Rob Herring <robh@kernel.org> wrote:
> 
> > On Wed, Feb 01, 2023 at 09:44:46PM +0100, Marijn Suijten wrote:
> > > Update the example to reflect a future requirement for the generic
> > > adc-chan node name on ADC channel nodes, while conveying the board name
> > > of the channel in a label instead.  
> > 
> > I don't think we've defined 'adc-chan' as THE generic name. Looks like 
> > we have:
> > 
> > adc-chan
> > adc-channel
> > channel
> > 
> > 'channel' is the most common (except for QCom).
> Good spot.
> 
> We also have that defined as the channel name in 
> bindings/iio/adc.yaml

Good point, let's match adc.yaml and use 'channel' instead.  I'll
respin this series with thas, as well as rebasing on -next to solve
conflicts with 8013295662f5 ("arm64: dts: qcom: sc8280xp: Add label
property to vadc channel nodes"): supposedly that DT originally relied
on the `@XX` suffix bug :)

> Now this particular binding doesn't use anything from that
> generic binding (other than trivial use of reg) but better to be
> consistent with it than not!

Should it inherit the common binding, or was it omitted for a reason?

- Marijn
Jonathan Cameron March 16, 2023, 5:44 p.m. UTC | #4
On Thu, 16 Mar 2023 13:43:07 +0100
Marijn Suijten <marijn.suijten@somainline.org> wrote:

> On 2023-02-05 15:06:45, Jonathan Cameron wrote:
> > On Fri, 3 Feb 2023 15:25:01 -0600
> > Rob Herring <robh@kernel.org> wrote:
> >   
> > > On Wed, Feb 01, 2023 at 09:44:46PM +0100, Marijn Suijten wrote:  
> > > > Update the example to reflect a future requirement for the generic
> > > > adc-chan node name on ADC channel nodes, while conveying the board name
> > > > of the channel in a label instead.    
> > > 
> > > I don't think we've defined 'adc-chan' as THE generic name. Looks like 
> > > we have:
> > > 
> > > adc-chan
> > > adc-channel
> > > channel
> > > 
> > > 'channel' is the most common (except for QCom).  
> > Good spot.
> > 
> > We also have that defined as the channel name in 
> > bindings/iio/adc.yaml  
> 
> Good point, let's match adc.yaml and use 'channel' instead.  I'll
> respin this series with thas, as well as rebasing on -next to solve
> conflicts with 8013295662f5 ("arm64: dts: qcom: sc8280xp: Add label
> property to vadc channel nodes"): supposedly that DT originally relied
> on the `@XX` suffix bug :)
> 
> > Now this particular binding doesn't use anything from that
> > generic binding (other than trivial use of reg) but better to be
> > consistent with it than not!  
> 
> Should it inherit the common binding, or was it omitted for a reason?

Harmless but little point as far as I can see given we don't happen
to have any of the generic elements defined in the generic channel
binding.

Jonathan

> 
> - Marijn
>
Marijn Suijten March 16, 2023, 10:44 p.m. UTC | #5
On 2023-03-16 17:44:28, Jonathan Cameron wrote:
<snip>
> > Should it inherit the common binding, or was it omitted for a reason?
> 
> Harmless but little point as far as I can see given we don't happen
> to have any of the generic elements defined in the generic channel
> binding.

Supposedly the reg property, and now also the node name.  Up to you to
say whether I should inherit this (and strip out the common bits) or
just focus on renaming the node name in the existing binding to channel.

- Marijn
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml
index 52ec18cf1eda..885c9e139848 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml
+++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml
@@ -178,10 +178,11 @@  examples:
             #io-channel-cells = <1>;
 
             /* Other properties are omitted */
-            conn-therm@4f {
+            adc-chan@4f {
                 reg = <ADC5_AMUX_THM3_100K_PU>;
                 qcom,ratiometric;
                 qcom,hw-settle-time = <200>;
+                label = "conn_therm";
             };
         };
 
@@ -217,16 +218,18 @@  examples:
             #io-channel-cells = <1>;
 
             /* Other properties are omitted */
-            xo-therm@44 {
+            adc-chan@44 {
                 reg = <PMK8350_ADC7_AMUX_THM1_100K_PU>;
                 qcom,ratiometric;
                 qcom,hw-settle-time = <200>;
+                label = "xo_therm";
             };
 
-            conn-therm@147 {
+            adc-chan@147 {
                 reg = <PM8350_ADC7_AMUX_THM4_100K_PU(1)>;
                 qcom,ratiometric;
                 qcom,hw-settle-time = <200>;
+                label = "conn_therm";
             };
         };