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

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

[Geert Uytterhoeven]

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(+)

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

Re: [PATCH] ARM: dts: r8a7742-iwg21d-q7-dbcm-ca: Add missing camera regulators

[Kieran Bingham]

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
>

Re: [PATCH] ARM: dts: r8a7742-iwg21d-q7-dbcm-ca: Add missing camera regulators

[Kieran Bingham]

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

Re: [PATCH] ARM: dts: r8a7742-iwg21d-q7-dbcm-ca: Add missing camera regulators

[Geert Uytterhoeven]

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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds