Re: [PATCH 1/1] smiapp-pll: Take existing divisor into account in minimum divisor check

Re: [PATCH 1/1] smiapp-pll: Take existing divisor into account in minimum divisor check

[Laurent Pinchart]

Hi Sakari,

(CC'ing linux-omap and Rajendra Nayak)

On Sunday 31 March 2013 23:07:42 Sakari Ailus wrote:
> Hi Pali and Laurent,
> 
> Laurent: Pali has been working to get N900 working again with (almost)
> mainline kernel.
> 
> On Sun, Mar 31, 2013 at 12:03:39PM +0200, Pali Rohár wrote:
> > On Sunday 31 March 2013 10:44:08 Sakari Ailus wrote:
> > > 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>
> > > ---
> > > Hi Pali,
> > > 
> > > This looks like a bug in the PLL calculator. Could you try if this patch
> > > fixes it? It's written on an N950 and also is fully untested. :)
> > > 
> > > If it works for you I'll submit it to mainline as well.
> > > 
> > > Happy Easter! :)
> > > 
> > >  drivers/media/i2c/smiapp-pll.c |    3 ++-
> > >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > Hi,
> > 
> > now I tried smiapp again with 3.9 kernel on n900 and it failing with this
> > error messae (smiapp driver is not loaded):
> > 
> > [   22.164001] omap3isp omap3isp: Revision 2.0 found
> > [   22.166900] omap3isp omap3isp: clk_set_rate for cam_mclk
> > failed
> > 
> > I bisected git commit 6d1aa02f10497b138e01ebe6eafabd6071729334
> > "omap3isp: Set cam_mclk rate directly" and after reverting it 3.9
> > kernel started loading smiapp driver. When I applied your patch
> 
> This shouldn't happen.
> 
> Laurent: did you test the patch on 3430 back then? I only tested 3630. AFAIR
> you did, but I don't remember for sure.

I can't remember for sure, but it seems I haven't, or at least not properly 
:-(

> I don't have time to debug this in the coming days unfortunately. If this
> turns out to be a non-trivial issue then it's probably good to continue on
> linux-omap.

It might not be too complex to fix, but linux-omap is a good idea nonetheless.

Could you try debugging the problem ? The starting point is clk_set_rate() in 
drivers/clk/clk.c. My guess is that clk_calc_new_rates() returns NULL, and 
that it does so because it walks to clock tree up to dpll4_m5x2_ck which has 
neither a round_rate operation nor the CLK_SET_RATE_PARENT flag set. Could you 
please confirm that ?

I wonder if the dpll4_m5x2_ck clock (in arch/arm/mach-omap2/cclock3xxx_data.c) 
should just have the CLK_SET_RATE_PARENT flag set as the dpll4_m5x2_ck_3630 
clock.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html