Hi Laurent,
> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Sent: 2023年7月28日 5:49
> To: G.N. Zhou (OSS) <guoniu.zhou@xxxxxxxxxxx>
> Cc: linux-media@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>;
> devicetree@xxxxxxxxxxxxxxx; mchehab@xxxxxxxxxx; robh+dt@xxxxxxxxxx;
> krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; Xavier Roumegue
> (OSS) <xavier.roumegue@xxxxxxxxxxx>; kernel@xxxxxxxxxxxxxx;
> jacopo.mondi@xxxxxxxxxxxxxxxx; sakari.ailus@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v5 3/3] media: nxp: imx8-isi: add ISI support for i.MX93
>
> Caution: This is an external email. Please take care when clicking links or opening
> attachments. When in doubt, report the message using the 'Report this email'
> button
>
>
> Hi Guoniu,
>
> Thank you for the patch.
>
> On Thu, Jun 29, 2023 at 09:36:21AM +0800, guoniu.zhou@xxxxxxxxxxx wrote:
> > From: "Guoniu.zhou" <guoniu.zhou@xxxxxxx>
> >
> > i.MX93 use a different gasket which has different register definition
>
> s/use/uses/
> s/definition/definitions/
>
> > compared with i.MX8. Hence implement the gasket callbacks in order to
> > add ISI support for i.MX93.
> >
> > Signed-off-by: Guoniu.zhou <guoniu.zhou@xxxxxxx>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> > .../platform/nxp/imx8-isi/imx8-isi-core.c | 15 ++++++++++
> > .../platform/nxp/imx8-isi/imx8-isi-core.h | 2 ++
> > .../platform/nxp/imx8-isi/imx8-isi-gasket.c | 30 +++++++++++++++++++
> > 3 files changed, 47 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 5165f8960c2c..27bd18b7ee65 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > @@ -307,6 +307,20 @@ static const struct mxc_isi_plat_data
> mxc_imx8mp_data = {
> > .has_36bit_dma = true,
> > };
> >
> > +static const struct mxc_isi_plat_data mxc_imx93_data = {
> > + .model = MXC_ISI_IMX93,
> > + .num_ports = 1,
> > + .num_channels = 1,
> > + .reg_offset = 0,
> > + .ier_reg = &mxc_imx8_isi_ier_v2,
> > + .set_thd = &mxc_imx8_isi_thd_v1,
> > + .clks = mxc_imx8mn_clks,
> > + .num_clks = ARRAY_SIZE(mxc_imx8mn_clks),
> > + .buf_active_reverse = true,
> > + .gasket_ops = &mxc_imx93_gasket_ops,
> > + .has_36bit_dma = false,
> > +};
> > +
> > /* -----------------------------------------------------------------------------
> > * Power management
> > */
> > @@ -518,6 +532,7 @@ static int 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,imx93-isi", .data = &mxc_imx93_data },
> > { /* sentinel */ },
> > };
> > MODULE_DEVICE_TABLE(of, mxc_isi_of_match); 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 78ca047d93d1..2810ebe9b5f7 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,7 @@ struct mxc_gasket_ops { enum model {
> > MXC_ISI_IMX8MN,
> > MXC_ISI_IMX8MP,
> > + MXC_ISI_IMX93,
> > };
> >
> > struct mxc_isi_plat_data {
> > @@ -295,6 +296,7 @@ struct mxc_isi_dev { };
> >
> > extern const struct mxc_gasket_ops mxc_imx8_gasket_ops;
> > +extern const struct mxc_gasket_ops mxc_imx93_gasket_ops;
> >
> > int mxc_isi_crossbar_init(struct mxc_isi_dev *isi); void
> > mxc_isi_crossbar_cleanup(struct mxc_isi_crossbar *xbar); diff --git
> > a/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c
> > b/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c
> > index 1d632dc60699..50ac1d3a2b6f 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c
> > @@ -53,3 +53,33 @@ const struct mxc_gasket_ops mxc_imx8_gasket_ops = {
> > .enable = mxc_imx8_gasket_enable,
> > .disable = mxc_imx8_gasket_disable, };
> > +
> > +/*
> > +---------------------------------------------------------------------
> > +--------
> > + * i.MX93 gasket
> > + **/
>
> Extra star here too.
>
> > +
> > +#define DISP_MIX_CAMERA_MUX 0x30
> > +#define DISP_MIX_CAMERA_MUX_DATA_TYPE(x) (((x) & 0x3f) << 3)
> > +#define DISP_MIX_CAMERA_MUX_GASKET_ENABLE BIT(16)
>
> Let's add a blank line here.
>
> With those small issues fixed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> I'll make the changes locally and include the series in my next pull request.
Okay, thanks.
>
> > +static void mxc_imx93_gasket_enable(struct mxc_isi_dev *isi,
> > + const struct v4l2_mbus_frame_desc
> *fd,
> > + const struct v4l2_mbus_framefmt
> *fmt,
> > + const unsigned int port) {
> > + u32 val;
> > +
> > + val = DISP_MIX_CAMERA_MUX_DATA_TYPE(fd->entry[0].bus.csi2.dt);
> > + val |= DISP_MIX_CAMERA_MUX_GASKET_ENABLE;
> > + regmap_write(isi->gasket, DISP_MIX_CAMERA_MUX, val); }
> > +
> > +static void mxc_imx93_gasket_disable(struct mxc_isi_dev *isi,
> > + unsigned int port) {
> > + regmap_write(isi->gasket, DISP_MIX_CAMERA_MUX, 0); }
> > +
> > +const struct mxc_gasket_ops mxc_imx93_gasket_ops = {
> > + .enable = mxc_imx93_gasket_enable,
> > + .disable = mxc_imx93_gasket_disable, };
>
> --
> Regards,
>
> Laurent Pinchart
