Message ID | 20170712154901.yrrhtwfpnokkmswe@ninjato |
---|---|
State | Accepted |
Headers | show |
> with much appreciated quality assurance from > ---------------------------------------------------------------- > Andy Shevchenko (17): > (Rev.) i2c: designware: Let slave adapter support be optional > (Rev.) i2c: designware: Make HW init functions static > (Rev.) i2c: pca-platform: propagate error from i2c_pca_add_numbered_bus > (Rev.) i2c: pca-platform: correctly set algo_data.reset_chip > (Rev.) i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices > (Rev.) i2c: designware: enable SLAVE in platform module > (Rev.) i2c: designware: add SLAVE mode functions > (Rev.) i2c: zx2967: add i2c controller driver for ZTE's zx2967 family > (Rev.) i2c: designware: introducing I2C_SLAVE definitions > (Rev.) i2c: designware: Cleaning and comment style fixes. > (Rev.) i2c: remove unneeded includes from core > (Rev.) docs: i2c: dev-interface: adapt to new filenames of the i2c core > (Rev.) i2c: break out ACPI support into separate file > (Rev.) i2c: break out OF support into separate file > (Rev.) i2c: break out smbus support into separate file > (Rev.) i2c: break out slave support into separate file > (Rev.) i2c: rename core source file to allow refactorization ^ This ^ Worth pointing out. Thanks a ton, Andy!
On Wed, Jul 12, 2017 at 8:49 AM, Wolfram Sang <wsa@the-dreams.de> wrote: > drivers/i2c/i2c-core-acpi.c | 665 ++++++++ > drivers/i2c/{i2c-core.c => i2c-core-base.c} | 1684 +------------------- > drivers/i2c/i2c-core-of.c | 276 ++++ > drivers/i2c/i2c-core-slave.c | 115 ++ > drivers/i2c/i2c-core-smbus.c | 594 +++++++ > drivers/i2c/i2c-core.h | 24 + > drivers/i2c/i2c-stub.c | 14 +- Side note on this.. (and doesn't affect the pull - I pulled it and it's going through my test build right now). Please don't do the silly "start every filename with the same prefix". It annoys people (ie me) that use tab-completion, and it just looks stupid. And this core re-org does it twice over - first with "i2c-" and then with "core-". Of *course* it's "i2c-something.c" - it's in the i2c directory. So that part is entirely pointless. And the "core-something.c" part seems to be entirely to keep the files together - but if the issue really is "sort files together", then that's why we have subdirectories. So I personally tend to much prefer drivers/i2c/core/acpi.c drivers/i2c/core/base.c drivers/i2c/core/of.c drivers/i2c/core/slave.c drivers/i2c/core/smbus.c drivers/i2c/core/core.h as the model. Then things *really* sort together, auto-complete works better, and tools like "git diff --dirstat" etc that group changes by directories also automatically just do the right thing. And notice how the filenames are smaller and prettier too? It's just a win/win situation. But I'm not going to enforce my style guide on you, since I very seldom actually end up touching those files. If this was some area where I often actually ended up looking at things, I'd almost require a sane directory structure, though. Because "name things with the same prefix" is not a sane directory structure. Linus
Hello, On Wed, Jul 12, 2017 at 10:16:32AM -0700, Linus Torvalds wrote: > On Wed, Jul 12, 2017 at 8:49 AM, Wolfram Sang <wsa@the-dreams.de> wrote: > > drivers/i2c/i2c-core-acpi.c | 665 ++++++++ > > drivers/i2c/{i2c-core.c => i2c-core-base.c} | 1684 +------------------- > > drivers/i2c/i2c-core-of.c | 276 ++++ > > drivers/i2c/i2c-core-slave.c | 115 ++ > > drivers/i2c/i2c-core-smbus.c | 594 +++++++ > > drivers/i2c/i2c-core.h | 24 + > > drivers/i2c/i2c-stub.c | 14 +- > > Side note on this.. (and doesn't affect the pull - I pulled it and > it's going through my test build right now). > > Please don't do the silly "start every filename with the same prefix". > It annoys people (ie me) that use tab-completion, and it just looks > stupid. > > And this core re-org does it twice over - first with "i2c-" and then > with "core-". > > Of *course* it's "i2c-something.c" - it's in the i2c directory. So > that part is entirely pointless. > > And the "core-something.c" part seems to be entirely to keep the files > together - but if the issue really is "sort files together", then > that's why we have subdirectories. > > So I personally tend to much prefer > > drivers/i2c/core/acpi.c > drivers/i2c/core/base.c > drivers/i2c/core/of.c > drivers/i2c/core/slave.c > drivers/i2c/core/smbus.c > drivers/i2c/core/core.h > > as the model. Then things *really* sort together, auto-complete works > better, and tools like "git diff --dirstat" etc that group changes by > directories also automatically just do the right thing. > > And notice how the filenames are smaller and prettier too? It's just a > win/win situation. > > But I'm not going to enforce my style guide on you, since I very > seldom actually end up touching those files. If this was some area > where I often actually ended up looking at things, I'd almost require > a sane directory structure, though. > > Because "name things with the same prefix" is not a sane directory structure. Another reason I remember for the prefix thing is that this way the kernel modules are not named of.ko, core.ko and base.ko. (But this can be fixed in the Makefile of course with some more prose.) Best regards Uwe
Linus, > So I personally tend to much prefer > > drivers/i2c/core/acpi.c > drivers/i2c/core/base.c > drivers/i2c/core/of.c > drivers/i2c/core/slave.c > drivers/i2c/core/smbus.c > drivers/i2c/core/core.h > > as the model. Then things *really* sort together, auto-complete works > better, and tools like "git diff --dirstat" etc that group changes by > directories also automatically just do the right thing. > > And notice how the filenames are smaller and prettier too? It's just a > win/win situation. I am easily convinced by these benefits, thanks for pointing them out. There was one request from distributions, though, when we once tried to refactor only ACPI out back then (which we had to revert because of this request): keep the module name constant. Given that, I am going to take some middle path and rename like this: drivers/i2c/acpi.c drivers/i2c/base.c drivers/i2c/of.c drivers/i2c/slave.c drivers/i2c/smbus.c drivers/i2c/core.h and keep the resulting object name 'i2c-core.o'. That implies that the root directory of 'i2c' will be core only. I think this is fair. Will implement it hopefully today and send you a pull request ASAP. Thanks, Wolfram
> request): keep the module name constant. Given that, I am going to take > some middle path and rename like this: > > drivers/i2c/acpi.c ... Where is my brown paper bag? Sorry, I mixed things up. I'll just come back once I'm done and happy...
On 2017-07-13 09:56, Uwe Kleine-König wrote: > Hello, > > On Wed, Jul 12, 2017 at 10:16:32AM -0700, Linus Torvalds wrote: >> On Wed, Jul 12, 2017 at 8:49 AM, Wolfram Sang <wsa@the-dreams.de> wrote: >>> drivers/i2c/i2c-core-acpi.c | 665 ++++++++ >>> drivers/i2c/{i2c-core.c => i2c-core-base.c} | 1684 +------------------- >>> drivers/i2c/i2c-core-of.c | 276 ++++ >>> drivers/i2c/i2c-core-slave.c | 115 ++ >>> drivers/i2c/i2c-core-smbus.c | 594 +++++++ >>> drivers/i2c/i2c-core.h | 24 + >>> drivers/i2c/i2c-stub.c | 14 +- >> >> Side note on this.. (and doesn't affect the pull - I pulled it and >> it's going through my test build right now). >> >> Please don't do the silly "start every filename with the same prefix". >> It annoys people (ie me) that use tab-completion, and it just looks >> stupid. >> >> And this core re-org does it twice over - first with "i2c-" and then >> with "core-". >> >> Of *course* it's "i2c-something.c" - it's in the i2c directory. So >> that part is entirely pointless. >> >> And the "core-something.c" part seems to be entirely to keep the files >> together - but if the issue really is "sort files together", then >> that's why we have subdirectories. >> >> So I personally tend to much prefer >> >> drivers/i2c/core/acpi.c >> drivers/i2c/core/base.c >> drivers/i2c/core/of.c >> drivers/i2c/core/slave.c >> drivers/i2c/core/smbus.c >> drivers/i2c/core/core.h >> >> as the model. Then things *really* sort together, auto-complete works >> better, and tools like "git diff --dirstat" etc that group changes by >> directories also automatically just do the right thing. >> >> And notice how the filenames are smaller and prettier too? It's just a >> win/win situation. >> >> But I'm not going to enforce my style guide on you, since I very >> seldom actually end up touching those files. If this was some area >> where I often actually ended up looking at things, I'd almost require >> a sane directory structure, though. >> >> Because "name things with the same prefix" is not a sane directory structure. > > Another reason I remember for the prefix thing is that this way the > kernel modules are not named of.ko, core.ko and base.ko. (But this can > be fixed in the Makefile of course with some more prose.) And if you, like me, sometimes take a peek at the core of several subsystems using emacs, the buffers are either named "core.c<subsys>" or "subsys-core.c". The latter naming is more friendly to tab-completion after ctrl-x b. It's not totally black-and-white... Cheers, Peter
On Thu, Jul 13, 2017 at 1:57 PM, Peter Rosin <peda@axentia.se> wrote: > > And if you, like me, sometimes take a peek at the core of several > subsystems using emacs, the buffers are either named "core.c<subsys>" > or "subsys-core.c". The latter naming is more friendly to tab-completion > after ctrl-x b. Here's a nickel, Kid. Buy a real editor. http://i.imgur.com/z96dZ0x.jpg If your editor can't show directories, the limitations in your environment shouldn't screw over everybody else. Linus
On 2017-07-13 23:05, Linus Torvalds wrote: > On Thu, Jul 13, 2017 at 1:57 PM, Peter Rosin <peda@axentia.se> wrote: >> >> And if you, like me, sometimes take a peek at the core of several >> subsystems using emacs, the buffers are either named "core.c<subsys>" >> or "subsys-core.c". The latter naming is more friendly to tab-completion >> after ctrl-x b. > > Here's a nickel, Kid. Buy a real editor. Ahh, thanks for calling me a kid, I think I'll have a shave too. > http://i.imgur.com/z96dZ0x.jpg > > If your editor can't show directories, the limitations in your > environment shouldn't screw over everybody else. FWIW, I agree that your points made sense. I was just pointing out one situation were I actually preferred the prefix. Cheers, Peter