Re: [PATCH] Staging: winbond: wb35reg: fixed some line over 80 characters
On Fri 2013-08-02 13:37:56, Iker Pedrosa wrote: > Fixed some coding style issues > > Signed-off-by: Iker Pedrosa Reviewed-by: Pavel Machek -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: olpc_dcon: replace some magic numbers
This patch replace some magic numbers. I believe it makes the driver more readable. The magic number 0x26 is the XO system embedded controller (EC) command 'DCON power enable/disable'. Number 0x41, and 0x42 are special memory controller settings register. The 0x41 initialize bit sequence 0x101 means: enable memory power down function and special SDRAM clock delay for synchronize SDRAM output and clock signal. The 0x42 initialize squence 0x101 is wrong. According to the specification Bit 8 is reserved, thus not in use. I removed it. Signed-off-by: Jens Frederich diff --git a/drivers/staging/olpc_dcon/olpc_dcon.c b/drivers/staging/olpc_dcon/olpc_dcon.c index 7c460f2..5ca4fa4 100644 --- a/drivers/staging/olpc_dcon/olpc_dcon.c +++ b/drivers/staging/olpc_dcon/olpc_dcon.c @@ -90,9 +90,10 @@ static int dcon_hw_init(struct dcon_priv *dcon, int is_init) /* SDRAM setup/hold time */ dcon_write(dcon, 0x3a, 0xc040); - dcon_write(dcon, 0x41, 0x); - dcon_write(dcon, 0x41, 0x0101); - dcon_write(dcon, 0x42, 0x0101); + dcon_write(dcon, DCON_REG_MEM_OPT_A, 0x); /* clear option bits */ + dcon_write(dcon, DCON_REG_MEM_OPT_A, + MEM_DLL_CLOCK_DELAY | MEM_POWER_DOWN); + dcon_write(dcon, DCON_REG_MEM_OPT_B, MEM_SOFT_RESET); /* Colour swizzle, AA, no passthrough, backlight */ if (is_init) { @@ -126,7 +127,7 @@ static int dcon_bus_stabilize(struct dcon_priv *dcon, int is_powered_down) power_up: if (is_powered_down) { x = 1; - x = olpc_ec_cmd(0x26, (unsigned char *)&x, 1, NULL, 0); + x = olpc_ec_cmd(EC_DCON_POWER_MODE, (u8 *)&x, 1, NULL, 0); if (x) { pr_warn("unable to force dcon to power up: %d!\n", x); return x; @@ -144,7 +145,7 @@ power_up: pr_err("unable to stabilize dcon's smbus, reasserting power and praying.\n"); BUG_ON(olpc_board_at_least(olpc_board(0xc2))); x = 0; - olpc_ec_cmd(0x26, (unsigned char *)&x, 1, NULL, 0); + olpc_ec_cmd(EC_DCON_POWER_MODE, (u8 *)&x, 1, NULL, 0); msleep(100); is_powered_down = 1; goto power_up; /* argh, stupid hardware.. */ @@ -208,7 +209,7 @@ static void dcon_sleep(struct dcon_priv *dcon, bool sleep) if (sleep) { x = 0; - x = olpc_ec_cmd(0x26, (unsigned char *)&x, 1, NULL, 0); + x = olpc_ec_cmd(EC_DCON_POWER_MODE, (u8 *)&x, 1, NULL, 0); if (x) pr_warn("unable to force dcon to power down: %d!\n", x); else diff --git a/drivers/staging/olpc_dcon/olpc_dcon.h b/drivers/staging/olpc_dcon/olpc_dcon.h index 997bded..524ee49 100644 --- a/drivers/staging/olpc_dcon/olpc_dcon.h +++ b/drivers/staging/olpc_dcon/olpc_dcon.h @@ -22,15 +22,24 @@ #define MODE_DEBUG (1<<14) #define MODE_SELFTEST (1<<15) -#define DCON_REG_HRES 2 -#define DCON_REG_HTOTAL3 -#define DCON_REG_HSYNC_WIDTH 4 -#define DCON_REG_VRES 5 -#define DCON_REG_VTOTAL6 -#define DCON_REG_VSYNC_WIDTH 7 -#define DCON_REG_TIMEOUT 8 -#define DCON_REG_SCAN_INT 9 -#define DCON_REG_BRIGHT10 +#define DCON_REG_HRES 0x2 +#define DCON_REG_HTOTAL0x3 +#define DCON_REG_HSYNC_WIDTH 0x4 +#define DCON_REG_VRES 0x5 +#define DCON_REG_VTOTAL0x6 +#define DCON_REG_VSYNC_WIDTH 0x7 +#define DCON_REG_TIMEOUT 0x8 +#define DCON_REG_SCAN_INT 0x9 +#define DCON_REG_BRIGHT0x10 +#define DCON_REG_MEM_OPT_A 0x41 +#define DCON_REG_MEM_OPT_B 0x42 + +/* Load Delay Locked Loop (DLL) settings for clock delay */ +#define MEM_DLL_CLOCK_DELAY(1<<0) +/* Memory controller power down function */ +#define MEM_POWER_DOWN (1<<8) +/* Memory controller software reset */ +#define MEM_SOFT_RESET (1<<0) /* Status values */ diff --git a/include/linux/olpc-ec.h b/include/linux/olpc-ec.h index 5bb6e76..2925df3 100644 --- a/include/linux/olpc-ec.h +++ b/include/linux/olpc-ec.h @@ -6,6 +6,7 @@ #define EC_WRITE_SCI_MASK 0x1b #define EC_WAKE_UP_WLAN0x24 #define EC_WLAN_LEAVE_RESET0x25 +#define EC_DCON_POWER_MODE 0x26 #define EC_READ_EB_MODE0x2a #define EC_SET_SCI_INHIBIT 0x32 #define EC_SET_SCI_INHIBIT_RELEASE 0x34 -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: olpc_dcon: Already completed TODO entry removed
The TODO entry - drop global variables, use a proper olpc_dcon_priv struct - is already finished. The driver has no global variables. It uses the private structure 'dcon_priv'. Signed-off-by: Jens Frederich diff --git a/drivers/staging/olpc_dcon/TODO b/drivers/staging/olpc_dcon/TODO index f378e84..886e46e 100644 --- a/drivers/staging/olpc_dcon/TODO +++ b/drivers/staging/olpc_dcon/TODO @@ -3,7 +3,6 @@ TODO: share more code - allow simultaneous XO-1 and XO-1.5 support - console event notifier support - - drop global variables, use a proper olpc_dcon_priv struct - audit code for unnecessary code; old unsupported prototype workarounds, ancient variables (noaa?), etc - verify sane i2c API usage, update to new stuff if necessary -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: olpc_dcon: replace some magic numbers
Please Cc Daniel on these. Cjb and myself are no longer at olpc. On Sat, 3 Aug 2013 22:44:35 +0200 Jens Frederich wrote: > This patch replace some magic numbers. I believe it makes > the driver more readable. > > The magic number 0x26 is the XO system embedded controller > (EC) command 'DCON power enable/disable'. > > Number 0x41, and 0x42 are special memory controller settings > register. The 0x41 initialize bit sequence 0x101 means: > enable memory power down function and special SDRAM clock > delay for synchronize SDRAM output and clock signal. > > The 0x42 initialize squence 0x101 is wrong. According to > the specification Bit 8 is reserved, thus not in use. > I removed it. > > Signed-off-by: Jens Frederich > > diff --git a/drivers/staging/olpc_dcon/olpc_dcon.c > b/drivers/staging/olpc_dcon/olpc_dcon.c index 7c460f2..5ca4fa4 100644 > --- a/drivers/staging/olpc_dcon/olpc_dcon.c > +++ b/drivers/staging/olpc_dcon/olpc_dcon.c > @@ -90,9 +90,10 @@ static int dcon_hw_init(struct dcon_priv *dcon, > int is_init) > /* SDRAM setup/hold time */ > dcon_write(dcon, 0x3a, 0xc040); > - dcon_write(dcon, 0x41, 0x); > - dcon_write(dcon, 0x41, 0x0101); > - dcon_write(dcon, 0x42, 0x0101); > + dcon_write(dcon, DCON_REG_MEM_OPT_A, 0x); /* clear > option bits */ > + dcon_write(dcon, DCON_REG_MEM_OPT_A, > + MEM_DLL_CLOCK_DELAY | > MEM_POWER_DOWN); > + dcon_write(dcon, DCON_REG_MEM_OPT_B, MEM_SOFT_RESET); > > /* Colour swizzle, AA, no passthrough, backlight */ > if (is_init) { > @@ -126,7 +127,7 @@ static int dcon_bus_stabilize(struct dcon_priv > *dcon, int is_powered_down) power_up: > if (is_powered_down) { > x = 1; > - x = olpc_ec_cmd(0x26, (unsigned char *)&x, 1, NULL, > 0); > + x = olpc_ec_cmd(EC_DCON_POWER_MODE, (u8 *)&x, 1, > NULL, 0); if (x) { > pr_warn("unable to force dcon to power up: > %d!\n", x); return x; > @@ -144,7 +145,7 @@ power_up: > pr_err("unable to stabilize dcon's smbus, > reasserting power and praying.\n"); > BUG_ON(olpc_board_at_least(olpc_board(0xc2))); x = 0; > - olpc_ec_cmd(0x26, (unsigned char *)&x, 1, NULL, 0); > + olpc_ec_cmd(EC_DCON_POWER_MODE, (u8 *)&x, 1, NULL, > 0); msleep(100); > is_powered_down = 1; > goto power_up; /* argh, stupid hardware.. */ > @@ -208,7 +209,7 @@ static void dcon_sleep(struct dcon_priv *dcon, > bool sleep) > if (sleep) { > x = 0; > - x = olpc_ec_cmd(0x26, (unsigned char *)&x, 1, NULL, > 0); > + x = olpc_ec_cmd(EC_DCON_POWER_MODE, (u8 *)&x, 1, > NULL, 0); if (x) > pr_warn("unable to force dcon to power down: > %d!\n", x); else > diff --git a/drivers/staging/olpc_dcon/olpc_dcon.h > b/drivers/staging/olpc_dcon/olpc_dcon.h index 997bded..524ee49 100644 > --- a/drivers/staging/olpc_dcon/olpc_dcon.h > +++ b/drivers/staging/olpc_dcon/olpc_dcon.h > @@ -22,15 +22,24 @@ > #define MODE_DEBUG (1<<14) > #define MODE_SELFTEST(1<<15) > > -#define DCON_REG_HRES2 > -#define DCON_REG_HTOTAL 3 > -#define DCON_REG_HSYNC_WIDTH 4 > -#define DCON_REG_VRES5 > -#define DCON_REG_VTOTAL 6 > -#define DCON_REG_VSYNC_WIDTH 7 > -#define DCON_REG_TIMEOUT 8 > -#define DCON_REG_SCAN_INT9 > -#define DCON_REG_BRIGHT 10 > +#define DCON_REG_HRES0x2 > +#define DCON_REG_HTOTAL 0x3 > +#define DCON_REG_HSYNC_WIDTH 0x4 > +#define DCON_REG_VRES0x5 > +#define DCON_REG_VTOTAL 0x6 > +#define DCON_REG_VSYNC_WIDTH 0x7 > +#define DCON_REG_TIMEOUT 0x8 > +#define DCON_REG_SCAN_INT0x9 > +#define DCON_REG_BRIGHT 0x10 > +#define DCON_REG_MEM_OPT_A 0x41 > +#define DCON_REG_MEM_OPT_B 0x42 > + > +/* Load Delay Locked Loop (DLL) settings for clock delay */ > +#define MEM_DLL_CLOCK_DELAY (1<<0) > +/* Memory controller power down function */ > +#define MEM_POWER_DOWN (1<<8) > +/* Memory controller software reset */ > +#define MEM_SOFT_RESET (1<<0) > > /* Status values */ > > diff --git a/include/linux/olpc-ec.h b/include/linux/olpc-ec.h > index 5bb6e76..2925df3 100644 > --- a/include/linux/olpc-ec.h > +++ b/include/linux/olpc-ec.h > @@ -6,6 +6,7 @@ > #define EC_WRITE_SCI_MASK0x1b > #define EC_WAKE_UP_WLAN 0x24 > #define EC_WLAN_LEAVE_RESET 0x25 > +#define EC_DCON_POWER_MODE 0x26 > #define EC_READ_EB_MODE 0x2a > #define EC_SET_SCI_INHIBIT 0x32 > #define EC_SET_SCI_INHIBIT_RELEASE 0x34 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: olpc_dcon: replace some magic numbers
On Sat, Aug 3, 2013 at 11:16 PM, Andres Salomon wrote: > Please Cc Daniel on these. Cjb and myself are no longer at olpc. > Sorry, I've forgotten it. I will update the the TODO list. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: olpc_dcon: replace some magic numbers
On Sat, Aug 3, 2013 at 11:16 PM, Andres Salomon wrote: > Please Cc Daniel on these. Cjb and myself are no longer at olpc. > Do you know what's with Jon Nettleton? He is also on the TODO list? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: olpc_dcon: replace some magic numbers
On Sat, 3 Aug 2013 23:36:15 +0200 Jens Frederich wrote: > On Sat, Aug 3, 2013 at 11:16 PM, Andres Salomon > wrote: > > Please Cc Daniel on these. Cjb and myself are no longer at olpc. > > > > Do you know what's with Jon Nettleton? He is also on the TODO list? Let's ask. Jon, do you still want to be Cc'd on DCON and other OLPC patches? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: olpc_dcon: replace some magic numbers
On Sat, Aug 03, 2013 at 10:44:35PM +0200, Jens Frederich wrote: > @@ -126,7 +127,7 @@ static int dcon_bus_stabilize(struct dcon_priv *dcon, int > is_powered_down) > power_up: > if (is_powered_down) { > x = 1; > - x = olpc_ec_cmd(0x26, (unsigned char *)&x, 1, NULL, 0); > + x = olpc_ec_cmd(EC_DCON_POWER_MODE, (u8 *)&x, 1, NULL, 0); You didn't introduce this but using "x" as the inbuf here messy. It should be char instead of an int. The code won't work on big endian systems. I know this hardware is only available on little endian systems and that's why it's not a bug. It's just an ugly thing to do. (Since you didn't introduce this, it means your patch is fine and you can ignore this email. I am just commenting in case anyone wants to fix clean it up). > if (x) { > pr_warn("unable to force dcon to power up: %d!\n", x); > return x; regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: olpc_dcon: replace some magic numbers
On Sat, Aug 03, 2013 at 02:38:45PM -0700, Andres Salomon wrote: > On Sat, 3 Aug 2013 23:36:15 +0200 > Jens Frederich wrote: > > > On Sat, Aug 3, 2013 at 11:16 PM, Andres Salomon > > wrote: > > > Please Cc Daniel on these. Cjb and myself are no longer at olpc. > > > > > > > Do you know what's with Jon Nettleton? He is also on the TODO list? > > Let's ask. Jon, do you still want to be Cc'd on DCON and other OLPC > patches? No one reads the TODO files... Just update MAINTAINERS so that get_maintainer.pl CC's the right people automatically. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel