Hi Sachin,
> Hi Lukasz,
>
> On 25 September 2013 16:52, Lukasz Majewski <l.majewski@xxxxxxxxxxx>
> wrote:
> >
> > static void exynos4x12_set_apll(unsigned int index)
> > {
> > - unsigned int tmp, pdiv;
> > + unsigned int tmp, freq = apll_freq_4x12[index].freq;
>
> nit: It is better to put the 'freq' assignment on a new line.
checkpatch.pl wasn't complaining :-).
Also please consider below comment.
>
> >
> > - /* 1. MUX_CORE_SEL = MPLL, ARMCLK uses MPLL for lock time */
> > + /* MUX_CORE_SEL = MPLL, ARMCLK uses MPLL for lock time */
> > clk_set_parent(moutcore, mout_mpll);
> >
> > do {
> > @@ -140,24 +140,9 @@ static void exynos4x12_set_apll(unsigned int
> > index) tmp &= 0x7;
> > } while (tmp != 0x2);
> >
> > - /* 2. Set APLL Lock time */
> > - pdiv = ((apll_freq_4x12[index].mps >> 8) & 0x3f);
> > + clk_set_rate(mout_apll, freq * 1000);
>
> Don't we need to check the return value of this?
The broken code isn't handling errors now (*_set_apll() function is
defined as void). Since this patch is a regression fix (for v3.12) I
just wanted to change as little as possible to provide a functional fix.
I think that regression fix shall not change much functionality -
therefore the exynosXXXX-cpufreq.c cleanup will be done for next kernel
release.
>
> Same comments for the second patch too.
>
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
