[PATCH 1/2] Add correct PLL settings for DragonBall VZ.

[PATCH 1/2] Add correct PLL settings for DragonBall VZ.

[Daniel Palmer]

Signed-off-by: Daniel Palmer <danieruru@gmail.com>
---
 arch/m68k/platform/68000/head.S | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/m68k/platform/68000/head.S b/arch/m68k/platform/68000/head.S
index 536ef96..0d00489 100644
--- a/arch/m68k/platform/68000/head.S
+++ b/arch/m68k/platform/68000/head.S
@@ -91,12 +91,18 @@ _start:
 	movew	#0xe100, 0xfffff900		/* enable */
 #endif /* DEBUG_HEAD */
 
-#ifdef CONFIG_PILOT
+#if defined(CONFIG_PILOT)
 	movew	#0x2410, 0xfffff200		/* PLLCR */
+#elif defined(CONFIG_M68VZ328)
+	movew	#0x2493, 0xfffff200		/* PLLCR */
 #else
 	movew	#0x2400, 0xfffff200		/* PLLCR */
 #endif
+#if defined(CONFIG_M68VZ328)
+	movew	#0x0347, 0xfffff202		/* PLLFSR */
+#else
 	movew	#0x0123, 0xfffff202		/* PLLFSR */
+#endif
 	moveq	#0, %d0
 	movew	#16384, %d0			/* PLL settle wait loop */
 _pll_settle:
-- 
1.9.1

[PATCH 2/2] Fix mach_sched_init for EZ and VZ DragonBall chips.

[Daniel Palmer]

Signed-off-by: Daniel Palmer <danieruru@gmail.com>
---
 arch/m68k/platform/68000/m68EZ328.c | 3 ++-
 arch/m68k/platform/68000/m68VZ328.c | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/m68k/platform/68000/m68EZ328.c b/arch/m68k/platform/68000/m68EZ328.c
index 332b5e8..2195290 100644
--- a/arch/m68k/platform/68000/m68EZ328.c
+++ b/arch/m68k/platform/68000/m68EZ328.c
@@ -69,7 +69,8 @@ void __init config_BSP(char *command, int len)
   if (p) strcpy(p,command);
   else command[0] = 0;
 #endif
- 
+
+  mach_sched_init = hw_timer_init;
   mach_hwclk = m68328_hwclk;
   mach_reset = m68ez328_reset;
 }
diff --git a/arch/m68k/platform/68000/m68VZ328.c b/arch/m68k/platform/68000/m68VZ328.c
index fd66583..0e5e5a1 100644
--- a/arch/m68k/platform/68000/m68VZ328.c
+++ b/arch/m68k/platform/68000/m68VZ328.c
@@ -182,6 +182,7 @@ void __init config_BSP(char *command, int size)
 
 	init_hardware(command, size);
 
+	mach_sched_init = hw_timer_init;
 	mach_hwclk = m68328_hwclk;
 	mach_reset = m68vz328_reset;
 }
-- 
1.9.1

Re: [PATCH 2/2] Fix mach_sched_init for EZ and VZ DragonBall chips.

[Greg Ungerer]

Hi Daniel,

On 03/04/14 00:43, Daniel Palmer wrote:
> Signed-off-by: Daniel Palmer <danieruru@gmail.com>

Looks good. As soon as the current merge window has closed I'll push
this into the m68knommu git tree.

Regards
Greg



> ---
>   arch/m68k/platform/68000/m68EZ328.c | 3 ++-
>   arch/m68k/platform/68000/m68VZ328.c | 1 +
>   2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/m68k/platform/68000/m68EZ328.c b/arch/m68k/platform/68000/m68EZ328.c
> index 332b5e8..2195290 100644
> --- a/arch/m68k/platform/68000/m68EZ328.c
> +++ b/arch/m68k/platform/68000/m68EZ328.c
> @@ -69,7 +69,8 @@ void __init config_BSP(char *command, int len)
>     if (p) strcpy(p,command);
>     else command[0] = 0;
>   #endif
> -
> +
> +  mach_sched_init = hw_timer_init;
>     mach_hwclk = m68328_hwclk;
>     mach_reset = m68ez328_reset;
>   }
> diff --git a/arch/m68k/platform/68000/m68VZ328.c b/arch/m68k/platform/68000/m68VZ328.c
> index fd66583..0e5e5a1 100644
> --- a/arch/m68k/platform/68000/m68VZ328.c
> +++ b/arch/m68k/platform/68000/m68VZ328.c
> @@ -182,6 +182,7 @@ void __init config_BSP(char *command, int size)
>
>   	init_hardware(command, size);
>
> +	mach_sched_init = hw_timer_init;
>   	mach_hwclk = m68328_hwclk;
>   	mach_reset = m68vz328_reset;
>   }
>

Re: [PATCH 1/2] Add correct PLL settings for DragonBall VZ.

[Greg Ungerer]

Hi Daniel,

Looks good, nice and small, a single fix.

Can I suggest that you add an arch name to the patch title, so:

   [PATCH 1/2] m68k: Add correct PLL settings for DragonBall VZ

Obviously on this email list we know it is for m68k, but when merged
up stream it makes it much easier from the title to know what it
applies to.


On 03/04/14 00:43, Daniel Palmer wrote:
> Signed-off-by: Daniel Palmer <danieruru@gmail.com>
> ---
>   arch/m68k/platform/68000/head.S | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/m68k/platform/68000/head.S b/arch/m68k/platform/68000/head.S
> index 536ef96..0d00489 100644
> --- a/arch/m68k/platform/68000/head.S
> +++ b/arch/m68k/platform/68000/head.S
> @@ -91,12 +91,18 @@ _start:
>   	movew	#0xe100, 0xfffff900		/* enable */
>   #endif /* DEBUG_HEAD */
>
> -#ifdef CONFIG_PILOT
> +#if defined(CONFIG_PILOT)
>   	movew	#0x2410, 0xfffff200		/* PLLCR */
> +#elif defined(CONFIG_M68VZ328)
> +	movew	#0x2493, 0xfffff200		/* PLLCR */

Would it be cleaner to have a defined value here instead?
This moves the #ifdef'ery to just the PLLCR value, and keeps the
code instructions here cleaner. So something like this at the top of
this head.S:

#if defined(CONFIG_PILOT)
#define PLLCR	0x2410
#elif defined(CONFIG_M68VZ328)
#define PLLCR	0x2493
#else
#define PLLCR	0x2400
#endif

and then just

	movew	#PLLCR, 0xfffff200		/* PLLCR */

Regards
Greg


>   #else
>   	movew	#0x2400, 0xfffff200		/* PLLCR */
>   #endif
> +#if defined(CONFIG_M68VZ328)
> +	movew	#0x0347, 0xfffff202		/* PLLFSR */
> +#else
>   	movew	#0x0123, 0xfffff202		/* PLLFSR */
> +#endif
>   	moveq	#0, %d0
>   	movew	#16384, %d0			/* PLL settle wait loop */
>   _pll_settle:
>

Re: [PATCH 1/2] Add correct PLL settings for DragonBall VZ.

[Geert Uytterhoeven]

On Thu, Apr 3, 2014 at 2:41 PM, Greg Ungerer <gregungerer00@gmail.com> wrote:
>> -#ifdef CONFIG_PILOT
>> +#if defined(CONFIG_PILOT)
>>         movew   #0x2410, 0xfffff200             /* PLLCR */
>> +#elif defined(CONFIG_M68VZ328)
>> +       movew   #0x2493, 0xfffff200             /* PLLCR */
>
>
> Would it be cleaner to have a defined value here instead?
> This moves the #ifdef'ery to just the PLLCR value, and keeps the
> code instructions here cleaner. So something like this at the top of
> this head.S:
>
> #if defined(CONFIG_PILOT)
> #define PLLCR   0x2410
> #elif defined(CONFIG_M68VZ328)
> #define PLLCR   0x2493
> #else
> #define PLLCR   0x2400
> #endif
>
> and then just
>
>         movew   #PLLCR, 0xfffff200              /* PLLCR */

While I like this advice in general, I prefer to use a different name than
"PLLCR" for this definition.

One day someone may want to put "#define PLLCR 0xfffff200" in a
header file...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] Add correct PLL settings for DragonBall VZ.

[Daniel Palmer]

Only slightly related to the patch itself;

At the moment you can select to support multiple DragonBall chips in a
single kernel but I don't think that'll actually work if the PLL
register values are hardcoded like they are at the moment. So I think
the kconfig needs to be changed to only allow support for one chip.
Should I make this change and send a patch?

I doubt there are many people that are attempting to run Linux on
Palms or extremely rare development boards but it does seem like it's
possible to detect what chip the kernel is booting on from the chip id
register. So the kernel could pick the right PLL settings based on the
chip it's booting on and then the same kernel image could be used for
all DragonBall based machines.

Re: [PATCH 1/2] Add correct PLL settings for DragonBall VZ.

[Finn Thain]

On Fri, 4 Apr 2014, Daniel Palmer wrote:

> 
> I doubt there are many people that are attempting to run Linux on Palms 
> or extremely rare development boards but it does seem like it's possible 
> to detect what chip the kernel is booting on from the chip id register. 
> So the kernel could pick the right PLL settings based on the chip it's 
> booting on and then the same kernel image could be used for all 
> DragonBall based machines.

A look at the user manual for the VZ says that the CHIPID register takes 
the value of 0x5600 upon reset, but does anyone know what crystals are in 
use in the various VZ hardware that Linux runs on?

It's more difficult for EZ: a quick look at wikipedia shows that Palm 
shipped MC68EZ328 devices clocked at 16 MHz and 20 MHz. And the user 
manual for the chip doesn't say what the reset value for the silicon ID 
register is.

-- 

Re: [PATCH 1/2] Add correct PLL settings for DragonBall VZ.

[Greg Ungerer]

Hi Daniel,

On 04/04/14 13:18, Daniel Palmer wrote:
> Only slightly related to the patch itself;
>
> At the moment you can select to support multiple DragonBall chips in a
> single kernel but I don't think that'll actually work if the PLL
> register values are hardcoded like they are at the moment. So I think
> the kconfig needs to be changed to only allow support for one chip.
> Should I make this change and send a patch?

Sure, that sounds reasonable.


> I doubt there are many people that are attempting to run Linux on
> Palms or extremely rare development boards but it does seem like it's
> possible to detect what chip the kernel is booting on from the chip id
> register. So the kernel could pick the right PLL settings based on the
> chip it's booting on and then the same kernel image could be used for
> all DragonBall based machines.

The mutually exclusive board selection can be changed back if or when
code gets added to do this. No problem there.

Regards
Greg

Re: [PATCH 1/2] Add correct PLL settings for DragonBall VZ.

[Finn Thain]

On Thu, 3 Apr 2014, Geert Uytterhoeven wrote:

> On Thu, Apr 3, 2014 at 2:41 PM, Greg Ungerer <gregungerer00@gmail.com> wrote:
> >> -#ifdef CONFIG_PILOT
> >> +#if defined(CONFIG_PILOT)
> >>         movew   #0x2410, 0xfffff200             /* PLLCR */
> >> +#elif defined(CONFIG_M68VZ328)
> >> +       movew   #0x2493, 0xfffff200             /* PLLCR */
> >
> >
> > Would it be cleaner to have a defined value here instead?
> > This moves the #ifdef'ery to just the PLLCR value, and keeps the
> > code instructions here cleaner. So something like this at the top of
> > this head.S:
> >
> > #if defined(CONFIG_PILOT)
> > #define PLLCR   0x2410
> > #elif defined(CONFIG_M68VZ328)
> > #define PLLCR   0x2493
> > #else
> > #define PLLCR   0x2400
> > #endif
> >
> > and then just
> >
> >         movew   #PLLCR, 0xfffff200              /* PLLCR */

In this case, the #ifdef ... #elif ... #else ... #endif cascade is already 
present, so I wonder whether your way is in fact better here.

> 
> While I like this advice in general, I prefer to use a different name than
> "PLLCR" for this definition.
> 
> One day someone may want to put "#define PLLCR 0xfffff200" in a
> header file...
> 

It's defined in asm/MC68328.h:

#define PLLCR_ADDR      0xfffff200
...
#define PLLCR_DISPLL           0x0008   /* Disable PLL */
#define PLLCR_CLKEN            0x0010   /* Clock (CLKO pin) enable */
#define PLLCR_SYSCLK_SEL_MASK  0x0700   /* System Clock Selection */
#define PLLCR_SYSCLK_SEL_SHIFT 8
#define PLLCR_PIXCLK_SEL_MASK  0x3800   /* LCD Clock Selection */
#define PLLCR_PIXCLK_SEL_SHIFT 11

Some bit name definitions are missing, i.e.

#define PLLCR_PRESC1           0x0080

-- 

Re: [PATCH 1/2] Add correct PLL settings for DragonBall VZ.

[Greg Ungerer]

On 04/04/14 17:45, Finn Thain wrote:
> On Thu, 3 Apr 2014, Geert Uytterhoeven wrote:
>> On Thu, Apr 3, 2014 at 2:41 PM, Greg Ungerer <gregungerer00@gmail.com> wrote:
>>>> -#ifdef CONFIG_PILOT
>>>> +#if defined(CONFIG_PILOT)
>>>>          movew   #0x2410, 0xfffff200             /* PLLCR */
>>>> +#elif defined(CONFIG_M68VZ328)
>>>> +       movew   #0x2493, 0xfffff200             /* PLLCR */
>>>
>>>
>>> Would it be cleaner to have a defined value here instead?
>>> This moves the #ifdef'ery to just the PLLCR value, and keeps the
>>> code instructions here cleaner. So something like this at the top of
>>> this head.S:
>>>
>>> #if defined(CONFIG_PILOT)
>>> #define PLLCR   0x2410
>>> #elif defined(CONFIG_M68VZ328)
>>> #define PLLCR   0x2493
>>> #else
>>> #define PLLCR   0x2400
>>> #endif
>>>
>>> and then just
>>>
>>>          movew   #PLLCR, 0xfffff200              /* PLLCR */
>
> In this case, the #ifdef ... #elif ... #else ... #endif cascade is already
> present, so I wonder whether your way is in fact better here.

I don't follow what you mean. The intention is to move it away from the
actual code - to make it easier to read. Not add another #elif and make
it worse than it currently is.

Regards
Greg


>> While I like this advice in general, I prefer to use a different name than
>> "PLLCR" for this definition.
>>
>> One day someone may want to put "#define PLLCR 0xfffff200" in a
>> header file...
>>
>
> It's defined in asm/MC68328.h:
>
> #define PLLCR_ADDR      0xfffff200
> ...
> #define PLLCR_DISPLL           0x0008   /* Disable PLL */
> #define PLLCR_CLKEN            0x0010   /* Clock (CLKO pin) enable */
> #define PLLCR_SYSCLK_SEL_MASK  0x0700   /* System Clock Selection */
> #define PLLCR_SYSCLK_SEL_SHIFT 8
> #define PLLCR_PIXCLK_SEL_MASK  0x3800   /* LCD Clock Selection */
> #define PLLCR_PIXCLK_SEL_SHIFT 11
>
> Some bit name definitions are missing, i.e.
>
> #define PLLCR_PRESC1           0x0080
>

Re: [PATCH 1/2] Add correct PLL settings for DragonBall VZ.

[Finn Thain]

On Fri, 4 Apr 2014, Greg Ungerer wrote:

> I don't follow what you mean.

I wasn't being very clear I'm afraid.

> The intention is to move it away from the actual code - to make it 
> easier to read.

Your call.

> Not add another #elif and make it worse than it currently is.

Right.

--