On Sat, Nov 30, 2013 at 12:42:06PM +0100, Jonas Gorski wrote:
> Add a driver for the High Speed SPI controller found on newer BCM63XX SoCs.
>
> It does feature some new modes like 3-wire or dual spi, but neither of it
> is currently implemented.
This is basically good so I've applied it but there are a few minor
things that could use improving, please send incremental patches for
these.
> + clk = clk_get(dev, "hsspi");
Use devm_clk_get().
> + clk_prepare_enable(clk);
You should check the return value here.
> + /* register and we are done */
> + ret = spi_register_master(master);
> + if (ret)
> + goto out_put_master;
devm_spi_register_master().
> +#ifdef CONFIG_PM
This should be CONFIG_PM_SLEEP.
> +static int bcm63xx_hsspi_suspend(struct device *dev)
> +{
> + struct spi_master *master = dev_get_drvdata(dev);
> + struct bcm63xx_hsspi *bs = spi_master_get_devdata(master);
> +
> + spi_master_suspend(master);
> + clk_disable(bs->clk);
This ought to be disable_unprepare(). It would also be better to move
this to runtime PM (you can set auto_runtime_pm to have the core manage
the enable and disable for you) since that will save a bit of power.
> +static int bcm63xx_hsspi_resume(struct device *dev)
> +{
> + struct spi_master *master = dev_get_drvdata(dev);
> + struct bcm63xx_hsspi *bs = spi_master_get_devdata(master);
> +
> + clk_enable(bs->clk);
> + spi_master_resume(master);
Similar comments here, plus you should be checking errors.
Attachment:
signature.asc
Description: Digital signature
