[PATCH 1/2] ASoC: rt5640: fix sparse warnings

[PATCH 1/2] ASoC: rt5640: fix sparse warnings

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

This fixes:
975:9: sparse: Using plain integer as NULL pointer
1917:24: sparse: symbol 'rt5640_aif_dai_ops' was not declared. Should it be static?
1924:27: sparse: symbol 'rt5640_dai' was not declared. Should it be static?
2079:19: sparse: symbol 'rt5640_i2c_driver' was not declared. Should it be static?

Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 sound/soc/codecs/rt5640.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
index 8761552..ce585e3 100644
--- a/sound/soc/codecs/rt5640.c
+++ b/sound/soc/codecs/rt5640.c
@@ -974,7 +974,7 @@ static const struct snd_soc_dapm_widget rt5640_dapm_widgets[] = {
 	SND_SOC_DAPM_SUPPLY("LDO2", RT5640_PWR_ANLG1,
 			RT5640_PWR_LDO2_BIT, 0, NULL, 0),
 	SND_SOC_DAPM_SUPPLY("MICBIAS1", RT5640_PWR_ANLG2,
-			RT5640_PWR_MB1_BIT, 0, 0, 0),
+			RT5640_PWR_MB1_BIT, 0, NULL, 0),
 	/* Input Lines */
 	SND_SOC_DAPM_INPUT("DMIC1"),
 	SND_SOC_DAPM_INPUT("DMIC2"),
@@ -1915,14 +1915,14 @@ static int rt5640_resume(struct snd_soc_codec *codec)
 #define RT5640_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE | \
 			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S8)
 
-struct snd_soc_dai_ops rt5640_aif_dai_ops = {
+static const struct snd_soc_dai_ops rt5640_aif_dai_ops = {
 	.hw_params = rt5640_hw_params,
 	.set_fmt = rt5640_set_dai_fmt,
 	.set_sysclk = rt5640_set_dai_sysclk,
 	.set_pll = rt5640_set_dai_pll,
 };
 
-struct snd_soc_dai_driver rt5640_dai[] = {
+static struct snd_soc_dai_driver rt5640_dai[] = {
 	{
 		.name = "rt5640-aif1",
 		.id = RT5640_AIF1,
@@ -2112,7 +2112,7 @@ static int rt5640_i2c_remove(struct i2c_client *i2c)
 	return 0;
 }
 
-struct i2c_driver rt5640_i2c_driver = {
+static struct i2c_driver rt5640_i2c_driver = {
 	.driver = {
 		.name = "rt5640",
 		.owner = THIS_MODULE,
-- 
1.8.1.5

[PATCH 2/2] ASoC: rt5640: enable LDO1 before creating regmap

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

Enable the LDO1_EN GPIO to the CODEC, which enables the device to show
up on the I2C bus, before creating the regmap object. This guarantees
that if devm_regmap_init_i2c() were to attempt to communicate with the
device, it would be able to.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 sound/soc/codecs/rt5640.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
index ce585e3..ead953c 100644
--- a/sound/soc/codecs/rt5640.c
+++ b/sound/soc/codecs/rt5640.c
@@ -2053,14 +2053,6 @@ static int rt5640_i2c_probe(struct i2c_client *i2c,
 	} else
 		rt5640->pdata.ldo1_en = -EINVAL;
 
-	rt5640->regmap = devm_regmap_init_i2c(i2c, &rt5640_regmap);
-	if (IS_ERR(rt5640->regmap)) {
-		ret = PTR_ERR(rt5640->regmap);
-		dev_err(&i2c->dev, "Failed to allocate register map: %d\n",
-			ret);
-		return ret;
-	}
-
 	if (gpio_is_valid(rt5640->pdata.ldo1_en)) {
 		ret = devm_gpio_request_one(&i2c->dev, rt5640->pdata.ldo1_en,
 					    GPIOF_OUT_INIT_HIGH,
@@ -2073,6 +2065,14 @@ static int rt5640_i2c_probe(struct i2c_client *i2c,
 		msleep(400);
 	}
 
+	rt5640->regmap = devm_regmap_init_i2c(i2c, &rt5640_regmap);
+	if (IS_ERR(rt5640->regmap)) {
+		ret = PTR_ERR(rt5640->regmap);
+		dev_err(&i2c->dev, "Failed to allocate register map: %d\n",
+			ret);
+		return ret;
+	}
+
 	regmap_read(rt5640->regmap, RT5640_VENDOR_ID2, &val);
 	if ((val != RT5640_DEVICE_ID)) {
 		dev_err(&i2c->dev,
-- 
1.8.1.5

Re: [PATCH 1/2] ASoC: rt5640: fix sparse warnings

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 455 bytes --]

On Wed, Jun 12, 2013 at 03:34:23PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> This fixes:
> 975:9: sparse: Using plain integer as NULL pointer
> 1917:24: sparse: symbol 'rt5640_aif_dai_ops' was not declared. Should it be static?
> 1924:27: sparse: symbol 'rt5640_dai' was not declared. Should it be static?
> 2079:19: sparse: symbol 'rt5640_i2c_driver' was not declared. Should it be static?

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] ASoC: rt5640: enable LDO1 before creating regmap

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 632 bytes --]

On Wed, Jun 12, 2013 at 03:34:24PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Enable the LDO1_EN GPIO to the CODEC, which enables the device to show
> up on the I2C bus, before creating the regmap object. This guarantees
> that if devm_regmap_init_i2c() were to attempt to communicate with the
> device, it would be able to.

Is this an actual issue?  Unless specifically asked to the regmap code
really shouldn't be interacting with the hardware and the ordering the
code currently has is the standard one so if we're having problems then
a number of other drivers ought to be fixed.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] ASoC: rt5640: enable LDO1 before creating regmap

[Stephen Warren]

On 06/13/2013 03:30 AM, Mark Brown wrote:
> On Wed, Jun 12, 2013 at 03:34:24PM -0600, Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>> 
>> Enable the LDO1_EN GPIO to the CODEC, which enables the device to
>> show up on the I2C bus, before creating the regmap object. This
>> guarantees that if devm_regmap_init_i2c() were to attempt to
>> communicate with the device, it would be able to.
> 
> Is this an actual issue?  Unless specifically asked to the regmap
> code really shouldn't be interacting with the hardware and the
> ordering the code currently has is the standard one so if we're
> having problems then a number of other drivers ought to be fixed.

This doesn't solve a current failure. It just seems to me that one
shouldn't create the regmap object until it's actually possible to use it.

Re: [PATCH 2/2] ASoC: rt5640: enable LDO1 before creating regmap

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 623 bytes --]

On Thu, Jun 13, 2013 at 09:09:53AM -0600, Stephen Warren wrote:

> This doesn't solve a current failure. It just seems to me that one
> shouldn't create the regmap object until it's actually possible to use it.

That's true but one of the goals with regmap is to be able to just use
the cache with the device powered off (lots of ASoC operation relies on
that for example) - regmap itself shouldn't ever be forcing the device
to be powered on.  One could equally say you shouldn't think about using
the hardware until you've got a regmap object with which to interact
with it, which is clearly the more urgent priority! :P

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]