On Tue, May 20, 2014 at 01:53:42PM +0200, Marek Szyprowski wrote:
> + hub->clk = devm_clk_get(dev, "refclk");
> + if (!IS_ERR(hub->clk)) {This won't handle deferred probe - the driver should error out if it gets -EPROBE_DEFER since it means the clock exists and might be provided later on. > + unsigned long rate; > + > + clk_prepare_enable(hub->clk); > + rate = clk_get_rate(hub->clk); No error checking here. > + > + if (rate == 38400000 || rate == 26000000 || > + rate == 19200000 || rate == 12000000) > + hub->secondary_ref_clk = 0; > + else if (rate == 24000000 || rate == 27000000 || > + rate == 25000000 || rate == 50000000) > + hub->secondary_ref_clk = 1; > + else > + dev_err(dev, > + "unsupported reference clock rate > (%d)\n", > + rate); This looks like a switch statement. Should the driver not try to set the clock to a supported rate if it's not already at one rather than error out - it seems like a more constructive thing to do?
signature.asc
Description: Digital signature
