Re: [PATCH V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers

[Stefan Wahren <stefan.wahren@i2se.com>]

Hi Fabio,

> Fabio Estevam <festevam@gmail.com> hat am 12. Februar 2015 um 20:08
> geschrieben:
>
>
> Hi Stefan,
>
> On Thu, Feb 12, 2015 at 4:59 PM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> > Hi Fabio,
> >
> >> Fabio Estevam <festevam@gmail.com> hat am 11. Februar 2015 um 22:10
> >> geschrieben:
> >>
> >>
> >> On Wed, Feb 11, 2015 at 6:31 PM, Stefan Wahren <stefan.wahren@i2se.com>
> >> wrote:
> >>
> >> >> I always get 0x5e5b5513 with or without your patch.
> >> >
> >> > very strange. Do you have any idea why IO1_STABLE is permanent low?
> >>
> >> On my case it is always 1.
> >
> > i expected the same behavior on my hardware.
> >
> > Do you use u-boot as bootloader?
>
> Yes, I do.

i will try to test it with u-boot.

>
> >>
> >> > Can you confirm the behavior according to your flash issue?
> >>
> >> In my tests IO1_STABLE stays the same.
> >
> > This wasn't the intension of my second question. I wanted to know about the
> > state of the SPI NOR flash detection process.
> >
> > Does it sucessed if you apply the patch, but revert the changes in
> > clk_ref_set_rate() from clk-ref.c?
>
> I don't have my mx28evk setup available at the moment, but when I
> applied your patch and reverted all the changes in clk-ref.c, then the
> SPI flash detection works.
>
> I haven't tested to only reverting the changes inside
> clk_ref_set_rate(), but I can do it tomorrow.

I think the reason for the problem in the flash detection is caused by the
misaligned access on the frac register.

Maybe you want to try the following after the second patch is reverted.

Stefan

------------>8----------------------------------------------------------
diff --git a/drivers/clk/mxs/clk-imx23.c b/drivers/clk/mxs/clk-imx23.c
index 9fc9359..87969e3 100644
--- a/drivers/clk/mxs/clk-imx23.c
+++ b/drivers/clk/mxs/clk-imx23.c
@@ -73,8 +73,10 @@ static void __init clk_misc_init(void)
 	 * 480 MHz seems too high to be ssp clock source directly,
 	 * so set frac to get a 288 MHz ref_io.
 	 */
-	writel_relaxed(0x3f << BP_FRAC_IOFRAC, FRAC + CLR);
-	writel_relaxed(30 << BP_FRAC_IOFRAC, FRAC + SET);
+	val = readl_relaxed(FRAC);
+	val &= ~(0x3f << BP_FRAC_IOFRAC);
+	val |= 30 << BP_FRAC_IOFRAC;
+	writel_relaxed(val, FRAC);
 }
 
 static const char *sel_pll[]  __initconst = { "pll", "ref_xtal", };
diff --git a/drivers/clk/mxs/clk-imx28.c b/drivers/clk/mxs/clk-imx28.c
index a6c3501..e47ad69 100644
--- a/drivers/clk/mxs/clk-imx28.c
+++ b/drivers/clk/mxs/clk-imx28.c
@@ -118,10 +118,15 @@ static void __init clk_misc_init(void)
 	/*
 	 * 480 MHz seems too high to be ssp clock source directly,
 	 * so set frac0 to get a 288 MHz ref_io0 and ref_io1.
+	 * According to reference manual we must modify frac bytewise.
 	 */
 	val = readl_relaxed(FRAC0);
-	val &= ~((0x3f << BP_FRAC0_IO0FRAC) | (0x3f << BP_FRAC0_IO1FRAC));
-	val |= (30 << BP_FRAC0_IO0FRAC) | (30 << BP_FRAC0_IO1FRAC);
+	val &= ~(0x3f << BP_FRAC0_IO0FRAC);
+	val |= 30 << BP_FRAC0_IO0FRAC;
+	writel_relaxed(val, FRAC0);
+	val = readl_relaxed(FRAC0);
+	val &= ~(0x3f << BP_FRAC0_IO1FRAC);
+	val |= 30 << BP_FRAC0_IO1FRAC;
 	writel_relaxed(val, FRAC0);
 }