From: Jaya Kumar <jayakumar.lkml@gmail.com> To: "Rick L. Vinyard Jr." <rvinyard@cs.nmsu.edu> Cc: linux-kernel@vger.kernel.org, krzysztof.h1@wp.pl, akpm@linux-foundation.org, linux-usb@vger.kernel.org, oliver@neukum.org, linux-input@vger.kernel.org, jkosina@suse.cz Subject: Re: [PATCH] Logitech G13 driver (fixed cc list --- ignore others) Date: Tue, 5 Jan 2010 07:57:44 +0800 [thread overview] Message-ID: <45a44e481001041557t1f87f8d4i959abbbee0c4346@mail.gmail.com> (raw) In-Reply-To: <200912142122.nBELMW7d001243@mustang.cs.nmsu.edu> On Tue, Dec 15, 2009 at 5:22 AM, Rick L. Vinyard Jr. <rvinyard@cs.nmsu.edu> wrote: > Additionally, this device contains a 160x43 monochrome LCD display. > A registered framebuffer device manages this display. The design > of this portion of the driver was based on the design of the > hecubafb driver with deferred framebuffer I/O since there is > no real memory to map. Hi Rick, Interesting work. I recommend CCing linux-fbdev@vger.kernel.org too since it contains a fbdev interface. > +config LOGITECH_G13 > + tristate "Logitech G13 gameboard support" > + depends on HID_LOGITECH > + depends on FB > + select FB_SYS_FILLRECT > + select FB_SYS_COPYAREA > + select FB_SYS_IMAGEBLIT > + select FB_SYS_FOPS Does this need to select FB_DEFERRED_IO? > --- /dev/null > +++ b/drivers/hid/hid-g13-logo.xbm > @@ -0,0 +1,75 @@ > +#define g13_lcd_width 160 > +#define g13_lcd_height 43 > +static unsigned char g13_lcd_bits[] = { > + 0x00, 0x00, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, Was there a reason for having a new logo file? If so, you might want to put it in the comments and also put it together with the standard kernel logo. > +/* 160*43 rounded to nearest whole byte which is 160*48 since bytes are > + vertical the y component must be a multiple of 8 */ > +#define G13FB_SIZE (160*48/8) Minor nit, I think there is a macro for this, DIV_ROUND_UP, or maybe this could be better done in the function that uses this value. > +static ssize_t g13_set_mled(struct hid_device *hdev, unsigned mled); Does this need to be here or can the code be reordered? > +static struct fb_var_screeninfo g13fb_var = { > + .xres = G13FB_WIDTH, > + .yres = G13FB_HEIGHT, > + .xres_virtual = G13FB_WIDTH, > + .yres_virtual = G13FB_HEIGHT, > + .bits_per_pixel = 1, > + .nonstd = 1, > +}; I think the nonstd is a bug. Yes, hecubafb and metronomefb seem to have the same bug as well. nonstd is used if you want something like FB_NONSTD_HAM or FB_NONSTD_REV_PIX_IN_B, which I don't think is what you want. Hope this helps. Best regards, jaya
WARNING: multiple messages have this Message-ID (diff)
From: Jaya Kumar <jayakumar.lkml@gmail.com> To: "Rick L. Vinyard Jr." <rvinyard@cs.nmsu.edu> Cc: linux-kernel@vger.kernel.org, krzysztof.h1@wp.pl, akpm@linux-foundation.org, linux-usb@vger.kernel.org, oliver@neukum.org, linux-input@vger.kernel.org, jkosina@suse.cz Subject: Re: [PATCH] Logitech G13 driver (fixed cc list --- ignore others) Date: Tue, 5 Jan 2010 07:57:44 +0800 [thread overview] Message-ID: <45a44e481001041557t1f87f8d4i959abbbee0c4346@mail.gmail.com> (raw) In-Reply-To: <200912142122.nBELMW7d001243@mustang.cs.nmsu.edu> On Tue, Dec 15, 2009 at 5:22 AM, Rick L. Vinyard Jr. <rvinyard@cs.nmsu.edu> wrote: > Additionally, this device contains a 160x43 monochrome LCD display. > A registered framebuffer device manages this display. The design > of this portion of the driver was based on the design of the > hecubafb driver with deferred framebuffer I/O since there is > no real memory to map. Hi Rick, Interesting work. I recommend CCing linux-fbdev@vger.kernel.org too since it contains a fbdev interface. > +config LOGITECH_G13 > + tristate "Logitech G13 gameboard support" > + depends on HID_LOGITECH > + depends on FB > + select FB_SYS_FILLRECT > + select FB_SYS_COPYAREA > + select FB_SYS_IMAGEBLIT > + select FB_SYS_FOPS Does this need to select FB_DEFERRED_IO? > --- /dev/null > +++ b/drivers/hid/hid-g13-logo.xbm > @@ -0,0 +1,75 @@ > +#define g13_lcd_width 160 > +#define g13_lcd_height 43 > +static unsigned char g13_lcd_bits[] = { > + 0x00, 0x00, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, Was there a reason for having a new logo file? If so, you might want to put it in the comments and also put it together with the standard kernel logo. > +/* 160*43 rounded to nearest whole byte which is 160*48 since bytes are > + vertical the y component must be a multiple of 8 */ > +#define G13FB_SIZE (160*48/8) Minor nit, I think there is a macro for this, DIV_ROUND_UP, or maybe this could be better done in the function that uses this value. > +static ssize_t g13_set_mled(struct hid_device *hdev, unsigned mled); Does this need to be here or can the code be reordered? > +static struct fb_var_screeninfo g13fb_var = { > + .xres = G13FB_WIDTH, > + .yres = G13FB_HEIGHT, > + .xres_virtual = G13FB_WIDTH, > + .yres_virtual = G13FB_HEIGHT, > + .bits_per_pixel = 1, > + .nonstd = 1, > +}; I think the nonstd is a bug. Yes, hecubafb and metronomefb seem to have the same bug as well. nonstd is used if you want something like FB_NONSTD_HAM or FB_NONSTD_REV_PIX_IN_B, which I don't think is what you want. Hope this helps. Best regards, jaya -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2010-01-05 0:05 UTC|newest] Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top 2009-12-14 21:22 [PATCH] Logitech G13 driver (fixed cc list --- ignore others) Rick L. Vinyard Jr. 2009-12-14 21:26 ` Rick L. Vinyard, Jr. 2009-12-14 22:02 ` Felipe Balbi 2009-12-14 22:48 ` Rick L. Vinyard, Jr. 2009-12-16 10:34 ` Pavel Machek 2009-12-16 14:08 ` Jiri Kosina 2010-01-04 22:23 ` Rick L. Vinyard, Jr. 2010-01-04 22:48 ` Pavel Machek 2010-01-04 22:48 ` Pavel Machek 2010-01-05 0:14 ` Jaya Kumar 2010-01-14 21:08 ` Miguel Ojeda 2010-01-14 21:08 ` Miguel Ojeda 2010-01-14 21:48 ` Rick L. Vinyard, Jr. 2010-01-14 22:34 ` Miguel Ojeda 2010-01-14 22:34 ` Miguel Ojeda 2010-01-14 23:03 ` Rick L. Vinyard, Jr. 2010-01-14 23:03 ` Rick L. Vinyard, Jr. 2010-01-14 23:34 ` Miguel Ojeda 2010-01-14 23:34 ` Miguel Ojeda 2010-01-05 9:52 ` Jiri Kosina 2010-01-04 23:57 ` Jaya Kumar [this message] 2010-01-04 23:57 ` Jaya Kumar 2010-01-07 15:59 ` Rick L. Vinyard, Jr. 2010-01-07 15:59 ` Rick L. Vinyard, Jr. 2010-01-08 14:21 ` Giacomo A. Catenazzi 2010-01-08 14:21 ` Giacomo A. Catenazzi 2010-01-08 16:45 ` Rick L. Vinyard, Jr. 2010-01-08 16:45 ` Rick L. Vinyard, Jr. 2010-01-08 16:45 ` Rick L. Vinyard, Jr.
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=45a44e481001041557t1f87f8d4i959abbbee0c4346@mail.gmail.com \ --to=jayakumar.lkml@gmail.com \ --cc=akpm@linux-foundation.org \ --cc=jkosina@suse.cz \ --cc=krzysztof.h1@wp.pl \ --cc=linux-input@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-usb@vger.kernel.org \ --cc=oliver@neukum.org \ --cc=rvinyard@cs.nmsu.edu \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.