mbox series

[v2,00/17] Support ROHM BD71815 PMIC

Message ID cover.1611037866.git.matti.vaittinen@fi.rohmeurope.com
Headers show
Series Support ROHM BD71815 PMIC | expand

Message

Matti Vaittinen Jan. 19, 2021, 7:13 a.m. UTC
Patch series introducing support for ROHM BD71815 PMIC

Please note that this series depends on previously sent patches.

Already merged regulator patches (in regulator tree - not yet in Linus'
tree):
[PATCH] regulator: ROHM bd7xxxx: Do not depend on parent driver data
https://lore.kernel.org/lkml/20210105130221.GA3438042@localhost.localdomain/
[PATCH] regulator: bd718x7: Stop using parent data
https://lore.kernel.org/lkml/20210107122355.GA35080@localhost.localdomain/

Already merged GPIO patch (in GPIO tree - not yet in Linus' tree):
[PATCH] gpio: bd7xxxx: Do not depend on parent driver data
https://lore.kernel.org/lkml/20210105125335.GA3430233@localhost.localdomain/


This series also includes already sent patches:
The patches 1, 2 and 3 have been sent separately to mfd/regulator, rtc and clk
subsystems - but they have not yet been reviewed / applied. They are present
also in this series because the series depends on those changes.
I will drop those patches from this series if they get merged to
(MFD or regulator)/RTC/clk trees from previous submits.

patch 1 previous submit:
regulator: bd718x7, bd71828, Fix dvs voltage levels
https://lore.kernel.org/lkml/20210118075851.GA1016281@localhost.localdomain/

patch 2 previous submit:
rtc: bd70528: Do not require parent data
https://lore.kernel.org/lkml/20210105152350.GA3714833@localhost.localdomain/

patch 3 previous submit:
clk: BD718x7: Do not depend on parent driver data
https://lore.kernel.org/lkml/20210105123028.GA3409663@localhost.localdomain/


Please note that due to the dependencies all of the patches are probably
not applying cleanly to all subsystem trees. (regulator/RTC patches to
other subsystems). I understand it perfectly well if this series can't be
merged before all dependencies are merged to all relevant subsystem trees
but I would be grateful if I got the first set of review comments even
before that.


ROHM BD71815 is a power management IC used in some battery powered
systems. It contains regulators, GPO(s), charger + coulomb counter, RTC
and a clock gate.

All regulators can be controlled via I2C. LDO4 can additionally be set to
be enabled/disabled by a GPIO. LDO3 voltage could be selected from two
voltages written into separate VSEL reisters using GPIO but this mode is
not supported by driver. On top of that the PMIC has the typical HW
state machine which is present also on many other ROHM PMICs.

IC contains two GPOs - but one of the GPOs is marked as GND in
data-sheet. Thus the driver by default only exposes one GPO. The second
GPO can be enabled by special DT property.

RTC is almost similar to what is on BD71828. For currently used features
only the register address offset to RTC block differs.

The charger driver is not included in this series. ROHM has a charger
driver with some fuel-gauging logig written in but this is not included
here. I am working on separating the logic from HW specific driver and
supporting both BD71815 and BD71828 chargers in separate patch series.

Changelog v2:
  - Rebased on top of v5.11-rc3
  - Added another "preliminary patch" which fixes HW-dvs voltage
    handling (patch 1)
  - split regulator patch to two.
  - changed dt-binding patch ordering.
  regulators:
    - staticized probe
    - removed some unnecessary defines
    - updated comments
    - split rohm-regulator patch adding SNVS and supporting simple
      linear mapping into two - one adding support for mapping, other
      adding SNVS.
  GPIO:
    - removed unnecessary headers
    - clarified dev/parent->dev usage
    - removed forgotten #define DEBUG
  dt-bindings:
    - changed patch order to meet ref-dependencies
    - added missing regulator nodes
    - changed string property for clk mode to tristated
  MFD:
    - header cleanups.
  CLK:
    - fixed commit message

Matti Vaittinen (17):
  regulator: bd718x7, bd71828, Fix dvs voltage levels
  rtc: bd70528: Do not require parent data
  clk: BD718x7: Do not depend on parent driver data
  mfd: bd718x7: simplify by cleaning unnecessary device data
  dt_bindings: bd71828: Add clock output mode
  dt_bindings: regulator: Add ROHM BD71815 PMIC regulators
  dt_bindings: mfd: Add ROHM BD71815 PMIC
  mfd: Add ROHM BD71815 ID
  mfd: Support for ROHM BD71815 PMIC core
  gpio: support ROHM BD71815 GPOs
  regulator: helpers: Export helper voltage listing
  regulator: rohm-regulator: linear voltage support
  regulator: rohm-regulator: Support SNVS HW state.
  regulator: Support ROHM BD71815 regulators
  clk: bd718x7: Add support for clk gate on ROHM BD71815 PMIC
  rtc: bd70528: Support RTC on ROHM BD71815
  MAINTAINERS: Add ROHM BD71815AGW

 .../bindings/mfd/rohm,bd71815-pmic.yaml       | 202 ++++++
 .../bindings/mfd/rohm,bd71828-pmic.yaml       |   6 +
 .../regulator/rohm,bd71815-regulator.yaml     | 116 +++
 MAINTAINERS                                   |   3 +
 drivers/clk/clk-bd718x7.c                     |  21 +-
 drivers/gpio/Kconfig                          |  10 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-bd71815.c                   | 171 +++++
 drivers/mfd/Kconfig                           |  15 +-
 drivers/mfd/rohm-bd71828.c                    | 416 ++++++++++-
 drivers/mfd/rohm-bd718x7.c                    |  43 +-
 drivers/regulator/Kconfig                     |  11 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/bd71815-regulator.c         | 682 ++++++++++++++++++
 drivers/regulator/helpers.c                   |  36 +-
 drivers/regulator/rohm-regulator.c            |  32 +-
 drivers/rtc/Kconfig                           |   6 +-
 drivers/rtc/rtc-bd70528.c                     | 104 ++-
 include/linux/mfd/rohm-bd71815.h              | 561 ++++++++++++++
 include/linux/mfd/rohm-bd71828.h              |   3 +
 include/linux/mfd/rohm-bd718x7.h              |  13 -
 include/linux/mfd/rohm-generic.h              |  19 +-
 include/linux/regulator/driver.h              |   2 +
 23 files changed, 2321 insertions(+), 153 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.yaml
 create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd71815-regulator.yaml
 create mode 100644 drivers/gpio/gpio-bd71815.c
 create mode 100644 drivers/regulator/bd71815-regulator.c
 create mode 100644 include/linux/mfd/rohm-bd71815.h

Comments

Bartosz Golaszewski Jan. 19, 2021, 11:07 a.m. UTC | #1
On Tue, Jan 19, 2021 at 8:23 AM Matti Vaittinen
<matti.vaittinen@fi.rohmeurope.com> wrote:
>
> Support GPO(s) found from ROHM BD71815 power management IC. The IC has two
> GPO pins but only one is properly documented in data-sheet. The driver
> exposes by default only the documented GPO. The second GPO is connected to
> E5 pin and is marked as GND in data-sheet. Control for this undocumented
> pin can be enabled using a special DT property.
>
> This driver is derived from work by Peter Yang <yanglsh@embest-tech.com>
> although not so much of original is left.
>
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

Hi Matti,

looks great, just a couple nits.

> ---
> Changes since v1:
>   - removed unneeded headers
>   - clarified dev/parent->dev usage
>   - removed forgotten #define DEBUG
>
>  drivers/gpio/Kconfig        |  10 +++
>  drivers/gpio/Makefile       |   1 +
>  drivers/gpio/gpio-bd71815.c | 171 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 182 insertions(+)
>  create mode 100644 drivers/gpio/gpio-bd71815.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index c70f46e80a3b..fd7283af858d 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1096,6 +1096,16 @@ config GPIO_BD70528
>           This driver can also be built as a module. If so, the module
>           will be called gpio-bd70528.
>
> +config GPIO_BD71815
> +       tristate "ROHM BD71815 PMIC GPIO support"
> +       depends on MFD_ROHM_BD71828
> +       help
> +         Support for GPO(s) on ROHM BD71815 PMIC. There are two GPOs
> +         available on the ROHM PMIC.
> +
> +         This driver can also be built as a module. If so, the module
> +         will be called gpio-bd71815.
> +
>  config GPIO_BD71828
>         tristate "ROHM BD71828 GPIO support"
>         depends on MFD_ROHM_BD71828
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 35e3b6026665..86bb680522a6 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -39,6 +39,7 @@ obj-$(CONFIG_GPIO_ATH79)              += gpio-ath79.o
>  obj-$(CONFIG_GPIO_BCM_KONA)            += gpio-bcm-kona.o
>  obj-$(CONFIG_GPIO_BCM_XGS_IPROC)       += gpio-xgs-iproc.o
>  obj-$(CONFIG_GPIO_BD70528)             += gpio-bd70528.o
> +obj-$(CONFIG_GPIO_BD71815)             += gpio-bd71815.o
>  obj-$(CONFIG_GPIO_BD71828)             += gpio-bd71828.o
>  obj-$(CONFIG_GPIO_BD9571MWV)           += gpio-bd9571mwv.o
>  obj-$(CONFIG_GPIO_BRCMSTB)             += gpio-brcmstb.o
> diff --git a/drivers/gpio/gpio-bd71815.c b/drivers/gpio/gpio-bd71815.c
> new file mode 100644
> index 000000000000..664de5f69bf1
> --- /dev/null
> +++ b/drivers/gpio/gpio-bd71815.c
> @@ -0,0 +1,171 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Support to GPOs on ROHM BD71815
> + */

Newline here.

> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/irq.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +/* For the BD71815 register definitions */
> +#include <linux/mfd/rohm-bd71815.h>
> +

Please arrange headers alphabetically.

> +struct bd71815_gpio {
> +       struct gpio_chip chip;
> +       struct device *dev;
> +       struct regmap *regmap;
> +       /*
> +        * Sigh. The BD71815 and BD71817 were originally designed to support two
> +        * GPO pins. At some point it was noticed the second GPO pin which is
> +        * the E5 pin located at the center of IC is hard to use on PCB (due to
> +        * the location). It was decided to not promote this second GPO and pin
> +        * is marked as GND on the data-sheet. The functionality is still there
> +        * though! I guess driving GPO connected to ground is a bad idea. Thus
> +        * we do not support it by default. OTOH - the original driver written
> +        * by colleagues at Embest did support controlling this second GPO. It
> +        * is thus possible this is used in some of the products.
> +        *
> +        * This driver does not by default support configuring this second GPO
> +        * but allows using it by providing the DT property
> +        * "rohm,enable-hidden-gpo".
> +        */
> +       bool e5_pin_is_gpo;
> +};
> +
> +static int bd71815gpo_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +       struct bd71815_gpio *bd71815 = gpiochip_get_data(chip);
> +       int ret = 0;
> +       int val;
> +
> +       ret = regmap_read(bd71815->regmap, BD71815_REG_GPO, &val);
> +       if (ret)
> +               return ret;
> +
> +       return (val >> offset) & 1;
> +}
> +
> +static void bd71815gpo_set(struct gpio_chip *chip, unsigned int offset,
> +                          int value)
> +{
> +       struct bd71815_gpio *bd71815 = gpiochip_get_data(chip);
> +       int ret, val, mask;
> +
> +       if (!bd71815->e5_pin_is_gpo && offset)
> +               return;
> +
> +       mask = BIT(offset);
> +       val = value ? mask : 0;

Maybe use regmap_set/clear_bits() here?

> +       ret = regmap_update_bits(bd71815->regmap, BD71815_REG_GPO, mask, val);
> +       if (ret)
> +               dev_warn(bd71815->dev, "failed to toggle GPO\n");
> +}
> +
> +static int bd71815_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
> +                                  unsigned long config)
> +{
> +       struct bd71815_gpio *bdgpio = gpiochip_get_data(chip);
> +
> +       if (!bdgpio->e5_pin_is_gpo && offset)
> +               return -EOPNOTSUPP;
> +
> +       switch (pinconf_to_config_param(config)) {
> +       case PIN_CONFIG_DRIVE_OPEN_DRAIN:
> +               return regmap_update_bits(bdgpio->regmap,
> +                                         BD71815_REG_GPO,
> +                                         BD71815_GPIO_DRIVE_MASK << offset,
> +                                         BD71815_GPIO_OPEN_DRAIN << offset);
> +       case PIN_CONFIG_DRIVE_PUSH_PULL:
> +               return regmap_update_bits(bdgpio->regmap,
> +                                         BD71815_REG_GPO,
> +                                         BD71815_GPIO_DRIVE_MASK << offset,
> +                                         BD71815_GPIO_CMOS << offset);
> +       default:
> +               break;
> +       }
> +       return -EOPNOTSUPP;
> +}
> +
> +/* BD71815 GPIO is actually GPO */
> +static int bd71815gpo_direction_get(struct gpio_chip *gc, unsigned int offset)
> +{
> +       return GPIO_LINE_DIRECTION_OUT;
> +}
> +
> +/* Template for GPIO chip */

So let's make it const?

> +static struct gpio_chip bd71815gpo_chip = {
> +       .label                  = "bd71815",
> +       .owner                  = THIS_MODULE,
> +       .get                    = bd71815gpo_get,
> +       .get_direction          = bd71815gpo_direction_get,
> +       .set                    = bd71815gpo_set,
> +       .set_config             = bd71815_gpio_set_config,
> +       .can_sleep              = 1,
> +};
> +
> +static int gpo_bd71815_probe(struct platform_device *pdev)
> +{
> +       int ret;
> +       struct bd71815_gpio *g;
> +       struct device *dev;
> +       struct device *parent;
> +
> +       /*
> +        * Bind devm lifetime to this platform device => use dev for devm.
> +        * also the prints should originate from this device.
> +        */
> +       dev = &pdev->dev;
> +       /* The device-tree and regmap come from MFD => use parent for that */
> +       parent = dev->parent;
> +
> +       g = devm_kzalloc(dev, sizeof(*g), GFP_KERNEL);
> +       if (!g)
> +               return -ENOMEM;
> +
> +       g->e5_pin_is_gpo = of_property_read_bool(parent->of_node,
> +                                                "rohm,enable-hidden-gpo");
> +       g->chip = bd71815gpo_chip;
> +       g->chip.base = -1;
> +
> +       if (g->e5_pin_is_gpo)
> +               g->chip.ngpio = 2;
> +       else
> +               g->chip.ngpio = 1;
> +
> +       g->chip.parent = parent;
> +       g->chip.of_node = parent->of_node;
> +       g->regmap = dev_get_regmap(parent, NULL);
> +       g->dev = dev;
> +
> +       ret = devm_gpiochip_add_data(dev, &g->chip, g);
> +       if (ret < 0) {
> +               dev_err(dev, "could not register gpiochip, %d\n", ret);
> +               return ret;
> +       }
> +
> +       return ret;
> +}
> +static const struct platform_device_id bd7181x_gpo_id[] = {
> +       { "bd71815-gpo" },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(platform, bd7181x_gpo_id);
> +
> +static struct platform_driver gpo_bd71815_driver = {
> +       .driver = {
> +               .name   = "bd71815-gpo",
> +               .owner  = THIS_MODULE,
> +       },
> +       .probe          = gpo_bd71815_probe,
> +       .id_table       = bd7181x_gpo_id,
> +};
> +
> +module_platform_driver(gpo_bd71815_driver);
> +
> +/* Note:  this hardware lives inside an I2C-based multi-function device. */
> +MODULE_ALIAS("platform:bd71815-gpo");
> +
> +MODULE_AUTHOR("Peter Yang <yanglsh@embest-tech.com>");
> +MODULE_DESCRIPTION("GPO interface for BD71815");
> +MODULE_LICENSE("GPL");
> --
> 2.25.4
>

Bartosz

>
> --
> Matti Vaittinen, Linux device drivers
> ROHM Semiconductors, Finland SWDC
> Kiviharjunlenkki 1E
> 90220 OULU
> FINLAND
>
> ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
> Simon says - in Latin please.
> ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
> Thanks to Simon Glass for the translation =]
Matti Vaittinen Jan. 19, 2021, 1:01 p.m. UTC | #2
Hi Bartosz,

On Tue, 2021-01-19 at 12:07 +0100, Bartosz Golaszewski wrote:
> On Tue, Jan 19, 2021 at 8:23 AM Matti Vaittinen
> <matti.vaittinen@fi.rohmeurope.com> wrote:
> > Support GPO(s) found from ROHM BD71815 power management IC. The IC
> > has two
> > GPO pins but only one is properly documented in data-sheet. The
> > driver
> > exposes by default only the documented GPO. The second GPO is
> > connected to
> > E5 pin and is marked as GND in data-sheet. Control for this
> > undocumented
> > pin can be enabled using a special DT property.
> > 
> > This driver is derived from work by Peter Yang <
> > yanglsh@embest-tech.com>
> > although not so much of original is left.
> > 
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> 
> Hi Matti,
> 
> looks great, just a couple nits.
> 

Thanks for the review! I'll store your finsings and fix them when I
respin this. I think all of your points were valid. As I know this is
largish series (and as I know I accidentally sent first 10 v2 patches
to all recipients no matter what subsystem was impacted) I'll wait for
a while before resending (at least a week). Besides I don't expect the
dependencies to be merged before next kernel release so this is not
urgent :)

- but thanks!

Br,
	Matti
Alexandre Belloni Jan. 19, 2021, 9:09 p.m. UTC | #3
On 19/01/2021 09:14:45+0200, Matti Vaittinen wrote:
> The ROHM BD71828 and BD71815 RTC drivers only need the regmap
> pointer from parent. Regmap can be obtained via dev_get_regmap()
> so do not require parent to populate driver data for that.
> 
> BD70528 on the other hand requires parent data to access the
> watchdog so leave the parent data for BD70528 here for now.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
> 
> Please note that this same change has been sent individually:
> https://lore.kernel.org/lkml/20210105152350.GA3714833@localhost.localdomain/
> It is present in this series only because some patches depend on it.
> 

Then I think it is best to have it as part of this series.

> ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~

I've alway wanted to comment on that, should he have to say "I don't
think" to vanish ? Because saying "I don't think so," means that he
thinks this is not so.

> Simon says - in Latin please.
> ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~

And I guess this should be simply "non cogito" ;)
Lee Jones Jan. 25, 2021, 1:56 p.m. UTC | #4
On Tue, 19 Jan 2021, Matti Vaittinen wrote:

> Most ROHM PMIC sub-devices only use the regmap pointer from
> parent device. They can obtain this by dev_get_regamap so in
> most cases the MFD device does not need to allocate and populate
> the driver data. Simplify drivers by removing this.
> 
> The BD70528 still needs the access to watchdog mutex so keep
> rohm_regmap_dev in use on BD70528 RTC and WDG drivers for now.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
> 
> No changes since v1
> 
>  drivers/mfd/rohm-bd718x7.c       | 43 ++++++++++++--------------------
>  include/linux/mfd/rohm-bd718x7.h | 13 ----------
>  2 files changed, 16 insertions(+), 40 deletions(-)

For my own reference (apply this as-is to your sign-off block):

  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
Lee Jones Jan. 25, 2021, 1:57 p.m. UTC | #5
On Tue, 19 Jan 2021, Matti Vaittinen wrote:

> Add chip ID for ROHM BD71815 and PMIC so that drivers can identify
> this IC.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
> No changes since v1.
> 
>  include/linux/mfd/rohm-generic.h | 1 +
>  1 file changed, 1 insertion(+)

For my own reference (apply this as-is to your sign-off block):

  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
Lee Jones Jan. 25, 2021, 2:10 p.m. UTC | #6
On Tue, 19 Jan 2021, Matti Vaittinen wrote:

> Add core support for ROHM BD71815 Power Management IC.
> 
> The IC integrates regulators, a battery charger with a coulomb counter,
> a real-time clock (RTC), clock gate and general-purpose outputs (GPO).
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
> Changes since v1:
>   - Used BIT() for better readability
>   - removed some unused definitions
> 
>  drivers/mfd/Kconfig              |  15 +-
>  drivers/mfd/rohm-bd71828.c       | 416 +++++++++++++++++++++--
>  include/linux/mfd/rohm-bd71815.h | 561 +++++++++++++++++++++++++++++++
>  include/linux/mfd/rohm-bd71828.h |   3 +
>  4 files changed, 952 insertions(+), 43 deletions(-)
>  create mode 100644 include/linux/mfd/rohm-bd71815.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index bdfce7b15621..59bfacb91898 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1984,19 +1984,20 @@ config MFD_ROHM_BD70528
>  	  charger.
>  
>  config MFD_ROHM_BD71828
> -	tristate "ROHM BD71828 Power Management IC"
> +	tristate "ROHM BD71828 and BD71815 Power Management IC"
>  	depends on I2C=y
>  	depends on OF
>  	select REGMAP_I2C
>  	select REGMAP_IRQ
>  	select MFD_CORE
>  	help
> -	  Select this option to get support for the ROHM BD71828 Power
> -	  Management IC. BD71828GW is a single-chip power management IC for
> -	  battery-powered portable devices. The IC integrates 7 buck
> -	  converters, 7 LDOs, and a 1500 mA single-cell linear charger.
> -	  Also included is a Coulomb counter, a real-time clock (RTC), and
> -	  a 32.768 kHz clock gate.
> +	  Select this option to get support for the ROHM BD71828 and BD71815
> +	  Power Management ICs. BD71828GW and BD71815AGW are single-chip power
> +	  management ICs mainly for battery-powered portable devices.
> +	  The BD71828 integrates 7 buck converters and 7 LDOs. The BD71815
> +	  has 5 bucks, 7 LDOs, and a boost for driving LEDs. Both ICs provide
> +	  also a single-cell linear charger, a Coulomb counter, a real-time
> +	  clock (RTC), GPIOs and a 32.768 kHz clock gate.
>  
>  config MFD_STM32_LPTIMER
>  	tristate "Support for STM32 Low-Power Timer"
> diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
> index 210261d026f2..28b82477ce4c 100644
> --- a/drivers/mfd/rohm-bd71828.c
> +++ b/drivers/mfd/rohm-bd71828.c
> @@ -2,7 +2,7 @@
>  //
>  // Copyright (C) 2019 ROHM Semiconductors
>  //
> -// ROHM BD71828 PMIC driver
> +// ROHM BD71828/BD71815 PMIC driver
>  
>  #include <linux/gpio_keys.h>
>  #include <linux/i2c.h>
> @@ -11,7 +11,9 @@
>  #include <linux/ioport.h>
>  #include <linux/irq.h>
>  #include <linux/mfd/core.h>
> +#include <linux/mfd/rohm-bd71815.h>
>  #include <linux/mfd/rohm-bd71828.h>
> +#include <linux/mfd/rohm-generic.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
>  #include <linux/regmap.h>
> @@ -29,12 +31,102 @@ static struct gpio_keys_platform_data bd71828_powerkey_data = {
>  	.name = "bd71828-pwrkey",
>  };
>  
> -static const struct resource rtc_irqs[] = {
> +static const struct resource bd71815_rtc_irqs[] = {
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC0, "bd71815-rtc-alm-0"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC1, "bd71815-rtc-alm-1"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC2, "bd71815-rtc-alm-2"),
> +};
> +
> +static const struct resource bd71828_rtc_irqs[] = {
>  	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC0, "bd71828-rtc-alm-0"),
>  	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC1, "bd71828-rtc-alm-1"),
>  	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC2, "bd71828-rtc-alm-2"),
>  };
>  
> +static struct resource bd71815_power_irqs[] = {
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_RMV, "bd71815-dcin-rmv"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_OUT, "bd71815-clps-out"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_IN, "bd71815-clps-in"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_OVP_RES, "bd71815-dcin-ovp-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_OVP_DET, "bd71815-dcin-ovp-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_MON_RES, "bd71815-dcin-mon-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_MON_DET, "bd71815-dcin-mon-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_UV_RES, "bd71815-vsys-uv-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_UV_DET, "bd71815-vsys-uv-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_LOW_RES, "bd71815-vsys-low-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_LOW_DET, "bd71815-vsys-low-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_RES, "bd71815-vsys-mon-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_RES, "bd71815-vsys-mon-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_WDG_TEMP, "bd71815-chg-wdg-temp"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_WDG_TIME, "bd71815-chg-wdg"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RECHARGE_RES, "bd71815-rechg-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RECHARGE_DET, "bd71815-rechg-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RANGED_TEMP_TRANSITION,
> +			     "bd71815-ranged-temp-transit"),

The new line limit is 100.  Feel free to run these out.

> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_STATE_TRANSITION,
> +			     "bd71815-chg-state-change"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_TEMP_NORMAL,
> +			     "bd71815-bat-temp-normal"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_TEMP_ERANGE,
> +			     "bd71815-bat-temp-erange"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_REMOVED, "bd71815-bat-rmv"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_DETECTED, "bd71815-bat-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_THERM_REMOVED, "bd71815-therm-rmv"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_THERM_DETECTED, "bd71815-therm-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_DEAD, "bd71815-bat-dead"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_SHORTC_RES,
> +			     "bd71815-bat-short-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_SHORTC_DET,
> +			     "bd71815-bat-short-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_LOW_VOLT_RES,
> +			     "bd71815-bat-low-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_LOW_VOLT_DET,
> +			     "bd71815-bat-low-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_VOLT_RES,
> +			     "bd71815-bat-over-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_VOLT_DET,
> +			     "bd71815-bat-over-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_MON_RES, "bd71815-bat-mon-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_MON_DET, "bd71815-bat-mon-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_CC_MON1, "bd71815-bat-cc-mon1"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_CC_MON2, "bd71815-bat-cc-mon2"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_CC_MON3, "bd71815-bat-cc-mon3"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_1_RES,
> +			     "bd71815-bat-oc1-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_1_DET,
> +			     "bd71815-bat-oc1-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_2_RES,
> +			     "bd71815-bat-oc2-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_2_DET,
> +			     "bd71815-bat-oc2-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_3_RES,
> +			     "bd71815-bat-oc3-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_3_DET,
> +			     "bd71815-bat-oc3-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_LOW_RES,
> +			     "bd71815-bat-low-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_LOW_DET,
> +			     "bd71815-bat-low-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_HI_RES, "bd71815-bat-hi-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_HI_DET, "bd71815-bat-hi-det"),
> +};

[...]

> +static const struct regmap_irq bd71815_irqs[] = {

[...]

> +	REGMAP_IRQ_REG(BD71815_INT_TEMP_CHIP_OVER_VF_DET, 10,
> +		       BD71815_INT_TEMP_CHIP_OVER_VF_DET_MASK),

As above.

> +	/* RTC Alarm */
> +	REGMAP_IRQ_REG(BD71815_INT_RTC0, 11, BD71815_INT_RTC0_MASK),
> +	REGMAP_IRQ_REG(BD71815_INT_RTC1, 11, BD71815_INT_RTC1_MASK),
> +	REGMAP_IRQ_REG(BD71815_INT_RTC2, 11, BD71815_INT_RTC2_MASK),
> +};

[...]

> +static int set_clk_mode(struct device *dev, struct regmap *regmap,
> +			int clkmode_reg)
> +{
> +	int ret;
> +	const char *mode;
> +
> +	ret = of_property_read_string_index(dev->of_node, "rohm,clkout-mode", 0,
> +					    &mode);
> +	if (ret) {
> +		if (ret == -EINVAL)
> +			return 0;
> +		return ret;
> +	}
> +	if (!strncmp(mode, "open-drain", 10)) {
> +		dev_dbg(dev, "configuring clk32kout as open-drain");

Do you *really* need these?

> +		ret = regmap_update_bits(regmap, clkmode_reg, OUT32K_MODE,
> +					 OUT32K_MODE_OPEN_DRAIN);
> +	} else if (!strncmp(mode, "cmos", 4)) {
> +		dev_dbg(dev, "configuring clk32kout as cmos");
> +		ret = regmap_update_bits(regmap, clkmode_reg, OUT32K_MODE,
> +					 OUT32K_MODE_CMOS);
> +	} else {
> +		dev_err(dev, "bad clk32kout mode configuration");
> +		return -EINVAL;
> +	}
> +	return ret;
> +}
> +
>  static int bd71828_i2c_probe(struct i2c_client *i2c)
>  {
> -	struct rohm_regmap_dev *chip;
>  	struct regmap_irq_chip_data *irq_data;
>  	int ret;
> +	struct regmap *regmap;
> +	const struct regmap_config *regmap_config;
> +	struct regmap_irq_chip *irqchip;
> +	unsigned int chip_type;
> +	struct mfd_cell *mfd;
> +	int cells;
> +	int button_irq;
> +	int clkmode_reg;
>  
>  	if (!i2c->irq) {
>  		dev_err(&i2c->dev, "No IRQ configured\n");
>  		return -EINVAL;
>  	}
>  
> -	chip = devm_kzalloc(&i2c->dev, sizeof(*chip), GFP_KERNEL);
> -	if (!chip)
> -		return -ENOMEM;
> -
> -	dev_set_drvdata(&i2c->dev, chip);
> +	chip_type = (unsigned int)(uintptr_t)
> +		    of_device_get_match_data(&i2c->dev);
> +	switch (chip_type) {
> +	case ROHM_CHIP_TYPE_BD71828:
> +		mfd = bd71828_mfd_cells;
> +		cells = ARRAY_SIZE(bd71828_mfd_cells);
> +		regmap_config = &bd71828_regmap;
> +		irqchip = &bd71828_irq_chip;
> +		clkmode_reg = BD71815_REG_OUT32K;
> +		button_irq = BD71828_INT_SHORTPUSH;
> +		dev_info(&i2c->dev, "BD71828 found\n");
> +		break;
> +	case ROHM_CHIP_TYPE_BD71815:
> +		mfd = bd71815_mfd_cells;
> +		cells = ARRAY_SIZE(bd71815_mfd_cells);
> +		regmap_config = &bd71815_regmap;
> +		irqchip = &bd71815_irq_chip;
> +		clkmode_reg = BD71828_REG_OUT32K;
> +		/*
> +		 * If BD71817 support is needed we should be able to handle it
> +		 * with proper DT configs + BD71815 drivers + power-button.
> +		 * BD71815 data-sheet does not list the power-button IRQ so we
> +		 * don't use it.
> +		 */
> +		button_irq = 0;
> +		dev_info(&i2c->dev, "BD71815 found\n");

Again, are these *really* useful to you and/or your users?

Besides, this device not *found* i.e. scanned/read and instantiated,
it has simply been matched from the local DTB.  It can still be
wrong.  You can probably omit them.

[...]

> diff --git a/include/linux/mfd/rohm-bd71815.h b/include/linux/mfd/rohm-bd71815.h
> new file mode 100644
> index 000000000000..8ee5874a5b73
> --- /dev/null
> +++ b/include/linux/mfd/rohm-bd71815.h
> @@ -0,0 +1,561 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright 2014 Embest Technology Co. Ltd. Inc.

Jeeze!  Is this for the Amiga?

> + * Author: yanglsh@embest-tech.com
> + *
> + * 2020, 2021 Heavily modified by:

You should probably add a proper copyright.

> + *	 Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> + */
> +
> +#ifndef _MFD_BD71815_H
> +#define _MFD_BD71815_H
> +
> +#include <linux/regmap.h>

[...]
Matti Vaittinen Jan. 25, 2021, 2:48 p.m. UTC | #7
Hello Lee,

Thanks again for the review!

On Mon, 2021-01-25 at 14:10 +0000, Lee Jones wrote:
> On Tue, 19 Jan 2021, Matti Vaittinen wrote:
> 
> > Add core support for ROHM BD71815 Power Management IC.
> > 
> > The IC integrates regulators, a battery charger with a coulomb
> > counter,
> > a real-time clock (RTC), clock gate and general-purpose outputs
> > (GPO).
> > 
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > ---
> > Changes since v1:
> >   - Used BIT() for better readability
> >   - removed some unused definitions
> > 
> >  drivers/mfd/Kconfig              |  15 +-
> >  drivers/mfd/rohm-bd71828.c       | 416 +++++++++++++++++++++--
> >  include/linux/mfd/rohm-bd71815.h | 561
> > +++++++++++++++++++++++++++++++
> >  include/linux/mfd/rohm-bd71828.h |   3 +
> >  4 files changed, 952 insertions(+), 43 deletions(-)
> >  create mode 100644 include/linux/mfd/rohm-bd71815.h
> > 
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index bdfce7b15621..59bfacb91898 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1984,19 +1984,20 @@ config MFD_ROHM_BD70528
> >  	  charger.
> >  
> >  config MFD_ROHM_BD71828
> > -	tristate "ROHM BD71828 Power Management IC"
> > +	tristate "ROHM BD71828 and BD71815 Power Management IC"
> >  	depends on I2C=y
> >  	depends on OF
> >  	select REGMAP_I2C
> >  	select REGMAP_IRQ
> >  	select MFD_CORE
> >  	help
> > -	  Select this option to get support for the ROHM BD71828 Power
> > -	  Management IC. BD71828GW is a single-chip power management IC
> > for
> > -	  battery-powered portable devices. The IC integrates 7 buck
> > -	  converters, 7 LDOs, and a 1500 mA single-cell linear charger.
> > -	  Also included is a Coulomb counter, a real-time clock (RTC),
> > and
> > -	  a 32.768 kHz clock gate.
> > +	  Select this option to get support for the ROHM BD71828 and
> > BD71815
> > +	  Power Management ICs. BD71828GW and BD71815AGW are single-
> > chip power
> > +	  management ICs mainly for battery-powered portable devices.
> > +	  The BD71828 integrates 7 buck converters and 7 LDOs. The
> > BD71815
> > +	  has 5 bucks, 7 LDOs, and a boost for driving LEDs. Both ICs
> > provide
> > +	  also a single-cell linear charger, a Coulomb counter, a real-
> > time
> > +	  clock (RTC), GPIOs and a 32.768 kHz clock gate.
> >  
> >  config MFD_STM32_LPTIMER
> >  	tristate "Support for STM32 Low-Power Timer"
> > diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-
> > bd71828.c
> > index 210261d026f2..28b82477ce4c 100644
> > --- a/drivers/mfd/rohm-bd71828.c
> > +++ b/drivers/mfd/rohm-bd71828.c
> > @@ -2,7 +2,7 @@
> >  //
> >  // Copyright (C) 2019 ROHM Semiconductors
> >  //
> > -// ROHM BD71828 PMIC driver
> > +// ROHM BD71828/BD71815 PMIC driver
> >  
> >  #include <linux/gpio_keys.h>
> >  #include <linux/i2c.h>
> > @@ -11,7 +11,9 @@
> >  #include <linux/ioport.h>
> >  #include <linux/irq.h>
> >  #include <linux/mfd/core.h>
> > +#include <linux/mfd/rohm-bd71815.h>
> >  #include <linux/mfd/rohm-bd71828.h>
> > +#include <linux/mfd/rohm-generic.h>
> >  #include <linux/module.h>
> >  #include <linux/of_device.h>
> >  #include <linux/regmap.h>
> > @@ -29,12 +31,102 @@ static struct gpio_keys_platform_data
> > bd71828_powerkey_data = {
> >  	.name = "bd71828-pwrkey",
> >  };
> >  
> > -static const struct resource rtc_irqs[] = {
> > +static const struct resource bd71815_rtc_irqs[] = {
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC0, "bd71815-rtc-alm-0"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC1, "bd71815-rtc-alm-1"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC2, "bd71815-rtc-alm-2"),
> > +};
> > +
> > +static const struct resource bd71828_rtc_irqs[] = {
> >  	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC0, "bd71828-rtc-alm-0"),
> >  	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC1, "bd71828-rtc-alm-1"),
> >  	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC2, "bd71828-rtc-alm-2"),
> >  };
> >  
> > +static struct resource bd71815_power_irqs[] = {
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_RMV, "bd71815-dcin-rmv"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_OUT, "bd71815-clps-out"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_IN, "bd71815-clps-in"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_OVP_RES, "bd71815-dcin-
> > ovp-res"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_OVP_DET, "bd71815-dcin-
> > ovp-det"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_MON_RES, "bd71815-dcin-
> > mon-res"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_MON_DET, "bd71815-dcin-
> > mon-det"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_UV_RES, "bd71815-vsys-uv-
> > res"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_UV_DET, "bd71815-vsys-uv-
> > det"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_LOW_RES, "bd71815-vsys-
> > low-res"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_LOW_DET, "bd71815-vsys-
> > low-det"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_RES, "bd71815-vsys-
> > mon-res"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_RES, "bd71815-vsys-
> > mon-det"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_WDG_TEMP, "bd71815-chg-
> > wdg-temp"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_WDG_TIME, "bd71815-chg-
> > wdg"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RECHARGE_RES, "bd71815-
> > rechg-res"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RECHARGE_DET, "bd71815-
> > rechg-det"),
> > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RANGED_TEMP_TRANSITION,
> > +			     "bd71815-ranged-temp-transit"),
> 
> The new line limit is 100.  Feel free to run these out.

I learn new things every day it seems. This change is more than
welcome!

> > +	if (!strncmp(mode, "open-drain", 10)) {
> > +		dev_dbg(dev, "configuring clk32kout as open-drain");
> 
> Do you *really* need these?

No. development leftover. Thanks for pointing that.

> > 
> > +		button_irq = 0;
> > +		dev_info(&i2c->dev, "BD71815 found\n");
> 
> Again, are these *really* useful to you and/or your users?
> 
> Besides, this device not *found* i.e. scanned/read and instantiated,
> it has simply been matched from the local DTB.  It can still be
> wrong.  You can probably omit them.

You're right. One can check the DT contents from /proc if he wants to
check what IC compatible was used. Thanks.
> 
> [...]
> 
> > diff --git a/include/linux/mfd/rohm-bd71815.h
> > b/include/linux/mfd/rohm-bd71815.h
> > new file mode 100644
> > index 000000000000..8ee5874a5b73
> > --- /dev/null
> > +++ b/include/linux/mfd/rohm-bd71815.h
> > @@ -0,0 +1,561 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright 2014 Embest Technology Co. Ltd. Inc.
> 
> Jeeze!  Is this for the Amiga?

Nah. Long live NXP SOCs! ;) I think BD71815 was _originally_ developed
for i.MX6.

> 
> > + * Author: yanglsh@embest-tech.com
> > + *
> > + * 2020, 2021 Heavily modified by:
> 
> You should probably add a proper copyright.
Ok. I guess I can do so. Thanks!

> > + *	 Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > + */
> > +
> > +#ifndef _MFD_BD71815_H
> > +#define _MFD_BD71815_H
> > +
> > +#include <linux/regmap.h>

Best Regards
-- Matti Vaittinen
Lee Jones Jan. 26, 2021, 8:29 a.m. UTC | #8
On Mon, 25 Jan 2021, Matti Vaittinen wrote:

> Hello Lee,
> 
> Thanks again for the review!
> 
> On Mon, 2021-01-25 at 14:10 +0000, Lee Jones wrote:
> > On Tue, 19 Jan 2021, Matti Vaittinen wrote:
> > 
> > > Add core support for ROHM BD71815 Power Management IC.
> > > 
> > > The IC integrates regulators, a battery charger with a coulomb
> > > counter,
> > > a real-time clock (RTC), clock gate and general-purpose outputs
> > > (GPO).
> > > 
> > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > > ---
> > > Changes since v1:
> > >   - Used BIT() for better readability
> > >   - removed some unused definitions
> > > 
> > >  drivers/mfd/Kconfig              |  15 +-
> > >  drivers/mfd/rohm-bd71828.c       | 416 +++++++++++++++++++++--
> > >  include/linux/mfd/rohm-bd71815.h | 561
> > > +++++++++++++++++++++++++++++++
> > >  include/linux/mfd/rohm-bd71828.h |   3 +
> > >  4 files changed, 952 insertions(+), 43 deletions(-)
> > >  create mode 100644 include/linux/mfd/rohm-bd71815.h
> > > 
> > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > > index bdfce7b15621..59bfacb91898 100644
> > > --- a/drivers/mfd/Kconfig
> > > +++ b/drivers/mfd/Kconfig
> > > @@ -1984,19 +1984,20 @@ config MFD_ROHM_BD70528
> > >  	  charger.
> > >  
> > >  config MFD_ROHM_BD71828
> > > -	tristate "ROHM BD71828 Power Management IC"
> > > +	tristate "ROHM BD71828 and BD71815 Power Management IC"
> > >  	depends on I2C=y
> > >  	depends on OF
> > >  	select REGMAP_I2C
> > >  	select REGMAP_IRQ
> > >  	select MFD_CORE
> > >  	help
> > > -	  Select this option to get support for the ROHM BD71828 Power
> > > -	  Management IC. BD71828GW is a single-chip power management IC
> > > for
> > > -	  battery-powered portable devices. The IC integrates 7 buck
> > > -	  converters, 7 LDOs, and a 1500 mA single-cell linear charger.
> > > -	  Also included is a Coulomb counter, a real-time clock (RTC),
> > > and
> > > -	  a 32.768 kHz clock gate.
> > > +	  Select this option to get support for the ROHM BD71828 and
> > > BD71815
> > > +	  Power Management ICs. BD71828GW and BD71815AGW are single-
> > > chip power
> > > +	  management ICs mainly for battery-powered portable devices.
> > > +	  The BD71828 integrates 7 buck converters and 7 LDOs. The
> > > BD71815
> > > +	  has 5 bucks, 7 LDOs, and a boost for driving LEDs. Both ICs
> > > provide
> > > +	  also a single-cell linear charger, a Coulomb counter, a real-
> > > time
> > > +	  clock (RTC), GPIOs and a 32.768 kHz clock gate.
> > >  
> > >  config MFD_STM32_LPTIMER
> > >  	tristate "Support for STM32 Low-Power Timer"
> > > diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-
> > > bd71828.c
> > > index 210261d026f2..28b82477ce4c 100644
> > > --- a/drivers/mfd/rohm-bd71828.c
> > > +++ b/drivers/mfd/rohm-bd71828.c
> > > @@ -2,7 +2,7 @@
> > >  //
> > >  // Copyright (C) 2019 ROHM Semiconductors
> > >  //
> > > -// ROHM BD71828 PMIC driver
> > > +// ROHM BD71828/BD71815 PMIC driver
> > >  
> > >  #include <linux/gpio_keys.h>
> > >  #include <linux/i2c.h>
> > > @@ -11,7 +11,9 @@
> > >  #include <linux/ioport.h>
> > >  #include <linux/irq.h>
> > >  #include <linux/mfd/core.h>
> > > +#include <linux/mfd/rohm-bd71815.h>
> > >  #include <linux/mfd/rohm-bd71828.h>
> > > +#include <linux/mfd/rohm-generic.h>
> > >  #include <linux/module.h>
> > >  #include <linux/of_device.h>
> > >  #include <linux/regmap.h>
> > > @@ -29,12 +31,102 @@ static struct gpio_keys_platform_data
> > > bd71828_powerkey_data = {
> > >  	.name = "bd71828-pwrkey",
> > >  };
> > >  
> > > -static const struct resource rtc_irqs[] = {
> > > +static const struct resource bd71815_rtc_irqs[] = {
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC0, "bd71815-rtc-alm-0"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC1, "bd71815-rtc-alm-1"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC2, "bd71815-rtc-alm-2"),
> > > +};
> > > +
> > > +static const struct resource bd71828_rtc_irqs[] = {
> > >  	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC0, "bd71828-rtc-alm-0"),
> > >  	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC1, "bd71828-rtc-alm-1"),
> > >  	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC2, "bd71828-rtc-alm-2"),
> > >  };
> > >  
> > > +static struct resource bd71815_power_irqs[] = {
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_RMV, "bd71815-dcin-rmv"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_OUT, "bd71815-clps-out"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_IN, "bd71815-clps-in"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_OVP_RES, "bd71815-dcin-
> > > ovp-res"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_OVP_DET, "bd71815-dcin-
> > > ovp-det"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_MON_RES, "bd71815-dcin-
> > > mon-res"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_MON_DET, "bd71815-dcin-
> > > mon-det"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_UV_RES, "bd71815-vsys-uv-
> > > res"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_UV_DET, "bd71815-vsys-uv-
> > > det"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_LOW_RES, "bd71815-vsys-
> > > low-res"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_LOW_DET, "bd71815-vsys-
> > > low-det"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_RES, "bd71815-vsys-
> > > mon-res"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_RES, "bd71815-vsys-
> > > mon-det"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_WDG_TEMP, "bd71815-chg-
> > > wdg-temp"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_WDG_TIME, "bd71815-chg-
> > > wdg"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RECHARGE_RES, "bd71815-
> > > rechg-res"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RECHARGE_DET, "bd71815-
> > > rechg-det"),
> > > +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RANGED_TEMP_TRANSITION,
> > > +			     "bd71815-ranged-temp-transit"),
> > 
> > The new line limit is 100.  Feel free to run these out.
> 
> I learn new things every day it seems. This change is more than
> welcome!

Please see:

 bdc48fa11e46 ("checkpatch/coding-style: deprecate 80-column warning")

... for a more complete description.
Matti Vaittinen March 23, 2021, 9:57 a.m. UTC | #9
On Tue, 2021-01-19 at 12:07 +0100, Bartosz Golaszewski wrote:
> On Tue, Jan 19, 2021 at 8:23 AM Matti Vaittinen
> <matti.vaittinen@fi.rohmeurope.com> wrote:
> > Support GPO(s) found from ROHM BD71815 power management IC. The IC
> > has two
> > GPO pins but only one is properly documented in data-sheet. The
> > driver
> > exposes by default only the documented GPO. The second GPO is
> > connected to
> > E5 pin and is marked as GND in data-sheet. Control for this
> > undocumented
> > pin can be enabled using a special DT property.
> > 
> > This driver is derived from work by Peter Yang <
> > yanglsh@embest-tech.com>
> > although not so much of original is left.
> > 
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> 
> Hi Matti,
> 
> looks great, just a couple nits.

Hello Bartosz,

I think fixed all the nits to v3. Can I translate this to an ack? (I
will respin the series as I guess the regulator part may have fallen
through the cracks so I'd like to add the relevant acks :] )

Best Regards
	Matti Vaittinen
Bartosz Golaszewski March 26, 2021, 2:44 p.m. UTC | #10
On Tue, Mar 23, 2021 at 10:57 AM Vaittinen, Matti
<Matti.Vaittinen@fi.rohmeurope.com> wrote:
>
>
> On Tue, 2021-01-19 at 12:07 +0100, Bartosz Golaszewski wrote:
> > On Tue, Jan 19, 2021 at 8:23 AM Matti Vaittinen
> > <matti.vaittinen@fi.rohmeurope.com> wrote:
> > > Support GPO(s) found from ROHM BD71815 power management IC. The IC
> > > has two
> > > GPO pins but only one is properly documented in data-sheet. The
> > > driver
> > > exposes by default only the documented GPO. The second GPO is
> > > connected to
> > > E5 pin and is marked as GND in data-sheet. Control for this
> > > undocumented
> > > pin can be enabled using a special DT property.
> > >
> > > This driver is derived from work by Peter Yang <
> > > yanglsh@embest-tech.com>
> > > although not so much of original is left.
> > >
> > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> >
> > Hi Matti,
> >
> > looks great, just a couple nits.
>
> Hello Bartosz,
>
> I think fixed all the nits to v3. Can I translate this to an ack? (I
> will respin the series as I guess the regulator part may have fallen
> through the cracks so I'd like to add the relevant acks :] )
>
> Best Regards
>         Matti Vaittinen

Yes:

Acked-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>