| 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
Linus, here is my first pull request this merge window for the I2C subsystem. A little later than usual but we needed to sort out one real build problem and then a few false positives from buildbot. But now everything seems in order. This pull request contains: * i2c core reorganization. One source file became too monolithic. It is now split up, yet we still have the same named object as the final output. This should ease maintenance. * new drivers: ZTE ZX2967 family, ASPEED 24XX/25XX, * designware driver gained slave mode support * xgene-slimpro driver gained ACPI support * bigger overhaul for pca-platform driver * the algo-bit module now supports messages with enforced STOP * slightly bigger than usual set of driver updates and improvements Please pull. Thanks, Wolfram The following changes since commit 5ed02dbb497422bf225783f46e6eadd237d23d6b: Linux 4.12-rc3 (2017-05-28 17:20:53 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-4.13 for you to fetch changes up to 8f1a357d41a22009150cf404b5aa5876efdb59b1: i2c: Provide a stub for i2c_detect_slave_mode() (2017-07-08 21:51:43 +0200) ---------------------------------------------------------------- Andy Shevchenko (1): i2c: Provide a stub for i2c_detect_slave_mode() Arnd Bergmann (1): i2c: xgene-slimpro: include linux/io.h for memremap Arvind Yadav (3): i2c: at91: Handle return value of clk_prepare_enable i2c: at91: Fix compilation warning. i2c: emev2: Handle return value of clk_prepare_enable Baoyou Xie (2): dt: bindings: add documentation for zx2967 family i2c controller i2c: zx2967: add i2c controller driver for ZTE's zx2967 family Brendan Higgins (4): i2c: aspeed: added documentation for Aspeed I2C driver i2c: aspeed: added driver for Aspeed I2C i2c: aspeed: added slave support for Aspeed I2C driver MAINTAINERS: add entry for Aspeed I2C driver Chris Packham (7): dt-bindings: add bindings for i2c-pca-platform i2c: pca-platform: switch to struct gpio_desc i2c: pca-platform: add devicetree awareness i2c: pca-platform: use device managed allocations i2c: pca-platform: use dev_warn/dev_info instead of printk i2c: pca-platform: correctly set algo_data.reset_chip i2c: pca-platform: propagate error from i2c_pca_add_numbered_bus Colin Ian King (1): i2c: designware: fix spelling mistakes George Cherian (1): i2c: xlp9xx: Enable HWMON class probing for xlp9xx Hans de Goede (1): i2c: acpi: Do not create i2c-clients for LNXVIDEO ACPI devices Hoan Tran (2): i2c: xgene-slimpro: Use a single function to send command message i2c: xgene-slimpro: Add ACPI support by using PCC mailbox Jarkko Nikula (2): i2c: designware: Make HW init functions static i2c: designware: Let slave adapter support be optional Jean Delvare (1): i2c: algo-bit: add support for I2C_M_STOP Luis Oliveira (6): i2c: designware: Cleaning and comment style fixes. i2c: designware: refactoring of the i2c-designware i2c: designware: MASTER mode as separated driver i2c: designware: introducing I2C_SLAVE definitions i2c: designware: add SLAVE mode functions i2c: designware: enable SLAVE in platform module Matt Weber (1): i2c: cadance: fix ctrl/addr reg write order Michael Thalmeier (1): i2c: mxs: change error printing to debug for mxs_i2c_pio_wait_xfer_end Shawn Guo (1): i2c: zx2967: drop COMPILE_TEST dependency Srinivas Pandruvada (1): i2c: i801: Add support for Intel Cannon Lake Wolfram Sang (15): i2c: rename core source file to allow refactorization i2c: break out slave support into separate file i2c: break out smbus support into separate file i2c: break out OF support into separate file i2c: break out ACPI support into separate file docs: i2c: dev-interface: adapt to new filenames of the i2c core i2c: remove unneeded includes from core i2c: reformat core-base file header i2c: stub: use pr_fmt i2c: sh_mobile: remove platform_data i2c: sh_mobile: drop needless check for of_node i2c: stub: fix build warning regression i2c: use proper name for the R-Car SoC i2c: rcar: document HW incapabilities i2c: zx2967: always use the same device when printing errors 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 Geert Uytterhoeven (2): (Rev.) i2c: sh_mobile: drop needless check for of_node (Rev.) i2c: sh_mobile: remove platform_data Hoan Tran (1): (Rev.) i2c: xgene-slimpro: include linux/io.h for memremap Jean Delvare (1): (Rev.) i2c: stub: use pr_fmt Jun Nie (1): (Rev.) i2c: zx2967: add i2c controller driver for ZTE's zx2967 family Luis Oliveira (2): (Test) i2c: designware: Let slave adapter support be optional (Test) i2c: designware: Make HW init functions static Mika Westerberg (2): (Rev.) i2c: break out ACPI support into separate file (Rev.) i2c: rename core source file to allow refactorization Wolfram Sang (2): (Test) i2c: algo-bit: add support for I2C_M_STOP (Test) i2c: at91: Fix compilation warning. .../devicetree/bindings/i2c/i2c-aspeed.txt | 48 + .../devicetree/bindings/i2c/i2c-designware.txt | 16 +- .../devicetree/bindings/i2c/i2c-pca-platform.txt | 29 + .../devicetree/bindings/i2c/i2c-zx2967.txt | 22 + Documentation/driver-api/i2c.rst | 5 +- Documentation/i2c/busses/i2c-i801 | 2 + Documentation/i2c/dev-interface | 14 +- MAINTAINERS | 13 + drivers/i2c/Makefile | 7 +- drivers/i2c/algos/i2c-algo-bit.c | 13 +- drivers/i2c/busses/Kconfig | 34 +- drivers/i2c/busses/Makefile | 6 + drivers/i2c/busses/i2c-aspeed.c | 891 +++++++++++ drivers/i2c/busses/i2c-at91.c | 16 +- drivers/i2c/busses/i2c-cadence.c | 6 +- drivers/i2c/busses/i2c-designware-common.c | 281 ++++ drivers/i2c/busses/i2c-designware-core.h | 182 ++- ...c-designware-core.c => i2c-designware-master.c} | 474 +----- drivers/i2c/busses/i2c-designware-pcidrv.c | 9 +- drivers/i2c/busses/i2c-designware-platdrv.c | 117 +- drivers/i2c/busses/i2c-designware-slave.c | 393 +++++ drivers/i2c/busses/i2c-emev2.c | 4 +- drivers/i2c/busses/i2c-i801.c | 8 + drivers/i2c/busses/i2c-mxs.c | 6 +- drivers/i2c/busses/i2c-pca-platform.c | 133 +- drivers/i2c/busses/i2c-rcar.c | 9 +- drivers/i2c/busses/i2c-sh_mobile.c | 27 +- drivers/i2c/busses/i2c-xgene-slimpro.c | 242 ++- drivers/i2c/busses/i2c-xlp9xx.c | 1 + drivers/i2c/busses/i2c-zx2967.c | 609 +++++++ 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 +- include/linux/i2c.h | 2 + include/linux/i2c/i2c-sh_mobile.h | 11 - include/trace/events/i2c.h | 226 +-- include/trace/events/smbus.h | 249 +++ 41 files changed, 4969 insertions(+), 2508 deletions(-) create mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt create mode 100644 Documentation/devicetree/bindings/i2c/i2c-pca-platform.txt create mode 100644 Documentation/devicetree/bindings/i2c/i2c-zx2967.txt create mode 100644 drivers/i2c/busses/i2c-aspeed.c create mode 100644 drivers/i2c/busses/i2c-designware-common.c rename drivers/i2c/busses/{i2c-designware-core.c => i2c-designware-master.c} (59%) create mode 100644 drivers/i2c/busses/i2c-designware-slave.c create mode 100644 drivers/i2c/busses/i2c-zx2967.c create mode 100644 drivers/i2c/i2c-core-acpi.c rename drivers/i2c/{i2c-core.c => i2c-core-base.c} (58%) create mode 100644 drivers/i2c/i2c-core-of.c create mode 100644 drivers/i2c/i2c-core-slave.c create mode 100644 drivers/i2c/i2c-core-smbus.c delete mode 100644 include/linux/i2c/i2c-sh_mobile.h create mode 100644 include/trace/events/smbus.h