Re: [PATCH spi-next] spi: add Broadcom BCM63xx SPI controller driver

[Florian Fainelli <florian@openwrt.org>]

Hi  Shubhrajyoti

Le mardi 22 novembre 2011 09:26:07, Shubhrajyoti Datta a écrit :
> Hi Florian,
> 
> On Tue, Nov 22, 2011 at 1:46 AM, Florian Fainelli <florian@openwrt.org> 
wrote:
[snip]
> > +       bs->irq = irq;
> > +       bs->clk = clk;
> > +       bs->fifo_size = pdata->fifo_size;
> > +
> > +       ret = request_irq(irq, bcm63xx_spi_interrupt, 0, pdev->name,
> > master); +       if (ret) {
> > +               dev_err(dev, "unable to request irq\n");
> > +               goto out_unmap;
> > +       }
> 
> Could this be a threaded irq ?

I see no reasons why it could not. Is this a requirement for accepting new 
drivers? I see no drivers doing this in Grant's spi/next branch.

> 
> > +
> > +       master->bus_num = pdata->bus_num;
> > +       master->num_chipselect = pdata->num_chipselect;
> > +       master->setup = bcm63xx_spi_setup;
> > +       master->transfer = bcm63xx_transfer;
> > +       bs->speed_hz = pdata->speed_hz;
> > +       bs->stopping = 0;
> > +       bs->tx_io = (u8 *)(bs->regs + bcm63xx_spireg(SPI_MSG_DATA));
> > +       bs->rx_io = (const u8 *)(bs->regs + bcm63xx_spireg(SPI_RX_DATA));
> > +       spin_lock_init(&bs->lock);
> > +
> > +       /* Initialize hardware */
> > +       clk_enable(bs->clk);
> > +       bcm_spi_writeb(bs, SPI_INTR_CLEAR_ALL, SPI_INT_STATUS);
> > +
> > +       /* register and we are done */
> > +       ret = spi_register_master(master);
> > +       if (ret) {
> > +               dev_err(dev, "spi register failed\n");
> > +               goto out_reset_hw;
> > +       }
> > +
> > +       dev_info(dev, "at 0x%08x (irq %d, FIFOs size %d) v%s\n",
> > +                r->start, irq, bs->fifo_size, DRV_VER);
> > +
> > +       return 0;
> > +
> > +out_reset_hw:
> > +       clk_disable(clk);
> > +       free_irq(irq, master);
> > +out_unmap:
> > +       iounmap(bs->regs);
> > +out_put_master:
> > +       spi_master_put(master);
> > +out_free:
> > +       clk_put(clk);
> > +out:
> > +       return ret;
> > +}
> > +
> > +static int __exit bcm63xx_spi_remove(struct platform_device *pdev)
> > +{
> > +       struct spi_master *master = platform_get_drvdata(pdev);
> > +       struct bcm63xx_spi *bs = spi_master_get_devdata(master);
> > +       struct resource *r = platform_get_resource(pdev, IORESOURCE_MEM,
> > 0); +
> > +       /* reset spi block */
> > +       bcm_spi_writeb(bs, 0, SPI_INT_MASK);
> > +       spin_lock(&bs->lock);
> > +       bs->stopping = 1;
> > +
> > +       /* HW shutdown */
> > +       clk_disable(bs->clk);
> > +       clk_put(bs->clk);
> > +
> > +       spin_unlock(&bs->lock);
> > +
> > +       free_irq(bs->irq, master);
> > +       iounmap(bs->regs);
> > +       release_mem_region(r->start, r->end - r->start);
> > +       platform_set_drvdata(pdev, 0);
> > +       spi_unregister_master(master);
> > +
> > +       return 0;
> > +}
> > +
> > +#ifdef CONFIG_PM
> > +static int bcm63xx_spi_suspend(struct platform_device *pdev,
> > pm_message_t mesg) +{
> > +       struct spi_master *master = platform_get_drvdata(pdev);
> > +       struct bcm63xx_spi *bs = spi_master_get_devdata(master);
> > +
> > +       clk_disable(bs->clk);
> > +
> > +       return 0;
> > +}
> > +
> > +static int bcm63xx_spi_resume(struct platform_device *pdev)
> > +{
> > +       struct spi_master *master = platform_get_drvdata(pdev);
> > +       struct bcm63xx_spi *bs = spi_master_get_devdata(master);
> > +
> > +       clk_enable(bs->clk);
> > +
> > +       return 0;
> > +}
> > +#else
> > +#define bcm63xx_spi_suspend    NULL
> > +#define bcm63xx_spi_resume     NULL
> > +#endif
> > +
> > +static struct platform_driver bcm63xx_spi_driver = {
> > +       .driver = {
> > +               .name   = "bcm63xx-spi",
> > +               .owner  = THIS_MODULE,
> > +       },
> > +       .probe          = bcm63xx_spi_probe,
> > +       .remove         = __exit_p(bcm63xx_spi_remove),
> > +       .suspend        = bcm63xx_spi_suspend,
> > +       .resume         = bcm63xx_spi_resume,
> 
> Could we move to dev pm ops?

Sure, I have fixed that in version 2 of the patch.

-- 
Florian