On Mon, 2007-05-07 at 16:11 +0200, Nicolas Ferre wrote:
> From: Nicolas Ferre <[EMAIL PROTECTED]>
>
> Adds a framebuffer driver to ATMEL AT91SAM9x and AT32
> aka AVR32 platforms. Those chips share quite the same
> IP and this code is suitable for both architectures.
>
> Signed-off-by: Nicolas Ferre <[EMAIL PROTECTED]>
> ---
>
> +#if defined(CONFIG_ARCH_AT91)
> +#define ATMEL_LCDFB_FBINFO_DEFAULT FBINFO_DEFAULT
> +
> +static inline void atmel_lcdfb_update_dma2d(struct atmel_lcdfb_info *sinfo,
> + struct fb_var_screeninfo *var)
> +{
> +
> +}
Or
#define atmel_lcdfb_update(...) do {} while (0)
?
> +
> +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.
> +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).
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.
> +
> +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.
> +{
> + chan &= 0xffff;
> + chan >>= 16 - bf->length;
> + return chan << bf->offset;
> +}
> +
> +
> +
> +
> +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?
> + info->var.activate |= FB_ACTIVATE_FORCE | FB_ACTIVATE_NOW;
> +
> + dev_info(info->device,
> + "%luKiB frame buffer at %08lx (mapped at %p)\n",
> + (unsigned long)info->fix.smem_len / 1024,
> + (unsigned long)info->fix.smem_start,
> + info->screen_base);
> +
> + /* Allocate colormap */
> + ret = fb_alloc_cmap(&info->cmap, 256, 0);
> + if (ret < 0)
> + dev_err(info->device, "Alloc color map failed\n");
> +
> + return ret;
> +}
> +
> +
> +
> +static int __init atmel_lcdfb_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct fb_info *info;
> + struct atmel_lcdfb_info *sinfo;
> + struct atmel_lcdfb_info *pdata_sinfo;
> + struct resource *regs = NULL;
> + struct resource *map = NULL;
> + int ret;
> +
> + dev_dbg(dev, "%s BEGIN\n", __func__);
> +
> + ret = -ENOMEM;
> + info = framebuffer_alloc(sizeof(struct atmel_lcdfb_info), dev);
> + if (!info) {
> + dev_err(dev, "cannot allocate memory\n");
> + goto out;
> + }
> +
> + sinfo = info->par;
> +
> + 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?
> +
> +free_cmap:
> + fb_dealloc_cmap(&info->cmap);
> +unregister_irqs:
> + free_irq(sinfo->irq_base, info);
> +unmap_mmio:
> + iounmap(sinfo->mmio);
> +release_mem:
> + release_mem_region(info->fix.mmio_start, info->fix.mmio_len);
> +free_fb:
> + if (map) {
> + iounmap(info->screen_base);
> + } else {
> + atmel_lcdfb_free_video_memory(sinfo);
> + }
> +
> +release_intmem:
> + if (map) {
> + release_mem_region(info->fix.smem_start, info->fix.smem_len);
> + }
Unnecessary curly braces
> +
> +static struct platform_driver atmel_lcdfb_driver = {
> + .remove = __exit_p(atmel_lcdfb_remove),
> + .driver = {
> + .name = "atmel_lcdfb",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +static int __init atmel_lcdfb_init(void)
> +{
> + return platform_driver_probe(&atmel_lcdfb_driver, atmel_lcdfb_probe);
> +}
Is this intentional? Why not platform_driver_register()?
> +
> +static void __exit atmel_lcdfb_exit(void)
> +{
> + platform_driver_unregister(&atmel_lcdfb_driver);
> +}
> +
Tony
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
