Messages in this thread | | | Subject | Re: [PATCH v2] MFD: PCAP core driver. | From | Daniel Ribeiro <> | Date | Tue, 26 May 2009 02:10:35 -0300 |
| |
Hi Philipp, Thanks for the review! :)
Em Seg, 2009-05-25 às 21:55 +0200, pHilipp Zabel escreveu: > > +static int pcap_add_subdev(struct spi_device *spi, struct pcap_subdev *subdev) > > +{ > > + struct platform_device *pdev; > > + > > + pdev = platform_device_alloc(subdev->name, subdev->id); > > + pdev->dev.parent = &spi->dev; > > + pdev->dev.platform_data = subdev->platform_data; > > + > > + return platform_device_add(pdev); > > +} > > This pcap_subdev, pcap_remove/add_subdev business looks a bit like > you're duplicating the MFD core just because it doesn't handle > resource-less mfd_cells. I wonder whether it'd be better to fix that > and then use > mfd_cell, mfd_add/remove_devices instead?
Due to the lack of comments on this driver, i started grepping LKML for previous MFD reviews. Now the driver allows multiple instances and no longer keeps pcap_chip as a global(as per Samuel comments on U300/AB3100 driver), result is that resource-less mfd_cells are no longer the only issue so I can use the mfd_ API: Now I also need a way to pass a pcap_chip pointer to the subdevs.
> > + set_irq_type(pdata->irq, IRQ_TYPE_EDGE_RISING); > > + set_irq_chained_handler(pdata->irq, pcap_irq_handler); > > + set_irq_wake(pdata->irq, 1); > > Looking at the other MFD drivers that register a chained handler, > providing this IRQ via a resource seems to be the default.
Ok, will do.
I will send a new version of the patch tomorrow, after i finish converting all the subdevices to the new code.
Thanks!
-- Daniel Ribeiro
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |