[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <529E3B9A.50207@wwwdotorg.org>
Date: Tue, 03 Dec 2013 13:14:18 -0700
From: Stephen Warren <swarren@...dotorg.org>
To: Hiroshi Doyu <hdoyu@...dia.com>
CC: "grant.likely@...aro.org" <grant.likely@...aro.org>,
"thierry.reding@...il.com" <thierry.reding@...il.com>,
"robherring2@...il.com" <robherring2@...il.com>,
"joro@...tes.org" <joro@...tes.org>,
Stephen Warren <swarren@...dia.com>,
"will.deacon@....com" <will.deacon@....com>,
"mark.rutland@....com" <mark.rutland@....com>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"lorenzo.pieralisi@....com" <lorenzo.pieralisi@....com>,
"galak@...eaurora.org" <galak@...eaurora.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC][PATCHv6+++ 01/13] of: introduce of_property_for_earch_phandle_with_args()
On 12/02/2013 04:02 AM, Hiroshi Doyu wrote:
...
> Iterating over a property containing a list of phandles with arguments
> is a common operation for device drivers. This patch adds a new
> of_property_for_each_phandle_with_args() macro to make the iteration
> simpler.
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> +const __be32 *of_phandle_iter_init(const struct device_node *np,
> + const char *list_name,
> + const __be32 **end)
> +{
> + size_t bytes;
> + const __be32 *cur;
> +
> + cur = of_get_property(np, list_name, &bytes);
> + if (bytes)
> + *end = cur + bytes / sizeof(*cur);
> +
> + return cur;
> +}
"bytes" doesn't seem like the correct thing to check in that if
statement. The property might exist but be zero length. Perhaps if (cur)
would be better, or just initializing *end in all cases (the value won't
be used if (!cur), so setting *end to a bogus value in that case won't
matter).
> +const __be32 *of_phandle_iter_next(const char *cells_name, int cell_count,
> + const __be32 *cur, const __be32 *end,
> + struct of_phandle_args *out_args)
...
> + if (!cur)
> + return NULL;
> +
> + if (end - cur <= 0)
> + return NULL;
Related, don't you want if (cur >= end) there; just compare the pointers
directly rather than explicitly calculating and testing the difference?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists