| Message ID | 20230216175347.99778-3-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | RZ/G2L: Add CRU, CSI support | expand |
Hi Prabhakar, On Thu, Feb 16, 2023 at 6:57 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > OV5645 sensor can be connected to the CN1 connector on the > carrier board. This patch adds the ov5645 node under i2c0 node. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Thanks for your patch! > --- a/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts > @@ -6,6 +6,10 @@ > */ > > /dts-v1/; > + > +/* comment the #define statement if OV5645 is not connected to CN1 */ > +#define CAM_INTERFACE_OV5645 1 I believe the camera is optional ("can be connector" above). So shouldn't this be commented out by default? The rest LGTM, so with the above sorted out: Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
Hi Geert. Thank you for the review. On Mon, Mar 6, 2023 at 7:27 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Thu, Feb 16, 2023 at 6:57 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > OV5645 sensor can be connected to the CN1 connector on the > > carrier board. This patch adds the ov5645 node under i2c0 node. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Thanks for your patch! > > > --- a/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts > > @@ -6,6 +6,10 @@ > > */ > > > > /dts-v1/; > > + > > +/* comment the #define statement if OV5645 is not connected to CN1 */ > > +#define CAM_INTERFACE_OV5645 1 > > I believe the camera is optional ("can be connector" above). > So shouldn't this be commented out by default? > Yep, that makes sense. Shall I send this change or will you be generous to do this while applying ;) Cheers, Prabhakar > The rest LGTM, so with the above sorted out: > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > 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
Hi Prabhakar, On Wed, Mar 8, 2023 at 12:08 PM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > On Mon, Mar 6, 2023 at 7:27 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Thu, Feb 16, 2023 at 6:57 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > OV5645 sensor can be connected to the CN1 connector on the > > > carrier board. This patch adds the ov5645 node under i2c0 node. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Thanks for your patch! > > > > > --- a/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts > > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts > > > @@ -6,6 +6,10 @@ > > > */ > > > > > > /dts-v1/; > > > + > > > +/* comment the #define statement if OV5645 is not connected to CN1 */ > > > +#define CAM_INTERFACE_OV5645 1 > > > > I believe the camera is optional ("can be connector" above). > > So shouldn't this be commented out by default? > > > Yep, that makes sense. > > Shall I send this change or will you be generous to do this while applying ;) As this has an unresolved dependency on "[PATCH v2 0/2] Share RZ/G2L SoC DTSI with RZ/V2L SoC", I cannot do that yet... Gr{oetje,eeting}s, Geert
On Mon, Mar 06, 2023 at 08:27:19PM +0100, Geert Uytterhoeven wrote: > Hi Prabhakar, > > On Thu, Feb 16, 2023 at 6:57 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > OV5645 sensor can be connected to the CN1 connector on the > > carrier board. This patch adds the ov5645 node under i2c0 node. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Thanks for your patch! > > > --- a/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts > > @@ -6,6 +6,10 @@ > > */ > > > > /dts-v1/; > > + > > +/* comment the #define statement if OV5645 is not connected to CN1 */ > > +#define CAM_INTERFACE_OV5645 1 > > I believe the camera is optional ("can be connector" above). > So shouldn't this be commented out by default? Even better, could you turn this into a DT overlay ? > The rest LGTM, so with the above sorted out: > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Hi Geert, On Wed, Mar 8, 2023 at 10:59 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > On Mon, Mar 06, 2023 at 08:27:19PM +0100, Geert Uytterhoeven wrote: > > Hi Prabhakar, > > > > On Thu, Feb 16, 2023 at 6:57 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > OV5645 sensor can be connected to the CN1 connector on the > > > carrier board. This patch adds the ov5645 node under i2c0 node. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Thanks for your patch! > > > > > --- a/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts > > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts > > > @@ -6,6 +6,10 @@ > > > */ > > > > > > /dts-v1/; > > > + > > > +/* comment the #define statement if OV5645 is not connected to CN1 */ > > > +#define CAM_INTERFACE_OV5645 1 > > > > I believe the camera is optional ("can be connector" above). > > So shouldn't this be commented out by default? > > Even better, could you turn this into a DT overlay ? > Please let me know your thoughts on this. > > The rest LGTM, so with the above sorted out: > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > Cheers, Prabhakar
Hi Prabhakar, On Mon, Mar 13, 2023 at 9:54 PM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > On Wed, Mar 8, 2023 at 10:59 PM Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: > > On Mon, Mar 06, 2023 at 08:27:19PM +0100, Geert Uytterhoeven wrote: > > > On Thu, Feb 16, 2023 at 6:57 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > > > OV5645 sensor can be connected to the CN1 connector on the > > > > carrier board. This patch adds the ov5645 node under i2c0 node. > > > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > Thanks for your patch! > > > > > > > --- a/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts > > > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts > > > > @@ -6,6 +6,10 @@ > > > > */ > > > > > > > > /dts-v1/; > > > > + > > > > +/* comment the #define statement if OV5645 is not connected to CN1 */ > > > > +#define CAM_INTERFACE_OV5645 1 > > > > > > I believe the camera is optional ("can be connector" above). > > > So shouldn't this be commented out by default? > > > > Even better, could you turn this into a DT overlay ? > > > Please let me know your thoughts on this. I agree with Laurent. Gr{oetje,eeting}s, Geert
diff --git a/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts b/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts index bc2af6c92ccd..275e760324f3 100644 --- a/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts +++ b/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts @@ -6,6 +6,10 @@ */ /dts-v1/; + +/* comment the #define statement if OV5645 is not connected to CN1 */ +#define CAM_INTERFACE_OV5645 1 + #include "r9a07g044l2.dtsi" #include "rzg2l-smarc-som.dtsi" #include "rzg2l-smarc-pinfunction.dtsi" diff --git a/arch/arm64/boot/dts/renesas/rz-smarc-common.dtsi b/arch/arm64/boot/dts/renesas/rz-smarc-common.dtsi index 3962d47b3e59..915ad7e3935d 100644 --- a/arch/arm64/boot/dts/renesas/rz-smarc-common.dtsi +++ b/arch/arm64/boot/dts/renesas/rz-smarc-common.dtsi @@ -38,6 +38,38 @@ audio_mclock: audio_mclock { clock-frequency = <11289600>; }; +#ifdef CAM_INTERFACE_OV5645 + ov5645_vdddo_1v8: 1p8v { + compatible = "regulator-fixed"; + regulator-name = "camera_vdddo"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + }; + + ov5645_vdda_2v8: 2p8v { + compatible = "regulator-fixed"; + regulator-name = "camera_vdda"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + regulator-always-on; + }; + + ov5645_vddd_1v5: 1p5v { + compatible = "regulator-fixed"; + regulator-name = "camera_vddd"; + regulator-min-microvolt = <1500000>; + regulator-max-microvolt = <1500000>; + regulator-always-on; + }; + + ov5645_fixed_clk: osc25250_clk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <24000000>; + }; +#endif + snd_rzg2l: sound { compatible = "simple-audio-card"; simple-audio-card,format = "i2s"; @@ -119,6 +151,27 @@ &i2c0 { pinctrl-names = "default"; status = "okay"; + +#ifdef CAM_INTERFACE_OV5645 + ov5645: camera@3c { + compatible = "ovti,ov5645"; + reg = <0x3c>; + clocks = <&ov5645_fixed_clk>; + clock-frequency = <24000000>; + vdddo-supply = <&ov5645_vdddo_1v8>; + vdda-supply = <&ov5645_vdda_2v8>; + vddd-supply = <&ov5645_vddd_1v5>; + enable-gpios = <&pinctrl RZG2L_GPIO(2, 0) GPIO_ACTIVE_HIGH>; + reset-gpios = <&pinctrl RZG2L_GPIO(40, 2) GPIO_ACTIVE_LOW>; + + port { + ov5645_ep: endpoint { + clock-lanes = <0>; + data-lanes = <1 2>; + }; + }; + }; +#endif }; &i2c1 {