On Thu, Jan 14, 2021 at 2:07 PM Jaehoon Chung <jh80.ch...@samsung.com> wrote:
>
> On 1/14/21 2:00 AM, Tim Harvey wrote:
> > This adds basic register access and child regulator binding
> > for the Monolithic MP5416 Power Management IC which integrates
> > four DC/DC switching converters and five LDO regulators.
> >
> > Signed-off-by: Tim Harvey <thar...@gateworks.com>
> > ---
> > drivers/power/pmic/Kconfig | 15 +++++++
> > drivers/power/pmic/Makefile | 1 +
> > drivers/power/pmic/mp5416.c | 98
> > +++++++++++++++++++++++++++++++++++++++++++++
> > include/power/mp5416.h | 41 +++++++++++++++++++
> > 4 files changed, 155 insertions(+)
> > create mode 100644 drivers/power/pmic/mp5416.c
> > create mode 100644 include/power/mp5416.h
> >
> > diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
> > index 7d51510..583fd3d 100644
> > --- a/drivers/power/pmic/Kconfig
> > +++ b/drivers/power/pmic/Kconfig
> > @@ -91,6 +91,21 @@ config DM_PMIC_FAN53555
> > The driver implements read/write operations for use with the
> > FAN53555
> > regulator driver and binds the regulator driver to its node.
> >
> > +config DM_PMIC_MP5416
> > + bool "Enable Driver Model for PMIC MP5416"
> > + depends on DM_PMIC
> > + help
> > + This config enables implementation of driver-model pmic uclass
> > features
> > + for PMIC MP5416. The driver implements read/write operations.
> > +
> > +config SPL_DM_PMIC_MP5416
> > + bool "Enable Driver Model for PMIC MP5416 in SPL stage"
> > + depends on DM_PMIC
> > + help
> > + This config enables implementation of driver-model pmic uclass
> > + features for PMIC MP5416. The driver implements read/write
> > + operations.
> > +
> > config DM_PMIC_PCA9450
> > bool "Enable Driver Model for PMIC PCA9450"
> > depends on DM_PMIC
> > diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
> > index 9cd6c37..2b2a6dd 100644
> > --- a/drivers/power/pmic/Makefile
> > +++ b/drivers/power/pmic/Makefile
> > @@ -10,6 +10,7 @@ obj-$(CONFIG_DM_PMIC_MAX77686) += max77686.o
> > obj-$(CONFIG_DM_PMIC_MAX8998) += max8998.o
> > obj-$(CONFIG_DM_PMIC_MC34708) += mc34708.o
> > obj-$(CONFIG_$(SPL_)DM_PMIC_BD71837) += bd71837.o
> > +obj-$(CONFIG_$(SPL_)DM_PMIC_MP5416) += mp5416.o
> > obj-$(CONFIG_$(SPL_)DM_PMIC_PFUZE100) += pfuze100.o
> > obj-$(CONFIG_$(SPL_)DM_PMIC_PCA9450) += pca9450.o
> > obj-$(CONFIG_PMIC_S2MPS11) += s2mps11.o
> > diff --git a/drivers/power/pmic/mp5416.c b/drivers/power/pmic/mp5416.c
> > new file mode 100644
> > index 0000000..458c4df
> > --- /dev/null
> > +++ b/drivers/power/pmic/mp5416.c
> > @@ -0,0 +1,98 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2020 Gateworks Corporation
> > + */
> > +#include <common.h>
> > +#include <errno.h>
> > +#include <dm.h>
> > +#include <i2c.h>
> > +#include <log.h>
> > +#include <power/pmic.h>
> > +#include <power/regulator.h>
> > +#include <power/mp5416.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +static const struct pmic_child_info pmic_children_info[] = {
> > + /* buck */
> > + { .prefix = "b", .driver = MP6416_REGULATOR_DRIVER },
> > + /* ldo */
> > + { .prefix = "l", .driver = MP6416_REGULATOR_DRIVER },
> > + { },
> > +};
> > +
> > +static int mp5416_reg_count(struct udevice *dev)
> > +{
> > + return MP5416_NUM_OF_REGS - 1;
> > +}
> > +
> > +static int mp5416_write(struct udevice *dev, uint reg, const uint8_t *buff,
> > + int len)
> > +{
> > + if (dm_i2c_write(dev, reg, buff, len)) {
> > + pr_err("write error to device: %p register: %#x!", dev, reg);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int mp5416_read(struct udevice *dev, uint reg, uint8_t *buff, int
> > len)
> > +{
> > + if (dm_i2c_read(dev, reg, buff, len)) {
> > + pr_err("read error from device: %p register: %#x!", dev, reg);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int mp5416_bind(struct udevice *dev)
> > +{
> > + int children;
> > + ofnode regulators_node;
> > +
> > + debug("%s %s\n", __func__, dev->name);
> > + regulators_node = dev_read_subnode(dev, "regulators");
> > + if (!ofnode_valid(regulators_node)) {
> > + debug("%s: %s regulators subnode not found!\n", __func__,
> > + dev->name);
> > + return -ENXIO;
> > + }
> > +
> > + debug("%s: '%s' - found regulators subnode\n", __func__, dev->name);
> > +
> > + children = pmic_bind_children(dev, regulators_node,
> > pmic_children_info);
> > + if (!children)
> > + debug("%s: %s - no child found\n", __func__, dev->name);
> > +
> > + /* Always return success for this device */
> > + return 0;
> > +}
> > +
> > +static int mp5416_probe(struct udevice *dev)
> > +{
> > + debug("%s %s\n", __func__, dev->name);
> > +
> > + return 0;
> > +}
> > +
> > +static struct dm_pmic_ops mp5416_ops = {
> > + .reg_count = mp5416_reg_count,
> > + .read = mp5416_read,
> > + .write = mp5416_write,
> > +};
> > +
> > +static const struct udevice_id mp5416_ids[] = {
> > + { .compatible = "mps,mp5416", },
> > + { }
> > +};
> > +
> > +U_BOOT_DRIVER(pmic_mp5416) = {
> > + .name = "mp5416 pmic",
> > + .id = UCLASS_PMIC,
> > + .of_match = mp5416_ids,
> > + .bind = mp5416_bind,
> > + .probe = mp5416_probe,
> > + .ops = &mp5416_ops,
> > +};
> > diff --git a/include/power/mp5416.h b/include/power/mp5416.h
> > new file mode 100644
> > index 0000000..dc096fe
> > --- /dev/null
> > +++ b/include/power/mp5416.h
> > @@ -0,0 +1,41 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/* Copyright (C) 2020 Gateworks Corporation */
> > +
> > +#ifndef MP5416_H_
> > +#define MP5416_H_
> > +
> > +#define MP6416_REGULATOR_DRIVER "mp5416_regulator"
> > +
> > +enum {
> > + MP5416_CTL0 = 0x00,
> > + MP5416_CTL1 = 0x01,
> > + MP5416_CTL2 = 0x02,
> > + MP5416_ILIMIT = 0x03,
> > + MP5416_VSET_SW1 = 0x04,
> > + MP5416_VSET_SW2 = 0x05,
> > + MP5416_VSET_SW3 = 0x06,
> > + MP5416_VSET_SW4 = 0x07,
> > + MP5416_VSET_LDO2 = 0x08,
> > + MP5416_VSET_LDO3 = 0x09,
> > + MP5416_VSET_LDO4 = 0x0a,
> > + MP5416_VSET_LDO5 = 0x0b,
> > + MP5416_STATUS1 = 0x0d,
> > + MP5416_STATUS2 = 0x0e,
> > + MP5416_STATUS3 = 0x0f,
> > + MP5416_ID2 = 0x11,
> > + MP5416_NUM_OF_REGS = 0x12,
> > +};
> > +
> > +#define MP5416_VSET_EN BIT(7)
> > +#define MP5416_VSET_SW1_GVAL(x) ((((x) & 0x7f) * 12500) + 600000)
> > +#define MP5416_VSET_SW2_GVAL(x) ((((x) & 0x7f) * 25000) + 800000)
> > +#define MP5416_VSET_SW3_GVAL(x) ((((x) & 0x7f) * 12500) + 600000)
> > +#define MP5416_VSET_SW4_GVAL(x) ((((x) & 0x7f) * 25000) + 800000)
> > +#define MP5416_VSET_LDO_GVAL(x) ((((x) & 0x7f) * 25000) + 800000)
> > +#define MP5416_VSET_LDO_SVAL(x) ((((x) & 0x7f) * 25000) + 800000)
> > +#define MP5416_VSET_SW1_SVAL(x) (((x) - 600000) / 12500)
> > +#define MP5416_VSET_SW2_SVAL(x) (((x) - 800000) / 25000)
> > +#define MP5416_VSET_SW3_SVAL(x) (((x) - 600000) / 12500)
> > +#define MP5416_VSET_SW4_SVAL(x) (((x) - 800000) / 25000)
>
> I didn't find where above macros are used, except MP5416_VSET_SW3_SVAL().
>
Jaehoon,
Thanks for the review!
Correct, the venice board support only requires MP5416_VSET_SW3_SVAL
currently but I figured if I needed to go to the trouble of creating a
pmic driver I would expose all the functionality for future users. Is
this not the right thing to do?
Best regards,
Tim
