Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Frank Li <Frank.li@nxp.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Michael Riesch <michael.riesch@collabora.com>,
	Mehdi Djait <mehdi.djait@linux.intel.com>,
	Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
	Hans Verkuil <hverkuil@kernel.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Heiko Stuebner <heiko@sntech.de>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Sebastian Reichel <sebastian.reichel@collabora.com>,
	Nicolas Dufresne <nicolas.dufresne@collabora.com>,
	Collabora Kernel Team <kernel@collabora.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/3] media: rockchip: add driver for the rockchip mipi csi-2 receiver
Date: Mon, 1 Dec 2025 12:59:25 -0500	[thread overview]
Message-ID: <aS3XfZFF8rmt8ZT6@lizhi-Precision-Tower-5810> (raw)
In-Reply-To: <20251201033700.GB21943@pendragon.ideasonboard.com>

On Mon, Dec 01, 2025 at 12:37:00PM +0900, Laurent Pinchart wrote:
> Hello,
>
> On Wed, Nov 26, 2025 at 09:57:31PM +0100, Michael Riesch wrote:
> > On 11/18/25 23:08, Frank Li wrote:
> > > On Tue, Nov 18, 2025 at 07:59:14PM +0100, Michael Riesch wrote:
> > >> On 11/18/25 18:15, Frank Li wrote:
> > >>> On Tue, Nov 18, 2025 at 12:12:26PM +0100, Michael Riesch via B4 Relay wrote:
> > >>>> From: Michael Riesch <michael.riesch@collabora.com>
> > >>>>
> > >>>> [...]
> > >>>> +#define CSI2HOST_N_LANES     0x04
> > >>>> +#define CSI2HOST_CSI2_RESETN 0x10
> > >>>> +#define CSI2HOST_PHY_STATE   0x14
> > >>>> +#define CSI2HOST_ERR1	     0x20
> > >>>> +#define CSI2HOST_ERR2	     0x24
> > >>>> +#define CSI2HOST_MSK1	     0x28
> > >>>> +#define CSI2HOST_MSK2	     0x2c
> > >>>> +#define CSI2HOST_CONTROL     0x40
> > >>>
> > >>> Look like that is designware CSI2 controller, can we build common library
> > >>> for all dwc csi2 controller, instead of every vendor create individual one.
> > >>>
> > >>> First try at
> > >>> https://lore.kernel.org/linux-media/20250821-95_cam-v3-21-c9286fbb34b9@nxp.com/
> > >>>
> > >>> Toshiba have similar patch
> > >>> https://lore.kernel.org/linux-media/aPZd39riAxqfw3mT@lizhi-Precision-Tower-5810/
> > >>>
> > >>> Frank
> > >>
> > >> This has been discussed already a while ago:
> > >> https://lore.kernel.org/all/20250507083837.GA11152@pendragon.ideasonboard.com/
> > >
> > > https://lore.kernel.org/all/20250702093806.GF16835@pendragon.ideasonboard.com/
> > >
> > > Laurent Pinchart prevent imx93 DWC CSI2 driver at Jul, your discussion at
> > > May, I think Laurent Pinchart change the mind.
> > >
> > > We can choose not base on imx6, but we should create a standard dwc2 under
> > > drivers/media/platform/synopsys/
> >
> > In principle, I agree with refactoring out common code. However, I am
> > not sure how similar these IP cores really are. Again, the answer I
> > received from Rockchip states that this is *not* a Synopsys IP core.
>
> That's what I based my opinion on. If we're dealing with different IP
> cores from different vendors, we can have separate drivers. Even within
> the family of CSI-2 receivers from Synopsys, I'm fine having different
> drivers if the version differ significantly enough that using a single
> driver would cause more pain than gain.

Laurent Pinchart:

	We need use unified standard for every one. In the patch below patch
is enough show the difference between Sysnopsys version.

media: synopsys: csi2: Add basic v150* version register
https://lore.kernel.org/imx/20250821-95_cam-v3-20-c9286fbb34b9@nxp.com/

	The register layout is differense, most field defination are the
same and some changed.

	I know it is hard to define how much pain and gain.

	But I think this one is closer to imx6's version even though they
claim NOT DWC CSI2 IP than imx93 v150 dwc CSI2 version. (for example, many
iio sensor/gpio expandor use the same set of register define for difference
vendor, but still use one driver for these).

https://lore.kernel.org/all/20250702093806.GF16835@pendragon.ideasonboard.com/#t

"This seems to be at least the third instance of a driver for the CSI-2
receiver, the first two being
drivers/media/platform/raspberrypi/rp1-cfe/dphy.c and the second one
drivers/media/platform/renesas/rcar-csi2.c (the latter seems to support
multiple CSI-2 receivers). drivers/staging/media/imx/imx6-mipi-csi2.c
seem related too, likely for an old version of the IP.

Could we please try to avoid code duplication ?
"

	Or do you think I should go through this ways (direct create one
dwc for v150) instead base on imx6 one to enable imx93's dwc CSI2?

>
> To explain this differently, for Synopsys CSI-2 receivers, I would like
> to try and support multiple IP cores integrated in different SoCs in a
> single driver. If, during that development, we find good reasons to use
> separate drivers, then we can do so.

I think if reviewing and landing process could go through smooth, imx6 can
support this device with a little bit modify. There are not technology's
bar so far.

I split to more simple patches from big 32 patch serises.

https://lore.kernel.org/imx/20251111-stage-csi2-cleanup-v2-0-371bb951776f@nxp.com/

Only comments change

https://lore.kernel.org/linux-media/20251105195808.297645-1-Frank.Li@nxp.com/

Frank

>
> > Can you maybe test whether the Rockchip MIPI CSI-2 Receiver driver in
> > this series works on your hardware? Then we will know if (and what)
> > parts can be shared.
> >
> > > Laurent Pinchart:
> > >
> > > 	Can you provide direction?
> > >
> > >> Bottom line from Laurent:
> > >>
> > >> "Let's keep this driver Rockchip-specific then. Thanks for checking."
> > >>
> > >>>  [...]
>
> --
> Regards,
>
> Laurent Pinchart


  reply	other threads:[~2025-12-01 17:59 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-18 11:12 [PATCH v2 0/3] media: rockchip: add driver for the rockchip mipi csi-2 receiver Michael Riesch via B4 Relay
2025-11-18 11:12 ` [PATCH v2 1/3] media: dt-bindings: add " Michael Riesch via B4 Relay
2025-11-18 11:12 ` [PATCH v2 2/3] media: rockchip: add driver for the " Michael Riesch via B4 Relay
2025-11-18 17:15   ` Frank Li
2025-11-18 18:59     ` Michael Riesch
2025-11-18 22:08       ` Frank Li
2025-11-26 20:57         ` Michael Riesch
2025-12-01  3:37           ` Laurent Pinchart
2025-12-01 17:59             ` Frank Li [this message]
2025-11-18 11:12 ` [PATCH v2 3/3] arm64: defconfig: enable " Michael Riesch via B4 Relay

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aS3XfZFF8rmt8ZT6@lizhi-Precision-Tower-5810 \
    --to=frank.li@nxp.com \
    --cc=bryan.odonoghue@linaro.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=heiko@sntech.de \
    --cc=hverkuil@kernel.org \
    --cc=kernel@collabora.com \
    --cc=krzk+dt@kernel.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mchehab@kernel.org \
    --cc=mehdi.djait@linux.intel.com \
    --cc=michael.riesch@collabora.com \
    --cc=nicolas.dufresne@collabora.com \
    --cc=p.zabel@pengutronix.de \
    --cc=robh@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=sebastian.reichel@collabora.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox