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? Regards, Wolfram
Description: PGP signature