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

[Charles Keepax]

On Tue, Jul 23, 2019 at 04:21:41PM +0200, Cezary Rojewski wrote:
> 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)
> >>>+
> >>>+	n = madera_get_variable_u32_array(madera->dev, "cirrus,inmode",
> >>>+					  tmp, ARRAY_SIZE(tmp),
> >>>+					  MADERA_MAX_MUXED_CHANNELS);
> >>>+	if (n < 0)
> >>>+		return;
> >>
> >>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.
> 

Ah.. yes I follow. Yes this is intentional, all the DT fields in
question are optional so the driver should proceed if even if they
are corrupt or missing.

madera_prop_get_inmode checks the return value because additional
code is required to insert the values into the pdata structure, so
that code should be skipped in the case cirrus,inmode is not
present/fails. Most of the calls in madera_prop_get_pdata are
self-contained not requiring further handling other than reading
the data directly into the pdata structure. Uou can see the same
on the read of cirrus,out-mono the additional processing is
skipped in the case of an error.

Thanks,
Charles