On Tue, 2019-11-05 at 15:11 +0200, andriy.shevchenko@linux.intel.com wrote:
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@... 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).
Right. Uwe explained that to me.
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.
Not unneeded. A few of us see the value of naming the 1 and 0.
- but I guess that was expected as I don't like
ternary.
And I like them, so, what to do?
Well, if you maintain those files and like ternary, then they can be ternary. I don't really care as long as I don't need to be the one maintaining them. Not really a battle I want to invest in. I can even go and drop the patches for files you are maintaining if you see that's the way to go. It's easier for me.
As I said, I don't plan to change the meaning of 1 and 0 - although I thought that allowing it might help in the future. Main goal was to name the 1 and 0. Naming can be done even if all users were not converted to use the names - as long as values behind names are not changed. Changing the values is a burden that can be carried by others when it is needed - we can now just make it easier or harder.
So as to what to do - please just give me the final decision so that I know if I should just drop the intel patches or use the ternary. Unfortunately I don't right now have the time to waste arguing over it ;)
Br, Matti