[v2] Support for Samsung S2MU005 PMIC and its sub-devices

[PATCH v2 07/12] mfd: sec: store hardware revision in sec_pmic_dev and add S2MU005 support

Posted by Kaustabh Chakraborty 2 weeks ago

The device revision matters in cases when in some PMICs, the correct
register offsets very in different revisions. Instead of just debug
printing the value, store it in the driver data struct.

Unlike other devices, S2MU005 has its hardware revision ID in register
offset 0x73. Allow handling different devices and add support for S2MU005.

Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
 drivers/mfd/sec-common.c         | 41 ++++++++++++++++++++++++++++++----------
 include/linux/mfd/samsung/core.h |  1 +
 2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
index bc2a1f2c6dc7a..069a1ba9aa1f1 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>
@@ -111,17 +112,38 @@ static const struct mfd_cell s2mu005_devs[] = {
 	MFD_CELL_OF("s2mu005-rgb", NULL, NULL, 0, 0, "samsung,s2mu005-rgb"),
 };
 
-static void sec_pmic_dump_rev(struct sec_pmic_dev *sec_pmic)
+static int sec_pmic_store_rev(struct sec_pmic_dev *sec_pmic)
 {
-	unsigned int val;
+	unsigned int reg, mask, shift;
+	int ret;
 
-	/* For s2mpg1x, the revision is in a different regmap */
-	if (sec_pmic->device_type == S2MPG10)
-		return;
+	switch (sec_pmic->device_type) {
+	case S2MPG10:
+		/* For s2mpg1x, the revision is in a different regmap */
+		return 0;
+	case S2MU005:
+		reg = S2MU005_REG_ID;
+		mask = S2MU005_ID_MASK;
+		shift = S2MU005_ID_SHIFT;
+		break;
+	default:
+		/* For other device types, the REG_ID is always the first register. */
+		reg = S2MPS11_REG_ID;
+		mask = ~0;
+		shift = 0;
+	}
+
+	ret = regmap_read(sec_pmic->regmap_pmic, reg, &sec_pmic->revision);
+	if (ret) {
+		dev_err(sec_pmic->dev, "Failed to read PMIC revision (%d)\n", ret);
+		return ret;
+	}
+
+	sec_pmic->revision &= mask;
+	sec_pmic->revision >>= shift;
 
-	/* 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);
+	dev_dbg(sec_pmic->dev, "Revision: 0x%x\n", sec_pmic->revision);
+	return 0;
 }
 
 static void sec_pmic_configure(struct sec_pmic_dev *sec_pmic)
@@ -262,9 +284,8 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
 		return ret;
 
 	sec_pmic_configure(sec_pmic);
-	sec_pmic_dump_rev(sec_pmic);
 
-	return ret;
+	return sec_pmic_store_rev(sec_pmic);
 }
 EXPORT_SYMBOL_GPL(sec_pmic_probe);
 
diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
index 43e0c5e55f5d3..56aa33d7e3d60 100644
--- a/include/linux/mfd/samsung/core.h
+++ b/include/linux/mfd/samsung/core.h
@@ -70,6 +70,7 @@ struct sec_pmic_dev {
 
 	int device_type;
 	int irq;
+	unsigned int revision;
 };
 
 struct sec_platform_data {

-- 
2.52.0

Re: [PATCH v2 07/12] mfd: sec: store hardware revision in sec_pmic_dev and add S2MU005 support

Posted by André Draszik 4 days, 19 hours ago

Hi Kaustabh,

On Mon, 2026-01-26 at 00:37 +0530, Kaustabh Chakraborty wrote:
> The device revision matters in cases when in some PMICs, the correct
> register offsets very in different revisions. Instead of just debug

s/very/vary

> printing the value, store it in the driver data struct.

Please mention that you're not doing that for s2mpg1x, though.

> 
> Unlike other devices, S2MU005 has its hardware revision ID in register
> offset 0x73. Allow handling different devices and add support for S2MU005.
> 
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
>  drivers/mfd/sec-common.c         | 41 ++++++++++++++++++++++++++++++----------
>  include/linux/mfd/samsung/core.h |  1 +
>  2 files changed, 32 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
> index bc2a1f2c6dc7a..069a1ba9aa1f1 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>
> @@ -111,17 +112,38 @@ static const struct mfd_cell s2mu005_devs[] = {
>  	MFD_CELL_OF("s2mu005-rgb", NULL, NULL, 0, 0, "samsung,s2mu005-rgb"),
>  };
>  
> -static void sec_pmic_dump_rev(struct sec_pmic_dev *sec_pmic)
> +static int sec_pmic_store_rev(struct sec_pmic_dev *sec_pmic)
>  {
> -	unsigned int val;
> +	unsigned int reg, mask, shift;
> +	int ret;
>  
> -	/* For s2mpg1x, the revision is in a different regmap */
> -	if (sec_pmic->device_type == S2MPG10)
> -		return;
> +	switch (sec_pmic->device_type) {
> +	case S2MPG10:
> +		/* For s2mpg1x, the revision is in a different regmap */
> +		return 0;
> +	case S2MU005:
> +		reg = S2MU005_REG_ID;
> +		mask = S2MU005_ID_MASK;
> +		shift = S2MU005_ID_SHIFT;
> +		break;
> +	default:
> +		/* For other device types, the REG_ID is always the first register. */
> +		reg = S2MPS11_REG_ID;
> +		mask = ~0;
> +		shift = 0;
> +	}
> +
> +	ret = regmap_read(sec_pmic->regmap_pmic, reg, &sec_pmic->revision);
> +	if (ret) {
> +		dev_err(sec_pmic->dev, "Failed to read PMIC revision (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	sec_pmic->revision &= mask;
> +	sec_pmic->revision >>= shift;
>  
> -	/* 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);
> +	dev_dbg(sec_pmic->dev, "Revision: 0x%x\n", sec_pmic->revision);
> +	return 0;
>  }
>  
>  static void sec_pmic_configure(struct sec_pmic_dev *sec_pmic)
> @@ -262,9 +284,8 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
>  		return ret;
>  
>  	sec_pmic_configure(sec_pmic);
> -	sec_pmic_dump_rev(sec_pmic);
>  
> -	return ret;
> +	return sec_pmic_store_rev(sec_pmic);
>  }
>  EXPORT_SYMBOL_GPL(sec_pmic_probe);
>  
> diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
> index 43e0c5e55f5d3..56aa33d7e3d60 100644
> --- a/include/linux/mfd/samsung/core.h
> +++ b/include/linux/mfd/samsung/core.h
> @@ -70,6 +70,7 @@ struct sec_pmic_dev {
>  
>  	int device_type;
>  	int irq;
> +	unsigned int revision;

kerneldoc needs to be updated.

Given the LED driver is the only driver & device so far which needs the
PMIC revision, maybe for now that driver could determine the revision
itself instead of adding this new member for everybody?

Cheers,
Andre'

>  };
>  
>  struct sec_platform_data {

Re: [PATCH v2 07/12] mfd: sec: store hardware revision in sec_pmic_dev and add S2MU005 support

Posted by Kaustabh Chakraborty 4 days, 18 hours ago

On 2026-02-04 14:17 +00:00, André Draszik wrote:
> Hi Kaustabh,
>
> On Mon, 2026-01-26 at 00:37 +0530, Kaustabh Chakraborty wrote:
>> The device revision matters in cases when in some PMICs, the correct
>> register offsets very in different revisions. Instead of just debug
>
> s/very/vary
>
>> printing the value, store it in the driver data struct.
>
> Please mention that you're not doing that for s2mpg1x, though.
>
>> 
>> Unlike other devices, S2MU005 has its hardware revision ID in register
>> offset 0x73. Allow handling different devices and add support for S2MU005.
>> 
>> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
>> ---
>>  drivers/mfd/sec-common.c         | 41 ++++++++++++++++++++++++++++++----------
>>  include/linux/mfd/samsung/core.h |  1 +
>>  2 files changed, 32 insertions(+), 10 deletions(-)
>> 
>> diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
>> index bc2a1f2c6dc7a..069a1ba9aa1f1 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>
>> @@ -111,17 +112,38 @@ static const struct mfd_cell s2mu005_devs[] = {
>>  	MFD_CELL_OF("s2mu005-rgb", NULL, NULL, 0, 0, "samsung,s2mu005-rgb"),
>>  };
>>  
>> -static void sec_pmic_dump_rev(struct sec_pmic_dev *sec_pmic)
>> +static int sec_pmic_store_rev(struct sec_pmic_dev *sec_pmic)
>>  {
>> -	unsigned int val;
>> +	unsigned int reg, mask, shift;
>> +	int ret;
>>  
>> -	/* For s2mpg1x, the revision is in a different regmap */
>> -	if (sec_pmic->device_type == S2MPG10)
>> -		return;
>> +	switch (sec_pmic->device_type) {
>> +	case S2MPG10:
>> +		/* For s2mpg1x, the revision is in a different regmap */
>> +		return 0;
>> +	case S2MU005:
>> +		reg = S2MU005_REG_ID;
>> +		mask = S2MU005_ID_MASK;
>> +		shift = S2MU005_ID_SHIFT;
>> +		break;
>> +	default:
>> +		/* For other device types, the REG_ID is always the first register. */
>> +		reg = S2MPS11_REG_ID;
>> +		mask = ~0;
>> +		shift = 0;
>> +	}
>> +
>> +	ret = regmap_read(sec_pmic->regmap_pmic, reg, &sec_pmic->revision);
>> +	if (ret) {
>> +		dev_err(sec_pmic->dev, "Failed to read PMIC revision (%d)\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	sec_pmic->revision &= mask;
>> +	sec_pmic->revision >>= shift;
>>  
>> -	/* 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);
>> +	dev_dbg(sec_pmic->dev, "Revision: 0x%x\n", sec_pmic->revision);
>> +	return 0;
>>  }
>>  
>>  static void sec_pmic_configure(struct sec_pmic_dev *sec_pmic)
>> @@ -262,9 +284,8 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
>>  		return ret;
>>  
>>  	sec_pmic_configure(sec_pmic);
>> -	sec_pmic_dump_rev(sec_pmic);
>>  
>> -	return ret;
>> +	return sec_pmic_store_rev(sec_pmic);
>>  }
>>  EXPORT_SYMBOL_GPL(sec_pmic_probe);
>>  
>> diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
>> index 43e0c5e55f5d3..56aa33d7e3d60 100644
>> --- a/include/linux/mfd/samsung/core.h
>> +++ b/include/linux/mfd/samsung/core.h
>> @@ -70,6 +70,7 @@ struct sec_pmic_dev {
>>  
>>  	int device_type;
>>  	int irq;
>> +	unsigned int revision;
>
> kerneldoc needs to be updated.

Seems like it needs cleanup anyway, I will send a patch for that
separately (if this patch gets dropped in the next rev, see below).

>
> Given the LED driver is the only driver & device so far which needs the
> PMIC revision, maybe for now that driver could determine the revision
> itself instead of adding this new member for everybody?

Hmm, implementing that would make this patch redundant. I'll do so.

>
> Cheers,
> Andre'
>
>>  };
>>  
>>  struct sec_platform_data {

Re: [PATCH v2 07/12] mfd: sec: store hardware revision in sec_pmic_dev and add S2MU005 support

Posted by Kaustabh Chakraborty 3 days, 17 hours ago

On 2026-02-04 20:35 +05:30, Kaustabh Chakraborty wrote:
> On 2026-02-04 14:17 +00:00, André Draszik wrote:
>> Hi Kaustabh,
>>
>> On Mon, 2026-01-26 at 00:37 +0530, Kaustabh Chakraborty wrote:
>>> The device revision matters in cases when in some PMICs, the correct
>>> register offsets very in different revisions. Instead of just debug
>>
>> s/very/vary
>>
>>> printing the value, store it in the driver data struct.
>>
>> Please mention that you're not doing that for s2mpg1x, though.
>>
>>> 
>>> Unlike other devices, S2MU005 has its hardware revision ID in register
>>> offset 0x73. Allow handling different devices and add support for S2MU005.
>>> 
>>> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
>>> ---
>>>  drivers/mfd/sec-common.c         | 41 ++++++++++++++++++++++++++++++----------
>>>  include/linux/mfd/samsung/core.h |  1 +
>>>  2 files changed, 32 insertions(+), 10 deletions(-)
>>> 
>>> diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
>>> index bc2a1f2c6dc7a..069a1ba9aa1f1 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>
>>> @@ -111,17 +112,38 @@ static const struct mfd_cell s2mu005_devs[] = {
>>>  	MFD_CELL_OF("s2mu005-rgb", NULL, NULL, 0, 0, "samsung,s2mu005-rgb"),
>>>  };
>>>  
>>> -static void sec_pmic_dump_rev(struct sec_pmic_dev *sec_pmic)
>>> +static int sec_pmic_store_rev(struct sec_pmic_dev *sec_pmic)
>>>  {
>>> -	unsigned int val;
>>> +	unsigned int reg, mask, shift;
>>> +	int ret;
>>>  
>>> -	/* For s2mpg1x, the revision is in a different regmap */
>>> -	if (sec_pmic->device_type == S2MPG10)
>>> -		return;
>>> +	switch (sec_pmic->device_type) {
>>> +	case S2MPG10:
>>> +		/* For s2mpg1x, the revision is in a different regmap */
>>> +		return 0;
>>> +	case S2MU005:
>>> +		reg = S2MU005_REG_ID;
>>> +		mask = S2MU005_ID_MASK;
>>> +		shift = S2MU005_ID_SHIFT;
>>> +		break;
>>> +	default:
>>> +		/* For other device types, the REG_ID is always the first register. */
>>> +		reg = S2MPS11_REG_ID;
>>> +		mask = ~0;
>>> +		shift = 0;
>>> +	}
>>> +
>>> +	ret = regmap_read(sec_pmic->regmap_pmic, reg, &sec_pmic->revision);
>>> +	if (ret) {
>>> +		dev_err(sec_pmic->dev, "Failed to read PMIC revision (%d)\n", ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	sec_pmic->revision &= mask;
>>> +	sec_pmic->revision >>= shift;
>>>  
>>> -	/* 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);
>>> +	dev_dbg(sec_pmic->dev, "Revision: 0x%x\n", sec_pmic->revision);
>>> +	return 0;
>>>  }
>>>  
>>>  static void sec_pmic_configure(struct sec_pmic_dev *sec_pmic)
>>> @@ -262,9 +284,8 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
>>>  		return ret;
>>>  
>>>  	sec_pmic_configure(sec_pmic);
>>> -	sec_pmic_dump_rev(sec_pmic);
>>>  
>>> -	return ret;
>>> +	return sec_pmic_store_rev(sec_pmic);
>>>  }
>>>  EXPORT_SYMBOL_GPL(sec_pmic_probe);
>>>  
>>> diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
>>> index 43e0c5e55f5d3..56aa33d7e3d60 100644
>>> --- a/include/linux/mfd/samsung/core.h
>>> +++ b/include/linux/mfd/samsung/core.h
>>> @@ -70,6 +70,7 @@ struct sec_pmic_dev {
>>>  
>>>  	int device_type;
>>>  	int irq;
>>> +	unsigned int revision;
>>
>> kerneldoc needs to be updated.
>
> Seems like it needs cleanup anyway, I will send a patch for that
> separately (if this patch gets dropped in the next rev, see below).
>
>>
>> Given the LED driver is the only driver & device so far which needs the
>> PMIC revision, maybe for now that driver could determine the revision
>> itself instead of adding this new member for everybody?
>
> Hmm, implementing that would make this patch redundant. I'll do so.

It however seems weird to not handle it here; we're already reading the
revision value; it also makes sense to store it in the MFD driver
itself.

Either way, the patch won't be redundant, because the reading part at
least needs to be implemented here.

>
>>
>> Cheers,
>> Andre'
>>
>>>  };
>>>  
>>>  struct sec_platform_data {