Re: [PATCH 2/3] clk: let of_clk_get_parent_name() fail for invalid clock-indices

Stephen Boyd
2015-11-24T00:53:27Z

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