Re: [U-Boot] [PATCH 1/2] mx31: make HSP clock for mx3fb driver available
On Monday, September 05, 2011 01:47:06 PM Helmut Raiger wrote: > This additionally updates mx31/generic.c by > - replacing __REG() macro accesses with readl() and writel() > - providing macros for PDR0 and PLL bit accesses > It also fixes a warning about the prototype of imx_get_uartclk(void) > > Signed-off-by: Helmut Raiger > --- > V2: uses macros and readl(), writel(), see commit message This looks awesome. Please, when submitting V2, also add V2 to [PATCH 1/2] ... like [PATCH V2 1/2]. Minor thing though. Acked-by: Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/2] mx31: make HSP clock for mx3fb driver available
This additionally updates mx31/generic.c by
- replacing __REG() macro accesses with readl() and writel()
- providing macros for PDR0 and PLL bit accesses
It also fixes a warning about the prototype of imx_get_uartclk(void)
Signed-off-by: Helmut Raiger
---
V2: uses macros and readl(), writel(), see commit message
arch/arm/cpu/arm1136/mx31/generic.c | 40 +++-
arch/arm/include/asm/arch-mx31/clock.h|3 +-
arch/arm/include/asm/arch-mx31/imx-regs.h | 14 ++
3 files changed, 43 insertions(+), 14 deletions(-)
diff --git a/arch/arm/cpu/arm1136/mx31/generic.c
b/arch/arm/cpu/arm1136/mx31/generic.c
index 248431b..9a7612b 100644
--- a/arch/arm/cpu/arm1136/mx31/generic.c
+++ b/arch/arm/cpu/arm1136/mx31/generic.c
@@ -28,10 +28,10 @@
static u32 mx31_decode_pll(u32 reg, u32 infreq)
{
- u32 mfi = (reg >> 10) & 0xf;
- u32 mfn = reg & 0x3ff;
- u32 mfd = (reg >> 16) & 0x3ff;
- u32 pd = (reg >> 26) & 0xf;
+ u32 mfi = GET_PLL_MFI(reg);
+ u32 mfn = GET_PLL_MFN(reg);
+ u32 mfd = GET_PLL_MFD(reg);
+ u32 pd = GET_PLL_PD(reg);
mfi = mfi <= 5 ? 5 : mfi;
mfd += 1;
@@ -45,12 +45,12 @@ static u32 mx31_get_mpl_dpdgck_clk(void)
{
u32 infreq;
- if ((__REG(CCM_CCMR) & CCMR_PRCS_MASK) == CCMR_FPM)
+ if ((readl(CCM_CCMR) & CCMR_PRCS_MASK) == CCMR_FPM)
infreq = CONFIG_MX31_CLK32 * 1024;
else
infreq = CONFIG_MX31_HCLK_FREQ;
- return mx31_decode_pll(__REG(CCM_MPCTL), infreq);
+ return mx31_decode_pll(readl(CCM_MPCTL), infreq);
}
static u32 mx31_get_mcu_main_clk(void)
@@ -64,10 +64,21 @@ static u32 mx31_get_mcu_main_clk(void)
static u32 mx31_get_ipg_clk(void)
{
u32 freq = mx31_get_mcu_main_clk();
- u32 pdr0 = __REG(CCM_PDR0);
+ u32 pdr0 = readl(CCM_PDR0);
- freq /= ((pdr0 >> 3) & 0x7) + 1;
- freq /= ((pdr0 >> 6) & 0x3) + 1;
+ freq /= GET_PDR0_MAX_PODF(pdr0) + 1;
+ freq /= GET_PDR0_IPG_PODF(pdr0) + 1;
+
+ return freq;
+}
+
+/* hsp is the clock for the ipu */
+static u32 mx31_get_hsp_clk(void)
+{
+ u32 freq = mx31_get_mcu_main_clk();
+ u32 pdr0 = readl(CCM_PDR0);
+
+ freq /= GET_PDR0_HSP_PODF(pdr0) + 1;
return freq;
}
@@ -77,6 +88,7 @@ void mx31_dump_clocks(void)
u32 cpufreq = mx31_get_mcu_main_clk();
printf("mx31 cpu clock: %dMHz\n",cpufreq / 100);
printf("ipg clock : %dHz\n", mx31_get_ipg_clk());
+ printf("hsp clock : %dHz\n", mx31_get_hsp_clk());
}
unsigned int mxc_get_clock(enum mxc_clock clk)
@@ -88,6 +100,8 @@ unsigned int mxc_get_clock(enum mxc_clock clk)
case MXC_CSPI_CLK:
case MXC_UART_CLK:
return mx31_get_ipg_clk();
+ case MXC_IPU_CLK:
+ return mx31_get_hsp_clk();
}
return -1;
}
@@ -104,10 +118,10 @@ void mx31_gpio_mux(unsigned long mode)
reg = IOMUXC_BASE + (mode & 0x1fc);
shift = (~mode & 0x3) * 8;
- tmp = __REG(reg);
+ tmp = readl(reg);
tmp &= ~(0xff << shift);
tmp |= ((mode >> IOMUX_MODE_POS) & 0xff) << shift;
- __REG(reg) = tmp;
+ writel(tmp, reg);
}
void mx31_set_pad(enum iomux_pins pin, u32 config)
@@ -118,10 +132,10 @@ void mx31_set_pad(enum iomux_pins pin, u32 config)
reg = (IOMUXC_BASE + 0x154) + (pin + 2) / 3 * 4;
field = (pin + 2) % 3;
- l = __REG(reg);
+ l = readl(reg);
l &= ~(0x1ff << (field * 10));
l |= config << (field * 10);
- __REG(reg) = l;
+ writel(l, reg);
}
diff --git a/arch/arm/include/asm/arch-mx31/clock.h
b/arch/arm/include/asm/arch-mx31/clock.h
index fb035c4..1ea5a47 100644
--- a/arch/arm/include/asm/arch-mx31/clock.h
+++ b/arch/arm/include/asm/arch-mx31/clock.h
@@ -29,10 +29,11 @@ enum mxc_clock {
MXC_IPG_CLK,
MXC_CSPI_CLK,
MXC_UART_CLK,
+ MXC_IPU_CLK
};
unsigned int mxc_get_clock(enum mxc_clock clk);
-extern u32 imx_get_uartclk();
+extern u32 imx_get_uartclk(void);
extern void mx31_gpio_mux(unsigned long mode);
extern void mx31_set_pad(enum iomux_pins pin, u32 config);
diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h
b/arch/arm/include/asm/arch-mx31/imx-regs.h
index 3c8d607..c24dae2 100644
--- a/arch/arm/include/asm/arch-mx31/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx31/imx-regs.h
@@ -518,6 +518,20 @@ enum iomux_pins {
#define PLL_MFI(x) (((x) & 0xf) << 10)
#define PLL_MFN(x) (((x) & 0x3ff) << 0)
+#define GET_PDR0_CSI_PODF(x) (((x) >> 23) & 0x1ff)
+#define GET_PDR0_PER_PODF(x) (((x) >> 16) & 0x1f)
+#define GET_PDR0_HSP_PODF(x) (((x) >> 11) & 0x7)
+#define GET_PDR0_NFC_PODF(x) (((x) >> 8) & 0x7)
+#define GET_PDR0_IPG_PODF(x) (((x) >> 6) & 0x3)
+#define GET_PDR0_MAX_PODF(x) (((x) >> 3) & 0x7)
+#define GET_PDR0_MCU_PODF(x) ((x) & 0x7)
+
+#define GET_PLL_PD(x) (((x) >> 26) & 0xf)
+#define GET_PL
Re: [U-Boot] [PATCH 1/2] mx31: make HSP clock for mx3fb driver available
On Wednesday, August 24, 2011 08:55:03 AM Helmut Raiger wrote: > On 08/22/2011 06:02 PM, Marek Vasut wrote: > > ... _COSMETIC_, while this new mx3fb driver really works nicely ;-) > > Oh my sense for metrosexuality of code really kicks in here ;-) > > Arousal was not my first objective, but deliberately endorsed :-P Hehe :-D > > I'll do what can be done easily concerning the bit accesses. > Helmut Thanks! > > -- > Scanned by MailScanner. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] mx31: make HSP clock for mx3fb driver available
On 08/22/2011 06:02 PM, Marek Vasut wrote: > ... _COSMETIC_, while this new mx3fb driver really works nicely ;-) > Oh my sense for metrosexuality of code really kicks in here ;-) Arousal was not my first objective, but deliberately endorsed :-P I'll do what can be done easily concerning the bit accesses. Helmut -- Scanned by MailScanner. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] mx31: make HSP clock for mx3fb driver available
On Monday, August 22, 2011 06:02:10 PM Marek Vasut wrote: > On Monday, August 22, 2011 05:51:05 PM Helmut Raiger wrote: > > On 08/22/2011 05:02 PM, Marek Vasut wrote: > > >> Masks are not defined for i.MX31 (imx-regs.h), so I simply followed > > >> the code, which was in the file. The generic.c file for MX35 however > > >> is a different story. > > >> > > >> To keep things simple, I suggest we leave it at that for now. > > > > > > And noone's gonna fix it later. No please, define the proper constants > > > please. > > > > Do you really expect me to fix code in all the arch/arm/cpu/arm1136/mx31 > > files (2000+ lines) that use that kind of access, because I add 15 lines > > to one of them? There are some 50 other drivers and boards etc. that > > depend on imx-regs.h, which probably use these access methods as-well! > > Hey, calm down, I didn't say anything about fixing gazilion of crap (even > though that'd be nice of you !). > > Actually, I did a quick look and see arch/arm/include/arch-mx35/crm_regs.h > containing PDR0 definitions. Though they don't match your constants. It's > either bug in the crm_regs.h or your code. Ah sorry, it's mx31. Right, the defines are missing. Do what Stefano said then. And really ... noone wants you to modify gazilions of lines, just your few :) Cheers and sorry for misguiding you about mx35. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] mx31: make HSP clock for mx3fb driver available
On 08/22/2011 05:51 PM, Helmut Raiger wrote: > On 08/22/2011 05:02 PM, Marek Vasut wrote: >> >>> Masks are not defined for i.MX31 (imx-regs.h), so I simply followed the >>> code, which was in the file. The generic.c file for MX35 however is a >>> different story. >>> >>> To keep things simple, I suggest we leave it at that for now. >> And noone's gonna fix it later. No please, define the proper constants >> please. >> Hi Helmut, > Do you really expect me to fix code in all the arch/arm/cpu/arm1136/mx31 > files (2000+ lines) that use that kind of access, because I add 15 lines > to one of them? No, nobody thinks you should do this. However, should be not better to fix it for the 15 lines of code you added ? Agree that a lot of things in imx-regs.h for i.MX31 are missing. The i.MX31 was the first i.MX processor added to u-boot, and there are still some parts that are different from the rest of IMX code. However, can you add a simple macro to get the HSP_PODF field ? I agree it does not change the behavior, but it helps a bit to understand the code without running to the i.MX31 manual... Macro to set the PODR register are already defined in imx-regs.h. As you pointed out, there is no macro to get values from the register. >There are some 50 other drivers and boards etc. that > depend on imx-regs.h, which probably use these access methods as-well! Yes, I know. I am trying to uniform the behavior between i.MX processors. > I'm not prepared to take this effort, sorry. Do not do it, it is not asked. We make always a step-by-step approach. Add defines only for the part you added (as I can see, it is only related to get the HSP_PODF field in podr register), and let the rest of the 2000+ lines untouched. Best regards, Stefano Babic -- = DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] mx31: make HSP clock for mx3fb driver available
On Monday, August 22, 2011 05:51:05 PM Helmut Raiger wrote: > On 08/22/2011 05:02 PM, Marek Vasut wrote: > >> Masks are not defined for i.MX31 (imx-regs.h), so I simply followed the > >> code, which was in the file. The generic.c file for MX35 however is a > >> different story. > >> > >> To keep things simple, I suggest we leave it at that for now. > > > > And noone's gonna fix it later. No please, define the proper constants > > please. > > Do you really expect me to fix code in all the arch/arm/cpu/arm1136/mx31 > files (2000+ lines) that use that kind of access, because I add 15 lines > to one of them? There are some 50 other drivers and boards etc. that > depend on imx-regs.h, which probably use these access methods as-well! Hey, calm down, I didn't say anything about fixing gazilion of crap (even though that'd be nice of you !). Actually, I did a quick look and see arch/arm/include/arch-mx35/crm_regs.h containing PDR0 definitions. Though they don't match your constants. It's either bug in the crm_regs.h or your code. > > I'm not prepared to take this effort, sorry. After all this is > _COSMETIC_, while this new mx3fb driver really works nicely ;-) Oh my sense for metrosexuality of code really kicks in here ;-) > If anyone out there modifies these files, I'll definitely rebase the > driver to it. It has nothing to do with your driver, the problem I have is with these few magic numbers you use in 1/2 (make HSP clock ...). You can use constants from crm_regs.h, see above. > > Helmut Cheers! > > > -- > Scanned by MailScanner. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] mx31: make HSP clock for mx3fb driver available
On 08/22/2011 05:02 PM, Marek Vasut wrote: > >> Masks are not defined for i.MX31 (imx-regs.h), so I simply followed the >> code, which was in the file. The generic.c file for MX35 however is a >> different story. >> >> To keep things simple, I suggest we leave it at that for now. > And noone's gonna fix it later. No please, define the proper constants please. > Do you really expect me to fix code in all the arch/arm/cpu/arm1136/mx31 files (2000+ lines) that use that kind of access, because I add 15 lines to one of them? There are some 50 other drivers and boards etc. that depend on imx-regs.h, which probably use these access methods as-well! I'm not prepared to take this effort, sorry. After all this is _COSMETIC_, while this new mx3fb driver really works nicely ;-) If anyone out there modifies these files, I'll definitely rebase the driver to it. Helmut -- Scanned by MailScanner. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] mx31: make HSP clock for mx3fb driver available
On Monday, August 22, 2011 05:00:15 PM Helmut Raiger wrote: > On 08/22/2011 04:00 PM, Marek Vasut wrote: > > + u32 pdr0 = __REG(CCM_PDR0); > > Can't we do readl() here? > > > >> + > >> + /* divided by HSP_PODF in PDR0 */ > >> + freq /= ((pdr0>> 11)& 0x7) + 1; > > > > Um, don't we have defined constants for those magic numbers already? > > Masks are not defined for i.MX31 (imx-regs.h), so I simply followed the > code, which was in the file. The generic.c file for MX35 however is a > different story. > > To keep things simple, I suggest we leave it at that for now. And noone's gonna fix it later. No please, define the proper constants please. Thanks in advance > > Helmut > > > > > > -- > Scanned by MailScanner. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] mx31: make HSP clock for mx3fb driver available
On 08/22/2011 04:00 PM, Marek Vasut wrote: > + u32 pdr0 = __REG(CCM_PDR0); > Can't we do readl() here? > >> + >> +/* divided by HSP_PODF in PDR0 */ >> +freq /= ((pdr0>> 11)& 0x7) + 1; > Um, don't we have defined constants for those magic numbers already? Masks are not defined for i.MX31 (imx-regs.h), so I simply followed the code, which was in the file. The generic.c file for MX35 however is a different story. To keep things simple, I suggest we leave it at that for now. Helmut -- Scanned by MailScanner. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] mx31: make HSP clock for mx3fb driver available
On Monday, August 22, 2011 03:41:36 PM Helmut Raiger wrote:
> Signed-off-by: Helmut Raiger
> ---
> arch/arm/cpu/arm1136/mx31/generic.c| 14 ++
> arch/arm/include/asm/arch-mx31/clock.h |1 +
> 2 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/cpu/arm1136/mx31/generic.c
> b/arch/arm/cpu/arm1136/mx31/generic.c index 248431b..b077d29 100644
> --- a/arch/arm/cpu/arm1136/mx31/generic.c
> +++ b/arch/arm/cpu/arm1136/mx31/generic.c
> @@ -72,6 +72,18 @@ static u32 mx31_get_ipg_clk(void)
> return freq;
> }
>
> +/* hsp is the clock for the ipu */
> +static u32 mx31_get_hsp_clk(void)
> +{
> + u32 freq = mx31_get_mcu_main_clk();
> + u32 pdr0 = __REG(CCM_PDR0);
Can't we do readl() here?
> +
> + /* divided by HSP_PODF in PDR0 */
> + freq /= ((pdr0 >> 11) & 0x7) + 1;
Um, don't we have defined constants for those magic numbers already?
> +
> + return freq;
> +}
> +
> void mx31_dump_clocks(void)
> {
> u32 cpufreq = mx31_get_mcu_main_clk();
> @@ -88,6 +100,8 @@ unsigned int mxc_get_clock(enum mxc_clock clk)
> case MXC_CSPI_CLK:
> case MXC_UART_CLK:
> return mx31_get_ipg_clk();
> + case MXC_IPU_CLK:
> + return mx31_get_hsp_clk();
> }
> return -1;
> }
> diff --git a/arch/arm/include/asm/arch-mx31/clock.h
> b/arch/arm/include/asm/arch-mx31/clock.h index fb035c4..b0aa1ce 100644
> --- a/arch/arm/include/asm/arch-mx31/clock.h
> +++ b/arch/arm/include/asm/arch-mx31/clock.h
> @@ -29,6 +29,7 @@ enum mxc_clock {
> MXC_IPG_CLK,
> MXC_CSPI_CLK,
> MXC_UART_CLK,
> + MXC_IPU_CLK
> };
>
> unsigned int mxc_get_clock(enum mxc_clock clk);
Cheers
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/2] mx31: make HSP clock for mx3fb driver available
Signed-off-by: Helmut Raiger
---
arch/arm/cpu/arm1136/mx31/generic.c| 14 ++
arch/arm/include/asm/arch-mx31/clock.h |1 +
2 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/arch/arm/cpu/arm1136/mx31/generic.c
b/arch/arm/cpu/arm1136/mx31/generic.c
index 248431b..b077d29 100644
--- a/arch/arm/cpu/arm1136/mx31/generic.c
+++ b/arch/arm/cpu/arm1136/mx31/generic.c
@@ -72,6 +72,18 @@ static u32 mx31_get_ipg_clk(void)
return freq;
}
+/* hsp is the clock for the ipu */
+static u32 mx31_get_hsp_clk(void)
+{
+ u32 freq = mx31_get_mcu_main_clk();
+ u32 pdr0 = __REG(CCM_PDR0);
+
+ /* divided by HSP_PODF in PDR0 */
+ freq /= ((pdr0 >> 11) & 0x7) + 1;
+
+ return freq;
+}
+
void mx31_dump_clocks(void)
{
u32 cpufreq = mx31_get_mcu_main_clk();
@@ -88,6 +100,8 @@ unsigned int mxc_get_clock(enum mxc_clock clk)
case MXC_CSPI_CLK:
case MXC_UART_CLK:
return mx31_get_ipg_clk();
+ case MXC_IPU_CLK:
+ return mx31_get_hsp_clk();
}
return -1;
}
diff --git a/arch/arm/include/asm/arch-mx31/clock.h
b/arch/arm/include/asm/arch-mx31/clock.h
index fb035c4..b0aa1ce 100644
--- a/arch/arm/include/asm/arch-mx31/clock.h
+++ b/arch/arm/include/asm/arch-mx31/clock.h
@@ -29,6 +29,7 @@ enum mxc_clock {
MXC_IPG_CLK,
MXC_CSPI_CLK,
MXC_UART_CLK,
+ MXC_IPU_CLK
};
unsigned int mxc_get_clock(enum mxc_clock clk);
--
1.7.4.4
--
Scanned by MailScanner.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
