[alsa-devel] [PATCH v2] ASoC: madera: Read device tree configuration

[Cezary Rojewski]

On 2019-07-23 10:17, Charles Keepax wrote:
> On Tue, Jul 23, 2019 at 12:07:32AM +0200, Cezary Rojewski wrote:
>> On 2019-07-22 15:52, Charles Keepax wrote:
>>> +static void madera_prop_get_inmode(struct madera_priv *priv)
>>> +{
>>> +	struct madera *madera = priv->madera;
>>> +	struct madera_codec_pdata *pdata = &madera->pdata.codec;
>>> +	u32 tmp[MADERA_MAX_INPUT * MADERA_MAX_MUXED_CHANNELS];
>>> +	int n, i, in_idx, ch_idx;
>>> +
>>> +	BUILD_BUG_ON(ARRAY_SIZE(pdata->inmode) != MADERA_MAX_INPUT);
>>> +	BUILD_BUG_ON(ARRAY_SIZE(pdata->inmode[0]) != MADERA_MAX_MUXED_CHANNELS);
>>> +
>>> +	n = madera_get_variable_u32_array(madera->dev, "cirrus,inmode",
>>> +					  tmp, ARRAY_SIZE(tmp),
>>> +					  MADERA_MAX_MUXED_CHANNELS);
>>> +	if (n < 0)
>>> +		return;
>>> +
>>> +	in_idx = 0;
>>> +	ch_idx = 0;
>>> +	for (i = 0; i < n; ++i) {
>>> +		pdata->inmode[in_idx][ch_idx] = tmp[i];
>>> +
>>> +		if (++ch_idx == MADERA_MAX_MUXED_CHANNELS) {
>>> +			ch_idx = 0;
>>> +			++in_idx;
>>> +		}
>>> +	}
>>> +}
>>> +
>>> +static void madera_prop_get_pdata(struct madera_priv *priv)
>>> +{
>>> +	struct madera *madera = priv->madera;
>>> +	struct madera_codec_pdata *pdata = &madera->pdata.codec;
>>> +	u32 out_mono[ARRAY_SIZE(pdata->out_mono)];
>>> +	int i, n;
>>> +
>>> +	madera_prop_get_inmode(priv);
>>
>> Hmm, madera_get_variable_u32_array calls are not permissive within
>> madera_prop_get_inmode yet here they are. Is this intentional?
>>
> 
> Apologies but could you clarify what you mean by "not
> permissive"?
> 
> I can't see anything that would prevent the function from
> being called (indeed it builds and works), and the binding
> documentation does specify that this field can be of variable
> size.
> 
> Thanks,
> Charles

No worries. By "permissive" I described the usage of 
_get_variable_u32_array within madera_prop_get_pdata. In 
madera_prop_get_inmode you do care about the return value. In 
madera_prop_get_pdata however, you ignore all of them. From 
_get_variable_u32_array declaration, it seems function may fail.
Sometimes it's desired to be permissive, simply asking if that's 
intentional.

Czarek