* [PATCH v5 10/11] extcon: add support for Samsung S2M series PMIC extcon devices
From: Kaustabh Chakraborty @ 2026-04-23 19:39 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, Shuah Khan, Nam Tran,
Łukasz Lebiedziński
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc, Kaustabh Chakraborty
In-Reply-To: <20260424-s2mu005-pmic-v5-0-fcbc9da5a004@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 | 345 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 356 insertions(+)
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index 68d9df7d2dae0..19c712e591955 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -183,6 +183,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 6482f2bfd6611..e3939786f3474 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 0000000000000..e57031b0066bb
--- /dev/null
+++ b/drivers/extcon/extcon-s2m.c
@@ -0,0 +1,345 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Extcon Driver for Samsung S2M series PMICs.
+ *
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd
+ * Copyright (C) 2026 Kaustabh Chakraborty <kauschluss@disroot.org>
+ */
+
+#include <linux/delay.h>
+#include <linux/extcon-provider.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/samsung/core.h>
+#include <linux/mfd/samsung/s2mu005.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.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 char *name;
+ int (*const handler)(struct s2m_muic *);
+ int irq;
+};
+
+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,
+ FIELD_PREP(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,
+ FIELD_PREP(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,
+ FIELD_PREP(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[] = {
+ {
+ .name = "attach",
+ .handler = s2mu005_muic_attach
+ }, {
+ .name = "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].irq)
+ 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;
+ int ret;
+ int i;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, priv);
+ priv->dev = dev;
+ priv->regmap = pmic_drvdata->regmap_pmic;
+
+ switch (platform_get_device_id(pdev)->driver_data) {
+ case S2MU005:
+ priv->extcon_cable = s2mu005_muic_extcon_cable;
+ priv->irq_data = s2mu005_muic_irq_data;
+ /* 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(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 irq = platform_get_irq_byname_optional(pdev,
+ priv->irq_data[i].name);
+ if (irq == -ENXIO)
+ continue;
+ if (irq <= 0)
+ return dev_err_probe(dev, -EINVAL, "failed to get IRQ %s\n",
+ priv->irq_data[i].name);
+
+ priv->irq_data[i].irq = irq;
+ ret = devm_request_threaded_irq(dev, irq, NULL,
+ s2m_muic_irq_func, IRQF_ONESHOT,
+ priv->irq_data[i].name, priv);
+ if (ret)
+ return 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);
+
+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);
+
+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.53.0
^ permalink raw reply related
* [PATCH v5 11/11] power: supply: add support for Samsung S2M series PMIC charger device
From: Kaustabh Chakraborty @ 2026-04-23 19:39 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, Shuah Khan, Nam Tran,
Łukasz Lebiedziński
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc, Kaustabh Chakraborty
In-Reply-To: <20260424-s2mu005-pmic-v5-0-fcbc9da5a004@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, and USB 2.0 device negotiations
working.
The driver includes initial support for the S2MU005 PMIC charger.
Co-developed-by: Łukasz Lebiedziński <kernel@lvkasz.us>
Signed-off-by: Łukasz Lebiedziński <kernel@lvkasz.us>
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
drivers/power/supply/Kconfig | 11 ++
drivers/power/supply/Makefile | 1 +
drivers/power/supply/s2m-charger.c | 299 +++++++++++++++++++++++++++++++++++++
3 files changed, 311 insertions(+)
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 83392ed6a8da9..6270e6d16fbbb 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -856,6 +856,17 @@ config CHARGER_RK817
help
Say Y to include support for Rockchip RK817 Battery Charger.
+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 CHARGER_SMB347
tristate "Summit Microelectronics SMB3XX Battery Charger"
depends on I2C
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 7ee839dca7f33..738814650ea0f 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -107,6 +107,7 @@ obj-$(CONFIG_CHARGER_BQ25890) += bq25890_charger.o
obj-$(CONFIG_CHARGER_BQ25980) += bq25980_charger.o
obj-$(CONFIG_CHARGER_BQ256XX) += bq256xx_charger.o
obj-$(CONFIG_CHARGER_RK817) += rk817_charger.o
+obj-$(CONFIG_CHARGER_S2M) += s2m-charger.o
obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o
obj-$(CONFIG_CHARGER_TPS65090) += tps65090-charger.o
obj-$(CONFIG_CHARGER_TPS65217) += tps65217_charger.o
diff --git a/drivers/power/supply/s2m-charger.c b/drivers/power/supply/s2m-charger.c
new file mode 100644
index 0000000000000..b32cea55b8b04
--- /dev/null
+++ b/drivers/power/supply/s2m-charger.c
@@ -0,0 +1,299 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Battery Charger Driver for Samsung S2M series PMICs.
+ *
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd
+ * Copyright (c) 2026 Kaustabh Chakraborty <kauschluss@disroot.org>
+ * Copyright (c) 2026 Łukasz Lebiedziński <kernel@lvkasz.us>
+ */
+
+#include <linux/devm-helpers.h>
+#include <linux/extcon.h>
+#include <linux/mfd/samsung/core.h>
+#include <linux/mfd/samsung/s2mu005.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_graph.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 void s2mu005_chgr_get_usb_type(struct s2m_chgr *priv, int *value)
+{
+ if (extcon_get_state(priv->extcon, EXTCON_CHG_USB_CDP))
+ *value = POWER_SUPPLY_USB_TYPE_CDP;
+ if (extcon_get_state(priv->extcon, EXTCON_CHG_USB_SDP))
+ *value = POWER_SUPPLY_USB_TYPE_SDP;
+ if (extcon_get_state(priv->extcon, EXTCON_CHG_USB_DCP))
+ *value = POWER_SUPPLY_USB_TYPE_DCP;
+ else
+ *value = POWER_SUPPLY_USB_TYPE_UNKNOWN;
+}
+
+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;
+ case POWER_SUPPLY_PROP_USB_TYPE:
+ s2mu005_chgr_get_usb_type(priv, &val->intval);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static int s2mu005_chgr_mode_set_host(struct s2m_chgr *priv)
+{
+ int ret;
+
+ /* set mode to OTG */
+ ret = regmap_update_bits(priv->regmap, S2MU005_REG_CHGR_CTRL0,
+ S2MU005_CHGR_OP_MODE,
+ FIELD_PREP(S2MU005_CHGR_OP_MODE,
+ S2MU005_CHGR_OP_MODE_OTG));
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to set OTG mode (%d)\n", ret);
+ return ret;
+ }
+
+ /* set boost frequency to 2MHz */
+ ret = regmap_update_bits(priv->regmap, S2MU005_REG_CHGR_CTRL11,
+ S2MU005_CHGR_OSC_BOOST,
+ FIELD_PREP(S2MU005_CHGR_OSC_BOOST,
+ S2MU005_CHGR_OSC_BOOST_2MHZ));
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to set boost frequency (%d)\n", ret);
+ return ret;
+ }
+
+ /* set OTG current limit to 1.5 A */
+ ret = regmap_update_bits(priv->regmap, S2MU005_REG_CHGR_CTRL4,
+ S2MU005_CHGR_OTG_OCP,
+ FIELD_PREP(S2MU005_CHGR_OTG_OCP,
+ S2MU005_CHGR_OTG_OCP_1P5A));
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to set OTG current limit (%d)\n", ret);
+ return ret;
+ }
+
+ /* VBUS switches are OFF when OTG over-current happens */
+ ret = regmap_set_bits(priv->regmap, S2MU005_REG_CHGR_CTRL4,
+ S2MU005_CHGR_OTG_OCP_OFF);
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to set OTG OCP switch (%d)\n", ret);
+ return ret;
+ }
+
+ /* set OTG voltage to 5.1 V */
+ ret = regmap_update_bits(priv->regmap, S2MU005_REG_CHGR_CTRL5,
+ S2MU005_CHGR_VMID_BOOST,
+ FIELD_PREP(S2MU005_CHGR_VMID_BOOST,
+ S2MU005_CHGR_VMID_BOOST_5P1V));
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to set OTG voltage (%d)\n", ret);
+ return ret;
+ }
+
+ /* turn on OTG */
+ ret = regmap_update_bits(priv->regmap, S2MU005_REG_CHGR_CTRL15,
+ S2MU005_CHGR_OTG_EN,
+ FIELD_PREP(S2MU005_CHGR_OTG_EN,
+ S2MU005_CHGR_OTG_EN_ON));
+ if (ret < 0)
+ dev_err(priv->dev, "failed to turn on OTG (%d)\n", ret);
+ return ret;
+}
+
+static int s2mu005_chgr_mode_set_charger(struct s2m_chgr *priv)
+{
+ int ret;
+
+ /* first reset to mode 0 */
+ ret = regmap_clear_bits(priv->regmap, S2MU005_REG_CHGR_CTRL0,
+ S2MU005_CHGR_OP_MODE);
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to reset opmode (%d)\n", ret);
+ return ret;
+ }
+
+ /* wait for the charger to settle before switching to charging mode */
+ msleep(50);
+ /* then set to charging mode */
+ ret = regmap_update_bits(priv->regmap, S2MU005_REG_CHGR_CTRL0,
+ S2MU005_CHGR_OP_MODE,
+ FIELD_PREP(S2MU005_CHGR_OP_MODE,
+ S2MU005_CHGR_OP_MODE_CHG));
+ if (ret < 0)
+ dev_err(priv->dev, "failed to set opmode to charging (%d)\n", ret);
+ return ret;
+}
+
+static int s2mu005_chgr_mode_unset(struct s2m_chgr *priv)
+{
+ int ret;
+
+ /* turn off OTG */
+ ret = regmap_clear_bits(priv->regmap, S2MU005_REG_CHGR_CTRL15,
+ S2MU005_CHGR_OTG_EN);
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to turn off OTG (%d)\n", ret);
+ return ret;
+ }
+
+ /* reset operation mode */
+ ret = regmap_clear_bits(priv->regmap, S2MU005_REG_CHGR_CTRL0,
+ S2MU005_CHGR_OP_MODE);
+ if (ret < 0)
+ dev_err(priv->dev, "failed to reset opmode (%d)\n", ret);
+ return ret;
+}
+
+static void s2mu005_chgr_extcon_work(struct work_struct *work)
+{
+ struct s2m_chgr *priv = container_of(work, struct s2m_chgr, extcon_work);
+
+ if (extcon_get_state(priv->extcon, EXTCON_USB_HOST))
+ s2mu005_chgr_mode_set_host(priv);
+ else if (extcon_get_state(priv->extcon, EXTCON_USB))
+ s2mu005_chgr_mode_set_charger(priv);
+ else
+ s2mu005_chgr_mode_unset(priv);
+
+ power_supply_changed(priv->psy);
+}
+
+static const enum power_supply_property s2mu005_chgr_properties[] = {
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_USB_TYPE,
+};
+
+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,
+ .usb_types = BIT(POWER_SUPPLY_USB_TYPE_CDP) |
+ BIT(POWER_SUPPLY_USB_TYPE_SDP) |
+ BIT(POWER_SUPPLY_USB_TYPE_DCP) |
+ BIT(POWER_SUPPLY_USB_TYPE_UNKNOWN),
+};
+
+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 __free(device_node) = NULL;
+ 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 -ENOMEM;
+
+ 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;
+ psy_cfg.fwnode = dev_fwnode(dev->parent);
+ 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, "muic");
+ 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);
+
+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_AUTHOR("Łukasz Lebiedziński <kernel@lvkasz.us>");
+MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v1 3/4] ACPI: TAD: RTC: Refine timer value computations and checks
From: Alexandre Belloni @ 2026-04-23 20:30 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux ACPI, linux-rtc, LKML
In-Reply-To: <3414608.aeNJFYEL58@rafael.j.wysocki>
On 22/04/2026 17:26:49+0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Since rtc_tm_to_ktime() may overflow for large RTC time values and
> full second granularity is sufficient in timer value computations
> in acpi_tad_rtc_set_alarm() and acpi_tad_rtc_read_alarm(), use
> rtc_tm_to_time64() instead of that function, which also allows the
> computations to be simplified.
>
> Moreover, U32_MAX is a special "timer disabled" value, so make
> acpi_tad_rtc_set_alarm() reject it when attempting to program the
> alarm timers.
>
> Fixes: 7572dcabe38d ("ACPI: TAD: Add alarm support to the RTC class device interface")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> drivers/acpi/acpi_tad.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> --- a/drivers/acpi/acpi_tad.c
> +++ b/drivers/acpi/acpi_tad.c
> @@ -680,9 +680,8 @@ static int acpi_tad_rtc_set_alarm(struct
>
> acpi_tad_rt_to_tm(&rt, &tm_now);
>
> - value = ktime_divns(ktime_sub(rtc_tm_to_ktime(t->time),
> - rtc_tm_to_ktime(tm_now)), NSEC_PER_SEC);
> - if (value <= 0 || value > U32_MAX)
> + value = rtc_tm_to_time64(&t->time) - rtc_tm_to_time64(&tm_now);
> + if (value <= 0 || value >= U32_MAX)
> return -EINVAL;
> }
>
> @@ -745,8 +744,7 @@ static int acpi_tad_rtc_read_alarm(struc
>
> if (retval != ACPI_TAD_WAKE_DISABLED) {
> t->enabled = 1;
> - t->time = rtc_ktime_to_tm(ktime_add_ns(rtc_tm_to_ktime(tm_now),
> - (u64)retval * NSEC_PER_SEC));
> + rtc_time64_to_tm(rtc_tm_to_time64(&tm_now) + retval, &t->time);
> } else {
> t->enabled = 0;
> t->time = tm_now;
>
>
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* RE: [PATCH v7 1/2] dt-bindings: rtc: Add pcf85053 support
From: Lakshay Piplani @ 2026-04-24 2:37 UTC (permalink / raw)
To: alexandre.belloni@bootlin.com, linux-rtc@vger.kernel.org,
linux-kernel@vger.kernel.org, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, devicetree@vger.kernel.org
Cc: Vikash Bansal, Priyanka Jain, Pankit Garg, Conor Dooley
In-Reply-To: <20251127120456.1849177-1-lakshay.piplani@nxp.com>
> +
> + i2c1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + rtc@6f {
> + compatible = "nxp,pcf85053";
> + reg = <0x6f>;
> + nxp,interface = "secondary";
> + };
> + };
> --
> 2.25.1
Hi,
This is a gentle follow up on the patch series I submitted in November
(regarding the v7 of PCF85053 RTC driver). I haven't seen any review feedback
Yet. So, I wanted to check whether you might have had a chance to look at
the series.
Please let me know if there is anything further needed from my side.
Thanks,
Lakshay Piplani
^ permalink raw reply
* RE: [PATCH v4 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config
From: Lakshay Piplani @ 2026-04-24 2:42 UTC (permalink / raw)
To: alexandre.belloni@bootlin.com, linux-rtc@vger.kernel.org,
linux-kernel@vger.kernel.org, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, devicetree@vger.kernel.org,
wim@linux-watchdog.org, linux@roeck-us.net,
linux-watchdog@vger.kernel.org
Cc: Vikash Bansal, Priyanka Jain
In-Reply-To: <20251121121137.3043764-1-lakshay.piplani@nxp.com>
> +/* TSR3 modes */
> +#define PCF85363_TSR3_NONE 0x00
> +#define PCF85363_TSR3_FB 0x01
> +#define PCF85363_TSR3_LB 0x02
> +#define PCF85363_TSR3_LV 0x03
> +
> +#endif /* _DT_BINDINGS_RTC_PCF85363_TSR_H */
> --
> 2.25.1
Hi,
This is a gentle follow up on the patch series I submitted in November
(regarding the v4 of PCF85363 RTC driver). I haven't seen any review feedback
Yet. So, I wanted to check whether you might have had a chance to look at
the series.
Please let me know if there is anything further is needed from my side.
Thanks,
Lakshay Piplani
^ permalink raw reply
* Re: (subset) [PATCH v3 3/5] mfd: sprd-sc27xx: Switch to devm_mfd_add_devices()
From: Lee Jones @ 2026-04-24 7:53 UTC (permalink / raw)
To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Lee Jones, Pavel Machek,
Liam Girdwood, Mark Brown, Sebastian Reichel, Otto Pflüger
Cc: linux-rtc, devicetree, linux-kernel, linux-leds, linux-pm
In-Reply-To: <20260329-sc27xx-mfd-cells-v3-3-9158dee41f74@abscue.de>
On Sun, 29 Mar 2026 09:27:47 +0200, Otto Pflüger wrote:
> To allow instantiating subdevices such as the regulator and poweroff
> devices that do not have corresponding device tree nodes with a
> "compatible" property, use devm_mfd_add_devices() with MFD cells instead
> of devm_of_platform_populate(). Since different PMICs in the SC27xx
> series contain different components, use separate MFD cell tables for
> each PMIC model. Define cells for all components that have upstream
> drivers at this point.
>
> [...]
Applied, thanks!
[3/5] mfd: sprd-sc27xx: Switch to devm_mfd_add_devices()
commit: a7999115bcecdf2620b864080fbb4f6e87269102
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH 0/9] bitfield: add FIELD_GET_SIGNED()
From: Jonathan Cameron @ 2026-04-24 12:09 UTC (permalink / raw)
To: Yury Norov
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, David Lechner,
Nuno Sá, Andy Shevchenko, Ping-Ke Shih, Richard Cochran,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alexandre Belloni, Yury Norov, Rasmus Villemoes,
Hans de Goede, Linus Walleij, Sakari Ailus, Salah Triki,
Achim Gratz, Ben Collins, linux-kernel, linux-iio, linux-wireless,
netdev, linux-rtc
In-Reply-To: <20260417173621.368914-1-ynorov@nvidia.com>
On Fri, 17 Apr 2026 13:36:11 -0400
Yury Norov <ynorov@nvidia.com> wrote:
> The bitfields are designed in assumption that fields contain unsigned
> integer values, thus extracting the values from the field implies
> zero-extending.
>
> Some drivers need to sign-extend their fields, and currently do it like:
>
> dc_re += sign_extend32(FIELD_GET(0xfff000, tmp), 11);
> dc_im += sign_extend32(FIELD_GET(0xfff, tmp), 11);
>
> It's error-prone because it relies on user to provide the correct
> index of the most significant bit.
>
> This series adds a signed version of FIELD_GET(), which is the more
> convenient and compiles (on x86_64) to just a couple instructions:
> shl and sar.
>
> Patch #1 adds FIELD_GET_SIGNED(), and the rest of the series applies it
> tree-wide.
>
Just a quick heads up that I'm beginning to assume that this series
will land in some form. If it does can we do it as an immutable branch
as I'm suggesting it gets used in some other patches in that should land
in the new cycle.
Thanks,
Jonathan
> Yury Norov (9):
> bitfield: add FIELD_GET_SIGNED()
> x86/extable: switch to using FIELD_GET_SIGNED()
> iio: intel_dc_ti_adc: switch to using
> iio: magnetometer: yas530: switch to using FIELD_GET_SIGNED()
> iio: pressure: bmp280: switch to using
> iio: mcp9600: switch to using FIELD_GET_SIGNED()
> wifi: rtw89: switch to using FIELD_GET_SIGNED()
> rtc: rv3032: switch to using FIELD_GET_SIGNED()
> ptp: switch to using FIELD_GET_SIGNED()
>
> arch/x86/include/asm/extable_fixup_types.h | 13 ++++---------
> arch/x86/mm/extable.c | 2 +-
> drivers/iio/adc/intel_dc_ti_adc.c | 4 ++--
> drivers/iio/magnetometer/yamaha-yas530.c | 12 ++++++------
> drivers/iio/pressure/bmp280-core.c | 2 +-
> drivers/iio/temperature/mcp9600.c | 2 +-
> .../net/wireless/realtek/rtw89/rtw8852a_rfk.c | 4 ++--
> .../net/wireless/realtek/rtw89/rtw8852b_common.c | 4 ++--
> .../net/wireless/realtek/rtw89/rtw8852b_rfk.c | 4 ++--
> drivers/net/wireless/realtek/rtw89/rtw8852c.c | 4 ++--
> drivers/ptp/ptp_fc3.c | 4 ++--
> drivers/rtc/rtc-rv3032.c | 2 +-
> include/linux/bitfield.h | 16 ++++++++++++++++
> 13 files changed, 42 insertions(+), 31 deletions(-)
>
^ permalink raw reply
* Re: [PATCH 1/9] bitfield: add FIELD_GET_SIGNED()
From: David Laight @ 2026-04-24 14:50 UTC (permalink / raw)
To: Johannes Berg
Cc: Yury Norov, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Andy Lutomirski, Peter Zijlstra,
Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Ping-Ke Shih, Richard Cochran, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexandre Belloni,
Yury Norov, Rasmus Villemoes, Hans de Goede, Linus Walleij,
Sakari Ailus, Salah Triki, Achim Gratz, Ben Collins, linux-kernel,
linux-iio, linux-wireless, netdev, linux-rtc
In-Reply-To: <6170788fcab2ec835597e3d7411928d36850c20a.camel@sipsolutions.net>
On Mon, 20 Apr 2026 10:43:08 +0200
Johannes Berg <johannes@sipsolutions.net> wrote:
...
> I (personally) tend to prefer the "__MAKE_OP" versions (*_get_bits()
> etc.), in particular because WiFi and firmware interfaces deal a lot
> with fixed endian fields.
>
> Any chance it'd be simple to generate u32_get_bits_signed() etc.? Could
> be especially useful for le32_get_bits_signed() for example, to have the
> endian conversion built-in unlike FIELD_GET_SIGNED().
The problem is that the number of options explodes.
You need PREP/GET/GET_SIGNED functions x_64() x_64le() x_64be() x_32() x_32le()
x_32be() x_16le() and x_16be().
I make that 24 functions.
For UPDATE you also need x_16() and x_8() for another 10.
So at least 34 definitions - it is all getting silly.
(And I've excluded the pointless 8/16 bit functions where the result
is promoted to signed int.)
Then you have the problem that some (common) architectures don't have
byteswap instructions, but do have byteswapping memory accesses (maybe not
16bit). Which means that is it generally better to byteswap the value
before passing to FIELD_GET() and after building the full 'word' using
FIELD_PREP() (and just | for single bits).
(Except it is probably always better to byteswap constants.)
It is also worth nothing that FIELD_PREP() will reject requests to
fill signed fields with negative constants.
David
>
> johannes
>
^ permalink raw reply
* Re: [PATCH 0/9] bitfield: add FIELD_GET_SIGNED()
From: Yury Norov @ 2026-04-24 15:50 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, David Lechner,
Nuno Sá, Andy Shevchenko, Ping-Ke Shih, Richard Cochran,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alexandre Belloni, Yury Norov, Rasmus Villemoes,
Hans de Goede, Linus Walleij, Sakari Ailus, Salah Triki,
Achim Gratz, Ben Collins, linux-kernel, linux-iio, linux-wireless,
netdev, linux-rtc
In-Reply-To: <20260424130927.349ad3ae@jic23-huawei>
On Fri, Apr 24, 2026 at 01:09:27PM +0100, Jonathan Cameron wrote:
> On Fri, 17 Apr 2026 13:36:11 -0400
> Yury Norov <ynorov@nvidia.com> wrote:
>
> > The bitfields are designed in assumption that fields contain unsigned
> > integer values, thus extracting the values from the field implies
> > zero-extending.
> >
> > Some drivers need to sign-extend their fields, and currently do it like:
> >
> > dc_re += sign_extend32(FIELD_GET(0xfff000, tmp), 11);
> > dc_im += sign_extend32(FIELD_GET(0xfff, tmp), 11);
> >
> > It's error-prone because it relies on user to provide the correct
> > index of the most significant bit.
> >
> > This series adds a signed version of FIELD_GET(), which is the more
> > convenient and compiles (on x86_64) to just a couple instructions:
> > shl and sar.
> >
> > Patch #1 adds FIELD_GET_SIGNED(), and the rest of the series applies it
> > tree-wide.
> >
>
> Just a quick heads up that I'm beginning to assume that this series
> will land in some form. If it does can we do it as an immutable branch
> as I'm suggesting it gets used in some other patches in that should land
> in the new cycle.
I'm going to submit v2 soon, as seemingly the discussion is boiled
down, and then will likely merge it with my tree. I'll create an
immutable branch for you before the end of day.
Thanks,
Yury
^ permalink raw reply
* Re: [PATCH 1/9] bitfield: add FIELD_GET_SIGNED()
From: Yury Norov @ 2026-04-24 16:35 UTC (permalink / raw)
To: Johannes Berg
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Ping-Ke Shih,
Richard Cochran, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alexandre Belloni, Yury Norov,
Rasmus Villemoes, Hans de Goede, Linus Walleij, Sakari Ailus,
Salah Triki, Achim Gratz, Ben Collins, linux-kernel, linux-iio,
linux-wireless, netdev, linux-rtc
In-Reply-To: <6170788fcab2ec835597e3d7411928d36850c20a.camel@sipsolutions.net>
On Mon, Apr 20, 2026 at 10:43:08AM +0200, Johannes Berg wrote:
> On Fri, 2026-04-17 at 13:36 -0400, Yury Norov wrote:
> > The bitfields are designed in assumption that fields contain unsigned
> > integer values, thus extracting the values from the field implies
> > zero-extending.
> >
> > Some drivers need to sign-extend their fields, and currently do it like:
> >
> > dc_re += sign_extend32(FIELD_GET(0xfff000, tmp), 11);
> > dc_im += sign_extend32(FIELD_GET(0xfff, tmp), 11);
>
> That's indeed pretty awful...
>
>
> > +#define FIELD_GET_SIGNED(mask, reg) \
> >
>
> [...]
>
> I (personally) tend to prefer the "__MAKE_OP" versions (*_get_bits()
> etc.), in particular because WiFi and firmware interfaces deal a lot
> with fixed endian fields.
I don't like that __MAKE_OP magic because whatever it generates is not
greppable. And because we disable strict type checks for kernel, but
this API claims to typecheck the parameters for the user. So, the
following compiles well:
u64 val = 0;
ret = le16_get_bits(val, GENMASK(15, 10));
I don't like autogeneration in general. We generate, for example,
be32_get_bits(), but never use it. We don't even know the level of
the bloat.
> Any chance it'd be simple to generate u32_get_bits_signed() etc.? Could
> be especially useful for le32_get_bits_signed() for example, to have the
> endian conversion built-in unlike FIELD_GET_SIGNED().
Maybe this:
FIELD_GET_SIGNED(mask, le32_to_cpu(reg))
Thanks,
Yury
^ permalink raw reply
* Re: [PATCH 0/9] bitfield: add FIELD_GET_SIGNED()
From: Yury Norov @ 2026-04-24 17:10 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, David Lechner,
Nuno Sá, Andy Shevchenko, Ping-Ke Shih, Richard Cochran,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alexandre Belloni, Yury Norov, Rasmus Villemoes,
Hans de Goede, Linus Walleij, Sakari Ailus, Salah Triki,
Achim Gratz, Ben Collins, linux-kernel, linux-iio, linux-wireless,
netdev, linux-rtc
In-Reply-To: <aeuRMiws8zCdkGXX@yury>
On Fri, Apr 24, 2026 at 11:50:10AM -0400, Yury Norov wrote:
> On Fri, Apr 24, 2026 at 01:09:27PM +0100, Jonathan Cameron wrote:
> > On Fri, 17 Apr 2026 13:36:11 -0400
> > Yury Norov <ynorov@nvidia.com> wrote:
> >
> > > The bitfields are designed in assumption that fields contain unsigned
> > > integer values, thus extracting the values from the field implies
> > > zero-extending.
> > >
> > > Some drivers need to sign-extend their fields, and currently do it like:
> > >
> > > dc_re += sign_extend32(FIELD_GET(0xfff000, tmp), 11);
> > > dc_im += sign_extend32(FIELD_GET(0xfff, tmp), 11);
> > >
> > > It's error-prone because it relies on user to provide the correct
> > > index of the most significant bit.
> > >
> > > This series adds a signed version of FIELD_GET(), which is the more
> > > convenient and compiles (on x86_64) to just a couple instructions:
> > > shl and sar.
> > >
> > > Patch #1 adds FIELD_GET_SIGNED(), and the rest of the series applies it
> > > tree-wide.
> > >
> >
> > Just a quick heads up that I'm beginning to assume that this series
> > will land in some form. If it does can we do it as an immutable branch
> > as I'm suggesting it gets used in some other patches in that should land
> > in the new cycle.
>
> I'm going to submit v2 soon, as seemingly the discussion is boiled
> down, and then will likely merge it with my tree. I'll create an
> immutable branch for you before the end of day.
Here it is:
https://github.com/norov/linux/pull/new/fgsv2
It builds well for me, but I'll wait for a while for robots feedback
before making it 'officially' immutable.
Thanks,
Yury
^ permalink raw reply
* Re: [PATCH 1/9] bitfield: add FIELD_GET_SIGNED()
From: David Laight @ 2026-04-24 21:37 UTC (permalink / raw)
To: Yury Norov
Cc: Johannes Berg, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Andy Lutomirski, Peter Zijlstra,
Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Ping-Ke Shih, Richard Cochran, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexandre Belloni,
Yury Norov, Rasmus Villemoes, Hans de Goede, Linus Walleij,
Sakari Ailus, Salah Triki, Achim Gratz, Ben Collins, linux-kernel,
linux-iio, linux-wireless, netdev, linux-rtc
In-Reply-To: <aeub59FBHbCy-KKP@yury>
On Fri, 24 Apr 2026 12:35:51 -0400
Yury Norov <ynorov@nvidia.com> wrote:
...
> > Any chance it'd be simple to generate u32_get_bits_signed() etc.? Could
> > be especially useful for le32_get_bits_signed() for example, to have the
> > endian conversion built-in unlike FIELD_GET_SIGNED().
>
> Maybe this:
>
> x = FIELD_GET_SIGNED(mask, le32_to_cpu(reg))
But if you are going to follow it by:
x1 = FIELD_GET_SIGNED(mask1, le32_to_cpu(reg))
you really want to to the byteswap once, best as:
reg = le32_to_cpu(struct->member);
David
>
> Thanks,
> Yury
>
^ permalink raw reply
* [PATCH 1/2] rtc: isl1208: Fix returning errno as irqreturn_t in IRQ handler
From: John Madieu @ 2026-04-25 15:49 UTC (permalink / raw)
To: alexandre.belloni
Cc: ryan, akpm, m.grzeschik, Denis.Osterland, linux-rtc, linux-kernel,
biju.das.jz, john.madieu, John Madieu
In-Reply-To: <20260425154959.2796261-1-john.madieu.xa@bp.renesas.com>
isl1208_rtc_interrupt() is of irqreturn_t type but two paths
return a negative i2c errno instead of an IRQ_* value:
- The SR-poll loop on timeout: `return sr;`
- The post-alarm cleanup path: `return err;`
genirq's note_interrupt() casts the return to unsigned int and
flags any value above IRQ_HANDLED|IRQ_WAKE_THREAD as a bogus
return, logging "irq event N: bogus return value X" each time it
happens.
Return IRQ_NONE when the SR read failed (no progress, can't claim
the interrupt) and IRQ_HANDLED when toggle_alarm failed.
Fixes: cf044f0ed526 ("drivers/rtc/rtc-isl1208.c: add alarm support")
Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---
drivers/rtc/rtc-isl1208.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index f71a6bb77b2a..c93998c53e7a 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -654,7 +654,7 @@ isl1208_rtc_interrupt(int irq, void *data)
if (time_after(jiffies, timeout)) {
dev_err(&client->dev, "%s: reading SR failed\n",
__func__);
- return sr;
+ return IRQ_NONE;
}
}
@@ -666,7 +666,7 @@ isl1208_rtc_interrupt(int irq, void *data)
/* Disable the alarm */
err = isl1208_rtc_toggle_alarm(client, 0);
if (err)
- return err;
+ return IRQ_HANDLED;
fsleep(275);
--
2.25.1
^ permalink raw reply related
* [PATCH 0/2] rtc: isl1208: Fix IRQ return value and wake reference leak
From: John Madieu @ 2026-04-25 15:49 UTC (permalink / raw)
To: alexandre.belloni
Cc: ryan, akpm, m.grzeschik, Denis.Osterland, linux-rtc, linux-kernel,
biju.das.jz, john.madieu, John Madieu
Hi all,
This series fixes two issues in rtc-isl1208. The first is
isl1208_rtc_interrupt() returning negative i2c errnos where
irqreturn_t is expected, which genirq flags as bogus and logs
on each IRQ. The second is a wake-reference leak: setup_irq()
calls enable_irq_wake() on success but the driver has no remove
path that balances it, so each rebind cycle leaks one wake
reference per IRQ.
John Madieu (2):
rtc: isl1208: Fix returning errno as irqreturn_t in IRQ handler
rtc: isl1208: Balance enable_irq_wake() with disable_irq_wake() on
cleanup
drivers/rtc/rtc-isl1208.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
--
2.25.1
^ permalink raw reply
* [PATCH 2/2] rtc: isl1208: Balance enable_irq_wake() with disable_irq_wake() on cleanup
From: John Madieu @ 2026-04-25 15:49 UTC (permalink / raw)
To: alexandre.belloni
Cc: ryan, akpm, m.grzeschik, Denis.Osterland, linux-rtc, linux-kernel,
biju.das.jz, john.madieu, John Madieu
In-Reply-To: <20260425154959.2796261-1-john.madieu.xa@bp.renesas.com>
isl1208_setup_irq() calls enable_irq_wake() after a successful
IRQ request, but the driver has no remove path that balances it.
The driver is devm-only, so on unbind devm releases the IRQ -
but enable_irq_wake() is not undone by IRQ release, so the wake
count for that IRQ stays incremented.
Each rebind therefore leaks one wake reference; the leak doubles
for the chip variant that has a separate evdet IRQ, since
isl1208_setup_irq() is then called twice during probe.
Register a devm action that calls disable_irq_wake() per IRQ.
While at it, check enable_irq_wake()'s return value:
on failure, propagate the error rather than silently registering
a disable action for an IRQ whose wake state was never enabled.
Fixes: 9ece7cd833a3 ("rtc: isl1208: Add "evdet" interrupt source for isl1219")
Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---
drivers/rtc/rtc-isl1208.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index c93998c53e7a..1de80fc9c9c9 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -822,6 +822,11 @@ static const struct nvmem_config isl1208_nvmem_config = {
.reg_write = isl1208_nvmem_write,
};
+static void isl1208_disable_irq_wake_action(void *data)
+{
+ disable_irq_wake((unsigned long)data);
+}
+
static int isl1208_setup_irq(struct i2c_client *client, int irq)
{
int rc = devm_request_threaded_irq(&client->dev, irq, NULL,
@@ -831,7 +836,15 @@ static int isl1208_setup_irq(struct i2c_client *client, int irq)
client);
if (!rc) {
device_init_wakeup(&client->dev, true);
- enable_irq_wake(irq);
+ rc = enable_irq_wake(irq);
+ if (rc)
+ return rc;
+
+ rc = devm_add_action_or_reset(&client->dev,
+ isl1208_disable_irq_wake_action,
+ (void *)(unsigned long)irq);
+ if (rc)
+ return rc;
} else {
dev_err(&client->dev,
"Unable to request irq %d, no alarm support\n",
--
2.25.1
^ permalink raw reply related
* RE: [PATCH 2/2] rtc: isl1208: Balance enable_irq_wake() with disable_irq_wake() on cleanup
From: Biju Das @ 2026-04-25 16:39 UTC (permalink / raw)
To: John Madieu, alexandre.belloni@bootlin.com
Cc: ryan@bluewatersys.com, akpm@linux-foundation.org,
m.grzeschik@pengutronix.de, Denis.Osterland@diehl.com,
linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org,
john.madieu@gmail.com
In-Reply-To: <20260425154959.2796261-3-john.madieu.xa@bp.renesas.com>
Hi John,
> -----Original Message-----
> From: John Madieu <john.madieu.xa@bp.renesas.com>
> Sent: 25 April 2026 16:50
> Subject: [PATCH 2/2] rtc: isl1208: Balance enable_irq_wake() with disable_irq_wake() on cleanup
>
> isl1208_setup_irq() calls enable_irq_wake() after a successful IRQ request, but the driver has no
> remove path that balances it.
> The driver is devm-only, so on unbind devm releases the IRQ - but enable_irq_wake() is not undone by
> IRQ release, so the wake count for that IRQ stays incremented.
>
> Each rebind therefore leaks one wake reference; the leak doubles for the chip variant that has a
> separate evdet IRQ, since
> isl1208_setup_irq() is then called twice during probe.
Is removal of RTC device possible [1]?
[1]
https://patchwork.ozlabs.org/project/rtc-linux/patch/20230922081208.26334-1-biju.das.jz@bp.renesas.com/#3195765
Cheers,
Biju
^ permalink raw reply
* RE: [PATCH 1/2] rtc: isl1208: Fix returning errno as irqreturn_t in IRQ handler
From: Biju Das @ 2026-04-25 17:16 UTC (permalink / raw)
To: John Madieu, alexandre.belloni@bootlin.com
Cc: ryan@bluewatersys.com, akpm@linux-foundation.org,
m.grzeschik@pengutronix.de, Denis.Osterland@diehl.com,
linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org,
john.madieu@gmail.com
In-Reply-To: <20260425154959.2796261-2-john.madieu.xa@bp.renesas.com>
Hi John,
Thanks for the patch.
> -----Original Message-----
> From: John Madieu <john.madieu.xa@bp.renesas.com>
> Sent: 25 April 2026 16:50
> Subject: [PATCH 1/2] rtc: isl1208: Fix returning errno as irqreturn_t in IRQ handler
>
> isl1208_rtc_interrupt() is of irqreturn_t type but two paths return a negative i2c errno instead of an
> IRQ_* value:
>
> - The SR-poll loop on timeout: `return sr;`
> - The post-alarm cleanup path: `return err;`
>
> genirq's note_interrupt() casts the return to unsigned int and flags any value above
> IRQ_HANDLED|IRQ_WAKE_THREAD as a bogus return, logging "irq event N: bogus return value X" each time it
> happens.
>
> Return IRQ_NONE when the SR read failed (no progress, can't claim the interrupt) and IRQ_HANDLED when
> toggle_alarm failed.
>
> Fixes: cf044f0ed526 ("drivers/rtc/rtc-isl1208.c: add alarm support")
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> ---
> drivers/rtc/rtc-isl1208.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c index f71a6bb77b2a..c93998c53e7a
> 100644
> --- a/drivers/rtc/rtc-isl1208.c
> +++ b/drivers/rtc/rtc-isl1208.c
> @@ -654,7 +654,7 @@ isl1208_rtc_interrupt(int irq, void *data)
> if (time_after(jiffies, timeout)) {
> dev_err(&client->dev, "%s: reading SR failed\n",
> __func__);
> - return sr;
> + return IRQ_NONE;
Maybe you can use a goto statement?? that will take care of handled IRQ's
goto err_irq:
err_irq:
return handled ? IRQ_HANDLED : IRQ_NONE;
> }
> }
>
> @@ -666,7 +666,7 @@ isl1208_rtc_interrupt(int irq, void *data)
> /* Disable the alarm */
> err = isl1208_rtc_toggle_alarm(client, 0);
> if (err)
> - return err;
> + return IRQ_HANDLED;
Same as above.
Cheers,
Biju
>
> fsleep(275);
>
> --
> 2.25.1
^ permalink raw reply
* [GIT PULL] RTC for 7.1
From: Alexandre Belloni @ 2026-04-25 18:33 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-rtc, linux-kernel
Hello Linus,
Here is the RTC subsystem pull request for 7.1. It is super late, I
hoped I could squeeze a few more patches in but I didn't have the time.
It is quite small and half of the changes are in device tree bindings.
The following changes since commit 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f:
Linux 7.0-rc1 (2026-02-22 13:18:59 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git tags/rtc-7.1
for you to fetch changes up to 0fedce7244e4b85c049ce579c87e298a1b0b811d:
rtc: abx80x: Disable alarm feature if no interrupt attached (2026-04-13 00:02:59 +0200)
----------------------------------------------------------------
RTC for 7.1
Subsystem:
- add data_race() in rtc_dev_poll()
Drivers:
- remove i2c_match_id usage
- abx80x: Disable alarm feature if no interrupt attached
- ti-k3: support resuming from IO DDR low power mode
----------------------------------------------------------------
Akashdeep Kaur (1):
rtc: ti-k3: Add support to resume from IO DDR low power mode
Andrew Davis (6):
rtc: abx80x: Remove use of i2c_match_id()
rtc: m41t80: Remove use of i2c_match_id()
rtc: pcf2127: Remove use of i2c_match_id()
rtc: rs5c372: Remove use of i2c_match_id()
rtc: rv8803: Remove use of i2c_match_id()
rtc: rx8025: Remove use of i2c_match_id()
Anthony Pighin (Nokia) (1):
rtc: abx80x: Disable alarm feature if no interrupt attached
Anushka Badhe (1):
dt-bindings: rtc: add olpc,xo1-rtc to trivial-rtc
Brian Masney (1):
rtc: pic32: allow driver to be compiled with COMPILE_TEST
Conor Dooley (1):
dt-bindings: rtc: mpfs-rtc: permit resets
Frieder Schrempf (1):
dt-bindings: rtc: microcrystal,rv3028: Allow to specify vdd-supply
Johan Hovold (1):
rtc: ntxec: fix OF node reference imbalance
Mauricio Faria de Oliveira (1):
rtc: add data_race() in rtc_dev_poll()
Otto Pflüger (1):
dt-bindings: rtc: sc2731: Add compatible for SC2730
Piyush Patle (1):
dt-bindings: rtc: isl12026: convert to YAML schema
Rafael J. Wysocki (1):
rtc: cmos: Use platform_get_irq_optional() in cmos_platform_probe()
Rosen Penev (1):
rtc: armada38x: zalloc + calloc to single allocation
Svyatoslav Ryhel (1):
rtc: max77686: convert to i2c_new_ancillary_device
.../devicetree/bindings/rtc/isil,isl12026.txt | 28 ----------
.../devicetree/bindings/rtc/isil,isl12026.yaml | 59 ++++++++++++++++++++++
.../bindings/rtc/microchip,mpfs-rtc.yaml | 3 ++
.../bindings/rtc/microcrystal,rv3028.yaml | 2 +
.../devicetree/bindings/rtc/olpc-xo1-rtc.txt | 5 --
.../devicetree/bindings/rtc/sprd,sc2731-rtc.yaml | 7 ++-
.../devicetree/bindings/rtc/trivial-rtc.yaml | 2 +
drivers/rtc/Kconfig | 2 +-
drivers/rtc/dev.c | 11 +++-
drivers/rtc/rtc-abx80x.c | 5 +-
drivers/rtc/rtc-armada38x.c | 9 +---
drivers/rtc/rtc-cmos.c | 13 ++++-
drivers/rtc/rtc-m41t80.c | 8 +--
drivers/rtc/rtc-max77686.c | 14 ++++-
drivers/rtc/rtc-ntxec.c | 2 +-
drivers/rtc/rtc-pcf2127.c | 23 +++------
drivers/rtc/rtc-rs5c372.c | 7 +--
drivers/rtc/rtc-rv8803.c | 8 +--
drivers/rtc/rtc-rx8025.c | 4 +-
drivers/rtc/rtc-ti-k3.c | 10 +++-
20 files changed, 132 insertions(+), 90 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/rtc/isil,isl12026.txt
create mode 100644 Documentation/devicetree/bindings/rtc/isil,isl12026.yaml
delete mode 100644 Documentation/devicetree/bindings/rtc/olpc-xo1-rtc.txt
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 2/2] rtc: isl1208: Balance enable_irq_wake() with disable_irq_wake() on cleanup
From: Alexandre Belloni @ 2026-04-25 18:36 UTC (permalink / raw)
To: Biju Das
Cc: John Madieu, ryan@bluewatersys.com, akpm@linux-foundation.org,
m.grzeschik@pengutronix.de, Denis.Osterland@diehl.com,
linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org,
john.madieu@gmail.com
In-Reply-To: <TY3PR01MB1134607A936EAE3F7F2185D3086282@TY3PR01MB11346.jpnprd01.prod.outlook.com>
On 25/04/2026 16:39:16+0000, Biju Das wrote:
> Hi John,
>
> > -----Original Message-----
> > From: John Madieu <john.madieu.xa@bp.renesas.com>
> > Sent: 25 April 2026 16:50
> > Subject: [PATCH 2/2] rtc: isl1208: Balance enable_irq_wake() with disable_irq_wake() on cleanup
> >
> > isl1208_setup_irq() calls enable_irq_wake() after a successful IRQ request, but the driver has no
> > remove path that balances it.
> > The driver is devm-only, so on unbind devm releases the IRQ - but enable_irq_wake() is not undone by
> > IRQ release, so the wake count for that IRQ stays incremented.
> >
> > Each rebind therefore leaks one wake reference; the leak doubles for the chip variant that has a
> > separate evdet IRQ, since
> > isl1208_setup_irq() is then called twice during probe.
>
> Is removal of RTC device possible [1]?
>
> [1]
> https://patchwork.ozlabs.org/project/rtc-linux/patch/20230922081208.26334-1-biju.das.jz@bp.renesas.com/#3195765
>
I'd say yes if this is not the RTC that is backing alarmtimer or
alarmtimer is not compiled in the kernel.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* RE: [PATCH 1/2] rtc: isl1208: Fix returning errno as irqreturn_t in IRQ handler
From: John Madieu @ 2026-04-26 17:48 UTC (permalink / raw)
To: Biju Das, alexandre.belloni@bootlin.com
Cc: ryan@bluewatersys.com, akpm@linux-foundation.org,
m.grzeschik@pengutronix.de, Denis.Osterland@diehl.com,
linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org,
john.madieu@gmail.com
In-Reply-To: <TY3PR01MB1134639F6A3A38551180B626386282@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Hi Biju,
Thanks fort he review.
> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Samstag, 25. April 2026 19:16
> To: John Madieu <john.madieu.xa@bp.renesas.com>;
> alexandre.belloni@bootlin.com
> Subject: RE: [PATCH 1/2] rtc: isl1208: Fix returning errno as irqreturn_t
> in IRQ handler
>
> Hi John,
>
> Thanks for the patch.
>
> > -----Original Message-----
> > From: John Madieu <john.madieu.xa@bp.renesas.com>
> > Sent: 25 April 2026 16:50
> > Subject: [PATCH 1/2] rtc: isl1208: Fix returning errno as irqreturn_t
> > in IRQ handler
> >
> > isl1208_rtc_interrupt() is of irqreturn_t type but two paths return a
> > negative i2c errno instead of an
> > IRQ_* value:
> >
> > - The SR-poll loop on timeout: `return sr;`
> > - The post-alarm cleanup path: `return err;`
> >
> > genirq's note_interrupt() casts the return to unsigned int and flags
> > any value above IRQ_HANDLED|IRQ_WAKE_THREAD as a bogus return, logging
> > "irq event N: bogus return value X" each time it happens.
> >
> > Return IRQ_NONE when the SR read failed (no progress, can't claim the
> > interrupt) and IRQ_HANDLED when toggle_alarm failed.
> >
> > Fixes: cf044f0ed526 ("drivers/rtc/rtc-isl1208.c: add alarm support")
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > ---
> > drivers/rtc/rtc-isl1208.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
> > index f71a6bb77b2a..c93998c53e7a
> > 100644
> > --- a/drivers/rtc/rtc-isl1208.c
> > +++ b/drivers/rtc/rtc-isl1208.c
> > @@ -654,7 +654,7 @@ isl1208_rtc_interrupt(int irq, void *data)
> > if (time_after(jiffies, timeout)) {
> > dev_err(&client->dev, "%s: reading SR failed\n",
> > __func__);
> > - return sr;
> > + return IRQ_NONE;
>
> Maybe you can use a goto statement?? that will take care of handled IRQ's
>
> goto err_irq:
>
> err_irq:
> return handled ? IRQ_HANDLED : IRQ_NONE;
Agreed. I'll do it your way in v2.
>
> > }
> > }
> >
> > @@ -666,7 +666,7 @@ isl1208_rtc_interrupt(int irq, void *data)
> > /* Disable the alarm */
> > err = isl1208_rtc_toggle_alarm(client, 0);
> > if (err)
> > - return err;
> > + return IRQ_HANDLED;
>
> Same as above.
>
I'll set handled = 1 so that goto can return IRQ_HANDLED.
Regards,
John
> Cheers,
> Biju
>
> >
> > fsleep(275);
> >
> > --
> > 2.25.1
^ permalink raw reply
* RE: [PATCH 2/2] rtc: isl1208: Balance enable_irq_wake() with disable_irq_wake() on cleanup
From: John Madieu @ 2026-04-26 18:23 UTC (permalink / raw)
To: Alexandre Belloni, Biju Das
Cc: ryan@bluewatersys.com, akpm@linux-foundation.org,
m.grzeschik@pengutronix.de, Denis.Osterland@diehl.com,
linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org,
john.madieu@gmail.com
In-Reply-To: <202604251836574d655eb1@mail.local>
Hi Biju, Alexandre,
Thanks for the feedback.
> -----Original Message-----
> From: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Sent: Samstag, 25. April 2026 20:37
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Subject: Re: [PATCH 2/2] rtc: isl1208: Balance enable_irq_wake() with
> disable_irq_wake() on cleanup
>
> On 25/04/2026 16:39:16+0000, Biju Das wrote:
> > Hi John,
> >
> > > -----Original Message-----
> > > From: John Madieu <john.madieu.xa@bp.renesas.com>
> > > Sent: 25 April 2026 16:50
> > > Subject: [PATCH 2/2] rtc: isl1208: Balance enable_irq_wake() with
> > > disable_irq_wake() on cleanup
> > >
> > > isl1208_setup_irq() calls enable_irq_wake() after a successful IRQ
> > > request, but the driver has no remove path that balances it.
> > > The driver is devm-only, so on unbind devm releases the IRQ - but
> > > enable_irq_wake() is not undone by IRQ release, so the wake count for
> that IRQ stays incremented.
> > >
> > > Each rebind therefore leaks one wake reference; the leak doubles for
> > > the chip variant that has a separate evdet IRQ, since
> > > isl1208_setup_irq() is then called twice during probe.
> >
> > Is removal of RTC device possible [1]?
> >
This patch addresses the per-IRQ wake refcount leak, which I
think is independent of whether alarmtimer is holding the RTC.
Found this by code inspection while working on patch [1/2]'s
issue.
> > [1]
> > https://patchwork.ozlabs.org/project/rtc-linux/patch/20230922081208.26
> > 334-1-biju.das.jz@bp.renesas.com/#3195765
> >
>
> I'd say yes if this is not the RTC that is backing alarmtimer or
> alarmtimer is not compiled in the kernel.
>
Thanks for the clarification. That said, looking around I noticed
most RTC drivers don't bother balancing enable_irq_wake() either.
Alexandre, do you want this patch as-is, or would you rather I
drop it? I'm fine either way.
Regards,
John
^ permalink raw reply
* Re: [GIT PULL] RTC for 7.1
From: pr-tracker-bot @ 2026-04-26 19:35 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: Linus Torvalds, linux-rtc, linux-kernel
In-Reply-To: <20260425183335392f3a5c@mail.local>
The pull request you sent on Sat, 25 Apr 2026 20:33:35 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git tags/rtc-7.1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/211d5933141197b37a7501271e49e4b88540615f
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* RE: [PATCH 1/2] rtc: isl1208: Fix returning errno as irqreturn_t in IRQ handler
From: Biju Das @ 2026-04-27 5:51 UTC (permalink / raw)
To: John Madieu, alexandre.belloni@bootlin.com
Cc: ryan@bluewatersys.com, akpm@linux-foundation.org,
m.grzeschik@pengutronix.de, Denis.Osterland@diehl.com,
linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org,
john.madieu@gmail.com, open list:PIN CONTROLLER - RENESAS
In-Reply-To: <TY6PR01MB173776D008E8D0D5DAD622A0BFF292@TY6PR01MB17377.jpnprd01.prod.outlook.com>
Hi John,
> -----Original Message-----
> From: John Madieu <john.madieu.xa@bp.renesas.com>
> Sent: 26 April 2026 18:48
> To: Biju Das <biju.das.jz@bp.renesas.com>; alexandre.belloni@bootlin.com
> Cc: ryan@bluewatersys.com; akpm@linux-foundation.org; m.grzeschik@pengutronix.de;
> Denis.Osterland@diehl.com; linux-rtc@vger.kernel.org; linux-kernel@vger.kernel.org;
> john.madieu@gmail.com
> Subject: RE: [PATCH 1/2] rtc: isl1208: Fix returning errno as irqreturn_t in IRQ handler
>
> Hi Biju,
>
> Thanks fort he review.
>
> > -----Original Message-----
> > From: Biju Das <biju.das.jz@bp.renesas.com>
> > Sent: Samstag, 25. April 2026 19:16
> > To: John Madieu <john.madieu.xa@bp.renesas.com>;
> > alexandre.belloni@bootlin.com
> > Subject: RE: [PATCH 1/2] rtc: isl1208: Fix returning errno as
> > irqreturn_t in IRQ handler
> >
> > Hi John,
> >
> > Thanks for the patch.
> >
> > > -----Original Message-----
> > > From: John Madieu <john.madieu.xa@bp.renesas.com>
> > > Sent: 25 April 2026 16:50
> > > Subject: [PATCH 1/2] rtc: isl1208: Fix returning errno as
> > > irqreturn_t in IRQ handler
> > >
> > > isl1208_rtc_interrupt() is of irqreturn_t type but two paths return
> > > a negative i2c errno instead of an
> > > IRQ_* value:
> > >
> > > - The SR-poll loop on timeout: `return sr;`
> > > - The post-alarm cleanup path: `return err;`
> > >
> > > genirq's note_interrupt() casts the return to unsigned int and flags
> > > any value above IRQ_HANDLED|IRQ_WAKE_THREAD as a bogus return,
> > > logging "irq event N: bogus return value X" each time it happens.
> > >
> > > Return IRQ_NONE when the SR read failed (no progress, can't claim
> > > the
> > > interrupt) and IRQ_HANDLED when toggle_alarm failed.
> > >
> > > Fixes: cf044f0ed526 ("drivers/rtc/rtc-isl1208.c: add alarm support")
> > > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > > ---
> > > drivers/rtc/rtc-isl1208.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
> > > index f71a6bb77b2a..c93998c53e7a
> > > 100644
> > > --- a/drivers/rtc/rtc-isl1208.c
> > > +++ b/drivers/rtc/rtc-isl1208.c
> > > @@ -654,7 +654,7 @@ isl1208_rtc_interrupt(int irq, void *data)
> > > if (time_after(jiffies, timeout)) {
> > > dev_err(&client->dev, "%s: reading SR failed\n",
> > > __func__);
> > > - return sr;
> > > + return IRQ_NONE;
> >
> > Maybe you can use a goto statement?? that will take care of handled
> > IRQ's
> >
> > goto err_irq:
> >
> > err_irq:
> > return handled ? IRQ_HANDLED : IRQ_NONE;
>
> Agreed. I'll do it your way in v2.
>
> >
> > > }
> > > }
> > >
> > > @@ -666,7 +666,7 @@ isl1208_rtc_interrupt(int irq, void *data)
> > > /* Disable the alarm */
> > > err = isl1208_rtc_toggle_alarm(client, 0);
> > > if (err)
> > > - return err;
> > > + return IRQ_HANDLED;
> >
> > Same as above.
> >
>
> I'll set handled = 1 so that goto can return IRQ_HANDLED.
Looks this is change in behaviour compared to original code,
previous code does not set, handled = 1 for this path.
Cheers,
Biju
^ permalink raw reply
* RE: [PATCH 2/2] rtc: isl1208: Balance enable_irq_wake() with disable_irq_wake() on cleanup
From: Biju Das @ 2026-04-27 6:07 UTC (permalink / raw)
To: Alexandre Belloni
Cc: John Madieu, ryan@bluewatersys.com, akpm@linux-foundation.org,
m.grzeschik@pengutronix.de, Denis.Osterland@diehl.com,
linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org,
john.madieu@gmail.com, open list:PIN CONTROLLER - RENESAS
In-Reply-To: <202604251836574d655eb1@mail.local>
Hi Alexandre Belloni,
Thanks for the feedback.
> -----Original Message-----
> From: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Sent: 25 April 2026 19:37
> Subject: Re: [PATCH 2/2] rtc: isl1208: Balance enable_irq_wake() with disable_irq_wake() on cleanup
>
> On 25/04/2026 16:39:16+0000, Biju Das wrote:
> > Hi John,
> >
> > > -----Original Message-----
> > > From: John Madieu <john.madieu.xa@bp.renesas.com>
> > > Sent: 25 April 2026 16:50
> > > Subject: [PATCH 2/2] rtc: isl1208: Balance enable_irq_wake() with
> > > disable_irq_wake() on cleanup
> > >
> > > isl1208_setup_irq() calls enable_irq_wake() after a successful IRQ
> > > request, but the driver has no remove path that balances it.
> > > The driver is devm-only, so on unbind devm releases the IRQ - but
> > > enable_irq_wake() is not undone by IRQ release, so the wake count for that IRQ stays incremented.
> > >
> > > Each rebind therefore leaks one wake reference; the leak doubles for
> > > the chip variant that has a separate evdet IRQ, since
> > > isl1208_setup_irq() is then called twice during probe.
> >
> > Is removal of RTC device possible [1]?
> >
> > [1]
> > https://patchwork.ozlabs.org/project/rtc-linux/patch/20230922081208.26
> > 334-1-biju.das.jz@bp.renesas.com/#3195765
> >
>
> I'd say yes if this is not the RTC that is backing alarmtimer or alarmtimer is not compiled in the
> kernel.
obj-y += timeconv.o timecounter.o alarmtimer.o
Alarm timer is always compiled.
Now, On RZ/G3E SMARC EVK, only single RTC that have backing
alarmtimer support. On this platform, it cannot wake up the device
from deepsleep on RTC ALARM. But it can do timekeeping.
Cheers,
Biju
^ permalink raw reply
* Re: [PATCH 1/9] bitfield: add FIELD_GET_SIGNED()
From: Johannes Berg @ 2026-04-27 8:29 UTC (permalink / raw)
To: Yury Norov
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Ping-Ke Shih,
Richard Cochran, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alexandre Belloni, Yury Norov,
Rasmus Villemoes, Hans de Goede, Linus Walleij, Sakari Ailus,
Salah Triki, Achim Gratz, Ben Collins, linux-kernel, linux-iio,
linux-wireless, netdev, linux-rtc
In-Reply-To: <aeub59FBHbCy-KKP@yury>
On Fri, 2026-04-24 at 12:35 -0400, Yury Norov wrote:
> > I (personally) tend to prefer the "__MAKE_OP" versions (*_get_bits()
> > etc.), in particular because WiFi and firmware interfaces deal a lot
> > with fixed endian fields.
>
> I don't like that __MAKE_OP magic because whatever it generates is not
> greppable. And because we disable strict type checks for kernel, but
> this API claims to typecheck the parameters for the user. So, the
> following compiles well:
>
> u64 val = 0;
> ret = le16_get_bits(val, GENMASK(15, 10));
>
> I don't like autogeneration in general. We generate, for example,
> be32_get_bits(), but never use it.
That's a lot of "I don't like", but whatever.
> We don't even know the level of the bloat.
These are static inlines so there's no binary cost, and given that
you're complaining about them being generated you can't really *also*
complain about too much code...
> > Any chance it'd be simple to generate u32_get_bits_signed() etc.? Could
> > be especially useful for le32_get_bits_signed() for example, to have the
> > endian conversion built-in unlike FIELD_GET_SIGNED().
>
> Maybe this:
>
> FIELD_GET_SIGNED(mask, le32_to_cpu(reg))
Awful. "I don't like". But we rarely deal with bit-packed signed values
anyway.
johannes
^ 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