Re: [PATCH] Don't clobber access methods when !regmap

[Pantelis Antoniou]

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@xxxxxxxxxxxxxxxxxxxxxxx>
> 
> 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);
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/