On 4/17/19 1:25 AM, Greg KH wrote:
On Tue, Apr 16, 2019 at 05:13:18PM -0500, Madhumitha Prabakaran wrote:
Fix a blank line after structure declarations. Also, convert macros into inline functions in order to maintain Linux kernel coding style based on which the inline function is preferable over the macro.
Blank line fixes are suggested by checkpatch.pl
Signed-off-by: Madhumitha Prabakaran madhumithabiw@gmail.com
Changes in v2:
- To maintain consistency in driver greybus, all the instances of macro
with container_of are fixed in a single patch.
drivers/staging/greybus/bundle.h | 6 +++++- drivers/staging/greybus/control.h | 6 +++++- drivers/staging/greybus/gbphy.h | 12 ++++++++++-- drivers/staging/greybus/greybus.h | 6 +++++- drivers/staging/greybus/hd.h | 6 +++++- drivers/staging/greybus/interface.h | 6 +++++- drivers/staging/greybus/module.h | 6 +++++- drivers/staging/greybus/svc.h | 6 +++++- 8 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/greybus/bundle.h b/drivers/staging/greybus/bundle.h index 8734d2055657..84956eedb1c4 100644 --- a/drivers/staging/greybus/bundle.h +++ b/drivers/staging/greybus/bundle.h @@ -31,7 +31,11 @@ struct gb_bundle { struct list_head links; /* interface->bundles */ }; -#define to_gb_bundle(d) container_of(d, struct gb_bundle, dev)
+static inline struct gb_bundle *to_gb_bundle(struct device *d) +{
- return container_of(d, struct gb_bundle, dev);
+}
Why are we changing this to an inline function? The #define is fine, and how we "normally" do this type of container_of conversion.
I'm not completely sure about the inline function, but on the no blank lines thing (and many other minor issues) "checkpatch.pl" is to blame. There are lots of examples of issues that checkpatch points out that are matters of opinion and not hardened kernel style rules.
We try to encourage people to get involved with kernel development by fixing minor problems, and we tell them a good way to find them is by running checkpatch and "fixing" what it reports. Unfortunately, it is often things of this type, and reviewers balk and say "no, please leave it," and the poor new person has a bad first experience.
I *like* "checkpatch.pl". And the fact that it can point out some of these sorts of things is great. But it would be nice if certain types of problems (like multiple blank lines, or lines that are 81 characters wide for example) would only be reported when a "--strict" option or something were supplied.
-Alex
I understand the objection of the "no blank line", but this was the "original" style that we used to create these #defines from the very beginning of the driver model work a decade ago. Changing that muscle-memory is going to be hard for some of us. Look at drivers/base/base.h for other examples of this.
thanks,
greg k-h _______________________________________________ greybus-dev mailing list greybus-dev@lists.linaro.org https://lists.linaro.org/mailman/listinfo/greybus-dev