[PATCH v2 1/2] media: imx: imx7-media-csi: add support for imx8mq

[PATCH v2 1/2] media: imx: imx7-media-csi: add support for imx8mq

[Martin Kepplinger]

Modeled after the NXP driver mx6s_capture.c that this driver is based on,
imx8mq needs different settings for the baseaddr_switch mechanism. Define
the needed bits and set that for imx8mq.

Without these settings, the system will "sometimes" hang completely when
starting to stream (the interrupt will never be called).

Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
---

revision history
----------------
v2: (thank you Rui and Laurent)
 * rename function and enum
 * remove unrealted newline
 * add Laurents reviewed tag to the bindings patch

v1:
https://lore.kernel.org/linux-media/20211117092710.3084034-1-martin.kepplinger@puri.sm/T/#t



 drivers/staging/media/imx/imx7-media-csi.c | 32 ++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 2288dadb2683..1f3d9e27270d 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -12,6 +12,7 @@
 #include <linux/interrupt.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/of_graph.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
@@ -122,6 +123,10 @@
 #define BIT_DATA_FROM_MIPI		BIT(22)
 #define BIT_MIPI_YU_SWAP		BIT(21)
 #define BIT_MIPI_DOUBLE_CMPNT		BIT(20)
+#define BIT_MASK_OPTION_FIRST_FRAME	(0 << 18)
+#define BIT_MASK_OPTION_CSI_EN		(1 << 18)
+#define BIT_MASK_OPTION_SECOND_FRAME	(2 << 18)
+#define BIT_MASK_OPTION_ON_DATA		(3 << 18)
 #define BIT_BASEADDR_CHG_ERR_EN		BIT(9)
 #define BIT_BASEADDR_SWITCH_SEL		BIT(5)
 #define BIT_BASEADDR_SWITCH_EN		BIT(4)
@@ -154,6 +159,11 @@
 #define CSI_CSICR18			0x48
 #define CSI_CSICR19			0x4c
 
+enum imx_csi_model {
+	IMX7_CSI_IMX7 = 0,
+	IMX7_CSI_IMX8MQ,
+};
+
 struct imx7_csi {
 	struct device *dev;
 	struct v4l2_subdev sd;
@@ -189,6 +199,8 @@ struct imx7_csi {
 	bool is_csi2;
 
 	struct completion last_eof_completion;
+
+	enum imx_csi_model model;
 };
 
 static struct imx7_csi *
@@ -537,6 +549,16 @@ static void imx7_csi_deinit(struct imx7_csi *csi,
 	clk_disable_unprepare(csi->mclk);
 }
 
+static void imx7_csi_baseaddr_switch_on_second_frame(struct imx7_csi *csi)
+{
+	u32 cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
+
+	cr18 |= BIT_BASEADDR_SWITCH_EN | BIT_BASEADDR_SWITCH_SEL |
+		BIT_BASEADDR_CHG_ERR_EN;
+	cr18 |= BIT_MASK_OPTION_SECOND_FRAME;
+	imx7_csi_reg_write(csi, cr18, CSI_CSICR18);
+}
+
 static void imx7_csi_enable(struct imx7_csi *csi)
 {
 	/* Clear the Rx FIFO and reflash the DMA controller. */
@@ -552,6 +574,9 @@ static void imx7_csi_enable(struct imx7_csi *csi)
 	/* Enable the RxFIFO DMA and the CSI. */
 	imx7_csi_dmareq_rff_enable(csi);
 	imx7_csi_hw_enable(csi);
+
+	if (csi->model == IMX7_CSI_IMX8MQ)
+		imx7_csi_baseaddr_switch_on_second_frame(csi);
 }
 
 static void imx7_csi_disable(struct imx7_csi *csi)
@@ -1155,6 +1180,8 @@ static int imx7_csi_probe(struct platform_device *pdev)
 	if (IS_ERR(csi->regbase))
 		return PTR_ERR(csi->regbase);
 
+	csi->model = (enum imx_csi_model)of_device_get_match_data(&pdev->dev);
+
 	spin_lock_init(&csi->irqlock);
 	mutex_init(&csi->lock);
 
@@ -1249,8 +1276,9 @@ static int imx7_csi_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id imx7_csi_of_match[] = {
-	{ .compatible = "fsl,imx7-csi" },
-	{ .compatible = "fsl,imx6ul-csi" },
+	{ .compatible = "fsl,imx8mq-csi", .data = (void *)IMX7_CSI_IMX8MQ },
+	{ .compatible = "fsl,imx7-csi", .data = (void *)IMX7_CSI_IMX7 },
+	{ .compatible = "fsl,imx6ul-csi", .data = (void *)IMX7_CSI_IMX7 },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, imx7_csi_of_match);
-- 
2.30.2

[PATCH v2 2/2] dt-bindings: media: document imx8mq support for imx7-csi

[Martin Kepplinger]

Add the fsl,imx8mq-csi compatible string to the bindings for nxp,imx7-csi.
The i.MX8MQ SoC contains the same CSI bridge controller as the i.MX7.

Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml b/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
index 5922a2795167..4f7b78265336 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
@@ -17,6 +17,7 @@ properties:
   compatible:
     oneOf:
       - enum:
+          - fsl,imx8mq-csi
           - fsl,imx7-csi
           - fsl,imx6ul-csi
       - items:
-- 
2.30.2

Re: [PATCH v2 1/2] media: imx: imx7-media-csi: add support for imx8mq

[Rui Miguel Silva]

Hi Martin,
Thanks for this version.

On Thu Nov 18, 2021 at 6:33 AM WET, Martin Kepplinger wrote:

> Modeled after the NXP driver mx6s_capture.c that this driver is based on,
> imx8mq needs different settings for the baseaddr_switch mechanism. Define
> the needed bits and set that for imx8mq.
>
> Without these settings, the system will "sometimes" hang completely when
> starting to stream (the interrupt will never be called).
>
> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>

LGTM

Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>

------
Cheers,
     Rui
> ---
>
> revision history
> ----------------
> v2: (thank you Rui and Laurent)
>  * rename function and enum
>  * remove unrealted newline
>  * add Laurents reviewed tag to the bindings patch
>
> v1:
> https://lore.kernel.org/linux-media/20211117092710.3084034-1-martin.kepplinger@puri.sm/T/#t
>
>
>
>  drivers/staging/media/imx/imx7-media-csi.c | 32 ++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> index 2288dadb2683..1f3d9e27270d 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -12,6 +12,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/module.h>
> +#include <linux/of_device.h>
>  #include <linux/of_graph.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/platform_device.h>
> @@ -122,6 +123,10 @@
>  #define BIT_DATA_FROM_MIPI		BIT(22)
>  #define BIT_MIPI_YU_SWAP		BIT(21)
>  #define BIT_MIPI_DOUBLE_CMPNT		BIT(20)
> +#define BIT_MASK_OPTION_FIRST_FRAME	(0 << 18)
> +#define BIT_MASK_OPTION_CSI_EN		(1 << 18)
> +#define BIT_MASK_OPTION_SECOND_FRAME	(2 << 18)
> +#define BIT_MASK_OPTION_ON_DATA		(3 << 18)
>  #define BIT_BASEADDR_CHG_ERR_EN		BIT(9)
>  #define BIT_BASEADDR_SWITCH_SEL		BIT(5)
>  #define BIT_BASEADDR_SWITCH_EN		BIT(4)
> @@ -154,6 +159,11 @@
>  #define CSI_CSICR18			0x48
>  #define CSI_CSICR19			0x4c
>  
> +enum imx_csi_model {
> +	IMX7_CSI_IMX7 = 0,
> +	IMX7_CSI_IMX8MQ,
> +};
> +
>  struct imx7_csi {
>  	struct device *dev;
>  	struct v4l2_subdev sd;
> @@ -189,6 +199,8 @@ struct imx7_csi {
>  	bool is_csi2;
>  
>  	struct completion last_eof_completion;
> +
> +	enum imx_csi_model model;
>  };
>  
>  static struct imx7_csi *
> @@ -537,6 +549,16 @@ static void imx7_csi_deinit(struct imx7_csi *csi,
>  	clk_disable_unprepare(csi->mclk);
>  }
>  
> +static void imx7_csi_baseaddr_switch_on_second_frame(struct imx7_csi *csi)
> +{
> +	u32 cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
> +
> +	cr18 |= BIT_BASEADDR_SWITCH_EN | BIT_BASEADDR_SWITCH_SEL |
> +		BIT_BASEADDR_CHG_ERR_EN;
> +	cr18 |= BIT_MASK_OPTION_SECOND_FRAME;
> +	imx7_csi_reg_write(csi, cr18, CSI_CSICR18);
> +}
> +
>  static void imx7_csi_enable(struct imx7_csi *csi)
>  {
>  	/* Clear the Rx FIFO and reflash the DMA controller. */
> @@ -552,6 +574,9 @@ static void imx7_csi_enable(struct imx7_csi *csi)
>  	/* Enable the RxFIFO DMA and the CSI. */
>  	imx7_csi_dmareq_rff_enable(csi);
>  	imx7_csi_hw_enable(csi);
> +
> +	if (csi->model == IMX7_CSI_IMX8MQ)
> +		imx7_csi_baseaddr_switch_on_second_frame(csi);
>  }
>  
>  static void imx7_csi_disable(struct imx7_csi *csi)
> @@ -1155,6 +1180,8 @@ static int imx7_csi_probe(struct platform_device *pdev)
>  	if (IS_ERR(csi->regbase))
>  		return PTR_ERR(csi->regbase);
>  
> +	csi->model = (enum imx_csi_model)of_device_get_match_data(&pdev->dev);
> +
>  	spin_lock_init(&csi->irqlock);
>  	mutex_init(&csi->lock);
>  
> @@ -1249,8 +1276,9 @@ static int imx7_csi_remove(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id imx7_csi_of_match[] = {
> -	{ .compatible = "fsl,imx7-csi" },
> -	{ .compatible = "fsl,imx6ul-csi" },
> +	{ .compatible = "fsl,imx8mq-csi", .data = (void *)IMX7_CSI_IMX8MQ },
> +	{ .compatible = "fsl,imx7-csi", .data = (void *)IMX7_CSI_IMX7 },
> +	{ .compatible = "fsl,imx6ul-csi", .data = (void *)IMX7_CSI_IMX7 },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, imx7_csi_of_match);
> -- 
> 2.30.2

Re: [PATCH v2 1/2] media: imx: imx7-media-csi: add support for imx8mq

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5316 bytes --]

Hi Martin,

I love your patch! Perhaps something to improve:

[auto build test WARNING on media-tree/master]
[also build test WARNING on v5.16-rc1 next-20211118]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Martin-Kepplinger/media-imx-imx7-media-csi-add-support-for-imx8mq/20211118-143635
base:   git://linuxtv.org/media_tree.git master
config: arm64-randconfig-r003-20211118 (attached as .config)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/0day-ci/linux/commit/053ee9aefb4758625038e03df68bb468abf0cd4a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Martin-Kepplinger/media-imx-imx7-media-csi-add-support-for-imx8mq/20211118-143635
        git checkout 053ee9aefb4758625038e03df68bb468abf0cd4a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/staging/media/imx/imx7-media-csi.c:1183:15: warning: cast to smaller integer type 'enum imx_csi_model' from 'const void *' [-Wvoid-pointer-to-enum-cast]
           csi->model = (enum imx_csi_model)of_device_get_match_data(&pdev->dev);
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +1183 drivers/staging/media/imx/imx7-media-csi.c

  1153	
  1154	static int imx7_csi_probe(struct platform_device *pdev)
  1155	{
  1156		struct device *dev = &pdev->dev;
  1157		struct device_node *node = dev->of_node;
  1158		struct imx_media_dev *imxmd;
  1159		struct imx7_csi *csi;
  1160		int i, ret;
  1161	
  1162		csi = devm_kzalloc(&pdev->dev, sizeof(*csi), GFP_KERNEL);
  1163		if (!csi)
  1164			return -ENOMEM;
  1165	
  1166		csi->dev = dev;
  1167	
  1168		csi->mclk = devm_clk_get(&pdev->dev, "mclk");
  1169		if (IS_ERR(csi->mclk)) {
  1170			ret = PTR_ERR(csi->mclk);
  1171			dev_err(dev, "Failed to get mclk: %d", ret);
  1172			return ret;
  1173		}
  1174	
  1175		csi->irq = platform_get_irq(pdev, 0);
  1176		if (csi->irq < 0)
  1177			return csi->irq;
  1178	
  1179		csi->regbase = devm_platform_ioremap_resource(pdev, 0);
  1180		if (IS_ERR(csi->regbase))
  1181			return PTR_ERR(csi->regbase);
  1182	
> 1183		csi->model = (enum imx_csi_model)of_device_get_match_data(&pdev->dev);
  1184	
  1185		spin_lock_init(&csi->irqlock);
  1186		mutex_init(&csi->lock);
  1187	
  1188		/* install interrupt handler */
  1189		ret = devm_request_irq(dev, csi->irq, imx7_csi_irq_handler, 0, "csi",
  1190				       (void *)csi);
  1191		if (ret < 0) {
  1192			dev_err(dev, "Request CSI IRQ failed.\n");
  1193			goto destroy_mutex;
  1194		}
  1195	
  1196		/* add media device */
  1197		imxmd = imx_media_dev_init(dev, NULL);
  1198		if (IS_ERR(imxmd)) {
  1199			ret = PTR_ERR(imxmd);
  1200			goto destroy_mutex;
  1201		}
  1202		platform_set_drvdata(pdev, &csi->sd);
  1203	
  1204		ret = imx_media_of_add_csi(imxmd, node);
  1205		if (ret < 0 && ret != -ENODEV && ret != -EEXIST)
  1206			goto cleanup;
  1207	
  1208		ret = imx_media_dev_notifier_register(imxmd, NULL);
  1209		if (ret < 0)
  1210			goto cleanup;
  1211	
  1212		csi->imxmd = imxmd;
  1213		v4l2_subdev_init(&csi->sd, &imx7_csi_subdev_ops);
  1214		v4l2_set_subdevdata(&csi->sd, csi);
  1215		csi->sd.internal_ops = &imx7_csi_internal_ops;
  1216		csi->sd.entity.ops = &imx7_csi_entity_ops;
  1217		csi->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
  1218		csi->sd.dev = &pdev->dev;
  1219		csi->sd.owner = THIS_MODULE;
  1220		csi->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
  1221		csi->sd.grp_id = IMX_MEDIA_GRP_ID_CSI;
  1222		snprintf(csi->sd.name, sizeof(csi->sd.name), "csi");
  1223	
  1224		for (i = 0; i < IMX7_CSI_PADS_NUM; i++)
  1225			csi->pad[i].flags = (i == IMX7_CSI_PAD_SINK) ?
  1226				MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
  1227	
  1228		ret = media_entity_pads_init(&csi->sd.entity, IMX7_CSI_PADS_NUM,
  1229					     csi->pad);
  1230		if (ret < 0)
  1231			goto cleanup;
  1232	
  1233		ret = imx7_csi_async_register(csi);
  1234		if (ret)
  1235			goto subdev_notifier_cleanup;
  1236	
  1237		return 0;
  1238	
  1239	subdev_notifier_cleanup:
  1240		v4l2_async_nf_unregister(&csi->notifier);
  1241		v4l2_async_nf_cleanup(&csi->notifier);
  1242	
  1243	cleanup:
  1244		v4l2_async_nf_unregister(&imxmd->notifier);
  1245		v4l2_async_nf_cleanup(&imxmd->notifier);
  1246		v4l2_device_unregister(&imxmd->v4l2_dev);
  1247		media_device_unregister(&imxmd->md);
  1248		media_device_cleanup(&imxmd->md);
  1249	
  1250	destroy_mutex:
  1251		mutex_destroy(&csi->lock);
  1252	
  1253		return ret;
  1254	}
  1255	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 42207 bytes --]