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?
Attachment:
signature.asc
Description: Digital signature
