* [PATCH v7 01/10] dt-bindings: leds: document Samsung S2M series PMIC flash LED device
2026-05-15 21:38 [PATCH v7 00/10] Support for Samsung S2MU005 PMIC and its sub-devices Kaustabh Chakraborty
@ 2026-05-15 21:38 ` Kaustabh Chakraborty
2026-05-15 21:38 ` [PATCH v7 02/10] dt-bindings: extcon: document Samsung S2M series PMIC extcon device Kaustabh Chakraborty
` (8 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Kaustabh Chakraborty @ 2026-05-15 21:38 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, Yassine Oudjana
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc, Kaustabh Chakraborty, Conor Dooley
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.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
.../bindings/leds/samsung,s2mu005-flash.yaml | 52 ++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/Documentation/devicetree/bindings/leds/samsung,s2mu005-flash.yaml b/Documentation/devicetree/bindings/leds/samsung,s2mu005-flash.yaml
new file mode 100644
index 0000000000000..36051ab20509f
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/samsung,s2mu005-flash.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/samsung,s2mu005-flash.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Flash and Torch LED Controller for Samsung S2M series PMICs
+
+maintainers:
+ - Kaustabh Chakraborty <kauschluss@disroot.org>
+
+description: |
+ The Samsung S2M series PMIC flash LED has two led channels (typically
+ as back and front camera flashes), with support for both torch and
+ flash modes.
+
+ 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-flash
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+patternProperties:
+ "^led@[0-1]$":
+ type: object
+ $ref: common.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ enum: [0, 1]
+
+ required:
+ - reg
+
+required:
+ - compatible
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: false
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v7 02/10] dt-bindings: extcon: document Samsung S2M series PMIC extcon device
2026-05-15 21:38 [PATCH v7 00/10] Support for Samsung S2MU005 PMIC and its sub-devices Kaustabh Chakraborty
2026-05-15 21:38 ` [PATCH v7 01/10] dt-bindings: leds: document Samsung S2M series PMIC flash LED device Kaustabh Chakraborty
@ 2026-05-15 21:38 ` Kaustabh Chakraborty
2026-05-15 21:38 ` [PATCH v7 03/10] dt-bindings: mfd: add documentation for S2MU005 PMIC Kaustabh Chakraborty
` (7 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Kaustabh Chakraborty @ 2026-05-15 21:38 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, Yassine Oudjana
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc, Kaustabh Chakraborty, Conor Dooley,
Krzysztof Kozlowski
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.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
.../bindings/extcon/samsung,s2mu005-muic.yaml | 40 ++++++++++++++++++++++
1 file changed, 40 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 0000000000000..0a320d5e2a352
--- /dev/null
+++ b/Documentation/devicetree/bindings/extcon/samsung,s2mu005-muic.yaml
@@ -0,0 +1,40 @@
+# 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: MUIC Device for Samsung S2M series PMICs
+
+maintainers:
+ - Kaustabh Chakraborty <kauschluss@disroot.org>
+
+description: |
+ The Samsung S2M series PMIC MUIC 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
+
+ connector:
+ $ref: /schemas/connector/usb-connector.yaml#
+
+ port:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: Port connecting to the USB controller or PHY.
+
+required:
+ - compatible
+ - connector
+ - port
+
+additionalProperties: false
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v7 03/10] dt-bindings: mfd: add documentation for S2MU005 PMIC
2026-05-15 21:38 [PATCH v7 00/10] Support for Samsung S2MU005 PMIC and its sub-devices Kaustabh Chakraborty
2026-05-15 21:38 ` [PATCH v7 01/10] dt-bindings: leds: document Samsung S2M series PMIC flash LED device Kaustabh Chakraborty
2026-05-15 21:38 ` [PATCH v7 02/10] dt-bindings: extcon: document Samsung S2M series PMIC extcon device Kaustabh Chakraborty
@ 2026-05-15 21:38 ` Kaustabh Chakraborty
2026-05-15 21:38 ` [PATCH v7 04/10] mfd: sec: add support " Kaustabh Chakraborty
` (6 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Kaustabh Chakraborty @ 2026-05-15 21:38 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, Yassine Oudjana
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc, Kaustabh Chakraborty, Krzysztof Kozlowski
Samsung's S2MU005 PMIC includes subdevices for a charger, an MUIC (Micro
USB Interface Controller), and flash and RGB LED controllers.
Add the compatible and documentation for the S2MU005 PMIC. Also, add an
example for nodes for supported sub-devices, i.e. MUIC, flash LEDs, and
RGB LEDs. Charger sub-device uses the node of the parent.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
.../bindings/mfd/samsung,s2mu005-pmic.yaml | 120 +++++++++++++++++++++
1 file changed, 120 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/samsung,s2mu005-pmic.yaml b/Documentation/devicetree/bindings/mfd/samsung,s2mu005-pmic.yaml
new file mode 100644
index 0000000000000..aff68c035b38e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/samsung,s2mu005-pmic.yaml
@@ -0,0 +1,120 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/samsung,s2mu005-pmic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung S2MU005 Power Management IC
+
+maintainers:
+ - Kaustabh Chakraborty <kauschluss@disroot.org>
+
+description: |
+ The S2MU005 is a companion power management IC which includes subdevices for
+ a charger controller, an MUIC (Micro USB Interface Controller), and flash and
+ RGB LED controllers.
+
+allOf:
+ - $ref: /schemas/power/supply/power-supply.yaml#
+
+properties:
+ compatible:
+ const: samsung,s2mu005-pmic
+
+ flash:
+ $ref: /schemas/leds/samsung,s2mu005-flash.yaml#
+ description:
+ Child node describing flash LEDs.
+
+ interrupts:
+ maxItems: 1
+
+ muic:
+ $ref: /schemas/extcon/samsung,s2mu005-muic.yaml#
+ description:
+ Child node describing MUIC device.
+
+ multi-led:
+ type: object
+
+ allOf:
+ - $ref: /schemas/leds/leds-class-multicolor.yaml#
+
+ properties:
+ compatible:
+ const: samsung,s2mu005-rgb
+
+ required:
+ - compatible
+
+ unevaluatedProperties: false
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/leds/common.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pmic@3d {
+ compatible = "samsung,s2mu005-pmic";
+ reg = <0x3d>;
+ interrupt-parent = <&gpa2>;
+ interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+
+ monitored-battery = <&battery>;
+
+ flash {
+ compatible = "samsung,s2mu005-flash";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led@0 {
+ reg = <0>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_FLASH;
+ };
+
+ led@1 {
+ reg = <1>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_FLASH;
+ function-enumerator = <1>;
+ };
+ };
+
+ muic {
+ compatible = "samsung,s2mu005-muic";
+
+ connector {
+ compatible = "usb-b-connector";
+ label = "micro-USB";
+ type = "micro";
+ };
+
+ port {
+ muic_to_usb: endpoint {
+ remote-endpoint = <&usb_to_muic>;
+ };
+ };
+ };
+
+ multi-led {
+ compatible = "samsung,s2mu005-rgb";
+ color = <LED_COLOR_ID_RGB>;
+ function = LED_FUNCTION_INDICATOR;
+ linux,default-trigger = "pattern";
+ };
+ };
+ };
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v7 04/10] mfd: sec: add support for S2MU005 PMIC
2026-05-15 21:38 [PATCH v7 00/10] Support for Samsung S2MU005 PMIC and its sub-devices Kaustabh Chakraborty
` (2 preceding siblings ...)
2026-05-15 21:38 ` [PATCH v7 03/10] dt-bindings: mfd: add documentation for S2MU005 PMIC Kaustabh Chakraborty
@ 2026-05-15 21:38 ` Kaustabh Chakraborty
2026-05-15 22:25 ` sashiko-bot
2026-05-15 21:38 ` [PATCH v7 05/10] mfd: sec: set DMA coherent mask Kaustabh Chakraborty
` (5 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Kaustabh Chakraborty @ 2026-05-15 21:38 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, Yassine Oudjana
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc, Kaustabh Chakraborty
Samsung's S2MU005 PMIC includes subdevices for a charger, an MUIC (Micro
USB Interface Controller), and flash and RGB LED controllers.
S2MU005's interrupt registers divided into three domains, each for the
charger, flash LEDs, and the MUIC, packed into a single regmap IRQ chip
construct.
In devices other than S2MPG1X, the revision can be retrieved from the
first register of the PMIC regmap. In S2MU005 however, the location is
in offset 0x73. Introduce a switch-case block to allow selecting the
REG_ID register. S2MU005 also has a field mask for the revision. Apply
it using FIELD_GET() and get the extracted value.
Add initial support for S2MU005 in the PMIC driver, along with its three
interrupt chips, and support for allowing to fetch revision based on the
device variant.
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/mfd/sec-common.c | 34 +++-
drivers/mfd/sec-i2c.c | 29 ++++
drivers/mfd/sec-irq.c | 73 ++++++++
include/linux/mfd/samsung/core.h | 1 +
include/linux/mfd/samsung/irq.h | 66 +++++++
include/linux/mfd/samsung/s2mu005.h | 332 ++++++++++++++++++++++++++++++++++++
6 files changed, 529 insertions(+), 6 deletions(-)
diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
index bd8b5f9686892..22f6c74eb6c0e 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>
@@ -105,22 +106,39 @@ static const struct mfd_cell s2mpu05_devs[] = {
MFD_CELL_RES("s2mps15-rtc", s2mpu05_rtc_resources),
};
+static const struct resource s2mu005_muic_resources[] = {
+ DEFINE_RES_IRQ_NAMED(S2MU005_IRQ_MUIC_ATTACH, "attach"),
+ DEFINE_RES_IRQ_NAMED(S2MU005_IRQ_MUIC_DETACH, "detach"),
+};
+
+static const struct mfd_cell s2mu005_devs[] = {
+ MFD_CELL_NAME("s2mu005-charger"),
+ MFD_CELL_OF("s2mu005-flash", NULL, NULL, 0, 0, "samsung,s2mu005-flash"),
+ MFD_CELL_OF("s2mu005-muic", s2mu005_muic_resources, NULL, 0, 0, "samsung,s2mu005-muic"),
+ MFD_CELL_OF("s2mu005-rgb", NULL, NULL, 0, 0, "samsung,s2mu005-rgb"),
+};
+
static void sec_pmic_dump_rev(struct sec_pmic_dev *sec_pmic)
{
- unsigned int val;
+ unsigned int reg, mask, val;
- /* For s2mpg1x, the revision is in a different regmap */
switch (sec_pmic->device_type) {
case S2MPG10:
case S2MPG11:
+ /* For s2mpg1x, the revision is in a different regmap */
return;
- default:
+ case S2MU005:
+ reg = S2MU005_REG_ID;
+ mask = S2MU005_ID_MASK;
break;
+ default:
+ /* For other device types, REG_ID is always the first register. */
+ reg = S2MPS11_REG_ID;
+ mask = ~0;
}
- /* 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);
+ if (!regmap_read(sec_pmic->regmap_pmic, reg, &val))
+ dev_dbg(sec_pmic->dev, "Revision: 0x%x\n", field_get(mask, val));
}
static void sec_pmic_configure(struct sec_pmic_dev *sec_pmic)
@@ -250,6 +268,10 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
sec_devs = s2mpu05_devs;
num_sec_devs = ARRAY_SIZE(s2mpu05_devs);
break;
+ case S2MU005:
+ sec_devs = s2mu005_devs;
+ num_sec_devs = ARRAY_SIZE(s2mu005_devs);
+ break;
default:
return dev_err_probe(sec_pmic->dev, -EINVAL,
"Unsupported device type %d\n",
diff --git a/drivers/mfd/sec-i2c.c b/drivers/mfd/sec-i2c.c
index 3132b849b4bc4..d8609886fcc80 100644
--- a/drivers/mfd/sec-i2c.c
+++ b/drivers/mfd/sec-i2c.c
@@ -17,6 +17,7 @@
#include <linux/mfd/samsung/s2mps14.h>
#include <linux/mfd/samsung/s2mps15.h>
#include <linux/mfd/samsung/s2mpu02.h>
+#include <linux/mfd/samsung/s2mu005.h>
#include <linux/mfd/samsung/s5m8767.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
@@ -66,6 +67,19 @@ static bool s2mpu02_volatile(struct device *dev, unsigned int reg)
}
}
+static bool s2mu005_volatile(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case S2MU005_REG_CHGR_INT1M:
+ case S2MU005_REG_FLED_INT1M:
+ case S2MU005_REG_MUIC_INT1M:
+ case S2MU005_REG_MUIC_INT2M:
+ return false;
+ default:
+ return true;
+ }
+}
+
static const struct regmap_config s2dos05_regmap_config = {
.reg_bits = 8,
.val_bits = 8,
@@ -130,6 +144,15 @@ static const struct regmap_config s2mpu05_regmap_config = {
.val_bits = 8,
};
+static const struct regmap_config s2mu005_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+
+ .max_register = S2MU005_REG_MUIC_LDOADC_H,
+ .volatile_reg = s2mu005_volatile,
+ .cache_type = REGCACHE_FLAT_S,
+};
+
static const struct regmap_config s5m8767_regmap_config = {
.reg_bits = 8,
.val_bits = 8,
@@ -203,6 +226,11 @@ static const struct sec_pmic_i2c_platform_data s2mpu05_data = {
.device_type = S2MPU05,
};
+static const struct sec_pmic_i2c_platform_data s2mu005_data = {
+ .regmap_cfg = &s2mu005_regmap_config,
+ .device_type = S2MU005,
+};
+
static const struct sec_pmic_i2c_platform_data s5m8767_data = {
.regmap_cfg = &s5m8767_regmap_config,
.device_type = S5M8767X,
@@ -217,6 +245,7 @@ static const struct of_device_id sec_pmic_i2c_of_match[] = {
{ .compatible = "samsung,s2mps15-pmic", .data = &s2mps15_data, },
{ .compatible = "samsung,s2mpu02-pmic", .data = &s2mpu02_data, },
{ .compatible = "samsung,s2mpu05-pmic", .data = &s2mpu05_data, },
+ { .compatible = "samsung,s2mu005-pmic", .data = &s2mu005_data, },
{ .compatible = "samsung,s5m8767-pmic", .data = &s5m8767_data, },
{ },
};
diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
index 133188391f7c2..42862807be1a0 100644
--- a/drivers/mfd/sec-irq.c
+++ b/drivers/mfd/sec-irq.c
@@ -16,6 +16,7 @@
#include <linux/mfd/samsung/s2mps14.h>
#include <linux/mfd/samsung/s2mpu02.h>
#include <linux/mfd/samsung/s2mpu05.h>
+#include <linux/mfd/samsung/s2mu005.h>
#include <linux/mfd/samsung/s5m8767.h>
#include <linux/regmap.h>
#include "sec-core.h"
@@ -223,6 +224,65 @@ static const struct regmap_irq s2mpu05_irqs[] = {
REGMAP_IRQ_REG(S2MPU05_IRQ_TSD, 2, S2MPU05_IRQ_TSD_MASK),
};
+static const struct regmap_irq s2mu005_irqs[] = {
+ REGMAP_IRQ_REG(S2MU005_IRQ_CHGR_DETBAT, 0, S2MU005_IRQ_CHGR_DETBAT_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_CHGR_BAT, 0, S2MU005_IRQ_CHGR_BAT_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_CHGR_IVR, 0, S2MU005_IRQ_CHGR_IVR_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_CHGR_EVENT, 0, S2MU005_IRQ_CHGR_EVENT_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_CHGR_CHG, 0, S2MU005_IRQ_CHGR_CHG_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_CHGR_VMID, 0, S2MU005_IRQ_CHGR_VMID_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_CHGR_WCIN, 0, S2MU005_IRQ_CHGR_WCIN_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_CHGR_VBUS, 0, S2MU005_IRQ_CHGR_VBUS_MASK),
+
+ REGMAP_IRQ_REG(S2MU005_IRQ_FLED_LBPROT, 1, S2MU005_IRQ_FLED_LBPROT_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_FLED_OPENCH2, 1, S2MU005_IRQ_FLED_OPENCH2_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_FLED_OPENCH1, 1, S2MU005_IRQ_FLED_OPENCH1_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_FLED_SHORTCH2, 1, S2MU005_IRQ_FLED_SHORTCH2_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_FLED_SHORTCH1, 1, S2MU005_IRQ_FLED_SHORTCH1_MASK),
+
+ REGMAP_IRQ_REG(S2MU005_IRQ_MUIC_ATTACH, 2, S2MU005_IRQ_MUIC_ATTACH_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_MUIC_DETACH, 2, S2MU005_IRQ_MUIC_DETACH_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_MUIC_KP, 2, S2MU005_IRQ_MUIC_KP_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_MUIC_LKP, 2, S2MU005_IRQ_MUIC_LKP_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_MUIC_LKR, 2, S2MU005_IRQ_MUIC_LKR_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_MUIC_RIDCHG, 2, S2MU005_IRQ_MUIC_RIDCHG_MASK),
+
+ REGMAP_IRQ_REG(S2MU005_IRQ_MUIC_VBUSON, 3, S2MU005_IRQ_MUIC_VBUSON_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_MUIC_RSVD, 3, S2MU005_IRQ_MUIC_RSVD_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_MUIC_ADC, 3, S2MU005_IRQ_MUIC_ADC_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_MUIC_STUCK, 3, S2MU005_IRQ_MUIC_STUCK_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_MUIC_STUCKRCV, 3, S2MU005_IRQ_MUIC_STUCKRCV_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_MUIC_MHDL, 3, S2MU005_IRQ_MUIC_MHDL_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_MUIC_AVCHG, 3, S2MU005_IRQ_MUIC_AVCHG_MASK),
+ REGMAP_IRQ_REG(S2MU005_IRQ_MUIC_VBUSOFF, 3, S2MU005_IRQ_MUIC_VBUSOFF_MASK),
+};
+
+static unsigned int s2mu005_irq_get_reg(struct regmap_irq_chip_data *data,
+ unsigned int base, int index)
+{
+ const unsigned int irqf_regs[] = {
+ S2MU005_REG_CHGR_INT1,
+ S2MU005_REG_FLED_INT1,
+ S2MU005_REG_MUIC_INT1,
+ S2MU005_REG_MUIC_INT2,
+ };
+ const unsigned int mask_regs[] = {
+ S2MU005_REG_CHGR_INT1M,
+ S2MU005_REG_FLED_INT1M,
+ S2MU005_REG_MUIC_INT1M,
+ S2MU005_REG_MUIC_INT2M,
+ };
+
+ switch (base) {
+ case S2MU005_REG_CHGR_INT1:
+ return irqf_regs[index];
+ case S2MU005_REG_CHGR_INT1M:
+ return mask_regs[index];
+ }
+
+ return base;
+}
+
static const struct regmap_irq s5m8767_irqs[] = {
REGMAP_IRQ_REG(S5M8767_IRQ_PWRR, 0, S5M8767_IRQ_PWRR_MASK),
REGMAP_IRQ_REG(S5M8767_IRQ_PWRF, 0, S5M8767_IRQ_PWRF_MASK),
@@ -337,6 +397,16 @@ static const struct regmap_irq_chip s2mpu05_irq_chip = {
.ack_base = S2MPU05_REG_INT1,
};
+static const struct regmap_irq_chip s2mu005_irq_chip = {
+ .name = "s2mu005",
+ .irqs = s2mu005_irqs,
+ .num_irqs = ARRAY_SIZE(s2mu005_irqs),
+ .num_regs = 4,
+ .status_base = S2MU005_REG_CHGR_INT1,
+ .mask_base = S2MU005_REG_CHGR_INT1M,
+ .get_irq_reg = s2mu005_irq_get_reg,
+};
+
static const struct regmap_irq_chip s5m8767_irq_chip = {
.name = "s5m8767",
.irqs = s5m8767_irqs,
@@ -442,6 +512,9 @@ struct regmap_irq_chip_data *sec_irq_init(struct sec_pmic_dev *sec_pmic)
case S2MPU05:
sec_irq_chip = &s2mpu05_irq_chip;
break;
+ case S2MU005:
+ sec_irq_chip = &s2mu005_irq_chip;
+ break;
default:
return dev_err_ptr_probe(sec_pmic->dev, -EINVAL, "Unsupported device type %d\n",
sec_pmic->device_type);
diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
index 4480c631110a6..6191f409de945 100644
--- a/include/linux/mfd/samsung/core.h
+++ b/include/linux/mfd/samsung/core.h
@@ -47,6 +47,7 @@ enum sec_device_type {
S2MPS15X,
S2MPU02,
S2MPU05,
+ S2MU005,
};
/**
diff --git a/include/linux/mfd/samsung/irq.h b/include/linux/mfd/samsung/irq.h
index 6eab95de6fa83..19d0f0e12944f 100644
--- a/include/linux/mfd/samsung/irq.h
+++ b/include/linux/mfd/samsung/irq.h
@@ -408,6 +408,72 @@ enum s2mpu05_irq {
#define S2MPU05_IRQ_INT140C_MASK BIT(1)
#define S2MPU05_IRQ_TSD_MASK BIT(2)
+enum s2mu005_irq {
+ S2MU005_IRQ_CHGR_DETBAT,
+ S2MU005_IRQ_CHGR_BAT,
+ S2MU005_IRQ_CHGR_IVR,
+ S2MU005_IRQ_CHGR_EVENT,
+ S2MU005_IRQ_CHGR_CHG,
+ S2MU005_IRQ_CHGR_VMID,
+ S2MU005_IRQ_CHGR_WCIN,
+ S2MU005_IRQ_CHGR_VBUS,
+
+ S2MU005_IRQ_FLED_LBPROT,
+ S2MU005_IRQ_FLED_OPENCH2,
+ S2MU005_IRQ_FLED_OPENCH1,
+ S2MU005_IRQ_FLED_SHORTCH2,
+ S2MU005_IRQ_FLED_SHORTCH1,
+
+ S2MU005_IRQ_MUIC_ATTACH,
+ S2MU005_IRQ_MUIC_DETACH,
+ S2MU005_IRQ_MUIC_KP,
+ S2MU005_IRQ_MUIC_LKP,
+ S2MU005_IRQ_MUIC_LKR,
+ S2MU005_IRQ_MUIC_RIDCHG,
+
+ S2MU005_IRQ_MUIC_VBUSON,
+ S2MU005_IRQ_MUIC_RSVD,
+ S2MU005_IRQ_MUIC_ADC,
+ S2MU005_IRQ_MUIC_STUCK,
+ S2MU005_IRQ_MUIC_STUCKRCV,
+ S2MU005_IRQ_MUIC_MHDL,
+ S2MU005_IRQ_MUIC_AVCHG,
+ S2MU005_IRQ_MUIC_VBUSOFF,
+
+ S2MU005_IRQ_NR,
+};
+
+#define S2MU005_IRQ_CHGR_DETBAT_MASK BIT(0)
+#define S2MU005_IRQ_CHGR_BAT_MASK BIT(1)
+#define S2MU005_IRQ_CHGR_IVR_MASK BIT(2)
+#define S2MU005_IRQ_CHGR_EVENT_MASK BIT(3)
+#define S2MU005_IRQ_CHGR_CHG_MASK BIT(4)
+#define S2MU005_IRQ_CHGR_VMID_MASK BIT(5)
+#define S2MU005_IRQ_CHGR_WCIN_MASK BIT(6)
+#define S2MU005_IRQ_CHGR_VBUS_MASK BIT(7)
+
+#define S2MU005_IRQ_FLED_LBPROT_MASK BIT(2)
+#define S2MU005_IRQ_FLED_OPENCH2_MASK BIT(4)
+#define S2MU005_IRQ_FLED_OPENCH1_MASK BIT(5)
+#define S2MU005_IRQ_FLED_SHORTCH2_MASK BIT(6)
+#define S2MU005_IRQ_FLED_SHORTCH1_MASK BIT(7)
+
+#define S2MU005_IRQ_MUIC_ATTACH_MASK BIT(0)
+#define S2MU005_IRQ_MUIC_DETACH_MASK BIT(1)
+#define S2MU005_IRQ_MUIC_KP_MASK BIT(2)
+#define S2MU005_IRQ_MUIC_LKP_MASK BIT(3)
+#define S2MU005_IRQ_MUIC_LKR_MASK BIT(4)
+#define S2MU005_IRQ_MUIC_RIDCHG_MASK BIT(5)
+
+#define S2MU005_IRQ_MUIC_VBUSON_MASK BIT(0)
+#define S2MU005_IRQ_MUIC_RSVD_MASK BIT(1)
+#define S2MU005_IRQ_MUIC_ADC_MASK BIT(2)
+#define S2MU005_IRQ_MUIC_STUCK_MASK BIT(3)
+#define S2MU005_IRQ_MUIC_STUCKRCV_MASK BIT(4)
+#define S2MU005_IRQ_MUIC_MHDL_MASK BIT(5)
+#define S2MU005_IRQ_MUIC_AVCHG_MASK BIT(6)
+#define S2MU005_IRQ_MUIC_VBUSOFF_MASK BIT(7)
+
enum s5m8767_irq {
S5M8767_IRQ_PWRR,
S5M8767_IRQ_PWRF,
diff --git a/include/linux/mfd/samsung/s2mu005.h b/include/linux/mfd/samsung/s2mu005.h
new file mode 100644
index 0000000000000..46e7759545af2
--- /dev/null
+++ b/include/linux/mfd/samsung/s2mu005.h
@@ -0,0 +1,332 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd
+ * Copyright (c) 2026 Kaustabh Chakraborty <kauschluss@disroot.org>
+ * Copyright (c) 2026 Łukasz Lebiedziński <kernel@lvkasz.us>
+ */
+
+#ifndef __LINUX_MFD_S2MU005_H
+#define __LINUX_MFD_S2MU005_H
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+
+/* S2MU005 registers */
+enum s2mu005_reg {
+ S2MU005_REG_CHGR_INT1,
+ S2MU005_REG_CHGR_INT1M,
+
+ S2MU005_REG_FLED_INT1,
+ S2MU005_REG_FLED_INT1M,
+
+ S2MU005_REG_MUIC_INT1,
+ S2MU005_REG_MUIC_INT2,
+ S2MU005_REG_MUIC_INT1M,
+ S2MU005_REG_MUIC_INT2M,
+
+ S2MU005_REG_CHGR_STATUS0,
+ S2MU005_REG_CHGR_STATUS1,
+ S2MU005_REG_CHGR_STATUS2,
+ S2MU005_REG_CHGR_STATUS3,
+ S2MU005_REG_CHGR_STATUS4,
+ S2MU005_REG_CHGR_STATUS5,
+ S2MU005_REG_CHGR_CTRL0,
+ S2MU005_REG_CHGR_CTRL1,
+ S2MU005_REG_CHGR_CTRL2,
+ S2MU005_REG_CHGR_CTRL3,
+ S2MU005_REG_CHGR_CTRL4,
+ S2MU005_REG_CHGR_CTRL5,
+ S2MU005_REG_CHGR_CTRL6,
+ S2MU005_REG_CHGR_CTRL7,
+ S2MU005_REG_CHGR_CTRL8,
+ S2MU005_REG_CHGR_CTRL9,
+ S2MU005_REG_CHGR_CTRL10,
+ S2MU005_REG_CHGR_CTRL11,
+ S2MU005_REG_CHGR_CTRL12,
+ S2MU005_REG_CHGR_CTRL13,
+ S2MU005_REG_CHGR_CTRL14,
+ S2MU005_REG_CHGR_CTRL15,
+ S2MU005_REG_CHGR_CTRL16,
+ S2MU005_REG_CHGR_CTRL17,
+ S2MU005_REG_CHGR_CTRL18,
+ S2MU005_REG_CHGR_CTRL19,
+ S2MU005_REG_CHGR_TEST0,
+ S2MU005_REG_CHGR_TEST1,
+ S2MU005_REG_CHGR_TEST2,
+ S2MU005_REG_CHGR_TEST3,
+ S2MU005_REG_CHGR_TEST4,
+ S2MU005_REG_CHGR_TEST5,
+ S2MU005_REG_CHGR_TEST6,
+ S2MU005_REG_CHGR_TEST7,
+ S2MU005_REG_CHGR_TEST8,
+ S2MU005_REG_CHGR_TEST9,
+ S2MU005_REG_CHGR_TEST10,
+
+ S2MU005_REG_FLED_STATUS,
+ S2MU005_REG_FLED_CH0_CTRL0,
+ S2MU005_REG_FLED_CH0_CTRL1,
+ S2MU005_REG_FLED_CH0_CTRL2,
+ S2MU005_REG_FLED_CH0_CTRL3,
+ S2MU005_REG_FLED_CH1_CTRL0,
+ S2MU005_REG_FLED_CH1_CTRL1,
+ S2MU005_REG_FLED_CH1_CTRL2,
+ S2MU005_REG_FLED_CH1_CTRL3,
+ S2MU005_REG_FLED_CTRL0,
+ S2MU005_REG_FLED_CTRL1,
+ S2MU005_REG_FLED_CTRL2,
+ S2MU005_REG_FLED_CTRL3,
+ S2MU005_REG_FLED_CTRL4,
+ S2MU005_REG_FLED_CTRL5,
+ S2MU005_REG_FLED_CTRL6,
+
+ S2MU005_REG_RGB_EN,
+ S2MU005_REG_RGB_CH0_CTRL,
+ S2MU005_REG_RGB_CH1_CTRL,
+ S2MU005_REG_RGB_CH2_CTRL,
+ S2MU005_REG_RGB_CH0_RAMP,
+ S2MU005_REG_RGB_CH0_STAY,
+ S2MU005_REG_RGB_CH1_RAMP,
+ S2MU005_REG_RGB_CH1_STAY,
+ S2MU005_REG_RGB_CH2_RAMP,
+ S2MU005_REG_RGB_CH2_STAY,
+ S2MU005_REG_RGB_TEST0,
+ S2MU005_REG_RGB_CTRL0,
+
+ S2MU005_REG_MUIC_ADC,
+ S2MU005_REG_MUIC_DEV1,
+ S2MU005_REG_MUIC_DEV2,
+ S2MU005_REG_MUIC_DEV3,
+ S2MU005_REG_MUIC_BUTTON1,
+ S2MU005_REG_MUIC_BUTTON2,
+ S2MU005_REG_MUIC_RESET,
+ S2MU005_REG_MUIC_CHGTYPE,
+ S2MU005_REG_MUIC_DEVAPPLE,
+ S2MU005_REG_MUIC_BCDRESCAN,
+ S2MU005_REG_MUIC_TEST1,
+ S2MU005_REG_MUIC_TEST2,
+ S2MU005_REG_MUIC_TEST3,
+
+ S2MU005_REG_ID = 0x73,
+
+ S2MU005_REG_MUIC_CTRL1 = 0xb2,
+ S2MU005_REG_MUIC_TIMERSET1,
+ S2MU005_REG_MUIC_TIMERSET2,
+ S2MU005_REG_MUIC_SWCTRL,
+ S2MU005_REG_MUIC_TIMERSET3,
+ S2MU005_REG_MUIC_CTRL2,
+ S2MU005_REG_MUIC_CTRL3,
+
+ S2MU005_REG_MUIC_LDOADC_L = 0xbf,
+ S2MU005_REG_MUIC_LDOADC_H,
+};
+
+#define S2MU005_REG_FLED_CH_CTRL0(x) (S2MU005_REG_FLED_CH0_CTRL0 + 4 * (x))
+#define S2MU005_REG_FLED_CH_CTRL1(x) (S2MU005_REG_FLED_CH0_CTRL1 + 4 * (x))
+#define S2MU005_REG_FLED_CH_CTRL2(x) (S2MU005_REG_FLED_CH0_CTRL2 + 4 * (x))
+#define S2MU005_REG_FLED_CH_CTRL3(x) (S2MU005_REG_FLED_CH0_CTRL3 + 4 * (x))
+
+#define S2MU005_REG_RGB_CH_CTRL(x) (S2MU005_REG_RGB_CH0_CTRL + 1 * (x))
+#define S2MU005_REG_RGB_CH_RAMP(x) (S2MU005_REG_RGB_CH0_RAMP + 2 * (x))
+#define S2MU005_REG_RGB_CH_STAY(x) (S2MU005_REG_RGB_CH0_STAY + 2 * (x))
+
+/* S2MU005_REG_CHGR_STATUS0 */
+#define S2MU005_CHGR_VBUS BIT(7)
+#define S2MU005_CHGR_WCIN BIT(6)
+#define S2MU005_CHGR_VMID BIT(5)
+#define S2MU005_CHGR_CHG BIT(4)
+#define S2MU005_CHGR_STAT GENMASK(3, 0)
+
+#define S2MU005_CHGR_STAT_DONE 8
+#define S2MU005_CHGR_STAT_TOPOFF 7
+#define S2MU005_CHGR_STAT_DONE_FLAG 6
+#define S2MU005_CHGR_STAT_CV 5
+#define S2MU005_CHGR_STAT_CC 4
+#define S2MU005_CHGR_STAT_COOL_CHG 3
+#define S2MU005_CHGR_STAT_PRE_CHG 2
+
+/* S2MU005_REG_CHGR_STATUS1 */
+#define S2MU005_CHGR_DETBAT BIT(7)
+#define S2MU005_CHGR_VBUS_OVP GENMASK(6, 4)
+
+#define S2MU005_CHGR_VBUS_OVP_OVERVOLT 2
+
+/* S2MU005_REG_CHGR_STATUS2 */
+#define S2MU005_CHGR_BAT GENMASK(6, 4)
+
+#define S2MU005_CHGR_BAT_VOLT_DET 7
+#define S2MU005_CHGR_BAT_FAST_CHG_DET 6
+#define S2MU005_CHGR_BAT_COOL_CHG_DET 5
+#define S2MU005_CHGR_BAT_LOW_CHG 2
+#define S2MU005_CHGR_BAT_SELF_DISCHG 1
+#define S2MU005_CHGR_BAT_OVP_DET 0
+
+/* S2MU005_REG_CHGR_STATUS3 */
+#define S2MU005_CHGR_EVT GENMASK(3, 0)
+
+#define S2MU005_CHGR_EVT_WDT_RST 6
+#define S2MU005_CHGR_EVT_WDT_SUSP 5
+#define S2MU005_CHGR_EVT_VSYS_VUVLO 4
+#define S2MU005_CHGR_EVT_VSYS_VOVP 3
+#define S2MU005_CHGR_EVT_THERM_FOLDBACK 2
+#define S2MU005_CHGR_EVT_THERM_SHUTDOWN 1
+
+/* S2MU005_REG_CHGR_CTRL0 */
+#define S2MU005_CHGR_CHG_EN BIT(4)
+#define S2MU005_CHGR_OP_MODE GENMASK(2, 0)
+
+#define S2MU005_CHGR_OP_MODE_OTG BIT(2)
+#define S2MU005_CHGR_OP_MODE_CHG BIT(1)
+
+/* S2MU005_REG_CHGR_CTRL1 */
+#define S2MU005_CHGR_VIN_DROP GENMASK(6, 4)
+
+/* S2MU005_REG_CHGR_CTRL2 */
+#define S2MU005_CHGR_IN_CURR_LIM GENMASK(5, 0)
+
+/* S2MU005_REG_CHGR_CTRL4 */
+#define S2MU005_CHGR_OTG_OCP_ON BIT(5)
+#define S2MU005_CHGR_OTG_OCP_OFF BIT(4)
+#define S2MU005_CHGR_OTG_OCP GENMASK(3, 2)
+#define S2MU005_CHGR_OTG_OCP_1P5A 0x3
+
+/* S2MU005_REG_CHGR_CTRL5 */
+#define S2MU005_CHGR_VMID_BOOST GENMASK(4, 0)
+#define S2MU005_CHGR_VMID_BOOST_5P1V 0x16
+
+/* S2MU005_REG_CHGR_CTRL6 */
+#define S2MU005_CHGR_COOL_CHG_CURR GENMASK(5, 0)
+
+/* S2MU005_REG_CHGR_CTRL7 */
+#define S2MU005_CHGR_FAST_CHG_CURR GENMASK(5, 0)
+
+/* S2MU005_REG_CHGR_CTRL8 */
+#define S2MU005_CHGR_VF_VBAT GENMASK(6, 1)
+
+/* S2MU005_REG_CHGR_CTRL10 */
+#define S2MU005_CHGR_TOPOFF_CURR(x) (GENMASK(3, 0) << 4 * (x))
+
+/* S2MU005_REG_CHGR_CTRL11 */
+#define S2MU005_CHGR_OSC_BOOST GENMASK(6, 5)
+#define S2MU005_CHGR_OSC_BUCK GENMASK(4, 3)
+#define S2MU005_CHGR_OSC_BOOST_2MHZ 0x3
+
+/* S2MU005_REG_CHGR_CTRL12 */
+#define S2MU005_CHGR_WDT GENMASK(2, 0)
+
+#define S2MU005_CHGR_WDT_ON BIT(2)
+#define S2MU005_CHGR_WDT_OFF BIT(1)
+
+/* S2MU005_REG_CHGR_CTRL15 */
+#define S2MU005_CHGR_OTG_EN GENMASK(3, 2)
+#define S2MU005_CHGR_OTG_EN_ON 0x3
+
+/* S2MU005_REG_FLED_STATUS */
+#define S2MU005_FLED_FLASH_STATUS(x) (BIT(7) >> 2 * (x))
+#define S2MU005_FLED_TORCH_STATUS(x) (BIT(6) >> 2 * (x))
+
+/* S2MU005_REG_FLED_CHx_CTRL0 */
+#define S2MU005_FLED_FLASH_IOUT GENMASK(3, 0)
+
+/* S2MU005_REG_FLED_CHx_CTRL1 */
+#define S2MU005_FLED_TORCH_IOUT GENMASK(3, 0)
+
+/* S2MU005_REG_FLED_CHx_CTRL2 */
+#define S2MU005_FLED_TORCH_TIMEOUT GENMASK(3, 0)
+
+/* S2MU005_REG_FLED_CHx_CTRL3 */
+#define S2MU005_FLED_FLASH_TIMEOUT GENMASK(3, 0)
+
+/* S2MU005_REG_FLED_CTRL1 */
+#define S2MU005_FLED_CH_EN BIT(7)
+
+/*
+ * S2MU005_REG_FLED_CTRL4 - Rev. EVT0
+ * S2MU005_REG_FLED_CTRL6 - Rev. EVT1 and later
+ */
+#define S2MU005_FLED_FLASH_EN(x) (GENMASK(7, 6) >> 4 * (x))
+#define S2MU005_FLED_TORCH_EN(x) (GENMASK(5, 4) >> 4 * (x))
+
+/* S2MU005_REG_RGB_EN */
+#define S2MU005_RGB_RESET BIT(6)
+#define S2MU005_RGB_SLOPE GENMASK(5, 0)
+
+#define S2MU005_RGB_SLOPE_CONST (BIT(4) | BIT(2) | BIT(0))
+#define S2MU005_RGB_SLOPE_SMOOTH (BIT(5) | BIT(3) | BIT(1))
+
+/* S2MU005_REG_RGB_CHx_RAMP */
+#define S2MU005_RGB_CH_RAMP_UP GENMASK(7, 4)
+#define S2MU005_RGB_CH_RAMP_DN GENMASK(3, 0)
+
+/* S2MU005_REG_RGB_CHx_STAY */
+#define S2MU005_RGB_CH_STAY_HI GENMASK(7, 4)
+#define S2MU005_RGB_CH_STAY_LO GENMASK(3, 0)
+
+/* S2MU005_REG_MUIC_DEV1 */
+#define S2MU005_MUIC_OTG BIT(7)
+#define S2MU005_MUIC_DCP BIT(6)
+#define S2MU005_MUIC_CDP BIT(5)
+#define S2MU005_MUIC_T1_T2_CHG BIT(4)
+#define S2MU005_MUIC_UART BIT(3)
+#define S2MU005_MUIC_SDP BIT(2)
+#define S2MU005_MUIC_LANHUB BIT(1)
+#define S2MU005_MUIC_AUDIO BIT(0)
+
+/* S2MU005_REG_MUIC_DEV2 */
+#define S2MU005_MUIC_SDP_1P8S BIT(7)
+#define S2MU005_MUIC_AV BIT(6)
+#define S2MU005_MUIC_TTY BIT(5)
+#define S2MU005_MUIC_PPD BIT(4)
+#define S2MU005_MUIC_JIG_UART_OFF BIT(3)
+#define S2MU005_MUIC_JIG_UART_ON BIT(2)
+#define S2MU005_MUIC_JIG_USB_OFF BIT(1)
+#define S2MU005_MUIC_JIG_USB_ON BIT(0)
+
+/* S2MU005_REG_MUIC_DEV3 */
+#define S2MU005_MUIC_U200_CHG BIT(7)
+#define S2MU005_MUIC_VBUS_AV BIT(4)
+#define S2MU005_MUIC_VBUS_R255 BIT(1)
+#define S2MU005_MUIC_MHL BIT(0)
+
+/* S2MU005_REG_MUIC_DEVAPPLE */
+#define S2MU005_MUIC_APPLE_CHG_0P5A BIT(7)
+#define S2MU005_MUIC_APPLE_CHG_1P0A BIT(6)
+#define S2MU005_MUIC_APPLE_CHG_2P0A BIT(5)
+#define S2MU005_MUIC_APPLE_CHG_2P4A BIT(4)
+#define S2MU005_MUIC_SDP_DCD_OUT BIT(3)
+#define S2MU005_MUIC_RID_WAKEUP BIT(2)
+#define S2MU005_MUIC_VBUS_WAKEUP BIT(1)
+#define S2MU005_MUIC_BCV1P2_OR_OPEN BIT(0)
+
+/* S2MU005_REG_ID */
+#define S2MU005_ID_MASK GENMASK(3, 0)
+
+/* S2MU005_REG_MUIC_SWCTRL */
+#define S2MU005_MUIC_DM_DP GENMASK(7, 2)
+#define S2MU005_MUIC_JIG BIT(0)
+
+#define S2MU005_MUIC_DM_DP_UART 0x12
+#define S2MU005_MUIC_DM_DP_USB 0x09
+
+/* S2MU005_REG_MUIC_CTRL1 */
+#define S2MU005_MUIC_OPEN BIT(4)
+#define S2MU005_MUIC_RAW_DATA BIT(3)
+#define S2MU005_MUIC_MAN_SW BIT(2)
+#define S2MU005_MUIC_WAIT BIT(1)
+#define S2MU005_MUIC_IRQ BIT(0)
+
+/* S2MU005_REG_MUIC_CTRL3 */
+#define S2MU005_MUIC_ONESHOT_ADC BIT(2)
+
+/* S2MU005_REG_MUIC_LDOADC_L and S2MU005_REG_MUIC_LDOADC_H */
+#define S2MU005_MUIC_VSET GENMASK(4, 0)
+
+#define S2MU005_MUIC_VSET_3P0V 0x1f
+#define S2MU005_MUIC_VSET_2P6V 0x0e
+#define S2MU005_MUIC_VSET_2P4V 0x0c
+#define S2MU005_MUIC_VSET_2P2V 0x0a
+#define S2MU005_MUIC_VSET_2P0V 0x08
+#define S2MU005_MUIC_VSET_1P5V 0x03
+#define S2MU005_MUIC_VSET_1P4V 0x02
+#define S2MU005_MUIC_VSET_1P2V 0x00
+
+#endif /* __LINUX_MFD_S2MU005_H */
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v7 04/10] mfd: sec: add support for S2MU005 PMIC
2026-05-15 21:38 ` [PATCH v7 04/10] mfd: sec: add support " Kaustabh Chakraborty
@ 2026-05-15 22:25 ` sashiko-bot
2026-05-16 10:37 ` Kaustabh Chakraborty
0 siblings, 1 reply; 20+ messages in thread
From: sashiko-bot @ 2026-05-15 22:25 UTC (permalink / raw)
To: Kaustabh Chakraborty; +Cc: robh, krzk+dt, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing `.use_ack = true` in `s2mu005_irq_chip` causes an infinite interrupt storm.
--
commit 6e3b1141ae78b94854e84436a682b1574f6933ac
Author: Kaustabh Chakraborty <kauschluss@disroot.org>
mfd: sec: add support for S2MU005 PMIC
This commit adds support for Samsung's S2MU005 PMIC. It introduces a new
device type and handles its specific revision register fetching, as well
as providing its interrupt chips and child cell definitions.
> diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
> index 133188391f7c2..42862807be1a0 100644
> --- a/drivers/mfd/sec-irq.c
> +++ b/drivers/mfd/sec-irq.c
[ ... ]
> @@ -337,6 +397,16 @@ static const struct regmap_irq_chip s2mpu05_irq_chip = {
> .ack_base = S2MPU05_REG_INT1,
> };
>
> +static const struct regmap_irq_chip s2mu005_irq_chip = {
> + .name = "s2mu005",
> + .irqs = s2mu005_irqs,
> + .num_irqs = ARRAY_SIZE(s2mu005_irqs),
> + .num_regs = 4,
> + .status_base = S2MU005_REG_CHGR_INT1,
> + .mask_base = S2MU005_REG_CHGR_INT1M,
> + .get_irq_reg = s2mu005_irq_get_reg,
> +};
Does this structure need .use_ack = true?
In s2mu005_irq_chip, .ack_base is not explicitly set, so it implicitly
initializes to 0. The .status_base is set to S2MU005_REG_CHGR_INT1, which is
the first enumerator in s2mu005_reg and thus equals 0.
The regmap-irq core framework relies on the condition if (chip->ack_base ||
chip->use_ack) to determine if it should write back to the hardware registers
to acknowledge and clear the interrupts. Because both evaluate to 0 (false),
regmap-irq assumes the chip is clear-on-read and completely skips the ACK
execution.
Since Samsung PMICs require a Write-1-to-Clear sequence to clear interrupt
status registers, without the ACK write, the hardware PMIC continuously
asserts the physical IRQ line.
As the interrupt uses IRQF_ONESHOT, the moment the threaded handler completes
and unmasks the interrupt at the controller, the still-asserted hardware line
will immediately re-trigger the interrupt.
Could this loop cause an infinite interrupt storm, starving the CPU and
leading to a hard lockup?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260516-s2mu005-pmic-v7-0-73f9702fb461@disroot.org?part=4
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v7 04/10] mfd: sec: add support for S2MU005 PMIC
2026-05-15 22:25 ` sashiko-bot
@ 2026-05-16 10:37 ` Kaustabh Chakraborty
0 siblings, 0 replies; 20+ messages in thread
From: Kaustabh Chakraborty @ 2026-05-16 10:37 UTC (permalink / raw)
To: sashiko-reviews, Kaustabh Chakraborty; +Cc: robh, krzk+dt, conor+dt, devicetree
On 2026-05-15 22:25 +00:00, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Missing `.use_ack = true` in `s2mu005_irq_chip` causes an infinite interrupt storm.
> --
>
> commit 6e3b1141ae78b94854e84436a682b1574f6933ac
> Author: Kaustabh Chakraborty <kauschluss@disroot.org>
>
> mfd: sec: add support for S2MU005 PMIC
>
> This commit adds support for Samsung's S2MU005 PMIC. It introduces a new
> device type and handles its specific revision register fetching, as well
> as providing its interrupt chips and child cell definitions.
>
>> diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
>> index 133188391f7c2..42862807be1a0 100644
>> --- a/drivers/mfd/sec-irq.c
>> +++ b/drivers/mfd/sec-irq.c
>
> [ ... ]
>
>> @@ -337,6 +397,16 @@ static const struct regmap_irq_chip s2mpu05_irq_chip = {
>> .ack_base = S2MPU05_REG_INT1,
>> };
>>
>> +static const struct regmap_irq_chip s2mu005_irq_chip = {
>> + .name = "s2mu005",
>> + .irqs = s2mu005_irqs,
>> + .num_irqs = ARRAY_SIZE(s2mu005_irqs),
>> + .num_regs = 4,
>> + .status_base = S2MU005_REG_CHGR_INT1,
>> + .mask_base = S2MU005_REG_CHGR_INT1M,
>> + .get_irq_reg = s2mu005_irq_get_reg,
>> +};
>
> Does this structure need .use_ack = true?
>
> In s2mu005_irq_chip, .ack_base is not explicitly set, so it implicitly
> initializes to 0. The .status_base is set to S2MU005_REG_CHGR_INT1, which is
> the first enumerator in s2mu005_reg and thus equals 0.
>
> The regmap-irq core framework relies on the condition if (chip->ack_base ||
> chip->use_ack) to determine if it should write back to the hardware registers
> to acknowledge and clear the interrupts. Because both evaluate to 0 (false),
> regmap-irq assumes the chip is clear-on-read and completely skips the ACK
> execution.
>
> Since Samsung PMICs require a Write-1-to-Clear sequence to clear interrupt
> status registers, without the ACK write, the hardware PMIC continuously
> asserts the physical IRQ line.
>
> As the interrupt uses IRQF_ONESHOT, the moment the threaded handler completes
> and unmasks the interrupt at the controller, the still-asserted hardware line
> will immediately re-trigger the interrupt.
>
> Could this loop cause an infinite interrupt storm, starving the CPU and
> leading to a hard lockup?
This actually does not happen, this PMIC device doesn't require ack.
There's no interrupt storm when testing, and neither does the downstream
driver implement ack-ing code.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v7 05/10] mfd: sec: set DMA coherent mask
2026-05-15 21:38 [PATCH v7 00/10] Support for Samsung S2MU005 PMIC and its sub-devices Kaustabh Chakraborty
` (3 preceding siblings ...)
2026-05-15 21:38 ` [PATCH v7 04/10] mfd: sec: add support " Kaustabh Chakraborty
@ 2026-05-15 21:38 ` Kaustabh Chakraborty
2026-05-15 22:24 ` sashiko-bot
2026-05-15 21:38 ` [PATCH v7 06/10] leds: flash: add support for Samsung S2M series PMIC flash LED device Kaustabh Chakraborty
` (4 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Kaustabh Chakraborty @ 2026-05-15 21:38 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, Yassine Oudjana
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc, Kaustabh Chakraborty
Kernel logs are filled with "DMA mask not set" messages for every
sub-device. The device does not use DMA for communication, so these
messages are useless. Disable the coherent DMA mask for the PMIC device,
which is also propagated to sub-devices.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
drivers/mfd/sec-common.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
index 22f6c74eb6c0e..fe92bc4a3dd26 100644
--- a/drivers/mfd/sec-common.c
+++ b/drivers/mfd/sec-common.c
@@ -221,6 +221,9 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
if (IS_ERR(irq_data))
return PTR_ERR(irq_data);
+ dev->coherent_dma_mask = 0;
+ dev->dma_mask = &dev->coherent_dma_mask;
+
pm_runtime_set_active(sec_pmic->dev);
switch (sec_pmic->device_type) {
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v7 05/10] mfd: sec: set DMA coherent mask
2026-05-15 21:38 ` [PATCH v7 05/10] mfd: sec: set DMA coherent mask Kaustabh Chakraborty
@ 2026-05-15 22:24 ` sashiko-bot
2026-05-16 12:18 ` Kaustabh Chakraborty
0 siblings, 1 reply; 20+ messages in thread
From: sashiko-bot @ 2026-05-15 22:24 UTC (permalink / raw)
To: Kaustabh Chakraborty; +Cc: krzk+dt, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Shared dma_mask pointer across MFD child devices causes a data race and violates DMA API contracts.
--
commit cc615ed9ba9013f030c19361a7a4d79aad2e467d
Author: Kaustabh Chakraborty <kauschluss@disroot.org>
mfd: sec: set DMA coherent mask
This commit manually sets the coherent DMA mask to 0 and points the device's
dma_mask to it. This is intended to suppress "DMA mask not set" warnings
for sub-devices, as the device does not use DMA for communication.
> diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
> index 22f6c74eb6c0e..fe92bc4a3dd26 100644
> --- a/drivers/mfd/sec-common.c
> +++ b/drivers/mfd/sec-common.c
> @@ -221,6 +221,9 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
> if (IS_ERR(irq_data))
> return PTR_ERR(irq_data);
>
> + dev->coherent_dma_mask = 0;
> + dev->dma_mask = &dev->coherent_dma_mask;
When the MFD core adds child devices via mfd_add_device(), it copies the
parent's dma_mask pointer to the child devices:
pdev->dev.dma_mask = parent->dma_mask;
Because sub-devices for this PMIC probe asynchronously, they can concurrently
execute platform_dma_configure(), which eventually calls of_dma_configure_id()
and performs a lockless read-modify-write:
*dev->dma_mask &= mask;
Since all child devices share the same dma_mask pointer referencing the
parent's coherent_dma_mask, does this cause an unsynchronized data race on
the parent's memory?
Additionally, sharing a single dma_mask pointer across multiple devices breaks
the driver model. If any child device were to legitimately call dma_set_mask(),
wouldn't it silently overwrite the DMA mask for all sibling devices?
Finally, manually modifying device structures to fake an invalid mask of 0
bypasses DMA subsystem validation. Helpers like dma_coerce_mask_and_coherent()
explicitly reject a mask of 0. Is manually assigning 0 here the recommended way
to suppress the warning?
> +
> pm_runtime_set_active(sec_pmic->dev);
>
> switch (sec_pmic->device_type) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260516-s2mu005-pmic-v7-0-73f9702fb461@disroot.org?part=5
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v7 05/10] mfd: sec: set DMA coherent mask
2026-05-15 22:24 ` sashiko-bot
@ 2026-05-16 12:18 ` Kaustabh Chakraborty
0 siblings, 0 replies; 20+ messages in thread
From: Kaustabh Chakraborty @ 2026-05-16 12:18 UTC (permalink / raw)
To: sashiko-reviews, Kaustabh Chakraborty; +Cc: krzk+dt, devicetree, robh, conor+dt
On 2026-05-15 22:24 +00:00, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Shared dma_mask pointer across MFD child devices causes a data race and violates DMA API contracts.
> --
>
> commit cc615ed9ba9013f030c19361a7a4d79aad2e467d
> Author: Kaustabh Chakraborty <kauschluss@disroot.org>
>
> mfd: sec: set DMA coherent mask
>
> This commit manually sets the coherent DMA mask to 0 and points the device's
> dma_mask to it. This is intended to suppress "DMA mask not set" warnings
> for sub-devices, as the device does not use DMA for communication.
>
>> diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
>> index 22f6c74eb6c0e..fe92bc4a3dd26 100644
>> --- a/drivers/mfd/sec-common.c
>> +++ b/drivers/mfd/sec-common.c
>> @@ -221,6 +221,9 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
>> if (IS_ERR(irq_data))
>> return PTR_ERR(irq_data);
>>
>> + dev->coherent_dma_mask = 0;
>> + dev->dma_mask = &dev->coherent_dma_mask;
>
> When the MFD core adds child devices via mfd_add_device(), it copies the
> parent's dma_mask pointer to the child devices:
>
> pdev->dev.dma_mask = parent->dma_mask;
>
> Because sub-devices for this PMIC probe asynchronously, they can concurrently
> execute platform_dma_configure(), which eventually calls of_dma_configure_id()
> and performs a lockless read-modify-write:
>
> *dev->dma_mask &= mask;
>
> Since all child devices share the same dma_mask pointer referencing the
> parent's coherent_dma_mask, does this cause an unsynchronized data race on
> the parent's memory?
>
> Additionally, sharing a single dma_mask pointer across multiple devices breaks
> the driver model. If any child device were to legitimately call dma_set_mask(),
> wouldn't it silently overwrite the DMA mask for all sibling devices?
>
> Finally, manually modifying device structures to fake an invalid mask of 0
> bypasses DMA subsystem validation. Helpers like dma_coerce_mask_and_coherent()
> explicitly reject a mask of 0. Is manually assigning 0 here the recommended way
> to suppress the warning?
Note that change follows the same reasoning as in commit 0b7cbe811ca5
("mfd: cpcap: Fix cpcap dmamask not set warnings").
>> +
>> pm_runtime_set_active(sec_pmic->dev);
>>
>> switch (sec_pmic->device_type) {
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v7 06/10] leds: flash: add support for Samsung S2M series PMIC flash LED device
2026-05-15 21:38 [PATCH v7 00/10] Support for Samsung S2MU005 PMIC and its sub-devices Kaustabh Chakraborty
` (4 preceding siblings ...)
2026-05-15 21:38 ` [PATCH v7 05/10] mfd: sec: set DMA coherent mask Kaustabh Chakraborty
@ 2026-05-15 21:38 ` Kaustabh Chakraborty
2026-05-15 22:15 ` sashiko-bot
2026-05-15 21:38 ` [PATCH v7 07/10] leds: rgb: add support for Samsung S2M series PMIC RGB " Kaustabh Chakraborty
` (3 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Kaustabh Chakraborty @ 2026-05-15 21:38 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, Yassine Oudjana
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc, Kaustabh Chakraborty
Add support for flash LEDs found in certain Samsung S2M series PMICs.
The device has two channels for LEDs, typically for the back and front
cameras in mobile devices. Both channels can be independently
controlled, and can be operated in torch or flash modes.
The driver includes initial support for the S2MU005 PMIC flash LEDs.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
drivers/leds/flash/Kconfig | 11 ++
drivers/leds/flash/Makefile | 1 +
drivers/leds/flash/leds-s2m-flash.c | 350 ++++++++++++++++++++++++++++++++++++
3 files changed, 362 insertions(+)
diff --git a/drivers/leds/flash/Kconfig b/drivers/leds/flash/Kconfig
index 5e08102a67841..435b358f91243 100644
--- a/drivers/leds/flash/Kconfig
+++ b/drivers/leds/flash/Kconfig
@@ -114,6 +114,17 @@ config LEDS_RT8515
To compile this driver as a module, choose M here: the module
will be called leds-rt8515.
+config LEDS_S2M_FLASH
+ tristate "Samsung S2M series PMICs flash/torch LED support"
+ depends on LEDS_CLASS
+ depends on MFD_SEC_CORE
+ depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
+ help
+ This option enables support for the flash/torch LEDs found in certain
+ Samsung S2M series PMICs, such as the S2MU005. It has a LED channel
+ dedicated for every physical LED. The LEDs can be controlled in flash
+ and torch modes.
+
config LEDS_SGM3140
tristate "LED support for the SGM3140"
depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
diff --git a/drivers/leds/flash/Makefile b/drivers/leds/flash/Makefile
index 712fb737a428e..44e6c1b4beb37 100644
--- a/drivers/leds/flash/Makefile
+++ b/drivers/leds/flash/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_LEDS_MAX77693) += leds-max77693.o
obj-$(CONFIG_LEDS_QCOM_FLASH) += leds-qcom-flash.o
obj-$(CONFIG_LEDS_RT4505) += leds-rt4505.o
obj-$(CONFIG_LEDS_RT8515) += leds-rt8515.o
+obj-$(CONFIG_LEDS_S2M_FLASH) += leds-s2m-flash.o
obj-$(CONFIG_LEDS_SGM3140) += leds-sgm3140.o
obj-$(CONFIG_LEDS_SY7802) += leds-sy7802.o
obj-$(CONFIG_LEDS_TPS6131X) += leds-tps6131x.o
diff --git a/drivers/leds/flash/leds-s2m-flash.c b/drivers/leds/flash/leds-s2m-flash.c
new file mode 100644
index 0000000000000..6ee8db094611a
--- /dev/null
+++ b/drivers/leds/flash/leds-s2m-flash.c
@@ -0,0 +1,350 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Flash and Torch LED Driver for Samsung S2M series PMICs.
+ *
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd
+ * Copyright (c) 2026 Kaustabh Chakraborty <kauschluss@disroot.org>
+ */
+
+#include <linux/container_of.h>
+#include <linux/led-class-flash.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>
+#include <media/v4l2-flash-led-class.h>
+
+#define MAX_CHANNELS 2
+
+struct s2m_led {
+ struct regmap *regmap;
+ struct led_classdev_flash fled;
+ struct v4l2_flash *v4l2_flash;
+ /*
+ * The mutex object prevents the concurrent access of flash control
+ * registers by the LED and V4L2 subsystems.
+ */
+ struct mutex lock;
+ unsigned int reg_enable;
+ u8 channel;
+ u8 flash_brightness;
+ u8 flash_timeout;
+};
+
+static struct s2m_led *to_s2m_led(struct led_classdev_flash *fled)
+{
+ return container_of(fled, struct s2m_led, fled);
+}
+
+static struct led_classdev_flash *to_s2m_fled(struct led_classdev *cdev)
+{
+ return container_of(cdev, struct led_classdev_flash, led_cdev);
+}
+
+static int s2m_fled_flash_brightness_set(struct led_classdev_flash *fled, u32 brightness)
+{
+ struct s2m_led *led = to_s2m_led(fled);
+ struct led_flash_setting *setting = &fled->brightness;
+
+ mutex_lock(&led->lock);
+ led->flash_brightness = (brightness - setting->min) / setting->step;
+ mutex_unlock(&led->lock);
+
+ return 0;
+}
+
+static int s2m_fled_flash_timeout_set(struct led_classdev_flash *fled, u32 timeout)
+{
+ struct s2m_led *led = to_s2m_led(fled);
+ struct led_flash_setting *setting = &fled->timeout;
+
+ mutex_lock(&led->lock);
+ led->flash_timeout = (timeout - setting->min) / setting->step;
+ mutex_unlock(&led->lock);
+
+ return 0;
+}
+
+#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
+static int s2m_fled_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable)
+{
+ struct s2m_led *led = to_s2m_led(v4l2_flash->fled_cdev);
+
+ return led->fled.ops->strobe_set(&led->fled, enable);
+}
+
+static const struct v4l2_flash_ops s2m_fled_v4l2_flash_ops = {
+ .external_strobe_set = s2m_fled_flash_external_strobe_set,
+};
+#else
+static const struct v4l2_flash_ops s2m_fled_v4l2_flash_ops;
+#endif
+
+static void s2m_fled_v4l2_flash_release(void *v4l2_flash)
+{
+ v4l2_flash_release(v4l2_flash);
+}
+
+static int s2mu005_fled_torch_brightness_set(struct led_classdev *cdev, enum led_brightness value)
+{
+ struct s2m_led *led = to_s2m_led(to_s2m_fled(cdev));
+ int ret;
+
+ mutex_lock(&led->lock);
+
+ if (!value) {
+ ret = regmap_clear_bits(led->regmap, led->reg_enable,
+ S2MU005_FLED_TORCH_EN(led->channel));
+ if (ret)
+ dev_err(cdev->dev, "failed to disable torch LED\n");
+ goto unlock;
+ }
+
+ ret = regmap_update_bits(led->regmap, S2MU005_REG_FLED_CH_CTRL1(led->channel),
+ S2MU005_FLED_TORCH_IOUT,
+ FIELD_PREP(S2MU005_FLED_TORCH_IOUT, value - 1));
+ if (ret) {
+ dev_err(cdev->dev, "failed to set torch current\n");
+ goto unlock;
+ }
+
+ ret = regmap_set_bits(led->regmap, led->reg_enable, S2MU005_FLED_TORCH_EN(led->channel));
+ if (ret) {
+ dev_err(cdev->dev, "failed to enable torch LED\n");
+ goto unlock;
+ }
+
+unlock:
+ mutex_unlock(&led->lock);
+
+ return ret;
+}
+
+static int s2mu005_fled_flash_strobe_set(struct led_classdev_flash *fled, bool state)
+{
+ struct s2m_led *led = to_s2m_led(fled);
+ int ret;
+
+ mutex_lock(&led->lock);
+
+ ret = regmap_clear_bits(led->regmap, led->reg_enable, S2MU005_FLED_FLASH_EN(led->channel));
+ if (ret) {
+ dev_err(fled->led_cdev.dev, "failed to disable flash LED\n");
+ goto unlock;
+ }
+
+ if (!state)
+ goto unlock;
+
+ ret = regmap_update_bits(led->regmap, S2MU005_REG_FLED_CH_CTRL0(led->channel),
+ S2MU005_FLED_FLASH_IOUT,
+ FIELD_PREP(S2MU005_FLED_FLASH_IOUT, led->flash_brightness));
+ if (ret) {
+ dev_err(fled->led_cdev.dev, "failed to set flash brightness\n");
+ goto unlock;
+ }
+
+ ret = regmap_update_bits(led->regmap, S2MU005_REG_FLED_CH_CTRL3(led->channel),
+ S2MU005_FLED_FLASH_TIMEOUT,
+ FIELD_PREP(S2MU005_FLED_FLASH_TIMEOUT, led->flash_timeout));
+ if (ret) {
+ dev_err(fled->led_cdev.dev, "failed to set flash timeout\n");
+ goto unlock;
+ }
+
+ ret = regmap_set_bits(led->regmap, led->reg_enable, S2MU005_FLED_FLASH_EN(led->channel));
+ if (ret) {
+ dev_err(fled->led_cdev.dev, "failed to enable flash LED\n");
+ goto unlock;
+ }
+
+unlock:
+ mutex_unlock(&led->lock);
+
+ return ret;
+}
+
+static int s2mu005_fled_flash_strobe_get(struct led_classdev_flash *fled, bool *state)
+{
+ struct s2m_led *led = to_s2m_led(fled);
+ u32 val;
+ int ret;
+
+ mutex_lock(&led->lock);
+
+ ret = regmap_read(led->regmap, S2MU005_REG_FLED_STATUS, &val);
+ if (ret) {
+ dev_err(fled->led_cdev.dev, "failed to fetch LED status\n");
+ goto unlock;
+ }
+
+ *state = !!(val & S2MU005_FLED_FLASH_STATUS(led->channel));
+
+unlock:
+ mutex_unlock(&led->lock);
+
+ return ret;
+}
+
+static const struct led_flash_ops s2mu005_fled_flash_ops = {
+ .flash_brightness_set = s2m_fled_flash_brightness_set,
+ .timeout_set = s2m_fled_flash_timeout_set,
+ .strobe_set = s2mu005_fled_flash_strobe_set,
+ .strobe_get = s2mu005_fled_flash_strobe_get,
+};
+
+static int s2mu005_fled_init(struct s2m_led *led, struct device *dev, struct regmap *regmap,
+ unsigned int nr_channels)
+{
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(regmap, S2MU005_REG_ID, &val);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to read revision\n");
+
+ for (int i = 0; i < nr_channels; i++) {
+ /*
+ * Read the revision register. Revision EVT0 has the register
+ * at CTRL4, while EVT1 and higher have it at CTRL6.
+ */
+ if (FIELD_GET(S2MU005_ID_MASK, val) == 0)
+ led[i].reg_enable = S2MU005_REG_FLED_CTRL4;
+ else
+ led[i].reg_enable = S2MU005_REG_FLED_CTRL6;
+ }
+
+ /* Enable the LED channels. */
+ ret = regmap_set_bits(regmap, S2MU005_REG_FLED_CTRL1, S2MU005_FLED_CH_EN);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to enable LED channels\n");
+
+ return 0;
+}
+
+static int s2mu005_fled_init_channel(struct s2m_led *led, struct device *dev,
+ struct fwnode_handle *fwnp)
+{
+ struct led_classdev *cdev = &led->fled.led_cdev;
+ struct led_init_data init_data = {};
+ struct v4l2_flash_config v4l2_cfg = {};
+ int ret;
+
+ cdev->max_brightness = 16;
+ cdev->brightness_set_blocking = s2mu005_fled_torch_brightness_set;
+ cdev->flags |= LED_DEV_CAP_FLASH;
+
+ led->fled.timeout.min = 62000;
+ led->fled.timeout.step = 62000;
+ led->fled.timeout.max = 992000;
+ led->fled.timeout.val = 992000;
+
+ led->fled.brightness.min = 25000;
+ led->fled.brightness.step = 25000;
+ led->fled.brightness.max = 375000; /* 400000 causes flickering */
+ led->fled.brightness.val = 375000;
+
+ s2m_fled_flash_timeout_set(&led->fled, led->fled.timeout.val);
+ s2m_fled_flash_brightness_set(&led->fled, led->fled.brightness.val);
+
+ led->fled.ops = &s2mu005_fled_flash_ops;
+
+ init_data.fwnode = fwnp;
+ ret = devm_led_classdev_flash_register_ext(dev, &led->fled, &init_data);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to create LED flash device\n");
+
+ v4l2_cfg.intensity.min = led->fled.brightness.min;
+ v4l2_cfg.intensity.step = led->fled.brightness.step;
+ v4l2_cfg.intensity.max = led->fled.brightness.max;
+ v4l2_cfg.intensity.val = led->fled.brightness.val;
+
+ v4l2_cfg.has_external_strobe = true;
+
+ led->v4l2_flash = v4l2_flash_init(dev, fwnp, &led->fled, &s2m_fled_v4l2_flash_ops,
+ &v4l2_cfg);
+ if (IS_ERR(led->v4l2_flash))
+ return dev_err_probe(dev, PTR_ERR(led->v4l2_flash),
+ "failed to create V4L2 flash device\n");
+
+ ret = devm_add_action_or_reset(dev, s2m_fled_v4l2_flash_release, led->v4l2_flash);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to add cleanup action\n");
+
+ return 0;
+}
+
+static int s2m_fled_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct sec_pmic_dev *ddata = dev_get_drvdata(dev->parent);
+ struct s2m_led *led;
+ int ret;
+
+ led = devm_kzalloc(dev, sizeof(*led) * MAX_CHANNELS, GFP_KERNEL);
+ if (!led)
+ return -ENOMEM;
+
+ /* Initialize LED controller with variant-specific implementation. */
+ ret = s2mu005_fled_init(led, dev, ddata->regmap_pmic, MAX_CHANNELS);
+ if (ret)
+ return ret;
+
+ device_for_each_child_node_scoped(dev, child) {
+ u32 reg;
+
+ if (fwnode_property_read_u32(child, "reg", ®))
+ continue;
+
+ if (reg >= MAX_CHANNELS) {
+ dev_warn(dev, "channel %d is non-existent\n", reg);
+ continue;
+ }
+
+ if (led[reg].regmap) {
+ dev_warn(dev, "duplicate node for channel %d\n", reg);
+ continue;
+ }
+
+ led[reg].regmap = ddata->regmap_pmic;
+ led[reg].channel = (u8)reg;
+
+ ret = devm_mutex_init(dev, &led[reg].lock);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to create mutex\n");
+
+ /* Initialize LED channel with variant-specific implementation. */
+ ret = s2mu005_fled_init_channel(led + reg, dev, child);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct platform_device_id s2m_fled_id_table[] = {
+ { "s2mu005-flash", S2MU005 },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(platform, s2m_fled_id_table);
+
+static const struct of_device_id s2m_fled_of_match_table[] = {
+ { .compatible = "samsung,s2mu005-flash", .data = (void *)S2MU005 },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, s2m_fled_of_match_table);
+
+static struct platform_driver s2m_fled_driver = {
+ .driver = {
+ .name = "s2m-flash",
+ },
+ .probe = s2m_fled_probe,
+ .id_table = s2m_fled_id_table,
+};
+module_platform_driver(s2m_fled_driver);
+
+MODULE_DESCRIPTION("Flash/Torch LED Driver for Samsung S2M Series PMICs");
+MODULE_AUTHOR("Kaustabh Chakraborty <kauschluss@disroot.org>");
+MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v7 06/10] leds: flash: add support for Samsung S2M series PMIC flash LED device
2026-05-15 21:38 ` [PATCH v7 06/10] leds: flash: add support for Samsung S2M series PMIC flash LED device Kaustabh Chakraborty
@ 2026-05-15 22:15 ` sashiko-bot
2026-05-16 12:43 ` Kaustabh Chakraborty
0 siblings, 1 reply; 20+ messages in thread
From: sashiko-bot @ 2026-05-15 22:15 UTC (permalink / raw)
To: Kaustabh Chakraborty; +Cc: conor+dt, devicetree, robh, krzk+dt
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Medium] The V4L2 external_strobe_set callback triggers a software strobe instead of configuring a hardware strobe.
- [Medium] The OF match table is declared but never assigned to the platform_driver struct.
- [Medium] Inconsistent Torch maximum brightness constraints between Sysfs and V4L2.
- [Medium] Lack of mutual exclusion between Torch and Flash modes.
- [Medium] Hardware state leak on probe failure or module unload.
--
commit b304c8fc634e3a0a1b0e1d670a3987ad37675a8c
Author: Kaustabh Chakraborty <kauschluss@disroot.org>
leds: flash: add support for Samsung S2M series PMIC flash LED device
This commit adds support for flash LEDs found in Samsung S2M series PMICs,
such as the S2MU005. The driver supports two independently controlled channels
that can operate in both torch and flash modes.
> diff --git a/drivers/leds/flash/leds-s2m-flash.c b/drivers/leds/flash/leds-s2m-flash.c
> new file mode 100644
> index 0000000000000..6ee8db094611a
> --- /dev/null
> +++ b/drivers/leds/flash/leds-s2m-flash.c
[ ... ]
> +static int s2m_fled_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable)
> +{
> + struct s2m_led *led = to_s2m_led(v4l2_flash->fled_cdev);
> +
> + return led->fled.ops->strobe_set(&led->fled, enable);
> +}
Does this implementation fire the flash immediately instead of configuring
it to wait for a hardware strobe signal?
The V4L2 API contract expects external_strobe_set to merely arm the device
for an external hardware pin assertion. If the PMIC lacks true
hardware-based external strobe support, should has_external_strobe be
omitted instead?
[ ... ]
> +static int s2mu005_fled_torch_brightness_set(struct led_classdev *cdev, enum led_brightness value)
> +{
[ ... ]
> + ret = regmap_set_bits(led->regmap, led->reg_enable, S2MU005_FLED_TORCH_EN(led->channel));
> + if (ret) {
> + dev_err(cdev->dev, "failed to enable torch LED\n");
> + goto unlock;
> + }
> +
> +unlock:
> + mutex_unlock(&led->lock);
> +
> + return ret;
> +}
> +
> +static int s2mu005_fled_flash_strobe_set(struct led_classdev_flash *fled, bool state)
> +{
[ ... ]
> + ret = regmap_set_bits(led->regmap, led->reg_enable, S2MU005_FLED_FLASH_EN(led->channel));
> + if (ret) {
> + dev_err(fled->led_cdev.dev, "failed to enable flash LED\n");
> + goto unlock;
> + }
> +
> +unlock:
> + mutex_unlock(&led->lock);
> +
> + return ret;
> +}
Since the LED subsystem allows Sysfs to enable Torch and Flash
independently, is there a risk that both modes could be active
simultaneously in the PMIC registers?
Neither s2mu005_fled_torch_brightness_set() nor
s2mu005_fled_flash_strobe_set() appears to clear the enable bit for the
opposite mode.
[ ... ]
> +static int s2mu005_fled_init(struct s2m_led *led, struct device *dev, struct regmap *regmap,
> + unsigned int nr_channels)
> +{
[ ... ]
> + /* Enable the LED channels. */
> + ret = regmap_set_bits(regmap, S2MU005_REG_FLED_CTRL1, S2MU005_FLED_CH_EN);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to enable LED channels\n");
> +
> + return 0;
> +}
If the probe fails during child node iteration, or if the module is
unloaded, does this leave the PMIC LED block permanently powered on?
It looks like S2MU005_FLED_CH_EN is set globally here but never cleared on
failure or removal, which could cause a resource and power leak.
[ ... ]
> +static int s2mu005_fled_init_channel(struct s2m_led *led, struct device *dev,
> + struct fwnode_handle *fwnp)
> +{
[ ... ]
> + cdev->max_brightness = 16;
> + cdev->brightness_set_blocking = s2mu005_fled_torch_brightness_set;
> + cdev->flags |= LED_DEV_CAP_FLASH;
[ ... ]
> + v4l2_cfg.intensity.min = led->fled.brightness.min;
> + v4l2_cfg.intensity.step = led->fled.brightness.step;
> + v4l2_cfg.intensity.max = led->fled.brightness.max;
> + v4l2_cfg.intensity.val = led->fled.brightness.val;
Does this create an inconsistency between the Sysfs and V4L2 Torch maximum
brightness constraints?
The code sets cdev->max_brightness to 16, but v4l2_cfg.intensity is
initialized using the Flash constraints. Since led->fled.brightness limits
the brightness to 375,000 uA (effectively level 15), V4L2 users might be
artificially restricted to level 15 while Sysfs users can reach 16.
[ ... ]
> +static const struct of_device_id s2m_fled_of_match_table[] = {
> + { .compatible = "samsung,s2mu005-flash", .data = (void *)S2MU005 },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, s2m_fled_of_match_table);
> +
> +static struct platform_driver s2m_fled_driver = {
> + .driver = {
> + .name = "s2m-flash",
> + },
> + .probe = s2m_fled_probe,
> + .id_table = s2m_fled_id_table,
> +};
> +module_platform_driver(s2m_fled_driver);
Should s2m_fled_of_match_table be assigned to the .of_match_table field
within the driver structure here?
It appears to be declared and exported above, but never assigned to the
driver. This might prevent Device Tree compatible string matching.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260516-s2mu005-pmic-v7-0-73f9702fb461@disroot.org?part=6
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v7 06/10] leds: flash: add support for Samsung S2M series PMIC flash LED device
2026-05-15 22:15 ` sashiko-bot
@ 2026-05-16 12:43 ` Kaustabh Chakraborty
0 siblings, 0 replies; 20+ messages in thread
From: Kaustabh Chakraborty @ 2026-05-16 12:43 UTC (permalink / raw)
To: sashiko-reviews, Kaustabh Chakraborty; +Cc: conor+dt, devicetree, robh, krzk+dt
On 2026-05-15 22:15 +00:00, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [Medium] The V4L2 external_strobe_set callback triggers a software strobe instead of configuring a hardware strobe.
> - [Medium] The OF match table is declared but never assigned to the platform_driver struct.
> - [Medium] Inconsistent Torch maximum brightness constraints between Sysfs and V4L2.
> - [Medium] Lack of mutual exclusion between Torch and Flash modes.
> - [Medium] Hardware state leak on probe failure or module unload.
> --
>
> commit b304c8fc634e3a0a1b0e1d670a3987ad37675a8c
> Author: Kaustabh Chakraborty <kauschluss@disroot.org>
>
> leds: flash: add support for Samsung S2M series PMIC flash LED device
>
> This commit adds support for flash LEDs found in Samsung S2M series PMICs,
> such as the S2MU005. The driver supports two independently controlled channels
> that can operate in both torch and flash modes.
>
>> diff --git a/drivers/leds/flash/leds-s2m-flash.c b/drivers/leds/flash/leds-s2m-flash.c
>> new file mode 100644
>> index 0000000000000..6ee8db094611a
>> --- /dev/null
>> +++ b/drivers/leds/flash/leds-s2m-flash.c
>
> [ ... ]
>
>> +static int s2m_fled_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable)
>> +{
>> + struct s2m_led *led = to_s2m_led(v4l2_flash->fled_cdev);
>> +
>> + return led->fled.ops->strobe_set(&led->fled, enable);
>> +}
>
> Does this implementation fire the flash immediately instead of configuring
> it to wait for a hardware strobe signal?
>
> The V4L2 API contract expects external_strobe_set to merely arm the device
> for an external hardware pin assertion. If the PMIC lacks true
> hardware-based external strobe support, should has_external_strobe be
> omitted instead?
This function performs hardware-based external strobing, and the flash
seems to fire immediately. I don't have access to the documentation for
this device, so I can't know internal details, but it does work as
expected.
> [ ... ]
>
>> +static int s2mu005_fled_torch_brightness_set(struct led_classdev *cdev, enum led_brightness value)
>> +{
> [ ... ]
>> + ret = regmap_set_bits(led->regmap, led->reg_enable, S2MU005_FLED_TORCH_EN(led->channel));
>> + if (ret) {
>> + dev_err(cdev->dev, "failed to enable torch LED\n");
>> + goto unlock;
>> + }
>> +
>> +unlock:
>> + mutex_unlock(&led->lock);
>> +
>> + return ret;
>> +}
>> +
>> +static int s2mu005_fled_flash_strobe_set(struct led_classdev_flash *fled, bool state)
>> +{
> [ ... ]
>> + ret = regmap_set_bits(led->regmap, led->reg_enable, S2MU005_FLED_FLASH_EN(led->channel));
>> + if (ret) {
>> + dev_err(fled->led_cdev.dev, "failed to enable flash LED\n");
>> + goto unlock;
>> + }
>> +
>> +unlock:
>> + mutex_unlock(&led->lock);
>> +
>> + return ret;
>> +}
>
> Since the LED subsystem allows Sysfs to enable Torch and Flash
> independently, is there a risk that both modes could be active
> simultaneously in the PMIC registers?
>
> Neither s2mu005_fled_torch_brightness_set() nor
> s2mu005_fled_flash_strobe_set() appears to clear the enable bit for the
> opposite mode.
Yes, both can be simultaneously active, but no, there's no risk. The
reason is that the torch and the flash are two separate, independent
LEDs, thus being controllable independently. So there is no need of
negotiation required between torch and flash mode.
> [ ... ]
>
>> +static int s2mu005_fled_init(struct s2m_led *led, struct device *dev, struct regmap *regmap,
>> + unsigned int nr_channels)
>> +{
> [ ... ]
>> + /* Enable the LED channels. */
>> + ret = regmap_set_bits(regmap, S2MU005_REG_FLED_CTRL1, S2MU005_FLED_CH_EN);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "failed to enable LED channels\n");
>> +
>> + return 0;
>> +}
>
> If the probe fails during child node iteration, or if the module is
> unloaded, does this leave the PMIC LED block permanently powered on?
>
> It looks like S2MU005_FLED_CH_EN is set globally here but never cleared on
> failure or removal, which could cause a resource and power leak.
FIXME. Although a non-issue in testing, it is duly appropriate to unset
it, in the driver .remove() callback.
> [ ... ]
>
>> +static int s2mu005_fled_init_channel(struct s2m_led *led, struct device *dev,
>> + struct fwnode_handle *fwnp)
>> +{
> [ ... ]
>> + cdev->max_brightness = 16;
>> + cdev->brightness_set_blocking = s2mu005_fled_torch_brightness_set;
>> + cdev->flags |= LED_DEV_CAP_FLASH;
> [ ... ]
>> + v4l2_cfg.intensity.min = led->fled.brightness.min;
>> + v4l2_cfg.intensity.step = led->fled.brightness.step;
>> + v4l2_cfg.intensity.max = led->fled.brightness.max;
>> + v4l2_cfg.intensity.val = led->fled.brightness.val;
>
> Does this create an inconsistency between the Sysfs and V4L2 Torch maximum
> brightness constraints?
>
> The code sets cdev->max_brightness to 16, but v4l2_cfg.intensity is
> initialized using the Flash constraints. Since led->fled.brightness limits
> the brightness to 375,000 uA (effectively level 15), V4L2 users might be
> artificially restricted to level 15 while Sysfs users can reach 16.
As explained above, torch and flash LEDs are different. The flash is
restricted to 15 as the level-16 equivalent causes flickering (the cause
is unknown), but the same is not true for the torch LED. Hence the
discrepancy.
> [ ... ]
>
>> +static const struct of_device_id s2m_fled_of_match_table[] = {
>> + { .compatible = "samsung,s2mu005-flash", .data = (void *)S2MU005 },
>> + { /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, s2m_fled_of_match_table);
>> +
>> +static struct platform_driver s2m_fled_driver = {
>> + .driver = {
>> + .name = "s2m-flash",
>> + },
>> + .probe = s2m_fled_probe,
>> + .id_table = s2m_fled_id_table,
>> +};
>> +module_platform_driver(s2m_fled_driver);
>
> Should s2m_fled_of_match_table be assigned to the .of_match_table field
> within the driver structure here?
>
> It appears to be declared and exported above, but never assigned to the
> driver. This might prevent Device Tree compatible string matching.
This is intended behavior, as was also documented in a previous revision
of this patch.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v7 07/10] leds: rgb: add support for Samsung S2M series PMIC RGB LED device
2026-05-15 21:38 [PATCH v7 00/10] Support for Samsung S2MU005 PMIC and its sub-devices Kaustabh Chakraborty
` (5 preceding siblings ...)
2026-05-15 21:38 ` [PATCH v7 06/10] leds: flash: add support for Samsung S2M series PMIC flash LED device Kaustabh Chakraborty
@ 2026-05-15 21:38 ` Kaustabh Chakraborty
2026-05-15 22:10 ` sashiko-bot
2026-05-15 21:38 ` [PATCH v7 08/10] Documentation: leds: document pattern behavior of Samsung S2M series PMIC RGB LEDs Kaustabh Chakraborty
` (2 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Kaustabh Chakraborty @ 2026-05-15 21:38 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, Yassine Oudjana
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc, Kaustabh Chakraborty
Add support for the RGB LEDs found in certain Samsung S2M series PMICs.
The device has three LED channels, controlled as a single device. These
LEDs are typically used as status indicators in mobile phones.
The driver includes initial support for the S2MU005 PMIC RGB LEDs.
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
drivers/leds/rgb/Kconfig | 10 +
drivers/leds/rgb/Makefile | 1 +
drivers/leds/rgb/leds-s2m-rgb.c | 426 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 437 insertions(+)
diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig
index 28ef4c487367c..b16144b48b8f8 100644
--- a/drivers/leds/rgb/Kconfig
+++ b/drivers/leds/rgb/Kconfig
@@ -75,6 +75,16 @@ config LEDS_QCOM_LPG
If compiled as a module, the module will be named leds-qcom-lpg.
+config LEDS_S2M_RGB
+ tristate "Samsung S2M series PMICs RGB LED support"
+ depends on LEDS_CLASS
+ depends on MFD_SEC_CORE
+ help
+ This option enables support for the S2MU005 RGB LEDs. These devices
+ have three LED channels, with 8-bit brightness control for each
+ channel. The S2MU005 is usually found in mobile phones as status
+ indicators.
+
config LEDS_MT6370_RGB
tristate "LED Support for MediaTek MT6370 PMIC"
depends on MFD_MT6370
diff --git a/drivers/leds/rgb/Makefile b/drivers/leds/rgb/Makefile
index be45991f63f50..98050e1aa4255 100644
--- a/drivers/leds/rgb/Makefile
+++ b/drivers/leds/rgb/Makefile
@@ -6,4 +6,5 @@ obj-$(CONFIG_LEDS_LP5812) += leds-lp5812.o
obj-$(CONFIG_LEDS_NCP5623) += leds-ncp5623.o
obj-$(CONFIG_LEDS_PWM_MULTICOLOR) += leds-pwm-multicolor.o
obj-$(CONFIG_LEDS_QCOM_LPG) += leds-qcom-lpg.o
+obj-$(CONFIG_LEDS_S2M_RGB) += leds-s2m-rgb.o
obj-$(CONFIG_LEDS_MT6370_RGB) += leds-mt6370-rgb.o
diff --git a/drivers/leds/rgb/leds-s2m-rgb.c b/drivers/leds/rgb/leds-s2m-rgb.c
new file mode 100644
index 0000000000000..d239f54eee901
--- /dev/null
+++ b/drivers/leds/rgb/leds-s2m-rgb.c
@@ -0,0 +1,426 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RGB LED Driver for Samsung S2M series PMICs.
+ *
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd
+ * Copyright (c) 2026 Kaustabh Chakraborty <kauschluss@disroot.org>
+ */
+
+#include <linux/container_of.h>
+#include <linux/led-class-multicolor.h>
+#include <linux/mfd/samsung/core.h>
+#include <linux/mfd/samsung/s2mu005.h>
+#include <linux/minmax.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+struct s2m_rgb {
+ struct device *dev;
+ struct regmap *regmap;
+ struct led_classdev_mc mc;
+ /*
+ * The mutex object prevents race conditions when evaluation and
+ * application of LED pattern state.
+ */
+ struct mutex lock;
+ /*
+ * State variables representing the current LED pattern, these only to
+ * be accessed when lock is held.
+ */
+ u8 ramp_up;
+ u8 ramp_dn;
+ u8 stay_hi;
+ u8 stay_lo;
+};
+
+static struct led_classdev_mc *to_s2m_mc(struct led_classdev *cdev)
+{
+ return container_of(cdev, struct led_classdev_mc, led_cdev);
+}
+
+static struct s2m_rgb *to_s2m_rgb(struct led_classdev_mc *mc)
+{
+ return container_of(mc, struct s2m_rgb, mc);
+}
+
+static const u32 s2mu005_rgb_lut_ramp[] = {
+ 0, 100, 200, 300, 400, 500, 600, 700,
+ 800, 1000, 1200, 1400, 1600, 1800, 2000, 2200,
+};
+
+static const u32 s2mu005_rgb_lut_stay_hi[] = {
+ 100, 200, 300, 400, 500, 750, 1000, 1250,
+ 1500, 1750, 2000, 2250, 2500, 2750, 3000, 3250,
+};
+
+static const u32 s2mu005_rgb_lut_stay_lo[] = {
+ 0, 500, 1000, 1500, 2000, 2500, 3000, 3500,
+ 4000, 4500, 5000, 6000, 7000, 8000, 10000, 12000,
+};
+
+static int s2mu005_rgb_apply_params(struct s2m_rgb *rgb)
+{
+ struct regmap *regmap = rgb->regmap;
+ unsigned int ramp_val = 0;
+ unsigned int stay_val = 0;
+ int ret;
+
+ ramp_val |= FIELD_PREP(S2MU005_RGB_CH_RAMP_UP, rgb->ramp_up);
+ ramp_val |= FIELD_PREP(S2MU005_RGB_CH_RAMP_DN, rgb->ramp_dn);
+
+ stay_val |= FIELD_PREP(S2MU005_RGB_CH_STAY_HI, rgb->stay_hi);
+ stay_val |= FIELD_PREP(S2MU005_RGB_CH_STAY_LO, rgb->stay_lo);
+
+ ret = regmap_write(regmap, S2MU005_REG_RGB_EN, S2MU005_RGB_RESET);
+ if (ret) {
+ dev_err(rgb->dev, "failed to reset RGB LEDs\n");
+ return ret;
+ }
+
+ for (int i = 0; i < rgb->mc.num_colors; i++) {
+ ret = regmap_write(regmap, S2MU005_REG_RGB_CH_CTRL(i),
+ rgb->mc.subled_info[i].brightness);
+ if (ret) {
+ dev_err(rgb->dev, "failed to set LED brightness\n");
+ return ret;
+ }
+
+ ret = regmap_write(regmap, S2MU005_REG_RGB_CH_RAMP(i), ramp_val);
+ if (ret) {
+ dev_err(rgb->dev, "failed to set ramp timings\n");
+ return ret;
+ }
+
+ ret = regmap_write(regmap, S2MU005_REG_RGB_CH_STAY(i), stay_val);
+ if (ret) {
+ dev_err(rgb->dev, "failed to set stay timings\n");
+ return ret;
+ }
+ }
+
+ ret = regmap_write(regmap, S2MU005_REG_RGB_EN, S2MU005_RGB_SLOPE_SMOOTH);
+ if (ret) {
+ dev_err(rgb->dev, "failed to set ramp slope\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int s2mu005_rgb_reset_params(struct s2m_rgb *rgb)
+{
+ struct regmap *regmap = rgb->regmap;
+ int ret;
+
+ ret = regmap_write(regmap, S2MU005_REG_RGB_EN, S2MU005_RGB_RESET);
+ if (ret) {
+ dev_err(rgb->dev, "failed to reset RGB LEDs\n");
+ return ret;
+ }
+
+ rgb->ramp_up = 0;
+ rgb->ramp_dn = 0;
+ rgb->stay_hi = 0;
+ rgb->stay_lo = 0;
+
+ return 0;
+}
+
+/*
+ * s2m_rgb_lut_get_closest_duration - find closest duration in look-up table
+ * @lut: the look-up table to search for the closest timing
+ * @len: number of elements in the look-up table array
+ * @duration: the timing duration requested
+ *
+ * This function does a binary search on the given array, and finds the closest
+ * value to the requested timing. It is expected that the look-up table to be
+ * provided, is already sorted.
+ *
+ * This function returns a negative error code, or a non-negative index of the
+ * value in the look-up table closest to the one requested.
+ */
+static int s2m_rgb_lut_get_closest_duration(const u32 *lut, const size_t len, const u32 duration)
+{
+ u32 closest_distance = abs(duration - lut[0]);
+ int closest_index = 0;
+ int lo = 0;
+ int hi = len - 1;
+
+ /*
+ * Allow a small amount of extrapolation beyond the highest timing value.
+ *
+ * Consider x and y to be the two last values in the table, and x < y.
+ * Since (y - x) / 2 integers, in the range [x + (y - x) / 2, y)
+ * returns y as the closest, allow extrapolation for the succeeding
+ * (y - x) / 2 integers as well, viz, up to (y, y + (y - x) / 2].
+ * Anything beyond that is invalid.
+ */
+ if (len >= 2 && duration > lut[len - 1] + (lut[len - 1] - lut[len - 2]) / 2)
+ return -EINVAL;
+
+ while (lo <= hi) {
+ int mid = lo + (hi - lo) / 2;
+
+ /* Narrow down search window as per binary-search algorithm. */
+ if (duration < lut[mid])
+ hi = mid - 1;
+ else
+ lo = mid + 1;
+
+ if (abs(duration - lut[mid]) < closest_distance) {
+ closest_distance = abs(duration - lut[mid]);
+ closest_index = mid;
+ }
+ }
+
+ return closest_index;
+}
+
+static int s2m_rgb_pattern_set(struct led_classdev *cdev, struct led_pattern *pattern, u32 len,
+ int repeat)
+{
+ struct s2m_rgb *rgb = to_s2m_rgb(to_s2m_mc(cdev));
+ const u32 *lut_ramp_up, *lut_ramp_dn, *lut_stay_hi, *lut_stay_lo;
+ size_t lut_ramp_up_len, lut_ramp_dn_len, lut_stay_hi_len, lut_stay_lo_len;
+ int brightness_peak = 0;
+ u32 time_hi = 0, time_lo = 0;
+ bool ramp_up_en = false, ramp_dn_en = false;
+ int ret;
+
+ /*
+ * The typical pattern supported by this device can be represented with
+ * the following graph:
+ *
+ * 255 T ''''''-. .-'''''''-.
+ * | '. .' '.
+ * | \ / \
+ * | '. .' '.
+ * | '-...........-' '-
+ * 0 +----------------------------------------------------> time (s)
+ *
+ * <---- HIGH ----><-- LOW --><-------- HIGH --------->
+ * <-----><-------><---------><-------><-----><------->
+ * stay_hi ramp_dn stay_lo ramp_up stay_hi ramp_dn
+ *
+ * There are two states, named HIGH and LOW. HIGH has a non-zero
+ * brightness level, while LOW is of zero brightness. The pattern
+ * provided should mention only one zero and non-zero brightness level.
+ * The hardware always starts the pattern from the HIGH state, as shown
+ * in the graph.
+ *
+ * The HIGH state can be divided in three somewhat equal timings:
+ * ramp_up, stay_hi, and ramp_dn. The LOW state has only one timing:
+ * stay_lo.
+ */
+
+ /* Only indefinitely looping patterns are supported. */
+ if (repeat != -1)
+ return -EINVAL;
+
+ /* Pattern should consist of at least two tuples. */
+ if (len < 2)
+ return -EINVAL;
+
+ for (int i = 0; i < len; i++) {
+ int brightness = pattern[i].brightness;
+ u32 delta_t = pattern[i].delta_t;
+
+ if (brightness) {
+ /*
+ * The pattern should define only one non-zero
+ * brightness in the HIGH state. The device doesn't
+ * have any provisions to handle multiple peak
+ * brightness levels.
+ */
+ if (brightness_peak && brightness_peak != brightness)
+ return -EINVAL;
+
+ brightness_peak = brightness;
+ time_hi += delta_t;
+ ramp_dn_en = !!delta_t;
+ } else {
+ time_lo += delta_t;
+ ramp_up_en = !!delta_t;
+ }
+ }
+
+ /* LUTs are specific to device variant. */
+ lut_ramp_up = s2mu005_rgb_lut_ramp;
+ lut_ramp_up_len = ARRAY_SIZE(s2mu005_rgb_lut_ramp);
+ lut_ramp_dn = s2mu005_rgb_lut_ramp;
+ lut_ramp_dn_len = ARRAY_SIZE(s2mu005_rgb_lut_ramp);
+ lut_stay_hi = s2mu005_rgb_lut_stay_hi;
+ lut_stay_hi_len = ARRAY_SIZE(s2mu005_rgb_lut_stay_hi);
+ lut_stay_lo = s2mu005_rgb_lut_stay_lo;
+ lut_stay_lo_len = ARRAY_SIZE(s2mu005_rgb_lut_stay_lo);
+
+ mutex_lock(&rgb->lock);
+
+ /*
+ * The timings ramp_up, stay_hi, and ramp_dn of the HIGH state are
+ * roughly equal. Firstly, calculate and set timings for ramp_up and
+ * ramp_dn (making sure they're exactly equal).
+ */
+ rgb->ramp_up = 0;
+ rgb->ramp_dn = 0;
+
+ if (ramp_up_en) {
+ ret = s2m_rgb_lut_get_closest_duration(lut_ramp_up, lut_ramp_up_len, time_hi / 3);
+ if (ret < 0)
+ goto param_fail;
+ rgb->ramp_up = (u8)ret;
+ }
+
+ if (ramp_dn_en) {
+ ret = s2m_rgb_lut_get_closest_duration(lut_ramp_dn, lut_ramp_dn_len, time_hi / 3);
+ if (ret < 0)
+ goto param_fail;
+ rgb->ramp_dn = (u8)ret;
+ }
+
+ /*
+ * Subtract the allocated ramp timings from time_hi (and also making
+ * sure it doesn't underflow!). The remaining time is allocated to
+ * stay_hi.
+ */
+ time_hi -= min(time_hi, lut_ramp_up[rgb->ramp_up]);
+ time_hi -= min(time_hi, lut_ramp_dn[rgb->ramp_dn]);
+
+ ret = s2m_rgb_lut_get_closest_duration(lut_stay_hi, lut_stay_hi_len, time_hi);
+ if (ret < 0)
+ goto param_fail;
+ rgb->stay_hi = (u8)ret;
+
+ ret = s2m_rgb_lut_get_closest_duration(lut_stay_lo, lut_stay_lo_len, time_lo);
+ if (ret < 0)
+ goto param_fail;
+ rgb->stay_lo = (u8)ret;
+
+ led_mc_calc_color_components(&rgb->mc, brightness_peak);
+ /* Apply params with variant-specific implementation. */
+ ret = s2mu005_rgb_apply_params(rgb);
+ if (ret)
+ goto param_fail;
+
+ mutex_unlock(&rgb->lock);
+
+ return 0;
+
+param_fail:
+ rgb->ramp_up = 0;
+ rgb->ramp_dn = 0;
+ rgb->stay_hi = 0;
+ rgb->stay_lo = 0;
+
+ mutex_unlock(&rgb->lock);
+
+ return ret;
+}
+
+static int s2m_rgb_pattern_clear(struct led_classdev *cdev)
+{
+ struct s2m_rgb *rgb = to_s2m_rgb(to_s2m_mc(cdev));
+ int ret = 0;
+
+ mutex_lock(&rgb->lock);
+
+ /* Reset params with variant-specific implementation. */
+ ret = s2mu005_rgb_reset_params(rgb);
+
+ mutex_unlock(&rgb->lock);
+
+ return ret;
+}
+
+static int s2m_rgb_brightness_set(struct led_classdev *cdev, enum led_brightness value)
+{
+ struct s2m_rgb *rgb = to_s2m_rgb(to_s2m_mc(cdev));
+ int ret = 0;
+
+ if (!value)
+ return s2m_rgb_pattern_clear(cdev);
+
+ mutex_lock(&rgb->lock);
+
+ led_mc_calc_color_components(&rgb->mc, value);
+ /* Apply params with variant-specific implementation. */
+ ret = s2mu005_rgb_apply_params(rgb);
+
+ mutex_unlock(&rgb->lock);
+
+ return ret;
+}
+
+static const struct mc_subled s2mu005_rgb_subled_info[] = {
+ { .channel = 0, .color_index = LED_COLOR_ID_BLUE },
+ { .channel = 1, .color_index = LED_COLOR_ID_GREEN },
+ { .channel = 2, .color_index = LED_COLOR_ID_RED },
+};
+
+static int s2m_rgb_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct sec_pmic_dev *pmic_drvdata = dev_get_drvdata(dev->parent);
+ struct s2m_rgb *rgb;
+ struct led_init_data init_data = {};
+ int ret;
+
+ rgb = devm_kzalloc(dev, sizeof(*rgb), GFP_KERNEL);
+ if (!rgb)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, rgb);
+ rgb->dev = dev;
+ rgb->regmap = pmic_drvdata->regmap_pmic;
+
+ /* Configure variant-specific details. */
+ rgb->mc.num_colors = ARRAY_SIZE(s2mu005_rgb_subled_info);
+ rgb->mc.subled_info = devm_kmemdup(dev, s2mu005_rgb_subled_info,
+ sizeof(s2mu005_rgb_subled_info), GFP_KERNEL);
+ if (!rgb->mc.subled_info)
+ return -ENOMEM;
+
+ rgb->mc.led_cdev.max_brightness = 255;
+ rgb->mc.led_cdev.brightness_set_blocking = s2m_rgb_brightness_set;
+ rgb->mc.led_cdev.pattern_set = s2m_rgb_pattern_set;
+ rgb->mc.led_cdev.pattern_clear = s2m_rgb_pattern_clear;
+
+ ret = devm_mutex_init(dev, &rgb->lock);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to create mutex lock\n");
+
+ init_data.fwnode = of_fwnode_handle(dev->of_node);
+ ret = devm_led_classdev_multicolor_register_ext(dev, &rgb->mc, &init_data);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to create LED device\n");
+
+ return 0;
+}
+
+static const struct platform_device_id s2m_rgb_id_table[] = {
+ { "s2mu005-rgb", S2MU005 },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(platform, s2m_rgb_id_table);
+
+static const struct of_device_id s2m_rgb_of_match_table[] = {
+ { .compatible = "samsung,s2mu005-rgb", .data = (void *)S2MU005 },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, s2m_rgb_of_match_table);
+
+static struct platform_driver s2m_rgb_driver = {
+ .driver = {
+ .name = "s2m-rgb",
+ },
+ .probe = s2m_rgb_probe,
+ .id_table = s2m_rgb_id_table,
+};
+module_platform_driver(s2m_rgb_driver);
+
+MODULE_DESCRIPTION("RGB LED Driver for Samsung S2M Series PMICs");
+MODULE_AUTHOR("Kaustabh Chakraborty <kauschluss@disroot.org>");
+MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v7 07/10] leds: rgb: add support for Samsung S2M series PMIC RGB LED device
2026-05-15 21:38 ` [PATCH v7 07/10] leds: rgb: add support for Samsung S2M series PMIC RGB " Kaustabh Chakraborty
@ 2026-05-15 22:10 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-05-15 22:10 UTC (permalink / raw)
To: Kaustabh Chakraborty; +Cc: robh, devicetree, krzk+dt, conor+dt
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [High] The driver uses the `abs()` macro on unsigned integer subtractions, causing a build failure and integer underflow.
- [High] Unconditional dereference of `dev->parent` without validation causes a kernel panic if instantiated via a standalone DT node.
- [High] Setting a solid brightness does not clear previously configured hardware pattern timings, causing the LED to continue blinking.
- [High] Missing Kconfig dependency on `LEDS_CLASS_MULTICOLOR`.
- [Medium] The driver silently aggregates complex multi-pulse patterns and forcibly alters blink semantics instead of rejecting unsupported patterns.
- [Medium] Software and hardware pattern states become desynchronized on error paths.
- [Medium] The driver incorrectly uses the array loop index instead of the `channel` field for hardware register offsets.
- [Low] The `of_match_table` is exported but not assigned to the platform driver.
--
commit d2d65a1923271678db5e8b3090e9b23768e600c4
Author: Kaustabh Chakraborty <kauschluss@disroot.org>
leds: rgb: add support for Samsung S2M series PMIC RGB LED device
This commit adds support for the RGB LEDs found in Samsung S2M series PMICs,
starting with the S2MU005. It controls three LED channels as a single
device, providing 8-bit brightness control for each channel.
> diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig
> index 28ef4c487367c..b16144b48b8f8 100644
> --- a/drivers/leds/rgb/Kconfig
> +++ b/drivers/leds/rgb/Kconfig
> @@ -75,6 +75,16 @@ config LEDS_QCOM_LPG
>
> If compiled as a module, the module will be named leds-qcom-lpg.
>
> +config LEDS_S2M_RGB
> + tristate "Samsung S2M series PMICs RGB LED support"
> + depends on LEDS_CLASS
> + depends on MFD_SEC_CORE
Should this also depend on LEDS_CLASS_MULTICOLOR?
The driver uses multicolor LED APIs like led_mc_calc_color_components and
devm_led_classdev_multicolor_register_ext. Without a Kconfig dependency on
CONFIG_LEDS_CLASS_MULTICOLOR, building the kernel with the multicolor class
disabled could result in silent functional failures or link errors.
> diff --git a/drivers/leds/rgb/leds-s2m-rgb.c b/drivers/leds/rgb/leds-s2m-rgb.c
> new file mode 100644
> index 0000000000000..d239f54eee901
> --- /dev/null
> +++ b/drivers/leds/rgb/leds-s2m-rgb.c
[ ... ]
> +static int s2mu005_rgb_apply_params(struct s2m_rgb *rgb)
> +{
[ ... ]
> + for (int i = 0; i < rgb->mc.num_colors; i++) {
> + ret = regmap_write(regmap, S2MU005_REG_RGB_CH_CTRL(i),
> + rgb->mc.subled_info[i].brightness);
Is it safe to use the loop index i to calculate the hardware channel?
While i currently matches the channel sequentially, any reordering of the
subled_info array might cause the index to map to the wrong physical channel.
Would it be better to extract and use rgb->mc.subled_info[i].channel instead?
[ ... ]
> +static int s2m_rgb_lut_get_closest_duration(const u32 *lut, const size_t len, const u32 duration)
> +{
> + u32 closest_distance = abs(duration - lut[0]);
Can this cause a compile-time error and integer underflow?
Both duration and lut[0] are u32 variables, and subtracting a larger unsigned
integer from a smaller one will wrap around. Additionally, the abs macro in
linux/math.h is designed to reject unsigned types and evaluates to void.
Could abs_diff be used here instead?
[ ... ]
> +static int s2m_rgb_pattern_set(struct led_classdev *cdev, struct led_pattern *pattern, u32 len,
> + int repeat)
> +{
[ ... ]
> + for (int i = 0; i < len; i++) {
> + int brightness = pattern[i].brightness;
> + u32 delta_t = pattern[i].delta_t;
> +
> + if (brightness) {
> + /*
> + * The pattern should define only one non-zero
> + * brightness in the HIGH state. The device doesn't
> + * have any provisions to handle multiple peak
> + * brightness levels.
> + */
> + if (brightness_peak && brightness_peak != brightness)
> + return -EINVAL;
> +
> + brightness_peak = brightness;
> + time_hi += delta_t;
> + ramp_dn_en = !!delta_t;
> + } else {
> + time_lo += delta_t;
> + ramp_up_en = !!delta_t;
> + }
> + }
Instead of accumulating durations to force a breathing effect, should the
driver reject unsupported patterns by returning -EINVAL?
If a sequence with more than two states or sharp step transitions is requested,
rejecting it would allow the LED core to correctly fall back to
software-driven blinking instead of silently distorting the sequence.
[ ... ]
> +param_fail:
> + rgb->ramp_up = 0;
> + rgb->ramp_dn = 0;
> + rgb->stay_hi = 0;
> + rgb->stay_lo = 0;
> +
> + mutex_unlock(&rgb->lock);
> +
> + return ret;
> +}
Could this error path leave the software and hardware pattern states
desynchronized?
Zeroing out the timing state variables without clearing the hardware pattern
means the hardware continues blinking. A subsequent brightness update might
then unintentionally alter the running pattern by writing this zeroed state.
[ ... ]
> +static int s2m_rgb_brightness_set(struct led_classdev *cdev, enum led_brightness value)
> +{
> + struct s2m_rgb *rgb = to_s2m_rgb(to_s2m_mc(cdev));
> + int ret = 0;
> +
> + if (!value)
> + return s2m_rgb_pattern_clear(cdev);
> +
> + mutex_lock(&rgb->lock);
> +
> + led_mc_calc_color_components(&rgb->mc, value);
> + /* Apply params with variant-specific implementation. */
> + ret = s2mu005_rgb_apply_params(rgb);
Does this need to clear previously configured hardware pattern timings when
setting a solid brightness?
If a hardware blink pattern was previously active, the old non-zero timings
will be written back to the registers along with the new peak brightness,
causing the LED to continue blinking instead of emitting a solid color.
[ ... ]
> +static int s2m_rgb_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct sec_pmic_dev *pmic_drvdata = dev_get_drvdata(dev->parent);
What happens if this driver is instantiated via a standalone device tree
node rather than as an MFD child?
If dev->parent is NULL, this unconditional dereference will cause a kernel
panic.
Should dev->parent be validated before use?
[ ... ]
> +static struct platform_driver s2m_rgb_driver = {
> + .driver = {
> + .name = "s2m-rgb",
> + },
> + .probe = s2m_rgb_probe,
> + .id_table = s2m_rgb_id_table,
> +};
Is the of_match_table assignment missing from the driver struct?
s2m_rgb_of_match_table is declared earlier in the file to support explicit
device tree matching but is not assigned here, preventing direct Open Firmware
matching.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260516-s2mu005-pmic-v7-0-73f9702fb461@disroot.org?part=7
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v7 08/10] Documentation: leds: document pattern behavior of Samsung S2M series PMIC RGB LEDs
2026-05-15 21:38 [PATCH v7 00/10] Support for Samsung S2MU005 PMIC and its sub-devices Kaustabh Chakraborty
` (6 preceding siblings ...)
2026-05-15 21:38 ` [PATCH v7 07/10] leds: rgb: add support for Samsung S2M series PMIC RGB " Kaustabh Chakraborty
@ 2026-05-15 21:38 ` Kaustabh Chakraborty
2026-05-15 21:38 ` [PATCH v7 09/10] extcon: add support for Samsung S2M series PMIC extcon devices Kaustabh Chakraborty
2026-05-15 21:38 ` [PATCH v7 10/10] power: supply: add support for Samsung S2M series PMIC charger device Kaustabh Chakraborty
9 siblings, 0 replies; 20+ messages in thread
From: Kaustabh Chakraborty @ 2026-05-15 21:38 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, Yassine Oudjana
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc, Kaustabh Chakraborty
Add documentation to describe how hardware patterns (as defined by the
documentation of leds-trigger-pattern) 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 bebf440042787..23fa9ff7aaf4b 100644
--- a/Documentation/leds/index.rst
+++ b/Documentation/leds/index.rst
@@ -28,6 +28,7 @@ LEDs
leds-lp5812
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 0000000000000..4f89a8c89ea86
--- /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, 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.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v7 09/10] extcon: add support for Samsung S2M series PMIC extcon devices
2026-05-15 21:38 [PATCH v7 00/10] Support for Samsung S2MU005 PMIC and its sub-devices Kaustabh Chakraborty
` (7 preceding siblings ...)
2026-05-15 21:38 ` [PATCH v7 08/10] Documentation: leds: document pattern behavior of Samsung S2M series PMIC RGB LEDs Kaustabh Chakraborty
@ 2026-05-15 21:38 ` Kaustabh Chakraborty
2026-05-15 22:10 ` sashiko-bot
2026-05-15 21:38 ` [PATCH v7 10/10] power: supply: add support for Samsung S2M series PMIC charger device Kaustabh Chakraborty
9 siblings, 1 reply; 20+ messages in thread
From: Kaustabh Chakraborty @ 2026-05-15 21:38 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, Yassine Oudjana
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc, Kaustabh Chakraborty
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 | 9 ++
drivers/extcon/Makefile | 1 +
drivers/extcon/extcon-s2m.c | 370 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 380 insertions(+)
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index 68d9df7d2dae0..b052da947fc92 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -183,6 +183,15 @@ 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
+ 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..8450fdf2ff3ed
--- /dev/null
+++ b/drivers/extcon/extcon-s2m.c
@@ -0,0 +1,370 @@
+// 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>
+
+#include "extcon.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 *);
+ bool call_on_probe;
+ 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) {
+ 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) {
+ 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) {
+ 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) {
+ 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_CDP | 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) {
+ 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) {
+ 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) {
+ 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_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;
+ default:
+ dev_warn(priv->dev,
+ "failed to recognize the device attached, unknown or bad type\n");
+ }
+
+ return ret;
+}
+
+static int s2mu005_muic_init(struct s2m_muic *priv)
+{
+ int ret;
+
+ 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) {
+ 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) {
+ 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) {
+ dev_err(priv->dev, "failed to enable MUIC interrupts\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+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 const struct s2m_muic_irq_data s2mu005_muic_irq_data[] = {
+ {
+ .name = "attach",
+ .handler = s2mu005_muic_attach,
+ .call_on_probe = true,
+ }, {
+ .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)
+ 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 = devm_kmemdup(dev, s2mu005_muic_irq_data,
+ sizeof(s2mu005_muic_irq_data),
+ GFP_KERNEL);
+ /* Initialize MUIC */
+ ret = s2mu005_muic_init(priv);
+ break;
+ default:
+ return dev_err_probe(dev, -ENODEV, "device type not supported by driver\n");
+ }
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to initialize MUIC\n");
+
+ if (!priv->irq_data)
+ return -ENOMEM;
+
+ 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++) {
+ ret = platform_get_irq_byname_optional(pdev, priv->irq_data[i].name);
+ if (ret == -ENXIO)
+ continue;
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "failed to get IRQ %s\n",
+ priv->irq_data[i].name);
+
+ priv->irq_data[i].irq = ret;
+ ret = devm_request_threaded_irq(dev, priv->irq_data[i].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");
+
+ if (priv->irq_data[i].call_on_probe)
+ priv->irq_data[i].handler(priv);
+ }
+
+ return 0;
+}
+
+static void s2m_muic_remove(struct platform_device *pdev)
+{
+ struct s2m_muic *priv = dev_get_drvdata(&pdev->dev);
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->extcon->lock, flags);
+
+ /*
+ * 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;
+ default:
+ WARN_ON_ONCE("execution shouldn't have reached here!");
+ }
+
+ spin_unlock_irqrestore(&priv->extcon->lock, flags);
+}
+
+static const struct platform_device_id s2m_muic_id_table[] = {
+ { "s2mu005-muic", S2MU005 },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(platform, s2m_muic_id_table);
+
+/*
+ * Device is instantiated through parent MFD device and device matching
+ * is done through platform_device_id.
+ *
+ * However if device's DT node contains proper clock 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);
+
+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 [flat|nested] 20+ messages in thread* Re: [PATCH v7 09/10] extcon: add support for Samsung S2M series PMIC extcon devices
2026-05-15 21:38 ` [PATCH v7 09/10] extcon: add support for Samsung S2M series PMIC extcon devices Kaustabh Chakraborty
@ 2026-05-15 22:10 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-05-15 22:10 UTC (permalink / raw)
To: Kaustabh Chakraborty; +Cc: krzk+dt, robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [Critical] Self-deadlock in `s2m_muic_remove()` due to recursive acquisition of the internal extcon spinlock.
- [High] Sleeping I2C regmap accesses are performed in atomic context while holding a spinlock.
- [High] Missing mutual exclusion allows unprotected concurrent execution of attach/detach handlers, leading to state corruption.
- [Medium] Unhandled 'no device attached' state causes a spurious warning on boot.
- [Medium] The driver uses a fragile `switch` statement on a register read that is explicitly treated as a bitmask.
- [Low] Misuse of `WARN_ON_ONCE` by passing a string literal instead of a logical condition.
- [Low] A failed memory allocation error is masked if the subsequent initialization function fails, and initialization is performed unnecessarily.
--
commit fa85a485ca17ccd71c4ade607d5194878ea06e29
Author: Kaustabh Chakraborty <kauschluss@disroot.org>
extcon: add support for Samsung S2M series PMIC extcon devices
This commit adds a driver for MUIC devices found in certain Samsung S2M
series PMICs. These devices report multiple cable states depending on the
ID-GND resistance measured by an internal ADC.
> diff --git a/drivers/extcon/extcon-s2m.c b/drivers/extcon/extcon-s2m.c
> new file mode 100644
> index 0000000000000..8450fdf2ff3ed
> --- /dev/null
> +++ b/drivers/extcon/extcon-s2m.c
[ ... ]
> +static int s2mu005_muic_attach(struct s2m_muic *priv)
> +{
[ ... ]
> + ret = regmap_read(priv->regmap, S2MU005_REG_MUIC_DEV1, &type);
[ ... ]
> + if (type & (S2MU005_MUIC_OTG | S2MU005_MUIC_CDP | 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));
[ ... ]
> + 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;
[ ... ]
> + default:
> + dev_warn(priv->dev,
> + "failed to recognize the device attached, unknown or bad type\n");
> + }
> +
> + return ret;
> +}
Could this switch fail to match if multiple bits are set simultaneously,
given that the earlier code treats type as a bitmask? If multiple flags
can be set, independent bitwise checks might be safer.
Also, since call_on_probe is true for the attach handler, if no cable
is attached at boot, won't type be 0? Does this cause the driver to emit
this warning on boot when no device is attached?
[ ... ]
> +static int s2m_muic_probe(struct platform_device *pdev)
> +{
[ ... ]
> + switch (platform_get_device_id(pdev)->driver_data) {
> + case S2MU005:
> + priv->extcon_cable = s2mu005_muic_extcon_cable;
> + priv->irq_data = devm_kmemdup(dev, s2mu005_muic_irq_data,
> + sizeof(s2mu005_muic_irq_data),
> + GFP_KERNEL);
> + /* Initialize MUIC */
> + ret = s2mu005_muic_init(priv);
> + break;
> + default:
> + return dev_err_probe(dev, -ENODEV, "device type not supported by driver\n");
> + }
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to initialize MUIC\n");
> +
> + if (!priv->irq_data)
> + return -ENOMEM;
If devm_kmemdup() fails but s2mu005_muic_init() succeeds, does this
mask the -ENOMEM error? It also seems to perform hardware I2C initialization
even if the core data structure allocation failed. Could the allocation
check be moved immediately after the allocation?
[ ... ]
> + for (i = 0; priv->irq_data[i].handler; i++) {
> + ret = platform_get_irq_byname_optional(pdev, priv->irq_data[i].name);
> + if (ret == -ENXIO)
> + continue;
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to get IRQ %s\n",
> + priv->irq_data[i].name);
> +
> + priv->irq_data[i].irq = ret;
> + ret = devm_request_threaded_irq(dev, priv->irq_data[i].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");
> +
> + if (priv->irq_data[i].call_on_probe)
> + priv->irq_data[i].handler(priv);
> + }
> +
> + return 0;
> +}
Is there a possibility of concurrent execution here? The attach handler is
invoked manually after devm_request_threaded_irq() has already enabled the
interrupt.
If the IRQ fires immediately, the probe thread and the IRQ thread could
execute s2mu005_muic_attach() simultaneously. Is there a need for
driver-level synchronization (like a mutex) to protect the priv->attached
flag and the regmap configuration sequence?
[ ... ]
> +static void s2m_muic_remove(struct platform_device *pdev)
> +{
> + struct s2m_muic *priv = dev_get_drvdata(&pdev->dev);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&priv->extcon->lock, flags);
> +
> + /*
> + * 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);
Will this cause a self-deadlock when the driver is unbound?
s2mu005_muic_detach() calls extcon_get_state() and extcon_set_state_sync():
s2mu005_muic_detach()
extcon_get_state()
spin_lock_irqsave(&edev->lock, flags);
Since s2m_muic_remove() already acquired the exact same priv->extcon->lock
before calling s2mu005_muic_detach(), and Linux spinlocks are not
recursive, won't this hang the kernel?
Additionally, is it safe to perform I2C operations in this context?
s2m_muic_remove() acquires a spinlock with IRQs disabled. While holding
this lock, s2mu005_muic_detach() performs multiple regmap_set_bits()
operations. Since the PMIC is an I2C device, these operations can sleep,
which violates atomic context rules.
Could this be avoided by using a driver-level mutex instead of bypassing the
extcon API to access the internal lock?
> + break;
> + default:
> + WARN_ON_ONCE("execution shouldn't have reached here!");
This isn't a bug, but WARN_ON_ONCE expects a boolean-like condition, not
a string literal. While the string evaluates to true, it causes a
pointer-to-integer conversion warning in static analysis, and the string
itself isn't formatted into the output message. Should this be
WARN_ONCE(1, ...)?
> + }
> +
> + spin_unlock_irqrestore(&priv->extcon->lock, flags);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260516-s2mu005-pmic-v7-0-73f9702fb461@disroot.org?part=9
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v7 10/10] power: supply: add support for Samsung S2M series PMIC charger device
2026-05-15 21:38 [PATCH v7 00/10] Support for Samsung S2MU005 PMIC and its sub-devices Kaustabh Chakraborty
` (8 preceding siblings ...)
2026-05-15 21:38 ` [PATCH v7 09/10] extcon: add support for Samsung S2M series PMIC extcon devices Kaustabh Chakraborty
@ 2026-05-15 21:38 ` Kaustabh Chakraborty
2026-05-15 22:22 ` sashiko-bot
9 siblings, 1 reply; 20+ messages in thread
From: Kaustabh Chakraborty @ 2026-05-15 21:38 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, Yassine Oudjana
Cc: linux-leds, devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-rtc, linux-doc, Kaustabh Chakraborty
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 | 10 ++
drivers/power/supply/Makefile | 1 +
drivers/power/supply/s2m-charger.c | 313 +++++++++++++++++++++++++++++++++++++
3 files changed, 324 insertions(+)
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 83392ed6a8da9..899c929f31620 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -856,6 +856,16 @@ 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
+ 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..4d1f2c2c71446
--- /dev/null
+++ b/drivers/power/supply/s2m-charger.c
@@ -0,0 +1,313 @@
+// 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;
+
+ ret = regmap_read(priv->regmap, S2MU005_REG_CHGR_STATUS0, &val);
+ if (ret) {
+ dev_err(priv->dev, "failed to read register (%d)\n", ret);
+ return ret;
+ }
+
+ *value = !!(val & S2MU005_CHGR_CHG);
+
+ return 0;
+}
+
+static void s2mu005_chgr_get_usb_type(struct s2m_chgr *priv, int *value)
+{
+ if (extcon_get_state(priv->extcon, EXTCON_CHG_USB_CDP) > 0)
+ *value = POWER_SUPPLY_USB_TYPE_CDP;
+ else if (extcon_get_state(priv->extcon, EXTCON_CHG_USB_SDP) > 0)
+ *value = POWER_SUPPLY_USB_TYPE_SDP;
+ else if (extcon_get_state(priv->extcon, EXTCON_CHG_USB_DCP) > 0)
+ *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;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_ONLINE:
+ ret = s2mu005_chgr_get_online(priv, &val->intval);
+ if (ret)
+ return ret;
+ break;
+ case POWER_SUPPLY_PROP_USB_TYPE:
+ s2mu005_chgr_get_usb_type(priv, &val->intval);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+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) {
+ 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) {
+ 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) {
+ 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) {
+ 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) {
+ 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) {
+ dev_err(priv->dev, "failed to turn on OTG (%d)\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+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) {
+ 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) {
+ dev_err(priv->dev, "failed to set opmode to charging (%d)\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+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) {
+ 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) {
+ dev_err(priv->dev, "failed to reset opmode (%d)\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+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) > 0)
+ s2mu005_chgr_mode_set_host(priv);
+ else if (extcon_get_state(priv->extcon, EXTCON_USB) > 0)
+ 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);
+ }
+
+ /* MUIC is mandatory. If unavailable, request probe deferral */
+ extcon_node = of_get_child_by_name(dev->parent->of_node, "muic");
+ if (!extcon_node)
+ return dev_err_probe(dev, -ENODEV, "MUIC node required but not found\n");
+
+ priv->extcon = extcon_find_edev_by_node(extcon_node);
+ if (IS_ERR(priv->extcon))
+ return -EPROBE_DEFER;
+
+ 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");
+
+ 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)
+ return 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 [flat|nested] 20+ messages in thread* Re: [PATCH v7 10/10] power: supply: add support for Samsung S2M series PMIC charger device
2026-05-15 21:38 ` [PATCH v7 10/10] power: supply: add support for Samsung S2M series PMIC charger device Kaustabh Chakraborty
@ 2026-05-15 22:22 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-05-15 22:22 UTC (permalink / raw)
To: Kaustabh Chakraborty; +Cc: devicetree, robh, krzk+dt, conor+dt
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Missing initial configuration of hardware state on probe.
- [High] Race condition between devres cleanup and extcon notifier leads to use-after-free.
- [High] Missing hardware disablement upon driver unbind leaves PMIC sourcing power.
- [High] Dangling pointer to `extcon_dev` if the provider unbinds, leading to use-after-free.
- [Medium] Unconditional charger reset on every extcon notification disrupts charging.
- [Low] Missing explicit header includes for `msleep()` and `FIELD_PREP()`.
--
commit 41ed08773e6f6eb12cab89eb389202b30e4d9e42
Author: Kaustabh Chakraborty <kauschluss@disroot.org>
power: supply: add support for Samsung S2M series PMIC charger device
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.
> diff --git a/drivers/power/supply/s2m-charger.c b/drivers/power/supply/s2m-charger.c
> new file mode 100644
> index 0000000000000..4d1f2c2c71446
> --- /dev/null
> +++ b/drivers/power/supply/s2m-charger.c
> @@ -0,0 +1,313 @@
> +// 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>
This isn't a bug, but is it possible to add explicit includes for
<linux/delay.h> (for msleep) and <linux/bitfield.h> (for FIELD_PREP)? It
appears the driver relies on implicit transitive inclusions.
[ ... ]
> +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) > 0)
> + s2mu005_chgr_mode_set_host(priv);
> + else if (extcon_get_state(priv->extcon, EXTCON_USB) > 0)
> + s2mu005_chgr_mode_set_charger(priv);
Since the notifier is registered for all cables, a single physical USB
connection can generate multiple extcon notifications (e.g., EXTCON_USB
followed by EXTCON_CHG_USB_SDP after negotiation).
Does this unconditionally clear the OP_MODE register and interrupt charging
on every notification? It seems the driver should verify the current hardware
state and only reset if a state change is actually required.
[ ... ]
> +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;
> +}
If an extcon event fires concurrently during driver unbind, can the notifier
callback invoke schedule_work() after cancel_work_sync() has already returned?
During devres cleanup, devm_extcon_unregister_notifier_all() runs first,
followed by cancel_work_sync() from devm_work_autocancel(). Extcon uses a
raw_notifier_chain, so unregistering the notifier unlinks it but does not wait
for currently executing callbacks to finish.
Could this queue a freed work_struct into the system workqueue, resulting in a
use-after-free when the work executes?
[ ... ]
> + /* MUIC is mandatory. If unavailable, request probe deferral */
> + extcon_node = of_get_child_by_name(dev->parent->of_node, "muic");
> + if (!extcon_node)
> + return dev_err_probe(dev, -ENODEV, "MUIC node required but not found\n");
> +
> + priv->extcon = extcon_find_edev_by_node(extcon_node);
> + if (IS_ERR(priv->extcon))
> + return -EPROBE_DEFER;
If the extcon provider (e.g., the MUIC driver) is manually unbound via sysfs,
the extcon_dev structure is freed. Does the charger driver retain a dangling
pointer in priv->extcon?
Because extcon_find_edev_by_node() does not increment the reference count or
establish a device link, any subsequent power supply property query or delayed
execution of s2mu005_chgr_extcon_work() will dereference this freed pointer
when calling extcon_get_state(), causing a use-after-free.
[ ... ]
> + priv->extcon_nb.notifier_call = s2m_chgr_extcon_notifier;
> + ret = devm_extcon_register_notifier_all(dev, priv->extcon, &priv->extcon_nb);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to register extcon notifier\n");
> +
> + return 0;
> +}
Extcon notifiers only fire on state changes. If a USB cable is already
connected when the device boots and the driver loads, wouldn't the hardware
remain unconfigured in its default mode and not begin charging?
Should the driver explicitly schedule priv->extcon_work or manually synchronize
the initial state at the end of s2m_chgr_probe()?
Also, if the driver is unbound via sysfs while operating in OTG mode (sourcing
5V on the VBUS line), is the hardware left indefinitely sourcing power with no
software managing it?
There doesn't appear to be a .remove callback or a dedicated devm teardown
action to transition the hardware to a safe state (e.g., calling
s2mu005_chgr_mode_unset()) during teardown.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260516-s2mu005-pmic-v7-0-73f9702fb461@disroot.org?part=10
^ permalink raw reply [flat|nested] 20+ messages in thread