* Re: [PATCH v7 1/4] mfd: bd71837: mfd driver for ROHM BD71837 PMIC
From: Lee Jones @ 2018-07-05 5:52 UTC (permalink / raw)
To: Enric Balletbo Serra
Cc: Dmitry Torokhov, matti.vaittinen, Michael Turquette, sboyd,
Rob Herring, Mark Rutland, Liam Girdwood, Mark Brown,
Matti Vaittinen, Arnd Bergmann, Sebastian Reichel, chenjh,
Andrey Smirnov, Linus Walleij, Kate Stewart, Heiko Stübner,
Greg Kroah-Hartman, linux-clk, devicetree@vger.kernel.org
In-Reply-To: <CAFqH_50UshTTc0Cv0wBhUoPdm0jZVgcvRKr3qzb=M-jCcMhdtA@mail.gmail.com>
On Wed, 04 Jul 2018, Enric Balletbo Serra wrote:
> Missatge de Dmitry Torokhov <dmitry.torokhov@gmail.com> del dia dc., 4
> de jul. 2018 a les 17:10:
> >
> > Hi Enric,
> >
> > On Tue, Jun 26, 2018 at 11:06:33AM +0200, Enric Balletbo Serra wrote:
> > > Hi Matti,
> > >
> > > Thanks for the patch, a few comments below, some are feedback I
> > > received when I sent some patches to this subsystem.
> > >
> > > Missatge de Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> del
> > > dia dt., 19 de juny 2018 a les 12:57:
> > > > +};
> > > > +
> > > > +/* bd71837 multi function cells */
> > > > +static struct mfd_cell bd71837_mfd_cells[] = {
> > > > + {
> > > > + .name = "bd71837-clk",
> > > > + }, {
> > > > + .name = "bd718xx-pwrkey",
> > > > + .resources = &irqs[0],
> > > > + .num_resources = ARRAY_SIZE(irqs),
> > > > + }, {
> > > > + .name = "bd71837-pmic",
> > > > + },
> > > nit: no comma at the end
> >
> > Actually, trailing comma is preferred on structures/arrays without
> > sentinels, because if one needs to add a new entry/new member, then in
> > the diff there will have only one new line added, instead of one line
> > being changed (adding now necessary comma) and one added.
[...]
> Dmitry, I really appreciate this kind of comments, especially when
> there is a justification, so I can learn more and more to do better
> patches.
+1
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH v9 0/2] mfd/regulator/clk/input: bd71837: ROHM BD71837 PMIC driver
From: Matti Vaittinen @ 2018-07-05 7:45 UTC (permalink / raw)
To: mturquette, robh+dt, mark.rutland, lee.jones, lgirdwood, broonie,
mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo
Cc: sboyd, linux-clk, devicetree, linux-kernel, linux-input,
mikko.mutanen, heikki.haikola
Patch series adding support for ROHM BD71837 PMIC.
BD71837 is a programmable Power Management IC for powering single-core,
dual-core, and quad-core SoC’s such as NXP-i.MX 8M. It is optimized for
low BOM cost and compact solution footprint. It integrates 8 buck
regulators and 7 LDO’s to provide all the power rails required by the
SoC and the commonly used peripherals.
This is reduced set of patches containing only the MFD and devicetree
bindings. This enables alrady applied regulator part and power button
support using gpio-keys. Clock and reset support will be sent as separate
set of patches - possibly only after my vacations. Sorry for longish delay
which will follow.
Changelog v9
Fixes initiated by feedback from Enric Balletbo Serra, Lee and Dmitry.
- Simplified control flow for probe
- Removed accidentally left squash commit message
- Some comma presence toggling after last member in an array =)
- Styling of multi line comments.
Changelog v8
- Dropped clk-bd71837 from series (will send later)
- Dropped bd718xx-pwrkey driver and used gpio_keys instead.
- Added power-button short/long press duration configuratio to MFD
- Cleaned MFD driver according to comments from Enric Balletbo Serra.
(used devm, removed unnecessary header inclusions, removed redundant
assignment, styling issues, allow building MFD part as module, fixed
license mismatch).
Changelog v7
- patch 1: Cleaned MFD probe since MFD no longer directly reads DT
properties.
- patch 1/4: Moved power-key related definitions from powerkey patch (4)
to MFD patch (1) so that powerkey can be applied independently
- Patch 2 is unchanged.
- patch 3: Added missing allocation check back to clk probe
Changelog v6
- Added power-key input driver
Based on feedback from Rob Herring and Stephen Boyd
- Added link to datasheet
- Removed interrupt-controller from DT and fixed binding document
- clk styling fixes
- remove clkdev usage
- add clk bindings to MFD documentation
- removed clk binding document
Changelog v5
- dropped regulator patches which are already applied to Mark's tree
Based on feedback from Rob Herring and Stephen Boyd
- mfd bindings: explain why this can be interrupt-controller
- mfd bindings: describe interrupts better
- mfd bindings: require one cell interrupt specifier
- mfd bindings: use generic node names in example
- mfd driver: ack masked interrupt once at init
- clk bindings: use generic node names in example
- clk driver: use devm
- clk driver: use of_clk_add_hw_provider
- clk driver: change severity of print and how prints are emitted at
probe error path.
- clk driver: dropped forward declared functions
- clk configs: drop unnecessary dependencies
- clk driver: other styling issues
- mfd/clk DT: drop clk node.
Changelog v4
- remove mutex from regulator state check as core prevents simultaneous
accesses
- allow voltage change for bucks 1 to 4 when regulator is enabled
- fix indentiation problems
- properly correct SPDX comments
Changelog v3
- kill unused variable
- kill unused definitions
- use REGMAP_IRQ_REG
Changelog v2
Based on feedback from Mark Brown
- Squashed code and buildfile changes to same patch
- Fixed some styling issues
- Changed SPDX comments to CPP style
- Error out if voltage is changed when regulator is enabled instead of
Disabling the regulator for duration of change
- Use devm_regulator_register
- Remove compatible usage from regulators - use parent dev for config
- Add a note about using regulator-boot-on for BUCK6 and 7
- fixed warnings from kbuild test robot
patch 1:
MFD driver and definitions bringing interrupt support and
enabling clk, regulator and input subsystems.
patch 2:
MFD driver DT bindings
This patch series is based on for-mfd-next
---
Matti Vaittinen (2):
mfd: bd71837: mfd driver for ROHM BD71837 PMIC
mfd: bd71837: Devicetree bindings for ROHM BD71837 PMIC
.../devicetree/bindings/mfd/rohm,bd71837-pmic.txt | 67 ++++
drivers/mfd/Kconfig | 13 +
drivers/mfd/Makefile | 1 +
drivers/mfd/bd71837.c | 215 +++++++++++
include/linux/mfd/bd71837.h | 391 +++++++++++++++++++++
5 files changed, 687 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
create mode 100644 drivers/mfd/bd71837.c
create mode 100644 include/linux/mfd/bd71837.h
--
2.14.3
^ permalink raw reply
* [PATCH v9 1/2] mfd: bd71837: mfd driver for ROHM BD71837 PMIC
From: Matti Vaittinen @ 2018-07-05 7:46 UTC (permalink / raw)
To: mturquette, robh+dt, mark.rutland, lee.jones, lgirdwood, broonie,
mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo
Cc: sboyd, linux-clk, devicetree, linux-kernel, linux-input,
mikko.mutanen, heikki.haikola
In-Reply-To: <cover.1530776326.git.matti.vaittinen@fi.rohmeurope.com>
ROHM BD71837 PMIC MFD driver providing interrupts and support
for three subsystems:
- clk
- Regulators
- input/power-key
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
drivers/mfd/Kconfig | 13 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/bd71837.c | 215 ++++++++++++++++++++++++
include/linux/mfd/bd71837.h | 391 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 620 insertions(+)
create mode 100644 drivers/mfd/bd71837.c
create mode 100644 include/linux/mfd/bd71837.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index b860eb5aa194..d01a279f5e4a 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1787,6 +1787,19 @@ config MFD_STW481X
in various ST Microelectronics and ST-Ericsson embedded
Nomadik series.
+config MFD_BD71837
+ tristate "BD71837 Power Management chip"
+ 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 BD71837
+ Power Management chips. BD71837 is designed to power processors like
+ NXP i.MX8. It contains 8 BUCK outputs and 7 LDOs, voltage monitoring
+ and emergency shut down as well as 32,768KHz clock output.
+
config MFD_STM32_LPTIMER
tristate "Support for STM32 Low-Power Timer"
depends on (ARCH_STM32 && OF) || COMPILE_TEST
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e9fd20dba18d..09dc9eb3782c 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -227,4 +227,5 @@ obj-$(CONFIG_MFD_STM32_TIMERS) += stm32-timers.o
obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o
obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
+obj-$(CONFIG_MFD_BD71837) += bd71837.o
diff --git a/drivers/mfd/bd71837.c b/drivers/mfd/bd71837.c
new file mode 100644
index 000000000000..677097bcea17
--- /dev/null
+++ b/drivers/mfd/bd71837.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (C) 2018 ROHM Semiconductors
+// bd71837.c -- ROHM BD71837MWV mfd driver
+//
+// Datasheet available from
+// https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
+
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/bd71837.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/gpio_keys.h>
+
+static struct gpio_keys_button btns[] = {
+ {
+ .code = KEY_POWER,
+ .gpio = -1,
+ .type = EV_KEY,
+ },
+};
+
+static struct gpio_keys_platform_data bd718xx_powerkey_data = {
+ .buttons = &btns[0],
+ .nbuttons = ARRAY_SIZE(btns),
+ .name = "bd718xx-pwrkey",
+};
+
+static struct mfd_cell bd71837_mfd_cells[] = {
+ {
+ .name = "bd71837-clk",
+ }, {
+ .name = "gpio-keys",
+ .platform_data = &bd718xx_powerkey_data,
+ .pdata_size = sizeof(bd718xx_powerkey_data),
+ }, {
+ .name = "bd71837-pmic",
+ },
+};
+
+static const struct regmap_irq bd71837_irqs[] = {
+ REGMAP_IRQ_REG(BD71837_INT_SWRST, 0, BD71837_INT_SWRST_MASK),
+ REGMAP_IRQ_REG(BD71837_INT_PWRBTN_S, 0, BD71837_INT_PWRBTN_S_MASK),
+ REGMAP_IRQ_REG(BD71837_INT_PWRBTN_L, 0, BD71837_INT_PWRBTN_L_MASK),
+ REGMAP_IRQ_REG(BD71837_INT_PWRBTN, 0, BD71837_INT_PWRBTN_MASK),
+ REGMAP_IRQ_REG(BD71837_INT_WDOG, 0, BD71837_INT_WDOG_MASK),
+ REGMAP_IRQ_REG(BD71837_INT_ON_REQ, 0, BD71837_INT_ON_REQ_MASK),
+ REGMAP_IRQ_REG(BD71837_INT_STBY_REQ, 0, BD71837_INT_STBY_REQ_MASK),
+};
+
+static struct regmap_irq_chip bd71837_irq_chip = {
+ .name = "bd71837-irq",
+ .irqs = bd71837_irqs,
+ .num_irqs = ARRAY_SIZE(bd71837_irqs),
+ .num_regs = 1,
+ .irq_reg_stride = 1,
+ .status_base = BD71837_REG_IRQ,
+ .mask_base = BD71837_REG_MIRQ,
+ .ack_base = BD71837_REG_IRQ,
+ .init_ack_masked = true,
+ .mask_invert = false,
+};
+
+static const struct regmap_range pmic_status_range = {
+ .range_min = BD71837_REG_IRQ,
+ .range_max = BD71837_REG_POW_STATE,
+};
+
+static const struct regmap_access_table volatile_regs = {
+ .yes_ranges = &pmic_status_range,
+ .n_yes_ranges = 1,
+};
+
+static const struct regmap_config bd71837_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .volatile_table = &volatile_regs,
+ .max_register = BD71837_MAX_REGISTER - 1,
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static const struct of_device_id bd71837_of_match[] = {
+ { .compatible = "rohm,bd71837", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, bd71837_of_match);
+
+static int bd71837_i2c_probe(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+ struct bd71837 *bd71837;
+ struct bd71837_board *board_info;
+ int ret = -EINVAL;
+
+ bd71837 = devm_kzalloc(&i2c->dev, sizeof(struct bd71837), GFP_KERNEL);
+
+ if (!bd71837)
+ return -ENOMEM;
+
+ board_info = dev_get_platdata(&i2c->dev);
+
+ if (!board_info)
+ bd71837->chip_irq = i2c->irq;
+ else
+ bd71837->chip_irq = board_info->gpio_intr;
+
+ if (!bd71837->chip_irq) {
+ dev_err(&i2c->dev, "No IRQ configured\n");
+ goto err_out;
+ }
+
+ bd71837->dev = &i2c->dev;
+ bd71837->i2c_client = i2c;
+ i2c_set_clientdata(i2c, bd71837);
+
+ bd71837->regmap = devm_regmap_init_i2c(i2c, &bd71837_regmap_config);
+ if (IS_ERR(bd71837->regmap)) {
+ ret = PTR_ERR(bd71837->regmap);
+ dev_err(&i2c->dev, "regmap initialization failed\n");
+ goto err_out;
+ }
+
+ ret = bd71837_reg_read(bd71837, BD71837_REG_REV);
+ if (ret < 0) {
+ dev_err(&i2c->dev, "Read BD71837_REG_DEVICE failed\n");
+ goto err_out;
+ }
+
+ ret = devm_regmap_add_irq_chip(&i2c->dev, bd71837->regmap,
+ bd71837->chip_irq, IRQF_ONESHOT, 0,
+ &bd71837_irq_chip, &bd71837->irq_data);
+ if (ret < 0) {
+ dev_err(&i2c->dev, "Failed to add irq_chip\n");
+ goto err_out;
+ }
+
+ ret = regmap_update_bits(bd71837->regmap,
+ BD71837_REG_PWRONCONFIG0,
+ BD718XX_PWRBTN_PRESS_DURATION_MASK,
+ BD718XX_PWRBTN_SHORT_PRESS_10MS);
+ if (ret < 0) {
+ dev_err(&i2c->dev, "Failed to configure button short press timeout\n");
+ goto err_out;
+ }
+
+ /*
+ * According to BD71847 datasheet the HW default for long press
+ * detection is 10ms. So lets change it to 10 sec so we can actually
+ * get the short push and allow gracefull shut down
+ */
+ ret = regmap_update_bits(bd71837->regmap,
+ BD71837_REG_PWRONCONFIG1,
+ BD718XX_PWRBTN_PRESS_DURATION_MASK,
+ BD718XX_PWRBTN_LONG_PRESS_10S);
+
+ if (ret < 0) {
+ dev_err(&i2c->dev, "Failed to configure button long press timeout\n");
+ goto err_out;
+ }
+
+ btns[0].irq = regmap_irq_get_virq(bd71837->irq_data,
+ BD71837_INT_PWRBTN_S);
+
+ if (btns[0].irq < 0) {
+ ret = btns[0].irq;
+ dev_err(&i2c->dev, "Failed to get the IRQ\n");
+ goto err_out;
+ }
+
+ ret = devm_mfd_add_devices(bd71837->dev, PLATFORM_DEVID_AUTO,
+ bd71837_mfd_cells,
+ ARRAY_SIZE(bd71837_mfd_cells), NULL, 0,
+ regmap_irq_get_domain(bd71837->irq_data));
+ if (ret)
+ dev_err(&i2c->dev, "Failed to create subdevices\n");
+err_out:
+
+ return ret;
+}
+
+static const struct i2c_device_id bd71837_i2c_id[] = {
+ { .name = "bd71837", },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, bd71837_i2c_id);
+
+static struct i2c_driver bd71837_i2c_driver = {
+ .driver = {
+ .name = "bd71837-mfd",
+ .of_match_table = bd71837_of_match,
+ },
+ .probe = bd71837_i2c_probe,
+ .id_table = bd71837_i2c_id,
+};
+
+static int __init bd71837_i2c_init(void)
+{
+ return i2c_add_driver(&bd71837_i2c_driver);
+}
+/*
+ * init early so consumer devices can complete system boot
+ */
+subsys_initcall(bd71837_i2c_init);
+
+static void __exit bd71837_i2c_exit(void)
+{
+ i2c_del_driver(&bd71837_i2c_driver);
+}
+module_exit(bd71837_i2c_exit);
+
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
+MODULE_DESCRIPTION("BD71837 chip multi-function driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/bd71837.h b/include/linux/mfd/bd71837.h
new file mode 100644
index 000000000000..10b0dfe90f27
--- /dev/null
+++ b/include/linux/mfd/bd71837.h
@@ -0,0 +1,391 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Copyright (C) 2018 ROHM Semiconductors */
+
+/*
+ * ROHM BD71837MWV header file
+ */
+
+#ifndef __LINUX_MFD_BD71837_H__
+#define __LINUX_MFD_BD71837_H__
+
+#include <linux/regmap.h>
+
+enum {
+ BD71837_BUCK1 = 0,
+ BD71837_BUCK2,
+ BD71837_BUCK3,
+ BD71837_BUCK4,
+ BD71837_BUCK5,
+ BD71837_BUCK6,
+ BD71837_BUCK7,
+ BD71837_BUCK8,
+ BD71837_LDO1,
+ BD71837_LDO2,
+ BD71837_LDO3,
+ BD71837_LDO4,
+ BD71837_LDO5,
+ BD71837_LDO6,
+ BD71837_LDO7,
+ BD71837_REGULATOR_CNT,
+};
+
+#define BD71837_BUCK1_VOLTAGE_NUM 0x40
+#define BD71837_BUCK2_VOLTAGE_NUM 0x40
+#define BD71837_BUCK3_VOLTAGE_NUM 0x40
+#define BD71837_BUCK4_VOLTAGE_NUM 0x40
+
+#define BD71837_BUCK5_VOLTAGE_NUM 0x08
+#define BD71837_BUCK6_VOLTAGE_NUM 0x04
+#define BD71837_BUCK7_VOLTAGE_NUM 0x08
+#define BD71837_BUCK8_VOLTAGE_NUM 0x40
+
+#define BD71837_LDO1_VOLTAGE_NUM 0x04
+#define BD71837_LDO2_VOLTAGE_NUM 0x02
+#define BD71837_LDO3_VOLTAGE_NUM 0x10
+#define BD71837_LDO4_VOLTAGE_NUM 0x10
+#define BD71837_LDO5_VOLTAGE_NUM 0x10
+#define BD71837_LDO6_VOLTAGE_NUM 0x10
+#define BD71837_LDO7_VOLTAGE_NUM 0x10
+
+enum {
+ BD71837_REG_REV = 0x00,
+ BD71837_REG_SWRESET = 0x01,
+ BD71837_REG_I2C_DEV = 0x02,
+ BD71837_REG_PWRCTRL0 = 0x03,
+ BD71837_REG_PWRCTRL1 = 0x04,
+ BD71837_REG_BUCK1_CTRL = 0x05,
+ BD71837_REG_BUCK2_CTRL = 0x06,
+ BD71837_REG_BUCK3_CTRL = 0x07,
+ BD71837_REG_BUCK4_CTRL = 0x08,
+ BD71837_REG_BUCK5_CTRL = 0x09,
+ BD71837_REG_BUCK6_CTRL = 0x0A,
+ BD71837_REG_BUCK7_CTRL = 0x0B,
+ BD71837_REG_BUCK8_CTRL = 0x0C,
+ BD71837_REG_BUCK1_VOLT_RUN = 0x0D,
+ BD71837_REG_BUCK1_VOLT_IDLE = 0x0E,
+ BD71837_REG_BUCK1_VOLT_SUSP = 0x0F,
+ BD71837_REG_BUCK2_VOLT_RUN = 0x10,
+ BD71837_REG_BUCK2_VOLT_IDLE = 0x11,
+ BD71837_REG_BUCK3_VOLT_RUN = 0x12,
+ BD71837_REG_BUCK4_VOLT_RUN = 0x13,
+ BD71837_REG_BUCK5_VOLT = 0x14,
+ BD71837_REG_BUCK6_VOLT = 0x15,
+ BD71837_REG_BUCK7_VOLT = 0x16,
+ BD71837_REG_BUCK8_VOLT = 0x17,
+ BD71837_REG_LDO1_VOLT = 0x18,
+ BD71837_REG_LDO2_VOLT = 0x19,
+ BD71837_REG_LDO3_VOLT = 0x1A,
+ BD71837_REG_LDO4_VOLT = 0x1B,
+ BD71837_REG_LDO5_VOLT = 0x1C,
+ BD71837_REG_LDO6_VOLT = 0x1D,
+ BD71837_REG_LDO7_VOLT = 0x1E,
+ BD71837_REG_TRANS_COND0 = 0x1F,
+ BD71837_REG_TRANS_COND1 = 0x20,
+ BD71837_REG_VRFAULTEN = 0x21,
+ BD71837_REG_MVRFLTMASK0 = 0x22,
+ BD71837_REG_MVRFLTMASK1 = 0x23,
+ BD71837_REG_MVRFLTMASK2 = 0x24,
+ BD71837_REG_RCVCFG = 0x25,
+ BD71837_REG_RCVNUM = 0x26,
+ BD71837_REG_PWRONCONFIG0 = 0x27,
+ BD71837_REG_PWRONCONFIG1 = 0x28,
+ BD71837_REG_RESETSRC = 0x29,
+ BD71837_REG_MIRQ = 0x2A,
+ BD71837_REG_IRQ = 0x2B,
+ BD71837_REG_IN_MON = 0x2C,
+ BD71837_REG_POW_STATE = 0x2D,
+ BD71837_REG_OUT32K = 0x2E,
+ BD71837_REG_REGLOCK = 0x2F,
+ BD71837_REG_OTPVER = 0xFF,
+ BD71837_MAX_REGISTER = 0x100,
+};
+
+#define REGLOCK_PWRSEQ 0x1
+#define REGLOCK_VREG 0x10
+
+/* Generic BUCK control masks */
+#define BD71837_BUCK_SEL 0x02
+#define BD71837_BUCK_EN 0x01
+#define BD71837_BUCK_RUN_ON 0x04
+
+/* Generic LDO masks */
+#define BD71837_LDO_SEL 0x80
+#define BD71837_LDO_EN 0x40
+
+/* BD71837 BUCK ramp rate CTRL reg bits */
+#define BUCK_RAMPRATE_MASK 0xC0
+#define BUCK_RAMPRATE_10P00MV 0x0
+#define BUCK_RAMPRATE_5P00MV 0x1
+#define BUCK_RAMPRATE_2P50MV 0x2
+#define BUCK_RAMPRATE_1P25MV 0x3
+
+/* BD71837_REG_BUCK1_VOLT_RUN bits */
+#define BUCK1_RUN_MASK 0x3F
+#define BUCK1_RUN_DEFAULT 0x14
+
+/* BD71837_REG_BUCK1_VOLT_SUSP bits */
+#define BUCK1_SUSP_MASK 0x3F
+#define BUCK1_SUSP_DEFAULT 0x14
+
+/* BD71837_REG_BUCK1_VOLT_IDLE bits */
+#define BUCK1_IDLE_MASK 0x3F
+#define BUCK1_IDLE_DEFAULT 0x14
+
+/* BD71837_REG_BUCK2_VOLT_RUN bits */
+#define BUCK2_RUN_MASK 0x3F
+#define BUCK2_RUN_DEFAULT 0x1E
+
+/* BD71837_REG_BUCK2_VOLT_IDLE bits */
+#define BUCK2_IDLE_MASK 0x3F
+#define BUCK2_IDLE_DEFAULT 0x14
+
+/* BD71837_REG_BUCK3_VOLT_RUN bits */
+#define BUCK3_RUN_MASK 0x3F
+#define BUCK3_RUN_DEFAULT 0x1E
+
+/* BD71837_REG_BUCK4_VOLT_RUN bits */
+#define BUCK4_RUN_MASK 0x3F
+#define BUCK4_RUN_DEFAULT 0x1E
+
+/* BD71837_REG_BUCK5_VOLT bits */
+#define BUCK5_MASK 0x07
+#define BUCK5_DEFAULT 0x02
+
+/* BD71837_REG_BUCK6_VOLT bits */
+#define BUCK6_MASK 0x03
+#define BUCK6_DEFAULT 0x03
+
+/* BD71837_REG_BUCK7_VOLT bits */
+#define BUCK7_MASK 0x07
+#define BUCK7_DEFAULT 0x03
+
+/* BD71837_REG_BUCK8_VOLT bits */
+#define BUCK8_MASK 0x3F
+#define BUCK8_DEFAULT 0x1E
+
+/* BD71837_REG_IRQ bits */
+#define IRQ_SWRST 0x40
+#define IRQ_PWRON_S 0x20
+#define IRQ_PWRON_L 0x10
+#define IRQ_PWRON 0x08
+#define IRQ_WDOG 0x04
+#define IRQ_ON_REQ 0x02
+#define IRQ_STBY_REQ 0x01
+
+/* BD71837_REG_OUT32K bits */
+#define BD71837_OUT32K_EN 0x01
+
+/* BD71837 gated clock rate */
+#define BD71837_CLK_RATE 32768
+
+/* BD71837 irqs */
+enum {
+ BD71837_INT_STBY_REQ,
+ BD71837_INT_ON_REQ,
+ BD71837_INT_WDOG,
+ BD71837_INT_PWRBTN,
+ BD71837_INT_PWRBTN_L,
+ BD71837_INT_PWRBTN_S,
+ BD71837_INT_SWRST
+};
+
+/* BD71837 interrupt masks */
+#define BD71837_INT_SWRST_MASK 0x40
+#define BD71837_INT_PWRBTN_S_MASK 0x20
+#define BD71837_INT_PWRBTN_L_MASK 0x10
+#define BD71837_INT_PWRBTN_MASK 0x8
+#define BD71837_INT_WDOG_MASK 0x4
+#define BD71837_INT_ON_REQ_MASK 0x2
+#define BD71837_INT_STBY_REQ_MASK 0x1
+
+/* BD71837_REG_LDO1_VOLT bits */
+#define LDO1_MASK 0x03
+
+/* BD71837_REG_LDO1_VOLT bits */
+#define LDO2_MASK 0x20
+
+/* BD71837_REG_LDO3_VOLT bits */
+#define LDO3_MASK 0x0F
+
+/* BD71837_REG_LDO4_VOLT bits */
+#define LDO4_MASK 0x0F
+
+/* BD71837_REG_LDO5_VOLT bits */
+#define LDO5_MASK 0x0F
+
+/* BD71837_REG_LDO6_VOLT bits */
+#define LDO6_MASK 0x0F
+
+/* BD71837_REG_LDO7_VOLT bits */
+#define LDO7_MASK 0x0F
+
+/* Register write induced reset settings */
+
+/* Even though the bit zero is not SWRESET type we still want to write zero
+ * to it when changing type. Bit zero is 'SWRESET' trigger bit and if we
+ * write 1 to it we will trigger the action. So always write 0 to it when
+ * changning SWRESET action - no matter what we read from it.
+ */
+#define BD71837_SWRESET_TYPE_MASK 7
+#define BD71837_SWRESET_TYPE_DISABLED 0
+#define BD71837_SWRESET_TYPE_COLD 4
+#define BD71837_SWRESET_TYPE_WARM 6
+
+#define BD71837_SWRESET_RESET_MASK 1
+#define BD71837_SWRESET_RESET 1
+
+/* Poweroff state transition conditions */
+
+#define BD718XX_ON_REQ_POWEROFF_MASK 1
+#define BD718XX_SWRESET_POWEROFF_MASK 2
+#define BD718XX_WDOG_POWEROFF_MASK 4
+#define BD718XX_KEY_L_POWEROFF_MASK 8
+
+#define BD718XX_POWOFF_TO_SNVS 0
+#define BD718XX_POWOFF_TO_RDY 0xF
+
+#define BD718XX_POWOFF_TIME_MASK 0xF0
+enum {
+ BD718XX_POWOFF_TIME_5MS = 0,
+ BD718XX_POWOFF_TIME_10MS,
+ BD718XX_POWOFF_TIME_15MS,
+ BD718XX_POWOFF_TIME_20MS,
+ BD718XX_POWOFF_TIME_25MS,
+ BD718XX_POWOFF_TIME_30MS,
+ BD718XX_POWOFF_TIME_35MS,
+ BD718XX_POWOFF_TIME_40MS,
+ BD718XX_POWOFF_TIME_45MS,
+ BD718XX_POWOFF_TIME_50MS,
+ BD718XX_POWOFF_TIME_75MS,
+ BD718XX_POWOFF_TIME_100MS,
+ BD718XX_POWOFF_TIME_250MS,
+ BD718XX_POWOFF_TIME_500MS,
+ BD718XX_POWOFF_TIME_750MS,
+ BD718XX_POWOFF_TIME_1500MS
+};
+
+/* Poweron sequence state transition conditions */
+
+#define BD718XX_RDY_TO_SNVS_MASK 0xF
+#define BD718XX_SNVS_TO_RUN_MASK 0xF0
+
+#define BD718XX_PWR_TRIG_KEY_L 1
+#define BD718XX_PWR_TRIG_KEY_S 2
+#define BD718XX_PWR_TRIG_PMIC_ON 4
+#define BD718XX_PWR_TRIG_VSYS_UVLO 8
+#define BD718XX_RDY_TO_SNVS_SIFT 0
+#define BD718XX_SNVS_TO_RUN_SIFT 4
+
+
+#define BD718XX_PWRBTN_PRESS_DURATION_MASK 0xF
+
+/* Timeout value for detecting short press */
+
+enum {
+ BD718XX_PWRBTN_SHORT_PRESS_10MS = 0,
+ BD718XX_PWRBTN_SHORT_PRESS_500MS,
+ BD718XX_PWRBTN_SHORT_PRESS_1000MS,
+ BD718XX_PWRBTN_SHORT_PRESS_1500MS,
+ BD718XX_PWRBTN_SHORT_PRESS_2000MS,
+ BD718XX_PWRBTN_SHORT_PRESS_2500MS,
+ BD718XX_PWRBTN_SHORT_PRESS_3000MS,
+ BD718XX_PWRBTN_SHORT_PRESS_3500MS,
+ BD718XX_PWRBTN_SHORT_PRESS_4000MS,
+ BD718XX_PWRBTN_SHORT_PRESS_4500MS,
+ BD718XX_PWRBTN_SHORT_PRESS_5000MS,
+ BD718XX_PWRBTN_SHORT_PRESS_5500MS,
+ BD718XX_PWRBTN_SHORT_PRESS_6000MS,
+ BD718XX_PWRBTN_SHORT_PRESS_6500MS,
+ BD718XX_PWRBTN_SHORT_PRESS_7000MS,
+ BD718XX_PWRBTN_SHORT_PRESS_7500MS
+};
+
+/* Timeout value for detecting LONG press */
+
+enum {
+ BD718XX_PWRBTN_LONG_PRESS_10MS = 0,
+ BD718XX_PWRBTN_LONG_PRESS_1S,
+ BD718XX_PWRBTN_LONG_PRESS_2S,
+ BD718XX_PWRBTN_LONG_PRESS_3S,
+ BD718XX_PWRBTN_LONG_PRESS_4S,
+ BD718XX_PWRBTN_LONG_PRESS_5S,
+ BD718XX_PWRBTN_LONG_PRESS_6S,
+ BD718XX_PWRBTN_LONG_PRESS_7S,
+ BD718XX_PWRBTN_LONG_PRESS_8S,
+ BD718XX_PWRBTN_LONG_PRESS_9S,
+ BD718XX_PWRBTN_LONG_PRESS_10S,
+ BD718XX_PWRBTN_LONG_PRESS_11S,
+ BD718XX_PWRBTN_LONG_PRESS_12S,
+ BD718XX_PWRBTN_LONG_PRESS_13S,
+ BD718XX_PWRBTN_LONG_PRESS_14S,
+ BD718XX_PWRBTN_LONG_PRESS_15S
+};
+
+
+
+struct bd71837;
+struct bd71837_pmic;
+struct bd71837_clk;
+struct bd718xx_pwrkey;
+
+/*
+ * Board platform data may be used to initialize regulators.
+ */
+
+struct bd71837_board {
+ struct regulator_init_data *init_data[BD71837_REGULATOR_CNT];
+ int gpio_intr;
+};
+
+struct bd71837 {
+ struct device *dev;
+ struct i2c_client *i2c_client;
+ struct regmap *regmap;
+ unsigned long int id;
+
+ int chip_irq;
+ struct regmap_irq_chip_data *irq_data;
+
+ struct bd71837_pmic *pmic;
+ struct bd71837_clk *clk;
+ struct bd718xx_pwrkey *pwrkey;
+};
+
+/*
+ * bd71837 sub-driver chip access routines
+ */
+
+static inline int bd71837_reg_read(struct bd71837 *bd71837, u8 reg)
+{
+ int r, val;
+
+ r = regmap_read(bd71837->regmap, reg, &val);
+ if (r < 0)
+ return r;
+ return val;
+}
+
+static inline int bd71837_reg_write(struct bd71837 *bd71837, u8 reg,
+ unsigned int val)
+{
+ return regmap_write(bd71837->regmap, reg, val);
+}
+
+static inline int bd71837_set_bits(struct bd71837 *bd71837, u8 reg, u8 mask)
+{
+ return regmap_update_bits(bd71837->regmap, reg, mask, mask);
+}
+
+static inline int bd71837_clear_bits(struct bd71837 *bd71837, u8 reg,
+ u8 mask)
+{
+ return regmap_update_bits(bd71837->regmap, reg, mask, 0);
+}
+
+static inline int bd71837_update_bits(struct bd71837 *bd71837, u8 reg,
+ u8 mask, u8 val)
+{
+ return regmap_update_bits(bd71837->regmap, reg, mask, val);
+}
+
+#endif /* __LINUX_MFD_BD71837_H__ */
--
2.14.3
^ permalink raw reply related
* [PATCH v9 2/2] mfd: bd71837: Devicetree bindings for ROHM BD71837 PMIC
From: Matti Vaittinen @ 2018-07-05 7:48 UTC (permalink / raw)
To: mturquette, robh+dt, mark.rutland, lee.jones, lgirdwood, broonie,
mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo
Cc: sboyd, linux-clk, devicetree, linux-kernel, linux-input,
mikko.mutanen, heikki.haikola
In-Reply-To: <cover.1530776326.git.matti.vaittinen@fi.rohmeurope.com>
Document devicetree bindings for ROHM BD71837 PMIC MFD.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/mfd/rohm,bd71837-pmic.txt | 67 ++++++++++++++++++++++
1 file changed, 67 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
new file mode 100644
index 000000000000..67f2616288d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
@@ -0,0 +1,67 @@
+* ROHM BD71837 Power Management Integrated Circuit bindings
+
+BD71837MWV is a programmable Power Management IC for powering single-core,
+dual-core, and quad-core SoC’s such as NXP-i.MX 8M. It is optimized for
+low BOM cost and compact solution footprint. It integrates 8 Buck
+egulators and 7 LDO’s to provide all the power rails required by the SoC and
+the commonly used peripherals.
+
+Datasheet for PMIC is available at:
+https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
+
+Required properties:
+ - compatible : Should be "rohm,bd71837".
+ - reg : I2C slave address.
+ - interrupt-parent : Phandle to the parent interrupt controller.
+ - interrupts : The interrupt line the device is connected to.
+ - clocks : The parent clock connected to PMIC. If this is missng
+ 32768 KHz clock is assumed.
+ - #clock-cells : Should be 0
+ - regulators: : List of child nodes that specify the regulators
+ Please see ../regulator/rohm,bd71837-regulator.txt
+
+Optional properties:
+- clock-output-names : Should contain name for output clock.
+
+Example:
+
+ /* external oscillator node */
+ osc: oscillator {
+ compatible = "fixed-clock";
+ #clock-cells = <1>;
+ clock-frequency = <32768>;
+ clock-output-names = "osc";
+ };
+
+ /* PMIC node */
+
+ pmic: pmic@4b {
+ compatible = "rohm,bd71837";
+ reg = <0x4b>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <29 GPIO_ACTIVE_LOW>;
+ interrupt-names = "irq";
+ #clock-cells = <0>;
+ clocks = <&osc 0>;
+ clock-output-names = "bd71837-32k-out";
+
+ regulators {
+ buck1: BUCK1 {
+ regulator-name = "buck1";
+ regulator-min-microvolt = <700000>;
+ regulator-max-microvolt = <1300000>;
+ regulator-boot-on;
+ regulator-ramp-delay = <1250>;
+ };
+ /* ... */
+ };
+ };
+
+ /* Clock consumer node */
+
+ foo@0 {
+ compatible = "bar,foo";
+ /* ... */
+ clock-names = "my-clock";
+ clocks = <&pmic>;
+ };
--
2.14.3
^ permalink raw reply related
* Re: [PATCH v7 1/4] mfd: bd71837: mfd driver for ROHM BD71837 PMIC
From: Matti Vaittinen @ 2018-07-05 7:56 UTC (permalink / raw)
To: Enric Balletbo Serra
Cc: Dmitry Torokhov, Michael Turquette, sboyd, Rob Herring,
Mark Rutland, Lee Jones, Liam Girdwood, Mark Brown,
Matti Vaittinen, Arnd Bergmann, Sebastian Reichel, chenjh,
Andrey Smirnov, Linus Walleij, Kate Stewart, Heiko Stübner,
Greg Kroah-Hartman, linux-clk, devicetree@vger.kernel.org
In-Reply-To: <CAFqH_50UshTTc0Cv0wBhUoPdm0jZVgcvRKr3qzb=M-jCcMhdtA@mail.gmail.com>
On Wed, Jul 04, 2018 at 06:57:39PM +0200, Enric Balletbo Serra wrote:
> Missatge de Dmitry Torokhov <dmitry.torokhov@gmail.com> del dia dc., 4
> de jul. 2018 a les 17:10:
> >
> > Hi Enric,
> >
> > On Tue, Jun 26, 2018 at 11:06:33AM +0200, Enric Balletbo Serra wrote:
> > > Hi Matti,
> > >
> > > Thanks for the patch, a few comments below, some are feedback I
> > > received when I sent some patches to this subsystem.
> > >
> > > Missatge de Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> del
> > > dia dt., 19 de juny 2018 a les 12:57:
> > > > +};
> > > > +
> > > > +/* bd71837 multi function cells */
> > > > +static struct mfd_cell bd71837_mfd_cells[] = {
> > > > + {
> > > > + .name = "bd71837-clk",
> > > > + }, {
> > > > + .name = "bd718xx-pwrkey",
> > > > + .resources = &irqs[0],
> > > > + .num_resources = ARRAY_SIZE(irqs),
> > > > + }, {
> > > > + .name = "bd71837-pmic",
> > > > + },
> > > nit: no comma at the end
> >
> > Actually, trailing comma is preferred on structures/arrays without
> > sentinels, because if one needs to add a new entry/new member, then in
> > the diff there will have only one new line added, instead of one line
> > being changed (adding now necessary comma) and one added.
> >
>
> Many thanks for sharing your knowledge! That looks to me a good
> reason.
So in this specific ecample leaving the comma does not help. The opening
brace for new array element would be added to same line where the comma
is, right? In any case - this was educating and makes perfect sense for
arrays with simple items. Thanks.
> Matti, I don't want to beat about the bush with these nitpicks. It is
> not my intention. So I'd say, do what the maintainer wants :)
>
No problem.
Br,
Matti Vaittinen
^ permalink raw reply
* Re: [PATCH v9 1/2] mfd: bd71837: mfd driver for ROHM BD71837 PMIC
From: Enric Balletbo Serra @ 2018-07-05 8:04 UTC (permalink / raw)
To: matti.vaittinen
Cc: Michael Turquette, Rob Herring, Mark Rutland, Lee Jones,
Liam Girdwood, Mark Brown, Matti Vaittinen, Arnd Bergmann,
Dmitry Torokhov, Sebastian Reichel, chenjh, Andrey Smirnov,
Linus Walleij, Kate Stewart, Heiko Stübner,
Greg Kroah-Hartman, sboyd, linux-clk, devicetree@vger.kernel.org,
linux-kernel
In-Reply-To: <a304164cc3c1b31c7c6b62f8e233ce321a82cdd6.1530776326.git.matti.vaittinen@fi.rohmeurope.com>
Hi Matti,
Missatge de Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> del
dia dj., 5 de jul. 2018 a les 9:46:
>
> ROHM BD71837 PMIC MFD driver providing interrupts and support
> for three subsystems:
> - clk
> - Regulators
> - input/power-key
>
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
> drivers/mfd/Kconfig | 13 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/bd71837.c | 215 ++++++++++++++++++++++++
> include/linux/mfd/bd71837.h | 391 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 620 insertions(+)
> create mode 100644 drivers/mfd/bd71837.c
> create mode 100644 include/linux/mfd/bd71837.h
>
(snip)
Looks good to me.
Reviewed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Best Regards,
Enric
^ permalink raw reply
* Re: [PATCH v9 1/2] mfd: bd71837: mfd driver for ROHM BD71837 PMIC
From: Lee Jones @ 2018-07-05 9:18 UTC (permalink / raw)
To: Matti Vaittinen
Cc: mturquette, robh+dt, mark.rutland, lgirdwood, broonie,
mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo,
sboyd, linux-clk, devicetree, linux-kernel, linux-input,
mikko.mutanen, heikki.haikola
In-Reply-To: <a304164cc3c1b31c7c6b62f8e233ce321a82cdd6.1530776326.git.matti.vaittinen@fi.rohmeurope.com>
On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> ROHM BD71837 PMIC MFD driver providing interrupts and support
> for three subsystems:
> - clk
> - Regulators
> - input/power-key
>
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
> drivers/mfd/Kconfig | 13 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/bd71837.c | 215 ++++++++++++++++++++++++
> include/linux/mfd/bd71837.h | 391 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 620 insertions(+)
> create mode 100644 drivers/mfd/bd71837.c
> create mode 100644 include/linux/mfd/bd71837.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index b860eb5aa194..d01a279f5e4a 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1787,6 +1787,19 @@ config MFD_STW481X
> in various ST Microelectronics and ST-Ericsson embedded
> Nomadik series.
>
> +config MFD_BD71837
It's be nice if you prefix this with the manufacturer. Rohm?
> + tristate "BD71837 Power Management chip"
Again, missing manufacturer.
This should also be "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 BD71837
> + Power Management chips. BD71837 is designed to power processors like
"chips" is slang. Should be "ICs".
> + NXP i.MX8. It contains 8 BUCK outputs and 7 LDOs, voltage monitoring
> + and emergency shut down as well as 32,768KHz clock output.
> +
> config MFD_STM32_LPTIMER
> tristate "Support for STM32 Low-Power Timer"
> depends on (ARCH_STM32 && OF) || COMPILE_TEST
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e9fd20dba18d..09dc9eb3782c 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -227,4 +227,5 @@ obj-$(CONFIG_MFD_STM32_TIMERS) += stm32-timers.o
> obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
> obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o
> obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
> +obj-$(CONFIG_MFD_BD71837) += bd71837.o
>
> diff --git a/drivers/mfd/bd71837.c b/drivers/mfd/bd71837.c
rohm-bd71837.c would be nicer.
> new file mode 100644
> index 000000000000..677097bcea17
> --- /dev/null
> +++ b/drivers/mfd/bd71837.c
> @@ -0,0 +1,215 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
'\n' here please.
> +// Copyright (C) 2018 ROHM Semiconductors
'\n' here please.
> +// bd71837.c -- ROHM BD71837MWV mfd driver
Remove the filename -- they have a habit of becoming incorrect.
Also, and 'MFD driver' isn't a thing -- this is a PMIC driver.
> +// Datasheet available from
> +// https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
> +
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/bd71837.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/gpio_keys.h>
Alphabetical please.
> +static struct gpio_keys_button btns[] = {
> + {
> + .code = KEY_POWER,
> + .gpio = -1,
> + .type = EV_KEY,
> + },
> +};
Does this need to be an array?
> +static struct gpio_keys_platform_data bd718xx_powerkey_data = {
> + .buttons = &btns[0],
Why not just "btns"?
Also, I don't see any reason not to just call this 'buttons'.
Or even, 'button' if there will only ever be one of them.
> + .nbuttons = ARRAY_SIZE(btns),
> + .name = "bd718xx-pwrkey",
> +};
> +
> +static struct mfd_cell bd71837_mfd_cells[] = {
> + {
> + .name = "bd71837-clk",
> + }, {
> + .name = "gpio-keys",
> + .platform_data = &bd718xx_powerkey_data,
> + .pdata_size = sizeof(bd718xx_powerkey_data),
> + }, {
> + .name = "bd71837-pmic",
> + },
> +};
Place the one line entries on one line, like this:
{ .name = "bd71837-pmic", },
... and if ordering is not important, place them at the bottom.
> +static const struct regmap_irq bd71837_irqs[] = {
> + REGMAP_IRQ_REG(BD71837_INT_SWRST, 0, BD71837_INT_SWRST_MASK),
> + REGMAP_IRQ_REG(BD71837_INT_PWRBTN_S, 0, BD71837_INT_PWRBTN_S_MASK),
> + REGMAP_IRQ_REG(BD71837_INT_PWRBTN_L, 0, BD71837_INT_PWRBTN_L_MASK),
> + REGMAP_IRQ_REG(BD71837_INT_PWRBTN, 0, BD71837_INT_PWRBTN_MASK),
> + REGMAP_IRQ_REG(BD71837_INT_WDOG, 0, BD71837_INT_WDOG_MASK),
> + REGMAP_IRQ_REG(BD71837_INT_ON_REQ, 0, BD71837_INT_ON_REQ_MASK),
> + REGMAP_IRQ_REG(BD71837_INT_STBY_REQ, 0, BD71837_INT_STBY_REQ_MASK),
> +};
> +
> +static struct regmap_irq_chip bd71837_irq_chip = {
> + .name = "bd71837-irq",
> + .irqs = bd71837_irqs,
> + .num_irqs = ARRAY_SIZE(bd71837_irqs),
> + .num_regs = 1,
> + .irq_reg_stride = 1,
> + .status_base = BD71837_REG_IRQ,
> + .mask_base = BD71837_REG_MIRQ,
> + .ack_base = BD71837_REG_IRQ,
> + .init_ack_masked = true,
> + .mask_invert = false,
> +};
> +
> +static const struct regmap_range pmic_status_range = {
> + .range_min = BD71837_REG_IRQ,
> + .range_max = BD71837_REG_POW_STATE,
> +};
> +
> +static const struct regmap_access_table volatile_regs = {
> + .yes_ranges = &pmic_status_range,
> + .n_yes_ranges = 1,
> +};
> +
> +static const struct regmap_config bd71837_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .volatile_table = &volatile_regs,
> + .max_register = BD71837_MAX_REGISTER - 1,
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> +static const struct of_device_id bd71837_of_match[] = {
> + { .compatible = "rohm,bd71837", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, bd71837_of_match);
> +
> +static int bd71837_i2c_probe(struct i2c_client *i2c,
> + const struct i2c_device_id *id)
> +{
> + struct bd71837 *bd71837;
> + struct bd71837_board *board_info;
> + int ret = -EINVAL;
I'd prefer it if you set the error at the location it happened for
readability.
> + bd71837 = devm_kzalloc(&i2c->dev, sizeof(struct bd71837), GFP_KERNEL);
> +
Drop this line.
> + if (!bd71837)
> + return -ENOMEM;
> +
> + board_info = dev_get_platdata(&i2c->dev);
> +
> + if (!board_info)
> + bd71837->chip_irq = i2c->irq;
> + else
> + bd71837->chip_irq = board_info->gpio_intr;
> +
> + if (!bd71837->chip_irq) {
> + dev_err(&i2c->dev, "No IRQ configured\n");
> + goto err_out;
> + }
> +
> + bd71837->dev = &i2c->dev;
> + bd71837->i2c_client = i2c;
> + i2c_set_clientdata(i2c, bd71837);
> +
> + bd71837->regmap = devm_regmap_init_i2c(i2c, &bd71837_regmap_config);
> + if (IS_ERR(bd71837->regmap)) {
> + ret = PTR_ERR(bd71837->regmap);
> + dev_err(&i2c->dev, "regmap initialization failed\n");
> + goto err_out;
> + }
> +
> + ret = bd71837_reg_read(bd71837, BD71837_REG_REV);
> + if (ret < 0) {
Is 0 return a successful read? I suggest that it isn't.
> + dev_err(&i2c->dev, "Read BD71837_REG_DEVICE failed\n");
> + goto err_out;
> + }
> +
> + ret = devm_regmap_add_irq_chip(&i2c->dev, bd71837->regmap,
> + bd71837->chip_irq, IRQF_ONESHOT, 0,
> + &bd71837_irq_chip, &bd71837->irq_data);
> + if (ret < 0) {
Is a >0 return valid? If not, perhaps "if (ret)" would be better.
> + dev_err(&i2c->dev, "Failed to add irq_chip\n");
> + goto err_out;
> + }
> +
> + ret = regmap_update_bits(bd71837->regmap,
> + BD71837_REG_PWRONCONFIG0,
> + BD718XX_PWRBTN_PRESS_DURATION_MASK,
> + BD718XX_PWRBTN_SHORT_PRESS_10MS);
> + if (ret < 0) {
As above.
> + dev_err(&i2c->dev, "Failed to configure button short press timeout\n");
> + goto err_out;
> + }
> +
> + /*
> + * According to BD71847 datasheet the HW default for long press
> + * detection is 10ms. So lets change it to 10 sec so we can actually
> + * get the short push and allow gracefull shut down
> + */
I don't think this comment is necessary.
If anything it only needs to say "Configure long press to 10 seconds".
And if you do that, please add one for short press too.
> + ret = regmap_update_bits(bd71837->regmap,
> + BD71837_REG_PWRONCONFIG1,
> + BD718XX_PWRBTN_PRESS_DURATION_MASK,
> + BD718XX_PWRBTN_LONG_PRESS_10S);
> +
> + if (ret < 0) {
> + dev_err(&i2c->dev, "Failed to configure button long press timeout\n");
> + goto err_out;
> + }
> +
> + btns[0].irq = regmap_irq_get_virq(bd71837->irq_data,
> + BD71837_INT_PWRBTN_S);
> +
> + if (btns[0].irq < 0) {
> + ret = btns[0].irq;
> + dev_err(&i2c->dev, "Failed to get the IRQ\n");
> + goto err_out;
No unwinding to be done, just return directly and save yourself an
superflouous goto.
> + }
> +
> + ret = devm_mfd_add_devices(bd71837->dev, PLATFORM_DEVID_AUTO,
> + bd71837_mfd_cells,
> + ARRAY_SIZE(bd71837_mfd_cells), NULL, 0,
> + regmap_irq_get_domain(bd71837->irq_data));
> + if (ret)
> + dev_err(&i2c->dev, "Failed to create subdevices\n");
> +err_out:
> +
> + return ret;
> +}
> +
> +static const struct i2c_device_id bd71837_i2c_id[] = {
> + { .name = "bd71837", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, bd71837_i2c_id);
You shouldn't need this table anymore.
See da10c06a044b ("i2c: Make I2C ID tables non-mandatory for DT'ed devices")
> +static struct i2c_driver bd71837_i2c_driver = {
> + .driver = {
> + .name = "bd71837-mfd",
And "MFD" isn't really a thing. It's more common to just mention the
board name, or something like 'rohm-bd7183'.
> + .of_match_table = bd71837_of_match,
> + },
> + .probe = bd71837_i2c_probe,
> + .id_table = bd71837_i2c_id,
> +};
> +
> +static int __init bd71837_i2c_init(void)
> +{
> + return i2c_add_driver(&bd71837_i2c_driver);
> +}
> +/*
> + * init early so consumer devices can complete system boot
> + */
This is a single line commit. Please use the correct format.
/* Single line comment */
> +subsys_initcall(bd71837_i2c_init);
> +
> +static void __exit bd71837_i2c_exit(void)
> +{
> + i2c_del_driver(&bd71837_i2c_driver);
> +}
> +module_exit(bd71837_i2c_exit);
> +
> +MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
> +MODULE_DESCRIPTION("BD71837 chip multi-function driver");
'PMIC' or 'Power Management IC' rather than MFD.
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/bd71837.h b/include/linux/mfd/bd71837.h
> new file mode 100644
> index 000000000000..10b0dfe90f27
> --- /dev/null
> +++ b/include/linux/mfd/bd71837.h
> @@ -0,0 +1,391 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
I thought these were meant to use C++ (//) comments?
> +/* Copyright (C) 2018 ROHM Semiconductors */
> +
> +/*
> + * ROHM BD71837MWV header file
> + */
If you prefix the mentions of (BD,bd)71837 with (ROHM,rohm) then this
comment becomes redundant and you can remove it.
> +#ifndef __LINUX_MFD_BD71837_H__
> +#define __LINUX_MFD_BD71837_H__
> +
> +#include <linux/regmap.h>
> +
> +enum {
> + BD71837_BUCK1 = 0,
> + BD71837_BUCK2,
> + BD71837_BUCK3,
> + BD71837_BUCK4,
> + BD71837_BUCK5,
> + BD71837_BUCK6,
> + BD71837_BUCK7,
> + BD71837_BUCK8,
> + BD71837_LDO1,
> + BD71837_LDO2,
> + BD71837_LDO3,
> + BD71837_LDO4,
> + BD71837_LDO5,
> + BD71837_LDO6,
> + BD71837_LDO7,
> + BD71837_REGULATOR_CNT,
> +};
> +
> +#define BD71837_BUCK1_VOLTAGE_NUM 0x40
> +#define BD71837_BUCK2_VOLTAGE_NUM 0x40
> +#define BD71837_BUCK3_VOLTAGE_NUM 0x40
> +#define BD71837_BUCK4_VOLTAGE_NUM 0x40
> +
> +#define BD71837_BUCK5_VOLTAGE_NUM 0x08
> +#define BD71837_BUCK6_VOLTAGE_NUM 0x04
> +#define BD71837_BUCK7_VOLTAGE_NUM 0x08
> +#define BD71837_BUCK8_VOLTAGE_NUM 0x40
> +
> +#define BD71837_LDO1_VOLTAGE_NUM 0x04
> +#define BD71837_LDO2_VOLTAGE_NUM 0x02
> +#define BD71837_LDO3_VOLTAGE_NUM 0x10
> +#define BD71837_LDO4_VOLTAGE_NUM 0x10
> +#define BD71837_LDO5_VOLTAGE_NUM 0x10
> +#define BD71837_LDO6_VOLTAGE_NUM 0x10
> +#define BD71837_LDO7_VOLTAGE_NUM 0x10
> +
> +enum {
> + BD71837_REG_REV = 0x00,
> + BD71837_REG_SWRESET = 0x01,
> + BD71837_REG_I2C_DEV = 0x02,
> + BD71837_REG_PWRCTRL0 = 0x03,
> + BD71837_REG_PWRCTRL1 = 0x04,
> + BD71837_REG_BUCK1_CTRL = 0x05,
> + BD71837_REG_BUCK2_CTRL = 0x06,
> + BD71837_REG_BUCK3_CTRL = 0x07,
> + BD71837_REG_BUCK4_CTRL = 0x08,
> + BD71837_REG_BUCK5_CTRL = 0x09,
> + BD71837_REG_BUCK6_CTRL = 0x0A,
> + BD71837_REG_BUCK7_CTRL = 0x0B,
> + BD71837_REG_BUCK8_CTRL = 0x0C,
> + BD71837_REG_BUCK1_VOLT_RUN = 0x0D,
> + BD71837_REG_BUCK1_VOLT_IDLE = 0x0E,
> + BD71837_REG_BUCK1_VOLT_SUSP = 0x0F,
> + BD71837_REG_BUCK2_VOLT_RUN = 0x10,
> + BD71837_REG_BUCK2_VOLT_IDLE = 0x11,
> + BD71837_REG_BUCK3_VOLT_RUN = 0x12,
> + BD71837_REG_BUCK4_VOLT_RUN = 0x13,
> + BD71837_REG_BUCK5_VOLT = 0x14,
> + BD71837_REG_BUCK6_VOLT = 0x15,
> + BD71837_REG_BUCK7_VOLT = 0x16,
> + BD71837_REG_BUCK8_VOLT = 0x17,
> + BD71837_REG_LDO1_VOLT = 0x18,
> + BD71837_REG_LDO2_VOLT = 0x19,
> + BD71837_REG_LDO3_VOLT = 0x1A,
> + BD71837_REG_LDO4_VOLT = 0x1B,
> + BD71837_REG_LDO5_VOLT = 0x1C,
> + BD71837_REG_LDO6_VOLT = 0x1D,
> + BD71837_REG_LDO7_VOLT = 0x1E,
> + BD71837_REG_TRANS_COND0 = 0x1F,
> + BD71837_REG_TRANS_COND1 = 0x20,
> + BD71837_REG_VRFAULTEN = 0x21,
> + BD71837_REG_MVRFLTMASK0 = 0x22,
> + BD71837_REG_MVRFLTMASK1 = 0x23,
> + BD71837_REG_MVRFLTMASK2 = 0x24,
> + BD71837_REG_RCVCFG = 0x25,
> + BD71837_REG_RCVNUM = 0x26,
> + BD71837_REG_PWRONCONFIG0 = 0x27,
> + BD71837_REG_PWRONCONFIG1 = 0x28,
> + BD71837_REG_RESETSRC = 0x29,
> + BD71837_REG_MIRQ = 0x2A,
> + BD71837_REG_IRQ = 0x2B,
> + BD71837_REG_IN_MON = 0x2C,
> + BD71837_REG_POW_STATE = 0x2D,
> + BD71837_REG_OUT32K = 0x2E,
> + BD71837_REG_REGLOCK = 0x2F,
> + BD71837_REG_OTPVER = 0xFF,
> + BD71837_MAX_REGISTER = 0x100,
> +};
> +
> +#define REGLOCK_PWRSEQ 0x1
> +#define REGLOCK_VREG 0x10
> +
> +/* Generic BUCK control masks */
> +#define BD71837_BUCK_SEL 0x02
> +#define BD71837_BUCK_EN 0x01
> +#define BD71837_BUCK_RUN_ON 0x04
> +
> +/* Generic LDO masks */
> +#define BD71837_LDO_SEL 0x80
> +#define BD71837_LDO_EN 0x40
> +
> +/* BD71837 BUCK ramp rate CTRL reg bits */
> +#define BUCK_RAMPRATE_MASK 0xC0
> +#define BUCK_RAMPRATE_10P00MV 0x0
> +#define BUCK_RAMPRATE_5P00MV 0x1
> +#define BUCK_RAMPRATE_2P50MV 0x2
> +#define BUCK_RAMPRATE_1P25MV 0x3
> +
> +/* BD71837_REG_BUCK1_VOLT_RUN bits */
> +#define BUCK1_RUN_MASK 0x3F
> +#define BUCK1_RUN_DEFAULT 0x14
> +
> +/* BD71837_REG_BUCK1_VOLT_SUSP bits */
> +#define BUCK1_SUSP_MASK 0x3F
> +#define BUCK1_SUSP_DEFAULT 0x14
> +
> +/* BD71837_REG_BUCK1_VOLT_IDLE bits */
> +#define BUCK1_IDLE_MASK 0x3F
> +#define BUCK1_IDLE_DEFAULT 0x14
> +
> +/* BD71837_REG_BUCK2_VOLT_RUN bits */
> +#define BUCK2_RUN_MASK 0x3F
> +#define BUCK2_RUN_DEFAULT 0x1E
> +
> +/* BD71837_REG_BUCK2_VOLT_IDLE bits */
> +#define BUCK2_IDLE_MASK 0x3F
> +#define BUCK2_IDLE_DEFAULT 0x14
> +
> +/* BD71837_REG_BUCK3_VOLT_RUN bits */
> +#define BUCK3_RUN_MASK 0x3F
> +#define BUCK3_RUN_DEFAULT 0x1E
> +
> +/* BD71837_REG_BUCK4_VOLT_RUN bits */
> +#define BUCK4_RUN_MASK 0x3F
> +#define BUCK4_RUN_DEFAULT 0x1E
> +
> +/* BD71837_REG_BUCK5_VOLT bits */
> +#define BUCK5_MASK 0x07
> +#define BUCK5_DEFAULT 0x02
> +
> +/* BD71837_REG_BUCK6_VOLT bits */
> +#define BUCK6_MASK 0x03
> +#define BUCK6_DEFAULT 0x03
> +
> +/* BD71837_REG_BUCK7_VOLT bits */
> +#define BUCK7_MASK 0x07
> +#define BUCK7_DEFAULT 0x03
> +
> +/* BD71837_REG_BUCK8_VOLT bits */
> +#define BUCK8_MASK 0x3F
> +#define BUCK8_DEFAULT 0x1E
> +
> +/* BD71837_REG_IRQ bits */
> +#define IRQ_SWRST 0x40
> +#define IRQ_PWRON_S 0x20
> +#define IRQ_PWRON_L 0x10
> +#define IRQ_PWRON 0x08
> +#define IRQ_WDOG 0x04
> +#define IRQ_ON_REQ 0x02
> +#define IRQ_STBY_REQ 0x01
> +
> +/* BD71837_REG_OUT32K bits */
> +#define BD71837_OUT32K_EN 0x01
> +
> +/* BD71837 gated clock rate */
> +#define BD71837_CLK_RATE 32768
> +
> +/* BD71837 irqs */
> +enum {
> + BD71837_INT_STBY_REQ,
> + BD71837_INT_ON_REQ,
> + BD71837_INT_WDOG,
> + BD71837_INT_PWRBTN,
> + BD71837_INT_PWRBTN_L,
> + BD71837_INT_PWRBTN_S,
> + BD71837_INT_SWRST
> +};
> +
> +/* BD71837 interrupt masks */
> +#define BD71837_INT_SWRST_MASK 0x40
> +#define BD71837_INT_PWRBTN_S_MASK 0x20
> +#define BD71837_INT_PWRBTN_L_MASK 0x10
> +#define BD71837_INT_PWRBTN_MASK 0x8
> +#define BD71837_INT_WDOG_MASK 0x4
> +#define BD71837_INT_ON_REQ_MASK 0x2
> +#define BD71837_INT_STBY_REQ_MASK 0x1
> +
> +/* BD71837_REG_LDO1_VOLT bits */
> +#define LDO1_MASK 0x03
> +
> +/* BD71837_REG_LDO1_VOLT bits */
> +#define LDO2_MASK 0x20
> +
> +/* BD71837_REG_LDO3_VOLT bits */
> +#define LDO3_MASK 0x0F
> +
> +/* BD71837_REG_LDO4_VOLT bits */
> +#define LDO4_MASK 0x0F
> +
> +/* BD71837_REG_LDO5_VOLT bits */
> +#define LDO5_MASK 0x0F
> +
> +/* BD71837_REG_LDO6_VOLT bits */
> +#define LDO6_MASK 0x0F
> +
> +/* BD71837_REG_LDO7_VOLT bits */
> +#define LDO7_MASK 0x0F
> +
> +/* Register write induced reset settings */
> +
> +/* Even though the bit zero is not SWRESET type we still want to write zero
> + * to it when changing type. Bit zero is 'SWRESET' trigger bit and if we
> + * write 1 to it we will trigger the action. So always write 0 to it when
> + * changning SWRESET action - no matter what we read from it.
> + */
Please use the correct format for multi-line comments.
Hint: The first line should be empty.
> +#define BD71837_SWRESET_TYPE_MASK 7
> +#define BD71837_SWRESET_TYPE_DISABLED 0
> +#define BD71837_SWRESET_TYPE_COLD 4
> +#define BD71837_SWRESET_TYPE_WARM 6
> +
> +#define BD71837_SWRESET_RESET_MASK 1
> +#define BD71837_SWRESET_RESET 1
> +
> +/* Poweroff state transition conditions */
> +
> +#define BD718XX_ON_REQ_POWEROFF_MASK 1
> +#define BD718XX_SWRESET_POWEROFF_MASK 2
> +#define BD718XX_WDOG_POWEROFF_MASK 4
> +#define BD718XX_KEY_L_POWEROFF_MASK 8
> +
> +#define BD718XX_POWOFF_TO_SNVS 0
> +#define BD718XX_POWOFF_TO_RDY 0xF
Please align each of the values with tabs.
> +#define BD718XX_POWOFF_TIME_MASK 0xF0
> +enum {
> + BD718XX_POWOFF_TIME_5MS = 0,
> + BD718XX_POWOFF_TIME_10MS,
> + BD718XX_POWOFF_TIME_15MS,
> + BD718XX_POWOFF_TIME_20MS,
> + BD718XX_POWOFF_TIME_25MS,
> + BD718XX_POWOFF_TIME_30MS,
> + BD718XX_POWOFF_TIME_35MS,
> + BD718XX_POWOFF_TIME_40MS,
> + BD718XX_POWOFF_TIME_45MS,
> + BD718XX_POWOFF_TIME_50MS,
> + BD718XX_POWOFF_TIME_75MS,
> + BD718XX_POWOFF_TIME_100MS,
> + BD718XX_POWOFF_TIME_250MS,
> + BD718XX_POWOFF_TIME_500MS,
> + BD718XX_POWOFF_TIME_750MS,
> + BD718XX_POWOFF_TIME_1500MS
> +};
> +
> +/* Poweron sequence state transition conditions */
> +
> +#define BD718XX_RDY_TO_SNVS_MASK 0xF
> +#define BD718XX_SNVS_TO_RUN_MASK 0xF0
> +
> +#define BD718XX_PWR_TRIG_KEY_L 1
> +#define BD718XX_PWR_TRIG_KEY_S 2
> +#define BD718XX_PWR_TRIG_PMIC_ON 4
> +#define BD718XX_PWR_TRIG_VSYS_UVLO 8
> +#define BD718XX_RDY_TO_SNVS_SIFT 0
> +#define BD718XX_SNVS_TO_RUN_SIFT 4
As above.
> +
> +
No need for 2 '\n'.
> +#define BD718XX_PWRBTN_PRESS_DURATION_MASK 0xF
> +
> +/* Timeout value for detecting short press */
> +
> +enum {
> + BD718XX_PWRBTN_SHORT_PRESS_10MS = 0,
> + BD718XX_PWRBTN_SHORT_PRESS_500MS,
> + BD718XX_PWRBTN_SHORT_PRESS_1000MS,
> + BD718XX_PWRBTN_SHORT_PRESS_1500MS,
> + BD718XX_PWRBTN_SHORT_PRESS_2000MS,
> + BD718XX_PWRBTN_SHORT_PRESS_2500MS,
> + BD718XX_PWRBTN_SHORT_PRESS_3000MS,
> + BD718XX_PWRBTN_SHORT_PRESS_3500MS,
> + BD718XX_PWRBTN_SHORT_PRESS_4000MS,
> + BD718XX_PWRBTN_SHORT_PRESS_4500MS,
> + BD718XX_PWRBTN_SHORT_PRESS_5000MS,
> + BD718XX_PWRBTN_SHORT_PRESS_5500MS,
> + BD718XX_PWRBTN_SHORT_PRESS_6000MS,
> + BD718XX_PWRBTN_SHORT_PRESS_6500MS,
> + BD718XX_PWRBTN_SHORT_PRESS_7000MS,
> + BD718XX_PWRBTN_SHORT_PRESS_7500MS
> +};
> +
> +/* Timeout value for detecting LONG press */
> +
> +enum {
> + BD718XX_PWRBTN_LONG_PRESS_10MS = 0,
> + BD718XX_PWRBTN_LONG_PRESS_1S,
> + BD718XX_PWRBTN_LONG_PRESS_2S,
> + BD718XX_PWRBTN_LONG_PRESS_3S,
> + BD718XX_PWRBTN_LONG_PRESS_4S,
> + BD718XX_PWRBTN_LONG_PRESS_5S,
> + BD718XX_PWRBTN_LONG_PRESS_6S,
> + BD718XX_PWRBTN_LONG_PRESS_7S,
> + BD718XX_PWRBTN_LONG_PRESS_8S,
> + BD718XX_PWRBTN_LONG_PRESS_9S,
> + BD718XX_PWRBTN_LONG_PRESS_10S,
> + BD718XX_PWRBTN_LONG_PRESS_11S,
> + BD718XX_PWRBTN_LONG_PRESS_12S,
> + BD718XX_PWRBTN_LONG_PRESS_13S,
> + BD718XX_PWRBTN_LONG_PRESS_14S,
> + BD718XX_PWRBTN_LONG_PRESS_15S
> +};
> +
> +
> +
Certainly no need for 3 '\n'.
> +struct bd71837;
Why is this necessary?
> +struct bd71837_pmic;
> +struct bd71837_clk;
> +struct bd718xx_pwrkey;
Where are these forward declared from?
> +/*
> + * Board platform data may be used to initialize regulators.
> + */
> +
> +struct bd71837_board {
> + struct regulator_init_data *init_data[BD71837_REGULATOR_CNT];
> + int gpio_intr;
> +};
Where is this populated?
> +struct bd71837 {
> + struct device *dev;
> + struct i2c_client *i2c_client;
> + struct regmap *regmap;
> + unsigned long int id;
> +
> + int chip_irq;
> + struct regmap_irq_chip_data *irq_data;
> +
> + struct bd71837_pmic *pmic;
> + struct bd71837_clk *clk;
> + struct bd718xx_pwrkey *pwrkey;
> +};
> +
> +/*
> + * bd71837 sub-driver chip access routines
> + */
> +
> +static inline int bd71837_reg_read(struct bd71837 *bd71837, u8 reg)
> +{
> + int r, val;
> +
> + r = regmap_read(bd71837->regmap, reg, &val);
> + if (r < 0)
> + return r;
> + return val;
> +}
> +
> +static inline int bd71837_reg_write(struct bd71837 *bd71837, u8 reg,
> + unsigned int val)
> +{
> + return regmap_write(bd71837->regmap, reg, val);
> +}
> +
> +static inline int bd71837_set_bits(struct bd71837 *bd71837, u8 reg, u8 mask)
> +{
> + return regmap_update_bits(bd71837->regmap, reg, mask, mask);
> +}
> +
> +static inline int bd71837_clear_bits(struct bd71837 *bd71837, u8 reg,
> + u8 mask)
> +{
> + return regmap_update_bits(bd71837->regmap, reg, mask, 0);
> +}
> +
> +static inline int bd71837_update_bits(struct bd71837 *bd71837, u8 reg,
> + u8 mask, u8 val)
> +{
> + return regmap_update_bits(bd71837->regmap, reg, mask, val);
> +}
Please don't abstract APIs for no reason.
Use the regmap_* API directly instead.
> +#endif /* __LINUX_MFD_BD71837_H__ */
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v9 2/2] mfd: bd71837: Devicetree bindings for ROHM BD71837 PMIC
From: Lee Jones @ 2018-07-05 9:24 UTC (permalink / raw)
To: Matti Vaittinen
Cc: mturquette, robh+dt, mark.rutland, lgirdwood, broonie,
mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo,
sboyd, linux-clk, devicetree, linux-kernel, linux-input,
mikko.mutanen, heikki.haikola
In-Reply-To: <87cc68c82c3c95e82797e0ce3d401a4f1d6daf25.1530776326.git.matti.vaittinen@fi.rohmeurope.com>
On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> Document devicetree bindings for ROHM BD71837 PMIC MFD.
>
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt | 67 ++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> new file mode 100644
> index 000000000000..67f2616288d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> @@ -0,0 +1,67 @@
> +* ROHM BD71837 Power Management Integrated Circuit bindings
> +
> +BD71837MWV is a programmable Power Management IC for powering single-core,
> +dual-core, and quad-core SoC’s such as NXP-i.MX 8M. It is optimized for
Nit: s/SOC's/SOCs/
> +low BOM cost and compact solution footprint. It integrates 8 Buck
> +egulators and 7 LDO’s to provide all the power rails required by the SoC and
Nit: s/LDO's/LDOs/
> +the commonly used peripherals.
> +
> +Datasheet for PMIC is available at:
> +https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
> +
> +Required properties:
> + - compatible : Should be "rohm,bd71837".
> + - reg : I2C slave address.
> + - interrupt-parent : Phandle to the parent interrupt controller.
> + - interrupts : The interrupt line the device is connected to.
> + - clocks : The parent clock connected to PMIC. If this is missng
s/missng/missing/
> + 32768 KHz clock is assumed.
> + - #clock-cells : Should be 0
> + - regulators: : List of child nodes that specify the regulators
By your convention, the bottom two entries are missing '.'s.
> + Please see ../regulator/rohm,bd71837-regulator.txt
> +
> +Optional properties:
> +- clock-output-names : Should contain name for output clock.
> +
> +Example:
> +
> + /* external oscillator node */
Uppercase character to start a sentence.
> + osc: oscillator {
> + compatible = "fixed-clock";
> + #clock-cells = <1>;
> + clock-frequency = <32768>;
> + clock-output-names = "osc";
> + };
> +
> + /* PMIC node */
Is this comment really necessary? The node is called "pmic".
> + pmic: pmic@4b {
> + compatible = "rohm,bd71837";
> + reg = <0x4b>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <29 GPIO_ACTIVE_LOW>;
> + interrupt-names = "irq";
> + #clock-cells = <0>;
> + clocks = <&osc 0>;
> + clock-output-names = "bd71837-32k-out";
> +
> + regulators {
> + buck1: BUCK1 {
> + regulator-name = "buck1";
> + regulator-min-microvolt = <700000>;
> + regulator-max-microvolt = <1300000>;
> + regulator-boot-on;
> + regulator-ramp-delay = <1250>;
> + };
> + /* ... */
> + };
> + };
> +
> + /* Clock consumer node */
> +
> + foo@0 {
> + compatible = "bar,foo";
> + /* ... */
No need for this line. Additional properties are always expected.
> + clock-names = "my-clock";
> + clocks = <&pmic>;
> + };
Do you have a real example to give?
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v6 1/4] resource: Move reparent_resources() to kernel/resource.c and make it public
From: Baoquan He @ 2018-07-05 10:09 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Nicolas Pitre, brijesh.singh-5C7GfCeVMHo, devicetree,
David Airlie, linux-pci-u79uwXL29TY76Z2rM5mHXA,
richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w, Max Filippov,
baiyaowei-0p4V/sDNsUmm0O/7XYngnFaTQe2KTcn/, KY Srinivasan,
Frank Rowand, Lorenzo Pieralisi, Stephen Hemminger,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Patrik Jakobsson,
linux-input, Gustavo Padovan, Borislav Petkov, Dave Young,
Tom Lendacky, Haiyang Zhang, Maarten Lankhorst, Josh Triplett,
Jérôme Glisse
In-Reply-To: <CAHp75VfWLcQq4XGM3RyfO_ENx=3=WrCEhtOw+a0Sfwz_OpQ0UQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 07/04/18 at 07:46pm, Andy Shevchenko wrote:
> On Wed, Jul 4, 2018 at 7:10 AM, Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > reparent_resources() is duplicated in arch/microblaze/pci/pci-common.c
> > and arch/powerpc/kernel/pci-common.c, so move it to kernel/resource.c
> > so that it's shared.
>
> With couple of comments below,
Sure, will fix these and repost with your reviewed-by, thanks. Should be
after I reproduce and fix those issues reported by test robot.
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> P.S. In some commit message in this series you used 'likt' instead of 'like'.
>
> >
> > Signed-off-by: Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> > arch/microblaze/pci/pci-common.c | 37 -------------------------------------
> > arch/powerpc/kernel/pci-common.c | 35 -----------------------------------
> > include/linux/ioport.h | 1 +
> > kernel/resource.c | 39 +++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 40 insertions(+), 72 deletions(-)
> >
> > diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
> > index f34346d56095..7899bafab064 100644
> > --- a/arch/microblaze/pci/pci-common.c
> > +++ b/arch/microblaze/pci/pci-common.c
> > @@ -619,43 +619,6 @@ int pcibios_add_device(struct pci_dev *dev)
> > EXPORT_SYMBOL(pcibios_add_device);
> >
> > /*
> > - * Reparent resource children of pr that conflict with res
> > - * under res, and make res replace those children.
> > - */
> > -static int __init reparent_resources(struct resource *parent,
> > - struct resource *res)
> > -{
> > - struct resource *p, **pp;
> > - struct resource **firstpp = NULL;
> > -
> > - for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
> > - if (p->end < res->start)
> > - continue;
> > - if (res->end < p->start)
> > - break;
> > - if (p->start < res->start || p->end > res->end)
> > - return -1; /* not completely contained */
> > - if (firstpp == NULL)
> > - firstpp = pp;
> > - }
> > - if (firstpp == NULL)
> > - return -1; /* didn't find any conflicting entries? */
> > - res->parent = parent;
> > - res->child = *firstpp;
> > - res->sibling = *pp;
> > - *firstpp = res;
> > - *pp = NULL;
> > - for (p = res->child; p != NULL; p = p->sibling) {
> > - p->parent = res;
> > - pr_debug("PCI: Reparented %s [%llx..%llx] under %s\n",
> > - p->name,
> > - (unsigned long long)p->start,
> > - (unsigned long long)p->end, res->name);
> > - }
> > - return 0;
> > -}
> > -
> > -/*
> > * Handle resources of PCI devices. If the world were perfect, we could
> > * just allocate all the resource regions and do nothing more. It isn't.
> > * On the other hand, we cannot just re-allocate all devices, as it would
> > diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> > index fe9733ffffaa..926035bb378d 100644
> > --- a/arch/powerpc/kernel/pci-common.c
> > +++ b/arch/powerpc/kernel/pci-common.c
> > @@ -1088,41 +1088,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> > EXPORT_SYMBOL(pcibios_align_resource);
> >
> > /*
> > - * Reparent resource children of pr that conflict with res
> > - * under res, and make res replace those children.
> > - */
> > -static int reparent_resources(struct resource *parent,
> > - struct resource *res)
> > -{
> > - struct resource *p, **pp;
> > - struct resource **firstpp = NULL;
> > -
> > - for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
> > - if (p->end < res->start)
> > - continue;
> > - if (res->end < p->start)
> > - break;
> > - if (p->start < res->start || p->end > res->end)
> > - return -1; /* not completely contained */
> > - if (firstpp == NULL)
> > - firstpp = pp;
> > - }
> > - if (firstpp == NULL)
> > - return -1; /* didn't find any conflicting entries? */
> > - res->parent = parent;
> > - res->child = *firstpp;
> > - res->sibling = *pp;
> > - *firstpp = res;
> > - *pp = NULL;
> > - for (p = res->child; p != NULL; p = p->sibling) {
> > - p->parent = res;
> > - pr_debug("PCI: Reparented %s %pR under %s\n",
> > - p->name, p, res->name);
> > - }
> > - return 0;
> > -}
> > -
> > -/*
> > * Handle resources of PCI devices. If the world were perfect, we could
> > * just allocate all the resource regions and do nothing more. It isn't.
> > * On the other hand, we cannot just re-allocate all devices, as it would
> > diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> > index da0ebaec25f0..dfdcd0bfe54e 100644
> > --- a/include/linux/ioport.h
> > +++ b/include/linux/ioport.h
> > @@ -192,6 +192,7 @@ extern int allocate_resource(struct resource *root, struct resource *new,
> > struct resource *lookup_resource(struct resource *root, resource_size_t start);
> > int adjust_resource(struct resource *res, resource_size_t start,
> > resource_size_t size);
> > +int reparent_resources(struct resource *parent, struct resource *res);
> > resource_size_t resource_alignment(struct resource *res);
> > static inline resource_size_t resource_size(const struct resource *res)
> > {
> > diff --git a/kernel/resource.c b/kernel/resource.c
> > index 30e1bc68503b..d1cbf4b50e17 100644
> > --- a/kernel/resource.c
> > +++ b/kernel/resource.c
> > @@ -983,6 +983,45 @@ int adjust_resource(struct resource *res, resource_size_t start,
> > }
> > EXPORT_SYMBOL(adjust_resource);
> >
> > +/*
>
> /** ?
>
> > + * reparent_resources - reparent resource children of parent that res covers
> > + * @parent: parent resource descriptor
> > + * @res: resource descriptor desired by caller
> > + *
> > + * Reparent resource children of 'parent' that conflict with 'res'
> > + * under 'res', and make 'res' replace those children.
>
> Describe error codes, please.
>
> > + */
> > +int reparent_resources(struct resource *parent, struct resource *res)
> > +{
> > + struct resource *p, **pp;
> > + struct resource **firstpp = NULL;
> > +
> > + for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
> > + if (p->end < res->start)
> > + continue;
> > + if (res->end < p->start)
> > + break;
> > + if (p->start < res->start || p->end > res->end)
> > + return -ENOTSUPP; /* not completely contained */
> > + if (firstpp == NULL)
> > + firstpp = pp;
> > + }
> > + if (firstpp == NULL)
> > + return -ECANCELED; /* didn't find any conflicting entries? */
> > + res->parent = parent;
> > + res->child = *firstpp;
> > + res->sibling = *pp;
> > + *firstpp = res;
> > + *pp = NULL;
> > + for (p = res->child; p != NULL; p = p->sibling) {
> > + p->parent = res;
> > + pr_debug("PCI: Reparented %s %pR under %s\n",
> > + p->name, p, res->name);
> > + }
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(reparent_resources);
> > +
> > static void __init __reserve_region_with_split(struct resource *root,
> > resource_size_t start, resource_size_t end,
> > const char *name)
> > --
> > 2.13.6
> >
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v9 1/2] mfd: bd71837: mfd driver for ROHM BD71837 PMIC
From: Matti Vaittinen @ 2018-07-05 10:34 UTC (permalink / raw)
To: Lee Jones
Cc: mturquette, robh+dt, mark.rutland, lgirdwood, broonie,
mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo,
sboyd, linux-clk, devicetree, linux-kernel, linux-input,
mikko.mutanen, heikki.haikola
In-Reply-To: <20180705091813.GK496@dell>
Thanks for taking the time to check this =)
On Thu, Jul 05, 2018 at 10:18:13AM +0100, Lee Jones wrote:
> On Thu, 05 Jul 2018, Matti Vaittinen wrote:
>
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1787,6 +1787,19 @@ config MFD_STW481X
> > in various ST Microelectronics and ST-Ericsson embedded
> > Nomadik series.
> >
> > +config MFD_BD71837
>
> It's be nice if you prefix this with the manufacturer. Rohm?
Like MFD_ROHM_BD71837, right? How important this is? The already applied
regulator part has depends_on for MFD_BD71837 in Kconfig so changing
this will require change in regulator tree too. I guess it's doable if
this is important though =)
>
> > + tristate "BD71837 Power Management chip"
>
> Again, missing manufacturer.
>
> This should also be "Power Management IC"
I'll fix this
> > + select MFD_CORE
> > + help
> > + Select this option to get support for the ROHM BD71837
> > + Power Management chips. BD71837 is designed to power processors like
>
> "chips" is slang. Should be "ICs".
I'll fix this
> > +obj-$(CONFIG_MFD_BD71837) += bd71837.o
> >
> > diff --git a/drivers/mfd/bd71837.c b/drivers/mfd/bd71837.c
>
> rohm-bd71837.c would be nicer.
I'll fix this
> > new file mode 100644
> > index 000000000000..677097bcea17
> > --- /dev/null
> > +++ b/drivers/mfd/bd71837.c
> > @@ -0,0 +1,215 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
>
> '\n' here please.
> > +// Copyright (C) 2018 ROHM Semiconductors
>
> '\n' here please.
As an empty line after licence line? Ok.
> > +// bd71837.c -- ROHM BD71837MWV mfd driver
>
> Remove the filename -- they have a habit of becoming incorrect.
>
> Also, and 'MFD driver' isn't a thing -- this is a PMIC driver.
I'll fix this
> > +// Datasheet available from
> > +// https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
> > +
> > +#include <linux/i2c.h>
> > +#include <linux/input.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/mfd/bd71837.h>
> > +#include <linux/mfd/core.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +#include <linux/gpio_keys.h>
>
> Alphabetical please.
>
There is a problem with gpio_key.h - it uses bool but does not include
the header defining this type. So I get comple error when compiling this
using my GCC arm cross-compiler. I've submitted a patch to fix this but
as it is not yet accepted.
https://lore.kernel.org/lkml/20180627073458.GA27800@localhost.localdomain/
Hence the gpio_keys.h is last one.
> > +static struct gpio_keys_button btns[] = {
> > + {
> > + .code = KEY_POWER,
> > + .gpio = -1,
> > + .type = EV_KEY,
> > + },
> > +};
>
> Does this need to be an array?
https://elixir.bootlin.com/linux/v4.17-rc1/source/include/linux/gpio_keys.h#L39
gpio_keys_platform_data documentation states the buttons to be a pointer
to an array. Thus I did an array. Do you think I should change this to
single struct?
> > +static struct gpio_keys_platform_data bd718xx_powerkey_data = {
> > + .buttons = &btns[0],
>
> Why not just "btns"?
>
> Also, I don't see any reason not to just call this 'buttons'.
>
> Or even, 'button' if there will only ever be one of them.
>
I can rename this - and if I make the array to not be an array - well,
then the &foo[0] gets cleared too =)
> > +static struct mfd_cell bd71837_mfd_cells[] = {
> > + {
> > + .name = "bd71837-clk",
> > + }, {
> > + .name = "gpio-keys",
> > + .platform_data = &bd718xx_powerkey_data,
> > + .pdata_size = sizeof(bd718xx_powerkey_data),
> > + }, {
> > + .name = "bd71837-pmic",
> > + },
> > +};
>
> Place the one line entries on one line, like this:
>
> { .name = "bd71837-pmic", },
>
> ... and if ordering is not important, place them at the bottom.
I'll fix this
> > +static int bd71837_i2c_probe(struct i2c_client *i2c,
> > + const struct i2c_device_id *id)
> > +{
> > + struct bd71837 *bd71837;
> > + struct bd71837_board *board_info;
> > + int ret = -EINVAL;
>
> I'd prefer it if you set the error at the location it happened for
> readability.
I'll fix this
> > + bd71837 = devm_kzalloc(&i2c->dev, sizeof(struct bd71837), GFP_KERNEL);
> > +
>
> Drop this line.
I'll fix this
> > + ret = bd71837_reg_read(bd71837, BD71837_REG_REV);
> > + if (ret < 0) {
>
> Is 0 return a successful read? I suggest that it isn't.
>
> > + ret = devm_regmap_add_irq_chip(&i2c->dev, bd71837->regmap,
> > + bd71837->chip_irq, IRQF_ONESHOT, 0,
> > + &bd71837_irq_chip, &bd71837->irq_data);
> > + if (ret < 0) {
>
> Is a >0 return valid? If not, perhaps "if (ret)" would be better.
>
> > + ret = regmap_update_bits(bd71837->regmap,
> > + BD71837_REG_PWRONCONFIG0,
> > + BD718XX_PWRBTN_PRESS_DURATION_MASK,
> > + BD718XX_PWRBTN_SHORT_PRESS_10MS);
> > + if (ret < 0) {
>
> As above.
>
I will check these. Thanks.
> > +
> > + /*
> > + * According to BD71847 datasheet the HW default for long press
> > + * detection is 10ms. So lets change it to 10 sec so we can actually
> > + * get the short push and allow gracefull shut down
> > + */
>
> I don't think this comment is necessary.
>
> If anything it only needs to say "Configure long press to 10 seconds".
>
> And if you do that, please add one for short press too.
Difference between short and long push is that if PMIC detects log push
it will kill the power without any gracefull handling. And the chip I am
supporting right now is BD71837 - which has more sane default value (10
seconds). But I am going to add support for BD71847 - which was shipped
to me yesterday - and according the draft data-sheet the default on it
is 10ms - which makes short push unusable. So comment is intended to
point out _why_ I configure the long push here. Also someone might want
to use the HW default (I guess such default was selected for some
specific reason) so comment helps on removing this configuration for
specific use-cases. But I guess I can reduce the comment to "Configure
long press to 10 seconds" as you suggested - or do you think I should
make difference between ICs BD71837 and BD71847 more visible here?
>
> > + ret = regmap_update_bits(bd71837->regmap,
> > + BD71837_REG_PWRONCONFIG1,
> > + BD718XX_PWRBTN_PRESS_DURATION_MASK,
> > + BD718XX_PWRBTN_LONG_PRESS_10S);
> > +
> > + if (ret < 0) {
> > + dev_err(&i2c->dev, "Failed to configure button long press timeout\n");
> > + goto err_out;
> > + }
> > +
> > + btns[0].irq = regmap_irq_get_virq(bd71837->irq_data,
> > + BD71837_INT_PWRBTN_S);
> > +
> > + if (btns[0].irq < 0) {
> > + ret = btns[0].irq;
> > + dev_err(&i2c->dev, "Failed to get the IRQ\n");
> > + goto err_out;
>
> No unwinding to be done, just return directly and save yourself an
> superflouous goto.
So should I replace _all_ the gotos by in-place returns as Enric suggested
yesterday? As I explained yesterday, my preference is to have only one
point of exit but I can change gotos to returns if needed.
> > +static const struct i2c_device_id bd71837_i2c_id[] = {
> > + { .name = "bd71837", },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, bd71837_i2c_id);
>
> You shouldn't need this table anymore.
>
> See da10c06a044b ("i2c: Make I2C ID tables non-mandatory for DT'ed devices")
So if I read this correctly the
> > + .of_match_table = bd71837_of_match,
is now sufficient. Thanks.
> > +static struct i2c_driver bd71837_i2c_driver = {
> > + .driver = {
> > + .name = "bd71837-mfd",
>
> And "MFD" isn't really a thing. It's more common to just mention the
> board name, or something like 'rohm-bd7183'.
I'll fix this
> > +/*
> > + * init early so consumer devices can complete system boot
> > + */
>
> This is a single line commit. Please use the correct format.
>
> /* Single line comment */
I'll fix this. I misinterpreted your comment yesterday.
> > +MODULE_DESCRIPTION("BD71837 chip multi-function driver");
>
> 'PMIC' or 'Power Management IC' rather than MFD.
I'll fix this
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/linux/mfd/bd71837.h b/include/linux/mfd/bd71837.h
> > new file mode 100644
> > index 000000000000..10b0dfe90f27
> > --- /dev/null
> > +++ b/include/linux/mfd/bd71837.h
> > @@ -0,0 +1,391 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
>
> I thought these were meant to use C++ (//) comments?
The checkpatch.pl did complain if I used // comment on SPDX line for
header file. OTOH for c-file it required // comments and complained
about /* */ - version. So I did as it suggested and used
> > +/* Copyright (C) 2018 ROHM Semiconductors */
> > +
> > +/*
> > + * ROHM BD71837MWV header file
> > + */
>
> If you prefix the mentions of (BD,bd)71837 with (ROHM,rohm) then this
> comment becomes redundant and you can remove it.
I am sorry but I am not sure what you suggest here. Do you mean I should
add ROHM or rohm to all definitions here? I think that would make
definitions pretty long so I would certinly need to split more lines.
Such cange would also impact already applied patches. So maybe I
misinterpreted your comment? And in case I did not - can this prefixing of
types - be done as a separate patch set as it impacts to regulators and
clk too? (clk is not yet applied so that is easy to change though).
> > +/* Even though the bit zero is not SWRESET type we still want to write zero
> > + * to it when changing type. Bit zero is 'SWRESET' trigger bit and if we
> > + * write 1 to it we will trigger the action. So always write 0 to it when
> > + * changning SWRESET action - no matter what we read from it.
> > + */
>
> Please use the correct format for multi-line comments.
>
> Hint: The first line should be empty.
I'll fix this
> > +#define BD71837_SWRESET_TYPE_MASK 7
> > +#define BD71837_SWRESET_TYPE_DISABLED 0
> > +#define BD71837_SWRESET_TYPE_COLD 4
> > +#define BD71837_SWRESET_TYPE_WARM 6
> > +
> > +#define BD71837_SWRESET_RESET_MASK 1
> > +#define BD71837_SWRESET_RESET 1
> > +
> > +/* Poweroff state transition conditions */
> > +
> > +#define BD718XX_ON_REQ_POWEROFF_MASK 1
> > +#define BD718XX_SWRESET_POWEROFF_MASK 2
> > +#define BD718XX_WDOG_POWEROFF_MASK 4
> > +#define BD718XX_KEY_L_POWEROFF_MASK 8
> > +
> > +#define BD718XX_POWOFF_TO_SNVS 0
> > +#define BD718XX_POWOFF_TO_RDY 0xF
>
> Please align each of the values with tabs.
I'll fix this
> > +#define BD718XX_PWR_TRIG_KEY_L 1
> > +#define BD718XX_PWR_TRIG_KEY_S 2
> > +#define BD718XX_PWR_TRIG_PMIC_ON 4
> > +#define BD718XX_PWR_TRIG_VSYS_UVLO 8
> > +#define BD718XX_RDY_TO_SNVS_SIFT 0
> > +#define BD718XX_SNVS_TO_RUN_SIFT 4
>
> As above.
I'll fix this
> > +
> > +
>
> No need for 2 '\n'.
I'll fix this
> > + BD718XX_PWRBTN_LONG_PRESS_15S
> > +};
> > +
> > +
> > +
>
> Certainly no need for 3 '\n'.
I'll fix this
> > +struct bd71837;
>
> Why is this necessary?
>
I think it's not. I'll remove it.
> > +struct bd71837_pmic;
> > +struct bd71837_clk;
> > +struct bd718xx_pwrkey;
>
> Where are these forward declared from?
bd71837_pmic is in drivers/regulator/bd71837-regulator.c
clk will be in clk subdevice patch that is not yet applied.
It is pending some fixes. Pwrkey must be dropped as the power-key driver
I originally wrote was replaced using gpio_keys instead. Should there be
some comment explaining this? Suggestions on how to do this without
referring to file names which are subject to change... Should I just say
they are defined in subdevice drivers?
> > +/*
> > + * Board platform data may be used to initialize regulators.
> > + */
> > +
> > +struct bd71837_board {
> > + struct regulator_init_data *init_data[BD71837_REGULATOR_CNT];
> > + int gpio_intr;
> > +};
>
> Where is this populated?
>
Currently nowhere as I use device-tree for getting the regulator/irq
config. This is for architectures which do not use DT - but as I don't
have one for testing I did leave the depends_on OF. If it was populated
I would expect it to be done in some setup code.
> > +/*
> > + * bd71837 sub-driver chip access routines
> > + */
> > +
>
> Please don't abstract APIs for no reason.
>
> Use the regmap_* API directly instead.
>
Yes. This was already pointed out by Stephen Boyd. But again, as part of
the patches (reguators) were already applied using the wrappers - I asked
if I can remove these in separate patch set after getting this initial
version out.
> > +#endif /* __LINUX_MFD_BD71837_H__ */
>
> --
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v9 2/2] mfd: bd71837: Devicetree bindings for ROHM BD71837 PMIC
From: Matti Vaittinen @ 2018-07-05 10:39 UTC (permalink / raw)
To: Lee Jones
Cc: mturquette, robh+dt, mark.rutland, lgirdwood, broonie,
mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo,
sboyd, linux-clk, devicetree, linux-kernel, linux-input,
mikko.mutanen, heikki.haikola
In-Reply-To: <20180705092444.GL496@dell>
And thanks again Lee!
On Thu, Jul 05, 2018 at 10:24:44AM +0100, Lee Jones wrote:
> On Thu, 05 Jul 2018, Matti Vaittinen wrote:
>
> > Document devicetree bindings for ROHM BD71837 PMIC MFD.
> >
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> > .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt | 67 ++++++++++++++++++++++
> > 1 file changed, 67 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> > new file mode 100644
> > index 000000000000..67f2616288d9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> > @@ -0,0 +1,67 @@
> > +* ROHM BD71837 Power Management Integrated Circuit bindings
> > +
> > +BD71837MWV is a programmable Power Management IC for powering single-core,
> > +dual-core, and quad-core SoC’s such as NXP-i.MX 8M. It is optimized for
>
> Nit: s/SOC's/SOCs/
I'll change this.
> > +low BOM cost and compact solution footprint. It integrates 8 Buck
> > +egulators and 7 LDO’s to provide all the power rails required by the SoC and
>
> Nit: s/LDO's/LDOs/
I'll change this.
> > +the commonly used peripherals.
> > +
> > +Datasheet for PMIC is available at:
> > +https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
> > +
> > +Required properties:
> > + - compatible : Should be "rohm,bd71837".
> > + - reg : I2C slave address.
> > + - interrupt-parent : Phandle to the parent interrupt controller.
> > + - interrupts : The interrupt line the device is connected to.
> > + - clocks : The parent clock connected to PMIC. If this is missng
>
> s/missng/missing/
I'll change this.
> > + 32768 KHz clock is assumed.
> > + - #clock-cells : Should be 0
> > + - regulators: : List of child nodes that specify the regulators
>
> By your convention, the bottom two entries are missing '.'s.
I'll change this.
> > + Please see ../regulator/rohm,bd71837-regulator.txt
> > +
> > +Optional properties:
> > +- clock-output-names : Should contain name for output clock.
> > +
> > +Example:
> > +
> > + /* external oscillator node */
>
> Uppercase character to start a sentence.
I'll change this.
> > + osc: oscillator {
> > + compatible = "fixed-clock";
> > + #clock-cells = <1>;
> > + clock-frequency = <32768>;
> > + clock-output-names = "osc";
> > + };
> > +
> > + /* PMIC node */
>
> Is this comment really necessary? The node is called "pmic".
I'll remove this.
> > + pmic: pmic@4b {
> > + compatible = "rohm,bd71837";
> > + reg = <0x4b>;
> > + interrupt-parent = <&gpio1>;
> > + interrupts = <29 GPIO_ACTIVE_LOW>;
> > + interrupt-names = "irq";
> > + #clock-cells = <0>;
> > + clocks = <&osc 0>;
> > + clock-output-names = "bd71837-32k-out";
> > +
> > + regulators {
> > + buck1: BUCK1 {
> > + regulator-name = "buck1";
> > + regulator-min-microvolt = <700000>;
> > + regulator-max-microvolt = <1300000>;
> > + regulator-boot-on;
> > + regulator-ramp-delay = <1250>;
> > + };
> > + /* ... */
> > + };
> > + };
> > +
> > + /* Clock consumer node */
> > +
> > + foo@0 {
> > + compatible = "bar,foo";
> > + /* ... */
>
> No need for this line. Additional properties are always expected.
I'll remove this.
> > + clock-names = "my-clock";
> > + clocks = <&pmic>;
> > + };
>
> Do you have a real example to give?
For clock consumer? Sorry, no I don't.
Br,
Matti Vaittinen
> --
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v9 2/2] mfd: bd71837: Devicetree bindings for ROHM BD71837 PMIC
From: Lee Jones @ 2018-07-05 10:49 UTC (permalink / raw)
To: Matti Vaittinen
Cc: mturquette, robh+dt, mark.rutland, lgirdwood, broonie,
mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo,
sboyd, linux-clk, devicetree, linux-kernel, linux-input,
mikko.mutanen, heikki.haikola
In-Reply-To: <20180705103919.GB8426@localhost.localdomain>
On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> On Thu, Jul 05, 2018 at 10:24:44AM +0100, Lee Jones wrote:
> > On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> >
> > > Document devicetree bindings for ROHM BD71837 PMIC MFD.
> > >
> > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > ---
> > > .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt | 67 ++++++++++++++++++++++
> > > 1 file changed, 67 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> > > + clock-names = "my-clock";
> > > + clocks = <&pmic>;
> > > + };
> >
> > Do you have a real example to give?
>
> For clock consumer? Sorry, no I don't.
Might be better to drop it for the time being then.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v9 1/2] mfd: bd71837: mfd driver for ROHM BD71837 PMIC
From: Lee Jones @ 2018-07-05 11:17 UTC (permalink / raw)
To: Matti Vaittinen
Cc: mturquette, robh+dt, mark.rutland, lgirdwood, broonie,
mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo,
sboyd, linux-clk, devicetree, linux-kernel, linux-input,
mikko.mutanen, heikki.haikola
In-Reply-To: <20180705103442.GA8426@localhost.localdomain>
On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> Thanks for taking the time to check this =)
>
> On Thu, Jul 05, 2018 at 10:18:13AM +0100, Lee Jones wrote:
> > On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> >
> > > --- a/drivers/mfd/Kconfig
> > > +++ b/drivers/mfd/Kconfig
> > > @@ -1787,6 +1787,19 @@ config MFD_STW481X
> > > in various ST Microelectronics and ST-Ericsson embedded
> > > Nomadik series.
> > >
> > > +config MFD_BD71837
> >
> > It's be nice if you prefix this with the manufacturer. Rohm?
>
> Like MFD_ROHM_BD71837, right? How important this is? The already applied
> regulator part has depends_on for MFD_BD71837 in Kconfig so changing
> this will require change in regulator tree too. I guess it's doable if
> this is important though =)
How can it depend on something that doesn't exist. ;)
It's better, then similar devices can be grouped.
[...]
> > > new file mode 100644
> > > index 000000000000..677097bcea17
> > > --- /dev/null
> > > +++ b/drivers/mfd/bd71837.c
> > > @@ -0,0 +1,215 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> >
> > '\n' here please.
> > > +// Copyright (C) 2018 ROHM Semiconductors
> >
> > '\n' here please.
>
> As an empty line after licence line? Ok.
Just '//', yes.
> > > +#include <linux/i2c.h>
> > > +#include <linux/input.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/mfd/bd71837.h>
> > > +#include <linux/mfd/core.h>
> > > +#include <linux/module.h>
> > > +#include <linux/regmap.h>
> > > +#include <linux/gpio_keys.h>
> >
> > Alphabetical please.
> >
>
> There is a problem with gpio_key.h - it uses bool but does not include
> the header defining this type. So I get comple error when compiling this
> using my GCC arm cross-compiler. I've submitted a patch to fix this but
> as it is not yet accepted.
> https://lore.kernel.org/lkml/20180627073458.GA27800@localhost.localdomain/
> Hence the gpio_keys.h is last one.
If the ordering is important, separate it from the pack and place a
little comment in to say why you've done so. That way no well meaning
contributor will attempt to tidy it up in the future. If this will be
fixed anyway, perhaps the point is moot?
> > > +static struct gpio_keys_button btns[] = {
> > > + {
> > > + .code = KEY_POWER,
> > > + .gpio = -1,
> > > + .type = EV_KEY,
> > > + },
> > > +};
> >
> > Does this need to be an array?
>
> https://elixir.bootlin.com/linux/v4.17-rc1/source/include/linux/gpio_keys.h#L39
> gpio_keys_platform_data documentation states the buttons to be a pointer
> to an array. Thus I did an array. Do you think I should change this to
> single struct?
I can't think of any reason why not.
> > > +static struct gpio_keys_platform_data bd718xx_powerkey_data = {
> > > + .buttons = &btns[0],
> >
> > Why not just "btns"?
> >
> > Also, I don't see any reason not to just call this 'buttons'.
> >
> > Or even, 'button' if there will only ever be one of them.
>
> I can rename this - and if I make the array to not be an array - well,
> then the &foo[0] gets cleared too =)
Exactly.
> > > +static struct mfd_cell bd71837_mfd_cells[] = {
> > > + {
> > > + .name = "bd71837-clk",
> > > + }, {
> > > + .name = "gpio-keys",
> > > + .platform_data = &bd718xx_powerkey_data,
> > > + .pdata_size = sizeof(bd718xx_powerkey_data),
> > > + }, {
> > > + .name = "bd71837-pmic",
> > > + },
> > > +};
> >
> > Place the one line entries on one line, like this:
> >
> > { .name = "bd71837-pmic", },
> >
> > ... and if ordering is not important, place them at the bottom.
>
> I'll fix this
It's best to simply snip all the parts you agree with.
[...]
> > > + /*
> > > + * According to BD71847 datasheet the HW default for long press
> > > + * detection is 10ms. So lets change it to 10 sec so we can actually
> > > + * get the short push and allow gracefull shut down
> > > + */
> >
> > I don't think this comment is necessary.
> >
> > If anything it only needs to say "Configure long press to 10 seconds".
> >
> > And if you do that, please add one for short press too.
> Difference between short and long push is that if PMIC detects log push
> it will kill the power without any gracefull handling. And the chip I am
> supporting right now is BD71837 - which has more sane default value (10
> seconds). But I am going to add support for BD71847 - which was shipped
> to me yesterday - and according the draft data-sheet the default on it
> is 10ms - which makes short push unusable. So comment is intended to
> point out _why_ I configure the long push here. Also someone might want
> to use the HW default (I guess such default was selected for some
> specific reason) so comment helps on removing this configuration for
> specific use-cases. But I guess I can reduce the comment to "Configure
> long press to 10 seconds" as you suggested - or do you think I should
> make difference between ICs BD71837 and BD71847 more visible here?
Honestly, I don't think it's required. If anyone were to try to
change it, I would hope that they too would have read the datasheet
and understand what they are doing.
> > > + ret = regmap_update_bits(bd71837->regmap,
> > > + BD71837_REG_PWRONCONFIG1,
> > > + BD718XX_PWRBTN_PRESS_DURATION_MASK,
> > > + BD718XX_PWRBTN_LONG_PRESS_10S);
> > > +
> > > + if (ret < 0) {
> > > + dev_err(&i2c->dev, "Failed to configure button long press timeout\n");
> > > + goto err_out;
> > > + }
> > > +
> > > + btns[0].irq = regmap_irq_get_virq(bd71837->irq_data,
> > > + BD71837_INT_PWRBTN_S);
> > > +
> > > + if (btns[0].irq < 0) {
> > > + ret = btns[0].irq;
> > > + dev_err(&i2c->dev, "Failed to get the IRQ\n");
> > > + goto err_out;
> >
> > No unwinding to be done, just return directly and save yourself an
> > superflouous goto.
>
> So should I replace _all_ the gotos by in-place returns as Enric suggested
> yesterday? As I explained yesterday, my preference is to have only one
> point of exit but I can change gotos to returns if needed.
I'm not sure where the 'one point of exit' thing stems from and I
can't think of any good reason for it. The main reason for using a
goto in the exit path is to save lines of code and code duplication.
Your method *adds* lines of code by firstly setting 'ret', then
calling the goto. If nothing needs to be done before returning, just
return the error value with one line.
> > > +static const struct i2c_device_id bd71837_i2c_id[] = {
> > > + { .name = "bd71837", },
> > > + { }
> > > +};
> > > +MODULE_DEVICE_TABLE(i2c, bd71837_i2c_id);
> >
> > You shouldn't need this table anymore.
> >
> > See da10c06a044b ("i2c: Make I2C ID tables non-mandatory for DT'ed devices")
>
> So if I read this correctly the
> > > + .of_match_table = bd71837_of_match,
> is now sufficient. Thanks.
Right.
> > > +++ b/include/linux/mfd/bd71837.h
> > > @@ -0,0 +1,391 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> >
> > I thought these were meant to use C++ (//) comments?
>
> The checkpatch.pl did complain if I used // comment on SPDX line for
> header file. OTOH for c-file it required // comments and complained
> about /* */ - version. So I did as it suggested and used
Well that's just dandy -- who comes up with this stuff?
> > > +/* Copyright (C) 2018 ROHM Semiconductors */
> > > +
> > > +/*
> > > + * ROHM BD71837MWV header file
> > > + */
> >
> > If you prefix the mentions of (BD,bd)71837 with (ROHM,rohm) then this
> > comment becomes redundant and you can remove it.
>
> I am sorry but I am not sure what you suggest here. Do you mean I should
> add ROHM or rohm to all definitions here? I think that would make
> definitions pretty long so I would certinly need to split more lines.
> Such cange would also impact already applied patches. So maybe I
> misinterpreted your comment? And in case I did not - can this prefixing of
> types - be done as a separate patch set as it impacts to regulators and
> clk too? (clk is not yet applied so that is easy to change though).
Any lines which are already long, you can justify as not having to do
this, however I think for the filenames and function names it would
be nice if they were prefixed.
Filenames are particularly important. That way all of the Rohm
drivers will be grouped. Unless you can be assured that all Rohm
devices will be prefixed by 'bd', then the point is slightly mooted,
however since 'bd' doesn't really correlate with 'rohm' it's still
difficult to assume that bd* drivers are associated with Rohm -- if
you catch my drift.
For instance
- 'wm' for Wolfson Microelectonics
- 'max' for Maxim
- 'intel' for, well, you know
Etc.
[...]
> > > +struct bd71837_pmic;
> > > +struct bd71837_clk;
> > > +struct bd718xx_pwrkey;
> >
> > Where are these forward declared from?
>
> bd71837_pmic is in drivers/regulator/bd71837-regulator.c
> clk will be in clk subdevice patch that is not yet applied.
> It is pending some fixes. Pwrkey must be dropped as the power-key driver
> I originally wrote was replaced using gpio_keys instead. Should there be
> some comment explaining this? Suggestions on how to do this without
> referring to file names which are subject to change... Should I just say
> they are defined in subdevice drivers?
I just wanted you to think about if they were really necessary.
> > > +/*
> > > + * Board platform data may be used to initialize regulators.
> > > + */
> > > +
> > > +struct bd71837_board {
> > > + struct regulator_init_data *init_data[BD71837_REGULATOR_CNT];
> > > + int gpio_intr;
> > > +};
> >
> > Where is this populated?
> >
> Currently nowhere as I use device-tree for getting the regulator/irq
> config. This is for architectures which do not use DT - but as I don't
> have one for testing I did leave the depends_on OF. If it was populated
> I would expect it to be done in some setup code.
Please don't add code for 'what ifs'.
Please remove it and add it back when you need it.
> > > +/*
> > > + * bd71837 sub-driver chip access routines
> > > + */
> > > +
> >
> > Please don't abstract APIs for no reason.
> >
> > Use the regmap_* API directly instead.
> >
>
> Yes. This was already pointed out by Stephen Boyd. But again, as part of
> the patches (reguators) were already applied using the wrappers - I asked
> if I can remove these in separate patch set after getting this initial
> version out.
That is one of the issues with applying related patches without each
of them being reviewed first. Applying unsuitable code is not the
correct thing to do, sorry.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v9 2/2] mfd: bd71837: Devicetree bindings for ROHM BD71837 PMIC
From: Matti Vaittinen @ 2018-07-05 11:51 UTC (permalink / raw)
To: Lee Jones
Cc: mturquette, robh+dt, mark.rutland, lgirdwood, broonie,
mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo,
sboyd, linux-clk, devicetree, linux-kernel, linux-input,
mikko.mutanen, heikki.haikola
In-Reply-To: <20180705104919.GM496@dell>
On Thu, Jul 05, 2018 at 11:49:19AM +0100, Lee Jones wrote:
> On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> > On Thu, Jul 05, 2018 at 10:24:44AM +0100, Lee Jones wrote:
> > > On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> > >
> > > > Document devicetree bindings for ROHM BD71837 PMIC MFD.
> > > >
> > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > ---
> > > > .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt | 67 ++++++++++++++++++++++
> > > > 1 file changed, 67 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> > > > + clock-names = "my-clock";
> > > > + clocks = <&pmic>;
> > > > + };
> > >
> > > Do you have a real example to give?
> >
> > For clock consumer? Sorry, no I don't.
>
> Might be better to drop it for the time being then.
I have tested the clk driver using this dummy consumer. So in a sense it
"works" and can be used as an example on how to write a real clock
consumer node. Thus I see some value in this example node - even if it
does not match to any real world HW. If I had to use the clk from this
PMIC and write HW description I would appreciate this dummy exaple. I
can drop it if you insist - but I would at least like to hear what is
the downside on having it here?
Best regards,
Matti Vaittinen
^ permalink raw reply
* Re: [PATCH v9 1/2] mfd: bd71837: mfd driver for ROHM BD71837 PMIC
From: Matti Vaittinen @ 2018-07-05 12:29 UTC (permalink / raw)
To: Lee Jones
Cc: mturquette, robh+dt, mark.rutland, lgirdwood, broonie,
mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo,
sboyd, linux-clk, devicetree, linux-kernel, linux-input,
mikko.mutanen, heikki.haikola
In-Reply-To: <20180705111705.GN496@dell>
On Thu, Jul 05, 2018 at 12:17:05PM +0100, Lee Jones wrote:
> On Thu, 05 Jul 2018, Matti Vaittinen wrote:
>
> > Thanks for taking the time to check this =)
> >
> > On Thu, Jul 05, 2018 at 10:18:13AM +0100, Lee Jones wrote:
> > > On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> > >
> > > > --- a/drivers/mfd/Kconfig
> > > > +++ b/drivers/mfd/Kconfig
> > > > @@ -1787,6 +1787,19 @@ config MFD_STW481X
> > > > in various ST Microelectronics and ST-Ericsson embedded
> > > > Nomadik series.
> > > >
> > > > +config MFD_BD71837
> > >
> > > It's be nice if you prefix this with the manufacturer. Rohm?
> >
> > Like MFD_ROHM_BD71837, right? How important this is? The already applied
> > regulator part has depends_on for MFD_BD71837 in Kconfig so changing
> > this will require change in regulator tree too. I guess it's doable if
> > this is important though =)
>
> How can it depend on something that doesn't exist. ;)
>
> It's better, then similar devices can be grouped.
>
> > > > +#include <linux/i2c.h>
> > > > +#include <linux/input.h>
> > > > +#include <linux/interrupt.h>
> > > > +#include <linux/mfd/bd71837.h>
> > > > +#include <linux/mfd/core.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/regmap.h>
> > > > +#include <linux/gpio_keys.h>
> > >
> > > Alphabetical please.
> > >
> >
> > There is a problem with gpio_key.h - it uses bool but does not include
> > the header defining this type. So I get comple error when compiling this
> > using my GCC arm cross-compiler. I've submitted a patch to fix this but
> > as it is not yet accepted.
> > https://lore.kernel.org/lkml/20180627073458.GA27800@localhost.localdomain/
> > Hence the gpio_keys.h is last one.
>
> If the ordering is important, separate it from the pack and place a
> little comment in to say why you've done so. That way no well meaning
> contributor will attempt to tidy it up in the future. If this will be
> fixed anyway, perhaps the point is moot?
I have no idea if my patch to add missing include is going to be
applied. I've not heard anything from it in a week or so. So I'll just
do as you suggested and drop a comment on why it is left last.
> > > > + /*
> > > > + * According to BD71847 datasheet the HW default for long press
> > > > + * detection is 10ms. So lets change it to 10 sec so we can actually
> > > > + * get the short push and allow gracefull shut down
> > > > + */
> > >
> > > I don't think this comment is necessary.
> > >
> > > If anything it only needs to say "Configure long press to 10 seconds".
> > >
> > > And if you do that, please add one for short press too.
>
> > Difference between short and long push is that if PMIC detects log push
> > it will kill the power without any gracefull handling. And the chip I am
> > supporting right now is BD71837 - which has more sane default value (10
> > seconds). But I am going to add support for BD71847 - which was shipped
> > to me yesterday - and according the draft data-sheet the default on it
> > is 10ms - which makes short push unusable. So comment is intended to
> > point out _why_ I configure the long push here. Also someone might want
> > to use the HW default (I guess such default was selected for some
> > specific reason) so comment helps on removing this configuration for
> > specific use-cases. But I guess I can reduce the comment to "Configure
> > long press to 10 seconds" as you suggested - or do you think I should
> > make difference between ICs BD71837 and BD71847 more visible here?
>
> Honestly, I don't think it's required. If anyone were to try to
> change it, I would hope that they too would have read the datasheet
> and understand what they are doing.
You still have the hope and believe in good developers? ;) Allright,
I'll drop the comment.
> > > > + ret = regmap_update_bits(bd71837->regmap,
> > > > + BD71837_REG_PWRONCONFIG1,
> > > > + BD718XX_PWRBTN_PRESS_DURATION_MASK,
> > > > + BD718XX_PWRBTN_LONG_PRESS_10S);
> > > > +
> > > > + if (ret < 0) {
> > > > + dev_err(&i2c->dev, "Failed to configure button long press timeout\n");
> > > > + goto err_out;
> > > > + }
> > > > +
> > > > + btns[0].irq = regmap_irq_get_virq(bd71837->irq_data,
> > > > + BD71837_INT_PWRBTN_S);
> > > > +
> > > > + if (btns[0].irq < 0) {
> > > > + ret = btns[0].irq;
> > > > + dev_err(&i2c->dev, "Failed to get the IRQ\n");
> > > > + goto err_out;
> > >
> > > No unwinding to be done, just return directly and save yourself an
> > > superflouous goto.
> >
> > So should I replace _all_ the gotos by in-place returns as Enric suggested
> > yesterday? As I explained yesterday, my preference is to have only one
> > point of exit but I can change gotos to returns if needed.
>
> I'm not sure where the 'one point of exit' thing stems from and I
> can't think of any good reason for it. The main reason for using a
> goto in the exit path is to save lines of code and code duplication.
> Your method *adds* lines of code by firstly setting 'ret', then
> calling the goto. If nothing needs to be done before returning, just
> return the error value with one line.
The point stems from my personal experiences =) I've once spent almost
two years cleaning/fixing up a code which left resources/locks leaking
from various function exits at error branches. Back then I sweared I
would always build just one clean-up sequence at the end of function and
never return from the middle. So this is my personal fixation. But as I
said, I will replace gotos with in place returns - and cross my fingers
no one adds a (non devm) resource allocation or locking in the probe =)
> > > > +++ b/include/linux/mfd/bd71837.h
> > > > @@ -0,0 +1,391 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > >
> > > I thought these were meant to use C++ (//) comments?
> >
> > The checkpatch.pl did complain if I used // comment on SPDX line for
> > header file. OTOH for c-file it required // comments and complained
> > about /* */ - version. So I did as it suggested and used
>
> Well that's just dandy -- who comes up with this stuff?
Engineers I bet =)
> > > > +/* Copyright (C) 2018 ROHM Semiconductors */
> > > > +
> > > > +/*
> > > > + * ROHM BD71837MWV header file
> > > > + */
> > >
> > > If you prefix the mentions of (BD,bd)71837 with (ROHM,rohm) then this
> > > comment becomes redundant and you can remove it.
> >
> > I am sorry but I am not sure what you suggest here. Do you mean I should
> > add ROHM or rohm to all definitions here? I think that would make
> > definitions pretty long so I would certinly need to split more lines.
> > Such cange would also impact already applied patches. So maybe I
> > misinterpreted your comment? And in case I did not - can this prefixing of
> > types - be done as a separate patch set as it impacts to regulators and
> > clk too? (clk is not yet applied so that is easy to change though).
>
> Any lines which are already long, you can justify as not having to do
> this, however I think for the filenames and function names it would
> be nice if they were prefixed.
Right. For file names this should be easily doable. And when the regmap
wrappers are removed there are no public functions left. And I think the
name of file containing the functions is sufficient for grouping
functions under "Rohm", right?
> Filenames are particularly important. That way all of the Rohm
> drivers will be grouped. Unless you can be assured that all Rohm
> devices will be prefixed by 'bd', then the point is slightly mooted,
> however since 'bd' doesn't really correlate with 'rohm' it's still
> difficult to assume that bd* drivers are associated with Rohm -- if
> you catch my drift.
I guess I do catch it. And no, all ROHM stuff will definitely not be
prefixed with bd - which is the name of the power chip - I mean power IC
series.
> > > > +struct bd71837_pmic;
> > > > +struct bd71837_clk;
> > > > +struct bd718xx_pwrkey;
> > >
> > > Where are these forward declared from?
> >
> > bd71837_pmic is in drivers/regulator/bd71837-regulator.c
> > clk will be in clk subdevice patch that is not yet applied.
> > It is pending some fixes. Pwrkey must be dropped as the power-key driver
> > I originally wrote was replaced using gpio_keys instead. Should there be
> > some comment explaining this? Suggestions on how to do this without
> > referring to file names which are subject to change... Should I just say
> > they are defined in subdevice drivers?
>
> I just wanted you to think about if they were really necessary.
Right =) Well done on that=)
> > > > +/*
> > > > + * Board platform data may be used to initialize regulators.
> > > > + */
> > > > +
> > > > +struct bd71837_board {
> > > > + struct regulator_init_data *init_data[BD71837_REGULATOR_CNT];
> > > > + int gpio_intr;
> > > > +};
> > >
> > > Where is this populated?
> > >
> > Currently nowhere as I use device-tree for getting the regulator/irq
> > config. This is for architectures which do not use DT - but as I don't
> > have one for testing I did leave the depends_on OF. If it was populated
> > I would expect it to be done in some setup code.
>
> Please don't add code for 'what ifs'.
>
> Please remove it and add it back when you need it.
Allright. Although this will also break the regulator part then...
> > > > +/*
> > > > + * bd71837 sub-driver chip access routines
> > > > + */
> > > > +
> > >
> > > Please don't abstract APIs for no reason.
> > >
> > > Use the regmap_* API directly instead.
> > >
> >
> > Yes. This was already pointed out by Stephen Boyd. But again, as part of
> > the patches (reguators) were already applied using the wrappers - I asked
> > if I can remove these in separate patch set after getting this initial
> > version out.
>
> That is one of the issues with applying related patches without each
> of them being reviewed first. Applying unsuitable code is not the
> correct thing to do, sorry.
So I assume you are not Ok with adding the wrappers and removing them
with later set of patches? I'll do following workaround then:
1. Change MFD Kconfig option name => current regulator code will not be
compiled even if the MFD would be applied.
2. Change MFD according to this discussion (and break the compatibility
with applied regulator code)
3. Fix the regulator code with further patches to Mark
4. Fix the depends_on Kconfig option in regulator tree to match the new
one suggested here.
Does this sound reasonable?
Best Regards
Matti Vaittinen
> --
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v9 2/2] mfd: bd71837: Devicetree bindings for ROHM BD71837 PMIC
From: Lee Jones @ 2018-07-05 12:44 UTC (permalink / raw)
To: Matti Vaittinen
Cc: mturquette, robh+dt, mark.rutland, lgirdwood, broonie,
mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo,
sboyd, linux-clk, devicetree, linux-kernel, linux-input,
mikko.mutanen, heikki.haikola
In-Reply-To: <20180705115143.GC8426@localhost.localdomain>
On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> On Thu, Jul 05, 2018 at 11:49:19AM +0100, Lee Jones wrote:
> > On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> > > On Thu, Jul 05, 2018 at 10:24:44AM +0100, Lee Jones wrote:
> > > > On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> > > >
> > > > > Document devicetree bindings for ROHM BD71837 PMIC MFD.
> > > > >
> > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > > ---
> > > > > .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt | 67 ++++++++++++++++++++++
> > > > > 1 file changed, 67 insertions(+)
> > > > > create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> > > > > + clock-names = "my-clock";
> > > > > + clocks = <&pmic>;
> > > > > + };
> > > >
> > > > Do you have a real example to give?
> > >
> > > For clock consumer? Sorry, no I don't.
> >
> > Might be better to drop it for the time being then.
>
> I have tested the clk driver using this dummy consumer. So in a sense it
> "works" and can be used as an example on how to write a real clock
> consumer node. Thus I see some value in this example node - even if it
> does not match to any real world HW. If I had to use the clk from this
> PMIC and write HW description I would appreciate this dummy exaple. I
> can drop it if you insist - but I would at least like to hear what is
> the downside on having it here?
My suggestion then would be to make it look as authentic as possible.
It is only an example, so it doesn't *really* matter, but the current
foo,bar one just looks a bit crumby.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v9 1/2] mfd: bd71837: mfd driver for ROHM BD71837 PMIC
From: Lee Jones @ 2018-07-05 12:51 UTC (permalink / raw)
To: Matti Vaittinen
Cc: mturquette, robh+dt, mark.rutland, lgirdwood, broonie,
mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo,
sboyd, linux-clk, devicetree, linux-kernel, linux-input,
mikko.mutanen, heikki.haikola
In-Reply-To: <20180705122923.GD8426@localhost.localdomain>
> > > > > +++ b/include/linux/mfd/bd71837.h
> > > > > @@ -0,0 +1,391 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > >
> > > > I thought these were meant to use C++ (//) comments?
> > >
> > > The checkpatch.pl did complain if I used // comment on SPDX line for
> > > header file. OTOH for c-file it required // comments and complained
> > > about /* */ - version. So I did as it suggested and used
> >
> > Well that's just dandy -- who comes up with this stuff?
>
> Engineers I bet =)
Ones who do not suffer from OCD, clearly!
> > > > > +/* Copyright (C) 2018 ROHM Semiconductors */
> > > > > +
> > > > > +/*
> > > > > + * ROHM BD71837MWV header file
> > > > > + */
> > > >
> > > > If you prefix the mentions of (BD,bd)71837 with (ROHM,rohm) then this
> > > > comment becomes redundant and you can remove it.
> > >
> > > I am sorry but I am not sure what you suggest here. Do you mean I should
> > > add ROHM or rohm to all definitions here? I think that would make
> > > definitions pretty long so I would certinly need to split more lines.
> > > Such cange would also impact already applied patches. So maybe I
> > > misinterpreted your comment? And in case I did not - can this prefixing of
> > > types - be done as a separate patch set as it impacts to regulators and
> > > clk too? (clk is not yet applied so that is easy to change though).
> >
> > Any lines which are already long, you can justify as not having to do
> > this, however I think for the filenames and function names it would
> > be nice if they were prefixed.
>
> Right. For file names this should be easily doable. And when the regmap
> wrappers are removed there are no public functions left. And I think the
> name of file containing the functions is sufficient for grouping
> functions under "Rohm", right?
That's fine.
> > Filenames are particularly important. That way all of the Rohm
> > drivers will be grouped. Unless you can be assured that all Rohm
> > devices will be prefixed by 'bd', then the point is slightly mooted,
> > however since 'bd' doesn't really correlate with 'rohm' it's still
> > difficult to assume that bd* drivers are associated with Rohm -- if
> > you catch my drift.
>
> I guess I do catch it. And no, all ROHM stuff will definitely not be
> prefixed with bd - which is the name of the power chip
> I mean power IC series.
Now you're getting it! ;)
> > > > > +struct bd71837_board {
> > > > > + struct regulator_init_data *init_data[BD71837_REGULATOR_CNT];
> > > > > + int gpio_intr;
> > > > > +};
> > > >
> > > > Where is this populated?
> > > >
> > > Currently nowhere as I use device-tree for getting the regulator/irq
> > > config. This is for architectures which do not use DT - but as I don't
> > > have one for testing I did leave the depends_on OF. If it was populated
> > > I would expect it to be done in some setup code.
> >
> > Please don't add code for 'what ifs'.
> >
> > Please remove it and add it back when you need it.
>
> Allright. Although this will also break the regulator part then...
Well, it's broken anyway ...
> > > > > +/*
> > > > > + * bd71837 sub-driver chip access routines
> > > > > + */
> > > > > +
> > > >
> > > > Please don't abstract APIs for no reason.
> > > >
> > > > Use the regmap_* API directly instead.
> > > >
> > >
> > > Yes. This was already pointed out by Stephen Boyd. But again, as part of
> > > the patches (reguators) were already applied using the wrappers - I asked
> > > if I can remove these in separate patch set after getting this initial
> > > version out.
> >
> > That is one of the issues with applying related patches without each
> > of them being reviewed first. Applying unsuitable code is not the
> > correct thing to do, sorry.
>
> So I assume you are not Ok with adding the wrappers and removing them
> with later set of patches? I'll do following workaround then:
No, I'm not okay with that at all. :|
> 1. Change MFD Kconfig option name => current regulator code will not be
> compiled even if the MFD would be applied.
> 2. Change MFD according to this discussion (and break the compatibility
> with applied regulator code)
> 3. Fix the regulator code with further patches to Mark
> 4. Fix the depends_on Kconfig option in regulator tree to match the new
> one suggested here.
>
> Does this sound reasonable?
That's how I would do it.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH] Input: i8042 - add Lenovo LaVie Z to the i8042 reset list
From: Chen-Yu Tsai @ 2018-07-05 13:08 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Chen-Yu Tsai, linux-input, linux-kernel, stable
The Lenovo LaVie Z laptop requires i8042 to be reset in order to
consistently detect its Elantech touchpad. The nomux and kbdreset
quirks are not sufficient.
It's possible the other LaVie Z models from NEC require this as well.
Cc: stable@vger.kernel.org
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/input/serio/i8042-x86ia64io.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
index b353d494ad40..136f6e7bf797 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -527,6 +527,13 @@ static const struct dmi_system_id __initconst i8042_dmi_nomux_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "N24_25BU"),
},
},
+ {
+ /* Lenovo LaVie Z */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo LaVie Z"),
+ },
+ },
{ }
};
--
2.18.0
^ permalink raw reply related
* [PATCH v10 0/2] mfd/regulator/clk/input: bd71837: ROHM BD71837 PMIC driver
From: Matti Vaittinen @ 2018-07-05 14:31 UTC (permalink / raw)
To: mturquette, robh+dt, sboyd, mark.rutland, lee.jones, lgirdwood,
broonie, mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo
Cc: linux-clk, devicetree, linux-kernel, linux-input, mikko.mutanen,
heikki.haikola
Patch series adding support for ROHM BD71837 PMIC.
BD71837 is a programmable Power Management IC for powering single-core,
dual-core, and quad-core SoCs such as NXP-i.MX 8M. It is optimized for
low BOM cost and compact solution footprint. It integrates 8 buck
regulators and 7 LDOs to provide all the power rails required by the
SoC and the commonly used peripherals.
This is reduced set of patches containing only the MFD and devicetree
bindings. This enables alrady applied regulator part and power button
support using gpio-keys. Clock and reset support will be sent as separate
set of patches - possibly only after my vacations. Sorry for longish delay
which will follow.
Changelog v10
Fixes based on comments from Lee Jones
- Drop the platform data.
- Drop the regmap wrapper functions and use regmap directly.
- Rename files with rohm-prefix.
- Replace goto from probe by in-place returns.
- Drop unnecessary i2c_id table.
- Change gpio_keys_button array to simple struct.
- Fix return value checks for regmap functions.
- Various styling/spelling fixes.
- Replace foo-bar with something more meaningfull in DT example
Changelog v9
Fixes initiated by feedback from Enric Balletbo Serra, Lee and Dmitry.
- Simplified control flow for probe
- Removed accidentally left squash commit message
- Some comma presence toggling after last member in an array =)
- Styling of multi line comments.
Changelog v8
- Dropped clk-bd71837 from series (will send later)
- Dropped bd718xx-pwrkey driver and used gpio_keys instead.
- Added power-button short/long press duration configuratio to MFD
- Cleaned MFD driver according to comments from Enric Balletbo Serra.
(used devm, removed unnecessary header inclusions, removed redundant
assignment, styling issues, allow building MFD part as module, fixed
license mismatch).
Changelog v7
- patch 1: Cleaned MFD probe since MFD no longer directly reads DT
properties.
- patch 1/4: Moved power-key related definitions from powerkey patch (4)
to MFD patch (1) so that powerkey can be applied independently
- Patch 2 is unchanged.
- patch 3: Added missing allocation check back to clk probe
Changelog v6
- Added power-key input driver
Based on feedback from Rob Herring and Stephen Boyd
- Added link to datasheet
- Removed interrupt-controller from DT and fixed binding document
- clk styling fixes
- remove clkdev usage
- add clk bindings to MFD documentation
- removed clk binding document
Changelog v5
- dropped regulator patches which are already applied to Mark's tree
Based on feedback from Rob Herring and Stephen Boyd
- mfd bindings: explain why this can be interrupt-controller
- mfd bindings: describe interrupts better
- mfd bindings: require one cell interrupt specifier
- mfd bindings: use generic node names in example
- mfd driver: ack masked interrupt once at init
- clk bindings: use generic node names in example
- clk driver: use devm
- clk driver: use of_clk_add_hw_provider
- clk driver: change severity of print and how prints are emitted at
probe error path.
- clk driver: dropped forward declared functions
- clk configs: drop unnecessary dependencies
- clk driver: other styling issues
- mfd/clk DT: drop clk node.
Changelog v4
- remove mutex from regulator state check as core prevents simultaneous
accesses
- allow voltage change for bucks 1 to 4 when regulator is enabled
- fix indentiation problems
- properly correct SPDX comments
Changelog v3
- kill unused variable
- kill unused definitions
- use REGMAP_IRQ_REG
Changelog v2
Based on feedback from Mark Brown
- Squashed code and buildfile changes to same patch
- Fixed some styling issues
- Changed SPDX comments to CPP style
- Error out if voltage is changed when regulator is enabled instead of
Disabling the regulator for duration of change
- Use devm_regulator_register
- Remove compatible usage from regulators - use parent dev for config
- Add a note about using regulator-boot-on for BUCK6 and 7
- fixed warnings from kbuild test robot
patch 1:
MFD driver and definitions bringing interrupt support and
enabling clk, regulator and input subsystems.
patch 2:
MFD driver DT bindings
This patch series is based on for-mfd-next
---
Matti Vaittinen (2):
mfd: bd71837: mfd driver for ROHM BD71837 PMIC
mfd: bd71837: Devicetree bindings for ROHM BD71837 PMIC
.../devicetree/bindings/mfd/rohm,bd71837-pmic.txt | 62 ++++
drivers/mfd/Kconfig | 13 +
drivers/mfd/Makefile | 1 +
drivers/mfd/rohm-bd718x7.c | 198 ++++++++++++
include/linux/mfd/rohm-bd718x7.h | 333 +++++++++++++++++++++
5 files changed, 607 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
create mode 100644 drivers/mfd/rohm-bd718x7.c
create mode 100644 include/linux/mfd/rohm-bd718x7.h
--
2.14.3
^ permalink raw reply
* [PATCH v10 1/2] mfd: bd71837: mfd driver for ROHM BD71837 PMIC
From: Matti Vaittinen @ 2018-07-05 14:32 UTC (permalink / raw)
To: mturquette, robh+dt, sboyd, mark.rutland, lee.jones, lgirdwood,
broonie, mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo
Cc: linux-clk, devicetree, linux-kernel, linux-input, mikko.mutanen,
heikki.haikola
In-Reply-To: <cover.1530800707.git.matti.vaittinen@fi.rohmeurope.com>
ROHM BD71837 PMIC MFD driver providing interrupts and support
for three subsystems:
- clk
- Regulators
- input/power-key
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
drivers/mfd/Kconfig | 13 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/rohm-bd718x7.c | 198 +++++++++++++++++++++++
include/linux/mfd/rohm-bd718x7.h | 333 +++++++++++++++++++++++++++++++++++++++
4 files changed, 545 insertions(+)
create mode 100644 drivers/mfd/rohm-bd718x7.c
create mode 100644 include/linux/mfd/rohm-bd718x7.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index b860eb5aa194..ac317c9bf065 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1787,6 +1787,19 @@ config MFD_STW481X
in various ST Microelectronics and ST-Ericsson embedded
Nomadik series.
+config MFD_ROHM_BD718XX
+ tristate "ROHM BD71837 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 BD71837
+ Power Management ICs. BD71837 is designed to power processors like
+ NXP i.MX8. It contains 8 BUCK outputs and 7 LDOs, voltage monitoring
+ and emergency shut down as well as 32,768KHz clock output.
+
config MFD_STM32_LPTIMER
tristate "Support for STM32 Low-Power Timer"
depends on (ARCH_STM32 && OF) || COMPILE_TEST
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e9fd20dba18d..c7a5c3b0e776 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -227,4 +227,5 @@ obj-$(CONFIG_MFD_STM32_TIMERS) += stm32-timers.o
obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o
obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
+obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
new file mode 100644
index 000000000000..34df74940dfa
--- /dev/null
+++ b/drivers/mfd/rohm-bd718x7.c
@@ -0,0 +1,198 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+//
+// Copyright (C) 2018 ROHM Semiconductors
+//
+// ROHM BD71837MWV PMIC driver
+//
+// Datasheet available from
+// https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
+
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/rohm-bd718x7.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+/*
+ * gpio_keys.h requires definiton of bool. It is brought in
+ * by abowe includes. Keep this as last untill gpio_keys.h gets fixed.
+ */
+#include <linux/gpio_keys.h>
+
+static struct gpio_keys_button button = {
+ .code = KEY_POWER,
+ .gpio = -1,
+ .type = EV_KEY,
+};
+
+static struct gpio_keys_platform_data bd718xx_powerkey_data = {
+ .buttons = &button,
+ .nbuttons = 1,
+ .name = "bd718xx-pwrkey",
+};
+
+static struct mfd_cell bd71837_mfd_cells[] = {
+ {
+ .name = "gpio-keys",
+ .platform_data = &bd718xx_powerkey_data,
+ .pdata_size = sizeof(bd718xx_powerkey_data),
+ },
+ { .name = "bd71837-clk", },
+ { .name = "bd71837-pmic", },
+};
+
+static const struct regmap_irq bd71837_irqs[] = {
+ REGMAP_IRQ_REG(BD71837_INT_SWRST, 0, BD71837_INT_SWRST_MASK),
+ REGMAP_IRQ_REG(BD71837_INT_PWRBTN_S, 0, BD71837_INT_PWRBTN_S_MASK),
+ REGMAP_IRQ_REG(BD71837_INT_PWRBTN_L, 0, BD71837_INT_PWRBTN_L_MASK),
+ REGMAP_IRQ_REG(BD71837_INT_PWRBTN, 0, BD71837_INT_PWRBTN_MASK),
+ REGMAP_IRQ_REG(BD71837_INT_WDOG, 0, BD71837_INT_WDOG_MASK),
+ REGMAP_IRQ_REG(BD71837_INT_ON_REQ, 0, BD71837_INT_ON_REQ_MASK),
+ REGMAP_IRQ_REG(BD71837_INT_STBY_REQ, 0, BD71837_INT_STBY_REQ_MASK),
+};
+
+static struct regmap_irq_chip bd71837_irq_chip = {
+ .name = "bd71837-irq",
+ .irqs = bd71837_irqs,
+ .num_irqs = ARRAY_SIZE(bd71837_irqs),
+ .num_regs = 1,
+ .irq_reg_stride = 1,
+ .status_base = BD71837_REG_IRQ,
+ .mask_base = BD71837_REG_MIRQ,
+ .ack_base = BD71837_REG_IRQ,
+ .init_ack_masked = true,
+ .mask_invert = false,
+};
+
+static const struct regmap_range pmic_status_range = {
+ .range_min = BD71837_REG_IRQ,
+ .range_max = BD71837_REG_POW_STATE,
+};
+
+static const struct regmap_access_table volatile_regs = {
+ .yes_ranges = &pmic_status_range,
+ .n_yes_ranges = 1,
+};
+
+static const struct regmap_config bd71837_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .volatile_table = &volatile_regs,
+ .max_register = BD71837_MAX_REGISTER - 1,
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static const struct of_device_id bd71837_of_match[] = {
+ { .compatible = "rohm,bd71837", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, bd71837_of_match);
+
+static int bd71837_i2c_probe(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+ struct bd71837 *bd71837;
+ int ret;
+ unsigned int val;
+
+ bd71837 = devm_kzalloc(&i2c->dev, sizeof(struct bd71837), GFP_KERNEL);
+
+ if (!bd71837)
+ return -ENOMEM;
+
+ bd71837->chip_irq = i2c->irq;
+
+ if (!bd71837->chip_irq) {
+ dev_err(&i2c->dev, "No IRQ configured\n");
+ return -EINVAL;
+ }
+
+ bd71837->dev = &i2c->dev;
+ bd71837->i2c_client = i2c;
+ i2c_set_clientdata(i2c, bd71837);
+
+ bd71837->regmap = devm_regmap_init_i2c(i2c, &bd71837_regmap_config);
+ if (IS_ERR(bd71837->regmap)) {
+ dev_err(&i2c->dev, "regmap initialization failed\n");
+ return PTR_ERR(bd71837->regmap);
+ }
+
+ ret = regmap_read(bd71837->regmap, BD71837_REG_REV, &val);
+ if (ret) {
+ dev_err(&i2c->dev, "Read BD71837_REG_DEVICE failed\n");
+ return ret;
+ }
+
+ ret = devm_regmap_add_irq_chip(&i2c->dev, bd71837->regmap,
+ bd71837->chip_irq, IRQF_ONESHOT, 0,
+ &bd71837_irq_chip, &bd71837->irq_data);
+ if (ret) {
+ dev_err(&i2c->dev, "Failed to add irq_chip\n");
+ return ret;
+ }
+
+ /* Configure short press to 10 milliseconds */
+ ret = regmap_update_bits(bd71837->regmap,
+ BD71837_REG_PWRONCONFIG0,
+ BD718XX_PWRBTN_PRESS_DURATION_MASK,
+ BD718XX_PWRBTN_SHORT_PRESS_10MS);
+ if (ret) {
+ dev_err(&i2c->dev, "Failed to configure button short press timeout\n");
+ return ret;
+ }
+
+ /* Configure long press to 10 seconds */
+ ret = regmap_update_bits(bd71837->regmap,
+ BD71837_REG_PWRONCONFIG1,
+ BD718XX_PWRBTN_PRESS_DURATION_MASK,
+ BD718XX_PWRBTN_LONG_PRESS_10S);
+
+ if (ret) {
+ dev_err(&i2c->dev, "Failed to configure button long press timeout\n");
+ return ret;
+ }
+
+ button.irq = regmap_irq_get_virq(bd71837->irq_data,
+ BD71837_INT_PWRBTN_S);
+
+ if (button.irq < 0) {
+ dev_err(&i2c->dev, "Failed to get the IRQ\n");
+ return button.irq;
+ }
+
+ ret = devm_mfd_add_devices(bd71837->dev, PLATFORM_DEVID_AUTO,
+ bd71837_mfd_cells,
+ ARRAY_SIZE(bd71837_mfd_cells), NULL, 0,
+ regmap_irq_get_domain(bd71837->irq_data));
+ if (ret)
+ dev_err(&i2c->dev, "Failed to create subdevices\n");
+
+ return ret;
+}
+
+static struct i2c_driver bd71837_i2c_driver = {
+ .driver = {
+ .name = "rohm-bd718x7",
+ .of_match_table = bd71837_of_match,
+ },
+ .probe = bd71837_i2c_probe,
+};
+
+static int __init bd71837_i2c_init(void)
+{
+ return i2c_add_driver(&bd71837_i2c_driver);
+}
+/* init early so consumer devices can complete system boot */
+subsys_initcall(bd71837_i2c_init);
+
+static void __exit bd71837_i2c_exit(void)
+{
+ i2c_del_driver(&bd71837_i2c_driver);
+}
+module_exit(bd71837_i2c_exit);
+
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
+MODULE_DESCRIPTION("ROHM BD71837 Power Management IC driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/rohm-bd718x7.h b/include/linux/mfd/rohm-bd718x7.h
new file mode 100644
index 000000000000..749d72ed9ae5
--- /dev/null
+++ b/include/linux/mfd/rohm-bd718x7.h
@@ -0,0 +1,333 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Copyright (C) 2018 ROHM Semiconductors */
+
+#ifndef __LINUX_MFD_BD71837_H__
+#define __LINUX_MFD_BD71837_H__
+
+#include <linux/regmap.h>
+
+enum {
+ BD71837_BUCK1 = 0,
+ BD71837_BUCK2,
+ BD71837_BUCK3,
+ BD71837_BUCK4,
+ BD71837_BUCK5,
+ BD71837_BUCK6,
+ BD71837_BUCK7,
+ BD71837_BUCK8,
+ BD71837_LDO1,
+ BD71837_LDO2,
+ BD71837_LDO3,
+ BD71837_LDO4,
+ BD71837_LDO5,
+ BD71837_LDO6,
+ BD71837_LDO7,
+ BD71837_REGULATOR_CNT,
+};
+
+#define BD71837_BUCK1_VOLTAGE_NUM 0x40
+#define BD71837_BUCK2_VOLTAGE_NUM 0x40
+#define BD71837_BUCK3_VOLTAGE_NUM 0x40
+#define BD71837_BUCK4_VOLTAGE_NUM 0x40
+
+#define BD71837_BUCK5_VOLTAGE_NUM 0x08
+#define BD71837_BUCK6_VOLTAGE_NUM 0x04
+#define BD71837_BUCK7_VOLTAGE_NUM 0x08
+#define BD71837_BUCK8_VOLTAGE_NUM 0x40
+
+#define BD71837_LDO1_VOLTAGE_NUM 0x04
+#define BD71837_LDO2_VOLTAGE_NUM 0x02
+#define BD71837_LDO3_VOLTAGE_NUM 0x10
+#define BD71837_LDO4_VOLTAGE_NUM 0x10
+#define BD71837_LDO5_VOLTAGE_NUM 0x10
+#define BD71837_LDO6_VOLTAGE_NUM 0x10
+#define BD71837_LDO7_VOLTAGE_NUM 0x10
+
+enum {
+ BD71837_REG_REV = 0x00,
+ BD71837_REG_SWRESET = 0x01,
+ BD71837_REG_I2C_DEV = 0x02,
+ BD71837_REG_PWRCTRL0 = 0x03,
+ BD71837_REG_PWRCTRL1 = 0x04,
+ BD71837_REG_BUCK1_CTRL = 0x05,
+ BD71837_REG_BUCK2_CTRL = 0x06,
+ BD71837_REG_BUCK3_CTRL = 0x07,
+ BD71837_REG_BUCK4_CTRL = 0x08,
+ BD71837_REG_BUCK5_CTRL = 0x09,
+ BD71837_REG_BUCK6_CTRL = 0x0A,
+ BD71837_REG_BUCK7_CTRL = 0x0B,
+ BD71837_REG_BUCK8_CTRL = 0x0C,
+ BD71837_REG_BUCK1_VOLT_RUN = 0x0D,
+ BD71837_REG_BUCK1_VOLT_IDLE = 0x0E,
+ BD71837_REG_BUCK1_VOLT_SUSP = 0x0F,
+ BD71837_REG_BUCK2_VOLT_RUN = 0x10,
+ BD71837_REG_BUCK2_VOLT_IDLE = 0x11,
+ BD71837_REG_BUCK3_VOLT_RUN = 0x12,
+ BD71837_REG_BUCK4_VOLT_RUN = 0x13,
+ BD71837_REG_BUCK5_VOLT = 0x14,
+ BD71837_REG_BUCK6_VOLT = 0x15,
+ BD71837_REG_BUCK7_VOLT = 0x16,
+ BD71837_REG_BUCK8_VOLT = 0x17,
+ BD71837_REG_LDO1_VOLT = 0x18,
+ BD71837_REG_LDO2_VOLT = 0x19,
+ BD71837_REG_LDO3_VOLT = 0x1A,
+ BD71837_REG_LDO4_VOLT = 0x1B,
+ BD71837_REG_LDO5_VOLT = 0x1C,
+ BD71837_REG_LDO6_VOLT = 0x1D,
+ BD71837_REG_LDO7_VOLT = 0x1E,
+ BD71837_REG_TRANS_COND0 = 0x1F,
+ BD71837_REG_TRANS_COND1 = 0x20,
+ BD71837_REG_VRFAULTEN = 0x21,
+ BD71837_REG_MVRFLTMASK0 = 0x22,
+ BD71837_REG_MVRFLTMASK1 = 0x23,
+ BD71837_REG_MVRFLTMASK2 = 0x24,
+ BD71837_REG_RCVCFG = 0x25,
+ BD71837_REG_RCVNUM = 0x26,
+ BD71837_REG_PWRONCONFIG0 = 0x27,
+ BD71837_REG_PWRONCONFIG1 = 0x28,
+ BD71837_REG_RESETSRC = 0x29,
+ BD71837_REG_MIRQ = 0x2A,
+ BD71837_REG_IRQ = 0x2B,
+ BD71837_REG_IN_MON = 0x2C,
+ BD71837_REG_POW_STATE = 0x2D,
+ BD71837_REG_OUT32K = 0x2E,
+ BD71837_REG_REGLOCK = 0x2F,
+ BD71837_REG_OTPVER = 0xFF,
+ BD71837_MAX_REGISTER = 0x100,
+};
+
+#define REGLOCK_PWRSEQ 0x1
+#define REGLOCK_VREG 0x10
+
+/* Generic BUCK control masks */
+#define BD71837_BUCK_SEL 0x02
+#define BD71837_BUCK_EN 0x01
+#define BD71837_BUCK_RUN_ON 0x04
+
+/* Generic LDO masks */
+#define BD71837_LDO_SEL 0x80
+#define BD71837_LDO_EN 0x40
+
+/* BD71837 BUCK ramp rate CTRL reg bits */
+#define BUCK_RAMPRATE_MASK 0xC0
+#define BUCK_RAMPRATE_10P00MV 0x0
+#define BUCK_RAMPRATE_5P00MV 0x1
+#define BUCK_RAMPRATE_2P50MV 0x2
+#define BUCK_RAMPRATE_1P25MV 0x3
+
+/* BD71837_REG_BUCK1_VOLT_RUN bits */
+#define BUCK1_RUN_MASK 0x3F
+#define BUCK1_RUN_DEFAULT 0x14
+
+/* BD71837_REG_BUCK1_VOLT_SUSP bits */
+#define BUCK1_SUSP_MASK 0x3F
+#define BUCK1_SUSP_DEFAULT 0x14
+
+/* BD71837_REG_BUCK1_VOLT_IDLE bits */
+#define BUCK1_IDLE_MASK 0x3F
+#define BUCK1_IDLE_DEFAULT 0x14
+
+/* BD71837_REG_BUCK2_VOLT_RUN bits */
+#define BUCK2_RUN_MASK 0x3F
+#define BUCK2_RUN_DEFAULT 0x1E
+
+/* BD71837_REG_BUCK2_VOLT_IDLE bits */
+#define BUCK2_IDLE_MASK 0x3F
+#define BUCK2_IDLE_DEFAULT 0x14
+
+/* BD71837_REG_BUCK3_VOLT_RUN bits */
+#define BUCK3_RUN_MASK 0x3F
+#define BUCK3_RUN_DEFAULT 0x1E
+
+/* BD71837_REG_BUCK4_VOLT_RUN bits */
+#define BUCK4_RUN_MASK 0x3F
+#define BUCK4_RUN_DEFAULT 0x1E
+
+/* BD71837_REG_BUCK5_VOLT bits */
+#define BUCK5_MASK 0x07
+#define BUCK5_DEFAULT 0x02
+
+/* BD71837_REG_BUCK6_VOLT bits */
+#define BUCK6_MASK 0x03
+#define BUCK6_DEFAULT 0x03
+
+/* BD71837_REG_BUCK7_VOLT bits */
+#define BUCK7_MASK 0x07
+#define BUCK7_DEFAULT 0x03
+
+/* BD71837_REG_BUCK8_VOLT bits */
+#define BUCK8_MASK 0x3F
+#define BUCK8_DEFAULT 0x1E
+
+/* BD71837_REG_IRQ bits */
+#define IRQ_SWRST 0x40
+#define IRQ_PWRON_S 0x20
+#define IRQ_PWRON_L 0x10
+#define IRQ_PWRON 0x08
+#define IRQ_WDOG 0x04
+#define IRQ_ON_REQ 0x02
+#define IRQ_STBY_REQ 0x01
+
+/* BD71837_REG_OUT32K bits */
+#define BD71837_OUT32K_EN 0x01
+
+/* BD71837 gated clock rate */
+#define BD71837_CLK_RATE 32768
+
+/* ROHM BD71837 irqs */
+enum {
+ BD71837_INT_STBY_REQ,
+ BD71837_INT_ON_REQ,
+ BD71837_INT_WDOG,
+ BD71837_INT_PWRBTN,
+ BD71837_INT_PWRBTN_L,
+ BD71837_INT_PWRBTN_S,
+ BD71837_INT_SWRST
+};
+
+/* ROHM BD71837 interrupt masks */
+#define BD71837_INT_SWRST_MASK 0x40
+#define BD71837_INT_PWRBTN_S_MASK 0x20
+#define BD71837_INT_PWRBTN_L_MASK 0x10
+#define BD71837_INT_PWRBTN_MASK 0x8
+#define BD71837_INT_WDOG_MASK 0x4
+#define BD71837_INT_ON_REQ_MASK 0x2
+#define BD71837_INT_STBY_REQ_MASK 0x1
+
+/* BD71837_REG_LDO1_VOLT bits */
+#define LDO1_MASK 0x03
+
+/* BD71837_REG_LDO1_VOLT bits */
+#define LDO2_MASK 0x20
+
+/* BD71837_REG_LDO3_VOLT bits */
+#define LDO3_MASK 0x0F
+
+/* BD71837_REG_LDO4_VOLT bits */
+#define LDO4_MASK 0x0F
+
+/* BD71837_REG_LDO5_VOLT bits */
+#define LDO5_MASK 0x0F
+
+/* BD71837_REG_LDO6_VOLT bits */
+#define LDO6_MASK 0x0F
+
+/* BD71837_REG_LDO7_VOLT bits */
+#define LDO7_MASK 0x0F
+
+/* Register write induced reset settings */
+
+/*
+ * Even though the bit zero is not SWRESET type we still want to write zero
+ * to it when changing type. Bit zero is 'SWRESET' trigger bit and if we
+ * write 1 to it we will trigger the action. So always write 0 to it when
+ * changning SWRESET action - no matter what we read from it.
+ */
+#define BD71837_SWRESET_TYPE_MASK 7
+#define BD71837_SWRESET_TYPE_DISABLED 0
+#define BD71837_SWRESET_TYPE_COLD 4
+#define BD71837_SWRESET_TYPE_WARM 6
+
+#define BD71837_SWRESET_RESET_MASK 1
+#define BD71837_SWRESET_RESET 1
+
+/* Poweroff state transition conditions */
+
+#define BD718XX_ON_REQ_POWEROFF_MASK 1
+#define BD718XX_SWRESET_POWEROFF_MASK 2
+#define BD718XX_WDOG_POWEROFF_MASK 4
+#define BD718XX_KEY_L_POWEROFF_MASK 8
+
+#define BD718XX_POWOFF_TO_SNVS 0
+#define BD718XX_POWOFF_TO_RDY 0xF
+
+#define BD718XX_POWOFF_TIME_MASK 0xF0
+enum {
+ BD718XX_POWOFF_TIME_5MS = 0,
+ BD718XX_POWOFF_TIME_10MS,
+ BD718XX_POWOFF_TIME_15MS,
+ BD718XX_POWOFF_TIME_20MS,
+ BD718XX_POWOFF_TIME_25MS,
+ BD718XX_POWOFF_TIME_30MS,
+ BD718XX_POWOFF_TIME_35MS,
+ BD718XX_POWOFF_TIME_40MS,
+ BD718XX_POWOFF_TIME_45MS,
+ BD718XX_POWOFF_TIME_50MS,
+ BD718XX_POWOFF_TIME_75MS,
+ BD718XX_POWOFF_TIME_100MS,
+ BD718XX_POWOFF_TIME_250MS,
+ BD718XX_POWOFF_TIME_500MS,
+ BD718XX_POWOFF_TIME_750MS,
+ BD718XX_POWOFF_TIME_1500MS
+};
+
+/* Poweron sequence state transition conditions */
+#define BD718XX_RDY_TO_SNVS_MASK 0xF
+#define BD718XX_SNVS_TO_RUN_MASK 0xF0
+
+#define BD718XX_PWR_TRIG_KEY_L 1
+#define BD718XX_PWR_TRIG_KEY_S 2
+#define BD718XX_PWR_TRIG_PMIC_ON 4
+#define BD718XX_PWR_TRIG_VSYS_UVLO 8
+#define BD718XX_RDY_TO_SNVS_SIFT 0
+#define BD718XX_SNVS_TO_RUN_SIFT 4
+
+#define BD718XX_PWRBTN_PRESS_DURATION_MASK 0xF
+
+/* Timeout value for detecting short press */
+enum {
+ BD718XX_PWRBTN_SHORT_PRESS_10MS = 0,
+ BD718XX_PWRBTN_SHORT_PRESS_500MS,
+ BD718XX_PWRBTN_SHORT_PRESS_1000MS,
+ BD718XX_PWRBTN_SHORT_PRESS_1500MS,
+ BD718XX_PWRBTN_SHORT_PRESS_2000MS,
+ BD718XX_PWRBTN_SHORT_PRESS_2500MS,
+ BD718XX_PWRBTN_SHORT_PRESS_3000MS,
+ BD718XX_PWRBTN_SHORT_PRESS_3500MS,
+ BD718XX_PWRBTN_SHORT_PRESS_4000MS,
+ BD718XX_PWRBTN_SHORT_PRESS_4500MS,
+ BD718XX_PWRBTN_SHORT_PRESS_5000MS,
+ BD718XX_PWRBTN_SHORT_PRESS_5500MS,
+ BD718XX_PWRBTN_SHORT_PRESS_6000MS,
+ BD718XX_PWRBTN_SHORT_PRESS_6500MS,
+ BD718XX_PWRBTN_SHORT_PRESS_7000MS,
+ BD718XX_PWRBTN_SHORT_PRESS_7500MS
+};
+
+/* Timeout value for detecting LONG press */
+enum {
+ BD718XX_PWRBTN_LONG_PRESS_10MS = 0,
+ BD718XX_PWRBTN_LONG_PRESS_1S,
+ BD718XX_PWRBTN_LONG_PRESS_2S,
+ BD718XX_PWRBTN_LONG_PRESS_3S,
+ BD718XX_PWRBTN_LONG_PRESS_4S,
+ BD718XX_PWRBTN_LONG_PRESS_5S,
+ BD718XX_PWRBTN_LONG_PRESS_6S,
+ BD718XX_PWRBTN_LONG_PRESS_7S,
+ BD718XX_PWRBTN_LONG_PRESS_8S,
+ BD718XX_PWRBTN_LONG_PRESS_9S,
+ BD718XX_PWRBTN_LONG_PRESS_10S,
+ BD718XX_PWRBTN_LONG_PRESS_11S,
+ BD718XX_PWRBTN_LONG_PRESS_12S,
+ BD718XX_PWRBTN_LONG_PRESS_13S,
+ BD718XX_PWRBTN_LONG_PRESS_14S,
+ BD718XX_PWRBTN_LONG_PRESS_15S
+};
+
+struct bd71837_pmic;
+struct bd71837_clk;
+
+struct bd71837 {
+ struct device *dev;
+ struct i2c_client *i2c_client;
+ struct regmap *regmap;
+ unsigned long int id;
+
+ int chip_irq;
+ struct regmap_irq_chip_data *irq_data;
+
+ struct bd71837_pmic *pmic;
+ struct bd71837_clk *clk;
+};
+
+#endif /* __LINUX_MFD_BD71837_H__ */
--
2.14.3
^ permalink raw reply related
* [PATCH v10 2/2] mfd: bd71837: Devicetree bindings for ROHM BD71837 PMIC
From: Matti Vaittinen @ 2018-07-05 14:32 UTC (permalink / raw)
To: mturquette, robh+dt, sboyd, mark.rutland, lee.jones, lgirdwood,
broonie, mazziesaccount, arnd, dmitry.torokhov, sre, chenjh,
andrew.smirnov, linus.walleij, kstewart, heiko, gregkh, eballetbo
Cc: linux-clk, devicetree, linux-kernel, linux-input, mikko.mutanen,
heikki.haikola
In-Reply-To: <cover.1530800707.git.matti.vaittinen@fi.rohmeurope.com>
Document devicetree bindings for ROHM BD71837 PMIC MFD.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
.../devicetree/bindings/mfd/rohm,bd71837-pmic.txt | 62 ++++++++++++++++++++++
1 file changed, 62 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
new file mode 100644
index 000000000000..3ca56fdb5ffe
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
@@ -0,0 +1,62 @@
+* ROHM BD71837 Power Management Integrated Circuit bindings
+
+BD71837MWV is a programmable Power Management IC for powering single-core,
+dual-core, and quad-core SoCs such as NXP-i.MX 8M. It is optimized for
+low BOM cost and compact solution footprint. It integrates 8 Buck
+egulators and 7 LDOs to provide all the power rails required by the SoC and
+the commonly used peripherals.
+
+Datasheet for PMIC is available at:
+https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
+
+Required properties:
+ - compatible : Should be "rohm,bd71837".
+ - reg : I2C slave address.
+ - interrupt-parent : Phandle to the parent interrupt controller.
+ - interrupts : The interrupt line the device is connected to.
+ - clocks : The parent clock connected to PMIC. If this is missing
+ 32768 KHz clock is assumed.
+ - #clock-cells : Should be 0.
+ - regulators: : List of child nodes that specify the regulators.
+ Please see ../regulator/rohm,bd71837-regulator.txt
+
+Optional properties:
+- clock-output-names : Should contain name for output clock.
+
+Example:
+
+ /* external oscillator node */
+ osc: oscillator {
+ compatible = "fixed-clock";
+ #clock-cells = <1>;
+ clock-frequency = <32768>;
+ clock-output-names = "osc";
+ };
+
+ pmic: pmic@4b {
+ compatible = "rohm,bd71837";
+ reg = <0x4b>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <29 GPIO_ACTIVE_LOW>;
+ interrupt-names = "irq";
+ #clock-cells = <0>;
+ clocks = <&osc 0>;
+ clock-output-names = "bd71837-32k-out";
+
+ regulators {
+ buck1: BUCK1 {
+ regulator-name = "buck1";
+ regulator-min-microvolt = <700000>;
+ regulator-max-microvolt = <1300000>;
+ regulator-boot-on;
+ regulator-ramp-delay = <1250>;
+ };
+ };
+ };
+
+ /* Clock consumer node */
+ rtc@0 {
+ compatible = "company,my-rtc";
+ clock-names = "my-clock";
+ clocks = <&pmic>;
+ };
--
2.14.3
^ permalink raw reply related
* Re: [PATCH v10 1/2] mfd: bd71837: mfd driver for ROHM BD71837 PMIC
From: Enric Balletbo Serra @ 2018-07-05 14:41 UTC (permalink / raw)
To: matti.vaittinen
Cc: Michael Turquette, Rob Herring, sboyd, Mark Rutland, Lee Jones,
Liam Girdwood, Mark Brown, Matti Vaittinen, Arnd Bergmann,
Dmitry Torokhov, Sebastian Reichel, chenjh, Andrey Smirnov,
Linus Walleij, Kate Stewart, Heiko Stübner,
Greg Kroah-Hartman, linux-clk, devicetree@vger.kernel.org,
linux-kernel
In-Reply-To: <e737bb13d17b75ea5e55f59005ef3181d428e704.1530800707.git.matti.vaittinen@fi.rohmeurope.com>
Missatge de Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> del
dia dj., 5 de jul. 2018 a les 16:32:
>
> ROHM BD71837 PMIC MFD driver providing interrupts and support
> for three subsystems:
> - clk
> - Regulators
> - input/power-key
>
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
> drivers/mfd/Kconfig | 13 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/rohm-bd718x7.c | 198 +++++++++++++++++++++++
> include/linux/mfd/rohm-bd718x7.h | 333 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 545 insertions(+)
> create mode 100644 drivers/mfd/rohm-bd718x7.c
> create mode 100644 include/linux/mfd/rohm-bd718x7.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index b860eb5aa194..ac317c9bf065 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1787,6 +1787,19 @@ config MFD_STW481X
> in various ST Microelectronics and ST-Ericsson embedded
> Nomadik series.
>
> +config MFD_ROHM_BD718XX
> + tristate "ROHM BD71837 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 BD71837
> + Power Management ICs. BD71837 is designed to power processors like
> + NXP i.MX8. It contains 8 BUCK outputs and 7 LDOs, voltage monitoring
> + and emergency shut down as well as 32,768KHz clock output.
> +
> config MFD_STM32_LPTIMER
> tristate "Support for STM32 Low-Power Timer"
> depends on (ARCH_STM32 && OF) || COMPILE_TEST
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e9fd20dba18d..c7a5c3b0e776 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -227,4 +227,5 @@ obj-$(CONFIG_MFD_STM32_TIMERS) += stm32-timers.o
> obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
> obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o
> obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
> +obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
>
> diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
> new file mode 100644
> index 000000000000..34df74940dfa
> --- /dev/null
> +++ b/drivers/mfd/rohm-bd718x7.c
> @@ -0,0 +1,198 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +//
> +// Copyright (C) 2018 ROHM Semiconductors
> +//
> +// ROHM BD71837MWV PMIC driver
> +//
> +// Datasheet available from
> +// https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
> +
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/rohm-bd718x7.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +/*
> + * gpio_keys.h requires definiton of bool. It is brought in
> + * by abowe includes. Keep this as last untill gpio_keys.h gets fixed.
> + */
> +#include <linux/gpio_keys.h>
> +
> +static struct gpio_keys_button button = {
> + .code = KEY_POWER,
> + .gpio = -1,
> + .type = EV_KEY,
> +};
> +
> +static struct gpio_keys_platform_data bd718xx_powerkey_data = {
> + .buttons = &button,
> + .nbuttons = 1,
> + .name = "bd718xx-pwrkey",
> +};
> +
> +static struct mfd_cell bd71837_mfd_cells[] = {
> + {
> + .name = "gpio-keys",
> + .platform_data = &bd718xx_powerkey_data,
> + .pdata_size = sizeof(bd718xx_powerkey_data),
> + },
> + { .name = "bd71837-clk", },
> + { .name = "bd71837-pmic", },
> +};
> +
> +static const struct regmap_irq bd71837_irqs[] = {
> + REGMAP_IRQ_REG(BD71837_INT_SWRST, 0, BD71837_INT_SWRST_MASK),
> + REGMAP_IRQ_REG(BD71837_INT_PWRBTN_S, 0, BD71837_INT_PWRBTN_S_MASK),
> + REGMAP_IRQ_REG(BD71837_INT_PWRBTN_L, 0, BD71837_INT_PWRBTN_L_MASK),
> + REGMAP_IRQ_REG(BD71837_INT_PWRBTN, 0, BD71837_INT_PWRBTN_MASK),
> + REGMAP_IRQ_REG(BD71837_INT_WDOG, 0, BD71837_INT_WDOG_MASK),
> + REGMAP_IRQ_REG(BD71837_INT_ON_REQ, 0, BD71837_INT_ON_REQ_MASK),
> + REGMAP_IRQ_REG(BD71837_INT_STBY_REQ, 0, BD71837_INT_STBY_REQ_MASK),
> +};
> +
> +static struct regmap_irq_chip bd71837_irq_chip = {
> + .name = "bd71837-irq",
> + .irqs = bd71837_irqs,
> + .num_irqs = ARRAY_SIZE(bd71837_irqs),
> + .num_regs = 1,
> + .irq_reg_stride = 1,
> + .status_base = BD71837_REG_IRQ,
> + .mask_base = BD71837_REG_MIRQ,
> + .ack_base = BD71837_REG_IRQ,
> + .init_ack_masked = true,
> + .mask_invert = false,
> +};
> +
> +static const struct regmap_range pmic_status_range = {
> + .range_min = BD71837_REG_IRQ,
> + .range_max = BD71837_REG_POW_STATE,
> +};
> +
> +static const struct regmap_access_table volatile_regs = {
> + .yes_ranges = &pmic_status_range,
> + .n_yes_ranges = 1,
> +};
> +
> +static const struct regmap_config bd71837_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .volatile_table = &volatile_regs,
> + .max_register = BD71837_MAX_REGISTER - 1,
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> +static const struct of_device_id bd71837_of_match[] = {
> + { .compatible = "rohm,bd71837", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, bd71837_of_match);
> +
> +static int bd71837_i2c_probe(struct i2c_client *i2c,
> + const struct i2c_device_id *id)
> +{
> + struct bd71837 *bd71837;
> + int ret;
> + unsigned int val;
> +
> + bd71837 = devm_kzalloc(&i2c->dev, sizeof(struct bd71837), GFP_KERNEL);
> +
> + if (!bd71837)
> + return -ENOMEM;
> +
> + bd71837->chip_irq = i2c->irq;
> +
> + if (!bd71837->chip_irq) {
> + dev_err(&i2c->dev, "No IRQ configured\n");
> + return -EINVAL;
> + }
> +
> + bd71837->dev = &i2c->dev;
> + bd71837->i2c_client = i2c;
> + i2c_set_clientdata(i2c, bd71837);
> +
> + bd71837->regmap = devm_regmap_init_i2c(i2c, &bd71837_regmap_config);
> + if (IS_ERR(bd71837->regmap)) {
> + dev_err(&i2c->dev, "regmap initialization failed\n");
> + return PTR_ERR(bd71837->regmap);
> + }
> +
> + ret = regmap_read(bd71837->regmap, BD71837_REG_REV, &val);
> + if (ret) {
> + dev_err(&i2c->dev, "Read BD71837_REG_DEVICE failed\n");
> + return ret;
> + }
> +
> + ret = devm_regmap_add_irq_chip(&i2c->dev, bd71837->regmap,
> + bd71837->chip_irq, IRQF_ONESHOT, 0,
> + &bd71837_irq_chip, &bd71837->irq_data);
> + if (ret) {
> + dev_err(&i2c->dev, "Failed to add irq_chip\n");
> + return ret;
> + }
> +
> + /* Configure short press to 10 milliseconds */
> + ret = regmap_update_bits(bd71837->regmap,
> + BD71837_REG_PWRONCONFIG0,
> + BD718XX_PWRBTN_PRESS_DURATION_MASK,
> + BD718XX_PWRBTN_SHORT_PRESS_10MS);
> + if (ret) {
> + dev_err(&i2c->dev, "Failed to configure button short press timeout\n");
> + return ret;
> + }
> +
> + /* Configure long press to 10 seconds */
> + ret = regmap_update_bits(bd71837->regmap,
> + BD71837_REG_PWRONCONFIG1,
> + BD718XX_PWRBTN_PRESS_DURATION_MASK,
> + BD718XX_PWRBTN_LONG_PRESS_10S);
> +
> + if (ret) {
> + dev_err(&i2c->dev, "Failed to configure button long press timeout\n");
> + return ret;
> + }
> +
> + button.irq = regmap_irq_get_virq(bd71837->irq_data,
> + BD71837_INT_PWRBTN_S);
> +
> + if (button.irq < 0) {
> + dev_err(&i2c->dev, "Failed to get the IRQ\n");
> + return button.irq;
> + }
> +
> + ret = devm_mfd_add_devices(bd71837->dev, PLATFORM_DEVID_AUTO,
> + bd71837_mfd_cells,
> + ARRAY_SIZE(bd71837_mfd_cells), NULL, 0,
> + regmap_irq_get_domain(bd71837->irq_data));
> + if (ret)
> + dev_err(&i2c->dev, "Failed to create subdevices\n");
> +
> + return ret;
> +}
> +
> +static struct i2c_driver bd71837_i2c_driver = {
> + .driver = {
> + .name = "rohm-bd718x7",
> + .of_match_table = bd71837_of_match,
> + },
> + .probe = bd71837_i2c_probe,
> +};
> +
> +static int __init bd71837_i2c_init(void)
> +{
> + return i2c_add_driver(&bd71837_i2c_driver);
> +}
> +/* init early so consumer devices can complete system boot */
> +subsys_initcall(bd71837_i2c_init);
> +
> +static void __exit bd71837_i2c_exit(void)
> +{
> + i2c_del_driver(&bd71837_i2c_driver);
> +}
> +module_exit(bd71837_i2c_exit);
> +
> +MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
> +MODULE_DESCRIPTION("ROHM BD71837 Power Management IC driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/rohm-bd718x7.h b/include/linux/mfd/rohm-bd718x7.h
> new file mode 100644
> index 000000000000..749d72ed9ae5
> --- /dev/null
> +++ b/include/linux/mfd/rohm-bd718x7.h
> @@ -0,0 +1,333 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/* Copyright (C) 2018 ROHM Semiconductors */
> +
> +#ifndef __LINUX_MFD_BD71837_H__
> +#define __LINUX_MFD_BD71837_H__
> +
> +#include <linux/regmap.h>
> +
> +enum {
> + BD71837_BUCK1 = 0,
> + BD71837_BUCK2,
> + BD71837_BUCK3,
> + BD71837_BUCK4,
> + BD71837_BUCK5,
> + BD71837_BUCK6,
> + BD71837_BUCK7,
> + BD71837_BUCK8,
> + BD71837_LDO1,
> + BD71837_LDO2,
> + BD71837_LDO3,
> + BD71837_LDO4,
> + BD71837_LDO5,
> + BD71837_LDO6,
> + BD71837_LDO7,
> + BD71837_REGULATOR_CNT,
> +};
> +
> +#define BD71837_BUCK1_VOLTAGE_NUM 0x40
> +#define BD71837_BUCK2_VOLTAGE_NUM 0x40
> +#define BD71837_BUCK3_VOLTAGE_NUM 0x40
> +#define BD71837_BUCK4_VOLTAGE_NUM 0x40
> +
> +#define BD71837_BUCK5_VOLTAGE_NUM 0x08
> +#define BD71837_BUCK6_VOLTAGE_NUM 0x04
> +#define BD71837_BUCK7_VOLTAGE_NUM 0x08
> +#define BD71837_BUCK8_VOLTAGE_NUM 0x40
> +
> +#define BD71837_LDO1_VOLTAGE_NUM 0x04
> +#define BD71837_LDO2_VOLTAGE_NUM 0x02
> +#define BD71837_LDO3_VOLTAGE_NUM 0x10
> +#define BD71837_LDO4_VOLTAGE_NUM 0x10
> +#define BD71837_LDO5_VOLTAGE_NUM 0x10
> +#define BD71837_LDO6_VOLTAGE_NUM 0x10
> +#define BD71837_LDO7_VOLTAGE_NUM 0x10
> +
> +enum {
> + BD71837_REG_REV = 0x00,
> + BD71837_REG_SWRESET = 0x01,
> + BD71837_REG_I2C_DEV = 0x02,
> + BD71837_REG_PWRCTRL0 = 0x03,
> + BD71837_REG_PWRCTRL1 = 0x04,
> + BD71837_REG_BUCK1_CTRL = 0x05,
> + BD71837_REG_BUCK2_CTRL = 0x06,
> + BD71837_REG_BUCK3_CTRL = 0x07,
> + BD71837_REG_BUCK4_CTRL = 0x08,
> + BD71837_REG_BUCK5_CTRL = 0x09,
> + BD71837_REG_BUCK6_CTRL = 0x0A,
> + BD71837_REG_BUCK7_CTRL = 0x0B,
> + BD71837_REG_BUCK8_CTRL = 0x0C,
> + BD71837_REG_BUCK1_VOLT_RUN = 0x0D,
> + BD71837_REG_BUCK1_VOLT_IDLE = 0x0E,
> + BD71837_REG_BUCK1_VOLT_SUSP = 0x0F,
> + BD71837_REG_BUCK2_VOLT_RUN = 0x10,
> + BD71837_REG_BUCK2_VOLT_IDLE = 0x11,
> + BD71837_REG_BUCK3_VOLT_RUN = 0x12,
> + BD71837_REG_BUCK4_VOLT_RUN = 0x13,
> + BD71837_REG_BUCK5_VOLT = 0x14,
> + BD71837_REG_BUCK6_VOLT = 0x15,
> + BD71837_REG_BUCK7_VOLT = 0x16,
> + BD71837_REG_BUCK8_VOLT = 0x17,
> + BD71837_REG_LDO1_VOLT = 0x18,
> + BD71837_REG_LDO2_VOLT = 0x19,
> + BD71837_REG_LDO3_VOLT = 0x1A,
> + BD71837_REG_LDO4_VOLT = 0x1B,
> + BD71837_REG_LDO5_VOLT = 0x1C,
> + BD71837_REG_LDO6_VOLT = 0x1D,
> + BD71837_REG_LDO7_VOLT = 0x1E,
> + BD71837_REG_TRANS_COND0 = 0x1F,
> + BD71837_REG_TRANS_COND1 = 0x20,
> + BD71837_REG_VRFAULTEN = 0x21,
> + BD71837_REG_MVRFLTMASK0 = 0x22,
> + BD71837_REG_MVRFLTMASK1 = 0x23,
> + BD71837_REG_MVRFLTMASK2 = 0x24,
> + BD71837_REG_RCVCFG = 0x25,
> + BD71837_REG_RCVNUM = 0x26,
> + BD71837_REG_PWRONCONFIG0 = 0x27,
> + BD71837_REG_PWRONCONFIG1 = 0x28,
> + BD71837_REG_RESETSRC = 0x29,
> + BD71837_REG_MIRQ = 0x2A,
> + BD71837_REG_IRQ = 0x2B,
> + BD71837_REG_IN_MON = 0x2C,
> + BD71837_REG_POW_STATE = 0x2D,
> + BD71837_REG_OUT32K = 0x2E,
> + BD71837_REG_REGLOCK = 0x2F,
> + BD71837_REG_OTPVER = 0xFF,
> + BD71837_MAX_REGISTER = 0x100,
> +};
> +
> +#define REGLOCK_PWRSEQ 0x1
> +#define REGLOCK_VREG 0x10
> +
> +/* Generic BUCK control masks */
> +#define BD71837_BUCK_SEL 0x02
> +#define BD71837_BUCK_EN 0x01
> +#define BD71837_BUCK_RUN_ON 0x04
> +
> +/* Generic LDO masks */
> +#define BD71837_LDO_SEL 0x80
> +#define BD71837_LDO_EN 0x40
> +
> +/* BD71837 BUCK ramp rate CTRL reg bits */
> +#define BUCK_RAMPRATE_MASK 0xC0
> +#define BUCK_RAMPRATE_10P00MV 0x0
> +#define BUCK_RAMPRATE_5P00MV 0x1
> +#define BUCK_RAMPRATE_2P50MV 0x2
> +#define BUCK_RAMPRATE_1P25MV 0x3
> +
> +/* BD71837_REG_BUCK1_VOLT_RUN bits */
> +#define BUCK1_RUN_MASK 0x3F
> +#define BUCK1_RUN_DEFAULT 0x14
> +
> +/* BD71837_REG_BUCK1_VOLT_SUSP bits */
> +#define BUCK1_SUSP_MASK 0x3F
> +#define BUCK1_SUSP_DEFAULT 0x14
> +
> +/* BD71837_REG_BUCK1_VOLT_IDLE bits */
> +#define BUCK1_IDLE_MASK 0x3F
> +#define BUCK1_IDLE_DEFAULT 0x14
> +
> +/* BD71837_REG_BUCK2_VOLT_RUN bits */
> +#define BUCK2_RUN_MASK 0x3F
> +#define BUCK2_RUN_DEFAULT 0x1E
> +
> +/* BD71837_REG_BUCK2_VOLT_IDLE bits */
> +#define BUCK2_IDLE_MASK 0x3F
> +#define BUCK2_IDLE_DEFAULT 0x14
> +
> +/* BD71837_REG_BUCK3_VOLT_RUN bits */
> +#define BUCK3_RUN_MASK 0x3F
> +#define BUCK3_RUN_DEFAULT 0x1E
> +
> +/* BD71837_REG_BUCK4_VOLT_RUN bits */
> +#define BUCK4_RUN_MASK 0x3F
> +#define BUCK4_RUN_DEFAULT 0x1E
> +
> +/* BD71837_REG_BUCK5_VOLT bits */
> +#define BUCK5_MASK 0x07
> +#define BUCK5_DEFAULT 0x02
> +
> +/* BD71837_REG_BUCK6_VOLT bits */
> +#define BUCK6_MASK 0x03
> +#define BUCK6_DEFAULT 0x03
> +
> +/* BD71837_REG_BUCK7_VOLT bits */
> +#define BUCK7_MASK 0x07
> +#define BUCK7_DEFAULT 0x03
> +
> +/* BD71837_REG_BUCK8_VOLT bits */
> +#define BUCK8_MASK 0x3F
> +#define BUCK8_DEFAULT 0x1E
> +
> +/* BD71837_REG_IRQ bits */
> +#define IRQ_SWRST 0x40
> +#define IRQ_PWRON_S 0x20
> +#define IRQ_PWRON_L 0x10
> +#define IRQ_PWRON 0x08
> +#define IRQ_WDOG 0x04
> +#define IRQ_ON_REQ 0x02
> +#define IRQ_STBY_REQ 0x01
> +
> +/* BD71837_REG_OUT32K bits */
> +#define BD71837_OUT32K_EN 0x01
> +
> +/* BD71837 gated clock rate */
> +#define BD71837_CLK_RATE 32768
> +
> +/* ROHM BD71837 irqs */
> +enum {
> + BD71837_INT_STBY_REQ,
> + BD71837_INT_ON_REQ,
> + BD71837_INT_WDOG,
> + BD71837_INT_PWRBTN,
> + BD71837_INT_PWRBTN_L,
> + BD71837_INT_PWRBTN_S,
> + BD71837_INT_SWRST
> +};
> +
> +/* ROHM BD71837 interrupt masks */
> +#define BD71837_INT_SWRST_MASK 0x40
> +#define BD71837_INT_PWRBTN_S_MASK 0x20
> +#define BD71837_INT_PWRBTN_L_MASK 0x10
> +#define BD71837_INT_PWRBTN_MASK 0x8
> +#define BD71837_INT_WDOG_MASK 0x4
> +#define BD71837_INT_ON_REQ_MASK 0x2
> +#define BD71837_INT_STBY_REQ_MASK 0x1
> +
> +/* BD71837_REG_LDO1_VOLT bits */
> +#define LDO1_MASK 0x03
> +
> +/* BD71837_REG_LDO1_VOLT bits */
> +#define LDO2_MASK 0x20
> +
> +/* BD71837_REG_LDO3_VOLT bits */
> +#define LDO3_MASK 0x0F
> +
> +/* BD71837_REG_LDO4_VOLT bits */
> +#define LDO4_MASK 0x0F
> +
> +/* BD71837_REG_LDO5_VOLT bits */
> +#define LDO5_MASK 0x0F
> +
> +/* BD71837_REG_LDO6_VOLT bits */
> +#define LDO6_MASK 0x0F
> +
> +/* BD71837_REG_LDO7_VOLT bits */
> +#define LDO7_MASK 0x0F
> +
> +/* Register write induced reset settings */
> +
> +/*
> + * Even though the bit zero is not SWRESET type we still want to write zero
> + * to it when changing type. Bit zero is 'SWRESET' trigger bit and if we
> + * write 1 to it we will trigger the action. So always write 0 to it when
> + * changning SWRESET action - no matter what we read from it.
> + */
> +#define BD71837_SWRESET_TYPE_MASK 7
> +#define BD71837_SWRESET_TYPE_DISABLED 0
> +#define BD71837_SWRESET_TYPE_COLD 4
> +#define BD71837_SWRESET_TYPE_WARM 6
> +
> +#define BD71837_SWRESET_RESET_MASK 1
> +#define BD71837_SWRESET_RESET 1
> +
> +/* Poweroff state transition conditions */
> +
> +#define BD718XX_ON_REQ_POWEROFF_MASK 1
> +#define BD718XX_SWRESET_POWEROFF_MASK 2
> +#define BD718XX_WDOG_POWEROFF_MASK 4
> +#define BD718XX_KEY_L_POWEROFF_MASK 8
> +
> +#define BD718XX_POWOFF_TO_SNVS 0
> +#define BD718XX_POWOFF_TO_RDY 0xF
> +
> +#define BD718XX_POWOFF_TIME_MASK 0xF0
> +enum {
> + BD718XX_POWOFF_TIME_5MS = 0,
> + BD718XX_POWOFF_TIME_10MS,
> + BD718XX_POWOFF_TIME_15MS,
> + BD718XX_POWOFF_TIME_20MS,
> + BD718XX_POWOFF_TIME_25MS,
> + BD718XX_POWOFF_TIME_30MS,
> + BD718XX_POWOFF_TIME_35MS,
> + BD718XX_POWOFF_TIME_40MS,
> + BD718XX_POWOFF_TIME_45MS,
> + BD718XX_POWOFF_TIME_50MS,
> + BD718XX_POWOFF_TIME_75MS,
> + BD718XX_POWOFF_TIME_100MS,
> + BD718XX_POWOFF_TIME_250MS,
> + BD718XX_POWOFF_TIME_500MS,
> + BD718XX_POWOFF_TIME_750MS,
> + BD718XX_POWOFF_TIME_1500MS
> +};
> +
> +/* Poweron sequence state transition conditions */
> +#define BD718XX_RDY_TO_SNVS_MASK 0xF
> +#define BD718XX_SNVS_TO_RUN_MASK 0xF0
> +
> +#define BD718XX_PWR_TRIG_KEY_L 1
> +#define BD718XX_PWR_TRIG_KEY_S 2
> +#define BD718XX_PWR_TRIG_PMIC_ON 4
> +#define BD718XX_PWR_TRIG_VSYS_UVLO 8
> +#define BD718XX_RDY_TO_SNVS_SIFT 0
> +#define BD718XX_SNVS_TO_RUN_SIFT 4
> +
> +#define BD718XX_PWRBTN_PRESS_DURATION_MASK 0xF
> +
> +/* Timeout value for detecting short press */
> +enum {
> + BD718XX_PWRBTN_SHORT_PRESS_10MS = 0,
> + BD718XX_PWRBTN_SHORT_PRESS_500MS,
> + BD718XX_PWRBTN_SHORT_PRESS_1000MS,
> + BD718XX_PWRBTN_SHORT_PRESS_1500MS,
> + BD718XX_PWRBTN_SHORT_PRESS_2000MS,
> + BD718XX_PWRBTN_SHORT_PRESS_2500MS,
> + BD718XX_PWRBTN_SHORT_PRESS_3000MS,
> + BD718XX_PWRBTN_SHORT_PRESS_3500MS,
> + BD718XX_PWRBTN_SHORT_PRESS_4000MS,
> + BD718XX_PWRBTN_SHORT_PRESS_4500MS,
> + BD718XX_PWRBTN_SHORT_PRESS_5000MS,
> + BD718XX_PWRBTN_SHORT_PRESS_5500MS,
> + BD718XX_PWRBTN_SHORT_PRESS_6000MS,
> + BD718XX_PWRBTN_SHORT_PRESS_6500MS,
> + BD718XX_PWRBTN_SHORT_PRESS_7000MS,
> + BD718XX_PWRBTN_SHORT_PRESS_7500MS
> +};
> +
> +/* Timeout value for detecting LONG press */
> +enum {
> + BD718XX_PWRBTN_LONG_PRESS_10MS = 0,
> + BD718XX_PWRBTN_LONG_PRESS_1S,
> + BD718XX_PWRBTN_LONG_PRESS_2S,
> + BD718XX_PWRBTN_LONG_PRESS_3S,
> + BD718XX_PWRBTN_LONG_PRESS_4S,
> + BD718XX_PWRBTN_LONG_PRESS_5S,
> + BD718XX_PWRBTN_LONG_PRESS_6S,
> + BD718XX_PWRBTN_LONG_PRESS_7S,
> + BD718XX_PWRBTN_LONG_PRESS_8S,
> + BD718XX_PWRBTN_LONG_PRESS_9S,
> + BD718XX_PWRBTN_LONG_PRESS_10S,
> + BD718XX_PWRBTN_LONG_PRESS_11S,
> + BD718XX_PWRBTN_LONG_PRESS_12S,
> + BD718XX_PWRBTN_LONG_PRESS_13S,
> + BD718XX_PWRBTN_LONG_PRESS_14S,
> + BD718XX_PWRBTN_LONG_PRESS_15S
> +};
> +
> +struct bd71837_pmic;
> +struct bd71837_clk;
> +
> +struct bd71837 {
> + struct device *dev;
> + struct i2c_client *i2c_client;
> + struct regmap *regmap;
> + unsigned long int id;
> +
> + int chip_irq;
> + struct regmap_irq_chip_data *irq_data;
> +
> + struct bd71837_pmic *pmic;
> + struct bd71837_clk *clk;
> +};
> +
> +#endif /* __LINUX_MFD_BD71837_H__ */
> --
> 2.14.3
>
The main changes I requested are done so I think you can maintain my
reviewed-by.
Reviewed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Best Regards,
Enric
^ permalink raw reply
* [PATCH 0/8] Introduce STPMU1 PMIC Driver
From: Pascal PAILLET-LME @ 2018-07-05 15:14 UTC (permalink / raw)
To: dmitry.torokhov@gmail.com, robh+dt@kernel.org,
mark.rutland@arm.com, lee.jones@linaro.org, lgirdwood@gmail.com,
broonie@kernel.org, wim@linux-watchdog.org, linux@roeck-us.net,
linux-input@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org,
benjamin.gaignard@linaro.org
Cc: Pascal PAILLET-LME
From: pascal paillet <p.paillet@st.com>
The goal of this patch-set is to propose a driver for the STPMU1 PMIC from
ST Microelectronics.
The STPMU1 regulators supply power to an application processor as well as
to external system peripherals such as DDR, Flash memories and system
devices. It also features onkey button input and an hardware watchdog.
The STPMU1 is controlled via I2C.
Main driver is drivers/mfd/stpmu1 that handle I2C regmap configuration and
irqchip. stpmu1_regulator, stpmu1_onkey and stpmu1_wdt need stpmu1 mfd
as parent.
stpmu1 mfd regulator drivers maybe mandatory at boot time.
*** BLURB HERE ***
pascal paillet (8):
dt-bindings: mfd: document stpmu1 pmic
mfd: stpmu1: add stpmu1 pmic driver
dt-bindings: regulator: document stpmu1 pmic regulators
regulator: stpmu1: add stpmu1 regulator driver
dt-bindings: input: document stpmu1 pmic onkey
input: stpmu1: add stpmu1 onkey driver
dt-bindings: watchdog: document stpmu1 pmic watchdog
watchdog: stpmu1: add stpmu1 watchdog driver
.../devicetree/bindings/input/st,stpmu1-onkey.txt | 31 +
.../devicetree/bindings/mfd/st,stpmu1.txt | 138 ++++
.../bindings/regulator/st,stpmu1-regulator.txt | 72 ++
.../devicetree/bindings/watchdog/st,stpmu1-wdt.txt | 11 +
drivers/input/misc/Kconfig | 11 +
drivers/input/misc/Makefile | 2 +
drivers/input/misc/stpmu1_onkey.c | 321 +++++++
drivers/mfd/Kconfig | 14 +
drivers/mfd/Makefile | 1 +
drivers/mfd/stpmu1.c | 490 +++++++++++
drivers/regulator/Kconfig | 12 +
drivers/regulator/Makefile | 2 +
drivers/regulator/stpmu1_regulator.c | 919 +++++++++++++++++++++
drivers/watchdog/Kconfig | 12 +
drivers/watchdog/Makefile | 1 +
drivers/watchdog/stpmu1_wdt.c | 177 ++++
include/dt-bindings/mfd/st,stpmu1.h | 46 ++
include/linux/mfd/stpmu1.h | 220 +++++
18 files changed, 2480 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/st,stpmu1-onkey.txt
create mode 100644 Documentation/devicetree/bindings/mfd/st,stpmu1.txt
create mode 100644 Documentation/devicetree/bindings/regulator/st,stpmu1-regulator.txt
create mode 100644 Documentation/devicetree/bindings/watchdog/st,stpmu1-wdt.txt
create mode 100644 drivers/input/misc/stpmu1_onkey.c
create mode 100644 drivers/mfd/stpmu1.c
create mode 100644 drivers/regulator/stpmu1_regulator.c
create mode 100644 drivers/watchdog/stpmu1_wdt.c
create mode 100644 include/dt-bindings/mfd/st,stpmu1.h
create mode 100644 include/linux/mfd/stpmu1.h
--
1.9.1
^ permalink raw reply
* [PATCH 2/8] mfd: stpmu1: add stpmu1 pmic driver
From: Pascal PAILLET-LME @ 2018-07-05 15:14 UTC (permalink / raw)
To: dmitry.torokhov@gmail.com, robh+dt@kernel.org,
mark.rutland@arm.com, lee.jones@linaro.org, lgirdwood@gmail.com,
broonie@kernel.org, wim@linux-watchdog.org, linux@roeck-us.net,
linux-input@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org,
benjamin.gaignard@linaro.org
Cc: Pascal PAILLET-LME
In-Reply-To: <1530803657-17684-1-git-send-email-p.paillet@st.com>
From: pascal paillet <p.paillet@st.com>
stpmu1 is a pmic from STMicroelectronics. The stpmu1 integrates 10
regulators and 3 switches with various capabilities.
Signed-off-by: pascal paillet <p.paillet@st.com>
---
drivers/mfd/Kconfig | 14 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/stpmu1.c | 490 ++++++++++++++++++++++++++++++++++++
include/dt-bindings/mfd/st,stpmu1.h | 46 ++++
include/linux/mfd/stpmu1.h | 220 ++++++++++++++++
5 files changed, 771 insertions(+)
create mode 100644 drivers/mfd/stpmu1.c
create mode 100644 include/dt-bindings/mfd/st,stpmu1.h
create mode 100644 include/linux/mfd/stpmu1.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index b860eb5..e15140b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1812,6 +1812,20 @@ config MFD_STM32_TIMERS
for PWM and IIO Timer. This driver allow to share the
registers between the others drivers.
+config MFD_STPMU1
+ tristate "Support for STPMU1 PMIC"
+ depends on (I2C=y && OF)
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ select MFD_CORE
+ help
+ Support for ST Microelectronics STPMU1 PMIC. Stpmu1 mfd driver is
+ the core driver for stpmu1 component that mainly handles interrupts.
+
+ To compile this driver as a module, choose M here: the
+ module will be called stpmu1.
+
+
menu "Multimedia Capabilities Port drivers"
depends on ARCH_SA1100
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e9fd20d..f1c4be1 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -220,6 +220,7 @@ obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI) += intel_soc_pmic_chtdc_ti.o
obj-$(CONFIG_MFD_MT6397) += mt6397-core.o
obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o
+obj-$(CONFIG_MFD_STPMU1) += stpmu1.o
obj-$(CONFIG_MFD_SUN4I_GPADC) += sun4i-gpadc.o
obj-$(CONFIG_MFD_STM32_LPTIMER) += stm32-lptimer.o
diff --git a/drivers/mfd/stpmu1.c b/drivers/mfd/stpmu1.c
new file mode 100644
index 0000000..a284a3e
--- /dev/null
+++ b/drivers/mfd/stpmu1.c
@@ -0,0 +1,490 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
+ * Author: Philippe Peurichard <philippe.peurichard@st.com>,
+ * Pascal Paillet <p.paillet@st.com> for STMicroelectronics.
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/stpmu1.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm_wakeirq.h>
+#include <linux/regmap.h>
+#include <dt-bindings/mfd/st,stpmu1.h>
+
+static bool stpmu1_reg_readable(struct device *dev, unsigned int reg);
+static bool stpmu1_reg_writeable(struct device *dev, unsigned int reg);
+static bool stpmu1_reg_volatile(struct device *dev, unsigned int reg);
+
+const struct regmap_config stpmu1_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .cache_type = REGCACHE_RBTREE,
+ .max_register = PMIC_MAX_REGISTER_ADDRESS,
+ .readable_reg = stpmu1_reg_readable,
+ .writeable_reg = stpmu1_reg_writeable,
+ .volatile_reg = stpmu1_reg_volatile,
+};
+
+#define FILL_IRQS(_index) \
+ [(_index)] = { \
+ .reg_offset = ((_index) >> 3), \
+ .mask = (1 << (_index % 8)), \
+ }
+
+static const struct regmap_irq stpmu1_irqs[] = {
+ FILL_IRQS(IT_PONKEY_F),
+ FILL_IRQS(IT_PONKEY_R),
+ FILL_IRQS(IT_WAKEUP_F),
+ FILL_IRQS(IT_WAKEUP_R),
+ FILL_IRQS(IT_VBUS_OTG_F),
+ FILL_IRQS(IT_VBUS_OTG_R),
+ FILL_IRQS(IT_SWOUT_F),
+ FILL_IRQS(IT_SWOUT_R),
+
+ FILL_IRQS(IT_CURLIM_BUCK1),
+ FILL_IRQS(IT_CURLIM_BUCK2),
+ FILL_IRQS(IT_CURLIM_BUCK3),
+ FILL_IRQS(IT_CURLIM_BUCK4),
+ FILL_IRQS(IT_OCP_OTG),
+ FILL_IRQS(IT_OCP_SWOUT),
+ FILL_IRQS(IT_OCP_BOOST),
+ FILL_IRQS(IT_OVP_BOOST),
+
+ FILL_IRQS(IT_CURLIM_LDO1),
+ FILL_IRQS(IT_CURLIM_LDO2),
+ FILL_IRQS(IT_CURLIM_LDO3),
+ FILL_IRQS(IT_CURLIM_LDO4),
+ FILL_IRQS(IT_CURLIM_LDO5),
+ FILL_IRQS(IT_CURLIM_LDO6),
+ FILL_IRQS(IT_SHORT_SWOTG),
+ FILL_IRQS(IT_SHORT_SWOUT),
+
+ FILL_IRQS(IT_TWARN_F),
+ FILL_IRQS(IT_TWARN_R),
+ FILL_IRQS(IT_VINLOW_F),
+ FILL_IRQS(IT_VINLOW_R),
+ FILL_IRQS(IT_SWIN_F),
+ FILL_IRQS(IT_SWIN_R),
+};
+
+static const struct regmap_irq_chip stpmu1_regmap_irq_chip = {
+ .name = "pmic_irq",
+ .status_base = INT_PENDING_R1,
+ .mask_base = INT_CLEAR_MASK_R1,
+ .unmask_base = INT_SET_MASK_R1,
+ .ack_base = INT_CLEAR_R1,
+ .num_regs = STPMU1_PMIC_NUM_IRQ_REGS,
+ .irqs = stpmu1_irqs,
+ .num_irqs = ARRAY_SIZE(stpmu1_irqs),
+};
+
+static bool stpmu1_reg_readable(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case TURN_ON_SR:
+ case TURN_OFF_SR:
+ case ICC_LDO_TURN_OFF_SR:
+ case ICC_BUCK_TURN_OFF_SR:
+ case RREQ_STATE_SR:
+ case VERSION_SR:
+ case SWOFF_PWRCTRL_CR:
+ case PADS_PULL_CR:
+ case BUCKS_PD_CR:
+ case LDO14_PD_CR:
+ case LDO56_VREF_PD_CR:
+ case VBUS_DET_VIN_CR:
+ case PKEY_TURNOFF_CR:
+ case BUCKS_MASK_RANK_CR:
+ case BUCKS_MASK_RESET_CR:
+ case LDOS_MASK_RANK_CR:
+ case LDOS_MASK_RESET_CR:
+ case WCHDG_CR:
+ case WCHDG_TIMER_CR:
+ case BUCKS_ICCTO_CR:
+ case LDOS_ICCTO_CR:
+ case BUCK1_ACTIVE_CR:
+ case BUCK2_ACTIVE_CR:
+ case BUCK3_ACTIVE_CR:
+ case BUCK4_ACTIVE_CR:
+ case VREF_DDR_ACTIVE_CR:
+ case LDO1_ACTIVE_CR:
+ case LDO2_ACTIVE_CR:
+ case LDO3_ACTIVE_CR:
+ case LDO4_ACTIVE_CR:
+ case LDO5_ACTIVE_CR:
+ case LDO6_ACTIVE_CR:
+ case BUCK1_STDBY_CR:
+ case BUCK2_STDBY_CR:
+ case BUCK3_STDBY_CR:
+ case BUCK4_STDBY_CR:
+ case VREF_DDR_STDBY_CR:
+ case LDO1_STDBY_CR:
+ case LDO2_STDBY_CR:
+ case LDO3_STDBY_CR:
+ case LDO4_STDBY_CR:
+ case LDO5_STDBY_CR:
+ case LDO6_STDBY_CR:
+ case BST_SW_CR:
+ case INT_PENDING_R1:
+ case INT_PENDING_R2:
+ case INT_PENDING_R3:
+ case INT_PENDING_R4:
+ case INT_DBG_LATCH_R1:
+ case INT_DBG_LATCH_R2:
+ case INT_DBG_LATCH_R3:
+ case INT_DBG_LATCH_R4:
+ case INT_CLEAR_R1:
+ case INT_CLEAR_R2:
+ case INT_CLEAR_R3:
+ case INT_CLEAR_R4:
+ case INT_MASK_R1:
+ case INT_MASK_R2:
+ case INT_MASK_R3:
+ case INT_MASK_R4:
+ case INT_SET_MASK_R1:
+ case INT_SET_MASK_R2:
+ case INT_SET_MASK_R3:
+ case INT_SET_MASK_R4:
+ case INT_CLEAR_MASK_R1:
+ case INT_CLEAR_MASK_R2:
+ case INT_CLEAR_MASK_R3:
+ case INT_CLEAR_MASK_R4:
+ case INT_SRC_R1:
+ case INT_SRC_R2:
+ case INT_SRC_R3:
+ case INT_SRC_R4:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool stpmu1_reg_writeable(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case SWOFF_PWRCTRL_CR:
+ case PADS_PULL_CR:
+ case BUCKS_PD_CR:
+ case LDO14_PD_CR:
+ case LDO56_VREF_PD_CR:
+ case VBUS_DET_VIN_CR:
+ case PKEY_TURNOFF_CR:
+ case BUCKS_MASK_RANK_CR:
+ case BUCKS_MASK_RESET_CR:
+ case LDOS_MASK_RANK_CR:
+ case LDOS_MASK_RESET_CR:
+ case WCHDG_CR:
+ case WCHDG_TIMER_CR:
+ case BUCKS_ICCTO_CR:
+ case LDOS_ICCTO_CR:
+ case BUCK1_ACTIVE_CR:
+ case BUCK2_ACTIVE_CR:
+ case BUCK3_ACTIVE_CR:
+ case BUCK4_ACTIVE_CR:
+ case VREF_DDR_ACTIVE_CR:
+ case LDO1_ACTIVE_CR:
+ case LDO2_ACTIVE_CR:
+ case LDO3_ACTIVE_CR:
+ case LDO4_ACTIVE_CR:
+ case LDO5_ACTIVE_CR:
+ case LDO6_ACTIVE_CR:
+ case BUCK1_STDBY_CR:
+ case BUCK2_STDBY_CR:
+ case BUCK3_STDBY_CR:
+ case BUCK4_STDBY_CR:
+ case VREF_DDR_STDBY_CR:
+ case LDO1_STDBY_CR:
+ case LDO2_STDBY_CR:
+ case LDO3_STDBY_CR:
+ case LDO4_STDBY_CR:
+ case LDO5_STDBY_CR:
+ case LDO6_STDBY_CR:
+ case BST_SW_CR:
+ case INT_DBG_LATCH_R1:
+ case INT_DBG_LATCH_R2:
+ case INT_DBG_LATCH_R3:
+ case INT_DBG_LATCH_R4:
+ case INT_CLEAR_R1:
+ case INT_CLEAR_R2:
+ case INT_CLEAR_R3:
+ case INT_CLEAR_R4:
+ case INT_SET_MASK_R1:
+ case INT_SET_MASK_R2:
+ case INT_SET_MASK_R3:
+ case INT_SET_MASK_R4:
+ case INT_CLEAR_MASK_R1:
+ case INT_CLEAR_MASK_R2:
+ case INT_CLEAR_MASK_R3:
+ case INT_CLEAR_MASK_R4:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool stpmu1_reg_volatile(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case TURN_ON_SR:
+ case TURN_OFF_SR:
+ case ICC_LDO_TURN_OFF_SR:
+ case ICC_BUCK_TURN_OFF_SR:
+ case RREQ_STATE_SR:
+ case INT_PENDING_R1:
+ case INT_PENDING_R2:
+ case INT_PENDING_R3:
+ case INT_PENDING_R4:
+ case INT_SRC_R1:
+ case INT_SRC_R2:
+ case INT_SRC_R3:
+ case INT_SRC_R4:
+ case WCHDG_CR:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static int stpmu1_configure_from_dt(struct stpmu1_dev *pmic_dev)
+{
+ struct device_node *np = pmic_dev->np;
+ u32 reg = 0;
+ int ret = 0;
+ int irq;
+
+ irq = of_irq_get(np, 0);
+ if (irq <= 0) {
+ dev_err(pmic_dev->dev,
+ "Failed to get irq config: %d\n", irq);
+ return irq ? irq : -ENODEV;
+ }
+ pmic_dev->irq = irq;
+
+ irq = of_irq_get(np, 1);
+ if (irq <= 0) {
+ dev_err(pmic_dev->dev,
+ "Failed to get irq_wake config: %d\n", irq);
+ return irq ? irq : -ENODEV;
+ }
+ pmic_dev->irq_wake = irq;
+
+ device_init_wakeup(pmic_dev->dev, true);
+ ret = dev_pm_set_dedicated_wake_irq(pmic_dev->dev, pmic_dev->irq_wake);
+ if (ret)
+ dev_warn(pmic_dev->dev, "failed to set up wakeup irq");
+
+ if (!of_property_read_u32(np, "st,main_control_register", ®)) {
+ ret = regmap_update_bits(pmic_dev->regmap,
+ SWOFF_PWRCTRL_CR,
+ PWRCTRL_POLARITY_HIGH |
+ PWRCTRL_PIN_VALID |
+ RESTART_REQUEST_ENABLED,
+ reg);
+ if (ret) {
+ dev_err(pmic_dev->dev,
+ "Failed to update main control register: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
+ if (!of_property_read_u32(np, "st,pads_pull_register", ®)) {
+ ret = regmap_update_bits(pmic_dev->regmap,
+ PADS_PULL_CR,
+ WAKEUP_DETECTOR_DISABLED |
+ PWRCTRL_PD_ACTIVE |
+ PWRCTRL_PU_ACTIVE |
+ WAKEUP_PD_ACTIVE,
+ reg);
+ if (ret) {
+ dev_err(pmic_dev->dev,
+ "Failed to update pads control register: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
+ if (!of_property_read_u32(np, "st,vin_control_register", ®)) {
+ ret = regmap_update_bits(pmic_dev->regmap,
+ VBUS_DET_VIN_CR,
+ VINLOW_CTRL_REG_MASK,
+ reg);
+ if (ret) {
+ dev_err(pmic_dev->dev,
+ "Failed to update vin control register: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
+ if (!of_property_read_u32(np, "st,usb_control_register", ®)) {
+ ret = regmap_update_bits(pmic_dev->regmap, BST_SW_CR,
+ BOOST_OVP_DISABLED |
+ VBUS_OTG_DETECTION_DISABLED |
+ SW_OUT_DISCHARGE |
+ VBUS_OTG_DISCHARGE |
+ OCP_LIMIT_HIGH,
+ reg);
+ if (ret) {
+ dev_err(pmic_dev->dev,
+ "Failed to update usb control register: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+int stpmu1_device_init(struct stpmu1_dev *pmic_dev)
+{
+ int ret;
+ unsigned int val;
+
+ pmic_dev->regmap =
+ devm_regmap_init_i2c(pmic_dev->i2c, &stpmu1_regmap_config);
+
+ if (IS_ERR(pmic_dev->regmap)) {
+ ret = PTR_ERR(pmic_dev->regmap);
+ dev_err(pmic_dev->dev, "Failed to allocate register map: %d\n",
+ ret);
+ return ret;
+ }
+
+ ret = stpmu1_configure_from_dt(pmic_dev);
+ if (ret < 0) {
+ dev_err(pmic_dev->dev,
+ "Unable to configure PMIC from Device Tree: %d\n", ret);
+ return ret;
+ }
+
+ /* Read Version ID */
+ ret = regmap_read(pmic_dev->regmap, VERSION_SR, &val);
+ if (ret < 0) {
+ dev_err(pmic_dev->dev, "Unable to read pmic version\n");
+ return ret;
+ }
+ dev_dbg(pmic_dev->dev, "PMIC Chip Version: 0x%x\n", val);
+
+ /* Initialize PMIC IRQ Chip & IRQ domains associated */
+ ret = devm_regmap_add_irq_chip(pmic_dev->dev, pmic_dev->regmap,
+ pmic_dev->irq,
+ IRQF_ONESHOT | IRQF_SHARED,
+ 0, &stpmu1_regmap_irq_chip,
+ &pmic_dev->irq_data);
+ if (ret < 0) {
+ dev_err(pmic_dev->dev, "IRQ Chip registration failed: %d\n",
+ ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id stpmu1_dt_match[] = {
+ {.compatible = "st,stpmu1"},
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, stpmu1_dt_match);
+
+static int stpmu1_remove(struct i2c_client *i2c)
+{
+ struct stpmu1_dev *pmic_dev = i2c_get_clientdata(i2c);
+
+ of_platform_depopulate(pmic_dev->dev);
+
+ return 0;
+}
+
+static int stpmu1_probe(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+ struct stpmu1_dev *pmic;
+ struct device *dev = &i2c->dev;
+ int ret = 0;
+
+ pmic = devm_kzalloc(dev, sizeof(struct stpmu1_dev), GFP_KERNEL);
+ if (!pmic)
+ return -ENOMEM;
+
+ pmic->np = dev->of_node;
+
+ dev_set_drvdata(dev, pmic);
+ pmic->dev = dev;
+ pmic->i2c = i2c;
+
+ ret = stpmu1_device_init(pmic);
+ if (ret < 0)
+ goto err;
+
+ ret = of_platform_populate(pmic->np, NULL, NULL, pmic->dev);
+
+ dev_dbg(dev, "stpmu1 driver probed\n");
+err:
+ return ret;
+}
+
+static const struct i2c_device_id stpmu1_id[] = {
+ {"stpmu1", 0},
+ {}
+};
+
+MODULE_DEVICE_TABLE(i2c, stpmu1_id);
+
+#ifdef CONFIG_PM_SLEEP
+static int stpmu1_suspend(struct device *dev)
+{
+ struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
+ struct stpmu1_dev *pmic_dev = i2c_get_clientdata(i2c);
+
+ if (device_may_wakeup(dev))
+ enable_irq_wake(pmic_dev->irq_wake);
+
+ disable_irq(pmic_dev->irq);
+ return 0;
+}
+
+static int stpmu1_resume(struct device *dev)
+{
+ struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
+ struct stpmu1_dev *pmic_dev = i2c_get_clientdata(i2c);
+
+ regcache_sync(pmic_dev->regmap);
+
+ if (device_may_wakeup(dev))
+ disable_irq_wake(pmic_dev->irq_wake);
+
+ enable_irq(pmic_dev->irq);
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(stpmu1_pm, stpmu1_suspend, stpmu1_resume);
+
+static struct i2c_driver stpmu1_driver = {
+ .driver = {
+ .name = "stpmu1",
+ .owner = THIS_MODULE,
+ .pm = &stpmu1_pm,
+ .of_match_table = of_match_ptr(stpmu1_dt_match),
+ },
+ .probe = stpmu1_probe,
+ .remove = stpmu1_remove,
+ .id_table = stpmu1_id,
+};
+
+module_i2c_driver(stpmu1_driver);
+
+MODULE_DESCRIPTION("STPMU1 PMIC I2C Client");
+MODULE_AUTHOR("<philippe.peurichard@st.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/dt-bindings/mfd/st,stpmu1.h b/include/dt-bindings/mfd/st,stpmu1.h
new file mode 100644
index 0000000..2d3bdf1
--- /dev/null
+++ b/include/dt-bindings/mfd/st,stpmu1.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
+ * Author: Philippe Peurichard <philippe.peurichard@st.com>,
+ * Pascal Paillet <p.paillet@st.com> for STMicroelectronics.
+ */
+
+#ifndef __DT_BINDINGS_STPMU1_H__
+#define __DT_BINDINGS_STPMU1_H__
+
+/* IRQ definitions */
+#define IT_PONKEY_F 0
+#define IT_PONKEY_R 1
+#define IT_WAKEUP_F 2
+#define IT_WAKEUP_R 3
+#define IT_VBUS_OTG_F 4
+#define IT_VBUS_OTG_R 5
+#define IT_SWOUT_F 6
+#define IT_SWOUT_R 7
+
+#define IT_CURLIM_BUCK1 8
+#define IT_CURLIM_BUCK2 9
+#define IT_CURLIM_BUCK3 10
+#define IT_CURLIM_BUCK4 11
+#define IT_OCP_OTG 12
+#define IT_OCP_SWOUT 13
+#define IT_OCP_BOOST 14
+#define IT_OVP_BOOST 15
+
+#define IT_CURLIM_LDO1 16
+#define IT_CURLIM_LDO2 17
+#define IT_CURLIM_LDO3 18
+#define IT_CURLIM_LDO4 19
+#define IT_CURLIM_LDO5 20
+#define IT_CURLIM_LDO6 21
+#define IT_SHORT_SWOTG 22
+#define IT_SHORT_SWOUT 23
+
+#define IT_TWARN_F 24
+#define IT_TWARN_R 25
+#define IT_VINLOW_F 26
+#define IT_VINLOW_R 27
+#define IT_SWIN_F 30
+#define IT_SWIN_R 31
+
+#endif /* __DT_BINDINGS_STPMU1_H__ */
diff --git a/include/linux/mfd/stpmu1.h b/include/linux/mfd/stpmu1.h
new file mode 100644
index 0000000..05fd029
--- /dev/null
+++ b/include/linux/mfd/stpmu1.h
@@ -0,0 +1,220 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
+ * Author: Philippe Peurichard <philippe.peurichard@st.com>,
+ * Pascal Paillet <p.paillet@st.com> for STMicroelectronics.
+ */
+
+#ifndef __LINUX_MFD_STPMU1_H
+#define __LINUX_MFD_STPMU1_H
+
+#define TURN_ON_SR 0x1
+#define TURN_OFF_SR 0x2
+#define ICC_LDO_TURN_OFF_SR 0x3
+#define ICC_BUCK_TURN_OFF_SR 0x4
+#define RREQ_STATE_SR 0x5
+#define VERSION_SR 0x6
+
+#define SWOFF_PWRCTRL_CR 0x10
+#define PADS_PULL_CR 0x11
+#define BUCKS_PD_CR 0x12
+#define LDO14_PD_CR 0x13
+#define LDO56_VREF_PD_CR 0x14
+#define VBUS_DET_VIN_CR 0x15
+#define PKEY_TURNOFF_CR 0x16
+#define BUCKS_MASK_RANK_CR 0x17
+#define BUCKS_MASK_RESET_CR 0x18
+#define LDOS_MASK_RANK_CR 0x19
+#define LDOS_MASK_RESET_CR 0x1A
+#define WCHDG_CR 0x1B
+#define WCHDG_TIMER_CR 0x1C
+#define BUCKS_ICCTO_CR 0x1D
+#define LDOS_ICCTO_CR 0x1E
+
+#define BUCK1_ACTIVE_CR 0x20
+#define BUCK2_ACTIVE_CR 0x21
+#define BUCK3_ACTIVE_CR 0x22
+#define BUCK4_ACTIVE_CR 0x23
+#define VREF_DDR_ACTIVE_CR 0x24
+#define LDO1_ACTIVE_CR 0x25
+#define LDO2_ACTIVE_CR 0x26
+#define LDO3_ACTIVE_CR 0x27
+#define LDO4_ACTIVE_CR 0x28
+#define LDO5_ACTIVE_CR 0x29
+#define LDO6_ACTIVE_CR 0x2A
+
+#define BUCK1_STDBY_CR 0x30
+#define BUCK2_STDBY_CR 0x31
+#define BUCK3_STDBY_CR 0x32
+#define BUCK4_STDBY_CR 0x33
+#define VREF_DDR_STDBY_CR 0x34
+#define LDO1_STDBY_CR 0x35
+#define LDO2_STDBY_CR 0x36
+#define LDO3_STDBY_CR 0x37
+#define LDO4_STDBY_CR 0x38
+#define LDO5_STDBY_CR 0x39
+#define LDO6_STDBY_CR 0x3A
+
+#define BST_SW_CR 0x40
+
+#define INT_PENDING_R1 0x50
+#define INT_PENDING_R2 0x51
+#define INT_PENDING_R3 0x52
+#define INT_PENDING_R4 0x53
+
+#define INT_DBG_LATCH_R1 0x60
+#define INT_DBG_LATCH_R2 0x61
+#define INT_DBG_LATCH_R3 0x62
+#define INT_DBG_LATCH_R4 0x63
+
+#define INT_CLEAR_R1 0x70
+#define INT_CLEAR_R2 0x71
+#define INT_CLEAR_R3 0x72
+#define INT_CLEAR_R4 0x73
+
+#define INT_MASK_R1 0x80
+#define INT_MASK_R2 0x81
+#define INT_MASK_R3 0x82
+#define INT_MASK_R4 0x83
+
+#define INT_SET_MASK_R1 0x90
+#define INT_SET_MASK_R2 0x91
+#define INT_SET_MASK_R3 0x92
+#define INT_SET_MASK_R4 0x93
+
+#define INT_CLEAR_MASK_R1 0xA0
+#define INT_CLEAR_MASK_R2 0xA1
+#define INT_CLEAR_MASK_R3 0xA2
+#define INT_CLEAR_MASK_R4 0xA3
+
+#define INT_SRC_R1 0xB0
+#define INT_SRC_R2 0xB1
+#define INT_SRC_R3 0xB2
+#define INT_SRC_R4 0xB3
+
+#define PMIC_MAX_REGISTER_ADDRESS INT_SRC_R4
+
+#define STPMU1_PMIC_NUM_IRQ_REGS 4
+
+#define TURN_OFF_SR_ICC_EVENT 0x08
+
+#define LDO_VOLTAGE_MASK GENMASK(6, 2)
+#define BUCK_VOLTAGE_MASK GENMASK(7, 2)
+#define LDO_BUCK_VOLTAGE_SHIFT 2
+
+#define LDO_ENABLE_MASK BIT(0)
+#define BUCK_ENABLE_MASK BIT(0)
+
+#define BUCK_HPLP_ENABLE_MASK BIT(1)
+#define BUCK_HPLP_SHIFT 1
+
+#define STDBY_ENABLE_MASK BIT(0)
+
+#define BUCKS_PD_CR_REG_MASK GENMASK(7, 0)
+#define BUCK_MASK_RANK_REGISTER_MASK GENMASK(3, 0)
+#define BUCK_MASK_RESET_REGISTER_MASK GENMASK(3, 0)
+#define LDO1234_PULL_DOWN_REGISTER_MASK GENMASK(7, 0)
+#define LDO56_VREF_PD_CR_REG_MASK GENMASK(5, 0)
+#define LDO_MASK_RANK_REGISTER_MASK GENMASK(5, 0)
+#define LDO_MASK_RESET_REGISTER_MASK GENMASK(5, 0)
+
+#define BUCK1_PULL_DOWN_REG BUCKS_PD_CR
+#define BUCK1_PULL_DOWN_MASK BIT(0)
+#define BUCK2_PULL_DOWN_REG BUCKS_PD_CR
+#define BUCK2_PULL_DOWN_MASK BIT(2)
+#define BUCK3_PULL_DOWN_REG BUCKS_PD_CR
+#define BUCK3_PULL_DOWN_MASK BIT(4)
+#define BUCK4_PULL_DOWN_REG BUCKS_PD_CR
+#define BUCK4_PULL_DOWN_MASK BIT(6)
+
+#define LDO1_PULL_DOWN_REG LDO14_PD_CR
+#define LDO1_PULL_DOWN_MASK BIT(0)
+#define LDO2_PULL_DOWN_REG LDO14_PD_CR
+#define LDO2_PULL_DOWN_MASK BIT(2)
+#define LDO3_PULL_DOWN_REG LDO14_PD_CR
+#define LDO3_PULL_DOWN_MASK BIT(4)
+#define LDO4_PULL_DOWN_REG LDO14_PD_CR
+#define LDO4_PULL_DOWN_MASK BIT(6)
+#define LDO5_PULL_DOWN_REG LDO56_VREF_PD_CR
+#define LDO5_PULL_DOWN_MASK BIT(0)
+#define LDO6_PULL_DOWN_REG LDO56_VREF_PD_CR
+#define LDO6_PULL_DOWN_MASK BIT(2)
+#define VREF_DDR_PULL_DOWN_REG LDO56_VREF_PD_CR
+#define VREF_DDR_PULL_DOWN_MASK BIT(4)
+
+#define BUCKS_ICCTO_CR_REG_MASK GENMASK(6, 0)
+#define LDOS_ICCTO_CR_REG_MASK GENMASK(5, 0)
+
+#define LDO_BYPASS_MASK BIT(7)
+
+/* Main PMIC Control Register
+ * SWOFF_PWRCTRL_CR
+ * Address : 0x10
+ */
+#define ICC_EVENT_ENABLED BIT(4)
+#define PWRCTRL_POLARITY_HIGH BIT(3)
+#define PWRCTRL_PIN_VALID BIT(2)
+#define RESTART_REQUEST_ENABLED BIT(1)
+#define SOFTWARE_SWITCH_OFF_ENABLED BIT(0)
+
+/* Main PMIC PADS Control Register
+ * PADS_PULL_CR
+ * Address : 0x11
+ */
+#define WAKEUP_DETECTOR_DISABLED BIT(4)
+#define PWRCTRL_PD_ACTIVE BIT(3)
+#define PWRCTRL_PU_ACTIVE BIT(2)
+#define WAKEUP_PD_ACTIVE BIT(1)
+#define PONKEY_PU_ACTIVE BIT(0)
+
+/* Main PMIC VINLOW Control Register
+ * VBUS_DET_VIN_CRC DMSC
+ * Address : 0x15
+ */
+#define SWIN_DETECTOR_ENABLED BIT(7)
+#define SWOUT_DETECTOR_ENABLED BIT(6)
+#define VINLOW_ENABLED BIT(0)
+#define VINLOW_CTRL_REG_MASK GENMASK(7, 0)
+
+/* USB Control Register
+ * Address : 0x40
+ */
+#define BOOST_OVP_DISABLED BIT(7)
+#define VBUS_OTG_DETECTION_DISABLED BIT(6)
+#define SW_OUT_DISCHARGE BIT(5)
+#define VBUS_OTG_DISCHARGE BIT(4)
+#define OCP_LIMIT_HIGH BIT(3)
+#define SWIN_SWOUT_ENABLED BIT(2)
+#define USBSW_OTG_SWITCH_ENABLED BIT(1)
+#define BOOST_ENABLED BIT(0)
+
+/* PKEY_TURNOFF_CR
+ * Address : 0x16
+ */
+#define PONKEY_PWR_OFF BIT(7)
+#define PONKEY_CC_FLAG_CLEAR BIT(6)
+#define PONKEY_TURNOFF_TIMER_MASK GENMASK(3, 0)
+#define PONKEY_TURNOFF_MASK GENMASK(7, 0)
+
+/*
+ * struct stpmu1_dev - stpmu1 master device for sub-drivers
+ * @dev: master device of the chip (can be used to access platform data)
+ * @i2c: i2c client private data for regulator
+ * @np: device DT node pointer
+ * @irq_base: base IRQ numbers
+ * @irq: generic IRQ number
+ * @irq_wake: wakeup IRQ number
+ * @regmap_irq_chip_data: irq chip data
+ */
+struct stpmu1_dev {
+ struct device *dev;
+ struct i2c_client *i2c;
+ struct regmap *regmap;
+ struct device_node *np;
+ unsigned int irq_base;
+ int irq;
+ int irq_wake;
+ struct regmap_irq_chip_data *irq_data;
+};
+
+#endif /* __LINUX_MFD_STPMU1_H */
--
1.9.1
^ permalink raw reply related
* [PATCH 1/8] dt-bindings: mfd: document stpmu1 pmic
From: Pascal PAILLET-LME @ 2018-07-05 15:14 UTC (permalink / raw)
To: dmitry.torokhov@gmail.com, robh+dt@kernel.org,
mark.rutland@arm.com, lee.jones@linaro.org, lgirdwood@gmail.com,
broonie@kernel.org, wim@linux-watchdog.org, linux@roeck-us.net,
linux-input@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org,
benjamin.gaignard@linaro.org
Cc: Pascal PAILLET-LME
In-Reply-To: <1530803657-17684-1-git-send-email-p.paillet@st.com>
From: pascal paillet <p.paillet@st.com>
stpmu1 is a pmic from STMicroelectronics. The stpmu1 integrates 10
regulators and 3 switches with various capabilities.
Signed-off-by: pascal paillet <p.paillet@st.com>
---
.../devicetree/bindings/mfd/st,stpmu1.txt | 138 +++++++++++++++++++++
1 file changed, 138 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/st,stpmu1.txt
diff --git a/Documentation/devicetree/bindings/mfd/st,stpmu1.txt b/Documentation/devicetree/bindings/mfd/st,stpmu1.txt
new file mode 100644
index 0000000..53bdab4
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/st,stpmu1.txt
@@ -0,0 +1,138 @@
+* STMicroelectronics STPMU1 Power Management IC
+
+Required parent device properties:
+- compatible: "st,stpmu1"
+- reg: the I2C slave address for the stpmu1 chip
+- interrupts-extended: interrupt lines to use: second irq is for wakeup.
+- #interrupt-cells: should be 2.
+- interrupt-controller: describes the STPMU1 as an interrupt
+ controller (has its own domain). interrupt number are the following:
+ /* Interrupt Register 1 (0x50 for latch) */
+ IT_SWOUT_R=0
+ IT_SWOUT_F=1
+ IT_VBUS_OTG_R=2
+ IT_VBUS_OTG_F=3
+ IT_WAKEUP_R=4
+ IT_WAKEUP_F=5
+ IT_PONKEY_R=6
+ IT_PONKEY_F=7
+ /* Interrupt Register 2 (0x51 for latch) */
+ IT_OVP_BOOST=8
+ IT_OCP_BOOST=9
+ IT_OCP_SWOUT=10
+ IT_OCP_OTG=11
+ IT_CURLIM_BUCK4=12
+ IT_CURLIM_BUCK3=13
+ IT_CURLIM_BUCK2=14
+ IT_CURLIM_BUCK1=15
+ /* Interrupt Register 3 (0x52 for latch) */
+ IT_SHORT_SWOUT=16
+ IT_SHORT_SWOTG=17
+ IT_CURLIM_LDO6=18
+ IT_CURLIM_LDO5=19
+ IT_CURLIM_LDO4=20
+ IT_CURLIM_LDO3=21
+ IT_CURLIM_LDO2=22
+ IT_CURLIM_LDO1=23
+ /* Interrupt Register 3 (0x52 for latch) */
+ IT_SWIN_R=24
+ IT_SWIN_F=25
+ IT_RESERVED_1=26
+ IT_RESERVED_2=27
+ IT_VINLOW_R=28
+ IT_VINLOW_F=29
+ IT_TWARN_R=30
+ IT_TWARN_F=31
+
+Optional parent device properties:
+- st,main_control_register:
+ -bit 1: Power cycling will be performed on turn OFF condition
+ -bit 2: PWRCTRL is functional
+ -bit 3: PWRCTRL active high
+- st,pads_pull_register:
+ -bit 1: WAKEUP pull down is not active
+ -bit 2: PWRCTRL pull up is active
+ -bit 3: PWRCTRL pull down is active
+ -bit 4: WAKEUP detector is disabled
+- st,vin_control_register:
+ -bit 0: VINLOW monitoring is enabled
+ -bit [1...3]: VINLOW rising threshold
+ 000 VINOK_f + 50mV
+ 001 VINOK_f + 100mV
+ 010 VINOK_f + 150mV
+ 011 VINOK_f + 200mV
+ 100 VINOK_f + 250mV
+ 101 VINOK_f + 300mV
+ 110 VINOK_f + 350mV
+ 111 VINOK_f + 400mV
+ -bit [4...5]: VINLOW hyst
+ 00 100mV
+ 01 200mV
+ 10 300mV
+ 11 400mV
+ -bit 6: SW_OUT detector is disabled
+ -bit 7: SW_IN detector is enabled.
+- st,usb_control_register:
+ -bit 3: SW_OUT current limit
+ 0: 600mA
+ 1: 1.1A
+ -bit 4: VBUS_OTG discharge is enabled
+ -bit 5: SW_OUT discharge is enabled
+ -bit 6: VBUS_OTG detection is enabled
+ -bit 7: BOOST_OVP is disabled
+
+
+stpmu1 consists is a varied group of sub-devices:
+
+Device Description
+------ ------------
+stpmu1-onkey : On key
+stpmu1-regulators : Regulators
+stpmu1-wdt : Watchdog
+
+each sub-device bindings is be described in associated driver
+documentation section.
+
+Example:
+
+pmic: stpmu1@33 {
+ compatible = "st,stpmu1";
+ reg = <0x33>;
+ interrupts = <0 2>;
+ interrupts-extended = <&intc GIC_SPI 149 IRQ_TYPE_NONE>,
+ <&exti 55 1>;
+ st,version_status = <0x10>;
+ st,main_control_register=<0x0c>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ onkey {
+ compatible = "st,stpmu1-onkey";
+ interrupt-parent = <&pmic>;
+ interrupts = <7 0>,<6 1>;
+ st,onkey-pwroff-enabled;
+ st,onkey-press-seconds = <10>;
+ };
+
+ watchdog {
+ compatible = "st,stpmu1-wdt";
+ };
+
+ regulators {
+ compatible = "st,stpmu1-regulators";
+
+ vdd_core: regulator@0 {
+ regulator-compatible = "buck1";
+ regulator-name = "vdd_core";
+ regulator-boot-on;
+ regulator-min-microvolt = <700000>;
+ regulator-max-microvolt = <1200000>;
+ };
+ vdd: regulator@1 {
+ regulator-compatible = "buck3";
+ regulator-name = "vdd";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-boot-on;
+ regulator-pull-down;
+ };
+ };
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox