Re: [PATCH 2/4] MFD: TPS6594x: Add new mfd device for TPS6594x PMIC

From: Lee Jones
Date: Thu Aug 11 2022 - 03:04:17 EST


On Wed, 10 Aug 2022, Matt Ranostay wrote:

> On Wed, Aug 10, 2022 at 12:27:25PM +0100, Lee Jones wrote:
> > On Thu, 04 Aug 2022, Matt Ranostay wrote:
> >
> > > From: Keerthy <j-keerthy@xxxxxx>
> > >
> > > The TPS6594x chip is a PMIC, and contains the following components:
> > >
> > > - Regulators
> > > - GPIO controller
> > > - RTC
> > >
> > > However initially only RTC is supported.
> > >
> > > Signed-off-by: Keerthy <j-keerthy@xxxxxx>
> > > Signed-off-by: Matt Ranostay <mranostay@xxxxxx>
> > > ---
> > > drivers/mfd/Kconfig | 14 +++++
> > > drivers/mfd/Makefile | 1 +
> > > drivers/mfd/tps6594x.c | 106 +++++++++++++++++++++++++++++++++++
> > > include/linux/mfd/tps6594x.h | 66 ++++++++++++++++++++++
> > > 4 files changed, 187 insertions(+)
> > > create mode 100644 drivers/mfd/tps6594x.c
> > > create mode 100644 include/linux/mfd/tps6594x.h
> > >
> > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > > index abb58ab1a1a4..cfb5b3d66b76 100644
> > > --- a/drivers/mfd/Kconfig
> > > +++ b/drivers/mfd/Kconfig
> > > @@ -1547,6 +1547,20 @@ config MFD_TI_LP873X
> > > This driver can also be built as a module. If so, the module
> > > will be called lp873x.
> > >
> > > +config MFD_TPS6594X
> > > + tristate "TI TPS6594X Power Management IC"
> > > + depends on I2C
> > > + select MFD_CORE
> > > + select REGMAP_I2C
> > > + help
> > > + If you say yes here then you get support for the TPS6594X series of
> > > + Power Management Integrated Circuits (PMIC).
> > > + These include voltage regulators, RTS, configurable
> > > + General Purpose Outputs (GPO) that are used in portable devices.
> > > +
> > > + This driver can also be built as a module. If so, the module
> > > + will be called tps7694x.
> > > +
> > > config MFD_TI_LP87565
> > > tristate "TI LP87565 Power Management IC"
> > > depends on I2C && OF
> > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > > index 858cacf659d6..7ff6a8a57d55 100644
> > > --- a/drivers/mfd/Makefile
> > > +++ b/drivers/mfd/Makefile
> > > @@ -105,6 +105,7 @@ obj-$(CONFIG_MFD_TPS65910) += tps65910.o
> > > obj-$(CONFIG_MFD_TPS65912) += tps65912-core.o
> > > obj-$(CONFIG_MFD_TPS65912_I2C) += tps65912-i2c.o
> > > obj-$(CONFIG_MFD_TPS65912_SPI) += tps65912-spi.o
> > > +obj-$(CONFIG_MFD_TPS6594X) += tps6594x.o
> > > obj-$(CONFIG_MENELAUS) += menelaus.o
> > >
> > > obj-$(CONFIG_TWL4030_CORE) += twl-core.o twl4030-irq.o twl6030-irq.o
> > > diff --git a/drivers/mfd/tps6594x.c b/drivers/mfd/tps6594x.c
> > > new file mode 100644
> > > index 000000000000..519162cc1fbe
> > > --- /dev/null
> > > +++ b/drivers/mfd/tps6594x.c
> > > @@ -0,0 +1,106 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + * tps6594x.c -- TI TPS6594x chip family multi-function driver
> >
> > No filenames in comments please.
> >
> > Also, there are too many spaces around the '--'.
> >
> > It's not a "multi-function driver" it's a PMIC Core driver.
> >
>
> Noted. Will change to PMIC core driver to be more concise.
>
> > > + * Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/
> > > + *
> > > + * Author: Keerthy <j-keerthy@xxxxxx>
> > > + */
> > > +
> > > +#include <linux/interrupt.h>
> > > +#include <linux/mfd/core.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of_device.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/regmap.h>
> >
> > Alphabetical.
> >
> > > +#include <linux/mfd/tps6594x.h>
> > > +
> > > +static const struct regmap_config tps6594x_regmap_config = {
> > > + .reg_bits = 8,
> > > + .val_bits = 8,
> > > + .max_register = TPS6594X_REG_MAX,
> > > +};
> > > +
> > > +static const struct mfd_cell tps6594x_cells[] = {
> > > + { .name = "tps6594x-rtc", },
> > > +};
> >
> > Where are the rest of the devices?
> >
> > This is not an MFD with only one device.
>
> There are other devices, however there isn't any drivers currently for them
> just the RTC. Should there be placeholders for the gpio, and regulators even
> if support currently doesn't exist.

If support doesn't exist for the other devices, just submit an
independent RTC driver.

--
Lee Jones [李琼斯]