Thread (27 messages) 27 messages, 3 authors, 2015-03-09

Re: [PATCH v5 3/8] pinctrl: cygnus: add initial IOMUX driver support

From: Ray Jui <rjui@broadcom.com>
Date: 2015-03-09 19:40:36
Also in: linux-arm-kernel, linux-gpio, lkml


On 3/9/2015 12:30 PM, Paul Bolle wrote:
Ray Jui schreef op ma 09-03-2015 om 12:00 [-0700]:
quoted
I think it depends on how you see it. Based on this logic, then one can
also argue comments in the code will be pre-processed away and are not
needed. They at least serve the same documentation purpose in a way.
So why not make them comments? And even that might not be needed:
- MODULE_LICENSE() only summarizes, in just a few words, what takes a
few paragraphs in the customary comment at the top of a file;
- MODULE_DESCRIPTION() repeats what, in general, has been said in the
Kconfig entry for that driver and in the git commit explanation;
- and I'm not sure what the benefit is of MODULE_AUTHOR() in the first
place (even for actually modular drivers).
quoted
So
far I haven't seen other people complaining that having these module
based macros in the driver are confusing when the Kconfig has a bool.
Perhaps that's just because review doesn't spot all issues. Patch
bandwidth exceeding review bandwidth and all that.
I don't see this as an "issue" to be quite honest. By saying that, I at
least agree with you that these are not information that's mandatory to
be in the driver given what we already have. MODULE_LICENSE is covered
by license header. MODULE_DESCRIPTION is covered by descriptions in
Kconfig. MODULE_AUTHOR is much less important than what's in the
MAINTAINERS list.

Since I have to submit a new patch series to address the "ngpios" issue
that Linus mentioned in the other email, I don't mind removing all these
MODULE_* macros in the driver all together.
Anyhow, right now there's another thread discussing the questions my
review comments raise. Eg, "The Kconfig symbol is bool, there is module
related code in the driver, why note make the Kconfig symbol tristate
(and the driver modular)?". I think that is one of the questions mixing
built-in and modular semantics raises.


Paul Bolle
Thanks,

Ray
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help