Re: [PATCH v3 05/12] media: imx8-isi: Add support for i.MX8QM and i.MX8QXP

[Frank Li]

On Thu, Mar 27, 2025 at 10:11:24PM +0200, Laurent Pinchart wrote:
> Hi Frank,
>
> Thank you for the patch.
>
> On Mon, Feb 10, 2025 at 03:59:24PM -0500, Frank Li wrote:
> > From: Robert Chiras <robert.chiras@xxxxxxx>
> >
> > Add compatibles and platform data for i.MX8QM and i.MX8QXP platforms.
> > There are 8 ISI channels on i.MX8QM while there are only 5 channels on
> > i.MX8QXP.
>
> There's a discrepancy between this series and the reference manuals.
> I've raised that issue in the review of the DT bindings, let's discuss
> it there. I'll skip that part of this patch for now.
>
> > Signed-off-by: Robert Chiras <robert.chiras@xxxxxxx>
> > Signed-off-by: Frank Li <Frank.Li@xxxxxxx>
> > ---
> > change from v2 to v3
> > - none
> >
> > change from v1 to v2
> > - remove intenal review tags
> > ---
> >  .../media/platform/nxp/imx8-isi/imx8-isi-core.c    | 47 ++++++++++++++++++++++
> >  .../media/platform/nxp/imx8-isi/imx8-isi-core.h    |  2 +
> >  2 files changed, 49 insertions(+)
> >
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > index 1e79b1211b603..a3237d76f7aa5 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > @@ -274,6 +274,25 @@ static const struct mxc_isi_set_thd mxc_imx8_isi_thd_v1 = {
> >  	.panic_set_thd_v = { .mask = 0xf0000, .offset = 16, .threshold = 0x7 },
> >  };
> >
> > +static const struct clk_bulk_data mxc_imx8qm_clks[] = {
> > +	{ .id = "per0" },
> > +	{ .id = "per1" },
> > +	{ .id = "per2" },
> > +	{ .id = "per3" },
> > +	{ .id = "per4" },
> > +	{ .id = "per5" },
> > +	{ .id = "per6" },
> > +	{ .id = "per7" },
> > +};
> > +
> > +static const struct clk_bulk_data mxc_imx8qxp_clks[] = {
> > +	{ .id = "per0" },
> > +	{ .id = "per4" },
> > +	{ .id = "per5" },
> > +	{ .id = "per6" },
> > +	{ .id = "per7" },
> > +};
> > +
> >  static const struct clk_bulk_data mxc_imx8mn_clks[] = {
> >  	{ .id = "axi" },
> >  	{ .id = "apb" },
> > @@ -334,6 +353,32 @@ static const struct mxc_isi_plat_data mxc_imx93_data = {
> >  	.has_36bit_dma		= false,
> >  };
> >
> > +static const struct mxc_isi_plat_data mxc_imx8qm_data = {
> > +	.model			= MXC_ISI_IMX8QM,
> > +	.num_ports		= 5,
> > +	.num_channels		= 8,
> > +	.reg_offset		= 0x10000,
> > +	.ier_reg		= &mxc_imx8_isi_ier_v1,
>
> Are you sure about this ? According to the reference manual, QXP seems
> to match mxc_imx8_isi_ier_v2, and QM doesn't seem to match either v1 or
> v2.

Thank you for found it. After check old code base and compared RM, you
are right, let me fix at next version.

Frank

>
> > +	.set_thd		= &mxc_imx8_isi_thd_v1,
> > +	.clks			= mxc_imx8qm_clks,
> > +	.num_clks		= ARRAY_SIZE(mxc_imx8qm_clks),
> > +	.buf_active_reverse	= true,
>
> I'll trust you on this value, I can't verify it.
>
> > +	.has_36bit_dma		= false,
> > +};
> > +
> > +static const struct mxc_isi_plat_data mxc_imx8qxp_data = {
> > +	.model			= MXC_ISI_IMX8QXP,
> > +	.num_ports		= 5,
> > +	.num_channels		= 5,
> > +	.reg_offset		= 0x10000,
> > +	.ier_reg		= &mxc_imx8_isi_ier_v1,
> > +	.set_thd		= &mxc_imx8_isi_thd_v1,
> > +	.clks			= mxc_imx8qxp_clks,
> > +	.num_clks		= ARRAY_SIZE(mxc_imx8qxp_clks),
> > +	.buf_active_reverse	= true,
> > +	.has_36bit_dma		= false,
> > +};
> > +
> >  /* -----------------------------------------------------------------------------
> >   * Power management
> >   */
> > @@ -541,6 +586,8 @@ static void mxc_isi_remove(struct platform_device *pdev)
> >  static const struct of_device_id mxc_isi_of_match[] = {
> >  	{ .compatible = "fsl,imx8mn-isi", .data = &mxc_imx8mn_data },
> >  	{ .compatible = "fsl,imx8mp-isi", .data = &mxc_imx8mp_data },
> > +	{ .compatible = "fsl,imx8qm-isi", .data = &mxc_imx8qm_data },
> > +	{ .compatible = "fsl,imx8qxp-isi", .data = &mxc_imx8qxp_data },
> >  	{ .compatible = "fsl,imx8ulp-isi", .data = &mxc_imx8ulp_data },
> >  	{ .compatible = "fsl,imx93-isi", .data = &mxc_imx93_data },
> >  	{ /* sentinel */ },
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > index 9c7fe9e5f941f..496cad3f9423d 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > @@ -158,6 +158,8 @@ struct mxc_gasket_ops {
> >  enum model {
> >  	MXC_ISI_IMX8MN,
> >  	MXC_ISI_IMX8MP,
> > +	MXC_ISI_IMX8QM,
> > +	MXC_ISI_IMX8QXP,
> >  	MXC_ISI_IMX8ULP,
> >  	MXC_ISI_IMX93,
> >  };
>
> --
> Regards,
>
> Laurent Pinchart