[PATCH 0/2] regulator: s2mps11: two small bug fixes (s2mpg1x)

[PATCH 0/2] regulator: s2mps11: two small bug fixes (s2mpg1x)

[André Draszik]

These two patches fix some code quality / legibility issues in the
newly added s2mpg1x support. Behaviour should not change.

Cheers,
Andre'

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
André Draszik (2):
      regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()
      regulator: s2mps11: fix pctrlsel macro usage in s2mpg10_of_parse_cb()

 drivers/regulator/s2mps11.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)
---
base-commit: 9845cf73f7db6094c0d8419d6adb848028f4a921
change-id: 20260209-s2mpg1x-regulators-fixes-2087fafa2389

Best regards,
-- 
André Draszik <andre.draszik@linaro.org>

[PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()

[André Draszik]

The sanity checks being removed in this commit are useless as earlier
code checks for out-of-bounds conditions already. They also are
incorrect (as they're off-by-one).

Simply remove this incorrect code.

No functional change.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/aYmsu8qREppwBESH@stanley.mountain/
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/regulator/s2mps11.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index 2d5510acd0780ab6f9296c48ddcde5efe15ff488..2d67c5c16f487506a2e9e4b119f33faa846269f7 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -478,8 +478,6 @@ static int s2mpg10_of_parse_cb(struct device_node *np,
 			return -EINVAL;
 		}
 
-		if (ext_control > ARRAY_SIZE(ext_control_s2mpg10))
-			return -EINVAL;
 		ext_control = ext_control_s2mpg10[ext_control];
 		break;
 
@@ -503,8 +501,6 @@ static int s2mpg10_of_parse_cb(struct device_node *np,
 			return -EINVAL;
 		}
 
-		if (ext_control > ARRAY_SIZE(ext_control_s2mpg11))
-			return -EINVAL;
 		ext_control = ext_control_s2mpg11[ext_control];
 		break;
 

-- 
2.53.0.rc2.204.g2597b5adb4-goog

Re: [PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()

[Krzysztof Kozlowski]

On 09/02/2026 16:07, André Draszik wrote:
> The sanity checks being removed in this commit are useless as earlier
> code checks for out-of-bounds conditions already. They also are
> incorrect (as they're off-by-one).
> 
> Simply remove this incorrect code.
> 
> No functional change.

If they are incorrect then how it could be "no functional change"? To me
original code looks buggy and this is a fix. Fix must have functional
change...

> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/all/aYmsu8qREppwBESH@stanley.mountain/
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---
Best regards,
Krzysztof

Re: [PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()

[André Draszik]

Hi Krzysztof,

On Mon, 2026-02-09 at 17:09 +0100, Krzysztof Kozlowski wrote:
> On 09/02/2026 16:07, André Draszik wrote:
> > The sanity checks being removed in this commit are useless as earlier
> > code checks for out-of-bounds conditions already. They also are
> > incorrect (as they're off-by-one).
> > 
> > Simply remove this incorrect code.
> > 
> > No functional change.
> 
> If they are incorrect then how it could be "no functional change"? To me
> original code looks buggy and this is a fix. Fix must have functional
> change...

Earlier code already checks for all conditions, including all error cases.
So the code being removed here has no effect, as any potential error it
could catch will already have been caught by earlier code. Removing it
therefore doesn't change behaviour or functionality.

I can reword to 'incomplete test' instead of 'incorrect code' if you think
that's more clear?

Cheers,
Andre'

Re: [PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()

[Krzysztof Kozlowski]

On 10/02/2026 06:59, André Draszik wrote:
> Hi Krzysztof,
> 
> On Mon, 2026-02-09 at 17:09 +0100, Krzysztof Kozlowski wrote:
>> On 09/02/2026 16:07, André Draszik wrote:
>>> The sanity checks being removed in this commit are useless as earlier
>>> code checks for out-of-bounds conditions already. They also are
>>> incorrect (as they're off-by-one).
>>>
>>> Simply remove this incorrect code.
>>>
>>> No functional change.
>>
>> If they are incorrect then how it could be "no functional change"? To me
>> original code looks buggy and this is a fix. Fix must have functional
>> change...
> 
> Earlier code already checks for all conditions, including all error cases.
> So the code being removed here has no effect, as any potential error it
> could catch will already have been caught by earlier code. Removing it
> therefore doesn't change behaviour or functionality.
> 
> I can reword to 'incomplete test' instead of 'incorrect code' if you think
> that's more clear?

Perhaps you should mention which "one" in off-by-one that it has no impact.

I also wonder why you left the second - ext_control_s2mpg11 - untouched.

Best regards,
Krzysztof

Re: [PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()

[André Draszik]

Hi Krzysztof,

On Tue, 2026-02-10 at 08:28 +0100, Krzysztof Kozlowski wrote:
> On 10/02/2026 06:59, André Draszik wrote:
> > Hi Krzysztof,
> > 
> > On Mon, 2026-02-09 at 17:09 +0100, Krzysztof Kozlowski wrote:
> > > On 09/02/2026 16:07, André Draszik wrote:
> > > > The sanity checks being removed in this commit are useless as earlier
> > > > code checks for out-of-bounds conditions already. They also are
> > > > incorrect (as they're off-by-one).
> > > > 
> > > > Simply remove this incorrect code.
> > > > 
> > > > No functional change.
> > > 
> > > If they are incorrect then how it could be "no functional change"? To me
> > > original code looks buggy and this is a fix. Fix must have functional
> > > change...
> > 
> > Earlier code already checks for all conditions, including all error cases.
> > So the code being removed here has no effect, as any potential error it
> > could catch will already have been caught by earlier code. Removing it
> > therefore doesn't change behaviour or functionality.
> > 
> > I can reword to 'incomplete test' instead of 'incorrect code' if you think
> > that's more clear?
> 
> Perhaps you should mention which "one" in off-by-one that it has no impact.

OK.

> I also wonder why you left the second - ext_control_s2mpg11 - untouched.

Could you point me to it please? I'm not sure I see what you mean.

Cheers,
Andre'

Re: [PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()

[Krzysztof Kozlowski]

On 10/02/2026 10:37, André Draszik wrote:
> Hi Krzysztof,
> 
> On Tue, 2026-02-10 at 08:28 +0100, Krzysztof Kozlowski wrote:
>> On 10/02/2026 06:59, André Draszik wrote:
>>> Hi Krzysztof,
>>>
>>> On Mon, 2026-02-09 at 17:09 +0100, Krzysztof Kozlowski wrote:
>>>> On 09/02/2026 16:07, André Draszik wrote:
>>>>> The sanity checks being removed in this commit are useless as earlier
>>>>> code checks for out-of-bounds conditions already. They also are
>>>>> incorrect (as they're off-by-one).
>>>>>
>>>>> Simply remove this incorrect code.
>>>>>
>>>>> No functional change.
>>>>
>>>> If they are incorrect then how it could be "no functional change"? To me
>>>> original code looks buggy and this is a fix. Fix must have functional
>>>> change...
>>>
>>> Earlier code already checks for all conditions, including all error cases.
>>> So the code being removed here has no effect, as any potential error it
>>> could catch will already have been caught by earlier code. Removing it
>>> therefore doesn't change behaviour or functionality.
>>>
>>> I can reword to 'incomplete test' instead of 'incorrect code' if you think
>>> that's more clear?
>>
>> Perhaps you should mention which "one" in off-by-one that it has no impact.
> 
> OK.
> 
>> I also wonder why you left the second - ext_control_s2mpg11 - untouched.
> 
> Could you point me to it please? I'm not sure I see what you mean.

There is exact same line, which you touch here, ~20 lines below. At
least in linux-next from 5th Feb.

Best regards,
Krzysztof

Re: [PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()

[André Draszik]

On Tue, 2026-02-10 at 11:51 +0100, Krzysztof Kozlowski wrote:
> On 10/02/2026 10:37, André Draszik wrote:
> > Hi Krzysztof,
> > 
> > On Tue, 2026-02-10 at 08:28 +0100, Krzysztof Kozlowski wrote:
> > 
> > 
> > > I also wonder why you left the second - ext_control_s2mpg11 - untouched.
> > 
> > Could you point me to it please? I'm not sure I see what you mean.
> 
> There is exact same line, which you touch here, ~20 lines below. At
> least in linux-next from 5th Feb.

So you mean
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/regulator/s2mps11.c#n506
? It is part of the patch:

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index 2d5510acd0780ab6f9296c48ddcde5efe15ff488..2d67c5c16f487506a2e9e4b119f33faa846269f7 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -478,8 +478,6 @@ static int s2mpg10_of_parse_cb(struct device_node *np,
 			return -EINVAL;
 		}
 
-		if (ext_control > ARRAY_SIZE(ext_control_s2mpg10))
-			return -EINVAL;
 		ext_control = ext_control_s2mpg10[ext_control];
 		break;
 
@@ -503,8 +501,6 @@ static int s2mpg10_of_parse_cb(struct device_node *np,
 			return -EINVAL;
 		}
 
-		if (ext_control > ARRAY_SIZE(ext_control_s2mpg11))
-			return -EINVAL;
 		ext_control = ext_control_s2mpg11[ext_control];
 		break;
 

Otherwise I still don't see it :-)


Cheers,
Andre'

Re: [PATCH 1/2] regulator: s2mps11: drop redundant sanity checks in s2mpg10_of_parse_cb()

[Krzysztof Kozlowski]

On 10/02/2026 12:35, André Draszik wrote:
> On Tue, 2026-02-10 at 11:51 +0100, Krzysztof Kozlowski wrote:
>> On 10/02/2026 10:37, André Draszik wrote:
>>> Hi Krzysztof,
>>>
>>> On Tue, 2026-02-10 at 08:28 +0100, Krzysztof Kozlowski wrote:
>>>
>>>
>>>> I also wonder why you left the second - ext_control_s2mpg11 - untouched.
>>>
>>> Could you point me to it please? I'm not sure I see what you mean.
>>
>> There is exact same line, which you touch here, ~20 lines below. At
>> least in linux-next from 5th Feb.
> 
> So you mean
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/regulator/s2mps11.c#n506
> ? It is part of the patch:
> 

D'oh!

With or without updates of commit msg:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

[PATCH 2/2] regulator: s2mps11: fix pctrlsel macro usage in s2mpg10_of_parse_cb()

[André Draszik]

Commit 979dd8da76eb ("regulator: s2mps11: add S2MPG11 regulator")
incorrectly ended up using macros for S2MPG10 in the S2MPG11 case. They
happen to end up giving the same result, but for clarity, the correct
macros should be used.

No functional change.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/regulator/s2mps11.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index 2d67c5c16f487506a2e9e4b119f33faa846269f7..81cfd60460f8bb35a460f44753abfbb745121322 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -440,15 +440,15 @@ static int s2mpg10_of_parse_cb(struct device_node *np,
 		[S2MPG10_EXTCTRL_LDO20M_EN] = S2MPG10_PCTRLSEL_LDO20M_EN,
 	};
 	static const u32 ext_control_s2mpg11[] = {
-		[S2MPG11_EXTCTRL_PWREN] = S2MPG10_PCTRLSEL_PWREN,
-		[S2MPG11_EXTCTRL_PWREN_MIF] = S2MPG10_PCTRLSEL_PWREN_MIF,
-		[S2MPG11_EXTCTRL_AP_ACTIVE_N] = S2MPG10_PCTRLSEL_AP_ACTIVE_N,
-		[S2MPG11_EXTCTRL_G3D_EN] = S2MPG10_PCTRLSEL_CPUCL1_EN,
-		[S2MPG11_EXTCTRL_G3D_EN2] = S2MPG10_PCTRLSEL_CPUCL1_EN2,
-		[S2MPG11_EXTCTRL_AOC_VDD] = S2MPG10_PCTRLSEL_CPUCL2_EN,
-		[S2MPG11_EXTCTRL_AOC_RET] = S2MPG10_PCTRLSEL_CPUCL2_EN2,
-		[S2MPG11_EXTCTRL_UFS_EN] = S2MPG10_PCTRLSEL_TPU_EN,
-		[S2MPG11_EXTCTRL_LDO13S_EN] = S2MPG10_PCTRLSEL_TPU_EN2,
+		[S2MPG11_EXTCTRL_PWREN] = S2MPG11_PCTRLSEL_PWREN,
+		[S2MPG11_EXTCTRL_PWREN_MIF] = S2MPG11_PCTRLSEL_PWREN_MIF,
+		[S2MPG11_EXTCTRL_AP_ACTIVE_N] = S2MPG11_PCTRLSEL_AP_ACTIVE_N,
+		[S2MPG11_EXTCTRL_G3D_EN] = S2MPG11_PCTRLSEL_G3D_EN,
+		[S2MPG11_EXTCTRL_G3D_EN2] = S2MPG11_PCTRLSEL_G3D_EN2,
+		[S2MPG11_EXTCTRL_AOC_VDD] = S2MPG11_PCTRLSEL_AOC_VDD,
+		[S2MPG11_EXTCTRL_AOC_RET] = S2MPG11_PCTRLSEL_AOC_RET,
+		[S2MPG11_EXTCTRL_UFS_EN] = S2MPG11_PCTRLSEL_UFS_EN,
+		[S2MPG11_EXTCTRL_LDO13S_EN] = S2MPG11_PCTRLSEL_LDO13S_EN,
 	};
 	u32 ext_control;
 

-- 
2.53.0.rc2.204.g2597b5adb4-goog