Disintegrate sharpsl_param.c
Subsystems (5)
RFC PATCH 1/1 Disintegrate sharpsl_param.c
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
arch/arm/common/Kconfig +0 -3
@@ -35,8 +35,5 @@ config DMABOUNCE config SHARP_LOCOMO bool -config SHARP_PARAM - bool - config SHARP_SCOOP bool
arch/arm/common/Makefile +0 -1
@@ -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
arch/arm/common/sharpsl_param.c deleted +0 -62
62 lines not on this page · raw message
arch/arm/include/asm/mach/sharpsl_param.h deleted +0 -37
@@ -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 ); -
arch/arm/mach-pxa/Kconfig +0 -1
@@ -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),
arch/arm/mach-pxa/corgi.c +0 -2
@@ -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 ())
arch/arm/mach-pxa/poodle.c +0 -2
@@ -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 );
arch/arm/mach-pxa/spitz.c +0 -2
@@ -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 );
arch/arm/mach-pxa/tosa.c +0 -2
@@ -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 );
arch/arm/mach-sa1100/Kconfig +0 -1
@@ -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.
arch/arm/mach-sa1100/collie.c +0 -2
@@ -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 = {
drivers/power/collie_battery.c +0 -1
@@ -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>
drivers/video/backlight/corgi_lcd.c +2 -7
@@ -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);
drivers/video/backlight/locomolcd.c +3 -5
@@ -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)
drivers/video/backlight/tosa_bl.c +1 -3
@@ -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) {
drivers/video/backlight/tosa_lcd.c +0 -2
@@ -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)
Signature · 1 line
1.7.4.1
RFC PATCH 1/1 Disintegrate sharpsl_param.c
Quoted 11 lines · 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.cis actually needed. Here's a comment fromsharpsl_param.cthat describes whatsharpsl_param.cactually 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.
Quoted 11 lines
So
sharpsl_save_param()is supposed to fill thesharpsl_param_infostruct 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
Quoted 5 lines
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?
Quoted 3 lines
So it seems to me that
sharpsl_param.cis 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 1/1 Disintegrate sharpsl_param.c
Quoted 50 lines · Richard Purdie
Quoted 6 lines · 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:Quoted 6 lines
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.cis actually needed. Here's a comment fromsharpsl_param.cthat describes whatsharpsl_param.cactually 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.
Quoted 13 lines
So
sharpsl_save_param()is supposed to fill thesharpsl_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.cQuoted 6 lines
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?
Quoted 5 lines
So it seems to me that
sharpsl_param.cis 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.
Quoted 11 lines
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 1/1 Disintegrate sharpsl_param.c
In reply to Richard PurdieHi!
Quoted 8 lines
Quoted 8 lines
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));
Quoted 8 lines
Quoted 3 lines
Commenting out the line makes the kernel boot just fine. This left me wondering whether
sharpsl_param.cis actually needed. Here's a comment fromsharpsl_param.cthat describes whatsharpsl_param.cactually 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.
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
RFC PATCH 1/1 Disintegrate sharpsl_param.c
Hi!
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
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.cis actually needed. Here's a comment fromsharpsl_param.cthat describes whatsharpsl_param.cactually does:
Quoted 8 lines
Quoted 4 lines
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).
Signature · 1 line
metan
RFC PATCH 1/1 Disintegrate sharpsl_param.c
Quoted 32 lines · Cyril Hrubis
Hi!
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
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.cis actually needed. Here's a comment fromsharpsl_param.cthat describes whatsharpsl_param.cactually does:Quoted 8 lines
Quoted 4 lines
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 1/1 Disintegrate sharpsl_param.c
Quoted 40 lines · Marko Kati?
Quoted 39 lines · Cyril Hrubis
Hi!
Quoted 16 lines
Quoted 16 lines
Quoted 15 lines
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 inarch/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 whethersharpsl_param.cis actually needed. Here's a comment
fromsharpsl_param.cthat describes whatsharpsl_param.cactually does:Quoted 8 lines
Quoted 4 lines
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.
Quoted 4 lines
Quoted 4 lines
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...