dt-bindings: iio: admv1014: make all regs required

Message ID 20230727113136.98037-1-antoniu.miclaus@analog.com
State New
Headers
Series dt-bindings: iio: admv1014: make all regs required |

Commit Message

Antoniu Miclaus July 27, 2023, 11:31 a.m. UTC
  Since the regulators are required in the driver implementation, make
them required also in the bindings.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
 .../devicetree/bindings/iio/frequency/adi,admv1014.yaml   | 8 ++++++++
 1 file changed, 8 insertions(+)
  

Comments

Krzysztof Kozlowski July 27, 2023, 11:47 a.m. UTC | #1
On 27/07/2023 13:31, Antoniu Miclaus wrote:
> Since the regulators are required in the driver implementation, make
> them required also in the bindings.
> 

The true reason should be whether the hardware requires them. Because if
hardware does not need some, the driver should be fixed.

Best regards,
Krzysztof
  
Antoniu Miclaus July 27, 2023, 12:02 p.m. UTC | #2
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Thursday, July 27, 2023 2:47 PM
> To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>; jic23@kernel.org;
> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] dt-bindings: iio: admv1014: make all regs required
> 
> [External]
> 
> On 27/07/2023 13:31, Antoniu Miclaus wrote:
> > Since the regulators are required in the driver implementation, make
> > them required also in the bindings.
> >
> 
> The true reason should be whether the hardware requires them. Because if
> hardware does not need some, the driver should be fixed.
The datasheet is not very explicit on this topic, but all the specifications of the
part are built around these pins being supplied.
Moreover, the evaluation board or the part comes with all VCC pins connected
to fixed supplies.
> Best regards,
> Krzysztof
  
Jonathan Cameron July 29, 2023, 11:55 a.m. UTC | #3
On Thu, 27 Jul 2023 12:02:04 +0000
"Miclaus, Antoniu" <Antoniu.Miclaus@analog.com> wrote:

> > -----Original Message-----
> > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Sent: Thursday, July 27, 2023 2:47 PM
> > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>; jic23@kernel.org;
> > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> > conor+dt@kernel.org; linux-iio@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] dt-bindings: iio: admv1014: make all regs required
> > 
> > [External]
> > 
> > On 27/07/2023 13:31, Antoniu Miclaus wrote:  
> > > Since the regulators are required in the driver implementation, make
> > > them required also in the bindings.

This bit is probably not strictly true.  Try no providing them and you will
probably find stub regulators created on assumption they are fixed regs
that we aren't interested in controlling.

> > >  
> > 
> > The true reason should be whether the hardware requires them. Because if
> > hardware does not need some, the driver should be fixed.  
> The datasheet is not very explicit on this topic, but all the specifications of the
> part are built around these pins being supplied.
> Moreover, the evaluation board or the part comes with all VCC pins connected
> to fixed supplies.

Agreed - these should be required in the binding.

There was an old disagreement on this in which it they were not marked
required in some bindings because the regulator subsystem would assume they
were fixed supplies that were just missing in the DT and so provides stub regulators.
My understanding at least has changed and now we mark the required even if
the driver works fine on some boards without them being supplied.

Note though that this means the whole of trivial-devices.yaml is garbage
as very few devices actually work without any power :)

Jonathan



> > Best regards,
> > Krzysztof  
>
  
Antoniu Miclaus July 31, 2023, 7:40 a.m. UTC | #4
> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Saturday, July 29, 2023 2:55 PM
> To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>;
> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] dt-bindings: iio: admv1014: make all regs required
> 
> [External]
> 
> On Thu, 27 Jul 2023 12:02:04 +0000
> "Miclaus, Antoniu" <Antoniu.Miclaus@analog.com> wrote:
> 
> > > -----Original Message-----
> > > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > Sent: Thursday, July 27, 2023 2:47 PM
> > > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>; jic23@kernel.org;
> > > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> > > conor+dt@kernel.org; linux-iio@vger.kernel.org;
> > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH] dt-bindings: iio: admv1014: make all regs required
> > >
> > > [External]
> > >
> > > On 27/07/2023 13:31, Antoniu Miclaus wrote:
> > > > Since the regulators are required in the driver implementation, make
> > > > them required also in the bindings.
> 
> This bit is probably not strictly true.  Try no providing them and you will
> probably find stub regulators created on assumption they are fixed regs
> that we aren't interested in controlling.
> 
> > > >
> > >
> > > The true reason should be whether the hardware requires them.
> Because if
> > > hardware does not need some, the driver should be fixed.
> > The datasheet is not very explicit on this topic, but all the specifications of
> the
> > part are built around these pins being supplied.
> > Moreover, the evaluation board or the part comes with all VCC pins
> connected
> > to fixed supplies.
> 
> Agreed - these should be required in the binding.
> 
Hello Jonathan,

Not sure if I fully understand. Should I drop this patch? 
And also drop the "required" of the regulators in the admv1013 patch?
https://patchwork.kernel.org/project/linux-iio/patch/20230727110121.93546-1-antoniu.miclaus@analog.com/

Thanks,
> There was an old disagreement on this in which it they were not marked
> required in some bindings because the regulator subsystem would assume
> they
> were fixed supplies that were just missing in the DT and so provides stub
> regulators.
> My understanding at least has changed and now we mark the required even
> if
> the driver works fine on some boards without them being supplied.
> 
> Note though that this means the whole of trivial-devices.yaml is garbage
> as very few devices actually work without any power :)
> 
> Jonathan
> 
> 
> 
> > > Best regards,
> > > Krzysztof
> >
  
Jonathan Cameron July 31, 2023, 10:13 a.m. UTC | #5
On Mon, 31 Jul 2023 07:40:14 +0000
"Miclaus, Antoniu" <Antoniu.Miclaus@analog.com> wrote:

> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Saturday, July 29, 2023 2:55 PM
> > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>
> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>;
> > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> > conor+dt@kernel.org; linux-iio@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] dt-bindings: iio: admv1014: make all regs required
> > 
> > [External]
> > 
> > On Thu, 27 Jul 2023 12:02:04 +0000
> > "Miclaus, Antoniu" <Antoniu.Miclaus@analog.com> wrote:
> >   
> > > > -----Original Message-----
> > > > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > Sent: Thursday, July 27, 2023 2:47 PM
> > > > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>; jic23@kernel.org;
> > > > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> > > > conor+dt@kernel.org; linux-iio@vger.kernel.org;
> > > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> > > > Subject: Re: [PATCH] dt-bindings: iio: admv1014: make all regs required
> > > >
> > > > [External]
> > > >
> > > > On 27/07/2023 13:31, Antoniu Miclaus wrote:  
> > > > > Since the regulators are required in the driver implementation, make
> > > > > them required also in the bindings.  
> > 
> > This bit is probably not strictly true.  Try no providing them and you will
> > probably find stub regulators created on assumption they are fixed regs
> > that we aren't interested in controlling.
> >   
> > > > >  
> > > >
> > > > The true reason should be whether the hardware requires them.  
> > Because if  
> > > > hardware does not need some, the driver should be fixed.  
> > > The datasheet is not very explicit on this topic, but all the specifications of  
> > the  
> > > part are built around these pins being supplied.
> > > Moreover, the evaluation board or the part comes with all VCC pins  
> > connected  
> > > to fixed supplies.  
> > 
> > Agreed - these should be required in the binding.
> >   
> Hello Jonathan,
> 
> Not sure if I fully understand. Should I drop this patch? 

Patch is good.  The binding should state these are required - it doesn't
mean that the driver will fail to probe on systems that doesn't provide
them.  It does encourage any new DTS files to incorporate them which is
good to have.

Only thing that needs changing is the patch should not justify the
addition of these to the binding because they 'are required in the
driver implementation'.  Just drop that statement and the patch is
fine.



> And also drop the "required" of the regulators in the admv1013 patch?
> https://patchwork.kernel.org/project/linux-iio/patch/20230727110121.93546-1-antoniu.miclaus@analog.com/

As long as the patch doesn't say anything about what the driver does
then it is fine.  Note DT bindings and patch descriptions for them should
never mention the driver.

Jonathan

> 
> Thanks,
> > There was an old disagreement on this in which it they were not marked
> > required in some bindings because the regulator subsystem would assume
> > they
> > were fixed supplies that were just missing in the DT and so provides stub
> > regulators.
> > My understanding at least has changed and now we mark the required even
> > if
> > the driver works fine on some boards without them being supplied.
> > 
> > Note though that this means the whole of trivial-devices.yaml is garbage
> > as very few devices actually work without any power :)
> > 
> > Jonathan
> > 
> > 
> >   
> > > > Best regards,
> > > > Krzysztof  
> > >  
>
  
Antoniu Miclaus July 31, 2023, 10:16 a.m. UTC | #6
> -----Original Message-----
> From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Sent: Monday, July 31, 2023 1:13 PM
> To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>
> Cc: Jonathan Cameron <jic23@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org>; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; linux-
> iio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] dt-bindings: iio: admv1014: make all regs required
> 
> [External]
> 
> On Mon, 31 Jul 2023 07:40:14 +0000
> "Miclaus, Antoniu" <Antoniu.Miclaus@analog.com> wrote:
> 
> > > -----Original Message-----
> > > From: Jonathan Cameron <jic23@kernel.org>
> > > Sent: Saturday, July 29, 2023 2:55 PM
> > > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>
> > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>;
> > > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> > > conor+dt@kernel.org; linux-iio@vger.kernel.org;
> > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH] dt-bindings: iio: admv1014: make all regs required
> > >
> > > [External]
> > >
> > > On Thu, 27 Jul 2023 12:02:04 +0000
> > > "Miclaus, Antoniu" <Antoniu.Miclaus@analog.com> wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > > Sent: Thursday, July 27, 2023 2:47 PM
> > > > > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>;
> jic23@kernel.org;
> > > > > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> > > > > conor+dt@kernel.org; linux-iio@vger.kernel.org;
> > > > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> > > > > Subject: Re: [PATCH] dt-bindings: iio: admv1014: make all regs
> required
> > > > >
> > > > > [External]
> > > > >
> > > > > On 27/07/2023 13:31, Antoniu Miclaus wrote:
> > > > > > Since the regulators are required in the driver implementation,
> make
> > > > > > them required also in the bindings.
> > >
> > > This bit is probably not strictly true.  Try no providing them and you will
> > > probably find stub regulators created on assumption they are fixed regs
> > > that we aren't interested in controlling.
> > >
> > > > > >
> > > > >
> > > > > The true reason should be whether the hardware requires them.
> > > Because if
> > > > > hardware does not need some, the driver should be fixed.
> > > > The datasheet is not very explicit on this topic, but all the specifications
> of
> > > the
> > > > part are built around these pins being supplied.
> > > > Moreover, the evaluation board or the part comes with all VCC pins
> > > connected
> > > > to fixed supplies.
> > >
> > > Agreed - these should be required in the binding.
> > >
> > Hello Jonathan,
> >
> > Not sure if I fully understand. Should I drop this patch?
> 
> Patch is good.  The binding should state these are required - it doesn't
> mean that the driver will fail to probe on systems that doesn't provide
> them.  It does encourage any new DTS files to incorporate them which is
> good to have.
> 
> Only thing that needs changing is the patch should not justify the
> addition of these to the binding because they 'are required in the
> driver implementation'.  Just drop that statement and the patch is
> fine.
Great, thanks a lot for the clarification.
> 
> 
> 
> > And also drop the "required" of the regulators in the admv1013 patch?
> > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-
> iio/patch/20230727110121.93546-1-
> antoniu.miclaus@analog.com/__;!!A3Ni8CS0y2Y!8dQigVuPYwGwS53vsq2RO
> CJx3RgfeZW4GZks5EFWhfUlJZTdAvsCr5MGlrWUVHXwqRKoASc4UBEFE8h6N
> KO6jrfSQ-oKjKtS$
> 
> As long as the patch doesn't say anything about what the driver does
> then it is fine.  Note DT bindings and patch descriptions for them should
> never mention the driver.
> 
> Jonathan
> 
> >
> > Thanks,
> > > There was an old disagreement on this in which it they were not marked
> > > required in some bindings because the regulator subsystem would
> assume
> > > they
> > > were fixed supplies that were just missing in the DT and so provides stub
> > > regulators.
> > > My understanding at least has changed and now we mark the required
> even
> > > if
> > > the driver works fine on some boards without them being supplied.
> > >
> > > Note though that this means the whole of trivial-devices.yaml is garbage
> > > as very few devices actually work without any power :)
> > >
> > > Jonathan
> > >
> > >
> > >
> > > > > Best regards,
> > > > > Krzysztof
> > > >
> >
  

Patch

diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,admv1014.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,admv1014.yaml
index ab86daa2c56e..8e4c5ff0da14 100644
--- a/Documentation/devicetree/bindings/iio/frequency/adi,admv1014.yaml
+++ b/Documentation/devicetree/bindings/iio/frequency/adi,admv1014.yaml
@@ -103,6 +103,14 @@  required:
   - clocks
   - clock-names
   - vcm-supply
+  - vcc-if-bb-supply
+  - vcc-vga-supply
+  - vcc-vva-supply
+  - vcc-lna-3p3-supply
+  - vcc-lna-1p5-supply
+  - vcc-bg-supply
+  - vcc-quad-supply
+  - vcc-mixer-supply
 
 allOf:
   - $ref: /schemas/spi/spi-peripheral-props.yaml#