All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 0/2] TI TPS6594 PMIC support (RTC, pinctrl, regulators)
@ 2023-06-28 13:30 Esteban Blanc
  2023-06-28 13:30 ` [PATCH v7 1/2] rtc: tps6594: Add driver for TPS6594 RTC Esteban Blanc
  2023-06-28 13:30 ` [PATCH v7 2/2] pinctrl: tps6594: Add driver for TPS6594 pinctrl and GPIOs Esteban Blanc
  0 siblings, 2 replies; 7+ messages in thread
From: Esteban Blanc @ 2023-06-28 13:30 UTC (permalink / raw)
  To: linus.walleij, a.zummo, alexandre.belloni
  Cc: linux-kernel, linux-gpio, linux-rtc, jpanis, jneanne, aseketeli,
	eblanc, u-kumar1

TPS6594 is a Power Management IC which provides regulators and others
features like GPIOs, RTC, watchdog, ESMs (Error Signal Monitor), and
PFSM (Pre-configurable Finite State Machine). The SoC and the PMIC can
communicate through the I2C or SPI interfaces.
TPS6594 is the super-set device while TPS6593 and LP8764 are derivatives.

This series adds support to TI TPS6594 PMIC and its derivatives.

This should be applied on top of other patch series:
- https://lore.kernel.org/all/20230511095126.105104-1-jpanis@baylibre.com/
  For core MFD driver. The necessary part of this patch series is already
  applied in linux-next.

The features implemented in this series are:
- RTC (child device)
- Pinmux/GPIO (child device)
- Regulator (child device)

RTC description:
The TPS6594 family has an RTC built-in, except for LP8764.
It provides time and an alarm.

Pinmux/GPIO:
TPS6594 family has 11 GPIOs. Those GPIO can also serve different
functions such as I2C or SPI interface, watchdog disable functions.
The driver provides both pinmuxing for the functions and GPIO capability.

Regulator:
TPS6594/TPS6593: 5 BUCKs and 4LDOs
LP8764: 4 BUCKs and no LDO
Bucks can be used in multipahse mode.

Regulators were applied to linux-next by Mark Brown on 06/06/2023 so this
patch has been dropped from the patch series.
There were some pending comments from Andy Shevchenko so a follow up patch will
be sent later.

Changes since v1:
https://lore.kernel.org/all/20230224133129.887203-1-eblanc@baylibre.com/
Rtc:
- Removed struct tps6594_rtc.
- Removed some dev_err messages.
- Removed some comments.
- Remove some whitespaces in comments and error messages.
- Check if RTC is running before reading a timestamp in read_rtc.
- Stop RTC at the end of probe to wait for a timestamp to be set.
- Add default MFD_TPS6594 to Kconfig.

Pinctrl:
- Removed #define DEBUG.
- Add default MFD_TPS6594 to Kconfig.
- Fix typo and reword help message of Kconfig.

Regulators:
Further to Mark Brown review:
- File header whole block C++ style.
- Configuring modes not supported: omit all mode operations
- Log the error before notifying.
- Request the interrupts while registering the regulators (then remove
  the lookup function).
Further to Matti review:
- Postponed: devm_regulator_irq_helper() and
  regulator_irq_map_event_simple() can probably be used but code.
  refactoring is not so trivial. This can be done later as an enhancement
  after this patch list is merged.
Buck Multi phase management:
- Multiphase property can take an array when 2 multi phase buck, buck12
  and buck34.
- Configuration multi phase buck34 without multiphase buck12 is not
  supported (when only one multiphase, must be buck12). Not clear from the
  spec but confirmed by TI.
- Supported multiphase conficurations: buck12, buck123, buck1234,
  buck12 + buck34.
- All interrupts are attached to the multiphase buck (ie: for regulator
  buck12, buck1 & buck2 interrupts are registered).

Changes since v2:
https://lore.kernel.org/all/20230328091448.648452-1-eblanc@baylibre.com/
Rtc:
- Add logic to avoid reinitializing a working clock.
- Fix some multiline comments format.

Regulators:
Further to Mark Brown review:
- Log the error before notifying.
- Request the interrupts while registering the regulators.
Further to Krzysztof Kozlowski:
https://lore.kernel.org/all/75f0a18d-aed9-8610-2925-4e604b4b0241@baylibre.com/
- Remove ti, multi-phase-id property which is redundant with buck dts naming
  rules.

Changes since v3:
https://lore.kernel.org/lkml/20230414101217.1342891-1-eblanc@baylibre.com/
RTC:
- Add wakeup source

Pinctrl:
- Switch to GPIO_REGMAP framework

Change since v4:
https://lore.kernel.org/lkml/20230512141755.1712358-1-eblanc@baylibre.com/
Update Copyright notice date
Reorder includes

RTC:
- Rework some comments, fixing punctuation and style
- Use NANO macro from units.h for PPB_MULT
- Rework to use bitwise types
- Remove unnecessary casts
- Add SAFETY comments
- Use `dev_err_probe(...)` instead of print then return

Pinctrl:
- Reword help message and add module name in Kconfig
- Rework code to use struct pinfunction and PINCTRL_PINFUNCTION() macro
- Remove unnecessary casts
- Use `dev_err_probe(...)` instead of print then return
- Replace TPS6594_REG_GPIO1_CONF with a comment for TPS6594_REG_GPIOX_CONF

Regulators:
- nits: Add missing tabs, standard spaces, group "buck_multi".
- Use OF dedicated of_node_cmp API instead of standard strcmp.
- Use devm_kmalloc_array(...) API instead of devm_kmalloc(...) wherever
  possible.
- return dev_err_probe(...) wherever possible.

Changes since v5:
https://lore.kernel.org/lkml/20230522163115.2592883-1-eblanc@baylibre.com/

Pinctrl:
- Rework code for clarity
- Rework macro to fix checkpatch macro argument reuse
- Coding style fixes
- Reword some comments

Rtc:
- Grammar fixes
- Removed unused macros
- Use type MIN/MAX macro instead of magic numbers
- Fix return code in calibration
- Use cpu_to_le16 and le16_to_cpu APIs instead of casting.
- Reintroduce mdelay before reading BIT_RUN as otherwise both AM62 and J784S4
  will report a -ENODEV on a working RTC.

Changes since v6:
https://lore.kernel.org/all/20230612125248.1235581-1-eblanc@baylibre.com/

Pinctrl:
- Remove a comment

Esteban Blanc (2):
  rtc: tps6594: Add driver for TPS6594 RTC
  pinctrl: tps6594: Add driver for TPS6594 pinctrl and GPIOs

 drivers/pinctrl/Kconfig           |  15 +
 drivers/pinctrl/Makefile          |   1 +
 drivers/pinctrl/pinctrl-tps6594.c | 368 ++++++++++++++++++++++++
 drivers/rtc/Kconfig               |  12 +
 drivers/rtc/Makefile              |   1 +
 drivers/rtc/rtc-tps6594.c         | 452 ++++++++++++++++++++++++++++++
 6 files changed, 849 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-tps6594.c
 create mode 100644 drivers/rtc/rtc-tps6594.c

-- 
2.41.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v7 1/2] rtc: tps6594: Add driver for TPS6594 RTC
  2023-06-28 13:30 [PATCH v7 0/2] TI TPS6594 PMIC support (RTC, pinctrl, regulators) Esteban Blanc
@ 2023-06-28 13:30 ` Esteban Blanc
  2023-07-05  9:45   ` andy.shevchenko
  2023-10-01 21:20   ` Alexandre Belloni
  2023-06-28 13:30 ` [PATCH v7 2/2] pinctrl: tps6594: Add driver for TPS6594 pinctrl and GPIOs Esteban Blanc
  1 sibling, 2 replies; 7+ messages in thread
From: Esteban Blanc @ 2023-06-28 13:30 UTC (permalink / raw)
  To: linus.walleij, a.zummo, alexandre.belloni
  Cc: linux-kernel, linux-gpio, linux-rtc, jpanis, jneanne, aseketeli,
	eblanc, u-kumar1

TPS6594 PMIC is a MFD. This patch adds support for
the RTC found inside TPS6594 family of PMIC.

Alarm is also supported.

Signed-off-by: Esteban Blanc <eblanc@baylibre.com>
---
 drivers/rtc/Kconfig       |  12 +
 drivers/rtc/Makefile      |   1 +
 drivers/rtc/rtc-tps6594.c | 452 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 465 insertions(+)
 create mode 100644 drivers/rtc/rtc-tps6594.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 753872408615..39b5c93c6282 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -578,6 +578,18 @@ config RTC_DRV_TPS6586X
 	  along with alarm. This driver supports the RTC driver for
 	  the TPS6586X RTC module.
 
+config RTC_DRV_TPS6594
+	tristate "TI TPS6594 RTC driver"
+	depends on MFD_TPS6594
+	default MFD_TPS6594
+	help
+	  TI Power Management IC TPS6594 supports RTC functionality
+	  along with alarm. This driver supports the RTC driver for
+	  the TPS6594 RTC module.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-tps6594.
+
 config RTC_DRV_TPS65910
 	tristate "TI TPS65910 RTC driver"
 	depends on MFD_TPS65910
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index ea445d1ebb17..3d3f8c9d0697 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_RTC_DRV_TEGRA)	+= rtc-tegra.o
 obj-$(CONFIG_RTC_DRV_TEST)	+= rtc-test.o
 obj-$(CONFIG_RTC_DRV_TI_K3)	+= rtc-ti-k3.o
 obj-$(CONFIG_RTC_DRV_TPS6586X)	+= rtc-tps6586x.o
+obj-$(CONFIG_RTC_DRV_TPS6594)	+= rtc-tps6594.o
 obj-$(CONFIG_RTC_DRV_TPS65910)	+= rtc-tps65910.o
 obj-$(CONFIG_RTC_DRV_TWL4030)	+= rtc-twl.o
 obj-$(CONFIG_RTC_DRV_VT8500)	+= rtc-vt8500.o
diff --git a/drivers/rtc/rtc-tps6594.c b/drivers/rtc/rtc-tps6594.c
new file mode 100644
index 000000000000..7328ea7da849
--- /dev/null
+++ b/drivers/rtc/rtc-tps6594.c
@@ -0,0 +1,452 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RTC driver for tps6594 PMIC
+ *
+ * Copyright (C) 2023 BayLibre Incorporated - https://www.baylibre.com/
+ */
+
+#include <linux/bcd.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/limits.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/rtc.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+#include <linux/mfd/tps6594.h>
+
+// Total number of RTC registers needed to set time
+#define NUM_TIME_REGS (TPS6594_REG_RTC_WEEKS - TPS6594_REG_RTC_SECONDS + 1)
+
+// Total number of RTC alarm registers
+#define NUM_TIME_ALARM_REGS (NUM_TIME_REGS - 1)
+
+/*
+ * Min and max values supported by 'offset' interface (swapped sign).
+ * After conversion, the values do not exceed the range [-32767, 33767]
+ * which COMP_REG must conform to.
+ */
+#define MIN_OFFSET (-277774)
+#define MAX_OFFSET (277774)
+
+// Number of ticks per hour
+#define TICKS_PER_HOUR (32768 * 3600)
+
+// Multiplier for ppb conversions
+#define PPB_MULT NANO
+
+static int tps6594_rtc_alarm_irq_enable(struct device *dev,
+					unsigned int enabled)
+{
+	struct tps6594 *tps = dev_get_drvdata(dev->parent);
+	u8 val;
+
+	val = enabled ? TPS6594_BIT_IT_ALARM : 0;
+
+	return regmap_update_bits(tps->regmap, TPS6594_REG_RTC_INTERRUPTS,
+				  TPS6594_BIT_IT_ALARM, val);
+}
+
+/* Pulse GET_TIME field of RTC_CTRL_1 to store a timestamp in shadow registers. */
+static int tps6594_rtc_shadow_timestamp(struct device *dev, struct tps6594 *tps)
+{
+	int ret;
+
+	/*
+	 * Set GET_TIME to 0. Next time we set GET_TIME to 1 we will be sure to store
+	 * an up-to-date timestamp.
+	 */
+	ret = regmap_clear_bits(tps->regmap, TPS6594_REG_RTC_CTRL_1,
+				TPS6594_BIT_GET_TIME);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Copy content of RTC registers to shadow registers or latches to read
+	 * a coherent timestamp.
+	 */
+	return regmap_set_bits(tps->regmap, TPS6594_REG_RTC_CTRL_1,
+			       TPS6594_BIT_GET_TIME);
+}
+
+static int tps6594_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	unsigned char rtc_data[NUM_TIME_REGS];
+	struct tps6594 *tps = dev_get_drvdata(dev->parent);
+	int ret;
+
+	// Check if RTC is running.
+	ret = regmap_test_bits(tps->regmap, TPS6594_REG_RTC_STATUS,
+			       TPS6594_BIT_RUN);
+	if (ret < 0)
+		return ret;
+	if (ret == 0)
+		return -EINVAL;
+
+	ret = tps6594_rtc_shadow_timestamp(dev, tps);
+	if (ret < 0)
+		return ret;
+
+	// Read shadowed RTC registers.
+	ret = regmap_bulk_read(tps->regmap, TPS6594_REG_RTC_SECONDS, rtc_data,
+			       NUM_TIME_REGS);
+	if (ret < 0)
+		return ret;
+
+	tm->tm_sec = bcd2bin(rtc_data[0]);
+	tm->tm_min = bcd2bin(rtc_data[1]);
+	tm->tm_hour = bcd2bin(rtc_data[2]);
+	tm->tm_mday = bcd2bin(rtc_data[3]);
+	tm->tm_mon = bcd2bin(rtc_data[4]) - 1;
+	tm->tm_year = bcd2bin(rtc_data[5]) + 100;
+	tm->tm_wday = bcd2bin(rtc_data[6]);
+
+	return 0;
+}
+
+static int tps6594_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	unsigned char rtc_data[NUM_TIME_REGS];
+	struct tps6594 *tps = dev_get_drvdata(dev->parent);
+	int ret;
+
+	rtc_data[0] = bin2bcd(tm->tm_sec);
+	rtc_data[1] = bin2bcd(tm->tm_min);
+	rtc_data[2] = bin2bcd(tm->tm_hour);
+	rtc_data[3] = bin2bcd(tm->tm_mday);
+	rtc_data[4] = bin2bcd(tm->tm_mon + 1);
+	rtc_data[5] = bin2bcd(tm->tm_year - 100);
+	rtc_data[6] = bin2bcd(tm->tm_wday);
+
+	// Stop RTC while updating the RTC time registers.
+	ret = regmap_clear_bits(tps->regmap, TPS6594_REG_RTC_CTRL_1,
+				TPS6594_BIT_STOP_RTC);
+	if (ret < 0)
+		return ret;
+
+	// Update all the time registers in one shot.
+	ret = regmap_bulk_write(tps->regmap, TPS6594_REG_RTC_SECONDS, rtc_data,
+				NUM_TIME_REGS);
+	if (ret < 0)
+		return ret;
+
+	// Start back RTC.
+	return regmap_set_bits(tps->regmap, TPS6594_REG_RTC_CTRL_1,
+			       TPS6594_BIT_STOP_RTC);
+}
+
+static int tps6594_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
+{
+	unsigned char alarm_data[NUM_TIME_ALARM_REGS];
+	u32 int_val;
+	struct tps6594 *tps = dev_get_drvdata(dev->parent);
+	int ret;
+
+	ret = regmap_bulk_read(tps->regmap, TPS6594_REG_ALARM_SECONDS,
+			       alarm_data, NUM_TIME_ALARM_REGS);
+	if (ret < 0)
+		return ret;
+
+	alm->time.tm_sec = bcd2bin(alarm_data[0]);
+	alm->time.tm_min = bcd2bin(alarm_data[1]);
+	alm->time.tm_hour = bcd2bin(alarm_data[2]);
+	alm->time.tm_mday = bcd2bin(alarm_data[3]);
+	alm->time.tm_mon = bcd2bin(alarm_data[4]) - 1;
+	alm->time.tm_year = bcd2bin(alarm_data[5]) + 100;
+
+	ret = regmap_read(tps->regmap, TPS6594_REG_RTC_INTERRUPTS, &int_val);
+	if (ret < 0)
+		return ret;
+
+	alm->enabled = int_val & TPS6594_BIT_IT_ALARM;
+
+	return 0;
+}
+
+static int tps6594_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
+{
+	unsigned char alarm_data[NUM_TIME_ALARM_REGS];
+	struct tps6594 *tps = dev_get_drvdata(dev->parent);
+	int ret;
+
+	// Disable alarm irq before changing the alarm timestamp.
+	ret = tps6594_rtc_alarm_irq_enable(dev, 0);
+	if (ret)
+		return ret;
+
+	alarm_data[0] = bin2bcd(alm->time.tm_sec);
+	alarm_data[1] = bin2bcd(alm->time.tm_min);
+	alarm_data[2] = bin2bcd(alm->time.tm_hour);
+	alarm_data[3] = bin2bcd(alm->time.tm_mday);
+	alarm_data[4] = bin2bcd(alm->time.tm_mon + 1);
+	alarm_data[5] = bin2bcd(alm->time.tm_year - 100);
+
+	// Update all the alarm registers in one shot.
+	ret = regmap_bulk_write(tps->regmap, TPS6594_REG_ALARM_SECONDS,
+				alarm_data, NUM_TIME_ALARM_REGS);
+	if (ret < 0)
+		return ret;
+
+	if (alm->enabled)
+		ret = tps6594_rtc_alarm_irq_enable(dev, 1);
+
+	return ret;
+}
+
+static int tps6594_rtc_set_calibration(struct device *dev, int calibration)
+{
+	struct tps6594 *tps = dev_get_drvdata(dev->parent);
+	__le16 value;
+	int ret;
+
+	/*
+	 * TPS6594 uses two's complement 16 bit value for compensation of RTC
+	 * crystal inaccuracies. One time every hour when seconds counter
+	 * increments from 0 to 1 compensation value will be added to internal
+	 * RTC counter value.
+	 *
+	 * Valid range for compensation value: [-32767 .. 32767].
+	 */
+	if (calibration < S16_MIN + 1 || calibration > S16_MAX)
+		return -ERANGE;
+
+	value = cpu_to_le16(calibration);
+
+	// Update all the compensation registers in one shot.
+	ret = regmap_bulk_write(tps->regmap, TPS6594_REG_RTC_COMP_LSB, &value,
+				sizeof(value));
+	if (ret < 0)
+		return ret;
+
+	// Enable automatic compensation.
+	return regmap_set_bits(tps->regmap, TPS6594_REG_RTC_CTRL_1,
+			       TPS6594_BIT_AUTO_COMP);
+}
+
+static int tps6594_rtc_get_calibration(struct device *dev, int *calibration)
+{
+	struct tps6594 *tps = dev_get_drvdata(dev->parent);
+	unsigned int ctrl;
+	__le16 value;
+	int ret;
+
+	ret = regmap_read(tps->regmap, TPS6594_REG_RTC_CTRL_1, &ctrl);
+	if (ret < 0)
+		return ret;
+
+	// If automatic compensation is not enabled report back zero.
+	if (!(ctrl & TPS6594_BIT_AUTO_COMP)) {
+		*calibration = 0;
+		return 0;
+	}
+
+	ret = regmap_bulk_read(tps->regmap, TPS6594_REG_RTC_COMP_LSB, &value,
+			       sizeof(value));
+	if (ret < 0)
+		return ret;
+
+	*calibration = le16_to_cpu(value);
+
+	return 0;
+}
+
+static int tps6594_rtc_read_offset(struct device *dev, long *offset)
+{
+	int calibration;
+	s64 tmp;
+	int ret;
+
+	ret = tps6594_rtc_get_calibration(dev, &calibration);
+	if (ret < 0)
+		return ret;
+
+	// Convert from RTC calibration register format to ppb format.
+	tmp = calibration * PPB_MULT;
+
+	if (tmp < 0)
+		tmp -= TICKS_PER_HOUR / 2LL;
+	else
+		tmp += TICKS_PER_HOUR / 2LL;
+	tmp = div_s64(tmp, TICKS_PER_HOUR);
+
+	/*
+	 * SAFETY:
+	 * Compution is the reverse operation of the one done in
+	 * `tps6594_rtc_set_offset`. The safety remarks applie here too.
+	 */
+
+	/*
+	 * Offset value operates in negative way, so swap sign.
+	 * See 8.3.10.5, (32768 - COMP_REG).
+	 */
+	*offset = (long)-tmp;
+
+	return 0;
+}
+
+static int tps6594_rtc_set_offset(struct device *dev, long offset)
+{
+	int calibration;
+	s64 tmp;
+
+	// Make sure offset value is within supported range.
+	if (offset < MIN_OFFSET || offset > MAX_OFFSET)
+		return -ERANGE;
+
+	// Convert from ppb format to RTC calibration register format.
+
+	tmp = offset * TICKS_PER_HOUR;
+	if (tmp < 0)
+		tmp -= PPB_MULT / 2LL;
+	else
+		tmp += PPB_MULT / 2LL;
+	tmp = div_s64(tmp, PPB_MULT);
+
+	/*
+	 * SAFETY:
+	 * - tmp = offset * TICK_PER_HOUR :
+	 *	`offset` can't be more than 277774, so `tmp` can't exceed 277774000000000
+	 *	which is lower than the maximum value in an `s64` (2^63-1). No overflow here.
+	 *
+	 * - tmp += TICK_PER_HOUR / 2LL :
+	 *	tmp will have a maximum value of 277774117964800 which is still inferior to 2^63-1.
+	 */
+
+	// Offset value operates in negative way, so swap sign.
+	calibration = (int)-tmp;
+
+	return tps6594_rtc_set_calibration(dev, calibration);
+}
+
+static irqreturn_t tps6594_rtc_interrupt(int irq, void *rtc)
+{
+	struct device *dev = rtc;
+	unsigned long events = 0;
+	struct tps6594 *tps = dev_get_drvdata(dev->parent);
+	struct rtc_device *rtc_dev = dev_get_drvdata(dev);
+	int ret;
+	u32 rtc_reg;
+
+	ret = regmap_read(tps->regmap, TPS6594_REG_RTC_STATUS, &rtc_reg);
+	if (ret)
+		return IRQ_NONE;
+
+	if (rtc_reg & TPS6594_BIT_ALARM)
+		events = RTC_IRQF | RTC_AF;
+
+	// Notify RTC core on event.
+	rtc_update_irq(rtc_dev, 1, events);
+
+	return IRQ_HANDLED;
+}
+
+static const struct rtc_class_ops tps6594_rtc_ops = {
+	.read_time = tps6594_rtc_read_time,
+	.set_time = tps6594_rtc_set_time,
+	.read_alarm = tps6594_rtc_read_alarm,
+	.set_alarm = tps6594_rtc_set_alarm,
+	.alarm_irq_enable = tps6594_rtc_alarm_irq_enable,
+	.read_offset = tps6594_rtc_read_offset,
+	.set_offset = tps6594_rtc_set_offset,
+};
+
+static int tps6594_rtc_probe(struct platform_device *pdev)
+{
+	struct tps6594 *tps = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct rtc_device *rtc;
+	int irq;
+	int ret;
+
+	rtc = devm_kzalloc(dev, sizeof(*rtc), GFP_KERNEL);
+	if (!rtc)
+		return -ENOMEM;
+
+	rtc = devm_rtc_allocate_device(dev);
+	if (IS_ERR(rtc))
+		return PTR_ERR(rtc);
+
+	// Enable crystal oscillator.
+	ret = regmap_set_bits(tps->regmap, TPS6594_REG_RTC_CTRL_2,
+			      TPS6594_BIT_XTAL_EN);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_test_bits(tps->regmap, TPS6594_REG_RTC_STATUS,
+			       TPS6594_BIT_RUN);
+	if (ret < 0)
+		return ret;
+	// RTC not running.
+	if (ret == 0) {
+		ret = regmap_set_bits(tps->regmap, TPS6594_REG_RTC_CTRL_1,
+				      TPS6594_BIT_STOP_RTC);
+		if (ret < 0)
+			return ret;
+
+		/*
+		 * On some boards, a 40 ms delay is needed before BIT_RUN is set.
+		 * 80 ms should provide sufficient margin.
+		 */
+		mdelay(80);
+
+		/*
+		 * RTC should be running now. Check if this is the case.
+		 * If not it might be a missing oscillator.
+		 */
+		ret = regmap_test_bits(tps->regmap, TPS6594_REG_RTC_STATUS,
+				       TPS6594_BIT_RUN);
+		if (ret < 0)
+			return ret;
+		if (ret == 0)
+			return -ENODEV;
+
+		// Stop RTC until first call to `tps6594_rtc_set_time`.
+		ret = regmap_clear_bits(tps->regmap, TPS6594_REG_RTC_CTRL_1,
+					TPS6594_BIT_STOP_RTC);
+		if (ret < 0)
+			return ret;
+	}
+
+	platform_set_drvdata(pdev, rtc);
+
+	irq = platform_get_irq_byname(pdev, TPS6594_IRQ_NAME_ALARM);
+	if (irq < 0)
+		return dev_err_probe(dev, irq, "Failed to get irq\n");
+
+	ret = devm_request_threaded_irq(dev, irq, NULL, tps6594_rtc_interrupt,
+					IRQF_ONESHOT, TPS6594_IRQ_NAME_ALARM,
+					dev);
+	if (ret < 0)
+		return dev_err_probe(dev, ret,
+				     "Failed to request_threaded_irq\n");
+
+	ret = device_init_wakeup(dev, true);
+	if (ret < 0)
+		return dev_err_probe(dev, ret,
+				     "Failed to init rtc as wakeup source\n");
+
+	rtc->ops = &tps6594_rtc_ops;
+	rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
+	rtc->range_max = RTC_TIMESTAMP_END_2099;
+
+	return devm_rtc_register_device(rtc);
+}
+
+static struct platform_driver tps6594_rtc_driver = {
+	.probe		= tps6594_rtc_probe,
+	.driver		= {
+		.name	= "tps6594-rtc",
+	},
+};
+
+module_platform_driver(tps6594_rtc_driver);
+MODULE_ALIAS("platform:tps6594-rtc");
+MODULE_AUTHOR("Esteban Blanc <eblanc@baylibre.com>");
+MODULE_DESCRIPTION("TPS6594 RTC driver");
+MODULE_LICENSE("GPL");
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v7 2/2] pinctrl: tps6594: Add driver for TPS6594 pinctrl and GPIOs
  2023-06-28 13:30 [PATCH v7 0/2] TI TPS6594 PMIC support (RTC, pinctrl, regulators) Esteban Blanc
  2023-06-28 13:30 ` [PATCH v7 1/2] rtc: tps6594: Add driver for TPS6594 RTC Esteban Blanc
@ 2023-06-28 13:30 ` Esteban Blanc
  2023-06-29  7:00   ` Linus Walleij
  2023-07-03 21:47   ` andy.shevchenko
  1 sibling, 2 replies; 7+ messages in thread
From: Esteban Blanc @ 2023-06-28 13:30 UTC (permalink / raw)
  To: linus.walleij, a.zummo, alexandre.belloni
  Cc: linux-kernel, linux-gpio, linux-rtc, jpanis, jneanne, aseketeli,
	eblanc, u-kumar1

TI TPS6594 PMIC has 11 GPIOs which can be used
for different functions.

This patch adds a pinctrl and GPIO drivers in
order to use those functions.

Signed-off-by: Esteban Blanc <eblanc@baylibre.com>
---
 drivers/pinctrl/Kconfig           |  15 ++
 drivers/pinctrl/Makefile          |   1 +
 drivers/pinctrl/pinctrl-tps6594.c | 368 ++++++++++++++++++++++++++++++
 3 files changed, 384 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-tps6594.c

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 5787c579dcf6..d3cff8d27e5d 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -480,6 +480,21 @@ config PINCTRL_TB10X
 	depends on OF && ARC_PLAT_TB10X
 	select GPIOLIB
 
+config PINCTRL_TPS6594
+	tristate "Pinctrl and GPIO driver for TI TPS6594 PMIC"
+	depends on MFD_TPS6594
+	default MFD_TPS6594
+	select PINMUX
+	select GPIOLIB
+	select REGMAP
+	select GPIO_REGMAP
+	help
+	  Say Y to select the pinmuxing and GPIOs driver for the TPS6594
+	  PMICs chip family.
+
+	  This driver can also be built as a module
+	  called tps6594-pinctrl.
+
 config PINCTRL_ZYNQ
 	bool "Pinctrl driver for Xilinx Zynq"
 	depends on ARCH_ZYNQ
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index e196c6e324ad..28271a8d5275 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_PINCTRL_ST) 	+= pinctrl-st.o
 obj-$(CONFIG_PINCTRL_STMFX) 	+= pinctrl-stmfx.o
 obj-$(CONFIG_PINCTRL_SX150X)	+= pinctrl-sx150x.o
 obj-$(CONFIG_PINCTRL_TB10X)	+= pinctrl-tb10x.o
+obj-$(CONFIG_PINCTRL_TPS6594)	+= pinctrl-tps6594.o
 obj-$(CONFIG_PINCTRL_ZYNQMP)	+= pinctrl-zynqmp.o
 obj-$(CONFIG_PINCTRL_ZYNQ)	+= pinctrl-zynq.o
 
diff --git a/drivers/pinctrl/pinctrl-tps6594.c b/drivers/pinctrl/pinctrl-tps6594.c
new file mode 100644
index 000000000000..a1fcb9fd793b
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-tps6594.c
@@ -0,0 +1,368 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Pinmux and GPIO driver for tps6594 PMIC
+ *
+ * Copyright (C) 2023 BayLibre Incorporated - https://www.baylibre.com/
+ */
+
+#include <linux/gpio/driver.h>
+#include <linux/gpio/regmap.h>
+#include <linux/module.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/platform_device.h>
+
+#include <linux/mfd/tps6594.h>
+
+#define TPS6594_PINCTRL_PINS_NB 11
+
+#define TPS6594_PINCTRL_GPIO_FUNCTION 0
+#define TPS6594_PINCTRL_SCL_I2C2_CS_SPI_FUNCTION 1
+#define TPS6594_PINCTRL_TRIG_WDOG_FUNCTION 1
+#define TPS6594_PINCTRL_CLK32KOUT_FUNCTION 1
+#define TPS6594_PINCTRL_SCLK_SPMI_FUNCTION 1
+#define TPS6594_PINCTRL_SDATA_SPMI_FUNCTION 1
+#define TPS6594_PINCTRL_NERR_MCU_FUNCTION 1
+#define TPS6594_PINCTRL_PDOG_FUNCTION 1
+#define TPS6594_PINCTRL_SYNCCLKIN_FUNCTION 1
+#define TPS6594_PINCTRL_NRSTOUT_SOC_FUNCTION 2
+#define TPS6594_PINCTRL_SYNCCLKOUT_FUNCTION 2
+#define TPS6594_PINCTRL_SDA_I2C2_SDO_SPI_FUNCTION 2
+#define TPS6594_PINCTRL_NERR_SOC_FUNCTION 2
+#define TPS6594_PINCTRL_DISABLE_WDOG_FUNCTION 3
+#define TPS6594_PINCTRL_NSLEEP1_FUNCTION 4
+#define TPS6594_PINCTRL_NSLEEP2_FUNCTION 5
+#define TPS6594_PINCTRL_WKUP1_FUNCTION 6
+#define TPS6594_PINCTRL_WKUP2_FUNCTION 7
+
+/* Special muxval for recalcitrant pins */
+#define TPS6594_PINCTRL_DISABLE_WDOG_FUNCTION_GPIO8 2
+#define TPS6594_PINCTRL_SYNCCLKOUT_FUNCTION_GPIO8 3
+#define TPS6594_PINCTRL_CLK32KOUT_FUNCTION_GPIO9 3
+
+#define TPS6594_OFFSET_GPIO_SEL 5
+
+#define FUNCTION(fname, v)									\
+	{											\
+		.pinfunction = PINCTRL_PINFUNCTION(#fname,					\
+						tps6594_##fname##_func_group_names,		\
+						ARRAY_SIZE(tps6594_##fname##_func_group_names)),\
+		.muxval = v,									\
+	}
+
+static const struct pinctrl_pin_desc tps6594_pins[TPS6594_PINCTRL_PINS_NB] = {
+	PINCTRL_PIN(0, "GPIO0"),   PINCTRL_PIN(1, "GPIO1"),
+	PINCTRL_PIN(2, "GPIO2"),   PINCTRL_PIN(3, "GPIO3"),
+	PINCTRL_PIN(4, "GPIO4"),   PINCTRL_PIN(5, "GPIO5"),
+	PINCTRL_PIN(6, "GPIO6"),   PINCTRL_PIN(7, "GPIO7"),
+	PINCTRL_PIN(8, "GPIO8"),   PINCTRL_PIN(9, "GPIO9"),
+	PINCTRL_PIN(10, "GPIO10"),
+};
+
+static const char *const tps6594_gpio_func_group_names[] = {
+	"GPIO0", "GPIO1", "GPIO2", "GPIO3", "GPIO4", "GPIO5",
+	"GPIO6", "GPIO7", "GPIO8", "GPIO9", "GPIO10",
+};
+
+static const char *const tps6594_nsleep1_func_group_names[] = {
+	"GPIO0", "GPIO1", "GPIO2", "GPIO3", "GPIO4", "GPIO5",
+	"GPIO6", "GPIO7", "GPIO8", "GPIO9", "GPIO10",
+};
+
+static const char *const tps6594_nsleep2_func_group_names[] = {
+	"GPIO0", "GPIO1", "GPIO2", "GPIO3", "GPIO4", "GPIO5",
+	"GPIO6", "GPIO7", "GPIO8", "GPIO9", "GPIO10",
+};
+
+static const char *const tps6594_wkup1_func_group_names[] = {
+	"GPIO0", "GPIO1", "GPIO2", "GPIO3", "GPIO4", "GPIO5",
+	"GPIO6", "GPIO7", "GPIO8", "GPIO9", "GPIO10",
+};
+
+static const char *const tps6594_wkup2_func_group_names[] = {
+	"GPIO0", "GPIO1", "GPIO2", "GPIO3", "GPIO4", "GPIO5",
+	"GPIO6", "GPIO7", "GPIO8", "GPIO9", "GPIO10",
+};
+
+static const char *const tps6594_scl_i2c2_cs_spi_func_group_names[] = {
+	"GPIO0",
+	"GPIO1",
+};
+
+static const char *const tps6594_nrstout_soc_func_group_names[] = {
+	"GPIO0",
+	"GPIO10",
+};
+
+static const char *const tps6594_trig_wdog_func_group_names[] = {
+	"GPIO1",
+	"GPIO10",
+};
+
+static const char *const tps6594_sda_i2c2_sdo_spi_func_group_names[] = {
+	"GPIO1",
+};
+
+static const char *const tps6594_clk32kout_func_group_names[] = {
+	"GPIO2",
+	"GPIO3",
+	"GPIO7",
+};
+
+static const char *const tps6594_nerr_soc_func_group_names[] = {
+	"GPIO2",
+};
+
+static const char *const tps6594_sclk_spmi_func_group_names[] = {
+	"GPIO4",
+};
+
+static const char *const tps6594_sdata_spmi_func_group_names[] = {
+	"GPIO5",
+};
+
+static const char *const tps6594_nerr_mcu_func_group_names[] = {
+	"GPIO6",
+};
+
+static const char *const tps6594_syncclkout_func_group_names[] = {
+	"GPIO7",
+	"GPIO9",
+};
+
+static const char *const tps6594_disable_wdog_func_group_names[] = {
+	"GPIO7",
+	"GPIO8",
+};
+
+static const char *const tps6594_pdog_func_group_names[] = {
+	"GPIO8",
+};
+
+static const char *const tps6594_syncclkin_func_group_names[] = {
+	"GPIO9",
+};
+
+struct tps6594_pinctrl_function {
+	struct pinfunction pinfunction;
+	u8 muxval;
+};
+
+static const struct tps6594_pinctrl_function pinctrl_functions[] = {
+	FUNCTION(gpio, TPS6594_PINCTRL_GPIO_FUNCTION),
+	FUNCTION(nsleep1, TPS6594_PINCTRL_NSLEEP1_FUNCTION),
+	FUNCTION(nsleep2, TPS6594_PINCTRL_NSLEEP2_FUNCTION),
+	FUNCTION(wkup1, TPS6594_PINCTRL_WKUP1_FUNCTION),
+	FUNCTION(wkup2, TPS6594_PINCTRL_WKUP2_FUNCTION),
+	FUNCTION(scl_i2c2_cs_spi, TPS6594_PINCTRL_SCL_I2C2_CS_SPI_FUNCTION),
+	FUNCTION(nrstout_soc, TPS6594_PINCTRL_NRSTOUT_SOC_FUNCTION),
+	FUNCTION(trig_wdog, TPS6594_PINCTRL_TRIG_WDOG_FUNCTION),
+	FUNCTION(sda_i2c2_sdo_spi, TPS6594_PINCTRL_SDA_I2C2_SDO_SPI_FUNCTION),
+	FUNCTION(clk32kout, TPS6594_PINCTRL_CLK32KOUT_FUNCTION),
+	FUNCTION(nerr_soc, TPS6594_PINCTRL_NERR_SOC_FUNCTION),
+	FUNCTION(sclk_spmi, TPS6594_PINCTRL_SCLK_SPMI_FUNCTION),
+	FUNCTION(sdata_spmi, TPS6594_PINCTRL_SDATA_SPMI_FUNCTION),
+	FUNCTION(nerr_mcu, TPS6594_PINCTRL_NERR_MCU_FUNCTION),
+	FUNCTION(syncclkout, TPS6594_PINCTRL_SYNCCLKOUT_FUNCTION),
+	FUNCTION(disable_wdog, TPS6594_PINCTRL_DISABLE_WDOG_FUNCTION),
+	FUNCTION(pdog, TPS6594_PINCTRL_PDOG_FUNCTION),
+	FUNCTION(syncclkin, TPS6594_PINCTRL_SYNCCLKIN_FUNCTION),
+};
+
+struct tps6594_pinctrl {
+	struct tps6594 *tps;
+	struct gpio_regmap *gpio_regmap;
+	struct pinctrl_dev *pctl_dev;
+	const struct tps6594_pinctrl_function *funcs;
+	const struct pinctrl_pin_desc *pins;
+};
+
+static int tps6594_gpio_regmap_xlate(struct gpio_regmap *gpio,
+				     unsigned int base, unsigned int offset,
+				     unsigned int *reg, unsigned int *mask)
+{
+	unsigned int line = offset % 8;
+	unsigned int stride = offset / 8;
+
+	switch (base) {
+	case TPS6594_REG_GPIOX_CONF(0):
+		*reg = TPS6594_REG_GPIOX_CONF(offset);
+		*mask = TPS6594_BIT_GPIO_DIR;
+		return 0;
+	case TPS6594_REG_GPIO_IN_1:
+	case TPS6594_REG_GPIO_OUT_1:
+		*reg = base + stride;
+		*mask = BIT(line);
+		return 0;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int tps6594_pmx_func_cnt(struct pinctrl_dev *pctldev)
+{
+	return ARRAY_SIZE(pinctrl_functions);
+}
+
+static const char *tps6594_pmx_func_name(struct pinctrl_dev *pctldev,
+					 unsigned int selector)
+{
+	struct tps6594_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	return pinctrl->funcs[selector].pinfunction.name;
+}
+
+static int tps6594_pmx_func_groups(struct pinctrl_dev *pctldev,
+				   unsigned int selector,
+				   const char *const **groups,
+				   unsigned int *num_groups)
+{
+	struct tps6594_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	*groups = pinctrl->funcs[selector].pinfunction.groups;
+	*num_groups = pinctrl->funcs[selector].pinfunction.ngroups;
+
+	return 0;
+}
+
+static int tps6594_pmx_set(struct tps6594_pinctrl *pinctrl, unsigned int pin,
+			   u8 muxval)
+{
+	u8 mux_sel_val = muxval << TPS6594_OFFSET_GPIO_SEL;
+
+	return regmap_update_bits(pinctrl->tps->regmap,
+				  TPS6594_REG_GPIOX_CONF(pin),
+				  TPS6594_MASK_GPIO_SEL, mux_sel_val);
+}
+
+static int tps6594_pmx_set_mux(struct pinctrl_dev *pctldev,
+			       unsigned int function, unsigned int group)
+{
+	struct tps6594_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctldev);
+	u8 muxval = pinctrl->funcs[function].muxval;
+
+	/* Some pins don't have the same muxval for the same function... */
+	if (group == 8) {
+		if (muxval == TPS6594_PINCTRL_DISABLE_WDOG_FUNCTION)
+			muxval = TPS6594_PINCTRL_DISABLE_WDOG_FUNCTION_GPIO8;
+		else if (muxval == TPS6594_PINCTRL_SYNCCLKOUT_FUNCTION)
+			muxval = TPS6594_PINCTRL_SYNCCLKOUT_FUNCTION_GPIO8;
+	} else if (group == 9) {
+		if (muxval == TPS6594_PINCTRL_CLK32KOUT_FUNCTION)
+			muxval = TPS6594_PINCTRL_CLK32KOUT_FUNCTION_GPIO9;
+	}
+
+	return tps6594_pmx_set(pinctrl, group, muxval);
+}
+
+static int tps6594_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
+					  struct pinctrl_gpio_range *range,
+					  unsigned int offset, bool input)
+{
+	struct tps6594_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctldev);
+	u8 muxval = pinctrl->funcs[TPS6594_PINCTRL_GPIO_FUNCTION].muxval;
+
+	return tps6594_pmx_set(pinctrl, offset, muxval);
+}
+
+static const struct pinmux_ops tps6594_pmx_ops = {
+	.get_functions_count = tps6594_pmx_func_cnt,
+	.get_function_name = tps6594_pmx_func_name,
+	.get_function_groups = tps6594_pmx_func_groups,
+	.set_mux = tps6594_pmx_set_mux,
+	.gpio_set_direction = tps6594_pmx_gpio_set_direction,
+	.strict = true,
+};
+
+static int tps6594_groups_cnt(struct pinctrl_dev *pctldev)
+{
+	return ARRAY_SIZE(tps6594_pins);
+}
+
+static int tps6594_group_pins(struct pinctrl_dev *pctldev,
+			      unsigned int selector, const unsigned int **pins,
+			      unsigned int *num_pins)
+{
+	struct tps6594_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	*pins = &pinctrl->pins[selector].number;
+	*num_pins = 1;
+
+	return 0;
+}
+
+static const char *tps6594_group_name(struct pinctrl_dev *pctldev,
+				      unsigned int selector)
+{
+	struct tps6594_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	return pinctrl->pins[selector].name;
+}
+
+static const struct pinctrl_ops tps6594_pctrl_ops = {
+	.dt_node_to_map = pinconf_generic_dt_node_to_map_group,
+	.dt_free_map = pinconf_generic_dt_free_map,
+	.get_groups_count = tps6594_groups_cnt,
+	.get_group_name = tps6594_group_name,
+	.get_group_pins = tps6594_group_pins,
+};
+
+static int tps6594_pinctrl_probe(struct platform_device *pdev)
+{
+	struct tps6594 *tps = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct tps6594_pinctrl *pinctrl;
+	struct pinctrl_desc *pctrl_desc;
+	struct gpio_regmap_config config = {};
+
+	pctrl_desc = devm_kzalloc(dev, sizeof(*pctrl_desc), GFP_KERNEL);
+	if (!pctrl_desc)
+		return -ENOMEM;
+	pctrl_desc->name = dev_name(dev);
+	pctrl_desc->owner = THIS_MODULE;
+	pctrl_desc->pins = tps6594_pins;
+	pctrl_desc->npins = ARRAY_SIZE(tps6594_pins);
+	pctrl_desc->pctlops = &tps6594_pctrl_ops;
+	pctrl_desc->pmxops = &tps6594_pmx_ops;
+
+	pinctrl = devm_kzalloc(dev, sizeof(*pinctrl), GFP_KERNEL);
+	if (!pinctrl)
+		return -ENOMEM;
+	pinctrl->tps = dev_get_drvdata(dev->parent);
+	pinctrl->funcs = pinctrl_functions;
+	pinctrl->pins = tps6594_pins;
+	pinctrl->pctl_dev = devm_pinctrl_register(dev, pctrl_desc, pinctrl);
+	if (IS_ERR(pinctrl->pctl_dev))
+		return dev_err_probe(dev, PTR_ERR(pinctrl->pctl_dev),
+				     "Couldn't register pinctrl driver\n");
+
+	config.parent = tps->dev;
+	config.regmap = tps->regmap;
+	config.ngpio = TPS6594_PINCTRL_PINS_NB;
+	config.ngpio_per_reg = 8;
+	config.reg_dat_base = TPS6594_REG_GPIO_IN_1;
+	config.reg_set_base = TPS6594_REG_GPIO_OUT_1;
+	config.reg_dir_out_base = TPS6594_REG_GPIOX_CONF(0);
+	config.reg_mask_xlate = tps6594_gpio_regmap_xlate;
+
+	pinctrl->gpio_regmap = devm_gpio_regmap_register(dev, &config);
+	if (IS_ERR(pinctrl->gpio_regmap))
+		return dev_err_probe(dev, PTR_ERR(pinctrl->gpio_regmap),
+				     "Couldn't register gpio_regmap driver\n");
+
+	return 0;
+}
+
+static struct platform_driver tps6594_pinctrl_driver = {
+	.probe = tps6594_pinctrl_probe,
+	.driver = {
+		.name = "tps6594-pinctrl",
+	},
+};
+module_platform_driver(tps6594_pinctrl_driver);
+
+MODULE_ALIAS("platform:tps6594-pinctrl");
+MODULE_AUTHOR("Esteban Blanc <eblanc@baylibre.com>");
+MODULE_DESCRIPTION("TPS6594 pinctrl and GPIO driver");
+MODULE_LICENSE("GPL");
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 2/2] pinctrl: tps6594: Add driver for TPS6594 pinctrl and GPIOs
  2023-06-28 13:30 ` [PATCH v7 2/2] pinctrl: tps6594: Add driver for TPS6594 pinctrl and GPIOs Esteban Blanc
@ 2023-06-29  7:00   ` Linus Walleij
  2023-07-03 21:47   ` andy.shevchenko
  1 sibling, 0 replies; 7+ messages in thread
From: Linus Walleij @ 2023-06-29  7:00 UTC (permalink / raw)
  To: Esteban Blanc
  Cc: a.zummo, alexandre.belloni, linux-kernel, linux-gpio, linux-rtc,
	jpanis, jneanne, aseketeli, u-kumar1

On Wed, Jun 28, 2023 at 3:30 PM Esteban Blanc <eblanc@baylibre.com> wrote:

> TI TPS6594 PMIC has 11 GPIOs which can be used
> for different functions.
>
> This patch adds a pinctrl and GPIO drivers in
> order to use those functions.
>
> Signed-off-by: Esteban Blanc <eblanc@baylibre.com>

That's a very compact and nice driver. Surely good enough for
me to merge!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 2/2] pinctrl: tps6594: Add driver for TPS6594 pinctrl and GPIOs
  2023-06-28 13:30 ` [PATCH v7 2/2] pinctrl: tps6594: Add driver for TPS6594 pinctrl and GPIOs Esteban Blanc
  2023-06-29  7:00   ` Linus Walleij
@ 2023-07-03 21:47   ` andy.shevchenko
  1 sibling, 0 replies; 7+ messages in thread
From: andy.shevchenko @ 2023-07-03 21:47 UTC (permalink / raw)
  To: Esteban Blanc
  Cc: linus.walleij, a.zummo, alexandre.belloni, linux-kernel,
	linux-gpio, linux-rtc, jpanis, jneanne, aseketeli, u-kumar1

Wed, Jun 28, 2023 at 03:30:21PM +0200, Esteban Blanc kirjoitti:
> TI TPS6594 PMIC has 11 GPIOs which can be used
> for different functions.
> 
> This patch adds a pinctrl and GPIO drivers in
> order to use those functions.

A couple of nit-picks below, otherwise LGTM,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Esteban Blanc <eblanc@baylibre.com>
> ---
>  drivers/pinctrl/Kconfig           |  15 ++
>  drivers/pinctrl/Makefile          |   1 +
>  drivers/pinctrl/pinctrl-tps6594.c | 368 ++++++++++++++++++++++++++++++
>  3 files changed, 384 insertions(+)
>  create mode 100644 drivers/pinctrl/pinctrl-tps6594.c
> 
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 5787c579dcf6..d3cff8d27e5d 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -480,6 +480,21 @@ config PINCTRL_TB10X
>  	depends on OF && ARC_PLAT_TB10X
>  	select GPIOLIB
>  
> +config PINCTRL_TPS6594
> +	tristate "Pinctrl and GPIO driver for TI TPS6594 PMIC"
> +	depends on MFD_TPS6594
> +	default MFD_TPS6594
> +	select PINMUX
> +	select GPIOLIB
> +	select REGMAP
> +	select GPIO_REGMAP
> +	help
> +	  Say Y to select the pinmuxing and GPIOs driver for the TPS6594
> +	  PMICs chip family.
> +
> +	  This driver can also be built as a module
> +	  called tps6594-pinctrl.
> +
>  config PINCTRL_ZYNQ
>  	bool "Pinctrl driver for Xilinx Zynq"
>  	depends on ARCH_ZYNQ
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index e196c6e324ad..28271a8d5275 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -49,6 +49,7 @@ obj-$(CONFIG_PINCTRL_ST) 	+= pinctrl-st.o
>  obj-$(CONFIG_PINCTRL_STMFX) 	+= pinctrl-stmfx.o
>  obj-$(CONFIG_PINCTRL_SX150X)	+= pinctrl-sx150x.o
>  obj-$(CONFIG_PINCTRL_TB10X)	+= pinctrl-tb10x.o
> +obj-$(CONFIG_PINCTRL_TPS6594)	+= pinctrl-tps6594.o
>  obj-$(CONFIG_PINCTRL_ZYNQMP)	+= pinctrl-zynqmp.o
>  obj-$(CONFIG_PINCTRL_ZYNQ)	+= pinctrl-zynq.o
>  
> diff --git a/drivers/pinctrl/pinctrl-tps6594.c b/drivers/pinctrl/pinctrl-tps6594.c
> new file mode 100644
> index 000000000000..a1fcb9fd793b
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-tps6594.c
> @@ -0,0 +1,368 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Pinmux and GPIO driver for tps6594 PMIC
> + *
> + * Copyright (C) 2023 BayLibre Incorporated - https://www.baylibre.com/
> + */
> +
> +#include <linux/gpio/driver.h>
> +#include <linux/gpio/regmap.h>
> +#include <linux/module.h>
> +#include <linux/pinctrl/pinmux.h>
> +#include <linux/platform_device.h>
> +
> +#include <linux/mfd/tps6594.h>
> +
> +#define TPS6594_PINCTRL_PINS_NB 11
> +
> +#define TPS6594_PINCTRL_GPIO_FUNCTION 0
> +#define TPS6594_PINCTRL_SCL_I2C2_CS_SPI_FUNCTION 1
> +#define TPS6594_PINCTRL_TRIG_WDOG_FUNCTION 1
> +#define TPS6594_PINCTRL_CLK32KOUT_FUNCTION 1
> +#define TPS6594_PINCTRL_SCLK_SPMI_FUNCTION 1
> +#define TPS6594_PINCTRL_SDATA_SPMI_FUNCTION 1
> +#define TPS6594_PINCTRL_NERR_MCU_FUNCTION 1
> +#define TPS6594_PINCTRL_PDOG_FUNCTION 1
> +#define TPS6594_PINCTRL_SYNCCLKIN_FUNCTION 1
> +#define TPS6594_PINCTRL_NRSTOUT_SOC_FUNCTION 2
> +#define TPS6594_PINCTRL_SYNCCLKOUT_FUNCTION 2
> +#define TPS6594_PINCTRL_SDA_I2C2_SDO_SPI_FUNCTION 2
> +#define TPS6594_PINCTRL_NERR_SOC_FUNCTION 2
> +#define TPS6594_PINCTRL_DISABLE_WDOG_FUNCTION 3
> +#define TPS6594_PINCTRL_NSLEEP1_FUNCTION 4
> +#define TPS6594_PINCTRL_NSLEEP2_FUNCTION 5
> +#define TPS6594_PINCTRL_WKUP1_FUNCTION 6
> +#define TPS6594_PINCTRL_WKUP2_FUNCTION 7
> +
> +/* Special muxval for recalcitrant pins */
> +#define TPS6594_PINCTRL_DISABLE_WDOG_FUNCTION_GPIO8 2
> +#define TPS6594_PINCTRL_SYNCCLKOUT_FUNCTION_GPIO8 3
> +#define TPS6594_PINCTRL_CLK32KOUT_FUNCTION_GPIO9 3
> +
> +#define TPS6594_OFFSET_GPIO_SEL 5

> +#define FUNCTION(fname, v)									\
> +	{											\
> +		.pinfunction = PINCTRL_PINFUNCTION(#fname,					\
> +						tps6594_##fname##_func_group_names,		\
> +						ARRAY_SIZE(tps6594_##fname##_func_group_names)),\
> +		.muxval = v,									\
> +	}

In case you need a new version, you may drop indentation level by 1 TAB.

{											\
	.pinfunction = PINCTRL_PINFUNCTION(#fname,					\
					tps6594_##fname##_func_group_names,		\
					ARRAY_SIZE(tps6594_##fname##_func_group_names)),\
	.muxval = v,									\
}

> +static const struct pinctrl_pin_desc tps6594_pins[TPS6594_PINCTRL_PINS_NB] = {
> +	PINCTRL_PIN(0, "GPIO0"),   PINCTRL_PIN(1, "GPIO1"),
> +	PINCTRL_PIN(2, "GPIO2"),   PINCTRL_PIN(3, "GPIO3"),
> +	PINCTRL_PIN(4, "GPIO4"),   PINCTRL_PIN(5, "GPIO5"),
> +	PINCTRL_PIN(6, "GPIO6"),   PINCTRL_PIN(7, "GPIO7"),
> +	PINCTRL_PIN(8, "GPIO8"),   PINCTRL_PIN(9, "GPIO9"),
> +	PINCTRL_PIN(10, "GPIO10"),
> +};
> +
> +static const char *const tps6594_gpio_func_group_names[] = {
> +	"GPIO0", "GPIO1", "GPIO2", "GPIO3", "GPIO4", "GPIO5",
> +	"GPIO6", "GPIO7", "GPIO8", "GPIO9", "GPIO10",
> +};
> +
> +static const char *const tps6594_nsleep1_func_group_names[] = {
> +	"GPIO0", "GPIO1", "GPIO2", "GPIO3", "GPIO4", "GPIO5",
> +	"GPIO6", "GPIO7", "GPIO8", "GPIO9", "GPIO10",
> +};
> +
> +static const char *const tps6594_nsleep2_func_group_names[] = {
> +	"GPIO0", "GPIO1", "GPIO2", "GPIO3", "GPIO4", "GPIO5",
> +	"GPIO6", "GPIO7", "GPIO8", "GPIO9", "GPIO10",
> +};
> +
> +static const char *const tps6594_wkup1_func_group_names[] = {
> +	"GPIO0", "GPIO1", "GPIO2", "GPIO3", "GPIO4", "GPIO5",
> +	"GPIO6", "GPIO7", "GPIO8", "GPIO9", "GPIO10",
> +};
> +
> +static const char *const tps6594_wkup2_func_group_names[] = {
> +	"GPIO0", "GPIO1", "GPIO2", "GPIO3", "GPIO4", "GPIO5",
> +	"GPIO6", "GPIO7", "GPIO8", "GPIO9", "GPIO10",
> +};
> +
> +static const char *const tps6594_scl_i2c2_cs_spi_func_group_names[] = {
> +	"GPIO0",
> +	"GPIO1",
> +};
> +
> +static const char *const tps6594_nrstout_soc_func_group_names[] = {
> +	"GPIO0",
> +	"GPIO10",
> +};
> +
> +static const char *const tps6594_trig_wdog_func_group_names[] = {
> +	"GPIO1",
> +	"GPIO10",
> +};
> +
> +static const char *const tps6594_sda_i2c2_sdo_spi_func_group_names[] = {
> +	"GPIO1",
> +};
> +
> +static const char *const tps6594_clk32kout_func_group_names[] = {
> +	"GPIO2",
> +	"GPIO3",
> +	"GPIO7",
> +};
> +
> +static const char *const tps6594_nerr_soc_func_group_names[] = {
> +	"GPIO2",
> +};
> +
> +static const char *const tps6594_sclk_spmi_func_group_names[] = {
> +	"GPIO4",
> +};
> +
> +static const char *const tps6594_sdata_spmi_func_group_names[] = {
> +	"GPIO5",
> +};
> +
> +static const char *const tps6594_nerr_mcu_func_group_names[] = {
> +	"GPIO6",
> +};
> +
> +static const char *const tps6594_syncclkout_func_group_names[] = {
> +	"GPIO7",
> +	"GPIO9",
> +};
> +
> +static const char *const tps6594_disable_wdog_func_group_names[] = {
> +	"GPIO7",
> +	"GPIO8",
> +};
> +
> +static const char *const tps6594_pdog_func_group_names[] = {
> +	"GPIO8",
> +};
> +
> +static const char *const tps6594_syncclkin_func_group_names[] = {
> +	"GPIO9",
> +};
> +
> +struct tps6594_pinctrl_function {
> +	struct pinfunction pinfunction;
> +	u8 muxval;
> +};
> +
> +static const struct tps6594_pinctrl_function pinctrl_functions[] = {
> +	FUNCTION(gpio, TPS6594_PINCTRL_GPIO_FUNCTION),
> +	FUNCTION(nsleep1, TPS6594_PINCTRL_NSLEEP1_FUNCTION),
> +	FUNCTION(nsleep2, TPS6594_PINCTRL_NSLEEP2_FUNCTION),
> +	FUNCTION(wkup1, TPS6594_PINCTRL_WKUP1_FUNCTION),
> +	FUNCTION(wkup2, TPS6594_PINCTRL_WKUP2_FUNCTION),
> +	FUNCTION(scl_i2c2_cs_spi, TPS6594_PINCTRL_SCL_I2C2_CS_SPI_FUNCTION),
> +	FUNCTION(nrstout_soc, TPS6594_PINCTRL_NRSTOUT_SOC_FUNCTION),
> +	FUNCTION(trig_wdog, TPS6594_PINCTRL_TRIG_WDOG_FUNCTION),
> +	FUNCTION(sda_i2c2_sdo_spi, TPS6594_PINCTRL_SDA_I2C2_SDO_SPI_FUNCTION),
> +	FUNCTION(clk32kout, TPS6594_PINCTRL_CLK32KOUT_FUNCTION),
> +	FUNCTION(nerr_soc, TPS6594_PINCTRL_NERR_SOC_FUNCTION),
> +	FUNCTION(sclk_spmi, TPS6594_PINCTRL_SCLK_SPMI_FUNCTION),
> +	FUNCTION(sdata_spmi, TPS6594_PINCTRL_SDATA_SPMI_FUNCTION),
> +	FUNCTION(nerr_mcu, TPS6594_PINCTRL_NERR_MCU_FUNCTION),
> +	FUNCTION(syncclkout, TPS6594_PINCTRL_SYNCCLKOUT_FUNCTION),
> +	FUNCTION(disable_wdog, TPS6594_PINCTRL_DISABLE_WDOG_FUNCTION),
> +	FUNCTION(pdog, TPS6594_PINCTRL_PDOG_FUNCTION),
> +	FUNCTION(syncclkin, TPS6594_PINCTRL_SYNCCLKIN_FUNCTION),
> +};
> +
> +struct tps6594_pinctrl {
> +	struct tps6594 *tps;
> +	struct gpio_regmap *gpio_regmap;
> +	struct pinctrl_dev *pctl_dev;
> +	const struct tps6594_pinctrl_function *funcs;
> +	const struct pinctrl_pin_desc *pins;
> +};
> +
> +static int tps6594_gpio_regmap_xlate(struct gpio_regmap *gpio,
> +				     unsigned int base, unsigned int offset,
> +				     unsigned int *reg, unsigned int *mask)
> +{
> +	unsigned int line = offset % 8;
> +	unsigned int stride = offset / 8;
> +
> +	switch (base) {
> +	case TPS6594_REG_GPIOX_CONF(0):
> +		*reg = TPS6594_REG_GPIOX_CONF(offset);
> +		*mask = TPS6594_BIT_GPIO_DIR;
> +		return 0;
> +	case TPS6594_REG_GPIO_IN_1:
> +	case TPS6594_REG_GPIO_OUT_1:
> +		*reg = base + stride;
> +		*mask = BIT(line);
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}

> +	return 0;

Dead code.

> +}
> +
> +static int tps6594_pmx_func_cnt(struct pinctrl_dev *pctldev)
> +{
> +	return ARRAY_SIZE(pinctrl_functions);
> +}
> +
> +static const char *tps6594_pmx_func_name(struct pinctrl_dev *pctldev,
> +					 unsigned int selector)
> +{
> +	struct tps6594_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctldev);
> +
> +	return pinctrl->funcs[selector].pinfunction.name;
> +}
> +
> +static int tps6594_pmx_func_groups(struct pinctrl_dev *pctldev,
> +				   unsigned int selector,
> +				   const char *const **groups,
> +				   unsigned int *num_groups)
> +{
> +	struct tps6594_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctldev);
> +
> +	*groups = pinctrl->funcs[selector].pinfunction.groups;
> +	*num_groups = pinctrl->funcs[selector].pinfunction.ngroups;
> +
> +	return 0;
> +}
> +
> +static int tps6594_pmx_set(struct tps6594_pinctrl *pinctrl, unsigned int pin,
> +			   u8 muxval)
> +{
> +	u8 mux_sel_val = muxval << TPS6594_OFFSET_GPIO_SEL;
> +
> +	return regmap_update_bits(pinctrl->tps->regmap,
> +				  TPS6594_REG_GPIOX_CONF(pin),
> +				  TPS6594_MASK_GPIO_SEL, mux_sel_val);
> +}
> +
> +static int tps6594_pmx_set_mux(struct pinctrl_dev *pctldev,
> +			       unsigned int function, unsigned int group)
> +{
> +	struct tps6594_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctldev);
> +	u8 muxval = pinctrl->funcs[function].muxval;
> +
> +	/* Some pins don't have the same muxval for the same function... */
> +	if (group == 8) {
> +		if (muxval == TPS6594_PINCTRL_DISABLE_WDOG_FUNCTION)
> +			muxval = TPS6594_PINCTRL_DISABLE_WDOG_FUNCTION_GPIO8;
> +		else if (muxval == TPS6594_PINCTRL_SYNCCLKOUT_FUNCTION)
> +			muxval = TPS6594_PINCTRL_SYNCCLKOUT_FUNCTION_GPIO8;
> +	} else if (group == 9) {
> +		if (muxval == TPS6594_PINCTRL_CLK32KOUT_FUNCTION)
> +			muxval = TPS6594_PINCTRL_CLK32KOUT_FUNCTION_GPIO9;
> +	}
> +
> +	return tps6594_pmx_set(pinctrl, group, muxval);
> +}
> +
> +static int tps6594_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
> +					  struct pinctrl_gpio_range *range,
> +					  unsigned int offset, bool input)
> +{
> +	struct tps6594_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctldev);
> +	u8 muxval = pinctrl->funcs[TPS6594_PINCTRL_GPIO_FUNCTION].muxval;
> +
> +	return tps6594_pmx_set(pinctrl, offset, muxval);
> +}
> +
> +static const struct pinmux_ops tps6594_pmx_ops = {
> +	.get_functions_count = tps6594_pmx_func_cnt,
> +	.get_function_name = tps6594_pmx_func_name,
> +	.get_function_groups = tps6594_pmx_func_groups,
> +	.set_mux = tps6594_pmx_set_mux,
> +	.gpio_set_direction = tps6594_pmx_gpio_set_direction,
> +	.strict = true,
> +};
> +
> +static int tps6594_groups_cnt(struct pinctrl_dev *pctldev)
> +{
> +	return ARRAY_SIZE(tps6594_pins);
> +}
> +
> +static int tps6594_group_pins(struct pinctrl_dev *pctldev,
> +			      unsigned int selector, const unsigned int **pins,
> +			      unsigned int *num_pins)
> +{
> +	struct tps6594_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctldev);
> +
> +	*pins = &pinctrl->pins[selector].number;
> +	*num_pins = 1;
> +
> +	return 0;
> +}
> +
> +static const char *tps6594_group_name(struct pinctrl_dev *pctldev,
> +				      unsigned int selector)
> +{
> +	struct tps6594_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctldev);
> +
> +	return pinctrl->pins[selector].name;
> +}
> +
> +static const struct pinctrl_ops tps6594_pctrl_ops = {
> +	.dt_node_to_map = pinconf_generic_dt_node_to_map_group,
> +	.dt_free_map = pinconf_generic_dt_free_map,
> +	.get_groups_count = tps6594_groups_cnt,
> +	.get_group_name = tps6594_group_name,
> +	.get_group_pins = tps6594_group_pins,
> +};
> +
> +static int tps6594_pinctrl_probe(struct platform_device *pdev)
> +{
> +	struct tps6594 *tps = dev_get_drvdata(pdev->dev.parent);
> +	struct device *dev = &pdev->dev;
> +	struct tps6594_pinctrl *pinctrl;
> +	struct pinctrl_desc *pctrl_desc;
> +	struct gpio_regmap_config config = {};
> +
> +	pctrl_desc = devm_kzalloc(dev, sizeof(*pctrl_desc), GFP_KERNEL);
> +	if (!pctrl_desc)
> +		return -ENOMEM;
> +	pctrl_desc->name = dev_name(dev);
> +	pctrl_desc->owner = THIS_MODULE;
> +	pctrl_desc->pins = tps6594_pins;
> +	pctrl_desc->npins = ARRAY_SIZE(tps6594_pins);
> +	pctrl_desc->pctlops = &tps6594_pctrl_ops;
> +	pctrl_desc->pmxops = &tps6594_pmx_ops;
> +
> +	pinctrl = devm_kzalloc(dev, sizeof(*pinctrl), GFP_KERNEL);
> +	if (!pinctrl)
> +		return -ENOMEM;
> +	pinctrl->tps = dev_get_drvdata(dev->parent);
> +	pinctrl->funcs = pinctrl_functions;
> +	pinctrl->pins = tps6594_pins;
> +	pinctrl->pctl_dev = devm_pinctrl_register(dev, pctrl_desc, pinctrl);
> +	if (IS_ERR(pinctrl->pctl_dev))
> +		return dev_err_probe(dev, PTR_ERR(pinctrl->pctl_dev),
> +				     "Couldn't register pinctrl driver\n");
> +
> +	config.parent = tps->dev;
> +	config.regmap = tps->regmap;
> +	config.ngpio = TPS6594_PINCTRL_PINS_NB;
> +	config.ngpio_per_reg = 8;
> +	config.reg_dat_base = TPS6594_REG_GPIO_IN_1;
> +	config.reg_set_base = TPS6594_REG_GPIO_OUT_1;
> +	config.reg_dir_out_base = TPS6594_REG_GPIOX_CONF(0);
> +	config.reg_mask_xlate = tps6594_gpio_regmap_xlate;
> +
> +	pinctrl->gpio_regmap = devm_gpio_regmap_register(dev, &config);
> +	if (IS_ERR(pinctrl->gpio_regmap))
> +		return dev_err_probe(dev, PTR_ERR(pinctrl->gpio_regmap),
> +				     "Couldn't register gpio_regmap driver\n");
> +
> +	return 0;
> +}
> +
> +static struct platform_driver tps6594_pinctrl_driver = {
> +	.probe = tps6594_pinctrl_probe,
> +	.driver = {
> +		.name = "tps6594-pinctrl",
> +	},
> +};
> +module_platform_driver(tps6594_pinctrl_driver);
> +
> +MODULE_ALIAS("platform:tps6594-pinctrl");
> +MODULE_AUTHOR("Esteban Blanc <eblanc@baylibre.com>");
> +MODULE_DESCRIPTION("TPS6594 pinctrl and GPIO driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.41.0
> 

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 1/2] rtc: tps6594: Add driver for TPS6594 RTC
  2023-06-28 13:30 ` [PATCH v7 1/2] rtc: tps6594: Add driver for TPS6594 RTC Esteban Blanc
@ 2023-07-05  9:45   ` andy.shevchenko
  2023-10-01 21:20   ` Alexandre Belloni
  1 sibling, 0 replies; 7+ messages in thread
From: andy.shevchenko @ 2023-07-05  9:45 UTC (permalink / raw)
  To: Esteban Blanc
  Cc: linus.walleij, a.zummo, alexandre.belloni, linux-kernel,
	linux-gpio, linux-rtc, jpanis, jneanne, aseketeli, u-kumar1

Wed, Jun 28, 2023 at 03:30:20PM +0200, Esteban Blanc kirjoitti:
> TPS6594 PMIC is a MFD. This patch adds support for
> the RTC found inside TPS6594 family of PMIC.
> 
> Alarm is also supported.

LGTM from generic style and code comments perspective,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Esteban Blanc <eblanc@baylibre.com>
> ---
>  drivers/rtc/Kconfig       |  12 +
>  drivers/rtc/Makefile      |   1 +
>  drivers/rtc/rtc-tps6594.c | 452 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 465 insertions(+)
>  create mode 100644 drivers/rtc/rtc-tps6594.c
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 753872408615..39b5c93c6282 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -578,6 +578,18 @@ config RTC_DRV_TPS6586X
>  	  along with alarm. This driver supports the RTC driver for
>  	  the TPS6586X RTC module.
>  
> +config RTC_DRV_TPS6594
> +	tristate "TI TPS6594 RTC driver"
> +	depends on MFD_TPS6594
> +	default MFD_TPS6594
> +	help
> +	  TI Power Management IC TPS6594 supports RTC functionality
> +	  along with alarm. This driver supports the RTC driver for
> +	  the TPS6594 RTC module.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called rtc-tps6594.
> +
>  config RTC_DRV_TPS65910
>  	tristate "TI TPS65910 RTC driver"
>  	depends on MFD_TPS65910
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index ea445d1ebb17..3d3f8c9d0697 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -175,6 +175,7 @@ obj-$(CONFIG_RTC_DRV_TEGRA)	+= rtc-tegra.o
>  obj-$(CONFIG_RTC_DRV_TEST)	+= rtc-test.o
>  obj-$(CONFIG_RTC_DRV_TI_K3)	+= rtc-ti-k3.o
>  obj-$(CONFIG_RTC_DRV_TPS6586X)	+= rtc-tps6586x.o
> +obj-$(CONFIG_RTC_DRV_TPS6594)	+= rtc-tps6594.o
>  obj-$(CONFIG_RTC_DRV_TPS65910)	+= rtc-tps65910.o
>  obj-$(CONFIG_RTC_DRV_TWL4030)	+= rtc-twl.o
>  obj-$(CONFIG_RTC_DRV_VT8500)	+= rtc-vt8500.o
> diff --git a/drivers/rtc/rtc-tps6594.c b/drivers/rtc/rtc-tps6594.c
> new file mode 100644
> index 000000000000..7328ea7da849
> --- /dev/null
> +++ b/drivers/rtc/rtc-tps6594.c
> @@ -0,0 +1,452 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RTC driver for tps6594 PMIC
> + *
> + * Copyright (C) 2023 BayLibre Incorporated - https://www.baylibre.com/
> + */
> +
> +#include <linux/bcd.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/limits.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/rtc.h>
> +#include <linux/types.h>
> +#include <linux/units.h>
> +
> +#include <linux/mfd/tps6594.h>
> +
> +// Total number of RTC registers needed to set time
> +#define NUM_TIME_REGS (TPS6594_REG_RTC_WEEKS - TPS6594_REG_RTC_SECONDS + 1)
> +
> +// Total number of RTC alarm registers
> +#define NUM_TIME_ALARM_REGS (NUM_TIME_REGS - 1)
> +
> +/*
> + * Min and max values supported by 'offset' interface (swapped sign).
> + * After conversion, the values do not exceed the range [-32767, 33767]
> + * which COMP_REG must conform to.
> + */
> +#define MIN_OFFSET (-277774)
> +#define MAX_OFFSET (277774)
> +
> +// Number of ticks per hour
> +#define TICKS_PER_HOUR (32768 * 3600)
> +
> +// Multiplier for ppb conversions
> +#define PPB_MULT NANO
> +
> +static int tps6594_rtc_alarm_irq_enable(struct device *dev,
> +					unsigned int enabled)
> +{
> +	struct tps6594 *tps = dev_get_drvdata(dev->parent);
> +	u8 val;
> +
> +	val = enabled ? TPS6594_BIT_IT_ALARM : 0;
> +
> +	return regmap_update_bits(tps->regmap, TPS6594_REG_RTC_INTERRUPTS,
> +				  TPS6594_BIT_IT_ALARM, val);
> +}
> +
> +/* Pulse GET_TIME field of RTC_CTRL_1 to store a timestamp in shadow registers. */
> +static int tps6594_rtc_shadow_timestamp(struct device *dev, struct tps6594 *tps)
> +{
> +	int ret;
> +
> +	/*
> +	 * Set GET_TIME to 0. Next time we set GET_TIME to 1 we will be sure to store
> +	 * an up-to-date timestamp.
> +	 */
> +	ret = regmap_clear_bits(tps->regmap, TPS6594_REG_RTC_CTRL_1,
> +				TPS6594_BIT_GET_TIME);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * Copy content of RTC registers to shadow registers or latches to read
> +	 * a coherent timestamp.
> +	 */
> +	return regmap_set_bits(tps->regmap, TPS6594_REG_RTC_CTRL_1,
> +			       TPS6594_BIT_GET_TIME);
> +}
> +
> +static int tps6594_rtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> +	unsigned char rtc_data[NUM_TIME_REGS];
> +	struct tps6594 *tps = dev_get_drvdata(dev->parent);
> +	int ret;
> +
> +	// Check if RTC is running.
> +	ret = regmap_test_bits(tps->regmap, TPS6594_REG_RTC_STATUS,
> +			       TPS6594_BIT_RUN);
> +	if (ret < 0)
> +		return ret;
> +	if (ret == 0)
> +		return -EINVAL;
> +
> +	ret = tps6594_rtc_shadow_timestamp(dev, tps);
> +	if (ret < 0)
> +		return ret;
> +
> +	// Read shadowed RTC registers.
> +	ret = regmap_bulk_read(tps->regmap, TPS6594_REG_RTC_SECONDS, rtc_data,
> +			       NUM_TIME_REGS);
> +	if (ret < 0)
> +		return ret;
> +
> +	tm->tm_sec = bcd2bin(rtc_data[0]);
> +	tm->tm_min = bcd2bin(rtc_data[1]);
> +	tm->tm_hour = bcd2bin(rtc_data[2]);
> +	tm->tm_mday = bcd2bin(rtc_data[3]);
> +	tm->tm_mon = bcd2bin(rtc_data[4]) - 1;
> +	tm->tm_year = bcd2bin(rtc_data[5]) + 100;
> +	tm->tm_wday = bcd2bin(rtc_data[6]);
> +
> +	return 0;
> +}
> +
> +static int tps6594_rtc_set_time(struct device *dev, struct rtc_time *tm)
> +{
> +	unsigned char rtc_data[NUM_TIME_REGS];
> +	struct tps6594 *tps = dev_get_drvdata(dev->parent);
> +	int ret;
> +
> +	rtc_data[0] = bin2bcd(tm->tm_sec);
> +	rtc_data[1] = bin2bcd(tm->tm_min);
> +	rtc_data[2] = bin2bcd(tm->tm_hour);
> +	rtc_data[3] = bin2bcd(tm->tm_mday);
> +	rtc_data[4] = bin2bcd(tm->tm_mon + 1);
> +	rtc_data[5] = bin2bcd(tm->tm_year - 100);
> +	rtc_data[6] = bin2bcd(tm->tm_wday);
> +
> +	// Stop RTC while updating the RTC time registers.
> +	ret = regmap_clear_bits(tps->regmap, TPS6594_REG_RTC_CTRL_1,
> +				TPS6594_BIT_STOP_RTC);
> +	if (ret < 0)
> +		return ret;
> +
> +	// Update all the time registers in one shot.
> +	ret = regmap_bulk_write(tps->regmap, TPS6594_REG_RTC_SECONDS, rtc_data,
> +				NUM_TIME_REGS);
> +	if (ret < 0)
> +		return ret;
> +
> +	// Start back RTC.
> +	return regmap_set_bits(tps->regmap, TPS6594_REG_RTC_CTRL_1,
> +			       TPS6594_BIT_STOP_RTC);
> +}
> +
> +static int tps6594_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
> +{
> +	unsigned char alarm_data[NUM_TIME_ALARM_REGS];
> +	u32 int_val;
> +	struct tps6594 *tps = dev_get_drvdata(dev->parent);
> +	int ret;
> +
> +	ret = regmap_bulk_read(tps->regmap, TPS6594_REG_ALARM_SECONDS,
> +			       alarm_data, NUM_TIME_ALARM_REGS);
> +	if (ret < 0)
> +		return ret;
> +
> +	alm->time.tm_sec = bcd2bin(alarm_data[0]);
> +	alm->time.tm_min = bcd2bin(alarm_data[1]);
> +	alm->time.tm_hour = bcd2bin(alarm_data[2]);
> +	alm->time.tm_mday = bcd2bin(alarm_data[3]);
> +	alm->time.tm_mon = bcd2bin(alarm_data[4]) - 1;
> +	alm->time.tm_year = bcd2bin(alarm_data[5]) + 100;
> +
> +	ret = regmap_read(tps->regmap, TPS6594_REG_RTC_INTERRUPTS, &int_val);
> +	if (ret < 0)
> +		return ret;
> +
> +	alm->enabled = int_val & TPS6594_BIT_IT_ALARM;
> +
> +	return 0;
> +}
> +
> +static int tps6594_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
> +{
> +	unsigned char alarm_data[NUM_TIME_ALARM_REGS];
> +	struct tps6594 *tps = dev_get_drvdata(dev->parent);
> +	int ret;
> +
> +	// Disable alarm irq before changing the alarm timestamp.
> +	ret = tps6594_rtc_alarm_irq_enable(dev, 0);
> +	if (ret)
> +		return ret;
> +
> +	alarm_data[0] = bin2bcd(alm->time.tm_sec);
> +	alarm_data[1] = bin2bcd(alm->time.tm_min);
> +	alarm_data[2] = bin2bcd(alm->time.tm_hour);
> +	alarm_data[3] = bin2bcd(alm->time.tm_mday);
> +	alarm_data[4] = bin2bcd(alm->time.tm_mon + 1);
> +	alarm_data[5] = bin2bcd(alm->time.tm_year - 100);
> +
> +	// Update all the alarm registers in one shot.
> +	ret = regmap_bulk_write(tps->regmap, TPS6594_REG_ALARM_SECONDS,
> +				alarm_data, NUM_TIME_ALARM_REGS);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (alm->enabled)
> +		ret = tps6594_rtc_alarm_irq_enable(dev, 1);
> +
> +	return ret;
> +}
> +
> +static int tps6594_rtc_set_calibration(struct device *dev, int calibration)
> +{
> +	struct tps6594 *tps = dev_get_drvdata(dev->parent);
> +	__le16 value;
> +	int ret;
> +
> +	/*
> +	 * TPS6594 uses two's complement 16 bit value for compensation of RTC
> +	 * crystal inaccuracies. One time every hour when seconds counter
> +	 * increments from 0 to 1 compensation value will be added to internal
> +	 * RTC counter value.
> +	 *
> +	 * Valid range for compensation value: [-32767 .. 32767].
> +	 */
> +	if (calibration < S16_MIN + 1 || calibration > S16_MAX)
> +		return -ERANGE;
> +
> +	value = cpu_to_le16(calibration);
> +
> +	// Update all the compensation registers in one shot.
> +	ret = regmap_bulk_write(tps->regmap, TPS6594_REG_RTC_COMP_LSB, &value,
> +				sizeof(value));
> +	if (ret < 0)
> +		return ret;
> +
> +	// Enable automatic compensation.
> +	return regmap_set_bits(tps->regmap, TPS6594_REG_RTC_CTRL_1,
> +			       TPS6594_BIT_AUTO_COMP);
> +}
> +
> +static int tps6594_rtc_get_calibration(struct device *dev, int *calibration)
> +{
> +	struct tps6594 *tps = dev_get_drvdata(dev->parent);
> +	unsigned int ctrl;
> +	__le16 value;
> +	int ret;
> +
> +	ret = regmap_read(tps->regmap, TPS6594_REG_RTC_CTRL_1, &ctrl);
> +	if (ret < 0)
> +		return ret;
> +
> +	// If automatic compensation is not enabled report back zero.
> +	if (!(ctrl & TPS6594_BIT_AUTO_COMP)) {
> +		*calibration = 0;
> +		return 0;
> +	}
> +
> +	ret = regmap_bulk_read(tps->regmap, TPS6594_REG_RTC_COMP_LSB, &value,
> +			       sizeof(value));
> +	if (ret < 0)
> +		return ret;
> +
> +	*calibration = le16_to_cpu(value);
> +
> +	return 0;
> +}
> +
> +static int tps6594_rtc_read_offset(struct device *dev, long *offset)
> +{
> +	int calibration;
> +	s64 tmp;
> +	int ret;
> +
> +	ret = tps6594_rtc_get_calibration(dev, &calibration);
> +	if (ret < 0)
> +		return ret;
> +
> +	// Convert from RTC calibration register format to ppb format.
> +	tmp = calibration * PPB_MULT;
> +
> +	if (tmp < 0)
> +		tmp -= TICKS_PER_HOUR / 2LL;
> +	else
> +		tmp += TICKS_PER_HOUR / 2LL;
> +	tmp = div_s64(tmp, TICKS_PER_HOUR);
> +
> +	/*
> +	 * SAFETY:
> +	 * Compution is the reverse operation of the one done in
> +	 * `tps6594_rtc_set_offset`. The safety remarks applie here too.
> +	 */
> +
> +	/*
> +	 * Offset value operates in negative way, so swap sign.
> +	 * See 8.3.10.5, (32768 - COMP_REG).
> +	 */
> +	*offset = (long)-tmp;
> +
> +	return 0;
> +}
> +
> +static int tps6594_rtc_set_offset(struct device *dev, long offset)
> +{
> +	int calibration;
> +	s64 tmp;
> +
> +	// Make sure offset value is within supported range.
> +	if (offset < MIN_OFFSET || offset > MAX_OFFSET)
> +		return -ERANGE;
> +
> +	// Convert from ppb format to RTC calibration register format.
> +
> +	tmp = offset * TICKS_PER_HOUR;
> +	if (tmp < 0)
> +		tmp -= PPB_MULT / 2LL;
> +	else
> +		tmp += PPB_MULT / 2LL;
> +	tmp = div_s64(tmp, PPB_MULT);
> +
> +	/*
> +	 * SAFETY:
> +	 * - tmp = offset * TICK_PER_HOUR :
> +	 *	`offset` can't be more than 277774, so `tmp` can't exceed 277774000000000
> +	 *	which is lower than the maximum value in an `s64` (2^63-1). No overflow here.
> +	 *
> +	 * - tmp += TICK_PER_HOUR / 2LL :
> +	 *	tmp will have a maximum value of 277774117964800 which is still inferior to 2^63-1.
> +	 */
> +
> +	// Offset value operates in negative way, so swap sign.
> +	calibration = (int)-tmp;
> +
> +	return tps6594_rtc_set_calibration(dev, calibration);
> +}
> +
> +static irqreturn_t tps6594_rtc_interrupt(int irq, void *rtc)
> +{
> +	struct device *dev = rtc;
> +	unsigned long events = 0;
> +	struct tps6594 *tps = dev_get_drvdata(dev->parent);
> +	struct rtc_device *rtc_dev = dev_get_drvdata(dev);
> +	int ret;
> +	u32 rtc_reg;
> +
> +	ret = regmap_read(tps->regmap, TPS6594_REG_RTC_STATUS, &rtc_reg);
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	if (rtc_reg & TPS6594_BIT_ALARM)
> +		events = RTC_IRQF | RTC_AF;
> +
> +	// Notify RTC core on event.
> +	rtc_update_irq(rtc_dev, 1, events);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static const struct rtc_class_ops tps6594_rtc_ops = {
> +	.read_time = tps6594_rtc_read_time,
> +	.set_time = tps6594_rtc_set_time,
> +	.read_alarm = tps6594_rtc_read_alarm,
> +	.set_alarm = tps6594_rtc_set_alarm,
> +	.alarm_irq_enable = tps6594_rtc_alarm_irq_enable,
> +	.read_offset = tps6594_rtc_read_offset,
> +	.set_offset = tps6594_rtc_set_offset,
> +};
> +
> +static int tps6594_rtc_probe(struct platform_device *pdev)
> +{
> +	struct tps6594 *tps = dev_get_drvdata(pdev->dev.parent);
> +	struct device *dev = &pdev->dev;
> +	struct rtc_device *rtc;
> +	int irq;
> +	int ret;
> +
> +	rtc = devm_kzalloc(dev, sizeof(*rtc), GFP_KERNEL);
> +	if (!rtc)
> +		return -ENOMEM;
> +
> +	rtc = devm_rtc_allocate_device(dev);
> +	if (IS_ERR(rtc))
> +		return PTR_ERR(rtc);
> +
> +	// Enable crystal oscillator.
> +	ret = regmap_set_bits(tps->regmap, TPS6594_REG_RTC_CTRL_2,
> +			      TPS6594_BIT_XTAL_EN);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_test_bits(tps->regmap, TPS6594_REG_RTC_STATUS,
> +			       TPS6594_BIT_RUN);
> +	if (ret < 0)
> +		return ret;
> +	// RTC not running.
> +	if (ret == 0) {
> +		ret = regmap_set_bits(tps->regmap, TPS6594_REG_RTC_CTRL_1,
> +				      TPS6594_BIT_STOP_RTC);
> +		if (ret < 0)
> +			return ret;
> +
> +		/*
> +		 * On some boards, a 40 ms delay is needed before BIT_RUN is set.
> +		 * 80 ms should provide sufficient margin.
> +		 */
> +		mdelay(80);
> +
> +		/*
> +		 * RTC should be running now. Check if this is the case.
> +		 * If not it might be a missing oscillator.
> +		 */
> +		ret = regmap_test_bits(tps->regmap, TPS6594_REG_RTC_STATUS,
> +				       TPS6594_BIT_RUN);
> +		if (ret < 0)
> +			return ret;
> +		if (ret == 0)
> +			return -ENODEV;
> +
> +		// Stop RTC until first call to `tps6594_rtc_set_time`.
> +		ret = regmap_clear_bits(tps->regmap, TPS6594_REG_RTC_CTRL_1,
> +					TPS6594_BIT_STOP_RTC);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, rtc);
> +
> +	irq = platform_get_irq_byname(pdev, TPS6594_IRQ_NAME_ALARM);
> +	if (irq < 0)
> +		return dev_err_probe(dev, irq, "Failed to get irq\n");
> +
> +	ret = devm_request_threaded_irq(dev, irq, NULL, tps6594_rtc_interrupt,
> +					IRQF_ONESHOT, TPS6594_IRQ_NAME_ALARM,
> +					dev);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to request_threaded_irq\n");
> +
> +	ret = device_init_wakeup(dev, true);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to init rtc as wakeup source\n");
> +
> +	rtc->ops = &tps6594_rtc_ops;
> +	rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
> +	rtc->range_max = RTC_TIMESTAMP_END_2099;
> +
> +	return devm_rtc_register_device(rtc);
> +}
> +
> +static struct platform_driver tps6594_rtc_driver = {
> +	.probe		= tps6594_rtc_probe,
> +	.driver		= {
> +		.name	= "tps6594-rtc",
> +	},
> +};
> +
> +module_platform_driver(tps6594_rtc_driver);
> +MODULE_ALIAS("platform:tps6594-rtc");
> +MODULE_AUTHOR("Esteban Blanc <eblanc@baylibre.com>");
> +MODULE_DESCRIPTION("TPS6594 RTC driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.41.0
> 

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 1/2] rtc: tps6594: Add driver for TPS6594 RTC
  2023-06-28 13:30 ` [PATCH v7 1/2] rtc: tps6594: Add driver for TPS6594 RTC Esteban Blanc
  2023-07-05  9:45   ` andy.shevchenko
@ 2023-10-01 21:20   ` Alexandre Belloni
  1 sibling, 0 replies; 7+ messages in thread
From: Alexandre Belloni @ 2023-10-01 21:20 UTC (permalink / raw)
  To: Esteban Blanc
  Cc: linus.walleij, a.zummo, linux-kernel, linux-gpio, linux-rtc,
	jpanis, jneanne, aseketeli, u-kumar1

Hello,

What is the status of this series?

On 28/06/2023 15:30:20+0200, Esteban Blanc wrote:
> +static int tps6594_rtc_read_offset(struct device *dev, long *offset)
> +{
> +	int calibration;
> +	s64 tmp;
> +	int ret;
> +
> +	ret = tps6594_rtc_get_calibration(dev, &calibration);
> +	if (ret < 0)
> +		return ret;
> +
> +	// Convert from RTC calibration register format to ppb format.
> +	tmp = calibration * PPB_MULT;
> +
> +	if (tmp < 0)
> +		tmp -= TICKS_PER_HOUR / 2LL;
> +	else
> +		tmp += TICKS_PER_HOUR / 2LL;
> +	tmp = div_s64(tmp, TICKS_PER_HOUR);
> +
> +	/*
> +	 * SAFETY:
> +	 * Compution is the reverse operation of the one done in

Small typo -^

> +	 * `tps6594_rtc_set_offset`. The safety remarks applie here too.
> +	 */
> +
> +	/*
> +	 * Offset value operates in negative way, so swap sign.
> +	 * See 8.3.10.5, (32768 - COMP_REG).
> +	 */
> +	*offset = (long)-tmp;
> +
> +	return 0;
> +}
> +
> +static int tps6594_rtc_set_offset(struct device *dev, long offset)
> +{
> +	int calibration;
> +	s64 tmp;
> +
> +	// Make sure offset value is within supported range.
> +	if (offset < MIN_OFFSET || offset > MAX_OFFSET)
> +		return -ERANGE;
> +
> +	// Convert from ppb format to RTC calibration register format.
> +
> +	tmp = offset * TICKS_PER_HOUR;
> +	if (tmp < 0)
> +		tmp -= PPB_MULT / 2LL;
> +	else
> +		tmp += PPB_MULT / 2LL;
> +	tmp = div_s64(tmp, PPB_MULT);
> +
> +	/*
> +	 * SAFETY:
> +	 * - tmp = offset * TICK_PER_HOUR :
> +	 *	`offset` can't be more than 277774, so `tmp` can't exceed 277774000000000
> +	 *	which is lower than the maximum value in an `s64` (2^63-1). No overflow here.
> +	 *
> +	 * - tmp += TICK_PER_HOUR / 2LL :
> +	 *	tmp will have a maximum value of 277774117964800 which is still inferior to 2^63-1.
> +	 */
> +
> +	// Offset value operates in negative way, so swap sign.
> +	calibration = (int)-tmp;
> +
> +	return tps6594_rtc_set_calibration(dev, calibration);
> +}
> +
> +static irqreturn_t tps6594_rtc_interrupt(int irq, void *rtc)
> +{
> +	struct device *dev = rtc;
> +	unsigned long events = 0;
> +	struct tps6594 *tps = dev_get_drvdata(dev->parent);
> +	struct rtc_device *rtc_dev = dev_get_drvdata(dev);
> +	int ret;
> +	u32 rtc_reg;
> +
> +	ret = regmap_read(tps->regmap, TPS6594_REG_RTC_STATUS, &rtc_reg);
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	if (rtc_reg & TPS6594_BIT_ALARM)
> +		events = RTC_IRQF | RTC_AF;
> +
> +	// Notify RTC core on event.

Nit: I don't feel like the events varialbe and the comment are
necessary.

> +	rtc_update_irq(rtc_dev, 1, events);
> +
> +	return IRQ_HANDLED;
> +}
> +

If you resend, you can add:

Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-10-01 21:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-28 13:30 [PATCH v7 0/2] TI TPS6594 PMIC support (RTC, pinctrl, regulators) Esteban Blanc
2023-06-28 13:30 ` [PATCH v7 1/2] rtc: tps6594: Add driver for TPS6594 RTC Esteban Blanc
2023-07-05  9:45   ` andy.shevchenko
2023-10-01 21:20   ` Alexandre Belloni
2023-06-28 13:30 ` [PATCH v7 2/2] pinctrl: tps6594: Add driver for TPS6594 pinctrl and GPIOs Esteban Blanc
2023-06-29  7:00   ` Linus Walleij
2023-07-03 21:47   ` andy.shevchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.