Messages in this thread | | | Date | Fri, 17 Jun 2016 03:36:57 +0200 | From | "Luis R. Rodriguez" <> | Subject | Re: [PATCH v2 8/8] p54: convert to sysdata API |
| |
On Thu, Jun 16, 2016 at 02:21:02PM -1000, Linus Torvalds wrote: > So what is the advantage of this, since it needs to add more lines of code > than it removes? > > It doesn't seem to be a simplification. Quite the reverse. > > Your diffstat of the whole automated conversion did that too.
A lot of the diff stat of automated changes is space changes which consists of not wanted new lines in one case caused by Coccinelle. To get an idea by comparison one would have to cleanup the output. Typically its on par, sometimes you save, sometimes a bit more code due to the syn case needing a callback. In this case its more given the non-keep cases which need a callback, and also some changes around an #ifdef to make code cleaner.
> Why would we ever want to apply a patch like this?
Well, it depends, in terms of API it helps us extend it without having to do more collateral evolutions. The p54 driver happens to have both sync and async calls, and also has a few "KEEP" cases, the SmPL grammar does not deal with the keep cases -- this change to p54 was just a demo, but what I'd much prefer is to deal with the keep cases by folding the fw into devm to also avoid the free'ing in drivers just as with the non-keep cases. If that's desirable it needs discussion given its a significant change. The grammar still produces more changes than before for the sync cases given a callback is needed, its just that simple.
Reason this could not wait is folks seem to want to keep extending the API, which is another reason for this, do we want to put an end to an unflexible API now or should we wait ?
If we want to wait for devm integration -- that's cool with me, its however better to release often and release early. So the point of this particular series is to show where development is at on the front of a new flexible API that also avoids the usermode helper. I posted this also *now* given I saw the old API being extended further. So another option is to evaluate a merge now, and evolve the devm integration later.
Alternatively, since its only 2 drivers that explicitly require the usermode helper, another option is to just compartamentalize the usermode helper explicit callers with a its own API now and save the others with a strategy devised by the current sysdata API, without requiring any changes at all. This is a significant change though, so still requires discussion. As I see it this is worth considering just because we get the same end result if we transform drivers to sysdata or we just compartamentalize the usermode helper now to the 2 callers.
Ideas, patches, feedback, rants welcomed.
Luis
| |