| Message ID | 1419762402-4548-1-git-send-email-stefan.wahren@i2se.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sunday, December 28, 2014 at 11:26:42 AM, Stefan Wahren wrote: > According to i.MX23 and i.MX28 reference manual the fractional > clock control registers must be addressed by byte instructions. > > This patch fixes the erroneous 32-bit access to these registers > and extends the comment in the init functions. > > Btw the imx23 init now uses a R-M-W sequence just like imx28 init > to avoid any clock glitches. > > The changes has been tested only with a i.MX28 board, because i don't > have access to an i.MX23 board. > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> I don't see a problem, Reviewed-by: Marek Vasut <marex@denx.de> Best regards, Marek Vasut
Hi, > Marek Vasut <marex@denx.de> hat am 28. Dezember 2014 um 19:30 geschrieben: > > > On Sunday, December 28, 2014 at 11:26:42 AM, Stefan Wahren wrote: > > According to i.MX23 and i.MX28 reference manual the fractional > > clock control registers must be addressed by byte instructions. > > > > This patch fixes the erroneous 32-bit access to these registers > > and extends the comment in the init functions. > > > > Btw the imx23 init now uses a R-M-W sequence just like imx28 init > > to avoid any clock glitches. > > > > The changes has been tested only with a i.MX28 board, because i don't > > have access to an i.MX23 board. since i've also a i.MX23 board, i tested this patch successful. Any conserns about it? > > > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > > I don't see a problem, > > Reviewed-by: Marek Vasut <marex@denx.de> > > Best regards, > Marek Vasut > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Sun, Dec 28, 2014 at 4:26 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: > According to i.MX23 and i.MX28 reference manual the fractional > clock control registers must be addressed by byte instructions. > I don't think mx23 and mx28 have such limitation. I will double check with IC team about this. RTL is generated from a xml file. All registers implement is unified. I don't think only clock control register have such limitation and other registers not. best regards Frank Li (Frank.Li@freecale.com). > This patch fixes the erroneous 32-bit access to these registers > and extends the comment in the init functions. > > Btw the imx23 init now uses a R-M-W sequence just like imx28 init > to avoid any clock glitches. > > The changes has been tested only with a i.MX28 board, because i don't > have access to an i.MX23 board. > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > --- > Changes in V2: > - use relaxed access operations in clk-ref > > drivers/clk/mxs/clk-imx23.c | 11 ++++++++--- > drivers/clk/mxs/clk-imx28.c | 19 +++++++++++++------ > drivers/clk/mxs/clk-ref.c | 19 ++++++++++--------- > 3 files changed, 31 insertions(+), 18 deletions(-) > > diff --git a/drivers/clk/mxs/clk-imx23.c b/drivers/clk/mxs/clk-imx23.c > index 9fc9359..a084566 100644 > --- a/drivers/clk/mxs/clk-imx23.c > +++ b/drivers/clk/mxs/clk-imx23.c > @@ -46,11 +46,13 @@ static void __iomem *digctrl; > #define BP_CLKSEQ_BYPASS_SAIF 0 > #define BP_CLKSEQ_BYPASS_SSP 5 > #define BP_SAIF_DIV_FRAC_EN 16 > -#define BP_FRAC_IOFRAC 24 > + > +#define FRAC_IO 3 > > static void __init clk_misc_init(void) > { > u32 val; > + u8 frac; > > /* Gate off cpu clock in WFI for power saving */ > writel_relaxed(1 << BP_CPU_INTERRUPT_WAIT, CPU + SET); > @@ -72,9 +74,12 @@ 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. > + * According to reference manual we must access frac bytewise. > */ > - writel_relaxed(0x3f << BP_FRAC_IOFRAC, FRAC + CLR); > - writel_relaxed(30 << BP_FRAC_IOFRAC, FRAC + SET); > + frac = readb_relaxed(FRAC + FRAC_IO); > + frac &= ~0x3f; > + frac |= 30; > + writeb_relaxed(frac, FRAC + FRAC_IO); > } > > 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..c541377 100644 > --- a/drivers/clk/mxs/clk-imx28.c > +++ b/drivers/clk/mxs/clk-imx28.c > @@ -53,8 +53,9 @@ static void __iomem *clkctrl; > #define BP_ENET_SLEEP 31 > #define BP_CLKSEQ_BYPASS_SAIF0 0 > #define BP_CLKSEQ_BYPASS_SSP0 3 > -#define BP_FRAC0_IO1FRAC 16 > -#define BP_FRAC0_IO0FRAC 24 > + > +#define FRAC0_IO1 2 > +#define FRAC0_IO0 3 > > static void __iomem *digctrl; > #define DIGCTRL digctrl > @@ -85,6 +86,7 @@ int mxs_saif_clkmux_select(unsigned int clkmux) > static void __init clk_misc_init(void) > { > u32 val; > + u8 frac; > > /* Gate off cpu clock in WFI for power saving */ > writel_relaxed(1 << BP_CPU_INTERRUPT_WAIT, CPU + SET); > @@ -118,11 +120,16 @@ 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 access frac0 bytewise. > */ > - val = readl_relaxed(FRAC0); > - val &= ~((0x3f << BP_FRAC0_IO0FRAC) | (0x3f << BP_FRAC0_IO1FRAC)); > - val |= (30 << BP_FRAC0_IO0FRAC) | (30 << BP_FRAC0_IO1FRAC); > - writel_relaxed(val, FRAC0); > + frac = readb_relaxed(FRAC0 + FRAC0_IO0); > + frac &= ~0x3f; > + frac |= 30; > + writeb_relaxed(frac, FRAC0 + FRAC0_IO0); > + frac = readb_relaxed(FRAC0 + FRAC0_IO1); > + frac &= ~0x3f; > + frac |= 30; > + writeb_relaxed(frac, FRAC0 + FRAC0_IO1); > } > > static const char *sel_cpu[] __initconst = { "ref_cpu", "ref_xtal", }; > diff --git a/drivers/clk/mxs/clk-ref.c b/drivers/clk/mxs/clk-ref.c > index 4adeed6..ad3851c 100644 > --- a/drivers/clk/mxs/clk-ref.c > +++ b/drivers/clk/mxs/clk-ref.c > @@ -16,6 +16,8 @@ > #include <linux/slab.h> > #include "clk.h" > > +#define BF_CLKGATE BIT(7) > + > /** > * struct clk_ref - mxs reference clock > * @hw: clk_hw for the reference clock > @@ -39,7 +41,7 @@ static int clk_ref_enable(struct clk_hw *hw) > { > struct clk_ref *ref = to_clk_ref(hw); > > - writel_relaxed(1 << ((ref->idx + 1) * 8 - 1), ref->reg + CLR); > + writeb_relaxed(BF_CLKGATE, ref->reg + ref->idx + CLR); > > return 0; > } > @@ -48,7 +50,7 @@ static void clk_ref_disable(struct clk_hw *hw) > { > struct clk_ref *ref = to_clk_ref(hw); > > - writel_relaxed(1 << ((ref->idx + 1) * 8 - 1), ref->reg + SET); > + writeb_relaxed(BF_CLKGATE, ref->reg + ref->idx + SET); > } > > static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, > @@ -56,7 +58,7 @@ static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, > { > struct clk_ref *ref = to_clk_ref(hw); > u64 tmp = parent_rate; > - u8 frac = (readl_relaxed(ref->reg) >> (ref->idx * 8)) & 0x3f; > + u8 frac = readb_relaxed(ref->reg + ref->idx) & 0x3f; > > tmp *= 18; > do_div(tmp, frac); > @@ -93,8 +95,7 @@ static int clk_ref_set_rate(struct clk_hw *hw, unsigned long rate, > struct clk_ref *ref = to_clk_ref(hw); > unsigned long flags; > u64 tmp = parent_rate; > - u32 val; > - u8 frac, shift = ref->idx * 8; > + u8 frac, val; > > tmp = tmp * 18 + rate / 2; > do_div(tmp, rate); > @@ -107,10 +108,10 @@ static int clk_ref_set_rate(struct clk_hw *hw, unsigned long rate, > > spin_lock_irqsave(&mxs_lock, flags); > > - val = readl_relaxed(ref->reg); > - val &= ~(0x3f << shift); > - val |= frac << shift; > - writel_relaxed(val, ref->reg); > + val = readb_relaxed(ref->reg + ref->idx); > + val &= ~0x3f; > + val |= frac; > + writeb_relaxed(val, ref->reg + ref->idx); > > spin_unlock_irqrestore(&mxs_lock, flags); > > -- > 1.7.9.5 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wednesday, January 21, 2015 at 05:16:03 PM, Zhi Li wrote: > On Sun, Dec 28, 2014 at 4:26 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: > > According to i.MX23 and i.MX28 reference manual the fractional > > clock control registers must be addressed by byte instructions. > > I don't think mx23 and mx28 have such limitation. I will double check > with IC team about this. > RTL is generated from a xml file. All registers implement is unified. > I don't think only clock control register have such limitation and > other registers not. Hi, Section 10.8.24 in the MX28 datasheet (Fractional Clock Control Register 0) states otherwise, but maybe the documentation is simply not matching the silicon. Here's a quote: " This register controls the 9-phase fractional clock dividers. The fractional clock frequencies are a product of the values in these registers. NOTE: This register can only be addressed by byte instructions. Addressing word or half- word are not allowed. " I also recall seeing weird behavior when these registers were accessed by word access in U-Boot, so I believe the datasheet is correct. Best regards, Marek Vasut
Quoting Marek Vasut (2015-01-21 15:39:01) > On Wednesday, January 21, 2015 at 05:16:03 PM, Zhi Li wrote: > > On Sun, Dec 28, 2014 at 4:26 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: > > > According to i.MX23 and i.MX28 reference manual the fractional > > > clock control registers must be addressed by byte instructions. > > > > I don't think mx23 and mx28 have such limitation. I will double check > > with IC team about this. > > RTL is generated from a xml file. All registers implement is unified. > > I don't think only clock control register have such limitation and > > other registers not. > > Hi, > > Section 10.8.24 in the MX28 datasheet (Fractional Clock Control Register 0) > states otherwise, but maybe the documentation is simply not matching the > silicon. > > Here's a quote: > " > This register controls the 9-phase fractional clock dividers. The fractional > clock frequencies are a product of the values in these registers. NOTE: This > register can only be addressed by byte instructions. Addressing word or half- > word are not allowed. > " > > I also recall seeing weird behavior when these registers were accessed by word > access in U-Boot, so I believe the datasheet is correct. Hi Frank, Are you satisfied with this patch? Regards, Mike > > Best regards, > Marek Vasut
On Tue, Jan 27, 2015 at 7:51 PM, Mike Turquette <mturquette@linaro.org> wrote: > Quoting Marek Vasut (2015-01-21 15:39:01) >> On Wednesday, January 21, 2015 at 05:16:03 PM, Zhi Li wrote: >> > On Sun, Dec 28, 2014 at 4:26 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >> > > According to i.MX23 and i.MX28 reference manual the fractional >> > > clock control registers must be addressed by byte instructions. >> > >> > I don't think mx23 and mx28 have such limitation. I will double check >> > with IC team about this. >> > RTL is generated from a xml file. All registers implement is unified. >> > I don't think only clock control register have such limitation and >> > other registers not. >> >> Hi, >> >> Section 10.8.24 in the MX28 datasheet (Fractional Clock Control Register 0) >> states otherwise, but maybe the documentation is simply not matching the >> silicon. >> >> Here's a quote: >> " >> This register controls the 9-phase fractional clock dividers. The fractional >> clock frequencies are a product of the values in these registers. NOTE: This >> register can only be addressed by byte instructions. Addressing word or half- >> word are not allowed. >> " >> >> I also recall seeing weird behavior when these registers were accessed by word >> access in U-Boot, so I believe the datasheet is correct. > > Hi Frank, > > Are you satisfied with this patch? I asked IC designer about this. They will check RTL code. I will check their status again. Our released BSP code used 32bit WORD access. best regards Frank Li > > Regards, > Mike > >> >> Best regards, >> Marek Vasut
Hi, Am 28.01.2015 um 04:36 schrieb Zhi Li: > On Tue, Jan 27, 2015 at 7:51 PM, Mike Turquette <mturquette@linaro.org> wrote: >> Quoting Marek Vasut (2015-01-21 15:39:01) >>> On Wednesday, January 21, 2015 at 05:16:03 PM, Zhi Li wrote: >>>> On Sun, Dec 28, 2014 at 4:26 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >>>>> According to i.MX23 and i.MX28 reference manual the fractional >>>>> clock control registers must be addressed by byte instructions. >>>> I don't think mx23 and mx28 have such limitation. I will double check >>>> with IC team about this. >>>> RTL is generated from a xml file. All registers implement is unified. >>>> I don't think only clock control register have such limitation and >>>> other registers not. >>> Hi, >>> >>> Section 10.8.24 in the MX28 datasheet (Fractional Clock Control Register 0) >>> states otherwise, but maybe the documentation is simply not matching the >>> silicon. >>> >>> Here's a quote: >>> " >>> This register controls the 9-phase fractional clock dividers. The fractional >>> clock frequencies are a product of the values in these registers. NOTE: This >>> register can only be addressed by byte instructions. Addressing word or half- >>> word are not allowed. >>> " >>> >>> I also recall seeing weird behavior when these registers were accessed by word >>> access in U-Boot, so I believe the datasheet is correct. >> Hi Frank, >> >> Are you satisfied with this patch? > I asked IC designer about this. > They will check RTL code. > I will check their status again. > Our released BSP code used 32bit WORD access. i want to point out that the 32bit WORD is divided in 4 parts (IO0FRAC, IO1FRAC, EMIFRAC, CPUFRAC). Yes, it's true that BSP code access the register as 32bit, but it's never modify the complete 32bit at once just only 1 part (8bit) at a time. So here is my theory about Fractional Clock Control Register: Reading as 32bit WORD => safe Modify only 1 part (8bit) of the 32bit WORD => safe Modify more than 1 part of the 32bit WORD => unsafe !!! Best regards Stefan > > best regards > Frank Li >> Regards, >> Mike >> >>> Best regards, >>> Marek Vasut
On Wed, Jan 28, 2015 at 9:52 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: > Hi, > > Am 28.01.2015 um 04:36 schrieb Zhi Li: >> On Tue, Jan 27, 2015 at 7:51 PM, Mike Turquette <mturquette@linaro.org> wrote: >>> Quoting Marek Vasut (2015-01-21 15:39:01) >>>> On Wednesday, January 21, 2015 at 05:16:03 PM, Zhi Li wrote: >>>>> On Sun, Dec 28, 2014 at 4:26 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >>>>>> According to i.MX23 and i.MX28 reference manual the fractional >>>>>> clock control registers must be addressed by byte instructions. >>>>> I don't think mx23 and mx28 have such limitation. I will double check >>>>> with IC team about this. >>>>> RTL is generated from a xml file. All registers implement is unified. >>>>> I don't think only clock control register have such limitation and >>>>> other registers not. >>>> Hi, >>>> >>>> Section 10.8.24 in the MX28 datasheet (Fractional Clock Control Register 0) >>>> states otherwise, but maybe the documentation is simply not matching the >>>> silicon. >>>> >>>> Here's a quote: >>>> " >>>> This register controls the 9-phase fractional clock dividers. The fractional >>>> clock frequencies are a product of the values in these registers. NOTE: This >>>> register can only be addressed by byte instructions. Addressing word or half- >>>> word are not allowed. >>>> " >>>> >>>> I also recall seeing weird behavior when these registers were accessed by word >>>> access in U-Boot, so I believe the datasheet is correct. >>> Hi Frank, >>> >>> Are you satisfied with this patch? >> I asked IC designer about this. >> They will check RTL code. >> I will check their status again. >> Our released BSP code used 32bit WORD access. > > i want to point out that the 32bit WORD is divided in 4 parts (IO0FRAC, > IO1FRAC, EMIFRAC, CPUFRAC). Yes, it's true that BSP code access the > register as 32bit, but it's never modify the complete 32bit at once just > only 1 part (8bit) at a time. > > So here is my theory about Fractional Clock Control Register: > > Reading as 32bit WORD => safe > Modify only 1 part (8bit) of the 32bit WORD => safe > Modify more than 1 part of the 32bit WORD => unsafe !!! Yes, it is align with what I get from IC designer. best regards Frank Li > > Best regards > Stefan > >> >> best regards >> Frank Li >>> Regards, >>> Mike >>> >>>> Best regards, >>>> Marek Vasut > >
Hi, Am 28.01.2015 um 17:10 schrieb Zhi Li: > On Wed, Jan 28, 2015 at 9:52 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >> Hi, >> >> Am 28.01.2015 um 04:36 schrieb Zhi Li: >>> On Tue, Jan 27, 2015 at 7:51 PM, Mike Turquette <mturquette@linaro.org> wrote: >>>> Quoting Marek Vasut (2015-01-21 15:39:01) >>>>> On Wednesday, January 21, 2015 at 05:16:03 PM, Zhi Li wrote: >>>>>> On Sun, Dec 28, 2014 at 4:26 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >>>>>>> According to i.MX23 and i.MX28 reference manual the fractional >>>>>>> clock control registers must be addressed by byte instructions. >>>>>> I don't think mx23 and mx28 have such limitation. I will double check >>>>>> with IC team about this. >>>>>> RTL is generated from a xml file. All registers implement is unified. >>>>>> I don't think only clock control register have such limitation and >>>>>> other registers not. >>>>> Hi, >>>>> >>>>> Section 10.8.24 in the MX28 datasheet (Fractional Clock Control Register 0) >>>>> states otherwise, but maybe the documentation is simply not matching the >>>>> silicon. >>>>> >>>>> Here's a quote: >>>>> " >>>>> This register controls the 9-phase fractional clock dividers. The fractional >>>>> clock frequencies are a product of the values in these registers. NOTE: This >>>>> register can only be addressed by byte instructions. Addressing word or half- >>>>> word are not allowed. >>>>> " >>>>> >>>>> I also recall seeing weird behavior when these registers were accessed by word >>>>> access in U-Boot, so I believe the datasheet is correct. >>>> Hi Frank, >>>> >>>> Are you satisfied with this patch? >>> I asked IC designer about this. >>> They will check RTL code. >>> I will check their status again. >>> Our released BSP code used 32bit WORD access. >> i want to point out that the 32bit WORD is divided in 4 parts (IO0FRAC, >> IO1FRAC, EMIFRAC, CPUFRAC). Yes, it's true that BSP code access the >> register as 32bit, but it's never modify the complete 32bit at once just >> only 1 part (8bit) at a time. >> >> So here is my theory about Fractional Clock Control Register: >> >> Reading as 32bit WORD => safe >> Modify only 1 part (8bit) of the 32bit WORD => safe >> Modify more than 1 part of the 32bit WORD => unsafe !!! > Yes, it is align with what I get from IC designer. > > best regards > Frank Li okay, fine. The clk init for the i.MX28 in the kernel tries to the set IO0FRAC and IO1FRAC at once. So this patch fix this problem. And since i was on that, i changed all accesses on that register to 8bit to avoid this problem in the future. So what's your opinion about this patch? Best regards Stefan > >> Best regards >> Stefan >> >>> best regards >>> Frank Li >>>> Regards, >>>> Mike >>>> >>>>> Best regards, >>>>> Marek Vasut >> > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Jan 28, 2015 at 10:40 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: > The clk init for the i.MX28 in the kernel tries to the set > IO0FRAC and IO1FRAC at once. So this patch fix this problem. Okay, I am fine. I suggest add comment about only change one FRAC once.
Hi Stefan, On Sun, Dec 28, 2014 at 8:26 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: > According to i.MX23 and i.MX28 reference manual the fractional > clock control registers must be addressed by byte instructions. > > This patch fixes the erroneous 32-bit access to these registers > and extends the comment in the init functions. > > Btw the imx23 init now uses a R-M-W sequence just like imx28 init > to avoid any clock glitches. > > The changes has been tested only with a i.MX28 board, because i don't > have access to an i.MX23 board. As you recently had a chance to test it on mx23, maybe you could resend it and change this last parapgraph. Regards, Fabio Estevam
diff --git a/drivers/clk/mxs/clk-imx23.c b/drivers/clk/mxs/clk-imx23.c index 9fc9359..a084566 100644 --- a/drivers/clk/mxs/clk-imx23.c +++ b/drivers/clk/mxs/clk-imx23.c @@ -46,11 +46,13 @@ static void __iomem *digctrl; #define BP_CLKSEQ_BYPASS_SAIF 0 #define BP_CLKSEQ_BYPASS_SSP 5 #define BP_SAIF_DIV_FRAC_EN 16 -#define BP_FRAC_IOFRAC 24 + +#define FRAC_IO 3 static void __init clk_misc_init(void) { u32 val; + u8 frac; /* Gate off cpu clock in WFI for power saving */ writel_relaxed(1 << BP_CPU_INTERRUPT_WAIT, CPU + SET); @@ -72,9 +74,12 @@ 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. + * According to reference manual we must access frac bytewise. */ - writel_relaxed(0x3f << BP_FRAC_IOFRAC, FRAC + CLR); - writel_relaxed(30 << BP_FRAC_IOFRAC, FRAC + SET); + frac = readb_relaxed(FRAC + FRAC_IO); + frac &= ~0x3f; + frac |= 30; + writeb_relaxed(frac, FRAC + FRAC_IO); } 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..c541377 100644 --- a/drivers/clk/mxs/clk-imx28.c +++ b/drivers/clk/mxs/clk-imx28.c @@ -53,8 +53,9 @@ static void __iomem *clkctrl; #define BP_ENET_SLEEP 31 #define BP_CLKSEQ_BYPASS_SAIF0 0 #define BP_CLKSEQ_BYPASS_SSP0 3 -#define BP_FRAC0_IO1FRAC 16 -#define BP_FRAC0_IO0FRAC 24 + +#define FRAC0_IO1 2 +#define FRAC0_IO0 3 static void __iomem *digctrl; #define DIGCTRL digctrl @@ -85,6 +86,7 @@ int mxs_saif_clkmux_select(unsigned int clkmux) static void __init clk_misc_init(void) { u32 val; + u8 frac; /* Gate off cpu clock in WFI for power saving */ writel_relaxed(1 << BP_CPU_INTERRUPT_WAIT, CPU + SET); @@ -118,11 +120,16 @@ 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 access frac0 bytewise. */ - val = readl_relaxed(FRAC0); - val &= ~((0x3f << BP_FRAC0_IO0FRAC) | (0x3f << BP_FRAC0_IO1FRAC)); - val |= (30 << BP_FRAC0_IO0FRAC) | (30 << BP_FRAC0_IO1FRAC); - writel_relaxed(val, FRAC0); + frac = readb_relaxed(FRAC0 + FRAC0_IO0); + frac &= ~0x3f; + frac |= 30; + writeb_relaxed(frac, FRAC0 + FRAC0_IO0); + frac = readb_relaxed(FRAC0 + FRAC0_IO1); + frac &= ~0x3f; + frac |= 30; + writeb_relaxed(frac, FRAC0 + FRAC0_IO1); } static const char *sel_cpu[] __initconst = { "ref_cpu", "ref_xtal", }; diff --git a/drivers/clk/mxs/clk-ref.c b/drivers/clk/mxs/clk-ref.c index 4adeed6..ad3851c 100644 --- a/drivers/clk/mxs/clk-ref.c +++ b/drivers/clk/mxs/clk-ref.c @@ -16,6 +16,8 @@ #include <linux/slab.h> #include "clk.h" +#define BF_CLKGATE BIT(7) + /** * struct clk_ref - mxs reference clock * @hw: clk_hw for the reference clock @@ -39,7 +41,7 @@ static int clk_ref_enable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writel_relaxed(1 << ((ref->idx + 1) * 8 - 1), ref->reg + CLR); + writeb_relaxed(BF_CLKGATE, ref->reg + ref->idx + CLR); return 0; } @@ -48,7 +50,7 @@ static void clk_ref_disable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writel_relaxed(1 << ((ref->idx + 1) * 8 - 1), ref->reg + SET); + writeb_relaxed(BF_CLKGATE, ref->reg + ref->idx + SET); } static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, @@ -56,7 +58,7 @@ static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, { struct clk_ref *ref = to_clk_ref(hw); u64 tmp = parent_rate; - u8 frac = (readl_relaxed(ref->reg) >> (ref->idx * 8)) & 0x3f; + u8 frac = readb_relaxed(ref->reg + ref->idx) & 0x3f; tmp *= 18; do_div(tmp, frac); @@ -93,8 +95,7 @@ static int clk_ref_set_rate(struct clk_hw *hw, unsigned long rate, struct clk_ref *ref = to_clk_ref(hw); unsigned long flags; u64 tmp = parent_rate; - u32 val; - u8 frac, shift = ref->idx * 8; + u8 frac, val; tmp = tmp * 18 + rate / 2; do_div(tmp, rate); @@ -107,10 +108,10 @@ static int clk_ref_set_rate(struct clk_hw *hw, unsigned long rate, spin_lock_irqsave(&mxs_lock, flags); - val = readl_relaxed(ref->reg); - val &= ~(0x3f << shift); - val |= frac << shift; - writel_relaxed(val, ref->reg); + val = readb_relaxed(ref->reg + ref->idx); + val &= ~0x3f; + val |= frac; + writeb_relaxed(val, ref->reg + ref->idx); spin_unlock_irqrestore(&mxs_lock, flags);
According to i.MX23 and i.MX28 reference manual the fractional clock control registers must be addressed by byte instructions. This patch fixes the erroneous 32-bit access to these registers and extends the comment in the init functions. Btw the imx23 init now uses a R-M-W sequence just like imx28 init to avoid any clock glitches. The changes has been tested only with a i.MX28 board, because i don't have access to an i.MX23 board. Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> --- Changes in V2: - use relaxed access operations in clk-ref drivers/clk/mxs/clk-imx23.c | 11 ++++++++--- drivers/clk/mxs/clk-imx28.c | 19 +++++++++++++------ drivers/clk/mxs/clk-ref.c | 19 ++++++++++--------- 3 files changed, 31 insertions(+), 18 deletions(-)