Re: Re[6]: [PATCH] of: Add more stubs for non-OF builds

On Fri, 21 Jun 2013 19:56:29 +0100, Grant Likely [off-list ref] wrote:

On Fri, Jun 21, 2013 at 7:35 PM, Alexander Shiyan [off-list ref] wrote:

quoted

quoted

quoted

quoted

On Fri, Jun 21, 2013 at 7:17 AM, Alexander Shiyan [off-list ref] wrote:

quoted

quoted

On Thu, Jun 20, 2013 at 5:26 PM, Grant Likely [off-list ref] wrote:

quoted

On Thu, 20 Jun 2013 16:49:54 -0700, Bryan Wu [off-list ref] wrote:

quoted

Hi Rob,

Is this patch good for merging?

In Alex's one patch to add device tree supporting for a leds driver,
we got building errors due to miss definitions of some of_xxx api
functions.

It looks obviously to me that we need to fix this in device tree core
instead put #ifdef CONFIG_OF everywhere.

Actually, the reason those things aren't universally defined is to catch
exactly what it caught. The "leds-mc13783: Add devicetree support" patch
interleaves DT and non-DT parsing which isn't generally a good idea. The
DT parsing code should be shuffled off into a separate function and/or
contained with "if (IS_ENABLED(CONFIG_OF)) {}".

Agree, reasonable! I will remove this patch from my tree firstly.

Alex, could you please update your patch with Grant's feedback?

As far I understand you mean only the last part of patch. Where DT support
is introduced. Is not it?

Yes, exactly. I still keep those 2 non-DT related patches in my tree
and just removed that DT supporting patch.

Please update that patch and posted again to linux-leds and DT
maintainers as well.

I am really dont like any #ifdef in the source, but will do if this is only
one way to put this patch into the main tree.

You don't need any #ifdefs. Put all the DT-related calls into a
separate function specifically for decoding the DT information and
wrap the contents with:

    if (IS_ENABLED(CONFIG_OF)) { }

This macro does not help for undefined reference, unfortunately.
So #ifdef and/or stubs for OF-functions is needed in this case anymore.

if (IS_ENABLED(CONFIG_OF))
  some = of_get_next_child(foo, bar);
will produce an error at compile time if OF is not enabled,
because we have not empty function (if !OF) for this.

Check linux-next.

Oops, my apologies, of_get_next_child is still inside the CONFIG_OF
block. Arnd had written a patch to expose some of the OF prototypes, but
that was in of_platform.h, not of.h. Use a #ifdef block around the OF
parsing helper function for now until that is fixed in the header file.

g.