[PATCH] sh: sh7785lcr clock fixes

[PATCH] sh: sh7785lcr clock fixes

[Magnus Damm]

From: Magnus Damm <damm@igel.co.jp>

Update the sh7785lcr board defconfigs to fix
PCLK values and add mode4 run time check.

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 arch/sh/boards/board-sh7785lcr.c          |   10 ++++++++++
 arch/sh/configs/sh7785lcr_32bit_defconfig |    2 +-
 arch/sh/configs/sh7785lcr_defconfig       |    2 +-
 3 files changed, 12 insertions(+), 2 deletions(-)

--- 0001/arch/sh/boards/board-sh7785lcr.c
+++ work/arch/sh/boards/board-sh7785lcr.c	2009-05-07 18:34:15.000000000 +0900
@@ -289,6 +289,9 @@ static void sh7785lcr_power_off(void)
 		cpu_relax();
 }
 
+#define PNCR 0xffe70018
+#define PNDR 0xffe70038
+
 /* Initialize the board */
 static void __init sh7785lcr_setup(char **cmdline_p)
 {
@@ -301,6 +304,13 @@ static void __init sh7785lcr_setup(char 
 	/* sm501 DRAM configuration */
 	sm501_reg = (void __iomem *)0xb3e00000 + SM501_DRAM_CONTROL;
 	writel(0x000307c2, sm501_reg);
+
+	/* read PN5 pin to get MODE4/PLL configuration for clock code */
+	ctrl_outw(ctrl_inw(PNCR) | 0x0c00, PNCR);
+	if (ctrl_inb(PNDR) & 0x20)
+		parse_early_options("mode4_pin=high");
+	else
+		parse_early_options("mode4_pin=low");
 }
 
 /*
--- 0001/arch/sh/configs/sh7785lcr_32bit_defconfig
+++ work/arch/sh/configs/sh7785lcr_32bit_defconfig	2009-05-07 18:34:23.000000000 +0900
@@ -258,7 +258,7 @@ CONFIG_SH_SH7785LCR=y
 #
 CONFIG_SH_TMU=y
 CONFIG_SH_TIMER_IRQ(
-CONFIG_SH_PCLK_FREQP000000
+CONFIG_SH_PCLK_FREQ3333333
 CONFIG_TICK_ONESHOT=y
 # CONFIG_NO_HZ is not set
 CONFIG_HIGH_RES_TIMERS=y
--- 0001/arch/sh/configs/sh7785lcr_defconfig
+++ work/arch/sh/configs/sh7785lcr_defconfig	2009-05-07 18:34:23.000000000 +0900
@@ -252,7 +252,7 @@ CONFIG_SH_SH7785LCR_29BIT_PHYSMAPS=y
 #
 CONFIG_SH_TMU=y
 CONFIG_SH_TIMER_IRQ(
-CONFIG_SH_PCLK_FREQP000000
+CONFIG_SH_PCLK_FREQ3333333
 CONFIG_TICK_ONESHOT=y
 # CONFIG_NO_HZ is not set
 CONFIG_HIGH_RES_TIMERS=y

Re: [PATCH] sh: sh7785lcr clock fixes

[Paul Mundt]

On Thu, May 07, 2009 at 07:48:13PM +0900, Magnus Damm wrote:
> --- 0001/arch/sh/boards/board-sh7785lcr.c
> +++ work/arch/sh/boards/board-sh7785lcr.c	2009-05-07 18:34:15.000000000 +0900
> @@ -289,6 +289,9 @@ static void sh7785lcr_power_off(void)
>  		cpu_relax();
>  }
>  
> +#define PNCR 0xffe70018
> +#define PNDR 0xffe70038
> +
>  /* Initialize the board */
>  static void __init sh7785lcr_setup(char **cmdline_p)
>  {
> @@ -301,6 +304,13 @@ static void __init sh7785lcr_setup(char 
>  	/* sm501 DRAM configuration */
>  	sm501_reg = (void __iomem *)0xb3e00000 + SM501_DRAM_CONTROL;
>  	writel(0x000307c2, sm501_reg);
> +
> +	/* read PN5 pin to get MODE4/PLL configuration for clock code */
> +	ctrl_outw(ctrl_inw(PNCR) | 0x0c00, PNCR);
> +	if (ctrl_inb(PNDR) & 0x20)
> +		parse_early_options("mode4_pin=high");
> +	else
> +		parse_early_options("mode4_pin=low");
>  }
>  
>  /*

This seems like a pretty lame way to avoid making a function call. Beyond
that, all you really want to do is:

	mode4_pin = !!(__raw_readb(PNDR) & 0x20);

Just have something like a __setup_mode4_pin() that does the real work,
and expose that as an accessor. The string parsing especially is utterly
superfluous.

On Thu, May 07, 2009 at 07:50:56PM +0900, Magnus Damm wrote:
> --- 0001/arch/sh/kernel/cpu/sh4a/clock-sh7785.c
> +++ work/arch/sh/kernel/cpu/sh4a/clock-sh7785.c	2009-05-07 16:28:24.000000000 +0900
> @@ -15,6 +15,22 @@
>  #include <asm/freq.h>
>  #include <asm/io.h>
>  
> +static int mode4_pin;
> +
> +static int __init setup_mode4_pin(char *buf)
> +{
> +	if (buf) {
> +		if (strstr(buf, "high"))
> +			mode4_pin = 1;
> +
> +		if (strstr(buf, "low"))
> +			mode4_pin = 0;
> +	}
> +
> +	return 0;
> +}
> +early_param("mode4_pin", setup_mode4_pin);
> +
I wonder how you really want to handle this, this will permit the kernel
param to clobber whatever the PNDR state returns, is that really what you
want? Anyways, the string parsing here too should be taken out and shot.
1 and 0 are quite acceptable values for a pin state, if folks can't work
that out, they shouldn't be toggling the pin in the first place.