| Message ID | 39efebe0396dd42de30d37c6f3c1ef2926c90210.1553515333.git.matti.vaittinen@fi.rohmeurope.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | support ROHM BD70528 PMIC | expand |
On Mon, 25 Mar 2019, Matti Vaittinen wrote: > ROHM BD70528MWV is an ultra-low quiescent current general > purpose single-chip power management IC for battery-powered > portable devices. > > Add MFD core which enables chip access for following subdevices: > - regulators/LED drivers > - battery-charger > - gpios > - 32.768kHz clk > - RTC > - watchdog > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > --- > drivers/mfd/Kconfig | 17 ++ > drivers/mfd/Makefile | 1 + > drivers/mfd/rohm-bd70528.c | 438 +++++++++++++++++++++++++++++++ > include/linux/mfd/rohm-bd70528.h | 383 +++++++++++++++++++++++++++ > 4 files changed, 839 insertions(+) > create mode 100644 drivers/mfd/rohm-bd70528.c > create mode 100644 include/linux/mfd/rohm-bd70528.h > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 0ce2d8dfc5f1..2fbd6ed14329 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -1866,6 +1866,23 @@ config MFD_ROHM_BD718XX > NXP i.MX8. It contains 8 BUCK outputs and 7 LDOs, voltage monitoring > and emergency shut down as well as 32,768KHz clock output. > > +config MFD_ROHM_BD70528 > + tristate "ROHM BD70528 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 BD70528 Power > + Management IC. BD71837 is general purpose single-chip power > + management IC for battery-powered portable devices. It contains > + 3 ultra-low current consumption buck converters, 3 LDOs and 2 LED > + Drivers. Also included are 4 GPIOs, a real-time clock (RTC), a 32kHz Nit: "drivers" > + crystal oscillator, high-accuracy VREF for use with an external ADC, > + 10 bits SAR ADC for battery temperature monitor and 1S battery > + charger. > + > config MFD_STM32_LPTIMER > tristate "Support for STM32 Low-Power Timer" > depends on (ARCH_STM32 && OF) || COMPILE_TEST > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index b4569ed7f3f3..dc1c9431c8d9 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -246,4 +246,5 @@ obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o > obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o > obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o > obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o > +obj-$(CONFIG_MFD_ROHM_BD70528) += rohm-bd70528.o > > diff --git a/drivers/mfd/rohm-bd70528.c b/drivers/mfd/rohm-bd70528.c > new file mode 100644 > index 000000000000..a46bbcdefce0 > --- /dev/null > +++ b/drivers/mfd/rohm-bd70528.c > @@ -0,0 +1,438 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +// > +// Copyright (C) 2019 ROHM Semiconductors > +// > +// ROHM BD70528 PMIC driver > + > +#include <linux/i2c.h> > +#include <linux/interrupt.h> > +#include <linux/ioport.h> > +#include <linux/irq.h> > +#include <linux/mfd/core.h> > +#include <linux/mfd/rohm-bd70528.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/regmap.h> > +#include <linux/types.h> > + > +struct pmic_data { > + struct rohm_regmap_dev chip; > + struct mutex rtc_timer_lock; > +}; > + > +static const struct resource rtc_irqs[] = { > + DEFINE_RES_IRQ_NAMED(BD70528_INT_RTC_ALARM, "bd70528-rtc-alm"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_ELPS_TIM, "bd70528-elapsed-timer"), > +}; > + > +static const struct resource charger_irqs[] = { > + DEFINE_RES_IRQ_NAMED(BD70528_INT_BAT_OV_RES, "bd70528-bat-ov-res"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_BAT_OV_DET, "bd70528-bat-ov-det"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_DBAT_DET, "bd70528-bat-dead"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_BATTSD_COLD_RES, "bd70528-bat-warmed"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_BATTSD_COLD_DET, "bd70528-bat-cold"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_BATTSD_HOT_RES, "bd70528-bat-cooled"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_BATTSD_HOT_DET, "bd70528-bat-hot"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_CHG_TSD, "bd70528-chg-tshd"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_BAT_RMV, "bd70528-bat-removed"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_BAT_DET, "bd70528-bat-detected"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_DCIN2_OV_RES, "bd70528-dcin2-ov-res"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_DCIN2_OV_DET, "bd70528-dcin2-ov-det"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_DCIN2_RMV, "bd70528-dcin2-removed"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_DCIN2_DET, "bd70528-dcin2-detected"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_DCIN1_RMV, "bd70528-dcin1-removed"), > + DEFINE_RES_IRQ_NAMED(BD70528_INT_DCIN1_DET, "bd70528-dcin1-detected"), > +}; > + > +static struct mfd_cell bd70528_mfd_cells[] = { > + { .name = "bd70528-pmic", }, > + { .name = "bd70528-gpio", }, > + /* > + * We use BD71837 driver to drive the clk block. Only differences to Nit: s/clk/clock/ > + * BD70528 clock gate are the register address and mask. > + */ > + { .name = "bd718xx-clk", }, > + { .name = "bd70528-wdt", }, > + { > + .name = "bd70528-power", > + .resources = charger_irqs, > + .num_resources = ARRAY_SIZE(charger_irqs), > + }, > + { Nit: Put these on the same line, like: }, { > + .name = "bd70528-rtc", > + .resources = rtc_irqs, > + .num_resources = ARRAY_SIZE(rtc_irqs), > + }, > +}; > + > +static const struct regmap_range volatile_ranges[] = { > + /* IRQ regs */ > + { > + .range_min = BD70528_REG_INT_MAIN, > + .range_max = BD70528_REG_INT_OP_FAIL, > + }, > + /* RTC regs */ These 2 comments are superfluous. > + { Same line as the one before. And for all the other instances of this. > + .range_min = BD70528_REG_RTC_COUNT_H, > + .range_max = BD70528_REG_RTC_ALM_REPEAT, > + }, > + /* > + * WDT control reg is special. Magic values must be > + * written to it in order to change the control. Should > + * not be cached. > + */ > + { > + .range_min = BD70528_REG_WDT_CTRL, > + .range_max = BD70528_REG_WDT_CTRL, > + }, > + /* > + * BD70528 also contains a few other registers which require > + * magic sequences to be written in order to update the value. > + * At least SHIPMODE, HWRESET, WARMRESET,and STANDBY > + */ > + { > + .range_min = BD70528_REG_SHIPMODE, > + .range_max = BD70528_REG_STANDBY, > + }, > +}; > + > +static const struct regmap_access_table volatile_regs = { > + .yes_ranges = &volatile_ranges[0], > + .n_yes_ranges = ARRAY_SIZE(volatile_ranges), > +}; > + > +static struct regmap_config bd70528_regmap = { > + .reg_bits = 8, > + .val_bits = 8, > + .volatile_table = &volatile_regs, > + .max_register = BD70528_MAX_REGISTER, > + .cache_type = REGCACHE_RBTREE, > +}; > + > +/* > + * Mapping of main IRQ register bits to sub irq register offsets so "sub-IRQ" > + * that we can access corect sub IRQ registers based on bits that "sub IRQ" is also fine, but please standardise. I do prefer "sub-IRQ" though. > + * are set in main IRQ register. > + */ > + > +/* bit [0] - Shutdown register */ > +unsigned int bit0_offsets[] = {0}; > +/* bit [1] - Power failure register */ > +unsigned int bit1_offsets[] = {1}; > +/* bit [2] - VR FAULT register */ > +unsigned int bit2_offsets[] = {2}; > +/* bit [3] - PMU register interrupts */ > +unsigned int bit3_offsets[] = {3}; > +/* bit [4] - Charger 1 and Charger 2 registers */ > +unsigned int bit4_offsets[] = {4, 5}; > +/* bit [5] - RTC register */ > +unsigned int bit5_offsets[] = {6}; > +/* bit [6] - GPIO register */ > +unsigned int bit6_offsets[] = {7}; > +/* bit [7] - Invalid operation register */ > +unsigned int bit7_offsets[] = {8}; unsigned int bit0_offsets[] = {0}; /* Shutdown register */ unsigned int bit1_offsets[] = {1}; /* Power failure register */ unsigned int bit2_offsets[] = {2}; /* VR FAULT register */ unsigned int bit3_offsets[] = {3}; /* PMU register interrupts*/ unsigned int bit4_offsets[] = {4, 5}; /* Charger 1 and Charger 2 registers */ unsigned int bit5_offsets[] = {6}; /* RTC register */ unsigned int bit6_offsets[] = {7}; /* GPIO register */ unsigned int bit7_offsets[] = {8}; /* Invalid operation register */ > +static struct regmap_irq_sub_irq_map bd70528_sub_irq_offsets[] = { > + REGMAP_IRQ_MAIN_REG_OFFSET(bit0_offsets), > + REGMAP_IRQ_MAIN_REG_OFFSET(bit1_offsets), > + REGMAP_IRQ_MAIN_REG_OFFSET(bit2_offsets), > + REGMAP_IRQ_MAIN_REG_OFFSET(bit3_offsets), > + REGMAP_IRQ_MAIN_REG_OFFSET(bit4_offsets), > + REGMAP_IRQ_MAIN_REG_OFFSET(bit5_offsets), > + REGMAP_IRQ_MAIN_REG_OFFSET(bit6_offsets), > + REGMAP_IRQ_MAIN_REG_OFFSET(bit7_offsets), > +}; > + > +static struct regmap_irq irqs[] = { Please prefix "irqs". > + REGMAP_IRQ_REG(BD70528_INT_LONGPUSH, 0, BD70528_INT_LONGPUSH_MASK), > + REGMAP_IRQ_REG(BD70528_INT_WDT, 0, BD70528_INT_WDT_MASK), > + REGMAP_IRQ_REG(BD70528_INT_HWRESET, 0, BD70528_INT_HWRESET_MASK), > + REGMAP_IRQ_REG(BD70528_INT_RSTB_FAULT, 0, BD70528_INT_RSTB_FAULT_MASK), > + REGMAP_IRQ_REG(BD70528_INT_VBAT_UVLO, 0, BD70528_INT_VBAT_UVLO_MASK), > + REGMAP_IRQ_REG(BD70528_INT_TSD, 0, BD70528_INT_TSD_MASK), > + REGMAP_IRQ_REG(BD70528_INT_RSTIN, 0, BD70528_INT_RSTIN_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BUCK1_FAULT, 1, > + BD70528_INT_BUCK1_FAULT_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BUCK2_FAULT, 1, > + BD70528_INT_BUCK2_FAULT_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BUCK3_FAULT, 1, > + BD70528_INT_BUCK3_FAULT_MASK), > + REGMAP_IRQ_REG(BD70528_INT_LDO1_FAULT, 1, BD70528_INT_LDO1_FAULT_MASK), > + REGMAP_IRQ_REG(BD70528_INT_LDO2_FAULT, 1, BD70528_INT_LDO2_FAULT_MASK), > + REGMAP_IRQ_REG(BD70528_INT_LDO3_FAULT, 1, BD70528_INT_LDO3_FAULT_MASK), > + REGMAP_IRQ_REG(BD70528_INT_LED1_FAULT, 1, BD70528_INT_LED1_FAULT_MASK), > + REGMAP_IRQ_REG(BD70528_INT_LED2_FAULT, 1, BD70528_INT_LED2_FAULT_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BUCK1_OCP, 2, BD70528_INT_BUCK1_OCP_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BUCK2_OCP, 2, BD70528_INT_BUCK2_OCP_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BUCK3_OCP, 2, BD70528_INT_BUCK3_OCP_MASK), > + REGMAP_IRQ_REG(BD70528_INT_LED1_OCP, 2, BD70528_INT_LED1_OCP_MASK), > + REGMAP_IRQ_REG(BD70528_INT_LED2_OCP, 2, BD70528_INT_LED2_OCP_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BUCK1_FULLON, 2, > + BD70528_INT_BUCK1_FULLON_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BUCK2_FULLON, 2, > + BD70528_INT_BUCK2_FULLON_MASK), > + REGMAP_IRQ_REG(BD70528_INT_SHORTPUSH, 3, BD70528_INT_SHORTPUSH_MASK), > + REGMAP_IRQ_REG(BD70528_INT_AUTO_WAKEUP, 3, > + BD70528_INT_AUTO_WAKEUP_MASK), > + REGMAP_IRQ_REG(BD70528_INT_STATE_CHANGE, 3, > + BD70528_INT_STATE_CHANGE_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BAT_OV_RES, 4, BD70528_INT_BAT_OV_RES_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BAT_OV_DET, 4, BD70528_INT_BAT_OV_DET_MASK), > + REGMAP_IRQ_REG(BD70528_INT_DBAT_DET, 4, BD70528_INT_DBAT_DET_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BATTSD_COLD_RES, 4, > + BD70528_INT_BATTSD_COLD_RES_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BATTSD_COLD_DET, 4, > + BD70528_INT_BATTSD_COLD_DET_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BATTSD_HOT_RES, 4, > + BD70528_INT_BATTSD_HOT_RES_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BATTSD_HOT_DET, 4, > + BD70528_INT_BATTSD_HOT_DET_MASK), > + REGMAP_IRQ_REG(BD70528_INT_CHG_TSD, 4, BD70528_INT_CHG_TSD_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BAT_RMV, 5, BD70528_INT_BAT_RMV_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BAT_DET, 5, BD70528_INT_BAT_DET_MASK), > + REGMAP_IRQ_REG(BD70528_INT_DCIN2_OV_RES, 5, > + BD70528_INT_DCIN2_OV_RES_MASK), > + REGMAP_IRQ_REG(BD70528_INT_DCIN2_OV_DET, 5, > + BD70528_INT_DCIN2_OV_DET_MASK), > + REGMAP_IRQ_REG(BD70528_INT_DCIN2_RMV, 5, BD70528_INT_DCIN2_RMV_MASK), > + REGMAP_IRQ_REG(BD70528_INT_DCIN2_DET, 5, BD70528_INT_DCIN2_DET_MASK), > + REGMAP_IRQ_REG(BD70528_INT_DCIN1_RMV, 5, BD70528_INT_DCIN1_RMV_MASK), > + REGMAP_IRQ_REG(BD70528_INT_DCIN1_DET, 5, BD70528_INT_DCIN1_DET_MASK), > + REGMAP_IRQ_REG(BD70528_INT_RTC_ALARM, 6, BD70528_INT_RTC_ALARM_MASK), > + REGMAP_IRQ_REG(BD70528_INT_ELPS_TIM, 6, BD70528_INT_ELPS_TIM_MASK), > + REGMAP_IRQ_REG(BD70528_INT_GPIO0, 7, BD70528_INT_GPIO0_MASK), > + REGMAP_IRQ_REG(BD70528_INT_GPIO1, 7, BD70528_INT_GPIO1_MASK), > + REGMAP_IRQ_REG(BD70528_INT_GPIO2, 7, BD70528_INT_GPIO2_MASK), > + REGMAP_IRQ_REG(BD70528_INT_GPIO3, 7, BD70528_INT_GPIO3_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BUCK1_DVS_OPFAIL, 8, > + BD70528_INT_BUCK1_DVS_OPFAIL_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BUCK2_DVS_OPFAIL, 8, > + BD70528_INT_BUCK2_DVS_OPFAIL_MASK), > + REGMAP_IRQ_REG(BD70528_INT_BUCK3_DVS_OPFAIL, 8, > + BD70528_INT_BUCK3_DVS_OPFAIL_MASK), > + REGMAP_IRQ_REG(BD70528_INT_LED1_VOLT_OPFAIL, 8, > + BD70528_INT_LED1_VOLT_OPFAIL_MASK), > + REGMAP_IRQ_REG(BD70528_INT_LED2_VOLT_OPFAIL, 8, > + BD70528_INT_LED2_VOLT_OPFAIL_MASK), > +}; > + > +static struct regmap_irq_chip bd70528_irq_chip = { > + .name = "bd70528_irq", > + .main_status = BD70528_REG_INT_MAIN, > + .irqs = &irqs[0], > + .num_irqs = ARRAY_SIZE(irqs), > + .status_base = BD70528_REG_INT_SHDN, > + .mask_base = BD70528_REG_INT_SHDN_MASK, > + .ack_base = BD70528_REG_INT_SHDN, > + .type_base = BD70528_REG_GPIO1_IN, > + .init_ack_masked = true, > + .num_regs = 9, > + .num_main_regs = 1, > + .num_type_reg = 4, > + .sub_reg_offsets = &bd70528_sub_irq_offsets[0], > + .num_main_status_bits = 8, > + .irq_reg_stride = 1, > +}; > + > +#define WD_CTRL_MAGIC1 0x55 > +#define WD_CTRL_MAGIC2 0xAA > +/** > + * bd70528_wdt_set - arm or disarm watchdog timer > + * > + * @data: device data for the PMIC instance we want to operate on > + * @enable: new state of WDT. zero to disable, non zero to enable > + * @old_state: previous state of WDT will be filled here > + * > + * Arm or disarm WDT on BD70528 PMIC. Expected to be called only by > + * BD70528 RTC and BD70528 WDT drivers. The rtc_timer_lock must be taken > + * by calling bd70528_wdt_lock before calling bd70528_wdt_set. > + */ > +int bd70528_wdt_set(struct rohm_regmap_dev *data, int enable, int *old_state) Why doesn't this reside in the watchdog driver? > +{ > + int ret, i; > + unsigned int tmp; > + struct pmic_data *bd70528 = container_of(data, struct pmic_data, > + chip); > + u8 wd_ctrl_arr[3] = { WD_CTRL_MAGIC1, WD_CTRL_MAGIC2, 0 }; > + u8 *wd_ctrl = &wd_ctrl_arr[2]; > + > + ret = regmap_read(bd70528->chip.regmap, BD70528_REG_WDT_CTRL, &tmp); > + if (ret) > + return ret; > + > + *wd_ctrl = (u8)tmp; > + > + if (old_state) { > + if (*wd_ctrl & BD70528_MASK_WDT_EN) > + *old_state |= BD70528_WDT_STATE_BIT; > + else > + *old_state &= ~BD70528_WDT_STATE_BIT; > + if ((!enable) == (!(*old_state & BD70528_WDT_STATE_BIT))) > + return 0; > + } > + > + if (enable) { > + if (*wd_ctrl & BD70528_MASK_WDT_EN) > + return 0; > + *wd_ctrl |= BD70528_MASK_WDT_EN; > + } else { > + if (*wd_ctrl & BD70528_MASK_WDT_EN) > + *wd_ctrl &= ~BD70528_MASK_WDT_EN; > + else > + return 0; > + } > + > + for (i = 0; i < 3; i++) { > + ret = regmap_write(bd70528->chip.regmap, BD70528_REG_WDT_CTRL, > + wd_ctrl_arr[i]); > + if (ret) > + return ret; > + } > + > + ret = regmap_read(bd70528->chip.regmap, BD70528_REG_WDT_CTRL, &tmp); > + if ((tmp & BD70528_MASK_WDT_EN) != (*wd_ctrl & BD70528_MASK_WDT_EN)) { > + dev_err(bd70528->chip.dev, > + "Watchdog ctrl mismatch (hw) 0x%x (set) 0x%x\n", > + tmp, *wd_ctrl); > + ret = -EIO; > + } > + > + return ret; > +} > +EXPORT_SYMBOL(bd70528_wdt_set); > + > +/** > + * bd70528_wdt_lock - take WDT lock > + * > + * @bd70528: device data for the PMIC instance we want to operate on > + * > + * Lock WDT for arming/disarming in order to avoid race condition caused > + * by WDT state changes initiated by WDT and RTC drivers. > + */ > +void bd70528_wdt_lock(struct rohm_regmap_dev *data) > +{ > + struct pmic_data *bd70528 = container_of(data, struct pmic_data, > + chip); > + > + mutex_lock(&bd70528->rtc_timer_lock); > +} > +EXPORT_SYMBOL(bd70528_wdt_lock); > + > +/** > + * bd70528_wdt_unlock - unlock WDT lock > + * > + * @bd70528: device data for the PMIC instance we want to operate on > + * > + * Unlock WDT lock which has previously been taken by call to > + * bd70528_wdt_lock. > + */ > +void bd70528_wdt_unlock(struct rohm_regmap_dev *data) > +{ > + struct pmic_data *bd70528 = container_of(data, struct pmic_data, > + chip); > + > + mutex_unlock(&bd70528->rtc_timer_lock); > +} > +EXPORT_SYMBOL(bd70528_wdt_unlock); Same goes for all of this Watchdog stuff. The parent device really shouldn't have to worry about all of this functionality. > +#define BD70528_NUM_OF_GPIOS 4 Pop all defines at the top of the file. > +static int bd70528_i2c_probe(struct i2c_client *i2c, > + const struct i2c_device_id *id) > +{ > + struct pmic_data *bd70528; > + struct regmap_irq_chip_data *irq_data; > + int ret, i; > + > + if (!i2c->irq) { > + dev_err(&i2c->dev, "No IRQ configured\n"); > + return -EINVAL; > + } > + > + bd70528 = devm_kzalloc(&i2c->dev, sizeof(*bd70528), GFP_KERNEL); > + if (!bd70528) > + return -ENOMEM; > + > + mutex_init(&bd70528->rtc_timer_lock); > + > + dev_set_drvdata(&i2c->dev, &bd70528->chip); > + > + bd70528->chip.chip_type = ROHM_CHIP_TYPE_BD70528; > + bd70528->chip.regmap = devm_regmap_init_i2c(i2c, &bd70528_regmap); > + if (IS_ERR(bd70528->chip.regmap)) { > + dev_err(&i2c->dev, "regmap initialization failed\n"); Nit: "Regmap" But better read as: "Failed to initialise Regmap" > + return PTR_ERR(bd70528->chip.regmap); > + } > + > + /* > + * Disallow type setting for all IRQs by default as Why the premature line feed? > + * most of them do not support setting type. > + */ > + for (i = 0; i < ARRAY_SIZE(irqs); i++) > + irqs[i].type.types_supported = 0; > + > + /* > + * Set IRQ typesetting information for GPIO pins 0 - 3 > + */ Please format this as a one line comment. > + for (i = 0; i < BD70528_NUM_OF_GPIOS; i++) { > + struct regmap_irq_type *type; > + > + type = &irqs[BD70528_INT_GPIO0 + i].type; > + type->type_reg_offset = 2 * i; > + type->type_rising_val = 0x20; > + type->type_falling_val = 0x10; > + type->type_level_high_val = 0x40; > + type->type_level_low_val = 0x50; > + type->types_supported = (IRQ_TYPE_EDGE_BOTH | > + IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW); > + } > + > + ret = devm_regmap_add_irq_chip(&i2c->dev, bd70528->chip.regmap, > + i2c->irq, IRQF_ONESHOT, 0, > + &bd70528_irq_chip, &irq_data); > + if (ret) { > + dev_err(&i2c->dev, "Failed to add irq_chip\n"); "Failed to add IRQ chip" > + return ret; > + } > + dev_dbg(&i2c->dev, "Registered %d irqs for chip\n", "IRQs" > + bd70528_irq_chip.num_irqs); > + > + /* > + * BD70528 IRQ controller is not touching the main mask register. > + * So enable the GPIO block interrupts at main level. We can just > + * leave them enabled as the IRQ controller should disable IRQs > + * from sub-registers when IRQ is disabled or freed. > + */ > + ret = regmap_update_bits(bd70528->chip.regmap, > + BD70528_REG_INT_MAIN_MASK, > + BD70528_INT_GPIO_MASK, 0); > + > + ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, > + bd70528_mfd_cells, > + ARRAY_SIZE(bd70528_mfd_cells), NULL, 0, > + regmap_irq_get_domain(irq_data)); > + if (ret) > + dev_err(&i2c->dev, "Failed to create subdevices\n"); > + > + return ret; > +} > + > +static const struct of_device_id bd70528_of_match[] = { > + { .compatible = "rohm,bd70528", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, bd70528_of_match); > + > +static struct i2c_driver bd70528_drv = { > + .driver = { > + .name = "rohm-bd70528", > + .of_match_table = bd70528_of_match, > + }, > + .probe = &bd70528_i2c_probe, > +}; > + > +module_i2c_driver(bd70528_drv); > + > +MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>"); > +MODULE_DESCRIPTION("ROHM BD70528 Power Management IC driver"); > +MODULE_LICENSE("GPL"); > diff --git a/include/linux/mfd/rohm-bd70528.h b/include/linux/mfd/rohm-bd70528.h > new file mode 100644 > index 000000000000..155dc9c89e13 > --- /dev/null > +++ b/include/linux/mfd/rohm-bd70528.h > @@ -0,0 +1,383 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* Copyright (C) 2018 ROHM Semiconductors */ > + > +#ifndef __LINUX_MFD_BD70528_H__ > +#define __LINUX_MFD_BD70528_H__ > + > +#include <linux/device.h> > +#include <linux/mfd/rohm-generic.h> > +#include <linux/regmap.h> > + > +enum { > + BD70528_BUCK1, > + BD70528_BUCK2, > + BD70528_BUCK3, > + BD70528_LDO1, > + BD70528_LDO2, > + BD70528_LDO3, > + BD70528_LED1, > + BD70528_LED2, > +}; > + > +#define BD70528_BUCK_VOLTS 17 > +#define BD70528_BUCK_VOLTS 17 > +#define BD70528_BUCK_VOLTS 17 > +#define BD70528_LDO_VOLTS 0x20 > + > +#define BD70528_REG_BUCK1_EN 0x0F > +#define BD70528_REG_BUCK1_VOLT 0x15 > +#define BD70528_REG_BUCK2_EN 0x10 > +#define BD70528_REG_BUCK2_VOLT 0x16 > +#define BD70528_REG_BUCK3_EN 0x11 > +#define BD70528_REG_BUCK3_VOLT 0x17 > +#define BD70528_REG_LDO1_EN 0x1b > +#define BD70528_REG_LDO1_VOLT 0x1e > +#define BD70528_REG_LDO2_EN 0x1c > +#define BD70528_REG_LDO2_VOLT 0x1f > +#define BD70528_REG_LDO3_EN 0x1d > +#define BD70528_REG_LDO3_VOLT 0x20 > +#define BD70528_REG_LED_CTRL 0x2b > +#define BD70528_REG_LED_VOLT 0x29 > +#define BD70528_REG_LED_EN 0x2a > + > +/* main irq registers */ > +#define BD70528_REG_INT_MAIN 0x7E > +#define BD70528_REG_INT_MAIN_MASK 0x74 > + > +/* 'sub irq' registers */ > +#define BD70528_REG_INT_SHDN 0x7F > +#define BD70528_REG_INT_PWR_FLT 0x80 > +#define BD70528_REG_INT_VR_FLT 0x81 > +#define BD70528_REG_INT_MISC 0x82 > +#define BD70528_REG_INT_BAT1 0x83 > +#define BD70528_REG_INT_BAT2 0x84 > +#define BD70528_REG_INT_RTC 0x85 > +#define BD70528_REG_INT_GPIO 0x86 > +#define BD70528_REG_INT_OP_FAIL 0x87 > + > +#define BD70528_REG_INT_SHDN_MASK 0x75 > +#define BD70528_REG_INT_PWR_FLT_MASK 0x76 > +#define BD70528_REG_INT_VR_FLT_MASK 0x77 > +#define BD70528_REG_INT_MISC_MASK 0x78 > +#define BD70528_REG_INT_BAT1_MASK 0x79 > +#define BD70528_REG_INT_BAT2_MASK 0x7a > +#define BD70528_REG_INT_RTC_MASK 0x7b > +#define BD70528_REG_INT_GPIO_MASK 0x7c > +#define BD70528_REG_INT_OP_FAIL_MASK 0x7d > + > +/* Reset related 'magic' registers */ > +#define BD70528_REG_SHIPMODE 0x03 > +#define BD70528_REG_HWRESET 0x04 > +#define BD70528_REG_WARMRESET 0x05 > +#define BD70528_REG_STANDBY 0x06 > + > +/* GPIO registers */ > +#define BD70528_REG_GPIO_STATE 0x8F > + > +#define BD70528_REG_GPIO1_IN 0x4d > +#define BD70528_REG_GPIO2_IN 0x4f > +#define BD70528_REG_GPIO3_IN 0x51 > +#define BD70528_REG_GPIO4_IN 0x53 > +#define BD70528_REG_GPIO1_OUT 0x4e > +#define BD70528_REG_GPIO2_OUT 0x50 > +#define BD70528_REG_GPIO3_OUT 0x52 > +#define BD70528_REG_GPIO4_OUT 0x54 > + > +/* clk control */ > + > +#define BD70528_REG_CLK_OUT 0x2c > + > +/* RTC */ > + > +#define BD70528_REG_RTC_COUNT_H 0x2d > +#define BD70528_REG_RTC_COUNT_L 0x2e > +#define BD70528_REG_RTC_SEC 0x2f > +#define BD70528_REG_RTC_MINUTE 0x30 > +#define BD70528_REG_RTC_HOUR 0x31 > +#define BD70528_REG_RTC_WEEK 0x32 > +#define BD70528_REG_RTC_DAY 0x33 > +#define BD70528_REG_RTC_MONTH 0x34 > +#define BD70528_REG_RTC_YEAR 0x35 > + > +#define BD70528_REG_RTC_ALM_SEC 0x36 > +#define BD70528_REG_RTC_ALM_START BD70528_REG_RTC_ALM_SEC > +#define BD70528_REG_RTC_ALM_MINUTE 0x37 > +#define BD70528_REG_RTC_ALM_HOUR 0x38 > +#define BD70528_REG_RTC_ALM_WEEK 0x39 > +#define BD70528_REG_RTC_ALM_DAY 0x3a > +#define BD70528_REG_RTC_ALM_MONTH 0x3b > +#define BD70528_REG_RTC_ALM_YEAR 0x3c > +#define BD70528_REG_RTC_ALM_MASK 0x3d > +#define BD70528_REG_RTC_ALM_REPEAT 0x3e > +#define BD70528_REG_RTC_START BD70528_REG_RTC_SEC > + > +#define BD70528_REG_RTC_WAKE_SEC 0x43 > +#define BD70528_REG_RTC_WAKE_START BD70528_REG_RTC_WAKE_SEC > +#define BD70528_REG_RTC_WAKE_MIN 0x44 > +#define BD70528_REG_RTC_WAKE_HOUR 0x45 > +#define BD70528_REG_RTC_WAKE_CTRL 0x46 > + > +#define BD70528_REG_ELAPSED_TIMER_EN 0x42 > +#define BD70528_REG_WAKE_EN 0x46 > + > +/* WDT registers */ > +#define BD70528_REG_WDT_CTRL 0x4A > +#define BD70528_REG_WDT_HOUR 0x49 > +#define BD70528_REG_WDT_MINUTE 0x48 > +#define BD70528_REG_WDT_SEC 0x47 > + > +/* Charger / Battery */ > +#define BD70528_REG_CHG_CURR_STAT 0x59 > +#define BD70528_REG_CHG_BAT_STAT 0x57 > +#define BD70528_REG_CHG_BAT_TEMP 0x58 > +#define BD70528_REG_CHG_IN_STAT 0x56 > +#define BD70528_REG_CHG_DCIN_ILIM 0x5d > +#define BD70528_REG_CHG_CHG_CURR_WARM 0x61 > +#define BD70528_REG_CHG_CHG_CURR_COLD 0x62 > + > + > +/* Masks for main IRQ register bits */ > +enum { > + BD70528_INT_SHDN, > +#define BD70528_INT_SHDN_MASK (1<<BD70528_INT_SHDN) > + BD70528_INT_PWR_FLT, > +#define BD70528_INT_PWR_FLT_MASK (1<<BD70528_INT_PWR_FLT) > + BD70528_INT_VR_FLT, > +#define BD70528_INT_VR_FLT_MASK (1<<BD70528_INT_VR_FLT) > + BD70528_INT_MISC, > +#define BD70528_INT_MISC_MASK (1<<BD70528_INT_MISC) > + BD70528_INT_BAT1, > +#define BD70528_INT_BAT1_MASK (1<<BD70528_INT_BAT1) > + BD70528_INT_RTC, > +#define BD70528_INT_RTC_MASK (1<<BD70528_INT_RTC) > + BD70528_INT_GPIO, > +#define BD70528_INT_GPIO_MASK (1<<BD70528_INT_GPIO) > + BD70528_INT_OP_FAIL, > +#define BD70528_INT_OP_FAIL_MASK (1<<BD70528_INT_OP_FAIL) > +}; > + > +/* IRQs */ > +enum { > + /* Shutdown register IRQs */ > + BD70528_INT_LONGPUSH, > + BD70528_INT_WDT, > + BD70528_INT_HWRESET, > + BD70528_INT_RSTB_FAULT, > + BD70528_INT_VBAT_UVLO, > + BD70528_INT_TSD, > + BD70528_INT_RSTIN, > + /* Power failure register IRQs */ > + BD70528_INT_BUCK1_FAULT, > + BD70528_INT_BUCK2_FAULT, > + BD70528_INT_BUCK3_FAULT, > + BD70528_INT_LDO1_FAULT, > + BD70528_INT_LDO2_FAULT, > + BD70528_INT_LDO3_FAULT, > + BD70528_INT_LED1_FAULT, > + BD70528_INT_LED2_FAULT, > + /* VR FAULT register IRQs */ > + BD70528_INT_BUCK1_OCP, > + BD70528_INT_BUCK2_OCP, > + BD70528_INT_BUCK3_OCP, > + BD70528_INT_LED1_OCP, > + BD70528_INT_LED2_OCP, > + BD70528_INT_BUCK1_FULLON, > + BD70528_INT_BUCK2_FULLON, > + /* PMU register interrupts */ > + BD70528_INT_SHORTPUSH, > + BD70528_INT_AUTO_WAKEUP, > + BD70528_INT_STATE_CHANGE, > + /* Charger 1 register IRQs */ > + BD70528_INT_BAT_OV_RES, > + BD70528_INT_BAT_OV_DET, > + BD70528_INT_DBAT_DET, > + BD70528_INT_BATTSD_COLD_RES, > + BD70528_INT_BATTSD_COLD_DET, > + BD70528_INT_BATTSD_HOT_RES, > + BD70528_INT_BATTSD_HOT_DET, > + BD70528_INT_CHG_TSD, > + /* Charger 2 register IRQs */ > + BD70528_INT_BAT_RMV, > + BD70528_INT_BAT_DET, > + BD70528_INT_DCIN2_OV_RES, > + BD70528_INT_DCIN2_OV_DET, > + BD70528_INT_DCIN2_RMV, > + BD70528_INT_DCIN2_DET, > + BD70528_INT_DCIN1_RMV, > + BD70528_INT_DCIN1_DET, > + /* RTC register IRQs */ > + BD70528_INT_RTC_ALARM, > + BD70528_INT_ELPS_TIM, > + /* GPIO register IRQs */ > + BD70528_INT_GPIO0, > + BD70528_INT_GPIO1, > + BD70528_INT_GPIO2, > + BD70528_INT_GPIO3, > + /* Invalid operation register IRQs */ > + BD70528_INT_BUCK1_DVS_OPFAIL, > + BD70528_INT_BUCK2_DVS_OPFAIL, > + BD70528_INT_BUCK3_DVS_OPFAIL, > + BD70528_INT_LED1_VOLT_OPFAIL, > + BD70528_INT_LED2_VOLT_OPFAIL, > +}; > + > +/* Masks */ > +#define BD70528_INT_LONGPUSH_MASK 0x1 > +#define BD70528_INT_WDT_MASK 0x2 > +#define BD70528_INT_HWRESET_MASK 0x4 > +#define BD70528_INT_RSTB_FAULT_MASK 0x8 > +#define BD70528_INT_VBAT_UVLO_MASK 0x10 > +#define BD70528_INT_TSD_MASK 0x20 > +#define BD70528_INT_RSTIN_MASK 0x40 > + > +#define BD70528_INT_BUCK1_FAULT_MASK 0x1 > +#define BD70528_INT_BUCK2_FAULT_MASK 0x2 > +#define BD70528_INT_BUCK3_FAULT_MASK 0x4 > +#define BD70528_INT_LDO1_FAULT_MASK 0x8 > +#define BD70528_INT_LDO2_FAULT_MASK 0x10 > +#define BD70528_INT_LDO3_FAULT_MASK 0x20 > +#define BD70528_INT_LED1_FAULT_MASK 0x40 > +#define BD70528_INT_LED2_FAULT_MASK 0x80 > + > +#define BD70528_INT_BUCK1_OCP_MASK 0x1 > +#define BD70528_INT_BUCK2_OCP_MASK 0x2 > +#define BD70528_INT_BUCK3_OCP_MASK 0x4 > +#define BD70528_INT_LED1_OCP_MASK 0x8 > +#define BD70528_INT_LED2_OCP_MASK 0x10 > +#define BD70528_INT_BUCK1_FULLON_MASK 0x20 > +#define BD70528_INT_BUCK2_FULLON_MASK 0x40 > + > +#define BD70528_INT_SHORTPUSH_MASK 0x1 > +#define BD70528_INT_AUTO_WAKEUP_MASK 0x2 > +#define BD70528_INT_STATE_CHANGE_MASK 0x10 > + > +#define BD70528_INT_BAT_OV_RES_MASK 0x1 > +#define BD70528_INT_BAT_OV_DET_MASK 0x2 > +#define BD70528_INT_DBAT_DET_MASK 0x4 > +#define BD70528_INT_BATTSD_COLD_RES_MASK 0x8 > +#define BD70528_INT_BATTSD_COLD_DET_MASK 0x10 > +#define BD70528_INT_BATTSD_HOT_RES_MASK 0x20 > +#define BD70528_INT_BATTSD_HOT_DET_MASK 0x40 > +#define BD70528_INT_CHG_TSD_MASK 0x80 > + > +#define BD70528_INT_BAT_RMV_MASK 0x1 > +#define BD70528_INT_BAT_DET_MASK 0x2 > +#define BD70528_INT_DCIN2_OV_RES_MASK 0x4 > +#define BD70528_INT_DCIN2_OV_DET_MASK 0x8 > +#define BD70528_INT_DCIN2_RMV_MASK 0x10 > +#define BD70528_INT_DCIN2_DET_MASK 0x20 > +#define BD70528_INT_DCIN1_RMV_MASK 0x40 > +#define BD70528_INT_DCIN1_DET_MASK 0x80 > + > +#define BD70528_INT_RTC_ALARM_MASK 0x1 > +#define BD70528_INT_ELPS_TIM_MASK 0x2 > + > +#define BD70528_INT_GPIO0_MASK 0x1 > +#define BD70528_INT_GPIO1_MASK 0x2 > +#define BD70528_INT_GPIO2_MASK 0x4 > +#define BD70528_INT_GPIO3_MASK 0x8 > + > +#define BD70528_INT_BUCK1_DVS_OPFAIL_MASK 0x1 > +#define BD70528_INT_BUCK2_DVS_OPFAIL_MASK 0x2 > +#define BD70528_INT_BUCK3_DVS_OPFAIL_MASK 0x4 > +#define BD70528_INT_LED1_VOLT_OPFAIL_MASK 0x10 > +#define BD70528_INT_LED2_VOLT_OPFAIL_MASK 0x20 > + > +#define BD70528_DEBOUNCE_MASK 0x3 > + > +#define BD70528_DEBOUNCE_DISABLE 0 > +#define BD70528_DEBOUNCE_15MS 1 > +#define BD70528_DEBOUNCE_30MS 2 > +#define BD70528_DEBOUNCE_50MS 3 > + > +#define BD70528_GPIO_DRIVE_MASK 0x2 > +#define BD70528_GPIO_PUSH_PULL 0x0 > +#define BD70528_GPIO_OPEN_DRAIN 0x2 > + > +#define BD70528_GPIO_OUT_EN_MASK 0x80 > +#define BD70528_GPIO_OUT_ENABLE 0x80 > +#define BD70528_GPIO_OUT_DISABLE 0x0 > + > +#define BD70528_GPIO_OUT_HI 0x1 > +#define BD70528_GPIO_OUT_LO 0x0 > +#define BD70528_GPIO_OUT_MASK 0x1 > + > +#define BD70528_GPIO_IN_STATE_BASE 1 > + > +#define BD70528_CLK_OUT_EN_MASK 0x1 > + > +/* RTC masks to mask out reserved bits */ > + > +#define BD70528_MASK_RTC_SEC 0x7f > +#define BD70528_MASK_RTC_MINUTE 0x7f > +#define BD70528_MASK_RTC_HOUR_24H 0x80 > +#define BD70528_MASK_RTC_HOUR_PM 0x20 > +#define BD70528_MASK_RTC_HOUR 0x1f > +#define BD70528_MASK_RTC_DAY 0x3f > +#define BD70528_MASK_RTC_WEEK 0x07 > +#define BD70528_MASK_RTC_MONTH 0x1f > +#define BD70528_MASK_RTC_YEAR 0xff > +#define BD70528_MASK_RTC_COUNT_L 0x7f > + > +#define BD70528_MASK_ELAPSED_TIMER_EN 0x1 > +/* Mask second, min and hour fields > + * HW would support ALM irq for over 24h > + * (by setting day, month and year too) > + * but as we wish to keep this same as for > + * wake-up we limit ALM to 24H and only > + * unmask sec, min and hour > + */ > +#define BD70528_MASK_ALM_EN 0x7 > +#define BD70528_MASK_WAKE_EN 0x1 > + > +/* WDT masks */ > +#define BD70528_MASK_WDT_EN 0x1 > +#define BD70528_MASK_WDT_HOUR 0x1 > +#define BD70528_MASK_WDT_MINUTE 0x7f > +#define BD70528_MASK_WDT_SEC 0x7f > + > +#define BD70528_WDT_STATE_BIT 0x1 > +#define BD70528_ELAPSED_STATE_BIT 0x2 > +#define BD70528_WAKE_STATE_BIT 0x4 > + > +/* Charger masks */ > +#define BD70528_MASK_CHG_STAT 0x7f > +#define BD70528_MASK_CHG_BAT_TIMER 0x20 > +#define BD70528_MASK_CHG_BAT_OVERVOLT 0x10 > +#define BD70528_MASK_CHG_BAT_DETECT 0x1 > +#define BD70528_MASK_CHG_DCIN1_UVLO 0x1 > +#define BD70528_MASK_CHG_DCIN_ILIM 0x3f > +#define BD70528_MASK_CHG_CHG_CURR 0x1f > +#define BD70528_MASK_CHG_TRICKLE_CURR 0x10 > + > +/* > + * Note, external battery register is the lonely rider at > + * address 0xc5. See how to stuff that in the regmap > + */ > +#define BD70528_MAX_REGISTER 0x94 > + > +/* Buck control masks */ > +#define BD70528_MASK_RUN_EN 0x4 > +#define BD70528_MASK_STBY_EN 0x2 > +#define BD70528_MASK_IDLE_EN 0x1 > +#define BD70528_MASK_LED1_EN 0x1 > +#define BD70528_MASK_LED2_EN 0x10 > + > +#define BD70528_MASK_BUCK_VOLT 0xf > +#define BD70528_MASK_LDO_VOLT 0x1f > +#define BD70528_MASK_LED1_VOLT 0x1 > +#define BD70528_MASK_LED2_VOLT 0x10 > + > +/* Misc irq masks */ > +#define BD70528_INT_MASK_SHORT_PUSH 1 > +#define BD70528_INT_MASK_AUTO_WAKE 2 > +#define BD70528_INT_MASK_POWER_STATE 4 > + > +#define BD70528_MASK_BUCK_RAMP 0x10 > +#define BD70528_SIFT_BUCK_RAMP 4 > + > +int bd70528_wdt_set(struct rohm_regmap_dev *data, int enable, int *old_state); > +void bd70528_wdt_lock(struct rohm_regmap_dev *data); > +void bd70528_wdt_unlock(struct rohm_regmap_dev *data); > + > +#endif /* __LINUX_MFD_BD70528_H__ */
Hello Lee, Thanks for taking a look on this again =) I agree with most of the comments and correct them at next version. On Wed, Apr 03, 2019 at 08:31:52AM +0100, Lee Jones wrote: > On Mon, 25 Mar 2019, Matti Vaittinen wrote: > > > ROHM BD70528MWV is an ultra-low quiescent current general > > purpose single-chip power management IC for battery-powered > > portable devices. > > > > Add MFD core which enables chip access for following subdevices: > > - regulators/LED drivers > > - battery-charger > > - gpios > > - 32.768kHz clk > > - RTC > > - watchdog > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > + * Mapping of main IRQ register bits to sub irq register offsets so > > "sub-IRQ" > > > + * that we can access corect sub IRQ registers based on bits that > > "sub IRQ" is also fine, but please standardise. > > I do prefer "sub-IRQ" though. I'll go with "sub-IRQ" then > > + > > +#define WD_CTRL_MAGIC1 0x55 > > +#define WD_CTRL_MAGIC2 0xAA > > +/** > > + * bd70528_wdt_set - arm or disarm watchdog timer > > + * > > + * @data: device data for the PMIC instance we want to operate on > > + * @enable: new state of WDT. zero to disable, non zero to enable > > + * @old_state: previous state of WDT will be filled here > > + * > > + * Arm or disarm WDT on BD70528 PMIC. Expected to be called only by > > + * BD70528 RTC and BD70528 WDT drivers. The rtc_timer_lock must be taken > > + * by calling bd70528_wdt_lock before calling bd70528_wdt_set. > > + */ > > +int bd70528_wdt_set(struct rohm_regmap_dev *data, int enable, int *old_state) > > Why doesn't this reside in the watchdog driver? If my memory serves me right we shortly discussed this already during v8 review ;) Cant blame you though as I have seen some of the mail traffic going through your inbox :D The motivation to have the functions exported from MFD is to not create sirect dependency between RTC and WDT. There may be cases where we want to leave either RTC or WDT out of compilation. MFD is always needed so the dependency from MFD to RTC/WDT does not harm. (Here's some discussion necromancy if you are interested in re-reading how we did end up with this implementation: https://lore.kernel.org/lkml/20190212091723.GZ20638@dell/) I hope you are still Ok with having the WDT control functions in MFD. Best Regards Matti Vaittinen
On Wed, 03 Apr 2019, Matti Vaittinen wrote: > Hello Lee, > > Thanks for taking a look on this again =) I agree with most of the > comments and correct them at next version. > > On Wed, Apr 03, 2019 at 08:31:52AM +0100, Lee Jones wrote: > > On Mon, 25 Mar 2019, Matti Vaittinen wrote: > > > > > ROHM BD70528MWV is an ultra-low quiescent current general > > > purpose single-chip power management IC for battery-powered > > > portable devices. > > > > > > Add MFD core which enables chip access for following subdevices: > > > - regulators/LED drivers > > > - battery-charger > > > - gpios > > > - 32.768kHz clk > > > - RTC > > > - watchdog > > > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > > + * Mapping of main IRQ register bits to sub irq register offsets so > > > > "sub-IRQ" > > > > > + * that we can access corect sub IRQ registers based on bits that > > > > "sub IRQ" is also fine, but please standardise. > > > > I do prefer "sub-IRQ" though. > > I'll go with "sub-IRQ" then > > > > + > > > +#define WD_CTRL_MAGIC1 0x55 > > > +#define WD_CTRL_MAGIC2 0xAA > > > +/** > > > + * bd70528_wdt_set - arm or disarm watchdog timer > > > + * > > > + * @data: device data for the PMIC instance we want to operate on > > > + * @enable: new state of WDT. zero to disable, non zero to enable > > > + * @old_state: previous state of WDT will be filled here > > > + * > > > + * Arm or disarm WDT on BD70528 PMIC. Expected to be called only by > > > + * BD70528 RTC and BD70528 WDT drivers. The rtc_timer_lock must be taken > > > + * by calling bd70528_wdt_lock before calling bd70528_wdt_set. > > > + */ > > > +int bd70528_wdt_set(struct rohm_regmap_dev *data, int enable, int *old_state) > > > > Why doesn't this reside in the watchdog driver? > > If my memory serves me right we shortly discussed this already during v8 > review ;) Cant blame you though as I have seen some of the mail traffic > going through your inbox :D > > The motivation to have the functions exported from MFD is to not create > sirect dependency between RTC and WDT. There may be cases where we want > to leave either RTC or WDT out of compilation. MFD is always needed so > the dependency from MFD to RTC/WDT does not harm. > > (Here's some discussion necromancy if you are interested in re-reading > how we did end up with this implementation: > https://lore.kernel.org/lkml/20190212091723.GZ20638@dell/) > > I hope you are still Ok with having the WDT control functions in MFD. OOI, why does the RTC need to control the WDT?
On Wed, Apr 03, 2019 at 10:30:15AM +0100, Lee Jones wrote: > On Wed, 03 Apr 2019, Matti Vaittinen wrote: > > > Hello Lee, > > > > Thanks for taking a look on this again =) I agree with most of the > > comments and correct them at next version. > > > > On Wed, Apr 03, 2019 at 08:31:52AM +0100, Lee Jones wrote: > > > On Mon, 25 Mar 2019, Matti Vaittinen wrote: > > > > > > > ROHM BD70528MWV is an ultra-low quiescent current general > > > > purpose single-chip power management IC for battery-powered > > > > portable devices. > > > > > > > > Add MFD core which enables chip access for following subdevices: > > > > - regulators/LED drivers > > > > - battery-charger > > > > - gpios > > > > - 32.768kHz clk > > > > - RTC > > > > - watchdog > > > > > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > > > + * Mapping of main IRQ register bits to sub irq register offsets so > > > > > > "sub-IRQ" > > > > > > > + * that we can access corect sub IRQ registers based on bits that > > > > > > "sub IRQ" is also fine, but please standardise. > > > > > > I do prefer "sub-IRQ" though. > > > > I'll go with "sub-IRQ" then > > > > > > + > > > > +#define WD_CTRL_MAGIC1 0x55 > > > > +#define WD_CTRL_MAGIC2 0xAA > > > > +/** > > > > + * bd70528_wdt_set - arm or disarm watchdog timer > > > > + * > > > > + * @data: device data for the PMIC instance we want to operate on > > > > + * @enable: new state of WDT. zero to disable, non zero to enable > > > > + * @old_state: previous state of WDT will be filled here > > > > + * > > > > + * Arm or disarm WDT on BD70528 PMIC. Expected to be called only by > > > > + * BD70528 RTC and BD70528 WDT drivers. The rtc_timer_lock must be taken > > > > + * by calling bd70528_wdt_lock before calling bd70528_wdt_set. > > > > + */ > > > > +int bd70528_wdt_set(struct rohm_regmap_dev *data, int enable, int *old_state) > > > > > > Why doesn't this reside in the watchdog driver? > > > > If my memory serves me right we shortly discussed this already during v8 > > review ;) Cant blame you though as I have seen some of the mail traffic > > going through your inbox :D > > > > The motivation to have the functions exported from MFD is to not create > > sirect dependency between RTC and WDT. There may be cases where we want > > to leave either RTC or WDT out of compilation. MFD is always needed so > > the dependency from MFD to RTC/WDT does not harm. > > > > (Here's some discussion necromancy if you are interested in re-reading > > how we did end up with this implementation: > > https://lore.kernel.org/lkml/20190212091723.GZ20638@dell/) > > > > I hope you are still Ok with having the WDT control functions in MFD. > > OOI, why does the RTC need to control the WDT? I thought I had a comment about this somewhere in code... O_o Must have been in some development branch I had :/ Anyways, setting the RTC counter may cause watchdog to trigger. It is not further explained why but I would guess watchdog uses RTC counter to check if it should've been pinged already. So RTC needs to disable watch dog for the duration of hwclock setting and enable it again after the new time is set. I can add a comment about this to MFD driver if it helps :) Br, Matti Vaittinen
On Wed, 03 Apr 2019, Matti Vaittinen wrote: > On Wed, Apr 03, 2019 at 10:30:15AM +0100, Lee Jones wrote: > > On Wed, 03 Apr 2019, Matti Vaittinen wrote: > > > > > Hello Lee, > > > > > > Thanks for taking a look on this again =) I agree with most of the > > > comments and correct them at next version. > > > > > > On Wed, Apr 03, 2019 at 08:31:52AM +0100, Lee Jones wrote: > > > > On Mon, 25 Mar 2019, Matti Vaittinen wrote: > > > > > > > > > ROHM BD70528MWV is an ultra-low quiescent current general > > > > > purpose single-chip power management IC for battery-powered > > > > > portable devices. > > > > > > > > > > Add MFD core which enables chip access for following subdevices: > > > > > - regulators/LED drivers > > > > > - battery-charger > > > > > - gpios > > > > > - 32.768kHz clk > > > > > - RTC > > > > > - watchdog > > > > > > > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > > > > + * Mapping of main IRQ register bits to sub irq register offsets so > > > > > > > > "sub-IRQ" > > > > > > > > > + * that we can access corect sub IRQ registers based on bits that > > > > > > > > "sub IRQ" is also fine, but please standardise. > > > > > > > > I do prefer "sub-IRQ" though. > > > > > > I'll go with "sub-IRQ" then > > > > > > > > + > > > > > +#define WD_CTRL_MAGIC1 0x55 > > > > > +#define WD_CTRL_MAGIC2 0xAA > > > > > +/** > > > > > + * bd70528_wdt_set - arm or disarm watchdog timer > > > > > + * > > > > > + * @data: device data for the PMIC instance we want to operate on > > > > > + * @enable: new state of WDT. zero to disable, non zero to enable > > > > > + * @old_state: previous state of WDT will be filled here > > > > > + * > > > > > + * Arm or disarm WDT on BD70528 PMIC. Expected to be called only by > > > > > + * BD70528 RTC and BD70528 WDT drivers. The rtc_timer_lock must be taken > > > > > + * by calling bd70528_wdt_lock before calling bd70528_wdt_set. > > > > > + */ > > > > > +int bd70528_wdt_set(struct rohm_regmap_dev *data, int enable, int *old_state) > > > > > > > > Why doesn't this reside in the watchdog driver? > > > > > > If my memory serves me right we shortly discussed this already during v8 > > > review ;) Cant blame you though as I have seen some of the mail traffic > > > going through your inbox :D > > > > > > The motivation to have the functions exported from MFD is to not create > > > sirect dependency between RTC and WDT. There may be cases where we want > > > to leave either RTC or WDT out of compilation. MFD is always needed so > > > the dependency from MFD to RTC/WDT does not harm. > > > > > > (Here's some discussion necromancy if you are interested in re-reading > > > how we did end up with this implementation: > > > https://lore.kernel.org/lkml/20190212091723.GZ20638@dell/) > > > > > > I hope you are still Ok with having the WDT control functions in MFD. > > > > OOI, why does the RTC need to control the WDT? > > I thought I had a comment about this somewhere in code... O_o Must have > been in some development branch I had :/ > > Anyways, setting the RTC counter may cause watchdog to trigger. It is not > further explained why but I would guess watchdog uses RTC counter to check > if it should've been pinged already. So RTC needs to disable watch dog for > the duration of hwclock setting and enable it again after the new time is > set. I can add a comment about this to MFD driver if it helps :) How does the user select between using the RTC and the WDT? Or are the generally both enabled at the same time?
On Wed, 2019-04-03 at 12:25 +0100, Lee Jones wrote: > On Wed, 03 Apr 2019, Matti Vaittinen wrote: > > > On Wed, Apr 03, 2019 at 10:30:15AM +0100, Lee Jones wrote: > > > On Wed, 03 Apr 2019, Matti Vaittinen wrote: > > > > > > > Hello Lee, > > > > > > > > Thanks for taking a look on this again =) I agree with most of > > > > the > > > > comments and correct them at next version. > > > > > > > > On Wed, Apr 03, 2019 at 08:31:52AM +0100, Lee Jones wrote: > > > > > On Mon, 25 Mar 2019, Matti Vaittinen wrote: > > > > > > > > > > > ROHM BD70528MWV is an ultra-low quiescent current general > > > > > > purpose single-chip power management IC for battery-powered > > > > > > portable devices. > > > > > > > > > > > > Add MFD core which enables chip access for following > > > > > > subdevices: > > > > > > - regulators/LED drivers > > > > > > - battery-charger > > > > > > - gpios > > > > > > - 32.768kHz clk > > > > > > - RTC > > > > > > - watchdog > > > > > > > > > > > > Signed-off-by: Matti Vaittinen < > > > > > > matti.vaittinen@fi.rohmeurope.com> > > > > > > + * Mapping of main IRQ register bits to sub irq register > > > > > > offsets so > > > > > > > > > > "sub-IRQ" > > > > > > > > > > > + * that we can access corect sub IRQ registers based on > > > > > > bits that > > > > > > > > > > "sub IRQ" is also fine, but please standardise. > > > > > > > > > > I do prefer "sub-IRQ" though. > > > > > > > > I'll go with "sub-IRQ" then > > > > > > > > > > + > > > > > > +#define WD_CTRL_MAGIC1 0x55 > > > > > > +#define WD_CTRL_MAGIC2 0xAA > > > > > > +/** > > > > > > + * bd70528_wdt_set - arm or disarm watchdog timer > > > > > > + * > > > > > > + * @data: device data for the PMIC instance we want to > > > > > > operate on > > > > > > + * @enable: new state of WDT. zero to disable, non > > > > > > zero to enable > > > > > > + * @old_state: previous state of WDT will be filled > > > > > > here > > > > > > + * > > > > > > + * Arm or disarm WDT on BD70528 PMIC. Expected to be > > > > > > called only by > > > > > > + * BD70528 RTC and BD70528 WDT drivers. The rtc_timer_lock > > > > > > must be taken > > > > > > + * by calling bd70528_wdt_lock before calling > > > > > > bd70528_wdt_set. > > > > > > + */ > > > > > > +int bd70528_wdt_set(struct rohm_regmap_dev *data, int > > > > > > enable, int *old_state) > > > > > > > > > > Why doesn't this reside in the watchdog driver? > > > > > > > > If my memory serves me right we shortly discussed this already > > > > during v8 > > > > review ;) Cant blame you though as I have seen some of the mail > > > > traffic > > > > going through your inbox :D > > > > > > > > The motivation to have the functions exported from MFD is to > > > > not create > > > > sirect dependency between RTC and WDT. There may be cases where > > > > we want > > > > to leave either RTC or WDT out of compilation. MFD is always > > > > needed so > > > > the dependency from MFD to RTC/WDT does not harm. > > > > > > > > (Here's some discussion necromancy if you are interested in re- > > > > reading > > > > how we did end up with this implementation: > > > > https://lore.kernel.org/lkml/20190212091723.GZ20638@dell/) > > > > > > > > I hope you are still Ok with having the WDT control functions > > > > in MFD. > > > > > > OOI, why does the RTC need to control the WDT? > > > > I thought I had a comment about this somewhere in code... O_o Must > > have > > been in some development branch I had :/ > > > > Anyways, setting the RTC counter may cause watchdog to trigger. It > > is not > > further explained why but I would guess watchdog uses RTC counter > > to check > > if it should've been pinged already. So RTC needs to disable watch > > dog for > > the duration of hwclock setting and enable it again after the new > > time is > > set. I can add a comment about this to MFD driver if it helps :) > > How does the user select between using the RTC and the WDT? > > Or are the generally both enabled at the same time? > Both RTC and WDT can be enabled at the same time. But they are not required to be used. When WDT is enabled, it uses current RTC time as 'base' (and RTC time is running no matter if we have the RTC driver here or not) - and time-out gets scheduled to specified amount of time into future. (Same setting timeout into the future happens when WDT is pinged). When we set RTC, we disable WDT (if it was enabled), set clock and re- enable WDT. This causes the previously used time-out value to be set to WDT again. This works Ok because BD70528 does not support 'short ping detection'. Only side-effect will be one 'prolonged' WDT feeding period when RTC is set. (absolute time when RTC was set minus absolute time when previous WD ping or enable was done) longer than reqular period. So user should not need to care about this 'dependency'. Basically the only possible problem I see is that someone could accidentally hang the system with something that keeps setting the RTC time - this would then prevent watch dog from doing the reset. This, I believe, is a corner case which I don't consider now - and if this is considered to be an issue then such a system may disable the RTC driver and do RTC setting in a what-ever-manner sees practical. I'm not sure if I answered to question you asked though =) Br, Matti Vaittinen
On Wed, 03 Apr 2019, Vaittinen, Matti wrote: > On Wed, 2019-04-03 at 12:25 +0100, Lee Jones wrote: > > On Wed, 03 Apr 2019, Matti Vaittinen wrote: > > > > > On Wed, Apr 03, 2019 at 10:30:15AM +0100, Lee Jones wrote: > > > > On Wed, 03 Apr 2019, Matti Vaittinen wrote: > > > > > > > > > Hello Lee, > > > > > > > > > > Thanks for taking a look on this again =) I agree with most of > > > > > the > > > > > comments and correct them at next version. > > > > > > > > > > On Wed, Apr 03, 2019 at 08:31:52AM +0100, Lee Jones wrote: > > > > > > On Mon, 25 Mar 2019, Matti Vaittinen wrote: > > > > > > > > > > > > > ROHM BD70528MWV is an ultra-low quiescent current general > > > > > > > purpose single-chip power management IC for battery-powered > > > > > > > portable devices. > > > > > > > > > > > > > > Add MFD core which enables chip access for following > > > > > > > subdevices: > > > > > > > - regulators/LED drivers > > > > > > > - battery-charger > > > > > > > - gpios > > > > > > > - 32.768kHz clk > > > > > > > - RTC > > > > > > > - watchdog > > > > > > > > > > > > > > Signed-off-by: Matti Vaittinen < > > > > > > > matti.vaittinen@fi.rohmeurope.com> > > > > > > > + * Mapping of main IRQ register bits to sub irq register > > > > > > > offsets so > > > > > > > > > > > > "sub-IRQ" > > > > > > > > > > > > > + * that we can access corect sub IRQ registers based on > > > > > > > bits that > > > > > > > > > > > > "sub IRQ" is also fine, but please standardise. > > > > > > > > > > > > I do prefer "sub-IRQ" though. > > > > > > > > > > I'll go with "sub-IRQ" then > > > > > > > > > > > > + > > > > > > > +#define WD_CTRL_MAGIC1 0x55 > > > > > > > +#define WD_CTRL_MAGIC2 0xAA > > > > > > > +/** > > > > > > > + * bd70528_wdt_set - arm or disarm watchdog timer > > > > > > > + * > > > > > > > + * @data: device data for the PMIC instance we want to > > > > > > > operate on > > > > > > > + * @enable: new state of WDT. zero to disable, non > > > > > > > zero to enable > > > > > > > + * @old_state: previous state of WDT will be filled > > > > > > > here > > > > > > > + * > > > > > > > + * Arm or disarm WDT on BD70528 PMIC. Expected to be > > > > > > > called only by > > > > > > > + * BD70528 RTC and BD70528 WDT drivers. The rtc_timer_lock > > > > > > > must be taken > > > > > > > + * by calling bd70528_wdt_lock before calling > > > > > > > bd70528_wdt_set. > > > > > > > + */ > > > > > > > +int bd70528_wdt_set(struct rohm_regmap_dev *data, int > > > > > > > enable, int *old_state) > > > > > > > > > > > > Why doesn't this reside in the watchdog driver? > > > > > > > > > > If my memory serves me right we shortly discussed this already > > > > > during v8 > > > > > review ;) Cant blame you though as I have seen some of the mail > > > > > traffic > > > > > going through your inbox :D > > > > > > > > > > The motivation to have the functions exported from MFD is to > > > > > not create > > > > > sirect dependency between RTC and WDT. There may be cases where > > > > > we want > > > > > to leave either RTC or WDT out of compilation. MFD is always > > > > > needed so > > > > > the dependency from MFD to RTC/WDT does not harm. > > > > > > > > > > (Here's some discussion necromancy if you are interested in re- > > > > > reading > > > > > how we did end up with this implementation: > > > > > https://lore.kernel.org/lkml/20190212091723.GZ20638@dell/) > > > > > > > > > > I hope you are still Ok with having the WDT control functions > > > > > in MFD. > > > > > > > > OOI, why does the RTC need to control the WDT? > > > > > > I thought I had a comment about this somewhere in code... O_o Must > > > have > > > been in some development branch I had :/ > > > > > > Anyways, setting the RTC counter may cause watchdog to trigger. It > > > is not > > > further explained why but I would guess watchdog uses RTC counter > > > to check > > > if it should've been pinged already. So RTC needs to disable watch > > > dog for > > > the duration of hwclock setting and enable it again after the new > > > time is > > > set. I can add a comment about this to MFD driver if it helps :) > > > > How does the user select between using the RTC and the WDT? > > > > Or are the generally both enabled at the same time? > > > > Both RTC and WDT can be enabled at the same time. But they are not > required to be used. When WDT is enabled, it uses current RTC time as > 'base' (and RTC time is running no matter if we have the RTC driver > here or not) - and time-out gets scheduled to specified amount of time > into future. (Same setting timeout into the future happens when WDT is > pinged). > > When we set RTC, we disable WDT (if it was enabled), set clock and re- > enable WDT. This causes the previously used time-out value to be set to > WDT again. This works Ok because BD70528 does not support 'short ping > detection'. Only side-effect will be one 'prolonged' WDT feeding period > when RTC is set. (absolute time when RTC was set minus absolute time > when previous WD ping or enable was done) longer than reqular period. > > So user should not need to care about this 'dependency'. Basically the > only possible problem I see is that someone could accidentally hang the > system with something that keeps setting the RTC time - this would then > prevent watch dog from doing the reset. This, I believe, is a corner > case which I don't consider now - and if this is considered to be an > issue then such a system may disable the RTC driver and do RTC setting > in a what-ever-manner sees practical. > > I'm not sure if I answered to question you asked though =) I think you answered it just fine. So my suggestion is to have the RTC depend on the WRT via Kconfig, and place this WRT code in the WRT driver where it belongs. These functions can be exported from the WRT driver and the RTC can call into them in the same way it calls into the MFD driver currently. Avoiding a dependency on the WRT would seem strange, because there is one.
On Thu, 2019-04-04 at 03:52 +0100, Lee Jones wrote: > On Wed, 03 Apr 2019, Vaittinen, Matti wrote: > > > On Wed, 2019-04-03 at 12:25 +0100, Lee Jones wrote: > > > On Wed, 03 Apr 2019, Matti Vaittinen wrote: > > > > > > > On Wed, Apr 03, 2019 at 10:30:15AM +0100, Lee Jones wrote: > > > > > On Wed, 03 Apr 2019, Matti Vaittinen wrote: > > > > > > > > > > > Hello Lee, > > > > > > > > > > > > Thanks for taking a look on this again =) I agree with most > > > > > > of > > > > > > the > > > > > > comments and correct them at next version. > > > > > > > > > > > > On Wed, Apr 03, 2019 at 08:31:52AM +0100, Lee Jones wrote: > > > > > > > On Mon, 25 Mar 2019, Matti Vaittinen wrote: > > > > > > > > > > > > > > > ROHM BD70528MWV is an ultra-low quiescent current > > > > > > > > general > > > > > > > > purpose single-chip power management IC for battery- > > > > > > > > powered > > > > > > > > portable devices. > > > > > > > > > > > > > > > > Add MFD core which enables chip access for following > > > > > > > > subdevices: > > > > > > > > - regulators/LED drivers > > > > > > > > - battery-charger > > > > > > > > - gpios > > > > > > > > - 32.768kHz clk > > > > > > > > - RTC > > > > > > > > - watchdog > > > > > > > > > > > > > > > > Signed-off-by: Matti Vaittinen < > > > > > > > > matti.vaittinen@fi.rohmeurope.com> > > > > > > > > + * Mapping of main IRQ register bits to sub irq > > > > > > > > register > > > > > > > > offsets so > > > > > > > > > > > > > > "sub-IRQ" > > > > > > > > > > > > > > > + * that we can access corect sub IRQ registers based > > > > > > > > on > > > > > > > > bits that > > > > > > > > > > > > > > "sub IRQ" is also fine, but please standardise. > > > > > > > > > > > > > > I do prefer "sub-IRQ" though. > > > > > > > > > > > > I'll go with "sub-IRQ" then > > > > > > > > > > > > > > + > > > > > > > > +#define WD_CTRL_MAGIC1 0x55 > > > > > > > > +#define WD_CTRL_MAGIC2 0xAA > > > > > > > > +/** > > > > > > > > + * bd70528_wdt_set - arm or disarm watchdog timer > > > > > > > > + * > > > > > > > > + * @data: device data for the PMIC instance we > > > > > > > > want to > > > > > > > > operate on > > > > > > > > + * @enable: new state of WDT. zero to disable, non > > > > > > > > zero to enable > > > > > > > > + * @old_state: previous state of WDT will be filled > > > > > > > > here > > > > > > > > + * > > > > > > > > + * Arm or disarm WDT on BD70528 PMIC. Expected to be > > > > > > > > called only by > > > > > > > > + * BD70528 RTC and BD70528 WDT drivers. The > > > > > > > > rtc_timer_lock > > > > > > > > must be taken > > > > > > > > + * by calling bd70528_wdt_lock before calling > > > > > > > > bd70528_wdt_set. > > > > > > > > + */ > > > > > > > > +int bd70528_wdt_set(struct rohm_regmap_dev *data, int > > > > > > > > enable, int *old_state) > > > > > > > > > > > > > > Why doesn't this reside in the watchdog driver? > > > > > > > > > > > > If my memory serves me right we shortly discussed this > > > > > > already > > > > > > during v8 > > > > > > review ;) Cant blame you though as I have seen some of the > > > > > > mail > > > > > > traffic > > > > > > going through your inbox :D > > > > > > > > > > > > The motivation to have the functions exported from MFD is > > > > > > to > > > > > > not create > > > > > > sirect dependency between RTC and WDT. There may be cases > > > > > > where > > > > > > we want > > > > > > to leave either RTC or WDT out of compilation. MFD is > > > > > > always > > > > > > needed so > > > > > > the dependency from MFD to RTC/WDT does not harm. > > > > > > > > > > > > (Here's some discussion necromancy if you are interested in > > > > > > re- > > > > > > reading > > > > > > how we did end up with this implementation: > > > > > > https://lore.kernel.org/lkml/20190212091723.GZ20638@dell/) > > > > > > > > > > > > I hope you are still Ok with having the WDT control > > > > > > functions > > > > > > in MFD. > > > > > > > > > > OOI, why does the RTC need to control the WDT? > > > > > > > > I thought I had a comment about this somewhere in code... O_o > > > > Must > > > > have > > > > been in some development branch I had :/ > > > > > > > > Anyways, setting the RTC counter may cause watchdog to trigger. > > > > It > > > > is not > > > > further explained why but I would guess watchdog uses RTC > > > > counter > > > > to check > > > > if it should've been pinged already. So RTC needs to disable > > > > watch > > > > dog for > > > > the duration of hwclock setting and enable it again after the > > > > new > > > > time is > > > > set. I can add a comment about this to MFD driver if it helps > > > > :) > > > > > > How does the user select between using the RTC and the WDT? > > > > > > Or are the generally both enabled at the same time? > > > > > > > Both RTC and WDT can be enabled at the same time. But they are not > > required to be used. When WDT is enabled, it uses current RTC time > > as > > 'base' (and RTC time is running no matter if we have the RTC driver > > here or not) - and time-out gets scheduled to specified amount of > > time > > into future. (Same setting timeout into the future happens when WDT > > is > > pinged). > > > > When we set RTC, we disable WDT (if it was enabled), set clock and > > re- > > enable WDT. This causes the previously used time-out value to be > > set to > > WDT again. This works Ok because BD70528 does not support 'short > > ping > > detection'. Only side-effect will be one 'prolonged' WDT feeding > > period > > when RTC is set. (absolute time when RTC was set minus absolute > > time > > when previous WD ping or enable was done) longer than reqular > > period. > > > > So user should not need to care about this 'dependency'. Basically > > the > > only possible problem I see is that someone could accidentally hang > > the > > system with something that keeps setting the RTC time - this would > > then > > prevent watch dog from doing the reset. This, I believe, is a > > corner > > case which I don't consider now - and if this is considered to be > > an > > issue then such a system may disable the RTC driver and do RTC > > setting > > in a what-ever-manner sees practical. > > > > I'm not sure if I answered to question you asked though =) > > I think you answered it just fine. > > So my suggestion is to have the RTC depend on the WRT via Kconfig, > and > place this WRT code in the WRT driver where it belongs. These > functions can be exported from the WRT driver and the RTC can call > into them in the same way it calls into the MFD driver currently. Yes, we could. But then we need to always compile the watch dog driver when we want to use RTC. It is not a huge driver though so it probably won't matter. So, I don't like this but I can do it so if you insist :] > Avoiding a dependency on the WRT would seem strange, because there is > one. The dependency is artificial. It's caused by the current driver design. If watchdog is not used, then RTC has no reason to touch the watchdog block. RTC works just fine without watchdog. So from HW point there is no dependency. Actually, now that I thik of it the right way to do this would have been the function pointer in parent data as was done in original patch set. HW-colleagues tend to re-use HW blocks, and we like to re-use our drivers. If the next PMIC from ROHM uses same RTC block but does not provide watchdog - then it is cleanest solution to fall back to function pointer and leave it to NULL when there is no WDT or when WDT is unused. Another option is to export dummy function - which is not so nice. Additional benefit from function pointer would have been that the function pointer can be only used by drivers which have acces to it. This exported function is globally visible. The WDT disable/enable is very specific procedure and it actually would be nicer design to not have it visible globally. It is not intended to be used by anything else besides the WDT and RTC here. But I can't say there will be PMIC with same RTC and no WDT coming from ROHM. Also, I am not terribly excited about the option of changing this back to function-pointer as I already removed the pointer from parent data and this changed parent data is already adapted to all sub drivers - so this is all just babbling. Maybe it is just my huge ego shouting there - 'I was right, I must have the final say'. As a side note, I already did submit v12 with other styling fixes but which left the WDT function in MFD. If you still see the WDT functions should be exported from WDT - then please ignore the v12. I'll do v13 at the afternoon (my time, which is only a bit after noon your time I guess) which will export these functions from WDT. (Well, I had to try once more :D) Br, Matti Vaittinen
On Thu, 04 Apr 2019, Vaittinen, Matti wrote: > On Thu, 2019-04-04 at 03:52 +0100, Lee Jones wrote: > > On Wed, 03 Apr 2019, Vaittinen, Matti wrote: > > > > > On Wed, 2019-04-03 at 12:25 +0100, Lee Jones wrote: > > > > On Wed, 03 Apr 2019, Matti Vaittinen wrote: > > > > > > > > > On Wed, Apr 03, 2019 at 10:30:15AM +0100, Lee Jones wrote: > > > > > > On Wed, 03 Apr 2019, Matti Vaittinen wrote: > > > > > > > > > > > > > Hello Lee, > > > > > > > > > > > > > > Thanks for taking a look on this again =) I agree with most > > > > > > > of > > > > > > > the > > > > > > > comments and correct them at next version. > > > > > > > > > > > > > > On Wed, Apr 03, 2019 at 08:31:52AM +0100, Lee Jones wrote: > > > > > > > > On Mon, 25 Mar 2019, Matti Vaittinen wrote: > > > > > > > > > > > > > > > > > ROHM BD70528MWV is an ultra-low quiescent current > > > > > > > > > general > > > > > > > > > purpose single-chip power management IC for battery- > > > > > > > > > powered > > > > > > > > > portable devices. > > > > > > > > > > > > > > > > > > Add MFD core which enables chip access for following > > > > > > > > > subdevices: > > > > > > > > > - regulators/LED drivers > > > > > > > > > - battery-charger > > > > > > > > > - gpios > > > > > > > > > - 32.768kHz clk > > > > > > > > > - RTC > > > > > > > > > - watchdog > > > > > > > > > > > > > > > > > > Signed-off-by: Matti Vaittinen < > > > > > > > > > matti.vaittinen@fi.rohmeurope.com> > > > > > > > > > + * Mapping of main IRQ register bits to sub irq > > > > > > > > > register > > > > > > > > > offsets so > > > > > > > > > > > > > > > > "sub-IRQ" > > > > > > > > > > > > > > > > > + * that we can access corect sub IRQ registers based > > > > > > > > > on > > > > > > > > > bits that > > > > > > > > > > > > > > > > "sub IRQ" is also fine, but please standardise. > > > > > > > > > > > > > > > > I do prefer "sub-IRQ" though. > > > > > > > > > > > > > > I'll go with "sub-IRQ" then > > > > > > > > > > > > > > > > + > > > > > > > > > +#define WD_CTRL_MAGIC1 0x55 > > > > > > > > > +#define WD_CTRL_MAGIC2 0xAA > > > > > > > > > +/** > > > > > > > > > + * bd70528_wdt_set - arm or disarm watchdog timer > > > > > > > > > + * > > > > > > > > > + * @data: device data for the PMIC instance we > > > > > > > > > want to > > > > > > > > > operate on > > > > > > > > > + * @enable: new state of WDT. zero to disable, non > > > > > > > > > zero to enable > > > > > > > > > + * @old_state: previous state of WDT will be filled > > > > > > > > > here > > > > > > > > > + * > > > > > > > > > + * Arm or disarm WDT on BD70528 PMIC. Expected to be > > > > > > > > > called only by > > > > > > > > > + * BD70528 RTC and BD70528 WDT drivers. The > > > > > > > > > rtc_timer_lock > > > > > > > > > must be taken > > > > > > > > > + * by calling bd70528_wdt_lock before calling > > > > > > > > > bd70528_wdt_set. > > > > > > > > > + */ > > > > > > > > > +int bd70528_wdt_set(struct rohm_regmap_dev *data, int > > > > > > > > > enable, int *old_state) > > > > > > > > > > > > > > > > Why doesn't this reside in the watchdog driver? > > > > > > > > > > > > > > If my memory serves me right we shortly discussed this > > > > > > > already > > > > > > > during v8 > > > > > > > review ;) Cant blame you though as I have seen some of the > > > > > > > mail > > > > > > > traffic > > > > > > > going through your inbox :D > > > > > > > > > > > > > > The motivation to have the functions exported from MFD is > > > > > > > to > > > > > > > not create > > > > > > > sirect dependency between RTC and WDT. There may be cases > > > > > > > where > > > > > > > we want > > > > > > > to leave either RTC or WDT out of compilation. MFD is > > > > > > > always > > > > > > > needed so > > > > > > > the dependency from MFD to RTC/WDT does not harm. > > > > > > > > > > > > > > (Here's some discussion necromancy if you are interested in > > > > > > > re- > > > > > > > reading > > > > > > > how we did end up with this implementation: > > > > > > > https://lore.kernel.org/lkml/20190212091723.GZ20638@dell/) > > > > > > > > > > > > > > I hope you are still Ok with having the WDT control > > > > > > > functions > > > > > > > in MFD. > > > > > > > > > > > > OOI, why does the RTC need to control the WDT? > > > > > > > > > > I thought I had a comment about this somewhere in code... O_o > > > > > Must > > > > > have > > > > > been in some development branch I had :/ > > > > > > > > > > Anyways, setting the RTC counter may cause watchdog to trigger. > > > > > It > > > > > is not > > > > > further explained why but I would guess watchdog uses RTC > > > > > counter > > > > > to check > > > > > if it should've been pinged already. So RTC needs to disable > > > > > watch > > > > > dog for > > > > > the duration of hwclock setting and enable it again after the > > > > > new > > > > > time is > > > > > set. I can add a comment about this to MFD driver if it helps > > > > > :) > > > > > > > > How does the user select between using the RTC and the WDT? > > > > > > > > Or are the generally both enabled at the same time? > > > > > > > > > > Both RTC and WDT can be enabled at the same time. But they are not > > > required to be used. When WDT is enabled, it uses current RTC time > > > as > > > 'base' (and RTC time is running no matter if we have the RTC driver > > > here or not) - and time-out gets scheduled to specified amount of > > > time > > > into future. (Same setting timeout into the future happens when WDT > > > is > > > pinged). > > > > > > When we set RTC, we disable WDT (if it was enabled), set clock and > > > re- > > > enable WDT. This causes the previously used time-out value to be > > > set to > > > WDT again. This works Ok because BD70528 does not support 'short > > > ping > > > detection'. Only side-effect will be one 'prolonged' WDT feeding > > > period > > > when RTC is set. (absolute time when RTC was set minus absolute > > > time > > > when previous WD ping or enable was done) longer than reqular > > > period. > > > > > > So user should not need to care about this 'dependency'. Basically > > > the > > > only possible problem I see is that someone could accidentally hang > > > the > > > system with something that keeps setting the RTC time - this would > > > then > > > prevent watch dog from doing the reset. This, I believe, is a > > > corner > > > case which I don't consider now - and if this is considered to be > > > an > > > issue then such a system may disable the RTC driver and do RTC > > > setting > > > in a what-ever-manner sees practical. > > > > > > I'm not sure if I answered to question you asked though =) > > > > I think you answered it just fine. > > > > So my suggestion is to have the RTC depend on the WRT via Kconfig, > > and > > place this WRT code in the WRT driver where it belongs. These > > functions can be exported from the WRT driver and the RTC can call > > into them in the same way it calls into the MFD driver currently. > > Yes, we could. But then we need to always compile the watch dog driver > when we want to use RTC. It is not a huge driver though so it probably > won't matter. So, I don't like this but I can do it so if you insist :] > > > Avoiding a dependency on the WRT would seem strange, because there is > > one. > > The dependency is artificial. It's caused by the current driver design. > If watchdog is not used, then RTC has no reason to touch the watchdog > block. RTC works just fine without watchdog. So from HW point there is > no dependency. Great. > Actually, now that I thik of it the right way to do this would have > been the function pointer in parent data as was done in original patch > set. HW-colleagues tend to re-use HW blocks, and we like to re-use our > drivers. If the next PMIC from ROHM uses same RTC block but does not > provide watchdog - then it is cleanest solution to fall back to > function pointer and leave it to NULL when there is no WDT or when WDT > is unused. Another option is to export dummy function - which is not so > nice. I think the converse is true. Pointers to functions outside of a subsystem API context are generally horrible. It's much nicer to call a function which can be easily stubbed out in a header file based on a Kconfig option. It's how most kernel APIs work. Have a look for yourself how many there are: git grep -A5 inline -- include/linux/ > Additional benefit from function pointer would have been that the > function pointer can be only used by drivers which have acces to it. > This exported function is globally visible. The WDT disable/enable is > very specific procedure and it actually would be nicer design to not > have it visible globally. It is not intended to be used by anything > else besides the WDT and RTC here. Why would anything else try to use it? There are 1000's of exported functions in the kernel. If it's properly namespaced a consumer would have to purposely call it, which if they really wanted to, they could do anyway. I don't really see your point. > But I can't say there will be PMIC with same RTC and no WDT coming from > ROHM. Also, I am not terribly excited about the option of changing this > back to function-pointer as I already removed the pointer from parent > data and this changed parent data is already adapted to all sub drivers > - so this is all just babbling. Maybe it is just my huge ego shouting > there - 'I was right, I must have the final say'. No, a call-back function would be a back-step. Ego or no ego, you're wrong. =:-D > As a side note, I already did submit v12 with other styling fixes but > which left the WDT function in MFD. If you still see the WDT functions > should be exported from WDT - then please ignore the v12. I'll do v13 > at the afternoon (my time, which is only a bit after noon your time I > guess) which will export these functions from WDT. (Well, I had to try > once more :D) Please keep the WDT code in the WDT driver. Create a little stub for the cases where the WDT driver is not enabled - job done.
On Thu, Apr 04, 2019 at 07:54:52AM +0100, Lee Jones wrote: > On Thu, 04 Apr 2019, Vaittinen, Matti wrote: > > > Actually, now that I thik of it the right way to do this would have > > been the function pointer in parent data as was done in original patch > > set. HW-colleagues tend to re-use HW blocks, and we like to re-use our > > drivers. If the next PMIC from ROHM uses same RTC block but does not > > provide watchdog - then it is cleanest solution to fall back to > > function pointer and leave it to NULL when there is no WDT or when WDT > > is unused. Another option is to export dummy function - which is not so > > nice. > > I think the converse is true. > > Pointers to functions outside of a subsystem API context are generally > horrible. It's much nicer to call a function which can be easily > stubbed out in a header file based on a Kconfig option. It's how most > kernel APIs work. I hate to admit but I see your point. This nicely solves any issues in syncronizing the startup for driver providing function pointer and for driver using it. > > Additional benefit from function pointer would have been that the > > function pointer can be only used by drivers which have acces to it. > > This exported function is globally visible. The WDT disable/enable is > > very specific procedure and it actually would be nicer design to not > > have it visible globally. It is not intended to be used by anything > > else besides the WDT and RTC here. > > Why would anything else try to use it? > > There are 1000's of exported functions in the kernel. If it's > properly namespaced a consumer would have to purposely call it, which > if they really wanted to, they could do anyway. I don't really see > your point. I could still argue on this. It _is_ less obvous that an exported function is not meant to be publicly used than it is for function pointers. But as you say, this is not a strong enough point to see the trouble in synchronizing the WDT/RTC startup. > > But I can't say there will be PMIC with same RTC and no WDT coming from > > ROHM. Also, I am not terribly excited about the option of changing this > > back to function-pointer as I already removed the pointer from parent > > data and this changed parent data is already adapted to all sub drivers > > - so this is all just babbling. Maybe it is just my huge ego shouting > > there - 'I was right, I must have the final say'. > > No, a call-back function would be a back-step. You are probably right. > Ego or no ego, you're wrong. =:-D I'd rephrase that as "It's not that I am wrong, but you are right." =) > > As a side note, I already did submit v12 with other styling fixes but > > which left the WDT function in MFD. If you still see the WDT functions > > should be exported from WDT - then please ignore the v12. I'll do v13 > > at the afternoon (my time, which is only a bit after noon your time I > > guess) which will export these functions from WDT. (Well, I had to try > > once more :D) > > Please keep the WDT code in the WDT driver. Create a little stub for > the cases where the WDT driver is not enabled - job done. Yes Sir. Br, Matti Vaittinen
On 04/04/2019 10:24:49+0300, Matti Vaittinen wrote: > On Thu, Apr 04, 2019 at 07:54:52AM +0100, Lee Jones wrote: > > On Thu, 04 Apr 2019, Vaittinen, Matti wrote: > > > > > Actually, now that I thik of it the right way to do this would have > > > been the function pointer in parent data as was done in original patch > > > set. HW-colleagues tend to re-use HW blocks, and we like to re-use our > > > drivers. If the next PMIC from ROHM uses same RTC block but does not > > > provide watchdog - then it is cleanest solution to fall back to > > > function pointer and leave it to NULL when there is no WDT or when WDT > > > is unused. Another option is to export dummy function - which is not so > > > nice. > > > > I think the converse is true. > > > > Pointers to functions outside of a subsystem API context are generally > > horrible. It's much nicer to call a function which can be easily > > stubbed out in a header file based on a Kconfig option. It's how most > > kernel APIs work. > > I hate to admit but I see your point. This nicely solves any issues in > syncronizing the startup for driver providing function pointer and for > driver using it. > Wouldn't it be easier to register the watchdog driver as part of the RTC driver? As I see it, the wdt is just a glorified RTC alarm.
> > > ROHM. Also, I am not terribly excited about the option of changing this > > > back to function-pointer as I already removed the pointer from parent > > > data and this changed parent data is already adapted to all sub drivers > > > - so this is all just babbling. Maybe it is just my huge ego shouting > > > there - 'I was right, I must have the final say'. > > > > No, a call-back function would be a back-step. > > You are probably right. > > > Ego or no ego, you're wrong. =:-D > > I'd rephrase that as "It's not that I am wrong, but you are right." =) Works for me. > > > As a side note, I already did submit v12 with other styling fixes but > > > which left the WDT function in MFD. If you still see the WDT functions > > > should be exported from WDT - then please ignore the v12. I'll do v13 > > > at the afternoon (my time, which is only a bit after noon your time I > > > guess) which will export these functions from WDT. (Well, I had to try > > > once more :D) > > > > Please keep the WDT code in the WDT driver. Create a little stub for > > the cases where the WDT driver is not enabled - job done. > > Yes Sir. =;-)
Hello Alexandre, On Thu, 2019-04-04 at 09:56 +0200, Alexandre Belloni wrote: > On 04/04/2019 10:24:49+0300, Matti Vaittinen wrote: > > On Thu, Apr 04, 2019 at 07:54:52AM +0100, Lee Jones wrote: > > > On Thu, 04 Apr 2019, Vaittinen, Matti wrote: > > > > > > > Actually, now that I thik of it the right way to do this would > > > > have > > > > been the function pointer in parent data as was done in > > > > original patch > > > > set. HW-colleagues tend to re-use HW blocks, and we like to re- > > > > use our > > > > drivers. If the next PMIC from ROHM uses same RTC block but > > > > does not > > > > provide watchdog - then it is cleanest solution to fall back to > > > > function pointer and leave it to NULL when there is no WDT or > > > > when WDT > > > > is unused. Another option is to export dummy function - which > > > > is not so > > > > nice. > > > > > > I think the converse is true. > > > > > > Pointers to functions outside of a subsystem API context are > > > generally > > > horrible. It's much nicer to call a function which can be easily > > > stubbed out in a header file based on a Kconfig option. It's how > > > most > > > kernel APIs work. > > > > I hate to admit but I see your point. This nicely solves any issues > > in > > syncronizing the startup for driver providing function pointer and > > for > > driver using it. > > > > Wouldn't it be easier to register the watchdog driver as part of the > RTC > driver? > > As I see it, the wdt is just a glorified RTC alarm. Do you suggest me to put all the stuff now placed in drivers/watchdog/bd70528_wdt.c into rtc driver? It would be doable - but I'd rather kept the WDT independent module so that it can be left out of config if WDT needs not to be used. And same with RTC. Also, re- use of RTC driver in HW which does not include WDT is easier when WDT is a separate module. To me it looks much cleaner to have the WDT as own module than polluting the RTC driver with config ifdefs. But from HW perspective you are correct. The WDT in BD70528 seems to be kind of RTC alarm which shuts of the PMIC if triggered. Br, Matti Vaittinen
On Thu, 04 Apr 2019, Vaittinen, Matti wrote: > Hello Alexandre, > > On Thu, 2019-04-04 at 09:56 +0200, Alexandre Belloni wrote: > > On 04/04/2019 10:24:49+0300, Matti Vaittinen wrote: > > > On Thu, Apr 04, 2019 at 07:54:52AM +0100, Lee Jones wrote: > > > > On Thu, 04 Apr 2019, Vaittinen, Matti wrote: > > > > > > > > > Actually, now that I thik of it the right way to do this would > > > > > have > > > > > been the function pointer in parent data as was done in > > > > > original patch > > > > > set. HW-colleagues tend to re-use HW blocks, and we like to re- > > > > > use our > > > > > drivers. If the next PMIC from ROHM uses same RTC block but > > > > > does not > > > > > provide watchdog - then it is cleanest solution to fall back to > > > > > function pointer and leave it to NULL when there is no WDT or > > > > > when WDT > > > > > is unused. Another option is to export dummy function - which > > > > > is not so > > > > > nice. > > > > > > > > I think the converse is true. > > > > > > > > Pointers to functions outside of a subsystem API context are > > > > generally > > > > horrible. It's much nicer to call a function which can be easily > > > > stubbed out in a header file based on a Kconfig option. It's how > > > > most > > > > kernel APIs work. > > > > > > I hate to admit but I see your point. This nicely solves any issues > > > in > > > syncronizing the startup for driver providing function pointer and > > > for > > > driver using it. > > > > > > > Wouldn't it be easier to register the watchdog driver as part of the > > RTC > > driver? > > > > As I see it, the wdt is just a glorified RTC alarm. > > Do you suggest me to put all the stuff now placed in > drivers/watchdog/bd70528_wdt.c into rtc driver? It would be doable - > but I'd rather kept the WDT independent module so that it can be left > out of config if WDT needs not to be used. And same with RTC. Also, re- > use of RTC driver in HW which does not include WDT is easier when WDT > is a separate module. To me it looks much cleaner to have the WDT as > own module than polluting the RTC driver with config ifdefs. I haven't looked at the code, but I agree with this in principle. I'm a firm believer of having functionality in the most appropriate subsystem. IMHO, if a device can be neatly split 9/10 it should be. > But from HW perspective you are correct. The WDT in BD70528 seems to be > kind of RTC alarm which shuts of the PMIC if triggered. > > Br, > Matti Vaittinen
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 0ce2d8dfc5f1..2fbd6ed14329 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -1866,6 +1866,23 @@ config MFD_ROHM_BD718XX NXP i.MX8. It contains 8 BUCK outputs and 7 LDOs, voltage monitoring and emergency shut down as well as 32,768KHz clock output. +config MFD_ROHM_BD70528 + tristate "ROHM BD70528 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 BD70528 Power + Management IC. BD71837 is general purpose single-chip power + management IC for battery-powered portable devices. It contains + 3 ultra-low current consumption buck converters, 3 LDOs and 2 LED + Drivers. Also included are 4 GPIOs, a real-time clock (RTC), a 32kHz + crystal oscillator, high-accuracy VREF for use with an external ADC, + 10 bits SAR ADC for battery temperature monitor and 1S battery + charger. + config MFD_STM32_LPTIMER tristate "Support for STM32 Low-Power Timer" depends on (ARCH_STM32 && OF) || COMPILE_TEST diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index b4569ed7f3f3..dc1c9431c8d9 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -246,4 +246,5 @@ obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o +obj-$(CONFIG_MFD_ROHM_BD70528) += rohm-bd70528.o diff --git a/drivers/mfd/rohm-bd70528.c b/drivers/mfd/rohm-bd70528.c new file mode 100644 index 000000000000..a46bbcdefce0 --- /dev/null +++ b/drivers/mfd/rohm-bd70528.c @@ -0,0 +1,438 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +// +// Copyright (C) 2019 ROHM Semiconductors +// +// ROHM BD70528 PMIC driver + +#include <linux/i2c.h> +#include <linux/interrupt.h> +#include <linux/ioport.h> +#include <linux/irq.h> +#include <linux/mfd/core.h> +#include <linux/mfd/rohm-bd70528.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/regmap.h> +#include <linux/types.h> + +struct pmic_data { + struct rohm_regmap_dev chip; + struct mutex rtc_timer_lock; +}; + +static const struct resource rtc_irqs[] = { + DEFINE_RES_IRQ_NAMED(BD70528_INT_RTC_ALARM, "bd70528-rtc-alm"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_ELPS_TIM, "bd70528-elapsed-timer"), +}; + +static const struct resource charger_irqs[] = { + DEFINE_RES_IRQ_NAMED(BD70528_INT_BAT_OV_RES, "bd70528-bat-ov-res"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_BAT_OV_DET, "bd70528-bat-ov-det"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_DBAT_DET, "bd70528-bat-dead"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_BATTSD_COLD_RES, "bd70528-bat-warmed"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_BATTSD_COLD_DET, "bd70528-bat-cold"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_BATTSD_HOT_RES, "bd70528-bat-cooled"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_BATTSD_HOT_DET, "bd70528-bat-hot"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_CHG_TSD, "bd70528-chg-tshd"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_BAT_RMV, "bd70528-bat-removed"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_BAT_DET, "bd70528-bat-detected"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_DCIN2_OV_RES, "bd70528-dcin2-ov-res"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_DCIN2_OV_DET, "bd70528-dcin2-ov-det"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_DCIN2_RMV, "bd70528-dcin2-removed"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_DCIN2_DET, "bd70528-dcin2-detected"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_DCIN1_RMV, "bd70528-dcin1-removed"), + DEFINE_RES_IRQ_NAMED(BD70528_INT_DCIN1_DET, "bd70528-dcin1-detected"), +}; + +static struct mfd_cell bd70528_mfd_cells[] = { + { .name = "bd70528-pmic", }, + { .name = "bd70528-gpio", }, + /* + * We use BD71837 driver to drive the clk block. Only differences to + * BD70528 clock gate are the register address and mask. + */ + { .name = "bd718xx-clk", }, + { .name = "bd70528-wdt", }, + { + .name = "bd70528-power", + .resources = charger_irqs, + .num_resources = ARRAY_SIZE(charger_irqs), + }, + { + .name = "bd70528-rtc", + .resources = rtc_irqs, + .num_resources = ARRAY_SIZE(rtc_irqs), + }, +}; + +static const struct regmap_range volatile_ranges[] = { + /* IRQ regs */ + { + .range_min = BD70528_REG_INT_MAIN, + .range_max = BD70528_REG_INT_OP_FAIL, + }, + /* RTC regs */ + { + .range_min = BD70528_REG_RTC_COUNT_H, + .range_max = BD70528_REG_RTC_ALM_REPEAT, + }, + /* + * WDT control reg is special. Magic values must be + * written to it in order to change the control. Should + * not be cached. + */ + { + .range_min = BD70528_REG_WDT_CTRL, + .range_max = BD70528_REG_WDT_CTRL, + }, + /* + * BD70528 also contains a few other registers which require + * magic sequences to be written in order to update the value. + * At least SHIPMODE, HWRESET, WARMRESET,and STANDBY + */ + { + .range_min = BD70528_REG_SHIPMODE, + .range_max = BD70528_REG_STANDBY, + }, +}; + +static const struct regmap_access_table volatile_regs = { + .yes_ranges = &volatile_ranges[0], + .n_yes_ranges = ARRAY_SIZE(volatile_ranges), +}; + +static struct regmap_config bd70528_regmap = { + .reg_bits = 8, + .val_bits = 8, + .volatile_table = &volatile_regs, + .max_register = BD70528_MAX_REGISTER, + .cache_type = REGCACHE_RBTREE, +}; + +/* + * 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. + */ + +/* bit [0] - Shutdown register */ +unsigned int bit0_offsets[] = {0}; +/* bit [1] - Power failure register */ +unsigned int bit1_offsets[] = {1}; +/* bit [2] - VR FAULT register */ +unsigned int bit2_offsets[] = {2}; +/* bit [3] - PMU register interrupts */ +unsigned int bit3_offsets[] = {3}; +/* bit [4] - Charger 1 and Charger 2 registers */ +unsigned int bit4_offsets[] = {4, 5}; +/* bit [5] - RTC register */ +unsigned int bit5_offsets[] = {6}; +/* bit [6] - GPIO register */ +unsigned int bit6_offsets[] = {7}; +/* bit [7] - Invalid operation register */ +unsigned int bit7_offsets[] = {8}; + +static struct regmap_irq_sub_irq_map bd70528_sub_irq_offsets[] = { + REGMAP_IRQ_MAIN_REG_OFFSET(bit0_offsets), + REGMAP_IRQ_MAIN_REG_OFFSET(bit1_offsets), + REGMAP_IRQ_MAIN_REG_OFFSET(bit2_offsets), + REGMAP_IRQ_MAIN_REG_OFFSET(bit3_offsets), + REGMAP_IRQ_MAIN_REG_OFFSET(bit4_offsets), + REGMAP_IRQ_MAIN_REG_OFFSET(bit5_offsets), + REGMAP_IRQ_MAIN_REG_OFFSET(bit6_offsets), + REGMAP_IRQ_MAIN_REG_OFFSET(bit7_offsets), +}; + +static struct regmap_irq irqs[] = { + REGMAP_IRQ_REG(BD70528_INT_LONGPUSH, 0, BD70528_INT_LONGPUSH_MASK), + REGMAP_IRQ_REG(BD70528_INT_WDT, 0, BD70528_INT_WDT_MASK), + REGMAP_IRQ_REG(BD70528_INT_HWRESET, 0, BD70528_INT_HWRESET_MASK), + REGMAP_IRQ_REG(BD70528_INT_RSTB_FAULT, 0, BD70528_INT_RSTB_FAULT_MASK), + REGMAP_IRQ_REG(BD70528_INT_VBAT_UVLO, 0, BD70528_INT_VBAT_UVLO_MASK), + REGMAP_IRQ_REG(BD70528_INT_TSD, 0, BD70528_INT_TSD_MASK), + REGMAP_IRQ_REG(BD70528_INT_RSTIN, 0, BD70528_INT_RSTIN_MASK), + REGMAP_IRQ_REG(BD70528_INT_BUCK1_FAULT, 1, + BD70528_INT_BUCK1_FAULT_MASK), + REGMAP_IRQ_REG(BD70528_INT_BUCK2_FAULT, 1, + BD70528_INT_BUCK2_FAULT_MASK), + REGMAP_IRQ_REG(BD70528_INT_BUCK3_FAULT, 1, + BD70528_INT_BUCK3_FAULT_MASK), + REGMAP_IRQ_REG(BD70528_INT_LDO1_FAULT, 1, BD70528_INT_LDO1_FAULT_MASK), + REGMAP_IRQ_REG(BD70528_INT_LDO2_FAULT, 1, BD70528_INT_LDO2_FAULT_MASK), + REGMAP_IRQ_REG(BD70528_INT_LDO3_FAULT, 1, BD70528_INT_LDO3_FAULT_MASK), + REGMAP_IRQ_REG(BD70528_INT_LED1_FAULT, 1, BD70528_INT_LED1_FAULT_MASK), + REGMAP_IRQ_REG(BD70528_INT_LED2_FAULT, 1, BD70528_INT_LED2_FAULT_MASK), + REGMAP_IRQ_REG(BD70528_INT_BUCK1_OCP, 2, BD70528_INT_BUCK1_OCP_MASK), + REGMAP_IRQ_REG(BD70528_INT_BUCK2_OCP, 2, BD70528_INT_BUCK2_OCP_MASK), + REGMAP_IRQ_REG(BD70528_INT_BUCK3_OCP, 2, BD70528_INT_BUCK3_OCP_MASK), + REGMAP_IRQ_REG(BD70528_INT_LED1_OCP, 2, BD70528_INT_LED1_OCP_MASK), + REGMAP_IRQ_REG(BD70528_INT_LED2_OCP, 2, BD70528_INT_LED2_OCP_MASK), + REGMAP_IRQ_REG(BD70528_INT_BUCK1_FULLON, 2, + BD70528_INT_BUCK1_FULLON_MASK), + REGMAP_IRQ_REG(BD70528_INT_BUCK2_FULLON, 2, + BD70528_INT_BUCK2_FULLON_MASK), + REGMAP_IRQ_REG(BD70528_INT_SHORTPUSH, 3, BD70528_INT_SHORTPUSH_MASK), + REGMAP_IRQ_REG(BD70528_INT_AUTO_WAKEUP, 3, + BD70528_INT_AUTO_WAKEUP_MASK), + REGMAP_IRQ_REG(BD70528_INT_STATE_CHANGE, 3, + BD70528_INT_STATE_CHANGE_MASK), + REGMAP_IRQ_REG(BD70528_INT_BAT_OV_RES, 4, BD70528_INT_BAT_OV_RES_MASK), + REGMAP_IRQ_REG(BD70528_INT_BAT_OV_DET, 4, BD70528_INT_BAT_OV_DET_MASK), + REGMAP_IRQ_REG(BD70528_INT_DBAT_DET, 4, BD70528_INT_DBAT_DET_MASK), + REGMAP_IRQ_REG(BD70528_INT_BATTSD_COLD_RES, 4, + BD70528_INT_BATTSD_COLD_RES_MASK), + REGMAP_IRQ_REG(BD70528_INT_BATTSD_COLD_DET, 4, + BD70528_INT_BATTSD_COLD_DET_MASK), + REGMAP_IRQ_REG(BD70528_INT_BATTSD_HOT_RES, 4, + BD70528_INT_BATTSD_HOT_RES_MASK), + REGMAP_IRQ_REG(BD70528_INT_BATTSD_HOT_DET, 4, + BD70528_INT_BATTSD_HOT_DET_MASK), + REGMAP_IRQ_REG(BD70528_INT_CHG_TSD, 4, BD70528_INT_CHG_TSD_MASK), + REGMAP_IRQ_REG(BD70528_INT_BAT_RMV, 5, BD70528_INT_BAT_RMV_MASK), + REGMAP_IRQ_REG(BD70528_INT_BAT_DET, 5, BD70528_INT_BAT_DET_MASK), + REGMAP_IRQ_REG(BD70528_INT_DCIN2_OV_RES, 5, + BD70528_INT_DCIN2_OV_RES_MASK), + REGMAP_IRQ_REG(BD70528_INT_DCIN2_OV_DET, 5, + BD70528_INT_DCIN2_OV_DET_MASK), + REGMAP_IRQ_REG(BD70528_INT_DCIN2_RMV, 5, BD70528_INT_DCIN2_RMV_MASK), + REGMAP_IRQ_REG(BD70528_INT_DCIN2_DET, 5, BD70528_INT_DCIN2_DET_MASK), + REGMAP_IRQ_REG(BD70528_INT_DCIN1_RMV, 5, BD70528_INT_DCIN1_RMV_MASK), + REGMAP_IRQ_REG(BD70528_INT_DCIN1_DET, 5, BD70528_INT_DCIN1_DET_MASK), + REGMAP_IRQ_REG(BD70528_INT_RTC_ALARM, 6, BD70528_INT_RTC_ALARM_MASK), + REGMAP_IRQ_REG(BD70528_INT_ELPS_TIM, 6, BD70528_INT_ELPS_TIM_MASK), + REGMAP_IRQ_REG(BD70528_INT_GPIO0, 7, BD70528_INT_GPIO0_MASK), + REGMAP_IRQ_REG(BD70528_INT_GPIO1, 7, BD70528_INT_GPIO1_MASK), + REGMAP_IRQ_REG(BD70528_INT_GPIO2, 7, BD70528_INT_GPIO2_MASK), + REGMAP_IRQ_REG(BD70528_INT_GPIO3, 7, BD70528_INT_GPIO3_MASK), + REGMAP_IRQ_REG(BD70528_INT_BUCK1_DVS_OPFAIL, 8, + BD70528_INT_BUCK1_DVS_OPFAIL_MASK), + REGMAP_IRQ_REG(BD70528_INT_BUCK2_DVS_OPFAIL, 8, + BD70528_INT_BUCK2_DVS_OPFAIL_MASK), + REGMAP_IRQ_REG(BD70528_INT_BUCK3_DVS_OPFAIL, 8, + BD70528_INT_BUCK3_DVS_OPFAIL_MASK), + REGMAP_IRQ_REG(BD70528_INT_LED1_VOLT_OPFAIL, 8, + BD70528_INT_LED1_VOLT_OPFAIL_MASK), + REGMAP_IRQ_REG(BD70528_INT_LED2_VOLT_OPFAIL, 8, + BD70528_INT_LED2_VOLT_OPFAIL_MASK), +}; + +static struct regmap_irq_chip bd70528_irq_chip = { + .name = "bd70528_irq", + .main_status = BD70528_REG_INT_MAIN, + .irqs = &irqs[0], + .num_irqs = ARRAY_SIZE(irqs), + .status_base = BD70528_REG_INT_SHDN, + .mask_base = BD70528_REG_INT_SHDN_MASK, + .ack_base = BD70528_REG_INT_SHDN, + .type_base = BD70528_REG_GPIO1_IN, + .init_ack_masked = true, + .num_regs = 9, + .num_main_regs = 1, + .num_type_reg = 4, + .sub_reg_offsets = &bd70528_sub_irq_offsets[0], + .num_main_status_bits = 8, + .irq_reg_stride = 1, +}; + +#define WD_CTRL_MAGIC1 0x55 +#define WD_CTRL_MAGIC2 0xAA +/** + * bd70528_wdt_set - arm or disarm watchdog timer + * + * @data: device data for the PMIC instance we want to operate on + * @enable: new state of WDT. zero to disable, non zero to enable + * @old_state: previous state of WDT will be filled here + * + * Arm or disarm WDT on BD70528 PMIC. Expected to be called only by + * BD70528 RTC and BD70528 WDT drivers. The rtc_timer_lock must be taken + * by calling bd70528_wdt_lock before calling bd70528_wdt_set. + */ +int bd70528_wdt_set(struct rohm_regmap_dev *data, int enable, int *old_state) +{ + int ret, i; + unsigned int tmp; + struct pmic_data *bd70528 = container_of(data, struct pmic_data, + chip); + u8 wd_ctrl_arr[3] = { WD_CTRL_MAGIC1, WD_CTRL_MAGIC2, 0 }; + u8 *wd_ctrl = &wd_ctrl_arr[2]; + + ret = regmap_read(bd70528->chip.regmap, BD70528_REG_WDT_CTRL, &tmp); + if (ret) + return ret; + + *wd_ctrl = (u8)tmp; + + if (old_state) { + if (*wd_ctrl & BD70528_MASK_WDT_EN) + *old_state |= BD70528_WDT_STATE_BIT; + else + *old_state &= ~BD70528_WDT_STATE_BIT; + if ((!enable) == (!(*old_state & BD70528_WDT_STATE_BIT))) + return 0; + } + + if (enable) { + if (*wd_ctrl & BD70528_MASK_WDT_EN) + return 0; + *wd_ctrl |= BD70528_MASK_WDT_EN; + } else { + if (*wd_ctrl & BD70528_MASK_WDT_EN) + *wd_ctrl &= ~BD70528_MASK_WDT_EN; + else + return 0; + } + + for (i = 0; i < 3; i++) { + ret = regmap_write(bd70528->chip.regmap, BD70528_REG_WDT_CTRL, + wd_ctrl_arr[i]); + if (ret) + return ret; + } + + ret = regmap_read(bd70528->chip.regmap, BD70528_REG_WDT_CTRL, &tmp); + if ((tmp & BD70528_MASK_WDT_EN) != (*wd_ctrl & BD70528_MASK_WDT_EN)) { + dev_err(bd70528->chip.dev, + "Watchdog ctrl mismatch (hw) 0x%x (set) 0x%x\n", + tmp, *wd_ctrl); + ret = -EIO; + } + + return ret; +} +EXPORT_SYMBOL(bd70528_wdt_set); + +/** + * bd70528_wdt_lock - take WDT lock + * + * @bd70528: device data for the PMIC instance we want to operate on + * + * Lock WDT for arming/disarming in order to avoid race condition caused + * by WDT state changes initiated by WDT and RTC drivers. + */ +void bd70528_wdt_lock(struct rohm_regmap_dev *data) +{ + struct pmic_data *bd70528 = container_of(data, struct pmic_data, + chip); + + mutex_lock(&bd70528->rtc_timer_lock); +} +EXPORT_SYMBOL(bd70528_wdt_lock); + +/** + * bd70528_wdt_unlock - unlock WDT lock + * + * @bd70528: device data for the PMIC instance we want to operate on + * + * Unlock WDT lock which has previously been taken by call to + * bd70528_wdt_lock. + */ +void bd70528_wdt_unlock(struct rohm_regmap_dev *data) +{ + struct pmic_data *bd70528 = container_of(data, struct pmic_data, + chip); + + mutex_unlock(&bd70528->rtc_timer_lock); +} +EXPORT_SYMBOL(bd70528_wdt_unlock); + +#define BD70528_NUM_OF_GPIOS 4 + +static int bd70528_i2c_probe(struct i2c_client *i2c, + const struct i2c_device_id *id) +{ + struct pmic_data *bd70528; + struct regmap_irq_chip_data *irq_data; + int ret, i; + + if (!i2c->irq) { + dev_err(&i2c->dev, "No IRQ configured\n"); + return -EINVAL; + } + + bd70528 = devm_kzalloc(&i2c->dev, sizeof(*bd70528), GFP_KERNEL); + if (!bd70528) + return -ENOMEM; + + mutex_init(&bd70528->rtc_timer_lock); + + dev_set_drvdata(&i2c->dev, &bd70528->chip); + + bd70528->chip.chip_type = ROHM_CHIP_TYPE_BD70528; + bd70528->chip.regmap = devm_regmap_init_i2c(i2c, &bd70528_regmap); + if (IS_ERR(bd70528->chip.regmap)) { + dev_err(&i2c->dev, "regmap initialization failed\n"); + return PTR_ERR(bd70528->chip.regmap); + } + + /* + * Disallow type setting for all IRQs by default as + * most of them do not support setting type. + */ + for (i = 0; i < ARRAY_SIZE(irqs); i++) + irqs[i].type.types_supported = 0; + + /* + * Set IRQ typesetting information for GPIO pins 0 - 3 + */ + for (i = 0; i < BD70528_NUM_OF_GPIOS; i++) { + struct regmap_irq_type *type; + + type = &irqs[BD70528_INT_GPIO0 + i].type; + type->type_reg_offset = 2 * i; + type->type_rising_val = 0x20; + type->type_falling_val = 0x10; + type->type_level_high_val = 0x40; + type->type_level_low_val = 0x50; + type->types_supported = (IRQ_TYPE_EDGE_BOTH | + IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW); + } + + ret = devm_regmap_add_irq_chip(&i2c->dev, bd70528->chip.regmap, + i2c->irq, IRQF_ONESHOT, 0, + &bd70528_irq_chip, &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", + bd70528_irq_chip.num_irqs); + + /* + * BD70528 IRQ controller is not touching the main mask register. + * So enable the GPIO block interrupts at main level. We can just + * leave them enabled as the IRQ controller should disable IRQs + * from sub-registers when IRQ is disabled or freed. + */ + ret = regmap_update_bits(bd70528->chip.regmap, + BD70528_REG_INT_MAIN_MASK, + BD70528_INT_GPIO_MASK, 0); + + ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, + bd70528_mfd_cells, + ARRAY_SIZE(bd70528_mfd_cells), NULL, 0, + regmap_irq_get_domain(irq_data)); + if (ret) + dev_err(&i2c->dev, "Failed to create subdevices\n"); + + return ret; +} + +static const struct of_device_id bd70528_of_match[] = { + { .compatible = "rohm,bd70528", }, + { }, +}; +MODULE_DEVICE_TABLE(of, bd70528_of_match); + +static struct i2c_driver bd70528_drv = { + .driver = { + .name = "rohm-bd70528", + .of_match_table = bd70528_of_match, + }, + .probe = &bd70528_i2c_probe, +}; + +module_i2c_driver(bd70528_drv); + +MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>"); +MODULE_DESCRIPTION("ROHM BD70528 Power Management IC driver"); +MODULE_LICENSE("GPL"); diff --git a/include/linux/mfd/rohm-bd70528.h b/include/linux/mfd/rohm-bd70528.h new file mode 100644 index 000000000000..155dc9c89e13 --- /dev/null +++ b/include/linux/mfd/rohm-bd70528.h @@ -0,0 +1,383 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* Copyright (C) 2018 ROHM Semiconductors */ + +#ifndef __LINUX_MFD_BD70528_H__ +#define __LINUX_MFD_BD70528_H__ + +#include <linux/device.h> +#include <linux/mfd/rohm-generic.h> +#include <linux/regmap.h> + +enum { + BD70528_BUCK1, + BD70528_BUCK2, + BD70528_BUCK3, + BD70528_LDO1, + BD70528_LDO2, + BD70528_LDO3, + BD70528_LED1, + BD70528_LED2, +}; + +#define BD70528_BUCK_VOLTS 17 +#define BD70528_BUCK_VOLTS 17 +#define BD70528_BUCK_VOLTS 17 +#define BD70528_LDO_VOLTS 0x20 + +#define BD70528_REG_BUCK1_EN 0x0F +#define BD70528_REG_BUCK1_VOLT 0x15 +#define BD70528_REG_BUCK2_EN 0x10 +#define BD70528_REG_BUCK2_VOLT 0x16 +#define BD70528_REG_BUCK3_EN 0x11 +#define BD70528_REG_BUCK3_VOLT 0x17 +#define BD70528_REG_LDO1_EN 0x1b +#define BD70528_REG_LDO1_VOLT 0x1e +#define BD70528_REG_LDO2_EN 0x1c +#define BD70528_REG_LDO2_VOLT 0x1f +#define BD70528_REG_LDO3_EN 0x1d +#define BD70528_REG_LDO3_VOLT 0x20 +#define BD70528_REG_LED_CTRL 0x2b +#define BD70528_REG_LED_VOLT 0x29 +#define BD70528_REG_LED_EN 0x2a + +/* main irq registers */ +#define BD70528_REG_INT_MAIN 0x7E +#define BD70528_REG_INT_MAIN_MASK 0x74 + +/* 'sub irq' registers */ +#define BD70528_REG_INT_SHDN 0x7F +#define BD70528_REG_INT_PWR_FLT 0x80 +#define BD70528_REG_INT_VR_FLT 0x81 +#define BD70528_REG_INT_MISC 0x82 +#define BD70528_REG_INT_BAT1 0x83 +#define BD70528_REG_INT_BAT2 0x84 +#define BD70528_REG_INT_RTC 0x85 +#define BD70528_REG_INT_GPIO 0x86 +#define BD70528_REG_INT_OP_FAIL 0x87 + +#define BD70528_REG_INT_SHDN_MASK 0x75 +#define BD70528_REG_INT_PWR_FLT_MASK 0x76 +#define BD70528_REG_INT_VR_FLT_MASK 0x77 +#define BD70528_REG_INT_MISC_MASK 0x78 +#define BD70528_REG_INT_BAT1_MASK 0x79 +#define BD70528_REG_INT_BAT2_MASK 0x7a +#define BD70528_REG_INT_RTC_MASK 0x7b +#define BD70528_REG_INT_GPIO_MASK 0x7c +#define BD70528_REG_INT_OP_FAIL_MASK 0x7d + +/* Reset related 'magic' registers */ +#define BD70528_REG_SHIPMODE 0x03 +#define BD70528_REG_HWRESET 0x04 +#define BD70528_REG_WARMRESET 0x05 +#define BD70528_REG_STANDBY 0x06 + +/* GPIO registers */ +#define BD70528_REG_GPIO_STATE 0x8F + +#define BD70528_REG_GPIO1_IN 0x4d +#define BD70528_REG_GPIO2_IN 0x4f +#define BD70528_REG_GPIO3_IN 0x51 +#define BD70528_REG_GPIO4_IN 0x53 +#define BD70528_REG_GPIO1_OUT 0x4e +#define BD70528_REG_GPIO2_OUT 0x50 +#define BD70528_REG_GPIO3_OUT 0x52 +#define BD70528_REG_GPIO4_OUT 0x54 + +/* clk control */ + +#define BD70528_REG_CLK_OUT 0x2c + +/* RTC */ + +#define BD70528_REG_RTC_COUNT_H 0x2d +#define BD70528_REG_RTC_COUNT_L 0x2e +#define BD70528_REG_RTC_SEC 0x2f +#define BD70528_REG_RTC_MINUTE 0x30 +#define BD70528_REG_RTC_HOUR 0x31 +#define BD70528_REG_RTC_WEEK 0x32 +#define BD70528_REG_RTC_DAY 0x33 +#define BD70528_REG_RTC_MONTH 0x34 +#define BD70528_REG_RTC_YEAR 0x35 + +#define BD70528_REG_RTC_ALM_SEC 0x36 +#define BD70528_REG_RTC_ALM_START BD70528_REG_RTC_ALM_SEC +#define BD70528_REG_RTC_ALM_MINUTE 0x37 +#define BD70528_REG_RTC_ALM_HOUR 0x38 +#define BD70528_REG_RTC_ALM_WEEK 0x39 +#define BD70528_REG_RTC_ALM_DAY 0x3a +#define BD70528_REG_RTC_ALM_MONTH 0x3b +#define BD70528_REG_RTC_ALM_YEAR 0x3c +#define BD70528_REG_RTC_ALM_MASK 0x3d +#define BD70528_REG_RTC_ALM_REPEAT 0x3e +#define BD70528_REG_RTC_START BD70528_REG_RTC_SEC + +#define BD70528_REG_RTC_WAKE_SEC 0x43 +#define BD70528_REG_RTC_WAKE_START BD70528_REG_RTC_WAKE_SEC +#define BD70528_REG_RTC_WAKE_MIN 0x44 +#define BD70528_REG_RTC_WAKE_HOUR 0x45 +#define BD70528_REG_RTC_WAKE_CTRL 0x46 + +#define BD70528_REG_ELAPSED_TIMER_EN 0x42 +#define BD70528_REG_WAKE_EN 0x46 + +/* WDT registers */ +#define BD70528_REG_WDT_CTRL 0x4A +#define BD70528_REG_WDT_HOUR 0x49 +#define BD70528_REG_WDT_MINUTE 0x48 +#define BD70528_REG_WDT_SEC 0x47 + +/* Charger / Battery */ +#define BD70528_REG_CHG_CURR_STAT 0x59 +#define BD70528_REG_CHG_BAT_STAT 0x57 +#define BD70528_REG_CHG_BAT_TEMP 0x58 +#define BD70528_REG_CHG_IN_STAT 0x56 +#define BD70528_REG_CHG_DCIN_ILIM 0x5d +#define BD70528_REG_CHG_CHG_CURR_WARM 0x61 +#define BD70528_REG_CHG_CHG_CURR_COLD 0x62 + + +/* Masks for main IRQ register bits */ +enum { + BD70528_INT_SHDN, +#define BD70528_INT_SHDN_MASK (1<<BD70528_INT_SHDN) + BD70528_INT_PWR_FLT, +#define BD70528_INT_PWR_FLT_MASK (1<<BD70528_INT_PWR_FLT) + BD70528_INT_VR_FLT, +#define BD70528_INT_VR_FLT_MASK (1<<BD70528_INT_VR_FLT) + BD70528_INT_MISC, +#define BD70528_INT_MISC_MASK (1<<BD70528_INT_MISC) + BD70528_INT_BAT1, +#define BD70528_INT_BAT1_MASK (1<<BD70528_INT_BAT1) + BD70528_INT_RTC, +#define BD70528_INT_RTC_MASK (1<<BD70528_INT_RTC) + BD70528_INT_GPIO, +#define BD70528_INT_GPIO_MASK (1<<BD70528_INT_GPIO) + BD70528_INT_OP_FAIL, +#define BD70528_INT_OP_FAIL_MASK (1<<BD70528_INT_OP_FAIL) +}; + +/* IRQs */ +enum { + /* Shutdown register IRQs */ + BD70528_INT_LONGPUSH, + BD70528_INT_WDT, + BD70528_INT_HWRESET, + BD70528_INT_RSTB_FAULT, + BD70528_INT_VBAT_UVLO, + BD70528_INT_TSD, + BD70528_INT_RSTIN, + /* Power failure register IRQs */ + BD70528_INT_BUCK1_FAULT, + BD70528_INT_BUCK2_FAULT, + BD70528_INT_BUCK3_FAULT, + BD70528_INT_LDO1_FAULT, + BD70528_INT_LDO2_FAULT, + BD70528_INT_LDO3_FAULT, + BD70528_INT_LED1_FAULT, + BD70528_INT_LED2_FAULT, + /* VR FAULT register IRQs */ + BD70528_INT_BUCK1_OCP, + BD70528_INT_BUCK2_OCP, + BD70528_INT_BUCK3_OCP, + BD70528_INT_LED1_OCP, + BD70528_INT_LED2_OCP, + BD70528_INT_BUCK1_FULLON, + BD70528_INT_BUCK2_FULLON, + /* PMU register interrupts */ + BD70528_INT_SHORTPUSH, + BD70528_INT_AUTO_WAKEUP, + BD70528_INT_STATE_CHANGE, + /* Charger 1 register IRQs */ + BD70528_INT_BAT_OV_RES, + BD70528_INT_BAT_OV_DET, + BD70528_INT_DBAT_DET, + BD70528_INT_BATTSD_COLD_RES, + BD70528_INT_BATTSD_COLD_DET, + BD70528_INT_BATTSD_HOT_RES, + BD70528_INT_BATTSD_HOT_DET, + BD70528_INT_CHG_TSD, + /* Charger 2 register IRQs */ + BD70528_INT_BAT_RMV, + BD70528_INT_BAT_DET, + BD70528_INT_DCIN2_OV_RES, + BD70528_INT_DCIN2_OV_DET, + BD70528_INT_DCIN2_RMV, + BD70528_INT_DCIN2_DET, + BD70528_INT_DCIN1_RMV, + BD70528_INT_DCIN1_DET, + /* RTC register IRQs */ + BD70528_INT_RTC_ALARM, + BD70528_INT_ELPS_TIM, + /* GPIO register IRQs */ + BD70528_INT_GPIO0, + BD70528_INT_GPIO1, + BD70528_INT_GPIO2, + BD70528_INT_GPIO3, + /* Invalid operation register IRQs */ + BD70528_INT_BUCK1_DVS_OPFAIL, + BD70528_INT_BUCK2_DVS_OPFAIL, + BD70528_INT_BUCK3_DVS_OPFAIL, + BD70528_INT_LED1_VOLT_OPFAIL, + BD70528_INT_LED2_VOLT_OPFAIL, +}; + +/* Masks */ +#define BD70528_INT_LONGPUSH_MASK 0x1 +#define BD70528_INT_WDT_MASK 0x2 +#define BD70528_INT_HWRESET_MASK 0x4 +#define BD70528_INT_RSTB_FAULT_MASK 0x8 +#define BD70528_INT_VBAT_UVLO_MASK 0x10 +#define BD70528_INT_TSD_MASK 0x20 +#define BD70528_INT_RSTIN_MASK 0x40 + +#define BD70528_INT_BUCK1_FAULT_MASK 0x1 +#define BD70528_INT_BUCK2_FAULT_MASK 0x2 +#define BD70528_INT_BUCK3_FAULT_MASK 0x4 +#define BD70528_INT_LDO1_FAULT_MASK 0x8 +#define BD70528_INT_LDO2_FAULT_MASK 0x10 +#define BD70528_INT_LDO3_FAULT_MASK 0x20 +#define BD70528_INT_LED1_FAULT_MASK 0x40 +#define BD70528_INT_LED2_FAULT_MASK 0x80 + +#define BD70528_INT_BUCK1_OCP_MASK 0x1 +#define BD70528_INT_BUCK2_OCP_MASK 0x2 +#define BD70528_INT_BUCK3_OCP_MASK 0x4 +#define BD70528_INT_LED1_OCP_MASK 0x8 +#define BD70528_INT_LED2_OCP_MASK 0x10 +#define BD70528_INT_BUCK1_FULLON_MASK 0x20 +#define BD70528_INT_BUCK2_FULLON_MASK 0x40 + +#define BD70528_INT_SHORTPUSH_MASK 0x1 +#define BD70528_INT_AUTO_WAKEUP_MASK 0x2 +#define BD70528_INT_STATE_CHANGE_MASK 0x10 + +#define BD70528_INT_BAT_OV_RES_MASK 0x1 +#define BD70528_INT_BAT_OV_DET_MASK 0x2 +#define BD70528_INT_DBAT_DET_MASK 0x4 +#define BD70528_INT_BATTSD_COLD_RES_MASK 0x8 +#define BD70528_INT_BATTSD_COLD_DET_MASK 0x10 +#define BD70528_INT_BATTSD_HOT_RES_MASK 0x20 +#define BD70528_INT_BATTSD_HOT_DET_MASK 0x40 +#define BD70528_INT_CHG_TSD_MASK 0x80 + +#define BD70528_INT_BAT_RMV_MASK 0x1 +#define BD70528_INT_BAT_DET_MASK 0x2 +#define BD70528_INT_DCIN2_OV_RES_MASK 0x4 +#define BD70528_INT_DCIN2_OV_DET_MASK 0x8 +#define BD70528_INT_DCIN2_RMV_MASK 0x10 +#define BD70528_INT_DCIN2_DET_MASK 0x20 +#define BD70528_INT_DCIN1_RMV_MASK 0x40 +#define BD70528_INT_DCIN1_DET_MASK 0x80 + +#define BD70528_INT_RTC_ALARM_MASK 0x1 +#define BD70528_INT_ELPS_TIM_MASK 0x2 + +#define BD70528_INT_GPIO0_MASK 0x1 +#define BD70528_INT_GPIO1_MASK 0x2 +#define BD70528_INT_GPIO2_MASK 0x4 +#define BD70528_INT_GPIO3_MASK 0x8 + +#define BD70528_INT_BUCK1_DVS_OPFAIL_MASK 0x1 +#define BD70528_INT_BUCK2_DVS_OPFAIL_MASK 0x2 +#define BD70528_INT_BUCK3_DVS_OPFAIL_MASK 0x4 +#define BD70528_INT_LED1_VOLT_OPFAIL_MASK 0x10 +#define BD70528_INT_LED2_VOLT_OPFAIL_MASK 0x20 + +#define BD70528_DEBOUNCE_MASK 0x3 + +#define BD70528_DEBOUNCE_DISABLE 0 +#define BD70528_DEBOUNCE_15MS 1 +#define BD70528_DEBOUNCE_30MS 2 +#define BD70528_DEBOUNCE_50MS 3 + +#define BD70528_GPIO_DRIVE_MASK 0x2 +#define BD70528_GPIO_PUSH_PULL 0x0 +#define BD70528_GPIO_OPEN_DRAIN 0x2 + +#define BD70528_GPIO_OUT_EN_MASK 0x80 +#define BD70528_GPIO_OUT_ENABLE 0x80 +#define BD70528_GPIO_OUT_DISABLE 0x0 + +#define BD70528_GPIO_OUT_HI 0x1 +#define BD70528_GPIO_OUT_LO 0x0 +#define BD70528_GPIO_OUT_MASK 0x1 + +#define BD70528_GPIO_IN_STATE_BASE 1 + +#define BD70528_CLK_OUT_EN_MASK 0x1 + +/* RTC masks to mask out reserved bits */ + +#define BD70528_MASK_RTC_SEC 0x7f +#define BD70528_MASK_RTC_MINUTE 0x7f +#define BD70528_MASK_RTC_HOUR_24H 0x80 +#define BD70528_MASK_RTC_HOUR_PM 0x20 +#define BD70528_MASK_RTC_HOUR 0x1f +#define BD70528_MASK_RTC_DAY 0x3f +#define BD70528_MASK_RTC_WEEK 0x07 +#define BD70528_MASK_RTC_MONTH 0x1f +#define BD70528_MASK_RTC_YEAR 0xff +#define BD70528_MASK_RTC_COUNT_L 0x7f + +#define BD70528_MASK_ELAPSED_TIMER_EN 0x1 +/* Mask second, min and hour fields + * HW would support ALM irq for over 24h + * (by setting day, month and year too) + * but as we wish to keep this same as for + * wake-up we limit ALM to 24H and only + * unmask sec, min and hour + */ +#define BD70528_MASK_ALM_EN 0x7 +#define BD70528_MASK_WAKE_EN 0x1 + +/* WDT masks */ +#define BD70528_MASK_WDT_EN 0x1 +#define BD70528_MASK_WDT_HOUR 0x1 +#define BD70528_MASK_WDT_MINUTE 0x7f +#define BD70528_MASK_WDT_SEC 0x7f + +#define BD70528_WDT_STATE_BIT 0x1 +#define BD70528_ELAPSED_STATE_BIT 0x2 +#define BD70528_WAKE_STATE_BIT 0x4 + +/* Charger masks */ +#define BD70528_MASK_CHG_STAT 0x7f +#define BD70528_MASK_CHG_BAT_TIMER 0x20 +#define BD70528_MASK_CHG_BAT_OVERVOLT 0x10 +#define BD70528_MASK_CHG_BAT_DETECT 0x1 +#define BD70528_MASK_CHG_DCIN1_UVLO 0x1 +#define BD70528_MASK_CHG_DCIN_ILIM 0x3f +#define BD70528_MASK_CHG_CHG_CURR 0x1f +#define BD70528_MASK_CHG_TRICKLE_CURR 0x10 + +/* + * Note, external battery register is the lonely rider at + * address 0xc5. See how to stuff that in the regmap + */ +#define BD70528_MAX_REGISTER 0x94 + +/* Buck control masks */ +#define BD70528_MASK_RUN_EN 0x4 +#define BD70528_MASK_STBY_EN 0x2 +#define BD70528_MASK_IDLE_EN 0x1 +#define BD70528_MASK_LED1_EN 0x1 +#define BD70528_MASK_LED2_EN 0x10 + +#define BD70528_MASK_BUCK_VOLT 0xf +#define BD70528_MASK_LDO_VOLT 0x1f +#define BD70528_MASK_LED1_VOLT 0x1 +#define BD70528_MASK_LED2_VOLT 0x10 + +/* Misc irq masks */ +#define BD70528_INT_MASK_SHORT_PUSH 1 +#define BD70528_INT_MASK_AUTO_WAKE 2 +#define BD70528_INT_MASK_POWER_STATE 4 + +#define BD70528_MASK_BUCK_RAMP 0x10 +#define BD70528_SIFT_BUCK_RAMP 4 + +int bd70528_wdt_set(struct rohm_regmap_dev *data, int enable, int *old_state); +void bd70528_wdt_lock(struct rohm_regmap_dev *data); +void bd70528_wdt_unlock(struct rohm_regmap_dev *data); + +#endif /* __LINUX_MFD_BD70528_H__ */
ROHM BD70528MWV is an ultra-low quiescent current general purpose single-chip power management IC for battery-powered portable devices. Add MFD core which enables chip access for following subdevices: - regulators/LED drivers - battery-charger - gpios - 32.768kHz clk - RTC - watchdog Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> --- drivers/mfd/Kconfig | 17 ++ drivers/mfd/Makefile | 1 + drivers/mfd/rohm-bd70528.c | 438 +++++++++++++++++++++++++++++++ include/linux/mfd/rohm-bd70528.h | 383 +++++++++++++++++++++++++++ 4 files changed, 839 insertions(+) create mode 100644 drivers/mfd/rohm-bd70528.c create mode 100644 include/linux/mfd/rohm-bd70528.h