Re: [PATCH v2 7/7] mfd: Remove tps68470 MFD driver

From: Laurent Pinchart
Date: Mon Jan 18 2021 - 02:43:44 EST

Hi Daniel,

Thank you for the patch.

On Mon, Jan 18, 2021 at 12:34:28AM +0000, Daniel Scally wrote:
> This driver only covered one scenario in which ACPI devices with _HID
> INT3472 are found, and its functionality has been taken over by the
> intel-skl-int3472 module, so remove it.
>
> Signed-off-by: Daniel Scally <djrscally@xxxxxxxxx>
> ---
> Changes in v2:
>
> - Introduced
>
> drivers/acpi/pmic/Kconfig | 1 -
> drivers/gpio/Kconfig | 1 -
> drivers/mfd/Kconfig | 18 --------
> drivers/mfd/Makefile | 1 -
> drivers/mfd/tps68470.c | 97 ---------------------------------------
> 5 files changed, 118 deletions(-)
> delete mode 100644 drivers/mfd/tps68470.c
>
> diff --git a/drivers/acpi/pmic/Kconfig b/drivers/acpi/pmic/Kconfig
> index 56bbcb2ce61b..e27d8ef3a32c 100644
> --- a/drivers/acpi/pmic/Kconfig
> +++ b/drivers/acpi/pmic/Kconfig
> @@ -52,7 +52,6 @@ endif # PMIC_OPREGION
>
> config TPS68470_PMIC_OPREGION
> bool "ACPI operation region support for TPS68470 PMIC"
> - depends on MFD_TPS68470

Should this now depend on INTEL_SKL_INT3472 ?

> help
> This config adds ACPI operation region support for TI TPS68470 PMIC.
> TPS68470 device is an advanced power management unit that powers
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index c70f46e80a3b..07ff8f24b0d9 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1343,7 +1343,6 @@ config GPIO_TPS65912
>
> config GPIO_TPS68470
> bool "TPS68470 GPIO"
> - depends on MFD_TPS68470

Same here.

This won't deal with the case where th TPS68470 is instantiated through
DT, but that's not supported yet, so it can be dealt with it later when
the need arises.

Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>

> help
> Select this option to enable GPIO driver for the TPS68470
> chip family.
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index bdfce7b15621..9a1f648efde0 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1520,24 +1520,6 @@ config MFD_TPS65217
> This driver can also be built as a module. If so, the module
> will be called tps65217.
>
> -config MFD_TPS68470
> - bool "TI TPS68470 Power Management / LED chips"
> - depends on ACPI && PCI && I2C=y
> - depends on I2C_DESIGNWARE_PLATFORM=y
> - select MFD_CORE
> - select REGMAP_I2C
> - help
> - If you say yes here you get support for the TPS68470 series of
> - Power Management / LED chips.
> -
> - These include voltage regulators, LEDs and other features
> - that are often used in portable devices.
> -
> - This option is a bool as it provides an ACPI operation
> - region, which must be available before any of the devices
> - using this are probed. This option also configures the
> - designware-i2c driver to be built-in, for the same reason.
> -
> config MFD_TI_LP873X
> tristate "TI LP873X Power Management IC"
> depends on I2C
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 14fdb188af02..5994e812f479 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -105,7 +105,6 @@ 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_TPS68470) += tps68470.o
> obj-$(CONFIG_MFD_TPS80031) += tps80031.o
> obj-$(CONFIG_MENELAUS) += menelaus.o
>
> diff --git a/drivers/mfd/tps68470.c b/drivers/mfd/tps68470.c
> deleted file mode 100644
> index 4a4df4ffd18c..000000000000
> --- a/drivers/mfd/tps68470.c
> +++ /dev/null
> @@ -1,97 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * TPS68470 chip Parent driver
> - *
> - * Copyright (C) 2017 Intel Corporation
> - *
> - * Authors:
> - * Rajmohan Mani <rajmohan.mani@xxxxxxxxx>
> - * Tianshu Qiu <tian.shu.qiu@xxxxxxxxx>
> - * Jian Xu Zheng <jian.xu.zheng@xxxxxxxxx>
> - * Yuning Pu <yuning.pu@xxxxxxxxx>
> - */
> -
> -#include <linux/acpi.h>
> -#include <linux/delay.h>
> -#include <linux/i2c.h>
> -#include <linux/init.h>
> -#include <linux/mfd/core.h>
> -#include <linux/mfd/tps68470.h>
> -#include <linux/regmap.h>
> -
> -static const struct mfd_cell tps68470s[] = {
> - { .name = "tps68470-gpio" },
> - { .name = "tps68470_pmic_opregion" },
> -};
> -
> -static const struct regmap_config tps68470_regmap_config = {
> - .reg_bits = 8,
> - .val_bits = 8,
> - .max_register = TPS68470_REG_MAX,
> -};
> -
> -static int tps68470_chip_init(struct device *dev, struct regmap *regmap)
> -{
> - unsigned int version;
> - int ret;
> -
> - /* Force software reset */
> - ret = regmap_write(regmap, TPS68470_REG_RESET, TPS68470_REG_RESET_MASK);
> - if (ret)
> - return ret;
> -
> - ret = regmap_read(regmap, TPS68470_REG_REVID, &version);
> - if (ret) {
> - dev_err(dev, "Failed to read revision register: %d\n", ret);
> - return ret;
> - }
> -
> - dev_info(dev, "TPS68470 REVID: 0x%x\n", version);
> -
> - return 0;
> -}
> -
> -static int tps68470_probe(struct i2c_client *client)
> -{
> - struct device *dev = &client->dev;
> - struct regmap *regmap;
> - int ret;
> -
> - regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
> - if (IS_ERR(regmap)) {
> - dev_err(dev, "devm_regmap_init_i2c Error %ld\n",
> - PTR_ERR(regmap));
> - return PTR_ERR(regmap);
> - }
> -
> - i2c_set_clientdata(client, regmap);
> -
> - ret = tps68470_chip_init(dev, regmap);
> - if (ret < 0) {
> - dev_err(dev, "TPS68470 Init Error %d\n", ret);
> - return ret;
> - }
> -
> - ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, tps68470s,
> - ARRAY_SIZE(tps68470s), NULL, 0, NULL);
> - if (ret < 0) {
> - dev_err(dev, "devm_mfd_add_devices failed: %d\n", ret);
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> -static const struct acpi_device_id tps68470_acpi_ids[] = {
> - {"INT3472"},
> - {},
> -};
> -
> -static struct i2c_driver tps68470_driver = {
> - .driver = {
> - .name = "tps68470",
> - .acpi_match_table = tps68470_acpi_ids,
> - },
> - .probe_new = tps68470_probe,
> -};
> -builtin_i2c_driver(tps68470_driver);

--
Regards,

Laurent Pinchart