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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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@nxp.com>
> 
> 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@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> 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.

> +	.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