| Message ID | 1363382873-20077-2-git-send-email-g.liakhovetski@gmx.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Hi Guennadi, On 03/15/2013 10:27 PM, Guennadi Liakhovetski wrote: > 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 It seems there is much more significant progress in adding CCF support to various platforms than with our temporary clocks API ;) $ git show linux-next/master:drivers/clk/Makefile ... # SoCs specific obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o obj-$(CONFIG_ARCH_NOMADIK) += clk-nomadik.o obj-$(CONFIG_ARCH_HIGHBANK) += clk-highbank.o obj-$(CONFIG_ARCH_MXS) += mxs/ obj-$(CONFIG_ARCH_SOCFPGA) += socfpga/ obj-$(CONFIG_PLAT_SPEAR) += spear/ obj-$(CONFIG_ARCH_U300) += clk-u300.o obj-$(CONFIG_COMMON_CLK_VERSATILE) += versatile/ obj-$(CONFIG_ARCH_PRIMA2) += clk-prima2.o obj-$(CONFIG_PLAT_ORION) += mvebu/ ifeq ($(CONFIG_COMMON_CLK), y) obj-$(CONFIG_ARCH_MMP) += mmp/ endif obj-$(CONFIG_MACH_LOONGSON1) += clk-ls1x.o obj-$(CONFIG_ARCH_U8500) += ux500/ obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o obj-$(CONFIG_ARCH_ZYNQ) += clk-zynq.o obj-$(CONFIG_ARCH_TEGRA) += tegra/ obj-$(CONFIG_PLAT_SAMSUNG) += samsung/ obj-$(CONFIG_X86) += x86/ # Chip specific obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o obj-$(CONFIG_COMMON_CLK_MAX77686) += clk-max77686.o obj-$(CONFIG_CLK_TWL6040) += clk-twl6040.o > 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@gmx.de> > --- > > v6: changed clock name to just<I2C adapter ID>-<I2C address> to avoid > having to wait for an I2C subdevice driver to probe. Added a subdevice > pointer to struct v4l2_clk for subdevice and bridge binding. > > drivers/media/v4l2-core/Makefile | 2 +- > drivers/media/v4l2-core/v4l2-clk.c | 184 ++++++++++++++++++++++++++++++++++++ > include/media/v4l2-clk.h | 55 +++++++++++ > 3 files changed, 240 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 a9d3552..aea7aea 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 0000000..3505972 > --- /dev/null > +++ b/drivers/media/v4l2-core/v4l2-clk.c > @@ -0,0 +1,184 @@ > +/* > + * V4L2 clock service > + * > + * Copyright (C) 2012, Guennadi Liakhovetski<g.liakhovetski@gmx.de> 2013 ? > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include<linux/atomic.h> > +#include<linux/errno.h> > +#include<linux/list.h> > +#include<linux/module.h> > +#include<linux/mutex.h> > +#include<linux/string.h> > + > +#include<media/v4l2-clk.h> > +#include<media/v4l2-subdev.h> > + > +static DEFINE_MUTEX(clk_lock); > +static LIST_HEAD(clk_list); > + > +static struct v4l2_clk *v4l2_clk_find(const struct v4l2_subdev *sd, > + const char *dev_id, const char *id) > +{ > + struct v4l2_clk *clk; > + > + list_for_each_entry(clk,&clk_list, list) { > + if (!sd || !(sd->flags& V4L2_SUBDEV_FL_IS_I2C)) { > + if (strcmp(dev_id, clk->dev_id)) > + continue; > + } else { > + char *i2c = strstr(dev_id, clk->dev_id); > + if (!i2c || i2c == dev_id || *(i2c - 1) != ' ') > + continue; > + } > + > + if (!id || !clk->id || !strcmp(clk->id, id)) > + return clk; > + } > + > + return ERR_PTR(-ENODEV); > +} > + > +struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id) > +{ > + struct v4l2_clk *clk; > + > + mutex_lock(&clk_lock); > + clk = v4l2_clk_find(sd, sd->name, id); Couldn't we just pass the I2C client's struct device name to this function ? And if the host driver that registers a clock for its sub-device knows the type of device (I2C, SPI client, etc.) why we need to even bother with checking the subdev/bus type in v4l2_clk_find() function above, when the host could properly format dev_id when it registers a clock ? Then the subdev would just pass its struct device pointer to this API to find its clock. What am I missing here ? > + if (!IS_ERR(clk)&& !try_module_get(clk->ops->owner)) > + clk = ERR_PTR(-ENODEV); > + mutex_unlock(&clk_lock); > + > + if (!IS_ERR(clk)) { > + clk->subdev = sd; Why is this needed ? It seems a strange addition that might potentially make transition to the common clocks API more difficult. > + atomic_inc(&clk->use_count); > + } > + > + return clk; > +} > +EXPORT_SYMBOL(v4l2_clk_get); > + [...] > +unsigned long v4l2_clk_get_rate(struct v4l2_clk *clk) > +{ > + if (!clk->ops->get_rate) > + return -ENOSYS; I guess we should just WARN if this callback is null and return 0 or return value type of this function needs to be 'long'. Otherwise we'll get insanely large frequency value by casting this error code to unsigned long. > + > + return clk->ops->get_rate(clk); > +} > +EXPORT_SYMBOL(v4l2_clk_get_rate); > + > +int v4l2_clk_set_rate(struct v4l2_clk *clk, unsigned long rate) > +{ > + if (!clk->ops->set_rate) > + return -ENOSYS; > + > + return clk->ops->set_rate(clk, rate); > +} > +EXPORT_SYMBOL(v4l2_clk_set_rate); > + > +struct v4l2_clk *v4l2_clk_register(const struct v4l2_clk_ops *ops, > + const char *dev_id, > + const char *id, void *priv) > +{ > + struct v4l2_clk *clk; > + int ret; > + > + if (!ops || !dev_id) > + return ERR_PTR(-EINVAL); > + > + clk = kzalloc(sizeof(struct v4l2_clk), GFP_KERNEL); > + if (!clk) > + return ERR_PTR(-ENOMEM); > + > + clk->id = kstrdup(id, GFP_KERNEL); > + clk->dev_id = kstrdup(dev_id, GFP_KERNEL); > + if ((id&& !clk->id) || !clk->dev_id) { > + ret = -ENOMEM; > + goto ealloc; > + } > + clk->ops = ops; > + clk->priv = priv; > + atomic_set(&clk->use_count, 0); > + mutex_init(&clk->lock); > + > + mutex_lock(&clk_lock); > + if (!IS_ERR(v4l2_clk_find(NULL, dev_id, id))) { > + mutex_unlock(&clk_lock); > + ret = -EEXIST; > + goto eexist; > + } > + list_add_tail(&clk->list,&clk_list); > + mutex_unlock(&clk_lock); > + > + return clk; > + > +eexist: > +ealloc: nit: Why not to make the label's name dependant on what code follows a label and avoid this double labelling ? In general it seems a good idea to name the label after where we jump to, not where we start from. > + kfree(clk->id); > + kfree(clk->dev_id); > + kfree(clk); > + return ERR_PTR(ret); > +} > +EXPORT_SYMBOL(v4l2_clk_register); -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sylwester Thanks for reviewing. On Mon, 18 Mar 2013, Sylwester Nawrocki wrote: > Hi Guennadi, > > On 03/15/2013 10:27 PM, Guennadi Liakhovetski wrote: > > 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 > > It seems there is much more significant progress in adding CCF support to > various platforms than with our temporary clocks API ;) > > $ git show linux-next/master:drivers/clk/Makefile > ... > # SoCs specific > obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o > obj-$(CONFIG_ARCH_NOMADIK) += clk-nomadik.o > obj-$(CONFIG_ARCH_HIGHBANK) += clk-highbank.o > obj-$(CONFIG_ARCH_MXS) += mxs/ > obj-$(CONFIG_ARCH_SOCFPGA) += socfpga/ > obj-$(CONFIG_PLAT_SPEAR) += spear/ > obj-$(CONFIG_ARCH_U300) += clk-u300.o > obj-$(CONFIG_COMMON_CLK_VERSATILE) += versatile/ > obj-$(CONFIG_ARCH_PRIMA2) += clk-prima2.o > obj-$(CONFIG_PLAT_ORION) += mvebu/ > ifeq ($(CONFIG_COMMON_CLK), y) > obj-$(CONFIG_ARCH_MMP) += mmp/ > endif > obj-$(CONFIG_MACH_LOONGSON1) += clk-ls1x.o > obj-$(CONFIG_ARCH_U8500) += ux500/ > obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o > obj-$(CONFIG_ARCH_ZYNQ) += clk-zynq.o > obj-$(CONFIG_ARCH_TEGRA) += tegra/ > obj-$(CONFIG_PLAT_SAMSUNG) += samsung/ > > obj-$(CONFIG_X86) += x86/ > > # Chip specific > obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o > obj-$(CONFIG_COMMON_CLK_MAX77686) += clk-max77686.o > obj-$(CONFIG_CLK_TWL6040) += clk-twl6040.o > > > > 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@gmx.de> > > --- > > > > v6: changed clock name to just<I2C adapter ID>-<I2C address> to avoid > > having to wait for an I2C subdevice driver to probe. Added a subdevice > > pointer to struct v4l2_clk for subdevice and bridge binding. > > > > drivers/media/v4l2-core/Makefile | 2 +- > > drivers/media/v4l2-core/v4l2-clk.c | 184 > > ++++++++++++++++++++++++++++++++++++ > > include/media/v4l2-clk.h | 55 +++++++++++ > > 3 files changed, 240 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 a9d3552..aea7aea 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 0000000..3505972 > > --- /dev/null > > +++ b/drivers/media/v4l2-core/v4l2-clk.c > > @@ -0,0 +1,184 @@ > > +/* > > + * V4L2 clock service > > + * > > + * Copyright (C) 2012, Guennadi Liakhovetski<g.liakhovetski@gmx.de> > > 2013 ? > > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + */ > > + > > +#include<linux/atomic.h> > > +#include<linux/errno.h> > > +#include<linux/list.h> > > +#include<linux/module.h> > > +#include<linux/mutex.h> > > +#include<linux/string.h> > > + > > +#include<media/v4l2-clk.h> > > +#include<media/v4l2-subdev.h> > > + > > +static DEFINE_MUTEX(clk_lock); > > +static LIST_HEAD(clk_list); > > + > > +static struct v4l2_clk *v4l2_clk_find(const struct v4l2_subdev *sd, > > + const char *dev_id, const char *id) > > +{ > > + struct v4l2_clk *clk; > > + > > + list_for_each_entry(clk,&clk_list, list) { > > + if (!sd || !(sd->flags& V4L2_SUBDEV_FL_IS_I2C)) { > > + if (strcmp(dev_id, clk->dev_id)) > > + continue; > > + } else { > > + char *i2c = strstr(dev_id, clk->dev_id); > > + if (!i2c || i2c == dev_id || *(i2c - 1) != ' ') > > + continue; > > + } > > + > > + if (!id || !clk->id || !strcmp(clk->id, id)) > > + return clk; > > + } > > + > > + return ERR_PTR(-ENODEV); > > +} > > + > > +struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id) > > +{ > > + struct v4l2_clk *clk; > > + > > + mutex_lock(&clk_lock); > > + clk = v4l2_clk_find(sd, sd->name, id); > > Couldn't we just pass the I2C client's struct device name to this function ? Certainly not. This is a part of the generic V4L2 clock API, it's not I2C specific. > And if the host driver that registers a clock for its sub-device knows the > type > of device (I2C, SPI client, etc.) why we need to even bother with checking the > subdev/bus type in v4l2_clk_find() function above, when the host could > properly > format dev_id when it registers a clock ? This has been discussed. The host doesn't know the name of the I2C driver, that would attach to this subdevice at the time, it registers the clock. This is the easiest way to oversome this problem. > Then the subdev would just pass its > struct device pointer to this API to find its clock. What am I missing here ? I don't think there's a 1-to-1 correspondence between devices and V4L2 subdevices. > > + if (!IS_ERR(clk)&& !try_module_get(clk->ops->owner)) > > + clk = ERR_PTR(-ENODEV); > > + mutex_unlock(&clk_lock); > > + > > + if (!IS_ERR(clk)) { > > + clk->subdev = sd; > > Why is this needed ? It seems a strange addition that might potentially > make transition to the common clocks API more difficult. We got rid of the v4l2_clk_bind() function and the .bind() callback. Now I need a pointer to subdevice _before_ v4l2_clk_register() (former v4l2_clk_bound()), that's why I have to store it here. > > + atomic_inc(&clk->use_count); > > + } > > + > > + return clk; > > +} > > +EXPORT_SYMBOL(v4l2_clk_get); > > + > [...] > > +unsigned long v4l2_clk_get_rate(struct v4l2_clk *clk) > > +{ > > + if (!clk->ops->get_rate) > > + return -ENOSYS; > > I guess we should just WARN if this callback is null and return 0 > or return value type of this function needs to be 'long'. Otherwise > we'll get insanely large frequency value by casting this error code > to unsigned long. Right, we either have to make the return type signed or return 0 if unavailable, the above in inconsistent. I'm ok either way. > > + > > + return clk->ops->get_rate(clk); > > +} > > +EXPORT_SYMBOL(v4l2_clk_get_rate); > > + > > +int v4l2_clk_set_rate(struct v4l2_clk *clk, unsigned long rate) > > +{ > > + if (!clk->ops->set_rate) > > + return -ENOSYS; > > + > > + return clk->ops->set_rate(clk, rate); > > +} > > +EXPORT_SYMBOL(v4l2_clk_set_rate); > > + > > +struct v4l2_clk *v4l2_clk_register(const struct v4l2_clk_ops *ops, > > + const char *dev_id, > > + const char *id, void *priv) > > +{ > > + struct v4l2_clk *clk; > > + int ret; > > + > > + if (!ops || !dev_id) > > + return ERR_PTR(-EINVAL); > > + > > + clk = kzalloc(sizeof(struct v4l2_clk), GFP_KERNEL); > > + if (!clk) > > + return ERR_PTR(-ENOMEM); > > + > > + clk->id = kstrdup(id, GFP_KERNEL); > > + clk->dev_id = kstrdup(dev_id, GFP_KERNEL); > > + if ((id&& !clk->id) || !clk->dev_id) { > > + ret = -ENOMEM; > > + goto ealloc; > > + } > > + clk->ops = ops; > > + clk->priv = priv; > > + atomic_set(&clk->use_count, 0); > > + mutex_init(&clk->lock); > > + > > + mutex_lock(&clk_lock); > > + if (!IS_ERR(v4l2_clk_find(NULL, dev_id, id))) { > > + mutex_unlock(&clk_lock); > > + ret = -EEXIST; > > + goto eexist; > > + } > > + list_add_tail(&clk->list,&clk_list); > > + mutex_unlock(&clk_lock); > > + > > + return clk; > > + > > +eexist: > > +ealloc: > > nit: Why not to make the label's name dependant on what code follows > a label and avoid this double labelling ? In general it seems a good > idea to name the label after where we jump to, not where we start from. Thanks, I think, this is a matter of personal preference, each method has its advantages, this is my current preference. > > + kfree(clk->id); > > + kfree(clk->dev_id); > > + kfree(clk); > > + return ERR_PTR(ret); > > +} > > +EXPORT_SYMBOL(v4l2_clk_register); Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Guennadi, On 03/19/2013 08:32 AM, Guennadi Liakhovetski wrote: > On Mon, 18 Mar 2013, Sylwester Nawrocki wrote: >> On 03/15/2013 10:27 PM, Guennadi Liakhovetski wrote: [...] >>> diff --git a/drivers/media/v4l2-core/v4l2-clk.c >>> b/drivers/media/v4l2-core/v4l2-clk.c >>> new file mode 100644 >>> index 0000000..3505972 >>> --- /dev/null >>> +++ b/drivers/media/v4l2-core/v4l2-clk.c >>> @@ -0,0 +1,184 @@ >>> +/* >>> + * V4L2 clock service >>> + * >>> + * Copyright (C) 2012, Guennadi Liakhovetski<g.liakhovetski@gmx.de> >> >> 2013 ? >> >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License version 2 as >>> + * published by the Free Software Foundation. >>> + */ >>> + >>> +#include<linux/atomic.h> >>> +#include<linux/errno.h> >>> +#include<linux/list.h> >>> +#include<linux/module.h> >>> +#include<linux/mutex.h> >>> +#include<linux/string.h> >>> + >>> +#include<media/v4l2-clk.h> >>> +#include<media/v4l2-subdev.h> >>> + >>> +static DEFINE_MUTEX(clk_lock); >>> +static LIST_HEAD(clk_list); >>> + >>> +static struct v4l2_clk *v4l2_clk_find(const struct v4l2_subdev *sd, >>> + const char *dev_id, const char *id) >>> +{ >>> + struct v4l2_clk *clk; >>> + >>> + list_for_each_entry(clk,&clk_list, list) { >>> + if (!sd || !(sd->flags& V4L2_SUBDEV_FL_IS_I2C)) { >>> + if (strcmp(dev_id, clk->dev_id)) >>> + continue; >>> + } else { >>> + char *i2c = strstr(dev_id, clk->dev_id); >>> + if (!i2c || i2c == dev_id || *(i2c - 1) != ' ') >>> + continue; >>> + } >>> + >>> + if (!id || !clk->id || !strcmp(clk->id, id)) >>> + return clk; >>> + } >>> + >>> + return ERR_PTR(-ENODEV); >>> +} >>> + >>> +struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id) >>> +{ >>> + struct v4l2_clk *clk; >>> + >>> + mutex_lock(&clk_lock); >>> + clk = v4l2_clk_find(sd, sd->name, id); >> >> Couldn't we just pass the I2C client's struct device name to this function ? > > Certainly not. This is a part of the generic V4L2 clock API, it's not I2C > specific. I have been thinking about something like dev_name(sd->dev), but struct v4l2_subdev doesn't have struct device associated with it. >> And if the host driver that registers a clock for its sub-device knows the >> type >> of device (I2C, SPI client, etc.) why we need to even bother with checking the >> subdev/bus type in v4l2_clk_find() function above, when the host could >> properly >> format dev_id when it registers a clock ? > > This has been discussed. The host doesn't know the name of the I2C driver, > that would attach to this subdevice at the time, it registers the clock. > This is the easiest way to oversome this problem. OK, thanks for reminding. It would be probably much easier to associate the clock with struct device, not with subdev driver. Devices have more clear naming rules (at last I2C, SPI clients). And most host drivers already have information about I2C bus id, just I2C slave address would need to be passed to the host driver so it can register a clock for its subdev. >> Then the subdev would just pass its >> struct device pointer to this API to find its clock. What am I missing here ? > > I don't think there's a 1-to-1 correspondence between devices and V4L2 > subdevices. I would expect at least a subdev that needs a clock to have struct device associated with it. It would be also much easier this way to use generic clocks API in the device tree instantiated systems. >>> + if (!IS_ERR(clk)&& !try_module_get(clk->ops->owner)) >>> + clk = ERR_PTR(-ENODEV); >>> + mutex_unlock(&clk_lock); >>> + >>> + if (!IS_ERR(clk)) { >>> + clk->subdev = sd; >> >> Why is this needed ? It seems a strange addition that might potentially >> make transition to the common clocks API more difficult. > > We got rid of the v4l2_clk_bind() function and the .bind() callback. Now I > need a pointer to subdevice _before_ v4l2_clk_register() (former > v4l2_clk_bound()), that's why I have to store it here. Hmm, sorry, I'm not following. How can we store a subdev pointer in the clock data structure that has not been registered yet and thus cannot be found with v4l2_clk_find() ? >>> + atomic_inc(&clk->use_count); >>> + } >>> + >>> + return clk; >>> +} >>> +EXPORT_SYMBOL(v4l2_clk_get); -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 19 Mar 2013, Sylwester Nawrocki wrote: > >>> + if (!IS_ERR(clk)&& !try_module_get(clk->ops->owner)) > >>> + clk = ERR_PTR(-ENODEV); > >>> + mutex_unlock(&clk_lock); > >>> + > >>> + if (!IS_ERR(clk)) { > >>> + clk->subdev = sd; > >> > >> Why is this needed ? It seems a strange addition that might potentially > >> make transition to the common clocks API more difficult. > > > > We got rid of the v4l2_clk_bind() function and the .bind() callback. Now I > > need a pointer to subdevice _before_ v4l2_clk_register() (former > > v4l2_clk_bound()), that's why I have to store it here. > > Hmm, sorry, I'm not following. How can we store a subdev pointer in the clock > data structure that has not been registered yet and thus cannot be found > with v4l2_clk_find() ? sorry, I meant v4l2_async_subdev_register(), not v4l2_clk_register(), my mistake. And I meant v4l2_async_subdev_bind(), v4l2_async_subdev_unbind(). Before we had in the subdev driver (see imx074 example) /* Tell the bridge the subdevice is about to bind */ v4l2_async_subdev_bind(); /* get a clock */ clk = v4l2_clk_get(); if (IS_ERR(clk)) return -EPROBE_DEFER; /* * enable the clock - this needs a subdev pointer, that we passed * to the bridge with v4l2_async_subdev_bind() above */ v4l2_clk_enable(clk); do_probe(); v4l2_clk_disable(clk); /* inform the bridge: binding successful */ v4l2_async_subdev_bound(); Now we have just /* get a clock */ clk = v4l2_clk_get(); if (IS_ERR(clk)) return -EPROBE_DEFER; /* * enable the clock - this needs a subdev pointer, that we stored * in the clock object for the bridge driver to use with * v4l2_clk_get() above */ v4l2_clk_enable(clk); do_probe(); v4l2_clk_disable(clk); /* inform the bridge: binding successful */ v4l2_async_subdev_bound(); Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Guennadi, On Sat, Mar 16, 2013 at 2:57 AM, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > 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@gmx.de> > --- > > v6: changed clock name to just <I2C adapter ID>-<I2C address> to avoid > having to wait for an I2C subdevice driver to probe. Added a subdevice > pointer to struct v4l2_clk for subdevice and bridge binding. > > drivers/media/v4l2-core/Makefile | 2 +- > drivers/media/v4l2-core/v4l2-clk.c | 184 ++++++++++++++++++++++++++++++++++++ > include/media/v4l2-clk.h | 55 +++++++++++ > 3 files changed, 240 insertions(+), 1 deletions(-) > create mode 100644 drivers/media/v4l2-core/v4l2-clk.c > create mode 100644 include/media/v4l2-clk.h > While trying out this patch I got following error (using 3.9-rc3):- drivers/media/v4l2-core/v4l2-clk.c: In function 'v4l2_clk_register': drivers/media/v4l2-core/v4l2-clk.c:134:2: error: implicit declaration of function 'kzalloc' drivers/media/v4l2-core/v4l2-clk.c:134:6: warning: assignment makes pointer from integer without a cast drivers/media/v4l2-core/v4l2-clk.c:162:2: error: implicit declaration of function 'kfree' make[3]: *** [drivers/media/v4l2-core/v4l2-clk.o] Error 1 make[2]: *** [drivers/media/v4l2-core] Error 2 Regards, --Prabhakar -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 21 Mar 2013 13:49:50 +0530 Prabhakar Lad <prabhakar.csengg@gmail.com> wrote: ... > > drivers/media/v4l2-core/Makefile | 2 +- > > drivers/media/v4l2-core/v4l2-clk.c | 184 ++++++++++++++++++++++++++++++++++++ > > include/media/v4l2-clk.h | 55 +++++++++++ > > 3 files changed, 240 insertions(+), 1 deletions(-) > > create mode 100644 drivers/media/v4l2-core/v4l2-clk.c > > create mode 100644 include/media/v4l2-clk.h > > > While trying out this patch I got following error (using 3.9-rc3):- > > drivers/media/v4l2-core/v4l2-clk.c: In function 'v4l2_clk_register': > drivers/media/v4l2-core/v4l2-clk.c:134:2: error: implicit declaration > of function 'kzalloc' > drivers/media/v4l2-core/v4l2-clk.c:134:6: warning: assignment makes > pointer from integer without a cast > drivers/media/v4l2-core/v4l2-clk.c:162:2: error: implicit declaration > of function 'kfree' > make[3]: *** [drivers/media/v4l2-core/v4l2-clk.o] Error 1 > make[2]: *** [drivers/media/v4l2-core] Error 2 please try adding #include <linux/slab.h> in the affected file. Thanks, Anatolij -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Anatolij, On Thu, Mar 21, 2013 at 2:40 PM, Anatolij Gustschin <agust@denx.de> wrote: > On Thu, 21 Mar 2013 13:49:50 +0530 > Prabhakar Lad <prabhakar.csengg@gmail.com> wrote: > ... >> > drivers/media/v4l2-core/Makefile | 2 +- >> > drivers/media/v4l2-core/v4l2-clk.c | 184 ++++++++++++++++++++++++++++++++++++ >> > include/media/v4l2-clk.h | 55 +++++++++++ >> > 3 files changed, 240 insertions(+), 1 deletions(-) >> > create mode 100644 drivers/media/v4l2-core/v4l2-clk.c >> > create mode 100644 include/media/v4l2-clk.h >> > >> While trying out this patch I got following error (using 3.9-rc3):- >> >> drivers/media/v4l2-core/v4l2-clk.c: In function 'v4l2_clk_register': >> drivers/media/v4l2-core/v4l2-clk.c:134:2: error: implicit declaration >> of function 'kzalloc' >> drivers/media/v4l2-core/v4l2-clk.c:134:6: warning: assignment makes >> pointer from integer without a cast >> drivers/media/v4l2-core/v4l2-clk.c:162:2: error: implicit declaration >> of function 'kfree' >> make[3]: *** [drivers/media/v4l2-core/v4l2-clk.o] Error 1 >> make[2]: *** [drivers/media/v4l2-core] Error 2 > > please try adding > > #include <linux/slab.h> > > in the affected file. > Thanks for pointing it :), I have already fixed it. Just wanted to point Guennadi so that he could fix it in his next version. Cheers, --Prabhakar > Thanks, > > Anatolij -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Tuesday 19 March 2013 10:52:29 Sylwester Nawrocki wrote: > On 03/19/2013 08:32 AM, Guennadi Liakhovetski wrote: > > On Mon, 18 Mar 2013, Sylwester Nawrocki wrote: > >> On 03/15/2013 10:27 PM, Guennadi Liakhovetski wrote: > [...] > > >>> diff --git a/drivers/media/v4l2-core/v4l2-clk.c > >>> b/drivers/media/v4l2-core/v4l2-clk.c > >>> new file mode 100644 > >>> index 0000000..3505972 > >>> --- /dev/null > >>> +++ b/drivers/media/v4l2-core/v4l2-clk.c [snip] > >>> +static struct v4l2_clk *v4l2_clk_find(const struct v4l2_subdev *sd, > >>> + const char *dev_id, const char *id) > >>> +{ > >>> + struct v4l2_clk *clk; > >>> + > >>> + list_for_each_entry(clk,&clk_list, list) { > >>> + if (!sd || !(sd->flags& V4L2_SUBDEV_FL_IS_I2C)) { > >>> + if (strcmp(dev_id, clk->dev_id)) > >>> + continue; > >>> + } else { > >>> + char *i2c = strstr(dev_id, clk->dev_id); > >>> + if (!i2c || i2c == dev_id || *(i2c - 1) != ' ') > >>> + continue; > >>> + } > >>> + > >>> + if (!id || !clk->id || !strcmp(clk->id, id)) > >>> + return clk; > >>> + } > >>> + > >>> + return ERR_PTR(-ENODEV); > >>> +} > >>> + > >>> +struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id) > >>> +{ > >>> + struct v4l2_clk *clk; > >>> + > >>> + mutex_lock(&clk_lock); > >>> + clk = v4l2_clk_find(sd, sd->name, id); > >> > >> Couldn't we just pass the I2C client's struct device name to this > >> function ? > > > > Certainly not. This is a part of the generic V4L2 clock API, it's not I2C > > specific. > > I have been thinking about something like dev_name(sd->dev), but struct > v4l2_subdev doesn't have struct device associated with it. But the caller of v4l2_clk_get() will have a struct device * available, so I think it would make sense to pass a struct device * to v4l2_clk_get() and call dev_name() on it internally. Clocks would be registered with the device ID as well. This requires knowledge of the clock user device in the clock provider, but no knowledge of the clock user module name. > >> And if the host driver that registers a clock for its sub-device knows > >> the type of device (I2C, SPI client, etc.) why we need to even bother > >> with checking the subdev/bus type in v4l2_clk_find() function above, when > >> the host could properly format dev_id when it registers a clock ? > > > > This has been discussed. The host doesn't know the name of the I2C driver, > > that would attach to this subdevice at the time, it registers the clock. > > This is the easiest way to oversome this problem. > > OK, thanks for reminding. It would be probably much easier to associate > the clock with struct device, not with subdev driver. Devices have more > clear naming rules (at last I2C, SPI clients). And most host drivers > already have information about I2C bus id, just I2C slave address would > need to be passed to the host driver so it can register a clock for its > subdev. > > >> Then the subdev would just pass its struct device pointer to this API to > >> find its clock. What am I missing here ? > > I don't think there's a 1-to-1 correspondence between devices and V4L2 > > subdevices. > > I would expect at least a subdev that needs a clock to have struct device > associated with it. It would be also much easier this way to use generic > clocks API in the device tree instantiated systems. I agree. Let's not overdesign the v4l2-clock API to support clock users without a struct device. This is a transitional API only after all. > >>> + if (!IS_ERR(clk)&& !try_module_get(clk->ops->owner)) > >>> + clk = ERR_PTR(-ENODEV); > >>> + mutex_unlock(&clk_lock); > >>> + > >>> + if (!IS_ERR(clk)) { > >>> + clk->subdev = sd; > >> > >> Why is this needed ? It seems a strange addition that might potentially > >> make transition to the common clocks API more difficult. > > > > We got rid of the v4l2_clk_bind() function and the .bind() callback. Now I > > need a pointer to subdevice _before_ v4l2_clk_register() (former > > v4l2_clk_bound()), that's why I have to store it here. > > Hmm, sorry, I'm not following. How can we store a subdev pointer in the > clock data structure that has not been registered yet and thus cannot be > found with v4l2_clk_find() ? > > >>> + atomic_inc(&clk->use_count); > >>> + } > >>> + > >>> + return clk; > >>> +} > >>> +EXPORT_SYMBOL(v4l2_clk_get);
Hi Guennadi, On Tuesday 19 March 2013 11:27:56 Guennadi Liakhovetski wrote: > On Tue, 19 Mar 2013, Sylwester Nawrocki wrote: > > >>> + if (!IS_ERR(clk)&& !try_module_get(clk->ops->owner)) > > >>> + clk = ERR_PTR(-ENODEV); > > >>> + mutex_unlock(&clk_lock); > > >>> + > > >>> + if (!IS_ERR(clk)) { > > >>> + clk->subdev = sd; > > >> > > >> Why is this needed ? It seems a strange addition that might potentially > > >> make transition to the common clocks API more difficult. > > > > > > We got rid of the v4l2_clk_bind() function and the .bind() callback. Now > > > I need a pointer to subdevice _before_ v4l2_clk_register() (former > > > v4l2_clk_bound()), that's why I have to store it here. > > > > Hmm, sorry, I'm not following. How can we store a subdev pointer in the > > clock data structure that has not been registered yet and thus cannot be > > found with v4l2_clk_find() ? > > sorry, I meant v4l2_async_subdev_register(), not v4l2_clk_register(), my > mistake. And I meant v4l2_async_subdev_bind(), v4l2_async_subdev_unbind(). > Before we had in the subdev driver (see imx074 example) > > /* Tell the bridge the subdevice is about to bind */ > v4l2_async_subdev_bind(); > > /* get a clock */ > clk = v4l2_clk_get(); > if (IS_ERR(clk)) > return -EPROBE_DEFER; > > /* > * enable the clock - this needs a subdev pointer, that we passed > * to the bridge with v4l2_async_subdev_bind() above > */ > v4l2_clk_enable(clk); > do_probe(); > v4l2_clk_disable(clk); > > /* inform the bridge: binding successful */ > v4l2_async_subdev_bound(); > > Now we have just > > /* get a clock */ > clk = v4l2_clk_get(); > if (IS_ERR(clk)) > return -EPROBE_DEFER; > > /* > * enable the clock - this needs a subdev pointer, that we stored > * in the clock object for the bridge driver to use with > * v4l2_clk_get() above > */ > v4l2_clk_enable(clk); > do_probe(); > v4l2_clk_disable(clk); I'm sorry, but I still don't understand why you need a pointer to the subdev in the clock provider implementation of v4l2_clk_enable/disable() :-) > /* inform the bridge: binding successful */ > v4l2_async_subdev_bound();
On Mon, 18 Mar 2013, Sylwester Nawrocki wrote: [snip] > > +unsigned long v4l2_clk_get_rate(struct v4l2_clk *clk) > > +{ > > + if (!clk->ops->get_rate) > > + return -ENOSYS; > > I guess we should just WARN if this callback is null and return 0 > or return value type of this function needs to be 'long'. Otherwise > we'll get insanely large frequency value by casting this error code > to unsigned long. Comparing to the CCF: AFAICS, they do the same, you're supposed to use IS_ERR_VALUE() on the clock rate, obtained from clk_get_rate(). Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/08/2013 12:36 PM, Guennadi Liakhovetski wrote: > On Mon, 18 Mar 2013, Sylwester Nawrocki wrote: > > [snip] > >>> +unsigned long v4l2_clk_get_rate(struct v4l2_clk *clk) >>> +{ >>> + if (!clk->ops->get_rate) >>> + return -ENOSYS; >> >> I guess we should just WARN if this callback is null and return 0 >> or return value type of this function needs to be 'long'. Otherwise >> we'll get insanely large frequency value by casting this error code >> to unsigned long. > > Comparing to the CCF: AFAICS, they do the same, you're supposed to use > IS_ERR_VALUE() on the clock rate, obtained from clk_get_rate(). Hmm, that might work. Nevertheless I consider that a pretty horrible pattern. I couldn't find any references to IS_ERR_VALUE in the clock code though. Only that 0 is returned when clk is NULL. Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-sh" 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/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile index a9d3552..aea7aea 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 0000000..3505972 --- /dev/null +++ b/drivers/media/v4l2-core/v4l2-clk.c @@ -0,0 +1,184 @@ +/* + * V4L2 clock service + * + * Copyright (C) 2012, Guennadi Liakhovetski <g.liakhovetski@gmx.de> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/atomic.h> +#include <linux/errno.h> +#include <linux/list.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/string.h> + +#include <media/v4l2-clk.h> +#include <media/v4l2-subdev.h> + +static DEFINE_MUTEX(clk_lock); +static LIST_HEAD(clk_list); + +static struct v4l2_clk *v4l2_clk_find(const struct v4l2_subdev *sd, + const char *dev_id, const char *id) +{ + struct v4l2_clk *clk; + + list_for_each_entry(clk, &clk_list, list) { + if (!sd || !(sd->flags & V4L2_SUBDEV_FL_IS_I2C)) { + if (strcmp(dev_id, clk->dev_id)) + continue; + } else { + char *i2c = strstr(dev_id, clk->dev_id); + if (!i2c || i2c == dev_id || *(i2c - 1) != ' ') + continue; + } + + if (!id || !clk->id || !strcmp(clk->id, id)) + return clk; + } + + return ERR_PTR(-ENODEV); +} + +struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id) +{ + struct v4l2_clk *clk; + + mutex_lock(&clk_lock); + clk = v4l2_clk_find(sd, sd->name, id); + + if (!IS_ERR(clk) && !try_module_get(clk->ops->owner)) + clk = ERR_PTR(-ENODEV); + mutex_unlock(&clk_lock); + + if (!IS_ERR(clk)) { + clk->subdev = sd; + atomic_inc(&clk->use_count); + } + + return clk; +} +EXPORT_SYMBOL(v4l2_clk_get); + +void v4l2_clk_put(struct v4l2_clk *clk) +{ + if (!IS_ERR(clk)) { + atomic_dec(&clk->use_count); + module_put(clk->ops->owner); + } +} +EXPORT_SYMBOL(v4l2_clk_put); + +int v4l2_clk_enable(struct v4l2_clk *clk) +{ + int ret; + mutex_lock(&clk->lock); + if (++clk->enable == 1 && clk->ops->enable) { + ret = clk->ops->enable(clk); + if (ret < 0) + clk->enable--; + } else { + ret = 0; + } + mutex_unlock(&clk->lock); + return ret; +} +EXPORT_SYMBOL(v4l2_clk_enable); + +void v4l2_clk_disable(struct v4l2_clk *clk) +{ + int enable; + + mutex_lock(&clk->lock); + enable = --clk->enable; + if (WARN(enable < 0, "Unbalanced %s() on %s:%s!\n", __func__, + clk->dev_id, clk->id)) + clk->enable++; + else if (!enable && clk->ops->disable) + clk->ops->disable(clk); + mutex_unlock(&clk->lock); +} +EXPORT_SYMBOL(v4l2_clk_disable); + +unsigned long v4l2_clk_get_rate(struct v4l2_clk *clk) +{ + if (!clk->ops->get_rate) + return -ENOSYS; + + return clk->ops->get_rate(clk); +} +EXPORT_SYMBOL(v4l2_clk_get_rate); + +int v4l2_clk_set_rate(struct v4l2_clk *clk, unsigned long rate) +{ + if (!clk->ops->set_rate) + return -ENOSYS; + + return clk->ops->set_rate(clk, rate); +} +EXPORT_SYMBOL(v4l2_clk_set_rate); + +struct v4l2_clk *v4l2_clk_register(const struct v4l2_clk_ops *ops, + const char *dev_id, + const char *id, void *priv) +{ + struct v4l2_clk *clk; + int ret; + + if (!ops || !dev_id) + return ERR_PTR(-EINVAL); + + clk = kzalloc(sizeof(struct v4l2_clk), GFP_KERNEL); + if (!clk) + return ERR_PTR(-ENOMEM); + + clk->id = kstrdup(id, GFP_KERNEL); + clk->dev_id = kstrdup(dev_id, GFP_KERNEL); + if ((id && !clk->id) || !clk->dev_id) { + ret = -ENOMEM; + goto ealloc; + } + clk->ops = ops; + clk->priv = priv; + atomic_set(&clk->use_count, 0); + mutex_init(&clk->lock); + + mutex_lock(&clk_lock); + if (!IS_ERR(v4l2_clk_find(NULL, dev_id, id))) { + mutex_unlock(&clk_lock); + ret = -EEXIST; + goto eexist; + } + list_add_tail(&clk->list, &clk_list); + mutex_unlock(&clk_lock); + + return clk; + +eexist: +ealloc: + kfree(clk->id); + kfree(clk->dev_id); + kfree(clk); + return ERR_PTR(ret); +} +EXPORT_SYMBOL(v4l2_clk_register); + +void v4l2_clk_unregister(struct v4l2_clk *clk) +{ + if (WARN(atomic_read(&clk->use_count), + "%s(): Refusing to unregister ref-counted %s:%s clock!\n", + __func__, clk->dev_id, clk->id)) + return; + + mutex_lock(&clk_lock); + list_del(&clk->list); + mutex_unlock(&clk_lock); + + kfree(clk->id); + kfree(clk->dev_id); + kfree(clk); +} +EXPORT_SYMBOL(v4l2_clk_unregister); diff --git a/include/media/v4l2-clk.h b/include/media/v4l2-clk.h new file mode 100644 index 0000000..6d3c2e2 --- /dev/null +++ b/include/media/v4l2-clk.h @@ -0,0 +1,55 @@ +/* + * V4L2 clock service + * + * Copyright (C) 2012, Guennadi Liakhovetski <g.liakhovetski@gmx.de> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * ATTENTION: This is a temporary API and it shall be replaced by the generic + * clock API, when the latter becomes widely available. + */ + +#ifndef MEDIA_V4L2_CLK_H +#define MEDIA_V4L2_CLK_H + +#include <linux/atomic.h> +#include <linux/list.h> +#include <linux/mutex.h> + +struct module; +struct v4l2_subdev; + +struct v4l2_clk { + struct list_head list; + const struct v4l2_clk_ops *ops; + const char *dev_id; + const char *id; + int enable; + struct mutex lock; /* Protect the enable count */ + atomic_t use_count; + struct v4l2_subdev *subdev; + void *priv; +}; + +struct v4l2_clk_ops { + struct module *owner; + int (*enable)(struct v4l2_clk *clk); + void (*disable)(struct v4l2_clk *clk); + unsigned long (*get_rate)(struct v4l2_clk *clk); + int (*set_rate)(struct v4l2_clk *clk, unsigned long); +}; + +struct v4l2_clk *v4l2_clk_register(const struct v4l2_clk_ops *ops, + const char *dev_name, + const char *name, void *priv); +void v4l2_clk_unregister(struct v4l2_clk *clk); +struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id); +void v4l2_clk_put(struct v4l2_clk *clk); +int v4l2_clk_enable(struct v4l2_clk *clk); +void v4l2_clk_disable(struct v4l2_clk *clk); +unsigned long v4l2_clk_get_rate(struct v4l2_clk *clk); +int v4l2_clk_set_rate(struct v4l2_clk *clk, unsigned long rate); + +#endif
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@gmx.de> --- v6: changed clock name to just <I2C adapter ID>-<I2C address> to avoid having to wait for an I2C subdevice driver to probe. Added a subdevice pointer to struct v4l2_clk for subdevice and bridge binding. drivers/media/v4l2-core/Makefile | 2 +- drivers/media/v4l2-core/v4l2-clk.c | 184 ++++++++++++++++++++++++++++++++++++ include/media/v4l2-clk.h | 55 +++++++++++ 3 files changed, 240 insertions(+), 1 deletions(-) create mode 100644 drivers/media/v4l2-core/v4l2-clk.c create mode 100644 include/media/v4l2-clk.h