lkml.org 
[lkml]   [2016]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] reset: generate reset_control_get variants with macro expansion
From
Date
Hi Masahiro,

Am Donnerstag, den 21.07.2016, 18:53 +0900 schrieb Masahiro Yamada:
[...]
> For ctags, scripts/tags.sh exists for that purpose, doesn't it?
>
> For example,
> commit e0e5070b20e01f0321f97db4e4e174f3f6b49e50
> converted func defines and adjusted tags.sh at the same time.

True. So ctags is covered, alhough this somewhat moves the maintenance
effort from nearly duplicated function definitions to keeping
scripts/tags.sh up to date with all the different macro function
generators instead.

> > And secondly, we would now have detailed kerneldoc comments for two
> > functions that are never called directly, but the public facing API
> > itself is completely without documentation. I wouldn't mind removing
> > duplicated documentation paragraphs though, for example by referencing
> > reset_control_get_* from the of_reset_control_get_* kerneldoc comments.
>
> Linux is not always kind enough to document every public API.
>
> As you see, we generally do not document
> static inline wrappers.

Why not? As long as there is a meaningful difference between them, it
would make sense to describe them individually.

> In our case, they are all simply derived from two base functions.
> Documenting the two is enough.

I disagree. As a user of an API and its documentation, I would like to
have descriptions of functions that are actually used in the code, not
descriptions of the internal implementation behind them. This also
allows to have shorter paragraphs for each function, since they only
have to describe the behavior of that particular variant.

> I need to expand drivers/usb/host/ehci-platform.c
> to support multiple reset lines, so I really want
> devm_reset_control_get_optional_shared_by_index() supported.
>
> (So, probably
> devm_reset_control_get_exclusive_by_index
> devm_reset_control_get_shared_by_index
> devm_reset_control_get_optional_exclusive_by_index
> as well)
>
> If you are unhappy with this patch,
> I will send an alternative one,
> which adds 4 more functions in a stupid and straightforward way.

Yes, I would prefer straightforward addition of the needed inline
functions.

regards
Philipp

\
 
 \ /
  Last update: 2016-07-22 15:41    [W:0.065 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site