On Tue, Nov 05, 2019 at 12:54:55PM +0000, Vaittinen, Matti wrote: > On Tue, 2019-11-05 at 14:20 +0200, Andy Shevchenko wrote: > > On Tue, Nov 05, 2019 at 12:09:10PM +0200, Matti Vaittinen wrote: > > > The patch series adds definitions for GPIO line directions. > > > > > > For occasional GPIO contributor like me it is always a pain to > > > remember > > > whether 1 or 0 was used for GPIO direction INPUT/OUTPUT. Judging > > > the > > > fact that I removed few comments like: > > > > > > /* Return 0 if output, 1 if input */ > > > /* This means "out" */ > > > return 1; /* input */ > > > return 0; /* output */ > > > > > > it seems at least some others may find it hard to remember too. > > > Adding > > > defines for these values helps us who really have good - but short > > > duration - memory :] > > > > > > Please also see the last patch. It adds warning prints to be > > > emitted > > > from gpiolib if other than defined values are used. This patch can > > > be > > > dropped out if there is a reason for that to still be allowed. > > > > > > This idea comes from RFC series for ROHM BD71828 PMIC and was > > > initially > > > discussed with Linus Walleij here: > > > https://lore.kernel.org/lkml/c06725c3dd34118a324907137758d8b85b3d4043.camel@xxxxxxxxxxxxxxxxx/ > > > but as this has no dependencies to BD71828 work (which probably > > > takes a > > > while) I decided to make it independent series. > > > > > > Patches are compile-tested only. I have no HW to really test them. > > > Thus I'd > > > appreciate carefull review. This work is mainly about converting > > > zeros > > > and ones to the new defines but it wouldn't be first time I get it > > > wrong > > > in one of the patches :) > > > > For all patches I have been Cc'ed to, NAK. > > > > I don't see the advantages now since all known hardware uses the > > single bit to > > describe direction (even for Intel where we have separate gating for > > in and out > > buffers the direction we basically rely on the bit that enables out > > buffer). > > > > So, that said the direction is always 1 bit and basically 0/1 value. > > Yes. At least for now. And this patch didn't change that although it > makes it possible to do so if required. > > > Which one > > is in and which one is out just a matter of an agreement we did. > > This one is exactly the problem patch series was created for. Which one > is IN and which is OUT is indeed a matter of an agreement - but this > agreement should be clearly visible, well defined and same for all. > > It's *annoying* to try finding out whether it was 1 or 0 one should > return from get_direction callback for OUTPUT. This is probably the > reason we have comments like > > return 1; /* input */ > > there. > > As this is global agreement for all GPIO framework users - not > something that can be agreed per driver basis - we should have GPIO > framework wide definitions for this. We can just add definitions for > new drivers to benefit - but I think adding them also for existing > drivers improves readability significantly (see below). > > > I would also like to see bloat-o-meter statistics before and after > > your patch. > > My guts tell me that the result will be not in the favour of yours > > solution. > > Can you please tell me what type of stats you hope to see? bloat-o-meter. It's a script that compares two object files to see which functions got fatter, and which are slimmer. You may find it in the kernel tree sources (scripts/ folder). > I can try > generating what you are after. The cover letter contained typical +/- > change stats from git and summary: > > 62 files changed, 228 insertions(+), 104 deletions(-) > > It sure shows that amount of lines was increased (roughly 2 lines more > / changed file) Which is making unneeded churn. > - but I guess that was expected as I don't like > ternary. And I like them, so, what to do? -- With Best Regards, Andy Shevchenko _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
