diff mbox

mt9p031: Do not use PLL if external frequency is the same as target frequency.

Message ID 1315303380-20698-1-git-send-email-javier.martin@vista-silicon.com (mailing list archive)
State New, archived
Headers show

Commit Message

Javier Martin Sept. 6, 2011, 10:03 a.m. UTC
This patch adds a check to see whether ext_freq and target_freq are equal and,
if true, PLL won't be used.

Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
---
 drivers/media/video/mt9p031.c |   18 +++++++++++++++---
 1 files changed, 15 insertions(+), 3 deletions(-)

Comments

Laurent Pinchart Sept. 6, 2011, 10:27 a.m. UTC | #1
Hi Javier,

On Tuesday 06 September 2011 12:03:00 Javier Martin wrote:
> This patch adds a check to see whether ext_freq and target_freq are equal
> and, if true, PLL won't be used.

Thanks for the patch.

As you're touching PLL code, what about fixing PLL setup by computing 
parameters dynamically instead of using a table of hardcoded values ? :-)

> Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
> ---
>  drivers/media/video/mt9p031.c |   18 +++++++++++++++---
>  1 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/video/mt9p031.c b/drivers/media/video/mt9p031.c
> index 5cfa39f..42b5d18 100644
> --- a/drivers/media/video/mt9p031.c
> +++ b/drivers/media/video/mt9p031.c
> @@ -117,6 +117,7 @@ struct mt9p031 {
>  	u16 xskip;
>  	u16 yskip;
> 
> +	bool use_pll;
>  	const struct mt9p031_pll_divs *pll;
> 
>  	/* Registers cache */
> @@ -201,10 +202,16 @@ static int mt9p031_pll_get_divs(struct mt9p031
> *mt9p031) struct i2c_client *client =
> v4l2_get_subdevdata(&mt9p031->subdev); int i;
> 
> +	if (mt9p031->pdata->ext_freq == mt9p031->pdata->target_freq) {
> +		mt9p031->use_pll = false;
> +		return 0;
> +	}
> +
>  	for (i = 0; i < ARRAY_SIZE(mt9p031_divs); i++) {
>  		if (mt9p031_divs[i].ext_freq == mt9p031->pdata->ext_freq &&
>  		  mt9p031_divs[i].target_freq == mt9p031->pdata->target_freq) {
>  			mt9p031->pll = &mt9p031_divs[i];
> +			mt9p031->use_pll = true;
>  			return 0;
>  		}
>  	}
> @@ -385,8 +392,10 @@ static int mt9p031_s_stream(struct v4l2_subdev
> *subdev, int enable) MT9P031_OUTPUT_CONTROL_CEN, 0);
>  		if (ret < 0)
>  			return ret;
> -
> -		return mt9p031_pll_disable(mt9p031);
> +		if (mt9p031->use_pll)
> +			return mt9p031_pll_disable(mt9p031);
> +		else
> +			return 0;
>  	}
> 
>  	ret = mt9p031_set_params(mt9p031);
> @@ -399,7 +408,10 @@ static int mt9p031_s_stream(struct v4l2_subdev
> *subdev, int enable) if (ret < 0)
>  		return ret;
> 
> -	return mt9p031_pll_enable(mt9p031);
> +	if (mt9p031->use_pll)
> +		return mt9p031_pll_enable(mt9p031);
> +	else
> +		return 0;
>  }
> 
>  static int mt9p031_enum_mbus_code(struct v4l2_subdev *subdev,
Javier Martin Sept. 6, 2011, 10:34 a.m. UTC | #2
On 6 September 2011 12:27, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Javier,
>
> On Tuesday 06 September 2011 12:03:00 Javier Martin wrote:
>> This patch adds a check to see whether ext_freq and target_freq are equal
>> and, if true, PLL won't be used.
>
> Thanks for the patch.
>
> As you're touching PLL code, what about fixing PLL setup by computing
> parameters dynamically instead of using a table of hardcoded values ? :-)

Hi Laurent,
I'm not exactly struggling with PLL code right now. I've just get a
new prototype which provides an external 48MHz oscillator for the
clock. So, no need to use PLL there and thus the purpose of this
patch.

However, as you said, dynamic configuration of PLL is one of the
pending issues on the driver and I might address it myself in the
future, but it depends on requirements of the project.


Regards.
diff mbox

Patch

diff --git a/drivers/media/video/mt9p031.c b/drivers/media/video/mt9p031.c
index 5cfa39f..42b5d18 100644
--- a/drivers/media/video/mt9p031.c
+++ b/drivers/media/video/mt9p031.c
@@ -117,6 +117,7 @@  struct mt9p031 {
 	u16 xskip;
 	u16 yskip;
 
+	bool use_pll;
 	const struct mt9p031_pll_divs *pll;
 
 	/* Registers cache */
@@ -201,10 +202,16 @@  static int mt9p031_pll_get_divs(struct mt9p031 *mt9p031)
 	struct i2c_client *client = v4l2_get_subdevdata(&mt9p031->subdev);
 	int i;
 
+	if (mt9p031->pdata->ext_freq == mt9p031->pdata->target_freq) {
+		mt9p031->use_pll = false;
+		return 0;
+	}
+
 	for (i = 0; i < ARRAY_SIZE(mt9p031_divs); i++) {
 		if (mt9p031_divs[i].ext_freq == mt9p031->pdata->ext_freq &&
 		  mt9p031_divs[i].target_freq == mt9p031->pdata->target_freq) {
 			mt9p031->pll = &mt9p031_divs[i];
+			mt9p031->use_pll = true;
 			return 0;
 		}
 	}
@@ -385,8 +392,10 @@  static int mt9p031_s_stream(struct v4l2_subdev *subdev, int enable)
 						 MT9P031_OUTPUT_CONTROL_CEN, 0);
 		if (ret < 0)
 			return ret;
-
-		return mt9p031_pll_disable(mt9p031);
+		if (mt9p031->use_pll)
+			return mt9p031_pll_disable(mt9p031);
+		else
+			return 0;
 	}
 
 	ret = mt9p031_set_params(mt9p031);
@@ -399,7 +408,10 @@  static int mt9p031_s_stream(struct v4l2_subdev *subdev, int enable)
 	if (ret < 0)
 		return ret;
 
-	return mt9p031_pll_enable(mt9p031);
+	if (mt9p031->use_pll)
+		return mt9p031_pll_enable(mt9p031);
+	else
+		return 0;
 }
 
 static int mt9p031_enum_mbus_code(struct v4l2_subdev *subdev,