linux-kernel - Re: [Linux-fbdev-devel] [PATCH] atmel_lcdfb: AT91/AT32 LCDController framebuffer driver

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]

Message-ID: <4641E1E9.5060800@rfo.atmel.com>
Date:	Wed, 09 May 2007 16:59:53 +0200
From:	Nicolas Ferre <nicolas.ferre@....atmel.com>
To:	"Antonino A. Daplas" <adaplas@...il.com>
CC:	linux-fbdev-devel@...ts.sourceforge.net,
	Andrew Victor <andrew@...people.com>,
	Linux Kernel list <linux-kernel@...r.kernel.org>,
	Haavard Skinnemoen <hskinnemoen@...el.com>,
	Patrice Vilchez <patrice.vilchez@....atmel.com>
Subject: Re: [Linux-fbdev-devel] [PATCH] atmel_lcdfb: AT91/AT32 LCDController framebuffer driver

Antonino A. Daplas :
> On Mon, 2007-05-07 at 16:11 +0200, Nicolas Ferre wrote:
>> From: Nicolas Ferre <nicolas.ferre@....atmel.com>

>> +
>> +static struct fb_fix_screeninfo atmel_lcdfb_fix __initdata = {
>> +	.type		= FB_TYPE_PACKED_PIXELS,
>> +	.visual		= FB_VISUAL_TRUECOLOR,
>> +	.xpanstep	= 0,
>> +	.ypanstep	= 0,
>> +	.ywrapstep	= 0,
>> +	.accel		= FB_ACCEL_NONE,
>> +};
>> +
> 
> This driver has fb_pan_display() which I assume works. However, you also
> need to set ypanstep and/or xpanstep and/or ywrapstep to a nonzero
> value, depending on the hardware/drive capability.

True, this function is here because one of our products can do pan display.
The technique is not used for the moment but will be in a future version
of this driver.

>> +static u32 pseudo_palette[16] = {
>> +	0x000000,
>> +	0xaa0000,
>> +	0x00aa00,
>> +	0xaa5500,
>> +	0x0000aa,
>> +	0xaa00aa,
>> +	0x00aaaa,
>> +	0xaaaaaa,
>> +	0x555555,
>> +	0xff5555,
>> +	0x55ff55,
>> +	0xffff55,
>> +	0x5555ff,
>> +	0xff55ff,
>> +	0x55ffff,
>> +	0xffffff
>> +};
>> +
> 
> Do you really need to pre-fill pseudo_palette[]? The contents are going
> to be overwritten by the console anyway (The pseudo_palette is for
> fbcon's use only).

Ok, I have tested it with fbcon and no pre-fill : colors seem ok.

> You can also include pseudo_palette[] as part of struct
> atmel_lcdfb_info, then in your probe routine:
> 
> info->pseudo_palette = par->pseudo_palette;
> 
> to reduce the size of the kernel image. 

Done.

>> +static inline u_int chan_to_field(u_int chan, const struct fb_bitfield *bf)
> 
> Might as well change u_int to u32 or unsigned int, for consistency.

Ok, changed to unsigned int.

>> +static int __init atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo)
>> +{
>> +	struct fb_info *info = sinfo->info;
>> +	int ret = 0;
>> +
>> +	memset(info->screen_base, 0, info->fix.smem_len);
> 
> memset_io?

ok, modified.


>> +	if (dev->platform_data) {
>> +		pdata_sinfo = (struct atmel_lcdfb_info *)dev->platform_data;
>> +		sinfo->default_bpp = pdata_sinfo->default_bpp;
>> +		sinfo->default_dmacon = pdata_sinfo->default_dmacon;
>> +		sinfo->default_lcdcon2 = pdata_sinfo->default_lcdcon2;
>> +		sinfo->default_monspecs = pdata_sinfo->default_monspecs;
>> +		sinfo->atmel_lcdfb_power_control = pdata_sinfo->atmel_lcdfb_power_control;
>> +		sinfo->guard_time = pdata_sinfo->guard_time;
>> +	} else {
>> +		dev_err(dev, "cannot get default configuration\n");
>> +		goto out;
> 
> Wrong goto? Should be goto free_info?

Right. modified to "goto free_info".
 
>> +release_intmem:
>> +	if (map) {
>> +		release_mem_region(info->fix.smem_start, info->fix.smem_len);
>> +	}
> 
> Unnecessary curly braces

I like curly braces... Anyway, removed.

Thanks you for your comments.

I resend a modified driver now in the following email.

Regards,
-- 
Nicolas Ferre


-
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/