[PATCH 0/3] pinctrl: qcom: Correct MSM_PULL_MASK and fix resolving TLMM register address.

[PATCH 0/3] pinctrl: qcom: Correct MSM_PULL_MASK and fix resolving TLMM register address.

[Sneh Mankad]

Modify MSM_PULL_MASK to accurately represent the hardware configuration.
Also fix the register address resolution when 'reg-names' property is
not present in device node.

Also avoid assigning thread_op_remain in unthreaded test since it is
only used by threaded IRQ handler.

Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
---
Sneh Mankad (3):
      pinctrl: qcom: Modify MSM_PULL_MASK to accurately represent PULL bits
      pinctrl: qcom: Fix resolving register base address from device node
      pinctrl: qcom: Avoid assigning thread_op_remain in unthreaded test

 drivers/pinctrl/qcom/tlmm-test.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)
---
base-commit: 8fde5d1d47f69db6082dfa34500c27f8485389a5
change-id: 20260529-tlmm_test_changes-6061f95c70bd

Best regards,
-- 
Sneh Mankad <sneh.mankad@oss.qualcomm.com>

[PATCH 1/3] pinctrl: qcom: Modify MSM_PULL_MASK to accurately represent PULL bits

[Sneh Mankad]

MSM_PULL_MASK currently spans bits [2:0], but the GPIO_PULL field in the
GPIO_CFG register only occupies bits [1:0]. Bit 2 belongs to
FUNC_SEL.

MSM_PULL_MASK is used to isolate the GPIO_PULL bits before writing the
pull configuration (PULL_DOWN: 0x1, PULL_UP: 0x3) to the GPIO_CFG
register. Narrow it to bits [1:0] to prevent unintended modification of
the FUNC_SEL field.

This causes no functional change since the driver currently does not
modify the FUNC_SEL bit, but align the mask with hardware configuration
nonetheless.

Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
---
 drivers/pinctrl/qcom/tlmm-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/qcom/tlmm-test.c b/drivers/pinctrl/qcom/tlmm-test.c
index 7d7fff538755a2bcebf94dfb1ca00d4e9948c748..b655de5b4c5f08e7b138f0da24546274bc5fa93b 100644
--- a/drivers/pinctrl/qcom/tlmm-test.c
+++ b/drivers/pinctrl/qcom/tlmm-test.c
@@ -33,7 +33,7 @@
  * dynamically, rather then relying on e.g. Devicetree and phandles.
  */
 
-#define MSM_PULL_MASK		GENMASK(2, 0)
+#define MSM_PULL_MASK		GENMASK(1, 0)
 #define MSM_PULL_DOWN		1
 #define MSM_PULL_UP		3
 #define TLMM_REG_SIZE		0x1000

-- 
2.34.1

Re: [PATCH 1/3] pinctrl: qcom: Modify MSM_PULL_MASK to accurately represent PULL bits

[Konrad Dybcio]

On 29-May-26 14:55, Sneh Mankad wrote:
> MSM_PULL_MASK currently spans bits [2:0], but the GPIO_PULL field in the
> GPIO_CFG register only occupies bits [1:0]. Bit 2 belongs to
> FUNC_SEL.
> 
> MSM_PULL_MASK is used to isolate the GPIO_PULL bits before writing the
> pull configuration (PULL_DOWN: 0x1, PULL_UP: 0x3) to the GPIO_CFG
> register. Narrow it to bits [1:0] to prevent unintended modification of
> the FUNC_SEL field.
> 
> This causes no functional change since the driver currently does not
> modify the FUNC_SEL bit, but align the mask with hardware configuration
> nonetheless.
> 
> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> ---

Seems to be the case down to MSM8960 indeed!

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH 1/3] pinctrl: qcom: Modify MSM_PULL_MASK to accurately represent PULL bits

[Dmitry Baryshkov]

On Fri, May 29, 2026 at 06:25:44PM +0530, Sneh Mankad wrote:
> MSM_PULL_MASK currently spans bits [2:0], but the GPIO_PULL field in the
> GPIO_CFG register only occupies bits [1:0]. Bit 2 belongs to
> FUNC_SEL.
> 
> MSM_PULL_MASK is used to isolate the GPIO_PULL bits before writing the
> pull configuration (PULL_DOWN: 0x1, PULL_UP: 0x3) to the GPIO_CFG
> register. Narrow it to bits [1:0] to prevent unintended modification of
> the FUNC_SEL field.
> 
> This causes no functional change since the driver currently does not
> modify the FUNC_SEL bit, but align the mask with hardware configuration
> nonetheless.
> 
> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> ---
>  drivers/pinctrl/qcom/tlmm-test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

[PATCH 2/3] pinctrl: qcom: Fix resolving register base address from device node

[Sneh Mankad]

Commit 56ffb63749f4 ("pinctrl: qcom: add multi TLMM region option parameter")
added reg-names property based register reading. However multiple platforms
are not using the reg-names as they have only single TLMM register region.

Commit tried to handle this using the default_region module parameter,
however this condition is unreachable as the error return precedes it by
just checking if reg-names property exists or not, making it impossible
to use tlmm-test for the SoCs (x1e80100) which don't have reg-names
property in TLMM device.

Fix this by moving the default_region check at the start of the
tlmm_reg_base().

Fixes: 56ffb63749f4 ("pinctrl: qcom: add multi TLMM region option parameter")
Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
---
 drivers/pinctrl/qcom/tlmm-test.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/pinctrl/qcom/tlmm-test.c b/drivers/pinctrl/qcom/tlmm-test.c
index b655de5b4c5f08e7b138f0da24546274bc5fa93b..007d6539ceced294e81cfbe93a00c75a98c858de 100644
--- a/drivers/pinctrl/qcom/tlmm-test.c
+++ b/drivers/pinctrl/qcom/tlmm-test.c
@@ -581,6 +581,9 @@ static int tlmm_reg_base(struct device_node *tlmm, struct resource *res)
 	int ret;
 	int i;
 
+	if (!strcmp(tlmm_reg_name, "default_region"))
+		return of_address_to_resource(tlmm, 0, res);
+
 	count = of_property_count_strings(tlmm, "reg-names");
 	if (count <= 0) {
 		pr_err("failed to find tlmm reg name\n");
@@ -597,18 +600,14 @@ static int tlmm_reg_base(struct device_node *tlmm, struct resource *res)
 		return -EINVAL;
 	}
 
-	if (!strcmp(tlmm_reg_name, "default_region")) {
-		ret = of_address_to_resource(tlmm, 0, res);
-	} else {
-		for (i = 0; i < count; i++) {
-			if (!strcmp(reg_names[i], tlmm_reg_name)) {
-				ret = of_address_to_resource(tlmm, i, res);
-				break;
-			}
+	for (i = 0; i < count; i++) {
+		if (!strcmp(reg_names[i], tlmm_reg_name)) {
+			ret = of_address_to_resource(tlmm, i, res);
+			break;
 		}
-		if (i == count)
-			ret = -EINVAL;
 	}
+	if (i == count)
+		ret = -EINVAL;
 
 	kfree(reg_names);
 

-- 
2.34.1

Re: [PATCH 2/3] pinctrl: qcom: Fix resolving register base address from device node

[Dmitry Baryshkov]

On Fri, May 29, 2026 at 06:25:45PM +0530, Sneh Mankad wrote:
> Commit 56ffb63749f4 ("pinctrl: qcom: add multi TLMM region option parameter")
> added reg-names property based register reading. However multiple platforms
> are not using the reg-names as they have only single TLMM register region.
> 
> Commit tried to handle this using the default_region module parameter,
> however this condition is unreachable as the error return precedes it by
> just checking if reg-names property exists or not, making it impossible
> to use tlmm-test for the SoCs (x1e80100) which don't have reg-names
> property in TLMM device.
> 
> Fix this by moving the default_region check at the start of the
> tlmm_reg_base().
> 
> Fixes: 56ffb63749f4 ("pinctrl: qcom: add multi TLMM region option parameter")
> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> ---
>  drivers/pinctrl/qcom/tlmm-test.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

[PATCH 3/3] pinctrl: qcom: Avoid assigning thread_op_remain in unthreaded test

[Sneh Mankad]

tlmm_test_rising_while_disabled() sets thread_op_remain to 10, but this
variable is only used by the threaded IRQ handler to control the number
of GPIO pin toggles. Since tlmm_test_rising_while_disabled() does not
register a threaded IRQ handler, the assignment is never used.

Remove the thread_op_remain assignment from
tlmm_test_rising_while_disabled().

This does not cause any change in functionality.

Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
---
 drivers/pinctrl/qcom/tlmm-test.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pinctrl/qcom/tlmm-test.c b/drivers/pinctrl/qcom/tlmm-test.c
index 007d6539ceced294e81cfbe93a00c75a98c858de..e9e04300ab3687825255885821ebde0f3ee586a8 100644
--- a/drivers/pinctrl/qcom/tlmm-test.c
+++ b/drivers/pinctrl/qcom/tlmm-test.c
@@ -521,7 +521,6 @@ static void tlmm_test_rising_while_disabled(struct kunit *test)
 	unsigned int before_edge;
 
 	priv->intr_op = TLMM_TEST_COUNT;
-	atomic_set(&priv->thread_op_remain, 10);
 
 	tlmm_output_low();
 

-- 
2.34.1

Re: [PATCH 3/3] pinctrl: qcom: Avoid assigning thread_op_remain in unthreaded test

[Konrad Dybcio]

On 29-May-26 14:55, Sneh Mankad wrote:
> tlmm_test_rising_while_disabled() sets thread_op_remain to 10, but this
> variable is only used by the threaded IRQ handler to control the number
> of GPIO pin toggles. Since tlmm_test_rising_while_disabled() does not
> register a threaded IRQ handler, the assignment is never used.
> 
> Remove the thread_op_remain assignment from
> tlmm_test_rising_while_disabled().
> 
> This does not cause any change in functionality.
> 
> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> ---
>  drivers/pinctrl/qcom/tlmm-test.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/qcom/tlmm-test.c b/drivers/pinctrl/qcom/tlmm-test.c
> index 007d6539ceced294e81cfbe93a00c75a98c858de..e9e04300ab3687825255885821ebde0f3ee586a8 100644
> --- a/drivers/pinctrl/qcom/tlmm-test.c
> +++ b/drivers/pinctrl/qcom/tlmm-test.c
> @@ -521,7 +521,6 @@ static void tlmm_test_rising_while_disabled(struct kunit *test)
>  	unsigned int before_edge;
>  
>  	priv->intr_op = TLMM_TEST_COUNT;
> -	atomic_set(&priv->thread_op_remain, 10);

Should this be setting priv->intr_op_remain instead?

Konrad

Re: [PATCH 3/3] pinctrl: qcom: Avoid assigning thread_op_remain in unthreaded test

[Sneh Mankad]

On 03-Jun-26 7:28 PM, Konrad Dybcio wrote:
> 
> 
> On 29-May-26 14:55, Sneh Mankad wrote:
>> tlmm_test_rising_while_disabled() sets thread_op_remain to 10, but this
>> variable is only used by the threaded IRQ handler to control the number
>> of GPIO pin toggles. Since tlmm_test_rising_while_disabled() does not
>> register a threaded IRQ handler, the assignment is never used.
>>
>> Remove the thread_op_remain assignment from
>> tlmm_test_rising_while_disabled().
>>
>> This does not cause any change in functionality.
>>
>> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
>> ---
>>  drivers/pinctrl/qcom/tlmm-test.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/pinctrl/qcom/tlmm-test.c b/drivers/pinctrl/qcom/tlmm-test.c
>> index 007d6539ceced294e81cfbe93a00c75a98c858de..e9e04300ab3687825255885821ebde0f3ee586a8 100644
>> --- a/drivers/pinctrl/qcom/tlmm-test.c
>> +++ b/drivers/pinctrl/qcom/tlmm-test.c
>> @@ -521,7 +521,6 @@ static void tlmm_test_rising_while_disabled(struct kunit *test)
>>  	unsigned int before_edge;
>>  
>>  	priv->intr_op = TLMM_TEST_COUNT;
>> -	atomic_set(&priv->thread_op_remain, 10);
> 
> Should this be setting priv->intr_op_remain instead?

Should not be needed.
priv->intr_op_remain is to indicate the hard IRQ handler how many times to toggle the GPIO line within the irq handler.
Here the test case does not require any toggles within IRQ handler.

I also found other test cases where intr_op_remain is assigned but never used. Will remove them in v2.

Thanks,
Sneh

Re: [PATCH 3/3] pinctrl: qcom: Avoid assigning thread_op_remain in unthreaded test

[Konrad Dybcio]

On 6/10/26 8:16 AM, Sneh Mankad wrote:
> 
> 
> On 03-Jun-26 7:28 PM, Konrad Dybcio wrote:
>>
>>
>> On 29-May-26 14:55, Sneh Mankad wrote:
>>> tlmm_test_rising_while_disabled() sets thread_op_remain to 10, but this
>>> variable is only used by the threaded IRQ handler to control the number
>>> of GPIO pin toggles. Since tlmm_test_rising_while_disabled() does not
>>> register a threaded IRQ handler, the assignment is never used.
>>>
>>> Remove the thread_op_remain assignment from
>>> tlmm_test_rising_while_disabled().
>>>
>>> This does not cause any change in functionality.
>>>
>>> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
>>> ---
>>>  drivers/pinctrl/qcom/tlmm-test.c | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/drivers/pinctrl/qcom/tlmm-test.c b/drivers/pinctrl/qcom/tlmm-test.c
>>> index 007d6539ceced294e81cfbe93a00c75a98c858de..e9e04300ab3687825255885821ebde0f3ee586a8 100644
>>> --- a/drivers/pinctrl/qcom/tlmm-test.c
>>> +++ b/drivers/pinctrl/qcom/tlmm-test.c
>>> @@ -521,7 +521,6 @@ static void tlmm_test_rising_while_disabled(struct kunit *test)
>>>  	unsigned int before_edge;
>>>  
>>>  	priv->intr_op = TLMM_TEST_COUNT;
>>> -	atomic_set(&priv->thread_op_remain, 10);
>>
>> Should this be setting priv->intr_op_remain instead?
> 
> Should not be needed.
> priv->intr_op_remain is to indicate the hard IRQ handler how many times to toggle the GPIO line within the irq handler.
> Here the test case does not require any toggles within IRQ handler.

Oh yeah seems you're right

> I also found other test cases where intr_op_remain is assigned but never used. Will remove them in v2.

Thanks!

Konrad

Re: [PATCH 0/3] pinctrl: qcom: Correct MSM_PULL_MASK and fix resolving TLMM register address.

[Linus Walleij]

Hi Sneh,

thanks for your patches!

On Fri, May 29, 2026 at 2:56 PM Sneh Mankad
<sneh.mankad@oss.qualcomm.com> wrote:

> Modify MSM_PULL_MASK to accurately represent the hardware configuration.
> Also fix the register address resolution when 'reg-names' property is
> not present in device node.
>
> Also avoid assigning thread_op_remain in unthreaded test since it is
> only used by threaded IRQ handler.
>
> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> ---
> Sneh Mankad (3):
>       pinctrl: qcom: Modify MSM_PULL_MASK to accurately represent PULL bits
>       pinctrl: qcom: Fix resolving register base address from device node

Patches 1 & 2 applied since they are ACKed and look good.

>       pinctrl: qcom: Avoid assigning thread_op_remain in unthreaded test

There are comments on this one so revise and resend this one only.

Yours,
Linus Walleij