linux-kernel - Re: [PATCH v5] video: add OpenCores VGA/LCD 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: <20140113102006.GA30877@chokladfabriken.org>
Date:	Mon, 13 Jan 2014 12:20:07 +0200
From:	Stefan Kristiansson <stefan.kristiansson@...nalahti.fi>
To:	Tomi Valkeinen <tomi.valkeinen@...com>
Cc:	linux-kernel@...r.kernel.org, linux-fbdev@...r.kernel.org,
	plagnioj@...osoft.com
Subject: Re: [PATCH v5] video: add OpenCores VGA/LCD framebuffer driver

On Mon, Jan 13, 2014 at 10:21:16AM +0200, Tomi Valkeinen wrote:
> 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@...nalahti.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.
> 

Yes, I can't really figure out how that would be handled neither.

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

The VGA/LCD in the subject comes from the name of the core,
the core itself presents a "vga-type" interface, but it can basically
be hooked up to any type of display (with a bit of glue-logic in some cases).

That said - the reason why I went for the mode_options solution in the
first place, is that when I initially wrote this driver
(>2 years ago, time flies...) the display-timing device-tree bindings weren't
invented yet, so it seemed like the most viable option without coming up
with custom device-tree bindings.

It's completely possible that this design choice should be revised now,
and I wouldn't mind converting this driver to make use of the display-timing
dt properties if you think that's a good idea?

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