Re: [PATCH 6/8] arm64: dts: freescale: imx8mm-verdin: Add Toradex OV5640 CSI Cameras

[Frank Li <Frank.li@oss.nxp.com>]

On Fri, Jul 24, 2026 at 03:41:59PM +0200, Ernest Van Hoecke wrote:
> On Thu, Jul 23, 2026 at 02:30:07PM -0500, Frank Li wrote:
> > On Wed, Jul 22, 2026 at 06:15:50PM +0200, Ernest Van Hoecke wrote:
> > > On Wed, Jul 22, 2026 at 09:43:37AM -0500, Frank Li wrote:
> > > > On Wed, Jul 22, 2026 at 12:59:14PM +0200, Francesco Dolcini wrote:
> > > > > Hello Kieran,
> > > > >
> > > > > On Wed, Jul 22, 2026 at 11:30:29AM +0100, Kieran Bingham wrote:
> > > > > > Quoting Ernest Van Hoecke (2026-07-22 10:27:32)
> > > > > > > On Mon, Jul 20, 2026 at 02:16:46PM -0400, Frank Li wrote:
> > > > > > > > On Mon, Jul 13, 2026 at 05:06:27PM +0200, Ernest Van Hoecke wrote:
> > > > > > > > > From: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
> > > > > > > > >
> > > > > > > > > Add device tree overlays for the Toradex OV5640 CSI Camera on Verdin CSI_1.
> > > > > > > > >
> > > > > > > > > The default overlay describes the current CSI Camera Set 5MP OV5640 with a
> > > > > > > > > 27 MHz on-board oscillator. Add a separate 24 MHz overlay for the legacy
> > > > > > > > > camera module.
> > > > > > > > >
> > > > > > > > > Link: https://developer.toradex.com/hardware/accessories/cameras/csi-camera-module-5mp-ov5640-arducam
> > > > > > > > > Link: https://www.toradex.com/accessories/csi-camera-ov5640
> > > > > > > > > Link: https://developer.toradex.com/hardware/legacy-products/other/csi-camera-module-5mp-ov5640/
> > > > > > > > > Signed-off-by: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
> > > > > > > > > ---
> > > > > > > > >  arch/arm64/boot/dts/freescale/Makefile             |  6 ++
> > > > > > > > >  .../dts/freescale/imx8mm-verdin-ov5640-24mhz.dtso  | 17 +++++
> > > > > > > > >  .../boot/dts/freescale/imx8mm-verdin-ov5640.dtsi   | 78 ++++++++++++++++++++++
> > > > > > > > >  .../boot/dts/freescale/imx8mm-verdin-ov5640.dtso   | 18 +++++
> > > > > > > > >  4 files changed, 119 insertions(+)
> > > > > ...
> > > > > > I think with the Toradex ecosystem there would be some value in
> > > > > > supporting or helping with the ongoing dt-connectors or dt-addons topics
> > > > > > so that we can abstract the hardware which is being 'added'.
> > > > > >
> > > > > > I think it's important that we tackle the problem of combinatorial
> > > > > > explosions of overlays when we can add a component to multiple
> > > > > > platforms.
> > > > > >
> > > > > > For example, your OV5640 camera could be added to many different boards
> > > > > > - and each board could have many different cameras - in different ports.
> > > > > >
> > > > > > We should not be copy/pasting overlays for each combination, or we'll
> > > > > > have 'thousands' of identical overlays.
> > > > >
> > > > > I see your point, and I agree that it would be valuable to move this
> > > > > topic forward. I will raise it internally at Toradex and see how we can
> > > > > contribute.
> > > > >
> > > > > At the same time, quoting Marex (https://lore.kernel.org/all/dec2a7f6-80fd-4692-8936-969f8837a555@nabladev.com/):
> > > > >
> > > > >  | DT connectors have been discussed for the last 10 or so years and three
> > > > >  | is still no real progress.
> > > > >  |
> > > > >  | I would be happy to send a follow up patchset which would convert the
> > > > >  | DTOs to whatever connector implementation format lands in the future,
> > > > >  | but I am concerned that waiting for DT connectors will block this
> > > > >  | patchset from landing for a long time.
> > > > >
> > > >
> > > > I do quick check for ov5640, we can use partial connectors, which already
> > > > in kernel tree.
> > > >
> > > > use duplicate label like
> > > >
> > > > in mainboard
> > > > csi_io_i2c: &i2c3 {
> > > > 	...
> > > > }
> > > >
> > > > csi_io_regualotor_3v3: ....
> > > >
> > > >
> > > > csi_io: connector_csi {
> > > > 	compatible = "...";
> > > > 	gpio-map = <0 0 &gpio1 3 3>;
> > > > 	...
> > > > 	irq-map =<0 &gpio1 4 1>;
> > > > 	...
> > > > }
> > > >
> > > > in 5640 overlay
> > > >
> > > > &csi_io_i2c {
> > > > 	camera@xx {
> > > >
> > > > 		reset-gpios = <&csi_io 0 ACTIVE_HIGHT>;
> > > > 	}
> > > > }
> > > >
> > > > https://lore.kernel.org/imx/AM9PR04MB8353AC7DF91C5F73C34019E0E3F72@AM9PR04MB8353.eurprd04.prod.outlook.com/
> > > >
> > > > Only clock part have dependence. other parts. I am working mipi dsi for
> > > > display, almost work except panel show have some problem, which is not
> > > > related with connector.
> > > >
> > > > Frank
> > >
> > > This is very interesting and I agree with both you and Kieran that it
> > > would be great to abstract these connections away and kill the need for
> > > so many variations.
> > >
> > > Certainly I will read up on this and spread it internally as well.
> > >
> > > That said, since there are still unresolved dependencies, I am not
> > > convinced that we should already head this way with this series. It
> > > supports hardware that exists today in the currently standard and clean
> > > way.
> >
> > Your ov5640 have not such dependence. You can try connector firstly.
> >
> > display most like can go through connector, (I remember pwm already
> > supported, but not sure).
> >
> > others continue go through old method until dependence clear.
> >
> > Frank
> >
>
> The ov5640 still requires a clock nexus which is not merged yet.
>
> I have been playing around with it locally now and find the work here
> very interesting. Certainly this is also useful for Toradex and I would
> like to use it to reduce duplication.
>
> We will repeat this work for the Verdin iMX8MP. So I see real value in
> using these new mechanisms to map the GPIOs such that the Verdin ov5640
> overlay can be reused. However, this would also require a new binding
> for a "verdin_csi" device.
>
> Therefore, I could consider dropping the ov5640 overlays from this
> series and sending them as a follow up with the partial connectors
> support. But I don't want to block the series from getting merged for
> this non-trivial effort.
>
> If we go that way, I wonder where the verdin-ov5640 overlay should go?
> We also have Verdin modules with a TI SoC, with the OV5640 overlays
> already merged, that could be refactored. Those live in ti/, not
> freescale/. Do you have any input on how we should deal with that?

1: need make both work by duplicate overlay source file, which under TI
and freescale.
2: try to create common dir, and merge two file to one, which may need
involve both TI, freescale and Soc maintainer to discuss where to put.
the discussion start utill real case happen. This one may first real
user case.

>
> Personally I am still more in favour of merging now and refactoring with
> our next series for other SoMs, but I understand the desire to start on
> this now so there are more consumers of this new behaviour.

I suggest use this 8mm ov5640 as start, you can refact other later step by
step.

I can increase this part review priority.

Frank

>
> Thanks for your input and kind regards,
> Ernest
>