Re: [PATCH v6 0/5] clk: clock deregistration support
Quoting Sylwester Nawrocki (2013-10-15 13:04:17)
Hi,
(adding linux-media mailing list at Cc)
On 09/25/2013 11:47 AM, Laurent Pinchart wrote:
On Tuesday 24 September 2013 23:38:44 Sylwester Nawrocki wrote:
[...]
The only issue I found might be at the omap3isp driver, which provides
clock to its sub-drivers and takes reference on the sub-driver modules.
When sub-driver calls clk_get() all modules would get locked in memory,
due to circular reference. One solution to that could be to pass NULL
struct device pointer, as in the below patch.
Doesn't that introduce race conditions ? If the sub-drivers require the
clock,
they want to be sure that the clock won't disappear beyond their backs. I
agree that the circular dependency needs to be solved somehow, but we
probably
need a more generic solution. The problem will become more widespread in the
future with DT-based device instantiation in both V4L2 and KMS.
I'm wondering whether subsystems and drivers itself should be written so
they deal with such dependencies they are aware of.
There is similar situation in the regulator API, regulator_get() simply
takes a reference on a module providing the regulator object.
Before a more generic solution is available, what do you think about
keeping obtaining a reference on a clock provider module in clk_get() and
doing clk_get(), clk_prepare_enable(), ..., clk_unprepare_disable(),
clk_put() in sub-driver whenever a clock is actively used, to avoid
permanent circular reference ?
Laurent,
Did you have any feedback on this proposal? I would like to merge these
patches so that folks with clock driver modules can use them properly.
We can fix up things in the core code as we figure them out.
Regards,
Mike
--
Thanks,
Sylwester
-8--
From ca5963041aad67e31324cb5d4d5e2cfce1706d4f Mon Sep 17 00:00:00 2001
From: Sylwester Nawrockis.nawro...@samsung.com
Date: Thu, 19 Sep 2013 23:52:04 +0200
Subject: [PATCH] omap3isp: Pass NULL device pointer to clk_register()
Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com
---
drivers/media/platform/omap3isp/isp.c | 15 ++-
drivers/media/platform/omap3isp/isp.h |1 +
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index df3a0ec..d7f3c98 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -290,9 +290,11 @@ static int isp_xclk_init(struct isp_device *isp)
struct clk_init_data init;
unsigned int i;
+for (i = 0; i ARRAY_SIZE(isp-xclks); ++i)
+isp-xclks[i] = ERR_PTR(-EINVAL);
+
for (i = 0; i ARRAY_SIZE(isp-xclks); ++i) {
struct isp_xclk *xclk =isp-xclks[i];
-struct clk *clk;
xclk-isp = isp;
xclk-id = i == 0 ? ISP_XCLK_A : ISP_XCLK_B;
@@ -306,9 +308,9 @@ static int isp_xclk_init(struct isp_device *isp)
xclk-hw.init =init;
-clk = devm_clk_register(isp-dev,xclk-hw);
-if (IS_ERR(clk))
-return PTR_ERR(clk);
+xclk-clk = clk_register(NULL,xclk-hw);
+if (IS_ERR(xclk-clk))
+return PTR_ERR(xclk-clk);
if (pdata-xclks[i].con_id == NULL
pdata-xclks[i].dev_id == NULL)
@@ -320,7 +322,7 @@ static int isp_xclk_init(struct isp_device *isp)
xclk-lookup-con_id = pdata-xclks[i].con_id;
xclk-lookup-dev_id = pdata-xclks[i].dev_id;
-xclk-lookup-clk = clk;
+xclk-lookup-clk = xclk-clk;
clkdev_add(xclk-lookup);
}
@@ -335,6 +337,9 @@ static void isp_xclk_cleanup(struct isp_device *isp)
for (i = 0; i ARRAY_SIZE(isp-xclks); ++i) {
struct isp_xclk *xclk =isp-xclks[i];
+if (!IS_ERR(xclk-clk))
+clk_unregister(xclk-clk);
+
if (xclk-lookup)
clkdev_drop(xclk-lookup);
}
diff --git a/drivers/media/platform/omap3isp/isp.h
b/drivers/media/platform/omap3isp/isp.h
index cd3eff4..1498f2b 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -135,6 +135,7 @@ struct isp_xclk {
struct isp_device *isp;
struct clk_hw hw;
struct clk_lookup *lookup;
+struct clk *clk;
enum isp_xclk_id id;
spinlock_t lock;/* Protects enabled and divider */
-8--
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 1/7] media: V4L2: add temporary clock helpers
Quoting Barry Song (2013-04-11 01:59:28) 2013/4/11 Guennadi Liakhovetski g.liakhovet...@gmx.de: On Thu, 11 Apr 2013, Barry Song wrote: 2013/4/11 Guennadi Liakhovetski g.liakhovet...@gmx.de: Hi Barry On Thu, 11 Apr 2013, Barry Song wrote: Hi Guennadi, Typical video devices like camera sensors require an external clock source. Many such devices cannot even access their hardware registers without a running clock. These clock sources should be controlled by their consumers. This should be performed, using the generic clock framework. Unfortunately so far only very few systems have been ported to that framework. This patch adds a set of temporary helpers, mimicking the generic clock API, to V4L2. Platforms, adopting the clock API, should switch to using it. Eventually this temporary API should be removed. Signed-off-by: Guennadi Liakhovetski g.liakhovetski@xx --- for your patch 1/8 and 3/8, i think it makes a lot of senses to let the object manages its own clock by itself. is it possible for us to implement v4l2-clk.c directly as an instance of standard clk driver for those systems which don't have generic clock, and remove the V4L2 clock APIs like v4l2_clk_get, v4l2_clk_enable from the first day? i mean v4l2-clk.c becomes a temp and fake clock controller driver. finally, after people have generically clk, remove it. I don't think you can force-enable the CFF on systems, that don't support it, e.g. PXA. yes. we can. clock is only a framework, has it any limitation to implement a driver instance on any platform? So, you enable CFF, it provides its own clk_* implementation like clk_get_rate() etc. Now, PXA already has it defined in arch/arm/mach-pxa/clock.c. Don't think this is going to fly. agree. Hi, I came into this thread late and don't have the actual patches in my inbox for review. That said, I don't understand why V4L2 cares about the clk framework *implementation*? The clk.h api is the same for platforms using the common struct clk and those still using the legacy method of defining their own struct clk. If drivers are only consumers of the clk.h api then the implementation underneath should not matter. Regards, Mike Thanks Guennadi people have tried to move to common clk and generic framework for a long time, now you still try to provide a v4l2 specific clock APIs, it just makes v4l2 unacceptable and much complex. Thanks Guennadi v8: Updated both (C) dates drivers/media/v4l2-core/Makefile |2 +- drivers/media/v4l2-core/v4l2-clk.c | 177 include/media/v4l2-clk.h | 54 +++ 3 files changed, 232 insertions(+), 1 deletions(-) create mode 100644 drivers/media/v4l2-core/v4l2-clk.c create mode 100644 include/media/v4l2-clk.h diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile index aa50c46..628c630 100644 --- a/drivers/media/v4l2-core/Makefile +++ b/drivers/media/v4l2-core/Makefile @@ -5,7 +5,7 @@ tuner-objs := tuner-core.o videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \ - v4l2-event.o v4l2-ctrls.o v4l2-subdev.o + v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o ifeq ($(CONFIG_COMPAT),y) videodev-objs += v4l2-compat-ioctl32.o endif diff --git a/drivers/media/v4l2-core/v4l2-clk.c b/drivers/media/v4l2-core/v4l2-clk.c new file mode 100644 index 000..d7cc13e --- /dev/null +++ b/drivers/media/v4l2-core/v4l2-clk.c @@ -0,0 +1,177 @@ -barry -barry ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] omap3isp: Use the common clock framework
Quoting Laurent Pinchart (2013-04-04 04:51:40)
Expose the two ISP external clocks XCLKA and XCLKB as common clocks for
subdev drivers.
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Acked-by: Mike Turquette mturque...@linaro.org
Regards,
Mike
---
drivers/media/platform/omap3isp/isp.c | 270
--
drivers/media/platform/omap3isp/isp.h | 22 ++-
include/media/omap3isp.h | 10 +-
3 files changed, 218 insertions(+), 84 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index 6e5ad8e..694470d 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -55,6 +55,7 @@
#include asm/cacheflush.h
#include linux/clk.h
+#include linux/clkdev.h
#include linux/delay.h
#include linux/device.h
#include linux/dma-mapping.h
@@ -148,6 +149,194 @@ void omap3isp_flush(struct isp_device *isp)
isp_reg_readl(isp, OMAP3_ISP_IOMEM_MAIN, ISP_REVISION);
}
+/*
-
+ * XCLK
+ */
+
+#define to_isp_xclk(_hw) container_of(_hw, struct isp_xclk, hw)
+
+static void isp_xclk_update(struct isp_xclk *xclk, u32 divider)
+{
+ switch (xclk-id) {
+ case ISP_XCLK_A:
+ isp_reg_clr_set(xclk-isp, OMAP3_ISP_IOMEM_MAIN,
ISP_TCTRL_CTRL,
+ ISPTCTRL_CTRL_DIVA_MASK,
+ divider ISPTCTRL_CTRL_DIVA_SHIFT);
+ break;
+ case ISP_XCLK_B:
+ isp_reg_clr_set(xclk-isp, OMAP3_ISP_IOMEM_MAIN,
ISP_TCTRL_CTRL,
+ ISPTCTRL_CTRL_DIVB_MASK,
+ divider ISPTCTRL_CTRL_DIVB_SHIFT);
+ break;
+ }
+}
+
+static int isp_xclk_prepare(struct clk_hw *hw)
+{
+ struct isp_xclk *xclk = to_isp_xclk(hw);
+
+ omap3isp_get(xclk-isp);
+
+ return 0;
+}
+
+static void isp_xclk_unprepare(struct clk_hw *hw)
+{
+ struct isp_xclk *xclk = to_isp_xclk(hw);
+
+ omap3isp_put(xclk-isp);
+}
+
+static int isp_xclk_enable(struct clk_hw *hw)
+{
+ struct isp_xclk *xclk = to_isp_xclk(hw);
+ unsigned long flags;
+
+ spin_lock_irqsave(xclk-lock, flags);
+ isp_xclk_update(xclk, xclk-divider);
+ xclk-enabled = true;
+ spin_unlock_irqrestore(xclk-lock, flags);
+
+ return 0;
+}
+
+static void isp_xclk_disable(struct clk_hw *hw)
+{
+ struct isp_xclk *xclk = to_isp_xclk(hw);
+ unsigned long flags;
+
+ spin_lock_irqsave(xclk-lock, flags);
+ isp_xclk_update(xclk, 0);
+ xclk-enabled = false;
+ spin_unlock_irqrestore(xclk-lock, flags);
+}
+
+static unsigned long isp_xclk_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct isp_xclk *xclk = to_isp_xclk(hw);
+
+ return parent_rate / xclk-divider;
+}
+
+static u32 isp_xclk_calc_divider(unsigned long *rate, unsigned long
parent_rate)
+{
+ u32 divider;
+
+ if (*rate = parent_rate) {
+ *rate = parent_rate;
+ return ISPTCTRL_CTRL_DIV_BYPASS;
+ }
+
+ divider = DIV_ROUND_CLOSEST(parent_rate, *rate);
+ if (divider = ISPTCTRL_CTRL_DIV_BYPASS)
+ divider = ISPTCTRL_CTRL_DIV_BYPASS - 1;
+
+ *rate = parent_rate / divider;
+ return divider;
+}
+
+static long isp_xclk_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ isp_xclk_calc_divider(rate, *parent_rate);
+ return rate;
+}
+
+static int isp_xclk_set_rate(struct clk_hw *hw, unsigned long rate,
+unsigned long parent_rate)
+{
+ struct isp_xclk *xclk = to_isp_xclk(hw);
+ unsigned long flags;
+ u32 divider;
+
+ divider = isp_xclk_calc_divider(rate, parent_rate);
+
+ spin_lock_irqsave(xclk-lock, flags);
+
+ xclk-divider = divider;
+ if (xclk-enabled)
+ isp_xclk_update(xclk, divider);
+
+ spin_unlock_irqrestore(xclk-lock, flags);
+
+ dev_dbg(xclk-isp-dev, %s: cam_xclk%c set to %lu Hz (div %u)\n,
+ __func__, xclk-id == ISP_XCLK_A ? 'a' : 'b', rate, divider);
+ return 0;
+}
+
+static const struct clk_ops isp_xclk_ops = {
+ .prepare = isp_xclk_prepare,
+ .unprepare = isp_xclk_unprepare,
+ .enable = isp_xclk_enable,
+ .disable = isp_xclk_disable,
+ .recalc_rate = isp_xclk_recalc_rate,
+ .round_rate = isp_xclk_round_rate,
+ .set_rate = isp_xclk_set_rate,
+};
+
+static const char *isp_xclk_parent_name = cam_mclk;
+
+static int isp_xclk_init(struct isp_device *isp)
+{
+ struct isp_platform_data *pdata = isp-pdata;
+ unsigned int i
Re: [PATCH] [media] s5p-mfc: Change MFC clock reference w.r.t Common Clock Framework
Quoting Prasanna Kumar (2013-03-25 22:20:51)
From: Prasanna Kumar prasanna...@samsung.com
According to Common Clock framework , modified the method of getting
clock for MFC Block.
Signed-off-by: Prasanna Kumar prasanna...@samsung.com
---
drivers/media/platform/s5p-mfc/s5p_mfc_pm.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
index 6aa38a5..b8ac8f6 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c
@@ -50,7 +50,7 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev)
goto err_p_ip_clk;
}
- pm-clock = clk_get(dev-plat_dev-dev, dev-variant-mclk_name);
+ pm-clock = clk_get_parent(pm-clock_gate);
Ok, I'll bite. Why make this change? Was there an issue using
clkdev/clk_get to get the clock you needed?
Regards,
Mike
if (IS_ERR(pm-clock)) {
mfc_err(Failed to get MFC clock\n);
ret = PTR_ERR(pm-clock);
--
1.7.5.4
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] OMAP3 ISP: Simplify clock usage
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 mturque...@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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
