On Sat, Nov 24, 2007 at 04:26:13PM +0900, Paul Mundt wrote: > On Fri, Nov 23, 2007 at 07:49:33PM -0500, Jeff Garzik wrote: > > Anton Vorontsov wrote: > > >Here is the PATA Platform driver using OF infrastructure. > > > > > >Mostly it's just a wrapper around a bit modified pata_platform > > >driver. > > > > > >Patches are well split for the easier review: > > > > > >First one factors out platform_device specific bits and modifies > > >pata_platform to be a library-alike driver (with platform_device > > >default binding). > > > > The only issue I have here is that this library-like version has subtle > semantic changes that will break existing drivers. Actually I've tried to keep semantics intact: +static int __devinit pata_platform_probe(struct platform_device *pdev) [...] + /* + * And the IRQ + */ + irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); + if (irq_res) + irq_res->flags = pp_info ? pp_info->irq_flags : 0; [...] So, I'm passing flags from the platform data. Did you overlook these bits, or I'm still changing semantics somewhere else? > irq_flags exists in struct pata_platform_info precisely for the IRQ > resource IRQ flags (as opposed to the IORESOURCE flags, which are what > the IRQ resource flags refer to instead). This change takes that for > granted and just assumes we're going to be using the res->flags, which is > both an invalid assumption, and will utterly break blackfin and others > that depend on it. > > Incidentally, we already have an include/linux/pata_platform.h. If this > is going to be library-like, through the prototypes in there, rather than > splitting them up betewen include/linux and drivers/ata. We don't need > two headers. My intention was: keep "private" declarations in the drivers/ata/ and "public" in the include/linux/ -- to not confuse pata_platform users by __pata_platform_* internal stuff. But okay, I'll merge them. > These patches basically look fine to me otherwise, though it would be > nice if the semantic-changing bits had been abstracted. So as long as the > old irq_flags behaviour is maintained and that irq_res->flags stuff is > ripped out, I'll add my Acked-by as well. Much thanks, -- Anton Vorontsov email: cbou@xxxxxxx backup email: ya-cbou@xxxxxxxxx irc://irc.freenode.net/bd2 - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
