* [U-Boot] [PATCH 0/4] arm: mxs: mxs_set_gpmiclk
@ 2019-09-12 9:17 Rasmus Villemoes
2019-09-12 9:17 ` [U-Boot] [PATCH 1/4] arm: mxs: fix register definitions for clkctrl_gpmi and clkctrl_sspX Rasmus Villemoes
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Rasmus Villemoes @ 2019-09-12 9:17 UTC (permalink / raw)
To: u-boot
While trying to implement an mxs_set_gpmiclk() I stumbled on a few minor things.
Rasmus Villemoes (4):
arm: mxs: fix register definitions for clkctrl_gpmi and clkctrl_sspX
arm: mxs: fix comments in arch_cpu_init to match the code
arm: mxs: be more careful when enabling gpmi_clk
arm: mxs: implement mxs_set_gpmiclk
arch/arm/cpu/arm926ejs/mxs/clock.c | 41 +++++++++++++++++++
arch/arm/cpu/arm926ejs/mxs/mxs.c | 9 ++--
arch/arm/include/asm/arch-mxs/clock.h | 1 +
.../include/asm/arch-mxs/regs-clkctrl-mx23.h | 6 ++-
.../include/asm/arch-mxs/regs-clkctrl-mx28.h | 15 ++++---
5 files changed, 62 insertions(+), 10 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/4] arm: mxs: fix register definitions for clkctrl_gpmi and clkctrl_sspX
2019-09-12 9:17 [U-Boot] [PATCH 0/4] arm: mxs: mxs_set_gpmiclk Rasmus Villemoes
@ 2019-09-12 9:17 ` Rasmus Villemoes
2019-09-12 9:17 ` [U-Boot] [PATCH 2/4] arm: mxs: fix comments in arch_cpu_init to match the code Rasmus Villemoes
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Rasmus Villemoes @ 2019-09-12 9:17 UTC (permalink / raw)
To: u-boot
I tried clearing a bit by writing to hw_clkctrl_gpmi_clr, then
busy-waiting for it to actually clear. My board hung. The data sheet
agrees, these registers do not have _set, _clr, _tog, so fix up the
definitions. git grep -E 'clkctrl_(gpmi|ssp[0-9])_' says that nobody
uses those non-existing ops registers.
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
arch/arm/include/asm/arch-mxs/regs-clkctrl-mx23.h | 6 ++++--
arch/arm/include/asm/arch-mxs/regs-clkctrl-mx28.h | 15 ++++++++++-----
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/arch/arm/include/asm/arch-mxs/regs-clkctrl-mx23.h b/arch/arm/include/asm/arch-mxs/regs-clkctrl-mx23.h
index 6e9ffeb6d5..50fdc9cd03 100644
--- a/arch/arm/include/asm/arch-mxs/regs-clkctrl-mx23.h
+++ b/arch/arm/include/asm/arch-mxs/regs-clkctrl-mx23.h
@@ -24,8 +24,10 @@ struct mxs_clkctrl_regs {
mxs_reg_32(hw_clkctrl_xbus) /* 0x40 */
mxs_reg_32(hw_clkctrl_xtal) /* 0x50 */
mxs_reg_32(hw_clkctrl_pix) /* 0x60 */
- mxs_reg_32(hw_clkctrl_ssp0) /* 0x70 */
- mxs_reg_32(hw_clkctrl_gpmi) /* 0x80 */
+ uint32_t hw_clkctrl_ssp0; /* 0x70 */
+ uint32_t reserved_ssp0[3]; /* 0x74-0x7c */
+ uint32_t hw_clkctrl_gpmi; /* 0x80 */
+ uint32_t reserved_gpmi[3]; /* 0x84-0x8c */
mxs_reg_32(hw_clkctrl_spdif) /* 0x90 */
mxs_reg_32(hw_clkctrl_emi) /* 0xa0 */
diff --git a/arch/arm/include/asm/arch-mxs/regs-clkctrl-mx28.h b/arch/arm/include/asm/arch-mxs/regs-clkctrl-mx28.h
index 01e0a7a053..caef9e4b1f 100644
--- a/arch/arm/include/asm/arch-mxs/regs-clkctrl-mx28.h
+++ b/arch/arm/include/asm/arch-mxs/regs-clkctrl-mx28.h
@@ -27,11 +27,16 @@ struct mxs_clkctrl_regs {
mxs_reg_32(hw_clkctrl_hbus) /* 0x60 */
mxs_reg_32(hw_clkctrl_xbus) /* 0x70 */
mxs_reg_32(hw_clkctrl_xtal) /* 0x80 */
- mxs_reg_32(hw_clkctrl_ssp0) /* 0x90 */
- mxs_reg_32(hw_clkctrl_ssp1) /* 0xa0 */
- mxs_reg_32(hw_clkctrl_ssp2) /* 0xb0 */
- mxs_reg_32(hw_clkctrl_ssp3) /* 0xc0 */
- mxs_reg_32(hw_clkctrl_gpmi) /* 0xd0 */
+ uint32_t hw_clkctrl_ssp0; /* 0x90 */
+ uint32_t reserved_ssp0[3]; /* 0x94-0x9c */
+ uint32_t hw_clkctrl_ssp1; /* 0xa0 */
+ uint32_t reserved_ssp1[3]; /* 0xa4-0xac */
+ uint32_t hw_clkctrl_ssp2; /* 0xb0 */
+ uint32_t reserved_ssp2[3]; /* 0xb4-0xbc */
+ uint32_t hw_clkctrl_ssp3; /* 0xc0 */
+ uint32_t reserved_ssp3[3]; /* 0xc4-0xcc */
+ uint32_t hw_clkctrl_gpmi; /* 0xd0 */
+ uint32_t reserved_gpmi[3]; /* 0xd4-0xdc */
mxs_reg_32(hw_clkctrl_spdif) /* 0xe0 */
mxs_reg_32(hw_clkctrl_emi) /* 0xf0 */
mxs_reg_32(hw_clkctrl_saif0) /* 0x100 */
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 2/4] arm: mxs: fix comments in arch_cpu_init to match the code
2019-09-12 9:17 [U-Boot] [PATCH 0/4] arm: mxs: mxs_set_gpmiclk Rasmus Villemoes
2019-09-12 9:17 ` [U-Boot] [PATCH 1/4] arm: mxs: fix register definitions for clkctrl_gpmi and clkctrl_sspX Rasmus Villemoes
@ 2019-09-12 9:17 ` Rasmus Villemoes
2019-09-12 9:17 ` [U-Boot] [PATCH 3/4] arm: mxs: be more careful when enabling gpmi_clk Rasmus Villemoes
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Rasmus Villemoes @ 2019-09-12 9:17 UTC (permalink / raw)
To: u-boot
The comment says to clear the bypass bit, but in fact it sets it, thus
selecting ref_xtal. And the next line of code does not set the divider
to 12, but to (the reset value of) 1.
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
arch/arm/cpu/arm926ejs/mxs/mxs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/mxs.c b/arch/arm/cpu/arm926ejs/mxs/mxs.c
index 85c65dcb44..585c53baf6 100644
--- a/arch/arm/cpu/arm926ejs/mxs/mxs.c
+++ b/arch/arm/cpu/arm926ejs/mxs/mxs.c
@@ -98,11 +98,11 @@ int arch_cpu_init(void)
/*
* Enable NAND clock
*/
- /* Clear bypass bit */
+ /* Set bypass bit */
writel(CLKCTRL_CLKSEQ_BYPASS_GPMI,
&clkctrl_regs->hw_clkctrl_clkseq_set);
- /* Set GPMI clock to ref_gpmi / 12 */
+ /* Set GPMI clock to ref_xtal / 1 */
clrsetbits_le32(&clkctrl_regs->hw_clkctrl_gpmi,
CLKCTRL_GPMI_CLKGATE | CLKCTRL_GPMI_DIV_MASK, 1);
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 3/4] arm: mxs: be more careful when enabling gpmi_clk
2019-09-12 9:17 [U-Boot] [PATCH 0/4] arm: mxs: mxs_set_gpmiclk Rasmus Villemoes
2019-09-12 9:17 ` [U-Boot] [PATCH 1/4] arm: mxs: fix register definitions for clkctrl_gpmi and clkctrl_sspX Rasmus Villemoes
2019-09-12 9:17 ` [U-Boot] [PATCH 2/4] arm: mxs: fix comments in arch_cpu_init to match the code Rasmus Villemoes
@ 2019-09-12 9:17 ` Rasmus Villemoes
2019-09-12 9:17 ` [U-Boot] [PATCH 4/4] arm: mxs: implement mxs_set_gpmiclk Rasmus Villemoes
2019-09-27 8:33 ` [U-Boot] [PATCH 0/4] arm: mxs: mxs_set_gpmiclk Rasmus Villemoes
4 siblings, 0 replies; 8+ messages in thread
From: Rasmus Villemoes @ 2019-09-12 9:17 UTC (permalink / raw)
To: u-boot
The data sheet says that the DIV field cannot change while the CLKGATE
bit is set or modified. So do it a little more carefully, by first
clearing the bit, waiting for that to appear, then setting the DIV
field.
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
arch/arm/cpu/arm926ejs/mxs/mxs.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/mxs.c b/arch/arm/cpu/arm926ejs/mxs/mxs.c
index 585c53baf6..183aa40b6d 100644
--- a/arch/arm/cpu/arm926ejs/mxs/mxs.c
+++ b/arch/arm/cpu/arm926ejs/mxs/mxs.c
@@ -103,8 +103,11 @@ int arch_cpu_init(void)
&clkctrl_regs->hw_clkctrl_clkseq_set);
/* Set GPMI clock to ref_xtal / 1 */
+ clrbits_le32(&clkctrl_regs->hw_clkctrl_gpmi, CLKCTRL_GPMI_CLKGATE);
+ while (readl(&clkctrl_regs->hw_clkctrl_gpmi) & CLKCTRL_GPMI_CLKGATE)
+ ;
clrsetbits_le32(&clkctrl_regs->hw_clkctrl_gpmi,
- CLKCTRL_GPMI_CLKGATE | CLKCTRL_GPMI_DIV_MASK, 1);
+ CLKCTRL_GPMI_DIV_MASK, 1);
udelay(1000);
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 4/4] arm: mxs: implement mxs_set_gpmiclk
2019-09-12 9:17 [U-Boot] [PATCH 0/4] arm: mxs: mxs_set_gpmiclk Rasmus Villemoes
` (2 preceding siblings ...)
2019-09-12 9:17 ` [U-Boot] [PATCH 3/4] arm: mxs: be more careful when enabling gpmi_clk Rasmus Villemoes
@ 2019-09-12 9:17 ` Rasmus Villemoes
2019-09-27 8:33 ` [U-Boot] [PATCH 0/4] arm: mxs: mxs_set_gpmiclk Rasmus Villemoes
4 siblings, 0 replies; 8+ messages in thread
From: Rasmus Villemoes @ 2019-09-12 9:17 UTC (permalink / raw)
To: u-boot
This allows a board file to set the gpmiclk to something other than
the default 24 MHz based on ref_xtal.
I don't have an mx23-based board, but I believe there's a bug in the
current mxs_get_gpmiclk: According to the data sheet, the gpmiclk can
be derived from ref_io, not ref_cpu. Since other clocks are also
derived from ref_io, it seems most sensible to require the board file
to set that appropriately first, then derive the divider from its
current setting.
For mx28 boards, OTOH, there's a separate ref_gpmi only used for
clk_gpmi. For simplicity, if !xtal, simply enable that at its maximum
frequency.
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
arch/arm/cpu/arm926ejs/mxs/clock.c | 41 +++++++++++++++++++++++++++
arch/arm/include/asm/arch-mxs/clock.h | 1 +
2 files changed, 42 insertions(+)
diff --git a/arch/arm/cpu/arm926ejs/mxs/clock.c b/arch/arm/cpu/arm926ejs/mxs/clock.c
index 43d044d917..d247da56fe 100644
--- a/arch/arm/cpu/arm926ejs/mxs/clock.c
+++ b/arch/arm/cpu/arm926ejs/mxs/clock.c
@@ -138,6 +138,47 @@ static uint32_t mxs_get_gpmiclk(void)
return (PLL_FREQ_MHZ * PLL_FREQ_COEF / frac) / div;
}
+void mxs_set_gpmiclk(uint32_t freq, int xtal)
+{
+ struct mxs_clkctrl_regs *clkctrl_regs =
+ (struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;
+ uint32_t clk, div;
+
+ if (xtal) {
+ clk = XTAL_FREQ_KHZ;
+ } else {
+#if defined(CONFIG_MX23)
+ clk = mxs_get_ioclk(MXC_IOCLK0);
+#elif defined(CONFIG_MX28)
+ /* enable ref_gpmi at 480 MHz. */
+ writeb(CLKCTRL_FRAC_CLKGATE,
+ &clkctrl_regs->hw_clkctrl_frac1_set[CLKCTRL_FRAC1_GPMI]);
+ writeb(CLKCTRL_FRAC_CLKGATE | PLL_FREQ_COEF,
+ &clkctrl_regs->hw_clkctrl_frac1[CLKCTRL_FRAC1_GPMI]);
+ writeb(CLKCTRL_FRAC_CLKGATE,
+ &clkctrl_regs->hw_clkctrl_frac1_clr[CLKCTRL_FRAC1_GPMI]);
+ clk = PLL_FREQ_KHZ;
+#endif
+ }
+ if (freq > clk)
+ return;
+ div = clk / freq;
+ if (div > CLKCTRL_GPMI_DIV_MASK)
+ div = CLKCTRL_GPMI_DIV_MASK;
+
+ clrbits_le32(&clkctrl_regs->hw_clkctrl_gpmi, CLKCTRL_GPMI_CLKGATE);
+ while (readl(&clkctrl_regs->hw_clkctrl_gpmi) & CLKCTRL_GPMI_CLKGATE)
+ ;
+ clrsetbits_le32(&clkctrl_regs->hw_clkctrl_gpmi, CLKCTRL_GPMI_DIV_MASK, div);
+ while (readl(&clkctrl_regs->hw_clkctrl_gpmi) & CLKCTRL_GPMI_BUSY)
+ ;
+
+ if (xtal)
+ writel(CLKCTRL_CLKSEQ_BYPASS_GPMI, &clkctrl_regs->hw_clkctrl_clkseq_set);
+ else
+ writel(CLKCTRL_CLKSEQ_BYPASS_GPMI, &clkctrl_regs->hw_clkctrl_clkseq_clr);
+}
+
/*
* Set IO clock frequency, in kHz
*/
diff --git a/arch/arm/include/asm/arch-mxs/clock.h b/arch/arm/include/asm/arch-mxs/clock.h
index ee56d10fec..0a6625eb90 100644
--- a/arch/arm/include/asm/arch-mxs/clock.h
+++ b/arch/arm/include/asm/arch-mxs/clock.h
@@ -44,6 +44,7 @@ uint32_t mxc_get_clock(enum mxc_clock clk);
void mxs_set_ioclk(enum mxs_ioclock io, uint32_t freq);
void mxs_set_sspclk(enum mxs_sspclock ssp, uint32_t freq, int xtal);
+void mxs_set_gpmiclk(uint32_t freq, int xtal);
void mxs_set_ssp_busclock(unsigned int bus, uint32_t freq);
void mxs_set_lcdclk(uint32_t __maybe_unused lcd_base, uint32_t freq);
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 0/4] arm: mxs: mxs_set_gpmiclk
2019-09-12 9:17 [U-Boot] [PATCH 0/4] arm: mxs: mxs_set_gpmiclk Rasmus Villemoes
` (3 preceding siblings ...)
2019-09-12 9:17 ` [U-Boot] [PATCH 4/4] arm: mxs: implement mxs_set_gpmiclk Rasmus Villemoes
@ 2019-09-27 8:33 ` Rasmus Villemoes
2020-01-03 12:42 ` Stefano Babic
4 siblings, 1 reply; 8+ messages in thread
From: Rasmus Villemoes @ 2019-09-27 8:33 UTC (permalink / raw)
To: u-boot
On 12/09/2019 11.17, Rasmus Villemoes wrote:
> While trying to implement an mxs_set_gpmiclk() I stumbled on a few minor things.
>
> Rasmus Villemoes (4):
> arm: mxs: fix register definitions for clkctrl_gpmi and clkctrl_sspX
> arm: mxs: fix comments in arch_cpu_init to match the code
> arm: mxs: be more careful when enabling gpmi_clk
> arm: mxs: implement mxs_set_gpmiclk
ping
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/4] arm: mxs: mxs_set_gpmiclk
2019-09-27 8:33 ` [U-Boot] [PATCH 0/4] arm: mxs: mxs_set_gpmiclk Rasmus Villemoes
@ 2020-01-03 12:42 ` Stefano Babic
2020-01-04 14:13 ` Rasmus Villemoes
0 siblings, 1 reply; 8+ messages in thread
From: Stefano Babic @ 2020-01-03 12:42 UTC (permalink / raw)
To: u-boot
Hi Rasmus,
On 27/09/19 10:33, Rasmus Villemoes wrote:
> On 12/09/2019 11.17, Rasmus Villemoes wrote:
>> While trying to implement an mxs_set_gpmiclk() I stumbled on a few minor things.
>>
>> Rasmus Villemoes (4):
>> arm: mxs: fix register definitions for clkctrl_gpmi and clkctrl_sspX
>> arm: mxs: fix comments in arch_cpu_init to match the code
>> arm: mxs: be more careful when enabling gpmi_clk
>> arm: mxs: implement mxs_set_gpmiclk
>
> ping
>
You're right, patch was lost (..just because the list of patches for
i.MX was very long and I have not seen..), I found the series again.
I applied the first 3 patches, they are improvements / clarifications.
The fourth patch has no user, so I ask who is suing it. Nevertheless,
patch 4/4 breaks some boards (I found at least "xfi3") because it is
compiled even for not i.MX boards and then prototype is missing:
arm: + xfi3
+arch/arm/cpu/arm926ejs/mxs/clock.c: In function 'mxs_set_gpmiclk':
+arch/arm/cpu/arm926ejs/mxs/clock.c:151:9: error: implicit declaration
of function 'mxs_get_ioclk'; did you mean 'mxs_set_ioclk'?
[-Werror=implicit-function-declaration]
+ clk = mxs_get_ioclk(MXC_IOCLK0);
+ ^~~~~~~~~~~~~
+ mxs_set_ioclk
+arch/arm/cpu/arm926ejs/mxs/clock.c: At top level:
+arch/arm/cpu/arm926ejs/mxs/clock.c:218:17: error: conflicting types for
'mxs_get_ioclk'
+ static uint32_t mxs_get_ioclk(enum mxs_ioclock io)
+ ^~~~~~~~~~~~~
+arch/arm/cpu/arm926ejs/mxs/clock.c:151:9: note: previous implicit
declaration of 'mxs_get_ioclk' was here
+cc1: all warnings being treated as errors
+make[3]: *** [arch/arm/cpu/arm926ejs/mxs/clock.o] Error 1
+make[2]: *** [arch/arm/cpu/arm926ejs/mxs] Error 2
+make[1]: *** [arch/arm/cpu/arm926ejs] Error 2
+make: *** [sub-make] Error 2
I have nothing against to merge this, too, but the issue above must be
fixed - thanks !
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/4] arm: mxs: mxs_set_gpmiclk
2020-01-03 12:42 ` Stefano Babic
@ 2020-01-04 14:13 ` Rasmus Villemoes
0 siblings, 0 replies; 8+ messages in thread
From: Rasmus Villemoes @ 2020-01-04 14:13 UTC (permalink / raw)
To: u-boot
On 03/01/2020 13.42, Stefano Babic wrote:
> Hi Rasmus,
>
> On 27/09/19 10:33, Rasmus Villemoes wrote:
>> On 12/09/2019 11.17, Rasmus Villemoes wrote:
>>> While trying to implement an mxs_set_gpmiclk() I stumbled on a few minor things.
>>>
>>> Rasmus Villemoes (4):
>>> arm: mxs: fix register definitions for clkctrl_gpmi and clkctrl_sspX
>>> arm: mxs: fix comments in arch_cpu_init to match the code
>>> arm: mxs: be more careful when enabling gpmi_clk
>>> arm: mxs: implement mxs_set_gpmiclk
>>
>> ping
>>
>
> You're right, patch was lost (..just because the list of patches for
> i.MX was very long and I have not seen..), I found the series again.
>
> I applied the first 3 patches, they are improvements / clarifications.
Thanks.
> The fourth patch has no user, so I ask who is suing it.
I had a user in the branch I was working on, from which I tried to carve
out some upstreamable chunks. Unfortunately, the focus internally has
now shifted away from that mx28 board, so I don't know if and when I'll
be working on that again. But I'll try to remember to fix the break you
report the next time around.
Thanks,
Rasmus
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-01-04 14:13 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-12 9:17 [U-Boot] [PATCH 0/4] arm: mxs: mxs_set_gpmiclk Rasmus Villemoes
2019-09-12 9:17 ` [U-Boot] [PATCH 1/4] arm: mxs: fix register definitions for clkctrl_gpmi and clkctrl_sspX Rasmus Villemoes
2019-09-12 9:17 ` [U-Boot] [PATCH 2/4] arm: mxs: fix comments in arch_cpu_init to match the code Rasmus Villemoes
2019-09-12 9:17 ` [U-Boot] [PATCH 3/4] arm: mxs: be more careful when enabling gpmi_clk Rasmus Villemoes
2019-09-12 9:17 ` [U-Boot] [PATCH 4/4] arm: mxs: implement mxs_set_gpmiclk Rasmus Villemoes
2019-09-27 8:33 ` [U-Boot] [PATCH 0/4] arm: mxs: mxs_set_gpmiclk Rasmus Villemoes
2020-01-03 12:42 ` Stefano Babic
2020-01-04 14:13 ` Rasmus Villemoes
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.