On 09/09/15 01:12, Guenter Roeck wrote:
On 09/08/2015 08:58 PM, Greg KH wrote:
On Tue, Sep 08, 2015 at 06:10:16PM -0700, Guenter Roeck wrote:
Hi Emilio,
On 09/08/2015 05:51 PM, Emilio López wrote:
Hi Greg & Guenter,
[ ... ]
Unless I am missing something, this is not explained anywhere,
but it is
not entirely trivial to understand. I think it should be documented.
I agree. I couldn't find any mention of what this int was supposed
to be by looking at Documentation/ (is_visible is not even mentioned
:/) or include/linux/sysfs.h. Once we settle on something I'll
document it before sending a v2.
In the include file ? No strong preference, though.
By the way, I wrote a quick coccinelle script to match is_visible()
users which reference the index (included below), and it found
references to drivers which do not seem to use any binary
attributes, so I believe changing the index meaning shouldn't be an
issue.
Good.
I agree, make i the number of the bin attribute and that should solve
this issue.
No, that would conflict with the "normal" use of is_visible for
non-binary
attributes, and make the index all but useless, since the
is_visible function
would have to search through all the attributes anyway to figure
out which one
is being checked.
Yeah, using the same indexes would be somewhat pointless, although
not many seem to be using it anyway (only 14 files matched). Others
seem to be comparing the attr* instead. An alternative would be to
use negative indexes for binary attributes and positive indexes for
normal attributes.
... and I probably wrote or reviewed a significant percentage of
those ;-).
Using negative numbers for binary attributes is an interesting idea.
Kind of unusual, though. Greg, any thoughts on that ?
Ick, no, that's a mess, maybe we just could drop the index alltogether?
No, please don't. Having to manually compare dozens of index pointers
would be
even more of a mess.
So, what about keeping it the way it is in the patch, and documenting it
thoroughly? Otherwise, we could introduce another "is_bin_visible"
function to do this same thing but just on binary attributes, but I'd
rather not add a new function pointer if possible.
Cheers,
Emilio
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
