[PATCH 0/2] OMAP3 ISP: Simplify clock usage

[PATCH 0/2] OMAP3 ISP: Simplify clock usage

[Laurent Pinchart]

Hello,

Now that the OMAP3 supports the common clock framework, clock rate
back-propagation is available for the ISP clocks. Instead of setting the
cam_mclk parent clock rate to control the cam_mclk clock rate, we can mark the
dpll4_m5x2_ck_3630 and cam_mclk clocks as supporting back-propagation, and set
the cam_mclk rate directly. This simplifies the ISP clocks configuration.

Laurent Pinchart (2):
  ARM: OMAP3: clock: Back-propagate rate change from cam_mclk to
    dpll4_m5
  omap3isp: Set cam_mclk rate directly

 arch/arm/mach-omap2/cclock3xxx_data.c |   10 +++++++++-
 drivers/media/platform/omap3isp/isp.c |   18 ++----------------
 drivers/media/platform/omap3isp/isp.h |    8 +++-----
 3 files changed, 14 insertions(+), 22 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH 1/2] ARM: OMAP3: clock: Back-propagate rate change from cam_mclk to dpll4_m5

[Laurent Pinchart]

The cam_mclk clock is generated through the following clocks chain:

dpll4 -> dpll4_m5 -> dpll4_m5x2 -> cam_mclk

As dpll4_m5 and dpll4_m5x2 do not driver any clock other than cam_mclk,
back-propagate the cam_clk rate changes up to dpll4_m5.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 arch/arm/mach-omap2/cclock3xxx_data.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/cclock3xxx_data.c b/arch/arm/mach-omap2/cclock3xxx_data.c
index bdf3948..326b6ad 100644
--- a/arch/arm/mach-omap2/cclock3xxx_data.c
+++ b/arch/arm/mach-omap2/cclock3xxx_data.c
@@ -426,6 +426,7 @@ static struct clk dpll4_m5x2_ck_3630 = {
 	.parent_names	= dpll4_m5x2_ck_parent_names,
 	.num_parents	= ARRAY_SIZE(dpll4_m5x2_ck_parent_names),
 	.ops		= &dpll4_m5x2_ck_3630_ops,
+	.flags		= CLK_SET_RATE_PARENT,
 };
 
 static struct clk cam_mclk;
@@ -443,7 +444,14 @@ static struct clk_hw_omap cam_mclk_hw = {
 	.clkdm_name	= "cam_clkdm",
 };
 
-DEFINE_STRUCT_CLK(cam_mclk, cam_mclk_parent_names, aes2_ick_ops);
+static struct clk cam_mclk = {
+	.name		= "cam_mclk",
+	.hw		= &cam_mclk_hw.hw,
+	.parent_names	= cam_mclk_parent_names,
+	.num_parents	= ARRAY_SIZE(cam_mclk_parent_names),
+	.ops		= &aes2_ick_ops,
+	.flags		= CLK_SET_RATE_PARENT,
+};
 
 static const struct clksel_rate clkout2_src_core_rates[] = {
 	{ .div = 1, .val = 0, .flags = RATE_IN_3XXX },
-- 
1.7.8.6

[PATCH 2/2] omap3isp: Set cam_mclk rate directly

[Laurent Pinchart]

Now that the cam_mclk rate changes are back-propagated to dpll4_m5_ck we
can set the cam_mclk rate directly instead of manually setting the rate
of the parent clock.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/omap3isp/isp.c |   18 ++----------------
 drivers/media/platform/omap3isp/isp.h |    8 +++-----
 2 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 07eea5b..63a583f 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -1338,28 +1338,15 @@ static int isp_enable_clocks(struct isp_device *isp)
 {
 	int r;
 	unsigned long rate;
-	int divisor;
-
-	/*
-	 * cam_mclk clock chain:
-	 *   dpll4 -> dpll4_m5 -> dpll4_m5x2 -> cam_mclk
-	 *
-	 * In OMAP3630 dpll4_m5x2 != 2 x dpll4_m5 but both are
-	 * set to the same value. Hence the rate set for dpll4_m5
-	 * has to be twice of what is set on OMAP3430 to get
-	 * the required value for cam_mclk
-	 */
-	divisor = isp->revision == ISP_REVISION_15_0 ? 1 : 2;
 
 	r = clk_prepare_enable(isp->clock[ISP_CLK_CAM_ICK]);
 	if (r) {
 		dev_err(isp->dev, "failed to enable cam_ick clock\n");
 		goto out_clk_enable_ick;
 	}
-	r = clk_set_rate(isp->clock[ISP_CLK_DPLL4_M5_CK],
-			 CM_CAM_MCLK_HZ/divisor);
+	r = clk_set_rate(isp->clock[ISP_CLK_CAM_MCLK], CM_CAM_MCLK_HZ);
 	if (r) {
-		dev_err(isp->dev, "clk_set_rate for dpll4_m5_ck failed\n");
+		dev_err(isp->dev, "clk_set_rate for cam_mclk failed\n");
 		goto out_clk_enable_mclk;
 	}
 	r = clk_prepare_enable(isp->clock[ISP_CLK_CAM_MCLK]);
@@ -1401,7 +1388,6 @@ static void isp_disable_clocks(struct isp_device *isp)
 static const char *isp_clocks[] = {
 	"cam_ick",
 	"cam_mclk",
-	"dpll4_m5_ck",
 	"csi2_96m_fck",
 	"l3_ick",
 };
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index 517d348..c77e1f2 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -147,7 +147,6 @@ struct isp_platform_callback {
  * @ref_count: Reference count for handling multiple ISP requests.
  * @cam_ick: Pointer to camera interface clock structure.
  * @cam_mclk: Pointer to camera functional clock structure.
- * @dpll4_m5_ck: Pointer to DPLL4 M5 clock structure.
  * @csi2_fck: Pointer to camera CSI2 complexIO clock structure.
  * @l3_ick: Pointer to OMAP3 L3 bus interface clock.
  * @irq: Currently attached ISP ISR callbacks information structure.
@@ -189,10 +188,9 @@ struct isp_device {
 	u32 xclk_divisor[2];	/* Two clocks, a and b. */
 #define ISP_CLK_CAM_ICK		0
 #define ISP_CLK_CAM_MCLK	1
-#define ISP_CLK_DPLL4_M5_CK	2
-#define ISP_CLK_CSI2_FCK	3
-#define ISP_CLK_L3_ICK		4
-	struct clk *clock[5];
+#define ISP_CLK_CSI2_FCK	2
+#define ISP_CLK_L3_ICK		3
+	struct clk *clock[4];
 
 	/* ISP modules */
 	struct ispstat isp_af;
-- 
1.7.8.6

Re: [PATCH 0/2] OMAP3 ISP: Simplify clock usage

[Mike Turquette]

Quoting Laurent Pinchart (2013-01-08 05:43:52)
> Hello,
> 
> Now that the OMAP3 supports the common clock framework, clock rate
> back-propagation is available for the ISP clocks. Instead of setting the
> cam_mclk parent clock rate to control the cam_mclk clock rate, we can mark the
> dpll4_m5x2_ck_3630 and cam_mclk clocks as supporting back-propagation, and set
> the cam_mclk rate directly. This simplifies the ISP clocks configuration.
>

I'm pleased to see this feature get used on OMAP.  Plus your driver gets
a negative diffstat :)

Reviewed-by: Mike Turquette <mturquette@linaro.org>
 
> Laurent Pinchart (2):
>   ARM: OMAP3: clock: Back-propagate rate change from cam_mclk to
>     dpll4_m5
>   omap3isp: Set cam_mclk rate directly
> 
>  arch/arm/mach-omap2/cclock3xxx_data.c |   10 +++++++++-
>  drivers/media/platform/omap3isp/isp.c |   18 ++----------------
>  drivers/media/platform/omap3isp/isp.h |    8 +++-----
>  3 files changed, 14 insertions(+), 22 deletions(-)
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Re: [PATCH 0/2] OMAP3 ISP: Simplify clock usage

[Laurent Pinchart]

Hi Mike,

On Monday 14 January 2013 17:10:15 Mike Turquette wrote:
> Quoting Laurent Pinchart (2013-01-08 05:43:52)
> 
> > Hello,
> > 
> > Now that the OMAP3 supports the common clock framework, clock rate
> > back-propagation is available for the ISP clocks. Instead of setting the
> > cam_mclk parent clock rate to control the cam_mclk clock rate, we can mark
> > the dpll4_m5x2_ck_3630 and cam_mclk clocks as supporting
> > back-propagation, and set the cam_mclk rate directly. This simplifies the
> > ISP clocks configuration.
>
> I'm pleased to see this feature get used on OMAP.  Plus your driver gets
> a negative diffstat :)
> 
> Reviewed-by: Mike Turquette <mturquette@linaro.org>

Thanks.

Would you like to take the arch/ patch in your tree, or should I push it 
through the linux-media tree along with the omap3isp patch ?

> > Laurent Pinchart (2):
> >   ARM: OMAP3: clock: Back-propagate rate change from cam_mclk to
> >     dpll4_m5
> >   omap3isp: Set cam_mclk rate directly
> >  
> >  arch/arm/mach-omap2/cclock3xxx_data.c |   10 +++++++++-
> >  drivers/media/platform/omap3isp/isp.c |   18 ++----------------
> >  drivers/media/platform/omap3isp/isp.h |    8 +++-----
> >  3 files changed, 14 insertions(+), 22 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] OMAP3 ISP: Simplify clock usage

[Tony Lindgren]

* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [130121 05:37]:
> Hi Mike,
> 
> On Monday 14 January 2013 17:10:15 Mike Turquette wrote:
> > Quoting Laurent Pinchart (2013-01-08 05:43:52)
> > 
> > > Hello,
> > > 
> > > Now that the OMAP3 supports the common clock framework, clock rate
> > > back-propagation is available for the ISP clocks. Instead of setting the
> > > cam_mclk parent clock rate to control the cam_mclk clock rate, we can mark
> > > the dpll4_m5x2_ck_3630 and cam_mclk clocks as supporting
> > > back-propagation, and set the cam_mclk rate directly. This simplifies the
> > > ISP clocks configuration.
> >
> > I'm pleased to see this feature get used on OMAP.  Plus your driver gets
> > a negative diffstat :)
> > 
> > Reviewed-by: Mike Turquette <mturquette@linaro.org>
> 
> Thanks.
> 
> Would you like to take the arch/ patch in your tree, or should I push it 
> through the linux-media tree along with the omap3isp patch ?

The arch/arm/*omap* clock changes need to be queued by Paul to avoid
potential stupid merge conflicts when the clock data gets moved to
live under drivers/clk/omap.

Regards,

Tony

Re: [PATCH 0/2] OMAP3 ISP: Simplify clock usage

[Laurent Pinchart]

Hi Tony,

On Monday 21 January 2013 09:18:12 Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [130121 05:37]:
> > On Monday 14 January 2013 17:10:15 Mike Turquette wrote:
> > > Quoting Laurent Pinchart (2013-01-08 05:43:52)
> > > 
> > > > Hello,
> > > > 
> > > > Now that the OMAP3 supports the common clock framework, clock rate
> > > > back-propagation is available for the ISP clocks. Instead of setting
> > > > the cam_mclk parent clock rate to control the cam_mclk clock rate, we
> > > > can mark the dpll4_m5x2_ck_3630 and cam_mclk clocks as supporting
> > > > back-propagation, and set the cam_mclk rate directly. This simplifies
> > > > the ISP clocks configuration.
> > > 
> > > I'm pleased to see this feature get used on OMAP.  Plus your driver gets
> > > a negative diffstat :)
> > > 
> > > Reviewed-by: Mike Turquette <mturquette@linaro.org>
> > 
> > Thanks.
> > 
> > Would you like to take the arch/ patch in your tree, or should I push it
> > through the linux-media tree along with the omap3isp patch ?
> 
> The arch/arm/*omap* clock changes need to be queued by Paul to avoid
> potential stupid merge conflicts when the clock data gets moved to
> live under drivers/clk/omap.

OK. The omap3isp patch can go through Paul's tree as well, it won't conflict 
with other changes to the driver in this merge window.

Paul, can you take both patches together ? If so I'll send you a pull request.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] OMAP3 ISP: Simplify clock usage

[Mike Turquette]

Quoting Laurent Pinchart (2013-01-21 10:54:38)
> Hi Tony,
> 
> On Monday 21 January 2013 09:18:12 Tony Lindgren wrote:
> > * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [130121 05:37]:
> > > On Monday 14 January 2013 17:10:15 Mike Turquette wrote:
> > > > Quoting Laurent Pinchart (2013-01-08 05:43:52)
> > > > 
> > > > > Hello,
> > > > > 
> > > > > Now that the OMAP3 supports the common clock framework, clock rate
> > > > > back-propagation is available for the ISP clocks. Instead of setting
> > > > > the cam_mclk parent clock rate to control the cam_mclk clock rate, we
> > > > > can mark the dpll4_m5x2_ck_3630 and cam_mclk clocks as supporting
> > > > > back-propagation, and set the cam_mclk rate directly. This simplifies
> > > > > the ISP clocks configuration.
> > > > 
> > > > I'm pleased to see this feature get used on OMAP.  Plus your driver gets
> > > > a negative diffstat :)
> > > > 
> > > > Reviewed-by: Mike Turquette <mturquette@linaro.org>
> > > 
> > > Thanks.
> > > 
> > > Would you like to take the arch/ patch in your tree, or should I push it
> > > through the linux-media tree along with the omap3isp patch ?
> > 
> > The arch/arm/*omap* clock changes need to be queued by Paul to avoid
> > potential stupid merge conflicts when the clock data gets moved to
> > live under drivers/clk/omap.
> 
> OK. The omap3isp patch can go through Paul's tree as well, it won't conflict 
> with other changes to the driver in this merge window.
> 
> Paul, can you take both patches together ? If so I'll send you a pull request.
> 

+1

I don't take in driver changes/adaptations through the clk tree unless
it is necessary to avoid painful conflicts or merge ordering issues.
This has only happened a few times for MFD devices, etc.  Typically the
clk-next branch is only used for changes to the clk framework core or
clk drivers.

Regards,
Mike

> -- 
> Regards,
> 
> Laurent Pinchart

Re: [PATCH 0/2] OMAP3 ISP: Simplify clock usage

[Paul Walmsley]

On Mon, 21 Jan 2013, Laurent Pinchart wrote:

> OK. The omap3isp patch can go through Paul's tree as well, it won't conflict 
> with other changes to the driver in this merge window.
> 
> Paul, can you take both patches together ? If so I'll send you a pull request.

Yes I'll take them, as long as they won't cause conflicts outside of 
arch/arm/mach-omap2.  Otherwise the OMAP3 ISP patch should wait until the 
early v3.9-rc integration fixes timeframe.


- Paul

Re: [PATCH 0/2] OMAP3 ISP: Simplify clock usage

[Laurent Pinchart]

Hi Paul,

On Tuesday 22 January 2013 02:57:22 Paul Walmsley wrote:
> On Mon, 21 Jan 2013, Laurent Pinchart wrote:
> > OK. The omap3isp patch can go through Paul's tree as well, it won't
> > conflict with other changes to the driver in this merge window.
> > 
> > Paul, can you take both patches together ? If so I'll send you a pull
> > request.
>
> Yes I'll take them, as long as they won't cause conflicts outside of
> arch/arm/mach-omap2. Otherwise the OMAP3 ISP patch should wait until the
> early v3.9-rc integration fixes timeframe.

There's currently no conflict with the other OMAP3 ISP patches that I intend 
to push to v3.9.

The following changes since commit 7d1f9aeff1ee4a20b1aeb377dd0f579fe9647619:

  Linux 3.8-rc4 (2013-01-17 19:25:45 -0800)

are available in the git repository at:

  git://linuxtv.org/pinchartl/media.git omap3isp/clock

for you to fetch changes up to 6d1aa02f10497b138e01ebe6eafabd6071729334:

  omap3isp: Set cam_mclk rate directly (2013-01-23 00:44:04 +0100)

----------------------------------------------------------------
Laurent Pinchart (2):
      ARM: OMAP3: clock: Back-propagate rate change from cam_mclk to dpll4_m5
      omap3isp: Set cam_mclk rate directly

 arch/arm/mach-omap2/cclock3xxx_data.c | 10 +++++++++-
 drivers/media/platform/omap3isp/isp.c | 18 ++----------------
 drivers/media/platform/omap3isp/isp.h |  8 +++-----
 3 files changed, 14 insertions(+), 22 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] OMAP3 ISP: Simplify clock usage

[Sakari Ailus]

Hi Laurent,

On Tue, Jan 08, 2013 at 02:43:52PM +0100, Laurent Pinchart wrote:
> Hello,
> 
> Now that the OMAP3 supports the common clock framework, clock rate
> back-propagation is available for the ISP clocks. Instead of setting the
> cam_mclk parent clock rate to control the cam_mclk clock rate, we can mark the
> dpll4_m5x2_ck_3630 and cam_mclk clocks as supporting back-propagation, and set
> the cam_mclk rate directly. This simplifies the ISP clocks configuration.
> 
> Laurent Pinchart (2):
>   ARM: OMAP3: clock: Back-propagate rate change from cam_mclk to
>     dpll4_m5
>   omap3isp: Set cam_mclk rate directly
> 
>  arch/arm/mach-omap2/cclock3xxx_data.c |   10 +++++++++-
>  drivers/media/platform/omap3isp/isp.c |   18 ++----------------
>  drivers/media/platform/omap3isp/isp.h |    8 +++-----
>  3 files changed, 14 insertions(+), 22 deletions(-)

Thanks! It's so nice to see one of the last ugly bits of the omap3isp driver
disappear. :-)

Acked-by: Sakari Ailus <sakari.ailus@iki.fi>
Tested-by: Sakari Ailus <sakari.ailus@iki.fi>

(On 3630 / N950.)

-- 
Cheers,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk