On 07/23/2013 02:07 PM, Gerhard Sittig wrote:
[...]

>>> +	return freq_calc;
>>> +
>>> +err_invalid:
>>> +	dev_err(&ofdev->dev, "invalid clock source specification\n");
>>> +	return 0;
>>
>> return 0 in case of error? Please add a comment what this 0 means here.
> 
> The .get_clock() callback is supposed to return the resulting
> rate after the clock source was determined and the clock subtree
> was setup.  It wasn't (explicitly) documented before (in the
> non-common-clock case), so I did not bother to comment it in the
> parallel common-clock case.

Yes, but returning 0 in case of error should be documented, as it's very
uncommon.

> But it's true that returning zero in the error case may be
> unexpected, and I will add a comment in v4 (in all the
> .get_clock() implementations).

I think a one liner stating that the return value is the clock rate and
"0" indicates is an invalid clock rate, this meaning an error here, is
enough.

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |