diff mbox series

[1/5] media: dt-bindings: media: imx335: Add supply bindings

Message ID 20231010005126.3425444-2-kieran.bingham@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series media: Sony IMX335 improvements | expand

Commit Message

Kieran Bingham Oct. 10, 2023, 12:51 a.m. UTC
Add the bindings for the supply references used on the IMX335.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 .../bindings/media/i2c/sony,imx335.yaml          | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Umang Jain Oct. 10, 2023, 3:53 a.m. UTC | #1
Hi Kieran,

Thank you for the patch

On 10/10/23 6:21 AM, Kieran Bingham wrote:
> Add the bindings for the supply references used on the IMX335.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

LGTM,

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

> ---
>   .../bindings/media/i2c/sony,imx335.yaml          | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> index a167dcdb3a32..1863b5608a5c 100644
> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> @@ -32,6 +32,15 @@ properties:
>       description: Clock frequency from 6 to 27 MHz, 37.125MHz, 74.25MHz
>       maxItems: 1
>   
> +  avdd-supply:
> +    description: Analog power supply (2.9V)
> +
> +  ovdd-supply:
> +    description: Interface power supply (1.8V)
> +
> +  dvdd-supply:
> +    description: Digital power supply (1.2V)
> +
>     reset-gpios:
>       description: Reference to the GPIO connected to the XCLR pin, if any.
>       maxItems: 1
> @@ -60,6 +69,9 @@ required:
>     - compatible
>     - reg
>     - clocks
> +  - avdd-supply
> +  - ovdd-supply
> +  - dvdd-supply
>     - port
>   
>   additionalProperties: false
> @@ -79,6 +91,10 @@ examples:
>               assigned-clock-parents = <&imx335_clk_parent>;
>               assigned-clock-rates = <24000000>;
>   
> +            avdd-supply = <&camera_vdda_2v9>;
> +            ovdd-supply = <&camera_vddo_1v8>;
> +            dvdd-supply = <&camera_vddd_1v2>;
> +
>               port {
>                   imx335: endpoint {
>                       remote-endpoint = <&cam>;
Marco Felsch Oct. 10, 2023, 5:03 a.m. UTC | #2
On 23-10-10, Kieran Bingham wrote:
> Add the bindings for the supply references used on the IMX335.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

> ---
>  .../bindings/media/i2c/sony,imx335.yaml          | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> index a167dcdb3a32..1863b5608a5c 100644
> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> @@ -32,6 +32,15 @@ properties:
>      description: Clock frequency from 6 to 27 MHz, 37.125MHz, 74.25MHz
>      maxItems: 1
>  
> +  avdd-supply:
> +    description: Analog power supply (2.9V)
> +
> +  ovdd-supply:
> +    description: Interface power supply (1.8V)
> +
> +  dvdd-supply:
> +    description: Digital power supply (1.2V)
> +
>    reset-gpios:
>      description: Reference to the GPIO connected to the XCLR pin, if any.
>      maxItems: 1
> @@ -60,6 +69,9 @@ required:
>    - compatible
>    - reg
>    - clocks
> +  - avdd-supply
> +  - ovdd-supply
> +  - dvdd-supply
>    - port
>  
>  additionalProperties: false
> @@ -79,6 +91,10 @@ examples:
>              assigned-clock-parents = <&imx335_clk_parent>;
>              assigned-clock-rates = <24000000>;
>  
> +            avdd-supply = <&camera_vdda_2v9>;
> +            ovdd-supply = <&camera_vddo_1v8>;
> +            dvdd-supply = <&camera_vddd_1v2>;
> +
>              port {
>                  imx335: endpoint {
>                      remote-endpoint = <&cam>;
> -- 
> 2.34.1
> 
> 
>
Sakari Ailus Oct. 10, 2023, 6:06 a.m. UTC | #3
Hi Kieran,

On Tue, Oct 10, 2023 at 01:51:22AM +0100, Kieran Bingham wrote:
> Add the bindings for the supply references used on the IMX335.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  .../bindings/media/i2c/sony,imx335.yaml          | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> index a167dcdb3a32..1863b5608a5c 100644
> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> @@ -32,6 +32,15 @@ properties:
>      description: Clock frequency from 6 to 27 MHz, 37.125MHz, 74.25MHz
>      maxItems: 1
>  
> +  avdd-supply:
> +    description: Analog power supply (2.9V)
> +
> +  ovdd-supply:
> +    description: Interface power supply (1.8V)
> +
> +  dvdd-supply:
> +    description: Digital power supply (1.2V)

I wonder what's the policy in this case --- some of the regulators are
often hard-wired and the bindings didn't have them previously either (I
wonder why, maybe they were all hard wired in the board??).

Could they be optional? The driver will need to be able to do without these
in any case.

> +
>    reset-gpios:
>      description: Reference to the GPIO connected to the XCLR pin, if any.
>      maxItems: 1
> @@ -60,6 +69,9 @@ required:
>    - compatible
>    - reg
>    - clocks
> +  - avdd-supply
> +  - ovdd-supply
> +  - dvdd-supply
>    - port
>  
>  additionalProperties: false
> @@ -79,6 +91,10 @@ examples:
>              assigned-clock-parents = <&imx335_clk_parent>;
>              assigned-clock-rates = <24000000>;
>  
> +            avdd-supply = <&camera_vdda_2v9>;
> +            ovdd-supply = <&camera_vddo_1v8>;
> +            dvdd-supply = <&camera_vddd_1v2>;
> +
>              port {
>                  imx335: endpoint {
>                      remote-endpoint = <&cam>;
Rob Herring (Arm) Oct. 10, 2023, 5:09 p.m. UTC | #4
On Tue, Oct 10, 2023 at 01:51:22AM +0100, Kieran Bingham wrote:
> Add the bindings for the supply references used on the IMX335.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  .../bindings/media/i2c/sony,imx335.yaml          | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> index a167dcdb3a32..1863b5608a5c 100644
> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> @@ -32,6 +32,15 @@ properties:
>      description: Clock frequency from 6 to 27 MHz, 37.125MHz, 74.25MHz
>      maxItems: 1
>  
> +  avdd-supply:
> +    description: Analog power supply (2.9V)
> +
> +  ovdd-supply:
> +    description: Interface power supply (1.8V)
> +
> +  dvdd-supply:
> +    description: Digital power supply (1.2V)
> +
>    reset-gpios:
>      description: Reference to the GPIO connected to the XCLR pin, if any.
>      maxItems: 1
> @@ -60,6 +69,9 @@ required:
>    - compatible
>    - reg
>    - clocks
> +  - avdd-supply
> +  - ovdd-supply
> +  - dvdd-supply

New required properties are an ABI break. That's fine only if you can 
explain no one is using this binding.

>    - port
>  
>  additionalProperties: false
> @@ -79,6 +91,10 @@ examples:
>              assigned-clock-parents = <&imx335_clk_parent>;
>              assigned-clock-rates = <24000000>;
>  
> +            avdd-supply = <&camera_vdda_2v9>;
> +            ovdd-supply = <&camera_vddo_1v8>;
> +            dvdd-supply = <&camera_vddd_1v2>;
> +
>              port {
>                  imx335: endpoint {
>                      remote-endpoint = <&cam>;
> -- 
> 2.34.1
>
Sakari Ailus Oct. 11, 2023, 11:01 a.m. UTC | #5
Hi Kieran,

On Tue, Oct 10, 2023 at 02:25:09PM +0100, Kieran Bingham wrote:
> Hi Sakari,
> 
> Quoting Sakari Ailus (2023-10-10 07:06:26)
> > Hi Kieran,
> > 
> > On Tue, Oct 10, 2023 at 01:51:22AM +0100, Kieran Bingham wrote:
> > > Add the bindings for the supply references used on the IMX335.
> > > 
> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > ---
> > >  .../bindings/media/i2c/sony,imx335.yaml          | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> > > index a167dcdb3a32..1863b5608a5c 100644
> > > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> > > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> > > @@ -32,6 +32,15 @@ properties:
> > >      description: Clock frequency from 6 to 27 MHz, 37.125MHz, 74.25MHz
> > >      maxItems: 1
> > >  
> > > +  avdd-supply:
> > > +    description: Analog power supply (2.9V)
> > > +
> > > +  ovdd-supply:
> > > +    description: Interface power supply (1.8V)
> > > +
> > > +  dvdd-supply:
> > > +    description: Digital power supply (1.2V)
> > 
> > I wonder what's the policy in this case --- some of the regulators are
> > often hard-wired and the bindings didn't have them previously either (I
> > wonder why, maybe they were all hard wired in the board??).
> > 
> > Could they be optional? The driver will need to be able to do without these
> > in any case.
> 
> Indeed - many devices do not need to define how they are powered up.
> 
> But Krzysztof stated that supplies should be required by the bindings on
> my recent posting for a VCM driver:
> 
>  - https://lore.kernel.org/all/6e163f4d-061d-3c20-4c2e-44c74d529f10@linaro.org/
> 
> So based on that I have made these 'required'.

I guess it's good to align bindings regarding this, in practice the driver
will need to work without regulators (or with dummies), too.

> 
> Even in my case here, with a camera module that is compatible with the
> Raspberry Pi camera connector - there isn't really 3 supplies. It's just
> a single gpio enable pin to bring this device up for me. Of course
> that's specific to the module not the sensor.

How do you declare that in DT? One of the regulators will be a GPIO one?
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
index a167dcdb3a32..1863b5608a5c 100644
--- a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
@@ -32,6 +32,15 @@  properties:
     description: Clock frequency from 6 to 27 MHz, 37.125MHz, 74.25MHz
     maxItems: 1
 
+  avdd-supply:
+    description: Analog power supply (2.9V)
+
+  ovdd-supply:
+    description: Interface power supply (1.8V)
+
+  dvdd-supply:
+    description: Digital power supply (1.2V)
+
   reset-gpios:
     description: Reference to the GPIO connected to the XCLR pin, if any.
     maxItems: 1
@@ -60,6 +69,9 @@  required:
   - compatible
   - reg
   - clocks
+  - avdd-supply
+  - ovdd-supply
+  - dvdd-supply
   - port
 
 additionalProperties: false
@@ -79,6 +91,10 @@  examples:
             assigned-clock-parents = <&imx335_clk_parent>;
             assigned-clock-rates = <24000000>;
 
+            avdd-supply = <&camera_vdda_2v9>;
+            ovdd-supply = <&camera_vddo_1v8>;
+            dvdd-supply = <&camera_vddd_1v2>;
+
             port {
                 imx335: endpoint {
                     remote-endpoint = <&cam>;