From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Subject: Re: [RFC][PATCHv6+++ 01/13] of: introduce of_property_for_earch_phandle_with_args() Date: Tue, 03 Dec 2013 13:14:18 -0700 Message-ID: <529E3B9A.50207@wwwdotorg.org> References: <20131128.145818.1345100874304396564.hdoyu@nvidia.com><20131129.134625.431945240074254704.hdoyu@nvidia.com><529B8739.60701@wwwdotorg.org> <20131202.130220.1404999403649937134.hdoyu@nvidia.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20131202.130220.1404999403649937134.hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Hiroshi Doyu Cc: "mark.rutland-5wv7dgnIgG8@public.gmane.org" , "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org" , Stephen Warren , "will.deacon-5wv7dgnIgG8@public.gmane.org" , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org" , "thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" , "robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" , "galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org" , "grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: linux-tegra@vger.kernel.org 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?