On Sat, Oct 10, 2009 at 16:38, Mike Frysinger wrote: > On Fri, Oct 9, 2009 at 22:24, Barry Song wrote: >> On Sat, Oct 10, 2009 at 12:22 AM, Dmitry Torokhov wrote: >>> On Fri, Oct 09, 2009 at 06:11:45AM -0400, Mike Frysinger wrote: >>>> On Fri, Oct 9, 2009 at 05:18, Mike Frysinger <vapier.adi@xxxxxxxxx> wrote: >>>> > we could have three modules here: ad714x, ad714x-i2c, ad714x-spi >>>> >>>> are you ok with this approach Dmitry ? this way people can >>>> load/unload a specific bus without affecting the core or the other >>>> bus, and errors with loading one bus type wouldnt affect loading of >>>> the other. >>> >>> Yes, I think this is the most clean way. >> >> That will decrease the cohesion and increase coupling. It's tedious >> for one driver with single feature to have three modules. To fulfill >> that, ad714x need to expose data structures and export symbols to >> ad714x-i2c and ad714x-spi. But those data structures and symbols are >> more like local stuff. And I2C and SPI are just bottom level >> communication ways, they should be the bottom level of ad714x, but >> after splitting to 3 modules, it seems they become the top level of >> ad714x. > > there should already be a clear split between the bus and the driver > core, otherwise crap is being duplicated between the two interfaces. > i just tested splitting things and each bus code needs to know very > few things: > - probe/remove for initialization > - enable/disable for pm > > the only downside i see here is that we have to remove > __devinit/__devexit markings from the common probe/remove functions > and those weigh a hefty 2406/218 bytes a piece. i spent time packing common stuff in the probe code and cut it down by ~1k, so i'm fine with this little bit of overhead. Barry: can you test out the latest ad714x driver and make sure i didnt screw anything up ? i dont have any hardware to actually test these guys out ... -mike -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
