* [PATCH 11/13] Documentation: leds: document pattern behavior of Samsung S2M series PMIC RGB LEDs
From: Kaustabh Chakraborty @ 2025-11-13 19:05 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, MyungJoo Ham, Chanwoo Choi, Sebastian Reichel,
Krzysztof Kozlowski, André Draszik, Alexandre Belloni,
Jonathan Corbet
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc, Kaustabh Chakraborty
In-Reply-To: <20251114-s2mu005-pmic-v1-0-9e3184d3a0c9@disroot.org>
Add documentation to describe how hardware patterns (as defined by the
documentation of led-class-multicolor) are parsed and implemented by the
Samsung S2M series PMIC RGB LED driver.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
Documentation/leds/index.rst | 1 +
Documentation/leds/leds-s2m-rgb.rst | 60 +++++++++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+)
diff --git a/Documentation/leds/index.rst b/Documentation/leds/index.rst
index 76fae171039c..05d8e8517a80 100644
--- a/Documentation/leds/index.rst
+++ b/Documentation/leds/index.rst
@@ -27,6 +27,7 @@ LEDs
leds-lp55xx
leds-mlxcpld
leds-mt6370-rgb
+ leds-s2m-rgb
leds-sc27xx
leds-st1202
leds-qcom-lpg
diff --git a/Documentation/leds/leds-s2m-rgb.rst b/Documentation/leds/leds-s2m-rgb.rst
new file mode 100644
index 000000000000..cf91f0238093
--- /dev/null
+++ b/Documentation/leds/leds-s2m-rgb.rst
@@ -0,0 +1,60 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+======================================
+Samsung S2M Series PMIC RGB LED Driver
+======================================
+
+Description
+-----------
+
+The RGB LED on the S2M series PMIC hardware features a three-channel LED that is
+grouped together as a single device. Furthermore, the it supports 8-bit
+brightness control for each channel. This LED is typically used as a status
+indicator in mobile devices. It also supports various parameters for hardware
+patterns.
+
+The hardware pattern can be programmed using the "pattern" trigger, using the
+hw_pattern attribute.
+
+/sys/class/leds/<led>/repeat
+----------------------------
+
+The hardware supports only indefinitely repeating patterns. The repeat
+attribute must be set to -1 for hardware patterns to function.
+
+/sys/class/leds/<led>/hw_pattern
+--------------------------------
+
+Specify a hardware pattern for the RGB LEDs.
+
+The pattern is a series of brightness levels and durations in milliseconds.
+There should be only one non-zero brightness level. Unlike the results
+described in leds-trigger-pattern, the transitions between on and off states
+are smoothed out by the hardware.
+
+Simple pattern::
+
+ "255 3000 0 1000"
+
+ 255 -+ ''''''-. .-'''''''-.
+ | '. .' '.
+ | \ / \
+ | '. .' '.
+ | '-.......-' '-
+ 0 -+-------+-------+-------+-------+-------+-------+--> time (s)
+ 0 1 2 3 4 5 6
+
+As described in leds-trigger-pattern, it is also possible to use zero-length
+entries to disable the ramping mechanism.
+
+On-Off pattern::
+
+ "255 1000 255 0 0 1000 0 0"
+
+ 255 -+ ------+ +-------+ +-------+
+ | | | | | |
+ | | | | | |
+ | | | | | |
+ | +-------+ +-------+ +-------
+ 0 -+-------+-------+-------+-------+-------+-------+--> time (s)
+ 0 1 2 3 4 5 6
--
2.51.2
^ permalink raw reply related
* [PATCH 12/13] extcon: add support for Samsung S2M series PMIC extcon devices
From: Kaustabh Chakraborty @ 2025-11-13 19:05 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, MyungJoo Ham, Chanwoo Choi, Sebastian Reichel,
Krzysztof Kozlowski, André Draszik, Alexandre Belloni,
Jonathan Corbet
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc, Kaustabh Chakraborty
In-Reply-To: <20251114-s2mu005-pmic-v1-0-9e3184d3a0c9@disroot.org>
Add a driver for MUIC devices found in certain Samsung S2M series PMICs
These are USB port accessory detectors. These devices report multiple
cable states depending on the ID-GND resistance measured by an internal
ADC.
The driver includes initial support for the S2MU005 PMIC extcon.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
drivers/extcon/Kconfig | 10 ++
drivers/extcon/Makefile | 1 +
drivers/extcon/extcon-s2m.c | 355 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 366 insertions(+)
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index aec46bf03302..89b3427175f7 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -182,6 +182,16 @@ config EXTCON_RT8973A
and switch that is optimized to protect low voltage system
from abnormal high input voltage (up to 28V).
+config EXTCON_S2M
+ tristate "Samsung S2M series PMIC EXTCON support"
+ depends on MFD_SEC_CORE
+ select REGMAP_IRQ
+ help
+ This option enables support for MUIC devices found in certain
+ Samsung S2M series PMICs, such as the S2MU005. These devices
+ have internal ADCs measuring the ID-GND resistance, thereby
+ can be used as a USB port accessory detector.
+
config EXTCON_SM5502
tristate "Silicon Mitus SM5502/SM5504/SM5703 EXTCON support"
depends on I2C
diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
index 6482f2bfd661..e3939786f347 100644
--- a/drivers/extcon/Makefile
+++ b/drivers/extcon/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_EXTCON_PALMAS) += extcon-palmas.o
obj-$(CONFIG_EXTCON_PTN5150) += extcon-ptn5150.o
obj-$(CONFIG_EXTCON_QCOM_SPMI_MISC) += extcon-qcom-spmi-misc.o
obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o
+obj-$(CONFIG_EXTCON_S2M) += extcon-s2m.o
obj-$(CONFIG_EXTCON_SM5502) += extcon-sm5502.o
obj-$(CONFIG_EXTCON_USB_GPIO) += extcon-usb-gpio.o
obj-$(CONFIG_EXTCON_USBC_CROS_EC) += extcon-usbc-cros-ec.o
diff --git a/drivers/extcon/extcon-s2m.c b/drivers/extcon/extcon-s2m.c
new file mode 100644
index 000000000000..b27ee9e79bb6
--- /dev/null
+++ b/drivers/extcon/extcon-s2m.c
@@ -0,0 +1,355 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Extcon Driver for Samsung S2M series PMICs.
+ *
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd
+ * Copyright (C) 2025 Kaustabh Chakraborty <kauschluss@disroot.org>
+ */
+
+#include <linux/delay.h>
+#include <linux/extcon-provider.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/mfd/samsung/core.h>
+#include <linux/mfd/samsung/irq.h>
+#include <linux/mfd/samsung/s2mu005.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+
+struct s2m_muic {
+ struct device *dev;
+ struct regmap *regmap;
+ struct extcon_dev *extcon;
+ struct s2m_muic_irq_data *irq_data;
+ const unsigned int *extcon_cable;
+ bool attached;
+};
+
+struct s2m_muic_irq_data {
+ const enum s2mu005_muic_irq irq;
+ const char *name;
+ int (* const handler)(struct s2m_muic *);
+ int virq;
+};
+
+static int s2mu005_muic_detach(struct s2m_muic *priv)
+{
+ int ret;
+ int i;
+
+ ret = regmap_set_bits(priv->regmap, S2MU005_REG_MUIC_CTRL1,
+ S2MU005_MUIC_MAN_SW);
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to disable manual switching\n");
+ return ret;
+ }
+
+ ret = regmap_set_bits(priv->regmap, S2MU005_REG_MUIC_CTRL3,
+ S2MU005_MUIC_ONESHOT_ADC);
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to enable ADC oneshot mode\n");
+ return ret;
+ }
+
+ ret = regmap_clear_bits(priv->regmap, S2MU005_REG_MUIC_SWCTRL, ~0);
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to clear switch control register\n");
+ return ret;
+ }
+
+ /* Find all set states and clear them */
+ for (i = 0; priv->extcon_cable[i]; i++) {
+ unsigned int state = priv->extcon_cable[i];
+
+ if (extcon_get_state(priv->extcon, state) == true)
+ extcon_set_state_sync(priv->extcon, state, false);
+ }
+
+ priv->attached = false;
+
+ return 0;
+}
+
+static int s2mu005_muic_attach(struct s2m_muic *priv)
+{
+ unsigned int type;
+ int ret;
+
+ /* If any device is already attached, detach it */
+ if (priv->attached) {
+ s2mu005_muic_detach(priv);
+ msleep(100);
+ }
+
+ ret = regmap_read(priv->regmap, S2MU005_REG_MUIC_DEV1, &type);
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to read DEV1 register\n");
+ return ret;
+ }
+
+ /*
+ * All USB connections which require communication via its D+
+ * and D- wires need it.
+ */
+ if (type & (S2MU005_MUIC_OTG | S2MU005_MUIC_DCP | S2MU005_MUIC_SDP)) {
+ ret = regmap_update_bits(priv->regmap, S2MU005_REG_MUIC_SWCTRL,
+ S2MU005_MUIC_DM_DP,
+ S2MU005_MUIC_DM_DP_USB);
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to configure DM/DP pins\n");
+ return ret;
+ }
+ }
+
+ /*
+ * For OTG connections, enable manual switching and ADC oneshot
+ * mode. Since the port will now be supplying power, the
+ * internal ADC (measuring the ID-GND resistance) is made to
+ * poll periodically for any changes, so as to prevent any
+ * damages due to power.
+ */
+ if (type & S2MU005_MUIC_OTG) {
+ ret = regmap_clear_bits(priv->regmap, S2MU005_REG_MUIC_CTRL1,
+ S2MU005_MUIC_MAN_SW);
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to enable manual switching\n");
+ return ret;
+ }
+
+ ret = regmap_clear_bits(priv->regmap, S2MU005_REG_MUIC_CTRL3,
+ S2MU005_MUIC_ONESHOT_ADC);
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to disable ADC oneshot mode\n");
+ return ret;
+ }
+ }
+
+ switch (type) {
+ case S2MU005_MUIC_OTG:
+ dev_dbg(priv->dev, "USB OTG connection detected\n");
+ extcon_set_state_sync(priv->extcon, EXTCON_USB_HOST, true);
+ priv->attached = true;
+ break;
+ case S2MU005_MUIC_CDP:
+ dev_dbg(priv->dev, "USB CDP connection detected\n");
+ extcon_set_state_sync(priv->extcon, EXTCON_USB, true);
+ extcon_set_state_sync(priv->extcon, EXTCON_CHG_USB_CDP, true);
+ priv->attached = true;
+ break;
+ case S2MU005_MUIC_SDP:
+ dev_dbg(priv->dev, "USB SDP connection detected\n");
+ extcon_set_state_sync(priv->extcon, EXTCON_USB, true);
+ extcon_set_state_sync(priv->extcon, EXTCON_CHG_USB_SDP, true);
+ priv->attached = true;
+ break;
+ case S2MU005_MUIC_DCP:
+ dev_dbg(priv->dev, "USB DCP connection detected\n");
+ extcon_set_state_sync(priv->extcon, EXTCON_USB, true);
+ extcon_set_state_sync(priv->extcon, EXTCON_CHG_USB_DCP, true);
+ priv->attached = true;
+ break;
+ case S2MU005_MUIC_UART:
+ dev_dbg(priv->dev, "UART connection detected\n");
+ extcon_set_state_sync(priv->extcon, EXTCON_JIG, true);
+ priv->attached = true;
+ break;
+ }
+
+ if (!priv->attached)
+ dev_warn(priv->dev, "failed to recognize the device attached\n");
+
+ return ret;
+}
+
+static int s2mu005_muic_init(struct s2m_muic *priv)
+{
+ int ret = 0;
+
+ ret = regmap_update_bits(priv->regmap, S2MU005_REG_MUIC_LDOADC_L,
+ S2MU005_MUIC_VSET, S2MU005_MUIC_VSET_3P0V);
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to set internal ADC voltage regulator\n");
+ return ret;
+ }
+
+ ret = regmap_update_bits(priv->regmap, S2MU005_REG_MUIC_LDOADC_H,
+ S2MU005_MUIC_VSET, S2MU005_MUIC_VSET_3P0V);
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to set internal ADC voltage regulator\n");
+ return ret;
+ }
+
+ ret = regmap_clear_bits(priv->regmap, S2MU005_REG_MUIC_CTRL1,
+ S2MU005_MUIC_IRQ);
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to enable MUIC interrupts\n");
+ return ret;
+ }
+
+ return s2mu005_muic_attach(priv);
+}
+
+static const unsigned int s2mu005_muic_extcon_cable[] = {
+ EXTCON_USB,
+ EXTCON_USB_HOST,
+ EXTCON_CHG_USB_SDP,
+ EXTCON_CHG_USB_DCP,
+ EXTCON_CHG_USB_CDP,
+ EXTCON_JIG,
+ EXTCON_NONE,
+};
+
+static struct s2m_muic_irq_data s2mu005_muic_irq_data[] = {
+ {
+ .irq = S2MU005_MUIC_IRQ_ATTACH,
+ .name = "s2mu005-muic-attach",
+ .handler = s2mu005_muic_attach
+ }, {
+ .irq = S2MU005_MUIC_IRQ_DETACH,
+ .name = "s2mu005-muic-detach",
+ .handler = s2mu005_muic_detach
+ }, {
+ /* sentinel */
+ }
+};
+
+static irqreturn_t s2m_muic_irq_func(int virq, void *data)
+{
+ struct s2m_muic *priv = data;
+ const struct s2m_muic_irq_data *irq_data = priv->irq_data;
+ int ret;
+ int i;
+
+ for (i = 0; irq_data[i].handler; i++) {
+ if (virq != irq_data[i].virq)
+ continue;
+
+ ret = irq_data[i].handler(priv);
+ if (ret < 0)
+ dev_err(priv->dev, "failed to handle interrupt for %s (%d)\n",
+ irq_data[i].name, ret);
+ break;
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int s2m_muic_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct sec_pmic_dev *pmic_drvdata = dev_get_drvdata(dev->parent);
+ struct s2m_muic *priv;
+ struct regmap_irq_chip_data *irq_chip_data;
+ int ret;
+ int i;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return dev_err_probe(dev, -ENOMEM, "failed to allocate driver private\n");
+
+ platform_set_drvdata(pdev, priv);
+ priv->dev = dev;
+ priv->regmap = pmic_drvdata->regmap_pmic;
+
+ switch (platform_get_device_id(pdev)->driver_data) {
+ case S2MU005:
+ irq_chip_data = pmic_drvdata->irq_data[S2MU005_MUIC_IRQ_CHIP];
+ priv->irq_data = s2mu005_muic_irq_data;
+ priv->extcon_cable = s2mu005_muic_extcon_cable;
+ /* Initialize MUIC */
+ ret = s2mu005_muic_init(priv);
+ break;
+ default:
+ return dev_err_probe(dev, -ENODEV,
+ "device type %d is not supported by driver\n",
+ pmic_drvdata->device_type);
+ }
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "failed to initialize MUIC\n");
+
+ priv->extcon = devm_extcon_dev_allocate(&pdev->dev, priv->extcon_cable);
+ if (IS_ERR(priv->extcon))
+ return dev_err_probe(dev, PTR_ERR(priv->extcon),
+ "failed to allocate memory for extcon\n");
+
+ ret = devm_extcon_dev_register(dev, priv->extcon);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to register extcon device\n");
+
+ for (i = 0; priv->irq_data[i].handler; i++) {
+ int virq = regmap_irq_get_virq(irq_chip_data,
+ priv->irq_data[i].irq);
+ if (virq <= 0)
+ return dev_err_probe(dev, -EINVAL, "failed to get virtual IRQ\n");
+
+ priv->irq_data[i].virq = virq;
+ ret = devm_request_threaded_irq(dev, virq, NULL,
+ s2m_muic_irq_func, IRQF_ONESHOT,
+ priv->irq_data[i].name, priv);
+ if (ret)
+ dev_err_probe(dev, ret, "failed to request IRQ\n");
+ }
+
+ return 0;
+}
+
+static void s2m_muic_remove(struct platform_device *pdev)
+{
+ struct s2m_muic *priv = dev_get_drvdata(&pdev->dev);
+
+ /*
+ * Disabling the MUIC device is important as it disables manual
+ * switching mode, thereby enabling auto switching mode.
+ *
+ * This is to ensure that when the board is powered off, it
+ * goes into LPM charging mode when a USB charger is connected.
+ */
+ switch (platform_get_device_id(pdev)->driver_data) {
+ case S2MU005:
+ s2mu005_muic_detach(priv);
+ break;
+ }
+}
+
+static const struct platform_device_id s2m_muic_id_table[] = {
+ { "s2mu005-muic", S2MU005 },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(platform, s2m_muic_id_table);
+
+#ifdef CONFIG_OF
+/*
+ * Device is instantiated through parent MFD device and device matching
+ * is done through platform_device_id.
+ *
+ * However if device's DT node contains proper compatible and driver is
+ * built as a module, then the *module* matching will be done through DT
+ * aliases. This requires of_device_id table. In the same time this will
+ * not change the actual *device* matching so do not add .of_match_table.
+ */
+static const struct of_device_id s2m_muic_of_match_table[] = {
+ {
+ .compatible = "samsung,s2mu005-muic",
+ .data = (void *)S2MU005,
+ }, {
+ /* sentinel */
+ },
+};
+MODULE_DEVICE_TABLE(of, s2m_muic_of_match_table);
+#endif
+
+static struct platform_driver s2m_muic_driver = {
+ .driver = {
+ .name = "s2m-muic",
+ },
+ .probe = s2m_muic_probe,
+ .remove = s2m_muic_remove,
+ .id_table = s2m_muic_id_table,
+};
+module_platform_driver(s2m_muic_driver);
+
+MODULE_DESCRIPTION("Extcon Driver For Samsung S2M Series PMICs");
+MODULE_AUTHOR("Kaustabh Chakraborty <kauschluss@disroot.org>");
+MODULE_LICENSE("GPL");
--
2.51.2
^ permalink raw reply related
* [PATCH 13/13] power: supply: add support for Samsung S2M series PMIC charger device
From: Kaustabh Chakraborty @ 2025-11-13 19:05 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, MyungJoo Ham, Chanwoo Choi, Sebastian Reichel,
Krzysztof Kozlowski, André Draszik, Alexandre Belloni,
Jonathan Corbet
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc, Kaustabh Chakraborty
In-Reply-To: <20251114-s2mu005-pmic-v1-0-9e3184d3a0c9@disroot.org>
Add a driver for charger controllers found in certain Samsung S2M series
PMICs. The driver has very basic support for the device, with only
charger online reporting working.
The driver includes initial support for the S2MU005 PMIC charger.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
drivers/extcon/extcon-s2m.c | 2 +-
drivers/power/supply/Kconfig | 11 ++
drivers/power/supply/Makefile | 1 +
drivers/power/supply/s2m-charger.c | 216 +++++++++++++++++++++++++++++++++++++
4 files changed, 229 insertions(+), 1 deletion(-)
diff --git a/drivers/extcon/extcon-s2m.c b/drivers/extcon/extcon-s2m.c
index b27ee9e79bb6..268ad1f65ffd 100644
--- a/drivers/extcon/extcon-s2m.c
+++ b/drivers/extcon/extcon-s2m.c
@@ -289,7 +289,7 @@ static int s2m_muic_probe(struct platform_device *pdev)
s2m_muic_irq_func, IRQF_ONESHOT,
priv->irq_data[i].name, priv);
if (ret)
- dev_err_probe(dev, ret, "failed to request IRQ\n");
+ return dev_err_probe(dev, ret, "failed to request IRQ\n");
}
return 0;
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 03c8525b480f..f8742d35a288 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -229,6 +229,17 @@ config BATTERY_SAMSUNG_SDI
Say Y to enable support for Samsung SDI battery data.
These batteries are used in Samsung mobile phones.
+config CHARGER_S2M
+ tristate "Samsung S2M series PMIC battery charger support"
+ depends on EXTCON_S2M
+ depends on MFD_SEC_CORE
+ select REGMAP_IRQ
+ help
+ This option enables support for charger devices found in
+ certain Samsung S2M series PMICs, such as the S2MU005. These
+ devices provide USB power supply information and also required
+ for USB OTG role switching.
+
config BATTERY_COLLIE
tristate "Sharp SL-5500 (collie) battery"
depends on SA1100_COLLIE && MCP_UCB1200
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 6e37a3edf7e3..dcfae32d1216 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_BATTERY_PMU) += pmu_battery.o
obj-$(CONFIG_BATTERY_QCOM_BATTMGR) += qcom_battmgr.o
obj-$(CONFIG_BATTERY_OLPC) += olpc_battery.o
obj-$(CONFIG_BATTERY_SAMSUNG_SDI) += samsung-sdi-battery.o
+obj-$(CONFIG_CHARGER_S2M) += s2m-charger.o
obj-$(CONFIG_BATTERY_COLLIE) += collie_battery.o
obj-$(CONFIG_BATTERY_INGENIC) += ingenic-battery.o
obj-$(CONFIG_BATTERY_INTEL_DC_TI) += intel_dc_ti_battery.o
diff --git a/drivers/power/supply/s2m-charger.c b/drivers/power/supply/s2m-charger.c
new file mode 100644
index 000000000000..0a3216a2b545
--- /dev/null
+++ b/drivers/power/supply/s2m-charger.c
@@ -0,0 +1,216 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Battery Charger Driver for Samsung S2M series PMICs.
+ *
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd
+ * Copyright (c) 2025 Kaustabh Chakraborty <kauschluss@disroot.org>
+ */
+
+#include <linux/devm-helpers.h>
+#include <linux/delay.h>
+#include <linux/extcon.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/mfd/samsung/core.h>
+#include <linux/mfd/samsung/irq.h>
+#include <linux/mfd/samsung/s2mu005.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+
+struct s2m_chgr {
+ struct device *dev;
+ struct regmap *regmap;
+ struct power_supply *psy;
+ struct extcon_dev *extcon;
+ struct work_struct extcon_work;
+ struct notifier_block extcon_nb;
+};
+
+static int s2mu005_chgr_get_online(struct s2m_chgr *priv, int *value)
+{
+ u32 val;
+ int ret = 0;
+
+ ret = regmap_read(priv->regmap, S2MU005_REG_CHGR_STATUS0, &val);
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to read register (%d)\n", ret);
+ return ret;
+ }
+
+ *value = !!(val & S2MU005_CHGR_CHG);
+
+ return ret;
+}
+
+static int s2mu005_chgr_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct s2m_chgr *priv = power_supply_get_drvdata(psy);
+ int ret = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_ONLINE:
+ ret = s2mu005_chgr_get_online(priv, &val->intval);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static void s2mu005_chgr_extcon_work(struct work_struct *work)
+{
+ struct s2m_chgr *priv = container_of(work, struct s2m_chgr,
+ extcon_work);
+ int ret;
+
+ if (extcon_get_state(priv->extcon, EXTCON_USB_HOST) == true) {
+ ret = regmap_update_bits(priv->regmap, S2MU005_REG_CHGR_CTRL0,
+ S2MU005_CHGR_OP_MODE,
+ S2MU005_CHGR_OP_MODE_OTG);
+ if (ret < 0)
+ dev_err(priv->dev, "failed to set operation mode to OTG (%d)\n",
+ ret);
+
+ goto psy_update;
+ }
+
+ if (extcon_get_state(priv->extcon, EXTCON_USB) == true) {
+ ret = regmap_update_bits(priv->regmap, S2MU005_REG_CHGR_CTRL0,
+ S2MU005_CHGR_OP_MODE,
+ S2MU005_CHGR_OP_MODE_CHG);
+ if (ret < 0)
+ dev_err(priv->dev, "failed to set operation mode to charging (%d)\n",
+ ret);
+
+ goto psy_update;
+ }
+
+ ret = regmap_clear_bits(priv->regmap, S2MU005_REG_CHGR_CTRL0,
+ S2MU005_CHGR_OP_MODE);
+ if (ret < 0)
+ dev_err(priv->dev, "failed to clear operation mode (%d)\n", ret);
+
+psy_update:
+ power_supply_changed(priv->psy);
+}
+
+static const enum power_supply_property s2mu005_chgr_properties[] = {
+ POWER_SUPPLY_PROP_ONLINE,
+};
+
+static const struct power_supply_desc s2mu005_chgr_psy_desc = {
+ .name = "s2mu005-charger",
+ .type = POWER_SUPPLY_TYPE_USB,
+ .properties = s2mu005_chgr_properties,
+ .num_properties = ARRAY_SIZE(s2mu005_chgr_properties),
+ .get_property = s2mu005_chgr_get_property,
+};
+
+static int s2m_chgr_extcon_notifier(struct notifier_block *nb,
+ unsigned long event, void *param)
+{
+ struct s2m_chgr *priv = container_of(nb, struct s2m_chgr, extcon_nb);
+
+ schedule_work(&priv->extcon_work);
+
+ return NOTIFY_OK;
+}
+
+static int s2m_chgr_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct sec_pmic_dev *pmic_drvdata = dev_get_drvdata(dev->parent);
+ struct s2m_chgr *priv;
+ struct device_node *extcon_node;
+ struct power_supply_config psy_cfg = {};
+ const struct power_supply_desc *psy_desc;
+ work_func_t extcon_work_func;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return dev_err_probe(dev, -ENOMEM, "failed to allocate driver private\n");
+
+ platform_set_drvdata(pdev, priv);
+ priv->dev = dev;
+ priv->regmap = pmic_drvdata->regmap_pmic;
+
+ switch (platform_get_device_id(pdev)->driver_data) {
+ case S2MU005:
+ psy_desc = &s2mu005_chgr_psy_desc;
+ extcon_work_func = s2mu005_chgr_extcon_work;
+ break;
+ default:
+ return dev_err_probe(dev, -ENODEV,
+ "device type %d is not supported by driver\n",
+ pmic_drvdata->device_type);
+ }
+
+ psy_cfg.drv_data = priv;
+ priv->psy = devm_power_supply_register(dev, psy_desc, &psy_cfg);
+ if (IS_ERR(priv->psy))
+ return dev_err_probe(dev, PTR_ERR(priv->psy),
+ "failed to register power supply subsystem\n");
+
+ /* MUIC is mandatory. If unavailable, request probe deferral */
+ extcon_node = of_get_child_by_name(dev->parent->of_node, "extcon");
+ priv->extcon = extcon_find_edev_by_node(extcon_node);
+ if (IS_ERR(priv->extcon))
+ return -EPROBE_DEFER;
+
+ ret = devm_work_autocancel(dev, &priv->extcon_work, extcon_work_func);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to initialize extcon work\n");
+
+ priv->extcon_nb.notifier_call = s2m_chgr_extcon_notifier;
+ ret = devm_extcon_register_notifier_all(dev, priv->extcon, &priv->extcon_nb);
+ if (ret)
+ dev_err_probe(dev, ret, "failed to register extcon notifier\n");
+
+ return 0;
+}
+
+static const struct platform_device_id s2m_chgr_id_table[] = {
+ { "s2mu005-charger", S2MU005 },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(platform, s2m_chgr_id_table);
+
+#ifdef CONFIG_OF
+/*
+ * Device is instantiated through parent MFD device and device matching
+ * is done through platform_device_id.
+ *
+ * However if device's DT node contains proper compatible and driver is
+ * built as a module, then the *module* matching will be done through DT
+ * aliases. This requires of_device_id table. In the same time this will
+ * not change the actual *device* matching so do not add .of_match_table.
+ */
+static const struct of_device_id s2m_chgr_of_match_table[] = {
+ {
+ .compatible = "samsung,s2mu005-charger",
+ .data = (void *)S2MU005,
+ }, {
+ /* sentinel */
+ },
+};
+MODULE_DEVICE_TABLE(of, s2m_chgr_of_match_table);
+#endif
+
+static struct platform_driver s2m_chgr_driver = {
+ .driver = {
+ .name = "s2m-charger",
+ },
+ .probe = s2m_chgr_probe,
+ .id_table = s2m_chgr_id_table,
+};
+module_platform_driver(s2m_chgr_driver);
+
+MODULE_DESCRIPTION("Battery Charger Driver For Samsung S2M Series PMICs");
+MODULE_AUTHOR("Kaustabh Chakraborty <kauschluss@disroot.org>");
+MODULE_LICENSE("GPL");
--
2.51.2
^ permalink raw reply related
* Re: [PATCH v6 1/2] dt-bindings: rtc: Add pcf85053 support
From: Conor Dooley @ 2025-11-13 19:42 UTC (permalink / raw)
To: Lakshay Piplani
Cc: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
conor+dt, devicetree, pankit.garg, vikash.bansal, priyanka.jain,
shashank.rebbapragada
In-Reply-To: <20251113054243.4045820-1-lakshay.piplani@nxp.com>
[-- Attachment #1: Type: text/plain, Size: 474 bytes --]
On Thu, Nov 13, 2025 at 11:12:42AM +0530, Lakshay Piplani wrote:
> Add device tree bindings for NXP PCF85053 RTC chip.
>
> Signed-off-by: Pankit Garg <pankit.garg@nxp.com>
> Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
> ---
> V5 -> V6: - Dropped driver-specific commentary from property descriptions.
> - Simplified and clarified descriptions for better readability.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 01/13] dt-bindings: leds: document Samsung S2M series PMIC flash LED device
From: Conor Dooley @ 2025-11-13 19:52 UTC (permalink / raw)
To: Kaustabh Chakraborty
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, MyungJoo Ham, Chanwoo Choi, Sebastian Reichel,
Krzysztof Kozlowski, André Draszik, Alexandre Belloni,
Jonathan Corbet, linux-leds, devicetree, linux-kernel, linux-pm,
linux-samsung-soc, linux-rtc, linux-doc
In-Reply-To: <20251114-s2mu005-pmic-v1-1-9e3184d3a0c9@disroot.org>
[-- Attachment #1: Type: text/plain, Size: 468 bytes --]
On Fri, Nov 14, 2025 at 12:35:02AM +0530, Kaustabh Chakraborty wrote:
> Certain Samsung S2M series PMICs have a flash LED controller with
> two LED channels, and with torch and flash control modes. Document the
> devicetree schema for the device.
>
> The initial driver introduced has support for S2MU005, add its
> compatible as well.
Drop this sentence please.
pw-bot: changes-requested
With it gone
Acked-by: Conor Dooley <conor.dooley@microchip.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 02/13] dt-bindings: leds: document Samsung S2M series PMIC RGB LED device
From: Conor Dooley @ 2025-11-13 19:53 UTC (permalink / raw)
To: Kaustabh Chakraborty
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, MyungJoo Ham, Chanwoo Choi, Sebastian Reichel,
Krzysztof Kozlowski, André Draszik, Alexandre Belloni,
Jonathan Corbet, linux-leds, devicetree, linux-kernel, linux-pm,
linux-samsung-soc, linux-rtc, linux-doc
In-Reply-To: <20251114-s2mu005-pmic-v1-2-9e3184d3a0c9@disroot.org>
[-- Attachment #1: Type: text/plain, Size: 2124 bytes --]
On Fri, Nov 14, 2025 at 12:35:03AM +0530, Kaustabh Chakraborty wrote:
> Certain Samsung S2M series PMICs have a three-channel LED device with
> independent brightness control for each channel, typically used as
> status indicators in mobile phones. Document the devicetree schema from
> this driver.
>
> The initial driver introduced has support for S2MU005, add its
> compatible as well.
Same here.
Additionally "Document the ... from this driver" should lose the driver
part, unless you mean it as "led driver" rather than "linux driver".
pw-bot: changes-requested
>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
> .../bindings/leds/samsung,s2mu005-rgb.yaml | 34 ++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/leds/samsung,s2mu005-rgb.yaml b/Documentation/devicetree/bindings/leds/samsung,s2mu005-rgb.yaml
> new file mode 100644
> index 000000000000..bad7080ff8f5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/samsung,s2mu005-rgb.yaml
> @@ -0,0 +1,34 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/samsung,s2mu005-rgb.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: RGB LED Driver for Samsung S2M series PMICs
> +
> +maintainers:
> + - Kaustabh Chakraborty <kauschluss@disroot.org>
> +
> +description: |
> + The Samsung S2M series PMIC RGB LED is a three-channel LED device with
> + 8-bit brightness control for each channel, typically used as status
> + indicators in mobile phones.
> +
> + This is a part of device tree bindings for S2M and S5M family of Power
> + Management IC (PMIC).
> +
> + See also Documentation/devicetree/bindings/mfd/samsung,s2mps11.yaml for
> + additional information and example.
> +
> +allOf:
> + - $ref: common.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + - samsung,s2mu005-rgb
> +
> +required:
> + - compatible
> +
> +unevaluatedProperties: false
>
> --
> 2.51.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 03/13] dt-bindings: extcon: document Samsung S2M series PMIC extcon device
From: Conor Dooley @ 2025-11-13 19:54 UTC (permalink / raw)
To: Kaustabh Chakraborty
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, MyungJoo Ham, Chanwoo Choi, Sebastian Reichel,
Krzysztof Kozlowski, André Draszik, Alexandre Belloni,
Jonathan Corbet, linux-leds, devicetree, linux-kernel, linux-pm,
linux-samsung-soc, linux-rtc, linux-doc
In-Reply-To: <20251114-s2mu005-pmic-v1-3-9e3184d3a0c9@disroot.org>
[-- Attachment #1: Type: text/plain, Size: 2024 bytes --]
On Fri, Nov 14, 2025 at 12:35:04AM +0530, Kaustabh Chakraborty wrote:
> Certain Samsung S2M series PMICs have a MUIC device which reports
> various cable states by measuring the ID-GND resistance with an internal
> ADC. Document the devicetree schema for this device.
>
> The initial driver introduced has support for S2MU005, add its
> compatible as well.
>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
> .../bindings/extcon/samsung,s2mu005-muic.yaml | 35 ++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/extcon/samsung,s2mu005-muic.yaml b/Documentation/devicetree/bindings/extcon/samsung,s2mu005-muic.yaml
> new file mode 100644
> index 000000000000..8511bb96b47a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/extcon/samsung,s2mu005-muic.yaml
> @@ -0,0 +1,35 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/extcon/samsung,s2mu005-muic.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Extcon Driver for Samsung S2M series PMICs
> +
> +maintainers:
> + - Kaustabh Chakraborty <kauschluss@disroot.org>
> +
> +description: |
> + The Samsung S2M series PMIC extcon device is a USB port accessory
> + detector. It reports multiple states depending on the ID-GND
> + resistance measured by an internal ADC.
> +
> + This is a part of device tree bindings for S2M and S5M family of Power
> + Management IC (PMIC).
> +
> + See also Documentation/devicetree/bindings/mfd/samsung,s2mps11.yaml for
> + additional information and example.
> +
> +properties:
> + compatible:
> + enum:
> + - samsung,s2mu005-muic
Why does this need a dedicated child node for just a port property?
> +
> + port:
> + $ref: /schemas/graph.yaml#/properties/port
> +
> +required:
> + - compatible
> + - port
> +
> +additionalProperties: false
>
> --
> 2.51.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 04/13] dt-bindings: power: supply: document Samsung S2M series PMIC charger device
From: Conor Dooley @ 2025-11-13 19:57 UTC (permalink / raw)
To: Kaustabh Chakraborty
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, MyungJoo Ham, Chanwoo Choi, Sebastian Reichel,
Krzysztof Kozlowski, André Draszik, Alexandre Belloni,
Jonathan Corbet, linux-leds, devicetree, linux-kernel, linux-pm,
linux-samsung-soc, linux-rtc, linux-doc
In-Reply-To: <20251114-s2mu005-pmic-v1-4-9e3184d3a0c9@disroot.org>
[-- Attachment #1: Type: text/plain, Size: 2540 bytes --]
On Fri, Nov 14, 2025 at 12:35:05AM +0530, Kaustabh Chakraborty wrote:
> Certain Samsung S2M series PMICs have a battery charger device which,
> among other things, manages power interfacing of the USB port. It may
> supply power, as done in USB OTG operation mode, or it may accept power
> and redirect it to the battery fuelgauge for charging.
>
> This driver depends on the MUIC device present in the same PMIC block.
>
> The initial driver introduced has support for S2MU005, add its
> compatible as well.
Similar comments in all these binding commit messages, they should only
really contain mentions of "drivers" if you are talking about electrical
circuitry.
>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
> .../power/supply/samsung,s2mu005-charger.yaml | 35 ++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/power/supply/samsung,s2mu005-charger.yaml b/Documentation/devicetree/bindings/power/supply/samsung,s2mu005-charger.yaml
> new file mode 100644
> index 000000000000..80292d6e2562
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/samsung,s2mu005-charger.yaml
> @@ -0,0 +1,35 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/supply/samsung,s2mu005-charger.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Battery Charger Driver for Samsung S2M series PMICs
> +
> +maintainers:
> + - Kaustabh Chakraborty <kauschluss@disroot.org>
> +
> +description: |
> + The Samsung S2M series PMIC battery charger manages power interfacing
> + of the USB port. It may supply power, as done in USB OTG operation
> + mode, or it may accept power and redirect it to the battery fuelgauge
> + for charging.
> +
> + This is a part of device tree bindings for S2M and S5M family of Power
> + Management IC (PMIC).
> +
> + See also Documentation/devicetree/bindings/mfd/samsung,s2mps11.yaml for
> + additional information and example.
> +
> +allOf:
> + - $ref: power-supply.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + - samsung,s2mu005-charger
Why do you need a dedicated child node for this? It's got one property,
other than the compatible, that you're using. It could easily just go
in the parent without a dedicated node etc.
> +
> +required:
> + - compatible
> +
> +unevaluatedProperties: false
>
> --
> 2.51.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 11/13] Documentation: leds: document pattern behavior of Samsung S2M series PMIC RGB LEDs
From: Randy Dunlap @ 2025-11-13 19:57 UTC (permalink / raw)
To: Kaustabh Chakraborty, Lee Jones, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, MyungJoo Ham, Chanwoo Choi,
Sebastian Reichel, Krzysztof Kozlowski, André Draszik,
Alexandre Belloni, Jonathan Corbet
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc
In-Reply-To: <20251114-s2mu005-pmic-v1-11-9e3184d3a0c9@disroot.org>
On 11/13/25 11:05 AM, Kaustabh Chakraborty wrote:
> Add documentation to describe how hardware patterns (as defined by the
> documentation of led-class-multicolor) are parsed and implemented by the
> Samsung S2M series PMIC RGB LED driver.
>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
> Documentation/leds/index.rst | 1 +
> Documentation/leds/leds-s2m-rgb.rst | 60 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 61 insertions(+)
>
> diff --git a/Documentation/leds/leds-s2m-rgb.rst b/Documentation/leds/leds-s2m-rgb.rst
> new file mode 100644
> index 000000000000..cf91f0238093
> --- /dev/null
> +++ b/Documentation/leds/leds-s2m-rgb.rst
> @@ -0,0 +1,60 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +======================================
> +Samsung S2M Series PMIC RGB LED Driver
> +======================================
> +
> +Description
> +-----------
> +
> +The RGB LED on the S2M series PMIC hardware features a three-channel LED that is
> +grouped together as a single device. Furthermore, the it supports 8-bit
drop the ^^^
> +brightness control for each channel. This LED is typically used as a status
> +indicator in mobile devices. It also supports various parameters for hardware
> +patterns.
--
~Randy
^ permalink raw reply
* Re: [PATCH v4 12/16] clk: clk-bd718x7: Support BD72720 clk gate
From: Stephen Boyd @ 2025-11-14 3:53 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sebastian Reichel, Liam Girdwood, Mark Brown,
Michael Turquette, Matti Vaittinen, Linus Walleij,
Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc,
Andreas Kemnade
In-Reply-To: <16c92cc14da67ec6354ee0ac4e1faef4af4d0994.1763022807.git.mazziesaccount@gmail.com>
Quoting Matti Vaittinen (2025-11-13 00:55:05)
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> The BD72720 has similar simple clk gate as a few other ROHM PMICs.
>
> Add support for BD72720 clk gate.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>
> ---
Acked-by: Stephen Boyd <sboyd@kernel.org>
^ permalink raw reply
* Re: [PATCH 06/13] mfd: sec-irq: add support for creating multiple IRQ chips
From: Alexandre Belloni @ 2025-11-14 7:50 UTC (permalink / raw)
To: Kaustabh Chakraborty
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, MyungJoo Ham, Chanwoo Choi, Sebastian Reichel,
Krzysztof Kozlowski, André Draszik, Jonathan Corbet,
linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc
In-Reply-To: <20251114-s2mu005-pmic-v1-6-9e3184d3a0c9@disroot.org>
On 14/11/2025 00:35:07+0530, Kaustabh Chakraborty wrote:
> The current state of the driver only allows creating only one IRQ chip
> per PMIC. On some PMICs, such as Samsung's S2MU005, there are multiple
> interrupt blocks, for which the current implementation stands insufficient.
>
> Add support for creating multiple IRQ chips for a PMIC. Every IRQ chip is
> given it's own index, which is used by sub-device drivers to request IRQs.
>
> A macro is defined which states the maximum number of chips supported.
> It's set to 1 as currently, no PMIC requires more than one IRQ chip. The
> value must be changed accordingly on adding new PMICs requiring multiple
> IRQ chips.
>
> Moreover, adjust the s5m RTC driver to initialize IRQs with the
> appropriate IRQ chip index.
>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> drivers/mfd/sec-irq.c | 163 +++++++++++++++++++++++----------------
> drivers/rtc/rtc-s5m.c | 15 +++-
> include/linux/mfd/samsung/core.h | 5 +-
> include/linux/mfd/samsung/irq.h | 14 ++++
> 4 files changed, 127 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
> index c5c80b1ba104..053c28f31ec9 100644
> --- a/drivers/mfd/sec-irq.c
> +++ b/drivers/mfd/sec-irq.c
> @@ -181,25 +181,31 @@ static const struct regmap_irq s5m8767_irqs[] = {
> };
>
> /* All S2MPG10 interrupt sources are read-only and don't require clearing */
> -static const struct regmap_irq_chip s2mpg10_irq_chip = {
> - .name = "s2mpg10",
> - .irqs = s2mpg10_irqs,
> - .num_irqs = ARRAY_SIZE(s2mpg10_irqs),
> - .num_regs = 6,
> - .status_base = S2MPG10_PMIC_INT1,
> - .mask_base = S2MPG10_PMIC_INT1M,
> +static const struct regmap_irq_chip s2mpg10_irq_chip[] = {
> + [S2MPG10_IRQ_CHIP] = {
> + .name = "s2mpg10",
> + .irqs = s2mpg10_irqs,
> + .num_irqs = ARRAY_SIZE(s2mpg10_irqs),
> + .num_regs = 6,
> + .status_base = S2MPG10_PMIC_INT1,
> + .mask_base = S2MPG10_PMIC_INT1M,
> + },
> };
>
> -static const struct regmap_irq_chip s2mps11_irq_chip = {
> - .name = "s2mps11",
> - .irqs = s2mps11_irqs,
> - .num_irqs = ARRAY_SIZE(s2mps11_irqs),
> - .num_regs = 3,
> - .status_base = S2MPS11_REG_INT1,
> - .mask_base = S2MPS11_REG_INT1M,
> - .ack_base = S2MPS11_REG_INT1,
> +static const struct regmap_irq_chip s2mps11_irq_chip[] = {
> + [S2MPS11_IRQ_CHIP] = {
> + .name = "s2mps11",
> + .irqs = s2mps11_irqs,
> + .num_irqs = ARRAY_SIZE(s2mps11_irqs),
> + .num_regs = 3,
> + .status_base = S2MPS11_REG_INT1,
> + .mask_base = S2MPS11_REG_INT1M,
> + .ack_base = S2MPS11_REG_INT1,
> + },
> };
>
> +#define S2MPS1X_IRQ_CHIP S2MPS14_IRQ_CHIP
> +
> #define S2MPS1X_IRQ_CHIP_COMMON_DATA \
> .irqs = s2mps14_irqs, \
> .num_irqs = ARRAY_SIZE(s2mps14_irqs), \
> @@ -208,85 +214,106 @@ static const struct regmap_irq_chip s2mps11_irq_chip = {
> .mask_base = S2MPS14_REG_INT1M, \
> .ack_base = S2MPS14_REG_INT1 \
>
> -static const struct regmap_irq_chip s2mps13_irq_chip = {
> - .name = "s2mps13",
> - S2MPS1X_IRQ_CHIP_COMMON_DATA,
> +static const struct regmap_irq_chip s2mps13_irq_chip[] = {
> + [S2MPS1X_IRQ_CHIP] = {
> + .name = "s2mps13",
> + S2MPS1X_IRQ_CHIP_COMMON_DATA,
> + },
> };
>
> -static const struct regmap_irq_chip s2mps14_irq_chip = {
> - .name = "s2mps14",
> - S2MPS1X_IRQ_CHIP_COMMON_DATA,
> +static const struct regmap_irq_chip s2mps14_irq_chip[] = {
> + [S2MPS1X_IRQ_CHIP] = {
> + .name = "s2mps14",
> + S2MPS1X_IRQ_CHIP_COMMON_DATA,
> + },
> };
>
> -static const struct regmap_irq_chip s2mps15_irq_chip = {
> - .name = "s2mps15",
> - S2MPS1X_IRQ_CHIP_COMMON_DATA,
> +static const struct regmap_irq_chip s2mps15_irq_chip[] = {
> + [S2MPS1X_IRQ_CHIP] = {
> + .name = "s2mps15",
> + S2MPS1X_IRQ_CHIP_COMMON_DATA,
> + },
> };
>
> -static const struct regmap_irq_chip s2mpu02_irq_chip = {
> - .name = "s2mpu02",
> - .irqs = s2mpu02_irqs,
> - .num_irqs = ARRAY_SIZE(s2mpu02_irqs),
> - .num_regs = 3,
> - .status_base = S2MPU02_REG_INT1,
> - .mask_base = S2MPU02_REG_INT1M,
> - .ack_base = S2MPU02_REG_INT1,
> +static const struct regmap_irq_chip s2mpu02_irq_chip[] = {
> + [S2MPU02_IRQ_CHIP] = {
> + .name = "s2mpu02",
> + .irqs = s2mpu02_irqs,
> + .num_irqs = ARRAY_SIZE(s2mpu02_irqs),
> + .num_regs = 3,
> + .status_base = S2MPU02_REG_INT1,
> + .mask_base = S2MPU02_REG_INT1M,
> + .ack_base = S2MPU02_REG_INT1,
> + },
> };
>
> -static const struct regmap_irq_chip s2mpu05_irq_chip = {
> - .name = "s2mpu05",
> - .irqs = s2mpu05_irqs,
> - .num_irqs = ARRAY_SIZE(s2mpu05_irqs),
> - .num_regs = 3,
> - .status_base = S2MPU05_REG_INT1,
> - .mask_base = S2MPU05_REG_INT1M,
> - .ack_base = S2MPU05_REG_INT1,
> +static const struct regmap_irq_chip s2mpu05_irq_chip[] = {
> + [S2MPU05_IRQ_CHIP] = {
> + .name = "s2mpu05",
> + .irqs = s2mpu05_irqs,
> + .num_irqs = ARRAY_SIZE(s2mpu05_irqs),
> + .num_regs = 3,
> + .status_base = S2MPU05_REG_INT1,
> + .mask_base = S2MPU05_REG_INT1M,
> + .ack_base = S2MPU05_REG_INT1,
> + },
> };
>
> -static const struct regmap_irq_chip s5m8767_irq_chip = {
> - .name = "s5m8767",
> - .irqs = s5m8767_irqs,
> - .num_irqs = ARRAY_SIZE(s5m8767_irqs),
> - .num_regs = 3,
> - .status_base = S5M8767_REG_INT1,
> - .mask_base = S5M8767_REG_INT1M,
> - .ack_base = S5M8767_REG_INT1,
> +static const struct regmap_irq_chip s5m8767_irq_chip[] = {
> + [S5M8767_IRQ_CHIP] = {
> + .name = "s5m8767",
> + .irqs = s5m8767_irqs,
> + .num_irqs = ARRAY_SIZE(s5m8767_irqs),
> + .num_regs = 3,
> + .status_base = S5M8767_REG_INT1,
> + .mask_base = S5M8767_REG_INT1M,
> + .ack_base = S5M8767_REG_INT1,
> + },
> };
>
> int sec_irq_init(struct sec_pmic_dev *sec_pmic)
> {
> const struct regmap_irq_chip *sec_irq_chip;
> - int ret;
> + int sec_irq_chip_num, i, ret;
>
> switch (sec_pmic->device_type) {
> case S5M8767X:
> - sec_irq_chip = &s5m8767_irq_chip;
> + sec_irq_chip = s5m8767_irq_chip;
> + sec_irq_chip_num = ARRAY_SIZE(s5m8767_irq_chip);
> break;
> case S2DOS05:
> return 0;
> case S2MPA01:
> - sec_irq_chip = &s2mps14_irq_chip;
> + sec_irq_chip = s2mps14_irq_chip;
> + sec_irq_chip_num = ARRAY_SIZE(s2mps14_irq_chip);
> break;
> case S2MPG10:
> - sec_irq_chip = &s2mpg10_irq_chip;
> + sec_irq_chip = s2mpg10_irq_chip;
> + sec_irq_chip_num = ARRAY_SIZE(s2mpg10_irq_chip);
> break;
> case S2MPS11X:
> - sec_irq_chip = &s2mps11_irq_chip;
> + sec_irq_chip = s2mps11_irq_chip;
> + sec_irq_chip_num = ARRAY_SIZE(s2mps11_irq_chip);
> break;
> case S2MPS13X:
> - sec_irq_chip = &s2mps13_irq_chip;
> + sec_irq_chip = s2mps13_irq_chip;
> + sec_irq_chip_num = ARRAY_SIZE(s2mps13_irq_chip);
> break;
> case S2MPS14X:
> - sec_irq_chip = &s2mps14_irq_chip;
> + sec_irq_chip = s2mps14_irq_chip;
> + sec_irq_chip_num = ARRAY_SIZE(s2mps14_irq_chip);
> break;
> case S2MPS15X:
> - sec_irq_chip = &s2mps15_irq_chip;
> + sec_irq_chip = s2mps15_irq_chip;
> + sec_irq_chip_num = ARRAY_SIZE(s2mps15_irq_chip);
> break;
> case S2MPU02:
> - sec_irq_chip = &s2mpu02_irq_chip;
> + sec_irq_chip = s2mpu02_irq_chip;
> + sec_irq_chip_num = ARRAY_SIZE(s2mpu02_irq_chip);
> break;
> case S2MPU05:
> - sec_irq_chip = &s2mpu05_irq_chip;
> + sec_irq_chip = s2mpu05_irq_chip;
> + sec_irq_chip_num = ARRAY_SIZE(s2mpu05_irq_chip);
> break;
> default:
> return dev_err_probe(sec_pmic->dev, -EINVAL,
> @@ -300,13 +327,19 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
> return 0;
> }
>
> - ret = devm_regmap_add_irq_chip(sec_pmic->dev, sec_pmic->regmap_pmic,
> - sec_pmic->irq, IRQF_ONESHOT,
> - 0, sec_irq_chip, &sec_pmic->irq_data);
> - if (ret)
> - return dev_err_probe(sec_pmic->dev, ret,
> - "Failed to add %s IRQ chip\n",
> - sec_irq_chip->name);
> + for (i = 0; i < sec_irq_chip_num; i++) {
> + ret = devm_regmap_add_irq_chip(sec_pmic->dev,
> + sec_pmic->regmap_pmic,
> + sec_pmic->irq,
> + IRQF_ONESHOT | IRQF_SHARED, 0,
> + sec_irq_chip + i,
> + sec_pmic->irq_data + i);
> + if (ret) {
> + return dev_err_probe(sec_pmic->dev, ret,
> + "Failed to add %s IRQ chip\n",
> + sec_irq_chip->name);
> + }
> + }
>
> /*
> * The rtc-s5m driver requests S2MPS14_IRQ_RTCA0 also for S2MPS11
> diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
> index a7220b4d0e8d..726915deff7a 100644
> --- a/drivers/rtc/rtc-s5m.c
> +++ b/drivers/rtc/rtc-s5m.c
> @@ -668,7 +668,7 @@ static int s5m_rtc_probe(struct platform_device *pdev)
> enum sec_device_type device_type =
> platform_get_device_id(pdev)->driver_data;
> struct s5m_rtc_info *info;
> - int ret, alarm_irq;
> + int ret, alarm_irq, irq_chip;
>
> info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> if (!info)
> @@ -684,21 +684,25 @@ static int s5m_rtc_probe(struct platform_device *pdev)
> regmap_cfg = &s2mps14_rtc_regmap_config;
> info->regs = &s2mps15_rtc_regs;
> alarm_irq = S2MPS14_IRQ_RTCA0;
> + irq_chip = S2MPS11_IRQ_CHIP;
> break;
> case S2MPS14X:
> regmap_cfg = &s2mps14_rtc_regmap_config;
> info->regs = &s2mps14_rtc_regs;
> alarm_irq = S2MPS14_IRQ_RTCA0;
> + irq_chip = S2MPS14_IRQ_CHIP;
> break;
> case S2MPS13X:
> regmap_cfg = &s2mps14_rtc_regmap_config;
> info->regs = &s2mps13_rtc_regs;
> alarm_irq = S2MPS14_IRQ_RTCA0;
> + irq_chip = S2MPS14_IRQ_CHIP;
> break;
> case S5M8767X:
> regmap_cfg = &s5m_rtc_regmap_config;
> info->regs = &s5m_rtc_regs;
> alarm_irq = S5M8767_IRQ_RTCA1;
> + irq_chip = S5M8767_IRQ_CHIP;
> break;
> default:
> return dev_err_probe(&pdev->dev, -ENODEV,
> @@ -720,6 +724,7 @@ static int s5m_rtc_probe(struct platform_device *pdev)
> } else if (device_type == S2MPG10) {
> info->regs = &s2mpg10_rtc_regs;
> alarm_irq = S2MPG10_IRQ_RTCA0;
> + irq_chip = S2MPG10_IRQ_CHIP;
> } else {
> return dev_err_probe(&pdev->dev, -ENODEV,
> "Unsupported device type %d\n",
> @@ -730,12 +735,14 @@ static int s5m_rtc_probe(struct platform_device *pdev)
> info->s5m87xx = s5m87xx;
> info->device_type = device_type;
>
> - if (s5m87xx->irq_data) {
> - info->irq = regmap_irq_get_virq(s5m87xx->irq_data, alarm_irq);
> - if (info->irq <= 0)
> + if (s5m87xx->irq_data[irq_chip]) {
> + info->irq = regmap_irq_get_virq(s5m87xx->irq_data[irq_chip],
> + alarm_irq);
> + if (info->irq <= 0) {
> return dev_err_probe(&pdev->dev, -EINVAL,
> "Failed to get virtual IRQ %d\n",
> alarm_irq);
> + }
> }
>
> platform_set_drvdata(pdev, info);
> diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
> index d785e101fe79..dcd741c4f0d6 100644
> --- a/include/linux/mfd/samsung/core.h
> +++ b/include/linux/mfd/samsung/core.h
> @@ -33,6 +33,9 @@
> #define STEP_12_5_MV 12500
> #define STEP_6_25_MV 6250
>
> +/* Maximum number of IRQ chips in a PMIC */
> +#define MAX_IRQ_CHIPS 1
> +
> struct gpio_desc;
>
> enum sec_device_type {
> @@ -69,7 +72,7 @@ struct sec_pmic_dev {
>
> int device_type;
> int irq;
> - struct regmap_irq_chip_data *irq_data;
> + struct regmap_irq_chip_data *irq_data[MAX_IRQ_CHIPS];
> };
>
> struct sec_platform_data {
> diff --git a/include/linux/mfd/samsung/irq.h b/include/linux/mfd/samsung/irq.h
> index b4805cbd949b..78eb894e350e 100644
> --- a/include/linux/mfd/samsung/irq.h
> +++ b/include/linux/mfd/samsung/irq.h
> @@ -34,6 +34,8 @@ enum s2mpa01_irq {
> S2MPA01_IRQ_NR,
> };
>
> +#define S2MPA01_IRQ_CHIP 0
> +
> #define S2MPA01_IRQ_PWRONF_MASK (1 << 0)
> #define S2MPA01_IRQ_PWRONR_MASK (1 << 1)
> #define S2MPA01_IRQ_JIGONBF_MASK (1 << 2)
> @@ -58,6 +60,8 @@ enum s2mpa01_irq {
> #define S2MPA01_IRQ_B35_TSD_MASK (1 << 5)
>
> enum s2mpg10_irq {
> +#define S2MPG10_IRQ_CHIP 0
> +
> /* PMIC */
> S2MPG10_IRQ_PWRONF,
> S2MPG10_IRQ_PWRONR,
> @@ -183,6 +187,8 @@ enum s2mps11_irq {
> S2MPS11_IRQ_NR,
> };
>
> +#define S2MPS11_IRQ_CHIP 0
> +
> #define S2MPS11_IRQ_PWRONF_MASK (1 << 0)
> #define S2MPS11_IRQ_PWRONR_MASK (1 << 1)
> #define S2MPS11_IRQ_JIGONBF_MASK (1 << 2)
> @@ -226,6 +232,8 @@ enum s2mps14_irq {
> S2MPS14_IRQ_NR,
> };
>
> +#define S2MPS14_IRQ_CHIP 0
> +
> enum s2mpu02_irq {
> S2MPU02_IRQ_PWRONF,
> S2MPU02_IRQ_PWRONR,
> @@ -250,6 +258,8 @@ enum s2mpu02_irq {
> S2MPU02_IRQ_NR,
> };
>
> +#define S2MPU02_IRQ_CHIP 0
> +
> /* Masks for interrupts are the same as in s2mps11 */
> #define S2MPS14_IRQ_TSD_MASK (1 << 2)
>
> @@ -277,6 +287,8 @@ enum s2mpu05_irq {
> S2MPU05_IRQ_NR,
> };
>
> +#define S2MPU05_IRQ_CHIP 0
> +
> #define S2MPU05_IRQ_PWRONF_MASK BIT(0)
> #define S2MPU05_IRQ_PWRONR_MASK BIT(1)
> #define S2MPU05_IRQ_JIGONBF_MASK BIT(2)
> @@ -321,6 +333,8 @@ enum s5m8767_irq {
> S5M8767_IRQ_NR,
> };
>
> +#define S5M8767_IRQ_CHIP 0
> +
> #define S5M8767_IRQ_PWRR_MASK (1 << 0)
> #define S5M8767_IRQ_PWRF_MASK (1 << 1)
> #define S5M8767_IRQ_PWR1S_MASK (1 << 3)
>
> --
> 2.51.2
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v4 04/16] dt-bindings: power: supply: BD72720 managed battery
From: Matti Vaittinen @ 2025-11-14 9:04 UTC (permalink / raw)
To: Rob Herring (Arm), Matti Vaittinen
Cc: Krzysztof Kozlowski, Mark Brown, Linus Walleij, linux-kernel,
Sebastian Reichel, Bartosz Golaszewski, Alexandre Belloni,
linux-clk, Michael Turquette, Matti Vaittinen, linux-leds,
Pavel Machek, Liam Girdwood, linux-gpio, linux-pm,
Andreas Kemnade, Conor Dooley, devicetree, linux-rtc, Lee Jones,
Stephen Boyd
In-Reply-To: <176303119683.3716572.16868393928566655866.robh@kernel.org>
On 13/11/2025 12:53, Rob Herring (Arm) wrote:
>
> On Thu, 13 Nov 2025 10:52:19 +0200, Matti Vaittinen wrote:
>> From: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> The BD72720 PMIC has a battery charger + coulomb counter block. These
>> can be used to manage charging of a lithium-ion battery and to do fuel
>> gauging.
>>
>> ROHM has developed a so called "zero-correction" -algorithm to improve
>> the fuel-gauging accuracy close to the point where battery is depleted.
>> This relies on battery specific "VDR" tables, which are measured from
>> the battery, and which describe the voltage drop rate. More thorough
>> explanation about the "zero correction" and "VDR" parameters is here:
>> https://lore.kernel.org/all/676253b9-ff69-7891-1f26-a8b5bb5a421b@fi.rohmeurope.com/
>>
>> Document the VDR zero-correction specific battery properties used by the
>> BD72720 and some other ROHM chargers.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> ---
>> NOTE:
>> Linus' rb-tag holds only if there's no further comments from Rob.
>>
>> Revision history:
>> v3 =>:
>> - No changes
>>
>> v2 => v3:
>> - Constrain VDR threshold voltage to 48V
>> - Use standard '-bp' -suffix for the rohm,volt-drop-soc
>>
>> RFCv1 => v2:
>> - Add units to rohm,volt-drop-soc (tenths of %)
>> - Give real temperatures matching the VDR tables, instead of vague
>> 'high', 'normal', 'low', 'very low'. (Add table of temperatures and
>> use number matching the right temperature index in the VDR table name).
>> - Fix typoed 'algorithm' in commit message.
>>
>> The parameters are describing the battery voltage drop rates - so they
>> are properties of the battery, not the charger. Thus they do not belong
>> in the charger node.
>>
// snip
> My bot found errors running 'make dt_binding_check' on your patch:
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/rohm,vdr-battery.example.dtb: battery (simple-battery): 'degrade-cycle-microamp-hours', 'rohm,volt-drop-0-microvolt', 'rohm,volt-drop-1-microvolt', 'rohm,volt-drop-2-microvolt', 'rohm,volt-drop-3-temp-microvolt', 'rohm,volt-drop-soc-bp', 'rohm,volt-drop-temperatures-millicelsius', 'rohm,voltage-vdr-thresh-microvolt' do not match any of the regexes: '^ocv-capacity-table-[0-9]+$', '^pinctrl-[0-9]+$'
> from schema $id: http://devicetree.org/schemas/power/supply/battery.yaml
>
Odd. I am pretty sure I didn't see this when I ran the make
dt_binding_check. Not 100% sure what happened there. I get this error
now though when including all the bindings to the check.
Do I get this right - these errors result from the properties used in
example not being included in the battery.yaml? So, this means that the
check is done based on the binding (battery.yaml) where the compatible
(simple-battery) is defined - not based on the properties which are
present in this file where the example resides, (and which references
the battery.yaml)?
...
Oh... Now that I wrote it I feel like an idiot.
This approach couldn't work for the validation, right? Let's assume I
had a VDR battery, and I added a static-battery -node for it. Running
the validation would pick the battery.yaml based on the compatible (just
as it does here), and be completely unaware of this vdr-battery.yaml. I
have no idea why I thought this would work. Probably because I only
thought this from the documentation POV.
So, as far as I understand, the only viable options are expanding the
existing battery.yaml with these properties (which I hoped to avoid, see
below)
>> The right place for them is the battery node, which is described by the
>> generic "battery.yaml". I was not comfortable with adding these
>> properties to the generic battery.yaml because they are:
>> - Meaningful only for those charger drivers which have the VDR
>> algorithm implemented. (And even though the algorithm is not
charger
>> specific, AFAICS, it is currently only used by some ROHM PMIC
>> drivers).
>> - Technique of measuring the VDR tables for a battery is not widely
>> known. AFAICS, only folks at ROHM are measuring those for some
>> customer products. We do have those tables available for some
of the
>> products though (Kobo?).
or, to add new compatible for the "vdr-battery".
AFAICS, adding new compatible would require us to wither duplicate the
used properties from battery.yaml here (as battery.yaml mandates the
"simple-battery" - compatible) - or to split the battery.yaml in two
files, one containing the generic properties, other containing the
"simple-battery" -compatible and referencing the generic one. Then the
"vdr-battery" could also reference the generic one.
Any suggestions for the next path to follow?
Oh, and sorry for asking to review something which is obviously not
working approach. I should've understood this from the beginning.
Yours,
-- Matti
---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* Re: [PATCH v4 14/16] power: supply: bd71828: Support wider register addresses
From: Andreas Kemnade @ 2025-11-14 11:15 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Matti Vaittinen, Lee Jones, Pavel Machek,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sebastian Reichel,
Liam Girdwood, Mark Brown, Michael Turquette, Stephen Boyd,
Linus Walleij, Bartosz Golaszewski, Alexandre Belloni, linux-leds,
devicetree, linux-kernel, linux-pm, linux-clk, linux-gpio,
linux-rtc
In-Reply-To: <6248200397d3582fe926938736da66d6bbf9535d.1763022807.git.mazziesaccount@gmail.com>
On Thu, 13 Nov 2025 10:55:39 +0200
Matti Vaittinen <matti.vaittinen@linux.dev> wrote:
> As a side note, we can reduce the "wasted space / member / instance" from
> 3 bytes to 1 byte, by using u16 instead of the unsigned int if needed. I
> rather use unsigned int to be initially prepared for devices with 32 bit
> registers if there is no need to count bytes.
Well, this is totally internal to the module, so no ABI/API changes, so
there is no advantage of using 32bit now I think. We can switch any time.
But we have 32bit stuff in the regmap cache anyways, so that is not above
the general level of wasting space.
Regards,
Andreas
^ permalink raw reply
* [PATCH 0/3] Samsung mfd/rtc driver alarm IRQ simplification
From: André Draszik @ 2025-11-14 11:47 UTC (permalink / raw)
To: Krzysztof Kozlowski, Lee Jones, Alexandre Belloni
Cc: Peter Griffin, Tudor Ambarus, Will McVicker, Juan Yescas,
Douglas Anderson, kernel-team, Kaustabh Chakraborty, linux-kernel,
linux-samsung-soc, linux-rtc, André Draszik
Hi,
With the attached patches the Samsung s5m RTC driver is simplified a
little bit with regards to alarm IRQ acquisition.
The end result is that instead of having a list of IRQ numbers for each
variant (and a BUILD_BUG_ON() to ensure consistency), the RTC driver
queries the 'alarm' platform resource from the parent (mfd cell).
Additionally, we can drop a now-useless field from runtime data,
reducing memory consumption slightly.
The attached patches must be applied in-order. I would expect them all
to go via the MFD tree. Alternatively, they could be applied one after
another during multiple kernel release cycles, but that seems a
needless complication.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
André Draszik (3):
mfd: sec: add rtc alarm IRQ as platform device resource
rtc: s5m: query platform device IRQ resource for alarm IRQ
mfd: sec: drop now unused struct sec_pmic_dev::irq_data
drivers/mfd/sec-common.c | 41 ++++++++++++++++++++++++++++++----------
drivers/mfd/sec-core.h | 2 +-
drivers/mfd/sec-irq.c | 10 ++--------
drivers/rtc/rtc-s5m.c | 21 ++++++++------------
include/linux/mfd/samsung/core.h | 1 -
5 files changed, 42 insertions(+), 33 deletions(-)
---
base-commit: b179ce312bafcb8c68dc718e015aee79b7939ff0
change-id: 20251114-s5m-alarm-3de705ea53ce
Best regards,
--
André Draszik <andre.draszik@linaro.org>
^ permalink raw reply
* [PATCH 1/3] mfd: sec: add rtc alarm IRQ as platform device resource
From: André Draszik @ 2025-11-14 11:47 UTC (permalink / raw)
To: Krzysztof Kozlowski, Lee Jones, Alexandre Belloni
Cc: Peter Griffin, Tudor Ambarus, Will McVicker, Juan Yescas,
Douglas Anderson, kernel-team, Kaustabh Chakraborty, linux-kernel,
linux-samsung-soc, linux-rtc, André Draszik
In-Reply-To: <20251114-s5m-alarm-v1-0-c9b3bebae65f@linaro.org>
By adding the RTC alarm IRQ to the MFD cell as a resource, the child
driver (rtc) can simply query that IRQ, instead of having a lookup
table itself.
This change therefore allows the child driver to be simplified with
regards to determining the alarm IRQ.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/mfd/sec-common.c | 38 +++++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
index 42d55e70e34c8d7cd68cddaecc88017e259365b4..77370db52a7ba81234136b29f85892f4b197f429 100644
--- a/drivers/mfd/sec-common.c
+++ b/drivers/mfd/sec-common.c
@@ -23,9 +23,13 @@
#include <linux/regmap.h>
#include "sec-core.h"
+static const struct resource s5m8767_rtc_resources[] = {
+ DEFINE_RES_IRQ_NAMED(S5M8767_IRQ_RTCA1, "alarm"),
+};
+
static const struct mfd_cell s5m8767_devs[] = {
MFD_CELL_NAME("s5m8767-pmic"),
- MFD_CELL_NAME("s5m-rtc"),
+ MFD_CELL_RES("s5m-rtc", s5m8767_rtc_resources),
MFD_CELL_OF("s5m8767-clk", NULL, NULL, 0, 0, "samsung,s5m8767-clk"),
};
@@ -33,50 +37,66 @@ static const struct mfd_cell s2dos05_devs[] = {
MFD_CELL_NAME("s2dos05-regulator"),
};
+static const struct resource s2mpg10_rtc_resources[] = {
+ DEFINE_RES_IRQ_NAMED(S2MPG10_IRQ_RTCA0, "alarm"),
+};
+
static const struct mfd_cell s2mpg10_devs[] = {
MFD_CELL_NAME("s2mpg10-meter"),
MFD_CELL_NAME("s2mpg10-regulator"),
- MFD_CELL_NAME("s2mpg10-rtc"),
+ MFD_CELL_RES("s2mpg10-rtc", s2mpg10_rtc_resources),
MFD_CELL_OF("s2mpg10-clk", NULL, NULL, 0, 0, "samsung,s2mpg10-clk"),
MFD_CELL_OF("s2mpg10-gpio", NULL, NULL, 0, 0, "samsung,s2mpg10-gpio"),
};
+static const struct resource s2mps11_rtc_resources[] = {
+ DEFINE_RES_IRQ_NAMED(S2MPS11_IRQ_RTCA0, "alarm"),
+};
+
static const struct mfd_cell s2mps11_devs[] = {
MFD_CELL_NAME("s2mps11-regulator"),
- MFD_CELL_NAME("s2mps14-rtc"),
+ MFD_CELL_RES("s2mps14-rtc", s2mps11_rtc_resources),
MFD_CELL_OF("s2mps11-clk", NULL, NULL, 0, 0, "samsung,s2mps11-clk"),
};
+static const struct resource s2mps14_rtc_resources[] = {
+ DEFINE_RES_IRQ_NAMED(S2MPS14_IRQ_RTCA0, "alarm"),
+};
+
static const struct mfd_cell s2mps13_devs[] = {
MFD_CELL_NAME("s2mps13-regulator"),
- MFD_CELL_NAME("s2mps13-rtc"),
+ MFD_CELL_RES("s2mps13-rtc", s2mps14_rtc_resources),
MFD_CELL_OF("s2mps13-clk", NULL, NULL, 0, 0, "samsung,s2mps13-clk"),
};
static const struct mfd_cell s2mps14_devs[] = {
MFD_CELL_NAME("s2mps14-regulator"),
- MFD_CELL_NAME("s2mps14-rtc"),
+ MFD_CELL_RES("s2mps14-rtc", s2mps14_rtc_resources),
MFD_CELL_OF("s2mps14-clk", NULL, NULL, 0, 0, "samsung,s2mps14-clk"),
};
static const struct mfd_cell s2mps15_devs[] = {
MFD_CELL_NAME("s2mps15-regulator"),
- MFD_CELL_NAME("s2mps15-rtc"),
+ MFD_CELL_RES("s2mps15-rtc", s2mps14_rtc_resources),
MFD_CELL_OF("s2mps13-clk", NULL, NULL, 0, 0, "samsung,s2mps13-clk"),
};
static const struct mfd_cell s2mpa01_devs[] = {
MFD_CELL_NAME("s2mpa01-pmic"),
- MFD_CELL_NAME("s2mps14-rtc"),
+ MFD_CELL_RES("s2mps14-rtc", s2mps14_rtc_resources),
};
static const struct mfd_cell s2mpu02_devs[] = {
MFD_CELL_NAME("s2mpu02-regulator"),
};
+static const struct resource s2mpu05_rtc_resources[] = {
+ DEFINE_RES_IRQ_NAMED(S2MPU05_IRQ_RTCA0, "alarm"),
+};
+
static const struct mfd_cell s2mpu05_devs[] = {
MFD_CELL_NAME("s2mpu05-regulator"),
- MFD_CELL_NAME("s2mps15-rtc"),
+ MFD_CELL_RES("s2mps15-rtc", s2mpu05_rtc_resources),
};
static void sec_pmic_dump_rev(struct sec_pmic_dev *sec_pmic)
@@ -220,7 +240,7 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
sec_pmic->device_type);
}
ret = devm_mfd_add_devices(sec_pmic->dev, -1, sec_devs, num_sec_devs,
- NULL, 0, NULL);
+ NULL, 0, regmap_irq_get_domain(sec_pmic->irq_data));
if (ret)
return ret;
--
2.52.0.rc1.455.g30608eb744-goog
^ permalink raw reply related
* [PATCH 2/3] rtc: s5m: query platform device IRQ resource for alarm IRQ
From: André Draszik @ 2025-11-14 11:47 UTC (permalink / raw)
To: Krzysztof Kozlowski, Lee Jones, Alexandre Belloni
Cc: Peter Griffin, Tudor Ambarus, Will McVicker, Juan Yescas,
Douglas Anderson, kernel-team, Kaustabh Chakraborty, linux-kernel,
linux-samsung-soc, linux-rtc, André Draszik
In-Reply-To: <20251114-s5m-alarm-v1-0-c9b3bebae65f@linaro.org>
The core driver now exposes the alarm IRQ as a resource, so we can drop
the lookup from here to simplify the code and make adding support for
additional variants easier in this driver.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/rtc/rtc-s5m.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
index a7220b4d0e8dd35786b060e2a4106e2a39fe743f..c6ed5a4ca8a0e4554b1c88c879b01fc384735007 100644
--- a/drivers/rtc/rtc-s5m.c
+++ b/drivers/rtc/rtc-s5m.c
@@ -15,7 +15,6 @@
#include <linux/rtc.h>
#include <linux/platform_device.h>
#include <linux/mfd/samsung/core.h>
-#include <linux/mfd/samsung/irq.h>
#include <linux/mfd/samsung/rtc.h>
#include <linux/mfd/samsung/s2mps14.h>
@@ -683,22 +682,18 @@ static int s5m_rtc_probe(struct platform_device *pdev)
case S2MPS15X:
regmap_cfg = &s2mps14_rtc_regmap_config;
info->regs = &s2mps15_rtc_regs;
- alarm_irq = S2MPS14_IRQ_RTCA0;
break;
case S2MPS14X:
regmap_cfg = &s2mps14_rtc_regmap_config;
info->regs = &s2mps14_rtc_regs;
- alarm_irq = S2MPS14_IRQ_RTCA0;
break;
case S2MPS13X:
regmap_cfg = &s2mps14_rtc_regmap_config;
info->regs = &s2mps13_rtc_regs;
- alarm_irq = S2MPS14_IRQ_RTCA0;
break;
case S5M8767X:
regmap_cfg = &s5m_rtc_regmap_config;
info->regs = &s5m_rtc_regs;
- alarm_irq = S5M8767_IRQ_RTCA1;
break;
default:
return dev_err_probe(&pdev->dev, -ENODEV,
@@ -719,7 +714,6 @@ static int s5m_rtc_probe(struct platform_device *pdev)
"Failed to allocate regmap\n");
} else if (device_type == S2MPG10) {
info->regs = &s2mpg10_rtc_regs;
- alarm_irq = S2MPG10_IRQ_RTCA0;
} else {
return dev_err_probe(&pdev->dev, -ENODEV,
"Unsupported device type %d\n",
@@ -730,13 +724,14 @@ static int s5m_rtc_probe(struct platform_device *pdev)
info->s5m87xx = s5m87xx;
info->device_type = device_type;
- if (s5m87xx->irq_data) {
- info->irq = regmap_irq_get_virq(s5m87xx->irq_data, alarm_irq);
- if (info->irq <= 0)
- return dev_err_probe(&pdev->dev, -EINVAL,
- "Failed to get virtual IRQ %d\n",
- alarm_irq);
- }
+ alarm_irq = platform_get_irq_byname_optional(pdev, "alarm");
+ if (alarm_irq > 0)
+ info->irq = alarm_irq;
+ else if (alarm_irq == -ENXIO)
+ info->irq = 0;
+ else
+ return dev_err_probe(&pdev->dev, alarm_irq ? : -EINVAL,
+ "IRQ 'alarm' not found\n");
platform_set_drvdata(pdev, info);
--
2.52.0.rc1.455.g30608eb744-goog
^ permalink raw reply related
* [PATCH 3/3] mfd: sec: drop now unused struct sec_pmic_dev::irq_data
From: André Draszik @ 2025-11-14 11:47 UTC (permalink / raw)
To: Krzysztof Kozlowski, Lee Jones, Alexandre Belloni
Cc: Peter Griffin, Tudor Ambarus, Will McVicker, Juan Yescas,
Douglas Anderson, kernel-team, Kaustabh Chakraborty, linux-kernel,
linux-samsung-soc, linux-rtc, André Draszik
In-Reply-To: <20251114-s5m-alarm-v1-0-c9b3bebae65f@linaro.org>
This was used only to allow the s5m RTC driver to deal with the alarm
IRQ. That driver now uses a different approach to acquire that IRQ, and
::irq_data doesn't need to be kept around anymore.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/mfd/sec-common.c | 5 +++--
drivers/mfd/sec-core.h | 2 +-
drivers/mfd/sec-irq.c | 10 ++--------
include/linux/mfd/samsung/core.h | 1 -
4 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
index 77370db52a7ba81234136b29f85892f4b197f429..794c4e5398e7dd1a816aff9a6559a6c19fec75a5 100644
--- a/drivers/mfd/sec-common.c
+++ b/drivers/mfd/sec-common.c
@@ -163,6 +163,7 @@ sec_pmic_parse_dt_pdata(struct device *dev)
int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
struct regmap *regmap, struct i2c_client *client)
{
+ struct regmap_irq_chip_data *irq_data;
struct sec_platform_data *pdata;
const struct mfd_cell *sec_devs;
struct sec_pmic_dev *sec_pmic;
@@ -187,7 +188,7 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
sec_pmic->pdata = pdata;
- ret = sec_irq_init(sec_pmic);
+ ret = sec_irq_init(sec_pmic, &irq_data);
if (ret)
return ret;
@@ -240,7 +241,7 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
sec_pmic->device_type);
}
ret = devm_mfd_add_devices(sec_pmic->dev, -1, sec_devs, num_sec_devs,
- NULL, 0, regmap_irq_get_domain(sec_pmic->irq_data));
+ NULL, 0, regmap_irq_get_domain(irq_data));
if (ret)
return ret;
diff --git a/drivers/mfd/sec-core.h b/drivers/mfd/sec-core.h
index 92c7558ab8b0de44a52e028eeb7998e38358cb4c..c639180ea686f4308af3f872cb1d2209d201b2e7 100644
--- a/drivers/mfd/sec-core.h
+++ b/drivers/mfd/sec-core.h
@@ -18,6 +18,6 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
struct regmap *regmap, struct i2c_client *client);
void sec_pmic_shutdown(struct device *dev);
-int sec_irq_init(struct sec_pmic_dev *sec_pmic);
+int sec_irq_init(struct sec_pmic_dev *sec_pmic, struct regmap_irq_chip_data **irq_data);
#endif /* __SEC_CORE_INT_H */
diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
index c5c80b1ba104e6c5a55b442d2f10a8554201a961..05d4cc350a351d994e00ba08f5ce966d0d5c6a0b 100644
--- a/drivers/mfd/sec-irq.c
+++ b/drivers/mfd/sec-irq.c
@@ -253,7 +253,7 @@ static const struct regmap_irq_chip s5m8767_irq_chip = {
.ack_base = S5M8767_REG_INT1,
};
-int sec_irq_init(struct sec_pmic_dev *sec_pmic)
+int sec_irq_init(struct sec_pmic_dev *sec_pmic, struct regmap_irq_chip_data **irq_data)
{
const struct regmap_irq_chip *sec_irq_chip;
int ret;
@@ -302,17 +302,11 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
ret = devm_regmap_add_irq_chip(sec_pmic->dev, sec_pmic->regmap_pmic,
sec_pmic->irq, IRQF_ONESHOT,
- 0, sec_irq_chip, &sec_pmic->irq_data);
+ 0, sec_irq_chip, irq_data);
if (ret)
return dev_err_probe(sec_pmic->dev, ret,
"Failed to add %s IRQ chip\n",
sec_irq_chip->name);
- /*
- * The rtc-s5m driver requests S2MPS14_IRQ_RTCA0 also for S2MPS11
- * so the interrupt number must be consistent.
- */
- BUILD_BUG_ON(((enum s2mps14_irq)S2MPS11_IRQ_RTCA0) != S2MPS14_IRQ_RTCA0);
-
return 0;
}
diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
index d785e101fe795a5d8f9cccf4ccc4232437e89416..c7c3c8cd8d5f99ef0cc3188e1c3b49031f4750f2 100644
--- a/include/linux/mfd/samsung/core.h
+++ b/include/linux/mfd/samsung/core.h
@@ -69,7 +69,6 @@ struct sec_pmic_dev {
int device_type;
int irq;
- struct regmap_irq_chip_data *irq_data;
};
struct sec_platform_data {
--
2.52.0.rc1.455.g30608eb744-goog
^ permalink raw reply related
* Re: [PATCH 06/13] mfd: sec-irq: add support for creating multiple IRQ chips
From: André Draszik @ 2025-11-14 11:55 UTC (permalink / raw)
To: Alexandre Belloni, Kaustabh Chakraborty
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, MyungJoo Ham, Chanwoo Choi, Sebastian Reichel,
Krzysztof Kozlowski, Jonathan Corbet, linux-leds, devicetree,
linux-kernel, linux-pm, linux-samsung-soc, linux-rtc, linux-doc
In-Reply-To: <20251114075004a444bff0@mail.local>
On Fri, 2025-11-14 at 08:50 +0100, Alexandre Belloni wrote:
> On 14/11/2025 00:35:07+0530, Kaustabh Chakraborty wrote:
> > The current state of the driver only allows creating only one IRQ chip
> > per PMIC. On some PMICs, such as Samsung's S2MU005, there are multiple
> > interrupt blocks, for which the current implementation stands insufficient.
> >
> > Add support for creating multiple IRQ chips for a PMIC. Every IRQ chip is
> > given it's own index, which is used by sub-device drivers to request IRQs.
> >
> > A macro is defined which states the maximum number of chips supported.
> > It's set to 1 as currently, no PMIC requires more than one IRQ chip. The
> > value must be changed accordingly on adding new PMICs requiring multiple
> > IRQ chips.
> >
> > Moreover, adjust the s5m RTC driver to initialize IRQs with the
> > appropriate IRQ chip index.
> >
> > Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>
> > ---
> > drivers/mfd/sec-irq.c | 163 +++++++++++++++++++++++----------------
> > drivers/rtc/rtc-s5m.c | 15 +++-
> > include/linux/mfd/samsung/core.h | 5 +-
> > include/linux/mfd/samsung/irq.h | 14 ++++
> > 4 files changed, 127 insertions(+), 70 deletions(-)
Your patch reminded me to finally send
https://lore.kernel.org/all/20251114-s5m-alarm-v1-0-c9b3bebae65f@linaro.org/
If applied first, you wouldn't need to touch rtc-s5m.c I believe.
Equally, I can rebase mine on top of yours - no strong feelings.
Cheers,
Andre'
^ permalink raw reply
* Re: [PATCH 08/13] mfd: sec: store hardware revision in sec_pmic_dev and add S2MU005 support
From: André Draszik @ 2025-11-14 13:14 UTC (permalink / raw)
To: Kaustabh Chakraborty, Lee Jones, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, MyungJoo Ham, Chanwoo Choi,
Sebastian Reichel, Krzysztof Kozlowski, Alexandre Belloni,
Jonathan Corbet
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc
In-Reply-To: <20251114-s2mu005-pmic-v1-8-9e3184d3a0c9@disroot.org>
On Fri, 2025-11-14 at 00:35 +0530, Kaustabh Chakraborty wrote:
> The device revision matters in cases when in some PMICs, the correct
> register offsets very in different revisions. Instead of just debug
> printing the value, store it in the driver data struct.
>
> Unlike other devices, S2MU005 has its hardware revision ID in register
> offset 0x73. Allow handling different devices and add support for S2MU005.
>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
> drivers/mfd/sec-common.c | 30 ++++++++++++++++++++++++------
> include/linux/mfd/samsung/core.h | 3 +++
> 2 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
> index 4c5f4dc2905b..f51c53e7a164 100644
> --- a/drivers/mfd/sec-common.c
> +++ b/drivers/mfd/sec-common.c
> @@ -16,6 +16,7 @@
> #include <linux/mfd/samsung/irq.h>
> #include <linux/mfd/samsung/s2mps11.h>
> #include <linux/mfd/samsung/s2mps13.h>
> +#include <linux/mfd/samsung/s2mu005.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/pm.h>
> @@ -86,17 +87,34 @@ static const struct mfd_cell s2mu005_devs[] = {
> MFD_CELL_OF("s2mu005-rgb", NULL, NULL, 0, 0, "samsung,s2mu005-rgb"),
> };
>
> -static void sec_pmic_dump_rev(struct sec_pmic_dev *sec_pmic)
> +static void sec_pmic_store_rev(struct sec_pmic_dev *sec_pmic)
> {
> - unsigned int val;
> + unsigned int reg, mask, shift;
>
> /* For s2mpg1x, the revision is in a different regmap */
> if (sec_pmic->device_type == S2MPG10)
> return;
>
> - /* For each device type, the REG_ID is always the first register */
> - if (!regmap_read(sec_pmic->regmap_pmic, S2MPS11_REG_ID, &val))
> - dev_dbg(sec_pmic->dev, "Revision: 0x%x\n", val);
> + switch (sec_pmic->device_type) {
> + case S2MU005:
> + reg = S2MU005_REG_ID;
> + mask = S2MU005_ID_MASK;
> + shift = S2MU005_ID_SHIFT;
> + break;
> + default:
> + /* For other device types, the REG_ID is always the first register. */
> + reg = S2MPS11_REG_ID;
> + mask = ~0;
> + shift = 0;
> + }
> +
> + if (!regmap_read(sec_pmic->regmap_pmic, reg, &sec_pmic->revision))
> + return;
You should probably propagate the error up to the caller here.
Cheers,
Andre'
> +
> + sec_pmic->revision &= mask;
> + sec_pmic->revision >>= shift;
> +
> + dev_dbg(sec_pmic->dev, "Revision: 0x%x\n", sec_pmic->revision);
> }
>
> static void sec_pmic_configure(struct sec_pmic_dev *sec_pmic)
> @@ -236,7 +254,7 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
> return ret;
>
> sec_pmic_configure(sec_pmic);
> - sec_pmic_dump_rev(sec_pmic);
> + sec_pmic_store_rev(sec_pmic);
>
> return ret;
> }
> diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
> index fc07f7944dcd..ccd1bfa15b85 100644
> --- a/include/linux/mfd/samsung/core.h
> +++ b/include/linux/mfd/samsung/core.h
> @@ -63,6 +63,7 @@ enum sec_device_type {
> * @irq_base: Base IRQ number for device, required for IRQs
> * @irq: Generic IRQ number for device
> * @irq_data: Runtime data structure for IRQ controller
> + * @revision: Revision number of the device
> * @wakeup: Whether or not this is a wakeup device
> */
> struct sec_pmic_dev {
> @@ -74,6 +75,8 @@ struct sec_pmic_dev {
> int device_type;
> int irq;
> struct regmap_irq_chip_data *irq_data[MAX_IRQ_CHIPS];
> +
> + unsigned int revision;
> };
>
> struct sec_platform_data {
^ permalink raw reply
* Re: [PATCH v4 14/16] power: supply: bd71828: Support wider register addresses
From: Matti Vaittinen @ 2025-11-14 13:22 UTC (permalink / raw)
To: Andreas Kemnade, Matti Vaittinen
Cc: Matti Vaittinen, Lee Jones, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sebastian Reichel,
Liam Girdwood, Mark Brown, Michael Turquette, Stephen Boyd,
Linus Walleij, Bartosz Golaszewski, Alexandre Belloni, linux-leds,
devicetree, linux-kernel, linux-pm, linux-clk, linux-gpio,
linux-rtc
In-Reply-To: <20251114121509.629d171b@kemnade.info>
Thanks Andreas,
On 14/11/2025 13:15, Andreas Kemnade wrote:
> On Thu, 13 Nov 2025 10:55:39 +0200
> Matti Vaittinen <matti.vaittinen@linux.dev> wrote:
>
>> As a side note, we can reduce the "wasted space / member / instance" from
>> 3 bytes to 1 byte, by using u16 instead of the unsigned int if needed. I
>> rather use unsigned int to be initially prepared for devices with 32 bit
>> registers if there is no need to count bytes.
>
> Well, this is totally internal to the module, so no ABI/API changes, so
> there is no advantage of using 32bit now I think. We can switch any time.
The only advantage is to avoid the churn if 32bit ICs are to be added.
> But we have 32bit stuff in the regmap cache anyways, so that is not above
> the general level of wasting space.
Exactly. And, I am not sure if sparing ~hundred bytes is worth the
hassle - even if it is hassle internal to the driver. But yeah, we can
squeeze a few bytes if it is seen beneficial. That's why I mentioned it
here :)
Yours,
-- Matti
---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* Re: [PATCH 2/3] rtc: s5m: query platform device IRQ resource for alarm IRQ
From: Alexandre Belloni @ 2025-11-14 15:58 UTC (permalink / raw)
To: André Draszik
Cc: Krzysztof Kozlowski, Lee Jones, Peter Griffin, Tudor Ambarus,
Will McVicker, Juan Yescas, Douglas Anderson, kernel-team,
Kaustabh Chakraborty, linux-kernel, linux-samsung-soc, linux-rtc
In-Reply-To: <20251114-s5m-alarm-v1-2-c9b3bebae65f@linaro.org>
On 14/11/2025 11:47:22+0000, André Draszik wrote:
> The core driver now exposes the alarm IRQ as a resource, so we can drop
> the lookup from here to simplify the code and make adding support for
> additional variants easier in this driver.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> drivers/rtc/rtc-s5m.c | 21 ++++++++-------------
> 1 file changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
> index a7220b4d0e8dd35786b060e2a4106e2a39fe743f..c6ed5a4ca8a0e4554b1c88c879b01fc384735007 100644
> --- a/drivers/rtc/rtc-s5m.c
> +++ b/drivers/rtc/rtc-s5m.c
> @@ -15,7 +15,6 @@
> #include <linux/rtc.h>
> #include <linux/platform_device.h>
> #include <linux/mfd/samsung/core.h>
> -#include <linux/mfd/samsung/irq.h>
> #include <linux/mfd/samsung/rtc.h>
> #include <linux/mfd/samsung/s2mps14.h>
>
> @@ -683,22 +682,18 @@ static int s5m_rtc_probe(struct platform_device *pdev)
> case S2MPS15X:
> regmap_cfg = &s2mps14_rtc_regmap_config;
> info->regs = &s2mps15_rtc_regs;
> - alarm_irq = S2MPS14_IRQ_RTCA0;
> break;
> case S2MPS14X:
> regmap_cfg = &s2mps14_rtc_regmap_config;
> info->regs = &s2mps14_rtc_regs;
> - alarm_irq = S2MPS14_IRQ_RTCA0;
> break;
> case S2MPS13X:
> regmap_cfg = &s2mps14_rtc_regmap_config;
> info->regs = &s2mps13_rtc_regs;
> - alarm_irq = S2MPS14_IRQ_RTCA0;
> break;
> case S5M8767X:
> regmap_cfg = &s5m_rtc_regmap_config;
> info->regs = &s5m_rtc_regs;
> - alarm_irq = S5M8767_IRQ_RTCA1;
> break;
> default:
> return dev_err_probe(&pdev->dev, -ENODEV,
> @@ -719,7 +714,6 @@ static int s5m_rtc_probe(struct platform_device *pdev)
> "Failed to allocate regmap\n");
> } else if (device_type == S2MPG10) {
> info->regs = &s2mpg10_rtc_regs;
> - alarm_irq = S2MPG10_IRQ_RTCA0;
> } else {
> return dev_err_probe(&pdev->dev, -ENODEV,
> "Unsupported device type %d\n",
> @@ -730,13 +724,14 @@ static int s5m_rtc_probe(struct platform_device *pdev)
> info->s5m87xx = s5m87xx;
> info->device_type = device_type;
>
> - if (s5m87xx->irq_data) {
> - info->irq = regmap_irq_get_virq(s5m87xx->irq_data, alarm_irq);
> - if (info->irq <= 0)
> - return dev_err_probe(&pdev->dev, -EINVAL,
> - "Failed to get virtual IRQ %d\n",
> - alarm_irq);
> - }
> + alarm_irq = platform_get_irq_byname_optional(pdev, "alarm");
> + if (alarm_irq > 0)
> + info->irq = alarm_irq;
> + else if (alarm_irq == -ENXIO)
> + info->irq = 0;
> + else
> + return dev_err_probe(&pdev->dev, alarm_irq ? : -EINVAL,
> + "IRQ 'alarm' not found\n");
>
> platform_set_drvdata(pdev, info);
>
>
> --
> 2.52.0.rc1.455.g30608eb744-goog
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v2 2/2] rtc: pcf85363: add support for additional features
From: Alexandre Belloni @ 2025-11-14 16:20 UTC (permalink / raw)
To: Lakshay Piplani
Cc: linux-rtc, linux-kernel, robh, krzk+dt, conor+dt, devicetree,
vikash.bansal, priyanka.jain, shashank.rebbapragada
In-Reply-To: <20250811082123.1099880-2-lakshay.piplani@nxp.com>
Hello,
On 11/08/2025 13:51:23+0530, Lakshay Piplani wrote:
> Add support for additional features to the NXP PCF8263/PCF85363 RTC driver:
> - Alarm2 (minute,hour,weekday)
> - Timestamps recording for TS pin and Battery switch-over events
> - Battery switch over detection
> - Offset calibration
> - Watchdog timer
This needs to be split per added function. The watchdog part has to be
reviewed by the watchdog maintainers
Alarm2 support definitively won't go in unless you provide an actual use
case.
>
> Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
> ---
> Changes in v2:
> - Watchdog related changes due to removal of vendor specific properties
> from device tree
> * remove vendor DT knobs (enable/timeout/stepsize/repeat)
> * use watchdog_init_timeout (with 10s default)
> * derive clock_sel from final timeout
> * default, repeat=true (repeat mode)
> - Fixed uninitalised warning on 'ret' (reported by kernel test robot)
> - Use dev_dbg instead of dev_info for debug related print messages
> - Minor cleanup and comments
>
> drivers/rtc/rtc-pcf85363.c | 557 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 522 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
> index 540042b9eec8..c5c59876bad5 100644
> --- a/drivers/rtc/rtc-pcf85363.c
> +++ b/drivers/rtc/rtc-pcf85363.c
> @@ -5,6 +5,10 @@
> * Driver for NXP PCF85363 real-time clock.
> *
> * Copyright (C) 2017 Eric Nelson
> + *
> + * Copyright 2025 NXP
> + * Added support for alarm2, timestamps, battery switch-over,
> + * watchdog, offset calibration.
> */
> #include <linux/module.h>
> #include <linux/i2c.h>
> @@ -15,7 +19,11 @@
> #include <linux/errno.h>
> #include <linux/bcd.h>
> #include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_device.h>
> #include <linux/regmap.h>
> +#include <linux/watchdog.h>
> +#include <linux/uaccess.h>
>
> /*
> * Date/Time registers
> @@ -100,19 +108,48 @@
> #define PIN_IO_INTA_OUT 2
> #define PIN_IO_INTA_HIZ 3
>
> +#define PIN_IO_TSPM GENMASK(3, 2)
> +#define PIN_IO_TSIM BIT(4)
> +
> #define OSC_CAP_SEL GENMASK(1, 0)
> #define OSC_CAP_6000 0x01
> #define OSC_CAP_12500 0x02
>
> #define STOP_EN_STOP BIT(0)
> +#define RTCM_BIT BIT(4)
>
> #define RESET_CPR 0xa4
>
> #define NVRAM_SIZE 0x40
>
> +#define TSR1_MASK 0x03
> +#define TSR2_MASK 0x07
> +#define TSR3_MASK 0x03
> +#define TSR1_SHIFT 0
> +#define TSR2_SHIFT 2
> +#define TSR3_SHIFT 6
> +
> +#define WD_MODE_REPEAT BIT(7)
> +#define WD_TIMEOUT_MASK GENMASK(6, 2)
> +#define WD_TIMEOUT_SHIFT 2
> +#define WD_CLKSEL_MASK GENMASK(1, 0)
> +#define WD_CLKSEL_0_25HZ 0x00
> +#define WD_CLKSEL_1HZ 0x01
> +#define WD_CLKSEL_4HZ 0x02
> +#define WD_CLKSEL_16HZ 0x03
> +
> +#define WD_TIMEOUT_MIN 1
> +#define WD_TIMEOUT_MAX 0x1F
> +
> +#define OFFSET_SIGN_BIT 7
> +#define OFFSET_MINIMUM -128
> +#define OFFSET_MAXIMUM 127
> +#define OFFSET_MASK 0xFF
> +
> struct pcf85363 {
> struct rtc_device *rtc;
> struct regmap *regmap;
> + u8 ts_valid_flags;
> };
>
> struct pcf85x63_config {
> @@ -120,6 +157,15 @@ struct pcf85x63_config {
> unsigned int num_nvram;
> };
>
> +struct pcf85363_watchdog {
> + struct watchdog_device wdd;
> + struct regmap *regmap;
> + struct device *dev;
> + u8 timeout_val;
> + u8 clock_sel;
> + bool repeat;
> +};
> +
> static int pcf85363_load_capacitance(struct pcf85363 *pcf85363, struct device_node *node)
> {
> u32 load = 7000;
> @@ -295,28 +341,147 @@ static int pcf85363_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
> {
> struct pcf85363 *pcf85363 = i2c_get_clientdata(dev_id);
> + bool handled = false;
> unsigned int flags;
> int err;
>
> err = regmap_read(pcf85363->regmap, CTRL_FLAGS, &flags);
> +
> if (err)
> return IRQ_NONE;
>
> + if (flags) {
> + dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s%s%s%s%s%s\n",
> + flags, (flags & FLAGS_A1F) ? " [A1F]" : "",
> + (flags & FLAGS_A2F) ? " [A2F]" : "",
> + (flags & FLAGS_BSF) ? " [BSF]" : "",
> + (flags & FLAGS_TSR1F) ? " [TSR1F]" : "",
> + (flags & FLAGS_TSR2F) ? " [TSR2F]" : "",
> + (flags & FLAGS_TSR3F) ? " [TSR3F]" : "",
> + (flags & FLAGS_WDF) ? " [WDF]" : "");
> + }
> +
> if (flags & FLAGS_A1F) {
> rtc_update_irq(pcf85363->rtc, 1, RTC_IRQF | RTC_AF);
> regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_A1F, 0);
> - return IRQ_HANDLED;
> + handled = true;
> + }
> +
> + if (flags & FLAGS_A2F) {
> + rtc_update_irq(pcf85363->rtc, 1, RTC_IRQF | RTC_AF);
> + regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_A2F, 0);
> + handled = true;
> + }
> +
> + if (flags & FLAGS_BSF) {
> + regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_BSF, 0);
> + handled = true;
> + }
> +
> + if (flags & FLAGS_TSR1F) {
> + regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_TSR1F, 0);
> + pcf85363->ts_valid_flags |= FLAGS_TSR1F;
> + handled = true;
> + }
> +
> + if (flags & FLAGS_TSR2F) {
> + regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_TSR2F, 0);
> + pcf85363->ts_valid_flags |= FLAGS_TSR2F;
> + handled = true;
> + }
> +
> + if (flags & FLAGS_TSR3F) {
> + regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_TSR3F, 0);
> + pcf85363->ts_valid_flags |= FLAGS_TSR3F;
> + handled = true;
> + }
> +
> + if (flags & FLAGS_WDF) {
> + regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_WDF, 0);
> + handled = true;
> + }
> +
> + return handled ? IRQ_HANDLED : IRQ_NONE;
> +}
> +
> +/*
> + * Read the current RTC offset from the CTRL_OFFSET
> + * register. This value is an 8-bit signed 2's complement
> + * value that corrects osciallator drift.
> + */
> +static int pcf85363_read_offset(struct device *dev, long *offset)
> +{
> + struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(pcf85363->regmap, CTRL_OFFSET, &val);
> +
> + if (ret)
> + return ret;
> +
> + *offset = sign_extend32(val & OFFSET_MASK, OFFSET_SIGN_BIT);
> +
> + return 0;
> +}
> +
> +/*
> + * Write an oscillator offset correction value to
> + * the CTRL_OFFSET register. The valid range is
> + * -128 to 127 (8-bit signed), typically used to fine
> + * tune accuracy.
> + */
> +static int pcf85363_set_offset(struct device *dev, long offset)
> +{
> + struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
> +
> + if (offset < OFFSET_MINIMUM || offset > OFFSET_MAXIMUM) {
> + dev_warn(dev, "Offset out of range: %ld\n", offset);
> + return -ERANGE;
> + }
> +
> + return regmap_write(pcf85363->regmap, CTRL_OFFSET, offset & OFFSET_MASK);
> +}
> +
> +static int pcf85363_rtc_ioctl(struct device *dev,
> + unsigned int cmd, unsigned long arg)
> +{
> + struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
> + unsigned int val;
> + int ret;
> +
> + switch (cmd) {
> + case RTC_VL_READ: {
> + u32 status = 0;
> +
> + ret = regmap_read(pcf85363->regmap, CTRL_FLAGS, &val);
> +
> + if (ret)
> + return ret;
> +
> + if (val & FLAGS_BSF)
> + status |= RTC_VL_BACKUP_SWITCH;
> +
> + return put_user(status, (u32 __user *)arg);
> }
>
> - return IRQ_NONE;
> + case RTC_VL_CLR:
> + return regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_BSF, 0);
> +
> + default:
> + return -ENOIOCTLCMD;
> + }
> }
>
> static const struct rtc_class_ops rtc_ops = {
> + .ioctl = pcf85363_rtc_ioctl,
> .read_time = pcf85363_rtc_read_time,
> .set_time = pcf85363_rtc_set_time,
> .read_alarm = pcf85363_rtc_read_alarm,
> .set_alarm = pcf85363_rtc_set_alarm,
> .alarm_irq_enable = pcf85363_rtc_alarm_irq_enable,
> + .read_offset = pcf85363_read_offset,
> + .set_offset = pcf85363_set_offset,
> };
>
> static int pcf85363_nvram_read(void *priv, unsigned int offset, void *val,
> @@ -379,11 +544,297 @@ static const struct pcf85x63_config pcf_85363_config = {
> .num_nvram = 2
> };
>
> +/*
> + * This function sets the watchdog control register based on the timeout,
> + * clock selection and repeat mode settings. It prepares the value to
> + * write into the watchdog control register (CTRL_WDOG).
> + */
> +static int pcf85363_wdt_reload(struct pcf85363_watchdog *wd)
> +{
> + u8 val;
> +
> + val = (wd->repeat ? WD_MODE_REPEAT : 0) |
> + ((wd->timeout_val & WD_TIMEOUT_MAX) << WD_TIMEOUT_SHIFT) |
> + (wd->clock_sel & WD_CLKSEL_MASK);
> +
> + return regmap_write(wd->regmap, CTRL_WDOG, val);
> +}
> +
> +static int pcf85363_wdt_start(struct watchdog_device *wdd)
> +{
> + struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
> +
> + return pcf85363_wdt_reload(wd);
> +}
> +
> +static int pcf85363_wdt_stop(struct watchdog_device *wdd)
> +{
> + struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
> +
> + return regmap_write(wd->regmap, CTRL_WDOG, 0);
> +}
> +
> +static int pcf85363_wdt_ping(struct watchdog_device *wdd)
> +{
> + struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
> +
> + regmap_update_bits(wd->regmap, CTRL_FLAGS, FLAGS_WDF, 0);
> +
> + return pcf85363_wdt_reload(wd);
> +}
> +
> +static int pcf85363_wdt_set_timeout(struct watchdog_device *wdd,
> + unsigned int timeout)
> +{
> + struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
> +
> + wd->timeout_val = clamp(timeout, WD_TIMEOUT_MIN, WD_TIMEOUT_MAX);
> + wdd->timeout = wd->timeout_val;
> +
> + return pcf85363_wdt_reload(wd);
> +}
> +
> +static const struct watchdog_info pcf85363_wdt_info = {
> + .identity = "PCF85363 Watchdog",
> + .options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
> +};
> +
> +static const struct watchdog_ops pcf85363_wdt_ops = {
> + .owner = THIS_MODULE,
> + .start = pcf85363_wdt_start,
> + .stop = pcf85363_wdt_stop,
> + .ping = pcf85363_wdt_ping,
> + .set_timeout = pcf85363_wdt_set_timeout,
> +};
> +
> +static int pcf85363_watchdog_init(struct device *dev, struct regmap *regmap)
> +{
> + struct pcf85363_watchdog *wd;
> + unsigned int timeout_sec;
> + int ret;
> +
> + if (!IS_ENABLED(CONFIG_WATCHDOG))
> + return 0;
> +
> + wd = devm_kzalloc(dev, sizeof(*wd), GFP_KERNEL);
> + if (!wd)
> + return -ENOMEM;
> +
> + wd->regmap = regmap;
> + wd->dev = dev;
> +
> + wd->wdd.info = &pcf85363_wdt_info;
> + wd->wdd.ops = &pcf85363_wdt_ops;
> + wd->wdd.min_timeout = WD_TIMEOUT_MIN;
> + wd->wdd.max_timeout = WD_TIMEOUT_MAX;
> + wd->wdd.parent = dev;
> + wd->wdd.status = WATCHDOG_NOWAYOUT_INIT_STATUS;
> +
> + ret = watchdog_init_timeout(&wd->wdd, 10, dev);
> + if (ret)
> + wd->wdd.timeout = clamp(10U, WD_TIMEOUT_MIN, WD_TIMEOUT_MAX);
> +
> + timeout_sec = wd->wdd.timeout;
> +
> + if (timeout_sec <= 2)
> + wd->clock_sel = WD_CLKSEL_16HZ;
> + else if (timeout_sec <= 8)
> + wd->clock_sel = WD_CLKSEL_4HZ;
> + else if (timeout_sec <= 16)
> + wd->clock_sel = WD_CLKSEL_1HZ;
> + else
> + wd->clock_sel = WD_CLKSEL_0_25HZ;
> +
> + wd->repeat = true;
> +
> + ret = regmap_update_bits(regmap, CTRL_FLAGS, FLAGS_WDF, 0);
> + if (ret) {
> + dev_err(dev, "failed to clear WDF:%d\n", ret);
> + return ret;
> + }
> +
> + watchdog_set_drvdata(&wd->wdd, wd);
> +
> + dev_dbg(dev, "pcf85363 watchdog registered (timeout=%us, clk_sel=%u)\n",
> + timeout_sec, wd->clock_sel);
> +
> + return devm_watchdog_register_device(dev, &wd->wdd);
> +}
> +
> +/*
> + * Parses a string in the format "min hour weekday", validates the values,
> + * converts them to BCD, writes them to the Alarm2 registers, and enables
> + * the Alarm2 time match bits (minute, hour, weekday).
> + */
> +static ssize_t alarm2_time_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
> + int min, hour, weekday;
> + u8 regbuf[3];
> + int ret;
> +
> + if (sscanf(buf, "%d %d %d", &min, &hour, &weekday) != 3)
> + return -EINVAL;
> +
> + if (min < 0 || min > 59 || hour < 0 || hour > 23 || weekday < 0 || weekday > 6)
> + return -EINVAL;
> +
> + regbuf[0] = bin2bcd(min);
> + regbuf[1] = bin2bcd(hour);
> + regbuf[2] = weekday & 0x07;
> +
> + ret = regmap_bulk_write(pcf85363->regmap, DT_MINUTE_ALM2, regbuf, sizeof(regbuf));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(pcf85363->regmap, DT_ALARM_EN,
> + ALRM_MIN_A2E | ALRM_HR_A2E | ALRM_DAY_A2E,
> + ALRM_MIN_A2E | ALRM_HR_A2E | ALRM_DAY_A2E);
> + if (ret)
> + return ret;
> +
> + return count;
> +}
> +
> +/*
> + * Parses a string ("0" or "1") to control Alarm2 interrupt generation.
> + * Also clears the Alarm2 flag if the alarm is being disabled.
> + */
> +static ssize_t alarm2_enable_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
> + unsigned long enable;
> + int ret;
> +
> + ret = kstrtoul(buf, 10, &enable);
> + if (ret)
> + return ret;
> +
> + if (enable) {
> + ret = regmap_update_bits(pcf85363->regmap, CTRL_INTA_EN,
> + INT_A2IE, INT_A2IE);
> + } else {
> + ret = regmap_update_bits(pcf85363->regmap, CTRL_INTA_EN,
> + INT_A2IE, 0);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(pcf85363->regmap, CTRL_FLAGS,
> + FLAGS_A2F, 0);
> + }
> +
> + if (ret)
> + return ret;
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR_WO(alarm2_time);
> +static DEVICE_ATTR_WO(alarm2_enable);
> +
> +static struct attribute *alarm2_attrs[] = {
> + &dev_attr_alarm2_time.attr,
> + &dev_attr_alarm2_enable.attr,
> + NULL
> +};
> +
> +static struct attribute_group alarm2_group = {
> + .name = "alarm2",
> + .attrs = alarm2_attrs,
> +};
> +
> +/*
> + * Reads 6 bytes of timestamp data starting at the given base register,
> + * converts them from BCD to binary, and formats the result into a
> + * human-readable string in "YYYY-MM-DD HH:MM:SS" format.
> + */
> +static int pcf85363_read_timestamp(struct pcf85363 *pcf85363, u8 base_reg, char *buf)
> +{
> + struct rtc_time tm;
> + u8 regs[6];
> + int ret;
> +
> + ret = regmap_bulk_read(pcf85363->regmap, base_reg, regs, sizeof(regs));
> +
> + if (ret)
> + return ret;
> +
> + tm.tm_sec = bcd2bin(regs[0]);
> + tm.tm_min = bcd2bin(regs[1]);
> + tm.tm_hour = bcd2bin(regs[2]);
> + tm.tm_mday = bcd2bin(regs[3]);
> + tm.tm_mon = bcd2bin(regs[4]) - 1;
> + tm.tm_year = bcd2bin(regs[5]) + 100;
> +
> + return sysfs_emit(buf, "%04d-%02d-%02d %02d:%02d:%02d\n",
> + tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
> + tm.tm_hour, tm.tm_min, tm.tm_sec);
> +}
> +
> +/*
> + * Checks whether a specific timestamp flag is set. If so, reads and
> + * returns the formatted timestamp. Otherwise, returns "00-00-00 00:00:00".
> + */
> +
> +static ssize_t pcf85363_timestamp_show(struct device *dev, char *buf,
> + u8 timestamp_flag, u8 base_reg)
> +{
> + struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
> +
> + if (!(pcf85363->ts_valid_flags & timestamp_flag))
> + return sysfs_emit(buf, "00-00-00 00:00:00\n");
> +
> + return pcf85363_read_timestamp(pcf85363, base_reg, buf);
> +}
> +
> +static ssize_t timestamp1_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + return pcf85363_timestamp_show(dev, buf, FLAGS_TSR1F, DT_TIMESTAMP1);
> +}
> +static DEVICE_ATTR_RO(timestamp1);
> +
> +static ssize_t timestamp2_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + return pcf85363_timestamp_show(dev, buf, FLAGS_TSR2F, DT_TIMESTAMP2);
> +}
> +static DEVICE_ATTR_RO(timestamp2);
> +
> +static ssize_t timestamp3_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + return pcf85363_timestamp_show(dev, buf, FLAGS_TSR3F, DT_TIMESTAMP3);
> +}
> +static DEVICE_ATTR_RO(timestamp3);
> +
> +static struct attribute *pcf85363_attrs[] = {
> + &dev_attr_timestamp1.attr,
> + &dev_attr_timestamp2.attr,
> + &dev_attr_timestamp3.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group pcf85363_attr_group = {
> + .attrs = pcf85363_attrs,
> +};
> +
> static int pcf85363_probe(struct i2c_client *client)
> {
> - struct pcf85363 *pcf85363;
> const struct pcf85x63_config *config = &pcf_85363_config;
> const void *data = of_device_get_match_data(&client->dev);
> + struct device *dev = &client->dev;
> + struct pcf85363 *pcf85363;
> + int irq_a = client->irq;
> + bool wakeup_source;
> + int ret, i, err;
> + u32 tsr_mode[3];
> + u8 val;
> +
> static struct nvmem_config nvmem_cfg[] = {
> {
> .name = "pcf85x63-",
> @@ -401,25 +852,43 @@ static int pcf85363_probe(struct i2c_client *client)
> .reg_write = pcf85363_nvram_write,
> },
> };
> - int ret, i, err;
> - bool wakeup_source;
>
> if (data)
> config = data;
>
> - pcf85363 = devm_kzalloc(&client->dev, sizeof(struct pcf85363),
> - GFP_KERNEL);
> + pcf85363 = devm_kzalloc(&client->dev, sizeof(*pcf85363), GFP_KERNEL);
> if (!pcf85363)
> return -ENOMEM;
>
> + pcf85363->ts_valid_flags = 0;
> +
> pcf85363->regmap = devm_regmap_init_i2c(client, &config->regmap);
> - if (IS_ERR(pcf85363->regmap)) {
> - dev_err(&client->dev, "regmap allocation failed\n");
> - return PTR_ERR(pcf85363->regmap);
> - }
> + if (IS_ERR(pcf85363->regmap))
> + return dev_err_probe(dev, PTR_ERR(pcf85363->regmap), "regmap init failed\n");
>
> i2c_set_clientdata(client, pcf85363);
>
> + ret = regmap_update_bits(pcf85363->regmap, CTRL_FUNCTION, RTCM_BIT, 0);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to enable RTC mode\n");
> +
> + if (!device_property_read_u32_array(dev, "nxp,timestamp-mode", tsr_mode, 3)) {
> + tsr_mode[0] &= TSR1_MASK;
> + tsr_mode[1] &= TSR2_MASK;
> + tsr_mode[2] &= TSR3_MASK;
> +
> + val = (tsr_mode[2] << TSR3_SHIFT) |
> + (tsr_mode[1] << TSR2_SHIFT) |
> + (tsr_mode[0] << TSR1_SHIFT);
> +
> + ret = regmap_write(pcf85363->regmap, DT_TS_MODE, val);
> + if (ret)
> + dev_warn(dev, "Failed to write timestamp mode register\n");
> +
> + dev_dbg(dev, "Timestamp mode set: TSR1=0x%x TSR2=0x%x TSR3=0x%x\n",
> + tsr_mode[0], tsr_mode[1], tsr_mode[2]);
> + }
> +
> pcf85363->rtc = devm_rtc_allocate_device(&client->dev);
> if (IS_ERR(pcf85363->rtc))
> return PTR_ERR(pcf85363->rtc);
> @@ -433,39 +902,57 @@ static int pcf85363_probe(struct i2c_client *client)
> pcf85363->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
> pcf85363->rtc->range_max = RTC_TIMESTAMP_END_2099;
>
> - wakeup_source = device_property_read_bool(&client->dev,
> - "wakeup-source");
> - if (client->irq > 0 || wakeup_source) {
> - regmap_write(pcf85363->regmap, CTRL_FLAGS, 0);
> - regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
> - PIN_IO_INTAPM, PIN_IO_INTA_OUT);
> - }
> + wakeup_source = device_property_read_bool(dev, "wakeup-source");
>
> - if (client->irq > 0) {
> - unsigned long irqflags = IRQF_TRIGGER_LOW;
> + ret = regmap_write(pcf85363->regmap, CTRL_FLAGS, 0x00);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to clear CTRL_FLAGS\n");
> +
> + if (irq_a > 0) {
> + regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO, PIN_IO_INTAPM, PIN_IO_INTA_OUT);
> + ret = devm_request_threaded_irq(dev, irq_a, NULL,
> + pcf85363_rtc_handle_irq,
> + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> + "pcf85363-inta", client);
>
> - if (dev_fwnode(&client->dev))
> - irqflags = 0;
> - ret = devm_request_threaded_irq(&client->dev, client->irq,
> - NULL, pcf85363_rtc_handle_irq,
> - irqflags | IRQF_ONESHOT,
> - "pcf85363", client);
> if (ret) {
> - dev_warn(&client->dev,
> - "unable to request IRQ, alarms disabled\n");
> - client->irq = 0;
> + dev_err_probe(dev, ret, "INTA IRQ request failed\n");
> + irq_a = 0;
> + } else {
> + regmap_write(pcf85363->regmap, CTRL_INTA_EN, INT_BSIE
> + | INT_TSRIE | INT_WDIE);
> }
> }
>
> - if (client->irq > 0 || wakeup_source) {
> - device_init_wakeup(&client->dev, true);
> - set_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
> - } else {
> - clear_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
> - }
> + regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
> + PIN_IO_TSPM | PIN_IO_TSIM,
> + PIN_IO_TSPM | PIN_IO_TSIM);
> +
> + ret = pcf85363_watchdog_init(dev, pcf85363->regmap);
> +
> + if (ret)
> + dev_err_probe(dev, ret, "Watchdog init failed\n");
> +
> + if (irq_a > 0 || wakeup_source)
> + device_init_wakeup(dev, true);
> +
> + dev_set_drvdata(&pcf85363->rtc->dev, pcf85363);
>
> ret = devm_rtc_register_device(pcf85363->rtc);
>
> + if (ret)
> + return dev_err_probe(dev, ret, "RTC registration failed\n");
> +
> + ret = sysfs_create_group(&pcf85363->rtc->dev.kobj, &alarm2_group);
> +
> + if (ret)
> + dev_err_probe(dev, ret, "Alarm2 sysfs creation failed\n");
> +
> + ret = sysfs_create_group(&pcf85363->rtc->dev.kobj, &pcf85363_attr_group);
Use rtc_add_group(), also, you are not allowed to fail after calling
devm_rtc_register_device)()
> +
> + if (ret)
> + dev_err_probe(dev, ret, "Timestamp sysfs creation failed\n");
> +
> for (i = 0; i < config->num_nvram; i++) {
> nvmem_cfg[i].priv = pcf85363;
> devm_rtc_nvmem_register(pcf85363->rtc, &nvmem_cfg[i]);
> --
> 2.25.1
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v4 01/16] dt-bindings: regulator: ROHM BD72720
From: Rob Herring (Arm) @ 2025-11-14 16:22 UTC (permalink / raw)
To: Matti Vaittinen
Cc: linux-kernel, Michael Turquette, devicetree, Krzysztof Kozlowski,
Lee Jones, Sebastian Reichel, Mark Brown, linux-clk,
Matti Vaittinen, linux-rtc, Conor Dooley, Andreas Kemnade,
Matti Vaittinen, Bartosz Golaszewski, Pavel Machek, Stephen Boyd,
Alexandre Belloni, linux-leds, linux-gpio, Liam Girdwood,
Linus Walleij, linux-pm
In-Reply-To: <21e83fccf2d2422f4bea1c482dcd3cb1aeda4085.1763022807.git.mazziesaccount@gmail.com>
On Thu, 13 Nov 2025 10:51:32 +0200, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> The ROHM BD72720 is a new PMIC with 10 BUCk and 11 LDO regulators.
>
> The BD72720 is designed to support using the BUCK10 as a supply for
> the LDOs 1 to 4. When the BUCK10 is used for this, it can be set to a
> LDON_HEAD mode. In this mode, the BUCK10 voltage can't be controlled by
> software, but the voltage is adjusted by PMIC to match the LDO1 .. LDO4
> voltages with a given offset. Offset can be 50mV .. 300mV and is
> changeable at 50mV steps.
>
> Add 'ldon-head-microvolt' property to denote a board which is designed
> to utilize the LDON_HEAD mode.
>
> All other properties are already existing.
>
> Add dt-binding doc for ROHM BD72720 regulators to make it usable.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>
> ---
> Revision history:
> v3 => v4:
> - Drop type from ldon-head
> - Fix the name patterns for regulator nodes and names
>
> v2 => v3:
> - drop unnecessary descriptions
> - use microvolts for the 'ldon-head' dt-property
>
> RFCv1 => v2:
> - No changes
> ---
> .../regulator/rohm,bd72720-regulator.yaml | 148 ++++++++++++++++++
> 1 file changed, 148 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd72720-regulator.yaml
>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 04/16] dt-bindings: power: supply: BD72720 managed battery
From: Rob Herring @ 2025-11-14 16:39 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Krzysztof Kozlowski, Mark Brown, Linus Walleij,
linux-kernel, Sebastian Reichel, Bartosz Golaszewski,
Alexandre Belloni, linux-clk, Michael Turquette, Matti Vaittinen,
linux-leds, Pavel Machek, Liam Girdwood, linux-gpio, linux-pm,
Andreas Kemnade, Conor Dooley, devicetree, linux-rtc, Lee Jones,
Stephen Boyd
In-Reply-To: <ee36d7d1-ef47-4a35-9aff-baa6ed32105a@gmail.com>
On Fri, Nov 14, 2025 at 11:04:27AM +0200, Matti Vaittinen wrote:
> On 13/11/2025 12:53, Rob Herring (Arm) wrote:
> >
> > On Thu, 13 Nov 2025 10:52:19 +0200, Matti Vaittinen wrote:
> > > From: Matti Vaittinen <mazziesaccount@gmail.com>
> > >
> > > The BD72720 PMIC has a battery charger + coulomb counter block. These
> > > can be used to manage charging of a lithium-ion battery and to do fuel
> > > gauging.
> > >
> > > ROHM has developed a so called "zero-correction" -algorithm to improve
> > > the fuel-gauging accuracy close to the point where battery is depleted.
> > > This relies on battery specific "VDR" tables, which are measured from
> > > the battery, and which describe the voltage drop rate. More thorough
> > > explanation about the "zero correction" and "VDR" parameters is here:
> > > https://lore.kernel.org/all/676253b9-ff69-7891-1f26-a8b5bb5a421b@fi.rohmeurope.com/
> > >
> > > Document the VDR zero-correction specific battery properties used by the
> > > BD72720 and some other ROHM chargers.
> > >
> > > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > >
> > > ---
> > > NOTE:
> > > Linus' rb-tag holds only if there's no further comments from Rob.
> > >
> > > Revision history:
> > > v3 =>:
> > > - No changes
> > >
> > > v2 => v3:
> > > - Constrain VDR threshold voltage to 48V
> > > - Use standard '-bp' -suffix for the rohm,volt-drop-soc
> > >
> > > RFCv1 => v2:
> > > - Add units to rohm,volt-drop-soc (tenths of %)
> > > - Give real temperatures matching the VDR tables, instead of vague
> > > 'high', 'normal', 'low', 'very low'. (Add table of temperatures and
> > > use number matching the right temperature index in the VDR table name).
> > > - Fix typoed 'algorithm' in commit message.
> > >
> > > The parameters are describing the battery voltage drop rates - so they
> > > are properties of the battery, not the charger. Thus they do not belong
> > > in the charger node.
> > >
>
> // snip
>
> > My bot found errors running 'make dt_binding_check' on your patch:
> >
> > yamllint warnings/errors:
> >
> > dtschema/dtc warnings/errors:
> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/rohm,vdr-battery.example.dtb: battery (simple-battery): 'degrade-cycle-microamp-hours', 'rohm,volt-drop-0-microvolt', 'rohm,volt-drop-1-microvolt', 'rohm,volt-drop-2-microvolt', 'rohm,volt-drop-3-temp-microvolt', 'rohm,volt-drop-soc-bp', 'rohm,volt-drop-temperatures-millicelsius', 'rohm,voltage-vdr-thresh-microvolt' do not match any of the regexes: '^ocv-capacity-table-[0-9]+$', '^pinctrl-[0-9]+$'
> > from schema $id: http://devicetree.org/schemas/power/supply/battery.yaml
> >
>
> Odd. I am pretty sure I didn't see this when I ran the make
> dt_binding_check. Not 100% sure what happened there. I get this error now
> though when including all the bindings to the check.
>
> Do I get this right - these errors result from the properties used in
> example not being included in the battery.yaml? So, this means that the
> check is done based on the binding (battery.yaml) where the compatible
> (simple-battery) is defined - not based on the properties which are present
> in this file where the example resides, (and which references the
> battery.yaml)?
>
> ...
>
> Oh... Now that I wrote it I feel like an idiot.
>
> This approach couldn't work for the validation, right? Let's assume I had a
> VDR battery, and I added a static-battery -node for it. Running the
> validation would pick the battery.yaml based on the compatible (just as it
> does here), and be completely unaware of this vdr-battery.yaml. I have no
> idea why I thought this would work. Probably because I only thought this
> from the documentation POV.
>
> So, as far as I understand, the only viable options are expanding the
> existing battery.yaml with these properties (which I hoped to avoid, see
> below)
>
> >> The right place for them is the battery node, which is described by the
> >> generic "battery.yaml". I was not comfortable with adding these
> >> properties to the generic battery.yaml because they are:
> >> - Meaningful only for those charger drivers which have the VDR
> >> algorithm implemented. (And even though the algorithm is not charger
> >> specific, AFAICS, it is currently only used by some ROHM PMIC
> >> drivers).
> >> - Technique of measuring the VDR tables for a battery is not widely
> >> known. AFAICS, only folks at ROHM are measuring those for some
> >> customer products. We do have those tables available for some of the
> >> products though (Kobo?).
>
> or, to add new compatible for the "vdr-battery".
> AFAICS, adding new compatible would require us to wither duplicate the used
> properties from battery.yaml here (as battery.yaml mandates the
> "simple-battery" - compatible) - or to split the battery.yaml in two files,
> one containing the generic properties, other containing the "simple-battery"
> -compatible and referencing the generic one. Then the "vdr-battery" could
> also reference the generic one.
>
> Any suggestions for the next path to follow?
Probably the latter option. You could do the former and make the new
properties conditional on the "vdr-battery" compatible. That's fine with
small differences, but gets messy as there are more properties and
variations.
But is "VDR" a type of battery though? Is there a certain type/chemistry
of battery we should be describing where VDR is applicable? I don't
think it scales well if we define battery compatibles for every
variation of charger algorithm. Honestly I don't mind just adding 1
property. I care more if we allow undocumented properties than
allowing documented but invalid for the platform properties. When it
becomes 10, 20, 30 properties, then I might start to care. If that
happens, either we are doing a poor job of generically describing
battery parameters or chargers and batteries are tightly coupled and
can't be described independently.
Rob
^ permalink raw reply
* Re: [PATCH v4 04/16] dt-bindings: power: supply: BD72720 managed battery
From: Andreas Kemnade @ 2025-11-14 17:40 UTC (permalink / raw)
To: Rob Herring
Cc: Matti Vaittinen, Matti Vaittinen, Krzysztof Kozlowski, Mark Brown,
Linus Walleij, linux-kernel, Sebastian Reichel,
Bartosz Golaszewski, Alexandre Belloni, linux-clk,
Michael Turquette, Matti Vaittinen, linux-leds, Pavel Machek,
Liam Girdwood, linux-gpio, linux-pm, Conor Dooley, devicetree,
linux-rtc, Lee Jones, Stephen Boyd
In-Reply-To: <20251114163954.GA3399895-robh@kernel.org>
On Fri, 14 Nov 2025 10:39:54 -0600
Rob Herring <robh@kernel.org> wrote:
> On Fri, Nov 14, 2025 at 11:04:27AM +0200, Matti Vaittinen wrote:
> > On 13/11/2025 12:53, Rob Herring (Arm) wrote:
> > >
> > > On Thu, 13 Nov 2025 10:52:19 +0200, Matti Vaittinen wrote:
> > > > From: Matti Vaittinen <mazziesaccount@gmail.com>
> > > >
> > > > The BD72720 PMIC has a battery charger + coulomb counter block. These
> > > > can be used to manage charging of a lithium-ion battery and to do fuel
> > > > gauging.
> > > >
> > > > ROHM has developed a so called "zero-correction" -algorithm to improve
> > > > the fuel-gauging accuracy close to the point where battery is depleted.
> > > > This relies on battery specific "VDR" tables, which are measured from
> > > > the battery, and which describe the voltage drop rate. More thorough
> > > > explanation about the "zero correction" and "VDR" parameters is here:
> > > > https://lore.kernel.org/all/676253b9-ff69-7891-1f26-a8b5bb5a421b@fi.rohmeurope.com/
> > > >
> > > > Document the VDR zero-correction specific battery properties used by the
> > > > BD72720 and some other ROHM chargers.
> > > >
> > > > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > >
> > > > ---
> > > > NOTE:
> > > > Linus' rb-tag holds only if there's no further comments from Rob.
> > > >
> > > > Revision history:
> > > > v3 =>:
> > > > - No changes
> > > >
> > > > v2 => v3:
> > > > - Constrain VDR threshold voltage to 48V
> > > > - Use standard '-bp' -suffix for the rohm,volt-drop-soc
> > > >
> > > > RFCv1 => v2:
> > > > - Add units to rohm,volt-drop-soc (tenths of %)
> > > > - Give real temperatures matching the VDR tables, instead of vague
> > > > 'high', 'normal', 'low', 'very low'. (Add table of temperatures and
> > > > use number matching the right temperature index in the VDR table name).
> > > > - Fix typoed 'algorithm' in commit message.
> > > >
> > > > The parameters are describing the battery voltage drop rates - so they
> > > > are properties of the battery, not the charger. Thus they do not belong
> > > > in the charger node.
> > > >
> >
> > // snip
> >
> > > My bot found errors running 'make dt_binding_check' on your patch:
> > >
> > > yamllint warnings/errors:
> > >
> > > dtschema/dtc warnings/errors:
> > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/rohm,vdr-battery.example.dtb: battery (simple-battery): 'degrade-cycle-microamp-hours', 'rohm,volt-drop-0-microvolt', 'rohm,volt-drop-1-microvolt', 'rohm,volt-drop-2-microvolt', 'rohm,volt-drop-3-temp-microvolt', 'rohm,volt-drop-soc-bp', 'rohm,volt-drop-temperatures-millicelsius', 'rohm,voltage-vdr-thresh-microvolt' do not match any of the regexes: '^ocv-capacity-table-[0-9]+$', '^pinctrl-[0-9]+$'
> > > from schema $id: http://devicetree.org/schemas/power/supply/battery.yaml
> > >
> >
> > Odd. I am pretty sure I didn't see this when I ran the make
> > dt_binding_check. Not 100% sure what happened there. I get this error now
> > though when including all the bindings to the check.
> >
> > Do I get this right - these errors result from the properties used in
> > example not being included in the battery.yaml? So, this means that the
> > check is done based on the binding (battery.yaml) where the compatible
> > (simple-battery) is defined - not based on the properties which are present
> > in this file where the example resides, (and which references the
> > battery.yaml)?
> >
> > ...
> >
> > Oh... Now that I wrote it I feel like an idiot.
> >
> > This approach couldn't work for the validation, right? Let's assume I had a
> > VDR battery, and I added a static-battery -node for it. Running the
> > validation would pick the battery.yaml based on the compatible (just as it
> > does here), and be completely unaware of this vdr-battery.yaml. I have no
> > idea why I thought this would work. Probably because I only thought this
> > from the documentation POV.
> >
> > So, as far as I understand, the only viable options are expanding the
> > existing battery.yaml with these properties (which I hoped to avoid, see
> > below)
> >
> > >> The right place for them is the battery node, which is described by the
> > >> generic "battery.yaml". I was not comfortable with adding these
> > >> properties to the generic battery.yaml because they are:
> > >> - Meaningful only for those charger drivers which have the VDR
> > >> algorithm implemented. (And even though the algorithm is not charger
> > >> specific, AFAICS, it is currently only used by some ROHM PMIC
> > >> drivers).
> > >> - Technique of measuring the VDR tables for a battery is not widely
> > >> known. AFAICS, only folks at ROHM are measuring those for some
> > >> customer products. We do have those tables available for some of the
> > >> products though (Kobo?).
> >
> > or, to add new compatible for the "vdr-battery".
> > AFAICS, adding new compatible would require us to wither duplicate the used
> > properties from battery.yaml here (as battery.yaml mandates the
> > "simple-battery" - compatible) - or to split the battery.yaml in two files,
> > one containing the generic properties, other containing the "simple-battery"
> > -compatible and referencing the generic one. Then the "vdr-battery" could
> > also reference the generic one.
> >
> > Any suggestions for the next path to follow?
>
> Probably the latter option. You could do the former and make the new
> properties conditional on the "vdr-battery" compatible. That's fine with
> small differences, but gets messy as there are more properties and
> variations.
>
just keep in mind, that several kobo devices have one pmic in one board
revision and another one in the other (e.g. Kobo Nia rev A vs rev C).
But probably the same battery. So if the "vdr-battery" is a compatible
just to allow a more properties,
then "simple-battery" should be allowed as fallback.
Regards,
Andreas
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox