* [PATCH v4 00/16] Support ROHM BD71815 PMIC
@ 2021-03-24 7:21 Matti Vaittinen
2021-03-24 7:22 ` [PATCH v4 01/16] rtc: bd70528: Do not require parent data Matti Vaittinen
` (14 more replies)
0 siblings, 15 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:21 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Liam Girdwood, Mark Brown,
Matti Vaittinen, Michael Turquette, Stephen Boyd, Linus Walleij,
Bartosz Golaszewski, Alessandro Zummo, Alexandre Belloni,
devicetree, linux-kernel, linux-power, linux-clk, linux-gpio,
linux-rtc
Patch series introducing support for ROHM BD71815 PMIC
ROHM BD71815 is a power management IC used in some battery powered
systems. It contains regulators, GPO(s), charger + coulomb counter, RTC
and a clock gate.
All regulators can be controlled via I2C. LDO4 can additionally be set to
be enabled/disabled by a GPIO. LDO3 voltage could be selected from two
voltages written into separate VSEL reisters using GPIO but this mode is
not supported by driver. On top of that the PMIC has the typical HW
state machine which is present also on many other ROHM PMICs.
IC contains two GPOs - but one of the GPOs is marked as GND in
data-sheet. Thus the driver by default only exposes one GPO. The second
GPO can be enabled by special DT property.
RTC is almost similar to what is on BD71828. For currently used features
only the register address offset to RTC block differs.
The charger driver is not included in this series. ROHM has a charger
driver with some fuel-gauging logig written in but this is not included
here. I am working on separating the logic from HW specific driver and
supporting both BD71815 and BD71828 chargers in separate patch series.
Changelog v4:
- Sorted ROHM chip ID enum
- Statcized DVS structures in regulator driver
- Minor styling for regulator driver
- rebased on v5.12-rc4
Changelog v3:
- GPIO clean-up as suggested by Bartosz
- MFD clean-up as suggested by Lee
- clk-mode dt-binding handling in MFD driver corrected to reflect new
property values.
- Dropped already applied patches
- Rebased on v5.12-rc2
Changelog v2:
- Rebased on top of v5.11-rc3
- Added another "preliminary patch" which fixes HW-dvs voltage
handling (patch 1)
- split regulator patch to two.
- changed dt-binding patch ordering.
regulators:
- staticized probe
- removed some unnecessary defines
- updated comments
- split rohm-regulator patch adding SNVS and supporting simple
linear mapping into two - one adding support for mapping, other
adding SNVS.
GPIO:
- removed unnecessary headers
- clarified dev/parent->dev usage
- removed forgotten #define DEBUG
dt-bindings:
- changed patch order to meet ref-dependencies
- added missing regulator nodes
- changed string property for clk mode to tristated
MFD:
- header cleanups.
CLK:
- fixed commit message
--
Matti Vaittinen (16):
rtc: bd70528: Do not require parent data
mfd: bd718x7: simplify by cleaning unnecessary device data
dt_bindings: bd71828: Add clock output mode
dt_bindings: regulator: Add ROHM BD71815 PMIC regulators
dt_bindings: mfd: Add ROHM BD71815 PMIC
mfd: Add ROHM BD71815 ID
mfd: Sort ROHM chip ID list for better readability
mfd: Support for ROHM BD71815 PMIC core
gpio: support ROHM BD71815 GPOs
regulator: helpers: Export helper voltage listing
regulator: rohm-regulator: linear voltage support
regulator: rohm-regulator: Support SNVS HW state.
regulator: Support ROHM BD71815 regulators
clk: bd718x7: Add support for clk gate on ROHM BD71815 PMIC
rtc: bd70528: Support RTC on ROHM BD71815
MAINTAINERS: Add ROHM BD71815AGW
.../bindings/mfd/rohm,bd71815-pmic.yaml | 201 ++++++
.../bindings/mfd/rohm,bd71828-pmic.yaml | 6 +
.../regulator/rohm,bd71815-regulator.yaml | 116 +++
MAINTAINERS | 3 +
drivers/clk/clk-bd718x7.c | 9 +-
drivers/gpio/Kconfig | 10 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-bd71815.c | 176 +++++
drivers/mfd/Kconfig | 15 +-
drivers/mfd/rohm-bd71828.c | 486 +++++++++----
drivers/mfd/rohm-bd718x7.c | 43 +-
drivers/regulator/Kconfig | 11 +
drivers/regulator/Makefile | 1 +
drivers/regulator/bd71815-regulator.c | 676 ++++++++++++++++++
drivers/regulator/helpers.c | 36 +-
drivers/regulator/rohm-regulator.c | 23 +-
drivers/rtc/Kconfig | 6 +-
drivers/rtc/rtc-bd70528.c | 104 +--
include/linux/mfd/rohm-bd71815.h | 562 +++++++++++++++
include/linux/mfd/rohm-bd71828.h | 3 +
include/linux/mfd/rohm-bd718x7.h | 13 -
include/linux/mfd/rohm-generic.h | 15 +-
include/linux/regulator/driver.h | 2 +
23 files changed, 2286 insertions(+), 232 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.yaml
create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd71815-regulator.yaml
create mode 100644 drivers/gpio/gpio-bd71815.c
create mode 100644 drivers/regulator/bd71815-regulator.c
create mode 100644 include/linux/mfd/rohm-bd71815.h
base-commit: 0d02ec6b3136c73c09e7859f0d0e4e2c4c07b49b
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v4 01/16] rtc: bd70528: Do not require parent data
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
@ 2021-03-24 7:22 ` Matti Vaittinen
2021-03-24 7:22 ` [PATCH v4 02/16] mfd: bd718x7: simplify by cleaning unnecessary device data Matti Vaittinen
` (13 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:22 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Matti Vaittinen, Alessandro Zummo, Alexandre Belloni,
linux-kernel, linux-power, linux-rtc
The ROHM BD71828 and BD71815 RTC drivers only need the regmap
pointer from parent. Regmap can be obtained via dev_get_regmap()
so do not require parent to populate driver data for that.
BD70528 on the other hand requires parent data to access the
watchdog so leave the parent data for BD70528 here for now.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
No changes since v3
drivers/rtc/rtc-bd70528.c | 67 ++++++++++++++++++---------------------
1 file changed, 31 insertions(+), 36 deletions(-)
diff --git a/drivers/rtc/rtc-bd70528.c b/drivers/rtc/rtc-bd70528.c
index 17cb67f5bf6e..fb4476bb5ab6 100644
--- a/drivers/rtc/rtc-bd70528.c
+++ b/drivers/rtc/rtc-bd70528.c
@@ -52,6 +52,7 @@ struct bd70528_rtc_alm {
struct bd70528_rtc {
struct rohm_regmap_dev *parent;
+ struct regmap *regmap;
struct device *dev;
u8 reg_time_start;
bool has_rtc_timers;
@@ -234,9 +235,8 @@ static int bd71828_set_alarm(struct device *dev, struct rtc_wkalrm *a)
int ret;
struct bd71828_rtc_alm alm;
struct bd70528_rtc *r = dev_get_drvdata(dev);
- struct rohm_regmap_dev *parent = r->parent;
- ret = regmap_bulk_read(parent->regmap, BD71828_REG_RTC_ALM_START,
+ ret = regmap_bulk_read(r->regmap, BD71828_REG_RTC_ALM_START,
&alm, sizeof(alm));
if (ret) {
dev_err(dev, "Failed to read alarm regs\n");
@@ -250,7 +250,7 @@ static int bd71828_set_alarm(struct device *dev, struct rtc_wkalrm *a)
else
alm.alm_mask |= BD70528_MASK_ALM_EN;
- ret = regmap_bulk_write(parent->regmap, BD71828_REG_RTC_ALM_START,
+ ret = regmap_bulk_write(r->regmap, BD71828_REG_RTC_ALM_START,
&alm, sizeof(alm));
if (ret)
dev_err(dev, "Failed to set alarm time\n");
@@ -265,17 +265,16 @@ static int bd70528_set_alarm(struct device *dev, struct rtc_wkalrm *a)
struct bd70528_rtc_alm alm;
int ret;
struct bd70528_rtc *r = dev_get_drvdata(dev);
- struct rohm_regmap_dev *parent = r->parent;
- ret = regmap_bulk_read(parent->regmap, BD70528_REG_RTC_WAKE_START,
- &wake, sizeof(wake));
+ ret = regmap_bulk_read(r->regmap, BD70528_REG_RTC_WAKE_START, &wake,
+ sizeof(wake));
if (ret) {
dev_err(dev, "Failed to read wake regs\n");
return ret;
}
- ret = regmap_bulk_read(parent->regmap, BD70528_REG_RTC_ALM_START,
- &alm, sizeof(alm));
+ ret = regmap_bulk_read(r->regmap, BD70528_REG_RTC_ALM_START, &alm,
+ sizeof(alm));
if (ret) {
dev_err(dev, "Failed to read alarm regs\n");
return ret;
@@ -292,15 +291,14 @@ static int bd70528_set_alarm(struct device *dev, struct rtc_wkalrm *a)
wake.ctrl &= ~BD70528_MASK_WAKE_EN;
}
- ret = regmap_bulk_write(parent->regmap,
- BD70528_REG_RTC_WAKE_START, &wake,
+ ret = regmap_bulk_write(r->regmap, BD70528_REG_RTC_WAKE_START, &wake,
sizeof(wake));
if (ret) {
dev_err(dev, "Failed to set wake time\n");
return ret;
}
- ret = regmap_bulk_write(parent->regmap, BD70528_REG_RTC_ALM_START,
- &alm, sizeof(alm));
+ ret = regmap_bulk_write(r->regmap, BD70528_REG_RTC_ALM_START, &alm,
+ sizeof(alm));
if (ret)
dev_err(dev, "Failed to set alarm time\n");
@@ -312,9 +310,8 @@ static int bd71828_read_alarm(struct device *dev, struct rtc_wkalrm *a)
int ret;
struct bd71828_rtc_alm alm;
struct bd70528_rtc *r = dev_get_drvdata(dev);
- struct rohm_regmap_dev *parent = r->parent;
- ret = regmap_bulk_read(parent->regmap, BD71828_REG_RTC_ALM_START,
+ ret = regmap_bulk_read(r->regmap, BD71828_REG_RTC_ALM_START,
&alm, sizeof(alm));
if (ret) {
dev_err(dev, "Failed to read alarm regs\n");
@@ -336,10 +333,9 @@ static int bd70528_read_alarm(struct device *dev, struct rtc_wkalrm *a)
struct bd70528_rtc_alm alm;
int ret;
struct bd70528_rtc *r = dev_get_drvdata(dev);
- struct rohm_regmap_dev *parent = r->parent;
- ret = regmap_bulk_read(parent->regmap, BD70528_REG_RTC_ALM_START,
- &alm, sizeof(alm));
+ ret = regmap_bulk_read(r->regmap, BD70528_REG_RTC_ALM_START, &alm,
+ sizeof(alm));
if (ret) {
dev_err(dev, "Failed to read alarm regs\n");
return ret;
@@ -360,14 +356,12 @@ static int bd70528_set_time_locked(struct device *dev, struct rtc_time *t)
int ret, tmpret, old_states;
struct bd70528_rtc_data rtc_data;
struct bd70528_rtc *r = dev_get_drvdata(dev);
- struct rohm_regmap_dev *parent = r->parent;
ret = bd70528_disable_rtc_based_timers(r, &old_states);
if (ret)
return ret;
- tmpret = regmap_bulk_read(parent->regmap,
- r->reg_time_start, &rtc_data,
+ tmpret = regmap_bulk_read(r->regmap, r->reg_time_start, &rtc_data,
sizeof(rtc_data));
if (tmpret) {
dev_err(dev, "Failed to read RTC time registers\n");
@@ -375,8 +369,7 @@ static int bd70528_set_time_locked(struct device *dev, struct rtc_time *t)
}
tm2rtc(t, &rtc_data);
- tmpret = regmap_bulk_write(parent->regmap,
- r->reg_time_start, &rtc_data,
+ tmpret = regmap_bulk_write(r->regmap, r->reg_time_start, &rtc_data,
sizeof(rtc_data));
if (tmpret) {
dev_err(dev, "Failed to set RTC time\n");
@@ -410,13 +403,11 @@ static int bd70528_set_time(struct device *dev, struct rtc_time *t)
static int bd70528_get_time(struct device *dev, struct rtc_time *t)
{
struct bd70528_rtc *r = dev_get_drvdata(dev);
- struct rohm_regmap_dev *parent = r->parent;
struct bd70528_rtc_data rtc_data;
int ret;
/* read the RTC date and time registers all at once */
- ret = regmap_bulk_read(parent->regmap,
- r->reg_time_start, &rtc_data,
+ ret = regmap_bulk_read(r->regmap, r->reg_time_start, &rtc_data,
sizeof(rtc_data));
if (ret) {
dev_err(dev, "Failed to read RTC time (err %d)\n", ret);
@@ -443,7 +434,7 @@ static int bd70528_alm_enable(struct device *dev, unsigned int enabled)
dev_err(dev, "Failed to change wake state\n");
goto out_unlock;
}
- ret = regmap_update_bits(r->parent->regmap, BD70528_REG_RTC_ALM_MASK,
+ ret = regmap_update_bits(r->regmap, BD70528_REG_RTC_ALM_MASK,
BD70528_MASK_ALM_EN, enableval);
if (ret)
dev_err(dev, "Failed to change alarm state\n");
@@ -462,7 +453,7 @@ static int bd71828_alm_enable(struct device *dev, unsigned int enabled)
if (!enabled)
enableval = 0;
- ret = regmap_update_bits(r->parent->regmap, BD71828_REG_RTC_ALM0_MASK,
+ ret = regmap_update_bits(r->regmap, BD71828_REG_RTC_ALM0_MASK,
BD70528_MASK_ALM_EN, enableval);
if (ret)
dev_err(dev, "Failed to change alarm state\n");
@@ -498,7 +489,6 @@ static int bd70528_probe(struct platform_device *pdev)
{
struct bd70528_rtc *bd_rtc;
const struct rtc_class_ops *rtc_ops;
- struct rohm_regmap_dev *parent;
const char *irq_name;
int ret;
struct rtc_device *rtc;
@@ -508,20 +498,25 @@ static int bd70528_probe(struct platform_device *pdev)
u8 hour_reg;
enum rohm_chip_type chip = platform_get_device_id(pdev)->driver_data;
- parent = dev_get_drvdata(pdev->dev.parent);
- if (!parent) {
- dev_err(&pdev->dev, "No MFD driver data\n");
- return -EINVAL;
- }
bd_rtc = devm_kzalloc(&pdev->dev, sizeof(*bd_rtc), GFP_KERNEL);
if (!bd_rtc)
return -ENOMEM;
- bd_rtc->parent = parent;
+ bd_rtc->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!bd_rtc->regmap) {
+ dev_err(&pdev->dev, "No regmap\n");
+ return -EINVAL;
+ }
+
bd_rtc->dev = &pdev->dev;
switch (chip) {
case ROHM_CHIP_TYPE_BD70528:
+ bd_rtc->parent = dev_get_drvdata(pdev->dev.parent);
+ if (!bd_rtc->parent) {
+ dev_err(&pdev->dev, "No MFD data\n");
+ return -EINVAL;
+ }
irq_name = "bd70528-rtc-alm";
bd_rtc->has_rtc_timers = true;
bd_rtc->reg_time_start = BD70528_REG_RTC_START;
@@ -547,7 +542,7 @@ static int bd70528_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, bd_rtc);
- ret = regmap_read(parent->regmap, hour_reg, &hr);
+ ret = regmap_read(bd_rtc->regmap, hour_reg, &hr);
if (ret) {
dev_err(&pdev->dev, "Failed to reag RTC clock\n");
@@ -595,7 +590,7 @@ static int bd70528_probe(struct platform_device *pdev)
* from sub-registers when IRQ is disabled or freed.
*/
if (enable_main_irq) {
- ret = regmap_update_bits(parent->regmap,
+ ret = regmap_update_bits(bd_rtc->regmap,
BD70528_REG_INT_MAIN_MASK,
BD70528_INT_RTC_MASK, 0);
if (ret) {
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 02/16] mfd: bd718x7: simplify by cleaning unnecessary device data
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
2021-03-24 7:22 ` [PATCH v4 01/16] rtc: bd70528: Do not require parent data Matti Vaittinen
@ 2021-03-24 7:22 ` Matti Vaittinen
2021-03-24 7:23 ` [PATCH v4 03/16] dt_bindings: bd71828: Add clock output mode Matti Vaittinen
` (12 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:22 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Matti Vaittinen, linux-kernel, linux-power
Most ROHM PMIC sub-devices only use the regmap pointer from
parent device. They can obtain this by dev_get_regamap so in
most cases the MFD device does not need to allocate and populate
the driver data. Simplify drivers by removing this.
The BD70528 still needs the access to watchdog mutex so keep
rohm_regmap_dev in use on BD70528 RTC and WDG drivers for now.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
No changes since v3
drivers/mfd/rohm-bd718x7.c | 43 ++++++++++++--------------------
include/linux/mfd/rohm-bd718x7.h | 13 ----------
2 files changed, 16 insertions(+), 40 deletions(-)
diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
index c32c1b6c98fa..bfd81f78beae 100644
--- a/drivers/mfd/rohm-bd718x7.c
+++ b/drivers/mfd/rohm-bd718x7.c
@@ -91,9 +91,9 @@ static const struct regmap_config bd718xx_regmap_config = {
.cache_type = REGCACHE_RBTREE,
};
-static int bd718xx_init_press_duration(struct bd718xx *bd718xx)
+static int bd718xx_init_press_duration(struct regmap *regmap,
+ struct device *dev)
{
- struct device* dev = bd718xx->chip.dev;
u32 short_press_ms, long_press_ms;
u32 short_press_value, long_press_value;
int ret;
@@ -102,8 +102,7 @@ static int bd718xx_init_press_duration(struct bd718xx *bd718xx)
&short_press_ms);
if (!ret) {
short_press_value = min(15u, (short_press_ms + 250) / 500);
- ret = regmap_update_bits(bd718xx->chip.regmap,
- BD718XX_REG_PWRONCONFIG0,
+ ret = regmap_update_bits(regmap, BD718XX_REG_PWRONCONFIG0,
BD718XX_PWRBTN_PRESS_DURATION_MASK,
short_press_value);
if (ret) {
@@ -116,8 +115,7 @@ static int bd718xx_init_press_duration(struct bd718xx *bd718xx)
&long_press_ms);
if (!ret) {
long_press_value = min(15u, (long_press_ms + 500) / 1000);
- ret = regmap_update_bits(bd718xx->chip.regmap,
- BD718XX_REG_PWRONCONFIG1,
+ ret = regmap_update_bits(regmap, BD718XX_REG_PWRONCONFIG1,
BD718XX_PWRBTN_PRESS_DURATION_MASK,
long_press_value);
if (ret) {
@@ -132,7 +130,8 @@ static int bd718xx_init_press_duration(struct bd718xx *bd718xx)
static int bd718xx_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *id)
{
- struct bd718xx *bd718xx;
+ struct regmap *regmap;
+ struct regmap_irq_chip_data *irq_data;
int ret;
unsigned int chip_type;
struct mfd_cell *mfd;
@@ -142,13 +141,6 @@ static int bd718xx_i2c_probe(struct i2c_client *i2c,
dev_err(&i2c->dev, "No IRQ configured\n");
return -EINVAL;
}
-
- bd718xx = devm_kzalloc(&i2c->dev, sizeof(struct bd718xx), GFP_KERNEL);
-
- if (!bd718xx)
- return -ENOMEM;
-
- bd718xx->chip_irq = i2c->irq;
chip_type = (unsigned int)(uintptr_t)
of_device_get_match_data(&i2c->dev);
switch (chip_type) {
@@ -164,29 +156,26 @@ static int bd718xx_i2c_probe(struct i2c_client *i2c,
dev_err(&i2c->dev, "Unknown device type");
return -EINVAL;
}
- bd718xx->chip.dev = &i2c->dev;
- dev_set_drvdata(&i2c->dev, bd718xx);
- bd718xx->chip.regmap = devm_regmap_init_i2c(i2c,
- &bd718xx_regmap_config);
- if (IS_ERR(bd718xx->chip.regmap)) {
+ regmap = devm_regmap_init_i2c(i2c, &bd718xx_regmap_config);
+ if (IS_ERR(regmap)) {
dev_err(&i2c->dev, "regmap initialization failed\n");
- return PTR_ERR(bd718xx->chip.regmap);
+ return PTR_ERR(regmap);
}
- ret = devm_regmap_add_irq_chip(&i2c->dev, bd718xx->chip.regmap,
- bd718xx->chip_irq, IRQF_ONESHOT, 0,
- &bd718xx_irq_chip, &bd718xx->irq_data);
+ ret = devm_regmap_add_irq_chip(&i2c->dev, regmap, i2c->irq,
+ IRQF_ONESHOT, 0, &bd718xx_irq_chip,
+ &irq_data);
if (ret) {
dev_err(&i2c->dev, "Failed to add irq_chip\n");
return ret;
}
- ret = bd718xx_init_press_duration(bd718xx);
+ ret = bd718xx_init_press_duration(regmap, &i2c->dev);
if (ret)
return ret;
- ret = regmap_irq_get_virq(bd718xx->irq_data, BD718XX_INT_PWRBTN_S);
+ ret = regmap_irq_get_virq(irq_data, BD718XX_INT_PWRBTN_S);
if (ret < 0) {
dev_err(&i2c->dev, "Failed to get the IRQ\n");
@@ -195,9 +184,9 @@ static int bd718xx_i2c_probe(struct i2c_client *i2c,
button.irq = ret;
- ret = devm_mfd_add_devices(bd718xx->chip.dev, PLATFORM_DEVID_AUTO,
+ ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO,
mfd, cells, NULL, 0,
- regmap_irq_get_domain(bd718xx->irq_data));
+ regmap_irq_get_domain(irq_data));
if (ret)
dev_err(&i2c->dev, "Failed to create subdevices\n");
diff --git a/include/linux/mfd/rohm-bd718x7.h b/include/linux/mfd/rohm-bd718x7.h
index bee2474a8f9f..df2918198d37 100644
--- a/include/linux/mfd/rohm-bd718x7.h
+++ b/include/linux/mfd/rohm-bd718x7.h
@@ -310,17 +310,4 @@ enum {
BD718XX_PWRBTN_LONG_PRESS_15S
};
-struct bd718xx {
- /*
- * Please keep this as the first member here as some
- * drivers (clk) supporting more than one chip may only know this
- * generic struct 'struct rohm_regmap_dev' and assume it is
- * the first chunk of parent device's private data.
- */
- struct rohm_regmap_dev chip;
-
- int chip_irq;
- struct regmap_irq_chip_data *irq_data;
-};
-
#endif /* __LINUX_MFD_BD718XX_H__ */
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 03/16] dt_bindings: bd71828: Add clock output mode
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
2021-03-24 7:22 ` [PATCH v4 01/16] rtc: bd70528: Do not require parent data Matti Vaittinen
2021-03-24 7:22 ` [PATCH v4 02/16] mfd: bd718x7: simplify by cleaning unnecessary device data Matti Vaittinen
@ 2021-03-24 7:23 ` Matti Vaittinen
2021-03-24 7:23 ` [PATCH v4 04/16] dt_bindings: regulator: Add ROHM BD71815 PMIC regulators Matti Vaittinen
` (11 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:23 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Matti Vaittinen, Michael Turquette,
Stephen Boyd, Linus Walleij, Bartosz Golaszewski, linux-kernel,
linux-power, linux-clk, linux-gpio
The BD71828 allows configuring the clk32kout pin mode to CMOS or
open-drain. Add device-tree property for specifying the preferred mode.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
No changes since v3
.../devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
index 3a6a1a26e2b3..8380166d176c 100644
--- a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
@@ -44,6 +44,12 @@ properties:
clock-output-names:
const: bd71828-32k-out
+ rohm,clkout-open-drain:
+ description: clk32kout mode. Set to 1 for "open-drain" or 0 for "cmos".
+ $ref: "/schemas/types.yaml#/definitions/uint32"
+ minimum: 0
+ maximum: 1
+
rohm,charger-sense-resistor-ohms:
minimum: 10000000
maximum: 50000000
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 04/16] dt_bindings: regulator: Add ROHM BD71815 PMIC regulators
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
` (2 preceding siblings ...)
2021-03-24 7:23 ` [PATCH v4 03/16] dt_bindings: bd71828: Add clock output mode Matti Vaittinen
@ 2021-03-24 7:23 ` Matti Vaittinen
2021-03-24 7:23 ` [PATCH v4 05/16] dt_bindings: mfd: Add ROHM BD71815 PMIC Matti Vaittinen
` (10 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:23 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Liam Girdwood, Mark Brown,
Matti Vaittinen, devicetree, linux-kernel, linux-power
Add binding documentation for regulators on ROHM BD71815 PMIC.
5 bucks, 7 LDOs and a boost for LED.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
No Changes since v3
.../regulator/rohm,bd71815-regulator.yaml | 116 ++++++++++++++++++
1 file changed, 116 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd71815-regulator.yaml
diff --git a/Documentation/devicetree/bindings/regulator/rohm,bd71815-regulator.yaml b/Documentation/devicetree/bindings/regulator/rohm,bd71815-regulator.yaml
new file mode 100644
index 000000000000..7d0adb74a396
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/rohm,bd71815-regulator.yaml
@@ -0,0 +1,116 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/rohm,bd71815-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD71815 Power Management Integrated Circuit regulators
+
+maintainers:
+ - Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
+
+description: |
+ This module is part of the ROHM BD718215 MFD device. For more details
+ see Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.yaml.
+
+ The regulator controller is represented as a sub-node of the PMIC node
+ on the device tree.
+
+ The valid names for BD71815 regulator nodes are
+ buck1, buck2, buck3, buck4, buck5,
+ ldo1, ldo2, ldo3, ldo4, ldo5,
+ ldodvref, ldolpsr, wled
+
+properties:
+ wled:
+ type: object
+ description:
+ properties for wled regulator
+ $ref: regulator.yaml#
+
+ properties:
+ regulator-name:
+ const: wled
+
+patternProperties:
+ "^((ldo|buck)[1-5]|ldolpsr|ldodvref)$":
+ type: object
+ description:
+ Properties for single LDO/BUCK regulator.
+ $ref: regulator.yaml#
+
+ properties:
+ regulator-name:
+ pattern: "^((ldo|buck)[1-5]|ldolpsr|ldodvref)$"
+ description:
+ should be "ldo1", ..., "ldo5", "buck1", ..., "buck5" and "ldolpsr"
+ for ldolpsr regulator, "ldodvref" for ldodvref reglator.
+
+ rohm,vsel-gpios:
+ description:
+ GPIO used to control ldo4 state (when ldo4 is controlled by GPIO).
+
+ rohm,dvs-run-voltage:
+ description:
+ PMIC "RUN" state voltage in uV when PMIC HW states are used. See
+ comments below for bucks/LDOs which support this. 0 means
+ regulator should be disabled at RUN state.
+ $ref: "/schemas/types.yaml#/definitions/uint32"
+ minimum: 0
+ maximum: 3300000
+
+ rohm,dvs-snvs-voltage:
+ description:
+ Whether to keep regulator enabled at "SNVS" state or not.
+ 0 means regulator should be disabled at SNVS state, non zero voltage
+ keeps regulator enabled. BD71815 does not change voltage level
+ when PMIC transitions to SNVS.SNVS voltage depends on the previous
+ state (from which the PMIC transitioned to SNVS).
+ $ref: "/schemas/types.yaml#/definitions/uint32"
+ minimum: 0
+ maximum: 3300000
+
+ rohm,dvs-suspend-voltage:
+ description:
+ PMIC "SUSPEND" state voltage in uV when PMIC HW states are used. See
+ comments below for bucks/LDOs which support this. 0 means
+ regulator should be disabled at SUSPEND state.
+ $ref: "/schemas/types.yaml#/definitions/uint32"
+ minimum: 0
+ maximum: 3300000
+
+ rohm,dvs-lpsr-voltage:
+ description:
+ PMIC "LPSR" state voltage in uV when PMIC HW states are used. See
+ comments below for bucks/LDOs which support this. 0 means
+ regulator should be disabled at LPSR state.
+ $ref: "/schemas/types.yaml#/definitions/uint32"
+ minimum: 0
+ maximum: 3300000
+
+ # Bucks 1 and 2 support giving separate voltages for operational states
+ # (RUN /CLEAN according to data-sheet) and non operational states
+ # (LPSR/SUSPEND). The voltage is automatically changed when HW
+ # state changes. Omitting these properties from bucks 1 and 2 leave
+ # buck voltages to not be toggled by HW state. Enable status may still
+ # be toggled by state changes depending on HW default settings.
+ #
+ # Bucks 3-5 and ldos 1-5 support setting the RUN state voltage here.
+ # Given RUN voltage is used at all states if regulator is enabled at
+ # given state.
+ # Values given for other states are regarded as enable/disable at
+ # given state (see below).
+ #
+ # All regulators except WLED support specifying enable/disable status
+ # for each of the HW states (RUN/SNVS/SUSPEND/LPSR). HW defaults can
+ # be overridden by setting voltage to 0 (regulator disabled at given
+ # state) or non-zero (regulator enabled at given state). Please note
+ # that setting non zero voltages for bucks 1/2 will also enable voltage
+ # changes according to state change.
+
+ required:
+ - regulator-name
+
+ unevaluatedProperties: false
+
+additionalProperties: false
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 05/16] dt_bindings: mfd: Add ROHM BD71815 PMIC
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
` (3 preceding siblings ...)
2021-03-24 7:23 ` [PATCH v4 04/16] dt_bindings: regulator: Add ROHM BD71815 PMIC regulators Matti Vaittinen
@ 2021-03-24 7:23 ` Matti Vaittinen
2021-03-24 7:25 ` [PATCH v4 06/16] mfd: Add ROHM BD71815 ID Matti Vaittinen
` (9 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:23 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Liam Girdwood, Mark Brown,
Matti Vaittinen, devicetree, linux-kernel, linux-power
Document DT bindings for ROHM BD71815.
BD71815 is a single-chip power management IC mainly for battery-powered
portable devices. The IC integrates 5 bucks, 7 LDOs, a boost driver for
LED, a battery charger with a Coulomb counter, a real-time clock, a 32kHz
clock and two general-purpose outputs although only one is documented by
the data-sheet.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
No changes since v3
.../bindings/mfd/rohm,bd71815-pmic.yaml | 201 ++++++++++++++++++
1 file changed, 201 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.yaml
diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.yaml
new file mode 100644
index 000000000000..fe265bcab50d
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd71815-pmic.yaml
@@ -0,0 +1,201 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/rohm,bd71815-pmic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD71815 Power Management Integrated Circuit bindings
+
+maintainers:
+ - Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
+
+description: |
+ BD71815AGW is a single-chip power management ICs for battery-powered
+ portable devices. It integrates 5 buck converters, 8 LDOs, a boost driver
+ for LED and a 500 mA single-cell linear charger. Also included is a Coulomb
+ counter, a real-time clock (RTC), and a 32.768 kHz clock gate and two GPOs.
+
+properties:
+ compatible:
+ const: rohm,bd71815
+
+ reg:
+ description:
+ I2C slave address.
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ gpio-controller: true
+
+ "#gpio-cells":
+ const: 2
+ description: |
+ The first cell is the pin number and the second cell is used to specify
+ flags. See ../gpio/gpio.txt for more information.
+
+ clocks:
+ maxItems: 1
+
+ "#clock-cells":
+ const: 0
+
+ clock-output-names:
+ const: bd71815-32k-out
+
+ rohm,clkout-open-drain:
+ description: clk32kout mode. Set to 1 for "open-drain" or 0 for "cmos".
+ $ref: "/schemas/types.yaml#/definitions/uint32"
+ minimum: 0
+ maximum: 1
+
+ rohm,charger-sense-resistor-ohms:
+ minimum: 10000000
+ maximum: 50000000
+ description: |
+ BD71827 and BD71828 have SAR ADC for measuring charging currents.
+ External sense resistor (RSENSE in data sheet) should be used. If
+ something other but 30MOhm resistor is used the resistance value
+ should be given here in Ohms.
+ default: 30000000
+
+ regulators:
+ $ref: ../regulator/rohm,bd71815-regulator.yaml
+ description:
+ List of child nodes that specify the regulators.
+
+ gpio-reserved-ranges:
+ description: |
+ Usage of BD71828 GPIO pins can be changed via OTP. This property can be
+ used to mark the pins which should not be configured for GPIO. Please see
+ the ../gpio/gpio.txt for more information.
+
+ rohm,enable-hidden-gpo:
+ description: |
+ The BD71815 has undocumented GPO at pin E5. Pin is marked as GND at the
+ data-sheet as it's location in the middle of GND pins makes it hard to
+ use on PCB. If your board has managed to use this pin you can enable the
+ second GPO by defining this property. Dont enable this if you are unsure
+ about how the E5 pin is connected on your board.
+ type: boolean
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - "#clock-cells"
+ - regulators
+ - gpio-controller
+ - "#gpio-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/leds/common.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ pmic: pmic@4b {
+ compatible = "rohm,bd71815";
+ reg = <0x4b>;
+
+ interrupt-parent = <&gpio1>;
+ interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
+
+ clocks = <&osc 0>;
+ #clock-cells = <0>;
+ clock-output-names = "bd71815-32k-out";
+
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ rohm,charger-sense-resistor-ohms = <10000000>;
+
+ regulators {
+ buck1: buck1 {
+ regulator-name = "buck1";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <2000000>;
+ regulator-always-on;
+ regulator-ramp-delay = <1250>;
+ rohm,dvs-run-voltage = <1150000>;
+ rohm,dvs-suspend-voltage = <950000>;
+ };
+ buck2: buck2 {
+ regulator-name = "buck2";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <2000000>;
+ regulator-always-on;
+ regulator-ramp-delay = <1250>;
+ rohm,dvs-run-voltage = <1150000>;
+ rohm,dvs-suspend-voltage = <950000>;
+ };
+ buck3: buck3 {
+ regulator-name = "buck3";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <2700000>;
+ regulator-always-on;
+ };
+ buck4: buck4 {
+ regulator-name = "buck4";
+ regulator-min-microvolt = <1100000>;
+ regulator-max-microvolt = <1850000>;
+ regulator-always-on;
+ };
+ buck5: buck5 {
+ regulator-name = "buck5";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
+ ldo1: ldo1 {
+ regulator-name = "ldo1";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
+ ldo2: ldo2 {
+ regulator-name = "ldo2";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
+ ldo3: ldo3 {
+ regulator-name = "ldo3";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
+ ldo4: ldo4 {
+ regulator-name = "ldo4";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
+ ldo5: ldo5 {
+ regulator-name = "ldo5";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
+ ldo6: ldodvref {
+ regulator-name = "ldodvref";
+ regulator-always-on;
+ };
+ ldo7: ldolpsr {
+ regulator-name = "ldolpsr";
+ regulator-always-on;
+ };
+
+ boost: wled {
+ regulator-name = "wled";
+ regulator-min-microamp = <10>;
+ regulator-max-microamp = <25000>;
+ };
+ };
+ };
+ };
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 06/16] mfd: Add ROHM BD71815 ID
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
` (4 preceding siblings ...)
2021-03-24 7:23 ` [PATCH v4 05/16] dt_bindings: mfd: Add ROHM BD71815 PMIC Matti Vaittinen
@ 2021-03-24 7:25 ` Matti Vaittinen
2021-03-24 7:26 ` [PATCH v4 07/16] mfd: Sort ROHM chip ID list for better readability Matti Vaittinen
` (8 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:25 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, matti.vaittinen, linux-kernel, linux-power, linux-clk,
linux-gpio, linux-rtc
Add chip ID for ROHM BD71815 and PMIC so that drivers can identify
this IC.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
Changes since v3:
- ID added to middle of the list and not to the bottom
Please note, this patch is likely to cause (trivial) conflict with
the BD9576/BD9573 series.
include/linux/mfd/rohm-generic.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/mfd/rohm-generic.h b/include/linux/mfd/rohm-generic.h
index 66f673c35303..e107b4769101 100644
--- a/include/linux/mfd/rohm-generic.h
+++ b/include/linux/mfd/rohm-generic.h
@@ -11,6 +11,7 @@ enum rohm_chip_type {
ROHM_CHIP_TYPE_BD71837 = 0,
ROHM_CHIP_TYPE_BD71847,
ROHM_CHIP_TYPE_BD70528,
+ ROHM_CHIP_TYPE_BD71815,
ROHM_CHIP_TYPE_BD71828,
ROHM_CHIP_TYPE_BD9571,
ROHM_CHIP_TYPE_BD9574,
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 07/16] mfd: Sort ROHM chip ID list for better readability
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
` (5 preceding siblings ...)
2021-03-24 7:25 ` [PATCH v4 06/16] mfd: Add ROHM BD71815 ID Matti Vaittinen
@ 2021-03-24 7:26 ` Matti Vaittinen
2021-03-24 7:26 ` [PATCH v4 08/16] mfd: Support for ROHM BD71815 PMIC core Matti Vaittinen
` (7 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:26 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, matti.vaittinen, linux-kernel, linux-power, linux-clk,
linux-gpio, linux-rtc
Sort the ID list so it is easier to see which ICs are present.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Suggested-by: Lee Jones <lee.jones@linaro.org>
---
Chnages since v3:
- New patch
Please note, this patch is likely to cause (trivial) conflict with
the BD9576/BD9573 series.
include/linux/mfd/rohm-generic.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/mfd/rohm-generic.h b/include/linux/mfd/rohm-generic.h
index e107b4769101..9e2880e06950 100644
--- a/include/linux/mfd/rohm-generic.h
+++ b/include/linux/mfd/rohm-generic.h
@@ -8,13 +8,13 @@
#include <linux/regulator/driver.h>
enum rohm_chip_type {
- ROHM_CHIP_TYPE_BD71837 = 0,
- ROHM_CHIP_TYPE_BD71847,
+ ROHM_CHIP_TYPE_BD9571,
+ ROHM_CHIP_TYPE_BD9574,
ROHM_CHIP_TYPE_BD70528,
ROHM_CHIP_TYPE_BD71815,
ROHM_CHIP_TYPE_BD71828,
- ROHM_CHIP_TYPE_BD9571,
- ROHM_CHIP_TYPE_BD9574,
+ ROHM_CHIP_TYPE_BD71837,
+ ROHM_CHIP_TYPE_BD71847,
ROHM_CHIP_TYPE_AMOUNT
};
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 08/16] mfd: Support for ROHM BD71815 PMIC core
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
` (6 preceding siblings ...)
2021-03-24 7:26 ` [PATCH v4 07/16] mfd: Sort ROHM chip ID list for better readability Matti Vaittinen
@ 2021-03-24 7:26 ` Matti Vaittinen
2021-03-24 7:29 ` [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs Matti Vaittinen
` (6 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:26 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Liam Girdwood, Mark Brown,
Matti Vaittinen, Michael Turquette, Stephen Boyd, Linus Walleij,
Bartosz Golaszewski, Alessandro Zummo, Alexandre Belloni,
devicetree, linux-kernel, linux-power, linux-clk, linux-gpio,
linux-rtc
Add core support for ROHM BD71815 Power Management IC.
The IC integrates regulators, a battery charger with a coulomb counter,
a real-time clock (RTC), clock gate and general-purpose outputs (GPO).
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
Changes since v3:
- removed extra newline
drivers/mfd/Kconfig | 15 +-
drivers/mfd/rohm-bd71828.c | 486 +++++++++++++++++++-------
include/linux/mfd/rohm-bd71815.h | 562 +++++++++++++++++++++++++++++++
include/linux/mfd/rohm-bd71828.h | 3 +
4 files changed, 933 insertions(+), 133 deletions(-)
create mode 100644 include/linux/mfd/rohm-bd71815.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index b74efa469e90..60d9ae559f0a 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1975,19 +1975,20 @@ config MFD_ROHM_BD70528
charger.
config MFD_ROHM_BD71828
- tristate "ROHM BD71828 Power Management IC"
+ tristate "ROHM BD71828 and BD71815 Power Management IC"
depends on I2C=y
depends on OF
select REGMAP_I2C
select REGMAP_IRQ
select MFD_CORE
help
- Select this option to get support for the ROHM BD71828 Power
- Management IC. BD71828GW is a single-chip power management IC for
- battery-powered portable devices. The IC integrates 7 buck
- converters, 7 LDOs, and a 1500 mA single-cell linear charger.
- Also included is a Coulomb counter, a real-time clock (RTC), and
- a 32.768 kHz clock gate.
+ Select this option to get support for the ROHM BD71828 and BD71815
+ Power Management ICs. BD71828GW and BD71815AGW are single-chip power
+ management ICs mainly for battery-powered portable devices.
+ The BD71828 integrates 7 buck converters and 7 LDOs. The BD71815
+ has 5 bucks, 7 LDOs, and a boost for driving LEDs. Both ICs provide
+ also a single-cell linear charger, a Coulomb counter, a real-time
+ clock (RTC), GPIOs and a 32.768 kHz clock gate.
config MFD_STM32_LPTIMER
tristate "Support for STM32 Low-Power Timer"
diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
index 210261d026f2..714d9fcbf07b 100644
--- a/drivers/mfd/rohm-bd71828.c
+++ b/drivers/mfd/rohm-bd71828.c
@@ -2,7 +2,7 @@
//
// Copyright (C) 2019 ROHM Semiconductors
//
-// ROHM BD71828 PMIC driver
+// ROHM BD71828/BD71815 PMIC driver
#include <linux/gpio_keys.h>
#include <linux/i2c.h>
@@ -11,7 +11,9 @@
#include <linux/ioport.h>
#include <linux/irq.h>
#include <linux/mfd/core.h>
+#include <linux/mfd/rohm-bd71815.h>
#include <linux/mfd/rohm-bd71828.h>
+#include <linux/mfd/rohm-generic.h>
#include <linux/module.h>
#include <linux/of_device.h>
#include <linux/regmap.h>
@@ -29,12 +31,84 @@ static struct gpio_keys_platform_data bd71828_powerkey_data = {
.name = "bd71828-pwrkey",
};
-static const struct resource rtc_irqs[] = {
+static const struct resource bd71815_rtc_irqs[] = {
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC0, "bd71815-rtc-alm-0"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC1, "bd71815-rtc-alm-1"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC2, "bd71815-rtc-alm-2"),
+};
+
+static const struct resource bd71828_rtc_irqs[] = {
DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC0, "bd71828-rtc-alm-0"),
DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC1, "bd71828-rtc-alm-1"),
DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC2, "bd71828-rtc-alm-2"),
};
+static struct resource bd71815_power_irqs[] = {
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_RMV, "bd71815-dcin-rmv"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_OUT, "bd71815-clps-out"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_IN, "bd71815-clps-in"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_OVP_RES, "bd71815-dcin-ovp-res"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_OVP_DET, "bd71815-dcin-ovp-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_MON_RES, "bd71815-dcin-mon-res"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_MON_DET, "bd71815-dcin-mon-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_UV_RES, "bd71815-vsys-uv-res"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_UV_DET, "bd71815-vsys-uv-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_LOW_RES, "bd71815-vsys-low-res"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_LOW_DET, "bd71815-vsys-low-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_RES, "bd71815-vsys-mon-res"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_RES, "bd71815-vsys-mon-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_WDG_TEMP, "bd71815-chg-wdg-temp"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_WDG_TIME, "bd71815-chg-wdg"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RECHARGE_RES, "bd71815-rechg-res"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RECHARGE_DET, "bd71815-rechg-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RANGED_TEMP_TRANSITION, "bd71815-ranged-temp-transit"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_STATE_TRANSITION, "bd71815-chg-state-change"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_TEMP_NORMAL, "bd71815-bat-temp-normal"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_TEMP_ERANGE, "bd71815-bat-temp-erange"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_REMOVED, "bd71815-bat-rmv"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_DETECTED, "bd71815-bat-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_THERM_REMOVED, "bd71815-therm-rmv"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_THERM_DETECTED, "bd71815-therm-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_DEAD, "bd71815-bat-dead"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_SHORTC_RES, "bd71815-bat-short-res"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_SHORTC_DET, "bd71815-bat-short-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_LOW_VOLT_RES, "bd71815-bat-low-res"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_LOW_VOLT_DET, "bd71815-bat-low-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_VOLT_RES, "bd71815-bat-over-res"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_VOLT_DET, "bd71815-bat-over-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_MON_RES, "bd71815-bat-mon-res"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_MON_DET, "bd71815-bat-mon-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_CC_MON1, "bd71815-bat-cc-mon1"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_CC_MON2, "bd71815-bat-cc-mon2"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_CC_MON3, "bd71815-bat-cc-mon3"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_1_RES, "bd71815-bat-oc1-res"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_1_DET, "bd71815-bat-oc1-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_2_RES, "bd71815-bat-oc2-res"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_2_DET, "bd71815-bat-oc2-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_3_RES, "bd71815-bat-oc3-res"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_3_DET, "bd71815-bat-oc3-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_LOW_RES, "bd71815-bat-low-res"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_LOW_DET, "bd71815-bat-low-det"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_HI_RES, "bd71815-bat-hi-res"),
+ DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_HI_DET, "bd71815-bat-hi-det"),
+};
+
+static struct mfd_cell bd71815_mfd_cells[] = {
+ { .name = "bd71815-pmic", },
+ { .name = "bd71815-clk", },
+ { .name = "bd71815-gpo", },
+ {
+ .name = "bd71815-power",
+ .num_resources = ARRAY_SIZE(bd71815_power_irqs),
+ .resources = &bd71815_power_irqs[0],
+ },
+ {
+ .name = "bd71815-rtc",
+ .num_resources = ARRAY_SIZE(bd71815_rtc_irqs),
+ .resources = &bd71815_rtc_irqs[0],
+ },
+};
+
static struct mfd_cell bd71828_mfd_cells[] = {
{ .name = "bd71828-pmic", },
{ .name = "bd71828-gpio", },
@@ -47,8 +121,8 @@ static struct mfd_cell bd71828_mfd_cells[] = {
{ .name = "bd71827-power", },
{
.name = "bd71828-rtc",
- .resources = rtc_irqs,
- .num_resources = ARRAY_SIZE(rtc_irqs),
+ .resources = bd71828_rtc_irqs,
+ .num_resources = ARRAY_SIZE(bd71828_rtc_irqs),
}, {
.name = "gpio-keys",
.platform_data = &bd71828_powerkey_data,
@@ -56,7 +130,35 @@ static struct mfd_cell bd71828_mfd_cells[] = {
},
};
-static const struct regmap_range volatile_ranges[] = {
+static const struct regmap_range bd71815_volatile_ranges[] = {
+ {
+ .range_min = BD71815_REG_SEC,
+ .range_max = BD71815_REG_YEAR,
+ }, {
+ .range_min = BD71815_REG_CONF,
+ .range_max = BD71815_REG_BAT_TEMP,
+ }, {
+ .range_min = BD71815_REG_VM_IBAT_U,
+ .range_max = BD71815_REG_CC_CTRL,
+ }, {
+ .range_min = BD71815_REG_CC_STAT,
+ .range_max = BD71815_REG_CC_CURCD_L,
+ }, {
+ .range_min = BD71815_REG_VM_BTMP_MON,
+ .range_max = BD71815_REG_VM_BTMP_MON,
+ }, {
+ .range_min = BD71815_REG_INT_STAT,
+ .range_max = BD71815_REG_INT_UPDATE,
+ }, {
+ .range_min = BD71815_REG_VM_VSYS_U,
+ .range_max = BD71815_REG_REX_CTRL_1,
+ }, {
+ .range_min = BD71815_REG_FULL_CCNTD_3,
+ .range_max = BD71815_REG_CCNTD_CHG_2,
+ },
+};
+
+static const struct regmap_range bd71828_volatile_ranges[] = {
{
.range_min = BD71828_REG_PS_CTRL_1,
.range_max = BD71828_REG_PS_CTRL_1,
@@ -80,15 +182,28 @@ static const struct regmap_range volatile_ranges[] = {
},
};
-static const struct regmap_access_table volatile_regs = {
- .yes_ranges = &volatile_ranges[0],
- .n_yes_ranges = ARRAY_SIZE(volatile_ranges),
+static const struct regmap_access_table bd71815_volatile_regs = {
+ .yes_ranges = &bd71815_volatile_ranges[0],
+ .n_yes_ranges = ARRAY_SIZE(bd71815_volatile_ranges),
+};
+
+static const struct regmap_access_table bd71828_volatile_regs = {
+ .yes_ranges = &bd71828_volatile_ranges[0],
+ .n_yes_ranges = ARRAY_SIZE(bd71828_volatile_ranges),
+};
+
+static const struct regmap_config bd71815_regmap = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .volatile_table = &bd71815_volatile_regs,
+ .max_register = BD71815_MAX_REGISTER - 1,
+ .cache_type = REGCACHE_RBTREE,
};
-static struct regmap_config bd71828_regmap = {
+static const struct regmap_config bd71828_regmap = {
.reg_bits = 8,
.val_bits = 8,
- .volatile_table = &volatile_regs,
+ .volatile_table = &bd71828_volatile_regs,
.max_register = BD71828_MAX_REGISTER,
.cache_type = REGCACHE_RBTREE,
};
@@ -96,7 +211,7 @@ static struct regmap_config bd71828_regmap = {
/*
* Mapping of main IRQ register bits to sub-IRQ register offsets so that we can
* access corect sub-IRQ registers based on bits that are set in main IRQ
- * register.
+ * register. BD71815 and BD71828 have same sub-register-block offests.
*/
static unsigned int bit0_offsets[] = {11}; /* RTC IRQ */
@@ -108,7 +223,7 @@ static unsigned int bit5_offsets[] = {3}; /* VSYS IRQ */
static unsigned int bit6_offsets[] = {1, 2}; /* DCIN IRQ */
static unsigned int bit7_offsets[] = {0}; /* BUCK IRQ */
-static struct regmap_irq_sub_irq_map bd71828_sub_irq_offsets[] = {
+static struct regmap_irq_sub_irq_map bd718xx_sub_irq_offsets[] = {
REGMAP_IRQ_MAIN_REG_OFFSET(bit0_offsets),
REGMAP_IRQ_MAIN_REG_OFFSET(bit1_offsets),
REGMAP_IRQ_MAIN_REG_OFFSET(bit2_offsets),
@@ -119,6 +234,88 @@ static struct regmap_irq_sub_irq_map bd71828_sub_irq_offsets[] = {
REGMAP_IRQ_MAIN_REG_OFFSET(bit7_offsets),
};
+static const struct regmap_irq bd71815_irqs[] = {
+ REGMAP_IRQ_REG(BD71815_INT_BUCK1_OCP, 0, BD71815_INT_BUCK1_OCP_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BUCK2_OCP, 0, BD71815_INT_BUCK2_OCP_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BUCK3_OCP, 0, BD71815_INT_BUCK3_OCP_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BUCK4_OCP, 0, BD71815_INT_BUCK4_OCP_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BUCK5_OCP, 0, BD71815_INT_BUCK5_OCP_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_LED_OVP, 0, BD71815_INT_LED_OVP_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_LED_OCP, 0, BD71815_INT_LED_OCP_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_LED_SCP, 0, BD71815_INT_LED_SCP_MASK),
+ /* DCIN1 interrupts */
+ REGMAP_IRQ_REG(BD71815_INT_DCIN_RMV, 1, BD71815_INT_DCIN_RMV_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_CLPS_OUT, 1, BD71815_INT_CLPS_OUT_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_CLPS_IN, 1, BD71815_INT_CLPS_IN_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_DCIN_OVP_RES, 1, BD71815_INT_DCIN_OVP_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_DCIN_OVP_DET, 1, BD71815_INT_DCIN_OVP_DET_MASK),
+ /* DCIN2 interrupts */
+ REGMAP_IRQ_REG(BD71815_INT_DCIN_MON_RES, 2, BD71815_INT_DCIN_MON_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_DCIN_MON_DET, 2, BD71815_INT_DCIN_MON_DET_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_WDOG, 2, BD71815_INT_WDOG_MASK),
+ /* Vsys */
+ REGMAP_IRQ_REG(BD71815_INT_VSYS_UV_RES, 3, BD71815_INT_VSYS_UV_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_VSYS_UV_DET, 3, BD71815_INT_VSYS_UV_DET_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_VSYS_LOW_RES, 3, BD71815_INT_VSYS_LOW_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_VSYS_LOW_DET, 3, BD71815_INT_VSYS_LOW_DET_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_VSYS_MON_RES, 3, BD71815_INT_VSYS_MON_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_VSYS_MON_DET, 3, BD71815_INT_VSYS_MON_DET_MASK),
+ /* Charger */
+ REGMAP_IRQ_REG(BD71815_INT_CHG_WDG_TEMP, 4, BD71815_INT_CHG_WDG_TEMP_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_CHG_WDG_TIME, 4, BD71815_INT_CHG_WDG_TIME_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_CHG_RECHARGE_RES, 4, BD71815_INT_CHG_RECHARGE_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_CHG_RECHARGE_DET, 4, BD71815_INT_CHG_RECHARGE_DET_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_CHG_RANGED_TEMP_TRANSITION, 4,
+ BD71815_INT_CHG_RANGED_TEMP_TRANSITION_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_CHG_STATE_TRANSITION, 4, BD71815_INT_CHG_STATE_TRANSITION_MASK),
+ /* Battery */
+ REGMAP_IRQ_REG(BD71815_INT_BAT_TEMP_NORMAL, 5, BD71815_INT_BAT_TEMP_NORMAL_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_TEMP_ERANGE, 5, BD71815_INT_BAT_TEMP_ERANGE_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_REMOVED, 5, BD71815_INT_BAT_REMOVED_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_DETECTED, 5, BD71815_INT_BAT_DETECTED_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_THERM_REMOVED, 5, BD71815_INT_THERM_REMOVED_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_THERM_DETECTED, 5, BD71815_INT_THERM_DETECTED_MASK),
+ /* Battery Mon 1 */
+ REGMAP_IRQ_REG(BD71815_INT_BAT_DEAD, 6, BD71815_INT_BAT_DEAD_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_SHORTC_RES, 6, BD71815_INT_BAT_SHORTC_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_SHORTC_DET, 6, BD71815_INT_BAT_SHORTC_DET_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_LOW_VOLT_RES, 6, BD71815_INT_BAT_LOW_VOLT_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_LOW_VOLT_DET, 6, BD71815_INT_BAT_LOW_VOLT_DET_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_OVER_VOLT_RES, 6, BD71815_INT_BAT_OVER_VOLT_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_OVER_VOLT_DET, 6, BD71815_INT_BAT_OVER_VOLT_DET_MASK),
+ /* Battery Mon 2 */
+ REGMAP_IRQ_REG(BD71815_INT_BAT_MON_RES, 7, BD71815_INT_BAT_MON_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_MON_DET, 7, BD71815_INT_BAT_MON_DET_MASK),
+ /* Battery Mon 3 (Coulomb counter) */
+ REGMAP_IRQ_REG(BD71815_INT_BAT_CC_MON1, 8, BD71815_INT_BAT_CC_MON1_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_CC_MON2, 8, BD71815_INT_BAT_CC_MON2_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_CC_MON3, 8, BD71815_INT_BAT_CC_MON3_MASK),
+ /* Battery Mon 4 */
+ REGMAP_IRQ_REG(BD71815_INT_BAT_OVER_CURR_1_RES, 9, BD71815_INT_BAT_OVER_CURR_1_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_OVER_CURR_1_DET, 9, BD71815_INT_BAT_OVER_CURR_1_DET_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_OVER_CURR_2_RES, 9, BD71815_INT_BAT_OVER_CURR_2_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_OVER_CURR_2_DET, 9, BD71815_INT_BAT_OVER_CURR_2_DET_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_OVER_CURR_3_RES, 9, BD71815_INT_BAT_OVER_CURR_3_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_BAT_OVER_CURR_3_DET, 9, BD71815_INT_BAT_OVER_CURR_3_DET_MASK),
+ /* Temperature */
+ REGMAP_IRQ_REG(BD71815_INT_TEMP_BAT_LOW_RES, 10, BD71815_INT_TEMP_BAT_LOW_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_TEMP_BAT_LOW_DET, 10, BD71815_INT_TEMP_BAT_LOW_DET_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_TEMP_BAT_HI_RES, 10, BD71815_INT_TEMP_BAT_HI_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_TEMP_BAT_HI_DET, 10, BD71815_INT_TEMP_BAT_HI_DET_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_TEMP_CHIP_OVER_125_RES, 10,
+ BD71815_INT_TEMP_CHIP_OVER_125_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_TEMP_CHIP_OVER_125_DET, 10,
+ BD71815_INT_TEMP_CHIP_OVER_125_DET_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_TEMP_CHIP_OVER_VF_RES, 10,
+ BD71815_INT_TEMP_CHIP_OVER_VF_RES_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_TEMP_CHIP_OVER_VF_DET, 10,
+ BD71815_INT_TEMP_CHIP_OVER_VF_DET_MASK),
+ /* RTC Alarm */
+ REGMAP_IRQ_REG(BD71815_INT_RTC0, 11, BD71815_INT_RTC0_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_RTC1, 11, BD71815_INT_RTC1_MASK),
+ REGMAP_IRQ_REG(BD71815_INT_RTC2, 11, BD71815_INT_RTC2_MASK),
+};
+
static struct regmap_irq bd71828_irqs[] = {
REGMAP_IRQ_REG(BD71828_INT_BUCK1_OCP, 0, BD71828_INT_BUCK1_OCP_MASK),
REGMAP_IRQ_REG(BD71828_INT_BUCK2_OCP, 0, BD71828_INT_BUCK2_OCP_MASK),
@@ -134,10 +331,8 @@ static struct regmap_irq bd71828_irqs[] = {
REGMAP_IRQ_REG(BD71828_INT_CLPS_OUT, 1, BD71828_INT_CLPS_OUT_MASK),
REGMAP_IRQ_REG(BD71828_INT_CLPS_IN, 1, BD71828_INT_CLPS_IN_MASK),
/* DCIN2 interrupts */
- REGMAP_IRQ_REG(BD71828_INT_DCIN_MON_RES, 2,
- BD71828_INT_DCIN_MON_RES_MASK),
- REGMAP_IRQ_REG(BD71828_INT_DCIN_MON_DET, 2,
- BD71828_INT_DCIN_MON_DET_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_DCIN_MON_RES, 2, BD71828_INT_DCIN_MON_RES_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_DCIN_MON_DET, 2, BD71828_INT_DCIN_MON_DET_MASK),
REGMAP_IRQ_REG(BD71828_INT_LONGPUSH, 2, BD71828_INT_LONGPUSH_MASK),
REGMAP_IRQ_REG(BD71828_INT_MIDPUSH, 2, BD71828_INT_MIDPUSH_MASK),
REGMAP_IRQ_REG(BD71828_INT_SHORTPUSH, 2, BD71828_INT_SHORTPUSH_MASK),
@@ -145,102 +340,59 @@ static struct regmap_irq bd71828_irqs[] = {
REGMAP_IRQ_REG(BD71828_INT_WDOG, 2, BD71828_INT_WDOG_MASK),
REGMAP_IRQ_REG(BD71828_INT_SWRESET, 2, BD71828_INT_SWRESET_MASK),
/* Vsys */
- REGMAP_IRQ_REG(BD71828_INT_VSYS_UV_RES, 3,
- BD71828_INT_VSYS_UV_RES_MASK),
- REGMAP_IRQ_REG(BD71828_INT_VSYS_UV_DET, 3,
- BD71828_INT_VSYS_UV_DET_MASK),
- REGMAP_IRQ_REG(BD71828_INT_VSYS_LOW_RES, 3,
- BD71828_INT_VSYS_LOW_RES_MASK),
- REGMAP_IRQ_REG(BD71828_INT_VSYS_LOW_DET, 3,
- BD71828_INT_VSYS_LOW_DET_MASK),
- REGMAP_IRQ_REG(BD71828_INT_VSYS_HALL_IN, 3,
- BD71828_INT_VSYS_HALL_IN_MASK),
- REGMAP_IRQ_REG(BD71828_INT_VSYS_HALL_TOGGLE, 3,
- BD71828_INT_VSYS_HALL_TOGGLE_MASK),
- REGMAP_IRQ_REG(BD71828_INT_VSYS_MON_RES, 3,
- BD71828_INT_VSYS_MON_RES_MASK),
- REGMAP_IRQ_REG(BD71828_INT_VSYS_MON_DET, 3,
- BD71828_INT_VSYS_MON_DET_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_VSYS_UV_RES, 3, BD71828_INT_VSYS_UV_RES_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_VSYS_UV_DET, 3, BD71828_INT_VSYS_UV_DET_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_VSYS_LOW_RES, 3, BD71828_INT_VSYS_LOW_RES_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_VSYS_LOW_DET, 3, BD71828_INT_VSYS_LOW_DET_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_VSYS_HALL_IN, 3, BD71828_INT_VSYS_HALL_IN_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_VSYS_HALL_TOGGLE, 3, BD71828_INT_VSYS_HALL_TOGGLE_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_VSYS_MON_RES, 3, BD71828_INT_VSYS_MON_RES_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_VSYS_MON_DET, 3, BD71828_INT_VSYS_MON_DET_MASK),
/* Charger */
- REGMAP_IRQ_REG(BD71828_INT_CHG_DCIN_ILIM, 4,
- BD71828_INT_CHG_DCIN_ILIM_MASK),
- REGMAP_IRQ_REG(BD71828_INT_CHG_TOPOFF_TO_DONE, 4,
- BD71828_INT_CHG_TOPOFF_TO_DONE_MASK),
- REGMAP_IRQ_REG(BD71828_INT_CHG_WDG_TEMP, 4,
- BD71828_INT_CHG_WDG_TEMP_MASK),
- REGMAP_IRQ_REG(BD71828_INT_CHG_WDG_TIME, 4,
- BD71828_INT_CHG_WDG_TIME_MASK),
- REGMAP_IRQ_REG(BD71828_INT_CHG_RECHARGE_RES, 4,
- BD71828_INT_CHG_RECHARGE_RES_MASK),
- REGMAP_IRQ_REG(BD71828_INT_CHG_RECHARGE_DET, 4,
- BD71828_INT_CHG_RECHARGE_DET_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_CHG_DCIN_ILIM, 4, BD71828_INT_CHG_DCIN_ILIM_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_CHG_TOPOFF_TO_DONE, 4, BD71828_INT_CHG_TOPOFF_TO_DONE_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_CHG_WDG_TEMP, 4, BD71828_INT_CHG_WDG_TEMP_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_CHG_WDG_TIME, 4, BD71828_INT_CHG_WDG_TIME_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_CHG_RECHARGE_RES, 4, BD71828_INT_CHG_RECHARGE_RES_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_CHG_RECHARGE_DET, 4, BD71828_INT_CHG_RECHARGE_DET_MASK),
REGMAP_IRQ_REG(BD71828_INT_CHG_RANGED_TEMP_TRANSITION, 4,
BD71828_INT_CHG_RANGED_TEMP_TRANSITION_MASK),
- REGMAP_IRQ_REG(BD71828_INT_CHG_STATE_TRANSITION, 4,
- BD71828_INT_CHG_STATE_TRANSITION_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_CHG_STATE_TRANSITION, 4, BD71828_INT_CHG_STATE_TRANSITION_MASK),
/* Battery */
- REGMAP_IRQ_REG(BD71828_INT_BAT_TEMP_NORMAL, 5,
- BD71828_INT_BAT_TEMP_NORMAL_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_TEMP_ERANGE, 5,
- BD71828_INT_BAT_TEMP_ERANGE_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_TEMP_WARN, 5,
- BD71828_INT_BAT_TEMP_WARN_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_REMOVED, 5,
- BD71828_INT_BAT_REMOVED_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_DETECTED, 5,
- BD71828_INT_BAT_DETECTED_MASK),
- REGMAP_IRQ_REG(BD71828_INT_THERM_REMOVED, 5,
- BD71828_INT_THERM_REMOVED_MASK),
- REGMAP_IRQ_REG(BD71828_INT_THERM_DETECTED, 5,
- BD71828_INT_THERM_DETECTED_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_TEMP_NORMAL, 5, BD71828_INT_BAT_TEMP_NORMAL_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_TEMP_ERANGE, 5, BD71828_INT_BAT_TEMP_ERANGE_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_TEMP_WARN, 5, BD71828_INT_BAT_TEMP_WARN_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_REMOVED, 5, BD71828_INT_BAT_REMOVED_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_DETECTED, 5, BD71828_INT_BAT_DETECTED_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_THERM_REMOVED, 5, BD71828_INT_THERM_REMOVED_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_THERM_DETECTED, 5, BD71828_INT_THERM_DETECTED_MASK),
/* Battery Mon 1 */
REGMAP_IRQ_REG(BD71828_INT_BAT_DEAD, 6, BD71828_INT_BAT_DEAD_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_SHORTC_RES, 6,
- BD71828_INT_BAT_SHORTC_RES_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_SHORTC_DET, 6,
- BD71828_INT_BAT_SHORTC_DET_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_LOW_VOLT_RES, 6,
- BD71828_INT_BAT_LOW_VOLT_RES_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_LOW_VOLT_DET, 6,
- BD71828_INT_BAT_LOW_VOLT_DET_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_VOLT_RES, 6,
- BD71828_INT_BAT_OVER_VOLT_RES_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_VOLT_DET, 6,
- BD71828_INT_BAT_OVER_VOLT_DET_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_SHORTC_RES, 6, BD71828_INT_BAT_SHORTC_RES_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_SHORTC_DET, 6, BD71828_INT_BAT_SHORTC_DET_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_LOW_VOLT_RES, 6, BD71828_INT_BAT_LOW_VOLT_RES_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_LOW_VOLT_DET, 6, BD71828_INT_BAT_LOW_VOLT_DET_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_VOLT_RES, 6, BD71828_INT_BAT_OVER_VOLT_RES_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_VOLT_DET, 6, BD71828_INT_BAT_OVER_VOLT_DET_MASK),
/* Battery Mon 2 */
- REGMAP_IRQ_REG(BD71828_INT_BAT_MON_RES, 7,
- BD71828_INT_BAT_MON_RES_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_MON_DET, 7,
- BD71828_INT_BAT_MON_DET_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_MON_RES, 7, BD71828_INT_BAT_MON_RES_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_MON_DET, 7, BD71828_INT_BAT_MON_DET_MASK),
/* Battery Mon 3 (Coulomb counter) */
- REGMAP_IRQ_REG(BD71828_INT_BAT_CC_MON1, 8,
- BD71828_INT_BAT_CC_MON1_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_CC_MON2, 8,
- BD71828_INT_BAT_CC_MON2_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_CC_MON3, 8,
- BD71828_INT_BAT_CC_MON3_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_CC_MON1, 8, BD71828_INT_BAT_CC_MON1_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_CC_MON2, 8, BD71828_INT_BAT_CC_MON2_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_CC_MON3, 8, BD71828_INT_BAT_CC_MON3_MASK),
/* Battery Mon 4 */
- REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_1_RES, 9,
- BD71828_INT_BAT_OVER_CURR_1_RES_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_1_DET, 9,
- BD71828_INT_BAT_OVER_CURR_1_DET_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_2_RES, 9,
- BD71828_INT_BAT_OVER_CURR_2_RES_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_2_DET, 9,
- BD71828_INT_BAT_OVER_CURR_2_DET_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_3_RES, 9,
- BD71828_INT_BAT_OVER_CURR_3_RES_MASK),
- REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_3_DET, 9,
- BD71828_INT_BAT_OVER_CURR_3_DET_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_1_RES, 9, BD71828_INT_BAT_OVER_CURR_1_RES_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_1_DET, 9, BD71828_INT_BAT_OVER_CURR_1_DET_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_2_RES, 9, BD71828_INT_BAT_OVER_CURR_2_RES_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_2_DET, 9, BD71828_INT_BAT_OVER_CURR_2_DET_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_3_RES, 9, BD71828_INT_BAT_OVER_CURR_3_RES_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_3_DET, 9, BD71828_INT_BAT_OVER_CURR_3_DET_MASK),
/* Temperature */
- REGMAP_IRQ_REG(BD71828_INT_TEMP_BAT_LOW_RES, 10,
- BD71828_INT_TEMP_BAT_LOW_RES_MASK),
- REGMAP_IRQ_REG(BD71828_INT_TEMP_BAT_LOW_DET, 10,
- BD71828_INT_TEMP_BAT_LOW_DET_MASK),
- REGMAP_IRQ_REG(BD71828_INT_TEMP_BAT_HI_RES, 10,
- BD71828_INT_TEMP_BAT_HI_RES_MASK),
- REGMAP_IRQ_REG(BD71828_INT_TEMP_BAT_HI_DET, 10,
- BD71828_INT_TEMP_BAT_HI_DET_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_TEMP_BAT_LOW_RES, 10, BD71828_INT_TEMP_BAT_LOW_RES_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_TEMP_BAT_LOW_DET, 10, BD71828_INT_TEMP_BAT_LOW_DET_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_TEMP_BAT_HI_RES, 10, BD71828_INT_TEMP_BAT_HI_RES_MASK),
+ REGMAP_IRQ_REG(BD71828_INT_TEMP_BAT_HI_DET, 10, BD71828_INT_TEMP_BAT_HI_DET_MASK),
REGMAP_IRQ_REG(BD71828_INT_TEMP_CHIP_OVER_125_RES, 10,
BD71828_INT_TEMP_CHIP_OVER_125_RES_MASK),
REGMAP_IRQ_REG(BD71828_INT_TEMP_CHIP_OVER_125_DET, 10,
@@ -267,57 +419,133 @@ static struct regmap_irq_chip bd71828_irq_chip = {
.init_ack_masked = true,
.num_regs = 12,
.num_main_regs = 1,
- .sub_reg_offsets = &bd71828_sub_irq_offsets[0],
+ .sub_reg_offsets = &bd718xx_sub_irq_offsets[0],
+ .num_main_status_bits = 8,
+ .irq_reg_stride = 1,
+};
+
+static struct regmap_irq_chip bd71815_irq_chip = {
+ .name = "bd71815_irq",
+ .main_status = BD71815_REG_INT_STAT,
+ .irqs = &bd71815_irqs[0],
+ .num_irqs = ARRAY_SIZE(bd71815_irqs),
+ .status_base = BD71815_REG_INT_STAT_01,
+ .mask_base = BD71815_REG_INT_EN_01,
+ .ack_base = BD71815_REG_INT_STAT_01,
+ .mask_invert = true,
+ .init_ack_masked = true,
+ .num_regs = 12,
+ .num_main_regs = 1,
+ .sub_reg_offsets = &bd718xx_sub_irq_offsets[0],
.num_main_status_bits = 8,
.irq_reg_stride = 1,
};
+static int set_clk_mode(struct device *dev, struct regmap *regmap,
+ int clkmode_reg)
+{
+ int ret;
+ unsigned int open_drain;
+
+ ret = of_property_read_u32(dev->of_node, "rohm,clkout-open-drain", &open_drain);
+ if (ret) {
+ if (ret == -EINVAL)
+ return 0;
+ return ret;
+ }
+ if (open_drain > 1) {
+ dev_err(dev, "bad clk32kout mode configuration");
+ return -EINVAL;
+ }
+
+ if (open_drain)
+ return regmap_update_bits(regmap, clkmode_reg, OUT32K_MODE,
+ OUT32K_MODE_OPEN_DRAIN);
+
+ return regmap_update_bits(regmap, clkmode_reg, OUT32K_MODE,
+ OUT32K_MODE_CMOS);
+}
+
static int bd71828_i2c_probe(struct i2c_client *i2c)
{
- struct rohm_regmap_dev *chip;
struct regmap_irq_chip_data *irq_data;
int ret;
+ struct regmap *regmap;
+ const struct regmap_config *regmap_config;
+ struct regmap_irq_chip *irqchip;
+ unsigned int chip_type;
+ struct mfd_cell *mfd;
+ int cells;
+ int button_irq;
+ int clkmode_reg;
if (!i2c->irq) {
dev_err(&i2c->dev, "No IRQ configured\n");
return -EINVAL;
}
- chip = devm_kzalloc(&i2c->dev, sizeof(*chip), GFP_KERNEL);
- if (!chip)
- return -ENOMEM;
-
- dev_set_drvdata(&i2c->dev, chip);
+ chip_type = (unsigned int)(uintptr_t)
+ of_device_get_match_data(&i2c->dev);
+ switch (chip_type) {
+ case ROHM_CHIP_TYPE_BD71828:
+ mfd = bd71828_mfd_cells;
+ cells = ARRAY_SIZE(bd71828_mfd_cells);
+ regmap_config = &bd71828_regmap;
+ irqchip = &bd71828_irq_chip;
+ clkmode_reg = BD71828_REG_OUT32K;
+ button_irq = BD71828_INT_SHORTPUSH;
+ break;
+ case ROHM_CHIP_TYPE_BD71815:
+ mfd = bd71815_mfd_cells;
+ cells = ARRAY_SIZE(bd71815_mfd_cells);
+ regmap_config = &bd71815_regmap;
+ irqchip = &bd71815_irq_chip;
+ clkmode_reg = BD71815_REG_OUT32K;
+ /*
+ * If BD71817 support is needed we should be able to handle it
+ * with proper DT configs + BD71815 drivers + power-button.
+ * BD71815 data-sheet does not list the power-button IRQ so we
+ * don't use it.
+ */
+ button_irq = 0;
+ break;
+ default:
+ dev_err(&i2c->dev, "Unknown device type");
+ return -EINVAL;
+ }
- chip->regmap = devm_regmap_init_i2c(i2c, &bd71828_regmap);
- if (IS_ERR(chip->regmap)) {
+ regmap = devm_regmap_init_i2c(i2c, regmap_config);
+ if (IS_ERR(regmap)) {
dev_err(&i2c->dev, "Failed to initialize Regmap\n");
- return PTR_ERR(chip->regmap);
+ return PTR_ERR(regmap);
}
- ret = devm_regmap_add_irq_chip(&i2c->dev, chip->regmap,
- i2c->irq, IRQF_ONESHOT, 0,
- &bd71828_irq_chip, &irq_data);
+ ret = devm_regmap_add_irq_chip(&i2c->dev, regmap, i2c->irq,
+ IRQF_ONESHOT, 0, irqchip, &irq_data);
if (ret) {
dev_err(&i2c->dev, "Failed to add IRQ chip\n");
return ret;
}
dev_dbg(&i2c->dev, "Registered %d IRQs for chip\n",
- bd71828_irq_chip.num_irqs);
+ irqchip->num_irqs);
- ret = regmap_irq_get_virq(irq_data, BD71828_INT_SHORTPUSH);
- if (ret < 0) {
- dev_err(&i2c->dev, "Failed to get the power-key IRQ\n");
- return ret;
+ if (button_irq) {
+ ret = regmap_irq_get_virq(irq_data, button_irq);
+ if (ret < 0) {
+ dev_err(&i2c->dev, "Failed to get the power-key IRQ\n");
+ return ret;
+ }
+
+ button.irq = ret;
}
- button.irq = ret;
+ ret = set_clk_mode(&i2c->dev, regmap, clkmode_reg);
+ if (ret)
+ return ret;
- ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO,
- bd71828_mfd_cells,
- ARRAY_SIZE(bd71828_mfd_cells), NULL, 0,
- regmap_irq_get_domain(irq_data));
+ ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, mfd, cells,
+ NULL, 0, regmap_irq_get_domain(irq_data));
if (ret)
dev_err(&i2c->dev, "Failed to create subdevices\n");
@@ -325,7 +553,13 @@ static int bd71828_i2c_probe(struct i2c_client *i2c)
}
static const struct of_device_id bd71828_of_match[] = {
- { .compatible = "rohm,bd71828", },
+ {
+ .compatible = "rohm,bd71828",
+ .data = (void *)ROHM_CHIP_TYPE_BD71828,
+ }, {
+ .compatible = "rohm,bd71815",
+ .data = (void *)ROHM_CHIP_TYPE_BD71815,
+ },
{ },
};
MODULE_DEVICE_TABLE(of, bd71828_of_match);
diff --git a/include/linux/mfd/rohm-bd71815.h b/include/linux/mfd/rohm-bd71815.h
new file mode 100644
index 000000000000..ec6d9612bebe
--- /dev/null
+++ b/include/linux/mfd/rohm-bd71815.h
@@ -0,0 +1,562 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright 2021 ROHM Semiconductors.
+ *
+ * Author: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
+ *
+ * Copyright 2014 Embest Technology Co. Ltd. Inc.
+ *
+ * Author: yanglsh@embest-tech.com
+ */
+
+#ifndef _MFD_BD71815_H
+#define _MFD_BD71815_H
+
+#include <linux/regmap.h>
+
+enum {
+ BD71815_BUCK1 = 0,
+ BD71815_BUCK2,
+ BD71815_BUCK3,
+ BD71815_BUCK4,
+ BD71815_BUCK5,
+ /* General Purpose */
+ BD71815_LDO1,
+ BD71815_LDO2,
+ BD71815_LDO3,
+ /* LDOs for SD Card and SD Card Interface */
+ BD71815_LDO4,
+ BD71815_LDO5,
+ /* LDO for DDR Reference Voltage */
+ BD71815_LDODVREF,
+ /* LDO for Low-Power State Retention */
+ BD71815_LDOLPSR,
+ BD71815_WLED,
+ BD71815_REGULATOR_CNT,
+};
+
+#define BD71815_SUPPLY_STATE_ENABLED 0x1
+
+enum {
+ BD71815_REG_DEVICE = 0,
+ BD71815_REG_PWRCTRL,
+ BD71815_REG_BUCK1_MODE,
+ BD71815_REG_BUCK2_MODE,
+ BD71815_REG_BUCK3_MODE,
+ BD71815_REG_BUCK4_MODE,
+ BD71815_REG_BUCK5_MODE,
+ BD71815_REG_BUCK1_VOLT_H,
+ BD71815_REG_BUCK1_VOLT_L,
+ BD71815_REG_BUCK2_VOLT_H,
+ BD71815_REG_BUCK2_VOLT_L,
+ BD71815_REG_BUCK3_VOLT,
+ BD71815_REG_BUCK4_VOLT,
+ BD71815_REG_BUCK5_VOLT,
+ BD71815_REG_LED_CTRL,
+ BD71815_REG_LED_DIMM,
+ BD71815_REG_LDO_MODE1,
+ BD71815_REG_LDO_MODE2,
+ BD71815_REG_LDO_MODE3,
+ BD71815_REG_LDO_MODE4,
+ BD71815_REG_LDO1_VOLT,
+ BD71815_REG_LDO2_VOLT,
+ BD71815_REG_LDO3_VOLT,
+ BD71815_REG_LDO4_VOLT,
+ BD71815_REG_LDO5_VOLT_H,
+ BD71815_REG_LDO5_VOLT_L,
+ BD71815_REG_BUCK_PD_DIS,
+ BD71815_REG_LDO_PD_DIS,
+ BD71815_REG_GPO,
+ BD71815_REG_OUT32K,
+ BD71815_REG_SEC,
+ BD71815_REG_MIN,
+ BD71815_REG_HOUR,
+ BD71815_REG_WEEK,
+ BD71815_REG_DAY,
+ BD71815_REG_MONTH,
+ BD71815_REG_YEAR,
+ BD71815_REG_ALM0_SEC,
+
+ BD71815_REG_ALM1_SEC = 0x2C,
+
+ BD71815_REG_ALM0_MASK = 0x33,
+ BD71815_REG_ALM1_MASK,
+ BD71815_REG_ALM2,
+ BD71815_REG_TRIM,
+ BD71815_REG_CONF,
+ BD71815_REG_SYS_INIT,
+ BD71815_REG_CHG_STATE,
+ BD71815_REG_CHG_LAST_STATE,
+ BD71815_REG_BAT_STAT,
+ BD71815_REG_DCIN_STAT,
+ BD71815_REG_VSYS_STAT,
+ BD71815_REG_CHG_STAT,
+ BD71815_REG_CHG_WDT_STAT,
+ BD71815_REG_BAT_TEMP,
+ BD71815_REG_IGNORE_0,
+ BD71815_REG_INHIBIT_0,
+ BD71815_REG_DCIN_CLPS,
+ BD71815_REG_VSYS_REG,
+ BD71815_REG_VSYS_MAX,
+ BD71815_REG_VSYS_MIN,
+ BD71815_REG_CHG_SET1,
+ BD71815_REG_CHG_SET2,
+ BD71815_REG_CHG_WDT_PRE,
+ BD71815_REG_CHG_WDT_FST,
+ BD71815_REG_CHG_IPRE,
+ BD71815_REG_CHG_IFST,
+ BD71815_REG_CHG_IFST_TERM,
+ BD71815_REG_CHG_VPRE,
+ BD71815_REG_CHG_VBAT_1,
+ BD71815_REG_CHG_VBAT_2,
+ BD71815_REG_CHG_VBAT_3,
+ BD71815_REG_CHG_LED_1,
+ BD71815_REG_VF_TH,
+ BD71815_REG_BAT_SET_1,
+ BD71815_REG_BAT_SET_2,
+ BD71815_REG_BAT_SET_3,
+ BD71815_REG_ALM_VBAT_TH_U,
+ BD71815_REG_ALM_VBAT_TH_L,
+ BD71815_REG_ALM_DCIN_TH,
+ BD71815_REG_ALM_VSYS_TH,
+ BD71815_REG_VM_IBAT_U,
+ BD71815_REG_VM_IBAT_L,
+ BD71815_REG_VM_VBAT_U,
+ BD71815_REG_VM_VBAT_L,
+ BD71815_REG_VM_BTMP,
+ BD71815_REG_VM_VTH,
+ BD71815_REG_VM_DCIN_U,
+ BD71815_REG_VM_DCIN_L,
+ BD71815_REG_VM_VSYS,
+ BD71815_REG_VM_VF,
+ BD71815_REG_VM_OCI_PRE_U,
+ BD71815_REG_VM_OCI_PRE_L,
+ BD71815_REG_VM_OCV_PRE_U,
+ BD71815_REG_VM_OCV_PRE_L,
+ BD71815_REG_VM_OCI_PST_U,
+ BD71815_REG_VM_OCI_PST_L,
+ BD71815_REG_VM_OCV_PST_U,
+ BD71815_REG_VM_OCV_PST_L,
+ BD71815_REG_VM_SA_VBAT_U,
+ BD71815_REG_VM_SA_VBAT_L,
+ BD71815_REG_VM_SA_IBAT_U,
+ BD71815_REG_VM_SA_IBAT_L,
+ BD71815_REG_CC_CTRL,
+ BD71815_REG_CC_BATCAP1_TH_U,
+ BD71815_REG_CC_BATCAP1_TH_L,
+ BD71815_REG_CC_BATCAP2_TH_U,
+ BD71815_REG_CC_BATCAP2_TH_L,
+ BD71815_REG_CC_BATCAP3_TH_U,
+ BD71815_REG_CC_BATCAP3_TH_L,
+ BD71815_REG_CC_STAT,
+ BD71815_REG_CC_CCNTD_3,
+ BD71815_REG_CC_CCNTD_2,
+ BD71815_REG_CC_CCNTD_1,
+ BD71815_REG_CC_CCNTD_0,
+ BD71815_REG_CC_CURCD_U,
+ BD71815_REG_CC_CURCD_L,
+ BD71815_REG_VM_OCUR_THR_1,
+ BD71815_REG_VM_OCUR_DUR_1,
+ BD71815_REG_VM_OCUR_THR_2,
+ BD71815_REG_VM_OCUR_DUR_2,
+ BD71815_REG_VM_OCUR_THR_3,
+ BD71815_REG_VM_OCUR_DUR_3,
+ BD71815_REG_VM_OCUR_MON,
+ BD71815_REG_VM_BTMP_OV_THR,
+ BD71815_REG_VM_BTMP_OV_DUR,
+ BD71815_REG_VM_BTMP_LO_THR,
+ BD71815_REG_VM_BTMP_LO_DUR,
+ BD71815_REG_VM_BTMP_MON,
+ BD71815_REG_INT_EN_01,
+
+ BD71815_REG_INT_EN_11 = 0x95,
+ BD71815_REG_INT_EN_12,
+ BD71815_REG_INT_STAT,
+ BD71815_REG_INT_STAT_01,
+ BD71815_REG_INT_STAT_02,
+ BD71815_REG_INT_STAT_03,
+ BD71815_REG_INT_STAT_04,
+ BD71815_REG_INT_STAT_05,
+ BD71815_REG_INT_STAT_06,
+ BD71815_REG_INT_STAT_07,
+ BD71815_REG_INT_STAT_08,
+ BD71815_REG_INT_STAT_09,
+ BD71815_REG_INT_STAT_10,
+ BD71815_REG_INT_STAT_11,
+ BD71815_REG_INT_STAT_12,
+ BD71815_REG_INT_UPDATE,
+
+ BD71815_REG_VM_VSYS_U = 0xC0,
+ BD71815_REG_VM_VSYS_L,
+ BD71815_REG_VM_SA_VSYS_U,
+ BD71815_REG_VM_SA_VSYS_L,
+
+ BD71815_REG_VM_SA_IBAT_MIN_U = 0xD0,
+ BD71815_REG_VM_SA_IBAT_MIN_L,
+ BD71815_REG_VM_SA_IBAT_MAX_U,
+ BD71815_REG_VM_SA_IBAT_MAX_L,
+ BD71815_REG_VM_SA_VBAT_MIN_U,
+ BD71815_REG_VM_SA_VBAT_MIN_L,
+ BD71815_REG_VM_SA_VBAT_MAX_U,
+ BD71815_REG_VM_SA_VBAT_MAX_L,
+ BD71815_REG_VM_SA_VSYS_MIN_U,
+ BD71815_REG_VM_SA_VSYS_MIN_L,
+ BD71815_REG_VM_SA_VSYS_MAX_U,
+ BD71815_REG_VM_SA_VSYS_MAX_L,
+ BD71815_REG_VM_SA_MINMAX_CLR,
+
+ BD71815_REG_REX_CCNTD_3 = 0xE0,
+ BD71815_REG_REX_CCNTD_2,
+ BD71815_REG_REX_CCNTD_1,
+ BD71815_REG_REX_CCNTD_0,
+ BD71815_REG_REX_SA_VBAT_U,
+ BD71815_REG_REX_SA_VBAT_L,
+ BD71815_REG_REX_CTRL_1,
+ BD71815_REG_REX_CTRL_2,
+ BD71815_REG_FULL_CCNTD_3,
+ BD71815_REG_FULL_CCNTD_2,
+ BD71815_REG_FULL_CCNTD_1,
+ BD71815_REG_FULL_CCNTD_0,
+ BD71815_REG_FULL_CTRL,
+
+ BD71815_REG_CCNTD_CHG_3 = 0xF0,
+ BD71815_REG_CCNTD_CHG_2,
+
+ BD71815_REG_TEST_MODE = 0xFE,
+ BD71815_MAX_REGISTER,
+};
+
+/* BD71815_REG_BUCK1_MODE bits */
+#define BD71815_BUCK_RAMPRATE_MASK 0xC0
+#define BD71815_BUCK_RAMPRATE_10P00MV 0x0
+#define BD71815_BUCK_RAMPRATE_5P00MV 0x01
+#define BD71815_BUCK_RAMPRATE_2P50MV 0x02
+#define BD71815_BUCK_RAMPRATE_1P25MV 0x03
+
+#define BD71815_BUCK_PWM_FIXED BIT(4)
+#define BD71815_BUCK_SNVS_ON BIT(3)
+#define BD71815_BUCK_RUN_ON BIT(2)
+#define BD71815_BUCK_LPSR_ON BIT(1)
+#define BD71815_BUCK_SUSP_ON BIT(0)
+
+/* BD71815_REG_BUCK1_VOLT_H bits */
+#define BD71815_BUCK_DVSSEL BIT(7)
+#define BD71815_BUCK_STBY_DVS BIT(6)
+#define BD71815_VOLT_MASK 0x3F
+#define BD71815_BUCK1_H_DEFAULT 0x14
+#define BD71815_BUCK1_L_DEFAULT 0x14
+
+/* BD71815_REG_BUCK2_VOLT_H bits */
+#define BD71815_BUCK2_H_DEFAULT 0x14
+#define BD71815_BUCK2_L_DEFAULT 0x14
+
+/* WLED output */
+/* current register mask */
+#define LED_DIMM_MASK 0x3f
+/* LED enable bits at LED_CTRL reg */
+#define LED_CHGDONE_EN BIT(4)
+#define LED_RUN_ON BIT(2)
+#define LED_LPSR_ON BIT(1)
+#define LED_SUSP_ON BIT(0)
+
+/* BD71815_REG_LDO1_CTRL bits */
+#define LDO1_EN BIT(0)
+#define LDO2_EN BIT(1)
+#define LDO3_EN BIT(2)
+#define DVREF_EN BIT(3)
+#define VOSNVS_SW_EN BIT(4)
+
+/* LDO_MODE1_register */
+#define LDO1_SNVS_ON BIT(7)
+#define LDO1_RUN_ON BIT(6)
+#define LDO1_LPSR_ON BIT(5)
+#define LDO1_SUSP_ON BIT(4)
+/* set => register control, unset => GPIO control */
+#define LDO4_MODE_MASK BIT(3)
+#define LDO4_MODE_I2C BIT(3)
+#define LDO4_MODE_GPIO 0
+/* set => register control, unset => start when DCIN connected */
+#define LDO3_MODE_MASK BIT(2)
+#define LDO3_MODE_I2C BIT(2)
+#define LDO3_MODE_DCIN 0
+
+/* LDO_MODE2 register */
+#define LDO3_SNVS_ON BIT(7)
+#define LDO3_RUN_ON BIT(6)
+#define LDO3_LPSR_ON BIT(5)
+#define LDO3_SUSP_ON BIT(4)
+#define LDO2_SNVS_ON BIT(3)
+#define LDO2_RUN_ON BIT(2)
+#define LDO2_LPSR_ON BIT(1)
+#define LDO2_SUSP_ON BIT(0)
+
+
+/* LDO_MODE3 register */
+#define LDO5_SNVS_ON BIT(7)
+#define LDO5_RUN_ON BIT(6)
+#define LDO5_LPSR_ON BIT(5)
+#define LDO5_SUSP_ON BIT(4)
+#define LDO4_SNVS_ON BIT(3)
+#define LDO4_RUN_ON BIT(2)
+#define LDO4_LPSR_ON BIT(1)
+#define LDO4_SUSP_ON BIT(0)
+
+/* LDO_MODE4 register */
+#define DVREF_SNVS_ON BIT(7)
+#define DVREF_RUN_ON BIT(6)
+#define DVREF_LPSR_ON BIT(5)
+#define DVREF_SUSP_ON BIT(4)
+#define LDO_LPSR_SNVS_ON BIT(3)
+#define LDO_LPSR_RUN_ON BIT(2)
+#define LDO_LPSR_LPSR_ON BIT(1)
+#define LDO_LPSR_SUSP_ON BIT(0)
+
+/* BD71815_REG_OUT32K bits */
+#define OUT32K_EN BIT(0)
+#define OUT32K_MODE BIT(1)
+#define OUT32K_MODE_CMOS BIT(1)
+#define OUT32K_MODE_OPEN_DRAIN 0
+
+/* BD71815_REG_BAT_STAT bits */
+#define BAT_DET BIT(5)
+#define BAT_DET_OFFSET 5
+#define BAT_DET_DONE BIT(4)
+#define VBAT_OV BIT(3)
+#define DBAT_DET BIT(0)
+
+/* BD71815_REG_VBUS_STAT bits */
+#define VBUS_DET BIT(0)
+
+#define BD71815_REG_RTC_START BD71815_REG_SEC
+#define BD71815_REG_RTC_ALM_START BD71815_REG_ALM0_SEC
+
+/* BD71815_REG_ALM0_MASK bits */
+#define A0_ONESEC BIT(7)
+
+/* BD71815_REG_INT_EN_00 bits */
+#define ALMALE BIT(0)
+
+/* BD71815_REG_INT_STAT_03 bits */
+#define DCIN_MON_DET BIT(1)
+#define DCIN_MON_RES BIT(0)
+#define POWERON_LONG BIT(2)
+#define POWERON_MID BIT(3)
+#define POWERON_SHORT BIT(4)
+#define POWERON_PRESS BIT(5)
+
+/* BD71805_REG_INT_STAT_08 bits */
+#define VBAT_MON_DET BIT(1)
+#define VBAT_MON_RES BIT(0)
+
+/* BD71805_REG_INT_STAT_11 bits */
+#define INT_STAT_11_VF_DET BIT(7)
+#define INT_STAT_11_VF_RES BIT(6)
+#define INT_STAT_11_VF125_DET BIT(5)
+#define INT_STAT_11_VF125_RES BIT(4)
+#define INT_STAT_11_OVTMP_DET BIT(3)
+#define INT_STAT_11_OVTMP_RES BIT(2)
+#define INT_STAT_11_LOTMP_DET BIT(1)
+#define INT_STAT_11_LOTMP_RES BIT(0)
+
+#define VBAT_MON_DET BIT(1)
+#define VBAT_MON_RES BIT(0)
+
+/* BD71815_REG_PWRCTRL bits */
+#define RESTARTEN BIT(0)
+
+/* BD71815_REG_GPO bits */
+#define READY_FORCE_LOW BIT(2)
+#define BD71815_GPIO_DRIVE_MASK BIT(4)
+#define BD71815_GPIO_OPEN_DRAIN 0
+#define BD71815_GPIO_CMOS BIT(4)
+
+/* BD71815 interrupt masks */
+enum {
+ BD71815_INT_EN_01_BUCKAST_MASK = 0x0F,
+ BD71815_INT_EN_02_DCINAST_MASK = 0x3E,
+ BD71815_INT_EN_03_DCINAST_MASK = 0x3F,
+ BD71815_INT_EN_04_VSYSAST_MASK = 0xCF,
+ BD71815_INT_EN_05_CHGAST_MASK = 0xFC,
+ BD71815_INT_EN_06_BATAST_MASK = 0xF3,
+ BD71815_INT_EN_07_BMONAST_MASK = 0xFE,
+ BD71815_INT_EN_08_BMONAST_MASK = 0x03,
+ BD71815_INT_EN_09_BMONAST_MASK = 0x07,
+ BD71815_INT_EN_10_BMONAST_MASK = 0x3F,
+ BD71815_INT_EN_11_TMPAST_MASK = 0xFF,
+ BD71815_INT_EN_12_ALMAST_MASK = 0x07,
+};
+/* BD71815 interrupt irqs */
+enum {
+ /* BUCK reg interrupts */
+ BD71815_INT_BUCK1_OCP,
+ BD71815_INT_BUCK2_OCP,
+ BD71815_INT_BUCK3_OCP,
+ BD71815_INT_BUCK4_OCP,
+ BD71815_INT_BUCK5_OCP,
+ BD71815_INT_LED_OVP,
+ BD71815_INT_LED_OCP,
+ BD71815_INT_LED_SCP,
+ /* DCIN1 interrupts */
+ BD71815_INT_DCIN_RMV,
+ BD71815_INT_CLPS_OUT,
+ BD71815_INT_CLPS_IN,
+ BD71815_INT_DCIN_OVP_RES,
+ BD71815_INT_DCIN_OVP_DET,
+ /* DCIN2 interrupts */
+ BD71815_INT_DCIN_MON_RES,
+ BD71815_INT_DCIN_MON_DET,
+ BD71815_INT_WDOG,
+ /* Vsys INT_STAT_04 */
+ BD71815_INT_VSYS_UV_RES,
+ BD71815_INT_VSYS_UV_DET,
+ BD71815_INT_VSYS_LOW_RES,
+ BD71815_INT_VSYS_LOW_DET,
+ BD71815_INT_VSYS_MON_RES,
+ BD71815_INT_VSYS_MON_DET,
+ /* Charger INT_STAT_05 */
+ BD71815_INT_CHG_WDG_TEMP,
+ BD71815_INT_CHG_WDG_TIME,
+ BD71815_INT_CHG_RECHARGE_RES,
+ BD71815_INT_CHG_RECHARGE_DET,
+ BD71815_INT_CHG_RANGED_TEMP_TRANSITION,
+ BD71815_INT_CHG_STATE_TRANSITION,
+ /* Battery INT_STAT_06 */
+ BD71815_INT_BAT_TEMP_NORMAL,
+ BD71815_INT_BAT_TEMP_ERANGE,
+ BD71815_INT_BAT_REMOVED,
+ BD71815_INT_BAT_DETECTED,
+ BD71815_INT_THERM_REMOVED,
+ BD71815_INT_THERM_DETECTED,
+ /* Battery Mon 1 INT_STAT_07 */
+ BD71815_INT_BAT_DEAD,
+ BD71815_INT_BAT_SHORTC_RES,
+ BD71815_INT_BAT_SHORTC_DET,
+ BD71815_INT_BAT_LOW_VOLT_RES,
+ BD71815_INT_BAT_LOW_VOLT_DET,
+ BD71815_INT_BAT_OVER_VOLT_RES,
+ BD71815_INT_BAT_OVER_VOLT_DET,
+ /* Battery Mon 2 INT_STAT_08 */
+ BD71815_INT_BAT_MON_RES,
+ BD71815_INT_BAT_MON_DET,
+ /* Battery Mon 3 (Coulomb counter) INT_STAT_09 */
+ BD71815_INT_BAT_CC_MON1,
+ BD71815_INT_BAT_CC_MON2,
+ BD71815_INT_BAT_CC_MON3,
+ /* Battery Mon 4 INT_STAT_10 */
+ BD71815_INT_BAT_OVER_CURR_1_RES,
+ BD71815_INT_BAT_OVER_CURR_1_DET,
+ BD71815_INT_BAT_OVER_CURR_2_RES,
+ BD71815_INT_BAT_OVER_CURR_2_DET,
+ BD71815_INT_BAT_OVER_CURR_3_RES,
+ BD71815_INT_BAT_OVER_CURR_3_DET,
+ /* Temperature INT_STAT_11 */
+ BD71815_INT_TEMP_BAT_LOW_RES,
+ BD71815_INT_TEMP_BAT_LOW_DET,
+ BD71815_INT_TEMP_BAT_HI_RES,
+ BD71815_INT_TEMP_BAT_HI_DET,
+ BD71815_INT_TEMP_CHIP_OVER_125_RES,
+ BD71815_INT_TEMP_CHIP_OVER_125_DET,
+ BD71815_INT_TEMP_CHIP_OVER_VF_RES,
+ BD71815_INT_TEMP_CHIP_OVER_VF_DET,
+ /* RTC Alarm INT_STAT_12 */
+ BD71815_INT_RTC0,
+ BD71815_INT_RTC1,
+ BD71815_INT_RTC2,
+};
+
+#define BD71815_INT_BUCK1_OCP_MASK BIT(0)
+#define BD71815_INT_BUCK2_OCP_MASK BIT(1)
+#define BD71815_INT_BUCK3_OCP_MASK BIT(2)
+#define BD71815_INT_BUCK4_OCP_MASK BIT(3)
+#define BD71815_INT_BUCK5_OCP_MASK BIT(4)
+#define BD71815_INT_LED_OVP_MASK BIT(5)
+#define BD71815_INT_LED_OCP_MASK BIT(6)
+#define BD71815_INT_LED_SCP_MASK BIT(7)
+
+#define BD71815_INT_DCIN_RMV_MASK BIT(1)
+#define BD71815_INT_CLPS_OUT_MASK BIT(2)
+#define BD71815_INT_CLPS_IN_MASK BIT(3)
+#define BD71815_INT_DCIN_OVP_RES_MASK BIT(4)
+#define BD71815_INT_DCIN_OVP_DET_MASK BIT(5)
+
+#define BD71815_INT_DCIN_MON_RES_MASK BIT(0)
+#define BD71815_INT_DCIN_MON_DET_MASK BIT(1)
+#define BD71815_INT_WDOG_MASK BIT(6)
+
+#define BD71815_INT_VSYS_UV_RES_MASK BIT(0)
+#define BD71815_INT_VSYS_UV_DET_MASK BIT(1)
+#define BD71815_INT_VSYS_LOW_RES_MASK BIT(2)
+#define BD71815_INT_VSYS_LOW_DET_MASK BIT(3)
+#define BD71815_INT_VSYS_MON_RES_MASK BIT(6)
+#define BD71815_INT_VSYS_MON_DET_MASK BIT(7)
+
+#define BD71815_INT_CHG_WDG_TEMP_MASK BIT(2)
+#define BD71815_INT_CHG_WDG_TIME_MASK BIT(3)
+#define BD71815_INT_CHG_RECHARGE_RES_MASK BIT(4)
+#define BD71815_INT_CHG_RECHARGE_DET_MASK BIT(5)
+#define BD71815_INT_CHG_RANGED_TEMP_TRANSITION_MASK BIT(6)
+#define BD71815_INT_CHG_STATE_TRANSITION_MASK BIT(7)
+
+#define BD71815_INT_BAT_TEMP_NORMAL_MASK BIT(0)
+#define BD71815_INT_BAT_TEMP_ERANGE_MASK BIT(1)
+#define BD71815_INT_BAT_REMOVED_MASK BIT(4)
+#define BD71815_INT_BAT_DETECTED_MASK BIT(5)
+#define BD71815_INT_THERM_REMOVED_MASK BIT(6)
+#define BD71815_INT_THERM_DETECTED_MASK BIT(7)
+
+#define BD71815_INT_BAT_DEAD_MASK BIT(1)
+#define BD71815_INT_BAT_SHORTC_RES_MASK BIT(2)
+#define BD71815_INT_BAT_SHORTC_DET_MASK BIT(3)
+#define BD71815_INT_BAT_LOW_VOLT_RES_MASK BIT(4)
+#define BD71815_INT_BAT_LOW_VOLT_DET_MASK BIT(5)
+#define BD71815_INT_BAT_OVER_VOLT_RES_MASK BIT(6)
+#define BD71815_INT_BAT_OVER_VOLT_DET_MASK BIT(7)
+
+#define BD71815_INT_BAT_MON_RES_MASK BIT(0)
+#define BD71815_INT_BAT_MON_DET_MASK BIT(1)
+
+#define BD71815_INT_BAT_CC_MON1_MASK BIT(0)
+#define BD71815_INT_BAT_CC_MON2_MASK BIT(1)
+#define BD71815_INT_BAT_CC_MON3_MASK BIT(2)
+
+#define BD71815_INT_BAT_OVER_CURR_1_RES_MASK BIT(0)
+#define BD71815_INT_BAT_OVER_CURR_1_DET_MASK BIT(1)
+#define BD71815_INT_BAT_OVER_CURR_2_RES_MASK BIT(2)
+#define BD71815_INT_BAT_OVER_CURR_2_DET_MASK BIT(3)
+#define BD71815_INT_BAT_OVER_CURR_3_RES_MASK BIT(4)
+#define BD71815_INT_BAT_OVER_CURR_3_DET_MASK BIT(5)
+
+#define BD71815_INT_TEMP_BAT_LOW_RES_MASK BIT(0)
+#define BD71815_INT_TEMP_BAT_LOW_DET_MASK BIT(1)
+#define BD71815_INT_TEMP_BAT_HI_RES_MASK BIT(2)
+#define BD71815_INT_TEMP_BAT_HI_DET_MASK BIT(3)
+#define BD71815_INT_TEMP_CHIP_OVER_125_RES_MASK BIT(4)
+#define BD71815_INT_TEMP_CHIP_OVER_125_DET_MASK BIT(5)
+#define BD71815_INT_TEMP_CHIP_OVER_VF_RES_MASK BIT(6)
+#define BD71815_INT_TEMP_CHIP_OVER_VF_DET_MASK BIT(7)
+
+#define BD71815_INT_RTC0_MASK BIT(0)
+#define BD71815_INT_RTC1_MASK BIT(1)
+#define BD71815_INT_RTC2_MASK BIT(2)
+
+/* BD71815_REG_CC_CTRL bits */
+#define CCNTRST 0x80
+#define CCNTENB 0x40
+#define CCCALIB 0x20
+
+/* BD71815_REG_CC_CURCD */
+#define CURDIR_Discharging 0x8000
+
+/* BD71815_REG_VM_SA_IBAT */
+#define IBAT_SA_DIR_Discharging 0x8000
+
+/* BD71815_REG_REX_CTRL_1 bits */
+#define REX_CLR BIT(4)
+
+/* BD71815_REG_REX_CTRL_1 bits */
+#define REX_PMU_STATE_MASK BIT(2)
+
+/* BD71815_REG_LED_CTRL bits */
+#define CHGDONE_LED_EN BIT(4)
+
+#endif /* __LINUX_MFD_BD71815_H */
diff --git a/include/linux/mfd/rohm-bd71828.h b/include/linux/mfd/rohm-bd71828.h
index 017a4c01cb31..c7ab69c87ee8 100644
--- a/include/linux/mfd/rohm-bd71828.h
+++ b/include/linux/mfd/rohm-bd71828.h
@@ -151,6 +151,9 @@ enum {
#define BD71828_REG_GPIO_CTRL3 0x49
#define BD71828_REG_IO_STAT 0xed
+/* clk */
+#define BD71828_REG_OUT32K 0x4b
+
/* RTC */
#define BD71828_REG_RTC_SEC 0x4c
#define BD71828_REG_RTC_MINUTE 0x4d
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
` (7 preceding siblings ...)
2021-03-24 7:26 ` [PATCH v4 08/16] mfd: Support for ROHM BD71815 PMIC core Matti Vaittinen
@ 2021-03-24 7:29 ` Matti Vaittinen
2021-03-25 9:35 ` Linus Walleij
2021-03-26 11:26 ` [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs Andy Shevchenko
2021-03-24 7:29 ` [PATCH v4 10/16] regulator: helpers: Export helper voltage listing Matti Vaittinen
` (5 subsequent siblings)
14 siblings, 2 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:29 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Matti Vaittinen, Linus Walleij,
Bartosz Golaszewski, devicetree, linux-kernel, linux-power,
linux-gpio
Support GPO(s) found from ROHM BD71815 power management IC. The IC has two
GPO pins but only one is properly documented in data-sheet. The driver
exposes by default only the documented GPO. The second GPO is connected to
E5 pin and is marked as GND in data-sheet. Control for this undocumented
pin can be enabled using a special DT property.
This driver is derived from work by Peter Yang <yanglsh@embest-tech.com>
although not so much of original is left.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
Changes since v3:
- No changes
drivers/gpio/Kconfig | 10 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-bd71815.c | 176 ++++++++++++++++++++++++++++++++++++
3 files changed, 187 insertions(+)
create mode 100644 drivers/gpio/gpio-bd71815.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index e3607ec4c2e8..d3b3de514f6e 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1105,6 +1105,16 @@ config GPIO_BD70528
This driver can also be built as a module. If so, the module
will be called gpio-bd70528.
+config GPIO_BD71815
+ tristate "ROHM BD71815 PMIC GPIO support"
+ depends on MFD_ROHM_BD71828
+ help
+ Support for GPO(s) on ROHM BD71815 PMIC. There are two GPOs
+ available on the ROHM PMIC.
+
+ This driver can also be built as a module. If so, the module
+ will be called gpio-bd71815.
+
config GPIO_BD71828
tristate "ROHM BD71828 GPIO support"
depends on MFD_ROHM_BD71828
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index c58a90a3c3b1..4c12f31db31f 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_GPIO_ATH79) += gpio-ath79.o
obj-$(CONFIG_GPIO_BCM_KONA) += gpio-bcm-kona.o
obj-$(CONFIG_GPIO_BCM_XGS_IPROC) += gpio-xgs-iproc.o
obj-$(CONFIG_GPIO_BD70528) += gpio-bd70528.o
+obj-$(CONFIG_GPIO_BD71815) += gpio-bd71815.o
obj-$(CONFIG_GPIO_BD71828) += gpio-bd71828.o
obj-$(CONFIG_GPIO_BD9571MWV) += gpio-bd9571mwv.o
obj-$(CONFIG_GPIO_BRCMSTB) += gpio-brcmstb.o
diff --git a/drivers/gpio/gpio-bd71815.c b/drivers/gpio/gpio-bd71815.c
new file mode 100644
index 000000000000..5552a23c8e53
--- /dev/null
+++ b/drivers/gpio/gpio-bd71815.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Support to GPOs on ROHM BD71815
+ */
+
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+/* For the BD71815 register definitions */
+#include <linux/mfd/rohm-bd71815.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+struct bd71815_gpio {
+ struct gpio_chip chip;
+ struct device *dev;
+ struct regmap *regmap;
+ /*
+ * Sigh. The BD71815 and BD71817 were originally designed to support two
+ * GPO pins. At some point it was noticed the second GPO pin which is
+ * the E5 pin located at the center of IC is hard to use on PCB (due to
+ * the location). It was decided to not promote this second GPO and pin
+ * is marked as GND on the data-sheet. The functionality is still there
+ * though! I guess driving GPO connected to ground is a bad idea. Thus
+ * we do not support it by default. OTOH - the original driver written
+ * by colleagues at Embest did support controlling this second GPO. It
+ * is thus possible this is used in some of the products.
+ *
+ * This driver does not by default support configuring this second GPO
+ * but allows using it by providing the DT property
+ * "rohm,enable-hidden-gpo".
+ */
+ bool e5_pin_is_gpo;
+};
+
+static int bd71815gpo_get(struct gpio_chip *chip, unsigned int offset)
+{
+ struct bd71815_gpio *bd71815 = gpiochip_get_data(chip);
+ int ret = 0;
+ int val;
+
+ ret = regmap_read(bd71815->regmap, BD71815_REG_GPO, &val);
+ if (ret)
+ return ret;
+
+ return (val >> offset) & 1;
+}
+
+static void bd71815gpo_set(struct gpio_chip *chip, unsigned int offset,
+ int value)
+{
+ struct bd71815_gpio *bd71815 = gpiochip_get_data(chip);
+ int ret, bit;
+
+ if (!bd71815->e5_pin_is_gpo && offset)
+ return;
+
+ bit = BIT(offset);
+
+ if (value)
+ ret = regmap_set_bits(bd71815->regmap, BD71815_REG_GPO, bit);
+ else
+ ret = regmap_clear_bits(bd71815->regmap, BD71815_REG_GPO, bit);
+
+ if (ret)
+ dev_warn(bd71815->dev, "failed to toggle GPO\n");
+}
+
+static int bd71815_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
+ unsigned long config)
+{
+ struct bd71815_gpio *bdgpio = gpiochip_get_data(chip);
+
+ if (!bdgpio->e5_pin_is_gpo && offset)
+ return -EOPNOTSUPP;
+
+ switch (pinconf_to_config_param(config)) {
+ case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+ return regmap_update_bits(bdgpio->regmap,
+ BD71815_REG_GPO,
+ BD71815_GPIO_DRIVE_MASK << offset,
+ BD71815_GPIO_OPEN_DRAIN << offset);
+ case PIN_CONFIG_DRIVE_PUSH_PULL:
+ return regmap_update_bits(bdgpio->regmap,
+ BD71815_REG_GPO,
+ BD71815_GPIO_DRIVE_MASK << offset,
+ BD71815_GPIO_CMOS << offset);
+ default:
+ break;
+ }
+ return -EOPNOTSUPP;
+}
+
+/* BD71815 GPIO is actually GPO */
+static int bd71815gpo_direction_get(struct gpio_chip *gc, unsigned int offset)
+{
+ return GPIO_LINE_DIRECTION_OUT;
+}
+
+/* Template for GPIO chip */
+static const struct gpio_chip bd71815gpo_chip = {
+ .label = "bd71815",
+ .owner = THIS_MODULE,
+ .get = bd71815gpo_get,
+ .get_direction = bd71815gpo_direction_get,
+ .set = bd71815gpo_set,
+ .set_config = bd71815_gpio_set_config,
+ .can_sleep = 1,
+};
+
+static int gpo_bd71815_probe(struct platform_device *pdev)
+{
+ int ret;
+ struct bd71815_gpio *g;
+ struct device *dev;
+ struct device *parent;
+
+ /*
+ * Bind devm lifetime to this platform device => use dev for devm.
+ * also the prints should originate from this device.
+ */
+ dev = &pdev->dev;
+ /* The device-tree and regmap come from MFD => use parent for that */
+ parent = dev->parent;
+
+ g = devm_kzalloc(dev, sizeof(*g), GFP_KERNEL);
+ if (!g)
+ return -ENOMEM;
+
+ g->e5_pin_is_gpo = of_property_read_bool(parent->of_node,
+ "rohm,enable-hidden-gpo");
+ g->chip = bd71815gpo_chip;
+ g->chip.base = -1;
+
+ if (g->e5_pin_is_gpo)
+ g->chip.ngpio = 2;
+ else
+ g->chip.ngpio = 1;
+
+ g->chip.parent = parent;
+ g->chip.of_node = parent->of_node;
+ g->regmap = dev_get_regmap(parent, NULL);
+ g->dev = dev;
+
+ ret = devm_gpiochip_add_data(dev, &g->chip, g);
+ if (ret < 0) {
+ dev_err(dev, "could not register gpiochip, %d\n", ret);
+ return ret;
+ }
+
+ return ret;
+}
+static const struct platform_device_id bd7181x_gpo_id[] = {
+ { "bd71815-gpo" },
+ { },
+};
+MODULE_DEVICE_TABLE(platform, bd7181x_gpo_id);
+
+static struct platform_driver gpo_bd71815_driver = {
+ .driver = {
+ .name = "bd71815-gpo",
+ .owner = THIS_MODULE,
+ },
+ .probe = gpo_bd71815_probe,
+ .id_table = bd7181x_gpo_id,
+};
+
+module_platform_driver(gpo_bd71815_driver);
+
+/* Note: this hardware lives inside an I2C-based multi-function device. */
+MODULE_ALIAS("platform:bd71815-gpo");
+
+MODULE_AUTHOR("Peter Yang <yanglsh@embest-tech.com>");
+MODULE_DESCRIPTION("GPO interface for BD71815");
+MODULE_LICENSE("GPL");
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 10/16] regulator: helpers: Export helper voltage listing
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
` (8 preceding siblings ...)
2021-03-24 7:29 ` [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs Matti Vaittinen
@ 2021-03-24 7:29 ` Matti Vaittinen
2021-03-24 7:30 ` [PATCH v4 12/16] regulator: rohm-regulator: Support SNVS HW state Matti Vaittinen
` (4 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:29 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Liam Girdwood, Mark Brown,
Matti Vaittinen, devicetree, linux-kernel, linux-power
Some drivers need to translate voltage values to selectors prior regulator
registration. Currently a regulator_desc based list_voltages helper is only
exported for regulators using the linear_ranges. Export similar helper also
for regulators using simple linear mapping.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
Changes since v3:
- No changes
drivers/regulator/helpers.c | 36 +++++++++++++++++++++++++-------
include/linux/regulator/driver.h | 2 ++
2 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/drivers/regulator/helpers.c b/drivers/regulator/helpers.c
index f42b394a0c46..3e19ecbf7267 100644
--- a/drivers/regulator/helpers.c
+++ b/drivers/regulator/helpers.c
@@ -508,6 +508,33 @@ int regulator_map_voltage_pickable_linear_range(struct regulator_dev *rdev,
}
EXPORT_SYMBOL_GPL(regulator_map_voltage_pickable_linear_range);
+/**
+ * regulator_desc_list_voltage_linear - List voltages with simple calculation
+ *
+ * @desc: Regulator desc for regulator which volatges are to be listed
+ * @selector: Selector to convert into a voltage
+ *
+ * Regulators with a simple linear mapping between voltages and
+ * selectors can set min_uV and uV_step in the regulator descriptor
+ * and then use this function prior regulator registration to list
+ * the voltages. This is useful when voltages need to be listed during
+ * device-tree parsing.
+ */
+int regulator_desc_list_voltage_linear(const struct regulator_desc *desc,
+ unsigned int selector)
+{
+ if (selector >= desc->n_voltages)
+ return -EINVAL;
+
+ if (selector < desc->linear_min_sel)
+ return 0;
+
+ selector -= desc->linear_min_sel;
+
+ return desc->min_uV + (desc->uV_step * selector);
+}
+EXPORT_SYMBOL_GPL(regulator_desc_list_voltage_linear);
+
/**
* regulator_list_voltage_linear - List voltages with simple calculation
*
@@ -521,14 +548,7 @@ EXPORT_SYMBOL_GPL(regulator_map_voltage_pickable_linear_range);
int regulator_list_voltage_linear(struct regulator_dev *rdev,
unsigned int selector)
{
- if (selector >= rdev->desc->n_voltages)
- return -EINVAL;
- if (selector < rdev->desc->linear_min_sel)
- return 0;
-
- selector -= rdev->desc->linear_min_sel;
-
- return rdev->desc->min_uV + (rdev->desc->uV_step * selector);
+ return regulator_desc_list_voltage_linear(rdev->desc, selector);
}
EXPORT_SYMBOL_GPL(regulator_list_voltage_linear);
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index d7c77ee370f3..39a540111645 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -543,4 +543,6 @@ void *regulator_get_init_drvdata(struct regulator_init_data *reg_init_data);
int regulator_desc_list_voltage_linear_range(const struct regulator_desc *desc,
unsigned int selector);
+int regulator_desc_list_voltage_linear(const struct regulator_desc *desc,
+ unsigned int selector);
#endif
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 12/16] regulator: rohm-regulator: Support SNVS HW state.
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
` (9 preceding siblings ...)
2021-03-24 7:29 ` [PATCH v4 10/16] regulator: helpers: Export helper voltage listing Matti Vaittinen
@ 2021-03-24 7:30 ` Matti Vaittinen
2021-03-24 7:30 ` [PATCH v4 13/16] regulator: Support ROHM BD71815 regulators Matti Vaittinen
` (3 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:30 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Liam Girdwood, Mark Brown,
Matti Vaittinen, devicetree, linux-kernel, linux-power
The ROHM BD71815 supports setting voltage levels/regulator status
for HW-states "RUN", "SUSPEND", "LPSR" and "SNVS". Add DT parsing
helper also for SNVS state.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
Changes since v3:
- No changes
drivers/regulator/rohm-regulator.c | 6 ++++++
include/linux/mfd/rohm-generic.h | 6 +++++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/rohm-regulator.c b/drivers/regulator/rohm-regulator.c
index 63aabb8c7786..6e0d9c08ec1c 100644
--- a/drivers/regulator/rohm-regulator.c
+++ b/drivers/regulator/rohm-regulator.c
@@ -95,6 +95,12 @@ int rohm_regulator_set_dvs_levels(const struct rohm_dvs_config *dvs,
mask = dvs->lpsr_mask;
omask = dvs->lpsr_on_mask;
break;
+ case ROHM_DVS_LEVEL_SNVS:
+ prop = "rohm,dvs-snvs-voltage";
+ reg = dvs->snvs_reg;
+ mask = dvs->snvs_mask;
+ omask = dvs->snvs_on_mask;
+ break;
default:
return -EINVAL;
}
diff --git a/include/linux/mfd/rohm-generic.h b/include/linux/mfd/rohm-generic.h
index 9e2880e06950..a9144284cf6d 100644
--- a/include/linux/mfd/rohm-generic.h
+++ b/include/linux/mfd/rohm-generic.h
@@ -27,7 +27,8 @@ struct rohm_regmap_dev {
#define ROHM_DVS_LEVEL_IDLE BIT(1)
#define ROHM_DVS_LEVEL_SUSPEND BIT(2)
#define ROHM_DVS_LEVEL_LPSR BIT(3)
-#define ROHM_DVS_LEVEL_VALID_AMOUNT 4
+#define ROHM_DVS_LEVEL_SNVS BIT(4)
+#define ROHM_DVS_LEVEL_VALID_AMOUNT 5
#define ROHM_DVS_LEVEL_UNKNOWN 0
/**
@@ -66,6 +67,9 @@ struct rohm_dvs_config {
unsigned int lpsr_reg;
unsigned int lpsr_mask;
unsigned int lpsr_on_mask;
+ unsigned int snvs_reg;
+ unsigned int snvs_mask;
+ unsigned int snvs_on_mask;
};
#if IS_ENABLED(CONFIG_REGULATOR_ROHM)
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 13/16] regulator: Support ROHM BD71815 regulators
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
` (10 preceding siblings ...)
2021-03-24 7:30 ` [PATCH v4 12/16] regulator: rohm-regulator: Support SNVS HW state Matti Vaittinen
@ 2021-03-24 7:30 ` Matti Vaittinen
2021-03-24 7:30 ` [PATCH v4 14/16] clk: bd718x7: Add support for clk gate on ROHM BD71815 PMIC Matti Vaittinen
` (2 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:30 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Liam Girdwood, Mark Brown,
Matti Vaittinen, devicetree, linux-kernel, linux-power
Support voltage control for regulators on ROHM BD71815 PMIC.
ROHM BD71815 contains 5 bucks, 7 LDOs and a boost (intended for LED).
Bucks 1 and 2 support HW state based voltage level and enable states. Other
regulators support HW state based enable states. All bucks and LDOs 1-5
allow voltage changes for RUN state and LDO4 can be enabled/disabled via
GPIO.
LDO3 does support changing between two predetermined voltages by using
a GPIO but this functionality is not included in this commit.
This work is derived from driver originally written by Tony Luo
<luofc@embedinfo.com> - although not much of original work is left.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
Changes since v3:
- minor styling
- staticized DVS structures
drivers/regulator/Kconfig | 11 +
drivers/regulator/Makefile | 1 +
drivers/regulator/bd71815-regulator.c | 676 ++++++++++++++++++++++++++
3 files changed, 688 insertions(+)
create mode 100644 drivers/regulator/bd71815-regulator.c
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 77c43134bc9e..6437348ce862 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -204,6 +204,17 @@ config REGULATOR_BD70528
This driver can also be built as a module. If so, the module
will be called bd70528-regulator.
+config REGULATOR_BD71815
+ tristate "ROHM BD71815 Power Regulator"
+ depends on MFD_ROHM_BD71828
+ help
+ This driver supports voltage regulators on ROHM BD71815 PMIC.
+ This will enable support for the software controllable buck
+ and LDO regulators and a current regulator for LEDs.
+
+ This driver can also be built as a module. If so, the module
+ will be called bd71815-regulator.
+
config REGULATOR_BD71828
tristate "ROHM BD71828 Power Regulator"
depends on MFD_ROHM_BD71828
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 44d2f8bf4b74..c6f84a332fdd 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_REGULATOR_ATC260X) += atc260x-regulator.o
obj-$(CONFIG_REGULATOR_AXP20X) += axp20x-regulator.o
obj-$(CONFIG_REGULATOR_BCM590XX) += bcm590xx-regulator.o
obj-$(CONFIG_REGULATOR_BD70528) += bd70528-regulator.o
+obj-$(CONFIG_REGULATOR_BD71815) += bd71815-regulator.o
obj-$(CONFIG_REGULATOR_BD71828) += bd71828-regulator.o
obj-$(CONFIG_REGULATOR_BD718XX) += bd718x7-regulator.o
obj-$(CONFIG_REGULATOR_BD9571MWV) += bd9571mwv-regulator.o
diff --git a/drivers/regulator/bd71815-regulator.c b/drivers/regulator/bd71815-regulator.c
new file mode 100644
index 000000000000..b1448e34c629
--- /dev/null
+++ b/drivers/regulator/bd71815-regulator.c
@@ -0,0 +1,676 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2014 Embest Technology Co. Ltd. Inc.
+ * bd71815-regulator.c ROHM BD71815 regulator driver
+ *
+ * Author: Tony Luo <luofc@embedinfo.com>
+ *
+ * Partially rewritten at 2021 by
+ * Matti Vaittinen <matti.vaitinen@fi.rohmeurope.com>
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/gpio.h>
+#include <linux/mfd/rohm-generic.h>
+#include <linux/mfd/rohm-bd71815.h>
+#include <linux/regulator/of_regulator.h>
+
+struct bd71815_regulator {
+ struct regulator_desc desc;
+ const struct rohm_dvs_config *dvs;
+};
+
+struct bd71815_pmic {
+ struct bd71815_regulator descs[BD71815_REGULATOR_CNT];
+ struct regmap *regmap;
+ struct device *dev;
+ struct gpio_descs *gps;
+ struct regulator_dev *rdev[BD71815_REGULATOR_CNT];
+};
+
+static const int bd7181x_wled_currents[] = {
+ 10, 20, 30, 50, 70, 100, 200, 300, 500, 700, 1000, 2000, 3000, 4000,
+ 5000, 6000, 7000, 8000, 9000, 10000, 11000, 12000, 13000, 14000, 15000,
+ 16000, 17000, 18000, 19000, 20000, 21000, 22000, 23000, 24000, 25000,
+};
+
+static const struct rohm_dvs_config buck1_dvs = {
+ .level_map = ROHM_DVS_LEVEL_RUN | ROHM_DVS_LEVEL_SNVS |
+ ROHM_DVS_LEVEL_SUSPEND | ROHM_DVS_LEVEL_LPSR,
+ .run_reg = BD71815_REG_BUCK1_VOLT_H,
+ .run_mask = BD71815_VOLT_MASK,
+ .run_on_mask = BD71815_BUCK_RUN_ON,
+ .snvs_on_mask = BD71815_BUCK_SNVS_ON,
+ .suspend_reg = BD71815_REG_BUCK1_VOLT_L,
+ .suspend_mask = BD71815_VOLT_MASK,
+ .suspend_on_mask = BD71815_BUCK_SUSP_ON,
+ .lpsr_reg = BD71815_REG_BUCK1_VOLT_L,
+ .lpsr_mask = BD71815_VOLT_MASK,
+ .lpsr_on_mask = BD71815_BUCK_LPSR_ON,
+};
+
+static const struct rohm_dvs_config buck2_dvs = {
+ .level_map = ROHM_DVS_LEVEL_RUN | ROHM_DVS_LEVEL_SNVS |
+ ROHM_DVS_LEVEL_SUSPEND | ROHM_DVS_LEVEL_LPSR,
+ .run_reg = BD71815_REG_BUCK2_VOLT_H,
+ .run_mask = BD71815_VOLT_MASK,
+ .run_on_mask = BD71815_BUCK_RUN_ON,
+ .snvs_on_mask = BD71815_BUCK_SNVS_ON,
+ .suspend_reg = BD71815_REG_BUCK2_VOLT_L,
+ .suspend_mask = BD71815_VOLT_MASK,
+ .suspend_on_mask = BD71815_BUCK_SUSP_ON,
+ .lpsr_reg = BD71815_REG_BUCK2_VOLT_L,
+ .lpsr_mask = BD71815_VOLT_MASK,
+ .lpsr_on_mask = BD71815_BUCK_LPSR_ON,
+};
+
+static const struct rohm_dvs_config buck3_dvs = {
+ .level_map = ROHM_DVS_LEVEL_RUN | ROHM_DVS_LEVEL_SNVS |
+ ROHM_DVS_LEVEL_SUSPEND | ROHM_DVS_LEVEL_LPSR,
+ .run_reg = BD71815_REG_BUCK3_VOLT,
+ .run_mask = BD71815_VOLT_MASK,
+ .run_on_mask = BD71815_BUCK_RUN_ON,
+ .snvs_on_mask = BD71815_BUCK_SNVS_ON,
+ .suspend_on_mask = BD71815_BUCK_SUSP_ON,
+ .lpsr_on_mask = BD71815_BUCK_LPSR_ON,
+};
+
+static const struct rohm_dvs_config buck4_dvs = {
+ .level_map = ROHM_DVS_LEVEL_RUN | ROHM_DVS_LEVEL_SNVS |
+ ROHM_DVS_LEVEL_SUSPEND | ROHM_DVS_LEVEL_LPSR,
+ .run_reg = BD71815_REG_BUCK4_VOLT,
+ .run_mask = BD71815_VOLT_MASK,
+ .run_on_mask = BD71815_BUCK_RUN_ON,
+ .snvs_on_mask = BD71815_BUCK_SNVS_ON,
+ .suspend_on_mask = BD71815_BUCK_SUSP_ON,
+ .lpsr_on_mask = BD71815_BUCK_LPSR_ON,
+};
+
+static const struct rohm_dvs_config ldo1_dvs = {
+ .level_map = ROHM_DVS_LEVEL_RUN | ROHM_DVS_LEVEL_SNVS |
+ ROHM_DVS_LEVEL_SUSPEND | ROHM_DVS_LEVEL_LPSR,
+ .run_reg = BD71815_REG_LDO_MODE1,
+ .run_mask = BD71815_VOLT_MASK,
+ .run_on_mask = LDO1_RUN_ON,
+ .snvs_on_mask = LDO1_SNVS_ON,
+ .suspend_on_mask = LDO1_SUSP_ON,
+ .lpsr_on_mask = LDO1_LPSR_ON,
+};
+
+static const struct rohm_dvs_config ldo2_dvs = {
+ .level_map = ROHM_DVS_LEVEL_RUN | ROHM_DVS_LEVEL_SNVS |
+ ROHM_DVS_LEVEL_SUSPEND | ROHM_DVS_LEVEL_LPSR,
+ .run_reg = BD71815_REG_LDO_MODE2,
+ .run_mask = BD71815_VOLT_MASK,
+ .run_on_mask = LDO2_RUN_ON,
+ .snvs_on_mask = LDO2_SNVS_ON,
+ .suspend_on_mask = LDO2_SUSP_ON,
+ .lpsr_on_mask = LDO2_LPSR_ON,
+};
+
+static const struct rohm_dvs_config ldo3_dvs = {
+ .level_map = ROHM_DVS_LEVEL_RUN | ROHM_DVS_LEVEL_SNVS |
+ ROHM_DVS_LEVEL_SUSPEND | ROHM_DVS_LEVEL_LPSR,
+ .run_reg = BD71815_REG_LDO_MODE2,
+ .run_mask = BD71815_VOLT_MASK,
+ .run_on_mask = LDO3_RUN_ON,
+ .snvs_on_mask = LDO3_SNVS_ON,
+ .suspend_on_mask = LDO3_SUSP_ON,
+ .lpsr_on_mask = LDO3_LPSR_ON,
+};
+
+static const struct rohm_dvs_config ldo4_dvs = {
+ .level_map = ROHM_DVS_LEVEL_RUN | ROHM_DVS_LEVEL_SNVS |
+ ROHM_DVS_LEVEL_SUSPEND | ROHM_DVS_LEVEL_LPSR,
+ .run_reg = BD71815_REG_LDO_MODE3,
+ .run_mask = BD71815_VOLT_MASK,
+ .run_on_mask = LDO4_RUN_ON,
+ .snvs_on_mask = LDO4_SNVS_ON,
+ .suspend_on_mask = LDO4_SUSP_ON,
+ .lpsr_on_mask = LDO4_LPSR_ON,
+};
+
+static const struct rohm_dvs_config ldo5_dvs = {
+ .level_map = ROHM_DVS_LEVEL_RUN | ROHM_DVS_LEVEL_SNVS |
+ ROHM_DVS_LEVEL_SUSPEND | ROHM_DVS_LEVEL_LPSR,
+ .run_reg = BD71815_REG_LDO_MODE3,
+ .run_mask = BD71815_VOLT_MASK,
+ .run_on_mask = LDO5_RUN_ON,
+ .snvs_on_mask = LDO5_SNVS_ON,
+ .suspend_on_mask = LDO5_SUSP_ON,
+ .lpsr_on_mask = LDO5_LPSR_ON,
+};
+
+static const struct rohm_dvs_config dvref_dvs = {
+ .level_map = ROHM_DVS_LEVEL_RUN | ROHM_DVS_LEVEL_SNVS |
+ ROHM_DVS_LEVEL_SUSPEND | ROHM_DVS_LEVEL_LPSR,
+ .run_on_mask = DVREF_RUN_ON,
+ .snvs_on_mask = DVREF_SNVS_ON,
+ .suspend_on_mask = DVREF_SUSP_ON,
+ .lpsr_on_mask = DVREF_LPSR_ON,
+};
+
+static const struct rohm_dvs_config ldolpsr_dvs = {
+ .level_map = ROHM_DVS_LEVEL_RUN | ROHM_DVS_LEVEL_SNVS |
+ ROHM_DVS_LEVEL_SUSPEND | ROHM_DVS_LEVEL_LPSR,
+ .run_on_mask = DVREF_RUN_ON,
+ .snvs_on_mask = DVREF_SNVS_ON,
+ .suspend_on_mask = DVREF_SUSP_ON,
+ .lpsr_on_mask = DVREF_LPSR_ON,
+};
+
+static const struct rohm_dvs_config buck5_dvs = {
+ .level_map = ROHM_DVS_LEVEL_RUN | ROHM_DVS_LEVEL_SNVS |
+ ROHM_DVS_LEVEL_SUSPEND | ROHM_DVS_LEVEL_LPSR,
+ .run_reg = BD71815_REG_BUCK5_VOLT,
+ .run_mask = BD71815_VOLT_MASK,
+ .run_on_mask = BD71815_BUCK_RUN_ON,
+ .snvs_on_mask = BD71815_BUCK_SNVS_ON,
+ .suspend_on_mask = BD71815_BUCK_SUSP_ON,
+ .lpsr_on_mask = BD71815_BUCK_LPSR_ON,
+};
+
+static int set_hw_dvs_levels(struct device_node *np,
+ const struct regulator_desc *desc,
+ struct regulator_config *cfg)
+{
+ struct bd71815_regulator *data;
+
+ data = container_of(desc, struct bd71815_regulator, desc);
+ return rohm_regulator_set_dvs_levels(data->dvs, np, desc, cfg->regmap);
+}
+
+/*
+ * Bucks 1 and 2 have two voltage selection registers where selected
+ * voltage can be set. Which of the registers is used can be either controlled
+ * by a control bit in register - or by HW state. If HW state specific voltages
+ * are given - then we assume HW state based control should be used.
+ *
+ * If volatge value is updated to currently selected register - then output
+ * voltage is immediately changed no matter what is set as ramp rate. Thus we
+ * default changing voltage by writing new value to inactive register and
+ * then updating the 'register selection' bit. This naturally only works when
+ * HW state machine is not used to select the voltage.
+ */
+static int buck12_set_hw_dvs_levels(struct device_node *np,
+ const struct regulator_desc *desc,
+ struct regulator_config *cfg)
+{
+ struct bd71815_regulator *data;
+ int ret = 0, val;
+
+ data = container_of(desc, struct bd71815_regulator, desc);
+
+ if (of_find_property(np, "rohm,dvs-run-voltage", NULL) ||
+ of_find_property(np, "rohm,dvs-suspend-voltage", NULL) ||
+ of_find_property(np, "rohm,dvs-lpsr-voltage", NULL) ||
+ of_find_property(np, "rohm,dvs-snvs-voltage", NULL)) {
+ ret = regmap_read(cfg->regmap, desc->vsel_reg, &val);
+ if (ret)
+ return ret;
+
+ if (!(BD71815_BUCK_STBY_DVS & val) &&
+ !(BD71815_BUCK_DVSSEL & val)) {
+ int val2;
+
+ /*
+ * We are currently using voltage from _L.
+ * We'd better copy it to _H and switch to it to
+ * avoid shutting us down if LPSR or SUSPEND is set to
+ * disabled. _L value is at reg _H + 1
+ */
+ ret = regmap_read(cfg->regmap, desc->vsel_reg + 1,
+ &val2);
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(cfg->regmap, desc->vsel_reg,
+ BD71815_VOLT_MASK |
+ BD71815_BUCK_DVSSEL,
+ val2 | BD71815_BUCK_DVSSEL);
+ if (ret)
+ return ret;
+ }
+ ret = rohm_regulator_set_dvs_levels(data->dvs, np, desc,
+ cfg->regmap);
+ if (ret)
+ return ret;
+ /*
+ * DVS levels were given => use HW-state machine for voltage
+ * controls. NOTE: AFAIK, This means that if voltage is changed
+ * by SW the ramp-rate is not respected. Should we disable
+ * SW voltage control when the HW state machine is used?
+ */
+ ret = regmap_update_bits(cfg->regmap, desc->vsel_reg,
+ BD71815_BUCK_STBY_DVS,
+ BD71815_BUCK_STBY_DVS);
+ }
+
+ return ret;
+}
+
+/*
+ * BUCK1/2
+ * BUCK1RAMPRATE[1:0] BUCK1 DVS ramp rate setting
+ * 00: 10.00mV/usec 10mV 1uS
+ * 01: 5.00mV/usec 10mV 2uS
+ * 10: 2.50mV/usec 10mV 4uS
+ * 11: 1.25mV/usec 10mV 8uS
+ */
+static int bd7181x_buck12_set_ramp_delay(struct regulator_dev *rdev,
+ int ramp_delay)
+{
+ struct bd71815_pmic *pmic = rdev_get_drvdata(rdev);
+ int id = rdev->desc->id;
+ unsigned int ramp_value = BD71815_BUCK_RAMPRATE_10P00MV;
+
+ switch (ramp_delay) {
+ case 1 ... 1250:
+ ramp_value = BD71815_BUCK_RAMPRATE_1P25MV;
+ break;
+ case 1251 ... 2500:
+ ramp_value = BD71815_BUCK_RAMPRATE_2P50MV;
+ break;
+ case 2501 ... 5000:
+ ramp_value = BD71815_BUCK_RAMPRATE_5P00MV;
+ break;
+ case 5001 ... 10000:
+ ramp_value = BD71815_BUCK_RAMPRATE_10P00MV;
+ break;
+ default:
+ ramp_value = BD71815_BUCK_RAMPRATE_10P00MV;
+ dev_err(pmic->dev,
+ "%s: ramp_delay: %d not supported, setting 10000mV//us\n",
+ rdev->desc->name, ramp_delay);
+ }
+
+ return regmap_update_bits(pmic->regmap, BD71815_REG_BUCK1_MODE + id*0x1,
+ BD71815_BUCK_RAMPRATE_MASK, ramp_value << 6);
+}
+
+static int bd7181x_led_set_current_limit(struct regulator_dev *rdev,
+ int min_uA, int max_uA)
+{
+ int ret;
+ int onstatus;
+
+ onstatus = regulator_is_enabled_regmap(rdev);
+
+ ret = regulator_set_current_limit_regmap(rdev, min_uA, max_uA);
+ if (!ret) {
+ int newstatus;
+
+ newstatus = regulator_is_enabled_regmap(rdev);
+ if (onstatus != newstatus) {
+ /*
+ * HW FIX: spurious led status change detected. Toggle
+ * state as a workaround
+ */
+ if (onstatus)
+ ret = regulator_enable_regmap(rdev);
+ else
+ ret = regulator_disable_regmap(rdev);
+
+ if (ret)
+ dev_err(rdev_get_dev(rdev),
+ "LED status error\n");
+ }
+ }
+ return ret;
+}
+
+static int bd7181x_buck12_get_voltage_sel(struct regulator_dev *rdev)
+{
+ struct bd71815_pmic *pmic = rdev_get_drvdata(rdev);
+ int rid = rdev_get_id(rdev);
+ int ret, regh, regl, val;
+
+ regh = BD71815_REG_BUCK1_VOLT_H + rid * 0x2;
+ regl = BD71815_REG_BUCK1_VOLT_L + rid * 0x2;
+
+ ret = regmap_read(pmic->regmap, regh, &val);
+ if (ret)
+ return ret;
+
+ /*
+ * If we use HW state machine based voltage reg selection - then we
+ * return BD71815_REG_BUCK1_VOLT_H which is used at RUN.
+ * Else we do return the BD71815_REG_BUCK1_VOLT_H or
+ * BD71815_REG_BUCK1_VOLT_L depending on which is selected to be used
+ * by BD71815_BUCK_DVSSEL bit
+ */
+ if ((!(val & BD71815_BUCK_STBY_DVS)) && (!(val & BD71815_BUCK_DVSSEL)))
+ ret = regmap_read(pmic->regmap, regl, &val);
+
+ if (ret)
+ return ret;
+
+ return val & BD71815_VOLT_MASK;
+}
+
+/*
+ * For Buck 1/2.
+ */
+static int bd7181x_buck12_set_voltage_sel(struct regulator_dev *rdev,
+ unsigned int sel)
+{
+ struct bd71815_pmic *pmic = rdev_get_drvdata(rdev);
+ int rid = rdev_get_id(rdev);
+ int ret, val, reg, regh, regl;
+
+ regh = BD71815_REG_BUCK1_VOLT_H + rid*0x2;
+ regl = BD71815_REG_BUCK1_VOLT_L + rid*0x2;
+
+ ret = regmap_read(pmic->regmap, regh, &val);
+ if (ret)
+ return ret;
+
+ /*
+ * If bucks 1 & 2 are controlled by state machine - then the RUN state
+ * voltage is set to BD71815_REG_BUCK1_VOLT_H. Changing SUSPEND/LPSR
+ * voltages at runtime is not supported by this driver.
+ */
+ if (((val & BD71815_BUCK_STBY_DVS))) {
+ return regmap_update_bits(pmic->regmap, regh, BD71815_VOLT_MASK,
+ sel);
+ }
+ /* Update new voltage to the register which is not selected now */
+ if (val & BD71815_BUCK_DVSSEL)
+ reg = regl;
+ else
+ reg = regh;
+
+ ret = regmap_update_bits(pmic->regmap, reg, BD71815_VOLT_MASK, sel);
+ if (ret)
+ return ret;
+
+ /* Select the other DVS register to be used */
+ return regmap_update_bits(pmic->regmap, regh, BD71815_BUCK_DVSSEL, ~val);
+}
+
+static const struct regulator_ops bd7181x_ldo_regulator_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .list_voltage = regulator_list_voltage_linear,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+};
+
+static const struct regulator_ops bd7181x_fixed_regulator_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .list_voltage = regulator_list_voltage_linear,
+};
+
+static const struct regulator_ops bd7181x_buck_regulator_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .list_voltage = regulator_list_voltage_linear,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
+};
+
+static const struct regulator_ops bd7181x_buck12_regulator_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .list_voltage = regulator_list_voltage_linear,
+ .set_voltage_sel = bd7181x_buck12_set_voltage_sel,
+ .get_voltage_sel = bd7181x_buck12_get_voltage_sel,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
+ .set_ramp_delay = bd7181x_buck12_set_ramp_delay,
+};
+
+static const struct regulator_ops bd7181x_led_regulator_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .set_current_limit = bd7181x_led_set_current_limit,
+ .get_current_limit = regulator_get_current_limit_regmap,
+};
+
+#define BD71815_FIXED_REG(_name, _id, ereg, emsk, voltage, _dvs) \
+ [(_id)] = { \
+ .desc = { \
+ .name = #_name, \
+ .of_match = of_match_ptr(#_name), \
+ .regulators_node = of_match_ptr("regulators"), \
+ .n_voltages = 1, \
+ .ops = &bd7181x_fixed_regulator_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .id = (_id), \
+ .owner = THIS_MODULE, \
+ .min_uV = (voltage), \
+ .enable_reg = (ereg), \
+ .enable_mask = (emsk), \
+ .of_parse_cb = set_hw_dvs_levels, \
+ }, \
+ .dvs = (_dvs), \
+ }
+
+#define BD71815_BUCK_REG(_name, _id, vsel, ereg, min, max, step, _dvs) \
+ [(_id)] = { \
+ .desc = { \
+ .name = #_name, \
+ .of_match = of_match_ptr(#_name), \
+ .regulators_node = of_match_ptr("regulators"), \
+ .n_voltages = ((max) - (min)) / (step) + 1, \
+ .ops = &bd7181x_buck_regulator_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .id = (_id), \
+ .owner = THIS_MODULE, \
+ .min_uV = (min), \
+ .uV_step = (step), \
+ .vsel_reg = (vsel), \
+ .vsel_mask = BD71815_VOLT_MASK, \
+ .enable_reg = (ereg), \
+ .enable_mask = BD71815_BUCK_RUN_ON, \
+ .of_parse_cb = set_hw_dvs_levels, \
+ }, \
+ .dvs = (_dvs), \
+ }
+
+#define BD71815_BUCK12_REG(_name, _id, vsel, ereg, min, max, step, \
+ _dvs) \
+ [(_id)] = { \
+ .desc = { \
+ .name = #_name, \
+ .of_match = of_match_ptr(#_name), \
+ .regulators_node = of_match_ptr("regulators"), \
+ .n_voltages = ((max) - (min)) / (step) + 1, \
+ .ops = &bd7181x_buck12_regulator_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .id = (_id), \
+ .owner = THIS_MODULE, \
+ .min_uV = (min), \
+ .uV_step = (step), \
+ .vsel_reg = (vsel), \
+ .vsel_mask = 0x3f, \
+ .enable_reg = (ereg), \
+ .enable_mask = 0x04, \
+ .of_parse_cb = buck12_set_hw_dvs_levels, \
+ }, \
+ .dvs = (_dvs), \
+ }
+
+#define BD71815_LED_REG(_name, _id, csel, mask, ereg, emsk, currents) \
+ [(_id)] = { \
+ .desc = { \
+ .name = #_name, \
+ .of_match = of_match_ptr(#_name), \
+ .regulators_node = of_match_ptr("regulators"), \
+ .n_current_limits = ARRAY_SIZE(currents), \
+ .ops = &bd7181x_led_regulator_ops, \
+ .type = REGULATOR_CURRENT, \
+ .id = (_id), \
+ .owner = THIS_MODULE, \
+ .curr_table = currents, \
+ .csel_reg = (csel), \
+ .csel_mask = (mask), \
+ .enable_reg = (ereg), \
+ .enable_mask = (emsk), \
+ }, \
+ }
+
+#define BD71815_LDO_REG(_name, _id, vsel, ereg, emsk, min, max, step, \
+ _dvs) \
+ [(_id)] = { \
+ .desc = { \
+ .name = #_name, \
+ .of_match = of_match_ptr(#_name), \
+ .regulators_node = of_match_ptr("regulators"), \
+ .n_voltages = ((max) - (min)) / (step) + 1, \
+ .ops = &bd7181x_ldo_regulator_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .id = (_id), \
+ .owner = THIS_MODULE, \
+ .min_uV = (min), \
+ .uV_step = (step), \
+ .vsel_reg = (vsel), \
+ .vsel_mask = BD71815_VOLT_MASK, \
+ .enable_reg = (ereg), \
+ .enable_mask = (emsk), \
+ .of_parse_cb = set_hw_dvs_levels, \
+ }, \
+ .dvs = (_dvs), \
+ }
+
+static struct bd71815_regulator bd71815_regulators[] = {
+ BD71815_BUCK12_REG(buck1, BD71815_BUCK1, BD71815_REG_BUCK1_VOLT_H,
+ BD71815_REG_BUCK1_MODE, 800000, 2000000, 25000,
+ &buck1_dvs),
+ BD71815_BUCK12_REG(buck2, BD71815_BUCK2, BD71815_REG_BUCK2_VOLT_H,
+ BD71815_REG_BUCK2_MODE, 800000, 2000000, 25000,
+ &buck2_dvs),
+ BD71815_BUCK_REG(buck3, BD71815_BUCK3, BD71815_REG_BUCK3_VOLT,
+ BD71815_REG_BUCK3_MODE, 1200000, 2700000, 50000,
+ &buck3_dvs),
+ BD71815_BUCK_REG(buck4, BD71815_BUCK4, BD71815_REG_BUCK4_VOLT,
+ BD71815_REG_BUCK4_MODE, 1100000, 1850000, 25000,
+ &buck4_dvs),
+ BD71815_BUCK_REG(buck5, BD71815_BUCK5, BD71815_REG_BUCK5_VOLT,
+ BD71815_REG_BUCK5_MODE, 1800000, 3300000, 50000,
+ &buck5_dvs),
+ BD71815_LDO_REG(ldo1, BD71815_LDO1, BD71815_REG_LDO1_VOLT,
+ BD71815_REG_LDO_MODE1, LDO1_RUN_ON, 800000, 3300000,
+ 50000, &ldo1_dvs),
+ BD71815_LDO_REG(ldo2, BD71815_LDO2, BD71815_REG_LDO2_VOLT,
+ BD71815_REG_LDO_MODE2, LDO2_RUN_ON, 800000, 3300000,
+ 50000, &ldo2_dvs),
+ /*
+ * Let's default LDO3 to be enabled by SW. We can override ops if DT
+ * says LDO3 should be enabled by HW when DCIN is connected.
+ */
+ BD71815_LDO_REG(ldo3, BD71815_LDO3, BD71815_REG_LDO3_VOLT,
+ BD71815_REG_LDO_MODE2, LDO3_RUN_ON, 800000, 3300000,
+ 50000, &ldo3_dvs),
+ BD71815_LDO_REG(ldo4, BD71815_LDO4, BD71815_REG_LDO4_VOLT,
+ BD71815_REG_LDO_MODE3, LDO4_RUN_ON, 800000, 3300000,
+ 50000, &ldo4_dvs),
+ BD71815_LDO_REG(ldo5, BD71815_LDO5, BD71815_REG_LDO5_VOLT_H,
+ BD71815_REG_LDO_MODE3, LDO5_RUN_ON, 800000, 3300000,
+ 50000, &ldo5_dvs),
+ BD71815_FIXED_REG(ldodvref, BD71815_LDODVREF, BD71815_REG_LDO_MODE4,
+ DVREF_RUN_ON, 3000000, &dvref_dvs),
+ BD71815_FIXED_REG(ldolpsr, BD71815_LDOLPSR, BD71815_REG_LDO_MODE4,
+ LDO_LPSR_RUN_ON, 1800000, &ldolpsr_dvs),
+ BD71815_LED_REG(wled, BD71815_WLED, BD71815_REG_LED_DIMM, LED_DIMM_MASK,
+ BD71815_REG_LED_CTRL, LED_RUN_ON,
+ bd7181x_wled_currents),
+};
+
+static int bd7181x_probe(struct platform_device *pdev)
+{
+ struct bd71815_pmic *pmic;
+ struct regulator_config config = {};
+ int i, ret;
+ struct gpio_desc *ldo4_en;
+
+ pmic = devm_kzalloc(&pdev->dev, sizeof(*pmic), GFP_KERNEL);
+ if (!pmic)
+ return -ENOMEM;
+
+ memcpy(pmic->descs, bd71815_regulators, sizeof(pmic->descs));
+
+ pmic->dev = &pdev->dev;
+ pmic->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!pmic->regmap) {
+ dev_err(pmic->dev, "No parent regmap\n");
+ return -ENODEV;
+ }
+ platform_set_drvdata(pdev, pmic);
+ ldo4_en = devm_gpiod_get_from_of_node(&pdev->dev,
+ pdev->dev.parent->of_node,
+ "rohm,vsel-gpios", 0,
+ GPIOD_ASIS, "ldo4-en");
+
+ if (IS_ERR(ldo4_en)) {
+ ret = PTR_ERR(ldo4_en);
+ if (ret != -ENOENT)
+ return ret;
+ ldo4_en = NULL;
+ }
+
+ /* Disable to go to ship-mode */
+ ret = regmap_update_bits(pmic->regmap, BD71815_REG_PWRCTRL,
+ RESTARTEN, 0);
+ if (ret)
+ return ret;
+
+ config.dev = pdev->dev.parent;
+ config.regmap = pmic->regmap;
+
+ for (i = 0; i < BD71815_REGULATOR_CNT; i++) {
+ struct regulator_desc *desc;
+ struct regulator_dev *rdev;
+
+ desc = &pmic->descs[i].desc;
+ if (i == BD71815_LDO4)
+ config.ena_gpiod = ldo4_en;
+
+ config.driver_data = pmic;
+
+ rdev = devm_regulator_register(&pdev->dev, desc, &config);
+ if (IS_ERR(rdev)) {
+ dev_err(&pdev->dev,
+ "failed to register %s regulator\n",
+ desc->name);
+ return PTR_ERR(rdev);
+ }
+ config.ena_gpiod = NULL;
+ pmic->rdev[i] = rdev;
+ }
+ return 0;
+}
+
+static const struct platform_device_id bd7181x_pmic_id[] = {
+ { "bd71815-pmic", ROHM_CHIP_TYPE_BD71815 },
+ { },
+};
+MODULE_DEVICE_TABLE(platform, bd7181x_pmic_id);
+
+static struct platform_driver bd7181x_regulator = {
+ .driver = {
+ .name = "bd7181x-pmic",
+ .owner = THIS_MODULE,
+ },
+ .probe = bd7181x_probe,
+ .id_table = bd7181x_pmic_id,
+};
+module_platform_driver(bd7181x_regulator);
+
+MODULE_AUTHOR("Tony Luo <luofc@embedinfo.com>");
+MODULE_DESCRIPTION("BD71815 voltage regulator driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:bd7181x-pmic");
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 14/16] clk: bd718x7: Add support for clk gate on ROHM BD71815 PMIC
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
` (11 preceding siblings ...)
2021-03-24 7:30 ` [PATCH v4 13/16] regulator: Support ROHM BD71815 regulators Matti Vaittinen
@ 2021-03-24 7:30 ` Matti Vaittinen
2021-03-24 7:31 ` [PATCH v4 15/16] rtc: bd70528: Support RTC on ROHM BD71815 Matti Vaittinen
2021-03-24 7:31 ` [PATCH v4 16/16] MAINTAINERS: Add ROHM BD71815AGW Matti Vaittinen
14 siblings, 0 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:30 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Matti Vaittinen, Michael Turquette, Stephen Boyd,
linux-kernel, linux-power, linux-clk
ROHM BD71815 also provide clk signal for RTC. Add control
for gating this clock.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
---
Changes since v3:
- No changes
drivers/clk/clk-bd718x7.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk-bd718x7.c b/drivers/clk/clk-bd718x7.c
index 17d90e09f1c0..d9e70e506d18 100644
--- a/drivers/clk/clk-bd718x7.c
+++ b/drivers/clk/clk-bd718x7.c
@@ -13,6 +13,8 @@
#include <linux/regmap.h>
/* clk control registers */
+/* BD71815 */
+#define BD71815_REG_OUT32K 0x1d
/* BD70528 */
#define BD70528_REG_OUT32K 0x2c
/* BD71828 */
@@ -118,6 +120,10 @@ static int bd71837_clk_probe(struct platform_device *pdev)
c->reg = BD70528_REG_OUT32K;
c->mask = CLK_OUT_EN_MASK;
break;
+ case ROHM_CHIP_TYPE_BD71815:
+ c->reg = BD71815_REG_OUT32K;
+ c->mask = CLK_OUT_EN_MASK;
+ break;
default:
dev_err(&pdev->dev, "Unknown clk chip\n");
return -EINVAL;
@@ -146,6 +152,7 @@ static const struct platform_device_id bd718x7_clk_id[] = {
{ "bd71847-clk", ROHM_CHIP_TYPE_BD71847 },
{ "bd70528-clk", ROHM_CHIP_TYPE_BD70528 },
{ "bd71828-clk", ROHM_CHIP_TYPE_BD71828 },
+ { "bd71815-clk", ROHM_CHIP_TYPE_BD71815 },
{ },
};
MODULE_DEVICE_TABLE(platform, bd718x7_clk_id);
@@ -161,6 +168,6 @@ static struct platform_driver bd71837_clk = {
module_platform_driver(bd71837_clk);
MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
-MODULE_DESCRIPTION("BD71837/BD71847/BD70528 chip clk driver");
+MODULE_DESCRIPTION("BD718(15/18/28/37/47/50) and BD70528 chip clk driver");
MODULE_LICENSE("GPL");
MODULE_ALIAS("platform:bd718xx-clk");
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 15/16] rtc: bd70528: Support RTC on ROHM BD71815
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
` (12 preceding siblings ...)
2021-03-24 7:30 ` [PATCH v4 14/16] clk: bd718x7: Add support for clk gate on ROHM BD71815 PMIC Matti Vaittinen
@ 2021-03-24 7:31 ` Matti Vaittinen
2021-03-24 7:31 ` [PATCH v4 16/16] MAINTAINERS: Add ROHM BD71815AGW Matti Vaittinen
14 siblings, 0 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:31 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Matti Vaittinen, Alessandro Zummo, Alexandre Belloni,
linux-kernel, linux-power, linux-rtc
BD71815 contains similar RTC block as BD71828. Only the address offsets
seem different. Support also BD71815 RTC using rtc-bd70528.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
Changes since v3:
- No changes
drivers/rtc/Kconfig | 6 +++---
drivers/rtc/rtc-bd70528.c | 45 ++++++++++++++++++++++++++++++++-------
2 files changed, 40 insertions(+), 11 deletions(-)
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index ce723dc54aa4..622af1314ece 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -501,11 +501,11 @@ config RTC_DRV_M41T80_WDT
watchdog timer in the ST M41T60 and M41T80 RTC chips series.
config RTC_DRV_BD70528
- tristate "ROHM BD70528 PMIC RTC"
- depends on MFD_ROHM_BD70528 && (BD70528_WATCHDOG || !BD70528_WATCHDOG)
+ tristate "ROHM BD70528, BD71815 and BD71828 PMIC RTC"
+ depends on MFD_ROHM_BD71828 || MFD_ROHM_BD70528 && (BD70528_WATCHDOG || !BD70528_WATCHDOG)
help
If you say Y here you will get support for the RTC
- block on ROHM BD70528 and BD71828 Power Management IC.
+ block on ROHM BD70528, BD71815 and BD71828 Power Management IC.
This driver can also be built as a module. If so, the module
will be called rtc-bd70528.
diff --git a/drivers/rtc/rtc-bd70528.c b/drivers/rtc/rtc-bd70528.c
index fb4476bb5ab6..6454afca02a6 100644
--- a/drivers/rtc/rtc-bd70528.c
+++ b/drivers/rtc/rtc-bd70528.c
@@ -6,6 +6,7 @@
#include <linux/bcd.h>
#include <linux/mfd/rohm-bd70528.h>
+#include <linux/mfd/rohm-bd71815.h>
#include <linux/mfd/rohm-bd71828.h>
#include <linux/module.h>
#include <linux/of.h>
@@ -13,6 +14,12 @@
#include <linux/regmap.h>
#include <linux/rtc.h>
+/*
+ * On BD71828 and BD71815 the ALM0 MASK is 14 bytes after the ALM0
+ * block start
+ */
+#define BD718XX_ALM_EN_OFFSET 14
+
/*
* We read regs RTC_SEC => RTC_YEAR
* this struct is ordered according to chip registers.
@@ -55,6 +62,7 @@ struct bd70528_rtc {
struct regmap *regmap;
struct device *dev;
u8 reg_time_start;
+ u8 bd718xx_alm_block_start;
bool has_rtc_timers;
};
@@ -236,8 +244,8 @@ static int bd71828_set_alarm(struct device *dev, struct rtc_wkalrm *a)
struct bd71828_rtc_alm alm;
struct bd70528_rtc *r = dev_get_drvdata(dev);
- ret = regmap_bulk_read(r->regmap, BD71828_REG_RTC_ALM_START,
- &alm, sizeof(alm));
+ ret = regmap_bulk_read(r->regmap, r->bd718xx_alm_block_start, &alm,
+ sizeof(alm));
if (ret) {
dev_err(dev, "Failed to read alarm regs\n");
return ret;
@@ -250,8 +258,8 @@ static int bd71828_set_alarm(struct device *dev, struct rtc_wkalrm *a)
else
alm.alm_mask |= BD70528_MASK_ALM_EN;
- ret = regmap_bulk_write(r->regmap, BD71828_REG_RTC_ALM_START,
- &alm, sizeof(alm));
+ ret = regmap_bulk_write(r->regmap, r->bd718xx_alm_block_start, &alm,
+ sizeof(alm));
if (ret)
dev_err(dev, "Failed to set alarm time\n");
@@ -311,8 +319,8 @@ static int bd71828_read_alarm(struct device *dev, struct rtc_wkalrm *a)
struct bd71828_rtc_alm alm;
struct bd70528_rtc *r = dev_get_drvdata(dev);
- ret = regmap_bulk_read(r->regmap, BD71828_REG_RTC_ALM_START,
- &alm, sizeof(alm));
+ ret = regmap_bulk_read(r->regmap, r->bd718xx_alm_block_start, &alm,
+ sizeof(alm));
if (ret) {
dev_err(dev, "Failed to read alarm regs\n");
return ret;
@@ -453,8 +461,9 @@ static int bd71828_alm_enable(struct device *dev, unsigned int enabled)
if (!enabled)
enableval = 0;
- ret = regmap_update_bits(r->regmap, BD71828_REG_RTC_ALM0_MASK,
- BD70528_MASK_ALM_EN, enableval);
+ ret = regmap_update_bits(r->regmap, r->bd718xx_alm_block_start +
+ BD718XX_ALM_EN_OFFSET, BD70528_MASK_ALM_EN,
+ enableval);
if (ret)
dev_err(dev, "Failed to change alarm state\n");
@@ -524,9 +533,28 @@ static int bd70528_probe(struct platform_device *pdev)
enable_main_irq = true;
rtc_ops = &bd70528_rtc_ops;
break;
+ case ROHM_CHIP_TYPE_BD71815:
+ irq_name = "bd71815-rtc-alm-0";
+ bd_rtc->reg_time_start = BD71815_REG_RTC_START;
+
+ /*
+ * See also BD718XX_ALM_EN_OFFSET:
+ * This works for BD71828 and BD71815 as they have same offset
+ * between ALM0 start and ALM0_MASK. If new ICs are to be
+ * added this requires proper check as ALM0_MASK is not located
+ * at the end of ALM0 block - but after all ALM blocks so if
+ * amount of ALMs differ the offset to enable/disable is likely
+ * to be incorrect and enable/disable must be given as own
+ * reg address here.
+ */
+ bd_rtc->bd718xx_alm_block_start = BD71815_REG_RTC_ALM_START;
+ hour_reg = BD71815_REG_HOUR;
+ rtc_ops = &bd71828_rtc_ops;
+ break;
case ROHM_CHIP_TYPE_BD71828:
irq_name = "bd71828-rtc-alm-0";
bd_rtc->reg_time_start = BD71828_REG_RTC_START;
+ bd_rtc->bd718xx_alm_block_start = BD71828_REG_RTC_ALM_START;
hour_reg = BD71828_REG_RTC_HOUR;
rtc_ops = &bd71828_rtc_ops;
break;
@@ -605,6 +633,7 @@ static int bd70528_probe(struct platform_device *pdev)
static const struct platform_device_id bd718x7_rtc_id[] = {
{ "bd70528-rtc", ROHM_CHIP_TYPE_BD70528 },
{ "bd71828-rtc", ROHM_CHIP_TYPE_BD71828 },
+ { "bd71815-rtc", ROHM_CHIP_TYPE_BD71815 },
{ },
};
MODULE_DEVICE_TABLE(platform, bd718x7_rtc_id);
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 16/16] MAINTAINERS: Add ROHM BD71815AGW
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
` (13 preceding siblings ...)
2021-03-24 7:31 ` [PATCH v4 15/16] rtc: bd70528: Support RTC on ROHM BD71815 Matti Vaittinen
@ 2021-03-24 7:31 ` Matti Vaittinen
14 siblings, 0 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-24 7:31 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Liam Girdwood, Mark Brown,
Matti Vaittinen, Michael Turquette, Stephen Boyd, Linus Walleij,
Bartosz Golaszewski, Alessandro Zummo, Alexandre Belloni,
devicetree, linux-kernel, linux-power, linux-clk, linux-gpio,
linux-rtc
Add maintainer entries for ROHM BD71815AGW drivers.
New regulator and GPIO drivers were introduced for these PMICs.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
Changes since v3:
- No changes
MAINTAINERS | 3 +++
1 file changed, 3 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 9e876927c60d..c251af6bfc03 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15452,18 +15452,21 @@ F: Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt
F: Documentation/devicetree/bindings/regulator/rohm,bd70528-regulator.txt
F: drivers/clk/clk-bd718x7.c
F: drivers/gpio/gpio-bd70528.c
+F: drivers/gpio/gpio-bd71815.c
F: drivers/gpio/gpio-bd71828.c
F: drivers/mfd/rohm-bd70528.c
F: drivers/mfd/rohm-bd71828.c
F: drivers/mfd/rohm-bd718x7.c
F: drivers/power/supply/bd70528-charger.c
F: drivers/regulator/bd70528-regulator.c
+F: drivers/regulator/bd71815-regulator.c
F: drivers/regulator/bd71828-regulator.c
F: drivers/regulator/bd718x7-regulator.c
F: drivers/regulator/rohm-regulator.c
F: drivers/rtc/rtc-bd70528.c
F: drivers/watchdog/bd70528_wdt.c
F: include/linux/mfd/rohm-bd70528.h
+F: include/linux/mfd/rohm-bd71815.h
F: include/linux/mfd/rohm-bd71828.h
F: include/linux/mfd/rohm-bd718x7.h
F: include/linux/mfd/rohm-generic.h
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs
2021-03-24 7:29 ` [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs Matti Vaittinen
@ 2021-03-25 9:35 ` Linus Walleij
2021-03-25 10:32 ` Matti Vaittinen
2021-03-26 11:26 ` [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs Andy Shevchenko
1 sibling, 1 reply; 28+ messages in thread
From: Linus Walleij @ 2021-03-25 9:35 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Lee Jones, Rob Herring, Bartosz Golaszewski,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel, linux-power, open list:GPIO SUBSYSTEM
On Wed, Mar 24, 2021 at 8:29 AM Matti Vaittinen
<matti.vaittinen@fi.rohmeurope.com> wrote:
> Support GPO(s) found from ROHM BD71815 power management IC. The IC has two
> GPO pins but only one is properly documented in data-sheet. The driver
> exposes by default only the documented GPO. The second GPO is connected to
> E5 pin and is marked as GND in data-sheet. Control for this undocumented
> pin can be enabled using a special DT property.
>
> This driver is derived from work by Peter Yang <yanglsh@embest-tech.com>
> although not so much of original is left.
>
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
> Changes since v3:
> - No changes
This looks OK to me:
Acked-by: Linus Walleij <linus.walleij@linaro.org>
It could potentially (like the other Rohm GPIO MFD PMIC drivers)
make some use of the gpio regmap library, but we have some
pending changes for that so look into it after the next merge
window.
I.e. for your TODO: look at the GPIO_REGMAP helper.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs
2021-03-25 9:35 ` Linus Walleij
@ 2021-03-25 10:32 ` Matti Vaittinen
2021-05-10 12:58 ` regmap-gpio: Support set_config and other not quite so standard ICs? Matti Vaittinen
0 siblings, 1 reply; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-25 10:32 UTC (permalink / raw)
To: Linus Walleij
Cc: Lee Jones, Rob Herring, Bartosz Golaszewski,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel, linux-power, open list:GPIO SUBSYSTEM
Hello Linus,
On Thu, 2021-03-25 at 10:35 +0100, Linus Walleij wrote:
> On Wed, Mar 24, 2021 at 8:29 AM Matti Vaittinen
> <matti.vaittinen@fi.rohmeurope.com> wrote:
>
> > Support GPO(s) found from ROHM BD71815 power management IC. The IC
> > has two
> > GPO pins but only one is properly documented in data-sheet. The
> > driver
> > exposes by default only the documented GPO. The second GPO is
> > connected to
> > E5 pin and is marked as GND in data-sheet. Control for this
> > undocumented
> > pin can be enabled using a special DT property.
> >
> > This driver is derived from work by Peter Yang <
> > yanglsh@embest-tech.com>
> > although not so much of original is left.
> >
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > ---
> > Changes since v3:
> > - No changes
>
> This looks OK to me:
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> It could potentially (like the other Rohm GPIO MFD PMIC drivers)
> make some use of the gpio regmap library, but we have some
> pending changes for that so look into it after the next merge
> window.
>
> I.e. for your TODO: look at the GPIO_REGMAP helper.
I just took a quick peek at gpio_regmap and it looks pretty good to me!
Any particular reason why gpio_regmap is not just part of gpio_chip? I
guess providing the 'gpio_regmap_direction_*()', 'gpio_regmap_get()',
'gpio_regmap_set()' as exported helpers and leaving calling the
(devm_)gpiochip_add_data() to IC driver would have allowed more
flexibility. Drivers could then use the gpio_regamap features which fit
the IC (by providing pointers to helper functions in gpio_chip) - and
handle potential oddball-features by using pointers to some customized
functions in gpio_chip.
Anyways, definitely worth getting familiar with! Thanks for the pointer
:]
Best Regards,
Matti Vaittinen
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs
2021-03-24 7:29 ` [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs Matti Vaittinen
2021-03-25 9:35 ` Linus Walleij
@ 2021-03-26 11:26 ` Andy Shevchenko
2021-03-26 11:27 ` Andy Shevchenko
2021-03-26 13:33 ` Matti Vaittinen
1 sibling, 2 replies; 28+ messages in thread
From: Andy Shevchenko @ 2021-03-26 11:26 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Lee Jones, Rob Herring, Linus Walleij,
Bartosz Golaszewski, devicetree, Linux Kernel Mailing List,
linux-power, open list:GPIO SUBSYSTEM
On Wed, Mar 24, 2021 at 12:20 PM Matti Vaittinen
<matti.vaittinen@fi.rohmeurope.com> wrote:
>
> Support GPO(s) found from ROHM BD71815 power management IC. The IC has two
> GPO pins but only one is properly documented in data-sheet. The driver
in the datasheet
> exposes by default only the documented GPO. The second GPO is connected to
> E5 pin and is marked as GND in data-sheet. Control for this undocumented
in the datasheet
> pin can be enabled using a special DT property.
>
> This driver is derived from work by Peter Yang <yanglsh@embest-tech.com>
> although not so much of original is left.
of the original
Below my comments independently on the fact if this driver will be
completely rewritten, consider them as a good practice for your new
contribution.
...
> +/*
> + * Support to GPOs on ROHM BD71815
> + */
This is effectively one line.
...
> +/* For the BD71815 register definitions */
> +#include <linux/mfd/rohm-bd71815.h>
Since it's component specific header(s) I would move it to a separate
group and locate...
> +#include <linux/module.h>
> +#include <linux/of.h>
You may do better than be OF-centric. See below.
> +#include <linux/platform_device.h>
> +
...somewhere here.
...
> + /*
> + * Sigh. The BD71815 and BD71817 were originally designed to support two
> + * GPO pins. At some point it was noticed the second GPO pin which is
> + * the E5 pin located at the center of IC is hard to use on PCB (due to
> + * the location). It was decided to not promote this second GPO and pin
> + * is marked as GND on the data-sheet. The functionality is still there
> + * though! I guess driving GPO connected to ground is a bad idea. Thus
a GPO
to the ground
> + * we do not support it by default. OTOH - the original driver written
> + * by colleagues at Embest did support controlling this second GPO. It
> + * is thus possible this is used in some of the products.
> + *
> + * This driver does not by default support configuring this second GPO
> + * but allows using it by providing the DT property
> + * "rohm,enable-hidden-gpo".
> + */
...
> + int ret = 0;
Redundant assignment.
> + int val;
> +
> + ret = regmap_read(bd71815->regmap, BD71815_REG_GPO, &val);
> + if (ret)
> + return ret;
> + return (val >> offset) & 1;
!!(val & BIT(offset)) can also work and be in alignment with the below code.
...
> + if (!bd71815->e5_pin_is_gpo && offset)
> + return;
I wonder if you can use valid_mask instead of this.
...
> + bit = BIT(offset);
Can be moved to the definition block.
...
> + if (!bdgpio->e5_pin_is_gpo && offset)
> + return -EOPNOTSUPP;
As above.
...
> + default:
> + break;
> + }
> + return -EOPNOTSUPP;
You may return directly from default.
...
> + int ret;
> + struct bd71815_gpio *g;
> + struct device *dev;
> + struct device *parent;
Reversed xmas tree order.
...
> + /*
> + * Bind devm lifetime to this platform device => use dev for devm.
> + * also the prints should originate from this device.
> + */
Why is this comment needed?
...
> + dev = &pdev->dev;
Can be done in the definition block.
...
> + /* The device-tree and regmap come from MFD => use parent for that */
Why do you need this comment?
> + parent = dev->parent;
Ditto, can be moved to the definition block.
...
> + g->e5_pin_is_gpo = of_property_read_bool(parent->of_node,
> + "rohm,enable-hidden-gpo");
You may use device_property_read_bool().
...
> + g->chip.of_node = parent->of_node;
Redundant. GPIO library does it for you and even better.
...
> + ret = devm_gpiochip_add_data(dev, &g->chip, g);
> + if (ret < 0) {
> + dev_err(dev, "could not register gpiochip, %d\n", ret);
> + return ret;
> + }
> +
> + return ret;
It's as simply as
return devm_gpiochip_add_data(...);
...
> +static const struct platform_device_id bd7181x_gpo_id[] = {
> + { "bd71815-gpo" },
> + { },
No comma for the terminator line.
> +};
> +MODULE_DEVICE_TABLE(platform, bd7181x_gpo_id);
Why do you need this ID table exactly?
You have the same name as in the platform driver structure below.
> +static struct platform_driver gpo_bd71815_driver = {
> + .driver = {
> + .name = "bd71815-gpo",
> + .owner = THIS_MODULE,
This is done by module_*_driver() macros, drop it.
> + },
> + .probe = gpo_bd71815_probe,
> + .id_table = bd7181x_gpo_id,
> +};
> +
Extra blank line.
> +module_platform_driver(gpo_bd71815_driver);
> +/* Note: this hardware lives inside an I2C-based multi-function device. */
> +MODULE_ALIAS("platform:bd71815-gpo");
> +
Ditto.
> +MODULE_AUTHOR("Peter Yang <yanglsh@embest-tech.com>");
And I don't see a match with a committer/submitter/co-developer/etc.
Please, make corresponding fields and this macro (or macros, you may
have as many MODULE_AUTHOR() entries as developers of the code)
aligned to each other.
> +MODULE_DESCRIPTION("GPO interface for BD71815");
> +MODULE_LICENSE("GPL");
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs
2021-03-26 11:26 ` [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs Andy Shevchenko
@ 2021-03-26 11:27 ` Andy Shevchenko
2021-03-26 13:33 ` Matti Vaittinen
1 sibling, 0 replies; 28+ messages in thread
From: Andy Shevchenko @ 2021-03-26 11:27 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Lee Jones, Rob Herring, Linus Walleij,
Bartosz Golaszewski, devicetree, Linux Kernel Mailing List,
linux-power, open list:GPIO SUBSYSTEM
On Fri, Mar 26, 2021 at 1:26 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Mar 24, 2021 at 12:20 PM Matti Vaittinen
> <matti.vaittinen@fi.rohmeurope.com> wrote:
> > +#include <linux/of.h>
>
> You may do better than be OF-centric. See below.
Ah, yep, when you switch to unified device property API, you would
need property.h and mod_devicetable.h instead of this.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs
2021-03-26 11:26 ` [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs Andy Shevchenko
2021-03-26 11:27 ` Andy Shevchenko
@ 2021-03-26 13:33 ` Matti Vaittinen
2021-03-26 17:51 ` Andy Shevchenko
1 sibling, 1 reply; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-26 13:33 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Lee Jones, Rob Herring, Linus Walleij, Bartosz Golaszewski,
devicetree, Linux Kernel Mailing List, linux-power,
open list:GPIO SUBSYSTEM
Hi Andy,
On Fri, 2021-03-26 at 13:26 +0200, Andy Shevchenko wrote:
> On Wed, Mar 24, 2021 at 12:20 PM Matti Vaittinen
> <matti.vaittinen@fi.rohmeurope.com> wrote:
> > Support GPO(s) found from ROHM BD71815 power management IC. The IC
> > has two
> > GPO pins but only one is properly documented in data-sheet. The
> > driver
>
> in the datasheet
>
> > exposes by default only the documented GPO. The second GPO is
> > connected to
> > E5 pin and is marked as GND in data-sheet. Control for this
> > undocumented
>
> in the datasheet
>
> > pin can be enabled using a special DT property.
> >
> > This driver is derived from work by Peter Yang <
> > yanglsh@embest-tech.com>
> > although not so much of original is left.
>
> of the original
>
> Below my comments independently on the fact if this driver will be
> completely rewritten, consider them as a good practice for your new
> contribution.
Thank you for the review. I appreciate your help although I don't
always agree necessity regarding all of the styling suggestions :) I
did not respond here to the suggestions I agreed with.
As Linus pointed out, few of the ROHM drivers should be revised for
regmap_gpio usage in near future. I will definitely go through all the
comments at that point if there is no reason to respin series earlier.
> + int val;
> > +
> > + ret = regmap_read(bd71815->regmap, BD71815_REG_GPO, &val);
> > + if (ret)
> > + return ret;
> > + return (val >> offset) & 1;
>
> !!(val & BIT(offset)) can also work and be in alignment with the
> below code.
This is an opinion, but to me !!(val & BIT(offset)) looks more
confusing. I don't see the benefit from the change.
>
> ...
>
> > + if (!bd71815->e5_pin_is_gpo && offset)
> > + return;
>
> I wonder if you can use valid_mask instead of this.
Do you mean re-naming the e5_pin_is_gpo to valid_mask? Or do you mean
some GPIO framework internal feature allowing to define valid pins? (If
my memory serves me right one can set invalid pins from DT - but by
default all pins are valid and here we want to invalidate a pin by
default). For renaming I don't see the value - if internal feature can
be used then there may be value. Thanks for the pointer, I'll look what
I find.
>
> ...
>
> > + bit = BIT(offset);
>
> Can be moved to the definition block.
I don't like doing the assignment before we check if the operation is
valid. And, making assignments which are not plain constants in
declaration make reading the declaration much harder.
> ...
>
> > + default:
> > + break;
> > + }
> > + return -EOPNOTSUPP;
>
> You may return directly from default.
I think there used to be compilers which didn't like having the return
inside a block - even if the block was a default. I also prefer seeing
return at the end of function which should return a value.
>
> ...
>
> > + int ret;
> > + struct bd71815_gpio *g;
> > + struct device *dev;
> > + struct device *parent;
>
> Reversed xmas tree order.
What is the added value here? I understand alphabetical sorting - it
helps looking if particular entry is included. I also understand type-
based sorting. But reverse Xmas tree? I thin I have heard it eases
reading declarations - which is questionable in this case. Double so
when you also suggest moving assignments to declaration block which
makes it _much_ harder to read? I won't change this unless it is
mandated by the maintainers.
>
> ...
>
> > + /*
> > + * Bind devm lifetime to this platform device => use dev
> > for devm.
> > + * also the prints should originate from this device.
> > + */
>
> Why is this comment needed?
>
>
> > + /* The device-tree and regmap come from MFD => use parent
> > for that */
>
> Why do you need this comment?
>
> > + parent = dev->parent;
It is not always obvious (especially for someone not reading MFD driver
code frequently) why we use parent device for some things and the
device being probed to some other stuff. Typically this is not needed
if the device is not MFD sub-device. And again, the comments in the
middle of declaration block look confusing to me. I think removing
comments and moving these to declaration make readability _much_ worse.
> ...
>
> > + g->e5_pin_is_gpo = of_property_read_bool(parent->of_node,
> > + "rohm,enable-
> > hidden-gpo");
>
> You may use device_property_read_bool().
Out of the curiosity - is there any other reason but ACPI? ACPI support
can be added later if needed. I still think you're correct. This is
definitely one of the points that fall in the category of things "I
must consider as a good practice for (my) new contribution". So I try
to keep this in mind in the future.
> > + g->chip.of_node = parent->of_node;
>
> Redundant. GPIO library does it for you and even better.
So I can nowadays just omit this? Thanks!
> > + ret = devm_gpiochip_add_data(dev, &g->chip, g);
> > + if (ret < 0) {
> > + dev_err(dev, "could not register gpiochip, %d\n",
> > ret);
> > + return ret;
> > + }
> > +
> > + return ret;
>
> It's as simply as
> return devm_gpiochip_add_data(...);
Hm. I think you're right. The print does not add much value here.
Thanks.
>
> ...
>
> > +static const struct platform_device_id bd7181x_gpo_id[] = {
> > + { "bd71815-gpo" },
> > + { },
>
> No comma for the terminator line.
>
> > +};
> > +MODULE_DEVICE_TABLE(platform, bd7181x_gpo_id);
>
> Why do you need this ID table exactly?
> You have the same name as in the platform driver structure below.
This driver was also supporting another IC (BD71817) - but as far as I
know the BD71817 is no longer used too much so I dropped it. The ID
table was left with this one entry only. I will see if this is any more
needed. Thanks.
>
> > +static struct platform_driver gpo_bd71815_driver = {
> > + .driver = {
> > + .name = "bd71815-gpo",
> > + .owner = THIS_MODULE,
>
> This is done by module_*_driver() macros, drop it.
>
> > + },
> > + .probe = gpo_bd71815_probe,
> > + .id_table = bd7181x_gpo_id,
> > +};
> > +
>
> > +MODULE_AUTHOR("Peter Yang <yanglsh@embest-tech.com>");
>
> And I don't see a match with a committer/submitter/co-developer/etc.
> Please, make corresponding fields and this macro (or macros, you may
> have as many MODULE_AUTHOR() entries as developers of the code)
> aligned to each other.
I never knew there could be many MODULE_AUTHOR() entries. Thanks for
pointing it out.
>
> > +MODULE_DESCRIPTION("GPO interface for BD71815");
> > +MODULE_LICENSE("GPL");
Best Regards
--Matti
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs
2021-03-26 13:33 ` Matti Vaittinen
@ 2021-03-26 17:51 ` Andy Shevchenko
2021-03-28 16:59 ` Matti Vaittinen
0 siblings, 1 reply; 28+ messages in thread
From: Andy Shevchenko @ 2021-03-26 17:51 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Lee Jones, Rob Herring, Linus Walleij, Bartosz Golaszewski,
devicetree, Linux Kernel Mailing List, linux-power,
open list:GPIO SUBSYSTEM
On Fri, Mar 26, 2021 at 3:33 PM Matti Vaittinen
<matti.vaittinen@fi.rohmeurope.com> wrote:
> On Fri, 2021-03-26 at 13:26 +0200, Andy Shevchenko wrote:
> > On Wed, Mar 24, 2021 at 12:20 PM Matti Vaittinen
> > <matti.vaittinen@fi.rohmeurope.com> wrote:
...
> > > + return (val >> offset) & 1;
> >
> > !!(val & BIT(offset)) can also work and be in alignment with the
> > below code.
>
> This is an opinion, but to me !!(val & BIT(offset)) looks more
> confusing. I don't see the benefit from the change.
I always try to find a compromise between two: your own style and
common practice used in the subsystem in question. AFAIR my proposal
is (recommended?) style for new code.
...
> > > + if (!bd71815->e5_pin_is_gpo && offset)
> > > + return;
> >
> > I wonder if you can use valid_mask instead of this.
>
> Do you mean re-naming the e5_pin_is_gpo to valid_mask? Or do you mean
> some GPIO framework internal feature allowing to define valid pins? (If
> my memory serves me right one can set invalid pins from DT - but by
> default all pins are valid and here we want to invalidate a pin by
> default). For renaming I don't see the value - if internal feature can
> be used then there may be value. Thanks for the pointer, I'll look what
> I find.
I mean to utilize internal valid_mask bitmap. Yes, you may fill it as
valid at the start of the driver and then simply call __clear_bit() /
clear_bit() against one you wanted to disable. Then core will take
care of the rest (AFAIR).
...
> > > + bit = BIT(offset);
> >
> > Can be moved to the definition block.
>
> I don't like doing the assignment before we check if the operation is
> valid. And, making assignments which are not plain constants in
> declaration make reading the declaration much harder.
OK.
...
> > > + default:
> > > + break;
> > > + }
> > > + return -EOPNOTSUPP;
> >
> > You may return directly from default.
>
> I think there used to be compilers which didn't like having the return
> inside a block - even if the block was a default. I also prefer seeing
> return at the end of function which should return a value.
I prefer less LOCs in the file when it makes sense. And here you seem
appealing to compilers from last century.
...
> > > + int ret;
> > > + struct bd71815_gpio *g;
> > > + struct device *dev;
> > > + struct device *parent;
> >
> > Reversed xmas tree order.
>
> What is the added value here? I understand alphabetical sorting - it
> helps looking if particular entry is included. I also understand type-
> based sorting. But reverse Xmas tree? I thin I have heard it eases
> reading declarations - which is questionable in this case. Double so
> when you also suggest moving assignments to declaration block which
> makes it _much_ harder to read? I won't change this unless it is
> mandated by the maintainers.
Compare to:
struct bd71815_gpio *g;
struct device *parent;
struct device *dev;
int ret;
It's easier to read, esp. taking into account that ret is going last.
It seems to me more natural, so we have a disagreement here, but I'm
not a maintainer, it's up to them.
...
> > > + parent = dev->parent;
>
> It is not always obvious (especially for someone not reading MFD driver
> code frequently) why we use parent device for some things and the
> device being probed to some other stuff. Typically this is not needed
> if the device is not MFD sub-device. And again, the comments in the
> middle of declaration block look confusing to me. I think removing
> comments and moving these to declaration make readability _much_ worse.
I disagree with you here again. To me it's like completely unneeded churn.
...
> > > + g->e5_pin_is_gpo = of_property_read_bool(parent->of_node,
> > > + "rohm,enable-
> > > hidden-gpo");
> >
> > You may use device_property_read_bool().
>
> Out of the curiosity - is there any other reason but ACPI?
We might have another property provider (by the fact we already have
the third one, but it's a specific one, called software nodes).
> ACPI support
> can be added later if needed.
Yes, but doing something OF centric which might have been used on
non-OF platforms is to do double effort and waste time and resources.
> I still think you're correct. This is
> definitely one of the points that fall in the category of things "I
> must consider as a good practice for (my) new contribution". So I try
> to keep this in mind in the future.
...
> > > + g->chip.of_node = parent->of_node;
> >
> > Redundant. GPIO library does it for you and even better.
>
> So I can nowadays just omit this? Thanks!
For a long time. I haven't checked the date when it started like this,
but couple of years sounds like a good approximation.
...
> > > +MODULE_DEVICE_TABLE(platform, bd7181x_gpo_id);
> >
> > Why do you need this ID table exactly?
> > You have the same name as in the platform driver structure below.
>
> This driver was also supporting another IC (BD71817) - but as far as I
> know the BD71817 is no longer used too much so I dropped it. The ID
> table was left with this one entry only. I will see if this is any more
> needed. Thanks.
Yes, but in that case you have to have driver data to differentiate
the chips, right? Otherwise for platform drivers this makes a little
sense b/c it effectively repeats .name from gpo_bd71815_driver.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs
2021-03-26 17:51 ` Andy Shevchenko
@ 2021-03-28 16:59 ` Matti Vaittinen
0 siblings, 0 replies; 28+ messages in thread
From: Matti Vaittinen @ 2021-03-28 16:59 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Lee Jones, Rob Herring, Linus Walleij, Bartosz Golaszewski,
devicetree, Linux Kernel Mailing List, linux-power,
open list:GPIO SUBSYSTEM
Hi Again Andy,
On Fri, 2021-03-26 at 19:51 +0200, Andy Shevchenko wrote:
> On Fri, Mar 26, 2021 at 3:33 PM Matti Vaittinen
> <matti.vaittinen@fi.rohmeurope.com> wrote:
> > On Fri, 2021-03-26 at 13:26 +0200, Andy Shevchenko wrote:
> > > On Wed, Mar 24, 2021 at 12:20 PM Matti Vaittinen
> > > <matti.vaittinen@fi.rohmeurope.com> wrote:
>
>
> > > > + if (!bd71815->e5_pin_is_gpo && offset)
> > > > + return;
> > >
> > > I wonder if you can use valid_mask instead of this.
> >
> > Do you mean re-naming the e5_pin_is_gpo to valid_mask? Or do you
> > mean
> > some GPIO framework internal feature allowing to define valid pins?
> > (If
> > my memory serves me right one can set invalid pins from DT - but by
> > default all pins are valid and here we want to invalidate a pin by
> > default). For renaming I don't see the value - if internal feature
> > can
> > be used then there may be value. Thanks for the pointer, I'll look
> > what
> > I find.
>
> I mean to utilize internal valid_mask bitmap. Yes, you may fill it as
> valid at the start of the driver and then simply call __clear_bit() /
> clear_bit() against one you wanted to disable. Then core will take
> care of the rest (AFAIR).
Right. I see there is the init_valid_mask callback which could be
populated. OTOH, I think this check is actually not needed at all if we
set the ngpios based on the DT flag. The check in set/get functions was
just a symptom of my paranoia. Anyways, I owe you as I just learned
something new :)
> > > > + int ret;
> > > > + struct bd71815_gpio *g;
> > > > + struct device *dev;
> > > > + struct device *parent;
> ...
>
> > > > + parent = dev->parent;
> >
> > It is not always obvious (especially for someone not reading MFD
> > driver
> > code frequently) why we use parent device for some things and the
> > device being probed to some other stuff. Typically this is not
> > needed
> > if the device is not MFD sub-device. And again, the comments in the
> > middle of declaration block look confusing to me. I think removing
> > comments and moving these to declaration make readability _much_
> > worse.
>
> I disagree with you here again. To me it's like completely unneeded
> churn.
>
I've seen even experienced developers making mistakes by binding the
lifetime of sub-device stuff to parent device's lifetime. I've also
seen even experienced developers who haven't used to dealing with MFD
getting confused when they see parent device's dt-node or regmap being
used. My view on this is that if the comment helps next reader to avoid
a mistake or understand why something is done - then it is worthy. I
have rarely seen comments which make code less readable or
understandable.
> > > > + g->chip.of_node = parent->of_node;
> > >
> > > Redundant. GPIO library does it for you and even better.
> >
> > So I can nowadays just omit this? Thanks!
>
> For a long time. I haven't checked the date when it started like
> this,
> but couple of years sounds like a good approximation.
Ok. Thanks for pointing it out.
Best Regards
Matti Vaittinen
^ permalink raw reply [flat|nested] 28+ messages in thread
* regmap-gpio: Support set_config and other not quite so standard ICs?
2021-03-25 10:32 ` Matti Vaittinen
@ 2021-05-10 12:58 ` Matti Vaittinen
2021-05-10 16:54 ` Andy Shevchenko
0 siblings, 1 reply; 28+ messages in thread
From: Matti Vaittinen @ 2021-05-10 12:58 UTC (permalink / raw)
To: Linus Walleij
Cc: Bartosz Golaszewski, GPIO SUBSYSTEM,
Álvaro Fernández Rojas, Linus Walleij, Michael Walle
[-- Attachment #1: Type: text/plain, Size: 2950 bytes --]
Hi Linus, All,
On Thu, 2021-03-25 at 12:32 +0200, Matti Vaittinen wrote:
> On Thu, 2021-03-25 at 10:35 +0100, Linus Walleij wrote:
snip
> > It could potentially (like the other Rohm GPIO MFD PMIC drivers)
> > make some use of the gpio regmap library, but we have some
> > pending changes for that so look into it after the next merge
> > window.
> >
> > I.e. for your TODO: look at the GPIO_REGMAP helper.
>
> I just took a quick peek at gpio_regmap and it looks pretty good to
> me!
>
> Any particular reason why gpio_regmap is not just part of gpio_chip?
> I
> guess providing the 'gpio_regmap_direction_*()', 'gpio_regmap_get()',
> 'gpio_regmap_set()' as exported helpers and leaving calling the
> (devm_)gpiochip_add_data() to IC driver would have allowed more
> flexibility. Drivers could then use the gpio_regamap features which
> fit
> the IC (by providing pointers to helper functions in gpio_chip) - and
> handle potential oddball-features by using pointers to some
> customized
> functions in gpio_chip.
So, v5.13-rc1 is out. I started wondering the gpio_regamap - and same
question persists. Why hiding the gpio_chip from gpio_regmap users?
Current IF makes it very hard (impossible?) for driver to override any
of the regmap_gpio functions (or provide own alternatives) for cases
which do not fit the generic regmap_gpio model.
My first obstacle is providing gpio_chip.set_config for BD71815.
1) I guess the method fitting current design would be adding drive-mode
register/mask(s) to the gpio_regmap_config. Certainly doable - but I
have a bad feeling of this approach. I am afraid this leads to bloating
the gpio_regmap_config with all kinds of IC specific workarounds (when
HW designers have invented new cool control registers setups) - or then
just not using the regmap_gpio for any ICs which have any quirks - even
if 90% of regmap_gpio logic would fit...
2) Other possibility is allowing IC driver to provide function pointers
for some operations (in my case for example for the set_config) - if
the default operation the regmap_gpio provides does not fit the IC.
This would require the regmap_gpio to be visible to IC drivers so that
IC drivers can access the regmap, device & register information - or
some way to convert the gpio_chip pointer to IC specific private data
pointer. Doable but still slightly bloat.
3) The last option would be adding pointer to regmap_gpio to gpio_chip
- and exporting the regmap_gpio functions as helpers - leaving the gpio
registration to be done by the IC driver. That would allow IC driver to
use the regmap_gpio helpers which suit the IC and write own functions
for rest of the stuff.
I'd like to hear opinions - should I draft some changes according to
these proposals (which one, 1,2,3 or something else?) - or as this all
been already discussed and am I just missing something?
Best Regards
Matti Vaittinen
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: regmap-gpio: Support set_config and other not quite so standard ICs?
2021-05-10 12:58 ` regmap-gpio: Support set_config and other not quite so standard ICs? Matti Vaittinen
@ 2021-05-10 16:54 ` Andy Shevchenko
2021-05-11 3:59 ` Matti Vaittinen
0 siblings, 1 reply; 28+ messages in thread
From: Andy Shevchenko @ 2021-05-10 16:54 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Linus Walleij, Bartosz Golaszewski, GPIO SUBSYSTEM,
Álvaro Fernández Rojas, Michael Walle
On Mon, May 10, 2021 at 4:41 PM Matti Vaittinen
<matti.vaittinen@fi.rohmeurope.com> wrote:
>
> Hi Linus, All,
>
> On Thu, 2021-03-25 at 12:32 +0200, Matti Vaittinen wrote:
> > On Thu, 2021-03-25 at 10:35 +0100, Linus Walleij wrote:
>
> snip
>
> > > It could potentially (like the other Rohm GPIO MFD PMIC drivers)
> > > make some use of the gpio regmap library, but we have some
> > > pending changes for that so look into it after the next merge
> > > window.
> > >
> > > I.e. for your TODO: look at the GPIO_REGMAP helper.
> >
> > I just took a quick peek at gpio_regmap and it looks pretty good to
> > me!
> >
> > Any particular reason why gpio_regmap is not just part of gpio_chip?
> > I
> > guess providing the 'gpio_regmap_direction_*()', 'gpio_regmap_get()',
> > 'gpio_regmap_set()' as exported helpers and leaving calling the
> > (devm_)gpiochip_add_data() to IC driver would have allowed more
> > flexibility. Drivers could then use the gpio_regamap features which
> > fit
> > the IC (by providing pointers to helper functions in gpio_chip) - and
> > handle potential oddball-features by using pointers to some
> > customized
> > functions in gpio_chip.
>
> So, v5.13-rc1 is out. I started wondering the gpio_regamap - and same
> question persists. Why hiding the gpio_chip from gpio_regmap users?
In general to me this sounds like opening a window for
non-controllable changes vs. controllable. Besides that, struct
gpio_chip has more than a few callbacks. On top of that, opening this
wide window means you won't be able to stop or refactoring become a
burden. I would be on the stricter side here.
> Current IF makes it very hard (impossible?) for driver to override any
> of the regmap_gpio functions (or provide own alternatives) for cases
> which do not fit the generic regmap_gpio model.
>
> My first obstacle is providing gpio_chip.set_config for BD71815.
>
> 1) I guess the method fitting current design would be adding drive-mode
> register/mask(s) to the gpio_regmap_config. Certainly doable - but I
> have a bad feeling of this approach. I am afraid this leads to bloating
> the gpio_regmap_config with all kinds of IC specific workarounds (when
> HW designers have invented new cool control registers setups) - or then
> just not using the regmap_gpio for any ICs which have any quirks - even
> if 90% of regmap_gpio logic would fit...
>
> 2) Other possibility is allowing IC driver to provide function pointers
> for some operations (in my case for example for the set_config) - if
> the default operation the regmap_gpio provides does not fit the IC.
> This would require the regmap_gpio to be visible to IC drivers so that
> IC drivers can access the regmap, device & register information - or
> some way to convert the gpio_chip pointer to IC specific private data
> pointer. Doable but still slightly bloat.
>
> 3) The last option would be adding pointer to regmap_gpio to gpio_chip
> - and exporting the regmap_gpio functions as helpers - leaving the gpio
> registration to be done by the IC driver. That would allow IC driver to
> use the regmap_gpio helpers which suit the IC and write own functions
> for rest of the stuff.
>
> I'd like to hear opinions - should I draft some changes according to
> these proposals (which one, 1,2,3 or something else?) - or as this all
> been already discussed and am I just missing something?
>
> Best Regards
> Matti Vaittinen
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: regmap-gpio: Support set_config and other not quite so standard ICs?
2021-05-10 16:54 ` Andy Shevchenko
@ 2021-05-11 3:59 ` Matti Vaittinen
2021-05-14 20:34 ` Michael Walle
0 siblings, 1 reply; 28+ messages in thread
From: Matti Vaittinen @ 2021-05-11 3:59 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Bartosz Golaszewski, GPIO SUBSYSTEM,
Álvaro Fernández Rojas, Michael Walle
[-- Attachment #1: Type: text/plain, Size: 3588 bytes --]
Morning Andy,
On Mon, 2021-05-10 at 19:54 +0300, Andy Shevchenko wrote:
> On Mon, May 10, 2021 at 4:41 PM Matti Vaittinen
> <matti.vaittinen@fi.rohmeurope.com> wrote:
> > Hi Linus, All,
> >
> > On Thu, 2021-03-25 at 12:32 +0200, Matti Vaittinen wrote:
> > > On Thu, 2021-03-25 at 10:35 +0100, Linus Walleij wrote:
> >
> > snip
> >
> > > > It could potentially (like the other Rohm GPIO MFD PMIC
> > > > drivers)
> > > > make some use of the gpio regmap library, but we have some
> > > > pending changes for that so look into it after the next merge
> > > > window.
> > > >
> > > > I.e. for your TODO: look at the GPIO_REGMAP helper.
> > >
> > > I just took a quick peek at gpio_regmap and it looks pretty good
> > > to
> > > me!
> > >
> > > Any particular reason why gpio_regmap is not just part of
> > > gpio_chip?
> > > I
> > > guess providing the 'gpio_regmap_direction_*()',
> > > 'gpio_regmap_get()',
> > > 'gpio_regmap_set()' as exported helpers and leaving calling the
> > > (devm_)gpiochip_add_data() to IC driver would have allowed more
> > > flexibility. Drivers could then use the gpio_regamap features
> > > which
> > > fit
> > > the IC (by providing pointers to helper functions in gpio_chip) -
> > > and
> > > handle potential oddball-features by using pointers to some
> > > customized
> > > functions in gpio_chip.
> >
> > So, v5.13-rc1 is out. I started wondering the gpio_regamap - and
> > same
> > question persists. Why hiding the gpio_chip from gpio_regmap users?
>
> In general to me this sounds like opening a window for
> non-controllable changes vs. controllable. Besides that, struct
> gpio_chip has more than a few callbacks. On top of that, opening this
> wide window means you won't be able to stop or refactoring become a
> burden. I would be on the stricter side here.
I kind of fail to see your point Andy. Or yes, I know exposing the
gpio_chip to user allows much more flexibility. But what are the
options? What would a driver developer do when his HW does almost fir
the standard regmap_gpio - but not just quite? Say that for example the
changing of gpio direction requires some odd additional register access
- but other than that the regmap_gpio operations like setting/getting
the value, IRQ options etc. fitted the regmap_gpio logic.
If he can not override this one function - then he will need to write
wholly new GPIO driver without re-using any of the regmap-gpio stuff.
You know, if one can't use regmap-gpio, he's likely to use the already
exposed gpio_chip anyways. I'd say this is much more of a pain to
maintain. Or maybe you add another work-around option in the
gpio_regmap_config to indicate this (and every other) oddball HW -
which eventually leads to a mess.
But this is all just my thinking - I'm kind of a "bystander" here and
that's why I asked for opinions. Thanks for sharing yours, Andy. I do
appreciate all the help and discussion.
> > 3) The last option would be adding pointer to regmap_gpio to
> > gpio_chip
> > - and exporting the regmap_gpio functions as helpers - leaving the
> > gpio
> > registration to be done by the IC driver. That would allow IC
> > driver to
> > use the regmap_gpio helpers which suit the IC and write own
> > functions
> > for rest of the stuff.
I was trying to describe here the approach that has been taken in use
at the regulator subsystem - which has used the regmap helpers for
quite a while. I think that approach is scaling quite Ok even for
strange HW.
Best Regards
Matti Vaittinen
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: regmap-gpio: Support set_config and other not quite so standard ICs?
2021-05-11 3:59 ` Matti Vaittinen
@ 2021-05-14 20:34 ` Michael Walle
2021-05-17 4:46 ` Vaittinen, Matti
0 siblings, 1 reply; 28+ messages in thread
From: Michael Walle @ 2021-05-14 20:34 UTC (permalink / raw)
To: matti.vaittinen
Cc: Andy Shevchenko, Linus Walleij, Bartosz Golaszewski,
GPIO SUBSYSTEM, Álvaro Fernández Rojas
Am 2021-05-11 05:59, schrieb Matti Vaittinen:
> Morning Andy,
>
> On Mon, 2021-05-10 at 19:54 +0300, Andy Shevchenko wrote:
>> On Mon, May 10, 2021 at 4:41 PM Matti Vaittinen
>> <matti.vaittinen@fi.rohmeurope.com> wrote:
>> > Hi Linus, All,
>> >
>> > On Thu, 2021-03-25 at 12:32 +0200, Matti Vaittinen wrote:
>> > > On Thu, 2021-03-25 at 10:35 +0100, Linus Walleij wrote:
>> >
>> > snip
>> >
>> > > > It could potentially (like the other Rohm GPIO MFD PMIC
>> > > > drivers)
>> > > > make some use of the gpio regmap library, but we have some
>> > > > pending changes for that so look into it after the next merge
>> > > > window.
>> > > >
>> > > > I.e. for your TODO: look at the GPIO_REGMAP helper.
>> > >
>> > > I just took a quick peek at gpio_regmap and it looks pretty good
>> > > to
>> > > me!
>> > >
>> > > Any particular reason why gpio_regmap is not just part of
>> > > gpio_chip?
>> > > I
>> > > guess providing the 'gpio_regmap_direction_*()',
>> > > 'gpio_regmap_get()',
>> > > 'gpio_regmap_set()' as exported helpers and leaving calling the
>> > > (devm_)gpiochip_add_data() to IC driver would have allowed more
>> > > flexibility. Drivers could then use the gpio_regamap features
>> > > which
>> > > fit
>> > > the IC (by providing pointers to helper functions in gpio_chip) -
>> > > and
>> > > handle potential oddball-features by using pointers to some
>> > > customized
>> > > functions in gpio_chip.
>> >
>> > So, v5.13-rc1 is out. I started wondering the gpio_regamap - and
>> > same
>> > question persists. Why hiding the gpio_chip from gpio_regmap users?
>>
>> In general to me this sounds like opening a window for
>> non-controllable changes vs. controllable. Besides that, struct
>> gpio_chip has more than a few callbacks. On top of that, opening this
>> wide window means you won't be able to stop or refactoring become a
>> burden. I would be on the stricter side here.
I tend to agree with Andy. Keep in mind that gpio-regmap was intended
to catch all the simple and similar controllers.
That being said, I'd still like to see new users. I've had a look
at existing drivers myself some time ago and determined that there
are quirks here and there which prevent porting that driver to
gpio-regmap, see for example gpio-mpc8xxx.c, there is a workaround
for some specific SoC which caches some values in the driver.
If we make this gpio-regmap more like a library where users can
just pick the functions they need, I fear that in the end it is
nearly impossible to change such a function because you'll always
break one or another user. But that is just a gut feeling.
> I kind of fail to see your point Andy. Or yes, I know exposing the
> gpio_chip to user allows much more flexibility. But what are the
> options? What would a driver developer do when his HW does almost fir
> the standard regmap_gpio - but not just quite? Say that for example the
> changing of gpio direction requires some odd additional register access
> - but other than that the regmap_gpio operations like setting/getting
> the value, IRQ options etc. fitted the regmap_gpio logic.
>
> If he can not override this one function - then he will need to write
> wholly new GPIO driver without re-using any of the regmap-gpio stuff.
> You know, if one can't use regmap-gpio, he's likely to use the already
> exposed gpio_chip anyways. I'd say this is much more of a pain to
> maintain. Or maybe you add another work-around option in the
> gpio_regmap_config to indicate this (and every other) oddball HW -
> which eventually leads to a mess.
Agreed, if possible, I'd not like to see options just for one
obscure HW.
> But this is all just my thinking - I'm kind of a "bystander" here and
> that's why I asked for opinions. Thanks for sharing yours, Andy. I do
> appreciate all the help and discussion.
>
>> > 3) The last option would be adding pointer to regmap_gpio to
>> > gpio_chip
>> > - and exporting the regmap_gpio functions as helpers - leaving the
>> > gpio
>> > registration to be done by the IC driver. That would allow IC
>> > driver to
>> > use the regmap_gpio helpers which suit the IC and write own
>> > functions
>> > for rest of the stuff.
>
> I was trying to describe here the approach that has been taken in use
> at the regulator subsystem - which has used the regmap helpers for
> quite a while. I think that approach is scaling quite Ok even for
> strange HW.
Do you have any pointers?
-michael
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: regmap-gpio: Support set_config and other not quite so standard ICs?
2021-05-14 20:34 ` Michael Walle
@ 2021-05-17 4:46 ` Vaittinen, Matti
0 siblings, 0 replies; 28+ messages in thread
From: Vaittinen, Matti @ 2021-05-17 4:46 UTC (permalink / raw)
To: michael; +Cc: andy.shevchenko, linux-gpio, bgolaszewski, noltari, linus.walleij
On Fri, 2021-05-14 at 22:34 +0200, Michael Walle wrote:
> Am 2021-05-11 05:59, schrieb Matti Vaittinen:
> > Morning Andy,
> >
> > On Mon, 2021-05-10 at 19:54 +0300, Andy Shevchenko wrote:
> > > On Mon, May 10, 2021 at 4:41 PM Matti Vaittinen
> > > <matti.vaittinen@fi.rohmeurope.com> wrote:
> > > > Hi Linus, All,
> > > >
> > > > On Thu, 2021-03-25 at 12:32 +0200, Matti Vaittinen wrote:
> > > > > On Thu, 2021-03-25 at 10:35 +0100, Linus Walleij wrote:
> > > >
> > > > snip
> > > >
> > > > > > It could potentially (like the other Rohm GPIO MFD PMIC
> > > > > > drivers)
> > > > > > make some use of the gpio regmap library, but we have some
> > > > > > pending changes for that so look into it after the next
> > > > > > merge
> > > > > > window.
> > > > > >
> > > > > > I.e. for your TODO: look at the GPIO_REGMAP helper.
> > > > >
> > > > > I just took a quick peek at gpio_regmap and it looks pretty
> > > > > good
> > > > > to
> > > > > me!
> > > > >
> > > > > Any particular reason why gpio_regmap is not just part of
> > > > > gpio_chip?
> > > > > I
> > > > > guess providing the 'gpio_regmap_direction_*()',
> > > > > 'gpio_regmap_get()',
> > > > > 'gpio_regmap_set()' as exported helpers and leaving calling
> > > > > the
> > > > > (devm_)gpiochip_add_data() to IC driver would have allowed
> > > > > more
> > > > > flexibility. Drivers could then use the gpio_regamap features
> > > > > which
> > > > > fit
> > > > > the IC (by providing pointers to helper functions in
> > > > > gpio_chip) -
> > > > > and
> > > > > handle potential oddball-features by using pointers to some
> > > > > customized
> > > > > functions in gpio_chip.
> > > >
> > > > So, v5.13-rc1 is out. I started wondering the gpio_regamap -
> > > > and
> > > > same
> > > > question persists. Why hiding the gpio_chip from gpio_regmap
> > > > users?
> > >
> > > In general to me this sounds like opening a window for
> > > non-controllable changes vs. controllable. Besides that, struct
> > > gpio_chip has more than a few callbacks. On top of that, opening
> > > this
> > > wide window means you won't be able to stop or refactoring become
> > > a
> > > burden. I would be on the stricter side here.
>
> I tend to agree with Andy. Keep in mind that gpio-regmap was intended
> to catch all the simple and similar controllers.
>
> That being said, I'd still like to see new users. I've had a look
> at existing drivers myself some time ago and determined that there
> are quirks here and there which prevent porting that driver to
> gpio-regmap, see for example gpio-mpc8xxx.c, there is a workaround
> for some specific SoC which caches some values in the driver.
Yes. In reality we have pretty limited amount of HW which has no quirks
or special things.
> If we make this gpio-regmap more like a library where users can
> just pick the functions they need, I fear that in the end it is
> nearly impossible to change such a function because you'll always
> break one or another user. But that is just a gut feeling.
I am proposing we keep these exported helpers as simple as possible.
When we allow users to use only functions that fit their HW - and write
own functions for HW requiring quirks - then we can indeed catch all
simple and similar controllers - and simple and similar controllers
only. This should allow keeping these functions clean and well defined
in the end :) This should also mean easier to change.
> > But this is all just my thinking - I'm kind of a "bystander" here
> > and
> > that's why I asked for opinions. Thanks for sharing yours, Andy. I
> > do
> > appreciate all the help and discussion.
> >
> > > > 3) The last option would be adding pointer to regmap_gpio to
> > > > gpio_chip
> > > > - and exporting the regmap_gpio functions as helpers - leaving
> > > > the
> > > > gpio
> > > > registration to be done by the IC driver. That would allow IC
> > > > driver to
> > > > use the regmap_gpio helpers which suit the IC and write own
> > > > functions
> > > > for rest of the stuff.
> >
> > I was trying to describe here the approach that has been taken in
> > use
> > at the regulator subsystem - which has used the regmap helpers for
> > quite a while. I think that approach is scaling quite Ok even for
> > strange HW.
>
> Do you have any pointers?
>
I'm not sure how familiar you are with the regulators but:
The exported helpers are in
drivers/regulator/helpers.c
These helpers can change/list voltages, enable/disable reguators etc.
based on the hardware (register) descritpion given at regulator
registration phase in struct regulator_desc (at
include/regulator/driver.h)
Each driver can fill the description according to it's own HW and use
the provided helpers where suitable (by giving the ops at regulator
registration, see struct regulator_ops, also at driver.h). Or, if the
provided helpers are not useful, user can just write own functions.
If this was brought to GPIO world, it would seem like the gpio_regmap
was not separate GPIO-driver but collection of exported helpers which
use the HW description information embedded in GPIO core.
Let's open the ROHM regulator drivers (which I happen to know :]) as an
example of a driver which uses both the helpers as such, and for some
cases own functions:
bd70528-regulator.c (ramp-delay is not handled by helpers, but it
probably could now when we added ramp-delay helper.)
bd71815-regulator.c (bucks 1 & 2 have special voltage-configuration
cases)
The upstream driver for BD71828 uses only standard helpers - but that's
just because it does not expose all HW features. Vendor driver provides
a 'run-level' specific control options - and can't use standard
functions for all operations.
bd718x7-regulator.c: The regulators require special handling when
voltage is changed (if regulators are enabled). And for some BD71837
bucks this is not allowed at all because change may cause voltage
spikes. So yes - mixture of standard ops and own code is again needed.
So, when looking at this which is kind of an analogy - All of the ROHM
regulator drivers use generic helper code - which saves code and helps
avoiding bugs - but all of them also need(ed) some own code to provide
proper support. If the regulator framework had used as strict policy as
gpio_regmap - then none of the ROHM regulators could use the standard
framework code. I believe pretty many other drivers have same kind of
mixtures. And still, the helpers code has been modified quite a bit
during the last three years I've followed it.
I've spent last 3 years writing mostly the PMIC code - so maybe this
explains my view on the gpio_regmap design :]
It may be the GPIO HW is not _as_ bad with the quirks - but as you
propably have found out already, quite many GPIO drivers have some
quirks which do not fit the gpio_regmap even if some parts would...
This is exactly the point I am addressing here :)
Best Regards
Matti Vaittinen
> -michael
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2021-05-17 4:46 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-24 7:21 [PATCH v4 00/16] Support ROHM BD71815 PMIC Matti Vaittinen
2021-03-24 7:22 ` [PATCH v4 01/16] rtc: bd70528: Do not require parent data Matti Vaittinen
2021-03-24 7:22 ` [PATCH v4 02/16] mfd: bd718x7: simplify by cleaning unnecessary device data Matti Vaittinen
2021-03-24 7:23 ` [PATCH v4 03/16] dt_bindings: bd71828: Add clock output mode Matti Vaittinen
2021-03-24 7:23 ` [PATCH v4 04/16] dt_bindings: regulator: Add ROHM BD71815 PMIC regulators Matti Vaittinen
2021-03-24 7:23 ` [PATCH v4 05/16] dt_bindings: mfd: Add ROHM BD71815 PMIC Matti Vaittinen
2021-03-24 7:25 ` [PATCH v4 06/16] mfd: Add ROHM BD71815 ID Matti Vaittinen
2021-03-24 7:26 ` [PATCH v4 07/16] mfd: Sort ROHM chip ID list for better readability Matti Vaittinen
2021-03-24 7:26 ` [PATCH v4 08/16] mfd: Support for ROHM BD71815 PMIC core Matti Vaittinen
2021-03-24 7:29 ` [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs Matti Vaittinen
2021-03-25 9:35 ` Linus Walleij
2021-03-25 10:32 ` Matti Vaittinen
2021-05-10 12:58 ` regmap-gpio: Support set_config and other not quite so standard ICs? Matti Vaittinen
2021-05-10 16:54 ` Andy Shevchenko
2021-05-11 3:59 ` Matti Vaittinen
2021-05-14 20:34 ` Michael Walle
2021-05-17 4:46 ` Vaittinen, Matti
2021-03-26 11:26 ` [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs Andy Shevchenko
2021-03-26 11:27 ` Andy Shevchenko
2021-03-26 13:33 ` Matti Vaittinen
2021-03-26 17:51 ` Andy Shevchenko
2021-03-28 16:59 ` Matti Vaittinen
2021-03-24 7:29 ` [PATCH v4 10/16] regulator: helpers: Export helper voltage listing Matti Vaittinen
2021-03-24 7:30 ` [PATCH v4 12/16] regulator: rohm-regulator: Support SNVS HW state Matti Vaittinen
2021-03-24 7:30 ` [PATCH v4 13/16] regulator: Support ROHM BD71815 regulators Matti Vaittinen
2021-03-24 7:30 ` [PATCH v4 14/16] clk: bd718x7: Add support for clk gate on ROHM BD71815 PMIC Matti Vaittinen
2021-03-24 7:31 ` [PATCH v4 15/16] rtc: bd70528: Support RTC on ROHM BD71815 Matti Vaittinen
2021-03-24 7:31 ` [PATCH v4 16/16] MAINTAINERS: Add ROHM BD71815AGW Matti Vaittinen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.