[PATCHv16 5/7] fbmon: add of_videomode helpers

[Rob Clark]

On Mon, Jan 7, 2013 at 2:46 AM, Mohammed, Afzal <afzal at ti.com> wrote:
> Hi Steffen,
>
> On Mon, Jan 07, 2013 at 13:36:48, Steffen Trumtrar wrote:
>> On Mon, Jan 07, 2013 at 06:10:13AM +0000, Mohammed, Afzal wrote:
>
>> > This breaks DaVinci (da8xx_omapl_defconfig), following change was
>> > required to get it build if OF_VIDEOMODE or/and FB_MODE_HELPERS
>> > is not defined. There may be better solutions, following was the
>> > one that was used by me to test this series.
>
>> 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.
>
> You are right, me idiot, error will happen only upon try to make use of
> of_get_fb_videomode() (defined in this patch) in the da8xx-fb driver
> (with da8xx_omapl_defconfig), to be exact upon adding,
>
> "video: da8xx-fb: obtain fb_videomode info from dt" of my patch series.
>
> 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.

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):

https://github.com/robclark/kernel-omap4/commit/e2aef5f281348afaaaeaa132699efc2831aa8384

BR,
-R

>
> And testing was done over v3.8-rc2.
>
>> > > +#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
>> >
>> > As _OF_VIDEOMODE is a bool type CONFIG, isn't,
>> >
>> > #ifdef CONFIG_OF_VIDEOMODE
>> >
>> > sufficient ?
>> >
>>
>> Yes, that is right. But I think IS_ENABLED is the preferred way to do it, isn't it?
>
> Now I realize it is.
>
> Regards
> Afzal