* [PATCH v1 0/3] TPS65224 PMIC driver
@ 2023-10-26 13:32 Gairuboina Sirisha
2023-10-26 13:32 ` [PATCH v1 1/3] drivers: mfd: Add support for TPS65224 Gairuboina Sirisha
` (3 more replies)
0 siblings, 4 replies; 21+ messages in thread
From: Gairuboina Sirisha @ 2023-10-26 13:32 UTC (permalink / raw)
To: linux-kernel; +Cc: lee, arnd, gregkh, Gairuboina Sirisha
Added support for TPS65224 PMIC in linux.
This patch set includes driver for core, i2c and pfsm.
The driver was tested on TI's custom AM62A EVM.
Gairuboina Sirisha (3):
drivers: mfd: Add support for TPS65224
drivers: mfd: Add support for TPS65224 i2c driver
drivers: misc: Add support for TPS65224 pfsm driver
drivers/mfd/Kconfig | 19 +
drivers/mfd/Makefile | 2 +
drivers/mfd/tps65224-core.c | 291 ++++++++++++
drivers/mfd/tps65224-i2c.c | 245 ++++++++++
drivers/misc/Kconfig | 12 +
drivers/misc/Makefile | 1 +
drivers/misc/tps65224-pfsm.c | 290 ++++++++++++
include/linux/mfd/tps65224.h | 735 +++++++++++++++++++++++++++++
include/uapi/linux/tps65224_pfsm.h | 36 ++
9 files changed, 1631 insertions(+)
create mode 100644 drivers/mfd/tps65224-core.c
create mode 100644 drivers/mfd/tps65224-i2c.c
create mode 100644 drivers/misc/tps65224-pfsm.c
create mode 100644 include/linux/mfd/tps65224.h
create mode 100644 include/uapi/linux/tps65224_pfsm.h
--
2.34.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v1 1/3] drivers: mfd: Add support for TPS65224
2023-10-26 13:32 [PATCH v1 0/3] TPS65224 PMIC driver Gairuboina Sirisha
@ 2023-10-26 13:32 ` Gairuboina Sirisha
2023-10-27 7:02 ` Greg KH
2023-10-26 13:32 ` [PATCH v1 2/3] drivers: mfd: Add support for TPS65224 i2c driver Gairuboina Sirisha
` (2 subsequent siblings)
3 siblings, 1 reply; 21+ messages in thread
From: Gairuboina Sirisha @ 2023-10-26 13:32 UTC (permalink / raw)
To: linux-kernel; +Cc: lee, arnd, gregkh, Gairuboina Sirisha
From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
Added support for tps65224 driver pmic core, header, Makefile and Kconfig
Signed-off-by: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
---
drivers/mfd/Kconfig | 5 +
drivers/mfd/Makefile | 1 +
drivers/mfd/tps65224-core.c | 291 ++++++++++++++
include/linux/mfd/tps65224.h | 735 +++++++++++++++++++++++++++++++++++
4 files changed, 1032 insertions(+)
create mode 100644 drivers/mfd/tps65224-core.c
create mode 100644 include/linux/mfd/tps65224.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 90ce58fd629e..2e4906484eed 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1767,6 +1767,11 @@ config MFD_TPS6594_SPI
This driver can also be built as a module. If so, the module
will be called tps6594-spi.
+config MFD_TPS65224
+ tristate
+ select MFD_CORE
+ select REGMAP
+ select REGMAP_IRQ
config TWL4030_CORE
bool "TI TWL4030/TWL5030/TWL6030/TPS659x0 Support"
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index c66f07edcd0e..ff4e158fa4db 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -102,6 +102,7 @@ obj-$(CONFIG_MFD_TPS65912_SPI) += tps65912-spi.o
obj-$(CONFIG_MFD_TPS6594) += tps6594-core.o
obj-$(CONFIG_MFD_TPS6594_I2C) += tps6594-i2c.o
obj-$(CONFIG_MFD_TPS6594_SPI) += tps6594-spi.o
+obj-$(CONFIG_MFD_TPS65224) += tps65224-core.o
obj-$(CONFIG_MENELAUS) += menelaus.o
obj-$(CONFIG_TWL4030_CORE) += twl-core.o twl4030-irq.o twl6030-irq.o
diff --git a/drivers/mfd/tps65224-core.c b/drivers/mfd/tps65224-core.c
new file mode 100644
index 000000000000..49efdb29e74c
--- /dev/null
+++ b/drivers/mfd/tps65224-core.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Core functions for TI TPS65224 PMIC
+ *
+ * Copyright (C) 2015 Texas Instruments Incorporated - https://www.ti.com/
+ *
+ * Based on the TPS6594 Driver by
+ * Julien Panis <jpanis@baylibre.com>
+ */
+
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+
+#include <linux/mfd/core.h>
+#include <linux/mfd/tps65224.h>
+
+/* Completion to synchronize CRC feature enabling on all PMICs */
+static DECLARE_COMPLETION(tps65224_crc_comp);
+
+static const struct resource tps65224_regulator_resources[] = {
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BUCK1_UVOV, TPS65224_IRQ_NAME_BUCK1_UVOV),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BUCK2_UVOV, TPS65224_IRQ_NAME_BUCK2_UVOV),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BUCK3_UVOV, TPS65224_IRQ_NAME_BUCK3_UVOV),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BUCK4_UVOV, TPS65224_IRQ_NAME_BUCK4_UVOV),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_LDO1_UVOV, TPS65224_IRQ_NAME_LDO1_UVOV),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_LDO2_UVOV, TPS65224_IRQ_NAME_LDO2_UVOV),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_LDO3_UVOV, TPS65224_IRQ_NAME_LDO3_UVOV),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_VCCA_UVOV, TPS65224_IRQ_NAME_VCCA_UVOV),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_VMON1_UVOV, TPS65224_IRQ_NAME_VMON1_UVOV),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_VMON2_UVOV, TPS65224_IRQ_NAME_VMON2_UVOV),
+};
+
+static const struct resource tps65224_pinctrl_resources[] = {
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_GPIO1, TPS65224_IRQ_NAME_GPIO1),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_GPIO2, TPS65224_IRQ_NAME_GPIO2),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_GPIO3, TPS65224_IRQ_NAME_GPIO3),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_GPIO4, TPS65224_IRQ_NAME_GPIO4),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_GPIO5, TPS65224_IRQ_NAME_GPIO5),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_GPIO6, TPS65224_IRQ_NAME_GPIO6),
+};
+
+static const struct resource tps65224_pfsm_resources[] = {
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_VSENSE, TPS65224_IRQ_NAME_VSENSE),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_ENABLE, TPS65224_IRQ_NAME_ENABLE),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_SHORT, TPS65224_IRQ_NAME_PB_SHORT),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_FSD, TPS65224_IRQ_NAME_FSD),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_SOFT_REBOOT, TPS65224_IRQ_NAME_SOFT_REBOOT),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BIST_PASS, TPS65224_IRQ_NAME_BIST_PASS),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_EXT_CLK, TPS65224_IRQ_NAME_EXT_CLK),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_REG_UNLOCK, TPS65224_IRQ_NAME_REG_UNLOCK),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_TWARN, TPS65224_IRQ_NAME_TWARN),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_LONG, TPS65224_IRQ_NAME_PB_LONG),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_FALL, TPS65224_IRQ_NAME_PB_FALL),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_RISE, TPS65224_IRQ_NAME_PB_RISE),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_ADC_CONV_READY, TPS65224_IRQ_NAME_ADC_CONV_READY),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_TSD_ORD, TPS65224_IRQ_NAME_TSD_ORD),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BIST_FAIL, TPS65224_IRQ_NAME_BIST_FAIL),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_REG_CRC_ERR, TPS65224_IRQ_NAME_REG_CRC_ERR),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_RECOV_CNT, TPS65224_IRQ_NAME_RECOV_CNT),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_TSD_IMM, TPS65224_IRQ_NAME_TSD_IMM),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_VCCA_OVP, TPS65224_IRQ_NAME_VCCA_OVP),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PFSM_ERR, TPS65224_IRQ_NAME_PFSM_ERR),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BG_XMON, TPS65224_IRQ_NAME_BG_XMON),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_IMM_SHUTDOWN, TPS65224_IRQ_NAME_IMM_SHUTDOWN),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_ORD_SHUTDOWN, TPS65224_IRQ_NAME_ORD_SHUTDOWN),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_MCU_PWR_ERR, TPS65224_IRQ_NAME_MCU_PWR_ERR),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_SOC_PWR_ERR, TPS65224_IRQ_NAME_SOC_PWR_ERR),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_COMM_ERR, TPS65224_IRQ_NAME_COMM_ERR),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_I2C2_ERR, TPS65224_IRQ_NAME_I2C2_ERR),
+};
+
+static const struct resource tps65224_esm_resources[] = {
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_ESM_MCU_PIN, TPS65224_IRQ_NAME_ESM_MCU_PIN),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_ESM_MCU_FAIL, TPS65224_IRQ_NAME_ESM_MCU_FAIL),
+ DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_ESM_MCU_RST, TPS65224_IRQ_NAME_ESM_MCU_RST),
+};
+
+static const struct mfd_cell tps65224_common_cells[] = {
+ MFD_CELL_RES("tps65224-regulator", tps65224_regulator_resources),
+ MFD_CELL_RES("tps65224-pinctrl", tps65224_pinctrl_resources),
+ MFD_CELL_RES("tps65224-pfsm", tps65224_pfsm_resources),
+ MFD_CELL_RES("tps65224-esm", tps65224_esm_resources),
+};
+
+static const struct regmap_irq tps65224_irqs[] = {
+ /* INT_BUCK register */
+ REGMAP_IRQ_REG(TPS65224_IRQ_BUCK1_UVOV, 0, TPS65224_BIT_BUCK1_UVOV_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_BUCK2_UVOV, 0, TPS65224_BIT_BUCK2_UVOV_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_BUCK3_UVOV, 0, TPS65224_BIT_BUCK3_UVOV_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_BUCK4_UVOV, 0, TPS65224_BIT_BUCK4_UVOV_INT),
+
+ /* INT_VMON_LDO register */
+ REGMAP_IRQ_REG(TPS65224_IRQ_LDO1_UVOV, 1, TPS65224_BIT_LDO1_UVOV_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_LDO2_UVOV, 1, TPS65224_BIT_LDO2_UVOV_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_LDO3_UVOV, 1, TPS65224_BIT_LDO3_UVOV_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_VCCA_UVOV, 1, TPS65224_BIT_VCCA_UVOV_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_VMON1_UVOV, 1, TPS65224_BIT_VMON1_UVOV_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_VMON2_UVOV, 1, TPS65224_BIT_VMON2_UVOV_INT),
+
+ /* INT_GPIO register */
+ REGMAP_IRQ_REG(TPS65224_IRQ_GPIO1, 2, TPS65224_BIT_GPIO1_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_GPIO2, 2, TPS65224_BIT_GPIO2_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_GPIO3, 2, TPS65224_BIT_GPIO3_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_GPIO4, 2, TPS65224_BIT_GPIO4_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_GPIO5, 2, TPS65224_BIT_GPIO5_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_GPIO6, 2, TPS65224_BIT_GPIO6_INT),
+
+ /* INT_STARTUP register */
+ REGMAP_IRQ_REG(TPS65224_IRQ_VSENSE, 3, TPS65224_BIT_VSENSE_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_ENABLE, 3, TPS65224_BIT_ENABLE_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_PB_SHORT, 3, TPS65224_BIT_PB_SHORT_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_FSD, 3, TPS65224_BIT_FSD_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_SOFT_REBOOT, 3, TPS65224_BIT_SOFT_REBOOT_INT),
+
+ /* INT_MISC register */
+ REGMAP_IRQ_REG(TPS65224_IRQ_BIST_PASS, 4, TPS65224_BIT_BIST_PASS_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_EXT_CLK, 4, TPS65224_BIT_EXT_CLK_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_REG_UNLOCK, 4, TPS65224_BIT_REG_UNLOCK_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_TWARN, 4, TPS65224_BIT_TWARN_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_PB_LONG, 4, TPS65224_BIT_PB_LONG_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_PB_FALL, 4, TPS65224_BIT_PB_FALL_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_PB_RISE, 4, TPS65224_BIT_PB_RISE_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_ADC_CONV_READY, 4, TPS65224_BIT_ADC_CONV_READY_INT),
+
+ /* INT_MODERATE_ERR register */
+ REGMAP_IRQ_REG(TPS65224_IRQ_TSD_ORD, 5, TPS65224_BIT_TSD_ORD_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_BIST_FAIL, 5, TPS65224_BIT_BIST_FAIL_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_REG_CRC_ERR, 5, TPS65224_BIT_REG_CRC_ERR_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_RECOV_CNT, 5, TPS65224_BIT_RECOV_CNT_INT),
+
+ /* INT_SEVERE_ERR register */
+ REGMAP_IRQ_REG(TPS65224_IRQ_TSD_IMM, 6, TPS65224_BIT_TSD_IMM_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_VCCA_OVP, 6, TPS65224_BIT_VCCA_OVP_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_PFSM_ERR, 6, TPS65224_BIT_PFSM_ERR_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_BG_XMON, 6, TPS65224_BIT_BG_XMON_INT),
+
+ /* INT_FSM_ERR register */
+ REGMAP_IRQ_REG(TPS65224_IRQ_IMM_SHUTDOWN, 7, TPS65224_BIT_IMM_SHUTDOWN_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_ORD_SHUTDOWN, 7, TPS65224_BIT_ORD_SHUTDOWN_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_MCU_PWR_ERR, 7, TPS65224_BIT_MCU_PWR_ERR_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_SOC_PWR_ERR, 7, TPS65224_BIT_SOC_PWR_ERR_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_COMM_ERR, 7, TPS65224_BIT_COMM_ERR_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_I2C2_ERR, 7, TPS65224_BIT_I2C2_ERR_INT),
+
+ /* INT_ESM register */
+ REGMAP_IRQ_REG(TPS65224_IRQ_ESM_MCU_PIN, 8, TPS65224_BIT_ESM_MCU_PIN_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_ESM_MCU_FAIL, 8, TPS65224_BIT_ESM_MCU_FAIL_INT),
+ REGMAP_IRQ_REG(TPS65224_IRQ_ESM_MCU_RST, 8, TPS65224_BIT_ESM_MCU_RST_INT),
+};
+
+static const unsigned int tps65224_irq_reg[] = {
+ TPS65224_REG_INT_BUCK,
+ TPS65224_REG_INT_LDO_VMON,
+ TPS65224_REG_INT_GPIO,
+ TPS65224_REG_INT_STARTUP,
+ TPS65224_REG_INT_MISC,
+ TPS65224_REG_INT_MODERATE_ERR,
+ TPS65224_REG_INT_SEVERE_ERR,
+ TPS65224_REG_INT_FSM_ERR,
+ TPS65224_REG_INT_ESM,
+};
+
+static inline unsigned int tps65224_get_irq_reg(struct regmap_irq_chip_data *data,
+ unsigned int base, int index)
+{
+ return tps65224_irq_reg[index];
+};
+
+static int tps65224_handle_post_irq(void *irq_drv_data)
+{
+ struct tps65224 *tps = irq_drv_data;
+ int ret = 0;
+
+ /*
+ * When CRC is enabled, writing to a read-only bit triggers an error,
+ * and COMM_ADR_ERR_INT bit is set. Besides, bits indicating interrupts
+ * (that must be cleared) and read-only bits are sometimes grouped in
+ * the same register.
+ * Since regmap clears interrupts by doing a write per register, clearing
+ * an interrupt bit in a register containing also a read-only bit makes
+ * COMM_ADR_ERR_INT bit set. Clear immediately this bit to avoid raising
+ * a new interrupt.
+ */
+ if (tps->use_crc)
+ ret = regmap_write_bits(tps->regmap, TPS65224_REG_INT_FSM_ERR,
+ TPS65224_BIT_COMM_ERR_INT,
+ TPS65224_BIT_COMM_ERR_INT);
+
+ return ret;
+};
+
+static struct regmap_irq_chip tps65224_irq_chip = {
+ .ack_base = TPS65224_REG_INT_BUCK,
+ .ack_invert = 1,
+ .clear_ack = 1,
+ .init_ack_masked = 1,
+ .num_regs = ARRAY_SIZE(tps65224_irq_reg),
+ .irqs = tps65224_irqs,
+ .num_irqs = ARRAY_SIZE(tps65224_irqs),
+ .get_irq_reg = tps65224_get_irq_reg,
+ .handle_post_irq = tps65224_handle_post_irq,
+};
+
+bool tps65224_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+ return (reg >= TPS65224_REG_INT_TOP && reg <= TPS65224_REG_STAT_SEVERE_ERR);
+}
+EXPORT_SYMBOL_GPL(tps65224_is_volatile_reg);
+
+static int tps65224_check_crc_mode(struct tps65224 *tps, bool pmic)
+{
+ int ret;
+
+ /*
+ * Check if CRC is enabled.
+ * Once CRC is enabled, it can't be disabled until next power cycle.
+ */
+ tps->use_crc = true;
+ ret = regmap_test_bits(tps->regmap, TPS65224_REG_CONFIG_2,
+ TPS65224_BIT_I2C1_SPI_CRC_EN);
+ if (ret == 0)
+ ret = -EIO;
+ else if (ret > 0)
+ ret = 0;
+
+ return ret;
+}
+
+static int tps65224_set_crc_feature(struct tps65224 *tps)
+{
+ int ret;
+
+ ret = tps65224_check_crc_mode(tps, true);
+ if (ret) {
+ /*
+ * If CRC is not already enabled, force PFSM I2C_2 trigger to enable it
+ * on PMIC.
+ */
+ tps->use_crc = false;
+ ret = regmap_write_bits(tps->regmap, TPS65224_REG_CONFIG_2,
+ TPS65224_BIT_I2C1_SPI_CRC_EN, TPS65224_BIT_I2C1_SPI_CRC_EN);
+ if (ret)
+ return ret;
+
+ ret = tps65224_check_crc_mode(tps, true);
+ }
+
+ return ret;
+}
+
+int tps65224_device_init(struct tps65224 *tps, bool enable_crc)
+{
+ struct device *dev = tps->dev;
+ int ret;
+
+ /* Enable CRC feature on PMIC */
+ ret = tps65224_set_crc_feature(tps);
+ if (ret)
+ return ret;
+
+ /* Keep PMIC in ACTIVE state */
+ ret = regmap_set_bits(tps->regmap, TPS65224_REG_FSM_NSLEEP_TRIGGERS,
+ TPS65224_BIT_NSLEEP1B | TPS65224_BIT_NSLEEP2B);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to set PMIC state\n");
+
+ tps65224_irq_chip.irq_drv_data = tps;
+ tps65224_irq_chip.name = devm_kasprintf(dev, GFP_KERNEL, "%s-%ld-0x%02x",
+ dev->driver->name, tps->chip_id, tps->reg);
+
+ ret = devm_regmap_add_irq_chip(dev, tps->regmap, tps->irq, IRQF_SHARED | IRQF_ONESHOT,
+ 0, &tps65224_irq_chip, &tps->irq_data);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to add regmap IRQ\n");
+
+ ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, tps65224_common_cells,
+ ARRAY_SIZE(tps65224_common_cells), NULL, 0,
+ regmap_irq_get_domain(tps->irq_data));
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to add common child devices\n");
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(tps65224_device_init);
+
+MODULE_AUTHOR("Gairuboina Sirisha <sirisha.gairuboina@ltts.com>");
+MODULE_DESCRIPTION("TPS65224 Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/tps65224.h b/include/linux/mfd/tps65224.h
new file mode 100644
index 000000000000..5a1df92d601d
--- /dev/null
+++ b/include/linux/mfd/tps65224.h
@@ -0,0 +1,735 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Functions to access TPS65224 Power Management IC
+ *
+ * Copyright (C) 2015 Texas Instruments Incorporated - https://www.ti.com/
+ */
+
+#ifndef __LINUX_MFD_TPS65224_H
+#define __LINUX_MFD_TPS65224_H
+
+#include <linux/device.h>
+#include <linux/regmap.h>
+
+struct regmap_irq_chip_data;
+
+/* Macro to get page index from register address */
+#define TPS65224_REG_TO_PAGE(reg) ((reg) >> 8)
+
+/* Registers for page 0 of TPS65224 */
+#define TPS65224_REG_DEV_REV 0x01
+
+#define TPS65224_REG_NVM_CODE_1 0x02
+#define TPS65224_REG_NVM_CODE_2 0x03
+
+#define TPS65224_REG_BUCKX_CTRL(buck_inst) (0x04 + ((buck_inst) << 1))
+#define TPS65224_REG_BUCKX_CONF(buck_inst) (0x05 + ((buck_inst) << 1))
+#define TPS65224_REG_BUCKX_VOUT(buck_inst) (0x0e + ((buck_inst) << 1))
+#define TPS65224_REG_BUCKX_PG_WINDOW(buck_inst) (0x18 + (buck_inst))
+
+#define TPS65224_REG_LDOX_CTRL(ldo_inst) (0x1d + (ldo_inst))
+#define TPS65224_REG_LDOX_VOUT(ldo_inst) (0x23 + (ldo_inst))
+#define TPS65224_REG_LDOX_PG_WINDOW(ldo_inst) (0x27 + (ldo_inst))
+
+#define TPS65224_REG_VCCA_VMON_CTRL 0x2b
+#define TPS65224_REG_VCCA_PG_WINDOW 0x2c
+#define TPS65224_REG_VMON1_PG_WINDOW 0x2d
+#define TPS65224_REG_VMON1_PG_LEVEL 0x2e
+#define TPS65224_REG_VMON2_PG_WINDOW 0x2f
+#define TPS65224_REG_VMON2_PG_LEVEL 0x30
+
+#define TPS65224_REG_GPIOX_CONF(gpio_inst) (0x31 + (gpio_inst))
+#define TPS65224_REG_POWER_ON_CONFIG 0x3c
+#define TPS65224_REG_GPIO_OUT_1 0x3d
+#define TPS65224_REG_GPIO_IN_1 0x3f
+
+#define TPS65224_REG_RAIL_SEL_1 0x41
+#define TPS65224_REG_RAIL_SEL_2 0x42
+#define TPS65224_REG_RAIL_SEL_3 0x43
+
+#define TPS65224_REG_FSM_TRIG_SEL_1 0x44
+#define TPS65224_REG_FSM_TRIG_SEL_2 0x45
+#define TPS65224_REG_FSM_TRIG_MASK_1 0x46
+#define TPS65224_REG_FSM_TRIG_MASK_2 0x47
+
+#define TPS65224_REG_MASK_BUCK 0x49
+#define TPS65224_REG_MASK_LDO_VMON 0x4c
+#define TPS65224_REG_MASK_GPIO_FALL 0x4f
+#define TPS65224_REG_MASK_GPIO_RISE 0x50
+#define TPS65224_REG_MASK_STARTUP 0x52
+#define TPS65224_REG_MASK_MISC 0x53
+#define TPS65224_REG_MASK_MODERATE_ERR 0x54
+#define TPS65224_REG_MASK_FSM_ERR 0x56
+#define TPS65224_REG_MASK_ESM 0x59
+
+#define TPS65224_REG_INT_TOP 0x5a
+#define TPS65224_REG_INT_BUCK 0x5b
+#define TPS65224_REG_INT_LDO_VMON 0x5f
+#define TPS65224_REG_INT_GPIO 0x63
+#define TPS65224_REG_INT_STARTUP 0x65
+#define TPS65224_REG_INT_MISC 0x66
+#define TPS65224_REG_INT_MODERATE_ERR 0x67
+#define TPS65224_REG_INT_SEVERE_ERR 0x68
+#define TPS65224_REG_INT_FSM_ERR 0x69
+#define TPS65224_REG_INT_ESM 0x6c
+
+#define TPS65224_REG_STAT_BUCK 0x6d
+#define TPS65224_REG_STAT_LDO_VMON 0x70
+#define TPS65224_REG_STAT_STARTUP 0x73
+#define TPS65224_REG_STAT_MISC 0x74
+#define TPS65224_REG_STAT_MODERATE_ERR 0x75
+#define TPS65224_REG_STAT_SEVERE_ERR 0x76
+
+#define TPS65224_REG_PLL_CTRL 0x7c
+
+#define TPS65224_REG_CONFIG_1 0x7d
+#define TPS65224_REG_CONFIG_2 0x7e
+
+#define TPS65224_REG_ENABLE_DRV_REG 0x80
+
+#define TPS65224_REG_MISC_CTRL 0x81
+
+#define TPS65224_REG_ENABLE_DRV_STAT 0x82
+
+#define TPS65224_REG_RECOV_CNT_REG_1 0x83
+#define TPS65224_REG_RECOV_CNT_REG_2 0x84
+
+#define TPS65224_REG_FSM_I2C_TRIGGERS 0x85
+#define TPS65224_REG_FSM_NSLEEP_TRIGGERS 0x86
+
+#define TPS65224_REG_BUCK_RESET_REG 0x87
+
+#define TPS65224_REG_SPREAD_SPECTRUM_1 0x88
+
+#define TPS65224_REG_FSM_STEP_SIZE 0x8b
+
+#define TPS65224_REG_USER_SPARE_REGS 0x8e
+
+#define TPS65224_REG_ESM_MCU_START_REG 0x8f
+#define TPS65224_REG_ESM_MCU_DELAY1_REG 0x90
+#define TPS65224_REG_ESM_MCU_DELAY2_REG 0x91
+#define TPS65224_REG_ESM_MCU_MODE_CFG 0x92
+#define TPS65224_REG_ESM_MCU_HMAX_REG 0x93
+#define TPS65224_REG_ESM_MCU_HMIN_REG 0x94
+#define TPS65224_REG_ESM_MCU_LMAX_REG 0x95
+#define TPS65224_REG_ESM_MCU_LMIN_REG 0x96
+#define TPS65224_REG_ESM_MCU_ERR_CNT_REG 0x97
+
+#define TPS65224_REG_REGISTER_LOCK 0xa1
+
+/* Added Additional reg's as per datasheet start*/
+#define TPS65224_REG_SRAM_ACCESS_1 0xa2
+#define TPS65224_REG_SRAM_ACCESS_2 0xa3
+#define TPS65224_REG_SRAM_ADDR_CTRL 0xa4
+#define TPS65224_REG_RECOV_CNT_PFSM_INCR 0xa5
+/* Added Additional reg's as per datasheet end*/
+
+#define TPS65224_REG_MANUFACTURING_VER 0xa6
+
+#define TPS65224_REG_CUSTOMER_NVM_ID_REG 0xa7
+
+#define TPS65224_REG_SOFT_REBOOT_REG 0xab
+
+/* Added Additional reg's as per datasheet start*/
+#define TPS65224_REG_ADC_CTRL 0xac
+#define TPS65224_REG_ADC_RESULT_REG_1 0xad
+#define TPS65224_REG_ADC_RESULT_REG_2 0xae
+#define TPS65224_REG_STARTUP_CTRL 0xc3
+
+#define TPS65224_REG_ADC_GAIN_COMP_REG 0xd0
+
+#define TPS65224_REG_CRC_CALC_CONTROL 0xef
+#define TPS65224_REG_REGMAP_USER_CRC_LOW 0xf0
+#define TPS65224_REG_REGMAP_USER_CRC_HIGH 0xf1
+/* Added Additional reg's as per datasheet end*/
+
+#define TPS65224_REG_SCRATCH_PAD_REG_1 0xc9
+#define TPS65224_REG_SCRATCH_PAD_REG_2 0xca
+#define TPS65224_REG_SCRATCH_PAD_REG_3 0xcb
+#define TPS65224_REG_SCRATCH_PAD_REG_4 0xcc
+
+#define TPS65224_REG_PFSM_DELAY_REG_1 0xcd
+#define TPS65224_REG_PFSM_DELAY_REG_2 0xce
+#define TPS65224_REG_PFSM_DELAY_REG_3 0xcf
+
+/* Registers for page 4 of TPS65224 */
+#define TPS65224_REG_WD_ANSWER_REG 0x401
+#define TPS65224_REG_WD_QUESTION_ANSW_CNT 0x402
+#define TPS65224_REG_WD_WIN1_CFG 0x403
+#define TPS65224_REG_WD_WIN2_CFG 0x404
+#define TPS65224_REG_WD_LONGWIN_CFG 0x405
+#define TPS65224_REG_WD_MODE_REG 0x406
+#define TPS65224_REG_WD_QA_CFG 0x407
+#define TPS65224_REG_WD_ERR_STATUS 0x408
+#define TPS65224_REG_WD_THR_CFG 0x409
+#define TPS65224_REG_WD_FAIL_CNT_REG 0x40a
+
+/* BUCKX_CTRL register field definition */
+#define TPS65224_BIT_BUCK_EN BIT(0)
+#define TPS65224_BIT_BUCK_FPWM BIT(1)
+#define TPS65224_BIT_BUCK_VMON_EN BIT(4)
+#define TPS65224_BIT_BUCK_PLDN BIT(5)
+
+/* BUCKX_CONF register field definition */
+#define TPS65224_MASK_BUCK_SLEW_RATE GENMASK(1, 0)
+
+/* BUCKX_PG_WINDOW register field definition */
+#define TPS65224_MASK_BUCK_VMON_THR GENMASK(1, 0)
+
+/* BUCKX VSET */
+#define TPS65224_MASK_BUCK1_VSET GENMASK(7, 0)
+#define TPS65224_MASK_BUCKS_VSET GENMASK(6, 0)
+
+/* LDOX_CTRL register field definition */
+#define TPS65224_BIT_LDO_EN BIT(0)
+#define TPS65224_BIT_LDO_VMON_EN BIT(4)
+#define TPS65224_BIT_LDO_DISCHARGE_EN BIT(5)
+
+/* LDOX_PG_WINDOW register field definition */
+#define TPS65224_MASK_LDO_VMON_THR GENMASK(1, 0)
+
+/* VCCA_VMON_CTRL register field definition */
+#define TPS65224_BIT_VMON_EN BIT(0)
+#define TPS65224_BIT_VMON1_EN BIT(1)
+#define TPS65224_BIT_VMON2_EN BIT(3)
+#define TPS65224_MASK_VMON_DEGLITCH_SEL GENMASK(7, 5)
+
+/* LDOX_VOUT register field definition */
+#define TPS65224_MASK_LDO_VSET GENMASK(6, 1)
+#define TPS65224_BIT_LDO_BYP_CONFIG BIT(7)
+
+/* VCCA_PG_WINDOW register field definition */
+#define TPS65224_MASK_VCCA_VMON_THR GENMASK(1, 0)
+#define TPS65224_BIT_VCCA_PG_SET BIT(6)
+
+/* VMONX_PG_WINDOW register field definition */
+#define TPS65224_MASK_VMONX_THR GENMASK(1, 0)
+
+/* GPIO_CONF register field definition */
+#define TPS65224_BIT_GPIO_DIR BIT(0)
+#define TPS65224_BIT_GPIO_OD BIT(1)
+#define TPS65224_BIT_GPIO_PU_SEL BIT(2)
+#define TPS65224_BIT_GPIO_PU_PD_EN BIT(3)
+#define TPS65224_BIT_GPIO_DEGLITCH_EN BIT(4)
+#define TPS65224_MASK_GPIO_SEL GENMASK(6, 5)
+#define TPS65224_MASK_GPIO_SEL_GPIO6 GENMASK(7, 5)
+
+/* POWER_ON_CONFIG register field definition */
+#define TPS65224_BIT_NINT_ENDRV_PU_SEL BIT(0)
+#define TPS65224_BIT_NINT_ENDRV_SEL BIT(1)
+#define TPS65224_BIT_EN_PB_DEGL BIT(5)
+#define TPS65224_MASK_EN_PB_VSENSE_CONFIG GENMASK(7, 6)
+
+/* GPIO_OUT_X register field definition */
+#define TPS65224_BIT_GPIOX_OUT(gpio_inst) BIT((gpio_inst))
+
+/* GPIO_IN_X register field definition */
+#define TPS65224_BIT_GPIOX_IN(gpio_inst) BIT((gpio_inst))
+
+
+/* RAIL_SEL_1 register field definition */
+#define TPS65224_MASK_BUCK1_GRP_SEL GENMASK(1, 0)
+#define TPS65224_MASK_BUCK2_GRP_SEL GENMASK(3, 2)
+#define TPS65224_MASK_BUCK3_GRP_SEL GENMASK(5, 4)
+#define TPS65224_MASK_BUCK4_GRP_SEL GENMASK(7, 6)
+
+/* RAIL_SEL_2 register field definition */
+#define TPS65224_MASK_LDO1_GRP_SEL GENMASK(3, 2)
+#define TPS65224_MASK_LDO2_GRP_SEL GENMASK(5, 4)
+#define TPS65224_MASK_LDO3_GRP_SEL GENMASK(7, 6)
+
+/* RAIL_SEL_3 register field definition */
+#define TPS65224_MASK_VCCA_GRP_SEL GENMASK(3, 2)
+#define TPS65224_MASK_VMON1_GRP_SEL GENMASK(5, 4)
+#define TPS65224_MASK_VMON2_GRP_SEL GENMASK(7, 6)
+
+/* FSM_TRIG_SEL_1 register field definition */
+#define TPS65224_MASK_MCU_RAIL_TRIG GENMASK(1, 0)
+#define TPS65224_MASK_SOC_RAIL_TRIG GENMASK(3, 2)
+#define TPS65224_MASK_OTHER_RAIL_TRIG GENMASK(5, 4)
+#define TPS65224_MASK_SEVERE_ERR_TRIG GENMASK(7, 6)
+
+/* FSM_TRIG_SEL_2 register field definition */
+#define TPS65224_MASK_MODERATE_ERR_TRIG GENMASK(1, 0)
+
+/* FSM_TRIG_MASK_X register field definition */
+#define TPS65224_BIT_GPIOX_FSM_MASK(gpio_inst) BIT(((gpio_inst) << 1) % 8)
+#define TPS65224_BIT_GPIOX_FSM_MASK_POL(gpio_inst) BIT(((gpio_inst) << 1) % 8 + 1)
+
+/* MASK_BUCK Register field definition */
+#define TPS65224_BIT_BUCK1_UVOV_MASK BIT(0)
+#define TPS65224_BIT_BUCK2_UVOV_MASK BIT(1)
+#define TPS65224_BIT_BUCK3_UVOV_MASK BIT(2)
+#define TPS65224_BIT_BUCK4_UVOV_MASK BIT(4)
+
+/* MASK_LDO_VMON register field definition */
+#define TPS65224_BIT_LDO1_UVOV_MASK BIT(0)
+#define TPS65224_BIT_LDO2_UVOV_MASK BIT(1)
+#define TPS65224_BIT_LDO3_UVOV_MASK BIT(2)
+#define TPS65224_BIT_VCCA_UVOV_MASK BIT(4)
+#define TPS65224_BIT_VMON1_UVOV_MASK BIT(5)
+#define TPS65224_BIT_VMON2_UVOV_MASK BIT(6)
+
+/* MASK_GPIOX register field definition */
+#define TPS65224_BIT_GPIOX_FALL_MASK(gpio_inst) BIT((gpio_inst))
+#define TPS65224_BIT_GPIOX_RISE_MASK(gpio_inst) BIT((gpio_inst))
+
+/* MASK_STARTUP register field definition */
+#define TPS65224_BIT_VSENSE_MASK BIT(0)
+#define TPS65224_BIT_ENABLE_MASK BIT(1)
+#define TPS65224_BIT_PB_SHORT_MASK BIT(2)
+#define TPS65224_BIT_FSD_MASK BIT(4)
+#define TPS65224_BIT_SOFT_REBOOT_MASK BIT(5)
+
+/* MASK_MISC register field definition */
+#define TPS65224_BIT_BIST_PASS_MASK BIT(0)
+#define TPS65224_BIT_EXT_CLK_MASK BIT(1)
+#define TPS65224_BIT_REG_UNLOCK_MASK BIT(2)
+#define TPS65224_BIT_TWARN_MASK BIT(3)
+#define TPS65224_BIT_PB_LONG_MASK BIT(4)
+#define TPS65224_BIT_PB_FALL_MASK BIT(5)
+#define TPS65224_BIT_PB_RISE_MASK BIT(6)
+#define TPS65224_BIT_ADC_CONV_READY_MASK BIT(7)
+
+/* MASK_MODERATE_ERR register field definition */
+#define TPS65224_BIT_BIST_FAIL_MASK BIT(1)
+#define TPS65224_BIT_REG_CRC_ERR_MASK BIT(2)
+
+/* MASK_FSM_ERR register field definition */
+#define TPS65224_BIT_IMM_SHUTDOWN_MASK BIT(0)
+#define TPS65224_BIT_ORD_SHUTDOWN_MASK BIT(1)
+#define TPS65224_BIT_MCU_PWR_ERR_MASK BIT(2)
+#define TPS65224_BIT_SOC_PWR_ERR_MASK BIT(3)
+#define TPS65224_BIT_COMM_ERR_MASK BIT(4)
+#define TPS65224_BIT_I2C2_ERR_MASK BIT(5)
+
+/* MASK_ESM register field definition */
+#define TPS65224_BIT_ESM_MCU_PIN_MASK BIT(3)
+#define TPS65224_BIT_ESM_MCU_FAIL_MASK BIT(4)
+#define TPS65224_BIT_ESM_MCU_RST_MASK BIT(5)
+
+/* INT_TOP register field definition */
+#define TPS65224_BIT_BUCK_INT BIT(0)
+#define TPS65224_BIT_LDO_VMON_INT BIT(1)
+#define TPS65224_BIT_GPIO_INT BIT(2)
+#define TPS65224_BIT_STARTUP_INT BIT(3)
+#define TPS65224_BIT_MISC_INT BIT(4)
+#define TPS65224_BIT_MODERATE_ERR_INT BIT(5)
+#define TPS65224_BIT_SEVERE_ERR_INT BIT(6)
+#define TPS65224_BIT_FSM_ERR_INT BIT(7)
+
+/* INT_BUCK register field definition */
+#define TPS65224_BIT_BUCK1_UVOV_INT BIT(0)
+#define TPS65224_BIT_BUCK2_UVOV_INT BIT(1)
+#define TPS65224_BIT_BUCK3_UVOV_INT BIT(2)
+#define TPS65224_BIT_BUCK4_UVOV_INT BIT(3)
+
+/* INT_LDO_VMON register field definition */
+#define TPS65224_BIT_LDO1_UVOV_INT BIT(0)
+#define TPS65224_BIT_LDO2_UVOV_INT BIT(1)
+#define TPS65224_BIT_LDO3_UVOV_INT BIT(2)
+#define TPS65224_BIT_VCCA_UVOV_INT BIT(4)
+#define TPS65224_BIT_VMON1_UVOV_INT BIT(5)
+#define TPS65224_BIT_VMON2_UVOV_INT BIT(6)
+
+/* INT_GPIO register field definition */
+#define TPS65224_BIT_GPIO1_INT BIT(0)
+#define TPS65224_BIT_GPIO2_INT BIT(1)
+#define TPS65224_BIT_GPIO3_INT BIT(2)
+#define TPS65224_BIT_GPIO4_INT BIT(3)
+#define TPS65224_BIT_GPIO5_INT BIT(4)
+#define TPS65224_BIT_GPIO6_INT BIT(5)
+
+/* INT_STARTUP register field definition */
+#define TPS65224_BIT_VSENSE_INT BIT(0)
+#define TPS65224_BIT_ENABLE_INT BIT(1)
+#define TPS65224_BIT_PB_SHORT_INT BIT(2)
+#define TPS65224_BIT_FSD_INT BIT(4)
+#define TPS65224_BIT_SOFT_REBOOT_INT BIT(5)
+
+/* INT_MISC register field definition */
+#define TPS65224_BIT_BIST_PASS_INT BIT(0)
+#define TPS65224_BIT_EXT_CLK_INT BIT(1)
+#define TPS65224_BIT_REG_UNLOCK_INT BIT(2)
+#define TPS65224_BIT_TWARN_INT BIT(3)
+#define TPS65224_BIT_PB_LONG_INT BIT(4)
+#define TPS65224_BIT_PB_FALL_INT BIT(5)
+#define TPS65224_BIT_PB_RISE_INT BIT(6)
+#define TPS65224_BIT_ADC_CONV_READY_INT BIT(7)
+
+/* INT_MODERATE_ERR register field definition */
+#define TPS65224_BIT_TSD_ORD_INT BIT(0)
+#define TPS65224_BIT_BIST_FAIL_INT BIT(1)
+#define TPS65224_BIT_REG_CRC_ERR_INT BIT(2)
+#define TPS65224_BIT_RECOV_CNT_INT BIT(3)
+
+/* INT_SEVERE_ERR register field definition */
+#define TPS65224_BIT_TSD_IMM_INT BIT(0)
+#define TPS65224_BIT_VCCA_OVP_INT BIT(1)
+#define TPS65224_BIT_PFSM_ERR_INT BIT(2)
+#define TPS65224_BIT_BG_XMON_INT BIT(3)
+
+/* INT_FSM_ERR register field definition */
+#define TPS65224_BIT_IMM_SHUTDOWN_INT BIT(0)
+#define TPS65224_BIT_ORD_SHUTDOWN_INT BIT(1)
+#define TPS65224_BIT_MCU_PWR_ERR_INT BIT(2)
+#define TPS65224_BIT_SOC_PWR_ERR_INT BIT(3)
+#define TPS65224_BIT_COMM_ERR_INT BIT(4)
+#define TPS65224_BIT_I2C2_ERR_INT BIT(5)
+#define TPS65224_BIT_ESM_INT BIT(6)
+#define TPS65224_BIT_WD_INT BIT(7)
+
+/* INT_ESM register field definition */
+#define TPS65224_BIT_ESM_MCU_PIN_INT BIT(3)
+#define TPS65224_BIT_ESM_MCU_FAIL_INT BIT(4)
+#define TPS65224_BIT_ESM_MCU_RST_INT BIT(5)
+
+/* STAT_LDO_VMON register field definition */
+#define TPS65224_BIT_LDO1_UVOV_STAT BIT(0)
+#define TPS65224_BIT_LDO2_UVOV_STAT BIT(1)
+#define TPS65224_BIT_LDO3_UVOV_STAT BIT(2)
+#define TPS65224_BIT_VCCA_UVOV_STAT BIT(4)
+#define TPS65224_BIT_VMON1_UVOV_STAT BIT(5)
+#define TPS65224_BIT_VMON2_UVOV_STAT BIT(6)
+
+/* STAT_STARTUP register field definition */
+#define TPS65224_BIT_VSENSE_STAT BIT(0)
+#define TPS65224_BIT_ENABLE_STAT BIT(1)
+#define TPS65224_BIT_PB_LEVEL_STAT BIT(2)
+
+/* STAT_MISC register field definition */
+#define TPS65224_BIT_EXT_CLK_STAT BIT(1)
+#define TPS65224_BIT_TWARN_STAT BIT(3)
+
+/* STAT_MODERATE_ERR register field definition */
+#define TPS65224_BIT_TSD_ORD_STAT BIT(0)
+
+/* STAT_SEVERE_ERR register field definition */
+#define TPS65224_BIT_TSD_IMM_STAT BIT(0)
+#define TPS65224_BIT_VCCA_OVP_STAT BIT(1)
+#define TPS65224_BIT_BG_XMON_STAT BIT(3)
+
+/* PLL_CTRL register field definition */
+#define TPS65224_MASK_EXT_CLK_FREQ GENMASK(1, 0)
+
+/* CONFIG_1 register field definition */
+#define TPS65224_BIT_TWARN_LEVEL BIT(0)
+#define TPS65224_BIT_TSD_ORD_LEVEL BIT(1)
+#define TPS65224_BIT_I2C1_HS BIT(3)
+#define TPS65224_BIT_I2C2_HS BIT(4)
+#define TPS65224_BIT_NSLEEP1_MASK BIT(6)
+#define TPS65224_BIT_NSLEEP2_MASK BIT(7)
+
+/* CONFIG_2 register field definition */
+#define TPS65224_BIT_I2C1_SPI_CRC_EN BIT(4)
+#define TPS65224_BIT_I2C2_CRC_EN BIT(5)
+
+/* ENABLE_DRV_REG register field definition */
+#define TPS65224_BIT_ENABLE_DRV BIT(0)
+
+/* MISC_CTRL register field definition */
+#define TPS65224_BIT_NRSTOUT BIT(0)
+#define TPS65224_BIT_LPM_EN BIT(2)
+#define TPS65224_BIT_SEL_EXT_CLK BIT(5)
+
+/* ENABLE_DRV_STAT register field definition */
+#define TPS65224_BIT_EN_DRV_IN BIT(0)
+#define TPS65224_BIT_NRSTOUT_IN BIT(1)
+#define TPS65224_BIT_FORCE_EN_DRV_LOW BIT(3)
+#define TPS65224_BIT_TSD_DISABLE BIT(5)
+
+/* RECOV_CNT_REG_1 register field definition */
+#define TPS65224_MASK_RECOV_CNT GENMASK(3, 0)
+
+/* RECOV_CNT_REG_2 register field definition */
+#define TPS65224_MASK_RECOV_CNT_THR GENMASK(3, 0)
+#define TPS65224_BIT_RECOV_CNT_CLR BIT(4)
+
+/* FSM_I2C_TRIGGERS register field definition */
+#define TPS65224_BIT_TRIGGER_I2C(bit) BIT(bit)
+
+/* FSM_NSLEEP_TRIGGERS register field definition */
+#define TPS65224_BIT_NSLEEP1B BIT(0)
+#define TPS65224_BIT_NSLEEP2B BIT(1)
+
+/* BUCK_RESET_REG register field definition */
+#define TPS65224_BIT_BUCKX_RESET(buck_inst) BIT(buck_inst)
+
+/* SPREAD_SPECTRUM_1 register field definition */
+#define TPS65224_BIT_SS_DEPTH BIT(0)
+#define TPS65224_BIT_SS_EN BIT(2)
+
+/* FSM_STEP_SIZE register field definition */
+#define TPS65224_MASK_PFSM_DELAY_STEP GENMASK(4, 0)
+
+/* USER_SPARE_REGS register field definition */
+#define TPS65224_BIT_USER_SPARE(bit) BIT(bit)
+
+/* ESM_MCU_START_REG register field definition */
+#define TPS65224_BIT_ESM_MCU_START BIT(0)
+
+/* ESM_MCU_MODE_CFG register field definition */
+#define TPS65224_MASK_ESM_MCU_ERR_CNT_TH GENMASK(3, 0)
+#define TPS65224_BIT_ESM_MCU_ENDRV BIT(5)
+#define TPS65224_BIT_ESM_MCU_EN BIT(6)
+#define TPS65224_BIT_ESM_MCU_MODE BIT(7)
+
+/* ESM_MCU_ERR_CNT_REG register field definition */
+#define TPS65224_MASK_ESM_MCU_ERR_CNT GENMASK(4, 0)
+
+/* REGISTER_LOCK register field definition */
+#define TPS65224_BIT_REGISTER_LOCK_STATUS BIT(0)
+
+/* SRAM_ACCESS_1 Register field definition */
+#define TPS65224_MASk_SRAM_UNLOCK_SEQ GENMASK(7, 0)
+
+/* SRAM_ACCESS_2 Register field definition */
+#define TPS65224_BIT_SRAM_WRITE_MODE BIT(0)
+#define TPS65224_BIT_OTP_PROG_USER BIT(1)
+#define TPS65224_BIT_OTP_PROG_PFSM BIT(2)
+#define TPS65224_BIT_OTP_PROG_STATUS BIT(3)
+#define TPS65224_BIT_SRAM_UNLOCKED BIT(6)
+#define TPS65224_USER_PROG_ALLOWED BIT(7)
+
+/* SRAM_ADDR_CTRL Register field definition */
+#define TPS65224_MASk_SRAM_SEL GENMASK(1, 0)
+
+/* RECOV_CNT_PFSM_INCR Register field definition */
+#define TPS65224_BIT_INCREMENT_RECOV_CNT BIT(0)
+
+/* MANUFACTURING_VER Register field definition */
+#define TPS65224_MASk_SILICON_REV GENMASK(7, 0)
+
+/* CUSTOMER_NVM_ID_REG Register field definition */
+#define TPS65224_MASk_CUSTOMER_NVM_ID GENMASK(7, 0)
+
+/* SOFT_REBOOT_REG register field definition */
+#define TPS65224_BIT_SOFT_REBOOT BIT(0)
+
+/* ADC_CTRL Register field definition */
+#define TPS65224_BIT_ADC_START BIT(0)
+#define TPS65224_BIT_ADC_CONT_CONV BIT(1)
+#define TPS65224_BIT_ADC_THERMAL_SEL BIT(2)
+#define TPS65224_BIT_ADC_RDIV_EN BIT(3)
+#define TPS65224_BIT_ADC_STATUS BIT(7)
+
+/* ADC_RESULT_REG_1 Register field definition */
+#define TPS65224_MASk_ADC_RESULT_11_4 GENMASK(7, 0)
+
+/* ADC_RESULT_REG_2 Register field definition */
+#define TPS65224_MASk_ADC_RESULT_3_0 GENMASK(7, 4)
+
+/* STARTUP_CTRL Register field definition */
+#define TPS65224_MASk_STARTUP_DEST GENMASK(6, 5)
+#define TPS65224_BIT_FIRST_STARTUP_DONE BIT(7)
+
+/* SCRATCH_PAD_REG_1 Register field definition */
+#define TPS65224_MASk_SCRATCH_PAD_1 GENMASK(7, 0)
+
+/* SCRATCH_PAD_REG_2 Register field definition */
+#define TPS65224_MASk_SCRATCH_PAD_2 GENMASK(7, 0)
+
+/* SCRATCH_PAD_REG_3 Register field definition */
+#define TPS65224_MASk_SCRATCH_PAD_3 GENMASK(7, 0)
+
+/* SCRATCH_PAD_REG_4 Register field definition */
+#define TPS65224_MASk_SCRATCH_PAD_4 GENMASK(7, 0)
+
+/* PFSM_DELAY_REG_1 Register field definition */
+#define TPS65224_MASk_PFSM_DELAY1 GENMASK(7, 0)
+
+/* PFSM_DELAY_REG_2 Register field definition */
+#define TPS65224_MASk_PFSM_DELAY2 GENMASK(7, 0)
+
+/* PFSM_DELAY_REG_3 Register field definition */
+#define TPS65224_MASk_PFSM_DELAY3 GENMASK(7, 0)
+
+/* CRC_CALC_CONTROL Register field definition */
+#define TPS65224_BIT_RUN_CRC_BIST BIT(0)
+#define TPS65224_BIT_RUN_CRC_UPDATE BIT(1)
+
+/* ADC_GAIN_COMP_REG Register field definition */
+#define TPS65224_MASk_ADC_GAIN_COMP GENMASK(7, 0)
+
+/* REGMAP_USER_CRC_LOW Register field definition */
+#define TPS65224_MASk_REGMAP_USER_CRC16_LOW GENMASK(7, 0)
+
+/* REGMAP_USER_CRC_HIGH Register field definition */
+#define TPS65224_MASk_REGMAP_USER_CRC16_HIGH GENMASK(7, 0)
+
+/* WD_ANSWER_REG Register field definition */
+#define TPS65224_MASk_WD_ANSWER GENMASK(7, 0)
+
+/* WD_QUESTION_ANSW_CNT register field definition */
+#define TPS65224_MASK_WD_QUESTION GENMASK(3, 0)
+#define TPS65224_MASK_WD_ANSW_CNT GENMASK(5, 4)
+#define TPS65224_BIT_INT_TOP_STATUS BIT(7)
+
+/* WD_MODE_REG register field definition */
+#define TPS65224_BIT_WD_RETURN_LONGWIN BIT(0)
+#define TPS65224_BIT_WD_MODE_SELECT BIT(1)
+#define TPS65224_BIT_WD_PWRHOLD BIT(2)
+#define TPS65224_BIT_WD_ENDRV_SEL BIT(6)
+#define TPS65224_BIT_WD_CNT_SEL BIT(7)
+
+/* WD_QA_CFG register field definition */
+#define TPS65224_MASK_WD_QUESTION_SEED GENMASK(3, 0)
+#define TPS65224_MASK_WD_QA_LFSR GENMASK(5, 4)
+#define TPS65224_MASK_WD_QA_FDBK GENMASK(7, 6)
+
+/* WD_ERR_STATUS register field definition */
+#define TPS65224_BIT_WD_LONGWIN_TIMEOUT_INT BIT(0)
+#define TPS65224_BIT_WD_TIMEOUT BIT(1)
+#define TPS65224_BIT_WD_TRIG_EARLY BIT(2)
+#define TPS65224_BIT_WD_ANSW_EARLY BIT(3)
+#define TPS65224_BIT_WD_SEQ_ERR BIT(4)
+#define TPS65224_BIT_WD_ANSW_ERR BIT(5)
+#define TPS65224_BIT_WD_FAIL_INT BIT(6)
+#define TPS65224_BIT_WD_RST_INT BIT(7)
+
+/* WD_THR_CFG register field definition */
+#define TPS65224_MASK_WD_RST_TH GENMASK(2, 0)
+#define TPS65224_MASK_WD_FAIL_TH GENMASK(5, 3)
+#define TPS65224_BIT_WD_EN BIT(6)
+#define TPS65224_BIT_WD_RST_EN BIT(7)
+
+/* WD_FAIL_CNT_REG register field definition */
+#define TPS65224_MASK_WD_FAIL_CNT GENMASK(3, 0)
+#define TPS65224_BIT_WD_FIRST_OK BIT(5)
+#define TPS65224_BIT_WD_BAD_EVENT BIT(6)
+
+/* CRC8 polynomial for I2C & SPI protocols */
+#define TPS65224_CRC8_POLYNOMIAL 0x07
+
+/* IRQs */
+enum tps65224_irqs {
+ /* INT_BUCK register */
+ TPS65224_IRQ_BUCK1_UVOV,
+ TPS65224_IRQ_BUCK2_UVOV,
+ TPS65224_IRQ_BUCK3_UVOV,
+ TPS65224_IRQ_BUCK4_UVOV,
+ /* INT_LDO_VMON register */
+ TPS65224_IRQ_LDO1_UVOV,
+ TPS65224_IRQ_LDO2_UVOV,
+ TPS65224_IRQ_LDO3_UVOV,
+ TPS65224_IRQ_VCCA_UVOV,
+ TPS65224_IRQ_VMON1_UVOV,
+ TPS65224_IRQ_VMON2_UVOV,
+ /* INT_GPIO register */
+ TPS65224_IRQ_GPIO1,
+ TPS65224_IRQ_GPIO2,
+ TPS65224_IRQ_GPIO3,
+ TPS65224_IRQ_GPIO4,
+ TPS65224_IRQ_GPIO5,
+ TPS65224_IRQ_GPIO6,
+ /* INT_STARTUP register */
+ TPS65224_IRQ_VSENSE,
+ TPS65224_IRQ_ENABLE,
+ TPS65224_IRQ_PB_SHORT,
+ TPS65224_IRQ_FSD,
+ TPS65224_IRQ_SOFT_REBOOT,
+ /* INT_MISC register */
+ TPS65224_IRQ_BIST_PASS,
+ TPS65224_IRQ_EXT_CLK,
+ TPS65224_IRQ_REG_UNLOCK,
+ TPS65224_IRQ_TWARN,
+ TPS65224_IRQ_PB_LONG,
+ TPS65224_IRQ_PB_FALL,
+ TPS65224_IRQ_PB_RISE,
+ TPS65224_IRQ_ADC_CONV_READY,
+ /* INT_MODERATE_ERR register */
+ TPS65224_IRQ_TSD_ORD,
+ TPS65224_IRQ_BIST_FAIL,
+ TPS65224_IRQ_REG_CRC_ERR,
+ TPS65224_IRQ_RECOV_CNT,
+ /* INT_SEVERE_ERR register */
+ TPS65224_IRQ_TSD_IMM,
+ TPS65224_IRQ_VCCA_OVP,
+ TPS65224_IRQ_PFSM_ERR,
+ TPS65224_IRQ_BG_XMON,
+ /* INT_FSM_ERR register */
+ TPS65224_IRQ_IMM_SHUTDOWN,
+ TPS65224_IRQ_ORD_SHUTDOWN,
+ TPS65224_IRQ_MCU_PWR_ERR,
+ TPS65224_IRQ_SOC_PWR_ERR,
+ TPS65224_IRQ_COMM_ERR,
+ TPS65224_IRQ_I2C2_ERR,
+ /* INT_ESM register */
+ TPS65224_IRQ_ESM_MCU_PIN,
+ TPS65224_IRQ_ESM_MCU_FAIL,
+ TPS65224_IRQ_ESM_MCU_RST,
+};
+
+#define TPS65224_IRQ_NAME_BUCK1_UVOV "buck1_uvov"
+#define TPS65224_IRQ_NAME_BUCK2_UVOV "buck2_uvov"
+#define TPS65224_IRQ_NAME_BUCK3_UVOV "buck3_uvov"
+#define TPS65224_IRQ_NAME_BUCK4_UVOV "buck4_uvov"
+#define TPS65224_IRQ_NAME_LDO1_UVOV "ldo1_uvov"
+#define TPS65224_IRQ_NAME_LDO2_UVOV "ldo2_uvov"
+#define TPS65224_IRQ_NAME_LDO3_UVOV "ldo3_uvov"
+#define TPS65224_IRQ_NAME_VCCA_UVOV "vcca_uvov"
+#define TPS65224_IRQ_NAME_VMON1_UVOV "vmon1_uvov"
+#define TPS65224_IRQ_NAME_VMON2_UVOV "vmon2_uvov"
+#define TPS65224_IRQ_NAME_GPIO1 "gpio1"
+#define TPS65224_IRQ_NAME_GPIO2 "gpio2"
+#define TPS65224_IRQ_NAME_GPIO3 "gpio3"
+#define TPS65224_IRQ_NAME_GPIO4 "gpio4"
+#define TPS65224_IRQ_NAME_GPIO5 "gpio5"
+#define TPS65224_IRQ_NAME_GPIO6 "gpio6"
+#define TPS65224_IRQ_NAME_VSENSE "vsense"
+#define TPS65224_IRQ_NAME_ENABLE "enable"
+#define TPS65224_IRQ_NAME_PB_SHORT "pb_short"
+#define TPS65224_IRQ_NAME_FSD "fsd"
+#define TPS65224_IRQ_NAME_SOFT_REBOOT "soft_reboot"
+#define TPS65224_IRQ_NAME_BIST_PASS "bist_pass"
+#define TPS65224_IRQ_NAME_EXT_CLK "ext_clk"
+#define TPS65224_IRQ_NAME_REG_UNLOCK "reg_unlock"
+#define TPS65224_IRQ_NAME_TWARN "twarn"
+#define TPS65224_IRQ_NAME_PB_LONG "pb_long"
+#define TPS65224_IRQ_NAME_PB_FALL "pb_fall"
+#define TPS65224_IRQ_NAME_PB_RISE "pb_rise"
+#define TPS65224_IRQ_NAME_ADC_CONV_READY "adc_conv_ready"
+#define TPS65224_IRQ_NAME_TSD_ORD "tsd_ord"
+#define TPS65224_IRQ_NAME_BIST_FAIL "bist_fail"
+#define TPS65224_IRQ_NAME_REG_CRC_ERR "reg_crc_err"
+#define TPS65224_IRQ_NAME_RECOV_CNT "recov_cnt"
+#define TPS65224_IRQ_NAME_TSD_IMM "tsd_imm"
+#define TPS65224_IRQ_NAME_VCCA_OVP "vcca_ovp"
+#define TPS65224_IRQ_NAME_PFSM_ERR "pfsm_err"
+#define TPS65224_IRQ_NAME_BG_XMON "bg_xmon"
+#define TPS65224_IRQ_NAME_IMM_SHUTDOWN "imm_shutdown"
+#define TPS65224_IRQ_NAME_ORD_SHUTDOWN "ord_shutdown"
+#define TPS65224_IRQ_NAME_MCU_PWR_ERR "mcu_pwr_err"
+#define TPS65224_IRQ_NAME_SOC_PWR_ERR "soc_pwr_err"
+#define TPS65224_IRQ_NAME_COMM_ERR "comm_err"
+#define TPS65224_IRQ_NAME_I2C2_ERR "i2c2_err"
+#define TPS65224_IRQ_NAME_ESM_MCU_PIN "esm_mcu_pin"
+#define TPS65224_IRQ_NAME_ESM_MCU_FAIL "esm_mcu_fail"
+#define TPS65224_IRQ_NAME_ESM_MCU_RST "esm_mcu_rst"
+#define TPS65224_IRQ_NAME_POWERUP "powerup"
+
+/**
+ * struct tps65224 - device private data structure
+ *
+ * @dev: MFD parent device
+ * @chip_id: chip ID
+ * @reg: I2C slave address or SPI chip select number
+ * @use_crc: if true, use CRC for I2C and SPI interface protocols
+ * @regmap: regmap for accessing the device registers
+ * @irq: irq generated by the device
+ * @irq_data: regmap irq data used for the irq chip
+ */
+struct tps65224 {
+ struct device *dev;
+ unsigned long chip_id;
+ unsigned short reg;
+ bool use_crc;
+ struct regmap *regmap;
+ int irq;
+ struct regmap_irq_chip_data *irq_data;
+};
+
+bool tps65224_is_volatile_reg(struct device *dev, unsigned int reg);
+int tps65224_device_init(struct tps65224 *tps, bool enable_crc);
+
+#endif /* __LINUX_MFD_TPS65224_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v1 2/3] drivers: mfd: Add support for TPS65224 i2c driver
2023-10-26 13:32 [PATCH v1 0/3] TPS65224 PMIC driver Gairuboina Sirisha
2023-10-26 13:32 ` [PATCH v1 1/3] drivers: mfd: Add support for TPS65224 Gairuboina Sirisha
@ 2023-10-26 13:32 ` Gairuboina Sirisha
2023-10-27 7:02 ` Greg KH
2023-10-27 8:08 ` Krzysztof Kozlowski
2023-10-26 13:32 ` [PATCH v1 3/3] drivers: misc: Add support for TPS65224 pfsm driver Gairuboina Sirisha
2023-11-03 8:52 ` [PATCH v1 0/3] TPS65224 PMIC driver Julien Panis
3 siblings, 2 replies; 21+ messages in thread
From: Gairuboina Sirisha @ 2023-10-26 13:32 UTC (permalink / raw)
To: linux-kernel; +Cc: lee, arnd, gregkh, Gairuboina Sirisha
From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
Added MFD driver that enables I2C communication with and without CRC
Signed-off-by: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
---
drivers/mfd/Kconfig | 14 +++
drivers/mfd/Makefile | 1 +
drivers/mfd/tps65224-i2c.c | 245 +++++++++++++++++++++++++++++++++++++
3 files changed, 260 insertions(+)
create mode 100644 drivers/mfd/tps65224-i2c.c
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 2e4906484eed..943d85d49fc5 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1767,12 +1767,26 @@ config MFD_TPS6594_SPI
This driver can also be built as a module. If so, the module
will be called tps6594-spi.
+
config MFD_TPS65224
tristate
select MFD_CORE
select REGMAP
select REGMAP_IRQ
+config MFD_TPS65224_I2C
+ tristate "TI TPS65224 Power Management chip with I2C"
+ select MFD_TPS65224
+ select REGMAP_I2C
+ select CRC8
+ depends on I2C
+ help
+ If you say yes here you get support for the TPS65224 series of
+ PM chips with I2C interface.
+
+ This driver can also be built as a module. If so, the module
+ will be called tps65224-i2c.
+
config TWL4030_CORE
bool "TI TWL4030/TWL5030/TWL6030/TPS659x0 Support"
depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index ff4e158fa4db..4963fecd3e93 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -103,6 +103,7 @@ obj-$(CONFIG_MFD_TPS6594) += tps6594-core.o
obj-$(CONFIG_MFD_TPS6594_I2C) += tps6594-i2c.o
obj-$(CONFIG_MFD_TPS6594_SPI) += tps6594-spi.o
obj-$(CONFIG_MFD_TPS65224) += tps65224-core.o
+obj-$(CONFIG_MFD_TPS65224) += tps65224-i2c.o
obj-$(CONFIG_MENELAUS) += menelaus.o
obj-$(CONFIG_TWL4030_CORE) += twl-core.o twl4030-irq.o twl6030-irq.o
diff --git a/drivers/mfd/tps65224-i2c.c b/drivers/mfd/tps65224-i2c.c
new file mode 100644
index 000000000000..c6300138ce4c
--- /dev/null
+++ b/drivers/mfd/tps65224-i2c.c
@@ -0,0 +1,245 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * I2C access driver for TI TPS65224 PMIC
+ *
+ * Copyright (C) 2015 Texas Instruments Incorporated - https://www.ti.com/
+ *
+ * Based on the TPS6594 I2C Interface Driver by
+ * Julien Panis <jpanis@baylibre.com>
+ */
+
+#include <linux/crc8.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/tps65224.h>
+
+static bool enable_crc;
+module_param(enable_crc, bool, 0444);
+MODULE_PARM_DESC(enable_crc, "Enable CRC feature for I2C interface");
+
+DECLARE_CRC8_TABLE(tps65224_i2c_crc_table);
+
+static int tps65224_i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+ int ret = i2c_transfer(adap, msgs, num);
+
+ if (ret == num)
+ return 0;
+ else if (ret < 0)
+ return ret;
+ else
+ return -EIO;
+}
+
+static int tps65224_i2c_reg_read_with_crc(struct i2c_client *client, u8 page, u8 reg, u8 *val)
+{
+ struct i2c_msg msgs[2];
+ u8 buf_rx[] = { 0, 0 };
+ /* I2C address = I2C base address + Page index */
+ const u8 addr = client->addr + page;
+ /*
+ * CRC is calculated from every bit included in the protocol
+ * except the ACK bits from the target. Byte stream is:
+ * - B0: (I2C_addr_7bits << 1) | WR_bit, with WR_bit = 0
+ * - B1: reg
+ * - B2: (I2C_addr_7bits << 1) | RD_bit, with RD_bit = 1
+ * - B3: val
+ * - B4: CRC from B0-B1-B2-B3
+ */
+ u8 crc_data[] = { addr << 1, reg, addr << 1 | 1, 0 };
+ int ret;
+
+ /* Write register */
+ msgs[0].addr = addr;
+ msgs[0].flags = 0;
+ msgs[0].len = 1;
+ msgs[0].buf = ®
+
+ /* Read data and CRC */
+ msgs[1].addr = msgs[0].addr;
+ msgs[1].flags = I2C_M_RD;
+ msgs[1].len = 2;
+ msgs[1].buf = buf_rx;
+
+ ret = tps65224_i2c_transfer(client->adapter, msgs, 2);
+ if (ret < 0)
+ return ret;
+
+ crc_data[sizeof(crc_data) - 1] = *val = buf_rx[0];
+ if (buf_rx[1] != crc8(tps65224_i2c_crc_table, crc_data, sizeof(crc_data), CRC8_INIT_VALUE))
+ return -EIO;
+
+ return ret;
+}
+
+static int tps65224_i2c_reg_write_with_crc(struct i2c_client *client, u8 page, u8 reg, u8 val)
+{
+ struct i2c_msg msg;
+ u8 buf[] = { reg, val, 0 };
+ /* I2C address = I2C base address + Page index */
+ const u8 addr = client->addr + page;
+ /*
+ * CRC is calculated from every bit included in the protocol
+ * except the ACK bits from the target. Byte stream is:
+ * - B0: (I2C_addr_7bits << 1) | WR_bit, with WR_bit = 0
+ * - B1: reg
+ * - B2: val
+ * - B3: CRC from B0-B1-B2
+ */
+ const u8 crc_data[] = { addr << 1, reg, val };
+
+ /* Write register, data and CRC */
+ msg.addr = addr;
+ msg.flags = client->flags & I2C_M_TEN;
+ msg.len = sizeof(buf);
+ msg.buf = buf;
+
+ buf[msg.len - 1] = crc8(tps65224_i2c_crc_table, crc_data,
+ sizeof(crc_data), CRC8_INIT_VALUE);
+
+ return tps65224_i2c_transfer(client->adapter, &msg, 1);
+}
+
+static int tps65224_i2c_read(void *context, const void *reg_buf, size_t reg_size,
+ void *val_buf, size_t val_size)
+{
+ struct i2c_client *client = context;
+ struct tps65224 *tps = i2c_get_clientdata(client);
+ struct i2c_msg msgs[2];
+ const u8 *reg_bytes = reg_buf;
+ u8 *val_bytes = val_buf;
+ const u8 page = reg_bytes[1];
+ u8 reg = reg_bytes[0];
+ int ret = 0;
+ int i;
+
+ if (tps->use_crc) {
+ /*
+ * Auto-increment feature does not support CRC protocol.
+ * Converts the bulk read operation into a series of single read operations.
+ */
+ for (i = 0 ; ret == 0 && i < val_size ; i++)
+ ret = tps65224_i2c_reg_read_with_crc(client, page, reg + i, val_bytes + i);
+
+ return ret;
+ }
+
+ /* Write register: I2C address = I2C base address + Page index */
+ msgs[0].addr = client->addr + page;
+ msgs[0].flags = 0;
+ msgs[0].len = 1;
+ msgs[0].buf = ®
+
+ /* Read data */
+ msgs[1].addr = msgs[0].addr;
+ msgs[1].flags = I2C_M_RD;
+ msgs[1].len = val_size;
+ msgs[1].buf = val_bytes;
+
+ return tps65224_i2c_transfer(client->adapter, msgs, 2);
+}
+
+static int tps65224_i2c_write(void *context, const void *data, size_t count)
+{
+ struct i2c_client *client = context;
+ struct tps65224 *tps = i2c_get_clientdata(client);
+ struct i2c_msg msg;
+ const u8 *bytes = data;
+ u8 *buf;
+ const u8 page = bytes[1];
+ const u8 reg = bytes[0];
+ int ret = 0;
+ int i;
+
+ if (tps->use_crc) {
+ /*
+ * Auto-increment feature does not support CRC protocol.
+ * Converts the bulk write operation into a series of single write operations.
+ */
+ for (i = 0 ; ret == 0 && i < count - 2 ; i++)
+ ret = tps65224_i2c_reg_write_with_crc(client, page, reg + i, bytes[i + 2]);
+
+ return ret;
+ }
+
+ /* Setup buffer: page byte is not sent */
+ buf = kzalloc(--count, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ buf[0] = reg;
+ for (i = 0 ; i < count - 1 ; i++)
+ buf[i + 1] = bytes[i + 2];
+
+ /* Write register and data: I2C address = I2C base address + Page index */
+ msg.addr = client->addr + page;
+ msg.flags = client->flags & I2C_M_TEN;
+ msg.len = count;
+ msg.buf = buf;
+
+ ret = tps65224_i2c_transfer(client->adapter, &msg, 1);
+
+ kfree(buf);
+ return ret;
+}
+
+static const struct regmap_config tps65224_i2c_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 8,
+ .max_register = TPS65224_REG_WD_FAIL_CNT_REG,
+ .volatile_reg = tps65224_is_volatile_reg,
+ .read = tps65224_i2c_read,
+ .write = tps65224_i2c_write,
+};
+
+static const struct of_device_id tps65224_i2c_of_match_table[] = {
+ { .compatible = "ti,tps65224-q1", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, tps65224_i2c_of_match_table);
+
+static int tps65224_i2c_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct tps65224 *tps;
+ const struct of_device_id *match;
+
+ tps = devm_kzalloc(dev, sizeof(*tps), GFP_KERNEL);
+ if (!tps)
+ return -ENOMEM;
+
+ i2c_set_clientdata(client, tps);
+
+ tps->dev = dev;
+ tps->reg = client->addr;
+ tps->irq = client->irq;
+
+ tps->regmap = devm_regmap_init(dev, NULL, client, &tps65224_i2c_regmap_config);
+ if (IS_ERR(tps->regmap))
+ return dev_err_probe(dev, PTR_ERR(tps->regmap), "Failed to init regmap\n");
+
+ match = of_match_device(tps65224_i2c_of_match_table, dev);
+ if (!match)
+ return dev_err_probe(dev, -EINVAL, "Failed to find matching chip ID\n");
+
+ crc8_populate_msb(tps65224_i2c_crc_table, TPS65224_CRC8_POLYNOMIAL);
+
+ return tps65224_device_init(tps, enable_crc);
+}
+
+static struct i2c_driver tps65224_i2c_driver = {
+ .driver = {
+ .name = "tps65224",
+ .of_match_table = tps65224_i2c_of_match_table,
+ },
+ .probe = tps65224_i2c_probe,
+};
+module_i2c_driver(tps65224_i2c_driver);
+
+MODULE_AUTHOR("Gairuboina Sirisha <sirisha.gairuboina@ltts.com");
+MODULE_DESCRIPTION("TPS65224 I2C Interface Driver");
+MODULE_LICENSE("GPL");
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v1 3/3] drivers: misc: Add support for TPS65224 pfsm driver
2023-10-26 13:32 [PATCH v1 0/3] TPS65224 PMIC driver Gairuboina Sirisha
2023-10-26 13:32 ` [PATCH v1 1/3] drivers: mfd: Add support for TPS65224 Gairuboina Sirisha
2023-10-26 13:32 ` [PATCH v1 2/3] drivers: mfd: Add support for TPS65224 i2c driver Gairuboina Sirisha
@ 2023-10-26 13:32 ` Gairuboina Sirisha
2023-10-27 7:05 ` Greg KH
2023-10-27 8:05 ` Krzysztof Kozlowski
2023-11-03 8:52 ` [PATCH v1 0/3] TPS65224 PMIC driver Julien Panis
3 siblings, 2 replies; 21+ messages in thread
From: Gairuboina Sirisha @ 2023-10-26 13:32 UTC (permalink / raw)
To: linux-kernel; +Cc: lee, arnd, gregkh, Gairuboina Sirisha
From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
Added support for pmic nvm set FSM_I2C_TRIGGER functions driver,
state control driver, Makefile and Kconfig
Signed-off-by: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
---
drivers/misc/Kconfig | 12 ++
drivers/misc/Makefile | 1 +
drivers/misc/tps65224-pfsm.c | 290 +++++++++++++++++++++++++++++
include/uapi/linux/tps65224_pfsm.h | 36 ++++
4 files changed, 339 insertions(+)
create mode 100644 drivers/misc/tps65224-pfsm.c
create mode 100644 include/uapi/linux/tps65224_pfsm.h
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index cadd4a820c03..6d12404b0fa6 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -562,6 +562,18 @@ config TPS6594_PFSM
This driver can also be built as a module. If so, the module
will be called tps6594-pfsm.
+config TPS65224_PFSM
+ tristate "TI TPS65224 Pre-configurable Finite State Machine support"
+ depends on MFD_TPS65224
+ default MFD_TPS65224
+ help
+ Support PFSM (Pre-configurable Finite State Machine) on TPS65224 PMIC devices.
+ These devices integrate a finite state machine engine, which manages the state
+ of the device during operating state transition.
+
+ This driver can also be built as a module. If so, the module
+ will be called tps65224-pfsm.
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index f2a4d1ff65d4..49dcacc19529 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -67,3 +67,4 @@ obj-$(CONFIG_TMR_MANAGER) += xilinx_tmr_manager.o
obj-$(CONFIG_TMR_INJECT) += xilinx_tmr_inject.o
obj-$(CONFIG_TPS6594_ESM) += tps6594-esm.o
obj-$(CONFIG_TPS6594_PFSM) += tps6594-pfsm.o
+obj-$(CONFIG_TPS65224_PFSM) += tps65224-pfsm.o
diff --git a/drivers/misc/tps65224-pfsm.c b/drivers/misc/tps65224-pfsm.c
new file mode 100644
index 000000000000..734cfa8c8393
--- /dev/null
+++ b/drivers/misc/tps65224-pfsm.c
@@ -0,0 +1,290 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PFSM (Pre-configurable Finite State Machine) driver for TI TPS65224 PMIC
+ *
+ * Copyright (C) 2015 Texas Instruments Incorporated - https://www.ti.com/
+ *
+ * Based on the TPS6594 Pre-configurable Finite State Machine Driver by
+ * Julien Panis <jpanis@baylibre.com>
+ */
+
+#include <linux/errno.h>
+#include <linux/fs.h>
+#include <linux/interrupt.h>
+#include <linux/ioctl.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/tps65224.h>
+
+#include <linux/tps65224_pfsm.h>
+
+#define TPS65224_STARTUP_DEST_MCU_ONLY_VAL 2
+#define TPS65224_STARTUP_DEST_ACTIVE_VAL 3
+#define TPS65224_STARTUP_DEST_SHIFT 5
+#define TPS65224_STARTUP_DEST_MCU_ONLY (TPS65224_STARTUP_DEST_MCU_ONLY_VAL \
+ << TPS65224_STARTUP_DEST_SHIFT)
+#define TPS65224_STARTUP_DEST_ACTIVE (TPS65224_STARTUP_DEST_ACTIVE_VAL \
+ << TPS65224_STARTUP_DEST_SHIFT)
+
+/*
+ * To update the PMIC firmware, the user must be able to access
+ * page 0 (user registers) and page 1 (NVM control and configuration).
+ */
+#define TPS65224_PMIC_MAX_POS 0x200
+
+#define TPS65224_FILE_TO_PFSM(f) container_of((f)->private_data, struct tps65224_pfsm, miscdev)
+
+/**
+ * struct tps65224_pfsm - device private data structure
+ *
+ * @miscdev: misc device infos
+ * @regmap: regmap for accessing the device registers
+ */
+struct tps65224_pfsm {
+ struct miscdevice miscdev;
+ struct regmap *regmap;
+};
+
+static ssize_t tps65224_pfsm_read(struct file *f, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct tps65224_pfsm *pfsm = TPS65224_FILE_TO_PFSM(f);
+ loff_t pos = *ppos;
+ unsigned int val;
+ int ret;
+ int i;
+
+ if (pos < 0)
+ return -EINVAL;
+ if (pos >= TPS65224_PMIC_MAX_POS)
+ return 0;
+ if (count > TPS65224_PMIC_MAX_POS - pos)
+ count = TPS65224_PMIC_MAX_POS - pos;
+
+ for (i = 0 ; i < count ; i++) {
+ ret = regmap_read(pfsm->regmap, pos + i, &val);
+ if (ret)
+ return ret;
+
+ if (put_user(val, buf + i))
+ return -EFAULT;
+ }
+
+ *ppos = pos + count;
+
+ return count;
+}
+
+static ssize_t tps65224_pfsm_write(struct file *f, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct tps65224_pfsm *pfsm = TPS65224_FILE_TO_PFSM(f);
+ loff_t pos = *ppos;
+ char val;
+ int ret;
+ int i;
+
+ if (pos < 0)
+ return -EINVAL;
+ if (pos >= TPS65224_PMIC_MAX_POS || !count)
+ return 0;
+ if (count > TPS65224_PMIC_MAX_POS - pos)
+ count = TPS65224_PMIC_MAX_POS - pos;
+
+ for (i = 0 ; i < count ; i++) {
+ if (get_user(val, buf + i))
+ return -EFAULT;
+
+ ret = regmap_write(pfsm->regmap, pos + i, val);
+ if (ret)
+ return ret;
+ }
+
+ *ppos = pos + count;
+
+ return count;
+}
+
+static int tps65224_pfsm_configure_ret_trig(struct regmap *regmap, u8 gpio_ret, u8 ddr_ret)
+{
+ int ret;
+
+ if (gpio_ret)
+ ret = regmap_set_bits(regmap, TPS65224_REG_FSM_I2C_TRIGGERS,
+ TPS65224_BIT_TRIGGER_I2C(5) | TPS65224_BIT_TRIGGER_I2C(6));
+ else
+ ret = regmap_clear_bits(regmap, TPS65224_REG_FSM_I2C_TRIGGERS,
+ TPS65224_BIT_TRIGGER_I2C(5) | TPS65224_BIT_TRIGGER_I2C(6));
+ if (ret)
+ return ret;
+
+ if (ddr_ret)
+ ret = regmap_set_bits(regmap, TPS65224_REG_FSM_I2C_TRIGGERS,
+ TPS65224_BIT_TRIGGER_I2C(7));
+ else
+ ret = regmap_clear_bits(regmap, TPS65224_REG_FSM_I2C_TRIGGERS,
+ TPS65224_BIT_TRIGGER_I2C(7));
+
+ return ret;
+}
+
+static long tps65224_pfsm_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
+{
+ struct tps65224_pfsm *pfsm = TPS65224_FILE_TO_PFSM(f);
+ struct pmic_state_opt state_opt;
+ void __user *argp = (void __user *)arg;
+ int ret = -ENOIOCTLCMD;
+
+ switch (cmd) {
+ case PMIC_GOTO_STANDBY:
+ /* Force trigger */
+ ret = regmap_write_bits(pfsm->regmap, TPS65224_REG_FSM_I2C_TRIGGERS,
+ TPS65224_BIT_TRIGGER_I2C(0), TPS65224_BIT_TRIGGER_I2C(0));
+ break;
+ case PMIC_UPDATE_PGM:
+ /* Force trigger */
+ ret = regmap_write_bits(pfsm->regmap, TPS65224_REG_FSM_I2C_TRIGGERS,
+ TPS65224_BIT_TRIGGER_I2C(3), TPS65224_BIT_TRIGGER_I2C(3));
+ break;
+ case PMIC_SET_ACTIVE_STATE:
+ /* Modify NSLEEP1-2 bits */
+ ret = regmap_set_bits(pfsm->regmap, TPS65224_REG_FSM_NSLEEP_TRIGGERS,
+ TPS65224_BIT_NSLEEP1B | TPS65224_BIT_NSLEEP2B);
+ break;
+ case PMIC_SET_MCU_ONLY_STATE:
+ if (copy_from_user(&state_opt, argp, sizeof(state_opt)))
+ return -EFAULT;
+
+ /* Configure retention triggers */
+ ret = tps65224_pfsm_configure_ret_trig(pfsm->regmap, state_opt.gpio_retention,
+ state_opt.ddr_retention);
+ if (ret)
+ return ret;
+
+ /* Modify NSLEEP1-2 bits */
+ ret = regmap_clear_bits(pfsm->regmap, TPS65224_REG_FSM_NSLEEP_TRIGGERS,
+ TPS65224_BIT_NSLEEP1B);
+ if (ret)
+ return ret;
+
+ ret = regmap_set_bits(pfsm->regmap, TPS65224_REG_FSM_NSLEEP_TRIGGERS,
+ TPS65224_BIT_NSLEEP2B);
+ break;
+ case PMIC_SET_RETENTION_STATE:
+ if (copy_from_user(&state_opt, argp, sizeof(state_opt)))
+ return -EFAULT;
+
+ /* Configure wake-up destination */
+ if (state_opt.mcu_only_startup_dest)
+ ret = regmap_write_bits(pfsm->regmap, TPS65224_REG_CONFIG_2,
+ TPS65224_BIT_STARTUP_INT,
+ TPS65224_STARTUP_DEST_MCU_ONLY);
+ else
+ ret = regmap_write_bits(pfsm->regmap, TPS65224_REG_CONFIG_2,
+ TPS65224_BIT_STARTUP_INT,
+ TPS65224_STARTUP_DEST_ACTIVE);
+ if (ret)
+ return ret;
+
+ /* Configure retention triggers */
+ ret = tps65224_pfsm_configure_ret_trig(pfsm->regmap, state_opt.gpio_retention,
+ state_opt.ddr_retention);
+ if (ret)
+ return ret;
+
+ /* Modify NSLEEP1-2 bits */
+ ret = regmap_clear_bits(pfsm->regmap, TPS65224_REG_FSM_NSLEEP_TRIGGERS,
+ TPS65224_BIT_NSLEEP2B);
+ break;
+ }
+
+ return ret;
+}
+
+static const struct file_operations tps65224_pfsm_fops = {
+ .owner = THIS_MODULE,
+ .llseek = generic_file_llseek,
+ .read = tps65224_pfsm_read,
+ .write = tps65224_pfsm_write,
+ .unlocked_ioctl = tps65224_pfsm_ioctl,
+ .compat_ioctl = compat_ptr_ioctl,
+};
+
+static irqreturn_t tps65224_pfsm_isr(int irq, void *dev_id)
+{
+ struct platform_device *pdev = dev_id;
+ int i;
+
+ for (i = 0 ; i < pdev->num_resources ; i++) {
+ if (irq == platform_get_irq_byname(pdev, pdev->resource[i].name)) {
+ dev_info(pdev->dev.parent, "%s event detected\n", pdev->resource[i].name);
+ return IRQ_HANDLED;
+ }
+ }
+
+ return IRQ_NONE;
+}
+
+static int tps65224_pfsm_probe(struct platform_device *pdev)
+{
+ struct tps65224_pfsm *pfsm;
+ struct tps65224 *tps = dev_get_drvdata(pdev->dev.parent);
+ struct device *dev = &pdev->dev;
+ int irq;
+ int ret;
+ int i;
+
+ pfsm = devm_kzalloc(dev, sizeof(struct tps65224_pfsm), GFP_KERNEL);
+ if (!pfsm)
+ return -ENOMEM;
+
+ pfsm->regmap = tps->regmap;
+
+ pfsm->miscdev.minor = MISC_DYNAMIC_MINOR;
+ pfsm->miscdev.name = devm_kasprintf(dev, GFP_KERNEL, "pfsm-%ld-0x%02x",
+ tps->chip_id, tps->reg);
+ pfsm->miscdev.fops = &tps65224_pfsm_fops;
+ pfsm->miscdev.parent = dev->parent;
+
+ for (i = 0 ; i < pdev->num_resources ; i++) {
+ irq = platform_get_irq_byname(pdev, pdev->resource[i].name);
+ if (irq < 0)
+ return dev_err_probe(dev, irq, "Failed to get %s irq\n",
+ pdev->resource[i].name);
+
+ ret = devm_request_threaded_irq(dev, irq, NULL,
+ tps65224_pfsm_isr, IRQF_ONESHOT,
+ pdev->resource[i].name, pdev);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to request irq\n");
+ }
+
+ platform_set_drvdata(pdev, pfsm);
+
+ return misc_register(&pfsm->miscdev);
+}
+
+static void tps65224_pfsm_remove(struct platform_device *pdev)
+{
+ struct tps65224_pfsm *pfsm = platform_get_drvdata(pdev);
+
+ misc_deregister(&pfsm->miscdev);
+}
+
+static struct platform_driver tps65224_pfsm_driver = {
+ .driver = {
+ .name = "tps65224-pfsm",
+ },
+ .probe = tps65224_pfsm_probe,
+ .remove_new = tps65224_pfsm_remove,
+};
+
+module_platform_driver(tps65224_pfsm_driver);
+
+MODULE_ALIAS("platform:tps65224-pfsm");
+MODULE_AUTHOR("Gairuboina Sirisha <sirisha.gairuboina@ltts.com>");
+MODULE_DESCRIPTION("TPS65224 Pre-configurable Finite State Machine Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/uapi/linux/tps65224_pfsm.h b/include/uapi/linux/tps65224_pfsm.h
new file mode 100644
index 000000000000..c0a135903b5d
--- /dev/null
+++ b/include/uapi/linux/tps65224_pfsm.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Userspace ABI for TPS65224 PMIC Pre-configurable Finite State Machine
+ *
+ * Copyright (C) 2015 Texas Instruments Incorporated - https://www.ti.com/
+ */
+
+#ifndef __TPS65224_PFSM_H
+#define __TPS65224_PFSM_H
+
+#include <linux/const.h>
+#include <linux/ioctl.h>
+#include <linux/types.h>
+
+/**
+ * struct pmic_state_opt - PMIC state options
+ * @gpio_retention: if enabled, power rails associated with GPIO retention remain active
+ * @ddr_retention: if enabled, power rails associated with DDR retention remain active
+ * @mcu_only_startup_dest: if enabled, startup destination state is MCU_ONLY
+ */
+struct pmic_state_opt {
+ __u8 gpio_retention;
+ __u8 ddr_retention;
+ __u8 mcu_only_startup_dest;
+};
+
+/* Commands */
+#define PMIC_BASE 'P'
+
+#define PMIC_GOTO_STANDBY _IO(PMIC_BASE, 0)
+#define PMIC_UPDATE_PGM _IO(PMIC_BASE, 1)
+#define PMIC_SET_ACTIVE_STATE _IO(PMIC_BASE, 2)
+#define PMIC_SET_MCU_ONLY_STATE _IOW(PMIC_BASE, 3, struct pmic_state_opt)
+#define PMIC_SET_RETENTION_STATE _IOW(PMIC_BASE, 4, struct pmic_state_opt)
+
+#endif /* __TPS65224_PFSM_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/3] drivers: mfd: Add support for TPS65224
2023-10-26 13:32 ` [PATCH v1 1/3] drivers: mfd: Add support for TPS65224 Gairuboina Sirisha
@ 2023-10-27 7:02 ` Greg KH
2023-11-07 11:40 ` Gairuboina Sirisha
0 siblings, 1 reply; 21+ messages in thread
From: Greg KH @ 2023-10-27 7:02 UTC (permalink / raw)
To: Gairuboina Sirisha; +Cc: linux-kernel, lee, arnd
On Thu, Oct 26, 2023 at 07:02:24PM +0530, Gairuboina Sirisha wrote:
> From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
>
> Added support for tps65224 driver pmic core, header, Makefile and Kconfig
What is a "tps65224"? Please provide some more information so we know
how to review this.
>
> Signed-off-by: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> ---
> drivers/mfd/Kconfig | 5 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/tps65224-core.c | 291 ++++++++++++++
> include/linux/mfd/tps65224.h | 735 +++++++++++++++++++++++++++++++++++
> 4 files changed, 1032 insertions(+)
> create mode 100644 drivers/mfd/tps65224-core.c
> create mode 100644 include/linux/mfd/tps65224.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 90ce58fd629e..2e4906484eed 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1767,6 +1767,11 @@ config MFD_TPS6594_SPI
>
> This driver can also be built as a module. If so, the module
> will be called tps6594-spi.
> +config MFD_TPS65224
You need a blank line before this, right?
> + tristate
> + select MFD_CORE
> + select REGMAP
> + select REGMAP_IRQ
No help text at all to describe what this is?
> --- /dev/null
> +++ b/drivers/mfd/tps65224-core.c
> @@ -0,0 +1,291 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Core functions for TI TPS65224 PMIC
> + *
> + * Copyright (C) 2015 Texas Instruments Incorporated - https://www.ti.com/
No changes have happened since 2015 to this code?
> +EXPORT_SYMBOL_GPL(tps65224_device_init);
> +
> +MODULE_AUTHOR("Gairuboina Sirisha <sirisha.gairuboina@ltts.com>");
That dosn't match the copyright line :(
> --- /dev/null
> +++ b/include/linux/mfd/tps65224.h
Why do you need a .h file at all? This is just used in one .c file,
right?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 2/3] drivers: mfd: Add support for TPS65224 i2c driver
2023-10-26 13:32 ` [PATCH v1 2/3] drivers: mfd: Add support for TPS65224 i2c driver Gairuboina Sirisha
@ 2023-10-27 7:02 ` Greg KH
2023-11-07 11:42 ` Gairuboina Sirisha
2023-10-27 8:08 ` Krzysztof Kozlowski
1 sibling, 1 reply; 21+ messages in thread
From: Greg KH @ 2023-10-27 7:02 UTC (permalink / raw)
To: Gairuboina Sirisha; +Cc: linux-kernel, lee, arnd
On Thu, Oct 26, 2023 at 07:02:25PM +0530, Gairuboina Sirisha wrote:
> From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
>
> Added MFD driver that enables I2C communication with and without CRC
>
> Signed-off-by: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> ---
> drivers/mfd/Kconfig | 14 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/tps65224-i2c.c | 245 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 260 insertions(+)
> create mode 100644 drivers/mfd/tps65224-i2c.c
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 2e4906484eed..943d85d49fc5 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1767,12 +1767,26 @@ config MFD_TPS6594_SPI
>
> This driver can also be built as a module. If so, the module
> will be called tps6594-spi.
> +
That line should have been added in the first patch.
> config MFD_TPS65224
> tristate
> select MFD_CORE
> select REGMAP
> select REGMAP_IRQ
>
> +config MFD_TPS65224_I2C
> + tristate "TI TPS65224 Power Management chip with I2C"
> + select MFD_TPS65224
> + select REGMAP_I2C
> + select CRC8
> + depends on I2C
> + help
> + If you say yes here you get support for the TPS65224 series of
> + PM chips with I2C interface.
> +
> + This driver can also be built as a module. If so, the module
> + will be called tps65224-i2c.
> +
> config TWL4030_CORE
> bool "TI TWL4030/TWL5030/TWL6030/TPS659x0 Support"
> depends on I2C=y
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index ff4e158fa4db..4963fecd3e93 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -103,6 +103,7 @@ obj-$(CONFIG_MFD_TPS6594) += tps6594-core.o
> obj-$(CONFIG_MFD_TPS6594_I2C) += tps6594-i2c.o
> obj-$(CONFIG_MFD_TPS6594_SPI) += tps6594-spi.o
> obj-$(CONFIG_MFD_TPS65224) += tps65224-core.o
> +obj-$(CONFIG_MFD_TPS65224) += tps65224-i2c.o
> obj-$(CONFIG_MENELAUS) += menelaus.o
>
> obj-$(CONFIG_TWL4030_CORE) += twl-core.o twl4030-irq.o twl6030-irq.o
> diff --git a/drivers/mfd/tps65224-i2c.c b/drivers/mfd/tps65224-i2c.c
> new file mode 100644
> index 000000000000..c6300138ce4c
> --- /dev/null
> +++ b/drivers/mfd/tps65224-i2c.c
> @@ -0,0 +1,245 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * I2C access driver for TI TPS65224 PMIC
> + *
> + * Copyright (C) 2015 Texas Instruments Incorporated - https://www.ti.com/
Again, check your copyright lines please.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 3/3] drivers: misc: Add support for TPS65224 pfsm driver
2023-10-26 13:32 ` [PATCH v1 3/3] drivers: misc: Add support for TPS65224 pfsm driver Gairuboina Sirisha
@ 2023-10-27 7:05 ` Greg KH
2023-11-07 11:44 ` Gairuboina Sirisha
2023-10-27 8:05 ` Krzysztof Kozlowski
1 sibling, 1 reply; 21+ messages in thread
From: Greg KH @ 2023-10-27 7:05 UTC (permalink / raw)
To: Gairuboina Sirisha; +Cc: linux-kernel, lee, arnd
On Thu, Oct 26, 2023 at 07:02:26PM +0530, Gairuboina Sirisha wrote:
> From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
>
> Added support for pmic nvm set FSM_I2C_TRIGGER functions driver,
> state control driver, Makefile and Kconfig
>
> Signed-off-by: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> ---
> drivers/misc/Kconfig | 12 ++
> drivers/misc/Makefile | 1 +
> drivers/misc/tps65224-pfsm.c | 290 +++++++++++++++++++++++++++++
> include/uapi/linux/tps65224_pfsm.h | 36 ++++
> 4 files changed, 339 insertions(+)
> create mode 100644 drivers/misc/tps65224-pfsm.c
> create mode 100644 include/uapi/linux/tps65224_pfsm.h
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index cadd4a820c03..6d12404b0fa6 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -562,6 +562,18 @@ config TPS6594_PFSM
> This driver can also be built as a module. If so, the module
> will be called tps6594-pfsm.
>
> +config TPS65224_PFSM
> + tristate "TI TPS65224 Pre-configurable Finite State Machine support"
> + depends on MFD_TPS65224
> + default MFD_TPS65224
> + help
> + Support PFSM (Pre-configurable Finite State Machine) on TPS65224 PMIC devices.
> + These devices integrate a finite state machine engine, which manages the state
> + of the device during operating state transition.
Please wrap your lines a bit better, like the rest of the file has.
> --- /dev/null
> +++ b/drivers/misc/tps65224-pfsm.c
> @@ -0,0 +1,290 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PFSM (Pre-configurable Finite State Machine) driver for TI TPS65224 PMIC
> + *
> + * Copyright (C) 2015 Texas Instruments Incorporated - https://www.ti.com/
Again, 2015?
> +static long tps65224_pfsm_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
> +{
> + struct tps65224_pfsm *pfsm = TPS65224_FILE_TO_PFSM(f);
> + struct pmic_state_opt state_opt;
> + void __user *argp = (void __user *)arg;
> + int ret = -ENOIOCTLCMD;
> +
> + switch (cmd) {
> + case PMIC_GOTO_STANDBY:
> + /* Force trigger */
> + ret = regmap_write_bits(pfsm->regmap, TPS65224_REG_FSM_I2C_TRIGGERS,
> + TPS65224_BIT_TRIGGER_I2C(0), TPS65224_BIT_TRIGGER_I2C(0));
> + break;
> + case PMIC_UPDATE_PGM:
> + /* Force trigger */
> + ret = regmap_write_bits(pfsm->regmap, TPS65224_REG_FSM_I2C_TRIGGERS,
> + TPS65224_BIT_TRIGGER_I2C(3), TPS65224_BIT_TRIGGER_I2C(3));
> + break;
> + case PMIC_SET_ACTIVE_STATE:
> + /* Modify NSLEEP1-2 bits */
> + ret = regmap_set_bits(pfsm->regmap, TPS65224_REG_FSM_NSLEEP_TRIGGERS,
> + TPS65224_BIT_NSLEEP1B | TPS65224_BIT_NSLEEP2B);
> + break;
> + case PMIC_SET_MCU_ONLY_STATE:
> + if (copy_from_user(&state_opt, argp, sizeof(state_opt)))
> + return -EFAULT;
> +
> + /* Configure retention triggers */
> + ret = tps65224_pfsm_configure_ret_trig(pfsm->regmap, state_opt.gpio_retention,
> + state_opt.ddr_retention);
No error checking of userspace values at all? That's brave.
> + if (ret)
> + return ret;
> +
> + /* Modify NSLEEP1-2 bits */
> + ret = regmap_clear_bits(pfsm->regmap, TPS65224_REG_FSM_NSLEEP_TRIGGERS,
> + TPS65224_BIT_NSLEEP1B);
> + if (ret)
> + return ret;
> +
> + ret = regmap_set_bits(pfsm->regmap, TPS65224_REG_FSM_NSLEEP_TRIGGERS,
> + TPS65224_BIT_NSLEEP2B);
> + break;
> + case PMIC_SET_RETENTION_STATE:
> + if (copy_from_user(&state_opt, argp, sizeof(state_opt)))
> + return -EFAULT;
Again, no checking of valid values?
> +
> + /* Configure wake-up destination */
> + if (state_opt.mcu_only_startup_dest)
> + ret = regmap_write_bits(pfsm->regmap, TPS65224_REG_CONFIG_2,
> + TPS65224_BIT_STARTUP_INT,
> + TPS65224_STARTUP_DEST_MCU_ONLY);
> + else
> + ret = regmap_write_bits(pfsm->regmap, TPS65224_REG_CONFIG_2,
> + TPS65224_BIT_STARTUP_INT,
> + TPS65224_STARTUP_DEST_ACTIVE);
> + if (ret)
> + return ret;
> +
> + /* Configure retention triggers */
> + ret = tps65224_pfsm_configure_ret_trig(pfsm->regmap, state_opt.gpio_retention,
> + state_opt.ddr_retention);
> + if (ret)
> + return ret;
> +
> + /* Modify NSLEEP1-2 bits */
> + ret = regmap_clear_bits(pfsm->regmap, TPS65224_REG_FSM_NSLEEP_TRIGGERS,
> + TPS65224_BIT_NSLEEP2B);
> + break;
> + }
> +
> + return ret;
> +}
Where is the example userspace code that uses these custom ioctls?
Where are they documented? How do we know what they should be doing?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 3/3] drivers: misc: Add support for TPS65224 pfsm driver
2023-10-26 13:32 ` [PATCH v1 3/3] drivers: misc: Add support for TPS65224 pfsm driver Gairuboina Sirisha
2023-10-27 7:05 ` Greg KH
@ 2023-10-27 8:05 ` Krzysztof Kozlowski
2023-11-07 11:48 ` Gairuboina Sirisha
1 sibling, 1 reply; 21+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-27 8:05 UTC (permalink / raw)
To: Gairuboina Sirisha, linux-kernel; +Cc: lee, arnd, gregkh
On 26/10/2023 15:32, Gairuboina Sirisha wrote:
> From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
>
> Added support for pmic nvm set FSM_I2C_TRIGGER functions driver,
> state control driver, Makefile and Kconfig
>
...
> +
> +static int tps65224_pfsm_probe(struct platform_device *pdev)
> +{
> + struct tps65224_pfsm *pfsm;
> + struct tps65224 *tps = dev_get_drvdata(pdev->dev.parent);
> + struct device *dev = &pdev->dev;
> + int irq;
> + int ret;
> + int i;
> +
> + pfsm = devm_kzalloc(dev, sizeof(struct tps65224_pfsm), GFP_KERNEL);
sizeof(*)
> + if (!pfsm)
> + return -ENOMEM;
> +
> + pfsm->regmap = tps->regmap;
> +
> + pfsm->miscdev.minor = MISC_DYNAMIC_MINOR;
> + pfsm->miscdev.name = devm_kasprintf(dev, GFP_KERNEL, "pfsm-%ld-0x%02x",
> + tps->chip_id, tps->reg);
> + pfsm->miscdev.fops = &tps65224_pfsm_fops;
> + pfsm->miscdev.parent = dev->parent;
> +
> + for (i = 0 ; i < pdev->num_resources ; i++) {
> + irq = platform_get_irq_byname(pdev, pdev->resource[i].name);
> + if (irq < 0)
> + return dev_err_probe(dev, irq, "Failed to get %s irq\n",
> + pdev->resource[i].name);
> +
> + ret = devm_request_threaded_irq(dev, irq, NULL,
> + tps65224_pfsm_isr, IRQF_ONESHOT,
> + pdev->resource[i].name, pdev);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to request irq\n");
> + }
> +
> + platform_set_drvdata(pdev, pfsm);
> +
> + return misc_register(&pfsm->miscdev);
> +}
> +
> +static void tps65224_pfsm_remove(struct platform_device *pdev)
> +{
> + struct tps65224_pfsm *pfsm = platform_get_drvdata(pdev);
> +
> + misc_deregister(&pfsm->miscdev);
> +}
> +
> +static struct platform_driver tps65224_pfsm_driver = {
> + .driver = {
> + .name = "tps65224-pfsm",
> + },
> + .probe = tps65224_pfsm_probe,
> + .remove_new = tps65224_pfsm_remove,
> +};
> +
> +module_platform_driver(tps65224_pfsm_driver);
> +
> +MODULE_ALIAS("platform:tps65224-pfsm");
You should not need MODULE_ALIAS() in normal cases. If you need it,
usually it means your device ID table is wrong.
> +MODULE_AUTHOR("Gairuboina Sirisha <sirisha.gairuboina@ltts.com>");
> +MODULE_DESCRIPTION("TPS65224 Pre-configurable Finite State Machine Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/uapi/linux/tps65224_pfsm.h b/include/uapi/linux/tps65224_pfsm.h
> new file mode 100644
> index 000000000000..c0a135903b5d
> --- /dev/null
> +++ b/include/uapi/linux/tps65224_pfsm.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Userspace ABI for TPS65224 PMIC Pre-configurable Finite State Machine
> + *
> + * Copyright (C) 2015 Texas Instruments Incorporated - https://www.ti.com/
> + */
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 2/3] drivers: mfd: Add support for TPS65224 i2c driver
2023-10-26 13:32 ` [PATCH v1 2/3] drivers: mfd: Add support for TPS65224 i2c driver Gairuboina Sirisha
2023-10-27 7:02 ` Greg KH
@ 2023-10-27 8:08 ` Krzysztof Kozlowski
2023-11-07 11:50 ` Gairuboina Sirisha
1 sibling, 1 reply; 21+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-27 8:08 UTC (permalink / raw)
To: Gairuboina Sirisha, linux-kernel; +Cc: lee, arnd, gregkh
On 26/10/2023 15:32, Gairuboina Sirisha wrote:
> From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
>
> Added MFD driver that enables I2C communication with and without CRC
>
> Signed-off-by: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> ---
> drivers/mfd/Kconfig | 14 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/tps65224-i2c.c | 245 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 260 insertions(+)
> create mode 100644 drivers/mfd/tps65224-i2c.c
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 2e4906484eed..943d85d49fc5 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1767,12 +1767,26 @@ config MFD_TPS6594_SPI
>
> This driver can also be built as a module. If so, the module
> will be called tps6594-spi.
> +
This does not belong to this patch.
> config MFD_TPS65224
> tristate
> select MFD_CORE
> select REGMAP
> select REGMAP_IRQ
...
}
> +
> +static const struct regmap_config tps65224_i2c_regmap_config = {
> + .reg_bits = 16,
> + .val_bits = 8,
> + .max_register = TPS65224_REG_WD_FAIL_CNT_REG,
> + .volatile_reg = tps65224_is_volatile_reg,
> + .read = tps65224_i2c_read,
> + .write = tps65224_i2c_write,
> +};
> +
> +static const struct of_device_id tps65224_i2c_of_match_table[] = {
> + { .compatible = "ti,tps65224-q1", },
Where is it documented? It seems nowhere, even though tools asked you to
do this. :(
Please run scripts/checkpatch.pl and fix reported warnings. Some
warnings can be ignored, but the code here looks like it needs a fix.
Feel free to get in touch if the warning is not clear.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/3] TPS65224 PMIC driver
2023-10-26 13:32 [PATCH v1 0/3] TPS65224 PMIC driver Gairuboina Sirisha
` (2 preceding siblings ...)
2023-10-26 13:32 ` [PATCH v1 3/3] drivers: misc: Add support for TPS65224 pfsm driver Gairuboina Sirisha
@ 2023-11-03 8:52 ` Julien Panis
2023-11-07 11:37 ` Gairuboina Sirisha
3 siblings, 1 reply; 21+ messages in thread
From: Julien Panis @ 2023-11-03 8:52 UTC (permalink / raw)
To: Gairuboina Sirisha, linux-kernel; +Cc: lee, arnd, gregkh, Krzysztof Kozlowski
On 10/26/23 15:32, Gairuboina Sirisha wrote:
> Added support for TPS65224 PMIC in linux.
> This patch set includes driver for core, i2c and pfsm.
> The driver was tested on TI's custom AM62A EVM.
>
> Gairuboina Sirisha (3):
> drivers: mfd: Add support for TPS65224
> drivers: mfd: Add support for TPS65224 i2c driver
> drivers: misc: Add support for TPS65224 pfsm driver
>
> drivers/mfd/Kconfig | 19 +
> drivers/mfd/Makefile | 2 +
> drivers/mfd/tps65224-core.c | 291 ++++++++++++
> drivers/mfd/tps65224-i2c.c | 245 ++++++++++
> drivers/misc/Kconfig | 12 +
> drivers/misc/Makefile | 1 +
> drivers/misc/tps65224-pfsm.c | 290 ++++++++++++
> include/linux/mfd/tps65224.h | 735 +++++++++++++++++++++++++++++
> include/uapi/linux/tps65224_pfsm.h | 36 ++
> 9 files changed, 1631 insertions(+)
> create mode 100644 drivers/mfd/tps65224-core.c
> create mode 100644 drivers/mfd/tps65224-i2c.c
> create mode 100644 drivers/misc/tps65224-pfsm.c
> create mode 100644 include/linux/mfd/tps65224.h
> create mode 100644 include/uapi/linux/tps65224_pfsm.h
Hi Sirisha,
These drivers strongly look like TPS6594 drivers.
Instead of submitting new drivers, you should consider reusing and
modifying the existing ones for TPS6594. You might add your new 'compatible'
entry ("ti,tps65224-q1") in TPS6594 dt-bindings (see 'ti,tps6594.yaml' file)
to identify your TPS65224 PMIC. This new 'compatible' would also be added
in the existing 'tps6594_i2c_of_match_table'. You can have a look at
'tps->chip_id' in 'tps6594-core.c' and see how we use it to deal with slight
differences between different PMIC IDs.
Julien
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/3] TPS65224 PMIC driver
2023-11-03 8:52 ` [PATCH v1 0/3] TPS65224 PMIC driver Julien Panis
@ 2023-11-07 11:37 ` Gairuboina Sirisha
2023-11-08 9:19 ` Julien Panis
0 siblings, 1 reply; 21+ messages in thread
From: Gairuboina Sirisha @ 2023-11-07 11:37 UTC (permalink / raw)
To: jpanis
Cc: arnd, gregkh, krzysztof.kozlowski+dt, lee, linux-kernel,
sirisha.gairuboina, Gairuboina Sirisha
From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> On 10/26/23 15:32, Gairuboina Sirisha wrote:
> > Added support for TPS65224 PMIC in linux.
> > This patch set includes driver for core, i2c and pfsm.
> > The driver was tested on TI's custom AM62A EVM.
> >
> > Gairuboina Sirisha (3):
> > drivers: mfd: Add support for TPS65224
> > drivers: mfd: Add support for TPS65224 i2c driver
> > drivers: misc: Add support for TPS65224 pfsm driver
> >
> > drivers/mfd/Kconfig | 19 +
> > drivers/mfd/Makefile | 2 +
> > drivers/mfd/tps65224-core.c | 291 ++++++++++++
> > drivers/mfd/tps65224-i2c.c | 245 ++++++++++
> > drivers/misc/Kconfig | 12 +
> > drivers/misc/Makefile | 1 +
> > drivers/misc/tps65224-pfsm.c | 290 ++++++++++++
> > include/linux/mfd/tps65224.h | 735 +++++++++++++++++++++++++++++
> > include/uapi/linux/tps65224_pfsm.h | 36 ++
> > 9 files changed, 1631 insertions(+)
> > create mode 100644 drivers/mfd/tps65224-core.c
> > create mode 100644 drivers/mfd/tps65224-i2c.c
> > create mode 100644 drivers/misc/tps65224-pfsm.c
> > create mode 100644 include/linux/mfd/tps65224.h
> > create mode 100644 include/uapi/linux/tps65224_pfsm.h
>
> Hi Sirisha,
>
> These drivers strongly look like TPS6594 drivers.
>
> Instead of submitting new drivers, you should consider reusing and
> modifying the existing ones for TPS6594. You might add your new 'compatible'
> entry ("ti,tps65224-q1") in TPS6594 dt-bindings (see 'ti,tps6594.yaml' file)
> to identify your TPS65224 PMIC. This new 'compatible' would also be added
> in the existing 'tps6594_i2c_of_match_table'. You can have a look at
> 'tps->chip_id' in 'tps6594-core.c' and see how we use it to deal with slight
> differences between different PMIC IDs.
Thanks for the response. While the TPS65224 drivers follow the format and
structure of TPS6594, the register maps, masks, and ADC feature differ.
The two PMICs have overlapping features but TPS65224 is not treated as a subset.
TPS65224 is treated as a separate and independent driver instead of adding
compatibility to the existing TPS6594 driver that would then support 3 PMICS.
This separation will better support our differing PMICs.
Thanks & Regards,
Sirisha G.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 1/3] drivers: mfd: Add support for TPS65224
2023-10-27 7:02 ` Greg KH
@ 2023-11-07 11:40 ` Gairuboina Sirisha
0 siblings, 0 replies; 21+ messages in thread
From: Gairuboina Sirisha @ 2023-11-07 11:40 UTC (permalink / raw)
To: gregkh; +Cc: arnd, lee, linux-kernel, sirisha.gairuboina
From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> On Thu, Oct 26, 2023 at 07:02:24PM +0530, Gairuboina Sirisha wrote:
> > From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> >
> > Added support for tps65224 driver pmic core, header, Makefile and Kconfig
>
> What is a "tps65224"? Please provide some more information so we know
> how to review this.
>
> >
> > Signed-off-by: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> > ---
> > drivers/mfd/Kconfig | 5 +
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/tps65224-core.c | 291 ++++++++++++++
> > include/linux/mfd/tps65224.h | 735 +++++++++++++++++++++++++++++++++++
> > 4 files changed, 1032 insertions(+)
> > create mode 100644 drivers/mfd/tps65224-core.c
> > create mode 100644 include/linux/mfd/tps65224.h
> >
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 90ce58fd629e..2e4906484eed 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1767,6 +1767,11 @@ config MFD_TPS6594_SPI
> >
> > This driver can also be built as a module. If so, the module
> > will be called tps6594-spi.
> > +config MFD_TPS65224
>
> You need a blank line before this, right?
>
> > + tristate
> > + select MFD_CORE
> > + select REGMAP
> > + select REGMAP_IRQ
>
> No help text at all to describe what this is?
>
> > --- /dev/null
> > +++ b/drivers/mfd/tps65224-core.c
> > @@ -0,0 +1,291 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Core functions for TI TPS65224 PMIC
> > + *
> > + * Copyright (C) 2015 Texas Instruments Incorporated - https://www.ti.com/
>
> No changes have happened since 2015 to this code?
>
> > +EXPORT_SYMBOL_GPL(tps65224_device_init);
> > +
> > +MODULE_AUTHOR("Gairuboina Sirisha <sirisha.gairuboina@ltts.com>");
>
> That dosn't match the copyright line :(
>
Thanks for pointing it out. We will update the cover letter to describe about tps65224.
Also, style and copyright will be corrected in the next patch version V2.
> > --- /dev/null
> > +++ b/include/linux/mfd/tps65224.h
>
> Why do you need a .h file at all? This is just used in one .c file,
> right?
This is a common header file for tps65224 PMIC which will be used for other drivers
like GPIO, Regulators, PFSM, ESM, SPI and I2C.
Thanks & Regards,
Sirisha G.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 2/3] drivers: mfd: Add support for TPS65224 i2c driver
2023-10-27 7:02 ` Greg KH
@ 2023-11-07 11:42 ` Gairuboina Sirisha
0 siblings, 0 replies; 21+ messages in thread
From: Gairuboina Sirisha @ 2023-11-07 11:42 UTC (permalink / raw)
To: gregkh; +Cc: arnd, lee, linux-kernel, sirisha.gairuboina
From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> On Thu, Oct 26, 2023 at 07:02:25PM +0530, Gairuboina Sirisha wrote:
> > From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> >
> > Added MFD driver that enables I2C communication with and without CRC
> >
> > Signed-off-by: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> > ---
> > drivers/mfd/Kconfig | 14 +++
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/tps65224-i2c.c | 245 +++++++++++++++++++++++++++++++++++++
> > 3 files changed, 260 insertions(+)
> > create mode 100644 drivers/mfd/tps65224-i2c.c
> >
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 2e4906484eed..943d85d49fc5 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1767,12 +1767,26 @@ config MFD_TPS6594_SPI
> >
> > This driver can also be built as a module. If so, the module
> > will be called tps6594-spi.
> > +
>
> That line should have been added in the first patch.
>
>
> > config MFD_TPS65224
> > tristate
> > select MFD_CORE
> > select REGMAP
> > select REGMAP_IRQ
> >
> > +config MFD_TPS65224_I2C
> > + tristate "TI TPS65224 Power Management chip with I2C"
> > + select MFD_TPS65224
> > + select REGMAP_I2C
> > + select CRC8
> > + depends on I2C
> > + help
> > + If you say yes here you get support for the TPS65224 series of
> > + PM chips with I2C interface.
> > +
> > + This driver can also be built as a module. If so, the module
> > + will be called tps65224-i2c.
> > +
> > config TWL4030_CORE
> > bool "TI TWL4030/TWL5030/TWL6030/TPS659x0 Support"
> > depends on I2C=y
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index ff4e158fa4db..4963fecd3e93 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -103,6 +103,7 @@ obj-$(CONFIG_MFD_TPS6594) += tps6594-core.o
> > obj-$(CONFIG_MFD_TPS6594_I2C) += tps6594-i2c.o
> > obj-$(CONFIG_MFD_TPS6594_SPI) += tps6594-spi.o
> > obj-$(CONFIG_MFD_TPS65224) += tps65224-core.o
> > +obj-$(CONFIG_MFD_TPS65224) += tps65224-i2c.o
> > obj-$(CONFIG_MENELAUS) += menelaus.o
> >
> > obj-$(CONFIG_TWL4030_CORE) += twl-core.o twl4030-irq.o twl6030-irq.o
> > diff --git a/drivers/mfd/tps65224-i2c.c b/drivers/mfd/tps65224-i2c.c
> > new file mode 100644
> > index 000000000000..c6300138ce4c
> > --- /dev/null
> > +++ b/drivers/mfd/tps65224-i2c.c
> > @@ -0,0 +1,245 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * I2C access driver for TI TPS65224 PMIC
> > + *
> > + * Copyright (C) 2015 Texas Instruments Incorporated - https://www.ti.com/
>
> Again, check your copyright lines please.
Will correct style and copyright issues in the next patch version V2.
Thanks & Regards,
Sirisha G.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 3/3] drivers: misc: Add support for TPS65224 pfsm driver
2023-10-27 7:05 ` Greg KH
@ 2023-11-07 11:44 ` Gairuboina Sirisha
0 siblings, 0 replies; 21+ messages in thread
From: Gairuboina Sirisha @ 2023-11-07 11:44 UTC (permalink / raw)
To: gregkh; +Cc: arnd, lee, linux-kernel, sirisha.gairuboina
From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> On Thu, Oct 26, 2023 at 07:02:26PM +0530, Gairuboina Sirisha wrote:
> > From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> >
> > Added support for pmic nvm set FSM_I2C_TRIGGER functions driver,
> > state control driver, Makefile and Kconfig
> >
> > Signed-off-by: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> > ---
> > drivers/misc/Kconfig | 12 ++
> > drivers/misc/Makefile | 1 +
> > drivers/misc/tps65224-pfsm.c | 290 +++++++++++++++++++++++++++++
> > include/uapi/linux/tps65224_pfsm.h | 36 ++++
> > 4 files changed, 339 insertions(+)
> > create mode 100644 drivers/misc/tps65224-pfsm.c
> > create mode 100644 include/uapi/linux/tps65224_pfsm.h
> >
> > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > index cadd4a820c03..6d12404b0fa6 100644
> > --- a/drivers/misc/Kconfig
> > +++ b/drivers/misc/Kconfig
> > @@ -562,6 +562,18 @@ config TPS6594_PFSM
> > This driver can also be built as a module. If so, the module
> > will be called tps6594-pfsm.
> >
> > +config TPS65224_PFSM
> > + tristate "TI TPS65224 Pre-configurable Finite State Machine support"
> > + depends on MFD_TPS65224
> > + default MFD_TPS65224
> > + help
> > + Support PFSM (Pre-configurable Finite State Machine) on TPS65224 PMIC devices.
> > + These devices integrate a finite state machine engine, which manages the state
> > + of the device during operating state transition.
>
> Please wrap your lines a bit better, like the rest of the file has.
>
> > --- /dev/null
> > +++ b/drivers/misc/tps65224-pfsm.c
> > @@ -0,0 +1,290 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * PFSM (Pre-configurable Finite State Machine) driver for TI TPS65224 PMIC
> > + *
> > + * Copyright (C) 2015 Texas Instruments Incorporated - https://www.ti.com/
>
> Again, 2015?
Will correct style and copyright issues in the next patch version V2.
> > +static long tps65224_pfsm_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
> > +{
> > + struct tps65224_pfsm *pfsm = TPS65224_FILE_TO_PFSM(f);
> > + struct pmic_state_opt state_opt;
> > + void __user *argp = (void __user *)arg;
> > + int ret = -ENOIOCTLCMD;
> > +
> > + switch (cmd) {
> > + case PMIC_GOTO_STANDBY:
> > + /* Force trigger */
> > + ret = regmap_write_bits(pfsm->regmap, TPS65224_REG_FSM_I2C_TRIGGERS,
> > + TPS65224_BIT_TRIGGER_I2C(0), TPS65224_BIT_TRIGGER_I2C(0));
> > + break;
> > + case PMIC_UPDATE_PGM:
> > + /* Force trigger */
> > + ret = regmap_write_bits(pfsm->regmap, TPS65224_REG_FSM_I2C_TRIGGERS,
> > + TPS65224_BIT_TRIGGER_I2C(3), TPS65224_BIT_TRIGGER_I2C(3));
> > + break;
> > + case PMIC_SET_ACTIVE_STATE:
> > + /* Modify NSLEEP1-2 bits */
> > + ret = regmap_set_bits(pfsm->regmap, TPS65224_REG_FSM_NSLEEP_TRIGGERS,
> > + TPS65224_BIT_NSLEEP1B | TPS65224_BIT_NSLEEP2B);
> > + break;
> > + case PMIC_SET_MCU_ONLY_STATE:
> > + if (copy_from_user(&state_opt, argp, sizeof(state_opt)))
> > + return -EFAULT;
> > +
> > + /* Configure retention triggers */
> > + ret = tps65224_pfsm_configure_ret_trig(pfsm->regmap, state_opt.gpio_retention,
> > + state_opt.ddr_retention);
>
> No error checking of userspace values at all? That's brave.
>
> > + if (ret)
> > + return ret;
> > +
> > + /* Modify NSLEEP1-2 bits */
> > + ret = regmap_clear_bits(pfsm->regmap, TPS65224_REG_FSM_NSLEEP_TRIGGERS,
> > + TPS65224_BIT_NSLEEP1B);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_set_bits(pfsm->regmap, TPS65224_REG_FSM_NSLEEP_TRIGGERS,
> > + TPS65224_BIT_NSLEEP2B);
> > + break;
> > + case PMIC_SET_RETENTION_STATE:
> > + if (copy_from_user(&state_opt, argp, sizeof(state_opt)))
> > + return -EFAULT;
> >
> > Again, no checking of valid values?
Thanks for your feedback. Will validate the user space buffer and submit in V2.
> > +
> > + /* Configure wake-up destination */
> > + if (state_opt.mcu_only_startup_dest)
> > + ret = regmap_write_bits(pfsm->regmap, TPS65224_REG_CONFIG_2,
> > + TPS65224_BIT_STARTUP_INT,
> > + TPS65224_STARTUP_DEST_MCU_ONLY);
> > + else
> > + ret = regmap_write_bits(pfsm->regmap, TPS65224_REG_CONFIG_2,
> > + TPS65224_BIT_STARTUP_INT,
> > + TPS65224_STARTUP_DEST_ACTIVE);
> > + if (ret)
> > + return ret;
> > +
> > + /* Configure retention triggers */
> > + ret = tps65224_pfsm_configure_ret_trig(pfsm->regmap, state_opt.gpio_retention,
> > + state_opt.ddr_retention);
> > + if (ret)
> > + return ret;
> > +
> > + /* Modify NSLEEP1-2 bits */
> > + ret = regmap_clear_bits(pfsm->regmap, TPS65224_REG_FSM_NSLEEP_TRIGGERS,
> > + TPS65224_BIT_NSLEEP2B);
> > + break;
> > + }
> > +
> > + return ret;
> > +}
>
> Where is the example userspace code that uses these custom ioctls?
> Where are they documented? How do we know what they should be doing?
We have tested the PFSM driver using sample code available in sampes/pfsm/pfsm-wakeup.c with minimal change on Number of PMICs.
Thanks & Regards,
Sirisha G.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 3/3] drivers: misc: Add support for TPS65224 pfsm driver
2023-10-27 8:05 ` Krzysztof Kozlowski
@ 2023-11-07 11:48 ` Gairuboina Sirisha
0 siblings, 0 replies; 21+ messages in thread
From: Gairuboina Sirisha @ 2023-11-07 11:48 UTC (permalink / raw)
To: krzk
Cc: arnd, gregkh, lee, linux-kernel, sirisha.gairuboina, Gairuboina Sirisha
From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> On 26/10/2023 15:32, Gairuboina Sirisha wrote:
> > From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> >
> > Added support for pmic nvm set FSM_I2C_TRIGGER functions driver,
> > state control driver, Makefile and Kconfig
> >
>
> ...
>
> > +
> > +static int tps65224_pfsm_probe(struct platform_device *pdev)
> > +{
> > + struct tps65224_pfsm *pfsm;
> > + struct tps65224 *tps = dev_get_drvdata(pdev->dev.parent);
> > + struct device *dev = &pdev->dev;
> > + int irq;
> > + int ret;
> > + int i;
> > +
> > + pfsm = devm_kzalloc(dev, sizeof(struct tps65224_pfsm), GFP_KERNEL);
>
> sizeof(*)
>
> > + if (!pfsm)
> > + return -ENOMEM;
> > +
> > + pfsm->regmap = tps->regmap;
> > +
> > + pfsm->miscdev.minor = MISC_DYNAMIC_MINOR;
> > + pfsm->miscdev.name = devm_kasprintf(dev, GFP_KERNEL, "pfsm-%ld-0x%02x",
> > + tps->chip_id, tps->reg);
> > + pfsm->miscdev.fops = &tps65224_pfsm_fops;
> > + pfsm->miscdev.parent = dev->parent;
> > +
> > + for (i = 0 ; i < pdev->num_resources ; i++) {
> > + irq = platform_get_irq_byname(pdev, pdev->resource[i].name);
> > + if (irq < 0)
> > + return dev_err_probe(dev, irq, "Failed to get %s irq\n",
> > + pdev->resource[i].name);
> > +
> > + ret = devm_request_threaded_irq(dev, irq, NULL,
> > + tps65224_pfsm_isr, IRQF_ONESHOT,
> > + pdev->resource[i].name, pdev);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to request irq\n");
> > + }
> > +
> > + platform_set_drvdata(pdev, pfsm);
> > +
> > + return misc_register(&pfsm->miscdev);
> > +}
> > +
> > +static void tps65224_pfsm_remove(struct platform_device *pdev)
> > +{
> > + struct tps65224_pfsm *pfsm = platform_get_drvdata(pdev);
> > +
> > + misc_deregister(&pfsm->miscdev);
> > +}
> > +
> > +static struct platform_driver tps65224_pfsm_driver = {
> > + .driver = {
> > + .name = "tps65224-pfsm",
> > + },
> > + .probe = tps65224_pfsm_probe,
> > + .remove_new = tps65224_pfsm_remove,
> > +};
> > +
> > +module_platform_driver(tps65224_pfsm_driver);
> > +
> > +MODULE_ALIAS("platform:tps65224-pfsm");
>
> You should not need MODULE_ALIAS() in normal cases. If you need it,
> usually it means your device ID table is wrong.
Thanks for your feedback. We will remove the same in the next patch version
as we don't have PMIC subset for now.
Thanks & Regards,
Sirisha G.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 2/3] drivers: mfd: Add support for TPS65224 i2c driver
2023-10-27 8:08 ` Krzysztof Kozlowski
@ 2023-11-07 11:50 ` Gairuboina Sirisha
0 siblings, 0 replies; 21+ messages in thread
From: Gairuboina Sirisha @ 2023-11-07 11:50 UTC (permalink / raw)
To: krzk
Cc: arnd, gregkh, lee, linux-kernel, sirisha.gairuboina, Gairuboina Sirisha
From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> On 26/10/2023 15:32, Gairuboina Sirisha wrote:
> > From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> >
> > Added MFD driver that enables I2C communication with and without CRC
> >
> > Signed-off-by: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
> > ---
> > drivers/mfd/Kconfig | 14 +++
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/tps65224-i2c.c | 245 +++++++++++++++++++++++++++++++++++++
> > 3 files changed, 260 insertions(+)
> > create mode 100644 drivers/mfd/tps65224-i2c.c
> >
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 2e4906484eed..943d85d49fc5 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1767,12 +1767,26 @@ config MFD_TPS6594_SPI
> >
> > This driver can also be built as a module. If so, the module
> > will be called tps6594-spi.
> > +
>
> This does not belong to this patch.
>
> > config MFD_TPS65224
> > tristate
> > select MFD_CORE
> > select REGMAP
> > select REGMAP_IRQ
>
> ...
>
> }
> > +
> > +static const struct regmap_config tps65224_i2c_regmap_config = {
> > + .reg_bits = 16,
> > + .val_bits = 8,
> > + .max_register = TPS65224_REG_WD_FAIL_CNT_REG,
> > + .volatile_reg = tps65224_is_volatile_reg,
> > + .read = tps65224_i2c_read,
> > + .write = tps65224_i2c_write,
> > +};
> > +
> > +static const struct of_device_id tps65224_i2c_of_match_table[] = {
> > + { .compatible = "ti,tps65224-q1", },
>
> Where is it documented? It seems nowhere, even though tools asked you to
> do this. :(
>
> Please run scripts/checkpatch.pl and fix reported warnings. Some
> warnings can be ignored, but the code here looks like it needs a fix.
> Feel free to get in touch if the warning is not clear.
Sure. We will submit the required documentation in the next version of patch.
Thanks & Regards,
Sirisha G.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/3] TPS65224 PMIC driver
2023-11-07 11:37 ` Gairuboina Sirisha
@ 2023-11-08 9:19 ` Julien Panis
2023-11-09 16:22 ` Shree Ramamoorthy
0 siblings, 1 reply; 21+ messages in thread
From: Julien Panis @ 2023-11-08 9:19 UTC (permalink / raw)
To: Gairuboina Sirisha
Cc: arnd, gregkh, krzysztof.kozlowski+dt, lee, linux-kernel
On 11/7/23 12:37, Gairuboina Sirisha wrote:
> From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
>
>> On 10/26/23 15:32, Gairuboina Sirisha wrote:
>>> Added support for TPS65224 PMIC in linux.
>>> This patch set includes driver for core, i2c and pfsm.
>>> The driver was tested on TI's custom AM62A EVM.
>>>
>>> Gairuboina Sirisha (3):
>>> drivers: mfd: Add support for TPS65224
>>> drivers: mfd: Add support for TPS65224 i2c driver
>>> drivers: misc: Add support for TPS65224 pfsm driver
>>>
>>> drivers/mfd/Kconfig | 19 +
>>> drivers/mfd/Makefile | 2 +
>>> drivers/mfd/tps65224-core.c | 291 ++++++++++++
>>> drivers/mfd/tps65224-i2c.c | 245 ++++++++++
>>> drivers/misc/Kconfig | 12 +
>>> drivers/misc/Makefile | 1 +
>>> drivers/misc/tps65224-pfsm.c | 290 ++++++++++++
>>> include/linux/mfd/tps65224.h | 735 +++++++++++++++++++++++++++++
>>> include/uapi/linux/tps65224_pfsm.h | 36 ++
>>> 9 files changed, 1631 insertions(+)
>>> create mode 100644 drivers/mfd/tps65224-core.c
>>> create mode 100644 drivers/mfd/tps65224-i2c.c
>>> create mode 100644 drivers/misc/tps65224-pfsm.c
>>> create mode 100644 include/linux/mfd/tps65224.h
>>> create mode 100644 include/uapi/linux/tps65224_pfsm.h
>> Hi Sirisha,
>>
>> These drivers strongly look like TPS6594 drivers.
>>
>> Instead of submitting new drivers, you should consider reusing and
>> modifying the existing ones for TPS6594. You might add your new 'compatible'
>> entry ("ti,tps65224-q1") in TPS6594 dt-bindings (see 'ti,tps6594.yaml' file)
>> to identify your TPS65224 PMIC. This new 'compatible' would also be added
>> in the existing 'tps6594_i2c_of_match_table'. You can have a look at
>> 'tps->chip_id' in 'tps6594-core.c' and see how we use it to deal with slight
>> differences between different PMIC IDs.
> Thanks for the response. While the TPS65224 drivers follow the format and
> structure of TPS6594, the register maps, masks, and ADC feature differ.
> The two PMICs have overlapping features but TPS65224 is not treated as a subset.
> TPS65224 is treated as a separate and independent driver instead of adding
> compatibility to the existing TPS6594 driver that would then support 3 PMICS.
> This separation will better support our differing PMICs.
>
> Thanks & Regards,
> Sirisha G.
I compared 'tps65224.h' with 'tps6594.h', especially register mapping.
There are less resources in TPS65224, but I don't see any incompatibility
between both PMIC register mappings. Some registers are not used by
your TPS65224, and some interrupts are not used either (that's not a
problem, they will not trigger, so). Beyond that, I2C and PFSM drivers
perform the same things for both PMICs. That's why according to me,
nothing prevents from re-using TPS6594 drivers. Even for ADC, which is
specific to your TPS65224 indeed, the register range does not overlap
with any of TPS6594 registers. You could conditionally add this driver
(that's what we did in 'tps6594-core.c' for RTC driver, which is not used
for one of the compatibles: you can do something similar for ADC).
You will probably add support for others TPS65224 drivers over the next
weeks: SPI, ESM, RTC, GPIOs, regulators, watchdog, and ADC. Most of them
should be compatible with both TPS6594 and TPS65224, I think (even
watchdog driver, which was not developed for TPS6594). ADC will not,
but as explained above you can easily deal with this one thanks to
the compatible.
For 'tps65224-core.c' only, a little bit of work might be necessary to
handle your TPS65224 specific functionalities. By using a different DT
compatible string, your driver can then select different options (or maybe
even different register ranges) for some features based on the compatible.
But except for 'tps65xx-core.c', there is "sufficient overlap" to justify
sharing as much as possible between TPS65224 and TPS6594, in my
opinion.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/3] TPS65224 PMIC driver
2023-11-08 9:19 ` Julien Panis
@ 2023-11-09 16:22 ` Shree Ramamoorthy
2023-11-10 4:26 ` Greg KH
0 siblings, 1 reply; 21+ messages in thread
From: Shree Ramamoorthy @ 2023-11-09 16:22 UTC (permalink / raw)
To: Julien Panis, Gairuboina Sirisha
Cc: arnd, gregkh, krzysztof.kozlowski+dt, lee, linux-kernel, d-gole
Hi Julien,
On 11/8/2023 3:19 AM, Julien Panis wrote:
> On 11/7/23 12:37, Gairuboina Sirisha wrote:
>> From: Gairuboina Sirisha <sirisha.gairuboina@ltts.com>
>>
>>> On 10/26/23 15:32, Gairuboina Sirisha wrote:
>>>> Added support for TPS65224 PMIC in linux.
>>>> This patch set includes driver for core, i2c and pfsm.
>>>> The driver was tested on TI's custom AM62A EVM.
>>>>
>>>> Gairuboina Sirisha (3):
>>>> drivers: mfd: Add support for TPS65224
>>>> drivers: mfd: Add support for TPS65224 i2c driver
>>>> drivers: misc: Add support for TPS65224 pfsm driver
>>>>
>>>> drivers/mfd/Kconfig | 19 +
>>>> drivers/mfd/Makefile | 2 +
>>>> drivers/mfd/tps65224-core.c | 291 ++++++++++++
>>>> drivers/mfd/tps65224-i2c.c | 245 ++++++++++
>>>> drivers/misc/Kconfig | 12 +
>>>> drivers/misc/Makefile | 1 +
>>>> drivers/misc/tps65224-pfsm.c | 290 ++++++++++++
>>>> include/linux/mfd/tps65224.h | 735
>>>> +++++++++++++++++++++++++++++
>>>> include/uapi/linux/tps65224_pfsm.h | 36 ++
>>>> 9 files changed, 1631 insertions(+)
>>>> create mode 100644 drivers/mfd/tps65224-core.c
>>>> create mode 100644 drivers/mfd/tps65224-i2c.c
>>>> create mode 100644 drivers/misc/tps65224-pfsm.c
>>>> create mode 100644 include/linux/mfd/tps65224.h
>>>> create mode 100644 include/uapi/linux/tps65224_pfsm.h
>>> Hi Sirisha,
>>>
>>> These drivers strongly look like TPS6594 drivers.
>>>
>>> Instead of submitting new drivers, you should consider reusing and
>>> modifying the existing ones for TPS6594. You might add your new
>>> 'compatible'
>>> entry ("ti,tps65224-q1") in TPS6594 dt-bindings (see
>>> 'ti,tps6594.yaml' file)
>>> to identify your TPS65224 PMIC. This new 'compatible' would also be
>>> added
>>> in the existing 'tps6594_i2c_of_match_table'. You can have a look at
>>> 'tps->chip_id' in 'tps6594-core.c' and see how we use it to deal
>>> with slight
>>> differences between different PMIC IDs.
>> Thanks for the response. While the TPS65224 drivers follow the format
>> and
>> structure of TPS6594, the register maps, masks, and ADC feature differ.
>> The two PMICs have overlapping features but TPS65224 is not treated
>> as a subset.
>> TPS65224 is treated as a separate and independent driver instead of
>> adding
>> compatibility to the existing TPS6594 driver that would then support
>> 3 PMICS.
>> This separation will better support our differing PMICs.
>>
>> Thanks & Regards,
>> Sirisha G.
>
> I compared 'tps65224.h' with 'tps6594.h', especially register mapping.
> There are less resources in TPS65224, but I don't see any incompatibility
> between both PMIC register mappings. Some registers are not used by
> your TPS65224, and some interrupts are not used either (that's not a
> problem, they will not trigger, so). Beyond that, I2C and PFSM drivers
> perform the same things for both PMICs. That's why according to me,
> nothing prevents from re-using TPS6594 drivers. Even for ADC, which is
> specific to your TPS65224 indeed, the register range does not overlap
> with any of TPS6594 registers. You could conditionally add this driver
> (that's what we did in 'tps6594-core.c' for RTC driver, which is not
> used
> for one of the compatibles: you can do something similar for ADC).
> You will probably add support for others TPS65224 drivers over the next
> weeks: SPI, ESM, RTC, GPIOs, regulators, watchdog, and ADC. Most of them
> should be compatible with both TPS6594 and TPS65224, I think (even
> watchdog driver, which was not developed for TPS6594). ADC will not,
> but as explained above you can easily deal with this one thanks to
> the compatible.
> For 'tps65224-core.c' only, a little bit of work might be necessary to
> handle your TPS65224 specific functionalities. By using a different DT
> compatible string, your driver can then select different options (or
> maybe
> even different register ranges) for some features based on the
> compatible.
> But except for 'tps65xx-core.c', there is "sufficient overlap" to justify
> sharing as much as possible between TPS65224 and TPS6594, in my
> opinion.
TI is positioning TPS65224 as a separate family from TPS6594, but shared
software drivers for PMICs that have different use cases would lead to
confusion. Re-scoping the project to accommodate these suggestions would
negatively affect the timeline we are trying to meet. We want to include
the restructure that addresses the compatibility, register maps, and
functionality similarities, but it would best solved after the upcoming
deadline has been met. With the growth of PMIC software device drivers,
we would prefer to have a separate series with the suggested changes and
proper naming convention to address that while they overlap, the two
PMICs devices are not a subset.
Best,
Shree Ramamoorthy
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/3] TPS65224 PMIC driver
2023-11-09 16:22 ` Shree Ramamoorthy
@ 2023-11-10 4:26 ` Greg KH
2023-11-10 20:07 ` [EXTERNAL] " Shree Ramamoorthy
2023-11-17 11:05 ` Lee Jones
0 siblings, 2 replies; 21+ messages in thread
From: Greg KH @ 2023-11-10 4:26 UTC (permalink / raw)
To: Shree Ramamoorthy
Cc: Julien Panis, Gairuboina Sirisha, arnd, krzysztof.kozlowski+dt,
lee, linux-kernel, d-gole
On Thu, Nov 09, 2023 at 10:22:00AM -0600, Shree Ramamoorthy wrote:
> > I compared 'tps65224.h' with 'tps6594.h', especially register mapping.
> > There are less resources in TPS65224, but I don't see any incompatibility
> > between both PMIC register mappings. Some registers are not used by
> > your TPS65224, and some interrupts are not used either (that's not a
> > problem, they will not trigger, so). Beyond that, I2C and PFSM drivers
> > perform the same things for both PMICs. That's why according to me,
> > nothing prevents from re-using TPS6594 drivers. Even for ADC, which is
> > specific to your TPS65224 indeed, the register range does not overlap
> > with any of TPS6594 registers. You could conditionally add this driver
> > (that's what we did in 'tps6594-core.c' for RTC driver, which is not
> > used
> > for one of the compatibles: you can do something similar for ADC).
> > You will probably add support for others TPS65224 drivers over the next
> > weeks: SPI, ESM, RTC, GPIOs, regulators, watchdog, and ADC. Most of them
> > should be compatible with both TPS6594 and TPS65224, I think (even
> > watchdog driver, which was not developed for TPS6594). ADC will not,
> > but as explained above you can easily deal with this one thanks to
> > the compatible.
> > For 'tps65224-core.c' only, a little bit of work might be necessary to
> > handle your TPS65224 specific functionalities. By using a different DT
> > compatible string, your driver can then select different options (or
> > maybe
> > even different register ranges) for some features based on the
> > compatible.
> > But except for 'tps65xx-core.c', there is "sufficient overlap" to justify
> > sharing as much as possible between TPS65224 and TPS6594, in my
> > opinion.
>
>
> TI is positioning TPS65224 as a separate family from TPS6594, but shared
> software drivers for PMICs that have different use cases would lead to
> confusion.
Why? No one cares what a driver's name is, only that it works for their
hardware. What different "use case" would cause problems here?
> Re-scoping the project to accommodate these suggestions would
> negatively affect the timeline we are trying to meet.
There are no timelines/deadlines with kernel development, sorry, that's
not our issue.
> We want to include the
> restructure that addresses the compatibility, register maps, and
> functionality similarities, but it would best solved after the upcoming
> deadline has been met.
Again, no deadline here. Please do the work properly, that's all we
care about.
> With the growth of PMIC software device drivers, we
> would prefer to have a separate series with the suggested changes and proper
> naming convention to address that while they overlap, the two PMICs devices
> are not a subset.
Why does the name matter? Again, all that a user cares about is if
their hardware device is supported, the name means nothing here.
Please do the correct thing and add support for this device to the
existing drivers, that's the correct thing to do. You will save time
and energy and code in the long-run, which is the important thing.
There is a reason that Linux drivers are, on average, 1/3 smaller than
other operating systems. And that's because they share common code with
other drivers. You aren't allowed to just copy an existing one and add
a few changes and make a whole new driver, you need to modify the
current one.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [EXTERNAL] Re: [PATCH v1 0/3] TPS65224 PMIC driver
2023-11-10 4:26 ` Greg KH
@ 2023-11-10 20:07 ` Shree Ramamoorthy
2023-11-17 11:05 ` Lee Jones
1 sibling, 0 replies; 21+ messages in thread
From: Shree Ramamoorthy @ 2023-11-10 20:07 UTC (permalink / raw)
To: Greg KH
Cc: Julien Panis, Gairuboina Sirisha, arnd, krzysztof.kozlowski+dt,
lee, linux-kernel, d-gole
Hi Greg,
On 11/9/2023 10:26 PM, Greg KH wrote:
> On Thu, Nov 09, 2023 at 10:22:00AM -0600, Shree Ramamoorthy wrote:
>>> I compared 'tps65224.h' with 'tps6594.h', especially register mapping.
>>> There are less resources in TPS65224, but I don't see any incompatibility
>>> between both PMIC register mappings. Some registers are not used by
>>> your TPS65224, and some interrupts are not used either (that's not a
>>> problem, they will not trigger, so). Beyond that, I2C and PFSM drivers
>>> perform the same things for both PMICs. That's why according to me,
>>> nothing prevents from re-using TPS6594 drivers. Even for ADC, which is
>>> specific to your TPS65224 indeed, the register range does not overlap
>>> with any of TPS6594 registers. You could conditionally add this driver
>>> (that's what we did in 'tps6594-core.c' for RTC driver, which is not
>>> used
>>> for one of the compatibles: you can do something similar for ADC).
>>> You will probably add support for others TPS65224 drivers over the next
>>> weeks: SPI, ESM, RTC, GPIOs, regulators, watchdog, and ADC. Most of them
>>> should be compatible with both TPS6594 and TPS65224, I think (even
>>> watchdog driver, which was not developed for TPS6594). ADC will not,
>>> but as explained above you can easily deal with this one thanks to
>>> the compatible.
>>> For 'tps65224-core.c' only, a little bit of work might be necessary to
>>> handle your TPS65224 specific functionalities. By using a different DT
>>> compatible string, your driver can then select different options (or
>>> maybe
>>> even different register ranges) for some features based on the
>>> compatible.
>>> But except for 'tps65xx-core.c', there is "sufficient overlap" to justify
>>> sharing as much as possible between TPS65224 and TPS6594, in my
>>> opinion.
>>
>> TI is positioning TPS65224 as a separate family from TPS6594, but shared
>> software drivers for PMICs that have different use cases would lead to
>> confusion.
> Why? No one cares what a driver's name is, only that it works for their
> hardware. What different "use case" would cause problems here?
>
>> Re-scoping the project to accommodate these suggestions would
>> negatively affect the timeline we are trying to meet.
> There are no timelines/deadlines with kernel development, sorry, that's
> not our issue.
>
>> We want to include the
>> restructure that addresses the compatibility, register maps, and
>> functionality similarities, but it would best solved after the upcoming
>> deadline has been met.
> Again, no deadline here. Please do the work properly, that's all we
> care about.
>
>> With the growth of PMIC software device drivers, we
>> would prefer to have a separate series with the suggested changes and proper
>> naming convention to address that while they overlap, the two PMICs devices
>> are not a subset.
> Why does the name matter? Again, all that a user cares about is if
> their hardware device is supported, the name means nothing here.
>
> Please do the correct thing and add support for this device to the
> existing drivers, that's the correct thing to do. You will save time
> and energy and code in the long-run, which is the important thing.
>
> There is a reason that Linux drivers are, on average, 1/3 smaller than
> other operating systems. And that's because they share common code with
> other drivers. You aren't allowed to just copy an existing one and add
> a few changes and make a whole new driver, you need to modify the
> current one.
>
> thanks,
>
> greg k-h
Those are all fair points to ensure minimal code repetition. It’s the right decision
long term, and we’ll do what is necessary to address this. We're working with
the 3rd party to accommodate all suggestions to ensure best software practices.
Thank you!
Shree Ramamoorthy
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v1 0/3] TPS65224 PMIC driver
2023-11-10 4:26 ` Greg KH
2023-11-10 20:07 ` [EXTERNAL] " Shree Ramamoorthy
@ 2023-11-17 11:05 ` Lee Jones
1 sibling, 0 replies; 21+ messages in thread
From: Lee Jones @ 2023-11-17 11:05 UTC (permalink / raw)
To: Greg KH
Cc: Shree Ramamoorthy, Julien Panis, Gairuboina Sirisha, arnd,
krzysztof.kozlowski+dt, linux-kernel, d-gole
On Fri, 10 Nov 2023, Greg KH wrote:
> On Thu, Nov 09, 2023 at 10:22:00AM -0600, Shree Ramamoorthy wrote:
> > > I compared 'tps65224.h' with 'tps6594.h', especially register mapping.
> > > There are less resources in TPS65224, but I don't see any incompatibility
> > > between both PMIC register mappings. Some registers are not used by
> > > your TPS65224, and some interrupts are not used either (that's not a
> > > problem, they will not trigger, so). Beyond that, I2C and PFSM drivers
> > > perform the same things for both PMICs. That's why according to me,
> > > nothing prevents from re-using TPS6594 drivers. Even for ADC, which is
> > > specific to your TPS65224 indeed, the register range does not overlap
> > > with any of TPS6594 registers. You could conditionally add this driver
> > > (that's what we did in 'tps6594-core.c' for RTC driver, which is not
> > > used
> > > for one of the compatibles: you can do something similar for ADC).
> > > You will probably add support for others TPS65224 drivers over the next
> > > weeks: SPI, ESM, RTC, GPIOs, regulators, watchdog, and ADC. Most of them
> > > should be compatible with both TPS6594 and TPS65224, I think (even
> > > watchdog driver, which was not developed for TPS6594). ADC will not,
> > > but as explained above you can easily deal with this one thanks to
> > > the compatible.
> > > For 'tps65224-core.c' only, a little bit of work might be necessary to
> > > handle your TPS65224 specific functionalities. By using a different DT
> > > compatible string, your driver can then select different options (or
> > > maybe
> > > even different register ranges) for some features based on the
> > > compatible.
> > > But except for 'tps65xx-core.c', there is "sufficient overlap" to justify
> > > sharing as much as possible between TPS65224 and TPS6594, in my
> > > opinion.
> >
> >
> > TI is positioning TPS65224 as a separate family from TPS6594, but shared
> > software drivers for PMICs that have different use cases would lead to
> > confusion.
>
> Why? No one cares what a driver's name is, only that it works for their
> hardware. What different "use case" would cause problems here?
>
> > Re-scoping the project to accommodate these suggestions would
> > negatively affect the timeline we are trying to meet.
>
> There are no timelines/deadlines with kernel development, sorry, that's
> not our issue.
>
> > We want to include the
> > restructure that addresses the compatibility, register maps, and
> > functionality similarities, but it would best solved after the upcoming
> > deadline has been met.
>
> Again, no deadline here. Please do the work properly, that's all we
> care about.
>
> > With the growth of PMIC software device drivers, we
> > would prefer to have a separate series with the suggested changes and proper
> > naming convention to address that while they overlap, the two PMICs devices
> > are not a subset.
>
> Why does the name matter? Again, all that a user cares about is if
> their hardware device is supported, the name means nothing here.
>
> Please do the correct thing and add support for this device to the
> existing drivers, that's the correct thing to do. You will save time
> and energy and code in the long-run, which is the important thing.
>
> There is a reason that Linux drivers are, on average, 1/3 smaller than
> other operating systems. And that's because they share common code with
> other drivers. You aren't allowed to just copy an existing one and add
> a few changes and make a whole new driver, you need to modify the
> current one.
>
> thanks,
>
> greg k-h
Ha! You took the words right out of my mouth!
Thanks.
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-11-17 11:05 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-26 13:32 [PATCH v1 0/3] TPS65224 PMIC driver Gairuboina Sirisha
2023-10-26 13:32 ` [PATCH v1 1/3] drivers: mfd: Add support for TPS65224 Gairuboina Sirisha
2023-10-27 7:02 ` Greg KH
2023-11-07 11:40 ` Gairuboina Sirisha
2023-10-26 13:32 ` [PATCH v1 2/3] drivers: mfd: Add support for TPS65224 i2c driver Gairuboina Sirisha
2023-10-27 7:02 ` Greg KH
2023-11-07 11:42 ` Gairuboina Sirisha
2023-10-27 8:08 ` Krzysztof Kozlowski
2023-11-07 11:50 ` Gairuboina Sirisha
2023-10-26 13:32 ` [PATCH v1 3/3] drivers: misc: Add support for TPS65224 pfsm driver Gairuboina Sirisha
2023-10-27 7:05 ` Greg KH
2023-11-07 11:44 ` Gairuboina Sirisha
2023-10-27 8:05 ` Krzysztof Kozlowski
2023-11-07 11:48 ` Gairuboina Sirisha
2023-11-03 8:52 ` [PATCH v1 0/3] TPS65224 PMIC driver Julien Panis
2023-11-07 11:37 ` Gairuboina Sirisha
2023-11-08 9:19 ` Julien Panis
2023-11-09 16:22 ` Shree Ramamoorthy
2023-11-10 4:26 ` Greg KH
2023-11-10 20:07 ` [EXTERNAL] " Shree Ramamoorthy
2023-11-17 11:05 ` Lee Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).