Messages in this thread | | | Date | Mon, 23 Nov 2015 16:53:24 -0800 | From | Stephen Boyd <> | Subject | Re: [PATCH 2/3] clk: let of_clk_get_parent_name() fail for invalid clock-indices |
| |
On 11/22, Masahiro Yamada wrote: > 2015-11-21 2:45 GMT+09:00 Stephen Boyd <sboyd@codeaurora.org>: > > > > What is np pointing at? Something like: > > > > consumer { > > clocks = <&oscillator 0>; > > }; > > > > Which would be invalid DT because oscillator doesn't have an > > output for index 0? > > > You are right. My example was confusing. > > > > oscillator: oscillator { > compatible = "myclocktype"; > #clock-cells = <1>; > clock-indices = <1>, <3>; > clock-output-names = "clka", "clkb"; > }; > > consumer { > compatible = "myclockconsumer"; > clocks = <&oscillator 0>; > }; > > Currently, of_clk_get_parent_name(consumer_np, 0) returns "clks", but > should return NULL; > > I will rephrase the git-log in v2. >
Ok. Thanks.
> > Why can't we leave everything in place and check count == len at > > the end? i.e. > > > > of_property_for_each_u32(clkspec.np, "clock-indices", prop, vp, pv) { > > if (index == pv) { > > index = count; > > break; > > } > > count++; > > } > > > > if (count == of_property_count_u32_elems(clkspec.np, "clock-indices")) > > return NULL > > > > > Of course we can, although we have to mention "clock-indices" twice. > > A good thing for of_get_property() is that we can get both the value > and the length > at the same time. >
Ok. Well if we don't want to count them again, perhaps a goto jump over an unconditional return NULL would be better?
of_property_for_each_u32(clkspec.np, "clock-indices", prop, vp, pv) { if (index == pv) { index = count; goto found; } count++; }
return NULL; found:
Or since the macro for of_property_for_each_u32() tests the vp poitner for NULL, we can check that pointer too...
of_property_for_each_u32(clkspec.np, "clock-indices", prop, vp, pv) { if (index == pv) { index = count; break; } count++; }
/* We didn't find anything */ if (!vp) return NULL;
I guess I prefer the latter approach here.
-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
| |