On Tue, Jun 18, 2013 at 03:30:28PM -0400, Scott Jiang wrote: > New spi controller is integrated into Blackfin 6xx processor. > Comparing to bf5xx spi controller, we support 32 bits word size > and independent receive and transmit DMA channels now. > Also mode 0 and 2 (CPHA = 0) can get fully supported > becasue cs line may be controlled by the software.
A few nits here but mostly looking good.
> + switch (spi->bits_per_word) {
> + case 8:
> + case 16:
> + case 32:
> + break;
> + default:
> + dev_err(&spi->dev, "%d bits_per_word is not supported\n",
> + spi->bits_per_word);
> + return -EINVAL;
> + }
Just set bits_per_word_mask in the master struct and let the core do
this for you.
> + if (chip->cs < MAX_CTRL_CS) {
> + peripheral_free(ssel[spi->master->bus_num]
> + [chip->cs-1]);
> + bfin_spi_cs_disable(drv_data, chip);
> + } else
> + gpio_free(chip->cs_gpio);
Braces on both sides of the if.
> + drv_data->regs = devm_ioremap_resource(dev, mem);
> + if (IS_ERR(drv_data->regs)) {
> + dev_err(dev, "can not map register memory\n");
> + ret = PTR_ERR(drv_data->regs);
> + goto err_put_master;
> + }
devm_ioremap_resource() will display an error message for you so you
don't need to bother, though in any case the return code should
generally be displayed.
> +#ifdef CONFIG_PM
> + .pm = &bfin_spi_pm_ops,
> +#endif
Use SET_SYSTEM_SLEEP_PM_OPS().
> +static int __init bfin_spi_init(void)
> +{
> + return platform_driver_probe(&bfin_spi_driver, bfin_spi_probe);
> +}
> +subsys_initcall(bfin_spi_init);
Should really use module_platform_driver() on modern systems, the
deferred probe infrastructure should remove the need for faffing around
with init order.
> +MODULE_DESCRIPTION("Analog Devices SPI3 controller driver");
Why not call the driver something like bfin_spi3 then?
signature.asc
Description: Digital signature
_______________________________________________ Uclinux-dist-devel mailing list Uclinux-dist-devel@blackfin.uclinux.org https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel
