[RFC][PATCH] Disintegrate sharpsl_param.c

[RFC][PATCH] Disintegrate sharpsl_param.c

[dromede at gmail.com]

From: Marko Katic <dromede.gmail.com>


I recently tried running 3.6-rc2 kernel on my Sharp Zaurus C-1000. It 
hanged almost immediately after uncompressing the kernel. I tracked the
problem down to a single line in arch/arm/common/sharpsl_param.c:

memcpy(&sharpsl_param, (void *)PARAM_BASE, sizeof(struct sharpsl_param_info));

Commenting out the line makes the kernel boot just fine. This left me
wondering whether sharpsl_param.c is actually needed. Here's a comment
from sharpsl_param.c that describes what sharpsl_param.c actually does:

/*
 * Certain hardware parameters determined at the time of device manufacture,
 * typically including LCD parameters are loaded by the bootloader at the
 * address PARAM_BASE. As the kernel will overwrite them, we need to store
 * them early in the boot process, then pass them to the appropriate drivers.
 * Not all devices use all parameters but the format is common to all.
 */
 
 So sharpsl_save_param() is supposed to fill the sharpsl_param_info struct
 with various parameters or fill some of the struct fields with -1. 
 I found only four drivers that use some of these
 parameters (only two parameters are used by these drivers, comadj and phadadj). 
 These drivers are:
 
 drivers/video/backlight/corgi_lcd.c
 drivers/video/backlight/locomolcd.c
 drivers/video/backlight/tosa_bl.c
 drivers/video/backlight/tosa_lcd.c
 
 These drivers also have default values when comadj or phadadj == -1.
 I checked what values are actually contained in this struct in
 earlier kernels. 3.2.24 kernel is the latest one i know of where
 sharpsl_save_param() works properly. And it also has these fields initialised
 to -1.
 
 So it seems to me that sharpsl_param.c is redundant as it currently only  
 serves to assign -1 to a few variables. And drivers that use these variables
 then handle -1 with assigning default values.
 
 I propose a patch that completely removes the usage of sharpsl_param.c.
 I confirm that this patch works fine on a C-1000 and therefore i assume that
 it should work fine on all machines that use the corgi_lcd.c driver.
 
 This patch should be tested on collie and tosa machines.

---
 arch/arm/common/Kconfig                   |    3 -
 arch/arm/common/Makefile                  |    1 -
 arch/arm/common/sharpsl_param.c           |   62 -----------------------------
 arch/arm/include/asm/mach/sharpsl_param.h |   37 -----------------
 arch/arm/mach-pxa/Kconfig                 |    1 -
 arch/arm/mach-pxa/corgi.c                 |    2 -
 arch/arm/mach-pxa/poodle.c                |    2 -
 arch/arm/mach-pxa/spitz.c                 |    2 -
 arch/arm/mach-pxa/tosa.c                  |    2 -
 arch/arm/mach-sa1100/Kconfig              |    1 -
 arch/arm/mach-sa1100/collie.c             |    2 -
 drivers/power/collie_battery.c            |    1 -
 drivers/video/backlight/corgi_lcd.c       |    9 +---
 drivers/video/backlight/locomolcd.c       |    8 +--
 drivers/video/backlight/tosa_bl.c         |    4 +-
 drivers/video/backlight/tosa_lcd.c        |    2 -
 16 files changed, 6 insertions(+), 133 deletions(-)
 delete mode 100644 arch/arm/common/sharpsl_param.c
 delete mode 100644 arch/arm/include/asm/mach/sharpsl_param.h

diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
index 283fa1d..8a2688a 100644
--- a/arch/arm/common/Kconfig
+++ b/arch/arm/common/Kconfig
@@ -35,8 +35,5 @@ config DMABOUNCE
 config SHARP_LOCOMO
 	bool
 
-config SHARP_PARAM
-	bool
-
 config SHARP_SCOOP
 	bool
diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
index e8a4e58..c8634c8 100644
--- a/arch/arm/common/Makefile
+++ b/arch/arm/common/Makefile
@@ -9,7 +9,6 @@ obj-$(CONFIG_SA1111)		+= sa1111.o
 obj-$(CONFIG_PCI_HOST_VIA82C505) += via82c505.o
 obj-$(CONFIG_DMABOUNCE)		+= dmabounce.o
 obj-$(CONFIG_SHARP_LOCOMO)	+= locomo.o
-obj-$(CONFIG_SHARP_PARAM)	+= sharpsl_param.o
 obj-$(CONFIG_SHARP_SCOOP)	+= scoop.o
 obj-$(CONFIG_PCI_HOST_ITE8152)  += it8152.o
 obj-$(CONFIG_ARM_TIMER_SP804)	+= timer-sp.o
diff --git a/arch/arm/common/sharpsl_param.c b/arch/arm/common/sharpsl_param.c
deleted file mode 100644
index d56c932..0000000
--- a/arch/arm/common/sharpsl_param.c
+++ /dev/null
@@ -1,62 +0,0 @@
-/*
- * Hardware parameter area specific to Sharp SL series devices
- *
- * Copyright (c) 2005 Richard Purdie
- *
- * Based on Sharp's 2.4 kernel patches
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/string.h>
-#include <asm/mach/sharpsl_param.h>
-
-/*
- * Certain hardware parameters determined at the time of device manufacture,
- * typically including LCD parameters are loaded by the bootloader at the
- * address PARAM_BASE. As the kernel will overwrite them, we need to store
- * them early in the boot process, then pass them to the appropriate drivers.
- * Not all devices use all parameters but the format is common to all.
- */
-#ifdef CONFIG_ARCH_SA1100
-#define PARAM_BASE	0xe8ffc000
-#else
-#define PARAM_BASE	0xa0000a00
-#endif
-#define MAGIC_CHG(a,b,c,d) ( ( d << 24 ) | ( c << 16 )  | ( b << 8 ) | a )
-
-#define COMADJ_MAGIC	MAGIC_CHG('C','M','A','D')
-#define UUID_MAGIC	MAGIC_CHG('U','U','I','D')
-#define TOUCH_MAGIC	MAGIC_CHG('T','U','C','H')
-#define AD_MAGIC	MAGIC_CHG('B','V','A','D')
-#define PHAD_MAGIC	MAGIC_CHG('P','H','A','D')
-
-struct sharpsl_param_info sharpsl_param;
-EXPORT_SYMBOL(sharpsl_param);
-
-void sharpsl_save_param(void)
-{
-	memcpy(&sharpsl_param, (void *)PARAM_BASE, sizeof(struct sharpsl_param_info));
-
-	if (sharpsl_param.comadj_keyword != COMADJ_MAGIC)
-		sharpsl_param.comadj=-1;
-
-	if (sharpsl_param.phad_keyword != PHAD_MAGIC)
-		sharpsl_param.phadadj=-1;
-
-	if (sharpsl_param.uuid_keyword != UUID_MAGIC)
-		sharpsl_param.uuid[0]=-1;
-
-	if (sharpsl_param.touch_keyword != TOUCH_MAGIC)
-		sharpsl_param.touch_xp=-1;
-
-	if (sharpsl_param.adadj_keyword != AD_MAGIC)
-		sharpsl_param.adadj=-1;
-}
-
-
diff --git a/arch/arm/include/asm/mach/sharpsl_param.h b/arch/arm/include/asm/mach/sharpsl_param.h
deleted file mode 100644
index 7a24ecf..0000000
--- a/arch/arm/include/asm/mach/sharpsl_param.h
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * Hardware parameter area specific to Sharp SL series devices
- *
- * Copyright (c) 2005 Richard Purdie
- *
- * Based on Sharp's 2.4 kernel patches
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- */
-
-struct sharpsl_param_info {
-  unsigned int comadj_keyword;
-  unsigned int comadj;
-
-  unsigned int uuid_keyword;
-  unsigned char uuid[16];
-
-  unsigned int touch_keyword;
-  unsigned int touch_xp;
-  unsigned int touch_yp;
-  unsigned int touch_xd;
-  unsigned int touch_yd;
-
-  unsigned int adadj_keyword;
-  unsigned int adadj;
-
-  unsigned int phad_keyword;
-  unsigned int phadadj;
-} __attribute__((packed));
-
-
-extern struct sharpsl_param_info sharpsl_param;
-extern void sharpsl_save_param(void);
-
diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig
index fe2d1f8..71e9429 100644
--- a/arch/arm/mach-pxa/Kconfig
+++ b/arch/arm/mach-pxa/Kconfig
@@ -471,7 +471,6 @@ config MACH_RAUMFELD_SPEAKER
 config PXA_SHARPSL
 	bool "SHARP Zaurus SL-5600, SL-C7xx and SL-Cxx00 Models"
 	select SHARP_SCOOP
-	select SHARP_PARAM
 	help
 	  Say Y here if you intend to run this kernel on a
 	  Sharp Zaurus SL-5600 (Poodle), SL-C700 (Corgi),
diff --git a/arch/arm/mach-pxa/corgi.c b/arch/arm/mach-pxa/corgi.c
index c1fe32d..1ddf87f 100644
--- a/arch/arm/mach-pxa/corgi.c
+++ b/arch/arm/mach-pxa/corgi.c
@@ -52,7 +52,6 @@
 #include <mach/corgi.h>
 #include <mach/sharpsl_pm.h>
 
-#include <asm/mach/sharpsl_param.h>
 #include <asm/hardware/scoop.h>
 
 #include "generic.h"
@@ -716,7 +715,6 @@ static void __init corgi_init(void)
 static void __init fixup_corgi(struct tag *tags, char **cmdline,
 			       struct meminfo *mi)
 {
-	sharpsl_save_param();
 	mi->nr_banks=1;
 	mi->bank[0].start = 0xa0000000;
 	if (machine_is_corgi())
diff --git a/arch/arm/mach-pxa/poodle.c b/arch/arm/mach-pxa/poodle.c
index 89d98c8..0c4c083 100644
--- a/arch/arm/mach-pxa/poodle.c
+++ b/arch/arm/mach-pxa/poodle.c
@@ -48,7 +48,6 @@
 
 #include <asm/hardware/scoop.h>
 #include <asm/hardware/locomo.h>
-#include <asm/mach/sharpsl_param.h>
 
 #include "generic.h"
 #include "devices.h"
@@ -457,7 +456,6 @@ static void __init poodle_init(void)
 static void __init fixup_poodle(struct tag *tags, char **cmdline,
 				struct meminfo *mi)
 {
-	sharpsl_save_param();
 	mi->nr_banks=1;
 	mi->bank[0].start = 0xa0000000;
 	mi->bank[0].size = (32*1024*1024);
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 363d91b..9a0d583 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -35,7 +35,6 @@
 #include <asm/setup.h>
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
-#include <asm/mach/sharpsl_param.h>
 #include <asm/hardware/scoop.h>
 
 #include <mach/pxa27x.h>
@@ -971,7 +970,6 @@ static void __init spitz_init(void)
 static void __init spitz_fixup(struct tag *tags, char **cmdline,
 			       struct meminfo *mi)
 {
-	sharpsl_save_param();
 	mi->nr_banks = 1;
 	mi->bank[0].start = 0xa0000000;
 	mi->bank[0].size = (64*1024*1024);
diff --git a/arch/arm/mach-pxa/tosa.c b/arch/arm/mach-pxa/tosa.c
index 4d4eb60..3f8fe17 100644
--- a/arch/arm/mach-pxa/tosa.c
+++ b/arch/arm/mach-pxa/tosa.c
@@ -53,7 +53,6 @@
 #include <mach/tosa.h>
 
 #include <asm/hardware/scoop.h>
-#include <asm/mach/sharpsl_param.h>
 
 #include "generic.h"
 #include "clock.h"
@@ -968,7 +967,6 @@ static void __init tosa_init(void)
 static void __init fixup_tosa(struct tag *tags, char **cmdline,
 			      struct meminfo *mi)
 {
-	sharpsl_save_param();
 	mi->nr_banks=1;
 	mi->bank[0].start = 0xa0000000;
 	mi->bank[0].size = (64*1024*1024);
diff --git a/arch/arm/mach-sa1100/Kconfig b/arch/arm/mach-sa1100/Kconfig
index 42625e4..87b8604 100644
--- a/arch/arm/mach-sa1100/Kconfig
+++ b/arch/arm/mach-sa1100/Kconfig
@@ -50,7 +50,6 @@ config SA1100_COLLIE
 	# FIXME: select CPU_FREQ_SA11x0
 	select SHARP_LOCOMO
 	select SHARP_SCOOP
-	select SHARP_PARAM
 	help
 	  Say Y here to support the Sharp Zaurus SL5500 PDAs.
 
diff --git a/arch/arm/mach-sa1100/collie.c b/arch/arm/mach-sa1100/collie.c
index ea5cff3..98287a4 100644
--- a/arch/arm/mach-sa1100/collie.c
+++ b/arch/arm/mach-sa1100/collie.c
@@ -43,7 +43,6 @@
 #include <asm/mach/serial_sa1100.h>
 
 #include <asm/hardware/scoop.h>
-#include <asm/mach/sharpsl_param.h>
 #include <asm/hardware/locomo.h>
 #include <mach/mcp.h>
 #include <mach/irqs.h>
@@ -366,7 +365,6 @@ static void __init collie_init(void)
 			    ARRAY_SIZE(collie_flash_resources));
 	sa11x0_register_mcp(&collie_mcp_data);
 
-	sharpsl_save_param();
 }
 
 static struct map_desc collie_io_desc[] __initdata = {
diff --git a/drivers/power/collie_battery.c b/drivers/power/collie_battery.c
index 74c6b23..e83b963 100644
--- a/drivers/power/collie_battery.c
+++ b/drivers/power/collie_battery.c
@@ -19,7 +19,6 @@
 #include <linux/gpio.h>
 #include <linux/mfd/ucb1x00.h>
 
-#include <asm/mach/sharpsl_param.h>
 #include <asm/mach-types.h>
 #include <mach/collie.h>
 
diff --git a/drivers/video/backlight/corgi_lcd.c b/drivers/video/backlight/corgi_lcd.c
index c781768..c28838a 100644
--- a/drivers/video/backlight/corgi_lcd.c
+++ b/drivers/video/backlight/corgi_lcd.c
@@ -25,7 +25,6 @@
 #include <linux/spi/spi.h>
 #include <linux/spi/corgi_lcd.h>
 #include <linux/slab.h>
-#include <asm/mach/sharpsl_param.h>
 
 #define POWER_IS_ON(pwr)	((pwr) <= FB_BLANK_NORMAL)
 
@@ -195,9 +194,7 @@ static void lcdtg_set_phadadj(struct corgi_lcd *lcd, int mode)
 	switch(mode) {
 	case CORGI_LCD_MODE_VGA:
 		/* Setting for VGA */
-		adj = sharpsl_param.phadadj;
-		adj = (adj < 0) ? PHACTRL_PHASE_MANUAL :
-				  PHACTRL_PHASE_MANUAL | ((adj & 0xf) << 1);
+		adj = PHACTRL_PHASE_MANUAL;
 		break;
 	case CORGI_LCD_MODE_QVGA:
 	default:
@@ -242,9 +239,7 @@ static void corgi_lcd_power_on(struct corgi_lcd *lcd)
 			PICTRL_INIT_STATE | PICTRL_COM_SIGNAL_OFF);
 
 	/* Set Common Voltage */
-	comadj = sharpsl_param.comadj;
-	if (comadj < 0)
-		comadj = DEFAULT_COMADJ;
+	comadj = DEFAULT_COMADJ;
 
 	lcdtg_set_common_voltage(lcd, POWER0_DAC_ON | POWER0_COM_OFF |
 				 POWER0_VCC5_OFF, comadj);
diff --git a/drivers/video/backlight/locomolcd.c b/drivers/video/backlight/locomolcd.c
index 3a6d541..62c4cb9 100644
--- a/drivers/video/backlight/locomolcd.c
+++ b/drivers/video/backlight/locomolcd.c
@@ -21,7 +21,6 @@
 
 #include <asm/hardware/locomo.h>
 #include <asm/irq.h>
-#include <asm/mach/sharpsl_param.h>
 #include <asm/mach-types.h>
 
 #include "../../../arch/arm/mach-sa1100/generic.h"
@@ -82,7 +81,7 @@ static void locomolcd_off(int comadj)
 
 void locomolcd_power(int on)
 {
-	int comadj = sharpsl_param.comadj;
+	int comadj;
 	unsigned long flags;
 
 	local_irq_save(flags);
@@ -92,10 +91,9 @@ void locomolcd_power(int on)
 		return;
 	}
 
-	/* read comadj */
-	if (comadj == -1 && machine_is_collie())
+	if (machine_is_collie())
 		comadj = 128;
-	if (comadj == -1 && machine_is_poodle())
+	if (machine_is_poodle())
 		comadj = 118;
 
 	if (on)
diff --git a/drivers/video/backlight/tosa_bl.c b/drivers/video/backlight/tosa_bl.c
index 49342e1..2918075 100644
--- a/drivers/video/backlight/tosa_bl.c
+++ b/drivers/video/backlight/tosa_bl.c
@@ -20,8 +20,6 @@
 #include <linux/backlight.h>
 #include <linux/slab.h>
 
-#include <asm/mach/sharpsl_param.h>
-
 #include <mach/tosa.h>
 
 #define COMADJ_DEFAULT	97
@@ -90,7 +88,7 @@ static int __devinit tosa_bl_probe(struct i2c_client *client,
 	if (!data)
 		return -ENOMEM;
 
-	data->comadj = sharpsl_param.comadj == -1 ? COMADJ_DEFAULT : sharpsl_param.comadj;
+	data->comadj = COMADJ_DEFAULT;
 
 	ret = devm_gpio_request(&client->dev, TOSA_GPIO_BL_C20MA, "backlight");
 	if (ret) {
diff --git a/drivers/video/backlight/tosa_lcd.c b/drivers/video/backlight/tosa_lcd.c
index 33047a66..6fc575f 100644
--- a/drivers/video/backlight/tosa_lcd.c
+++ b/drivers/video/backlight/tosa_lcd.c
@@ -21,8 +21,6 @@
 #include <linux/lcd.h>
 #include <linux/fb.h>
 
-#include <asm/mach/sharpsl_param.h>
-
 #include <mach/tosa.h>
 
 #define POWER_IS_ON(pwr)	((pwr) <= FB_BLANK_NORMAL)
-- 
1.7.4.1

[RFC][PATCH] Disintegrate sharpsl_param.c

[Richard Purdie]

On Wed, 2012-08-22 at 19:27 +0200, dromede at gmail.com wrote:
> From: Marko Katic <dromede.gmail.com>
>
> I recently tried running 3.6-rc2 kernel on my Sharp Zaurus C-1000. It 
> hanged almost immediately after uncompressing the kernel. I tracked the
> problem down to a single line in arch/arm/common/sharpsl_param.c:
> 
> memcpy(&sharpsl_param, (void *)PARAM_BASE, sizeof(struct sharpsl_param_info));
> 
> Commenting out the line makes the kernel boot just fine. This left me
> wondering whether sharpsl_param.c is actually needed. Here's a comment
> from sharpsl_param.c that describes what sharpsl_param.c actually does:

Sharp went to the trouble of saving these values into the hardware and
passing them into the drivers. They're basically used by the LCD
initialisation code from what I remember. I don't remember what the
risks are of incorrect values.

>  So sharpsl_save_param() is supposed to fill the sharpsl_param_info struct
>  with various parameters or fill some of the struct fields with -1. 
>  I found only four drivers that use some of these
>  parameters (only two parameters are used by these drivers, comadj and phadadj). 
>  These drivers are:
>  
>  drivers/video/backlight/corgi_lcd.c
>  drivers/video/backlight/locomolcd.c
>  drivers/video/backlight/tosa_bl.c
>  drivers/video/backlight/tosa_lcd.c
>  
>  These drivers also have default values when comadj or phadadj == -1.
>  I checked what values are actually contained in this struct in
>  earlier kernels. 3.2.24 kernel is the latest one i know of where
>  sharpsl_save_param() works properly. And it also has these fields initialised
>  to -1.

What values was it loading from the bootloader? Or are you saying that
it wasn't finding the bootloader information and therefore just using -1
for everything?
 
>  So it seems to me that sharpsl_param.c is redundant as it currently only  
>  serves to assign -1 to a few variables. And drivers that use these variables
>  then handle -1 with assigning default values.

Its not redundant, it sounds like its just broken at some point, at
least on the platform you tested and nobody has noticed/cared. The
better solution would be to fix it. Obviously its much easier to delete
the code though.

Cheers,

Richard

[RFC][PATCH] Disintegrate sharpsl_param.c

[Marko Katić]

On Wed, Aug 22, 2012 at 9:19 PM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> On Wed, 2012-08-22 at 19:27 +0200, dromede at gmail.com wrote:
> > From: Marko Katic <dromede.gmail.com>
> >
> > I recently tried running 3.6-rc2 kernel on my Sharp Zaurus C-1000. It
> > hanged almost immediately after uncompressing the kernel. I tracked the
> > problem down to a single line in arch/arm/common/sharpsl_param.c:
> >
> > memcpy(&sharpsl_param, (void *)PARAM_BASE, sizeof(struct
> > sharpsl_param_info));
> >
> > Commenting out the line makes the kernel boot just fine. This left me
> > wondering whether sharpsl_param.c is actually needed. Here's a comment
> > from sharpsl_param.c that describes what sharpsl_param.c actually does:
>
> Sharp went to the trouble of saving these values into the hardware and
> passing them into the drivers. They're basically used by the LCD
> initialisation code from what I remember. I don't remember what the
> risks are of incorrect values.
>
> >  So sharpsl_save_param() is supposed to fill the sharpsl_param_info
> > struct
> >  with various parameters or fill some of the struct fields with -1.
> >  I found only four drivers that use some of these
> >  parameters (only two parameters are used by these drivers, comadj and
> > phadadj).
> >  These drivers are:
> >
> >  drivers/video/backlight/corgi_lcd.c
> >  drivers/video/backlight/locomolcd.c
> >  drivers/video/backlight/tosa_bl.c
> >  drivers/video/backlight/tosa_lcd.c
> >
> >  These drivers also have default values when comadj or phadadj == -1.
> >  I checked what values are actually contained in this struct in
> >  earlier kernels. 3.2.24 kernel is the latest one i know of where
> >  sharpsl_save_param() works properly. And it also has these fields
> > initialised
> >  to -1.
>
> What values was it loading from the bootloader? Or are you saying that
> it wasn't finding the bootloader information and therefore just using -1
> for everything?
>
> >  So it seems to me that sharpsl_param.c is redundant as it currently
> > only
> >  serves to assign -1 to a few variables. And drivers that use these
> > variables
> >  then handle -1 with assigning default values.
>
> Its not redundant, it sounds like its just broken at some point, at
> least on the platform you tested and nobody has noticed/cared. The
> better solution would be to fix it. Obviously its much easier to delete
> the code though.
>
> Cheers,
>
> Richard
>
>
>

Hmmm, i ran some tests again and it seems that my previous tests were
wrong. Sharpsl_param works properly in 3.2.24. I see proper values in
comadj and phadaj fields. Please disregard this patch, i should've
been more thorough with my tests. I'll try to bisect and find out why
it doesn't work in 3.6-rc2. Actually, it's broken since at least 3.4,
that should narrow my search down.

[RFC][PATCH] Disintegrate sharpsl_param.c

[Pavel Machek]

Hi!

> > From: Marko Katic <dromede.gmail.com>
> >
> > I recently tried running 3.6-rc2 kernel on my Sharp Zaurus C-1000. It 
> > hanged almost immediately after uncompressing the kernel. I tracked the
> > problem down to a single line in arch/arm/common/sharpsl_param.c:
> > 
> > memcpy(&sharpsl_param, (void *)PARAM_BASE, sizeof(struct sharpsl_param_info));
> > 
> > Commenting out the line makes the kernel boot just fine. This left me
> > wondering whether sharpsl_param.c is actually needed. Here's a comment
> > from sharpsl_param.c that describes what sharpsl_param.c actually does:
> 
> Sharp went to the trouble of saving these values into the hardware and
> passing them into the drivers. They're basically used by the LCD
> initialisation code from what I remember. I don't remember what the
> risks are of incorrect values.

We were seeing strange flicker on LCD on some machines...? Particulary
metan's machine with wifi on did that. OTOH it looked like some kind
of DMA issue.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[RFC][PATCH] Disintegrate sharpsl_param.c

[Cyril Hrubis]

Hi!
> > > I recently tried running 3.6-rc2 kernel on my Sharp Zaurus C-1000. It 
> > > hanged almost immediately after uncompressing the kernel. I tracked the
> > > problem down to a single line in arch/arm/common/sharpsl_param.c:
> > > 
> > > memcpy(&sharpsl_param, (void *)PARAM_BASE, sizeof(struct sharpsl_param_info));
> > > 
> > > Commenting out the line makes the kernel boot just fine. This left me
> > > wondering whether sharpsl_param.c is actually needed. Here's a comment
> > > from sharpsl_param.c that describes what sharpsl_param.c actually does:
> > 
> > Sharp went to the trouble of saving these values into the hardware and
> > passing them into the drivers. They're basically used by the LCD
> > initialisation code from what I remember. I don't remember what the
> > risks are of incorrect values.
> 
> We were seeing strange flicker on LCD on some machines...? Particulary
> metan's machine with wifi on did that. OTOH it looked like some kind
> of DMA issue.

Yes, the problems with my LCD are DMA related (too much DMA to wifi cf card
causes timing issues on display refresh). When there is no network activity
it's okay.

And as far as I remeber there are some magic values needed for the LCD in order
not to have distorted picture (blurred, etc.) There is a callibration program
in the service menu that allows you to modify the values stored in the flash.

I remeber Stanislav did some research on that (CCing).

-- 
metan

[RFC][PATCH] Disintegrate sharpsl_param.c

[Marko Katić]

On Fri, Aug 24, 2012 at 1:25 PM, Cyril Hrubis <metan@ucw.cz> wrote:
> Hi!
>> > > I recently tried running 3.6-rc2 kernel on my Sharp Zaurus C-1000. It
>> > > hanged almost immediately after uncompressing the kernel. I tracked the
>> > > problem down to a single line in arch/arm/common/sharpsl_param.c:
>> > >
>> > > memcpy(&sharpsl_param, (void *)PARAM_BASE, sizeof(struct sharpsl_param_info));
>> > >
>> > > Commenting out the line makes the kernel boot just fine. This left me
>> > > wondering whether sharpsl_param.c is actually needed. Here's a comment
>> > > from sharpsl_param.c that describes what sharpsl_param.c actually does:
>> >
>> > Sharp went to the trouble of saving these values into the hardware and
>> > passing them into the drivers. They're basically used by the LCD
>> > initialisation code from what I remember. I don't remember what the
>> > risks are of incorrect values.
>>
>> We were seeing strange flicker on LCD on some machines...? Particulary
>> metan's machine with wifi on did that. OTOH it looked like some kind
>> of DMA issue.
>
> Yes, the problems with my LCD are DMA related (too much DMA to wifi cf card
> causes timing issues on display refresh). When there is no network activity
> it's okay.
>
> And as far as I remeber there are some magic values needed for the LCD in order
> not to have distorted picture (blurred, etc.) There is a callibration program
> in the service menu that allows you to modify the values stored in the flash.
>
> I remeber Stanislav did some research on that (CCing).
>
> --
> metan

[RFC][PATCH] Disintegrate sharpsl_param.c

[Marko Katić]

On Fri, Aug 24, 2012 at 2:53 PM, Marko Kati? <dromede@gmail.com> wrote:
> On Fri, Aug 24, 2012 at 1:25 PM, Cyril Hrubis <metan@ucw.cz> wrote:
>> Hi!
>>> > > I recently tried running 3.6-rc2 kernel on my Sharp Zaurus C-1000.
>>> > > It
>>> > > hanged almost immediately after uncompressing the kernel. I tracked
>>> > > the
>>> > > problem down to a single line in arch/arm/common/sharpsl_param.c:
>>> > >
>>> > > memcpy(&sharpsl_param, (void *)PARAM_BASE, sizeof(struct
>>> > > sharpsl_param_info));
>>> > >
>>> > > Commenting out the line makes the kernel boot just fine. This left
>>> > > me
>>> > > wondering whether sharpsl_param.c is actually needed. Here's a
>>> > > comment
>>> > > from sharpsl_param.c that describes what sharpsl_param.c actually
>>> > > does:
>>> >
>>> > Sharp went to the trouble of saving these values into the hardware and
>>> > passing them into the drivers. They're basically used by the LCD
>>> > initialisation code from what I remember. I don't remember what the
>>> > risks are of incorrect values.
>>>
>>> We were seeing strange flicker on LCD on some machines...? Particulary
>>> metan's machine with wifi on did that. OTOH it looked like some kind
>>> of DMA issue.
>>
>> Yes, the problems with my LCD are DMA related (too much DMA to wifi cf
>> card
>> causes timing issues on display refresh). When there is no network
>> activity
>> it's okay.
>>
>> And as far as I remeber there are some magic values needed for the LCD in
>> order
>> not to have distorted picture (blurred, etc.) There is a callibration
>> program
>> in the service menu that allows you to modify the values stored in the
>> flash.
>>
>> I remeber Stanislav did some research on that (CCing).
>>
>> --
>> metan

As i said, this patch is not valid since i wrote it under false
assumptions. My tests were wrong, sharpsl_save_param() works as it
should in all kernels before 3.3-rc1.

3.3-rc1 breaks sharpsl_save_param() with this commit:

[72662e01088394577be4a3f14da94cf87bea2591] ARM: head.S: only include
__turn_mmu_on in the initial identity mapping

I already have a patch ready for 3.6-rc3 that fixes this (confirmed
working on my akita), i just need to test it on a SA1100 machine
before commiting.

As for your lcd/dma problems, the arbiter control register might help.
There's a paragraph in the pxa270 dev manual that says that the lcd
controller should have top priority when large lcd displays are used.
Try setting ARB_CNTRL in your machine init code so the lcd controller
has top priority or lower the dma controller priority.

Actually i've written a nice sysfs interface for the arbiter bus
register a while ago and i can't find the patch...

[RFC][PATCH] Disintegrate sharpsl_param.c

[Stanislav Brabec]

metan wrote:
>And as far as I remeber there are some magic values needed for the >LCD in order not to have distorted picture (blurred, etc.) There is a >callibration program in the service menu that allows you to modify the >values stored in the flash.
>
>I remeber Stanislav did some research on that (CCing).

As far as I remember, there were two issues causing LCD problems:

- LCD phase setting from service menu. Incorrect value causes vertical blur or ghosts. Some operations (NAND cleanup) may reset these values. Fixing it in the service menu fixes the problem. I think that Linux does not touch these registers at all and even does not know how to handle LCD phase.

- Not touching brightness registers. It seems that the default (maximal value) of brightness causes a bit unstable brightness and flickering. To fix that, simply write lower value to brightness registers. (I think that kernel does not touch these registers during boot, just initialises the backlight control driver.)