Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH v4 0/3] TPS68470 PMIC drivers

47 views
Skip to first unread message

Rajmohan Mani

unread,
Jul 19, 2017, 7:30:06 PM7/19/17
to
This is the patch series for TPS68470 PMIC that works as a camera PMIC.

The patch series provide the following 3 drivers, to help configure the voltage regulators, clocks and GPIOs provided by the TPS68470 PMIC, to be able to use the camera sensors connected to this PMIC.

TPS68470 MFD driver:
This is the multi function driver that initializes the TPS68470 PMIC and supports the GPIO and Op Region functions.

TPS68470 GPIO driver:
This is the PMIC GPIO driver that will be used by the OS GPIO layer, when the BIOS / firmware triggered GPIO access is done.

TPS68470 Op Region driver:
This is the driver that will be invoked, when the BIOS / firmware configures the voltage / clock for the sensors / vcm devices connected to the PMIC.

---

Update on 2 GPIO chips implementation over 1:
- Attempted to implement 2 GPIO chips, but ran into couple of
issues in the kernel, so we couldn't get it to work.
- It was decided to postpone this change, since it is not
critical

Changes in v4:
- MFD driver:
- Removed board specific code and FIXME comment
- Moved i2c.h include from tps68470.h to tps68470.c
- Moved the TPS68470 REVID read code after PMIC reset
- Fixed typo in debug error message (on failure of
devm_mfd_add_devices() )
- Enhanced dependency on I2C by changing it to I2C=y
(to fix build errors if I2C is built as module
e.g tps68470.c:71: undefined reference to `__devm_regmap_init_i2c'
tps68470.c:117: undefined reference to `i2c_register_driver')
- Removed most of the unused header file definitions
- Moved devm_mfd_add_devices() after PMIC resett
- Used probe_new() and removed i2c_device_id table

The following patch from Andy is needed for the driver to be
probed.
http://marc.info/?l=linux-acpi&m=150030081523885&w=2

- GPIO driver:
- Added newline at the end of Kconfig description
- Updated commit message about the descriptive
names for the GPIOs and the typical usage model
of the GPIO driver

- Opregion driver:
- Added dependency on MFD_TPS68470
- Converted 2 liner into one line code

Changes in v3:
- MFD driver:
- Removed GPIO lookup table
- Reverted to probe() for consistency
- Addressed other comments from Andy

- GPIO driver:
- Removed the code that initializes the default values
of GPIOs to zeros
- Used gpiochip_get_data() to access data inside the gpio_chip

Changes in v2:
- MFD driver:
- Removed tps68470_* wrappers around regmap_* calls
- Removed "struct tps68470"
- used devm_mfd_add_devices and removed mutex in mfd driver
- Added reasoning about the need of having mfd driver
as bool/builtin

- Opregion driver:
- renamed opregion driver file / internal symbol names
with tps68470_pmic*
- Made opregion driver tables as const
- Removed unused *handler_context in common handler
- Replaced "int" with "unsigned int"
- Changed to WARN macro to dev_warn()
- Destroyed mutex on error
- Added reasoning about the need of having Opregion driver
as bool/builtin

- GPIO driver:
- Implemented get_direction() in the GPIO driver
- Setup gpio_chip.names
- Moved the GPIO lookup table code inside mfd driver
- Added reasoning about the need of having GPIO driver
as bool/builtin

---

Rajmohan Mani (3):
mfd: Add new mfd device TPS68470
gpio: Add support for TPS68470 GPIOs
ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

drivers/acpi/Kconfig | 16 ++
drivers/acpi/Makefile | 2 +
drivers/acpi/pmic/tps68470_pmic.c | 455 ++++++++++++++++++++++++++++++++++++++
drivers/gpio/Kconfig | 15 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-tps68470.c | 174 +++++++++++++++
drivers/mfd/Kconfig | 18 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/tps68470.c | 110 +++++++++
include/linux/mfd/tps68470.h | 97 ++++++++
10 files changed, 889 insertions(+)
create mode 100644 drivers/acpi/pmic/tps68470_pmic.c
create mode 100644 drivers/gpio/gpio-tps68470.c
create mode 100644 drivers/mfd/tps68470.c
create mode 100644 include/linux/mfd/tps68470.h

--
1.9.1

Rajmohan Mani

unread,
Jul 19, 2017, 7:30:06 PM7/19/17
to
The TPS68470 device is an advanced power management
unit that powers a Compact Camera Module (CCM),
generates clocks for image sensors, drives a dual
LED for Flash and incorporates two LED drivers for
general purpose indicators.

This patch adds support for TPS68470 mfd device.

Signed-off-by: Rajmohan Mani <rajmoh...@intel.com>
---
drivers/mfd/Kconfig | 18 +++++++
drivers/mfd/Makefile | 1 +
drivers/mfd/tps68470.c | 110 +++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/tps68470.h | 97 ++++++++++++++++++++++++++++++++++++++
4 files changed, 226 insertions(+)
create mode 100644 drivers/mfd/tps68470.c
create mode 100644 include/linux/mfd/tps68470.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 3eb5c93..960be43 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1311,6 +1311,24 @@ 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 && I2C=y
+ select MFD_CORE
+ select REGMAP_I2C
+ select I2C_DESIGNWARE_PLATFORM
+ help
+ If you say yes here you get support for the TPS68470 series of
+ Power Management / LED chips.
+
+ These include voltage regulators, led 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 builtin, 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 c16bf1e..6dd2b94 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -82,6 +82,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_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
new file mode 100644
index 0000000..a692af7
--- /dev/null
+++ b/drivers/mfd/tps68470.c
@@ -0,0 +1,110 @@
+/*
+ * TPS68470 chip family multi-function driver
+ *
+ * Copyright (C) 2017 Intel Corporation
+ * Authors:
+ * Rajmohan Mani <rajmoh...@intel.com>
+ * Tianshu Qiu <tian.s...@intel.com>
+ * Jian Xu Zheng <jian.x...@intel.com>
+ * Yuning Pu <yuni...@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#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 < 0)
+ return ret;
+
+ ret = regmap_read(regmap, TPS68470_REG_REVID, &version);
+ if (ret < 0) {
+ 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"},
+ {},
+};
+
+MODULE_DEVICE_TABLE(acpi, tps68470_acpi_ids);
+
+static struct i2c_driver tps68470_driver = {
+ .driver = {
+ .name = "tps68470",
+ .acpi_match_table = tps68470_acpi_ids,
+ },
+ .probe_new = tps68470_probe,
+};
+builtin_i2c_driver(tps68470_driver);
diff --git a/include/linux/mfd/tps68470.h b/include/linux/mfd/tps68470.h
new file mode 100644
index 0000000..44f9d9f
--- /dev/null
+++ b/include/linux/mfd/tps68470.h
@@ -0,0 +1,97 @@
+/*
+ * Copyright (c) 2017 Intel Corporation
+ *
+ * Functions to access TPS68470 power management chip.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __LINUX_MFD_TPS68470_H
+#define __LINUX_MFD_TPS68470_H
+
+/* Register addresses */
+#define TPS68470_REG_POSTDIV2 0x06
+#define TPS68470_REG_BOOSTDIV 0x07
+#define TPS68470_REG_BUCKDIV 0x08
+#define TPS68470_REG_PLLSWR 0x09
+#define TPS68470_REG_XTALDIV 0x0A
+#define TPS68470_REG_PLLDIV 0x0B
+#define TPS68470_REG_POSTDIV 0x0C
+#define TPS68470_REG_PLLCTL 0x0D
+#define TPS68470_REG_PLLCTL2 0x0E
+#define TPS68470_REG_CLKCFG1 0x0F
+#define TPS68470_REG_CLKCFG2 0x10
+#define TPS68470_REG_GPCTL0A 0x14
+#define TPS68470_REG_GPCTL0B 0x15
+#define TPS68470_REG_GPCTL1A 0x16
+#define TPS68470_REG_GPCTL1B 0x17
+#define TPS68470_REG_GPCTL2A 0x18
+#define TPS68470_REG_GPCTL2B 0x19
+#define TPS68470_REG_GPCTL3A 0x1A
+#define TPS68470_REG_GPCTL3B 0x1B
+#define TPS68470_REG_GPCTL4A 0x1C
+#define TPS68470_REG_GPCTL4B 0x1D
+#define TPS68470_REG_GPCTL5A 0x1E
+#define TPS68470_REG_GPCTL5B 0x1F
+#define TPS68470_REG_GPCTL6A 0x20
+#define TPS68470_REG_GPCTL6B 0x21
+#define TPS68470_REG_SGPO 0x22
+#define TPS68470_REG_GPDI 0x26
+#define TPS68470_REG_GPDO 0x27
+#define TPS68470_REG_VCMVAL 0x3C
+#define TPS68470_REG_VAUX1VAL 0x3D
+#define TPS68470_REG_VAUX2VAL 0x3E
+#define TPS68470_REG_VIOVAL 0x3F
+#define TPS68470_REG_VSIOVAL 0x40
+#define TPS68470_REG_VAVAL 0x41
+#define TPS68470_REG_VDVAL 0x42
+#define TPS68470_REG_S_I2C_CTL 0x43
+#define TPS68470_REG_VCMCTL 0x44
+#define TPS68470_REG_VAUX1CTL 0x45
+#define TPS68470_REG_VAUX2CTL 0x46
+#define TPS68470_REG_VACTL 0x47
+#define TPS68470_REG_VDCTL 0x48
+#define TPS68470_REG_RESET 0x50
+#define TPS68470_REG_REVID 0xFF
+
+#define TPS68470_REG_MAX TPS68470_REG_REVID
+
+/* Register field definitions */
+
+#define TPS68470_REG_RESET_MASK GENMASK(7, 0)
+#define TPS68470_VAVAL_AVOLT_MASK GENMASK(6, 0)
+
+#define TPS68470_VDVAL_DVOLT_MASK GENMASK(5, 0)
+#define TPS68470_VCMVAL_VCVOLT_MASK GENMASK(6, 0)
+#define TPS68470_VIOVAL_IOVOLT_MASK GENMASK(6, 0)
+#define TPS68470_VSIOVAL_IOVOLT_MASK GENMASK(6, 0)
+#define TPS68470_VAUX1VAL_AUX1VOLT_MASK GENMASK(6, 0)
+#define TPS68470_VAUX2VAL_AUX2VOLT_MASK GENMASK(6, 0)
+
+#define TPS68470_VACTL_EN_MASK GENMASK(0, 0)
+#define TPS68470_VDCTL_EN_MASK GENMASK(0, 0)
+#define TPS68470_VCMCTL_EN_MASK GENMASK(0, 0)
+#define TPS68470_S_I2C_CTL_EN_MASK GENMASK(1, 0)
+#define TPS68470_VAUX1CTL_EN_MASK GENMASK(0, 0)
+#define TPS68470_VAUX2CTL_EN_MASK GENMASK(0, 0)
+#define TPS68470_PLL_EN_MASK GENMASK(0, 0)
+
+#define TPS68470_CLKCFG1_MODE_A_MASK GENMASK(1, 0)
+#define TPS68470_CLKCFG1_MODE_B_MASK GENMASK(3, 2)
+
+#define TPS68470_GPIO_CTL_REG_A(x) (TPS68470_REG_GPCTL0A + (x) * 2)
+#define TPS68470_GPIO_CTL_REG_B(x) (TPS68470_REG_GPCTL0B + (x) * 2)
+#define TPS68470_GPIO_MODE_MASK GENMASK(1, 0)
+#define TPS68470_GPIO_MODE_IN 0
+#define TPS68470_GPIO_MODE_IN_PULLUP 1
+#define TPS68470_GPIO_MODE_OUT_CMOS 2
+#define TPS68470_GPIO_MODE_OUT_ODRAIN 3
+
+#endif /* __LINUX_MFD_TPS68470_H */
--
1.9.1

Rajmohan Mani

unread,
Jul 19, 2017, 7:30:07 PM7/19/17
to
This patch adds support for TPS68470 GPIOs.
There are 7 GPIOs and a few sensor related GPIOs.
These GPIOs can be requested and configured as
appropriate.

The GPIOs are also provided with descriptive names.
However, the typical use case is that the OS GPIO
driver will interact with TPS68470 GPIO driver
to configure these GPIOs, as requested by the
platform firmware.

Signed-off-by: Rajmohan Mani <rajmoh...@intel.com>
Reviewed-by: Linus Walleij <linus....@linaro.org>
---
drivers/gpio/Kconfig | 15 ++++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-tps68470.c | 174 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 190 insertions(+)
create mode 100644 drivers/gpio/gpio-tps68470.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 23ca51e..a8fb61c 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1041,6 +1041,21 @@ config GPIO_TPS65912
help
This driver supports TPS65912 gpio chip

+config GPIO_TPS68470
+ bool "TPS68470 GPIO"
+ depends on MFD_TPS68470
+ help
+ Select this option to enable GPIO driver for the TPS68470
+ chip family.
+ There are 7 GPIOs and few sensor related GPIOs supported
+ by the TPS68470. While the 7 GPIOs can be configured as
+ input or output as appropriate, the sensor related GPIOs
+ are "output only" GPIOs.
+
+ This driver config is bool, as the GPIO functionality
+ of the TPS68470 must be available before dependent
+ drivers are loaded.
+
config GPIO_TWL4030
tristate "TWL4030, TWL5030, and TPS659x0 GPIOs"
depends on TWL4030_CORE
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 68b9627..a2659cb 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -120,6 +120,7 @@ obj-$(CONFIG_GPIO_TPS65218) += gpio-tps65218.o
obj-$(CONFIG_GPIO_TPS6586X) += gpio-tps6586x.o
obj-$(CONFIG_GPIO_TPS65910) += gpio-tps65910.o
obj-$(CONFIG_GPIO_TPS65912) += gpio-tps65912.o
+obj-$(CONFIG_GPIO_TPS68470) += gpio-tps68470.o
obj-$(CONFIG_GPIO_TS4800) += gpio-ts4800.o
obj-$(CONFIG_GPIO_TS4900) += gpio-ts4900.o
obj-$(CONFIG_GPIO_TS5500) += gpio-ts5500.o
diff --git a/drivers/gpio/gpio-tps68470.c b/drivers/gpio/gpio-tps68470.c
new file mode 100644
index 0000000..5a1dd56
--- /dev/null
+++ b/drivers/gpio/gpio-tps68470.c
@@ -0,0 +1,174 @@
+/*
+ * GPIO driver for TPS68470 PMIC
+ *
+ * Copyright (C) 2017 Intel Corporation
+ * Authors:
+ * Antti Laakso <antti....@intel.com>
+ * Tianshu Qiu <tian.s...@intel.com>
+ * Jian Xu Zheng <jian.x...@intel.com>
+ * Yuning Pu <yuni...@intel.com>
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/gpio/driver.h>
+#include <linux/mfd/tps68470.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define TPS68470_N_LOGIC_OUTPUT 3
+#define TPS68470_N_REGULAR_GPIO 7
+#define TPS68470_N_GPIO (TPS68470_N_LOGIC_OUTPUT + TPS68470_N_REGULAR_GPIO)
+
+struct tps68470_gpio_data {
+ struct regmap *tps68470_regmap;
+ struct gpio_chip gc;
+};
+
+static int tps68470_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct tps68470_gpio_data *tps68470_gpio = gpiochip_get_data(gc);
+ struct regmap *regmap = tps68470_gpio->tps68470_regmap;
+ unsigned int reg = TPS68470_REG_GPDO;
+ int val, ret;
+
+ if (offset >= TPS68470_N_REGULAR_GPIO) {
+ offset -= TPS68470_N_REGULAR_GPIO;
+ reg = TPS68470_REG_SGPO;
+ }
+
+ ret = regmap_read(regmap, reg, &val);
+ if (ret) {
+ dev_err(tps68470_gpio->gc.parent, "reg 0x%x read failed\n",
+ TPS68470_REG_SGPO);
+ return ret;
+ }
+ return !!(val & BIT(offset));
+}
+
+/* Return 0 if output, 1 if input */
+static int tps68470_gpio_get_direction(struct gpio_chip *gc,
+ unsigned int offset)
+{
+ struct tps68470_gpio_data *tps68470_gpio = gpiochip_get_data(gc);
+ struct regmap *regmap = tps68470_gpio->tps68470_regmap;
+ int val, ret;
+
+ /* rest are always outputs */
+ if (offset >= TPS68470_N_REGULAR_GPIO)
+ return 0;
+
+ ret = regmap_read(regmap, TPS68470_GPIO_CTL_REG_A(offset), &val);
+ if (ret) {
+ dev_err(tps68470_gpio->gc.parent, "reg 0x%x read failed\n",
+ TPS68470_GPIO_CTL_REG_A(offset));
+ return ret;
+ }
+
+ val &= TPS68470_GPIO_MODE_MASK;
+ return val >= TPS68470_GPIO_MODE_OUT_CMOS ? 0 : 1;
+}
+
+static void tps68470_gpio_set(struct gpio_chip *gc, unsigned int offset,
+ int value)
+{
+ struct tps68470_gpio_data *tps68470_gpio = gpiochip_get_data(gc);
+ struct regmap *regmap = tps68470_gpio->tps68470_regmap;
+ unsigned int reg = TPS68470_REG_GPDO;
+
+ if (offset >= TPS68470_N_REGULAR_GPIO) {
+ reg = TPS68470_REG_SGPO;
+ offset -= TPS68470_N_REGULAR_GPIO;
+ }
+
+ regmap_update_bits(regmap, reg, BIT(offset), value ? BIT(offset) : 0);
+}
+
+static int tps68470_gpio_output(struct gpio_chip *gc, unsigned int offset,
+ int value)
+{
+ struct tps68470_gpio_data *tps68470_gpio = gpiochip_get_data(gc);
+ struct regmap *regmap = tps68470_gpio->tps68470_regmap;
+
+ /* rest are always outputs */
+ if (offset >= TPS68470_N_REGULAR_GPIO)
+ return 0;
+
+ /* Set the initial value */
+ tps68470_gpio_set(gc, offset, value);
+
+ return regmap_update_bits(regmap, TPS68470_GPIO_CTL_REG_A(offset),
+ TPS68470_GPIO_MODE_MASK,
+ TPS68470_GPIO_MODE_OUT_CMOS);
+}
+
+static int tps68470_gpio_input(struct gpio_chip *gc, unsigned int offset)
+{
+ struct tps68470_gpio_data *tps68470_gpio = gpiochip_get_data(gc);
+ struct regmap *regmap = tps68470_gpio->tps68470_regmap;
+
+ /* rest are always outputs */
+ if (offset >= TPS68470_N_REGULAR_GPIO)
+ return -EINVAL;
+
+ return regmap_update_bits(regmap, TPS68470_GPIO_CTL_REG_A(offset),
+ TPS68470_GPIO_MODE_MASK, 0x00);
+}
+
+static const char *tps68470_names[TPS68470_N_GPIO] = {
+ "gpio.0", "gpio.1", "gpio.2", "gpio.3",
+ "gpio.4", "gpio.5", "gpio.6",
+ "s_enable", "s_idle", "s_resetn",
+};
+
+static int tps68470_gpio_probe(struct platform_device *pdev)
+{
+ struct tps68470_gpio_data *tps68470_gpio;
+ int ret;
+
+ tps68470_gpio = devm_kzalloc(&pdev->dev, sizeof(*tps68470_gpio),
+ GFP_KERNEL);
+ if (!tps68470_gpio)
+ return -ENOMEM;
+
+ tps68470_gpio->tps68470_regmap = dev_get_drvdata(pdev->dev.parent);
+ tps68470_gpio->gc.label = "tps68470-gpio";
+ tps68470_gpio->gc.owner = THIS_MODULE;
+ tps68470_gpio->gc.direction_input = tps68470_gpio_input;
+ tps68470_gpio->gc.direction_output = tps68470_gpio_output;
+ tps68470_gpio->gc.get = tps68470_gpio_get;
+ tps68470_gpio->gc.get_direction = tps68470_gpio_get_direction;
+ tps68470_gpio->gc.set = tps68470_gpio_set;
+ tps68470_gpio->gc.can_sleep = true;
+ tps68470_gpio->gc.names = tps68470_names;
+ tps68470_gpio->gc.ngpio = TPS68470_N_GPIO;
+ tps68470_gpio->gc.base = -1;
+ tps68470_gpio->gc.parent = &pdev->dev;
+
+ ret = devm_gpiochip_add_data(&pdev->dev, &tps68470_gpio->gc,
+ tps68470_gpio);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to register gpio_chip: %d\n", ret);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, tps68470_gpio);
+
+ return ret;
+}
+
+static struct platform_driver tps68470_gpio_driver = {
+ .driver = {
+ .name = "tps68470-gpio",
+ },
+ .probe = tps68470_gpio_probe,
+};
+
+builtin_platform_driver(tps68470_gpio_driver)
--
1.9.1

Rajmohan Mani

unread,
Jul 19, 2017, 7:30:07 PM7/19/17
to
The Kabylake platform coreboot (Chrome OS equivalent of
BIOS) has defined 4 operation regions for the TI TPS68470 PMIC.
These operation regions are to enable/disable voltage
regulators, configure voltage regulators, enable/disable
clocks and to configure clocks.

This config adds ACPI operation region support for TI TPS68470 PMIC.
TPS68470 device is an advanced power management unit that powers
a Compact Camera Module (CCM), generates clocks for image sensors,
drives a dual LED for flash and incorporates two LED drivers for
general purpose indicators.
This driver enables ACPI operation region support to control voltage
regulators and clocks for the TPS68470 PMIC.

Signed-off-by: Rajmohan Mani <rajmoh...@intel.com>
---
drivers/acpi/Kconfig | 16 ++
drivers/acpi/Makefile | 2 +
drivers/acpi/pmic/tps68470_pmic.c | 455 ++++++++++++++++++++++++++++++++++++++
3 files changed, 473 insertions(+)
create mode 100644 drivers/acpi/pmic/tps68470_pmic.c

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 1ce52f8..e124a82d 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -535,4 +535,20 @@ if ARM64
source "drivers/acpi/arm64/Kconfig"
endif

+config TPS68470_PMIC_OPREGION
+ bool "ACPI operation region support for TPS68470 PMIC"
+ depends on MFD_TPS68470
+ help
+ This config adds ACPI operation region support for TI TPS68470 PMIC.
+ TPS68470 device is an advanced power management unit that powers
+ a Compact Camera Module (CCM), generates clocks for image sensors,
+ drives a dual LED for flash and incorporates two LED drivers for
+ general purpose indicators.
+ This driver enables ACPI operation region support control voltage
+ regulators and clocks.
+
+ 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.
+
endif # ACPI
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index b1aacfc..2d70a7f 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -106,6 +106,8 @@ obj-$(CONFIG_CHT_WC_PMIC_OPREGION) += pmic/intel_pmic_chtwc.o

obj-$(CONFIG_ACPI_CONFIGFS) += acpi_configfs.o

+obj-$(CONFIG_TPS68470_PMIC_OPREGION) += pmic/tps68470_pmic.o
+
video-objs += acpi_video.o video_detect.o
obj-y += dptf/

diff --git a/drivers/acpi/pmic/tps68470_pmic.c b/drivers/acpi/pmic/tps68470_pmic.c
new file mode 100644
index 0000000..f2626bd
--- /dev/null
+++ b/drivers/acpi/pmic/tps68470_pmic.c
@@ -0,0 +1,455 @@
+/*
+ * TI TPS68470 PMIC operation region driver
+ *
+ * Copyright (C) 2017 Intel Corporation. All rights reserved.
+ * Author: Rajmohan Mani <rajmoh...@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Based on drivers/acpi/pmic/intel_pmic* drivers
+ *
+ */
+
+#include <linux/acpi.h>
+#include <linux/mfd/tps68470.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+struct tps68470_pmic_table {
+ u32 address; /* operation region address */
+ u32 reg; /* corresponding register */
+ u32 bitmask; /* bit mask for power, clock */
+};
+
+#define TI_PMIC_POWER_OPREGION_ID 0xB0
+#define TI_PMIC_VR_VAL_OPREGION_ID 0xB1
+#define TI_PMIC_CLOCK_OPREGION_ID 0xB2
+#define TI_PMIC_CLKFREQ_OPREGION_ID 0xB3
+
+struct tps68470_pmic_opregion {
+ struct mutex lock;
+ struct regmap *regmap;
+};
+
+#define S_IO_I2C_EN (BIT(0) | BIT(1))
+
+static const struct tps68470_pmic_table power_table[] = {
+ {
+ .address = 0x00,
+ .reg = TPS68470_REG_S_I2C_CTL,
+ .bitmask = S_IO_I2C_EN,
+ /* S_I2C_CTL */
+ },
+ {
+ .address = 0x04,
+ .reg = TPS68470_REG_VCMCTL,
+ .bitmask = BIT(0),
+ /* VCMCTL */
+ },
+ {
+ .address = 0x08,
+ .reg = TPS68470_REG_VAUX1CTL,
+ .bitmask = BIT(0),
+ /* VAUX1_CTL */
+ },
+ {
+ .address = 0x0C,
+ .reg = TPS68470_REG_VAUX2CTL,
+ .bitmask = BIT(0),
+ /* VAUX2CTL */
+ },
+ {
+ .address = 0x10,
+ .reg = TPS68470_REG_VACTL,
+ .bitmask = BIT(0),
+ /* VACTL */
+ },
+ {
+ .address = 0x14,
+ .reg = TPS68470_REG_VDCTL,
+ .bitmask = BIT(0),
+ /* VDCTL */
+ },
+};
+
+/* Table to set voltage regulator value */
+static const struct tps68470_pmic_table vr_val_table[] = {
+ {
+ .address = 0x00,
+ .reg = TPS68470_REG_VSIOVAL,
+ .bitmask = TPS68470_VSIOVAL_IOVOLT_MASK,
+ /* TPS68470_REG_VSIOVAL */
+ },
+ {
+ .address = 0x04,
+ .reg = TPS68470_REG_VIOVAL,
+ .bitmask = TPS68470_VIOVAL_IOVOLT_MASK,
+ /* TPS68470_REG_VIOVAL */
+ },
+ {
+ .address = 0x08,
+ .reg = TPS68470_REG_VCMVAL,
+ .bitmask = TPS68470_VCMVAL_VCVOLT_MASK,
+ /* TPS68470_REG_VCMVAL */
+ },
+ {
+ .address = 0x0C,
+ .reg = TPS68470_REG_VAUX1VAL,
+ .bitmask = TPS68470_VAUX1VAL_AUX1VOLT_MASK,
+ /* TPS68470_REG_VAUX1VAL */
+ },
+ {
+ .address = 0x10,
+ .reg = TPS68470_REG_VAUX2VAL,
+ .bitmask = TPS68470_VAUX2VAL_AUX2VOLT_MASK,
+ /* TPS68470_REG_VAUX2VAL */
+ },
+ {
+ .address = 0x14,
+ .reg = TPS68470_REG_VAVAL,
+ .bitmask = TPS68470_VAVAL_AVOLT_MASK,
+ /* TPS68470_REG_VAVAL */
+ },
+ {
+ .address = 0x18,
+ .reg = TPS68470_REG_VDVAL,
+ .bitmask = TPS68470_VDVAL_DVOLT_MASK,
+ /* TPS68470_REG_VDVAL */
+ },
+};
+
+/* Table to configure clock frequency */
+static const struct tps68470_pmic_table clk_freq_table[] = {
+ {
+ .address = 0x00,
+ .reg = TPS68470_REG_POSTDIV2,
+ .bitmask = BIT(0) | BIT(1),
+ /* TPS68470_REG_POSTDIV2 */
+ },
+ {
+ .address = 0x04,
+ .reg = TPS68470_REG_BOOSTDIV,
+ .bitmask = 0x1F,
+ /* TPS68470_REG_BOOSTDIV */
+ },
+ {
+ .address = 0x08,
+ .reg = TPS68470_REG_BUCKDIV,
+ .bitmask = 0x0F,
+ /* TPS68470_REG_BUCKDIV */
+ },
+ {
+ .address = 0x0C,
+ .reg = TPS68470_REG_PLLSWR,
+ .bitmask = 0x13,
+ /* TPS68470_REG_PLLSWR */
+ },
+ {
+ .address = 0x10,
+ .reg = TPS68470_REG_XTALDIV,
+ .bitmask = 0xFF,
+ /* TPS68470_REG_XTALDIV */
+ },
+ {
+ .address = 0x14,
+ .reg = TPS68470_REG_PLLDIV,
+ .bitmask = 0xFF,
+ /* TPS68470_REG_PLLDIV */
+ },
+ {
+ .address = 0x18,
+ .reg = TPS68470_REG_POSTDIV,
+ .bitmask = 0x83,
+ /* TPS68470_REG_POSTDIV */
+ },
+};
+
+/* Table to configure and enable clocks */
+static const struct tps68470_pmic_table clk_table[] = {
+ {
+ .address = 0x00,
+ .reg = TPS68470_REG_PLLCTL,
+ .bitmask = 0xF5,
+ /* TPS68470_REG_PLLCTL */
+ },
+ {
+ .address = 0x04,
+ .reg = TPS68470_REG_PLLCTL2,
+ .bitmask = BIT(0),
+ /* TPS68470_REG_PLLCTL2 */
+ },
+ {
+ .address = 0x08,
+ .reg = TPS68470_REG_CLKCFG1,
+ .bitmask = TPS68470_CLKCFG1_MODE_A_MASK |
+ TPS68470_CLKCFG1_MODE_B_MASK,
+ /* TPS68470_REG_CLKCFG1 */
+ },
+ {
+ .address = 0x0C,
+ .reg = TPS68470_REG_CLKCFG2,
+ .bitmask = TPS68470_CLKCFG1_MODE_A_MASK |
+ TPS68470_CLKCFG1_MODE_B_MASK,
+ /* TPS68470_REG_CLKCFG2 */
+ },
+};
+
+static int pmic_get_reg_bit(u64 address,
+ const struct tps68470_pmic_table *table,
+ const unsigned int table_size, int *reg,
+ int *bitmask)
+{
+ u64 i;
+
+ i = address / 4;
+ if (i >= table_size)
+ return -ENOENT;
+
+ if (!reg || !bitmask)
+ return -EINVAL;
+
+ *reg = table[i].reg;
+ *bitmask = table[i].bitmask;
+
+ return 0;
+}
+
+static int tps68470_pmic_get_power(struct regmap *regmap, int reg,
+ int bitmask, u64 *value)
+{
+ unsigned int data;
+
+ if (regmap_read(regmap, reg, &data))
+ return -EIO;
+
+ *value = (data & bitmask) ? 1 : 0;
+ return 0;
+}
+
+static int tps68470_pmic_get_vr_val(struct regmap *regmap, int reg,
+ int bitmask, u64 *value)
+{
+ unsigned int data;
+
+ if (regmap_read(regmap, reg, &data))
+ return -EIO;
+
+ *value = data & bitmask;
+ return 0;
+}
+
+static int tps68470_pmic_get_clk(struct regmap *regmap, int reg,
+ int bitmask, u64 *value)
+{
+ unsigned int data;
+
+ if (regmap_read(regmap, reg, &data))
+ return -EIO;
+
+ *value = (data & bitmask) ? 1 : 0;
+ return 0;
+}
+
+static int tps68470_pmic_get_clk_freq(struct regmap *regmap, int reg,
+ int bitmask, u64 *value)
+{
+ unsigned int data;
+
+ if (regmap_read(regmap, reg, &data))
+ return -EIO;
+
+ *value = data & bitmask;
+ return 0;
+}
+
+static int ti_tps68470_regmap_update_bits(struct regmap *regmap, int reg,
+ int bitmask, u64 value)
+{
+ return regmap_update_bits(regmap, reg, bitmask, value);
+}
+
+static acpi_status tps68470_pmic_common_handler(u32 function,
+ acpi_physical_address address,
+ u32 bits, u64 *value,
+ void *region_context,
+ int (*get)(struct regmap *,
+ int, int, u64 *),
+ int (*update)(struct regmap *,
+ int, int, u64),
+ const struct tps68470_pmic_table *tbl,
+ unsigned int tbl_size)
+{
+ struct tps68470_pmic_opregion *opregion = region_context;
+ struct regmap *regmap = opregion->regmap;
+ int reg, ret, bitmask;
+
+ if (bits != 32)
+ return AE_BAD_PARAMETER;
+
+ ret = pmic_get_reg_bit(address, tbl, tbl_size, &reg, &bitmask);
+ if (ret < 0)
+ return AE_BAD_PARAMETER;
+
+ if (function == ACPI_WRITE && *value > bitmask)
+ return AE_BAD_PARAMETER;
+
+ mutex_lock(&opregion->lock);
+
+ ret = (function == ACPI_READ) ?
+ get(regmap, reg, bitmask, value) :
+ update(regmap, reg, bitmask, *value);
+
+ mutex_unlock(&opregion->lock);
+
+ return ret ? AE_ERROR : AE_OK;
+}
+
+static acpi_status tps68470_pmic_cfreq_handler(u32 function,
+ acpi_physical_address address,
+ u32 bits, u64 *value,
+ void *handler_context,
+ void *region_context)
+{
+ return tps68470_pmic_common_handler(function, address, bits, value,
+ region_context,
+ tps68470_pmic_get_clk_freq,
+ ti_tps68470_regmap_update_bits,
+ clk_freq_table,
+ ARRAY_SIZE(clk_freq_table));
+}
+
+static acpi_status tps68470_pmic_clk_handler(u32 function,
+ acpi_physical_address address, u32 bits,
+ u64 *value, void *handler_context,
+ void *region_context)
+{
+ return tps68470_pmic_common_handler(function, address, bits, value,
+ region_context,
+ tps68470_pmic_get_clk,
+ ti_tps68470_regmap_update_bits,
+ clk_table,
+ ARRAY_SIZE(clk_table));
+}
+
+static acpi_status tps68470_pmic_vrval_handler(u32 function,
+ acpi_physical_address address,
+ u32 bits, u64 *value,
+ void *handler_context,
+ void *region_context)
+{
+ return tps68470_pmic_common_handler(function, address, bits, value,
+ region_context,
+ tps68470_pmic_get_vr_val,
+ ti_tps68470_regmap_update_bits,
+ vr_val_table,
+ ARRAY_SIZE(vr_val_table));
+}
+
+static acpi_status tps68470_pmic_pwr_handler(u32 function,
+ acpi_physical_address address,
+ u32 bits, u64 *value,
+ void *handler_context,
+ void *region_context)
+{
+ if (bits != 32)
+ return AE_BAD_PARAMETER;
+
+ /* set/clear for bit 0, bits 0 and 1 together */
+ if (function == ACPI_WRITE &&
+ !(*value == 0 || *value == 1 || *value == 3)) {
+ return AE_BAD_PARAMETER;
+ }
+
+ return tps68470_pmic_common_handler(function, address, bits, value,
+ region_context,
+ tps68470_pmic_get_power,
+ ti_tps68470_regmap_update_bits,
+ power_table,
+ ARRAY_SIZE(power_table));
+}
+
+static int tps68470_pmic_opregion_probe(struct platform_device *pdev)
+{
+ struct regmap *tps68470_regmap = dev_get_drvdata(pdev->dev.parent);
+ acpi_handle handle = ACPI_HANDLE(pdev->dev.parent);
+ struct device *dev = &pdev->dev;
+ struct tps68470_pmic_opregion *opregion;
+ acpi_status status;
+
+ if (!dev || !tps68470_regmap) {
+ dev_warn(dev, "dev or regmap is NULL\n");
+ return -EINVAL;
+ }
+
+ if (!handle) {
+ dev_warn(dev, "acpi handle is NULL\n");
+ return -ENODEV;
+ }
+
+ opregion = devm_kzalloc(dev, sizeof(*opregion), GFP_KERNEL);
+ if (!opregion)
+ return -ENOMEM;
+
+ mutex_init(&opregion->lock);
+ opregion->regmap = tps68470_regmap;
+
+ status = acpi_install_address_space_handler(handle,
+ TI_PMIC_POWER_OPREGION_ID,
+ tps68470_pmic_pwr_handler,
+ NULL, opregion);
+ if (ACPI_FAILURE(status))
+ goto out_mutex_destroy;
+
+ status = acpi_install_address_space_handler(handle,
+ TI_PMIC_VR_VAL_OPREGION_ID,
+ tps68470_pmic_vrval_handler,
+ NULL, opregion);
+ if (ACPI_FAILURE(status))
+ goto out_remove_power_handler;
+
+ status = acpi_install_address_space_handler(handle,
+ TI_PMIC_CLOCK_OPREGION_ID,
+ tps68470_pmic_clk_handler,
+ NULL, opregion);
+ if (ACPI_FAILURE(status))
+ goto out_remove_vr_val_handler;
+
+ status = acpi_install_address_space_handler(handle,
+ TI_PMIC_CLKFREQ_OPREGION_ID,
+ tps68470_pmic_cfreq_handler,
+ NULL, opregion);
+ if (ACPI_FAILURE(status))
+ goto out_remove_clk_handler;
+
+ return 0;
+
+out_remove_clk_handler:
+ acpi_remove_address_space_handler(handle, TI_PMIC_CLOCK_OPREGION_ID,
+ tps68470_pmic_clk_handler);
+out_remove_vr_val_handler:
+ acpi_remove_address_space_handler(handle, TI_PMIC_VR_VAL_OPREGION_ID,
+ tps68470_pmic_vrval_handler);
+out_remove_power_handler:
+ acpi_remove_address_space_handler(handle, TI_PMIC_POWER_OPREGION_ID,
+ tps68470_pmic_pwr_handler);
+out_mutex_destroy:
+ mutex_destroy(&opregion->lock);
+ return -ENODEV;
+}
+
+static struct platform_driver tps68470_pmic_opregion_driver = {
+ .probe = tps68470_pmic_opregion_probe,
+ .driver = {
+ .name = "tps68470_pmic_opregion",
+ },
+};
+
+builtin_platform_driver(tps68470_pmic_opregion_driver)
--
1.9.1

Lee Jones

unread,
Jul 20, 2017, 5:10:08 AM7/20/17
to
LED(s)

> + 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

Remove the ','.

> + designware-i2c driver to be builtin, for the same reason.

built-in

> +
> config MFD_TI_LP873X
> tristate "TI LP873X Power Management IC"
> depends on I2C
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index c16bf1e..6dd2b94 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -82,6 +82,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_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
> new file mode 100644
> index 0000000..a692af7
> --- /dev/null
> +++ b/drivers/mfd/tps68470.c
> @@ -0,0 +1,110 @@
> +/*
> + * TPS68470 chip family multi-function driver

Does it really cover a family? If so 'TPS68470' seems very specific.

"Multi-Function Driver" or even better "Core" or "Parent" driver.

> + * Copyright (C) 2017 Intel Corporation

'\n' here.

> + * Authors:
> + * Rajmohan Mani <rajmoh...@intel.com>
> + * Tianshu Qiu <tian.s...@intel.com>
> + * Jian Xu Zheng <jian.x...@intel.com>
> + * Yuning Pu <yuni...@intel.com>

Tab these out:

* Authors:
* Rajmohan Mani <rajmoh...@intel.com>
* Tianshu Qiu <tian.s...@intel.com>
* Jian Xu Zheng <jian.x...@intel.com>
* Yuning Pu <yuni...@intel.com>

> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.

Can you use the short version?

> + */
> +
> +#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",
> + },
> +};

Make these one line each:

{ .name = "tps68470-gpio" }

... and drop the ','

> +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 < 0)

Will 'if (!ret)' do?

> + return ret;
> +
> + ret = regmap_read(regmap, TPS68470_REG_REVID, &version);
> + if (ret < 0) {

Same
Same

> + 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"},
> + {},
> +};
> +

Remove this line.
Short version?
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Mani, Rajmohan

unread,
Jul 20, 2017, 9:00:08 PM7/20/17
to
Hi Lee,

Thanks for the reviews.
Ack

> > + 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
>
> Remove the ','.
>

Ack

> > + designware-i2c driver to be builtin, for the same reason.
>
> built-in
>

Ack

> > +
> > config MFD_TI_LP873X
> > tristate "TI LP873X Power Management IC"
> > depends on I2C
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index
> > c16bf1e..6dd2b94 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -82,6 +82,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_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 new file
> > mode 100644 index 0000000..a692af7
> > --- /dev/null
> > +++ b/drivers/mfd/tps68470.c
> > @@ -0,0 +1,110 @@
> > +/*
> > + * TPS68470 chip family multi-function driver
>
> Does it really cover a family? If so 'TPS68470' seems very specific.
>
> "Multi-Function Driver" or even better "Core" or "Parent" driver.
>

No. This is just for TPS68470.
I picked "Parent" driver

> > + * Copyright (C) 2017 Intel Corporation
>
> '\n' here.
>

Ack

> > + * Authors:
> > + * Rajmohan Mani <rajmoh...@intel.com>
> > + * Tianshu Qiu <tian.s...@intel.com>
> > + * Jian Xu Zheng <jian.x...@intel.com>
> > + * Yuning Pu <yuni...@intel.com>
>
> Tab these out:
>
> * Authors:
> * Rajmohan Mani <rajmoh...@intel.com>
> * Tianshu Qiu <tian.s...@intel.com>
> * Jian Xu Zheng <jian.x...@intel.com>
> * Yuning Pu <yuni...@intel.com>
>

Ack

> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation version 2.
> > + *
> > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> > + * kind, whether express or implied; without even the implied
> > + warranty
> > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
>
> Can you use the short version?
>

Ack
I will update the other patches of this series too.

> > + */
> > +
> > +#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",
> > + },
> > +};
>
> Make these one line each:
>
> { .name = "tps68470-gpio" }
>
> ... and drop the ','
>

Ack

> > +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 < 0)
>
> Will 'if (!ret)' do?
>

We intend to check error conditions and bail out. So, if (ret < 0) works for this.

> > + return ret;
> > +
> > + ret = regmap_read(regmap, TPS68470_REG_REVID, &version);
> > + if (ret < 0) {
>
> Same
>

We intend to check error conditions and bail out. So, if (ret < 0) works for this.

> > + 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) {
>
> Same
>

We intend to check error conditions and bail out. So, if (ret < 0) works for this.

> > + 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"},
> > + {},
> > +};
> > +
>
> Remove this line.
>

Ack
> > +warranty
> > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
>
> Short version?
>

Ack

Mani, Rajmohan

unread,
Jul 21, 2017, 8:30:08 AM7/21/17
to
Hi Lee,

> > > + * This program is free software; you can redistribute it and/or
> > > + * modify it under the terms of the GNU General Public License as
> > > + * published by the Free Software Foundation version 2.
> > > + *
> > > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> > > + * kind, whether express or implied; without even the implied
> > > + warranty
> > > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> the
> > > + * GNU General Public License for more details.
> >
> > Can you use the short version?
> >
>
> Ack
> I will update the other patches of this series too.
>

I just confirmed that we are recommended to use the above long version of headers. So, these headers will stay as such.

Thanks
Raj

Rajmohan Mani

unread,
Jul 21, 2017, 11:00:08 AM7/21/17
to
This is the patch series for TPS68470 PMIC that works as a camera PMIC.

The patch series provide the following 3 drivers, to help configure the voltage regulators, clocks and GPIOs provided by the TPS68470 PMIC, to be able to use the camera sensors connected to this PMIC.

TPS68470 MFD driver:
This is the multi function driver that initializes the TPS68470 PMIC and supports the GPIO and Op Region functions.

TPS68470 GPIO driver:
This is the PMIC GPIO driver that will be used by the OS GPIO layer, when the BIOS / firmware triggered GPIO access is done.

TPS68470 Op Region driver:
This is the driver that will be invoked, when the BIOS / firmware configures the voltage / clock for the sensors / vcm devices connected to the PMIC.

---

Update on 2 GPIO chips implementation over 1:
- Attempted to implement 2 GPIO chips, but ran into couple of
issues in the kernel, so we couldn't get it to work.
- It was decided to postpone this change, since it is not
critical

Changes in v5:
- MFD driver:
- Fixed Kconfig description text
- Addressed other comments from Lee, related to formatting

- GPIO driver:
- Formatted the file header text

- Opregion driver:
- Formatted the file header text
mfd: Add new mfd device TPS68470
gpio: Add support for TPS68470 GPIOs
ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

drivers/acpi/Kconfig | 16 ++
drivers/acpi/Makefile | 2 +
drivers/acpi/pmic/tps68470_pmic.c | 455 ++++++++++++++++++++++++++++++++++++++
drivers/gpio/Kconfig | 15 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-tps68470.c | 176 +++++++++++++++
drivers/mfd/Kconfig | 18 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/tps68470.c | 106 +++++++++
include/linux/mfd/tps68470.h | 97 ++++++++
10 files changed, 887 insertions(+)
create mode 100644 drivers/acpi/pmic/tps68470_pmic.c
create mode 100644 drivers/gpio/gpio-tps68470.c
create mode 100644 drivers/mfd/tps68470.c
create mode 100644 include/linux/mfd/tps68470.h

--
1.9.1

Rajmohan Mani

unread,
Jul 21, 2017, 11:00:08 AM7/21/17
to
The TPS68470 device is an advanced power management
unit that powers a Compact Camera Module (CCM),
generates clocks for image sensors, drives a dual
LED for Flash and incorporates two LED drivers for
general purpose indicators.

This patch adds support for TPS68470 mfd device.

Signed-off-by: Rajmohan Mani <rajmoh...@intel.com>
---
drivers/mfd/Kconfig | 18 ++++++++
drivers/mfd/Makefile | 1 +
drivers/mfd/tps68470.c | 106 +++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/tps68470.h | 97 +++++++++++++++++++++++++++++++++++++++
4 files changed, 222 insertions(+)
create mode 100644 drivers/mfd/tps68470.c
create mode 100644 include/linux/mfd/tps68470.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 94ad2c1..74f41a1 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1337,6 +1337,24 @@ 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 && I2C=y
+ select MFD_CORE
+ select REGMAP_I2C
+ select I2C_DESIGNWARE_PLATFORM
+ 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 080793b..0b1fd82 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -83,6 +83,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_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
new file mode 100644
index 0000000..1e7716a
--- /dev/null
+++ b/drivers/mfd/tps68470.c
@@ -0,0 +1,106 @@
+/*
+ * TPS68470 chip Parent driver
+ *
+ * Copyright (C) 2017 Intel Corporation
+ *
+ * Authors:
+ * Rajmohan Mani <rajmoh...@intel.com>
+ * Tianshu Qiu <tian.s...@intel.com>
+ * Jian Xu Zheng <jian.x...@intel.com>
+ * Yuning Pu <yuni...@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#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 < 0)
+ return ret;
+
+ ret = regmap_read(regmap, TPS68470_REG_REVID, &version);
+ if (ret < 0) {
+ 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"},
+ {},
+};
+MODULE_DEVICE_TABLE(acpi, tps68470_acpi_ids);
+
+static struct i2c_driver tps68470_driver = {
+ .driver = {
+ .name = "tps68470",
+ .acpi_match_table = tps68470_acpi_ids,
+ },
+ .probe_new = tps68470_probe,
+};
+builtin_i2c_driver(tps68470_driver);
diff --git a/include/linux/mfd/tps68470.h b/include/linux/mfd/tps68470.h
new file mode 100644
index 0000000..44f9d9f
--- /dev/null
+++ b/include/linux/mfd/tps68470.h
@@ -0,0 +1,97 @@
+/*
+ * Copyright (c) 2017 Intel Corporation
+ *
+ * Functions to access TPS68470 power management chip.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
1.9.1

Rajmohan Mani

unread,
Jul 21, 2017, 11:00:08 AM7/21/17
to
The Kabylake platform coreboot (Chrome OS equivalent of
BIOS) has defined 4 operation regions for the TI TPS68470 PMIC.
These operation regions are to enable/disable voltage
regulators, configure voltage regulators, enable/disable
clocks and to configure clocks.

This config adds ACPI operation region support for TI TPS68470 PMIC.
TPS68470 device is an advanced power management unit that powers
a Compact Camera Module (CCM), generates clocks for image sensors,
drives a dual LED for flash and incorporates two LED drivers for
general purpose indicators.
This driver enables ACPI operation region support to control voltage
regulators and clocks for the TPS68470 PMIC.

Signed-off-by: Rajmohan Mani <rajmoh...@intel.com>
---
drivers/acpi/Kconfig | 16 ++
drivers/acpi/Makefile | 2 +
drivers/acpi/pmic/tps68470_pmic.c | 455 ++++++++++++++++++++++++++++++++++++++
3 files changed, 473 insertions(+)
create mode 100644 drivers/acpi/pmic/tps68470_pmic.c

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 1ce52f8..e124a82d 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -535,4 +535,20 @@ if ARM64
source "drivers/acpi/arm64/Kconfig"
endif

+config TPS68470_PMIC_OPREGION
+ bool "ACPI operation region support for TPS68470 PMIC"
+ depends on MFD_TPS68470
+ help
+ This config adds ACPI operation region support for TI TPS68470 PMIC.
+ TPS68470 device is an advanced power management unit that powers
+ a Compact Camera Module (CCM), generates clocks for image sensors,
+ drives a dual LED for flash and incorporates two LED drivers for
+ general purpose indicators.
+ This driver enables ACPI operation region support control voltage
+ regulators and clocks.
+
+ 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.
+
endif # ACPI
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index b1aacfc..2d70a7f 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -106,6 +106,8 @@ obj-$(CONFIG_CHT_WC_PMIC_OPREGION) += pmic/intel_pmic_chtwc.o

obj-$(CONFIG_ACPI_CONFIGFS) += acpi_configfs.o

+obj-$(CONFIG_TPS68470_PMIC_OPREGION) += pmic/tps68470_pmic.o
+
video-objs += acpi_video.o video_detect.o
obj-y += dptf/

diff --git a/drivers/acpi/pmic/tps68470_pmic.c b/drivers/acpi/pmic/tps68470_pmic.c
new file mode 100644
index 0000000..7f3c567
--- /dev/null
+++ b/drivers/acpi/pmic/tps68470_pmic.c
@@ -0,0 +1,455 @@
+/*
+ * TI TPS68470 PMIC operation region driver
+ *
+ * Copyright (C) 2017 Intel Corporation. All rights reserved.
+ *
+ * Author: Rajmohan Mani <rajmoh...@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Based on drivers/acpi/pmic/intel_pmic* drivers
+ */
+
+#include <linux/acpi.h>
+#include <linux/mfd/tps68470.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+struct tps68470_pmic_table {
+ u32 address; /* operation region address */
+ u32 reg; /* corresponding register */
+ u32 bitmask; /* bit mask for power, clock */
+};
+
+#define TI_PMIC_POWER_OPREGION_ID 0xB0
+#define TI_PMIC_VR_VAL_OPREGION_ID 0xB1
+#define TI_PMIC_CLOCK_OPREGION_ID 0xB2
+#define TI_PMIC_CLKFREQ_OPREGION_ID 0xB3
+
+struct tps68470_pmic_opregion {
+ struct mutex lock;
+ struct regmap *regmap;
+
+ return 0;
+}
+
+static int tps68470_pmic_get_power(struct regmap *regmap, int reg,
+ int bitmask, u64 *value)
+{
+ unsigned int data;
+
+ if (regmap_read(regmap, reg, &data))
+ return -EIO;
+
+ *value = (data & bitmask) ? 1 : 0;
+ return 0;
+}
+
+static int tps68470_pmic_get_vr_val(struct regmap *regmap, int reg,
+ int bitmask, u64 *value)
+{
+ unsigned int data;
+
+ if (regmap_read(regmap, reg, &data))
+ return -EIO;
+
+ *value = data & bitmask;
+ return 0;
+}
+
+static int tps68470_pmic_get_clk(struct regmap *regmap, int reg,
+ int bitmask, u64 *value)
+{
+ unsigned int data;
+
+ if (regmap_read(regmap, reg, &data))
+ return -EIO;
+
+ *value = (data & bitmask) ? 1 : 0;
+ return 0;
+}
+
+static int tps68470_pmic_get_clk_freq(struct regmap *regmap, int reg,
+ int bitmask, u64 *value)
+{
+ unsigned int data;
+
+ if (regmap_read(regmap, reg, &data))
+ return -EIO;
+
+ *value = data & bitmask;
+ return 0;
+}
+
+ if (ret < 0)
+
+ return 0;
+
+out_remove_clk_handler:
+ acpi_remove_address_space_handler(handle, TI_PMIC_CLOCK_OPREGION_ID,
+ tps68470_pmic_clk_handler);
+out_remove_vr_val_handler:
+ acpi_remove_address_space_handler(handle, TI_PMIC_VR_VAL_OPREGION_ID,
+ tps68470_pmic_vrval_handler);
+out_remove_power_handler:
+ acpi_remove_address_space_handler(handle, TI_PMIC_POWER_OPREGION_ID,
+ tps68470_pmic_pwr_handler);
+out_mutex_destroy:
+ mutex_destroy(&opregion->lock);
+ return -ENODEV;
+}
+
+static struct platform_driver tps68470_pmic_opregion_driver = {
+ .probe = tps68470_pmic_opregion_probe,
+ .driver = {
+ .name = "tps68470_pmic_opregion",
+ },
+};
+
+builtin_platform_driver(tps68470_pmic_opregion_driver)
--
1.9.1

Rajmohan Mani

unread,
Jul 21, 2017, 11:00:12 AM7/21/17
to
This patch adds support for TPS68470 GPIOs.
There are 7 GPIOs and a few sensor related GPIOs.
These GPIOs can be requested and configured as
appropriate.

The GPIOs are also provided with descriptive names.
However, the typical use case is that the OS GPIO
driver will interact with TPS68470 GPIO driver
to configure these GPIOs, as requested by the
platform firmware.

Signed-off-by: Rajmohan Mani <rajmoh...@intel.com>
Reviewed-by: Linus Walleij <linus....@linaro.org>
---
drivers/gpio/Kconfig | 15 ++++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-tps68470.c | 176 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 192 insertions(+)
create mode 100644 drivers/gpio/gpio-tps68470.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index f235eae..45a58ae 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1064,6 +1064,21 @@ config GPIO_TPS65912
help
This driver supports TPS65912 gpio chip

+config GPIO_TPS68470
+ bool "TPS68470 GPIO"
+ depends on MFD_TPS68470
+ help
+ Select this option to enable GPIO driver for the TPS68470
+ chip family.
+ There are 7 GPIOs and few sensor related GPIOs supported
+ by the TPS68470. While the 7 GPIOs can be configured as
+ input or output as appropriate, the sensor related GPIOs
+ are "output only" GPIOs.
+
+ This driver config is bool, as the GPIO functionality
+ of the TPS68470 must be available before dependent
+ drivers are loaded.
+
config GPIO_TWL4030
tristate "TWL4030, TWL5030, and TPS659x0 GPIOs"
depends on TWL4030_CORE
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index a9fda6c..37f2029 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -121,6 +121,7 @@ obj-$(CONFIG_GPIO_TPS65218) += gpio-tps65218.o
obj-$(CONFIG_GPIO_TPS6586X) += gpio-tps6586x.o
obj-$(CONFIG_GPIO_TPS65910) += gpio-tps65910.o
obj-$(CONFIG_GPIO_TPS65912) += gpio-tps65912.o
+obj-$(CONFIG_GPIO_TPS68470) += gpio-tps68470.o
obj-$(CONFIG_GPIO_TS4800) += gpio-ts4800.o
obj-$(CONFIG_GPIO_TS4900) += gpio-ts4900.o
obj-$(CONFIG_GPIO_TS5500) += gpio-ts5500.o
diff --git a/drivers/gpio/gpio-tps68470.c b/drivers/gpio/gpio-tps68470.c
new file mode 100644
index 0000000..fa2662f
--- /dev/null
+++ b/drivers/gpio/gpio-tps68470.c
@@ -0,0 +1,176 @@
+/*
+ * GPIO driver for TPS68470 PMIC
+ *
+ * Copyright (C) 2017 Intel Corporation
+ *
+ * Authors:
+ * Antti Laakso <antti....@intel.com>
+ * Tianshu Qiu <tian.s...@intel.com>
+ * Jian Xu Zheng <jian.x...@intel.com>
+ * Yuning Pu <yuni...@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/gpio/driver.h>
+#include <linux/mfd/tps68470.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define TPS68470_N_LOGIC_OUTPUT 3
+#define TPS68470_N_REGULAR_GPIO 7
+#define TPS68470_N_GPIO (TPS68470_N_LOGIC_OUTPUT + TPS68470_N_REGULAR_GPIO)
+
+struct tps68470_gpio_data {
+ struct regmap *tps68470_regmap;
+ struct gpio_chip gc;
+};
+
+static int tps68470_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct tps68470_gpio_data *tps68470_gpio = gpiochip_get_data(gc);
+ struct regmap *regmap = tps68470_gpio->tps68470_regmap;
+ unsigned int reg = TPS68470_REG_GPDO;
+ int val, ret;
+
+ if (offset >= TPS68470_N_REGULAR_GPIO) {
+ offset -= TPS68470_N_REGULAR_GPIO;
+ reg = TPS68470_REG_SGPO;
+ }
+
+ ret = regmap_read(regmap, reg, &val);
+ if (ret) {
+ dev_err(tps68470_gpio->gc.parent, "reg 0x%x read failed\n",
+ TPS68470_REG_SGPO);
+ return ret;
+ }
+ return !!(val & BIT(offset));
+}
+
+/* Return 0 if output, 1 if input */
+static int tps68470_gpio_get_direction(struct gpio_chip *gc,
+ unsigned int offset)
+{
+ struct tps68470_gpio_data *tps68470_gpio = gpiochip_get_data(gc);
+ struct regmap *regmap = tps68470_gpio->tps68470_regmap;
+ int val, ret;
+
+ /* rest are always outputs */
+ if (offset >= TPS68470_N_REGULAR_GPIO)
+ return 0;
+
+ ret = regmap_read(regmap, TPS68470_GPIO_CTL_REG_A(offset), &val);
+ if (ret) {
+ dev_err(tps68470_gpio->gc.parent, "reg 0x%x read failed\n",
+ TPS68470_GPIO_CTL_REG_A(offset));
+ return ret;
+ }
+
+ return 0;
+
+ /* Set the initial value */
+ tps68470_gpio_set(gc, offset, value);
+
+ return regmap_update_bits(regmap, TPS68470_GPIO_CTL_REG_A(offset),
+ TPS68470_GPIO_MODE_MASK,
+ TPS68470_GPIO_MODE_OUT_CMOS);
+}
+
+static int tps68470_gpio_input(struct gpio_chip *gc, unsigned int offset)
+{
+ struct tps68470_gpio_data *tps68470_gpio = gpiochip_get_data(gc);
+ struct regmap *regmap = tps68470_gpio->tps68470_regmap;
+
+ /* rest are always outputs */
+ if (offset >= TPS68470_N_REGULAR_GPIO)
+ return -EINVAL;
+
+ return regmap_update_bits(regmap, TPS68470_GPIO_CTL_REG_A(offset),
+ TPS68470_GPIO_MODE_MASK, 0x00);
+}
+
+static const char *tps68470_names[TPS68470_N_GPIO] = {
+ "gpio.0", "gpio.1", "gpio.2", "gpio.3",
+ "gpio.4", "gpio.5", "gpio.6",
+ "s_enable", "s_idle", "s_resetn",
+};
+
+static int tps68470_gpio_probe(struct platform_device *pdev)
+{
+ struct tps68470_gpio_data *tps68470_gpio;
+ int ret;
+
+ tps68470_gpio = devm_kzalloc(&pdev->dev, sizeof(*tps68470_gpio),
+ GFP_KERNEL);
+ if (!tps68470_gpio)
+ return -ENOMEM;
+
+ tps68470_gpio->tps68470_regmap = dev_get_drvdata(pdev->dev.parent);
+ tps68470_gpio->gc.label = "tps68470-gpio";
+ tps68470_gpio->gc.owner = THIS_MODULE;
+ tps68470_gpio->gc.direction_input = tps68470_gpio_input;
+ tps68470_gpio->gc.direction_output = tps68470_gpio_output;
+ tps68470_gpio->gc.get = tps68470_gpio_get;
+ tps68470_gpio->gc.get_direction = tps68470_gpio_get_direction;
+ tps68470_gpio->gc.set = tps68470_gpio_set;
+ tps68470_gpio->gc.can_sleep = true;
+ tps68470_gpio->gc.names = tps68470_names;
+ tps68470_gpio->gc.ngpio = TPS68470_N_GPIO;
+ tps68470_gpio->gc.base = -1;
+ tps68470_gpio->gc.parent = &pdev->dev;
+
+ ret = devm_gpiochip_add_data(&pdev->dev, &tps68470_gpio->gc,
+ tps68470_gpio);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to register gpio_chip: %d\n", ret);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, tps68470_gpio);
+
+ return ret;
+}
+
+static struct platform_driver tps68470_gpio_driver = {
+ .driver = {
+ .name = "tps68470-gpio",
+ },

Andy Shevchenko

unread,
Jul 21, 2017, 11:30:07 AM7/21/17
to
On Fri, Jul 21, 2017 at 5:46 PM, Rajmohan Mani <rajmoh...@intel.com> wrote:
> This is the patch series for TPS68470 PMIC that works as a camera PMIC.
>
> The patch series provide the following 3 drivers, to help configure the voltage regulators, clocks and GPIOs provided by the TPS68470 PMIC, to be able to use the camera sensors connected to this PMIC.
>
> TPS68470 MFD driver:
> This is the multi function driver that initializes the TPS68470 PMIC and supports the GPIO and Op Region functions.
>
> TPS68470 GPIO driver:
> This is the PMIC GPIO driver that will be used by the OS GPIO layer, when the BIOS / firmware triggered GPIO access is done.
>
> TPS68470 Op Region driver:
> This is the driver that will be invoked, when the BIOS / firmware configures the voltage / clock for the sensors / vcm devices connected to the PMIC.
>

All three patches are good to me (we did few rounds of internal review
before posting v4)

Reviewed-by: Andy Shevchenko <andy.sh...@gmail.com>
--
With Best Regards,
Andy Shevchenko

Rafael J. Wysocki

unread,
Jul 21, 2017, 5:00:07 PM7/21/17
to
On Friday, July 21, 2017 06:20:15 PM Andy Shevchenko wrote:
> On Fri, Jul 21, 2017 at 5:46 PM, Rajmohan Mani <rajmoh...@intel.com> wrote:
> > This is the patch series for TPS68470 PMIC that works as a camera PMIC.
> >
> > The patch series provide the following 3 drivers, to help configure the voltage regulators, clocks and GPIOs provided by the TPS68470 PMIC, to be able to use the camera sensors connected to this PMIC.
> >
> > TPS68470 MFD driver:
> > This is the multi function driver that initializes the TPS68470 PMIC and supports the GPIO and Op Region functions.
> >
> > TPS68470 GPIO driver:
> > This is the PMIC GPIO driver that will be used by the OS GPIO layer, when the BIOS / firmware triggered GPIO access is done.
> >
> > TPS68470 Op Region driver:
> > This is the driver that will be invoked, when the BIOS / firmware configures the voltage / clock for the sensors / vcm devices connected to the PMIC.
> >
>
> All three patches are good to me (we did few rounds of internal review
> before posting v4)
>
> Reviewed-by: Andy Shevchenko <andy.sh...@gmail.com>

OK, so how should they be routed?

Thanks,
Rafael

Andy Shevchenko

unread,
Jul 22, 2017, 1:20:09 PM7/22/17
to
Good question. I don't know how last time PMIC drivers were merged,
here I think is just sane to route vi MFD with immutable branch
created.

Rafael J. Wysocki

unread,
Jul 22, 2017, 6:10:04 PM7/22/17
to
OK

I will assume that the series will go in through MFD then.

Thanks,
Rafael

Lee Jones

unread,
Jul 25, 2017, 5:20:09 AM7/25/17
to
On Fri, 21 Jul 2017, Mani, Rajmohan wrote:
> > On Wed, 19 Jul 2017, Rajmohan Mani wrote:
> >
> > > The TPS68470 device is an advanced power management unit that powers
> > a
> > > Compact Camera Module (CCM), generates clocks for image sensors,
> > > drives a dual LED for Flash and incorporates two LED drivers for
> > > general purpose indicators.
> > >
> > > This patch adds support for TPS68470 mfd device.
> > >
> > > Signed-off-by: Rajmohan Mani <rajmoh...@intel.com>
> > > ---
> > > drivers/mfd/Kconfig | 18 +++++++
> > > drivers/mfd/Makefile | 1 +
> > > drivers/mfd/tps68470.c | 110
> > +++++++++++++++++++++++++++++++++++++++++++
> > > include/linux/mfd/tps68470.h | 97
> > > ++++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 226 insertions(+)
> > > create mode 100644 drivers/mfd/tps68470.c create mode 100644
> > > include/linux/mfd/tps68470.h

[...]

> > > +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 < 0)
> >
> > Will 'if (!ret)' do?
> >
>
> We intend to check error conditions and bail out. So, if (ret < 0) works for this.

Yes, 'if (!ret)' does that too.

> > > + return ret;
> > > +
> > > + ret = regmap_read(regmap, TPS68470_REG_REVID, &version);
> > > + if (ret < 0) {
> >
> > Same
> >
>
> We intend to check error conditions and bail out. So, if (ret < 0) works for this.

As above.
As above.

[...]

Andy Shevchenko

unread,
Jul 25, 2017, 6:30:09 AM7/25/17
to
On Tue, Jul 25, 2017 at 12:10 PM, Lee Jones <lee....@linaro.org> wrote:
> On Fri, 21 Jul 2017, Mani, Rajmohan wrote:
>> > On Wed, 19 Jul 2017, Rajmohan Mani wrote:

>> > > + /* Force software reset */
>> > > + ret = regmap_write(regmap, TPS68470_REG_RESET,
>> > TPS68470_REG_RESET_MASK);
>> > > + if (ret < 0)
>> >
>> > Will 'if (!ret)' do?
>> >
>>
>> We intend to check error conditions and bail out. So, if (ret < 0) works for this.
>
> Yes, 'if (!ret)' does that too.

Did you mean

if (ret)
return ret;

?

I briefly checked few ->read() and ->write() implementations and
didn't find any evidence of positive numbers that can be returned.
Documentation (kernel doc) doesn't shed a light on that. So, to me it
sounds unspecified.

So, for now (until documentation will be fixed) I would rely on
if (ret < 0)

Mani, Rajmohan

unread,
Jul 25, 2017, 1:10:09 PM7/25/17
to
Hi Lee,

> Subject: Re: [PATCH v4 1/3] mfd: Add new mfd device TPS68470
>
> > > > +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 < 0)
> > >
> > > Will 'if (!ret)' do?
> > >
> >
> > We intend to check error conditions and bail out. So, if (ret < 0) works for
> this.
>
> Yes, 'if (!ret)' does that too.
>

regmap_write() and regmap_read() functions return 0 on success. Hence we can not use 'if (!ret)' here, since we check for error conditions.

> > > > + return ret;
> > > > +
> > > > + ret = regmap_read(regmap, TPS68470_REG_REVID, &version);
> > > > + if (ret < 0) {
> > >
> > > Same
> > >
> >
> > We intend to check error conditions and bail out. So, if (ret < 0) works for
> this.
>
> As above.
>

Same as above
Same as above

Lee Jones

unread,
Jul 26, 2017, 4:30:09 AM7/26/17
to
On Tue, 25 Jul 2017, Andy Shevchenko wrote:

> On Tue, Jul 25, 2017 at 12:10 PM, Lee Jones <lee....@linaro.org> wrote:
> > On Fri, 21 Jul 2017, Mani, Rajmohan wrote:
> >> > On Wed, 19 Jul 2017, Rajmohan Mani wrote:
>
> >> > > + /* Force software reset */
> >> > > + ret = regmap_write(regmap, TPS68470_REG_RESET,
> >> > TPS68470_REG_RESET_MASK);
> >> > > + if (ret < 0)
> >> >
> >> > Will 'if (!ret)' do?
> >> >
> >>
> >> We intend to check error conditions and bail out. So, if (ret < 0) works for this.
> >
> > Yes, 'if (!ret)' does that too.
>
> Did you mean
>
> if (ret)
> return ret;
>
> ?

Yes, I just noticed!

Apologies for the confusion.

> I briefly checked few ->read() and ->write() implementations and
> didn't find any evidence of positive numbers that can be returned.
> Documentation (kernel doc) doesn't shed a light on that. So, to me it
> sounds unspecified.
>
> So, for now (until documentation will be fixed) I would rely on
> if (ret < 0)

It's not unspecified. The regmap methods call into regcache_write(),
where the kerneldoc is clear. Perhaps we can also change the regmap
kerneldoc headers too to match, which might lessen the disparity.

Lee Jones

unread,
Jul 26, 2017, 4:30:09 AM7/26/17
to
Sorry, this was a typo.

It should be 'if (ret)'.

What I'm trying to get at is the " < 0" is not required.

Andy Shevchenko

unread,
Jul 26, 2017, 6:30:09 AM7/26/17
to
On Wed, Jul 26, 2017 at 11:23 AM, Lee Jones <lee....@linaro.org> wrote:
> On Tue, 25 Jul 2017, Andy Shevchenko wrote:
>> On Tue, Jul 25, 2017 at 12:10 PM, Lee Jones <lee....@linaro.org> wrote:

>> I briefly checked few ->read() and ->write() implementations and
>> didn't find any evidence of positive numbers that can be returned.
>> Documentation (kernel doc) doesn't shed a light on that. So, to me it
>> sounds unspecified.
>>
>> So, for now (until documentation will be fixed) I would rely on
>> if (ret < 0)
>
> It's not unspecified. The regmap methods call into regcache_write(),
> where the kerneldoc is clear.

drivers/base/regmap/regcache.c:266

" * Return a negative value on failure, 0 on success."

I can hardly find this any cleaner than "unspecified".

> Perhaps we can also change the regmap
> kerneldoc headers too to match, which might lessen the disparity.

I'm not familiar with the guts of regmap API, so, I would stick with
if (ret < 0) for now until documentation specifies positive return values.

Mani, Rajmohan

unread,
Jul 27, 2017, 8:40:06 PM7/27/17
to
Hi Lee, Andy,

> Subject: Re: [PATCH v4 1/3] mfd: Add new mfd device TPS68470
>
> On Wed, Jul 26, 2017 at 11:23 AM, Lee Jones <lee....@linaro.org> wrote:
> > On Tue, 25 Jul 2017, Andy Shevchenko wrote:
> >> On Tue, Jul 25, 2017 at 12:10 PM, Lee Jones <lee....@linaro.org> wrote:
>
> >> I briefly checked few ->read() and ->write() implementations and
> >> didn't find any evidence of positive numbers that can be returned.
> >> Documentation (kernel doc) doesn't shed a light on that. So, to me it
> >> sounds unspecified.
> >>
> >> So, for now (until documentation will be fixed) I would rely on if
> >> (ret < 0)
> >
> > It's not unspecified. The regmap methods call into regcache_write(),
> > where the kerneldoc is clear.
>

Since, we are interested in the regmap for the I2C bus here, I looked into the implementation of
__devm_regmap_init()
__regmap_init()
regcache_init()
for I2C bus.

At the end of __devm_regmap_init() call from devm_regmap_init_i2c() inside tps68470_probe(), I see that both cache_bypass and defer_caching flags of i2c regmap struct are set. So, it looks regcache_write/read calls do not come into play here.

So, regmap_write()
_regmap_write()
map->reg_write (drivers/base/regmap/regmap.c:1665) translates to
regmap_i2c_write(drivers/base/regmap/regmap-i2c.c:128)

These checks in regmap_i2c_write() ensure all return values from i2c_master_send() other than the requested number of bytes to write, are converted into negative values.

if (ret == count)
return 0;
else if (ret < 0)
return ret;
else
return -EIO;

Similar argument goes for regmap_read() as well.
With that, for regmap over I2C bus, it sounds like 'if (ret < 0)' looks to be a better choice. Please see if I missed anything here.

> drivers/base/regmap/regcache.c:266
>
> " * Return a negative value on failure, 0 on success."
>
> I can hardly find this any cleaner than "unspecified".
>
> > Perhaps we can also change the regmap kerneldoc headers too to match,
> > which might lessen the disparity.
>
> I'm not familiar with the guts of regmap API, so, I would stick with if (ret < 0)
> for now until documentation specifies positive return values.
>

Ack

Andy Shevchenko

unread,
Jul 28, 2017, 5:20:09 AM7/28/17
to
It prooves exactly the Lee's point.

So, perhaps the best approach is to move to
if (ret)
return ret;

...if it will be a problem in the future, fix it accordingly.

Mani, Rajmohan

unread,
Jul 28, 2017, 7:50:05 PM7/28/17
to
Hi Andy,

> Subject: Re: [PATCH v4 1/3] mfd: Add new mfd device TPS68470
>
Ack.
We have spent enough time on this already.

Mani, Rajmohan

unread,
Jul 28, 2017, 7:50:09 PM7/28/17
to
> > > > > > +regmap
> > > > > > +*regmap) {
> > > > > > + unsigned int version;
> > > > > > + int ret;
> > > > > > +
> > > > > > + /* Force software reset */
> > > > > > + ret = regmap_write(regmap, TPS68470_REG_RESET,
> > > > > TPS68470_REG_RESET_MASK);
> > > > > > + if (ret < 0)
> > > > >
> > > > > Will 'if (!ret)' do?
> > > > >
> > > >
> > > > We intend to check error conditions and bail out. So, if (ret < 0)
> > > > works for
> > > this.
> > >
> > > Yes, 'if (!ret)' does that too.
> > >
> >
> > regmap_write() and regmap_read() functions return 0 on success. Hence
> we can not use 'if (!ret)' here, since we check for error conditions.
>
> Sorry, this was a typo.
>
> It should be 'if (ret)'.
>
> What I'm trying to get at is the " < 0" is not required.
>

Ack

Rajmohan Mani

unread,
Jul 28, 2017, 8:40:05 PM7/28/17
to
The TPS68470 device is an advanced power management
unit that powers a Compact Camera Module (CCM),
generates clocks for image sensors, drives a dual
LED for Flash and incorporates two LED drivers for
general purpose indicators.

This patch adds support for TPS68470 mfd device.

Signed-off-by: Rajmohan Mani <rajmoh...@intel.com>
---
drivers/mfd/Kconfig | 18 ++++++++
drivers/mfd/Makefile | 1 +
drivers/mfd/tps68470.c | 106 +++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/tps68470.h | 97 +++++++++++++++++++++++++++++++++++++++
4 files changed, 222 insertions(+)
create mode 100644 drivers/mfd/tps68470.c
create mode 100644 include/linux/mfd/tps68470.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 94ad2c1..74f41a1 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1337,6 +1337,24 @@ 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 && I2C=y
+ select MFD_CORE
+ select REGMAP_I2C
+ select I2C_DESIGNWARE_PLATFORM
+ 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 080793b..0b1fd82 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -83,6 +83,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_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
new file mode 100644
index 0000000..189efae
--- /dev/null
+++ b/drivers/mfd/tps68470.c
@@ -0,0 +1,106 @@
+/*
+ * TPS68470 chip Parent driver
+ *
+ * Copyright (C) 2017 Intel Corporation
+ *
+ * Authors:
+ * Rajmohan Mani <rajmoh...@intel.com>
+ * Tianshu Qiu <tian.s...@intel.com>
+ * Jian Xu Zheng <jian.x...@intel.com>
+ * Yuning Pu <yuni...@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#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"},
+ {},
+};
+MODULE_DEVICE_TABLE(acpi, tps68470_acpi_ids);
+
+static struct i2c_driver tps68470_driver = {
+ .driver = {
+ .name = "tps68470",
+ .acpi_match_table = tps68470_acpi_ids,
+ },
+ .probe_new = tps68470_probe,
+};
+builtin_i2c_driver(tps68470_driver);
diff --git a/include/linux/mfd/tps68470.h b/include/linux/mfd/tps68470.h
new file mode 100644
index 0000000..44f9d9f
--- /dev/null
+++ b/include/linux/mfd/tps68470.h
@@ -0,0 +1,97 @@
+/*
+ * Copyright (c) 2017 Intel Corporation
+ *
+ * Functions to access TPS68470 power management chip.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+

Rajmohan Mani

unread,
Jul 28, 2017, 8:40:05 PM7/28/17
to
This patch adds support for TPS68470 GPIOs.
There are 7 GPIOs and a few sensor related GPIOs.
These GPIOs can be requested and configured as
appropriate.

The GPIOs are also provided with descriptive names.
However, the typical use case is that the OS GPIO
driver will interact with TPS68470 GPIO driver
to configure these GPIOs, as requested by the
platform firmware.

Signed-off-by: Rajmohan Mani <rajmoh...@intel.com>
Reviewed-by: Linus Walleij <linus....@linaro.org>
---
drivers/gpio/Kconfig | 15 ++++
drivers/gpio/Makefile | 1 +
new file mode 100644
index 0000000..fa2662f
--- /dev/null
+++ b/drivers/gpio/gpio-tps68470.c
@@ -0,0 +1,176 @@
+/*
+ * GPIO driver for TPS68470 PMIC
+ *
+ * Copyright (C) 2017 Intel Corporation
+ *
+ * Authors:
+ * Antti Laakso <antti....@intel.com>
+ * Tianshu Qiu <tian.s...@intel.com>
+ * Jian Xu Zheng <jian.x...@intel.com>
+ * Yuning Pu <yuni...@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+ }
+
+ ret = regmap_read(regmap, reg, &val);
+ if (ret) {
+ dev_err(tps68470_gpio->gc.parent, "reg 0x%x read failed\n",
+ TPS68470_REG_SGPO);
+ return ret;
+ }
+ return !!(val & BIT(offset));
+}
+
+/* Return 0 if output, 1 if input */
+static int tps68470_gpio_get_direction(struct gpio_chip *gc,
+ unsigned int offset)
+{
+ struct tps68470_gpio_data *tps68470_gpio = gpiochip_get_data(gc);
+ struct regmap *regmap = tps68470_gpio->tps68470_regmap;
+ int val, ret;
+
+ /* rest are always outputs */
+ if (offset >= TPS68470_N_REGULAR_GPIO)
+ return 0;
+
+ ret = regmap_read(regmap, TPS68470_GPIO_CTL_REG_A(offset), &val);
+ if (ret) {
+ dev_err(tps68470_gpio->gc.parent, "reg 0x%x read failed\n",
+ TPS68470_GPIO_CTL_REG_A(offset));
+ return ret;
+ }
+
+ return 0;
+
+ int ret;
+
+ tps68470_gpio = devm_kzalloc(&pdev->dev, sizeof(*tps68470_gpio),
+ GFP_KERNEL);
+ if (!tps68470_gpio)
+ return -ENOMEM;
+
+ tps68470_gpio->tps68470_regmap = dev_get_drvdata(pdev->dev.parent);
+ tps68470_gpio->gc.label = "tps68470-gpio";
+ tps68470_gpio->gc.owner = THIS_MODULE;
+ tps68470_gpio->gc.direction_input = tps68470_gpio_input;
+ tps68470_gpio->gc.direction_output = tps68470_gpio_output;
+ tps68470_gpio->gc.get = tps68470_gpio_get;
+ tps68470_gpio->gc.get_direction = tps68470_gpio_get_direction;
+ tps68470_gpio->gc.set = tps68470_gpio_set;
+ tps68470_gpio->gc.can_sleep = true;
+ tps68470_gpio->gc.names = tps68470_names;
+ tps68470_gpio->gc.ngpio = TPS68470_N_GPIO;
+ tps68470_gpio->gc.base = -1;
+ tps68470_gpio->gc.parent = &pdev->dev;
+
+ ret = devm_gpiochip_add_data(&pdev->dev, &tps68470_gpio->gc,
+ tps68470_gpio);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to register gpio_chip: %d\n", ret);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, tps68470_gpio);
+
+ return ret;
+}
+
+static struct platform_driver tps68470_gpio_driver = {
+ .driver = {

Rajmohan Mani

unread,
Jul 28, 2017, 8:40:05 PM7/28/17
to
The Kabylake platform coreboot (Chrome OS equivalent of
BIOS) has defined 4 operation regions for the TI TPS68470 PMIC.
These operation regions are to enable/disable voltage
regulators, configure voltage regulators, enable/disable
clocks and to configure clocks.

This config adds ACPI operation region support for TI TPS68470 PMIC.
TPS68470 device is an advanced power management unit that powers
a Compact Camera Module (CCM), generates clocks for image sensors,
drives a dual LED for flash and incorporates two LED drivers for
general purpose indicators.
This driver enables ACPI operation region support to control voltage
regulators and clocks for the TPS68470 PMIC.

Signed-off-by: Rajmohan Mani <rajmoh...@intel.com>
---
drivers/acpi/Kconfig | 16 ++
drivers/acpi/Makefile | 2 +
drivers/acpi/pmic/tps68470_pmic.c | 455 ++++++++++++++++++++++++++++++++++++++
3 files changed, 473 insertions(+)
create mode 100644 drivers/acpi/pmic/tps68470_pmic.c

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 1ce52f8..e124a82d 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -535,4 +535,20 @@ if ARM64
source "drivers/acpi/arm64/Kconfig"
endif

+config TPS68470_PMIC_OPREGION
+ bool "ACPI operation region support for TPS68470 PMIC"
+ depends on MFD_TPS68470
+ help
+ This config adds ACPI operation region support for TI TPS68470 PMIC.
+ TPS68470 device is an advanced power management unit that powers
+ a Compact Camera Module (CCM), generates clocks for image sensors,
+ drives a dual LED for flash and incorporates two LED drivers for
+ general purpose indicators.
+ This driver enables ACPI operation region support control voltage
+ regulators and clocks.
+
+ 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.
+
endif # ACPI
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index b1aacfc..2d70a7f 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -106,6 +106,8 @@ obj-$(CONFIG_CHT_WC_PMIC_OPREGION) += pmic/intel_pmic_chtwc.o

obj-$(CONFIG_ACPI_CONFIGFS) += acpi_configfs.o

+obj-$(CONFIG_TPS68470_PMIC_OPREGION) += pmic/tps68470_pmic.o
+
video-objs += acpi_video.o video_detect.o
obj-y += dptf/

diff --git a/drivers/acpi/pmic/tps68470_pmic.c b/drivers/acpi/pmic/tps68470_pmic.c
new file mode 100644
index 0000000..7f3c567
--- /dev/null
+++ b/drivers/acpi/pmic/tps68470_pmic.c
@@ -0,0 +1,455 @@
+/*
+ * TI TPS68470 PMIC operation region driver
+ *
+ * Copyright (C) 2017 Intel Corporation. All rights reserved.
+ *
+ * Author: Rajmohan Mani <rajmoh...@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Based on drivers/acpi/pmic/intel_pmic* drivers
+ */
+
+#include <linux/acpi.h>
+#include <linux/mfd/tps68470.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+struct tps68470_pmic_table {
+ u32 address; /* operation region address */
+ u32 reg; /* corresponding register */
+ u32 bitmask; /* bit mask for power, clock */
+};
+
+#define TI_PMIC_POWER_OPREGION_ID 0xB0
+#define TI_PMIC_VR_VAL_OPREGION_ID 0xB1
+#define TI_PMIC_CLOCK_OPREGION_ID 0xB2
+#define TI_PMIC_CLKFREQ_OPREGION_ID 0xB3
+
+struct tps68470_pmic_opregion {
+ struct mutex lock;
+ struct regmap *regmap;
+ },
+};
+
+static int pmic_get_reg_bit(u64 address,
+ const struct tps68470_pmic_table *table,
+ const unsigned int table_size, int *reg,
+ int *bitmask)
+{
+ u64 i;
+
+ i = address / 4;
+ if (i >= table_size)
+ return -ENOENT;
+
+ if (!reg || !bitmask)
+ return -EINVAL;
+
+ *reg = table[i].reg;
+ *bitmask = table[i].bitmask;
+
+ return 0;
+}
+
+static int tps68470_pmic_get_power(struct regmap *regmap, int reg,
+ int bitmask, u64 *value)
+{
+ unsigned int data;
+
+ if (regmap_read(regmap, reg, &data))
+ return -EIO;
+
+ *value = (data & bitmask) ? 1 : 0;
+ return 0;
+}
+
+static int tps68470_pmic_get_vr_val(struct regmap *regmap, int reg,
+ int bitmask, u64 *value)
+{
+ unsigned int data;
+
+ if (regmap_read(regmap, reg, &data))
+ return -EIO;
+
+ *value = data & bitmask;
+ return 0;
+}
+
+static int tps68470_pmic_get_clk(struct regmap *regmap, int reg,
+ int bitmask, u64 *value)
+{
+ unsigned int data;
+
+ if (regmap_read(regmap, reg, &data))
+ return -EIO;
+
+ *value = (data & bitmask) ? 1 : 0;
+ return 0;
+}
+
+static int tps68470_pmic_get_clk_freq(struct regmap *regmap, int reg,
+ int bitmask, u64 *value)
+{
+ unsigned int data;
+
+ if (regmap_read(regmap, reg, &data))
+ return -EIO;
+
+ *value = data & bitmask;
+ return 0;
+}
+
+static int ti_tps68470_regmap_update_bits(struct regmap *regmap, int reg,
+ int bitmask, u64 value)
+{
+ return regmap_update_bits(regmap, reg, bitmask, value);
+}
+
+static acpi_status tps68470_pmic_common_handler(u32 function,
+ acpi_physical_address address,
+ u32 bits, u64 *value,
+ void *region_context,
+ int (*get)(struct regmap *,
+ int, int, u64 *),
+ int (*update)(struct regmap *,
+ int, int, u64),
+ const struct tps68470_pmic_table *tbl,
+ unsigned int tbl_size)
+{
+ struct tps68470_pmic_opregion *opregion = region_context;
+ struct regmap *regmap = opregion->regmap;
+ int reg, ret, bitmask;
+
+ if (bits != 32)
+ return AE_BAD_PARAMETER;
+
+ ret = pmic_get_reg_bit(address, tbl, tbl_size, &reg, &bitmask);
+ if (ret < 0)
+static int tps68470_pmic_opregion_probe(struct platform_device *pdev)
+{
+ struct regmap *tps68470_regmap = dev_get_drvdata(pdev->dev.parent);
+ acpi_handle handle = ACPI_HANDLE(pdev->dev.parent);
+ struct device *dev = &pdev->dev;
+ struct tps68470_pmic_opregion *opregion;
+ acpi_status status;
+
+ if (!dev || !tps68470_regmap) {
+ dev_warn(dev, "dev or regmap is NULL\n");
+ return -EINVAL;
+ }
+
+ if (!handle) {
+ dev_warn(dev, "acpi handle is NULL\n");
+ return -ENODEV;
+ }
+
+ opregion = devm_kzalloc(dev, sizeof(*opregion), GFP_KERNEL);
+ if (!opregion)
+ return -ENOMEM;
+
+
+ return 0;
+
+out_remove_clk_handler:
+ acpi_remove_address_space_handler(handle, TI_PMIC_CLOCK_OPREGION_ID,
+ tps68470_pmic_clk_handler);
+out_remove_vr_val_handler:
+ acpi_remove_address_space_handler(handle, TI_PMIC_VR_VAL_OPREGION_ID,
+ tps68470_pmic_vrval_handler);
+out_remove_power_handler:
+ acpi_remove_address_space_handler(handle, TI_PMIC_POWER_OPREGION_ID,
+ tps68470_pmic_pwr_handler);
+out_mutex_destroy:
+ mutex_destroy(&opregion->lock);
+ return -ENODEV;
+}
+
+static struct platform_driver tps68470_pmic_opregion_driver = {
+ .probe = tps68470_pmic_opregion_probe,
+ .driver = {

Rajmohan Mani

unread,
Jul 28, 2017, 8:40:05 PM7/28/17
to
This is the patch series for TPS68470 PMIC that works as a camera PMIC.

The patch series provide the following 3 drivers, to help configure the voltage regulators, clocks and GPIOs provided by the TPS68470 PMIC, to be able to use the camera sensors connected to this PMIC.

TPS68470 MFD driver:
This is the multi function driver that initializes the TPS68470 PMIC and supports the GPIO and Op Region functions.

TPS68470 GPIO driver:
This is the PMIC GPIO driver that will be used by the OS GPIO layer, when the BIOS / firmware triggered GPIO access is done.

TPS68470 Op Region driver:
This is the driver that will be invoked, when the BIOS / firmware configures the voltage / clock for the sensors / vcm devices connected to the PMIC.

---

Update on 2 GPIO chips implementation over 1:
- Attempted to implement 2 GPIO chips, but ran into couple of
issues in the kernel, so we couldn't get it to work.
- It was decided to postpone this change, since it is not
critical

Changes in v6:
- MFD driver:
- Used non-zero return values from regmap_write/read() calls
to detect error conditions
mfd: Add new mfd device TPS68470
gpio: Add support for TPS68470 GPIOs
ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

drivers/acpi/Kconfig | 16 ++
drivers/acpi/Makefile | 2 +
drivers/acpi/pmic/tps68470_pmic.c | 455 ++++++++++++++++++++++++++++++++++++++
drivers/gpio/Kconfig | 15 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-tps68470.c | 176 +++++++++++++++
drivers/mfd/Kconfig | 18 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/tps68470.c | 106 +++++++++
include/linux/mfd/tps68470.h | 97 ++++++++
10 files changed, 887 insertions(+)
create mode 100644 drivers/acpi/pmic/tps68470_pmic.c
create mode 100644 drivers/gpio/gpio-tps68470.c
create mode 100644 drivers/mfd/tps68470.c
create mode 100644 include/linux/mfd/tps68470.h

--
1.9.1

Lee Jones

unread,
Aug 8, 2017, 3:50:09 AM8/8/17
to
On Fri, 28 Jul 2017, Rajmohan Mani wrote:

> The TPS68470 device is an advanced power management
> unit that powers a Compact Camera Module (CCM),
> generates clocks for image sensors, drives a dual
> LED for Flash and incorporates two LED drivers for
> general purpose indicators.
>
> This patch adds support for TPS68470 mfd device.
>
> Signed-off-by: Rajmohan Mani <rajmoh...@intel.com>
> ---
> drivers/mfd/Kconfig | 18 ++++++++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/tps68470.c | 106 +++++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/tps68470.h | 97 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 222 insertions(+)
> create mode 100644 drivers/mfd/tps68470.c
> create mode 100644 include/linux/mfd/tps68470.h

Applied, thanks.

Mani, Rajmohan

unread,
Aug 8, 2017, 3:00:09 PM8/8/17
to


> -----Original Message-----
> From: Lee Jones [mailto:lee....@linaro.org]
> Sent: Tuesday, August 08, 2017 12:40 AM
> To: Mani, Rajmohan <rajmoh...@intel.com>
> Cc: linux-...@vger.kernel.org; linux...@vger.kernel.org; linux-
> ac...@vger.kernel.org; Linus Walleij <linus....@linaro.org>; Alexandre
> Courbot <gnu...@gmail.com>; Rafael J. Wysocki <r...@rjwysocki.net>; Len
> Brown <le...@kernel.org>; sakari...@linux.intel.com; Andy Shevchenko
> <andy.sh...@gmail.com>

Mani, Rajmohan

unread,
Aug 8, 2017, 3:10:09 PM8/8/17
to
Thanks Lee.

Sorry for the noise, as some keystrokes sent out the empty response earlier.

Raj

> -----Original Message-----
> From: Mani, Rajmohan
> Sent: Tuesday, August 08, 2017 11:58 AM
> To: 'Lee Jones' <lee....@linaro.org>
> Cc: linux-...@vger.kernel.org; linux...@vger.kernel.org; linux-
> ac...@vger.kernel.org; Linus Walleij <linus....@linaro.org>; Alexandre
> Courbot <gnu...@gmail.com>; Rafael J. Wysocki <r...@rjwysocki.net>; Len
> Brown <le...@kernel.org>; sakari...@linux.intel.com; Andy Shevchenko
> <andy.sh...@gmail.com>
> Subject: RE: [PATCH v6 1/3] mfd: Add new mfd device TPS68470
>
>
>
> > -----Original Message-----
> > From: Lee Jones [mailto:lee....@linaro.org]
> > Sent: Tuesday, August 08, 2017 12:40 AM
> > To: Mani, Rajmohan <rajmoh...@intel.com>
> > Cc: linux-...@vger.kernel.org; linux...@vger.kernel.org; linux-
> > ac...@vger.kernel.org; Linus Walleij <linus....@linaro.org>;
> > Alexandre Courbot <gnu...@gmail.com>; Rafael J. Wysocki
> > <r...@rjwysocki.net>; Len Brown <le...@kernel.org>;
> > sakari...@linux.intel.com; Andy Shevchenko
> > <andy.sh...@gmail.com>
> > Subject: Re: [PATCH v6 1/3] mfd: Add new mfd device TPS68470

Mani, Rajmohan

unread,
Aug 9, 2017, 9:20:05 PM8/9/17
to
Hi All,

Now that Lee has applied the MFD patch (https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next&id=6a58f36b0c49c8179804bead579f5ff6304f0245), can someone help clarify the next steps for the GPIO and ACPI / PMIC OpRegion patches of this series?

Both these patches depend on the MFD patch (MFD_TPS68470 config option) to compile.

Thanks
Raj

> -----Original Message-----
> From: Mani, Rajmohan
> Sent: Friday, July 28, 2017 5:30 PM
> To: linux-...@vger.kernel.org; linux...@vger.kernel.org; linux-
> ac...@vger.kernel.org
> Cc: Lee Jones <lee....@linaro.org>; Linus Walleij
> <linus....@linaro.org>; Alexandre Courbot <gnu...@gmail.com>; Rafael
> J. Wysocki <r...@rjwysocki.net>; Len Brown <le...@kernel.org>;
> sakari...@linux.intel.com; Andy Shevchenko
> <andy.sh...@gmail.com>; Mani, Rajmohan
> <rajmoh...@intel.com>
> Subject: [PATCH v6 0/3] TPS68470 PMIC drivers

Linus Walleij

unread,
Aug 14, 2017, 9:40:09 AM8/14/17
to
On Sat, Jul 29, 2017 at 2:30 AM, Rajmohan Mani <rajmoh...@intel.com> wrote:

> This patch adds support for TPS68470 GPIOs.
> There are 7 GPIOs and a few sensor related GPIOs.
> These GPIOs can be requested and configured as
> appropriate.
>
> The GPIOs are also provided with descriptive names.
> However, the typical use case is that the OS GPIO
> driver will interact with TPS68470 GPIO driver
> to configure these GPIOs, as requested by the
> platform firmware.
>
> Signed-off-by: Rajmohan Mani <rajmoh...@intel.com>
> Reviewed-by: Linus Walleij <linus....@linaro.org>

(...)

> +config GPIO_TPS68470
> + bool "TPS68470 GPIO"
> + depends on MFD_TPS68470

Guarded by this symbol, so:
patch applied to the GPIO tree, thanks!

Yours,
Linus Walleij

Mani, Rajmohan

unread,
Aug 14, 2017, 1:00:08 PM8/14/17
to
Hi Linus,

> -----Original Message-----
> From: Linus Walleij [mailto:linus....@linaro.org]
> Sent: Monday, August 14, 2017 6:39 AM
> To: Mani, Rajmohan <rajmoh...@intel.com>
> Cc: linux-...@vger.kernel.org; linux...@vger.kernel.org; ACPI Devel
> Maling List <linux...@vger.kernel.org>; Lee Jones <lee....@linaro.org>;
> Alexandre Courbot <gnu...@gmail.com>; Rafael J. Wysocki
> <r...@rjwysocki.net>; Len Brown <le...@kernel.org>;
> sakari...@linux.intel.com; Andy Shevchenko
> <andy.sh...@gmail.com>
> Subject: Re: [PATCH v6 2/3] gpio: Add support for TPS68470 GPIOs
Thanks for the reviews and update.
Raj

Sakari Ailus

unread,
Aug 14, 2017, 4:40:08 PM8/14/17
to
On Fri, Jul 28, 2017 at 05:30:26PM -0700, Rajmohan Mani wrote:
> The Kabylake platform coreboot (Chrome OS equivalent of
> BIOS) has defined 4 operation regions for the TI TPS68470 PMIC.
> These operation regions are to enable/disable voltage
> regulators, configure voltage regulators, enable/disable
> clocks and to configure clocks.
>
> This config adds ACPI operation region support for TI TPS68470 PMIC.
> TPS68470 device is an advanced power management unit that powers
> a Compact Camera Module (CCM), generates clocks for image sensors,
> drives a dual LED for flash and incorporates two LED drivers for
> general purpose indicators.
> This driver enables ACPI operation region support to control voltage
> regulators and clocks for the TPS68470 PMIC.
>
> Signed-off-by: Rajmohan Mani <rajmoh...@intel.com>

Acked-by: Sakari Ailus <sakari...@linux.intel.com>
--
Sakari Ailus
sakari...@linux.intel.com

Mani, Rajmohan

unread,
Aug 21, 2017, 7:00:09 PM8/21/17
to
Hi Andy,

> > >> > This is the patch series for TPS68470 PMIC that works as a camera PMIC.
> > >> >
> > >> > The patch series provide the following 3 drivers, to help configure the
> voltage regulators, clocks and GPIOs provided by the TPS68470 PMIC, to be
> able to use the camera sensors connected to this PMIC.
> > >> >
> > >> > TPS68470 MFD driver:
> > >> > This is the multi function driver that initializes the TPS68470 PMIC and
> supports the GPIO and Op Region functions.
> > >> >
> > >> > TPS68470 GPIO driver:
> > >> > This is the PMIC GPIO driver that will be used by the OS GPIO layer,
> when the BIOS / firmware triggered GPIO access is done.
> > >> >
> > >> > TPS68470 Op Region driver:
> > >> > This is the driver that will be invoked, when the BIOS / firmware
> configures the voltage / clock for the sensors / vcm devices connected to the
> PMIC.
> > >> >
> > >>
> > >> All three patches are good to me (we did few rounds of internal
> > >> review before posting v4)
> > >>
> > >> Reviewed-by: Andy Shevchenko <andy.sh...@gmail.com>
> > >
> > > OK, so how should they be routed?
> >
> > Good question. I don't know how last time PMIC drivers were merged,
> > here I think is just sane to route vi MFD with immutable branch
> > created.
>
> OK
>
> I will assume that the series will go in through MFD then.
>

Now that the MFD and GPIO patches of v6 of this series have been applied on respective trees, can you advise the next steps for the ACPI / PMIC Opregion driver?

Thanks
Raj

Rafael J. Wysocki

unread,
Aug 21, 2017, 8:10:09 PM8/21/17
to
Well, it would have been better to route the whole series through one
tree. Now it's better to wait until the two other trees get merged
and then apply the opregion patch.

Thanks,
Rafael

Mani, Rajmohan

unread,
Aug 21, 2017, 8:20:09 PM8/21/17
to
Hi Rafael,
Ack.
Let me get back once the other 2 trees are merged.

Thanks
Raj

Mani, Rajmohan

unread,
Sep 7, 2017, 8:50:08 PM9/7/17
to
Hi Rafael,

> Subject: RE: [PATCH v5 0/3] TPS68470 PMIC drivers
>
> Hi Rafael,
> Ack.
> Let me get back once the other 2 trees are merged.
>

Both MFD and GPIO patches of this series got merged upstream as of today.

Thanks
Raj

Mani, Rajmohan

unread,
Sep 25, 2017, 10:00:05 PM9/25/17
to
Hi Rafael, Andy,

Just pinging to see if there are updates on ACPI / PMIC opregion patch...

Thanks
Raj

> -----Original Message-----
> From: Mani, Rajmohan
> Sent: Thursday, September 07, 2017 5:46 PM
> To: 'Rafael J. Wysocki' <raf...@kernel.org>
> Cc: 'Rafael J. Wysocki' <r...@rjwysocki.net>; 'Andy Shevchenko'
> <andy.sh...@gmail.com>; 'linux-...@vger.kernel.org' <linux-
> ker...@vger.kernel.org>; 'linux...@vger.kernel.org' <linux-
> gp...@vger.kernel.org>; 'linux...@vger.kernel.org' <linux-
> ac...@vger.kernel.org>; 'Lee Jones' <lee....@linaro.org>; 'Linus Walleij'
> <linus....@linaro.org>; 'Alexandre Courbot' <gnu...@gmail.com>; 'Len
> Brown' <le...@kernel.org>; 'sakari...@linux.intel.com'
> <sakari...@linux.intel.com>
> Subject: RE: [PATCH v5 0/3] TPS68470 PMIC drivers
>
> Hi Rafael,
>
> > Subject: RE: [PATCH v5 0/3] TPS68470 PMIC drivers
> >
> > Hi Rafael,
> >

Sakari Ailus

unread,
Oct 2, 2017, 4:50:15 PM10/2/17
to
Hi Rafael,

On Mon, Aug 14, 2017 at 11:39:00PM +0300, Sakari Ailus wrote:
> On Fri, Jul 28, 2017 at 05:30:26PM -0700, Rajmohan Mani wrote:
> > The Kabylake platform coreboot (Chrome OS equivalent of
> > BIOS) has defined 4 operation regions for the TI TPS68470 PMIC.
> > These operation regions are to enable/disable voltage
> > regulators, configure voltage regulators, enable/disable
> > clocks and to configure clocks.
> >
> > This config adds ACPI operation region support for TI TPS68470 PMIC.
> > TPS68470 device is an advanced power management unit that powers
> > a Compact Camera Module (CCM), generates clocks for image sensors,
> > drives a dual LED for flash and incorporates two LED drivers for
> > general purpose indicators.
> > This driver enables ACPI operation region support to control voltage
> > regulators and clocks for the TPS68470 PMIC.
> >
> > Signed-off-by: Rajmohan Mani <rajmoh...@intel.com>
>
> Acked-by: Sakari Ailus <sakari...@linux.intel.com>

The other two patches from the set are in v4.14-rc1.

--
Kind regards,

Sakari Ailus
e-mail: sakari...@iki.fi
0 new messages