Hi Geert,

> >> In .recalc_rate of struct clk_ops, it is desirable to return 0 if an
> >> error occurs, but -EINVAL is returned. This patch fixes it.
> >>
> >> Fixes: 5b1defde7054 ("clk: renesas: cpg-mssr: Extract common R-Car Gen3 
> >> support code")
> >> Signed-off-by: Takeshi Kihara <takeshi.kihara...@renesas.com>
> >> Signed-off-by: Yoshihiro Kaneko <ykaneko0...@gmail.com>
> >
> > Reviewed-by: Wolfram Sang <wsa+rene...@sang-engineering.com>
> Why is it desirable to return 0 if an error occurs? Because the return type is
> unsigned long?

Yes, I'd think so. Instead of an arbitrary high, kind-of-random, number, just
return 0 which also indicates that something unexpected happened? Also, all
other drivers return zero in an error case (did some quick coccinelle
grepping before).

> Is this routine allowed to fail? I don't see any handling of zero by
> its callers.

From clk-provider.h:

 * @recalc_rate Recalculate the rate of this clock, by querying hardware. The
 *              parent rate is an input parameter.  It is up to the caller to
 *              ensure that the prepare_mutex is held across this call.
 *              Returns the calculated rate.  Optional, but recommended - if
 *              this op is not set then clock rate will be initialized to 0.

What would be the benefit of keeping -EINVAL in an unsigned long?



Attachment: signature.asc
Description: PGP signature

Reply via email to