Message ID | cover.1579527444.git.matti.vaittinen@fi.rohmeurope.com (mailing list archive) |
---|---|
Headers | show |
Series | Support ROHM BD71828 PMIC | expand |
On Mon, Jan 20, 2020 at 03:40:20PM +0200, Matti Vaittinen wrote: > Patch series introducing support for ROHM BD71828 PMIC > > ROHM BD71828 is a power management IC containing 7 bucks and 7 LDOs. All > regulators can be controlled individually via I2C. Bucks 1,2,6 and > 7 can also be assigned to a "regulator group" controlled by run-levels. This is the *third* version of this you've sent today alone. Please stop sending me this series until the MFD has been merged, perhaps just drop the subsystem patches while you resolve whatever the problems are that remain with the MFD? I'm pretty much just deleting these patches without even looking at them at this point, it's a large series, it's getting huge numbers of resends and I don't think any version I've had a chance to look at before it got resent had a change in the one regulator patch that'd cause me to have to re-review it.
On Mon, 2020-01-20 at 13:54 +0000, Mark Brown wrote: > On Mon, Jan 20, 2020 at 03:40:20PM +0200, Matti Vaittinen wrote: > > Patch series introducing support for ROHM BD71828 PMIC > > > > ROHM BD71828 is a power management IC containing 7 bucks and 7 > > LDOs. All > > regulators can be controlled individually via I2C. Bucks 1,2,6 and > > 7 can also be assigned to a "regulator group" controlled by run- > > levels. > > This is the *third* version of this you've sent today alone. Please > stop sending me this series until the MFD has been merged, perhaps > just > drop the subsystem patches while you resolve whatever the problems > are > that remain with the MFD? I'm pretty much just deleting these > patches > without even looking at them at this point, it's a large series, it's > getting huge numbers of resends and I don't think any version I've > had a > chance to look at before it got resent had a change in the one > regulator > patch that'd cause me to have to re-review it. Sorry Mark (and all). I guess this is annoying. Why I do resend whole series is that during the bd71837 work Lee instructed me to always resend whole series - not just the changed patches. I sure can learn and drop some of the recipients in the future - and actually I did for this last resend. Reason why you are in the recipients is that Lee asked me to get your ack for patch 3/11. Same goes with Stephen. Linus is involved as Lee asked me to get his ack for patch 11. But resending this series 3 times today is really my fault. (Well, of course, I did send this and no one else). I messed up the previous series. I was hoping the revision history in cover letter would be a fast way to determine if something relevant has changed. Additionally, I did try to include statement that "no changes" in the beginning of each patches for most of the versions. (for versions up-to 11 - this was omitted from v12 and v13 which only did very specific changes). But sure, I should try to be more careful so that I don't need to do so many resends - and I really could drop most of the recipients earlier. Thanks for pointing it out. Br, Matti Vaittinen
On Mon, 20 Jan 2020, Vaittinen, Matti wrote: > On Mon, 2020-01-20 at 13:54 +0000, Mark Brown wrote: > > On Mon, Jan 20, 2020 at 03:40:20PM +0200, Matti Vaittinen wrote: > > > Patch series introducing support for ROHM BD71828 PMIC > > > > > > ROHM BD71828 is a power management IC containing 7 bucks and 7 > > > LDOs. All > > > regulators can be controlled individually via I2C. Bucks 1,2,6 and > > > 7 can also be assigned to a "regulator group" controlled by run- > > > levels. > > > > This is the *third* version of this you've sent today alone. Please > > stop sending me this series until the MFD has been merged, perhaps > > just > > drop the subsystem patches while you resolve whatever the problems > > are > > that remain with the MFD? I'm pretty much just deleting these > > patches > > without even looking at them at this point, it's a large series, it's > > getting huge numbers of resends and I don't think any version I've > > had a > > chance to look at before it got resent had a change in the one > > regulator > > patch that'd cause me to have to re-review it. To be fair, yours is one of the reviews we're waiting for! See [PATCH 03/11]. > Sorry Mark (and all). I guess this is annoying. Why I do resend whole > series is that during the bd71837 work Lee instructed me to always > resend whole series - not just the changed patches. Which in general is the correct thing to do. Having a large threaded series on the list containing in the form of subsequent versions gets real confusing real quick: | [PATCH v2 01/05] | > [PATCH v3 01/05] | [PATCH v2 02/05] | > [PATCH v3 02/05] | -> [PATCH v4 02/05] | [PATCH v2 03/05] | [PATCH v2 04/05] | > [PATCH v3 04/05] | [PATCH v2 05/05] | > [PATCH v3 05/05] | -> [PATCH v4 05/05] | --> [PATCH v5 05/05] | ---> [PATCH v6 05/05] However, you should wait for a suitable period per submission, to give each maintainer a chance to review the patches they are responsible for. > I sure can learn and drop some of the recipients in the future - and > actually I did for this last resend. Reason why you are in the > recipients is that Lee asked me to get your ack for patch 3/11. Same > goes with Stephen. > > Linus is involved as Lee asked me to get his ack for patch 11. > > But resending this series 3 times today is really my fault. (Well, of > course, I did send this and no one else). I messed up the previous > series. > > I was hoping the revision history in cover letter would be a fast way > to determine if something relevant has changed. Additionally, I did try > to include statement that "no changes" in the beginning of each patches > for most of the versions. (for versions up-to 11 - this was omitted > from v12 and v13 which only did very specific changes). > > But sure, I should try to be more careful so that I don't need to do so > many resends - and I really could drop most of the recipients earlier. > Thanks for pointing it out. If dropping recipients is your tactic of choice (I wouldn't choose to do that myself), just ensure you keep everyone on Cc for the cover-letter ("[PATCH 00/00]") and maybe indicate the fact that they have been dropped and why ("dropped Lee since the MFD patches have been reviewed and are unchanged in this series"). Reason being; if/when a set is applied, the cover-letter is used as the Reply-to for sending out the pull-request to all those concerned.
On Mon, 20 Jan 2020, Mark Brown wrote: > On Mon, Jan 20, 2020 at 03:40:20PM +0200, Matti Vaittinen wrote: > > Patch series introducing support for ROHM BD71828 PMIC > > > > ROHM BD71828 is a power management IC containing 7 bucks and 7 LDOs. All > > regulators can be controlled individually via I2C. Bucks 1,2,6 and > > 7 can also be assigned to a "regulator group" controlled by run-levels. > > This is the *third* version of this you've sent today alone. Please > stop sending me this series until the MFD has been merged, perhaps just > drop the subsystem patches while you resolve whatever the problems are > that remain with the MFD? > I'm pretty much just deleting these patches > without even looking at them at this point Don't do that please. You are one of the reasons he's resending. Please review Patch 3, or you will block the submission. > it's a large series, it's > getting huge numbers of resends and I don't think any version I've had a > chance to look at before it got resent had a change in the one regulator > patch that'd cause me to have to re-review it.
On Mon, Jan 20, 2020 at 02:21:09PM +0000, Vaittinen, Matti wrote: > On Mon, 2020-01-20 at 13:54 +0000, Mark Brown wrote: > > stop sending me this series until the MFD has been merged, perhaps > > just > > drop the subsystem patches while you resolve whatever the problems > > are > > that remain with the MFD? I'm pretty much just deleting these > I was hoping the revision history in cover letter would be a fast way > to determine if something relevant has changed. Additionally, I did try All I'm looking at mostly is to see if my Acked-by tag went away due to changes, I might look at the rest of a series if I have time but if I've checked the bit that's relevant to me I'm not going to worry too much about the rest of it. > But sure, I should try to be more careful so that I don't need to do so > many resends - and I really could drop most of the recipients earlier. > Thanks for pointing it out. What I was suggesting was more just dropping the driver bits while the MFD gets resolved, I'm never 100% sure why but the core MFD often takes a long time to figure out as seems to be the case here. There might be as many individual resends overall but each maintainer will see fewer of them.
On Tue, Jan 21, 2020 at 08:36:58AM +0000, Lee Jones wrote: > On Mon, 20 Jan 2020, Mark Brown wrote: > > This is the *third* version of this you've sent today alone. Please > > stop sending me this series until the MFD has been merged, perhaps just > > drop the subsystem patches while you resolve whatever the problems are > > that remain with the MFD? > > I'm pretty much just deleting these patches > > without even looking at them at this point > Don't do that please. You are one of the reasons he's resending. Me not looking at something can't reasonably be triggering the volume of resends this series is getting, we're talking multiple times a week here. For example looking at the changelog in the cover letter the thing that triggered the resends yesterday was converting a macro to an enum, most of the others seem to have been changes for individual drivers. > Please review Patch 3, or you will block the submission. That's the patch mfd: rohm PMICs - use platform_device_id to match MFD sub-devices which given the title hadn't really registered as needing any attention...
On Tue, 2020-01-21 at 16:15 +0000, Mark Brown wrote: > On Tue, Jan 21, 2020 at 08:36:58AM +0000, Lee Jones wrote: > > > Please review Patch 3, or you will block the submission. > > That's the patch > > mfd: rohm PMICs - use platform_device_id to match MFD sub-devices > > which given the title hadn't really registered as needing any > attention... Naming is hard :) I think the title - as a commit title - describes this specific change pretty well. (Well, knowing the author this opinion may be biased). But I recon this might not trigger review for regulators or clocks - any suggestions for me how to catch correct people attention by title? I understand that with the current volume of emails one just can't _read_ through all mails but just the ones with proper title. And now when I am talking about this - I still need Mark's ack for the DT binding conversion patch (the BD718x7 PMIC bindings, not this series https://lore.kernel.org/lkml/20200115062921.GA9944@localhost.localdomain/ ) - which I resent last week just for attention - but I guess patch naming is failing there too. Mark, do you have any email hook for Cc: tag in patches? Should I use it for patches which I hope you to check? (I just learned the Cc: tag recently). Maybe that could help? Br, Matti Vaittinen
On Tue, 2020-01-21 at 13:11 +0000, Mark Brown wrote: > On Mon, Jan 20, 2020 at 02:21:09PM +0000, Vaittinen, Matti wrote: > > On Mon, 2020-01-20 at 13:54 +0000, Mark Brown wrote: > > > stop sending me this series until the MFD has been merged, > > > perhaps > > > just > > > drop the subsystem patches while you resolve whatever the > > > problems > > > are > > > that remain with the MFD? I'm pretty much just deleting these > > I was hoping the revision history in cover letter would be a fast > > way > > to determine if something relevant has changed. Additionally, I did > > try > > All I'm looking at mostly is to see if my Acked-by tag went away due > to > changes, I might look at the rest of a series if I have time but if > I've > checked the bit that's relevant to me I'm not going to worry too much > about the rest of it. > > > But sure, I should try to be more careful so that I don't need to > > do so > > many resends - and I really could drop most of the recipients > > earlier. > > Thanks for pointing it out. > > What I was suggesting was more just dropping the driver bits while > the > MFD gets resolved, I'm never 100% sure why but the core MFD often > takes > a long time to figure out as seems to be the case here. There might > be > as many individual resends overall but each maintainer will see fewer > of > them. Hmm. I can do that as well - or just resend the cover-letter + changed patches untill I get all the acks and then do send the (hopefully) 'final' version with all patches for merging. But this needs to work for Lee too. I could also add recipients manually per patch so that the whole series would not be sent to all - interested persons can always ask me to include them to the whole series - or look it up from lore.kernel.org - what ever works best with you guys. But I need common understanding from you regarding what is the best way. I believe I will initiate the next new ROHM PMIC patch series somewhere near summer - and possibly a charger chip series before that. I will try to change my ways for those if you will find a common understanding regarding the best way. Best regards Matti
On Tue, 21 Jan 2020, Mark Brown wrote: > On Tue, Jan 21, 2020 at 08:36:58AM +0000, Lee Jones wrote: > > On Mon, 20 Jan 2020, Mark Brown wrote: > > > > This is the *third* version of this you've sent today alone. Please > > > stop sending me this series until the MFD has been merged, perhaps just > > > drop the subsystem patches while you resolve whatever the problems are > > > that remain with the MFD? > > > > I'm pretty much just deleting these patches > > > without even looking at them at this point > > > Don't do that please. You are one of the reasons he's resending. > > Me not looking at something can't reasonably be triggering the volume of > resends this series is getting, we're talking multiple times a week > here. For example looking at the changelog in the cover letter the > thing that triggered the resends yesterday was converting a macro to an > enum, most of the others seem to have been changes for individual > drivers. > > > Please review Patch 3, or you will block the submission. > > That's the patch > > mfd: rohm PMICs - use platform_device_id to match MFD sub-devices > > which given the title hadn't really registered as needing any > attention... If I chose patches to review based on subject alone, I would miss a high percentage of reviews. Maybe your mail/manual/brain filters need to be improved.
On Wed, Jan 22, 2020 at 06:28:35AM +0000, Vaittinen, Matti wrote: > And now when I am talking about this - I still need Mark's ack for the > DT binding conversion patch (the BD718x7 PMIC bindings, not this > series > https://lore.kernel.org/lkml/20200115062921.GA9944@localhost.localdomain/ > ) - which I resent last week just for attention - but I guess patch > naming is failing there too. There's probably a bit of that as well but I think what happened the last time you sent stuff was that it was sent at the same time as this series and unless you're reading the subject lines carefully it looked like fragments of old versions of that series or something. > Mark, do you have any email hook for Cc: tag in patches? Should I use > it for patches which I hope you to check? (I just learned the Cc: tag > recently). Maybe that could help? No, I don't have that and people just CC me on random stuff so I'm not sure it'd help.
On Wed, Jan 22, 2020 at 07:32:30AM +0000, Lee Jones wrote: > On Tue, 21 Jan 2020, Mark Brown wrote: > > That's the patch > > mfd: rohm PMICs - use platform_device_id to match MFD sub-devices > > which given the title hadn't really registered as needing any > > attention... > If I chose patches to review based on subject alone, I would miss a > high percentage of reviews. Maybe your mail/manual/brain filters > need to be improved. It's the subject line in combination with the fact that it's in the middle of a big MFD series that's getting a lot of revisions where I know I've already gone through the bits relevant to my subsystems on earlier revisions - it's *very* unusual for new things to pop up in the middle of patches to other subsystems.
On Wed, Jan 22, 2020 at 06:44:06AM +0000, Vaittinen, Matti wrote: > Hmm. I can do that as well - or just resend the cover-letter + changed > patches untill I get all the acks and then do send the (hopefully) > 'final' version with all patches for merging. But this needs to work I think the ideal thing is if all the dependencies are Kconfig dependencies so people can just merge the parts for their subsystems without needing everything to get applied at once, you can't do that here due to the refactoring of the existing driver unfortunately. It also helps if the drivers in the main series are all simple and don't struggle like the LED one seems to have here but that depends a lot on how well the hardware fits the subsystem and is hard to control.
Stephen, Could you review patch 3 please? On Mon, 20 Jan 2020, Matti Vaittinen wrote: > Patch series introducing support for ROHM BD71828 PMIC > > ROHM BD71828 is a power management IC containing 7 bucks and 7 LDOs. All > regulators can be controlled individually via I2C. Bucks 1,2,6 and > 7 can also be assigned to a "regulator group" controlled by run-levels. > Eg. Run level specific voltages and enable/disable statuses for each of > these bucks can be set via register interface. The buck run-level group > assignment (selection if buck is to be controlled individually or via > run-levels) can be changed at run-time via I2C. > > This patch series brings only the basic support for controlling > regulators individually via I2C. > > In addition to the bucks and LDOs there are: > > - The usual clk gate > - 4 IO pins (mostly usable as GPO or tied to specific purpose) > - power button support > - RTC > - two LEDs > - battery charger > - HALL sensor input > > This patch series adds support to regulators, clk, RTC and GPIOs. > > Power-supply driver for charger or LED driver is not included in this series. > > Changelog v13: > - Added back the accidentally dropped GPIO patch > > Changelog v12: > - removed input: tag from MFD gpio-keys patch > - dropped LED patches for now. > > Changelog v11: > - converted a macro to enum member as was requested by Lee. > > Changelog v10: > - Split RTC patch to a BD70528 fix (which hopefully goes to 5.4) and to > BD71828 support > > Changelog v9: (Changes suggested by Lee) > - Added kerneldoc to struct rohm_dvs_config > - cleaned few comments > - updated copyright date > - renamed variable 'mfd' to 'parent'. > > Changelog v8: > LEDs: > - Fixed bunch of typos. > - Corrected the commit message which errorneously contained the > 'leds-compatible' which I dropped in favour of match_property. > - use max_brightness instead of LED_FULL if it is given when > handling the default_state, "on". > - clean fwnode_owned at the end of unsuccessful registration or > at the end of deregistration. > - fix the accidental linuxdoc comment. > - rename led_find_fwnode to led_get_fwnode as it increases refcount. > > Changelog v7: > - Rebased on top of v5.5-rc2 > - Dropped already applied patches > - basic regulator support (in regulator tree now) > - regulator dt-bindings (in regulator tree now) > - gpio devm function documentation (in GPIO tree now) > - Dropped new devm-variant of gpio_array getting for MFD sub-devices who > have consumer information in parent DT node as gpio consumer was > dropped from the series > - removed extra line-breaks from MFD driver and Makefile > - fixed RTC patch subject line (added missing colon) > - included a patch which adds compatible for ROHM BD71850 PMIC > > Changelog v6: > Rebased on top of v5.5-rc1 > LED core: > - Do new fw-node look-up only if the new match data is given. That > way behaviour for existing drivers is not changed > - Handle generic LED properties by core only if explisitly requested > in init-data. That way behaviour for existing drivers is not changed > until they are verified to work. > BD71828 LEDs: > - Fix module loading by adding "dummy" of_device_id table. > DT bindings: > All: > - Remove regulator run-level properties as run-level support was > dropped for now. > - Change SPDX to dual lisence > LED: > - added select: false > - replace oneOf + const by enum > Regulator: > - remove forgotten comments > - comment indenting > MFD: > - remove unnecessary descriptions > Regulators: > - Dropped patch 12 with run-level controls > - Dropped unnecessary ramp_delay_supported() - ram_delay ops were > already only filled for DVS bucks. > GPIO: > - rename internal function. > RTC: > - Added missing blank line > > Changelog v5: > Only LED patch (patch 15) changed, rest as in v4. > LED: > - Fixed issues reported by Dan Carpenter and kbuild-bot static > analysis. > Changelog v4 (first non RFC): > General: > - Changed subdevice loading and chip version identification to use > platform ID. > - License identifiers changed to GPL-2.0-only > MFD: > - Styling fixes mostly > DT-Bindings: > - a few more checks as suggested by Rob Herring. > - Order of DT patches changed. > - me as maintainer > - standard units to new properties (microvolts, ohms) > - runlevel values in an array > LED: > - BD71828 driver added (back) > - Added DT support > - Added LED DT node lookup in led framework when init_data is given > with DT node match information. > - Added common property parsing for default-state and > default-trigger. > Regulators: > - dropped sysfs interfaces > - fixed module unload/reload by binding gpio consumer information to > regulator device not to MFD. > GPIO: > - Added devm_gpiod_get_parent_array > - added few missing devm functions to documentation > > Changelog v3: > DT-Bindings: > - yamlify > - add LED binding doc > CLK: > - Move clk register definitions from MFD headers to clk driver > GPIO: > - Add generic direction define and use it. > LED: > - Drop LED driver from the series (for now). > > Changelog v2: Mainly RTC and GPIO fixes suggested by Alexandre and Bartosz > General: > -Patch ordering changed to provide dt binding documents right after the > MFD core. > DT-Bindings for regulators (Patch 3) > -Fix typo in PMIC model number > RTC (patch 11) > -Reverted renaming in order to reduce patch size. > -Reworded commit message > BD71828 regulator (patch 7) > -Add MODULE_ALIAS > GPIO (patch 12) > -Remove file-name from comment > -prefix IN and OUT defines with chip type > -improved documentation for the INPUT only pin. > -removed empty left-over function > -removed unnecessary #ifdef CONFIG_OF_GPIO > -removed unnecessary error print > -Add MODULE_ALIAS > > Patch 1: > dt-bindings for LEDs on BD71828 PMIC > Patch 2: > dt-bindings for BD71828 PMIC > Patch 3: > Convert rohm PMICs with common sub-devices to use platform_ > device_id to match MFD sub-devices > Patch 4: > Add compatible for BD71850 > Patch 5: > BD71828 MFD core. > Patch 6: > Power button support using GPIO keys. > Patch 7: > CLK gate support using existing clk-bd718x7 > Patch 8: > Split existing bd718x7 regulator driver to generic ROHM dt > parsing portion (used by more than one ROHM drivers) and > bd718x8 specific parts > Patch 9: > Fix BD70528 RTC HOUR mask > Patch 10: > Support BD71828 RTC block using BD70528 RTC driver > Patch 11: > Allow control of GP(I)O pins on BD71828 via GPIO subsystem > > --- > > > > Matti Vaittinen (11): > dt-bindings: leds: ROHM BD71282 PMIC LED driver > dt-bindings: mfd: Document ROHM BD71828 bindings > mfd: rohm PMICs - use platform_device_id to match MFD sub-devices > mfd: bd718x7: Add compatible for BD71850 > mfd: bd71828: Support ROHM BD71828 PMIC - core > mfd: bd71828: Add power-key support > clk: bd718x7: Support ROHM BD71828 clk block > regulator: bd718x7: Split driver to common and bd718x7 specific parts > mfd: bd70528: Fix hour register mask > rtc: bd70528: add BD71828 support > gpio: bd71828: Initial support for ROHM BD71828 PMIC GPIOs > > .../bindings/leds/rohm,bd71828-leds.yaml | 52 +++ > .../bindings/mfd/rohm,bd71828-pmic.yaml | 193 ++++++++ > drivers/clk/Kconfig | 6 +- > drivers/clk/clk-bd718x7.c | 50 ++- > drivers/gpio/Kconfig | 12 + > drivers/gpio/Makefile | 1 + > drivers/gpio/gpio-bd71828.c | 159 +++++++ > drivers/mfd/Kconfig | 15 + > drivers/mfd/Makefile | 1 + > drivers/mfd/rohm-bd70528.c | 3 +- > drivers/mfd/rohm-bd71828.c | 344 ++++++++++++++ > drivers/mfd/rohm-bd718x7.c | 43 +- > drivers/regulator/Kconfig | 4 + > drivers/regulator/Makefile | 1 + > drivers/regulator/bd718x7-regulator.c | 200 +++------ > drivers/regulator/rohm-regulator.c | 95 ++++ > drivers/rtc/Kconfig | 3 +- > drivers/rtc/rtc-bd70528.c | 220 +++++++-- > include/linux/mfd/rohm-bd70528.h | 19 +- > include/linux/mfd/rohm-bd71828.h | 423 ++++++++++++++++++ > include/linux/mfd/rohm-bd718x7.h | 6 - > include/linux/mfd/rohm-generic.h | 70 ++- > include/linux/mfd/rohm-shared.h | 21 + > 23 files changed, 1718 insertions(+), 223 deletions(-) > create mode 100644 Documentation/devicetree/bindings/leds/rohm,bd71828-leds.yaml > create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml > create mode 100644 drivers/gpio/gpio-bd71828.c > create mode 100644 drivers/mfd/rohm-bd71828.c > create mode 100644 drivers/regulator/rohm-regulator.c > create mode 100644 include/linux/mfd/rohm-bd71828.h > create mode 100644 include/linux/mfd/rohm-shared.h > > > base-commit: b3a987b0264d ("Linux 5.5-rc6")