Messages in this thread | | | From | Arnd Bergmann <> | Subject | Re: [PATCH 1/6] ARM: Add basic architecture support for VIA/WonderMedia 85xx SoC's | Date | Thu, 21 Oct 2010 14:01:52 +0200 |
| |
On Thursday 21 October 2010, Alexey Charkov wrote:
> >> + > >> +choice > >> + prompt "LCD panel size" > >> + depends on (FB_VT8500 || FB_WM8505) > >> + default WMT_PANEL_800X480 > >> + > >> +config WMT_PANEL_800X480 > >> + bool "7-inch with 800x480 resolution" > >> + help > >> + These are found in most of the netbooks in generic cases, as > >> + well as in Eken M001 tablets and possibly elsewhere. > >> + > >> +config WMT_PANEL_800X600 > >> + bool "8-inch with 800x600 resolution" > >> + help > >> + These are found in Eken M003 tablets and possibly elsewhere. > >> + > >> +config WMT_PANEL_1024X600 > >> + bool "10-inch with 1024x600 resolution" > >> + help > >> + These are found in Eken M006 tablets and possibly elsewhere. > >> + > >> +endchoice > > > > This should really be a run-time or at least boot-time option. If you > > set the frame buffer size at compile time, I guess you can no longer > > boot on a machine that uses a different size. > > > > It could be, but then I'd have to parse kernel command line at the > map_io stage. Is that fine? If yes, I could rework it to e.g. accept a > default value via Kconfig and allow it to be overriden via a boot > argument.
Parsing complex options in general is not ok, but something simpler probably is.
Having a Kconfig selected default is probably a good idea. The most simple way to select this at boot time would be to have a list of possible resolutions and pass a table index.
Would a __setup() call work for you?
> And due to the fact that the framebuffer size calculation is tied to > panel specification, it will boot in any case. The only problem that > one could encounter would be suboptimal display (for example, > offscreen pixmaps to become actually visible on screen if the panel is > taller > than expected, or some corruption in case it is wider).
Another option might be to have a submenu with the possible resolutions you want to allow and size the frame buffer for the largest of those, but allow overriding the actual one at boot time.
> >> +#include <mach/vt8500.h> > >> +#include "devices.h" > >> + > >> +static struct platform_device *devices[] __initdata = { > >> + &vt8500_device_uart0, > >> +#ifdef CONFIG_FB_VT8500 > >> + &vt8500_device_lcdc, > >> +#endif > >> +#ifdef CONFIG_USB_EHCI_HCD > >> + &vt8500_device_ehci, > >> +#endif > >> +#ifdef CONFIG_FB_WMT_GE_ROPS > >> + &vt8500_device_ge_rops, > >> +#endif > >> +#ifdef CONFIG_HAVE_PWM > >> + &vt8500_device_pwm, > >> +#endif > >> +#ifdef CONFIG_BACKLIGHT_PWM > >> + &vt8500_device_pwmbl, > >> +#endif > >> +#ifdef CONFIG_RTC_DRV_VT8500 > >> + &vt8500_device_rtc, > >> +#endif > >> +}; > > > > This doesn't work if the drivers are built as loadable modules, right? > > I wouldn't even make the definitions of the devices configuration dependent. > > The idea of the device model is that you describe what you have in one > > place and use that information to load the drivers for it. > > > > But with loadable modules those symbols should still be defined as 'm' > or something, shouldn't they? Anyway, I'll drop those conditions, > thanks for pointing out.
If you configure an option as a module you get e.g. CONFIG_USB_EHCI_HCD_MODULE=1, but CONFIG_USB_EHCI_HCD remains unset. It would be possible to check for both of them, but IMHO it's cleaner to just leave the code in unconditionally.
> >> +#ifndef __ASM_ARM_ARCH_IO_H > >> +#define __ASM_ARM_ARCH_IO_H > >> + > >> +#define IO_SPACE_LIMIT 0xffffffff > >> + > >> +#define __io(a) __typesafe_io(a) > >> +#define __mem_pci(a) (a) > >> + > >> +#endif > > > > > > This won't work if you ever want to use the PCI on vt8505 with devices > > that have I/O space mapping. > > > > You need to define IO_SPACE_LIMIT to the size of your I/O space and > > make the __io macro offset the address with the start of that window. > > > > The problem is that there is no documentation available for the PCI > bus in these systems (if it is even implemented there). > Vendor-provided sources do not really clarify it either, which you > have commented about at: > http://groups.google.com/group/vt8500-wm8505-linux-kernel/msg/97bf44bc5ea5d46a? > > With no possibilities to test this (as I don't know any of these > devices to really use PCI with enumeration rather than just static > platform-like definitions as in the vendor's kernel), I just can't > imagine how this could be done any better.
I can have a look again. It shouldn't be hard to do an almost correct implementation based on the source code we have, but I only own a wm8505 based device, so I also have no way of testing and it would very likely have some subtle bugs.
Arnd
| |