Re: [PATCH v5] video: add OpenCores VGA/LCD framebuffer driver

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 1715 bytes --]

On 2014-01-10 22:13, Stefan Kristiansson wrote:
> This adds support for the VGA/LCD core available from OpenCores:
> http://opencores.org/project,vga_lcd
> 
> The driver have been tested together with both OpenRISC and
> ARM (socfpga) processors.
> 
> Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>

> +/*
> + * Init and exit routines
> + */
> +static int __init ocfb_init(void)
> +{
> +#ifndef MODULE
> +	char *option = NULL;
> +
> +	if (fb_get_options("ocfb", &option))
> +		return -ENODEV;
> +	ocfb_setup(option);
> +#endif
> +	return platform_driver_register(&ocfb_driver);
> +}

I see this is how fb_get_options is used elsewhere also, but shouldn't
fb_get_options be called with a name that's somehow device specific? I
haven't used it in omapfb, so maybe I'm missing how it is supposed to
work, but if I'm not mistaken, if you have two ocfb devices on your
board, there's no way to specify individual modes for them. Even the
Documentation/fb/modedb.txt gives an example of a "VGA-1" which sounds
to me that it has been designed to be used with some kind of device id.

Although even if the above code handled the different devices, when
loading this as a module would still not work right as that code is not
called at all in the module case. Ah, well. I guess this is legacy
stuff, and it's just the way it works.

The subject says this is a VGA/LCD driver. Usually with LCD, the LCD
video timings are passed via device tree or platform data, as there's
just one possible set of timings for a board. Is that something that
you've thought about, or is the user always supposed to give the timings
explicitly via kernel cmdline?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]