drivers: add lcd display support
Subsystems (5)
PATCH v6 1/1 drivers: add lcd display support
miguelojeda-2.6.18-add-lcd-display-support.patch
Changelog (newer first)
-----------------------
V6 - Fixed mistakes found by Andrew Morton
- added locking
- removed "add const to class_create" part, as it is already accepted in 2.6.18-git10
- removed PRINT_PREFIX macro, use KERN_... instead
- reverted bitops to standard |= and &=
- added const to fop's static struct
- added error code checking for class_device_create()
- removed "if(foo > maxfoo) foo=maxfoo;", use min(foo,maxfoo) instead
- coding style:
- used #defined constants directly, not static constants = DEFINED_CONSTANT
- removed blank lines
- changed pointers declarations to "T *name"
- changed for's coding style
- added single spaces around arithmetic operators
- changed counter's nomencalture to i,j,k
- shorter function definitions
- removed newlines at some function calls
- udelay(2)
V5 - Fixed mistakes found by Joe Perches
- coding style
- const, spelling mistake
V4 - Fixed mistakes found by Pavel Machek
- no zero initialization
- GPL version
- use bitop generic functions
V3 - Apply for 2.6.18
- added const to class_create (2.6.18 hasn't add-const-to-class_create.patch)
V2 - Fixed mistakes found by Greg KH
- coding style
V1 - Fixed mistakes found by Alexey Dobriyan
- coding style
- init() and exit() error management
V0 - Original patch
Brief
-----
- Adds const to class_create (2.6.18 hasn't add-const-to-class_create.patch)
- Adds a "lcddisplay" class for registering LCD devices.
- Adds support for the ks0108 LCD Controller as a device driver.
(uses lcddisplay class and parport interface)
- Adds support for the cfag12864b LCD Display as a device driver.
(uses ks0108 device driver)
- Adds a new ioctl() magic number/range (0xFF) in the list for the cfag12864b device.
- Adds the usual Documentation, includes, Makefiles, Kconfigs, MAINTAINERS, CREDITS...
Patched files Index
-------------------
patching file drivers/Kconfig
patching file drivers/lcddisplay/cfag12864b.c patching file drivers/lcddisplay/cfag12864b_image.h patching file drivers/lcddisplay/Kconfig patching file drivers/lcddisplay/ks0108.c patching file drivers/lcddisplay/lcddisplay.c patching file drivers/lcddisplay/Makefile patching file drivers/Makefile
patching file include/linux/cfag12864b.h patching file include/linux/ks0108.h patching file include/linux/lcddisplay.h patching file Documentation/lcddisplay/cfag12864b patching file Documentation/lcddisplay/lcddisplay patching file Documentation/ioctl-number.txt patching file CREDITS
patching file MAINTAINERS
Please review.
miguelojeda-2.6.18-add-lcd-display-support.patch
---
drivers/Kconfig +2 -0
@@ -74,4 +74,6 @@ source "drivers/rtc/Kconfig" source "drivers/dma/Kconfig" +source "drivers/lcddisplay/Kconfig" + endmenu
drivers/lcddisplay/cfag12864b.c +592 -0
592 lines not on this page · raw message
drivers/lcddisplay/cfag12864b_image.h +95 -0
95 lines not on this page · raw message
drivers/lcddisplay/Kconfig +110 -0
110 lines not on this page · raw message
drivers/lcddisplay/ks0108.c +160 -0
160 lines not on this page · raw message
drivers/lcddisplay/lcddisplay.c +79 -0
79 lines not on this page · raw message
drivers/lcddisplay/Makefile +7 -0
@@ -0,0 +1,7 @@ +# +# Makefile for the kernel LCD Display device drivers. +# + +obj-$(CONFIG_LCDDISPLAY) += lcddisplay.o +obj-$(CONFIG_KS0108) += ks0108.o +obj-$(CONFIG_CFAG12864B) += cfag12864b.o
drivers/Makefile +1 -0
@@ -76,3 +76,4 @@ obj-$(CONFIG_CRYPTO) += crypto/ obj-$(CONFIG_SUPERH) += sh/ obj-$(CONFIG_GENERIC_TIME) += clocksource/ obj-$(CONFIG_DMA_ENGINE) += dma/ +obj-$(CONFIG_LCDDISPLAY) += lcddisplay/
include/linux/cfag12864b.h +58 -0
58 lines not on this page · raw message
include/linux/ks0108.h +36 -0
@@ -0,0 +1,36 @@ +/* +* Filename: ks0108.h +* Version: 0.1.0 +* Description: ks0108 LCD Display Controller Driver Header +* License: GPLv2 or later +* +* Author: Copyright (C) Miguel Ojeda Sandonis <[email protected]> +* Date: 2006-09-28 +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +* +*/ + +#ifndef _KS0108_H_ +#define _KS0108_H_ + +extern void ks0108_writedata (unsigned char byte); +extern void ks0108_writecontrol (unsigned char byte); +extern void ks0108_displaystate (unsigned char state); +extern void ks0108_startline (unsigned char startline); +extern void ks0108_address (unsigned char address); +extern void ks0108_page (unsigned char page); + +#endif /* _KS0108_H_ */
include/linux/lcddisplay.h +34 -0
@@ -0,0 +1,34 @@ +/* +* Filename: lcddisplay.h +* Version: 0.1.0 +* Description: Display Class Header +* License: GPLv2 or later +* +* Author: Copyright (C) Miguel Ojeda Sandonis <[email protected]> +* Date: 2006-09-28 +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +* +*/ + +#ifndef _LCDDISPLAY_H_ +#define _LCDDISPLAY_H_ + +#include <linux/device.h> + +extern struct class *lcddisplay_class; + +#endif /* _LCDDISPLAY_H_ */ +
Documentation/lcddisplay/cfag12864b +371 -0
371 lines not on this page · raw message
Documentation/lcddisplay/lcddisplay +37 -0
@@ -0,0 +1,37 @@ + ================================= + LCD Display Drivers Documentation + ================================= + +License: GPL +Author & Maintainer: Miguel Ojeda Sandonis <[email protected]> +Date: 2006-09-30 + +-------- +0. INDEX +-------- + + 1. NEW DISPLAY DRIVERS + 2. GENERAL TIPS + + + +---------------------- +1. NEW DISPLAY DRIVERS +---------------------- + +Feel free to send me new display drivers. I will try to do my best. + +If you don't get any answer, send your patch directly to the linux-kernel ml. + + + +--------------- +2. GENERAL TIPS +--------------- + +- Divide your driver into the controller driver, like ks0108, + and the specific LCD display series driver, like cfag12864b. + +- Claim for your IO ports in the controller driver. + +EOF
Documentation/ioctl-number.txt +2 -0
@@ -191,3 +191,5 @@ Code Seq# Include File Comments <mailto:[email protected]> 0xF3 00-3F video/sisfb.h sisfb (in development) <mailto:[email protected]> +0xFF 00-1F linux/cfag12864b.h cfag12864b LCD Display Driver + <mailto:[email protected]>
CREDITS +8 -0
@@ -2535,6 +2535,14 @@ S: Subiaco, 6008 S: Perth, Western Australia S: Australia +N: Miguel Ojeda Sandonis +E: [email protected] +D: Author: LCD Display Drivers (ks0108, cfag12864b) +D: Maintainer: LCD Display Drivers Tree (drivers/lcddisplay/*) +S: C/ Mieses 20, 9-B +S: Valladolid 47009 +S: Spain + N: Greg Page E: [email protected] D: IPX development and support
MAINTAINERS +5 -0
@@ -1707,6 +1707,11 @@ M: [email protected] L: [email protected] S: Maintained +LCD DISPLAY DRIVERS +P: Miguel Ojeda Sandonis +M: [email protected] +S: Maintained + LED SUBSYSTEM P: Richard Purdie M: [email protected]
Quoted 5 lines · Miguel Ojeda Sandonis
Patched files Index
-------------------
patching filedrivers/Kconfig
patching filedrivers/lcddisplay/cfag12864b.cpatching filedrivers/lcddisplay/cfag12864b_image.h
Quoted 8 lines
patching file
drivers/lcddisplay/Kconfig
patching filedrivers/lcddisplay/ks0108.c
patching filedrivers/lcddisplay/lcddisplay.c
patching filedrivers/lcddisplay/Makefile
patching fileinclude/linux/lcddisplay.h
patching fileDocumentation/lcddisplay/cfag12864b
patching fileDocumentation/lcddisplay/lcddisplay
patching fileDocumentation/ioctl-number.txt
AFAIK, the 'D' in LCD is already an abbreviation for "display", so could we name it lcdisplay or something like that?
MfG, JBG
Signature · 3 lines
Jan-Benedict Glaw [email protected] +49-172-7608481 Signature of: The real problem with C++ for kernel modules is: the language just sucks. the second : -- Linus Torvalds
Quoted 4 lines · Jan-Benedict Glaw
AFAIK, the 'D' in LCD is already an abbreviation for "display", so could we name it lcdisplay or something like that?
My first guess was "lcd" ("d" as display), but there were already devices with such name. So I chose "display", as they are additional small displays (LCD or not). However, people thought it was a very common name. Then I chose "LCD Display". Such name is more expressive (it is "a Display, LCD type") and doesn't makes trouble with new lcd screens, or the "lcd" drivers, or...
I don't care really about the name with or without "double d". Can someone else give his/her opinion about this?
Thank you,
Miguel Ojeda
Quoted 18 lines
MfG, JBG
--
Jan-Benedict Glaw [email protected] +49-172-7608481 Signature of: The real problem with C++ for kernel modules is: the language just sucks. the second : -- Linus Torvalds-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)iD8DBQFFHmWAHb1edYOZ4bsRAkdpAJ4g9+GPFYU4Y5W7JdGZXFDCmBDIggCfe6dk 3FbG+mcZQ+10MNkq5wC9J7w=
=KzkJ
-----END PGP SIGNATURE-----
Quoted 2 lines · On Sat, 30 Sep 2006 13:22:53 +0000 Miguel Ojeda Sandonis wrote:
Patched files Index
-------------------
This list should be done with 'diffstat -p1 -w70 patch_file_name' so that we can see the files and the patch sizes.
Quoted 22 lines
patching file
drivers/Kconfig
patching filedrivers/lcddisplay/cfag12864b.cpatching filedrivers/lcddisplay/cfag12864b_image.hpatching filedrivers/lcddisplay/Kconfigpatching filedrivers/lcddisplay/ks0108.cpatching filedrivers/lcddisplay/lcddisplay.cpatching filedrivers/lcddisplay/Makefilepatching filedrivers/Makefile
patching fileinclude/linux/cfag12864b.hpatching fileinclude/linux/ks0108.hpatching fileinclude/linux/lcddisplay.hpatching fileDocumentation/lcddisplay/cfag12864bpatching fileDocumentation/lcddisplay/lcddisplaypatching fileDocumentation/ioctl-number.txtpatching file CREDITS
patching fileMAINTAINERS---
diff -uprN -X dontdifflinux-2.6.18-vanilla/drivers/lcddisplay/cfag12864b.clinux-2.6.18/drivers/lcddisplay/cfag12864b.c
drivers/lcddisplay/cfag12864b.c line 9
Quoted 9 lines
drivers/lcddisplay/cfag12864b.c +9
-0@@ -0,0 +1,592 @@ + +#define CFAG12864B_NAME "cfag12864b" + +/* +* Device +*/ + +static const unsigned int cfag12864b_firstminor; +static const unsigned int cfag12864b_ndevices =1 ;
This driver only supports one device, right? Is that documented somewhere? I probably missed it.
Quoted 21 lines
drivers/lcddisplay/cfag12864b.c +21
-0+static int cfag12864b_major; +static int cfag12864b_minor; +static dev_t cfag12864b_device; +struct cdev cfag12864b_chardevice; +DECLARE_MUTEX (cfag12864b_mutex); + +/* +* cfag12864b Commands +*/ + +#define bit (n) (((unsigned char)1)<<(n)) + +static unsigned char cfag12864b_state; + +static void cfag12864b_set (void ) +{ +ks0108_writecontrol (cfag12864b_state); +} + +static void cfag12864b_setbit (unsigned char state,unsigned char n) +{
drivers/lcddisplay/cfag12864b.c line 76
Quoted 10 lines
drivers/lcddisplay/cfag12864b.c +10
-0+if (state) + cfag12864b_state |=bit (n); +else + cfag12864b_state &= ~bit (n); +cfag12864b_set (); +} + +static void cfag12864b_e (unsigned char state) +{ +cfag12864b_setbit (state,0 );
bit defintions? (here and below)
Quoted 33 lines
drivers/lcddisplay/cfag12864b.c +33
-0+} + +static void cfag12864b_cs1 (unsigned char state) +{ +cfag12864b_setbit (state,2 ); +} + +static void cfag12864b_cs2 (unsigned char state) +{ +cfag12864b_setbit (state,1 ); +} + +static void cfag12864b_di (unsigned char state) +{ +cfag12864b_setbit (state,3 ); +} + +static void cfag12864b_setcontrollers (unsigned char first,unsigned char second) +{ +if (first) +cfag12864b_cs1 (0 ); +else +cfag12864b_cs1 (1 ); + +if (second) +cfag12864b_cs2 (0 ); +else +cfag12864b_cs2 (1 ); +} + +static void cfag12864b_controller (unsigned char which) +{ +if (which ==0 )
drivers/lcddisplay/cfag12864b.c line 164
Quoted 10 lines
drivers/lcddisplay/cfag12864b.c +10
-0+cfag12864b_setcontrollers (1 ,0 ); + else if (which == 1) + cfag12864b_setcontrollers(0, 1); +} + +/* +* Auxiliary +*/ + +static void normalizeoffset (unsigned int * offset)
Kernel style is:
static void normalizeoffset(unsigned int *offset)
drivers/lcddisplay/cfag12864b.c line 173
Quoted 9 lines
drivers/lcddisplay/cfag12864b.c +9
-0+{ +if (*offset >=CFAG12864B_PAGES *CFAG12864B_ADDRESSES ) + *offset -=CFAG12864B_PAGES *CFAG12864B_ADDRESSES ; +} + +static unsigned char calcaddress (unsigned int offset) +{ +normalizeoffset (&offset); +return offset%CFAG12864B_ADDRESSES ;
spaces around '%'
drivers/lcddisplay/cfag12864b.c line 186
Quoted 13 lines
drivers/lcddisplay/cfag12864b.c +13
-0+} + +static unsigned char calccontroller (unsigned int offset) +{ +if (offset <CFAG12864B_PAGES *CFAG12864B_ADDRESSES ) +return 0 ; +return 1 ; +} + +static unsigned char calcpage (unsigned int offset) +{ +normalizeoffset (&offset); +return offset/CFAG12864B_ADDRESSES ;
spaces around '/'
drivers/lcddisplay/cfag12864b.c line 264
Quoted 5 lines
drivers/lcddisplay/cfag12864b.c +5
-0+} + +void cfag12864b_format_nolock (unsigned char *src) +{ +unsigned short i,j,k,n;
spaces after commas
drivers/lcddisplay/cfag12864b.c line 267
Quoted 3 lines
drivers/lcddisplay/cfag12864b.c +3
-0+unsigned char *dest; + + dest =kmalloc (sizeof (unsigned char ) *CFAG12864B_SIZE ,GFP_KERNEL );
Are there places where sizeof(unsigned char) is not 1?
drivers/lcddisplay/cfag12864b.c line 270
Quoted 3 lines
drivers/lcddisplay/cfag12864b.c +3
-0+if (dest ==NULL ) { + printk(KERN_ERR CFAG12864B_NAME ": " "format: ERROR: " + "can't alloc memory %i bytes\n",
use "%zd" to print sizeof() values, otherwise gcc says:
drivers/lcddisplay/cfag12864b.c:271: warning: format '%i' expects type 'int', but argument 2 has type 'long unsigned int'
drivers/lcddisplay/cfag12864b.c line 277
Quoted 7 lines
drivers/lcddisplay/cfag12864b.c +7
-0+ sizeof(unsigned char) * CFAG12864B_SIZE); +return ; + } + +for (i =0 ; i <CFAG12864B_CONTROLLERS ; i++) +for (j =0 ; j <CFAG12864B_PAGES ; j++) +for (k =0 ; k <CFAG12864B_ADDRESSES ; k++) {
questionable indentation
drivers/lcddisplay/cfag12864b.c line 279
Quoted 2 lines
drivers/lcddisplay/cfag12864b.c +2
-0+ dest[(i *CFAG12864B_PAGES + j) *CFAG12864B_ADDRESSES + k] =0 ; +for (n=0 ; n <8 ; n++)
spaces around '='
Quoted 6 lines
drivers/lcddisplay/cfag12864b.c +6
-0+if (src[i *CFAG12864B_ADDRESSES + k + (j *8 + n) *CFAG12864B_WIDTH ]) + dest[(i *CFAG12864B_PAGES + j) *CFAG12864B_ADDRESSES + k] |=bit (n); + } + +cfag12864b_write_nolock (0 , dest,CFAG12864B_SIZE ); +
drivers/lcddisplay/cfag12864b.c line 295
Quoted 10 lines
drivers/lcddisplay/cfag12864b.c +10
-0+kfree (dest); +} + +/* +* cfag12864b Exported Commands (do lock) +*/ + +void cfag12864b_on (void ) +{ +if (down_interruptible (&cfag12864b_mutex))
space after "if"
drivers/lcddisplay/cfag12864b.c line 305
Quoted 10 lines
drivers/lcddisplay/cfag12864b.c +10
-0+return ; + +cfag12864b_on_nolock (); + +up (&cfag12864b_mutex); +} + +void cfag12864b_off (void ) +{ +if (down_interruptible (&cfag12864b_mutex))
space after "if"
drivers/lcddisplay/cfag12864b.c line 315
Quoted 10 lines
drivers/lcddisplay/cfag12864b.c +10
-0+return ; + +cfag12864b_off_nolock (); + +up (&cfag12864b_mutex); +} + +void cfag12864b_clear (void ) +{ +if (down_interruptible (&cfag12864b_mutex))
ditto
drivers/lcddisplay/cfag12864b.c line 326
Quoted 11 lines
drivers/lcddisplay/cfag12864b.c +11
-0+return ; + +cfag12864b_clear_nolock (); + +up (&cfag12864b_mutex); +} + +void cfag12864b_write (unsigned short offset,const unsigned char *buffer, +unsigned short count) +{ +if (down_interruptible (&cfag12864b_mutex))
ditto
drivers/lcddisplay/cfag12864b.c line 336
Quoted 10 lines
drivers/lcddisplay/cfag12864b.c +10
-0+return ; + +cfag12864b_write_nolock (offset,buffer,count); + +up (&cfag12864b_mutex); +} + +void cfag12864b_format (unsigned char *src) +{ +if (down_interruptible (&cfag12864b_mutex))
ditto
Quoted 8 lines
drivers/lcddisplay/cfag12864b.c +8
-0+return ; + +cfag12864b_format_nolock (src); + +up (&cfag12864b_mutex); +} + +EXPORT_SYMBOL_GPL (cfag12864b_on);
drivers/lcddisplay/cfag12864b.c line 354
Quoted 10 lines
drivers/lcddisplay/cfag12864b.c +10
-0+EXPORT_SYMBOL_GPL (cfag12864b_off); +EXPORT_SYMBOL_GPL (cfag12864b_clear); +EXPORT_SYMBOL_GPL (cfag12864b_write); +EXPORT_SYMBOL_GPL (cfag12864b_format); + +/* +* cfag12864b ioctls (don't lock because ioctl fop do) +*/ + +static int cfag12864b_fopioctlformat (void __user * arg)
use "*arg" without space
drivers/lcddisplay/cfag12864b.c line 361
Quoted 7 lines
drivers/lcddisplay/cfag12864b.c +7
-0+{ +int result; +int ret = -ENOTTY ; +unsigned char *tmpbuffer; + + tmpbuffer =kmalloc ( +sizeof (unsigned char )*CFAG12864B_MATRIXSIZE ,GFP_KERNEL );
spacebar helps readability. add spaces around * and after comma.
drivers/lcddisplay/cfag12864b.c line 364
Quoted 3 lines
drivers/lcddisplay/cfag12864b.c +3
-0+if (tmpbuffer ==NULL ) { + printk(KERN_ERR CFAG12864B_NAME ": " "FOP ioctl: ERROR: " + "can't alloc memory %i bytes\n",
use "%zd" to print sizeof() values, otherwise gcc complains.
drivers/lcddisplay/cfag12864b.c line 365
Quoted 1 line
drivers/lcddisplay/cfag12864b.c +1
-0+ sizeof(unsigned char)*CFAG12864B_MATRIXSIZE);
space around '*'
drivers/lcddisplay/cfag12864b.c line 373
Quoted 8 lines
drivers/lcddisplay/cfag12864b.c +8
-0+ goto none; + } + + result =copy_from_user (tmpbuffer, arg, +sizeof (unsigned char ) *CFAG12864B_MATRIXSIZE ); +if (result !=0 ) { + printk(KERN_ERR CFAG12864B_NAME ": " "FOP ioctl: ERROR: " + "can't copy memory from user\n");
I don't think that we want a printk on every copy_from_user() error. (here and elsewhere)
Quoted 13 lines
drivers/lcddisplay/cfag12864b.c +13
-0+ goto bufferalloced; + } + +cfag12864b_format_nolock (tmpbuffer); + + ret =0 ; + +bufferalloced: +kfree (tmpbuffer); + +none: +return ret; +}
drivers/lcddisplay/cfag12864b.c line 396
Quoted 10 lines
drivers/lcddisplay/cfag12864b.c +10
-0+ +/* +* cfag12864b_fops (do lock) +*/ + +static loff_t cfag12864b_fopseek (struct file *filp,loff_t offset,int whence) +{ +loff_t ret = -EINVAL ; + +if (down_interruptible (&cfag12864b_mutex))
space after "if"
Quoted 25 lines
drivers/lcddisplay/cfag12864b.c +25
-0+return -ERESTARTSYS ; + +switch (whence) { +case SEEK_SET : + ret = offset; +break ; +case SEEK_CUR : + ret = filp->f_pos + offset; +break ; +case SEEK_END : + ret =CFAG12864B_SIZE + offset; +break ; + } + +if (ret <0 ) { + ret = -EINVAL ; + goto none; + } + + filp->f_pos = ret; + +none: +up (&cfag12864b_mutex); +return ret; +}
drivers/lcddisplay/cfag12864b.c line 431
Quoted 10 lines
drivers/lcddisplay/cfag12864b.c +10
-0+ + +static ssize_t cfag12864b_fopwrite (struct file *filp, +const char __user *buffer,size_t count,loff_t *offset) +{ +int ret = -EINVAL ; +int result; +unsigned char *tmpbuffer; + +if (down_interruptible (&cfag12864b_mutex))
space
drivers/lcddisplay/cfag12864b.c line 441
Quoted 10 lines
drivers/lcddisplay/cfag12864b.c +10
-0+return -ERESTARTSYS ; + +if (*offset >CFAG12864B_SIZE ) { + ret =0 ; + goto none; + } +if (*offset + count >CFAG12864B_SIZE ) + count =CFAG12864B_SIZE -*offset; + + tmpbuffer =kmalloc (count,GFP_KERNEL );
I would be very tempted to allocate a buffer at (open or init?) time, of size CFAG12864B_SIZE. It could be used here for fopwrite and in format_nolock without having to call kmalloc() repeatedly for those operations. However, fopioctlformat would still need to allocate its larger buffer.
drivers/lcddisplay/cfag12864b.c line 444
Quoted 3 lines
drivers/lcddisplay/cfag12864b.c +3
-0+if (tmpbuffer ==NULL ) { + printk(KERN_ERR CFAG12864B_NAME ": " "FOP write: ERROR: " + "can't alloc memory %i bytes\n",count);
use "%zd" to print sizeof values, otherwise gcc complains. use space after comma.
Quoted 21 lines
drivers/lcddisplay/cfag12864b.c +21
-0+ ret = -ENOMEM ; + goto none; + } + + result =copy_from_user (tmpbuffer, buffer, count); +if (result !=0 ) { + printk(KERN_ERR CFAG12864B_NAME ": " "FOP write: ERROR: " + "can't copy memory from user\n"); + ret = -EFAULT ; + goto bufferalloced; + } + +cfag12864b_write_nolock (*offset, tmpbuffer, count); + + *offset += count; + ret = count; + +bufferalloced: +kfree (tmpbuffer); + +none:
drivers/lcddisplay/cfag12864b.c line 475
Quoted 10 lines
drivers/lcddisplay/cfag12864b.c +10
-0+up (&cfag12864b_mutex); +return ret; +} + +static int cfag12864b_fopioctl (struct inode * inode,struct file * filp, +unsigned int cmd,unsigned long arg) +{ +int ret = -ENOTTY ; + +if (down_interruptible (&cfag12864b_mutex))
space after "if"
Quoted 69 lines
drivers/lcddisplay/cfag12864b.c +69
-0+return -ERESTARTSYS ; + +if (_IOC_TYPE (cmd) !=CFAG12864B_IOC_MAGIC ) + goto none; +if (_IOC_NR (cmd) >CFAG12864B_IOC_MAXNR ) + goto none; + +switch (cmd) { +case CFAG12864B_IOCON : +cfag12864b_on_nolock (); + ret =0 ; +break ; +case CFAG12864B_IOCOFF : +cfag12864b_off_nolock (); + ret =0 ; +break ; +case CFAG12864B_IOCCLEAR : +cfag12864b_clear_nolock (); + ret =0 ; +break ; +case CFAG12864B_IOCFORMAT : + ret =cfag12864b_fopioctlformat ((void __user *)arg); + } + +none: +up (&cfag12864b_mutex); +return ret; +} + +static const struct file_operations cfag12864b_fops = +{ + .owner =THIS_MODULE , + .llseek = cfag12864b_fopseek, + .write = cfag12864b_fopwrite, + .ioctl = cfag12864b_fopioctl, +}; + +/* +* Module Init & Exit +*/ + +static int __init cfag12864b_init (void ) +{ + +#include "cfag12864b_image.h" + +int result; +int ret = -EINVAL ; + + result =alloc_chrdev_region (&cfag12864b_device, cfag12864b_firstminor, + cfag12864b_ndevices,CFAG12864B_NAME ); +if (result <0 ) { + printk(KERN_ERR CFAG12864B_NAME ": " "ERROR: " + "can't alloc the char device region\n"); + ret = result; + goto none; + } + + cfag12864b_major =MAJOR (cfag12864b_device); + cfag12864b_minor = cfag12864b_firstminor; + cfag12864b_device =MKDEV (cfag12864b_major, cfag12864b_minor); + +cfag12864b_clear_nolock (); +cfag12864b_on_nolock (); +cfag12864b_write_nolock (0 , cfag12864b_image,CFAG12864B_SIZE ); + +cdev_init (&cfag12864b_chardevice,&cfag12864b_fops); + cfag12864b_chardevice.owner =THIS_MODULE ; + cfag12864b_chardevice.ops = &cfag12864b_fops;
drivers/lcddisplay/cfag12864b.c line 554
Quoted 10 lines
drivers/lcddisplay/cfag12864b.c +10
-0+ result =cdev_add (&cfag12864b_chardevice, cfag12864b_device, + cfag12864b_ndevices); +if (result <0 ) { + printk(KERN_ERR CFAG12864B_NAME ": " "ERROR: " + "unable to add a new char device\n"); + ret = result; + goto regionalloced; + } + + if(class_device_create(lcddisplay_class, NULL, cfag12864b_device, NULL,
space after "if"
drivers/lcddisplay/cfag12864b.c line 562
Quoted 8 lines
drivers/lcddisplay/cfag12864b.c +8
-0+ "cfag12864b%d", cfag12864b_minor) == NULL) { + printk(KERN_ERR CFAG12864B_NAME ": " "ERROR: " + "unable to create a device class\n"); + ret = -EINVAL ; + goto cdevadded; + } + + printk(KERN_INFO CFAG12864B_NAME ": " "Inited\n");
KERN_DEBUG ?
Quoted 12 lines
drivers/lcddisplay/cfag12864b.c +12
-0+ +return 0 ; + +cdevadded: +cdev_del (&cfag12864b_chardevice); + +regionalloced: +unregister_chrdev_region (cfag12864b_device, cfag12864b_ndevices); + +none: +return ret; +}
drivers/lcddisplay/cfag12864b.c line 584
Quoted 10 lines
drivers/lcddisplay/cfag12864b.c +10
-0+ +static void __exit cfag12864b_exit(void) +{ +cfag12864b_off_nolock (); + +class_device_destroy (lcddisplay_class, cfag12864b_device); +cdev_del (&cfag12864b_chardevice); +unregister_chrdev_region (cfag12864b_device, cfag12864b_ndevices); + + printk(KERN_INFO CFAG12864B_NAME ": " "Exited\n");
KERN_DEBUG ?
drivers/lcddisplay/cfag12864b.c line 592
Quoted 8 lines
drivers/lcddisplay/cfag12864b.c +8
-0+} + +module_init (cfag12864b_init); +module_exit (cfag12864b_exit); + +MODULE_LICENSE ("GPL" ); +MODULE_AUTHOR ("Miguel Ojeda Sandonis <[email protected]>" ); +MODULE_DESCRIPTION ("cfag12864b" );
That's not a useful MODULE_DESCRIPTION.
drivers/lcddisplay/cfag12864b_image.h line 4
Quoted 8 lines
diff -uprN -X dontdiff
linux-2.6.18-vanilla/drivers/lcddisplay/cfag12864b_image.hlinux-2.6.18/drivers/lcddisplay/cfag12864b_image.hdrivers/lcddisplay/cfag12864b_image.h +4
-0@@ -0,0 +1,95 @@ +#ifndef _CFAG12864B_IMAGE_H_ +#define _CFAG12864B_IMAGE_H_ + +const unsigned char cfag12864b_image[] = {
What is in this image array? and what format is it in? It's not just advertising/logo material, is it? Is it really needed in cfag12864b_init()?
[bits deleted]
drivers/lcddisplay/Kconfig line 8
Quoted 12 lines
diff -uprN -X dontdiff linux-2.6.18-vanilla/drivers/lcddisplay/Kconfig linux-2.6.18/drivers/lcddisplay/Kconfig
drivers/lcddisplay/Kconfig +8
-0@@ -0,0 +1,110 @@ +# +# For a description of the syntax of this configuration file, +# see Documentation/kbuild/kconfig-language.txt. +# +# LCD Display drivers configuration. +# +# Maintainer: Miguel Ojeda Sandonis <[email protected]> +#
Don't need any of those comments above.
drivers/lcddisplay/Kconfig line 19
Quoted 10 lines
drivers/lcddisplay/Kconfig +10
-0+menu "LCD Display support" + +config LCDDISPLAY + tristate "LCD Display support" + default n + ---help--- + If you have a LCD display, say Y. + + To compile this as a module, choose M here: + module will be called lcddisplay.
"the module will be..."
drivers/lcddisplay/Kconfig line 21
Quoted 1 line
drivers/lcddisplay/Kconfig +1
-0+ Most LCD drivers use a I/O port (like the parallel port)
use an I/O port
drivers/lcddisplay/Kconfig line 22
Quoted 1 line
drivers/lcddisplay/Kconfig +1
-0+ so you will need to say Y or M at them if you want to see
say Y or M for them
drivers/lcddisplay/Kconfig line 27
Quoted 5 lines
drivers/lcddisplay/Kconfig +5
-0+ more options in this menu. + + If unsure, say N. + + Maintainer: Miguel Ojeda Sandonis <[email protected]>
Just in the MAINTAINERS file.
drivers/lcddisplay/Kconfig line 36
Quoted 8 lines
drivers/lcddisplay/Kconfig +8
-0+comment "Parallel port dependent:" + +config KS0108 + tristate "KS0108 LCD Controller" + depends on LCDDISPLAY && PARPORT + default n + ---help--- + If you have a LCD display controlled by one or more KS0108
have an LCD display
drivers/lcddisplay/Kconfig line 42
Quoted 6 lines
drivers/lcddisplay/Kconfig +6
-0+ controllers, say Y. You will need also another more specific + driver for your LCD. + + Depends on Parallel Port support. If you say Y at + parport, you will be able to compile this as a module (M) + and built-in as well (Y). If you said M at parport,
s/and/or/
drivers/lcddisplay/Kconfig line 43
Quoted 1 line
drivers/lcddisplay/Kconfig +1
-0+ you will be able only to compile this as a module (M).
However, that is true everywhere, so we usually don't say it.
drivers/lcddisplay/Kconfig line 42
Quoted 2 lines
drivers/lcddisplay/Kconfig +2
-0+ To compile this as a module, choose M here: + module will be called ks0108.
"the module will be..."
drivers/lcddisplay/Kconfig line 27
Quoted 3 lines
drivers/lcddisplay/Kconfig +3
-0+ If unsure, say N. + + Maintainer: Miguel Ojeda Sandonis <[email protected]>
Just in MAINTAINERS file.
drivers/lcddisplay/Kconfig line 53
Quoted 2 lines
drivers/lcddisplay/Kconfig +2
-0+config KS0108_PORT + hex "Parallel port where the LCD is connected to"
...where the LCD is connected"
drivers/lcddisplay/Kconfig line 57
Quoted 4 lines
drivers/lcddisplay/Kconfig +4
-0+ depends on KS0108 + default 0x378 + ---help--- + The address of the parallel port where the LCD is connected to.
... where the LCD is connected.
drivers/lcddisplay/Kconfig line 66
Quoted 8 lines
drivers/lcddisplay/Kconfig +8
-0+ The first standard parallel port address is 0x378. + The second standard parallel port address is 0x278. + The third standard parallel port address is 0x3BC. + + You can specify a different address if you need. + + If you don't know what I'm talking about, load the parport module, + and execute "dmesg". You can see there how many parallel ports
comma after "parallel ports"
drivers/lcddisplay/Kconfig line 67
Quoted 1 line
drivers/lcddisplay/Kconfig +1
-0+ where detected and which address everyone has.
add comma, change wording, like: "where they are connected," "and which address each one has."
Quoted 22 lines
drivers/lcddisplay/Kconfig +22
-0+ Usually you only need to use 0x378. + + If you compile this as a module, you can still override this + using the module parameters. + +config KS0108_DELAY + int "Delay between each control writing (microseconds)" + depends on KS0108 + default "2" + ---help--- + Amount of time the ks0108 should wait between each control write + to the parallel port. + + If your driver seems to miss random writings, increment this. + + If you don't know what I'm talking about, ignore it. + + If you compile this as a module, you can still override this + using the module parameters. + +config CFAG12864B + tristate "CFAG12864B LCD Display"
drivers/lcddisplay/Kconfig line 100
Quoted 10 lines
drivers/lcddisplay/Kconfig +10
-0+ depends on KS0108 + default n + ---help--- + If you have a Crystalfontz 128x64 2-color LCD display, + cfag12864b Series, say Y. You also need the ks0108 LCD + Controller driver. + + For help about how to wire your LCD to the parallel port, + check this image: http://www.skippari.net/lcd/sekalaista + /crystalfontz_cfag12864B-TMI-V.png
check this image: <newline and put URL all on one line>
drivers/lcddisplay/Kconfig line 94
Quoted 2 lines
drivers/lcddisplay/Kconfig +2
-0+ To compile this as a module, choose M here: + module will be called cfag12864b.
"the module will be..."
drivers/lcddisplay/Kconfig line 27
Quoted 3 lines
drivers/lcddisplay/Kconfig +3
-0+ If unsure, say N. + + Maintainer: Miguel Ojeda Sandonis <[email protected]>
Only in MAINTAINERS file.
drivers/lcddisplay/ks0108.c line 5
Quoted 11 lines
drivers/lcddisplay/Kconfig +2
-0+endmenu +diff -uprN -X dontdiff
linux-2.6.18-vanilla/drivers/lcddisplay/ks0108.clinux-2.6.18/drivers/lcddisplay/ks0108.cdrivers/lcddisplay/ks0108.c +5
-0@@ -0,0 +1,160 @@ +/* +* ks0108 Exported cmds (don't lock) +* +* you _should_ lock in the top driver, so +* this functions _should not_ get race conditions in any way.
these functions
drivers/lcddisplay/ks0108.c line 74
Quoted 9 lines
drivers/lcddisplay/ks0108.c +9
-0+* Locking for each byte here would be so slow and useless. +*/ + +#define bit (n) (((unsigned char)1)<<(n)) + +void ks0108_writecontrol (unsigned char byte) +{ +udelay (ks0108_delay); +parport_write_control (ks0108_parport, byte ^ (bit (0 ) |bit (1 ) |bit (3 )));
what does the xor do? what are the bits?
drivers/lcddisplay/ks0108.c line 79
Quoted 5 lines
drivers/lcddisplay/ks0108.c +5
-0+} + +void ks0108_displaystate (unsigned char state) +{ +ks0108_writedata ((state ?bit (0 ) :0 ) |bit (1 ) |bit (2 ) |bit (3 ) |bit (4 ) |bit (5 ));
Are the state bits documented somewhere? Can you use meaningful mnemonic names for the bits?
drivers/lcddisplay/ks0108.c line 84
Quoted 5 lines
drivers/lcddisplay/ks0108.c +5
-0+} + +void ks0108_startline (unsigned char startline) +{ +ks0108_writedata (min (startline,(unsigned char )63 ) |bit (6 ) |bit (7 ));
bit definitions?
drivers/lcddisplay/ks0108.c line 89
Quoted 5 lines
drivers/lcddisplay/ks0108.c +5
-0+} + +void ks0108_address (unsigned char address) +{ +ks0108_writedata (min (address,(unsigned char )63 ) |bit (6 ));
bit definition?
drivers/lcddisplay/ks0108.c line 94
Quoted 5 lines
drivers/lcddisplay/ks0108.c +5
-0+} + +void ks0108_page (unsigned char page) +{ +ks0108_writedata (min (page,(unsigned char )7 ) |bit (3 ) |bit (4 ) |bit (5 ) |bit (7 ));
bit definitions?
drivers/lcddisplay/ks0108.c line 115
Quoted 10 lines
drivers/lcddisplay/ks0108.c +10
-0+} + +static int __init ks0108_init (void ) +{ +int result; +int ret = -EINVAL ; + + ks0108_parport =parport_find_base (ks0108_port); +if (ks0108_parport ==NULL ) { + printk(KERN_ERR KS0108_NAME ": " "ERROR: "
merge strings together, like:
printk(KERN_ERR KS0108_NAME ": ERROR: "
(throughout the source files)
drivers/lcddisplay/ks0108.c line 116
Quoted 1 line
drivers/lcddisplay/ks0108.c +1
-0+ "parport didn't find %i port\n",ks0108_port);
space after ","
Quoted 5 lines
drivers/lcddisplay/ks0108.c +5
-0+ goto none; + } + + ks0108_pardevice =parport_register_device (ks0108_parport,KS0108_NAME , +NULL ,NULL ,NULL ,PARPORT_DEV_EXCL ,NULL );
drivers/lcddisplay/ks0108.c line 131
Quoted 10 lines
drivers/lcddisplay/ks0108.c +10
-0+if (ks0108_pardevice ==NULL ) { + printk(KERN_ERR KS0108_NAME ": " "ERROR: " + "parport didn't register new device\n"); + goto none; + } + + result =parport_claim (ks0108_pardevice); +if (result !=0 ) { + printk(KERN_ERR KS0108_NAME ": " "ERROR: " + "can't claim %i parport, maybe in use\n",ks0108_port);
space after comma
Quoted 8 lines
drivers/lcddisplay/ks0108.c +8
-0+ ret = result; + goto registered; + } + + printk(KERN_INFO KS0108_NAME ": " "Inited - ks0108_port=0x%X ks0108_delay=%i\n", ks0108_port, ks0108_delay); +return 0 ; + +registered:
drivers/lcddisplay/ks0108.c line 159
Quoted 10 lines
drivers/lcddisplay/ks0108.c +10
-0+parport_unregister_device (ks0108_pardevice); + +none: +return ret; +} + + +MODULE_LICENSE ("GPL" ); +MODULE_AUTHOR ("Miguel Ojeda Sandonis <[email protected]>" ); +MODULE_DESCRIPTION ("ks0108" );
Use a real MODULE_DESCRIPTION, please.
drivers/lcddisplay/lcddisplay.c line 8
Quoted 12 lines
diff -uprN -X dontdiff
linux-2.6.18-vanilla/drivers/lcddisplay/lcddisplay.clinux-2.6.18/drivers/lcddisplay/lcddisplay.cdrivers/lcddisplay/lcddisplay.c +8
-0@@ -0,0 +1,79 @@ + +static int __init lcddisplay_init (void ) +{ +int ret = -EINVAL ; + + lcddisplay_class =class_create (THIS_MODULE ,LCDDISPLAY_NAME ); +if (IS_ERR (lcddisplay_class)) { + printk(KERN_ERR LCDDISPLAY_NAME ": " "ERROR: "
The multiple quotation marks is a bit odd & difficult to read. How about just doing:
printk(KERN_ERR LCDDISPLAY_NAME ": ERROR: "
(in MANY places)?
drivers/lcddisplay/lcddisplay.c line 58
Quoted 5 lines
drivers/lcddisplay/lcddisplay.c +5
-0+ "can't create %s class\n", LCDDISPLAY_NAME); + goto none; + } + + printk(KERN_INFO LCDDISPLAY_NAME ": " "Inited\n");
printk(KERN_INFO LCDDISPLAY_NAME ": Inited\n"); or better yet, printk(KERN_DEBUG LCDDISPLAY_NAME ": Inited\n");
drivers/lcddisplay/lcddisplay.c line 70
Quoted 11 lines
drivers/lcddisplay/lcddisplay.c +11
-0+return 0 ; + +none: +return ret; +} + +static void __exit lcddisplay_exit(void) +{ +class_destroy (lcddisplay_class); + + printk(KERN_INFO LCDDISPLAY_NAME ": " "Exited\n");
KERN_DEBUG & merge the strings?
drivers/lcddisplay/lcddisplay.c line 78
Quoted 8 lines
drivers/lcddisplay/lcddisplay.c +8
-0+} + +module_init (lcddisplay_init); +module_exit (lcddisplay_exit); + +MODULE_LICENSE ("GPL" ); +MODULE_AUTHOR ("Miguel Ojeda Sandonis <[email protected]>" ); +MODULE_DESCRIPTION ("lcddisplay" );
Not a useful MODULE_DESCRIPTION.
include/linux/cfag12864b.h line 9
Quoted 13 lines
diff -uprN -X dontdiff
linux-2.6.18-vanilla/include/linux/cfag12864b.hlinux-2.6.18/include/linux/cfag12864b.hinclude/linux/cfag12864b.h +9
-0@@ -0,0 +1,58 @@ + +#ifndef _CFAG12864B_H_ +#define _CFAG12864B_H_ + +#include <linux/ioctl.h> + +#define CFAG12864B_WIDTH 128 +#define CFAG12864B_HEIGHT 64 +#define CFAG12864B_MATRIXSIZE CFAG12864B_WIDTH*CFAG12864B_HEIGHT
Use parens around defined expressions (above).
include/linux/cfag12864b.h line 40
Quoted 6 lines
include/linux/cfag12864b.h +6
-0+#define CFAG12864B_CONTROLLERS 2 +#define CFAG12864B_PAGES 8 +#define CFAG12864B_ADDRESSES 64 +#define CFAG12864B_SIZE CFAG12864B_CONTROLLERS * \ + CFAG12864B_PAGES * \ + CFAG12864B_ADDRESSES
use parens.
Quoted 4 lines
include/linux/cfag12864b.h +4
-0+#define CFAG12864B_IOC_MAGIC 0xFF +#define CFAG12864B_IOC_MAXNR 0x03 + +#define CFAG12864B_IOCOFF _IO(CFAG12864B_IOC_MAGIC,0)
include/linux/cfag12864b.h line 55
Quoted 10 lines
include/linux/cfag12864b.h +10
-0+#define CFAG12864B_IOCON _IO(CFAG12864B_IOC_MAGIC,1) +#define CFAG12864B_IOCCLEAR _IO(CFAG12864B_IOC_MAGIC,2) +#define CFAG12864B_IOCFORMAT _IOW(CFAG12864B_IOC_MAGIC,3,void *) + +extern void cfag12864b_on (void ); +extern void cfag12864b_off (void ); +extern void cfag12864b_clear (void ); +extern void cfag12864b_write (unsigned short offset, +const unsigned char *buffer,unsigned short count); +extern void cfag12864b_format (unsigned char *src);
All function prototypes that have parameters (above):
kernel style is to use type + param_name, not just type.
Documentation/lcddisplay/cfag12864b line 6
Quoted 12 lines
include/linux/cfag12864b.h +2
-0+#endif /* _CFAG12864B_H_ */ +diff -uprN -X dontdiff linux-2.6.18-vanilla/Documentation/lcddisplay/cfag12864b linux-2.6.18/Documentation/lcddisplay/cfag12864b
Documentation/lcddisplay/cfag12864b +6
-0@@ -0,0 +1,371 @@ + +--------- +2. WIRING +--------- + +The cfag12864b LCD Display Series don't have a official wiring.
... don't have official wiring.
Quoted 11 lines
Documentation/lcddisplay/cfag12864b +10
-0+The common wiring is done to the parallel port: + +http://www.skippari.net/lcd/sekalaista/crystalfontz_cfag12864B-TMI-V.png + +You can get help at Crystalfontz and LCDInfo forums. + + +3.1. ioctl & 128*64 boolean matrix +------------------------------------------------------- +
Documentation/lcddisplay/cfag12864b line 91
Quoted 10 lines
Documentation/lcddisplay/cfag12864b +10
-0+This method is easier, but you have to update the entire display +each time you want to change it. + +Note: + + CFAG12864B_FORMATSIZE == + CFAG12864B_WIDTH * CFAG12864B_HEIGHT == + 128 * 64 + +Declare the matrix and other one:
other one what? another matrix buffer?
Documentation/lcddisplay/cfag12864b line 97
Quoted 5 lines
Documentation/lcddisplay/cfag12864b +5
-0+ unsigned char MyDrawing[CFAG12864B_WIDTH][CFAG12864B_HEIGHT]; + + unsigned char Buffer[CFAG12864B_FORMATSIZE]; + +Copy the 2d matrix to the buffer , like:
to the buffer,
Quoted 7 lines
Documentation/lcddisplay/cfag12864b +7
-0+ + for(i = 0; i < CFAG12864B_WIDTH; i++) + for(j = 0; j < CFAG12864B_HEIGHT; j++) + Buffer[i + j * CFAG12864B_WIDTH] = MyDrawing[i][j]; + +Call the ioctl: +
Documentation/lcddisplay/cfag12864b line 114
Quoted 10 lines
Documentation/lcddisplay/cfag12864b +10
-0+ ioctl(fdisplay, CFAG12864B_IOCFORMAT, Buffer); + +Voila! Your drawing should appear on the screen. + + + +3.2. Direct writing +------------------- + +This methods allows you to change each byte of the device,
This method
Documentation/lcddisplay/cfag12864b line 205
Quoted 8 lines
Documentation/lcddisplay/cfag12864b +7
-0+so you can achieve a higher update rate updating only the pixels +you are going to change. +4.2 Example BMP writer +---------------------- + +You can take ideas from this code and start programming. I think it is useful +for understanding how the driver can be used. It just work, don't expect
just works;
Documentation/lcddisplay/cfag12864b line 208
Quoted 3 lines
Documentation/lcddisplay/cfag12864b +3
-0+good BMP-related code. I chose such bitmap format because it is simple. + +The program reads a .bmp 128x64 2-colors file, convert it to a
"converts it to a"
Quoted 1 line
+boolean [128*64] buffer and then use ioctl to display it on the screen.
"then uses"
Documentation/ioctl-number.txt Code Seq# Include File Comments · line 195
Quoted 9 lines
diff -uprN -X dontdiff
linux-2.6.18-vanilla/Documentation/ioctl-number.txtlinux-2.6.18/Documentation/ioctl-number.txtDocumentation/ioctl-number.txt +2
-0@@ -191,3 +191,5 @@ Code Seq# Include File Comments <mailto:[email protected]> 0xF3 00-3F video/sisfb.h sisfb (in development) <mailto:[email protected]> +0xFF 00-1F linux/cfag12864b.h cfag12864b LCD Display Driver + <mailto:[email protected]>
Also need additions to Documenation/ABI/ ? Please read/check Documentation/ABI/README. and please read/check Documentation/SubmitChecklist.
MAINTAINERS M: [email protected] · line 1714
Quoted 12 lines
diff -uprN -X dontdiff linux-2.6.18-vanilla/MAINTAINERS linux-2.6.18/MAINTAINERS
MAINTAINERS +5
-0@@ -1707,6 +1707,11 @@ M: [email protected] L: [email protected] S: Maintained +LCD DISPLAY DRIVERS +P: Miguel Ojeda Sandonis +M: [email protected] +S: Maintained +
Needs a mailing list (L:) entry.
---
~Randy
...
Quoted 1 line
Quoted 1 line
patching file
drivers/lcddisplay/cfag12864b.c
...
What does the D in LCD stand for? I suggest this is named drivers/lcdisplay/ instead.
Signature · 3 lines
Stefan Richter -=====-=-==- =--= ====- http://arcgraph.de/sr/
Quoted 6 lines · On Sat, 30 Sep 2006 23:36:44 +0200 Stefan Richter wrote:
...
Quoted 1 line
Quoted 1 line
patching file
drivers/lcddisplay/cfag12864b.c...
What does the D in LCD stand for? I suggest this is named
drivers/lcdisplay/instead.
Yes, someone else mentioned that too.
It does mean Display.
not a big deal to me in this age of acronyms.
---
~Randy
Randy, I'm testing right now the V7 of the driver after fixing all your mistakes found. Thank you for your deep work: the sharing of the buffer was a very good point. Finally I allocated both 2 (the bigger and the smaller one at the initialization).
In a few minutes I will send the V7 patch.
About "lcdisplay":
Quoted 4 lines · Randy Dunlap
Quoted 2 lines · On Sat, 30 Sep 2006 23:36:44 +0200 Stefan Richter wrote:
...
Quoted 1 line
Quoted 1 line
patching file
drivers/lcddisplay/cfag12864b.c
Quoted 9 lines
Quoted 4 lines
...
What does the D in LCD stand for? I suggest this is named
drivers/lcdisplay/instead.Yes, someone else mentioned that too.
It does mean Display.
not a big deal to me in this age of acronyms.
Well, I can rename it, but there is a problem:
How should I write the name in other places?
I mean, for example, in Kconfig:
tristate "KS0108 LCD Controller"
if the ks0108 is a LCD controller, then
tristate "CFAG12864B LCD Display"
should be the right way to write it?
tristate "CFAG12864B LCDisplay"
seems so ugly to me. Is it? Remember, this applies for all the other places in the documentation / source code / ...
IMHO it seems better as LCD Display. Yeah, drivers/lcdisplay/ would be nice, but I don't think the same about other places:
LCD Controller.
LCDisplay??
Quoted 28 lines · On Sat, 30 Sep 2006 22:45:56 +0000 Miguel Ojeda wrote:
Randy, I'm testing right now the V7 of the driver after fixing all your mistakes found. Thank you for your deep work: the sharing of the buffer was a very good point. Finally I allocated both 2 (the bigger and the smaller one at the initialization).
In a few minutes I will send the V7 patch.
About "lcdisplay":
Quoted 4 lines · Randy Dunlap
Quoted 2 lines · On Sat, 30 Sep 2006 23:36:44 +0200 Stefan Richter wrote:
...
Quoted 1 line
Quoted 1 line
patching file
drivers/lcddisplay/cfag12864b.cQuoted 9 lines
Quoted 4 lines
...
What does the D in LCD stand for? I suggest this is named
drivers/lcdisplay/instead.Yes, someone else mentioned that too.
It does mean Display.
not a big deal to me in this age of acronyms.Well, I can rename it, but there is a problem:
How should I write the name in other places?
Quoted 7 lines
I mean, for example, in
Kconfig:tristate "KS0108 LCD Controller"
if the ks0108 is a LCD controller, then
tristate "CFAG12864B LCD Display"
That seems very common to me.
Quoted 11 lines
should be the right way to write it?
tristate "CFAG12864B LCDisplay"
seems so ugly to me. Is it? Remember, this applies for all the other places in the documentation / source code / ...
IMHO it seems better as LCD Display. Yeah,
drivers/lcdisplay/would be nice, but I don't think the same about other places:LCD Controller.
OK
Quoted 1 line
LCDisplay??
I would continue to use LCD display (small d).
---
~Randy
Quoted 1 line · Randy Dunlap
On Sat, 30 Sep 2006 22:45:56 +0000 Miguel Ojeda wrote:
...
Quoted 3 lines
Quoted 1 line
tristate "CFAG12864B LCD Display"
That seems very common to me.
It is as common as it is wrong. http://en.wikipedia.org/wiki/RAS_Syndrome
...
Quoted 1 line
Quoted 1 line
LCDisplay??
A contraction like LCDisplay, like my suggested lcdisplay for the path name, is IMO not suitable for use in normal written language. Use it at most for path names where we contract words into one or would write lc-display or lc_display.
Quoted 1 line
I would continue to use LCD display (small d).
Still wrong language.
"LCD" and "LC display" are correct.
Signature · 3 lines
Stefan Richter -=====-=-==- =-=- ----= http://arcgraph.de/sr/
Quoted 17 lines · On Sun, 01 Oct 2006 10:20:41 +0200 Stefan Richter wrote:
Quoted 1 line · Randy Dunlap
On Sat, 30 Sep 2006 22:45:56 +0000 Miguel Ojeda wrote:
...
Quoted 3 lines
Quoted 1 line
tristate "CFAG12864B LCD Display"
That seems very common to me.
It is as common as it is wrong. http://en.wikipedia.org/wiki/RAS_Syndrome
...
Quoted 1 line
Quoted 1 line
LCDisplay??
A contraction like LCDisplay, like my suggested lcdisplay for the path name, is IMO not suitable for use in normal written language. Use it at most for path names where we contract words into one or would write lc-display or
lc_display.
Quoted 5 lines
Quoted 1 line
I would continue to use LCD display (small d).
Still wrong language.
"LCD" and "LC display" are correct.
whatever.
I've never heard anyone speak of or write about an LC display.
---
~Randy
Quoted 1 line · Randy Dunlap
On Sun, 01 Oct 2006 10:20:41 +0200 Stefan Richter wrote:
...
Quoted 1 line
Quoted 1 line
It is as common as it is wrong. http://en.wikipedia.org/wiki/RAS_Syndrome
...
Quoted 4 lines
Quoted 1 line
"LCD" and "LC display" are correct.
whatever.
I've never heard anyone speak of or write about an LC display.
Nor does anybody say or write AT machine. A web search finds 15 times more occurrences of "LC-Display" on German pages than of "LC display" on English pages. So it seams my suggestion of "LC display" is a bad one for usage outside of Germany. BTW, "LCD-Display" is doubly as often found on German web pages than "LC-Display" on German pages, and I'm not surprised.
At some point, widely used words and expressions become accepted parts of a language. "LCD display" however is evidently still considered bad style --- in English as well as in German. But maybe this stylistic mistake is more prone to criticism among Germans than among North Americans. (Perhaps not) coincidentally, the other person who criticized it in this thread is obviously German too.
Signature · 3 lines
Stefan Richter -=====-=-==- =-=- ----= http://arcgraph.de/sr/
Quoted 8 lines · Stefan Richter
Nor does anybody say or write AT machine. A web search finds 15 times more occurrences of "LC-Display" on German pages than of "LC display" on English pages. So it seams my suggestion of "LC display" is a bad one for usage outside of Germany. BTW, "LCD-Display" is doubly as often found on German web pages than "LC-Display" on German pages, and I'm not surprised.
Searching Google for "pantalla LCD" you can find 1.590.000 pages.
And it is wrong ;-)
(Please continue the discussion the same "thread" (V7) as it is the last version of the patch I sent.)