[PATCH v4 0/3] add ISI support for iMX93

[PATCH v4 0/3] add ISI support for iMX93

[guoniu.zhou]

From: "Guoniu.zhou" <guoniu.zhou@nxp.com>

NXP i.MX93 family almost reuse ISI IP from i.MX8M family, so add
it support in current ISI driver

Changes in version 4:
- Update commit message to explain why need to move gasket ops to a structure
- Remove some redundant and unused header files
- Move gasket ops structure variable to imx8-isi-gasket.c and export them by extern
- Drop some check

Changes in version 3:
- Split [PATCH v2 2/2] into two patches, one create a separate file to
  store gasket operation and the other to add ISI support for i.MX93.
- Drop some debug message in gasket operation.
- Merge .gasket_enable and .gasket_config to .gasket_enable
- Drop some dead code
- Some other small updates

Changes in version 2:
- Remove two patches which used to rename imx8 to imx.
  [PATCH 1/4] media: dt-bindings: media: rename nxp,imx8-isi.yaml to nxp,imx-isi.yaml
  [PATCH 2/4] media: nxp: rename imx8-isi to imx-isi and remove reference to i.MX8
- Modify commit log to more accurately match its goal.
- Remove redundant "media" in patch subject.

Guoniu.zhou (3):
  media: dt-bindings: nxp,imx8-isi: add i.MX93 ISI compatible string
  media: nxp: imx8-isi: move i.MX8 gasket configuration to an ops
    structure
  media: nxp: imx8-isi: add ISI support for i.MX93

 .../bindings/media/nxp,imx8-isi.yaml          |  5 +-
 drivers/media/platform/nxp/imx8-isi/Makefile  |  4 +-
 .../platform/nxp/imx8-isi/imx8-isi-core.c     | 24 ++++--
 .../platform/nxp/imx8-isi/imx8-isi-core.h     | 22 ++---
 .../platform/nxp/imx8-isi/imx8-isi-crossbar.c | 40 ++--------
 .../platform/nxp/imx8-isi/imx8-isi-gasket.c   | 80 +++++++++++++++++++
 6 files changed, 123 insertions(+), 52 deletions(-)
 create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c

-- 
2.37.1

[PATCH v4 1/3] media: dt-bindings: nxp,imx8-isi: add i.MX93 ISI compatible string

[guoniu.zhou]

From: "Guoniu.zhou" <guoniu.zhou@nxp.com>

Add the compatible string support for i.MX93 ISI.

Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Tommaso Merciai <tomm.merciai@gmail.com>
---
 Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml
index 6038b9b5ab36..e4665469a86c 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml
@@ -21,6 +21,7 @@ properties:
     enum:
       - fsl,imx8mn-isi
       - fsl,imx8mp-isi
+      - fsl,imx93-isi
 
   reg:
     maxItems: 1
@@ -72,7 +73,9 @@ allOf:
       properties:
         compatible:
           contains:
-            const: fsl,imx8mn-isi
+            enum:
+              - fsl,imx8mn-isi
+              - fsl,imx93-isi
     then:
       properties:
         interrupts:
-- 
2.37.1

Re: [PATCH v4 1/3] media: dt-bindings: nxp,imx8-isi: add i.MX93 ISI compatible string

[Rob Herring]

On Wed, 28 Jun 2023 14:42:49 +0800, guoniu.zhou@oss.nxp.com wrote:
> From: "Guoniu.zhou" <guoniu.zhou@nxp.com>
> 
> Add the compatible string support for i.MX93 ISI.
> 
> Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Tommaso Merciai <tomm.merciai@gmail.com>
> ---
>  Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/clock/milbeaut-clock.example.dtb: /example-2/serial@1e700010: failed to match any schema with compatible: ['socionext,milbeaut-usio-uart']
Documentation/devicetree/bindings/clock/sprd,sc9863a-clk.example.dtb: /example-1/syscon@20e00000: failed to match any schema with compatible: ['sprd,sc9863a-glbregs', 'syscon', 'simple-mfd']
Documentation/devicetree/bindings/leds/common.example.dtb: /example-2/i2c/led-controller@30: failed to match any schema with compatible: ['panasonic,an30259a']
Documentation/devicetree/bindings/sound/audio-graph-card2.example.dtb: /example-0/cpu: failed to match any schema with compatible: ['cpu-driver']
Documentation/devicetree/bindings/sound/audio-graph-card2.example.dtb: /example-0/codec: failed to match any schema with compatible: ['codec-driver']
Documentation/devicetree/bindings/thermal/brcm,avs-ro-thermal.example.dtb: /example-0/avs-monitor@7d5d2000: failed to match any schema with compatible: ['brcm,bcm2711-avs-monitor', 'syscon', 'simple-mfd']
Documentation/devicetree/bindings/thermal/imx-thermal.example.dtb: /example-0/anatop@20c8000: failed to match any schema with compatible: ['fsl,imx6q-anatop', 'syscon', 'simple-mfd']
Documentation/devicetree/bindings/i2c/qcom,i2c-cci.example.dtb: /example-0/cci@ac4a000/i2c-bus@1/camera@60: failed to match any schema with compatible: ['ovti,ov7251']
Documentation/devicetree/bindings/iio/adc/ti,palmas-gpadc.example.dtb: /example-0/pmic: failed to match any schema with compatible: ['ti,twl6035-pmic', 'ti,palmas-pmic']
Documentation/devicetree/bindings/iio/adc/ti,palmas-gpadc.example.dtb: /example-0/pmic: failed to match any schema with compatible: ['ti,twl6035-pmic', 'ti,palmas-pmic']
Documentation/devicetree/bindings/input/mediatek,pmic-keys.example.dtb: /example-0/pmic: failed to match any schema with compatible: ['mediatek,mt6397']
Documentation/devicetree/bindings/input/sprd,sc27xx-vibrator.example.dtb: /example-0/pmic@0: failed to match any schema with compatible: ['sprd,sc2731']
Documentation/devicetree/bindings/dma/dma-controller.example.dtb: /example-0/dma-controller@48000000: failed to match any schema with compatible: ['ti,omap-sdma']
Documentation/devicetree/bindings/dma/dma-router.example.dtb: /example-0/dma-router@4a002b78: failed to match any schema with compatible: ['ti,dra7-dma-crossbar']
Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.example.dtb: /example-0/memory-controller@13410000/ethernet@6: failed to match any schema with compatible: ['davicom,dm9000']
Documentation/devicetree/bindings/arm/hisilicon/controller/sysctrl.example.dtb: /example-0/system-controller@802000/clock@0: failed to match any schema with compatible: ['hisilicon,hi3620-clock']
Documentation/devicetree/bindings/arm/hisilicon/controller/cpuctrl.example.dtb: /example-0/cpuctrl@a22000/clock@0: failed to match any schema with compatible: ['hisilicon,hix5hd2-clock']
Documentation/devicetree/bindings/arm/hisilicon/controller/hi3798cv200-perictrl.example.dtb: /example-0/peripheral-controller@8a20000/phy@850: failed to match any schema with compatible: ['hisilicon,hi3798cv200-combphy']
Documentation/devicetree/bindings/media/rockchip-isp1.example.dtb: /example-0/parent/i2c/camera@36: failed to match any schema with compatible: ['ovti,ov5695']
Documentation/devicetree/bindings/net/marvell,mvusb.example.dtb: /example-0/usb/mdio@1/switch@0: failed to match any schema with compatible: ['marvell,mv88e6190']
Documentation/devicetree/bindings/net/qca,ar71xx.example.dtb: /example-0/ethernet@1a000000/mdio/switch@10: failed to match any schema with compatible: ['qca,ar9331-switch']
Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.example.dtb: /example-0/iomcu@ffd7e000: failed to match any schema with compatible: ['hisilicon,hi3660-iomcu', 'syscon']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230628064251.1774296-2-guoniu.zhou@oss.nxp.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

RE: [PATCH v4 1/3] media: dt-bindings: nxp,imx8-isi: add i.MX93 ISI compatible string

[G.N. Zhou (OSS)]

Hi Rob,

Thanks for your report.

As you said, I have ran " make dt_binding_check DT_SCHEMA_FIELS=../../nxp,imx8-isi.yaml" before sending the patch and don't see errors. 
I try to reproduce the issue you said and update dtschema, yamllint, but can't only check nxp,imx8-isi.yaml. But I can reproduce the issue
no matter whether include my patches or not when enable all test examples. So seems these errors are not introduced by my patches. I'm
not sure if I miss something?

My test command:
# no error or warning found
make DT_CHECKER_FLAGS=-y dt_binding_check -j8 DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml

# found errors as you reported no matter whether include my patches or not
make DT_CHECKER_FLAGS=-m dt_binding_check

Kernel version:
V6.4_rc1

dtschema version: 2023.6.1
yamllint version: 1.32.0

Best Regards
G.N Zhou

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 2023年6月28日 17:18
> To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> Cc: krzysztof.kozlowski+dt@linaro.org; sakari.ailus@linux.intel.com;
> laurent.pinchart@ideasonboard.com; mchehab@kernel.org;
> robh+dt@kernel.org; Xavier Roumegue (OSS) <xavier.roumegue@oss.nxp.com>;
> conor+dt@kernel.org; linux-media@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>; kernel@pengutronix.de; devicetree@vger.kernel.org;
> jacopo.mondi@ideasonboard.com
> Subject: Re: [PATCH v4 1/3] media: dt-bindings: nxp,imx8-isi: add i.MX93 ISI
> compatible string
> 
> 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
> 
> 
> On Wed, 28 Jun 2023 14:42:49 +0800, guoniu.zhou@oss.nxp.com wrote:
> > From: "Guoniu.zhou" <guoniu.zhou@nxp.com>
> >
> > Add the compatible string support for i.MX93 ISI.
> >
> > Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
> > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Reviewed-by: Tommaso Merciai <tomm.merciai@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/clock/milbeaut-clock.example.dtb:
> /example-2/serial@1e700010: failed to match any schema with compatible:
> ['socionext,milbeaut-usio-uart']
> Documentation/devicetree/bindings/clock/sprd,sc9863a-clk.example.dtb:
> /example-1/syscon@20e00000: failed to match any schema with compatible:
> ['sprd,sc9863a-glbregs', 'syscon', 'simple-mfd']
> Documentation/devicetree/bindings/leds/common.example.dtb:
> /example-2/i2c/led-controller@30: failed to match any schema with compatible:
> ['panasonic,an30259a']
> Documentation/devicetree/bindings/sound/audio-graph-card2.example.dtb:
> /example-0/cpu: failed to match any schema with compatible: ['cpu-driver']
> Documentation/devicetree/bindings/sound/audio-graph-card2.example.dtb:
> /example-0/codec: failed to match any schema with compatible: ['codec-driver']
> Documentation/devicetree/bindings/thermal/brcm,avs-ro-thermal.example.dtb:
> /example-0/avs-monitor@7d5d2000: failed to match any schema with
> compatible: ['brcm,bcm2711-avs-monitor', 'syscon', 'simple-mfd']
> Documentation/devicetree/bindings/thermal/imx-thermal.example.dtb:
> /example-0/anatop@20c8000: failed to match any schema with compatible:
> ['fsl,imx6q-anatop', 'syscon', 'simple-mfd']
> Documentation/devicetree/bindings/i2c/qcom,i2c-cci.example.dtb:
> /example-0/cci@ac4a000/i2c-bus@1/camera@60: failed to match any schema
> with compatible: ['ovti,ov7251']
> Documentation/devicetree/bindings/iio/adc/ti,palmas-gpadc.example.dtb:
> /example-0/pmic: failed to match any schema with compatible:
> ['ti,twl6035-pmic', 'ti,palmas-pmic']
> Documentation/devicetree/bindings/iio/adc/ti,palmas-gpadc.example.dtb:
> /example-0/pmic: failed to match any schema with compatible:
> ['ti,twl6035-pmic', 'ti,palmas-pmic']
> Documentation/devicetree/bindings/input/mediatek,pmic-keys.example.dtb:
> /example-0/pmic: failed to match any schema with compatible:
> ['mediatek,mt6397']
> Documentation/devicetree/bindings/input/sprd,sc27xx-vibrator.example.dtb:
> /example-0/pmic@0: failed to match any schema with compatible:
> ['sprd,sc2731']
> Documentation/devicetree/bindings/dma/dma-controller.example.dtb:
> /example-0/dma-controller@48000000: failed to match any schema with
> compatible: ['ti,omap-sdma']
> Documentation/devicetree/bindings/dma/dma-router.example.dtb:
> /example-0/dma-router@4a002b78: failed to match any schema with
> compatible: ['ti,dra7-dma-crossbar']
> Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.example
> .dtb: /example-0/memory-controller@13410000/ethernet@6: failed to match
> any schema with compatible: ['davicom,dm9000']
> Documentation/devicetree/bindings/arm/hisilicon/controller/sysctrl.example.dt
> b: /example-0/system-controller@802000/clock@0: failed to match any schema
> with compatible: ['hisilicon,hi3620-clock']
> Documentation/devicetree/bindings/arm/hisilicon/controller/cpuctrl.example.dt
> b: /example-0/cpuctrl@a22000/clock@0: failed to match any schema with
> compatible: ['hisilicon,hix5hd2-clock']
> Documentation/devicetree/bindings/arm/hisilicon/controller/hi3798cv200-peric
> trl.example.dtb: /example-0/peripheral-controller@8a20000/phy@850: failed
> to match any schema with compatible: ['hisilicon,hi3798cv200-combphy']
> Documentation/devicetree/bindings/media/rockchip-isp1.example.dtb:
> /example-0/parent/i2c/camera@36: failed to match any schema with
> compatible: ['ovti,ov5695']
> Documentation/devicetree/bindings/net/marvell,mvusb.example.dtb:
> /example-0/usb/mdio@1/switch@0: failed to match any schema with
> compatible: ['marvell,mv88e6190']
> Documentation/devicetree/bindings/net/qca,ar71xx.example.dtb:
> /example-0/ethernet@1a000000/mdio/switch@10: failed to match any schema
> with compatible: ['qca,ar9331-switch']
> Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.example.dtb:
> /example-0/iomcu@ffd7e000: failed to match any schema with compatible:
> ['hisilicon,hi3660-iomcu', 'syscon']
> 
> doc reference errors (make refcheckdocs):
> 
> See
> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230628064
> 251.1774296-2-guoniu.zhou@oss.nxp.com
> 
> The base for the series is generally the latest rc1. A different dependency should
> be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above error(s),
> then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking your
> schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v4 1/3] media: dt-bindings: nxp,imx8-isi: add i.MX93 ISI compatible string

[Rob Herring]

On Wed, Jun 28, 2023 at 11:56:13AM +0000, G.N. Zhou (OSS) wrote:
> Hi Rob,
> 
> Thanks for your report.
> 
> As you said, I have ran " make dt_binding_check DT_SCHEMA_FIELS=../../nxp,imx8-isi.yaml" before sending the patch and don't see errors. 
> I try to reproduce the issue you said and update dtschema, yamllint, but can't only check nxp,imx8-isi.yaml. But I can reproduce the issue
> no matter whether include my patches or not when enable all test examples. So seems these errors are not introduced by my patches. I'm
> not sure if I miss something?

You can ignore this.

[PATCH v4 2/3] media: nxp: imx8-isi: move i.MX8 gasket configuration to an ops structure

[guoniu.zhou]

From: "Guoniu.zhou" <guoniu.zhou@nxp.com>

The i.MX93 includes an ISI instance compatible with the imx8-isi
driver, but with a different gasket. To prepare for this, make the
gasket configuration modular by moving the code to an ops structure.

Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
---
 drivers/media/platform/nxp/imx8-isi/Makefile  |  4 +-
 .../platform/nxp/imx8-isi/imx8-isi-core.c     |  9 ++--
 .../platform/nxp/imx8-isi/imx8-isi-core.h     | 20 +++----
 .../platform/nxp/imx8-isi/imx8-isi-crossbar.c | 40 +++-----------
 .../platform/nxp/imx8-isi/imx8-isi-gasket.c   | 52 +++++++++++++++++++
 5 files changed, 74 insertions(+), 51 deletions(-)

diff --git a/drivers/media/platform/nxp/imx8-isi/Makefile b/drivers/media/platform/nxp/imx8-isi/Makefile
index 9bff9297686d..4713c4e8b64b 100644
--- a/drivers/media/platform/nxp/imx8-isi/Makefile
+++ b/drivers/media/platform/nxp/imx8-isi/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
-imx8-isi-y := imx8-isi-core.o imx8-isi-crossbar.o imx8-isi-hw.o \
-	imx8-isi-pipe.o imx8-isi-video.o
+imx8-isi-y := imx8-isi-core.o imx8-isi-crossbar.o imx8-isi-gasket.o \
+	imx8-isi-hw.o imx8-isi-pipe.o imx8-isi-video.o
 imx8-isi-$(CONFIG_DEBUG_FS) += imx8-isi-debug.o
 imx8-isi-$(CONFIG_VIDEO_IMX8_ISI_M2M) += imx8-isi-m2m.o
 
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 253e77189b69..f2774325dd61 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
@@ -18,9 +18,6 @@
 #include <linux/string.h>
 #include <linux/types.h>
 
-#include <media/media-device.h>
-#include <media/v4l2-async.h>
-#include <media/v4l2-device.h>
 #include <media/v4l2-mc.h>
 
 #include "imx8-isi-core.h"
@@ -289,7 +286,7 @@ static const struct mxc_isi_plat_data mxc_imx8mn_data = {
 	.clks			= mxc_imx8mn_clks,
 	.num_clks		= ARRAY_SIZE(mxc_imx8mn_clks),
 	.buf_active_reverse	= false,
-	.has_gasket		= true,
+	.gasket_ops		= &mxc_imx8_gasket_ops,
 	.has_36bit_dma		= false,
 };
 
@@ -303,7 +300,7 @@ static const struct mxc_isi_plat_data mxc_imx8mp_data = {
 	.clks			= mxc_imx8mn_clks,
 	.num_clks		= ARRAY_SIZE(mxc_imx8mn_clks),
 	.buf_active_reverse	= true,
-	.has_gasket		= true,
+	.gasket_ops		= &mxc_imx8_gasket_ops,
 	.has_36bit_dma		= true,
 };
 
@@ -443,7 +440,7 @@ static int mxc_isi_probe(struct platform_device *pdev)
 		return PTR_ERR(isi->regs);
 	}
 
-	if (isi->pdata->has_gasket) {
+	if (isi->pdata->gasket_ops) {
 		isi->gasket = syscon_regmap_lookup_by_phandle(dev->of_node,
 							      "fsl,blk-ctrl");
 		if (IS_ERR(isi->gasket)) {
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 e469788a9e6c..87182ebb99f0 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
@@ -10,14 +10,6 @@
 #ifndef __MXC_ISI_CORE_H__
 #define __MXC_ISI_CORE_H__
 
-#include <linux/list.h>
-#include <linux/mutex.h>
-#include <linux/spinlock.h>
-#include <linux/types.h>
-#include <linux/videodev2.h>
-
-#include <media/media-device.h>
-#include <media/media-entity.h>
 #include <media/v4l2-async.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-dev.h>
@@ -147,6 +139,14 @@ struct mxc_isi_set_thd {
 	struct mxc_isi_panic_thd panic_set_thd_v;
 };
 
+struct mxc_gasket_ops {
+	void (*enable)(struct mxc_isi_dev *isi,
+		       const struct v4l2_mbus_frame_desc *fd,
+		       const struct v4l2_mbus_framefmt *fmt,
+		       const unsigned int port);
+	void (*disable)(struct mxc_isi_dev *isi, const unsigned int port);
+};
+
 enum model {
 	MXC_ISI_IMX8MN,
 	MXC_ISI_IMX8MP,
@@ -159,10 +159,10 @@ struct mxc_isi_plat_data {
 	unsigned int reg_offset;
 	const struct mxc_isi_ier_reg  *ier_reg;
 	const struct mxc_isi_set_thd *set_thd;
+	const struct mxc_gasket_ops *gasket_ops;
 	const struct clk_bulk_data *clks;
 	unsigned int num_clks;
 	bool buf_active_reverse;
-	bool has_gasket;
 	bool has_36bit_dma;
 };
 
@@ -286,6 +286,8 @@ struct mxc_isi_dev {
 	struct dentry			*debugfs_root;
 };
 
+extern const struct mxc_gasket_ops mxc_imx8_gasket_ops;
+
 int mxc_isi_crossbar_init(struct mxc_isi_dev *isi);
 void mxc_isi_crossbar_cleanup(struct mxc_isi_crossbar *xbar);
 int mxc_isi_crossbar_register(struct mxc_isi_crossbar *xbar);
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
index f7447b2f4d77..55637b6ffccf 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
@@ -8,15 +8,10 @@
 #include <linux/device.h>
 #include <linux/errno.h>
 #include <linux/kernel.h>
-#include <linux/minmax.h>
-#include <linux/regmap.h>
-#include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/types.h>
 
 #include <media/media-entity.h>
-#include <media/mipi-csi2.h>
-#include <media/v4l2-subdev.h>
 
 #include "imx8-isi-core.h"
 
@@ -25,32 +20,18 @@ static inline struct mxc_isi_crossbar *to_isi_crossbar(struct v4l2_subdev *sd)
 	return container_of(sd, struct mxc_isi_crossbar, sd);
 }
 
-/* -----------------------------------------------------------------------------
- * Media block control (i.MX8MN and i.MX8MP only)
- */
-#define GASKET_BASE(n)				(0x0060 + (n) * 0x30)
-
-#define GASKET_CTRL				0x0000
-#define GASKET_CTRL_DATA_TYPE(dt)		((dt) << 8)
-#define GASKET_CTRL_DATA_TYPE_MASK		(0x3f << 8)
-#define GASKET_CTRL_DUAL_COMP_ENABLE		BIT(1)
-#define GASKET_CTRL_ENABLE			BIT(0)
-
-#define GASKET_HSIZE				0x0004
-#define GASKET_VSIZE				0x0008
-
 static int mxc_isi_crossbar_gasket_enable(struct mxc_isi_crossbar *xbar,
 					  struct v4l2_subdev_state *state,
 					  struct v4l2_subdev *remote_sd,
 					  u32 remote_pad, unsigned int port)
 {
 	struct mxc_isi_dev *isi = xbar->isi;
+	const struct mxc_gasket_ops *gasket_ops = isi->pdata->gasket_ops;
 	const struct v4l2_mbus_framefmt *fmt;
 	struct v4l2_mbus_frame_desc fd;
-	u32 val;
 	int ret;
 
-	if (!isi->pdata->has_gasket)
+	if (!gasket_ops)
 		return 0;
 
 	/*
@@ -77,17 +58,7 @@ static int mxc_isi_crossbar_gasket_enable(struct mxc_isi_crossbar *xbar,
 	if (!fmt)
 		return -EINVAL;
 
-	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_HSIZE, fmt->width);
-	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_VSIZE, fmt->height);
-
-	val = GASKET_CTRL_DATA_TYPE(fd.entry[0].bus.csi2.dt)
-	    | GASKET_CTRL_ENABLE;
-
-	if (fd.entry[0].bus.csi2.dt == MIPI_CSI2_DT_YUV422_8B)
-		val |= GASKET_CTRL_DUAL_COMP_ENABLE;
-
-	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_CTRL, val);
-
+	gasket_ops->enable(isi, &fd, fmt, port);
 	return 0;
 }
 
@@ -95,11 +66,12 @@ static void mxc_isi_crossbar_gasket_disable(struct mxc_isi_crossbar *xbar,
 					    unsigned int port)
 {
 	struct mxc_isi_dev *isi = xbar->isi;
+	const struct mxc_gasket_ops *gasket_ops = isi->pdata->gasket_ops;
 
-	if (!isi->pdata->has_gasket)
+	if (!gasket_ops)
 		return;
 
-	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_CTRL, 0);
+	gasket_ops->disable(isi, port);
 }
 
 /* -----------------------------------------------------------------------------
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c
new file mode 100644
index 000000000000..f3758af59db1
--- /dev/null
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019-2023 NXP
+ */
+#include <linux/regmap.h>
+#include <media/mipi-csi2.h>
+
+#include "imx8-isi-core.h"
+
+/* GASKET (i.MX8MN and i.MX8MP only) */
+#define GASKET_BASE(n)				(0x0060 + (n) * 0x30)
+
+#define GASKET_CTRL				0x0000
+#define GASKET_CTRL_DATA_TYPE(dt)		((dt) << 8)
+#define GASKET_CTRL_DATA_TYPE_MASK		(0x3f << 8)
+#define GASKET_CTRL_DUAL_COMP_ENABLE		BIT(1)
+#define GASKET_CTRL_ENABLE			BIT(0)
+
+#define GASKET_HSIZE				0x0004
+#define GASKET_VSIZE				0x0008
+
+/* Configure and enable gasket for i.MX8MN and i.MX8P */
+static void mxc_imx8_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;
+
+	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_HSIZE, fmt->width);
+	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_VSIZE, fmt->height);
+
+	val = GASKET_CTRL_DATA_TYPE(fd->entry[0].bus.csi2.dt);
+	if (fd->entry[0].bus.csi2.dt == MIPI_CSI2_DT_YUV422_8B)
+		val |= GASKET_CTRL_DUAL_COMP_ENABLE;
+
+	val |= GASKET_CTRL_ENABLE;
+	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_CTRL, val);
+}
+
+/* Disable gasket for i.MX8MN and i.MX8P */
+static void mxc_imx8_gasket_disable(struct mxc_isi_dev *isi,
+				    const unsigned int port)
+{
+	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_CTRL, 0);
+}
+
+/* Gasket operations for i.MX8MN and i.MX8MP */
+const struct mxc_gasket_ops mxc_imx8_gasket_ops = {
+	.enable = mxc_imx8_gasket_enable,
+	.disable = mxc_imx8_gasket_disable,
+};
-- 
2.37.1

Re: [PATCH v4 2/3] media: nxp: imx8-isi: move i.MX8 gasket configuration to an ops structure

[Laurent Pinchart]

Hi Guoniu,

Thank you for the patch.

On Wed, Jun 28, 2023 at 02:42:50PM +0800, guoniu.zhou@oss.nxp.com wrote:
> From: "Guoniu.zhou" <guoniu.zhou@nxp.com>
> 
> The i.MX93 includes an ISI instance compatible with the imx8-isi
> driver, but with a different gasket. To prepare for this, make the
> gasket configuration modular by moving the code to an ops structure.
> 
> Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
> ---
>  drivers/media/platform/nxp/imx8-isi/Makefile  |  4 +-
>  .../platform/nxp/imx8-isi/imx8-isi-core.c     |  9 ++--
>  .../platform/nxp/imx8-isi/imx8-isi-core.h     | 20 +++----
>  .../platform/nxp/imx8-isi/imx8-isi-crossbar.c | 40 +++-----------
>  .../platform/nxp/imx8-isi/imx8-isi-gasket.c   | 52 +++++++++++++++++++
>  5 files changed, 74 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx8-isi/Makefile b/drivers/media/platform/nxp/imx8-isi/Makefile
> index 9bff9297686d..4713c4e8b64b 100644
> --- a/drivers/media/platform/nxp/imx8-isi/Makefile
> +++ b/drivers/media/platform/nxp/imx8-isi/Makefile
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  
> -imx8-isi-y := imx8-isi-core.o imx8-isi-crossbar.o imx8-isi-hw.o \
> -	imx8-isi-pipe.o imx8-isi-video.o
> +imx8-isi-y := imx8-isi-core.o imx8-isi-crossbar.o imx8-isi-gasket.o \
> +	imx8-isi-hw.o imx8-isi-pipe.o imx8-isi-video.o
>  imx8-isi-$(CONFIG_DEBUG_FS) += imx8-isi-debug.o
>  imx8-isi-$(CONFIG_VIDEO_IMX8_ISI_M2M) += imx8-isi-m2m.o
>  
> 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 253e77189b69..f2774325dd61 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> @@ -18,9 +18,6 @@
>  #include <linux/string.h>
>  #include <linux/types.h>
>  
> -#include <media/media-device.h>
> -#include <media/v4l2-async.h>
> -#include <media/v4l2-device.h>

Why do you remove those three headers ? The changes below in this file
are unrelated. Did you realize the headers are not needed, or is it a
change included by mistake ?

>  #include <media/v4l2-mc.h>
>  
>  #include "imx8-isi-core.h"
> @@ -289,7 +286,7 @@ static const struct mxc_isi_plat_data mxc_imx8mn_data = {
>  	.clks			= mxc_imx8mn_clks,
>  	.num_clks		= ARRAY_SIZE(mxc_imx8mn_clks),
>  	.buf_active_reverse	= false,
> -	.has_gasket		= true,
> +	.gasket_ops		= &mxc_imx8_gasket_ops,
>  	.has_36bit_dma		= false,
>  };
>  
> @@ -303,7 +300,7 @@ static const struct mxc_isi_plat_data mxc_imx8mp_data = {
>  	.clks			= mxc_imx8mn_clks,
>  	.num_clks		= ARRAY_SIZE(mxc_imx8mn_clks),
>  	.buf_active_reverse	= true,
> -	.has_gasket		= true,
> +	.gasket_ops		= &mxc_imx8_gasket_ops,
>  	.has_36bit_dma		= true,
>  };
>  
> @@ -443,7 +440,7 @@ static int mxc_isi_probe(struct platform_device *pdev)
>  		return PTR_ERR(isi->regs);
>  	}
>  
> -	if (isi->pdata->has_gasket) {
> +	if (isi->pdata->gasket_ops) {
>  		isi->gasket = syscon_regmap_lookup_by_phandle(dev->of_node,
>  							      "fsl,blk-ctrl");
>  		if (IS_ERR(isi->gasket)) {
> 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 e469788a9e6c..87182ebb99f0 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> @@ -10,14 +10,6 @@
>  #ifndef __MXC_ISI_CORE_H__
>  #define __MXC_ISI_CORE_H__
>  
> -#include <linux/list.h>
> -#include <linux/mutex.h>
> -#include <linux/spinlock.h>
> -#include <linux/types.h>
> -#include <linux/videodev2.h>
> -
> -#include <media/media-device.h>
> -#include <media/media-entity.h>

Same question here.

>  #include <media/v4l2-async.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-dev.h>
> @@ -147,6 +139,14 @@ struct mxc_isi_set_thd {
>  	struct mxc_isi_panic_thd panic_set_thd_v;
>  };
>  
> +struct mxc_gasket_ops {
> +	void (*enable)(struct mxc_isi_dev *isi,
> +		       const struct v4l2_mbus_frame_desc *fd,
> +		       const struct v4l2_mbus_framefmt *fmt,
> +		       const unsigned int port);
> +	void (*disable)(struct mxc_isi_dev *isi, const unsigned int port);
> +};
> +
>  enum model {
>  	MXC_ISI_IMX8MN,
>  	MXC_ISI_IMX8MP,
> @@ -159,10 +159,10 @@ struct mxc_isi_plat_data {
>  	unsigned int reg_offset;
>  	const struct mxc_isi_ier_reg  *ier_reg;
>  	const struct mxc_isi_set_thd *set_thd;
> +	const struct mxc_gasket_ops *gasket_ops;
>  	const struct clk_bulk_data *clks;
>  	unsigned int num_clks;
>  	bool buf_active_reverse;
> -	bool has_gasket;
>  	bool has_36bit_dma;
>  };
>  
> @@ -286,6 +286,8 @@ struct mxc_isi_dev {
>  	struct dentry			*debugfs_root;
>  };
>  
> +extern const struct mxc_gasket_ops mxc_imx8_gasket_ops;
> +
>  int mxc_isi_crossbar_init(struct mxc_isi_dev *isi);
>  void mxc_isi_crossbar_cleanup(struct mxc_isi_crossbar *xbar);
>  int mxc_isi_crossbar_register(struct mxc_isi_crossbar *xbar);
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> index f7447b2f4d77..55637b6ffccf 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> @@ -8,15 +8,10 @@
>  #include <linux/device.h>
>  #include <linux/errno.h>
>  #include <linux/kernel.h>
> -#include <linux/minmax.h>
> -#include <linux/regmap.h>
> -#include <linux/slab.h>

And here :-)

>  #include <linux/string.h>
>  #include <linux/types.h>
>  
>  #include <media/media-entity.h>
> -#include <media/mipi-csi2.h>

This is right, as the header is included only for
MIPI_CSI2_DT_YUV422_8B, which is removed below.

> -#include <media/v4l2-subdev.h>

But this doesn't seem right.

>  
>  #include "imx8-isi-core.h"
>  
> @@ -25,32 +20,18 @@ static inline struct mxc_isi_crossbar *to_isi_crossbar(struct v4l2_subdev *sd)
>  	return container_of(sd, struct mxc_isi_crossbar, sd);
>  }
>  
> -/* -----------------------------------------------------------------------------
> - * Media block control (i.MX8MN and i.MX8MP only)
> - */
> -#define GASKET_BASE(n)				(0x0060 + (n) * 0x30)
> -
> -#define GASKET_CTRL				0x0000
> -#define GASKET_CTRL_DATA_TYPE(dt)		((dt) << 8)
> -#define GASKET_CTRL_DATA_TYPE_MASK		(0x3f << 8)
> -#define GASKET_CTRL_DUAL_COMP_ENABLE		BIT(1)
> -#define GASKET_CTRL_ENABLE			BIT(0)
> -
> -#define GASKET_HSIZE				0x0004
> -#define GASKET_VSIZE				0x0008
> -
>  static int mxc_isi_crossbar_gasket_enable(struct mxc_isi_crossbar *xbar,
>  					  struct v4l2_subdev_state *state,
>  					  struct v4l2_subdev *remote_sd,
>  					  u32 remote_pad, unsigned int port)
>  {
>  	struct mxc_isi_dev *isi = xbar->isi;
> +	const struct mxc_gasket_ops *gasket_ops = isi->pdata->gasket_ops;
>  	const struct v4l2_mbus_framefmt *fmt;
>  	struct v4l2_mbus_frame_desc fd;
> -	u32 val;
>  	int ret;
>  
> -	if (!isi->pdata->has_gasket)
> +	if (!gasket_ops)
>  		return 0;
>  
>  	/*
> @@ -77,17 +58,7 @@ static int mxc_isi_crossbar_gasket_enable(struct mxc_isi_crossbar *xbar,
>  	if (!fmt)
>  		return -EINVAL;
>  
> -	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_HSIZE, fmt->width);
> -	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_VSIZE, fmt->height);
> -
> -	val = GASKET_CTRL_DATA_TYPE(fd.entry[0].bus.csi2.dt)
> -	    | GASKET_CTRL_ENABLE;
> -
> -	if (fd.entry[0].bus.csi2.dt == MIPI_CSI2_DT_YUV422_8B)
> -		val |= GASKET_CTRL_DUAL_COMP_ENABLE;
> -
> -	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_CTRL, val);
> -
> +	gasket_ops->enable(isi, &fd, fmt, port);
>  	return 0;
>  }
>  
> @@ -95,11 +66,12 @@ static void mxc_isi_crossbar_gasket_disable(struct mxc_isi_crossbar *xbar,
>  					    unsigned int port)
>  {
>  	struct mxc_isi_dev *isi = xbar->isi;
> +	const struct mxc_gasket_ops *gasket_ops = isi->pdata->gasket_ops;
>  
> -	if (!isi->pdata->has_gasket)
> +	if (!gasket_ops)
>  		return;
>  
> -	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_CTRL, 0);
> +	gasket_ops->disable(isi, port);
>  }
>  
>  /* -----------------------------------------------------------------------------
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c
> new file mode 100644
> index 000000000000..f3758af59db1
> --- /dev/null
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2019-2023 NXP
> + */

A blank line here would be nice, like in the other files.

> +#include <linux/regmap.h>

Here too.

> +#include <media/mipi-csi2.h>
> +
> +#include "imx8-isi-core.h"
> +

Let's use section headers like in the other files, to clearly separate
the i.MX8 and i.MX9 implementations.

/* -----------------------------------------------------------------------------
 * i.MX8MN and i.MX8MP gasket
 */

> +/* GASKET (i.MX8MN and i.MX8MP only) */

Then you can drop this comment, and the ones before the enable and
disable functions.

> +#define GASKET_BASE(n)				(0x0060 + (n) * 0x30)
> +
> +#define GASKET_CTRL				0x0000
> +#define GASKET_CTRL_DATA_TYPE(dt)		((dt) << 8)
> +#define GASKET_CTRL_DATA_TYPE_MASK		(0x3f << 8)
> +#define GASKET_CTRL_DUAL_COMP_ENABLE		BIT(1)
> +#define GASKET_CTRL_ENABLE			BIT(0)
> +
> +#define GASKET_HSIZE				0x0004
> +#define GASKET_VSIZE				0x0008
> +
> +/* Configure and enable gasket for i.MX8MN and i.MX8P */
> +static void mxc_imx8_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;
> +
> +	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_HSIZE, fmt->width);
> +	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_VSIZE, fmt->height);
> +
> +	val = GASKET_CTRL_DATA_TYPE(fd->entry[0].bus.csi2.dt);
> +	if (fd->entry[0].bus.csi2.dt == MIPI_CSI2_DT_YUV422_8B)
> +		val |= GASKET_CTRL_DUAL_COMP_ENABLE;
> +
> +	val |= GASKET_CTRL_ENABLE;
> +	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_CTRL, val);
> +}
> +
> +/* Disable gasket for i.MX8MN and i.MX8P */
> +static void mxc_imx8_gasket_disable(struct mxc_isi_dev *isi,
> +				    const unsigned int port)
> +{
> +	regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_CTRL, 0);
> +}
> +
> +/* Gasket operations for i.MX8MN and i.MX8MP */
> +const struct mxc_gasket_ops mxc_imx8_gasket_ops = {
> +	.enable = mxc_imx8_gasket_enable,
> +	.disable = mxc_imx8_gasket_disable,
> +};

-- 
Regards,

Laurent Pinchart

RE: [PATCH v4 2/3] media: nxp: imx8-isi: move i.MX8 gasket configuration to an ops structure

[G.N. Zhou (OSS)]

Hi Laurent,

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 2023年6月29日 5:14
> To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> Cc: linux-media@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> devicetree@vger.kernel.org; mchehab@kernel.org; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; Xavier Roumegue
> (OSS) <xavier.roumegue@oss.nxp.com>; kernel@pengutronix.de;
> jacopo.mondi@ideasonboard.com; sakari.ailus@linux.intel.com
> Subject: Re: [PATCH v4 2/3] media: nxp: imx8-isi: move i.MX8 gasket
> configuration to an ops structure
> 
> 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 Wed, Jun 28, 2023 at 02:42:50PM +0800, guoniu.zhou@oss.nxp.com wrote:
> > From: "Guoniu.zhou" <guoniu.zhou@nxp.com>
> >
> > The i.MX93 includes an ISI instance compatible with the imx8-isi
> > driver, but with a different gasket. To prepare for this, make the
> > gasket configuration modular by moving the code to an ops structure.
> >
> > Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
> > ---
> >  drivers/media/platform/nxp/imx8-isi/Makefile  |  4 +-
> >  .../platform/nxp/imx8-isi/imx8-isi-core.c     |  9 ++--
> >  .../platform/nxp/imx8-isi/imx8-isi-core.h     | 20 +++----
> >  .../platform/nxp/imx8-isi/imx8-isi-crossbar.c | 40 +++-----------
> >  .../platform/nxp/imx8-isi/imx8-isi-gasket.c   | 52 +++++++++++++++++++
> >  5 files changed, 74 insertions(+), 51 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx8-isi/Makefile
> > b/drivers/media/platform/nxp/imx8-isi/Makefile
> > index 9bff9297686d..4713c4e8b64b 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/Makefile
> > +++ b/drivers/media/platform/nxp/imx8-isi/Makefile
> > @@ -1,7 +1,7 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> >
> > -imx8-isi-y := imx8-isi-core.o imx8-isi-crossbar.o imx8-isi-hw.o \
> > -     imx8-isi-pipe.o imx8-isi-video.o
> > +imx8-isi-y := imx8-isi-core.o imx8-isi-crossbar.o imx8-isi-gasket.o \
> > +     imx8-isi-hw.o imx8-isi-pipe.o imx8-isi-video.o
> >  imx8-isi-$(CONFIG_DEBUG_FS) += imx8-isi-debug.o
> >  imx8-isi-$(CONFIG_VIDEO_IMX8_ISI_M2M) += imx8-isi-m2m.o
> >
> > 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 253e77189b69..f2774325dd61 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > @@ -18,9 +18,6 @@
> >  #include <linux/string.h>
> >  #include <linux/types.h>
> >
> > -#include <media/media-device.h>
> > -#include <media/v4l2-async.h>
> > -#include <media/v4l2-device.h>
> 
> Why do you remove those three headers ? The changes below in this file are
> unrelated. Did you realize the headers are not needed, or is it a change included
> by mistake ?

I remove them since they have been included by "imx8-isi-core.h". Same reason as bellow.
But as you said, they are not related to the bellow changes. I shouldn't include them in this patch.

> 
> >  #include <media/v4l2-mc.h>
> >
> >  #include "imx8-isi-core.h"
> > @@ -289,7 +286,7 @@ static const struct mxc_isi_plat_data
> mxc_imx8mn_data = {
> >       .clks                   = mxc_imx8mn_clks,
> >       .num_clks               = ARRAY_SIZE(mxc_imx8mn_clks),
> >       .buf_active_reverse     = false,
> > -     .has_gasket             = true,
> > +     .gasket_ops             = &mxc_imx8_gasket_ops,
> >       .has_36bit_dma          = false,
> >  };
> >
> > @@ -303,7 +300,7 @@ static const struct mxc_isi_plat_data
> mxc_imx8mp_data = {
> >       .clks                   = mxc_imx8mn_clks,
> >       .num_clks               = ARRAY_SIZE(mxc_imx8mn_clks),
> >       .buf_active_reverse     = true,
> > -     .has_gasket             = true,
> > +     .gasket_ops             = &mxc_imx8_gasket_ops,
> >       .has_36bit_dma          = true,
> >  };
> >
> > @@ -443,7 +440,7 @@ static int mxc_isi_probe(struct platform_device
> *pdev)
> >               return PTR_ERR(isi->regs);
> >       }
> >
> > -     if (isi->pdata->has_gasket) {
> > +     if (isi->pdata->gasket_ops) {
> >               isi->gasket =
> syscon_regmap_lookup_by_phandle(dev->of_node,
> >
> "fsl,blk-ctrl");
> >               if (IS_ERR(isi->gasket)) { 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 e469788a9e6c..87182ebb99f0 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > @@ -10,14 +10,6 @@
> >  #ifndef __MXC_ISI_CORE_H__
> >  #define __MXC_ISI_CORE_H__
> >
> > -#include <linux/list.h>
> > -#include <linux/mutex.h>
> > -#include <linux/spinlock.h>
> > -#include <linux/types.h>
> > -#include <linux/videodev2.h>
> > -
> > -#include <media/media-device.h>
> > -#include <media/media-entity.h>
> 
> Same question here.
> 
> >  #include <media/v4l2-async.h>
> >  #include <media/v4l2-ctrls.h>
> >  #include <media/v4l2-dev.h>
> > @@ -147,6 +139,14 @@ struct mxc_isi_set_thd {
> >       struct mxc_isi_panic_thd panic_set_thd_v;  };
> >
> > +struct mxc_gasket_ops {
> > +     void (*enable)(struct mxc_isi_dev *isi,
> > +                    const struct v4l2_mbus_frame_desc *fd,
> > +                    const struct v4l2_mbus_framefmt *fmt,
> > +                    const unsigned int port);
> > +     void (*disable)(struct mxc_isi_dev *isi, const unsigned int
> > +port); };
> > +
> >  enum model {
> >       MXC_ISI_IMX8MN,
> >       MXC_ISI_IMX8MP,
> > @@ -159,10 +159,10 @@ struct mxc_isi_plat_data {
> >       unsigned int reg_offset;
> >       const struct mxc_isi_ier_reg  *ier_reg;
> >       const struct mxc_isi_set_thd *set_thd;
> > +     const struct mxc_gasket_ops *gasket_ops;
> >       const struct clk_bulk_data *clks;
> >       unsigned int num_clks;
> >       bool buf_active_reverse;
> > -     bool has_gasket;
> >       bool has_36bit_dma;
> >  };
> >
> > @@ -286,6 +286,8 @@ struct mxc_isi_dev {
> >       struct dentry                   *debugfs_root;
> >  };
> >
> > +extern const struct mxc_gasket_ops mxc_imx8_gasket_ops;
> > +
> >  int mxc_isi_crossbar_init(struct mxc_isi_dev *isi);  void
> > mxc_isi_crossbar_cleanup(struct mxc_isi_crossbar *xbar);  int
> > mxc_isi_crossbar_register(struct mxc_isi_crossbar *xbar); diff --git
> > a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> > b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> > index f7447b2f4d77..55637b6ffccf 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> > @@ -8,15 +8,10 @@
> >  #include <linux/device.h>
> >  #include <linux/errno.h>
> >  #include <linux/kernel.h>
> > -#include <linux/minmax.h>
> > -#include <linux/regmap.h>
> > -#include <linux/slab.h>
> 
> And here :-)
> 
> >  #include <linux/string.h>
> >  #include <linux/types.h>
> >
> >  #include <media/media-entity.h>
> > -#include <media/mipi-csi2.h>
> 
> This is right, as the header is included only for MIPI_CSI2_DT_YUV422_8B, which
> is removed below.
> 
> > -#include <media/v4l2-subdev.h>
> 
> But this doesn't seem right.
> 
> >
> >  #include "imx8-isi-core.h"
> >
> > @@ -25,32 +20,18 @@ static inline struct mxc_isi_crossbar
> *to_isi_crossbar(struct v4l2_subdev *sd)
> >       return container_of(sd, struct mxc_isi_crossbar, sd);  }
> >
> > -/*
> > ----------------------------------------------------------------------
> > -------
> > - * Media block control (i.MX8MN and i.MX8MP only)
> > - */
> > -#define GASKET_BASE(n)                               (0x0060 + (n) *
> 0x30)
> > -
> > -#define GASKET_CTRL                          0x0000
> > -#define GASKET_CTRL_DATA_TYPE(dt)            ((dt) << 8)
> > -#define GASKET_CTRL_DATA_TYPE_MASK           (0x3f << 8)
> > -#define GASKET_CTRL_DUAL_COMP_ENABLE         BIT(1)
> > -#define GASKET_CTRL_ENABLE                   BIT(0)
> > -
> > -#define GASKET_HSIZE                         0x0004
> > -#define GASKET_VSIZE                         0x0008
> > -
> >  static int mxc_isi_crossbar_gasket_enable(struct mxc_isi_crossbar *xbar,
> >                                         struct v4l2_subdev_state
> *state,
> >                                         struct v4l2_subdev
> *remote_sd,
> >                                         u32 remote_pad, unsigned int
> > port)  {
> >       struct mxc_isi_dev *isi = xbar->isi;
> > +     const struct mxc_gasket_ops *gasket_ops =
> > + isi->pdata->gasket_ops;
> >       const struct v4l2_mbus_framefmt *fmt;
> >       struct v4l2_mbus_frame_desc fd;
> > -     u32 val;
> >       int ret;
> >
> > -     if (!isi->pdata->has_gasket)
> > +     if (!gasket_ops)
> >               return 0;
> >
> >       /*
> > @@ -77,17 +58,7 @@ static int mxc_isi_crossbar_gasket_enable(struct
> mxc_isi_crossbar *xbar,
> >       if (!fmt)
> >               return -EINVAL;
> >
> > -     regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_HSIZE,
> fmt->width);
> > -     regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_VSIZE,
> fmt->height);
> > -
> > -     val = GASKET_CTRL_DATA_TYPE(fd.entry[0].bus.csi2.dt)
> > -         | GASKET_CTRL_ENABLE;
> > -
> > -     if (fd.entry[0].bus.csi2.dt == MIPI_CSI2_DT_YUV422_8B)
> > -             val |= GASKET_CTRL_DUAL_COMP_ENABLE;
> > -
> > -     regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_CTRL, val);
> > -
> > +     gasket_ops->enable(isi, &fd, fmt, port);
> >       return 0;
> >  }
> >
> > @@ -95,11 +66,12 @@ static void mxc_isi_crossbar_gasket_disable(struct
> mxc_isi_crossbar *xbar,
> >                                           unsigned int port)  {
> >       struct mxc_isi_dev *isi = xbar->isi;
> > +     const struct mxc_gasket_ops *gasket_ops =
> > + isi->pdata->gasket_ops;
> >
> > -     if (!isi->pdata->has_gasket)
> > +     if (!gasket_ops)
> >               return;
> >
> > -     regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_CTRL, 0);
> > +     gasket_ops->disable(isi, port);
> >  }
> >
> >  /* -----------------------------------------------------------------------------
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c
> b/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c
> > new file mode 100644
> > index 000000000000..f3758af59db1
> > --- /dev/null
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c
> > @@ -0,0 +1,52 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2019-2023 NXP
> > + */
> 
> A blank line here would be nice, like in the other files.
> 
> > +#include <linux/regmap.h>
> 
> Here too.
> 
> > +#include <media/mipi-csi2.h>
> > +
> > +#include "imx8-isi-core.h"
> > +
> 
> Let's use section headers like in the other files, to clearly separate
> the i.MX8 and i.MX9 implementations.
> 
> /* -----------------------------------------------------------------------------
>  * i.MX8MN and i.MX8MP gasket
>  */
> 
> > +/* GASKET (i.MX8MN and i.MX8MP only) */
> 
> Then you can drop this comment, and the ones before the enable and
> disable functions.
> 
> > +#define GASKET_BASE(n)                               (0x0060 + (n) *
> 0x30)
> > +
> > +#define GASKET_CTRL                          0x0000
> > +#define GASKET_CTRL_DATA_TYPE(dt)            ((dt) << 8)
> > +#define GASKET_CTRL_DATA_TYPE_MASK           (0x3f << 8)
> > +#define GASKET_CTRL_DUAL_COMP_ENABLE         BIT(1)
> > +#define GASKET_CTRL_ENABLE                   BIT(0)
> > +
> > +#define GASKET_HSIZE                         0x0004
> > +#define GASKET_VSIZE                         0x0008
> > +
> > +/* Configure and enable gasket for i.MX8MN and i.MX8P */
> > +static void mxc_imx8_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;
> > +
> > +     regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_HSIZE,
> fmt->width);
> > +     regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_VSIZE,
> fmt->height);
> > +
> > +     val = GASKET_CTRL_DATA_TYPE(fd->entry[0].bus.csi2.dt);
> > +     if (fd->entry[0].bus.csi2.dt == MIPI_CSI2_DT_YUV422_8B)
> > +             val |= GASKET_CTRL_DUAL_COMP_ENABLE;
> > +
> > +     val |= GASKET_CTRL_ENABLE;
> > +     regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_CTRL, val);
> > +}
> > +
> > +/* Disable gasket for i.MX8MN and i.MX8P */
> > +static void mxc_imx8_gasket_disable(struct mxc_isi_dev *isi,
> > +                                 const unsigned int port)
> > +{
> > +     regmap_write(isi->gasket, GASKET_BASE(port) + GASKET_CTRL, 0);
> > +}
> > +
> > +/* Gasket operations for i.MX8MN and i.MX8MP */
> > +const struct mxc_gasket_ops mxc_imx8_gasket_ops = {
> > +     .enable = mxc_imx8_gasket_enable,
> > +     .disable = mxc_imx8_gasket_disable,
> > +};
> 
> --
> Regards,
> 
> Laurent Pinchart

[PATCH v4 3/3] media: nxp: imx8-isi: add ISI support for i.MX93

[guoniu.zhou]

From: "Guoniu.zhou" <guoniu.zhou@nxp.com>

i.MX93 use a different gasket which has different register definition
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@nxp.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../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   | 28 +++++++++++++++++++
 3 files changed, 45 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 f2774325dd61..13a0225de634 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
@@ -304,6 +304,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
  */
@@ -515,6 +529,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 87182ebb99f0..ddd3d8ce4000 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
@@ -150,6 +150,7 @@ struct mxc_gasket_ops {
 enum model {
 	MXC_ISI_IMX8MN,
 	MXC_ISI_IMX8MP,
+	MXC_ISI_IMX93,
 };
 
 struct mxc_isi_plat_data {
@@ -287,6 +288,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 f3758af59db1..96f36a6d8a3c 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c
@@ -19,6 +19,11 @@
 #define GASKET_HSIZE				0x0004
 #define GASKET_VSIZE				0x0008
 
+/* dispmix_GPR register (i.MX93 only) */
+#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)
+
 /* Configure and enable gasket for i.MX8MN and i.MX8P */
 static void mxc_imx8_gasket_enable(struct mxc_isi_dev *isi,
 				   const struct v4l2_mbus_frame_desc *fd,
@@ -50,3 +55,26 @@ const struct mxc_gasket_ops mxc_imx8_gasket_ops = {
 	.enable = mxc_imx8_gasket_enable,
 	.disable = mxc_imx8_gasket_disable,
 };
+
+/* Configure and enable gasket for i.MX93 */
+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,
+};
-- 
2.37.1

Re: [PATCH v4 3/3] media: nxp: imx8-isi: add ISI support for i.MX93

[Laurent Pinchart]

On Wed, Jun 28, 2023 at 02:42:51PM +0800, guoniu.zhou@oss.nxp.com wrote:
> From: "Guoniu.zhou" <guoniu.zhou@nxp.com>
> 
> i.MX93 use a different gasket which has different register definition
> 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@nxp.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../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   | 28 +++++++++++++++++++
>  3 files changed, 45 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 f2774325dd61..13a0225de634 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> @@ -304,6 +304,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
>   */
> @@ -515,6 +529,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 87182ebb99f0..ddd3d8ce4000 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> @@ -150,6 +150,7 @@ struct mxc_gasket_ops {
>  enum model {
>  	MXC_ISI_IMX8MN,
>  	MXC_ISI_IMX8MP,
> +	MXC_ISI_IMX93,
>  };
>  
>  struct mxc_isi_plat_data {
> @@ -287,6 +288,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 f3758af59db1..96f36a6d8a3c 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-gasket.c
> @@ -19,6 +19,11 @@
>  #define GASKET_HSIZE				0x0004
>  #define GASKET_VSIZE				0x0008
>  
> +/* dispmix_GPR register (i.MX93 only) */
> +#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)
> +
>  /* Configure and enable gasket for i.MX8MN and i.MX8P */
>  static void mxc_imx8_gasket_enable(struct mxc_isi_dev *isi,
>  				   const struct v4l2_mbus_frame_desc *fd,
> @@ -50,3 +55,26 @@ const struct mxc_gasket_ops mxc_imx8_gasket_ops = {
>  	.enable = mxc_imx8_gasket_enable,
>  	.disable = mxc_imx8_gasket_disable,
>  };

Following the comment on 2/3, let's add

/* -----------------------------------------------------------------------------
 * i.MX93 gasket
 */

, move the three DISP_MIX_CAMERA_MUX* macros here, and drop the comments
before the enable and disable functions.

> +
> +/* Configure and enable gasket for i.MX93 */
> +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);
> +}

Missing blank line.

> +const struct mxc_gasket_ops mxc_imx93_gasket_ops = {
> +	.enable = mxc_imx93_gasket_enable,
> +	.disable = mxc_imx93_gasket_disable,
> +};

-- 
Regards,

Laurent Pinchart