| Message ID | 1373021725-14006-1-git-send-email-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Laurent, On 07/05/2013 12:55 PM, Laurent Pinchart wrote: > Configure the device external clock using the common clock framework > instead of a board code callback function. > > Signed-off-by: Laurent Pinchart<laurent.pinchart@ideasonboard.com> > --- > drivers/media/i2c/mt9v032.c | 16 ++++++++++------ > include/media/mt9v032.h | 4 ---- > 2 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c > index 60c6f67..7b30640 100644 > --- a/drivers/media/i2c/mt9v032.c > +++ b/drivers/media/i2c/mt9v032.c > @@ -12,6 +12,7 @@ > * published by the Free Software Foundation. > */ > > +#include<linux/clk.h> > #include<linux/delay.h> > #include<linux/i2c.h> > #include<linux/log2.h> > @@ -135,6 +136,8 @@ struct mt9v032 { > struct mutex power_lock; > int power_count; > > + struct clk *clk; > + > struct mt9v032_platform_data *pdata; > > u32 sysclk; > @@ -219,10 +222,8 @@ static int mt9v032_power_on(struct mt9v032 *mt9v032) > struct i2c_client *client = v4l2_get_subdevdata(&mt9v032->subdev); > int ret; > > - if (mt9v032->pdata->set_clock) { > - mt9v032->pdata->set_clock(&mt9v032->subdev, mt9v032->sysclk); > - udelay(1); > - } > + clk_prepare_enable(mt9v032->clk); > + udelay(1); > > /* Reset the chip and stop data read out */ > ret = mt9v032_write(client, MT9V032_RESET, 1); > @@ -238,8 +239,7 @@ static int mt9v032_power_on(struct mt9v032 *mt9v032) > > static void mt9v032_power_off(struct mt9v032 *mt9v032) > { > - if (mt9v032->pdata->set_clock) > - mt9v032->pdata->set_clock(&mt9v032->subdev, 0); > + clk_disable_unprepare(mt9v032->clk); > } > > static int __mt9v032_set_power(struct mt9v032 *mt9v032, bool on) > @@ -748,6 +748,10 @@ static int mt9v032_probe(struct i2c_client *client, > if (!mt9v032) > return -ENOMEM; > > + mt9v032->clk = devm_clk_get(&client->dev, NULL); > + if (IS_ERR(mt9v032->clk)) > + return PTR_ERR(mt9v032->clk); > + > mutex_init(&mt9v032->power_lock); > mt9v032->pdata = pdata; > > diff --git a/include/media/mt9v032.h b/include/media/mt9v032.h > index 78fd39e..12175a6 100644 > --- a/include/media/mt9v032.h > +++ b/include/media/mt9v032.h > @@ -1,13 +1,9 @@ > #ifndef _MEDIA_MT9V032_H > #define _MEDIA_MT9V032_H > > -struct v4l2_subdev; > - > struct mt9v032_platform_data { > unsigned int clk_pol:1; > > - void (*set_clock)(struct v4l2_subdev *subdev, unsigned int rate); > - > const s64 *link_freqs; > s64 link_def_freq; > }; Is there clk_put() somewhere in this patch ? I would expect it somewhere around driver remove() callback, but can't see it. :-/ Regards, Sylwester -- 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 Sylwester, On Friday 26 July 2013 15:11:08 Sylwester Nawrocki wrote: > On 07/05/2013 12:55 PM, Laurent Pinchart wrote: > > Configure the device external clock using the common clock framework > > instead of a board code callback function. > > > > Signed-off-by: Laurent Pinchart<laurent.pinchart@ideasonboard.com> > > --- > > > > drivers/media/i2c/mt9v032.c | 16 ++++++++++------ > > include/media/mt9v032.h | 4 ---- > > 2 files changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c > > index 60c6f67..7b30640 100644 > > --- a/drivers/media/i2c/mt9v032.c > > +++ b/drivers/media/i2c/mt9v032.c > > @@ -12,6 +12,7 @@ > > * published by the Free Software Foundation. > > */ > > > > +#include<linux/clk.h> > > #include<linux/delay.h> > > #include<linux/i2c.h> > > #include<linux/log2.h> > > @@ -135,6 +136,8 @@ struct mt9v032 { > > struct mutex power_lock; > > int power_count; > > > > + struct clk *clk; > > + > > struct mt9v032_platform_data *pdata; > > > > u32 sysclk; > > @@ -219,10 +222,8 @@ static int mt9v032_power_on(struct mt9v032 *mt9v032) > > struct i2c_client *client = v4l2_get_subdevdata(&mt9v032->subdev); > > int ret; > > > > - if (mt9v032->pdata->set_clock) { > > - mt9v032->pdata->set_clock(&mt9v032->subdev, mt9v032->sysclk); > > - udelay(1); > > - } > > + clk_prepare_enable(mt9v032->clk); > > + udelay(1); > > > > /* Reset the chip and stop data read out */ > > ret = mt9v032_write(client, MT9V032_RESET, 1); > > @@ -238,8 +239,7 @@ static int mt9v032_power_on(struct mt9v032 *mt9v032) > > > > static void mt9v032_power_off(struct mt9v032 *mt9v032) > > { > > - if (mt9v032->pdata->set_clock) > > - mt9v032->pdata->set_clock(&mt9v032->subdev, 0); > > + clk_disable_unprepare(mt9v032->clk); > > } > > > > static int __mt9v032_set_power(struct mt9v032 *mt9v032, bool on) > > @@ -748,6 +748,10 @@ static int mt9v032_probe(struct i2c_client *client, > > if (!mt9v032) > > return -ENOMEM; > > > > + mt9v032->clk = devm_clk_get(&client->dev, NULL); > > + if (IS_ERR(mt9v032->clk)) > > + return PTR_ERR(mt9v032->clk); > > + > > mutex_init(&mt9v032->power_lock); > > mt9v032->pdata = pdata; > > > > diff --git a/include/media/mt9v032.h b/include/media/mt9v032.h > > index 78fd39e..12175a6 100644 > > --- a/include/media/mt9v032.h > > +++ b/include/media/mt9v032.h > > @@ -1,13 +1,9 @@ > > #ifndef _MEDIA_MT9V032_H > > #define _MEDIA_MT9V032_H > > > > -struct v4l2_subdev; > > - > > struct mt9v032_platform_data { > > unsigned int clk_pol:1; > > - void (*set_clock)(struct v4l2_subdev *subdev, unsigned int rate); > > - > > const s64 *link_freqs; > > s64 link_def_freq; > > }; > > Is there clk_put() somewhere in this patch ? I would expect it somewhere > around driver remove() callback, but can't see it. :-/ There's *devm_*clk_get() instead :-)
Hi Laurent, On 07/26/2013 03:15 PM, Laurent Pinchart wrote: > On Friday 26 July 2013 15:11:08 Sylwester Nawrocki wrote: >> On 07/05/2013 12:55 PM, Laurent Pinchart wrote: >>> Configure the device external clock using the common clock framework >>> instead of a board code callback function. >>> >>> Signed-off-by: Laurent Pinchart<laurent.pinchart@ideasonboard.com> >>> --- >>> >>> drivers/media/i2c/mt9v032.c | 16 ++++++++++------ >>> include/media/mt9v032.h | 4 ---- >>> 2 files changed, 10 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c >>> index 60c6f67..7b30640 100644 >>> --- a/drivers/media/i2c/mt9v032.c >>> +++ b/drivers/media/i2c/mt9v032.c >>> @@ -12,6 +12,7 @@ >>> * published by the Free Software Foundation. >>> */ >>> >>> +#include<linux/clk.h> >>> #include<linux/delay.h> >>> #include<linux/i2c.h> >>> #include<linux/log2.h> >>> @@ -135,6 +136,8 @@ struct mt9v032 { >>> struct mutex power_lock; >>> int power_count; >>> >>> + struct clk *clk; >>> + >>> struct mt9v032_platform_data *pdata; >>> >>> u32 sysclk; >>> @@ -219,10 +222,8 @@ static int mt9v032_power_on(struct mt9v032 *mt9v032) >>> struct i2c_client *client = v4l2_get_subdevdata(&mt9v032->subdev); >>> int ret; >>> >>> - if (mt9v032->pdata->set_clock) { >>> - mt9v032->pdata->set_clock(&mt9v032->subdev, mt9v032->sysclk); >>> - udelay(1); >>> - } >>> + clk_prepare_enable(mt9v032->clk); >>> + udelay(1); >>> >>> /* Reset the chip and stop data read out */ >>> ret = mt9v032_write(client, MT9V032_RESET, 1); >>> @@ -238,8 +239,7 @@ static int mt9v032_power_on(struct mt9v032 *mt9v032) >>> >>> static void mt9v032_power_off(struct mt9v032 *mt9v032) >>> { >>> - if (mt9v032->pdata->set_clock) >>> - mt9v032->pdata->set_clock(&mt9v032->subdev, 0); >>> + clk_disable_unprepare(mt9v032->clk); >>> } >>> >>> static int __mt9v032_set_power(struct mt9v032 *mt9v032, bool on) >>> @@ -748,6 +748,10 @@ static int mt9v032_probe(struct i2c_client *client, >>> if (!mt9v032) >>> return -ENOMEM; >>> >>> + mt9v032->clk = devm_clk_get(&client->dev, NULL); >>> + if (IS_ERR(mt9v032->clk)) >>> + return PTR_ERR(mt9v032->clk); >>> + >>> mutex_init(&mt9v032->power_lock); >>> mt9v032->pdata = pdata; >>> >>> diff --git a/include/media/mt9v032.h b/include/media/mt9v032.h >>> index 78fd39e..12175a6 100644 >>> --- a/include/media/mt9v032.h >>> +++ b/include/media/mt9v032.h >>> @@ -1,13 +1,9 @@ >>> #ifndef _MEDIA_MT9V032_H >>> #define _MEDIA_MT9V032_H >>> >>> -struct v4l2_subdev; >>> - >>> struct mt9v032_platform_data { >>> unsigned int clk_pol:1; >>> - void (*set_clock)(struct v4l2_subdev *subdev, unsigned int rate); >>> - >>> const s64 *link_freqs; >>> s64 link_def_freq; >>> }; >> >> Is there clk_put() somewhere in this patch ? I would expect it somewhere >> around driver remove() callback, but can't see it. :-/ > > There's *devm_*clk_get() instead :-) Ah, I knew I must have been forgetting or overlooking something! ;) Do you rely on the fact that __clk_get()/__clk_put() doesn't get reference on the clock supplier module (to avoid locking modules in memory) ? I was planning on adding module_get()/module_put() inside __clk_get()/__clk_out() for the common clock API implementation. Regards, Sylwester -- 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 Sylwester, On Friday 26 July 2013 15:36:22 Sylwester Nawrocki wrote: > On 07/26/2013 03:15 PM, Laurent Pinchart wrote: > > On Friday 26 July 2013 15:11:08 Sylwester Nawrocki wrote: > >> On 07/05/2013 12:55 PM, Laurent Pinchart wrote: > >>> Configure the device external clock using the common clock framework > >>> instead of a board code callback function. > >>> > >>> Signed-off-by: Laurent Pinchart<laurent.pinchart@ideasonboard.com> > >>> --- > >>> > >>> drivers/media/i2c/mt9v032.c | 16 ++++++++++------ > >>> include/media/mt9v032.h | 4 ---- > >>> 2 files changed, 10 insertions(+), 10 deletions(-) > >>> > >>> diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c > >>> index 60c6f67..7b30640 100644 > >>> --- a/drivers/media/i2c/mt9v032.c > >>> +++ b/drivers/media/i2c/mt9v032.c > >>> @@ -12,6 +12,7 @@ > >>> * published by the Free Software Foundation. > >>> */ > >>> > >>> +#include<linux/clk.h> > >>> #include<linux/delay.h> > >>> #include<linux/i2c.h> > >>> #include<linux/log2.h> > >>> @@ -135,6 +136,8 @@ struct mt9v032 { > >>> struct mutex power_lock; > >>> int power_count; > >>> > >>> + struct clk *clk; > >>> + > >>> struct mt9v032_platform_data *pdata; > >>> > >>> u32 sysclk; > >>> @@ -219,10 +222,8 @@ static int mt9v032_power_on(struct mt9v032 > >>> *mt9v032) > >>> struct i2c_client *client = v4l2_get_subdevdata(&mt9v032->subdev); > >>> int ret; > >>> > >>> - if (mt9v032->pdata->set_clock) { > >>> - mt9v032->pdata->set_clock(&mt9v032->subdev, mt9v032->sysclk); > >>> - udelay(1); > >>> - } > >>> + clk_prepare_enable(mt9v032->clk); > >>> + udelay(1); > >>> > >>> /* Reset the chip and stop data read out */ > >>> ret = mt9v032_write(client, MT9V032_RESET, 1); > >>> @@ -238,8 +239,7 @@ static int mt9v032_power_on(struct mt9v032 *mt9v032) > >>> > >>> static void mt9v032_power_off(struct mt9v032 *mt9v032) > >>> { > >>> - if (mt9v032->pdata->set_clock) > >>> - mt9v032->pdata->set_clock(&mt9v032->subdev, 0); > >>> + clk_disable_unprepare(mt9v032->clk); > >>> } > >>> > >>> static int __mt9v032_set_power(struct mt9v032 *mt9v032, bool on) > >>> @@ -748,6 +748,10 @@ static int mt9v032_probe(struct i2c_client *client, > >>> if (!mt9v032) > >>> return -ENOMEM; > >>> > >>> + mt9v032->clk = devm_clk_get(&client->dev, NULL); > >>> + if (IS_ERR(mt9v032->clk)) > >>> + return PTR_ERR(mt9v032->clk); > >>> + > >>> mutex_init(&mt9v032->power_lock); > >>> mt9v032->pdata = pdata; > >>> > >>> diff --git a/include/media/mt9v032.h b/include/media/mt9v032.h > >>> index 78fd39e..12175a6 100644 > >>> --- a/include/media/mt9v032.h > >>> +++ b/include/media/mt9v032.h > >>> @@ -1,13 +1,9 @@ > >>> #ifndef _MEDIA_MT9V032_H > >>> #define _MEDIA_MT9V032_H > >>> > >>> -struct v4l2_subdev; > >>> - > >>> struct mt9v032_platform_data { > >>> unsigned int clk_pol:1; > >>> - void (*set_clock)(struct v4l2_subdev *subdev, unsigned int rate); > >>> - > >>> const s64 *link_freqs; > >>> s64 link_def_freq; > >>> }; > >> > >> Is there clk_put() somewhere in this patch ? I would expect it somewhere > >> around driver remove() callback, but can't see it. :-/ > > > > There's *devm_*clk_get() instead :-) > > Ah, I knew I must have been forgetting or overlooking something! ;) > > Do you rely on the fact that __clk_get()/__clk_put() doesn't get reference > on the clock supplier module (to avoid locking modules in memory) ? I was > planning on adding module_get()/module_put() inside __clk_get()/__clk_out() > for the common clock API implementation. I'm currently relying on that, but I'm aware it's not a good idea. We need to find a solution to fix the problem in the context of the v4l2-async framework.
Hi Laurent, On 07/26/2013 05:55 PM, Laurent Pinchart wrote: > On Friday 26 July 2013 15:36:22 Sylwester Nawrocki wrote: >> On 07/26/2013 03:15 PM, Laurent Pinchart wrote: >>> On Friday 26 July 2013 15:11:08 Sylwester Nawrocki wrote: >>>> On 07/05/2013 12:55 PM, Laurent Pinchart wrote: >>>>> Configure the device external clock using the common clock framework >>>>> instead of a board code callback function. >>>>> >>>>> Signed-off-by: Laurent Pinchart<laurent.pinchart@ideasonboard.com> >>>>> --- >>>>> >>>>> drivers/media/i2c/mt9v032.c | 16 ++++++++++------ >>>>> include/media/mt9v032.h | 4 ---- >>>>> 2 files changed, 10 insertions(+), 10 deletions(-) >>>>> >>>>> diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c >>>>> index 60c6f67..7b30640 100644 >>>>> --- a/drivers/media/i2c/mt9v032.c >>>>> +++ b/drivers/media/i2c/mt9v032.c >>>>> @@ -12,6 +12,7 @@ >>>>> * published by the Free Software Foundation. >>>>> */ >>>>> >>>>> +#include<linux/clk.h> >>>>> #include<linux/delay.h> >>>>> #include<linux/i2c.h> >>>>> #include<linux/log2.h> >>>>> @@ -135,6 +136,8 @@ struct mt9v032 { >>>>> struct mutex power_lock; >>>>> int power_count; >>>>> >>>>> + struct clk *clk; >>>>> + >>>>> struct mt9v032_platform_data *pdata; >>>>> >>>>> u32 sysclk; >>>>> @@ -219,10 +222,8 @@ static int mt9v032_power_on(struct mt9v032 >>>>> *mt9v032) >>>>> struct i2c_client *client = v4l2_get_subdevdata(&mt9v032->subdev); >>>>> int ret; >>>>> >>>>> - if (mt9v032->pdata->set_clock) { >>>>> - mt9v032->pdata->set_clock(&mt9v032->subdev, mt9v032->sysclk); >>>>> - udelay(1); >>>>> - } >>>>> + clk_prepare_enable(mt9v032->clk); >>>>> + udelay(1); >>>>> >>>>> /* Reset the chip and stop data read out */ >>>>> ret = mt9v032_write(client, MT9V032_RESET, 1); >>>>> @@ -238,8 +239,7 @@ static int mt9v032_power_on(struct mt9v032 *mt9v032) >>>>> >>>>> static void mt9v032_power_off(struct mt9v032 *mt9v032) >>>>> { >>>>> - if (mt9v032->pdata->set_clock) >>>>> - mt9v032->pdata->set_clock(&mt9v032->subdev, 0); >>>>> + clk_disable_unprepare(mt9v032->clk); >>>>> } >>>>> >>>>> static int __mt9v032_set_power(struct mt9v032 *mt9v032, bool on) >>>>> @@ -748,6 +748,10 @@ static int mt9v032_probe(struct i2c_client *client, >>>>> if (!mt9v032) >>>>> return -ENOMEM; >>>>> >>>>> + mt9v032->clk = devm_clk_get(&client->dev, NULL); >>>>> + if (IS_ERR(mt9v032->clk)) >>>>> + return PTR_ERR(mt9v032->clk); >>>>> + >>>>> mutex_init(&mt9v032->power_lock); >>>>> mt9v032->pdata = pdata; >>>>> >>>>> diff --git a/include/media/mt9v032.h b/include/media/mt9v032.h >>>>> index 78fd39e..12175a6 100644 >>>>> --- a/include/media/mt9v032.h >>>>> +++ b/include/media/mt9v032.h >>>>> @@ -1,13 +1,9 @@ >>>>> #ifndef _MEDIA_MT9V032_H >>>>> #define _MEDIA_MT9V032_H >>>>> >>>>> -struct v4l2_subdev; >>>>> - >>>>> struct mt9v032_platform_data { >>>>> unsigned int clk_pol:1; >>>>> - void (*set_clock)(struct v4l2_subdev *subdev, unsigned int rate); >>>>> - >>>>> const s64 *link_freqs; >>>>> s64 link_def_freq; >>>>> }; >>>> >>>> Is there clk_put() somewhere in this patch ? I would expect it somewhere >>>> around driver remove() callback, but can't see it. :-/ >>> >>> There's *devm_*clk_get() instead :-) >> >> Ah, I knew I must have been forgetting or overlooking something! ;) >> >> Do you rely on the fact that __clk_get()/__clk_put() doesn't get reference >> on the clock supplier module (to avoid locking modules in memory) ? I was >> planning on adding module_get()/module_put() inside __clk_get()/__clk_out() >> for the common clock API implementation. > > I'm currently relying on that, but I'm aware it's not a good idea. We need to > find a solution to fix the problem in the context of the v4l2-async framework. There are few possible options I can see could be a resolution for that: 1. making the common clock API not getting reference on the clock supplier module; then dynamic clock deregistration needs to be handled, which has its own share of problems; 2. using sysfs unbind attribute of the subdev and/or the host drivers to release the circular references or create some additional sysfs entry for this - however requiring additional actions from user space to do something that worked before without them can be simply considered as an regression; 3. not keeping reference to a clock all times only when it is actively used, e.g. in subdev's s_power handler; this is in practice what v4l2-clk does, just would be coded using standard clk API calls; Any of these options isn't ideal but 3) seems to me most reasonable of them. I thought it would be better than using the v4l2-clk API, which is supposed to be a temporary measure only, for platforms that already have the common clock API support. I was wondering whether you don't need to also set the clock's frequency in this patch. I guess the sensor driver should at least call clk_get_rate() to see if current frequency is suitable for the device ? Regards, Sylwester -- 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 Sylwester, On Wednesday 07 August 2013 17:03:18 Sylwester Nawrocki wrote: > On 07/26/2013 05:55 PM, Laurent Pinchart wrote: > > On Friday 26 July 2013 15:36:22 Sylwester Nawrocki wrote: > >> On 07/26/2013 03:15 PM, Laurent Pinchart wrote: > >>> On Friday 26 July 2013 15:11:08 Sylwester Nawrocki wrote: > >>>> On 07/05/2013 12:55 PM, Laurent Pinchart wrote: > >>>>> Configure the device external clock using the common clock framework > >>>>> instead of a board code callback function. > >>>>> > >>>>> Signed-off-by: Laurent Pinchart<laurent.pinchart@ideasonboard.com> > >>>>> --- > >>>>> drivers/media/i2c/mt9v032.c | 16 ++++++++++------ > >>>>> include/media/mt9v032.h | 4 ---- > >>>>> 2 files changed, 10 insertions(+), 10 deletions(-) [snip] > >> Do you rely on the fact that __clk_get()/__clk_put() doesn't get > >> reference on the clock supplier module (to avoid locking modules in > >> memory) ? I was planning on adding module_get()/module_put() inside > >> __clk_get()/__clk_out() for the common clock API implementation. > > > > I'm currently relying on that, but I'm aware it's not a good idea. We need > > to find a solution to fix the problem in the context of the v4l2-async > > framework. > > There are few possible options I can see could be a resolution for that: > > 1. making the common clock API not getting reference on the clock supplier > module; then dynamic clock deregistration needs to be handled, which has > its own share of problems; > > 2. using sysfs unbind attribute of the subdev and/or the host drivers to > release the circular references or create some additional sysfs entry > for this - however requiring additional actions from user space to do > something that worked before without them can be simply considered as > an regression; Well, we've never had working DT support for those systems, so that's hardly a regression :-) > 3. not keeping reference to a clock all times only when it is actively > used, e.g. in subdev's s_power handler; this is in practice what > v4l2-clk does, just would be coded using standard clk API calls; > > Any of these options isn't ideal but 3) seems to me most reasonable of them. For now it might be, but I'm not sure whether it would scale. We will likely have similar issues with other kind of resources in the future. I would be enclined to investigate the second option. > I thought it would be better than using the v4l2-clk API, which is supposed > to be a temporary measure only, for platforms that already have the common > clock API support. > > I was wondering whether you don't need to also set the clock's frequency in > this patch. I guess the sensor driver should at least call clk_get_rate() to > see if current frequency is suitable for the device ? Yes indeed. I'm not sure how I've missed that. I'll fix it.
diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c index 60c6f67..7b30640 100644 --- a/drivers/media/i2c/mt9v032.c +++ b/drivers/media/i2c/mt9v032.c @@ -12,6 +12,7 @@ * published by the Free Software Foundation. */ +#include <linux/clk.h> #include <linux/delay.h> #include <linux/i2c.h> #include <linux/log2.h> @@ -135,6 +136,8 @@ struct mt9v032 { struct mutex power_lock; int power_count; + struct clk *clk; + struct mt9v032_platform_data *pdata; u32 sysclk; @@ -219,10 +222,8 @@ static int mt9v032_power_on(struct mt9v032 *mt9v032) struct i2c_client *client = v4l2_get_subdevdata(&mt9v032->subdev); int ret; - if (mt9v032->pdata->set_clock) { - mt9v032->pdata->set_clock(&mt9v032->subdev, mt9v032->sysclk); - udelay(1); - } + clk_prepare_enable(mt9v032->clk); + udelay(1); /* Reset the chip and stop data read out */ ret = mt9v032_write(client, MT9V032_RESET, 1); @@ -238,8 +239,7 @@ static int mt9v032_power_on(struct mt9v032 *mt9v032) static void mt9v032_power_off(struct mt9v032 *mt9v032) { - if (mt9v032->pdata->set_clock) - mt9v032->pdata->set_clock(&mt9v032->subdev, 0); + clk_disable_unprepare(mt9v032->clk); } static int __mt9v032_set_power(struct mt9v032 *mt9v032, bool on) @@ -748,6 +748,10 @@ static int mt9v032_probe(struct i2c_client *client, if (!mt9v032) return -ENOMEM; + mt9v032->clk = devm_clk_get(&client->dev, NULL); + if (IS_ERR(mt9v032->clk)) + return PTR_ERR(mt9v032->clk); + mutex_init(&mt9v032->power_lock); mt9v032->pdata = pdata; diff --git a/include/media/mt9v032.h b/include/media/mt9v032.h index 78fd39e..12175a6 100644 --- a/include/media/mt9v032.h +++ b/include/media/mt9v032.h @@ -1,13 +1,9 @@ #ifndef _MEDIA_MT9V032_H #define _MEDIA_MT9V032_H -struct v4l2_subdev; - struct mt9v032_platform_data { unsigned int clk_pol:1; - void (*set_clock)(struct v4l2_subdev *subdev, unsigned int rate); - const s64 *link_freqs; s64 link_def_freq; };
Configure the device external clock using the common clock framework instead of a board code callback function. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/media/i2c/mt9v032.c | 16 ++++++++++------ include/media/mt9v032.h | 4 ---- 2 files changed, 10 insertions(+), 10 deletions(-)