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.df@xxxxxxxxxxx>
> >> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@xxxxxxxxx>
> >
> > Reviewed-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
>
> 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?
Regards,
Wolfram
Attachment:
signature.asc
Description: PGP signature
