linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] Don't clobber access methods when !regmap
  2012-09-06 13:45 [PATCH] Don't clobber access methods when !regmap Pantelis Antoniou
@ 2012-09-05 16:36 ` Lars-Peter Clausen
  2012-09-05 16:43   ` Pantelis Antoniou
  0 siblings, 1 reply; 5+ messages in thread
From: Lars-Peter Clausen @ 2012-09-05 16:36 UTC (permalink / raw)
  To: Pantelis Antoniou; +Cc: linux-kernel, patches, Mark Brown, linux-omap

On 09/06/2012 03:45 PM, Pantelis Antoniou wrote:
> A snd-soc driver that doesn't support regmap blow up horribly
> when you assume that regmap is available. Fix it by marking
> the driver as not supporting regmap & not clobbering the codec
> access methods.
> 
> This is immediately noticeable on the beagleboard where we crash,
> since we might have REGMAP enabled, but it doesn't mean that the
> omap driver uses it.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>

But calling snd_soc_codec_set_cache_io sort of implies that you are using
regmap. If you are not using regmap your codec should not call
snd_soc_codec_set_cache_io.

- Lars

> 
> ---
>  sound/soc/soc-io.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/soc-io.c b/sound/soc/soc-io.c
> index 29183ef..4e5b4ae 100644
> --- a/sound/soc/soc-io.c
> +++ b/sound/soc/soc-io.c
> @@ -117,9 +117,6 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
>  	int ret;
>  
>  	memset(&config, 0, sizeof(config));
> -	codec->write = hw_write;
> -	codec->read = hw_read;
> -	codec->bulk_write_raw = snd_soc_hw_bulk_write_raw;
>  
>  	config.reg_bits = addr_bits;
>  	config.val_bits = data_bits;
> @@ -151,7 +148,9 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
>  			 * multiples */
>  			if (ret > 0)
>  				codec->val_bytes = ret;
> -		}
> +		} else
> +			codec->using_regmap = false;
> +
>  		break;
>  
>  	default:
> @@ -161,6 +160,13 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
>  	if (IS_ERR(codec->control_data))
>  		return PTR_ERR(codec->control_data);
>  
> +	/* only when using regmap; don't modify unconditionally */
> +	if (codec->using_regmap) {
> +		codec->write = hw_write;
> +		codec->read = hw_read;
> +		codec->bulk_write_raw = snd_soc_hw_bulk_write_raw;
> +	}
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(snd_soc_codec_set_cache_io);


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

* Re: [PATCH] Don't clobber access methods when !regmap
  2012-09-05 16:36 ` Lars-Peter Clausen
@ 2012-09-05 16:43   ` Pantelis Antoniou
  2012-09-06 14:01     ` Peter Ujfalusi
  0 siblings, 1 reply; 5+ messages in thread
From: Pantelis Antoniou @ 2012-09-05 16:43 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: linux-kernel, patches, Mark Brown, linux-omap

Hi Lars,

On Sep 5, 2012, at 7:36 PM, Lars-Peter Clausen wrote:

> On 09/06/2012 03:45 PM, Pantelis Antoniou wrote:
>> A snd-soc driver that doesn't support regmap blow up horribly
>> when you assume that regmap is available. Fix it by marking
>> the driver as not supporting regmap & not clobbering the codec
>> access methods.
>> 
>> This is immediately noticeable on the beagleboard where we crash,
>> since we might have REGMAP enabled, but it doesn't mean that the
>> omap driver uses it.
>> 
>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> 
> But calling snd_soc_codec_set_cache_io sort of implies that you are using
> regmap. If you are not using regmap your codec should not call
> snd_soc_codec_set_cache_io.
> 
> - Lars
> 

The driver in question (twl4030.c) does not call snd_soc_codec_set_cache_io().
However the snd-soc core does call it in sound/soc/soc-core.c in soc_probe_codec.

I do see that there's a commit 98d3088e534a2a61f6690b5426909b0c3b57a785 that is
fixing it in a different manner.

> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index f219b2f..f81c597 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1096,7 +1096,7 @@ static int soc_probe_codec(struct snd_soc_card *card,
>         }
>  
>         /* If the driver didn't set I/O up try regmap */
> -       if (!codec->control_data)
> +       if (!codec->write && dev_get_regmap(codec->dev, NULL))
>                 snd_soc_codec_set_cache_io(codec, 0, 0, SND_SOC_REGMAP);
>  
>         if (driver->controls)
> 
> 

Originally it was call unconditionally, but now it should work. Let me check this
and I'll report back.

Regards

-- Pantelis


>> 
>> ---
>> sound/soc/soc-io.c | 14 ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>> 
>> diff --git a/sound/soc/soc-io.c b/sound/soc/soc-io.c
>> index 29183ef..4e5b4ae 100644
>> --- a/sound/soc/soc-io.c
>> +++ b/sound/soc/soc-io.c
>> @@ -117,9 +117,6 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
>> 	int ret;
>> 
>> 	memset(&config, 0, sizeof(config));
>> -	codec->write = hw_write;
>> -	codec->read = hw_read;
>> -	codec->bulk_write_raw = snd_soc_hw_bulk_write_raw;
>> 
>> 	config.reg_bits = addr_bits;
>> 	config.val_bits = data_bits;
>> @@ -151,7 +148,9 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
>> 			 * multiples */
>> 			if (ret > 0)
>> 				codec->val_bytes = ret;
>> -		}
>> +		} else
>> +			codec->using_regmap = false;
>> +
>> 		break;
>> 
>> 	default:
>> @@ -161,6 +160,13 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
>> 	if (IS_ERR(codec->control_data))
>> 		return PTR_ERR(codec->control_data);
>> 
>> +	/* only when using regmap; don't modify unconditionally */
>> +	if (codec->using_regmap) {
>> +		codec->write = hw_write;
>> +		codec->read = hw_read;
>> +		codec->bulk_write_raw = snd_soc_hw_bulk_write_raw;
>> +	}
>> +
>> 	return 0;
>> }
>> EXPORT_SYMBOL_GPL(snd_soc_codec_set_cache_io);
> 


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

* [PATCH] Don't clobber access methods when !regmap
@ 2012-09-06 13:45 Pantelis Antoniou
  2012-09-05 16:36 ` Lars-Peter Clausen
  0 siblings, 1 reply; 5+ messages in thread
From: Pantelis Antoniou @ 2012-09-06 13:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: patches, Mark Brown, linux-omap, Pantelis Antoniou

A snd-soc driver that doesn't support regmap blow up horribly
when you assume that regmap is available. Fix it by marking
the driver as not supporting regmap & not clobbering the codec
access methods.

This is immediately noticeable on the beagleboard where we crash,
since we might have REGMAP enabled, but it doesn't mean that the
omap driver uses it.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>

---
 sound/soc/soc-io.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/sound/soc/soc-io.c b/sound/soc/soc-io.c
index 29183ef..4e5b4ae 100644
--- a/sound/soc/soc-io.c
+++ b/sound/soc/soc-io.c
@@ -117,9 +117,6 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
 	int ret;
 
 	memset(&config, 0, sizeof(config));
-	codec->write = hw_write;
-	codec->read = hw_read;
-	codec->bulk_write_raw = snd_soc_hw_bulk_write_raw;
 
 	config.reg_bits = addr_bits;
 	config.val_bits = data_bits;
@@ -151,7 +148,9 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
 			 * multiples */
 			if (ret > 0)
 				codec->val_bytes = ret;
-		}
+		} else
+			codec->using_regmap = false;
+
 		break;
 
 	default:
@@ -161,6 +160,13 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
 	if (IS_ERR(codec->control_data))
 		return PTR_ERR(codec->control_data);
 
+	/* only when using regmap; don't modify unconditionally */
+	if (codec->using_regmap) {
+		codec->write = hw_write;
+		codec->read = hw_read;
+		codec->bulk_write_raw = snd_soc_hw_bulk_write_raw;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(snd_soc_codec_set_cache_io);
-- 
1.7.12


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

* Re: [PATCH] Don't clobber access methods when !regmap
  2012-09-05 16:43   ` Pantelis Antoniou
@ 2012-09-06 14:01     ` Peter Ujfalusi
  2012-09-06 14:02       ` Pantelis Antoniou
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Ujfalusi @ 2012-09-06 14:01 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: Lars-Peter Clausen, linux-kernel, patches, Mark Brown, linux-omap

Hi,

On 09/05/2012 07:43 PM, Pantelis Antoniou wrote:
> The driver in question (twl4030.c) does not call snd_soc_codec_set_cache_io().
> However the snd-soc core does call it in sound/soc/soc-core.c in soc_probe_codec.
> 
> I do see that there's a commit 98d3088e534a2a61f6690b5426909b0c3b57a785 that is
> fixing it in a different manner.

I also noticed this (with 3.6-rc1 I think).
The commit you mention does fix it so we do not have issue anymore.
I sent two patches related to this:
http://mailman.alsa-project.org/pipermail/alsa-devel/2012-August/053864.html
http://mailman.alsa-project.org/pipermail/alsa-devel/2012-August/053876.html

Mark took a different route which gives us the commit you mentioned:
http://mailman.alsa-project.org/pipermail/alsa-devel/2012-August/053889.html

Regards,
Péter

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

* Re: [PATCH] Don't clobber access methods when !regmap
  2012-09-06 14:01     ` Peter Ujfalusi
@ 2012-09-06 14:02       ` Pantelis Antoniou
  0 siblings, 0 replies; 5+ messages in thread
From: Pantelis Antoniou @ 2012-09-06 14:02 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Lars-Peter Clausen, linux-kernel, patches, Mark Brown, linux-omap

Hi,

On Sep 6, 2012, at 5:01 PM, Peter Ujfalusi wrote:

> Hi,
> 
> On 09/05/2012 07:43 PM, Pantelis Antoniou wrote:
>> The driver in question (twl4030.c) does not call snd_soc_codec_set_cache_io().
>> However the snd-soc core does call it in sound/soc/soc-core.c in soc_probe_codec.
>> 
>> I do see that there's a commit 98d3088e534a2a61f6690b5426909b0c3b57a785 that is
>> fixing it in a different manner.
> 
> I also noticed this (with 3.6-rc1 I think).
> The commit you mention does fix it so we do not have issue anymore.
> I sent two patches related to this:
> http://mailman.alsa-project.org/pipermail/alsa-devel/2012-August/053864.html
> http://mailman.alsa-project.org/pipermail/alsa-devel/2012-August/053876.html
> 
> Mark took a different route which gives us the commit you mentioned:
> http://mailman.alsa-project.org/pipermail/alsa-devel/2012-August/053889.html
> 
> Regards,
> Péter

Quite similar to my patches. They're not needed now, since I have verified that
the issue is fixed.

Regards

-- Pantelis


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

end of thread, other threads:[~2012-09-06 14:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-06 13:45 [PATCH] Don't clobber access methods when !regmap Pantelis Antoniou
2012-09-05 16:36 ` Lars-Peter Clausen
2012-09-05 16:43   ` Pantelis Antoniou
2012-09-06 14:01     ` Peter Ujfalusi
2012-09-06 14:02       ` Pantelis Antoniou

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).