linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] pinctrl: qcom: lpass lpi: use default pullup/strength values
@ 2021-03-04 19:48 Jonathan Marek
  2021-03-05  0:05 ` Bjorn Andersson
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Jonathan Marek @ 2021-03-04 19:48 UTC (permalink / raw)
  To: linux-arm-msm
  Cc: Andy Gross, Bjorn Andersson, Linus Walleij, Srinivas Kandagatla,
	open list:PIN CONTROL SUBSYSTEM, open list

If these fields are not set in dts, the driver will use these variables
uninitialized to set the fields. Not only will it set garbage values for
these fields, but it can overflow into other fields and break those.

In the current sm8250 dts, the dmic01 entries do not have a pullup setting,
and might not work without this change.

Fixes: 6e261d1090d6 ("pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver")
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
index 369ee20a7ea95..2f19ab4db7208 100644
--- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
@@ -392,7 +392,7 @@ static int lpi_config_set(struct pinctrl_dev *pctldev, unsigned int group,
 			  unsigned long *configs, unsigned int nconfs)
 {
 	struct lpi_pinctrl *pctrl = dev_get_drvdata(pctldev->dev);
-	unsigned int param, arg, pullup, strength;
+	unsigned int param, arg, pullup = LPI_GPIO_BIAS_DISABLE, strength = 2;
 	bool value, output_enabled = false;
 	const struct lpi_pingroup *g;
 	unsigned long sval;
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] pinctrl: qcom: lpass lpi: use default pullup/strength values
  2021-03-04 19:48 [PATCH] pinctrl: qcom: lpass lpi: use default pullup/strength values Jonathan Marek
@ 2021-03-05  0:05 ` Bjorn Andersson
  2021-03-05  0:21   ` Jonathan Marek
  2021-03-05 13:10 ` Srinivas Kandagatla
  2021-03-10 23:13 ` Linus Walleij
  2 siblings, 1 reply; 5+ messages in thread
From: Bjorn Andersson @ 2021-03-05  0:05 UTC (permalink / raw)
  To: Jonathan Marek
  Cc: linux-arm-msm, Andy Gross, Linus Walleij, Srinivas Kandagatla,
	open list:PIN CONTROL SUBSYSTEM, open list

On Thu 04 Mar 13:48 CST 2021, Jonathan Marek wrote:

> If these fields are not set in dts, the driver will use these variables
> uninitialized to set the fields. Not only will it set garbage values for
> these fields, but it can overflow into other fields and break those.
> 
> In the current sm8250 dts, the dmic01 entries do not have a pullup setting,
> and might not work without this change.
> 

Perhaps you didn't see it, but Dan reported this a few days back. So
unless you object I would suggest that we include:

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>


Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> Fixes: 6e261d1090d6 ("pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver")
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> ---
>  drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> index 369ee20a7ea95..2f19ab4db7208 100644
> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> @@ -392,7 +392,7 @@ static int lpi_config_set(struct pinctrl_dev *pctldev, unsigned int group,
>  			  unsigned long *configs, unsigned int nconfs)
>  {
>  	struct lpi_pinctrl *pctrl = dev_get_drvdata(pctldev->dev);
> -	unsigned int param, arg, pullup, strength;
> +	unsigned int param, arg, pullup = LPI_GPIO_BIAS_DISABLE, strength = 2;
>  	bool value, output_enabled = false;
>  	const struct lpi_pingroup *g;
>  	unsigned long sval;
> -- 
> 2.26.1
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] pinctrl: qcom: lpass lpi: use default pullup/strength values
  2021-03-05  0:05 ` Bjorn Andersson
@ 2021-03-05  0:21   ` Jonathan Marek
  0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Marek @ 2021-03-05  0:21 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-arm-msm, Andy Gross, Linus Walleij, Srinivas Kandagatla,
	open list:PIN CONTROL SUBSYSTEM, open list

On 3/4/21 7:05 PM, Bjorn Andersson wrote:
> On Thu 04 Mar 13:48 CST 2021, Jonathan Marek wrote:
> 
>> If these fields are not set in dts, the driver will use these variables
>> uninitialized to set the fields. Not only will it set garbage values for
>> these fields, but it can overflow into other fields and break those.
>>
>> In the current sm8250 dts, the dmic01 entries do not have a pullup setting,
>> and might not work without this change.
>>
> 
> Perhaps you didn't see it, but Dan reported this a few days back. So
> unless you object I would suggest that we include:
> 

I did not see it. But feel free to add tags.

> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> 
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> Regards,
> Bjorn
> 
>> Fixes: 6e261d1090d6 ("pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver")
>> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
>> ---
>>   drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> index 369ee20a7ea95..2f19ab4db7208 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> @@ -392,7 +392,7 @@ static int lpi_config_set(struct pinctrl_dev *pctldev, unsigned int group,
>>   			  unsigned long *configs, unsigned int nconfs)
>>   {
>>   	struct lpi_pinctrl *pctrl = dev_get_drvdata(pctldev->dev);
>> -	unsigned int param, arg, pullup, strength;
>> +	unsigned int param, arg, pullup = LPI_GPIO_BIAS_DISABLE, strength = 2;
>>   	bool value, output_enabled = false;
>>   	const struct lpi_pingroup *g;
>>   	unsigned long sval;
>> -- 
>> 2.26.1
>>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] pinctrl: qcom: lpass lpi: use default pullup/strength values
  2021-03-04 19:48 [PATCH] pinctrl: qcom: lpass lpi: use default pullup/strength values Jonathan Marek
  2021-03-05  0:05 ` Bjorn Andersson
@ 2021-03-05 13:10 ` Srinivas Kandagatla
  2021-03-10 23:13 ` Linus Walleij
  2 siblings, 0 replies; 5+ messages in thread
From: Srinivas Kandagatla @ 2021-03-05 13:10 UTC (permalink / raw)
  To: Jonathan Marek, linux-arm-msm
  Cc: Andy Gross, Bjorn Andersson, Linus Walleij,
	open list:PIN CONTROL SUBSYSTEM, open list

Thanks Jonathan for fixing this!

On 04/03/2021 19:48, Jonathan Marek wrote:
> If these fields are not set in dts, the driver will use these variables
> uninitialized to set the fields. Not only will it set garbage values for
> these fields, but it can overflow into other fields and break those.
> 
> In the current sm8250 dts, the dmic01 entries do not have a pullup setting,
> and might not work without this change.
> 
> Fixes: 6e261d1090d6 ("pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver")
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> ---

LGTM,
Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>


--srini


>   drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> index 369ee20a7ea95..2f19ab4db7208 100644
> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> @@ -392,7 +392,7 @@ static int lpi_config_set(struct pinctrl_dev *pctldev, unsigned int group,
>   			  unsigned long *configs, unsigned int nconfs)
>   {
>   	struct lpi_pinctrl *pctrl = dev_get_drvdata(pctldev->dev);
> -	unsigned int param, arg, pullup, strength;
> +	unsigned int param, arg, pullup = LPI_GPIO_BIAS_DISABLE, strength = 2;
>   	bool value, output_enabled = false;
>   	const struct lpi_pingroup *g;
>   	unsigned long sval;
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] pinctrl: qcom: lpass lpi: use default pullup/strength values
  2021-03-04 19:48 [PATCH] pinctrl: qcom: lpass lpi: use default pullup/strength values Jonathan Marek
  2021-03-05  0:05 ` Bjorn Andersson
  2021-03-05 13:10 ` Srinivas Kandagatla
@ 2021-03-10 23:13 ` Linus Walleij
  2 siblings, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2021-03-10 23:13 UTC (permalink / raw)
  To: Jonathan Marek
  Cc: MSM, Andy Gross, Bjorn Andersson, Srinivas Kandagatla,
	open list:PIN CONTROL SUBSYSTEM, open list

On Thu, Mar 4, 2021 at 8:49 PM Jonathan Marek <jonathan@marek.ca> wrote:

> If these fields are not set in dts, the driver will use these variables
> uninitialized to set the fields. Not only will it set garbage values for
> these fields, but it can overflow into other fields and break those.
>
> In the current sm8250 dts, the dmic01 entries do not have a pullup setting,
> and might not work without this change.
>
> Fixes: 6e261d1090d6 ("pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver")
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>

Patch applied for fixes!

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-03-10 23:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-04 19:48 [PATCH] pinctrl: qcom: lpass lpi: use default pullup/strength values Jonathan Marek
2021-03-05  0:05 ` Bjorn Andersson
2021-03-05  0:21   ` Jonathan Marek
2021-03-05 13:10 ` Srinivas Kandagatla
2021-03-10 23:13 ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).