| Message ID | 1349735823-30315-1-git-send-email-festevam@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from <linux-media-owner@vger.kernel.org>) id 1TLLws-00082K-Ic for patchwork@linuxtv.org; Tue, 09 Oct 2012 00:37:18 +0200 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.75/mailfrontend-4) with esmtp for <patchwork@linuxtv.org> id 1TLLwr-0001zj-Bw; Tue, 09 Oct 2012 00:37:18 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755358Ab2JHWhP (ORCPT <rfc822;patchwork@linuxtv.org>); Mon, 8 Oct 2012 18:37:15 -0400 Received: from mail-yh0-f46.google.com ([209.85.213.46]:33939 "EHLO mail-yh0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755218Ab2JHWhM (ORCPT <rfc822; linux-media@vger.kernel.org>); Mon, 8 Oct 2012 18:37:12 -0400 Received: by mail-yh0-f46.google.com with SMTP id m54so1082457yhm.19 for <linux-media@vger.kernel.org>; Mon, 08 Oct 2012 15:37:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer; bh=LXd5B2bZLgQukhnhtc89toDHWMKhYvQN+epHyWkhu0o=; b=oKMZGYx07QSiAehdvOX1O8udKScZufeczBFhre/DrO2efUjDge2WzdsqJh0IkVKFlp wlksWrbORfks8tubgWzt4SI3zzXnYeKhrtpX5iBL981wQZ4rziWxAD4l2px4oNi0jLyf q3cteD8srcPztdRKzPG2rx4BHYaQuSck2LzfpIXSMSh0hpzYslUIiLVQ4QOTpyjVwZKE Qgv3AK3QWtDeJYwcPo/GmA3ie8wPfgB9CKh7K7xDIdbB/IQnXqNENOST42oqAYZCi9R8 dJDH7YaNI6IBWJHcrpjeS8c0fmeG+6zU21y3SkI7bTBYYKOyuh6LRWSjG2U3oOwOHvwf h/jA== Received: by 10.236.114.200 with SMTP id c48mr17272109yhh.118.1349735831886; Mon, 08 Oct 2012 15:37:11 -0700 (PDT) Received: from fabio-Latitude-E6410.cps.virtua.com.br ([201.82.136.72]) by mx.google.com with ESMTPS id b46sm27234690yhn.5.2012.10.08.15.37.09 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 08 Oct 2012 15:37:11 -0700 (PDT) From: Fabio Estevam <festevam@gmail.com> To: g.liakhovetski@gmx.de Cc: mchehab@infradead.org, javier.martin@vista-silicon.com, kernel@pengutronix.de, gcembed@gmail.com, linux-media@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Fabio Estevam <fabio.estevam@freescale.com> Subject: [PATCH v2 2/2] [media]: mx2_camera: Fix regression caused by clock conversion Date: Mon, 8 Oct 2012 19:37:03 -0300 Message-Id: <1349735823-30315-1-git-send-email-festevam@gmail.com> X-Mailer: git-send-email 1.7.9.5 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 5.6.1.2065439, Antispam-Engine: 2.7.2.376379, Antispam-Data: 2012.10.8.222720 X-PMX-Spam: Gauge=IIIIIIIII, Probability=9%, Report=' FORGED_FROM_GMAIL 0.1, MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODY_SIZE_4000_4999 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, DKIM_SIGNATURE 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CP_URI_IN_BODY 0, __FRAUD_WEBMAIL 0, __FRAUD_WEBMAIL_FROM 0, __FROM_GMAIL 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __MIME_TEXT_ONLY 0, __MULTIPLE_RCPTS_CC_X2 0, __PHISH_SPEAR_STRUCTURE_1 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_NO_WWW 0, __URI_NS ' |
Commit Message
Fabio Estevam
Oct. 8, 2012, 10:37 p.m. UTC
From: Fabio Estevam <fabio.estevam@freescale.com> Since mx27 transitioned to the commmon clock framework in 3.5, the correct way to acquire the csi clock is to get csi_ahb and csi_per clocks separately. By not doing so the camera sensor does not probe correctly: soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0 mx2-camera mx2-camera.0: Camera driver attached to camera 0 ov2640 0-0030: Product ID error fb:fb mx2-camera mx2-camera.0: Camera driver detached from camera 0 mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 66500000 Adapt the mx2_camera driver to the new clock framework and make it functional again. Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> --- Changes since v1: - Rebased against linux-next 20121008. drivers/media/platform/soc_camera/mx2_camera.c | 47 +++++++++++++++++------- 1 file changed, 34 insertions(+), 13 deletions(-)
Comments
On 9 October 2012 00:37, Fabio Estevam <festevam@gmail.com> wrote: > From: Fabio Estevam <fabio.estevam@freescale.com> > > Since mx27 transitioned to the commmon clock framework in 3.5, the correct way > to acquire the csi clock is to get csi_ahb and csi_per clocks separately. > > By not doing so the camera sensor does not probe correctly: > > soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0 > mx2-camera mx2-camera.0: Camera driver attached to camera 0 > ov2640 0-0030: Product ID error fb:fb > mx2-camera mx2-camera.0: Camera driver detached from camera 0 > mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 66500000 > > Adapt the mx2_camera driver to the new clock framework and make it functional > again. > > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> > --- > Changes since v1: > - Rebased against linux-next 20121008. > > drivers/media/platform/soc_camera/mx2_camera.c | 47 +++++++++++++++++------- > 1 file changed, 34 insertions(+), 13 deletions(-) > > diff --git a/drivers/media/platform/soc_camera/mx2_camera.c b/drivers/media/platform/soc_camera/mx2_camera.c > index 403d7f1..9f8c5f0 100644 > --- a/drivers/media/platform/soc_camera/mx2_camera.c > +++ b/drivers/media/platform/soc_camera/mx2_camera.c > @@ -272,7 +272,8 @@ struct mx2_camera_dev { > struct device *dev; > struct soc_camera_host soc_host; > struct soc_camera_device *icd; > - struct clk *clk_csi, *clk_emma_ahb, *clk_emma_ipg; > + struct clk *clk_emma_ahb, *clk_emma_ipg; > + struct clk *clk_csi_ahb, *clk_csi_per; > > void __iomem *base_csi, *base_emma; > > @@ -432,7 +433,8 @@ static void mx2_camera_deactivate(struct mx2_camera_dev *pcdev) > { > unsigned long flags; > > - clk_disable_unprepare(pcdev->clk_csi); > + clk_disable_unprepare(pcdev->clk_csi_ahb); > + clk_disable_unprepare(pcdev->clk_csi_per); > writel(0, pcdev->base_csi + CSICR1); > if (cpu_is_mx27()) { > writel(0, pcdev->base_emma + PRP_CNTL); > @@ -460,7 +462,11 @@ static int mx2_camera_add_device(struct soc_camera_device *icd) > if (pcdev->icd) > return -EBUSY; > > - ret = clk_prepare_enable(pcdev->clk_csi); > + ret = clk_prepare_enable(pcdev->clk_csi_ahb); > + if (ret < 0) > + return ret; > + > + ret = clk_prepare_enable(pcdev->clk_csi_per); > if (ret < 0) > return ret; > > @@ -1725,11 +1731,18 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev) > goto exit; > } > > - pcdev->clk_csi = devm_clk_get(&pdev->dev, "ahb"); > - if (IS_ERR(pcdev->clk_csi)) { > - dev_err(&pdev->dev, "Could not get csi clock\n"); > - err = PTR_ERR(pcdev->clk_csi); > - goto exit; > + pcdev->clk_csi_ahb = devm_clk_get(&pdev->dev, "ahb"); > + if (IS_ERR(pcdev->clk_csi_ahb)) { > + dev_err(&pdev->dev, "Could not get csi ahb clock\n"); > + err = PTR_ERR(pcdev->clk_csi_ahb); > + goto exit; > + } > + > + pcdev->clk_csi_per = devm_clk_get(&pdev->dev, "per"); > + if (IS_ERR(pcdev->clk_csi_per)) { > + dev_err(&pdev->dev, "Could not get csi per clock\n"); > + err = PTR_ERR(pcdev->clk_csi_per); > + goto exit_csi_ahb; > } > > pcdev->pdata = pdev->dev.platform_data; > @@ -1738,14 +1751,15 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev) > > pcdev->platform_flags = pcdev->pdata->flags; > > - rate = clk_round_rate(pcdev->clk_csi, pcdev->pdata->clk * 2); > + rate = clk_round_rate(pcdev->clk_csi_per, > + pcdev->pdata->clk * 2); > if (rate <= 0) { > err = -ENODEV; > - goto exit; > + goto exit_csi_per; > } > - err = clk_set_rate(pcdev->clk_csi, rate); > + err = clk_set_rate(pcdev->clk_csi_per, rate); > if (err < 0) > - goto exit; > + goto exit_csi_per; > } > > INIT_LIST_HEAD(&pcdev->capture); > @@ -1801,7 +1815,7 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev) > goto exit_free_emma; > > dev_info(&pdev->dev, "MX2 Camera (CSI) driver probed, clock frequency: %ld\n", > - clk_get_rate(pcdev->clk_csi)); > + clk_get_rate(pcdev->clk_csi_per)); > > return 0; > > @@ -1812,6 +1826,10 @@ eallocctx: > clk_disable_unprepare(pcdev->clk_emma_ipg); > clk_disable_unprepare(pcdev->clk_emma_ahb); > } > +exit_csi_per: > + clk_disable_unprepare(pcdev->clk_csi_per); > +exit_csi_ahb: > + clk_disable_unprepare(pcdev->clk_csi_ahb); > exit: > return err; > } > @@ -1831,6 +1849,9 @@ static int __devexit mx2_camera_remove(struct platform_device *pdev) > clk_disable_unprepare(pcdev->clk_emma_ahb); > } > > + clk_disable_unprepare(pcdev->clk_csi_per); > + clk_disable_unprepare(pcdev->clk_csi_ahb); > + > dev_info(&pdev->dev, "MX2 Camera driver unloaded\n"); > > return 0; > -- > 1.7.9.5 > This patch doesn't fix the BUG it claims to, since I have it working properly in our Visstrim M10 platform without it. Look: soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0 mx2-camera mx2-camera.0: Camera driver attached to camera 0 ov7670 0-0021: chip found @ 0x42 (imx-i2c) [..] mx2-camera mx2-camera.0: Camera driver detached from camera 0 mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 66500000 Furthermore, it's not correct, since there isn't such "per" clock for the CSI in 3.5 [1], 3.6 [2], linux-next-20121008[3], or next-20121009[4]. [1] http://lxr.linux.no/#linux+v3.5/arch/arm/mach-imx/clk-imx27.c#L226 [2] http://lxr.linux.no/#linux+v3.6/arch/arm/mach-imx/clk-imx27.c#L226 [3] http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=blob;f=arch/arm/mach-imx/clk-imx27.c;h=3b6b640eed247ea1b7848c7a7fa01801f0190cde;hb=cc925138a0dd9ae388135bb3cf11ee1729f9c4e9#l226 [4] http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=blob;f=arch/arm/mach-imx/clk-imx27.c;h=3b6b640eed247ea1b7848c7a7fa01801f0190cde;hb=b066f61482c7eac44e656499426a3c56d29c32ed#l226 Regards.
Hi Javier, On Tue, Oct 9, 2012 at 8:16 AM, javier Martin <javier.martin@vista-silicon.com> wrote: > This patch doesn't fix the BUG it claims to, since I have it working > properly in our Visstrim M10 platform without it. Look: Yes, it does fix a real bug. Without this patch the ov2640 cannot be probed, as it fails to read the product/vendor ID via I2C. I measure with the scope and do not get mclk at all without this patch. Again, camera probe does work fine on kernel 3.4. Does the mx27 feed the mclk to your camera? Which frequency do you get if you measure it with a scope. > > soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0 > mx2-camera mx2-camera.0: Camera driver attached to camera 0 > ov7670 0-0021: chip found @ 0x42 (imx-i2c) > [..] > mx2-camera mx2-camera.0: Camera driver detached from camera 0 > mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock > frequency: 66500000 This 66.5MHz is wrong. > > Furthermore, it's not correct, since there isn't such "per" clock for > the CSI in 3.5 [1], 3.6 [2], linux-next-20121008[3], or > next-20121009[4]. Well, you are looking to all git trees after the conversion to the common clock framework. Please look at 3.4 kernel instead and you will see that per4 is indeed used for csi. "DEFINE_CLOCK1(csi_clk, 0, NULL, 0, parent, &csi_clk1, &per4_clk);" In fact, the mx27 reference manual is the correct source for such information. Please check "Table 39-9. CSI Control Register 1 Field Descriptions (continued)": "Sensor Master Clock (MCLK) Divider. This field contains the divisor MCLK. The MCLK is derived from the PERCLK4." Can you let me know if this patch breaks things for you? Or what exactly you think is wrong with it? It seems that you are camera works by pure luck without this patch. Maybe you are turning per4 in the bootloader or you get the clock to your camera from somewhere else. Regards, Fabio Estevam -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 10/09/2012 12:37 AM, Fabio Estevam wrote: > From: Fabio Estevam <fabio.estevam@freescale.com> > > Since mx27 transitioned to the commmon clock framework in 3.5, the correct way > to acquire the csi clock is to get csi_ahb and csi_per clocks separately. > > By not doing so the camera sensor does not probe correctly: > > soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0 > mx2-camera mx2-camera.0: Camera driver attached to camera 0 > ov2640 0-0030: Product ID error fb:fb > mx2-camera mx2-camera.0: Camera driver detached from camera 0 > mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 66500000 > > Adapt the mx2_camera driver to the new clock framework and make it functional > again. > > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> > --- > Changes since v1: > - Rebased against linux-next 20121008. > > drivers/media/platform/soc_camera/mx2_camera.c | 47 +++++++++++++++++------- > 1 file changed, 34 insertions(+), 13 deletions(-) > > diff --git a/drivers/media/platform/soc_camera/mx2_camera.c b/drivers/media/platform/soc_camera/mx2_camera.c > index 403d7f1..9f8c5f0 100644 > --- a/drivers/media/platform/soc_camera/mx2_camera.c > +++ b/drivers/media/platform/soc_camera/mx2_camera.c > @@ -272,7 +272,8 @@ struct mx2_camera_dev { > struct device *dev; > struct soc_camera_host soc_host; > struct soc_camera_device *icd; > - struct clk *clk_csi, *clk_emma_ahb, *clk_emma_ipg; > + struct clk *clk_emma_ahb, *clk_emma_ipg; > + struct clk *clk_csi_ahb, *clk_csi_per; > > void __iomem *base_csi, *base_emma; > > @@ -432,7 +433,8 @@ static void mx2_camera_deactivate(struct mx2_camera_dev *pcdev) > { > unsigned long flags; > > - clk_disable_unprepare(pcdev->clk_csi); > + clk_disable_unprepare(pcdev->clk_csi_ahb); > + clk_disable_unprepare(pcdev->clk_csi_per); > writel(0, pcdev->base_csi + CSICR1); > if (cpu_is_mx27()) { > writel(0, pcdev->base_emma + PRP_CNTL); > @@ -460,7 +462,11 @@ static int mx2_camera_add_device(struct soc_camera_device *icd) > if (pcdev->icd) > return -EBUSY; > > - ret = clk_prepare_enable(pcdev->clk_csi); > + ret = clk_prepare_enable(pcdev->clk_csi_ahb); > + if (ret < 0) > + return ret; > + > + ret = clk_prepare_enable(pcdev->clk_csi_per); > if (ret < 0) > return ret; > > @@ -1725,11 +1731,18 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev) > goto exit; > } > > - pcdev->clk_csi = devm_clk_get(&pdev->dev, "ahb"); > - if (IS_ERR(pcdev->clk_csi)) { > - dev_err(&pdev->dev, "Could not get csi clock\n"); > - err = PTR_ERR(pcdev->clk_csi); > - goto exit; > + pcdev->clk_csi_ahb = devm_clk_get(&pdev->dev, "ahb"); > + if (IS_ERR(pcdev->clk_csi_ahb)) { > + dev_err(&pdev->dev, "Could not get csi ahb clock\n"); > + err = PTR_ERR(pcdev->clk_csi_ahb); > + goto exit; > + } > + > + pcdev->clk_csi_per = devm_clk_get(&pdev->dev, "per"); > + if (IS_ERR(pcdev->clk_csi_per)) { > + dev_err(&pdev->dev, "Could not get csi per clock\n"); > + err = PTR_ERR(pcdev->clk_csi_per); > + goto exit_csi_ahb; > } > > pcdev->pdata = pdev->dev.platform_data; > @@ -1738,14 +1751,15 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev) > > pcdev->platform_flags = pcdev->pdata->flags; > > - rate = clk_round_rate(pcdev->clk_csi, pcdev->pdata->clk * 2); > + rate = clk_round_rate(pcdev->clk_csi_per, > + pcdev->pdata->clk * 2); > if (rate <= 0) { > err = -ENODEV; > - goto exit; > + goto exit_csi_per; > } > - err = clk_set_rate(pcdev->clk_csi, rate); > + err = clk_set_rate(pcdev->clk_csi_per, rate); > if (err < 0) > - goto exit; > + goto exit_csi_per; > } > > INIT_LIST_HEAD(&pcdev->capture); > @@ -1801,7 +1815,7 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev) > goto exit_free_emma; > > dev_info(&pdev->dev, "MX2 Camera (CSI) driver probed, clock frequency: %ld\n", > - clk_get_rate(pcdev->clk_csi)); > + clk_get_rate(pcdev->clk_csi_per)); > > return 0; > > @@ -1812,6 +1826,10 @@ eallocctx: > clk_disable_unprepare(pcdev->clk_emma_ipg); > clk_disable_unprepare(pcdev->clk_emma_ahb); > } > +exit_csi_per: > + clk_disable_unprepare(pcdev->clk_csi_per); > +exit_csi_ahb: > + clk_disable_unprepare(pcdev->clk_csi_ahb); > exit: > return err; > } > @@ -1831,6 +1849,9 @@ static int __devexit mx2_camera_remove(struct platform_device *pdev) > clk_disable_unprepare(pcdev->clk_emma_ahb); > } > > + clk_disable_unprepare(pcdev->clk_csi_per); > + clk_disable_unprepare(pcdev->clk_csi_ahb); > + > dev_info(&pdev->dev, "MX2 Camera driver unloaded\n"); > > return 0; > I only test detection at kernel boot not streaming using Gstreamer due to lack of time. On imx27_3ds board with ov2640 sensor: ov2640 0-0030: ov2640 Product ID 26:42 Manufacturer ID 7f:a2 mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 44333333 On clone imx27_3ds board with mt9v111 sensor (draft driver): mt9v111 0-0048: Detected a MT9V111 chip ID 823a mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 44333333 Tested-by: Gaëtan Carlier <gcembed@gmail.com> -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 9 October 2012 14:48, Gaëtan Carlier <gcembed@gmail.com> wrote: > Hi, > > On 10/09/2012 12:37 AM, Fabio Estevam wrote: >> >> From: Fabio Estevam <fabio.estevam@freescale.com> >> >> Since mx27 transitioned to the commmon clock framework in 3.5, the correct >> way >> to acquire the csi clock is to get csi_ahb and csi_per clocks separately. >> >> By not doing so the camera sensor does not probe correctly: >> >> soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0 >> mx2-camera mx2-camera.0: Camera driver attached to camera 0 >> ov2640 0-0030: Product ID error fb:fb >> mx2-camera mx2-camera.0: Camera driver detached from camera 0 >> mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: >> 66500000 >> >> Adapt the mx2_camera driver to the new clock framework and make it >> functional >> again. >> >> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> >> --- >> Changes since v1: >> - Rebased against linux-next 20121008. >> >> drivers/media/platform/soc_camera/mx2_camera.c | 47 >> +++++++++++++++++------- >> 1 file changed, 34 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/media/platform/soc_camera/mx2_camera.c >> b/drivers/media/platform/soc_camera/mx2_camera.c >> index 403d7f1..9f8c5f0 100644 >> --- a/drivers/media/platform/soc_camera/mx2_camera.c >> +++ b/drivers/media/platform/soc_camera/mx2_camera.c >> @@ -272,7 +272,8 @@ struct mx2_camera_dev { >> struct device *dev; >> struct soc_camera_host soc_host; >> struct soc_camera_device *icd; >> - struct clk *clk_csi, *clk_emma_ahb, *clk_emma_ipg; >> + struct clk *clk_emma_ahb, *clk_emma_ipg; >> + struct clk *clk_csi_ahb, *clk_csi_per; >> >> void __iomem *base_csi, *base_emma; >> >> @@ -432,7 +433,8 @@ static void mx2_camera_deactivate(struct >> mx2_camera_dev *pcdev) >> { >> unsigned long flags; >> >> - clk_disable_unprepare(pcdev->clk_csi); >> + clk_disable_unprepare(pcdev->clk_csi_ahb); >> + clk_disable_unprepare(pcdev->clk_csi_per); >> writel(0, pcdev->base_csi + CSICR1); >> if (cpu_is_mx27()) { >> writel(0, pcdev->base_emma + PRP_CNTL); >> @@ -460,7 +462,11 @@ static int mx2_camera_add_device(struct >> soc_camera_device *icd) >> if (pcdev->icd) >> return -EBUSY; >> >> - ret = clk_prepare_enable(pcdev->clk_csi); >> + ret = clk_prepare_enable(pcdev->clk_csi_ahb); >> + if (ret < 0) >> + return ret; >> + >> + ret = clk_prepare_enable(pcdev->clk_csi_per); >> if (ret < 0) >> return ret; >> >> @@ -1725,11 +1731,18 @@ static int __devinit mx2_camera_probe(struct >> platform_device *pdev) >> goto exit; >> } >> >> - pcdev->clk_csi = devm_clk_get(&pdev->dev, "ahb"); >> - if (IS_ERR(pcdev->clk_csi)) { >> - dev_err(&pdev->dev, "Could not get csi clock\n"); >> - err = PTR_ERR(pcdev->clk_csi); >> - goto exit; >> + pcdev->clk_csi_ahb = devm_clk_get(&pdev->dev, "ahb"); >> + if (IS_ERR(pcdev->clk_csi_ahb)) { >> + dev_err(&pdev->dev, "Could not get csi ahb clock\n"); >> + err = PTR_ERR(pcdev->clk_csi_ahb); >> + goto exit; >> + } >> + >> + pcdev->clk_csi_per = devm_clk_get(&pdev->dev, "per"); >> + if (IS_ERR(pcdev->clk_csi_per)) { >> + dev_err(&pdev->dev, "Could not get csi per clock\n"); >> + err = PTR_ERR(pcdev->clk_csi_per); >> + goto exit_csi_ahb; >> } >> >> pcdev->pdata = pdev->dev.platform_data; >> @@ -1738,14 +1751,15 @@ static int __devinit mx2_camera_probe(struct >> platform_device *pdev) >> >> pcdev->platform_flags = pcdev->pdata->flags; >> >> - rate = clk_round_rate(pcdev->clk_csi, pcdev->pdata->clk * >> 2); >> + rate = clk_round_rate(pcdev->clk_csi_per, >> + pcdev->pdata->clk * 2); >> if (rate <= 0) { >> err = -ENODEV; >> - goto exit; >> + goto exit_csi_per; >> } >> - err = clk_set_rate(pcdev->clk_csi, rate); >> + err = clk_set_rate(pcdev->clk_csi_per, rate); >> if (err < 0) >> - goto exit; >> + goto exit_csi_per; >> } >> >> INIT_LIST_HEAD(&pcdev->capture); >> @@ -1801,7 +1815,7 @@ static int __devinit mx2_camera_probe(struct >> platform_device *pdev) >> goto exit_free_emma; >> >> dev_info(&pdev->dev, "MX2 Camera (CSI) driver probed, clock >> frequency: %ld\n", >> - clk_get_rate(pcdev->clk_csi)); >> + clk_get_rate(pcdev->clk_csi_per)); >> >> return 0; >> >> @@ -1812,6 +1826,10 @@ eallocctx: >> clk_disable_unprepare(pcdev->clk_emma_ipg); >> clk_disable_unprepare(pcdev->clk_emma_ahb); >> } >> +exit_csi_per: >> + clk_disable_unprepare(pcdev->clk_csi_per); >> +exit_csi_ahb: >> + clk_disable_unprepare(pcdev->clk_csi_ahb); >> exit: >> return err; >> } >> @@ -1831,6 +1849,9 @@ static int __devexit mx2_camera_remove(struct >> platform_device *pdev) >> clk_disable_unprepare(pcdev->clk_emma_ahb); >> } >> >> + clk_disable_unprepare(pcdev->clk_csi_per); >> + clk_disable_unprepare(pcdev->clk_csi_ahb); >> + >> dev_info(&pdev->dev, "MX2 Camera driver unloaded\n"); >> >> return 0; >> > I only test detection at kernel boot not streaming using Gstreamer due to > lack of time. > > On imx27_3ds board with ov2640 sensor: > ov2640 0-0030: ov2640 Product ID 26:42 Manufacturer ID 7f:a2 > mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: > 44333333 > > On clone imx27_3ds board with mt9v111 sensor (draft driver): > mt9v111 0-0048: Detected a MT9V111 chip ID 823a > mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: > 44333333 > > Tested-by: Gaëtan Carlier <gcembed@gmail.com> Sorry I missed patch 1/2. Both patches are correct: Tested-by: Javier Martin <javier.martin@vista-silicon.com>
On Mon, Oct 08, 2012 at 07:37:03PM -0300, Fabio Estevam wrote: > @@ -460,7 +462,11 @@ static int mx2_camera_add_device(struct soc_camera_device *icd) > if (pcdev->icd) > return -EBUSY; > > - ret = clk_prepare_enable(pcdev->clk_csi); > + ret = clk_prepare_enable(pcdev->clk_csi_ahb); > + if (ret < 0) > + return ret; > + > + ret = clk_prepare_enable(pcdev->clk_csi_per); > if (ret < 0) > return ret; From the point of view of error cleanup, this looks buggy to me. If the prepare_enable for the per clock fails, what cleans up the ahb clock? -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/platform/soc_camera/mx2_camera.c b/drivers/media/platform/soc_camera/mx2_camera.c index 403d7f1..9f8c5f0 100644 --- a/drivers/media/platform/soc_camera/mx2_camera.c +++ b/drivers/media/platform/soc_camera/mx2_camera.c @@ -272,7 +272,8 @@ struct mx2_camera_dev { struct device *dev; struct soc_camera_host soc_host; struct soc_camera_device *icd; - struct clk *clk_csi, *clk_emma_ahb, *clk_emma_ipg; + struct clk *clk_emma_ahb, *clk_emma_ipg; + struct clk *clk_csi_ahb, *clk_csi_per; void __iomem *base_csi, *base_emma; @@ -432,7 +433,8 @@ static void mx2_camera_deactivate(struct mx2_camera_dev *pcdev) { unsigned long flags; - clk_disable_unprepare(pcdev->clk_csi); + clk_disable_unprepare(pcdev->clk_csi_ahb); + clk_disable_unprepare(pcdev->clk_csi_per); writel(0, pcdev->base_csi + CSICR1); if (cpu_is_mx27()) { writel(0, pcdev->base_emma + PRP_CNTL); @@ -460,7 +462,11 @@ static int mx2_camera_add_device(struct soc_camera_device *icd) if (pcdev->icd) return -EBUSY; - ret = clk_prepare_enable(pcdev->clk_csi); + ret = clk_prepare_enable(pcdev->clk_csi_ahb); + if (ret < 0) + return ret; + + ret = clk_prepare_enable(pcdev->clk_csi_per); if (ret < 0) return ret; @@ -1725,11 +1731,18 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev) goto exit; } - pcdev->clk_csi = devm_clk_get(&pdev->dev, "ahb"); - if (IS_ERR(pcdev->clk_csi)) { - dev_err(&pdev->dev, "Could not get csi clock\n"); - err = PTR_ERR(pcdev->clk_csi); - goto exit; + pcdev->clk_csi_ahb = devm_clk_get(&pdev->dev, "ahb"); + if (IS_ERR(pcdev->clk_csi_ahb)) { + dev_err(&pdev->dev, "Could not get csi ahb clock\n"); + err = PTR_ERR(pcdev->clk_csi_ahb); + goto exit; + } + + pcdev->clk_csi_per = devm_clk_get(&pdev->dev, "per"); + if (IS_ERR(pcdev->clk_csi_per)) { + dev_err(&pdev->dev, "Could not get csi per clock\n"); + err = PTR_ERR(pcdev->clk_csi_per); + goto exit_csi_ahb; } pcdev->pdata = pdev->dev.platform_data; @@ -1738,14 +1751,15 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev) pcdev->platform_flags = pcdev->pdata->flags; - rate = clk_round_rate(pcdev->clk_csi, pcdev->pdata->clk * 2); + rate = clk_round_rate(pcdev->clk_csi_per, + pcdev->pdata->clk * 2); if (rate <= 0) { err = -ENODEV; - goto exit; + goto exit_csi_per; } - err = clk_set_rate(pcdev->clk_csi, rate); + err = clk_set_rate(pcdev->clk_csi_per, rate); if (err < 0) - goto exit; + goto exit_csi_per; } INIT_LIST_HEAD(&pcdev->capture); @@ -1801,7 +1815,7 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev) goto exit_free_emma; dev_info(&pdev->dev, "MX2 Camera (CSI) driver probed, clock frequency: %ld\n", - clk_get_rate(pcdev->clk_csi)); + clk_get_rate(pcdev->clk_csi_per)); return 0; @@ -1812,6 +1826,10 @@ eallocctx: clk_disable_unprepare(pcdev->clk_emma_ipg); clk_disable_unprepare(pcdev->clk_emma_ahb); } +exit_csi_per: + clk_disable_unprepare(pcdev->clk_csi_per); +exit_csi_ahb: + clk_disable_unprepare(pcdev->clk_csi_ahb); exit: return err; } @@ -1831,6 +1849,9 @@ static int __devexit mx2_camera_remove(struct platform_device *pdev) clk_disable_unprepare(pcdev->clk_emma_ahb); } + clk_disable_unprepare(pcdev->clk_csi_per); + clk_disable_unprepare(pcdev->clk_csi_ahb); + dev_info(&pdev->dev, "MX2 Camera driver unloaded\n"); return 0;