Re: [PATCH v2 2/2] media: nxp: add driver for i.MX93 MIPI CSI-2 controller and D-PHY

["Niklas Söderlund" <niklas.soderlund@ragnatech.se>]

Hello,

On 2023-09-18 00:25:09 +0300, Laurent Pinchart wrote:

[snip]

> > > diff --git a/drivers/media/platform/nxp/dwc-mipi-csi2.h 
> > > b/drivers/media/platform/nxp/dwc-mipi-csi2.h
> > > new file mode 100644
> > > index 000000000000..470023787c25
> > > --- /dev/null
> > > +++ b/drivers/media/platform/nxp/dwc-mipi-csi2.h
> > > @@ -0,0 +1,299 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/*
> > > + * Copyright 2023 NXP
> > > + */
> > > +
> > > +#ifndef __DWC_MIPI_CSI2_H__
> > > +#define __DWC_MIPI_CSI2_H__
> > > +
> > > +#include <linux/device.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/io.h>
> > > +
> > > +#include <media/v4l2-ctrls.h>
> > > +#include <media/v4l2-device.h>
> > > +#include <media/v4l2-fwnode.h>
> > > +#include <media/v4l2-mc.h>
> > > +#include <media/v4l2-subdev.h>
> > 
> > I don't think you need many of these headers here. Please move to the .c
> > file those you don't need here.
> > 
> > > +
> > > +/* MIPI CSI-2 Host Controller Registers Define */
> > > +
> 
> Niklas, is it me, or do the registers here look very familiar ? The
> R-Car V4H may have the same CSI-2 RX. Should we have a single driver ?

I agree it looks very familiar. The register space to indeed start out 
aligned between this driver and R-Car V4H (not checked every bit tho).  
But once we get to later registers it seems the two diverge a bit. If 
this is due to usage or something else I don't know. I have very sparse 
documentation for the V4H.

It becomes even more interesting. Have a look at this drivers 
hsfreqrange lookup table, dwc_csi_mbps_table[]. That overlaps with R-Car 
V3U (rcar-csi2.c hsfreqrange_v3u[]) instead of V4H, but V3U appears to 
support faster bus.

I think we can agree these two drivers are very similar, but not 
identical. The most interesting register here is the first one 
CSI2RX_VERSION. Maybe if we can figure that out we could start to think 
about a generic driver? Unfortunately I have no documentation for the 
content of this register. And the V4H driver do not use it, so I don't 
even know if it contains anything.

Is there any publicly documentation about this device? I have very 
sparse documentation about V4H, and nothing I can share. As to not block 
anyone maybe two drivers are OK and if we can figure out what parts, if 
any can be shared?

-- 
Kind Regards,
Niklas Söderlund