[PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Camel Guo]

From: Camel Guo <camelg@axis.com>

When gpio_reset is not well defined in devicetree, the
adcx140->gpio_reset is an error code instead of NULL. In this case,
adcx140->gpio_reset should not be used by adcx140_reset. This commit
sets it NULL to avoid accessing an invalid variable.

Signed-off-by: Camel Guo <camelg@axis.com>
---
 sound/soc/codecs/tlv320adcx140.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/tlv320adcx140.c b/sound/soc/codecs/tlv320adcx140.c
index 7ae6ec374be3..597dd1062943 100644
--- a/sound/soc/codecs/tlv320adcx140.c
+++ b/sound/soc/codecs/tlv320adcx140.c
@@ -984,8 +984,10 @@ static int adcx140_i2c_probe(struct i2c_client *i2c,
 
 	adcx140->gpio_reset = devm_gpiod_get_optional(adcx140->dev,
 						      "reset", GPIOD_OUT_LOW);
-	if (IS_ERR(adcx140->gpio_reset))
+	if (IS_ERR(adcx140->gpio_reset) || adcx140->gpio_reset == NULL) {
 		dev_info(&i2c->dev, "Reset GPIO not defined\n");
+		adcx140->gpio_reset = NULL;
+	}
 
 	adcx140->supply_areg = devm_regulator_get_optional(adcx140->dev,
 							   "areg");
-- 
2.20.1

[PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Camel Guo]

From: Camel Guo <camelg@axis.com>

When gpio_reset is not well defined in devicetree, the
adcx140->gpio_reset is an error code instead of NULL. In this case,
adcx140->gpio_reset should not be used by adcx140_reset. This commit
sets it NULL to avoid accessing an invalid variable.

Signed-off-by: Camel Guo <camelg@axis.com>
---
 sound/soc/codecs/tlv320adcx140.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/tlv320adcx140.c b/sound/soc/codecs/tlv320adcx140.c
index 7ae6ec374be3..597dd1062943 100644
--- a/sound/soc/codecs/tlv320adcx140.c
+++ b/sound/soc/codecs/tlv320adcx140.c
@@ -984,8 +984,10 @@ static int adcx140_i2c_probe(struct i2c_client *i2c,
 
 	adcx140->gpio_reset = devm_gpiod_get_optional(adcx140->dev,
 						      "reset", GPIOD_OUT_LOW);
-	if (IS_ERR(adcx140->gpio_reset))
+	if (IS_ERR(adcx140->gpio_reset) || adcx140->gpio_reset == NULL) {
 		dev_info(&i2c->dev, "Reset GPIO not defined\n");
+		adcx140->gpio_reset = NULL;
+	}
 
 	adcx140->supply_areg = devm_regulator_get_optional(adcx140->dev,
 							   "areg");
-- 
2.20.1

[PATCH 2/2] ASoC: tlv320adcx140: Wake up codec before accessing register

[Camel Guo]

From: Camel Guo <camelg@axis.com>

According to its datasheet, after reset this codec goes into sleep
mode. In this mode, any register accessing should be avoided except for
exiting sleep mode. Hence this commit moves SLEEP_CFG access before any
register accessing.

Signed-off-by: Camel Guo <camelg@axis.com>
---
 sound/soc/codecs/tlv320adcx140.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/sound/soc/codecs/tlv320adcx140.c b/sound/soc/codecs/tlv320adcx140.c
index 597dd1062943..6d456aa269ad 100644
--- a/sound/soc/codecs/tlv320adcx140.c
+++ b/sound/soc/codecs/tlv320adcx140.c
@@ -842,6 +842,18 @@ static int adcx140_codec_probe(struct snd_soc_component *component)
 	if (ret)
 		goto out;
 
+	if (adcx140->supply_areg == NULL)
+		sleep_cfg_val |= ADCX140_AREG_INTERNAL;
+
+	ret = regmap_write(adcx140->regmap, ADCX140_SLEEP_CFG, sleep_cfg_val);
+	if (ret) {
+		dev_err(adcx140->dev, "setting sleep config failed %d\n", ret);
+		goto out;
+	}
+
+	/* 8.4.3: Wait >= 1ms after entering active mode. */
+	usleep_range(1000, 100000);
+
 	pdm_count = device_property_count_u32(adcx140->dev,
 					      "ti,pdm-edge-select");
 	if (pdm_count <= ADCX140_NUM_PDM_EDGES && pdm_count > 0) {
@@ -889,18 +901,6 @@ static int adcx140_codec_probe(struct snd_soc_component *component)
 	if (ret)
 		goto out;
 
-	if (adcx140->supply_areg == NULL)
-		sleep_cfg_val |= ADCX140_AREG_INTERNAL;
-
-	ret = regmap_write(adcx140->regmap, ADCX140_SLEEP_CFG, sleep_cfg_val);
-	if (ret) {
-		dev_err(adcx140->dev, "setting sleep config failed %d\n", ret);
-		goto out;
-	}
-
-	/* 8.4.3: Wait >= 1ms after entering active mode. */
-	usleep_range(1000, 100000);
-
 	ret = regmap_update_bits(adcx140->regmap, ADCX140_BIAS_CFG,
 				ADCX140_MIC_BIAS_VAL_MSK |
 				ADCX140_MIC_BIAS_VREF_MSK, bias_cfg);
-- 
2.20.1

[PATCH 2/2] ASoC: tlv320adcx140: Wake up codec before accessing register

[Camel Guo]

From: Camel Guo <camelg@axis.com>

According to its datasheet, after reset this codec goes into sleep
mode. In this mode, any register accessing should be avoided except for
exiting sleep mode. Hence this commit moves SLEEP_CFG access before any
register accessing.

Signed-off-by: Camel Guo <camelg@axis.com>
---
 sound/soc/codecs/tlv320adcx140.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/sound/soc/codecs/tlv320adcx140.c b/sound/soc/codecs/tlv320adcx140.c
index 597dd1062943..6d456aa269ad 100644
--- a/sound/soc/codecs/tlv320adcx140.c
+++ b/sound/soc/codecs/tlv320adcx140.c
@@ -842,6 +842,18 @@ static int adcx140_codec_probe(struct snd_soc_component *component)
 	if (ret)
 		goto out;
 
+	if (adcx140->supply_areg == NULL)
+		sleep_cfg_val |= ADCX140_AREG_INTERNAL;
+
+	ret = regmap_write(adcx140->regmap, ADCX140_SLEEP_CFG, sleep_cfg_val);
+	if (ret) {
+		dev_err(adcx140->dev, "setting sleep config failed %d\n", ret);
+		goto out;
+	}
+
+	/* 8.4.3: Wait >= 1ms after entering active mode. */
+	usleep_range(1000, 100000);
+
 	pdm_count = device_property_count_u32(adcx140->dev,
 					      "ti,pdm-edge-select");
 	if (pdm_count <= ADCX140_NUM_PDM_EDGES && pdm_count > 0) {
@@ -889,18 +901,6 @@ static int adcx140_codec_probe(struct snd_soc_component *component)
 	if (ret)
 		goto out;
 
-	if (adcx140->supply_areg == NULL)
-		sleep_cfg_val |= ADCX140_AREG_INTERNAL;
-
-	ret = regmap_write(adcx140->regmap, ADCX140_SLEEP_CFG, sleep_cfg_val);
-	if (ret) {
-		dev_err(adcx140->dev, "setting sleep config failed %d\n", ret);
-		goto out;
-	}
-
-	/* 8.4.3: Wait >= 1ms after entering active mode. */
-	usleep_range(1000, 100000);
-
 	ret = regmap_update_bits(adcx140->regmap, ADCX140_BIAS_CFG,
 				ADCX140_MIC_BIAS_VAL_MSK |
 				ADCX140_MIC_BIAS_VREF_MSK, bias_cfg);
-- 
2.20.1

Re: [PATCH 2/2] ASoC: tlv320adcx140: Wake up codec before accessing register

[Dan Murphy]

Camel

On 9/8/20 3:35 AM, Camel Guo wrote:
> From: Camel Guo <camelg@axis.com>
>
> According to its datasheet, after reset this codec goes into sleep
> mode. In this mode, any register accessing should be avoided except for
> exiting sleep mode. Hence this commit moves SLEEP_CFG access before any
> register accessing.

This is interesting because our HW team suggested putting the device 
into sleep mode when doing register writes/reads because they were 
finding abnormalities in the register settings when the device is active.

I have a local patch that changes this as well that the HW team requested.

Mark

Let me run this by the HW team first before applying this patch.

Dan

Re: [PATCH 2/2] ASoC: tlv320adcx140: Wake up codec before accessing register

[Dan Murphy]

Camel

On 9/8/20 3:35 AM, Camel Guo wrote:
> From: Camel Guo <camelg@axis.com>
>
> According to its datasheet, after reset this codec goes into sleep
> mode. In this mode, any register accessing should be avoided except for
> exiting sleep mode. Hence this commit moves SLEEP_CFG access before any
> register accessing.

This is interesting because our HW team suggested putting the device 
into sleep mode when doing register writes/reads because they were 
finding abnormalities in the register settings when the device is active.

I have a local patch that changes this as well that the HW team requested.

Mark

Let me run this by the HW team first before applying this patch.

Dan

Re: [PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Dan Murphy]

Re: [PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Dan Murphy]

Camel

On 9/8/20 3:35 AM, Camel Guo wrote:
> From: Camel Guo <camelg@axis.com>
>
> When gpio_reset is not well defined in devicetree, the
> adcx140->gpio_reset is an error code instead of NULL. In this case,
> adcx140->gpio_reset should not be used by adcx140_reset. This commit
> sets it NULL to avoid accessing an invalid variable.
>
> Signed-off-by: Camel Guo <camelg@axis.com>
> ---
>   sound/soc/codecs/tlv320adcx140.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/codecs/tlv320adcx140.c b/sound/soc/codecs/tlv320adcx140.c
> index 7ae6ec374be3..597dd1062943 100644
> --- a/sound/soc/codecs/tlv320adcx140.c
> +++ b/sound/soc/codecs/tlv320adcx140.c
> @@ -984,8 +984,10 @@ static int adcx140_i2c_probe(struct i2c_client *i2c,
>   
>   	adcx140->gpio_reset = devm_gpiod_get_optional(adcx140->dev,
>   						      "reset", GPIOD_OUT_LOW);
> -	if (IS_ERR(adcx140->gpio_reset))
> +	if (IS_ERR(adcx140->gpio_reset) || adcx140->gpio_reset == NULL) {

This looks a bit off and seems like the NULL check was added just to 
print the message.

I would suggest removing the "or" check and just set the gpio_reset to 
NULL in an error case.

This avoids noise in the log especially if the gpio_reset is 
intentionally not populated in the DT

Dan

Re: [PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Dan Murphy]

Camel

On 9/8/20 3:35 AM, Camel Guo wrote:
> From: Camel Guo <camelg@axis.com>
>
> When gpio_reset is not well defined in devicetree, the
> adcx140->gpio_reset is an error code instead of NULL. In this case,
> adcx140->gpio_reset should not be used by adcx140_reset. This commit
> sets it NULL to avoid accessing an invalid variable.
>
> Signed-off-by: Camel Guo <camelg@axis.com>
> ---
>   sound/soc/codecs/tlv320adcx140.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/codecs/tlv320adcx140.c b/sound/soc/codecs/tlv320adcx140.c
> index 7ae6ec374be3..597dd1062943 100644
> --- a/sound/soc/codecs/tlv320adcx140.c
> +++ b/sound/soc/codecs/tlv320adcx140.c
> @@ -984,8 +984,10 @@ static int adcx140_i2c_probe(struct i2c_client *i2c,
>   
>   	adcx140->gpio_reset = devm_gpiod_get_optional(adcx140->dev,
>   						      "reset", GPIOD_OUT_LOW);
> -	if (IS_ERR(adcx140->gpio_reset))
> +	if (IS_ERR(adcx140->gpio_reset) || adcx140->gpio_reset == NULL) {

This looks a bit off and seems like the NULL check was added just to 
print the message.

I would suggest removing the "or" check and just set the gpio_reset to 
NULL in an error case.

This avoids noise in the log especially if the gpio_reset is 
intentionally not populated in the DT

Dan

Re: [PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Peter Ujfalusi]

On 08/09/2020 11.35, Camel Guo wrote:
> From: Camel Guo <camelg@axis.com>
> 
> When gpio_reset is not well defined in devicetree, the
> adcx140->gpio_reset is an error code instead of NULL. In this case,
> adcx140->gpio_reset should not be used by adcx140_reset. This commit
> sets it NULL to avoid accessing an invalid variable.
> 
> Signed-off-by: Camel Guo <camelg@axis.com>
> ---
>  sound/soc/codecs/tlv320adcx140.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/tlv320adcx140.c b/sound/soc/codecs/tlv320adcx140.c
> index 7ae6ec374be3..597dd1062943 100644
> --- a/sound/soc/codecs/tlv320adcx140.c
> +++ b/sound/soc/codecs/tlv320adcx140.c
> @@ -984,8 +984,10 @@ static int adcx140_i2c_probe(struct i2c_client *i2c,
>  
>  	adcx140->gpio_reset = devm_gpiod_get_optional(adcx140->dev,
>  						      "reset", GPIOD_OUT_LOW);
> -	if (IS_ERR(adcx140->gpio_reset))
> +	if (IS_ERR(adcx140->gpio_reset) || adcx140->gpio_reset == NULL) {
>  		dev_info(&i2c->dev, "Reset GPIO not defined\n");
> +		adcx140->gpio_reset = NULL;

the correct fix is to:
	dev_err(&i2c->dev, "Reset GPIO not defined\n");
	return PTR_ERR(adcx140->gpio_reset);

If the reset GPIO is specified and you get error when requesting it as
optional, there is a reason for that.
For example deferred probing.

> +	}
>  
>  	adcx140->supply_areg = devm_regulator_get_optional(adcx140->dev,
>  							   "areg");
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Peter Ujfalusi]

On 08/09/2020 11.35, Camel Guo wrote:
> From: Camel Guo <camelg@axis.com>
> 
> When gpio_reset is not well defined in devicetree, the
> adcx140->gpio_reset is an error code instead of NULL. In this case,
> adcx140->gpio_reset should not be used by adcx140_reset. This commit
> sets it NULL to avoid accessing an invalid variable.
> 
> Signed-off-by: Camel Guo <camelg@axis.com>
> ---
>  sound/soc/codecs/tlv320adcx140.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/tlv320adcx140.c b/sound/soc/codecs/tlv320adcx140.c
> index 7ae6ec374be3..597dd1062943 100644
> --- a/sound/soc/codecs/tlv320adcx140.c
> +++ b/sound/soc/codecs/tlv320adcx140.c
> @@ -984,8 +984,10 @@ static int adcx140_i2c_probe(struct i2c_client *i2c,
>  
>  	adcx140->gpio_reset = devm_gpiod_get_optional(adcx140->dev,
>  						      "reset", GPIOD_OUT_LOW);
> -	if (IS_ERR(adcx140->gpio_reset))
> +	if (IS_ERR(adcx140->gpio_reset) || adcx140->gpio_reset == NULL) {
>  		dev_info(&i2c->dev, "Reset GPIO not defined\n");
> +		adcx140->gpio_reset = NULL;

the correct fix is to:
	dev_err(&i2c->dev, "Reset GPIO not defined\n");
	return PTR_ERR(adcx140->gpio_reset);

If the reset GPIO is specified and you get error when requesting it as
optional, there is a reason for that.
For example deferred probing.

> +	}
>  
>  	adcx140->supply_areg = devm_regulator_get_optional(adcx140->dev,
>  							   "areg");
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Peter Ujfalusi]

Hi,

On 08/09/2020 14.59, Peter Ujfalusi wrote:
> 
> 
> On 08/09/2020 11.35, Camel Guo wrote:
>> From: Camel Guo <camelg@axis.com>
>>
>> When gpio_reset is not well defined in devicetree, the
>> adcx140->gpio_reset is an error code instead of NULL. In this case,
>> adcx140->gpio_reset should not be used by adcx140_reset. This commit
>> sets it NULL to avoid accessing an invalid variable.
>>
>> Signed-off-by: Camel Guo <camelg@axis.com>
>> ---
>>  sound/soc/codecs/tlv320adcx140.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/codecs/tlv320adcx140.c b/sound/soc/codecs/tlv320adcx140.c
>> index 7ae6ec374be3..597dd1062943 100644
>> --- a/sound/soc/codecs/tlv320adcx140.c
>> +++ b/sound/soc/codecs/tlv320adcx140.c
>> @@ -984,8 +984,10 @@ static int adcx140_i2c_probe(struct i2c_client *i2c,
>>  
>>  	adcx140->gpio_reset = devm_gpiod_get_optional(adcx140->dev,
>>  						      "reset", GPIOD_OUT_LOW);
>> -	if (IS_ERR(adcx140->gpio_reset))
>> +	if (IS_ERR(adcx140->gpio_reset) || adcx140->gpio_reset == NULL) {
>>  		dev_info(&i2c->dev, "Reset GPIO not defined\n");
>> +		adcx140->gpio_reset = NULL;
> 
> the correct fix is to:
> 	dev_err(&i2c->dev, "Reset GPIO not defined\n");

no need to print, I think gpio core will do that.

> 	return PTR_ERR(adcx140->gpio_reset);
> 
> If the reset GPIO is specified and you get error when requesting it as
> optional, there is a reason for that.
> For example deferred probing.
> 
>> +	}
>>  
>>  	adcx140->supply_areg = devm_regulator_get_optional(adcx140->dev,
>>  							   "areg");
>>
> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Peter Ujfalusi]

Re: [PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Peter Ujfalusi]

Hi,

On 08/09/2020 14.59, Peter Ujfalusi wrote:
> 
> 
> On 08/09/2020 11.35, Camel Guo wrote:
>> From: Camel Guo <camelg@axis.com>
>>
>> When gpio_reset is not well defined in devicetree, the
>> adcx140->gpio_reset is an error code instead of NULL. In this case,
>> adcx140->gpio_reset should not be used by adcx140_reset. This commit
>> sets it NULL to avoid accessing an invalid variable.
>>
>> Signed-off-by: Camel Guo <camelg@axis.com>
>> ---
>>  sound/soc/codecs/tlv320adcx140.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/codecs/tlv320adcx140.c b/sound/soc/codecs/tlv320adcx140.c
>> index 7ae6ec374be3..597dd1062943 100644
>> --- a/sound/soc/codecs/tlv320adcx140.c
>> +++ b/sound/soc/codecs/tlv320adcx140.c
>> @@ -984,8 +984,10 @@ static int adcx140_i2c_probe(struct i2c_client *i2c,
>>  
>>  	adcx140->gpio_reset = devm_gpiod_get_optional(adcx140->dev,
>>  						      "reset", GPIOD_OUT_LOW);
>> -	if (IS_ERR(adcx140->gpio_reset))
>> +	if (IS_ERR(adcx140->gpio_reset) || adcx140->gpio_reset == NULL) {
>>  		dev_info(&i2c->dev, "Reset GPIO not defined\n");
>> +		adcx140->gpio_reset = NULL;
> 
> the correct fix is to:
> 	dev_err(&i2c->dev, "Reset GPIO not defined\n");

no need to print, I think gpio core will do that.

> 	return PTR_ERR(adcx140->gpio_reset);
> 
> If the reset GPIO is specified and you get error when requesting it as
> optional, there is a reason for that.
> For example deferred probing.
> 
>> +	}
>>  
>>  	adcx140->supply_areg = devm_regulator_get_optional(adcx140->dev,
>>  							   "areg");
>>
> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 2/2] ASoC: tlv320adcx140: Wake up codec before accessing register

[Dan Murphy]

Camel

On 9/8/20 6:49 AM, Dan Murphy wrote:
> Camel
>
> On 9/8/20 3:35 AM, Camel Guo wrote:
>> From: Camel Guo <camelg@axis.com>
>>
>> According to its datasheet, after reset this codec goes into sleep
>> mode. In this mode, any register accessing should be avoided except for
>> exiting sleep mode. Hence this commit moves SLEEP_CFG access before any
>> register accessing.
>
> This is interesting because our HW team suggested putting the device 
> into sleep mode when doing register writes/reads because they were 
> finding abnormalities in the register settings when the device is active.
>
> I have a local patch that changes this as well that the HW team 
> requested.


OK I have clarification on this now.  Their original request was 
incorrect they indicate the BIAS, ADC and PLLs be powered down during 
writes and reads.


>
> Mark
>
> Let me run this by the HW team first before applying this patch.

Mark

Acked-by: Dan Murphy <dmurphy@ti.com>


>
> Dan
>

Re: [PATCH 2/2] ASoC: tlv320adcx140: Wake up codec before accessing register

[Dan Murphy]

Camel

On 9/8/20 6:49 AM, Dan Murphy wrote:
> Camel
>
> On 9/8/20 3:35 AM, Camel Guo wrote:
>> From: Camel Guo <camelg@axis.com>
>>
>> According to its datasheet, after reset this codec goes into sleep
>> mode. In this mode, any register accessing should be avoided except for
>> exiting sleep mode. Hence this commit moves SLEEP_CFG access before any
>> register accessing.
>
> This is interesting because our HW team suggested putting the device 
> into sleep mode when doing register writes/reads because they were 
> finding abnormalities in the register settings when the device is active.
>
> I have a local patch that changes this as well that the HW team 
> requested.


OK I have clarification on this now.  Their original request was 
incorrect they indicate the BIAS, ADC and PLLs be powered down during 
writes and reads.


>
> Mark
>
> Let me run this by the HW team first before applying this patch.

Mark

Acked-by: Dan Murphy <dmurphy@ti.com>


>
> Dan
>

Re: [PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Mark Brown]

On Tue, 8 Sep 2020 10:35:20 +0200, Camel Guo wrote:
> When gpio_reset is not well defined in devicetree, the
> adcx140->gpio_reset is an error code instead of NULL. In this case,
> adcx140->gpio_reset should not be used by adcx140_reset. This commit
> sets it NULL to avoid accessing an invalid variable.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: tlv320adcx140: Wake up codec before accessing register
      commit: 1a5ce48fd667128e369fdc7fb87e21539aed21b5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Mark Brown]

On Tue, 8 Sep 2020 10:35:20 +0200, Camel Guo wrote:
> When gpio_reset is not well defined in devicetree, the
> adcx140->gpio_reset is an error code instead of NULL. In this case,
> adcx140->gpio_reset should not be used by adcx140_reset. This commit
> sets it NULL to avoid accessing an invalid variable.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: tlv320adcx140: Wake up codec before accessing register
      commit: 1a5ce48fd667128e369fdc7fb87e21539aed21b5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Camel Guo]

Mark

On 9/8/20 7:20 PM, Mark Brown wrote:
> On Tue, 8 Sep 2020 10:35:20 +0200, Camel Guo wrote:
>> When gpio_reset is not well defined in devicetree, the
>> adcx140->gpio_reset is an error code instead of NULL. In this case,
>> adcx140->gpio_reset should not be used by adcx140_reset. This commit
>> sets it NULL to avoid accessing an invalid variable.
> 
> Applied to
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

I think it is "ASoC: tlv320adcx140: Wake up codec before register 
setting" that is applied to for next not this one.

> 
> Thanks!
> 
> [1/1] ASoC: tlv320adcx140: Wake up codec before accessing register
>        commit: 1a5ce48fd667128e369fdc7fb87e21539aed21b5
> 
> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.
> 
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
> 
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
> 
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
> 
> Thanks,
> Mark

Re: [PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Camel Guo]

Hi Peter,

Thanks for your comment. I will make a new one like what you said.

On 9/8/20 2:03 PM, Peter Ujfalusi wrote:
> Hi,
> 
> On 08/09/2020 14.59, Peter Ujfalusi wrote:
>> 
>> 
>> On 08/09/2020 11.35, Camel Guo wrote:
>>> From: Camel Guo <camelg@axis.com>
>>>
>>> When gpio_reset is not well defined in devicetree, the
>>> adcx140->gpio_reset is an error code instead of NULL. In this case,
>>> adcx140->gpio_reset should not be used by adcx140_reset. This commit
>>> sets it NULL to avoid accessing an invalid variable.
>>>
>>> Signed-off-by: Camel Guo <camelg@axis.com>
>>> ---
>>>  sound/soc/codecs/tlv320adcx140.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/sound/soc/codecs/tlv320adcx140.c b/sound/soc/codecs/tlv320adcx140.c
>>> index 7ae6ec374be3..597dd1062943 100644
>>> --- a/sound/soc/codecs/tlv320adcx140.c
>>> +++ b/sound/soc/codecs/tlv320adcx140.c
>>> @@ -984,8 +984,10 @@ static int adcx140_i2c_probe(struct i2c_client *i2c,
>>>  
>>>       adcx140->gpio_reset = devm_gpiod_get_optional(adcx140->dev,
>>>                                                     "reset", GPIOD_OUT_LOW);
>>> -    if (IS_ERR(adcx140->gpio_reset))
>>> +    if (IS_ERR(adcx140->gpio_reset) || adcx140->gpio_reset == NULL) {
>>>               dev_info(&i2c->dev, "Reset GPIO not defined\n");
>>> +            adcx140->gpio_reset = NULL;
>> 
>> the correct fix is to:
>>        dev_err(&i2c->dev, "Reset GPIO not defined\n");
> 
> no need to print, I think gpio core will do that.
> 
>>        return PTR_ERR(adcx140->gpio_reset);
>> 
>> If the reset GPIO is specified and you get error when requesting it as
>> optional, there is a reason for that.
>> For example deferred probing.

I agree! Please ignore this patch and I will make a new one like this.

>> 
>>> +    }
>>>  
>>>       adcx140->supply_areg = devm_regulator_get_optional(adcx140->dev,
>>>                                                          "areg");
>>>
>> 
>> - Péter
>> 
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>> 
> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

Re: [PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 516 bytes --]

On Wed, Sep 09, 2020 at 12:06:45PM +0200, Camel Guo wrote:
> On 9/8/20 7:20 PM, Mark Brown wrote:

> > Applied to

> > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

> I think it is "ASoC: tlv320adcx140: Wake up codec before register setting"
> that is applied to for next not this one.

As the mail says:

> > [1/1] ASoC: tlv320adcx140: Wake up codec before accessing register
> >        commit: 1a5ce48fd667128e369fdc7fb87e21539aed21b5

You didn't send a cover letter.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/2] ASoC: tlv320adcx140: Avoid accessing invalid gpio_reset

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 516 bytes --]

On Wed, Sep 09, 2020 at 12:06:45PM +0200, Camel Guo wrote:
> On 9/8/20 7:20 PM, Mark Brown wrote:

> > Applied to

> > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

> I think it is "ASoC: tlv320adcx140: Wake up codec before register setting"
> that is applied to for next not this one.

As the mail says:

> > [1/1] ASoC: tlv320adcx140: Wake up codec before accessing register
> >        commit: 1a5ce48fd667128e369fdc7fb87e21539aed21b5

You didn't send a cover letter.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]