[alsa-devel] [PATCH v2] ASoC: madera: Read device tree configuration
Read the configuration of the Madera ASoC driver from device tree.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com ---
Changes since v1: - Returned to using a helping in the madera driver itself, discussions around adding a generic helper indicated that all the error reporting would still need to be contained in the driver which constitutes the vast majority of the code in the helper anyway. - Updated the code to use the new device_property_count_u32 helper.
Thanks, Charles
sound/soc/codecs/madera.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+)
diff --git a/sound/soc/codecs/madera.c b/sound/soc/codecs/madera.c index 1b1be19a2f991..5f1e32a5a8556 100644 --- a/sound/soc/codecs/madera.c +++ b/sound/soc/codecs/madera.c @@ -300,6 +300,100 @@ int madera_free_overheat(struct madera_priv *priv) } EXPORT_SYMBOL_GPL(madera_free_overheat);
+static int madera_get_variable_u32_array(struct device *dev, + const char *propname, + u32 *dest, int n_max, + int multiple) +{ + int n, ret; + + n = device_property_count_u32(dev, propname); + if (n < 0) { + if (n == -EINVAL) + return 0; /* missing, ignore */ + + dev_warn(dev, "%s malformed (%d)\n", propname, n); + + return n; + } else if ((n % multiple) != 0) { + dev_warn(dev, "%s not a multiple of %d entries\n", + propname, multiple); + + return -EINVAL; + } + + if (n > n_max) + n = n_max; + + ret = device_property_read_u32_array(dev, propname, dest, n); + if (ret < 0) + return ret; + + return n; +} + +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); + + n = madera_get_variable_u32_array(madera->dev, "cirrus,out-mono", + out_mono, ARRAY_SIZE(out_mono), 1); + if (n > 0) + for (i = 0; i < n; ++i) + pdata->out_mono[i] = !!out_mono[i]; + + madera_get_variable_u32_array(madera->dev, + "cirrus,max-channels-clocked", + pdata->max_channels_clocked, + ARRAY_SIZE(pdata->max_channels_clocked), + 1); + + madera_get_variable_u32_array(madera->dev, "cirrus,pdm-fmt", + pdata->pdm_fmt, + ARRAY_SIZE(pdata->pdm_fmt), 1); + + madera_get_variable_u32_array(madera->dev, "cirrus,pdm-mute", + pdata->pdm_mute, + ARRAY_SIZE(pdata->pdm_mute), 1); + + madera_get_variable_u32_array(madera->dev, "cirrus,dmic-ref", + pdata->dmic_ref, + ARRAY_SIZE(pdata->dmic_ref), 1); +} + int madera_core_init(struct madera_priv *priv) { int i; @@ -308,6 +402,9 @@ int madera_core_init(struct madera_priv *priv) BUILD_BUG_ON(!madera_mixer_texts[MADERA_NUM_MIXER_INPUTS - 1]); BUILD_BUG_ON(!madera_mixer_values[MADERA_NUM_MIXER_INPUTS - 1]);
+ if (!dev_get_platdata(priv->madera->dev)) + madera_prop_get_pdata(priv); + mutex_init(&priv->rate_lock);
for (i = 0; i < MADERA_MAX_HP_OUTPUT; i++)
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);
- n = madera_get_variable_u32_array(madera->dev, "cirrus,out-mono",
out_mono, ARRAY_SIZE(out_mono), 1);- if (n > 0)
for (i = 0; i < n; ++i)pdata->out_mono[i] = !!out_mono[i];- madera_get_variable_u32_array(madera->dev,
"cirrus,max-channels-clocked",pdata->max_channels_clocked,ARRAY_SIZE(pdata->max_channels_clocked),1);- madera_get_variable_u32_array(madera->dev, "cirrus,pdm-fmt",
pdata->pdm_fmt,ARRAY_SIZE(pdata->pdm_fmt), 1);- madera_get_variable_u32_array(madera->dev, "cirrus,pdm-mute",
pdata->pdm_mute,ARRAY_SIZE(pdata->pdm_mute), 1);- madera_get_variable_u32_array(madera->dev, "cirrus,dmic-ref",
pdata->dmic_ref,ARRAY_SIZE(pdata->dmic_ref), 1);
Hmm, madera_get_variable_u32_array calls are not permissive within madera_prop_get_inmode yet here they are. Is this intentional?
+}
- int madera_core_init(struct madera_priv *priv) { int i;
@@ -308,6 +402,9 @@ int madera_core_init(struct madera_priv *priv) BUILD_BUG_ON(!madera_mixer_texts[MADERA_NUM_MIXER_INPUTS - 1]); BUILD_BUG_ON(!madera_mixer_values[MADERA_NUM_MIXER_INPUTS - 1]);
if (!dev_get_platdata(priv->madera->dev))
madera_prop_get_pdata(priv);mutex_init(&priv->rate_lock);
for (i = 0; i < MADERA_MAX_HP_OUTPUT; i++)
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
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
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
On Tue, Jul 23, 2019 at 04:01:25PM +0100, Charles Keepax wrote:
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.
If they're present but unparsable you should probably say something about that really. That's clearly different to optional.
On Tue, Jul 23, 2019 at 04:03:02PM +0100, Mark Brown wrote:
On Tue, Jul 23, 2019 at 04:01:25PM +0100, Charles Keepax wrote:
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.
If they're present but unparsable you should probably say something about that really. That's clearly different to optional.
The helper logs an error message itself since essentially the only difference in the messages is the name of the property, there is just no need to process the return value since we would take the same code path regardless of failure or success.
Thanks, Charles
The patch
ASoC: madera: Read device tree configuration
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 748fd07e2b9ca4132e3d2aae25395aedc4d1aee8 Mon Sep 17 00:00:00 2001
From: Charles Keepax ckeepax@opensource.cirrus.com Date: Mon, 22 Jul 2019 14:52:09 +0100 Subject: [PATCH] ASoC: madera: Read device tree configuration
Read the configuration of the Madera ASoC driver from device tree.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com Link: https://lore.kernel.org/r/20190722135209.30302-1-ckeepax@opensource.cirrus.c... Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/madera.c | 97 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+)
diff --git a/sound/soc/codecs/madera.c b/sound/soc/codecs/madera.c index 1b1be19a2f99..5f1e32a5a855 100644 --- a/sound/soc/codecs/madera.c +++ b/sound/soc/codecs/madera.c @@ -300,6 +300,100 @@ int madera_free_overheat(struct madera_priv *priv) } EXPORT_SYMBOL_GPL(madera_free_overheat);
+static int madera_get_variable_u32_array(struct device *dev, + const char *propname, + u32 *dest, int n_max, + int multiple) +{ + int n, ret; + + n = device_property_count_u32(dev, propname); + if (n < 0) { + if (n == -EINVAL) + return 0; /* missing, ignore */ + + dev_warn(dev, "%s malformed (%d)\n", propname, n); + + return n; + } else if ((n % multiple) != 0) { + dev_warn(dev, "%s not a multiple of %d entries\n", + propname, multiple); + + return -EINVAL; + } + + if (n > n_max) + n = n_max; + + ret = device_property_read_u32_array(dev, propname, dest, n); + if (ret < 0) + return ret; + + return n; +} + +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); + + n = madera_get_variable_u32_array(madera->dev, "cirrus,out-mono", + out_mono, ARRAY_SIZE(out_mono), 1); + if (n > 0) + for (i = 0; i < n; ++i) + pdata->out_mono[i] = !!out_mono[i]; + + madera_get_variable_u32_array(madera->dev, + "cirrus,max-channels-clocked", + pdata->max_channels_clocked, + ARRAY_SIZE(pdata->max_channels_clocked), + 1); + + madera_get_variable_u32_array(madera->dev, "cirrus,pdm-fmt", + pdata->pdm_fmt, + ARRAY_SIZE(pdata->pdm_fmt), 1); + + madera_get_variable_u32_array(madera->dev, "cirrus,pdm-mute", + pdata->pdm_mute, + ARRAY_SIZE(pdata->pdm_mute), 1); + + madera_get_variable_u32_array(madera->dev, "cirrus,dmic-ref", + pdata->dmic_ref, + ARRAY_SIZE(pdata->dmic_ref), 1); +} + int madera_core_init(struct madera_priv *priv) { int i; @@ -308,6 +402,9 @@ int madera_core_init(struct madera_priv *priv) BUILD_BUG_ON(!madera_mixer_texts[MADERA_NUM_MIXER_INPUTS - 1]); BUILD_BUG_ON(!madera_mixer_values[MADERA_NUM_MIXER_INPUTS - 1]);
+ if (!dev_get_platdata(priv->madera->dev)) + madera_prop_get_pdata(priv); + mutex_init(&priv->rate_lock);
for (i = 0; i < MADERA_MAX_HP_OUTPUT; i++)
participants (3)
-
Cezary Rojewski -
Charles Keepax -
Mark Brown