On Fri, Sep 11, 2015 at 11:22:04AM +0000, ker...@martin.sperl.org wrote:

> +     bs->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> +     if (bs->irq <= 0) {
> +             dev_err(&pdev->dev, "could not get IRQ: %d\n", bs->irq);
> +             err = bs->irq ? bs->irq : -ENODEV;
> +             goto out_master_put;
> +     }

Why are we using this unusual DT specific API here?  I'd expect to see
just use platform_get_irq() here and this seems to be the only DT
dependency in the driver. 

> +     err = devm_request_irq(&pdev->dev, bs->irq,
> +                            bcm2835aux_spi_interrupt,
> +                            IRQF_SHARED,
> +                            dev_name(&pdev->dev), master);
> +     if (err) {
> +             dev_err(&pdev->dev, "could not request IRQ: %d\n", err);
> +             goto out_clk_disable;
> +     }
> +
> +     /* reset SPI-HW block */
> +     bcm2835aux_spi_reset_hw(bs);

I'd expect to request the IRQ after we reset the hardware in order to
ensure that the hardware is in a known good state 

> +
> +     err = devm_spi_register_master(&pdev->dev, master);
> +     if (err) {
> +             dev_err(&pdev->dev, "could not register SPI master: %d\n", err);
> +             goto out_clk_disable;
> +     }
> +
> +     return 0;
> +
> +out_clk_disable:
> +     clk_disable_unprepare(bs->clk);
> +out_master_put:
> +     spi_master_put(master);
> +     return err;
> +}
> +
> +static int bcm2835aux_spi_remove(struct platform_device *pdev)
> +{
> +     struct spi_master *master = platform_get_drvdata(pdev);
> +     struct bcm2835aux_spi *bs = spi_master_get_devdata(master);
> +
> +     bcm2835aux_spi_reset_hw(bs);
> +
> +     /* disable the HW block by releasing the clock */
> +     clk_disable_unprepare(bs->clk);
> +
> +     return 0;
> +}
> +
> +static const struct of_device_id bcm2835aux_spi_match[] = {
> +     { .compatible = "brcm,bcm2835-aux-spi", },
> +     {}
> +};
> +MODULE_DEVICE_TABLE(of, bcm2835aux_spi_match);
> +
> +static struct platform_driver bcm2835aux_spi_driver = {
> +     .driver         = {
> +             .name           = "spi-bcm2835aux",
> +             .of_match_table = bcm2835aux_spi_match,
> +     },
> +     .probe          = bcm2835aux_spi_probe,
> +     .remove         = bcm2835aux_spi_remove,
> +};
> +module_platform_driver(bcm2835aux_spi_driver);
> +
> +MODULE_DESCRIPTION("SPI controller driver for Broadcom BCM2835 aux");
> +MODULE_AUTHOR("Martin Sperl <ker...@martin.sperl.org>");
> +MODULE_LICENSE("GPL v2");
> --
> 1.7.10.4
> 
> 

Attachment: signature.asc
Description: Digital signature

Reply via email to