Hi Rob,
On Tue, Jan 08, 2013 at 01:36:50, Rob Clark wrote:
On Mon, Jan 7, 2013 at 2:46 AM, Mohammed, Afzal [off-list ref] wrote:
quoted
On Mon, Jan 07, 2013 at 13:36:48, Steffen Trumtrar wrote:
quoted
quoted
I just did a quick "make da8xx_omapl_defconfig && make" and it builds just fine.
On what version did you apply the series?
At the moment I have the series sitting on 3.7. Didn't try any 3.8-rcx yet.
But fixing this shouldn't be a problem.
quoted
The change as I mentioned or something similar would be required as
any driver that is going to make use of of_get_fb_videomode() would
break if CONFIG_OF_VIDEOMODE or CONFIG_FB_MODE_HELPERS is not defined.
Shouldn't the driver that depends on CONFIG_OF_VIDEOMODE and
CONFIG_FB_MODE_HELPERS, explicitly select them? I don't really see
the point of having the static-inline fallbacks.
But here da8xx-fb driver does not depend on _OF_VIDEOMODE and
_FB_MODE_HELPERS, currently it works as a pure platform driver
for DaVinci SoC's without those CONFIG's. It is only upon
enhancing the driver to make use of of_get_fb_videomode() for
DT support those CONFIG's are being made use of.
As the driver can work w/o these CONFIG's and so as it is not a
dependency for driver on non-DT boot (as in the case of DaVinci),
I disagree in selecting those options always, but rather giving
user an option to select.
And selecting these options always will bring in some amount of code
onto Kernel image w/o any purpose in the case of DaVinci builds.
Another option would be to sprinkle driver with ifdef's to avoid
inline fallbacks, which is not a good thing to do.
Moreover having a static inline fallback is more in line with other
of_*'s.
fwiw, using 'select' is what I was doing for lcd panel support for
lcdc/da8xx drm driver (which was using the of videomode helpers,
albeit a slightly earlier version of the patches):
In your case as it is a new driver & is meant only for DT, that
is fine, but here it is an existing driver that works w/o these.
Regards
Afzal