mbox series

[v13,00/11] Support ROHM BD71828 PMIC

Message ID cover.1579527444.git.matti.vaittinen@fi.rohmeurope.com (mailing list archive)
Headers show
Series Support ROHM BD71828 PMIC | expand

Message

Vaittinen, Matti Jan. 20, 2020, 1:40 p.m. UTC
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")

Comments

Mark Brown Jan. 20, 2020, 1:54 p.m. UTC | #1
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.
Vaittinen, Matti Jan. 20, 2020, 2:21 p.m. UTC | #2
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
Lee Jones Jan. 21, 2020, 8:34 a.m. UTC | #3
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.
Lee Jones Jan. 21, 2020, 8:36 a.m. UTC | #4
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.
Mark Brown Jan. 21, 2020, 1:11 p.m. UTC | #5
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.
Mark Brown Jan. 21, 2020, 4:15 p.m. UTC | #6
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...
Vaittinen, Matti Jan. 22, 2020, 6:28 a.m. UTC | #7
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
Vaittinen, Matti Jan. 22, 2020, 6:44 a.m. UTC | #8
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
Lee Jones Jan. 22, 2020, 7:32 a.m. UTC | #9
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.
Mark Brown Jan. 22, 2020, 11:58 a.m. UTC | #10
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.
Mark Brown Jan. 22, 2020, 12:11 p.m. UTC | #11
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.
Mark Brown Jan. 22, 2020, 12:19 p.m. UTC | #12
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.
Lee Jones Jan. 22, 2020, 1:33 p.m. UTC | #13
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")