From: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
To: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Subject: Re: [PATCH] of: Turn of_match_node into a static inline when CONFIG_OF isn't set
Date: Mon, 17 Feb 2014 20:50:34 +0100 [thread overview]
Message-ID: <1740750.ctOzzXhXsT@avalon> (raw)
In-Reply-To: <20140217181829.B5F80C40372-WNowdnHR2B42iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
Hi Grant,
On Monday 17 February 2014 18:18:29 Grant Likely wrote:
> On Tue, 11 Feb 2014 13:36:51 +0100, Laurent Pinchart wrote:
> > when CONFIG_OF is disabled of_match_node is defined as a macro that
> > evaluates to NULL. This breaks compilation of drivers that dereference
> > the function's return value directly. Fix it by turning the macro into a
> > static inline function that returns NULL.
>
> Is it not more problematic that code is dereferencing the return value
> directly *without* checking to see if it is NULL first? I suppose it
> will cause a runtime oops on all platforms except no-mmu, but still.
> Compile time catches are better.
There should be no risk of oops if the of_match_ptr() call is guarded by a
compiler (as opposed to preprocessor) IS_ENABLED(CONFIG_OF) check. Now, if the
check is missing, we'll oops at runtime, but the problem already exists before
this patch.
Anyway I've decided to drop this patch and use an intermediate variable.
> > Signed-off-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> > ---
> >
> > include/linux/of.h | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/of.h b/include/linux/of.h
> > index 70c64ba..719e69f 100644
> > --- a/include/linux/of.h
> > +++ b/include/linux/of.h
> > @@ -560,7 +560,13 @@ static inline const char *of_prop_next_string(struct
> > property *prop,>
> > }
> >
> > #define of_match_ptr(_ptr) NULL
> >
> > -#define of_match_node(_matches, _node) NULL
> > +
> > +static inline const struct of_device_id *of_match_node(
> > + const struct of_device_id *matches, const struct device_node *node)
> > +{
> > + return NULL;
> > +}
> > +
> >
> > #endif /* CONFIG_OF */
> >
> > #if defined(CONFIG_OF) && defined(CONFIG_NUMA)
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Grant Likely <grant.likely@linaro.org>
Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Rob Herring <robh+dt@kernel.org>
Subject: Re: [PATCH] of: Turn of_match_node into a static inline when CONFIG_OF isn't set
Date: Mon, 17 Feb 2014 20:50:34 +0100 [thread overview]
Message-ID: <1740750.ctOzzXhXsT@avalon> (raw)
In-Reply-To: <20140217181829.B5F80C40372@trevor.secretlab.ca>
Hi Grant,
On Monday 17 February 2014 18:18:29 Grant Likely wrote:
> On Tue, 11 Feb 2014 13:36:51 +0100, Laurent Pinchart wrote:
> > when CONFIG_OF is disabled of_match_node is defined as a macro that
> > evaluates to NULL. This breaks compilation of drivers that dereference
> > the function's return value directly. Fix it by turning the macro into a
> > static inline function that returns NULL.
>
> Is it not more problematic that code is dereferencing the return value
> directly *without* checking to see if it is NULL first? I suppose it
> will cause a runtime oops on all platforms except no-mmu, but still.
> Compile time catches are better.
There should be no risk of oops if the of_match_ptr() call is guarded by a
compiler (as opposed to preprocessor) IS_ENABLED(CONFIG_OF) check. Now, if the
check is missing, we'll oops at runtime, but the problem already exists before
this patch.
Anyway I've decided to drop this patch and use an intermediate variable.
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >
> > include/linux/of.h | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/of.h b/include/linux/of.h
> > index 70c64ba..719e69f 100644
> > --- a/include/linux/of.h
> > +++ b/include/linux/of.h
> > @@ -560,7 +560,13 @@ static inline const char *of_prop_next_string(struct
> > property *prop,>
> > }
> >
> > #define of_match_ptr(_ptr) NULL
> >
> > -#define of_match_node(_matches, _node) NULL
> > +
> > +static inline const struct of_device_id *of_match_node(
> > + const struct of_device_id *matches, const struct device_node *node)
> > +{
> > + return NULL;
> > +}
> > +
> >
> > #endif /* CONFIG_OF */
> >
> > #if defined(CONFIG_OF) && defined(CONFIG_NUMA)
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2014-02-17 19:50 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1392122211-11422-1-git-send-email-laurent.pinchart@ ideasonboard.com>
2014-02-11 12:36 ` [PATCH] of: Turn of_match_node into a static inline when CONFIG_OF isn't set Laurent Pinchart
[not found] ` <1392122211-11422-1-git-send-email-laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
2014-02-11 14:41 ` Josh Cartwright
2014-02-11 14:41 ` Josh Cartwright
2014-02-11 14:55 ` Laurent Pinchart
2014-02-11 16:48 ` Josh Cartwright
[not found] ` <20140211164825.GC841-OP5zVEFNDbfdOxZ39nK119BPR1lH4CV8@public.gmane.org>
2014-02-11 17:20 ` Laurent Pinchart
2014-02-11 17:20 ` Laurent Pinchart
2014-02-11 18:08 ` Josh Cartwright
[not found] ` <20140211180845.GG841-OP5zVEFNDbfdOxZ39nK119BPR1lH4CV8@public.gmane.org>
2014-02-11 18:29 ` Geert Uytterhoeven
2014-02-11 18:29 ` Geert Uytterhoeven
2014-02-11 20:06 ` Arnd Bergmann
2014-02-12 21:54 ` Geert Uytterhoeven
[not found] ` <CAMuHMdVOkUMxEkUK70Y=Y126XmqgKMpyqmGRG5us7vZPhP8WNw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-02-13 1:01 ` Josh Cartwright
2014-02-13 1:01 ` Josh Cartwright
[not found] ` <CAMuHMdVkPLe5986hYPApoQr7oJSb6U2P71=nP+9mtvtaskdHXg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-02-11 21:30 ` Rob Herring
2014-02-11 21:30 ` Rob Herring
[not found] ` <CAL_JsqKnsr-9nSjCGvroC0nDK36vQUnwMWj=cPjvUbRymmHFOQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-02-11 21:55 ` Josh Cartwright
2014-02-11 21:55 ` Josh Cartwright
[not found] ` <20140211215519.GJ841-OP5zVEFNDbfdOxZ39nK119BPR1lH4CV8@public.gmane.org>
2014-02-11 23:14 ` Rob Herring
2014-02-11 23:14 ` Rob Herring
[not found] ` <CAL_JsqJdbBuv0TgN7Okrxa4MmEon6a74hv458VFS=8RpcyX5tw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-02-12 2:15 ` Josh Cartwright
2014-02-12 2:15 ` Josh Cartwright
2014-02-11 19:17 ` Laurent Pinchart
2014-02-17 18:19 ` Grant Likely
2014-02-17 18:19 ` Grant Likely
2014-02-17 18:18 ` Grant Likely
[not found] ` <20140217181829.B5F80C40372-WNowdnHR2B42iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2014-02-17 19:50 ` Laurent Pinchart [this message]
2014-02-17 19:50 ` Laurent Pinchart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1740750.ctOzzXhXsT@avalon \
--to=laurent.pinchart-rylnwiuwjnjg/c1bvhzhaw@public.gmane.org \
--cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.