Re: [PATCHv3 04/15] ov7670: get xclk

2017-03-10 Thread Hans Verkuil 
On 10/03/17 10:11, Sakari Ailus wrote:
> Hi Hans,
> 
> On Fri, Mar 10, 2017 at 09:55:53AM +0100, Hans Verkuil wrote:
>> On 09/03/17 21:38, Sakari Ailus wrote:
>>> Hi Hans,
>>>
>>> On Mon, Mar 06, 2017 at 03:56:05PM +0100, Hans Verkuil wrote:
 From: Hans Verkuil 

 Get the clock for this sensor.

 Signed-off-by: Hans Verkuil 
 ---
  drivers/media/i2c/ov7670.c | 12 
  1 file changed, 12 insertions(+)

 diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
 index 50e4466a2b37..da0843617a49 100644
 --- a/drivers/media/i2c/ov7670.c
 +++ b/drivers/media/i2c/ov7670.c
 @@ -10,6 +10,7 @@
   * This file may be distributed under the terms of the GNU General
   * Public License, version 2.
   */
 +#include 
  #include 
  #include 
  #include 
 @@ -227,6 +228,7 @@ struct ov7670_info {
struct v4l2_ctrl *hue;
};
struct ov7670_format_struct *fmt;  /* Current format */
 +  struct clk *clk;
int min_width;  /* Filter out smaller sizes */
int min_height; /* Filter out smaller sizes */
int clock_speed;/* External clock speed (MHz) */
 @@ -1587,6 +1589,15 @@ static int ov7670_probe(struct i2c_client *client,
info->pclk_hb_disable = true;
}
  
 +  info->clk = devm_clk_get(&client->dev, "xclk");
 +  if (IS_ERR(info->clk))
 +  return -EPROBE_DEFER;
 +  clk_prepare_enable(info->clk);
 +
 +  info->clock_speed = clk_get_rate(info->clk) / 100;
 +  if (info->clock_speed < 10 || info->clock_speed > 48)
 +  return -EINVAL;
>>>
>>> clk_disable_unprepare() before return?
>>
>> It is my understanding that devm_clk_get will call that for you.
>>
>> Correct me if I am wrong.
> 
> devm_clk_get() obtained clock is released using devm_clk_release() which is
> just calling clk_put(). Which caller prepared or enabled the clock is not
> tracked. It's the responsibility of the caller.
> 

You're right. Then I need to check for this.

Hmm, I'm pretty sure I looked at what other drivers do. I think I'm not the
only one who forgets to call clk_disable_unprepare.

Regards,

Hans

Re: [PATCHv3 04/15] ov7670: get xclk

2017-03-10 Thread Sakari Ailus 
Hi Hans,

On Fri, Mar 10, 2017 at 09:55:53AM +0100, Hans Verkuil wrote:
> On 09/03/17 21:38, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > On Mon, Mar 06, 2017 at 03:56:05PM +0100, Hans Verkuil wrote:
> >> From: Hans Verkuil 
> >>
> >> Get the clock for this sensor.
> >>
> >> Signed-off-by: Hans Verkuil 
> >> ---
> >>  drivers/media/i2c/ov7670.c | 12 
> >>  1 file changed, 12 insertions(+)
> >>
> >> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> >> index 50e4466a2b37..da0843617a49 100644
> >> --- a/drivers/media/i2c/ov7670.c
> >> +++ b/drivers/media/i2c/ov7670.c
> >> @@ -10,6 +10,7 @@
> >>   * This file may be distributed under the terms of the GNU General
> >>   * Public License, version 2.
> >>   */
> >> +#include 
> >>  #include 
> >>  #include 
> >>  #include 
> >> @@ -227,6 +228,7 @@ struct ov7670_info {
> >>struct v4l2_ctrl *hue;
> >>};
> >>struct ov7670_format_struct *fmt;  /* Current format */
> >> +  struct clk *clk;
> >>int min_width;  /* Filter out smaller sizes */
> >>int min_height; /* Filter out smaller sizes */
> >>int clock_speed;/* External clock speed (MHz) */
> >> @@ -1587,6 +1589,15 @@ static int ov7670_probe(struct i2c_client *client,
> >>info->pclk_hb_disable = true;
> >>}
> >>  
> >> +  info->clk = devm_clk_get(&client->dev, "xclk");
> >> +  if (IS_ERR(info->clk))
> >> +  return -EPROBE_DEFER;
> >> +  clk_prepare_enable(info->clk);
> >> +
> >> +  info->clock_speed = clk_get_rate(info->clk) / 100;
> >> +  if (info->clock_speed < 10 || info->clock_speed > 48)
> >> +  return -EINVAL;
> > 
> > clk_disable_unprepare() before return?
> 
> It is my understanding that devm_clk_get will call that for you.
> 
> Correct me if I am wrong.

devm_clk_get() obtained clock is released using devm_clk_release() which is
just calling clk_put(). Which caller prepared or enabled the clock is not
tracked. It's the responsibility of the caller.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCHv3 04/15] ov7670: get xclk

2017-03-10 Thread Hans Verkuil 
On 09/03/17 21:38, Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, Mar 06, 2017 at 03:56:05PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Get the clock for this sensor.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  drivers/media/i2c/ov7670.c | 12 
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
>> index 50e4466a2b37..da0843617a49 100644
>> --- a/drivers/media/i2c/ov7670.c
>> +++ b/drivers/media/i2c/ov7670.c
>> @@ -10,6 +10,7 @@
>>   * This file may be distributed under the terms of the GNU General
>>   * Public License, version 2.
>>   */
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -227,6 +228,7 @@ struct ov7670_info {
>>  struct v4l2_ctrl *hue;
>>  };
>>  struct ov7670_format_struct *fmt;  /* Current format */
>> +struct clk *clk;
>>  int min_width;  /* Filter out smaller sizes */
>>  int min_height; /* Filter out smaller sizes */
>>  int clock_speed;/* External clock speed (MHz) */
>> @@ -1587,6 +1589,15 @@ static int ov7670_probe(struct i2c_client *client,
>>  info->pclk_hb_disable = true;
>>  }
>>  
>> +info->clk = devm_clk_get(&client->dev, "xclk");
>> +if (IS_ERR(info->clk))
>> +return -EPROBE_DEFER;
>> +clk_prepare_enable(info->clk);
>> +
>> +info->clock_speed = clk_get_rate(info->clk) / 100;
>> +if (info->clock_speed < 10 || info->clock_speed > 48)
>> +return -EINVAL;
> 
> clk_disable_unprepare() before return?

It is my understanding that devm_clk_get will call that for you.

Correct me if I am wrong.

Regards,

Hans

> 
>> +
>>  /* Make sure it's an ov7670 */
>>  ret = ov7670_detect(sd);
>>  if (ret) {
>> @@ -1667,6 +1678,7 @@ static int ov7670_remove(struct i2c_client *client)
>>  
>>  v4l2_device_unregister_subdev(sd);
>>  v4l2_ctrl_handler_free(&info->hdl);
>> +clk_disable_unprepare(info->clk);
>>  return 0;
>>  }
>>  
>

Re: [PATCHv3 04/15] ov7670: get xclk

2017-03-09 Thread Sakari Ailus 
Hi Hans,

On Mon, Mar 06, 2017 at 03:56:05PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Get the clock for this sensor.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/i2c/ov7670.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> index 50e4466a2b37..da0843617a49 100644
> --- a/drivers/media/i2c/ov7670.c
> +++ b/drivers/media/i2c/ov7670.c
> @@ -10,6 +10,7 @@
>   * This file may be distributed under the terms of the GNU General
>   * Public License, version 2.
>   */
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -227,6 +228,7 @@ struct ov7670_info {
>   struct v4l2_ctrl *hue;
>   };
>   struct ov7670_format_struct *fmt;  /* Current format */
> + struct clk *clk;
>   int min_width;  /* Filter out smaller sizes */
>   int min_height; /* Filter out smaller sizes */
>   int clock_speed;/* External clock speed (MHz) */
> @@ -1587,6 +1589,15 @@ static int ov7670_probe(struct i2c_client *client,
>   info->pclk_hb_disable = true;
>   }
>  
> + info->clk = devm_clk_get(&client->dev, "xclk");
> + if (IS_ERR(info->clk))
> + return -EPROBE_DEFER;
> + clk_prepare_enable(info->clk);
> +
> + info->clock_speed = clk_get_rate(info->clk) / 100;
> + if (info->clock_speed < 10 || info->clock_speed > 48)
> + return -EINVAL;

clk_disable_unprepare() before return?

> +
>   /* Make sure it's an ov7670 */
>   ret = ov7670_detect(sd);
>   if (ret) {
> @@ -1667,6 +1678,7 @@ static int ov7670_remove(struct i2c_client *client)
>  
>   v4l2_device_unregister_subdev(sd);
>   v4l2_ctrl_handler_free(&info->hdl);
> + clk_disable_unprepare(info->clk);
>   return 0;
>  }
>  

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

[PATCHv3 04/15] ov7670: get xclk

2017-03-06 Thread Hans Verkuil 
From: Hans Verkuil 

Get the clock for this sensor.

Signed-off-by: Hans Verkuil 
---
 drivers/media/i2c/ov7670.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 50e4466a2b37..da0843617a49 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -10,6 +10,7 @@
  * This file may be distributed under the terms of the GNU General
  * Public License, version 2.
  */
+#include 
 #include 
 #include 
 #include 
@@ -227,6 +228,7 @@ struct ov7670_info {
struct v4l2_ctrl *hue;
};
struct ov7670_format_struct *fmt;  /* Current format */
+   struct clk *clk;
int min_width;  /* Filter out smaller sizes */
int min_height; /* Filter out smaller sizes */
int clock_speed;/* External clock speed (MHz) */
@@ -1587,6 +1589,15 @@ static int ov7670_probe(struct i2c_client *client,
info->pclk_hb_disable = true;
}
 
+   info->clk = devm_clk_get(&client->dev, "xclk");
+   if (IS_ERR(info->clk))
+   return -EPROBE_DEFER;
+   clk_prepare_enable(info->clk);
+
+   info->clock_speed = clk_get_rate(info->clk) / 100;
+   if (info->clock_speed < 10 || info->clock_speed > 48)
+   return -EINVAL;
+
/* Make sure it's an ov7670 */
ret = ov7670_detect(sd);
if (ret) {
@@ -1667,6 +1678,7 @@ static int ov7670_remove(struct i2c_client *client)
 
v4l2_device_unregister_subdev(sd);
v4l2_ctrl_handler_free(&info->hdl);
+   clk_disable_unprepare(info->clk);
return 0;
 }
 
-- 
2.11.0