diff mbox series

ARM: dts: r8a7742-iwg21d-q7-dbcm-ca: Add missing camera regulators

Message ID 300149c730931914b77e17df6bcce89b67c3005f.1634222546.git.geert+renesas@glider.be (mailing list archive)
State Mainlined
Commit 8262d6ca5605dda635da5184248d1a7923187640
Delegated to: Geert Uytterhoeven
Headers show
Series ARM: dts: r8a7742-iwg21d-q7-dbcm-ca: Add missing camera regulators | expand

Commit Message

Geert Uytterhoeven Oct. 14, 2021, 2:44 p.m. UTC 
make dtbs_check:

    arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dt.yaml: ov5640@3c: 'AVDD-supply' is a required property
	    From schema: Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
    arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dt.yaml: ov5640@3c: 'DVDD-supply' is a required property
	    From schema: Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
    arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dt.yaml: ov5640@3c: 'DOVDD-supply' is a required property
	    From schema: Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml

Fix this by describing the missing regulators.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
[PATCH v2 27/52] dt-bindings: media: Convert OV5640 binding to a schema
https://lore.kernel.org/all/20210901091852.479202-28-maxime@cerno.tech/
---
 arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts  | 16 ++++++++++++++++
 .../r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi    |  3 +++
 2 files changed, 19 insertions(+)

Comments

Kieran Bingham Oct. 15, 2021, 12:17 p.m. UTC | #1 
Quoting Geert Uytterhoeven (2021-10-14 15:44:12)
> make dtbs_check:
> 
>     arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dt.yaml: ov5640@3c: 'AVDD-supply' is a required property
>             From schema: Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
>     arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dt.yaml: ov5640@3c: 'DVDD-supply' is a required property
>             From schema: Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
>     arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dt.yaml: ov5640@3c: 'DOVDD-supply' is a required property
>             From schema: Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
> 
> Fix this by describing the missing regulators.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> [PATCH v2 27/52] dt-bindings: media: Convert OV5640 binding to a schema
> https://lore.kernel.org/all/20210901091852.479202-28-maxime@cerno.tech/

Given that the OV5640 datasheet explicitly states 

 "
 If 2.8V is used for I/O power, due to a high voltage drop at the
 internal DVDD regulator, there is a potential heat issue. Hence, for a
 2.8V power system, OmniVision recommends using an external DVDD source.
 Due to the higher power down current when using an external DVDD
 source, OmniVision strongly recommends cutting off all powers,
 including the external DVDD, when the sensor is not in use in the case
 of 2.8V I/O and external DVDD.
 "

I was expecting these not to be fixed regulators. But having checked in
with you, I hear you've followed the schematics so that is what we have
to live with ;-)


Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> ---
>  arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts  | 16 ++++++++++++++++
>  .../r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi    |  3 +++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
> index 7e7b1028108dd133..75258f480a99a57c 100644
> --- a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
> +++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
> @@ -44,6 +44,22 @@ mclk_cam4: mclk-cam4 {
>                 #clock-cells = <0>;
>                 clock-frequency = <26000000>;
>         };
> +
> +       reg_1p8v: 1p8v {
> +               compatible = "regulator-fixed";
> +               regulator-name = "1P8V";
> +               regulator-min-microvolt = <1800000>;
> +               regulator-max-microvolt = <1800000>;
> +               regulator-always-on;
> +       };
> +
> +       reg_2p8v: 2p8v {
> +               compatible = "regulator-fixed";
> +               regulator-name = "2P8V";
> +               regulator-min-microvolt = <2800000>;
> +               regulator-max-microvolt = <2800000>;
> +               regulator-always-on;
> +       };
>  };
>  
>  &avb {
> diff --git a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
> index 70c72ba4fe724a70..40cef0b1d1e6267f 100644
> --- a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
> +++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
> @@ -17,6 +17,9 @@ ov5640@3c {
>                 reg = <0x3c>;
>                 clocks = <&MCLK_CAM>;
>                 clock-names = "xclk";
> +               AVDD-supply = <&reg_2p8v>;
> +               DOVDD-supply = <&reg_2p8v>;
> +               DVDD-supply = <&reg_1p8v>;
>                 status = "okay";
>  
>                 port {
> -- 
> 2.25.1
>
Kieran Bingham Oct. 15, 2021, 12:25 p.m. UTC | #2 
Quoting Kieran Bingham (2021-10-15 13:17:27)
> Quoting Geert Uytterhoeven (2021-10-14 15:44:12)
> > make dtbs_check:
> > 
> >     arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dt.yaml: ov5640@3c: 'AVDD-supply' is a required property
> >             From schema: Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
> >     arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dt.yaml: ov5640@3c: 'DVDD-supply' is a required property
> >             From schema: Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
> >     arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dt.yaml: ov5640@3c: 'DOVDD-supply' is a required property
> >             From schema: Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
> > 
> > Fix this by describing the missing regulators.
> > 
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > [PATCH v2 27/52] dt-bindings: media: Convert OV5640 binding to a schema
> > https://lore.kernel.org/all/20210901091852.479202-28-maxime@cerno.tech/
> 
> Given that the OV5640 datasheet explicitly states 
> 
>  "
>  If 2.8V is used for I/O power, due to a high voltage drop at the
>  internal DVDD regulator, there is a potential heat issue. Hence, for a
>  2.8V power system, OmniVision recommends using an external DVDD source.
>  Due to the higher power down current when using an external DVDD
>  source, OmniVision strongly recommends cutting off all powers,
>  including the external DVDD, when the sensor is not in use in the case
>  of 2.8V I/O and external DVDD.
>  "
> 
> I was expecting these not to be fixed regulators. But having checked in
> with you, I hear you've followed the schematics so that is what we have
> to live with ;-)
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> > ---
> >  arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts  | 16 ++++++++++++++++
> >  .../r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi    |  3 +++
> >  2 files changed, 19 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
> > index 7e7b1028108dd133..75258f480a99a57c 100644
> > --- a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
> > +++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
> > @@ -44,6 +44,22 @@ mclk_cam4: mclk-cam4 {
> >                 #clock-cells = <0>;
> >                 clock-frequency = <26000000>;
> >         };
> > +
> > +       reg_1p8v: 1p8v {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "1P8V";
> > +               regulator-min-microvolt = <1800000>;
> > +               regulator-max-microvolt = <1800000>;
> > +               regulator-always-on;
> > +       };
> > +
> > +       reg_2p8v: 2p8v {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "2P8V";
> > +               regulator-min-microvolt = <2800000>;
> > +               regulator-max-microvolt = <2800000>;
> > +               regulator-always-on;
> > +       };
> >  };
> >  
> >  &avb {
> > diff --git a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
> > index 70c72ba4fe724a70..40cef0b1d1e6267f 100644
> > --- a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
> > +++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
> > @@ -17,6 +17,9 @@ ov5640@3c {
> >                 reg = <0x3c>;
> >                 clocks = <&MCLK_CAM>;
> >                 clock-names = "xclk";
> > +               AVDD-supply = <&reg_2p8v>;
> > +               DOVDD-supply = <&reg_2p8v>;
> > +               DVDD-supply = <&reg_1p8v>;

I see in the bindings however that this DVDD is specified as 1.5v.
I assume 1.8 is how the hardware is wired, and is within a tolerence
range?

The OV5640 datasheet does show 
 - VDD-A: Analog: 2.8v (AVDD)
 - VDD-DA: Digital Core: 1.5v (DVDD)
 - VDD-IO: Digital IO: 1.8v (DOVDD)

(Brackets my interpretations)

Should DVDD be 1.5v?

--
Kieran


> >                 status = "okay";
> >  
> >                 port {
> > -- 
> > 2.25.1
> >
Geert Uytterhoeven Oct. 15, 2021, 1:56 p.m. UTC | #3 
Hi Kieran,

On Fri, Oct 15, 2021 at 2:25 PM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
> Quoting Kieran Bingham (2021-10-15 13:17:27)
> > Quoting Geert Uytterhoeven (2021-10-14 15:44:12)
> > > make dtbs_check:
> > >
> > >     arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dt.yaml: ov5640@3c: 'AVDD-supply' is a required property
> > >             From schema: Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
> > >     arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dt.yaml: ov5640@3c: 'DVDD-supply' is a required property
> > >             From schema: Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
> > >     arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dt.yaml: ov5640@3c: 'DOVDD-supply' is a required property
> > >             From schema: Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
> > >
> > > Fix this by describing the missing regulators.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > > [PATCH v2 27/52] dt-bindings: media: Convert OV5640 binding to a schema
> > > https://lore.kernel.org/all/20210901091852.479202-28-maxime@cerno.tech/
> >
> > Given that the OV5640 datasheet explicitly states
> >
> >  "
> >  If 2.8V is used for I/O power, due to a high voltage drop at the
> >  internal DVDD regulator, there is a potential heat issue. Hence, for a
> >  2.8V power system, OmniVision recommends using an external DVDD source.
> >  Due to the higher power down current when using an external DVDD
> >  source, OmniVision strongly recommends cutting off all powers,
> >  including the external DVDD, when the sensor is not in use in the case
> >  of 2.8V I/O and external DVDD.
> >  "
> >
> > I was expecting these not to be fixed regulators. But having checked in
> > with you, I hear you've followed the schematics so that is what we have
> > to live with ;-)
> >
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Thanks!

> > > --- a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
> > > +++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
> > > @@ -44,6 +44,22 @@ mclk_cam4: mclk-cam4 {
> > >                 #clock-cells = <0>;
> > >                 clock-frequency = <26000000>;
> > >         };
> > > +
> > > +       reg_1p8v: 1p8v {
> > > +               compatible = "regulator-fixed";
> > > +               regulator-name = "1P8V";
> > > +               regulator-min-microvolt = <1800000>;
> > > +               regulator-max-microvolt = <1800000>;
> > > +               regulator-always-on;
> > > +       };
> > > +
> > > +       reg_2p8v: 2p8v {
> > > +               compatible = "regulator-fixed";
> > > +               regulator-name = "2P8V";
> > > +               regulator-min-microvolt = <2800000>;
> > > +               regulator-max-microvolt = <2800000>;
> > > +               regulator-always-on;
> > > +       };
> > >  };
> > >
> > >  &avb {
> > > diff --git a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
> > > index 70c72ba4fe724a70..40cef0b1d1e6267f 100644
> > > --- a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
> > > +++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
> > > @@ -17,6 +17,9 @@ ov5640@3c {
> > >                 reg = <0x3c>;
> > >                 clocks = <&MCLK_CAM>;
> > >                 clock-names = "xclk";
> > > +               AVDD-supply = <&reg_2p8v>;
> > > +               DOVDD-supply = <&reg_2p8v>;
> > > +               DVDD-supply = <&reg_1p8v>;
>
> I see in the bindings however that this DVDD is specified as 1.5v.
> I assume 1.8 is how the hardware is wired, and is within a tolerence
> range?
>
> The OV5640 datasheet does show
>  - VDD-A: Analog: 2.8v (AVDD)
>  - VDD-DA: Digital Core: 1.5v (DVDD)
>  - VDD-IO: Digital IO: 1.8v (DOVDD)
>
> (Brackets my interpretations)

That matches the part "digital inputs (typical conditions: AVDD =
2.8V, DVDD = 1.5V, DOVDD = 1.8V)" in Table 8-3 of the datasheet.

> Should DVDD be 1.5v?

I have used what's in the schematics I have.  Which matches the
part "internal DVDD short to DVDD, DVP output, AVDD = 2.8V, DOVDD =
2.8V" with footnote "using the internal DVDD regulator is strongly
recommended for minimum power down current" in Table 8-3, I guess.

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
index 7e7b1028108dd133..75258f480a99a57c 100644
--- a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
+++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
@@ -44,6 +44,22 @@  mclk_cam4: mclk-cam4 {
 		#clock-cells = <0>;
 		clock-frequency = <26000000>;
 	};
+
+	reg_1p8v: 1p8v {
+		compatible = "regulator-fixed";
+		regulator-name = "1P8V";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-always-on;
+	};
+
+	reg_2p8v: 2p8v {
+		compatible = "regulator-fixed";
+		regulator-name = "2P8V";
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
+		regulator-always-on;
+	};
 };
 
 &avb {
diff --git a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
index 70c72ba4fe724a70..40cef0b1d1e6267f 100644
--- a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
+++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ov5640-single.dtsi
@@ -17,6 +17,9 @@  ov5640@3c {
 		reg = <0x3c>;
 		clocks = <&MCLK_CAM>;
 		clock-names = "xclk";
+		AVDD-supply = <&reg_2p8v>;
+		DOVDD-supply = <&reg_2p8v>;
+		DVDD-supply = <&reg_1p8v>;
 		status = "okay";
 
 		port {