| Message ID | 20211118063347.3370678-1-martin.kepplinger@puri.sm (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] media: imx: imx7-media-csi: add support for imx8mq | expand |
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
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
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);
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(-)