linux-kernel - Re: [PATCH 1/3] picolcd: driver for PicoLCD HID device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 25 Feb 2010 12:07:38 +0100 (CET)
From:	Jiri Kosina <jkosina@...e.cz>
To:	Bruno Prémont <bonbons@...ux-vserver.org>
Cc:	linux-input@...r.kernel.org, linux-usb@...r.kernel.org,
	linux-fbdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	"Rick L. Vinyard Jr." <rvinyard@...nmsu.edu>,
	Nicu Pavel <npavel@...ner.com>
Subject: Re: [PATCH 1/3] picolcd: driver for PicoLCD HID device

On Wed, 24 Feb 2010, Bruno Prémont wrote:

> +config HID_PICOLCD
> +	tristate "Minibox PicoLCD (graphic version)"
> +	depends on FB
> +	select FB_DEFERRED_IO
> +	select FB_SYS_FILLRECT
> +	select FB_SYS_COPYAREA
> +	select FB_SYS_IMAGEBLIT
> +	select FB_SYS_FOPS
> +	select BACKLIGHT_LCD_SUPPORT
> +	select BACKLIGHT_CLASS_DEVICE
> +	select LCD_CLASS_DEVICE

I guess you need dependency on USB_HID as well, right?

> --- a/drivers/hid/hid-picolcd.c
> +++ b/drivers/hid/hid-picolcd.c
> @@ -0,0 +1,1075 @@
> +/***************************************************************************
> + *   Copyright (C) 2010 by Bruno Prémont <bonbons@...ux-vserver.org>       *
> + *                                                                         *
> + *   Based on Logitech G13 driver (v0.4)                                   *
> + *     Copyright (C) 2009 by Rick L. Vinyard, Jr. <rvinyard@...nmsu.edu>   *
> + *                                                                         *
> + *   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.        *

I support removing the 'or any later' clause, but I think your version has 
the grammar wrong :)


> +/* Update fb_vbitmap from the screen_base and send changed tiles to device */
> +static void picolcd_fb_update(struct picolcd_data *data)
> +{
> +	int chip, tile;
> +
> +	spin_lock(&data->lock);
> +	if (!(data->status & PICOLCD_READY_FB)) {
> +		spin_unlock(&data->lock);
> +		picolcd_fb_reset(data->hdev, 0);
> +	} else
> +		spin_unlock(&data->lock);

Please put the brackets to the else branch as well.

Also, it'd be great if the framebuffer part would get Ack from someone 
more familiar with framebuffer code than me.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/