diff mbox

[RFC,04/24] smiapp-pll: Take existing divisor into account in minimum divisor check

Message ID 1461532104-24032-5-git-send-email-ivo.g.dimitrov.75@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ivaylo Dimitrov April 24, 2016, 9:08 p.m. UTC
From: Sakari Ailus <sakari.ailus@iki.fi>

Required added multiplier (and divisor) calculation did not take into
account the existing divisor when checking the values against the minimum
divisor. Do just that.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 drivers/media/i2c/smiapp-pll.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Sakari Ailus May 1, 2016, 10:45 a.m. UTC | #1
Hi Ivaylo,

On Mon, Apr 25, 2016 at 12:08:04AM +0300, Ivaylo Dimitrov wrote:
> From: Sakari Ailus <sakari.ailus@iki.fi>
> 
> Required added multiplier (and divisor) calculation did not take into
> account the existing divisor when checking the values against the minimum
> divisor. Do just that.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
>  drivers/media/i2c/smiapp-pll.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
> index e3348db..5ad1edb 100644
> --- a/drivers/media/i2c/smiapp-pll.c
> +++ b/drivers/media/i2c/smiapp-pll.c
> @@ -227,7 +227,8 @@ static int __smiapp_pll_calculate(
>  
>  	more_mul_factor = lcm(div, pll->pre_pll_clk_div) / div;
>  	dev_dbg(dev, "more_mul_factor: %u\n", more_mul_factor);
> -	more_mul_factor = lcm(more_mul_factor, op_limits->min_sys_clk_div);
> +	more_mul_factor = lcm(more_mul_factor,
> +			      DIV_ROUND_UP(op_limits->min_sys_clk_div, div));
>  	dev_dbg(dev, "more_mul_factor: min_op_sys_clk_div: %d\n",
>  		more_mul_factor);
>  	i = roundup(more_mul_min, more_mul_factor);

I remember writing the patch, but I don't remember what for, or whether it
was really needed. Does the secondary sensor work without this one?
Ivaylo Dimitrov May 3, 2016, 6:25 p.m. UTC | #2
Hi,

On  1.05.2016 13:45, Sakari Ailus wrote:
> Hi Ivaylo,
>
> On Mon, Apr 25, 2016 at 12:08:04AM +0300, Ivaylo Dimitrov wrote:
>> From: Sakari Ailus <sakari.ailus@iki.fi>
>>
>> Required added multiplier (and divisor) calculation did not take into
>> account the existing divisor when checking the values against the minimum
>> divisor. Do just that.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
>> ---
>>   drivers/media/i2c/smiapp-pll.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
>> index e3348db..5ad1edb 100644
>> --- a/drivers/media/i2c/smiapp-pll.c
>> +++ b/drivers/media/i2c/smiapp-pll.c
>> @@ -227,7 +227,8 @@ static int __smiapp_pll_calculate(
>>
>>   	more_mul_factor = lcm(div, pll->pre_pll_clk_div) / div;
>>   	dev_dbg(dev, "more_mul_factor: %u\n", more_mul_factor);
>> -	more_mul_factor = lcm(more_mul_factor, op_limits->min_sys_clk_div);
>> +	more_mul_factor = lcm(more_mul_factor,
>> +			      DIV_ROUND_UP(op_limits->min_sys_clk_div, div));
>>   	dev_dbg(dev, "more_mul_factor: min_op_sys_clk_div: %d\n",
>>   		more_mul_factor);
>>   	i = roundup(more_mul_min, more_mul_factor);
>
> I remember writing the patch, but I don't remember what for, or whether it
> was really needed. Does the secondary sensor work without this one?
>

[  107.285919] smiapp 2-0010: lanes 1
[  107.286010] smiapp 2-0010: reset -2, nvm 0, clk 9600000, mode 0
[  107.286041] smiapp 2-0010: freq 0: 60000000
[  107.289306] twl4030reg_is_enabled VAUX4 0
[  107.303680] twl4030reg_enable VAUX4 0
[  107.352233] smiapp 2-0010: module 0x01-0x022b
[  107.352325] smiapp 2-0010: module revision 0x04-0x00 date 00-00-00
[  107.352355] smiapp 2-0010: sensor 0x00-0x0000
[  107.352386] smiapp 2-0010: sensor revision 0x00 firmware version 0x00
[  107.352416] smiapp 2-0010: smia version 10 smiapp version 00
[  107.352447] smiapp 2-0010: the sensor is called vs6555, ident 01022b04

.
.
.

[  107.595672] smiapp 2-0010: unable to compute pre_pll divisor
[  107.611816] smiapp 2-0010: link freq 60000000 Hz, bpp 10 not ok
[  107.611816] smiapp 2-0010: no valid link frequencies for 10 bpp
[  107.618072] smiapp 2-0010: no supported mbus code found

Or, in short, it does not work without this patch.

Thanks,
Ivo
--
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
Pali Rohár May 24, 2016, 9:09 a.m. UTC | #3
On Sunday 01 May 2016 13:45:24 Sakari Ailus wrote:
> Hi Ivaylo,
> 
> On Mon, Apr 25, 2016 at 12:08:04AM +0300, Ivaylo Dimitrov wrote:
> > From: Sakari Ailus <sakari.ailus@iki.fi>
> > 
> > Required added multiplier (and divisor) calculation did not take into
> > account the existing divisor when checking the values against the minimum
> > divisor. Do just that.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > ---
> >  drivers/media/i2c/smiapp-pll.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
> > index e3348db..5ad1edb 100644
> > --- a/drivers/media/i2c/smiapp-pll.c
> > +++ b/drivers/media/i2c/smiapp-pll.c
> > @@ -227,7 +227,8 @@ static int __smiapp_pll_calculate(
> >  
> >  	more_mul_factor = lcm(div, pll->pre_pll_clk_div) / div;
> >  	dev_dbg(dev, "more_mul_factor: %u\n", more_mul_factor);
> > -	more_mul_factor = lcm(more_mul_factor, op_limits->min_sys_clk_div);
> > +	more_mul_factor = lcm(more_mul_factor,
> > +			      DIV_ROUND_UP(op_limits->min_sys_clk_div, div));
> >  	dev_dbg(dev, "more_mul_factor: min_op_sys_clk_div: %d\n",
> >  		more_mul_factor);
> >  	i = roundup(more_mul_min, more_mul_factor);
> 
> I remember writing the patch, but I don't remember what for, or whether it
> was really needed. Does the secondary sensor work without this one?

Hi! You sent me this patch more then 3 years ago. Look at our private
email discussion, e.g. email with Message-Id <201303281524.10538@pali>
and subject "Re: Nokia N900 - smiapp driver" which was sent years ago
Thu, 28 Mar 2013 15:24:10 +0100.
Pavel Machek May 24, 2016, 10:17 a.m. UTC | #4
On Mon 2016-04-25 00:08:04, Ivaylo Dimitrov wrote:
> From: Sakari Ailus <sakari.ailus@iki.fi>
> 
> Required added multiplier (and divisor) calculation did not take into
> account the existing divisor when checking the values against the minimum
> divisor. Do just that.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>

Acked-by: Pavel Machek <pavel@ucw.cz>
diff mbox

Patch

diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
index e3348db..5ad1edb 100644
--- a/drivers/media/i2c/smiapp-pll.c
+++ b/drivers/media/i2c/smiapp-pll.c
@@ -227,7 +227,8 @@  static int __smiapp_pll_calculate(
 
 	more_mul_factor = lcm(div, pll->pre_pll_clk_div) / div;
 	dev_dbg(dev, "more_mul_factor: %u\n", more_mul_factor);
-	more_mul_factor = lcm(more_mul_factor, op_limits->min_sys_clk_div);
+	more_mul_factor = lcm(more_mul_factor,
+			      DIV_ROUND_UP(op_limits->min_sys_clk_div, div));
 	dev_dbg(dev, "more_mul_factor: min_op_sys_clk_div: %d\n",
 		more_mul_factor);
 	i = roundup(more_mul_min, more_mul_factor);