2023-10-10 00:52:01

by Kieran Bingham

[permalink] [raw]
Subject: [PATCH 1/5] media: dt-bindings: media: imx335: Add supply bindings

Add the bindings for the supply references used on the IMX335.

Signed-off-by: Kieran Bingham <[email protected]>
---
.../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


2023-10-10 03:53:38

by Umang Jain

[permalink] [raw]
Subject: Re: [PATCH 1/5] media: dt-bindings: media: imx335: Add supply bindings

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 <[email protected]>

LGTM,

Reviewed-by: Umang Jain <[email protected]>

> ---
> .../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>;

2023-10-10 05:03:36

by Marco Felsch

[permalink] [raw]
Subject: Re: [PATCH 1/5] media: dt-bindings: media: imx335: Add supply bindings

On 23-10-10, Kieran Bingham wrote:
> Add the bindings for the supply references used on the IMX335.
>
> Signed-off-by: Kieran Bingham <[email protected]>

Reviewed-by: Marco Felsch <[email protected]>

> ---
> .../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
>
>
>

2023-10-10 06:06:59

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH 1/5] media: dt-bindings: media: imx335: Add supply bindings

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 <[email protected]>
> ---
> .../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>;

--
Kind regards,

Sakari Ailus

2023-10-10 17:10:17

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/5] media: dt-bindings: media: imx335: Add supply bindings

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 <[email protected]>
> ---
> .../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
>

2023-10-11 11:01:47

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH 1/5] media: dt-bindings: media: imx335: Add supply bindings

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 <[email protected]>
> > > ---
> > > .../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/[email protected]/
>
> 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?

--
Regards,

Sakari Ailus