On Wed, Jul 07, 2021 at 06:25:37AM +0900, Jaehoon Chung wrote: > On 7/6/21 5:52 PM, Stephan Gerhold wrote: > > On Tue, Jul 06, 2021 at 10:08:03AM +0900, Jaehoon Chung wrote: > >> On 7/6/21 1:28 AM, Stephan Gerhold wrote: > >>> All devices based on ST-Ericsson Ux500 use a PMIC similar to AB8500 > >>> (Analog Baseband). There is AB8500, AB8505, AB9540 and AB8540 > >>> although in practice only AB8500 and AB8505 are relevant since the > >>> platforms with AB9540 and AB8540 were cancelled and never used in > >>> production. > >>> > >>> In general, the AB8500 PMIC uses I2C as control interface, where the > >>> different register banks are represented as separate I2C devices. > >>> However, in practice AB8500 is always connected to a special I2C bus > >>> on the DB8500 SoC that is controlled by the power/reset/clock > >>> management unit (PRCMU) firmware. > >>> > >>> Add a simple driver that allows reading/writing registers of the > >>> AB8500 PMIC. The driver directly accesses registers from the PRCMU > >>> parent device (represented by syscon in U-Boot). Abstracting it > >>> further (e.g. with the i2c uclass) would not provide any advantage > >>> because the PRCMU I2C bus is always just connected to AB8500 and > >>> vice-versa. > >>> > >>> The ab8500.h header is mostly taken as-is from Linux (with some > >>> minor adjustments) to allow using similar code in both Linux and > >>> U-Boot. > >>> > >>> Cc: Linus Walleij <linus.wall...@linaro.org> > >>> Signed-off-by: Stephan Gerhold <step...@gerhold.net> > >>> --- > >>> > >>> drivers/power/pmic/Kconfig | 10 ++ > >>> drivers/power/pmic/Makefile | 1 + > >>> drivers/power/pmic/ab8500.c | 258 ++++++++++++++++++++++++++++++++++++ > >>> include/power/ab8500.h | 125 +++++++++++++++++ > >>> 4 files changed, 394 insertions(+) > >>> create mode 100644 drivers/power/pmic/ab8500.c > >>> create mode 100644 include/power/ab8500.h > >>> > >>> diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig > >>> index 583fd3ddcd..fd6648b313 100644 > >>> --- a/drivers/power/pmic/Kconfig > >>> +++ b/drivers/power/pmic/Kconfig > >>> @@ -31,6 +31,16 @@ config SPL_PMIC_CHILDREN > >>> to call your regulator code (e.g. see rk8xx.c for direct functions > >>> for use in SPL). > >>> > >>> +config PMIC_AB8500 > >>> + bool "Enable driver for ST-Ericsson AB8500 PMIC via PRCMU" > >>> + depends on DM_PMIC > >>> + select REGMAP > >>> + select SYSCON > >>> + help > >>> + Enable support for the ST-Ericsson AB8500 (Analog Baseband) PMIC. > >>> + It connects with the ST-Ericsson DB8500 SoC via an I2C bus managed by > >>> + the power/reset/clock management unit (PRCMU) firmware. > >>> + > >>> config PMIC_ACT8846 > >>> bool "Enable support for the active-semi 8846 PMIC" > >>> depends on DM_PMIC && DM_I2C > >>> diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile > >>> index 89099fde57..5d1a97e5f6 100644 > >>> --- a/drivers/power/pmic/Makefile > >>> +++ b/drivers/power/pmic/Makefile > >>> @@ -15,6 +15,7 @@ obj-$(CONFIG_$(SPL_)DM_PMIC_PFUZE100) += pfuze100.o > >>> obj-$(CONFIG_$(SPL_)DM_PMIC_PCA9450) += pca9450.o > >>> obj-$(CONFIG_PMIC_S2MPS11) += s2mps11.o > >>> obj-$(CONFIG_DM_PMIC_SANDBOX) += sandbox.o i2c_pmic_emul.o > >>> +obj-$(CONFIG_PMIC_AB8500) += ab8500.o > >>> obj-$(CONFIG_PMIC_ACT8846) += act8846.o > >>> obj-$(CONFIG_PMIC_AS3722) += as3722.o as3722_gpio.o > >>> obj-$(CONFIG_PMIC_MAX8997) += max8997.o > >>> diff --git a/drivers/power/pmic/ab8500.c b/drivers/power/pmic/ab8500.c > >>> new file mode 100644 > >>> index 0000000000..a87a3b497c > >>> --- /dev/null > >>> +++ b/drivers/power/pmic/ab8500.c > >>> @@ -0,0 +1,258 @@ > >>> +// SPDX-License-Identifier: GPL-2.0+ > >>> +/* > >>> + * Copyright (C) 2019 Stephan Gerhold > >>> + * > >>> + * Adapted from old U-Boot and Linux kernel implementation: > >>> + * Copyright (C) STMicroelectronics 2009 > >>> + * Copyright (C) ST-Ericsson SA 2010 > >>> + */ > >>> + > >>> +#include <common.h> > >>> +#include <dm.h> > >>> +#include <regmap.h> > >>> +#include <syscon.h> > >>> +#include <linux/bitops.h> > >>> +#include <linux/err.h> > >>> +#include <power/ab8500.h> > >>> +#include <power/pmic.h> > >>> + > >>> +/* CPU mailbox registers */ > >>> +#define PRCM_MBOX_CPU_VAL 0x0fc > >>> +#define PRCM_MBOX_CPU_SET 0x100 > >>> +#define PRCM_MBOX_CPU_CLR 0x104 > >>> + > >>> +#define PRCM_ARM_IT1_CLR 0x48C > >>> +#define PRCM_ARM_IT1_VAL 0x494 > >>> + > >>> +#define PRCM_TCDM_RANGE 2 > >>> +#define PRCM_REQ_MB5 0xE44 > >>> +#define PRCM_ACK_MB5 0xDF4 > >>> +#define _PRCM_MBOX_HEADER 0xFE8 > >>> +#define PRCM_MBOX_HEADER_REQ_MB5 (_PRCM_MBOX_HEADER + 0x5) > >>> +#define PRCMU_I2C_MBOX_BIT BIT(5) > >>> + > >>> +/* Mailbox 5 Requests */ > >>> +#define PRCM_REQ_MB5_I2C_SLAVE_OP (PRCM_REQ_MB5 + 0x0) > >>> +#define PRCM_REQ_MB5_I2C_HW_BITS (PRCM_REQ_MB5 + 0x1) > >>> +#define PRCM_REQ_MB5_I2C_REG (PRCM_REQ_MB5 + 0x2) > >>> +#define PRCM_REQ_MB5_I2C_VAL (PRCM_REQ_MB5 + 0x3) > >>> +#define PRCMU_I2C(bank) (((bank) << 1) | BIT(6)) > >>> +#define PRCMU_I2C_WRITE 0 > >>> +#define PRCMU_I2C_READ 1 > >>> +#define PRCMU_I2C_STOP_EN BIT(3) > >>> + > >>> +/* Mailbox 5 ACKs */ > >>> +#define PRCM_ACK_MB5_I2C_STATUS (PRCM_ACK_MB5 + 0x1) > >>> +#define PRCM_ACK_MB5_I2C_VAL (PRCM_ACK_MB5 + 0x3) > >>> +#define PRCMU_I2C_WR_OK 0x1 > >>> +#define PRCMU_I2C_RD_OK 0x2 > >>> + > >>> +/* AB8500 version registers */ > >>> +#define AB8500_MISC_REV_REG AB8500_MISC(0x80) > >>> +#define AB8500_MISC_IC_NAME_REG AB8500_MISC(0x82) > >>> + > >>> +struct ab8500_priv { > >>> + struct ab8500 ab8500; > >>> + struct regmap *regmap; > >>> +}; > >>> + > >>> +static inline int prcmu_tcdm_readb(struct regmap *map, uint offset, u8 > >>> *valp) > >>> +{ > >>> + return regmap_raw_read_range(map, PRCM_TCDM_RANGE, offset, > >>> + valp, sizeof(*valp)); > >>> +} > >>> + > >>> +static inline int prcmu_tcdm_writeb(struct regmap *map, uint offset, u8 > >>> val) > >>> +{ > >>> + return regmap_raw_write_range(map, PRCM_TCDM_RANGE, offset, > >>> + &val, sizeof(val)); > >>> +} > >>> + > >>> +static int prcmu_wait_i2c_mbx_ready(struct ab8500_priv *priv) > >>> +{ > >>> + uint val; > >>> + int ret; > >>> + > >>> + ret = regmap_read(priv->regmap, PRCM_ARM_IT1_VAL, &val); > >> > >> Doesn't need to check about returned to error? > >> > >>> + if (ret == 0 && val & PRCMU_I2C_MBOX_BIT) { > >>> + printf("ab8500: warning: PRCMU i2c mailbox was not acked\n"); > >>> + /* clear mailbox 5 ack irq */ > >>> + regmap_write(priv->regmap, PRCM_ARM_IT1_CLR, > >>> PRCMU_I2C_MBOX_BIT); > >> > >> Not need to check the return value about regmap_write()? > >> > > > > Good point, it's probably better to return in case of errors here. > > > >>> [...] > >>> + > >>> + /* clear mailbox 5 ack irq */ > >>> + regmap_write(priv->regmap, PRCM_ARM_IT1_CLR, PRCMU_I2C_MBOX_BIT); > >> > >> Ditto. > >> > > > > But here it doesn't really make sense: The read/write is already done. > > Even if we don't manage to clear the IRQ the read/write will still have > > succeeded. I will add a comment to clarify this. > > > >>> + > >>> + if (status != expected_status) { > >>> + /* > >>> + * AB8500 does not have the AB8500_MISC_IC_NAME_REG register, > >>> + * but we need to try reading it to detect AB8505. > >>> + * In case of an error, assume that we have AB8500. > >>> + */ > >>> + if (op == PRCMU_I2C_READ && bank_reg == > >>> AB8500_MISC_IC_NAME_REG) { > >>> + *val = AB8500_VERSION_AB8500; > >>> + return 0; > >>> + } > >>> + > >>> + printf("%s: return status %d\n", __func__, status); > >>> + return -EIO; > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +static int ab8500_reg_count(struct udevice *dev) > >>> +{ > >>> + return AB8500_NUM_REGISTERS; > >>> +} > >>> + > >>> +static int ab8500_read(struct udevice *dev, uint reg, uint8_t *buf, int > >>> len) > >>> +{ > >>> + int ret; > >>> + > >>> + if (len != 1) > >>> + return -EINVAL; > >>> + > >>> + *buf = 0; > >>> + ret = ab8500_transfer(dev, reg, buf, PRCMU_I2C_READ, PRCMU_I2C_RD_OK); > >>> + if (ret) { > >>> + printf("%s failed: %d\n", __func__, ret); > >>> + return ret; > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +static int ab8500_write(struct udevice *dev, uint reg, const uint8_t > >>> *buf, int len) > >>> +{ > >>> + int ret; > >>> + u8 val; > >>> + > >>> + if (len != 1) > >>> + return -EINVAL; > >>> + > >>> + val = *buf; > >>> + ret = ab8500_transfer(dev, reg, &val, PRCMU_I2C_WRITE, PRCMU_I2C_WR_OK); > >>> + if (ret) { > >>> + printf("%s failed: %d\n", __func__, ret); > >>> + return ret; > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +static struct dm_pmic_ops ab8500_ops = { > >>> + .reg_count = ab8500_reg_count, > >>> + .read = ab8500_read, > >>> + .write = ab8500_write, > >>> +}; > >>> + > >>> +static int ab8500_probe(struct udevice *dev) > >>> +{ > >>> + struct ab8500_priv *priv = dev_get_priv(dev); > >>> + int ret; > >>> + > >>> + /* get regmap from the PRCMU parent device (syscon in U-Boot) */ > >>> + priv->regmap = syscon_get_regmap(dev->parent); > >>> + if (IS_ERR(priv->regmap)) > >>> + return PTR_ERR(priv->regmap); > >>> + > >>> + ret = pmic_reg_read(dev, AB8500_MISC_IC_NAME_REG); > >>> + if (ret < 0) { > >>> + printf("ab8500: failed to read chip version: %d\n", ret); > >>> + return ret; > >>> + } > >>> + priv->ab8500.version = ret; > >>> + > >>> + ret = pmic_reg_read(dev, AB8500_MISC_REV_REG); > >>> + if (ret < 0) { > >>> + printf("ab8500: failed to read chip id: %d\n", ret); > >>> + return ret; > >>> + } > >>> + priv->ab8500.chip_id = ret; > >>> + > >>> + debug("ab8500: version: %#x, chip id: %#x\n", > >>> + priv->ab8500.version, priv->ab8500.chip_id); > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +static const struct udevice_id ab8500_ids[] = { > >>> + { .compatible = "stericsson,ab8500" }, > >>> + { } > >>> +}; > >>> + > >>> +U_BOOT_DRIVER(pmic_ab8500) = { > >>> + .name = "pmic_ab8500", > >>> + .id = UCLASS_PMIC, > >>> + .of_match = ab8500_ids, > >>> + .bind = dm_scan_fdt_dev, > >>> + .probe = ab8500_probe, > >>> + .ops = &ab8500_ops, > >>> + .priv_auto = sizeof(struct ab8500_priv), > >>> +}; > >>> diff --git a/include/power/ab8500.h b/include/power/ab8500.h > >>> new file mode 100644 > >>> index 0000000000..157eb4a5b1 > >>> --- /dev/null > >>> +++ b/include/power/ab8500.h > >>> @@ -0,0 +1,125 @@ > >>> +/* SPDX-License-Identifier: GPL-2.0-only */ > >>> +/* > >>> + * Based on include/linux/mfd/abx500/ab8500.h from Linux > >>> + * Copyright (C) ST-Ericsson SA 2010 > >>> + * Author: Srinidhi Kasagar <srinidhi.kasa...@stericsson.com> > >>> + */ > >>> + > >>> +#ifndef _PMIC_AB8500_H_ > >>> +#define _PMIC_AB8500_H_ > >>> + > >>> +/* > >>> + * AB IC versions > >>> + * > >>> + * AB8500_VERSION_AB8500 should be 0xFF but will never be read as need a > >>> + * non-supported multi-byte I2C access via PRCMU. Set to 0x00 to ease the > >>> + * print of version string. > >>> + */ > >>> +enum ab8500_version { > >>> + AB8500_VERSION_AB8500 = 0x0, > >>> + AB8500_VERSION_AB8505 = 0x1, > >>> + AB8500_VERSION_AB9540 = 0x2, > >>> + AB8500_VERSION_AB8540 = 0x4, > >>> + AB8500_VERSION_UNDEFINED, > >>> +}; > >>> + > >>> +/* AB8500 CIDs*/ > >>> +#define AB8500_CUTEARLY 0x00 > >>> +#define AB8500_CUT1P0 0x10 > >>> +#define AB8500_CUT1P1 0x11 > >>> +#define AB8500_CUT1P2 0x12 /* Only valid for AB8540 */ > >>> +#define AB8500_CUT2P0 0x20 > >>> +#define AB8500_CUT3P0 0x30 > >>> +#define AB8500_CUT3P3 0x33 > >>> + > >>> +/* > >>> + * AB8500 bank addresses > >>> + */ > >>> +#define AB8500_BANK(bank, reg) (((bank) << 8) | (reg)) > >>> +#define AB8500_M_FSM_RANK(reg) AB8500_BANK(0x0, reg) > >>> +#define AB8500_SYS_CTRL1_BLOCK(reg) AB8500_BANK(0x1, reg) > >>> +#define AB8500_SYS_CTRL2_BLOCK(reg) AB8500_BANK(0x2, reg) > >>> +#define AB8500_REGU_CTRL1(reg) AB8500_BANK(0x3, reg) > >>> +#define AB8500_REGU_CTRL2(reg) AB8500_BANK(0x4, reg) > >>> +#define AB8500_USB(reg) AB8500_BANK(0x5, reg) > >>> +#define AB8500_TVOUT(reg) AB8500_BANK(0x6, reg) > >>> +#define AB8500_DBI(reg) AB8500_BANK(0x7, reg) > >>> +#define AB8500_ECI_AV_ACC(reg) AB8500_BANK(0x8, reg) > >>> +#define AB8500_RESERVED(reg) AB8500_BANK(0x9, reg) > >>> +#define AB8500_GPADC(reg) AB8500_BANK(0xA, reg) > >>> +#define AB8500_CHARGER(reg) AB8500_BANK(0xB, reg) > >>> +#define AB8500_GAS_GAUGE(reg) AB8500_BANK(0xC, reg) > >>> +#define AB8500_AUDIO(reg) AB8500_BANK(0xD, reg) > >>> +#define AB8500_INTERRUPT(reg) AB8500_BANK(0xE, reg) > >>> +#define AB8500_RTC(reg) AB8500_BANK(0xF, reg) > >>> +#define AB8500_GPIO(reg) AB8500_BANK(0x10, reg) > >>> +#define AB8500_MISC(reg) AB8500_BANK(0x10, reg) > >>> +#define AB8500_DEVELOPMENT(reg) AB8500_BANK(0x11, reg) > >>> +#define AB8500_DEBUG(reg) AB8500_BANK(0x12, reg) > >>> +#define AB8500_PROD_TEST(reg) AB8500_BANK(0x13, reg) > >>> +#define AB8500_STE_TEST(reg) AB8500_BANK(0x14, reg) > >>> +#define AB8500_OTP_EMUL(reg) AB8500_BANK(0x15, reg) > >>> + > >>> +#define AB8500_NUM_BANKS 0x16 > >>> +#define AB8500_NUM_REGISTERS AB8500_BANK(AB8500_NUM_BANKS, 0) > >>> + > >>> +struct ab8500 { > >>> + enum ab8500_version version; > >>> + u8 chip_id; > >>> +};> + > >> > >> I have checked that below codes are existed in linux kernel. > >> But I didn't find where the below codes is used in U-boot. > >> Will it be used in future? > >> > > > > At the moment I don't have any drivers myself that make use of this. > > However, it would certainly be needed if more advanced drivers for > > AB8500 are written. I don't know if this will happen anytime soon. > > > > I can remove these if you would prefer to have them added once they are > > actually needed. What do you think? > > If you want to maintain them, not need to remove them. :) >
Yes, I think they are fine. Will send a v2 with the other suggested changes soon. Thanks! Stephan