[PATCH 1/2] ASoC: topology: refine and log the header in the correct pass
The check (tplg->pass == le32_to_cpu(hdr->type)) makes no sense as it is comparing two different enums, refine the element loading functions, and log the information when the header is being parsed in the corresponding parsing pass.
Signed-off-by: Keyon Jie yang.jie@linux.intel.com --- sound/soc/soc-topology.c | 53 +++++++++++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 11 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 49875978a1ce..b3dae319c108 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -2636,6 +2636,17 @@ static int soc_tplg_manifest_load(struct soc_tplg *tplg, return ret; }
+int (*elem_load[])(struct soc_tplg *, struct snd_soc_tplg_hdr *) = { + [SOC_TPLG_PASS_MANIFEST] = soc_tplg_manifest_load, + [SOC_TPLG_PASS_VENDOR] = soc_tplg_vendor_load, + [SOC_TPLG_PASS_MIXER] = soc_tplg_kcontrol_elems_load, + [SOC_TPLG_PASS_WIDGET] = soc_tplg_dapm_widget_elems_load, + [SOC_TPLG_PASS_PCM_DAI] = soc_tplg_pcm_elems_load, + [SOC_TPLG_PASS_GRAPH] = soc_tplg_dapm_graph_elems_load, + [SOC_TPLG_PASS_BE_DAI] = soc_tplg_dai_elems_load, + [SOC_TPLG_PASS_LINK] = soc_tplg_link_elems_load, +}; + /* validate header magic, size and type */ static int soc_valid_header(struct soc_tplg *tplg, struct snd_soc_tplg_hdr *hdr) @@ -2685,19 +2696,31 @@ static int soc_valid_header(struct soc_tplg *tplg, return -EINVAL; }
- if (tplg->pass == le32_to_cpu(hdr->type)) + return 1; +} + +/* check and load the element for the current pass */ +static int soc_pass_load(struct soc_tplg *tplg, + struct snd_soc_tplg_hdr *hdr, + unsigned int hdr_pass) +{ + if (tplg->pass == hdr_pass) { dev_dbg(tplg->dev, "ASoC: Got 0x%x bytes of type %d version %d vendor %d at pass %d\n", hdr->payload_size, hdr->type, hdr->version, hdr->vendor_type, tplg->pass); + return elem_load[hdr_pass](tplg, hdr); + }
- return 1; + return 0; }
/* check header type and call appropriate handler */ static int soc_tplg_load_header(struct soc_tplg *tplg, struct snd_soc_tplg_hdr *hdr) { + unsigned int hdr_pass; + tplg->pos = tplg->hdr_pos + sizeof(struct snd_soc_tplg_hdr);
/* check for matching ID */ @@ -2711,27 +2734,35 @@ static int soc_tplg_load_header(struct soc_tplg *tplg, case SND_SOC_TPLG_TYPE_MIXER: case SND_SOC_TPLG_TYPE_ENUM: case SND_SOC_TPLG_TYPE_BYTES: - return soc_tplg_kcontrol_elems_load(tplg, hdr); + hdr_pass = SOC_TPLG_PASS_MIXER; + break; case SND_SOC_TPLG_TYPE_DAPM_GRAPH: - return soc_tplg_dapm_graph_elems_load(tplg, hdr); + hdr_pass = SOC_TPLG_PASS_GRAPH; + break; case SND_SOC_TPLG_TYPE_DAPM_WIDGET: - return soc_tplg_dapm_widget_elems_load(tplg, hdr); + hdr_pass = SOC_TPLG_PASS_WIDGET; + break; case SND_SOC_TPLG_TYPE_PCM: - return soc_tplg_pcm_elems_load(tplg, hdr); + hdr_pass = SOC_TPLG_PASS_PCM_DAI; + break; case SND_SOC_TPLG_TYPE_DAI: - return soc_tplg_dai_elems_load(tplg, hdr); + hdr_pass = SOC_TPLG_PASS_BE_DAI; + break; case SND_SOC_TPLG_TYPE_DAI_LINK: case SND_SOC_TPLG_TYPE_BACKEND_LINK: /* physical link configurations */ - return soc_tplg_link_elems_load(tplg, hdr); + hdr_pass = SOC_TPLG_PASS_LINK; + break; case SND_SOC_TPLG_TYPE_MANIFEST: - return soc_tplg_manifest_load(tplg, hdr); + hdr_pass = SOC_TPLG_PASS_MANIFEST; + break; default: /* bespoke vendor data object */ - return soc_tplg_vendor_load(tplg, hdr); + hdr_pass = SOC_TPLG_PASS_VENDOR; + break; }
- return 0; + return soc_pass_load(tplg, hdr, hdr_pass); }
/* process the topology file headers */
As we have check the 'pass' in the soc_elem_pass_load(), so no need to check it again in each specific elem_load function, at the same time, the tplg->pos will be reset to the next header base when the pass is mismatched, so the increasing of the tplg->pos in these cases made no sense. Here remove all of them.
Signed-off-by: Keyon Jie yang.jie@linux.intel.com --- sound/soc/soc-topology.c | 46 ++-------------------------------------- 1 file changed, 2 insertions(+), 44 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index b3dae319c108..682ffaa85e9e 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -246,8 +246,8 @@ static inline void soc_control_err(struct soc_tplg *tplg, }
/* pass vendor data to component driver for processing */ -static int soc_tplg_vendor_load_(struct soc_tplg *tplg, - struct snd_soc_tplg_hdr *hdr) +static int soc_tplg_vendor_load(struct soc_tplg *tplg, + struct snd_soc_tplg_hdr *hdr) { int ret = 0;
@@ -268,16 +268,6 @@ static int soc_tplg_vendor_load_(struct soc_tplg *tplg, return ret; }
-/* pass vendor data to component driver for processing */ -static int soc_tplg_vendor_load(struct soc_tplg *tplg, - struct snd_soc_tplg_hdr *hdr) -{ - if (tplg->pass != SOC_TPLG_PASS_VENDOR) - return 0; - - return soc_tplg_vendor_load_(tplg, hdr); -} - /* optionally pass new dynamic widget to component driver. This is mainly for * external widgets where we can assign private data/ops */ static int soc_tplg_widget_load(struct soc_tplg *tplg, @@ -1127,12 +1117,6 @@ static int soc_tplg_kcontrol_elems_load(struct soc_tplg *tplg, int ret; int i;
- if (tplg->pass != SOC_TPLG_PASS_MIXER) { - tplg->pos += le32_to_cpu(hdr->size) + - le32_to_cpu(hdr->payload_size); - return 0; - } - dev_dbg(tplg->dev, "ASoC: adding %d kcontrols at 0x%lx\n", hdr->count, soc_tplg_get_offset(tplg));
@@ -1204,14 +1188,6 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg,
count = le32_to_cpu(hdr->count);
- if (tplg->pass != SOC_TPLG_PASS_GRAPH) { - tplg->pos += - le32_to_cpu(hdr->size) + - le32_to_cpu(hdr->payload_size); - - return 0; - } - if (soc_tplg_check_elem_count(tplg, sizeof(struct snd_soc_tplg_dapm_graph_elem), count, le32_to_cpu(hdr->payload_size), "graph")) { @@ -1741,9 +1717,6 @@ static int soc_tplg_dapm_widget_elems_load(struct soc_tplg *tplg,
count = le32_to_cpu(hdr->count);
- if (tplg->pass != SOC_TPLG_PASS_WIDGET) - return 0; - dev_dbg(tplg->dev, "ASoC: adding %d DAPM widgets\n", count);
for (i = 0; i < count; i++) { @@ -2101,9 +2074,6 @@ static int soc_tplg_pcm_elems_load(struct soc_tplg *tplg,
count = le32_to_cpu(hdr->count);
- if (tplg->pass != SOC_TPLG_PASS_PCM_DAI) - return 0; - /* check the element size and count */ pcm = (struct snd_soc_tplg_pcm *)tplg->pos; size = le32_to_cpu(pcm->size); @@ -2386,12 +2356,6 @@ static int soc_tplg_link_elems_load(struct soc_tplg *tplg,
count = le32_to_cpu(hdr->count);
- if (tplg->pass != SOC_TPLG_PASS_LINK) { - tplg->pos += le32_to_cpu(hdr->size) + - le32_to_cpu(hdr->payload_size); - return 0; - } - /* check the element size and count */ link = (struct snd_soc_tplg_link_config *)tplg->pos; size = le32_to_cpu(link->size); @@ -2528,9 +2492,6 @@ static int soc_tplg_dai_elems_load(struct soc_tplg *tplg,
count = le32_to_cpu(hdr->count);
- if (tplg->pass != SOC_TPLG_PASS_BE_DAI) - return 0; - /* config the existing BE DAIs */ for (i = 0; i < count; i++) { dai = (struct snd_soc_tplg_dai *)tplg->pos; @@ -2610,9 +2571,6 @@ static int soc_tplg_manifest_load(struct soc_tplg *tplg, bool abi_match; int ret = 0;
- if (tplg->pass != SOC_TPLG_PASS_MANIFEST) - return 0; - manifest = (struct snd_soc_tplg_manifest *)tplg->pos;
/* check ABI version by size, create a new manifest if abi not match */
On 5/12/20 1:23 PM, Keyon Jie wrote:
As we have check the 'pass' in the soc_elem_pass_load(), so no need to check it again in each specific elem_load function, at the same time, the tplg->pos will be reset to the next header base when the pass is mismatched, so the increasing of the tplg->pos in these cases made no sense. Here remove all of them.
Sorry Keyon, I am not comfortable with the changes. This started as a removal of a debug log and we ended-up changing the execution flow in one of the more complex parts of the ASoC framework. I spent 30mn looking at the code and I don't have a feeling all corner cases are covered. The bar for core changes is higher, it'd help to have additional explanations or evidence that your proposal was tested extensively. You may be right but we should be careful with topology changes.
Signed-off-by: Keyon Jie yang.jie@linux.intel.com
sound/soc/soc-topology.c | 46 ++-------------------------------------- 1 file changed, 2 insertions(+), 44 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index b3dae319c108..682ffaa85e9e 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -246,8 +246,8 @@ static inline void soc_control_err(struct soc_tplg *tplg, }
/* pass vendor data to component driver for processing */ -static int soc_tplg_vendor_load_(struct soc_tplg *tplg,
- struct snd_soc_tplg_hdr *hdr)
+static int soc_tplg_vendor_load(struct soc_tplg *tplg,
struct snd_soc_tplg_hdr *hdr)@@ -268,16 +268,6 @@ static int soc_tplg_vendor_load_(struct soc_tplg *tplg, return ret; }
-/* pass vendor data to component driver for processing */ -static int soc_tplg_vendor_load(struct soc_tplg *tplg,
- struct snd_soc_tplg_hdr *hdr)
-{
- if (tplg->pass != SOC_TPLG_PASS_VENDOR)
return 0;- return soc_tplg_vendor_load_(tplg, hdr);
-}
- /* optionally pass new dynamic widget to component driver. This is mainly for
static int soc_tplg_widget_load(struct soc_tplg *tplg,
- external widgets where we can assign private data/ops */
@@ -1127,12 +1117,6 @@ static int soc_tplg_kcontrol_elems_load(struct soc_tplg *tplg, int ret; int i;
- if (tplg->pass != SOC_TPLG_PASS_MIXER) {
tplg->pos += le32_to_cpu(hdr->size) +le32_to_cpu(hdr->payload_size);return 0;- }
- dev_dbg(tplg->dev, "ASoC: adding %d kcontrols at 0x%lx\n", hdr->count, soc_tplg_get_offset(tplg));
@@ -1204,14 +1188,6 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg,
count = le32_to_cpu(hdr->count);
- if (tplg->pass != SOC_TPLG_PASS_GRAPH) {
tplg->pos +=le32_to_cpu(hdr->size) +le32_to_cpu(hdr->payload_size);return 0;- }
- if (soc_tplg_check_elem_count(tplg, sizeof(struct snd_soc_tplg_dapm_graph_elem), count, le32_to_cpu(hdr->payload_size), "graph")) {
@@ -1741,9 +1717,6 @@ static int soc_tplg_dapm_widget_elems_load(struct soc_tplg *tplg,
count = le32_to_cpu(hdr->count);
if (tplg->pass != SOC_TPLG_PASS_WIDGET)
return 0;dev_dbg(tplg->dev, "ASoC: adding %d DAPM widgets\n", count);
for (i = 0; i < count; i++) {
@@ -2101,9 +2074,6 @@ static int soc_tplg_pcm_elems_load(struct soc_tplg *tplg,
count = le32_to_cpu(hdr->count);
- if (tplg->pass != SOC_TPLG_PASS_PCM_DAI)
return 0;- /* check the element size and count */ pcm = (struct snd_soc_tplg_pcm *)tplg->pos; size = le32_to_cpu(pcm->size);
@@ -2386,12 +2356,6 @@ static int soc_tplg_link_elems_load(struct soc_tplg *tplg,
count = le32_to_cpu(hdr->count);
- if (tplg->pass != SOC_TPLG_PASS_LINK) {
tplg->pos += le32_to_cpu(hdr->size) +le32_to_cpu(hdr->payload_size);return 0;- }
- /* check the element size and count */ link = (struct snd_soc_tplg_link_config *)tplg->pos; size = le32_to_cpu(link->size);
@@ -2528,9 +2492,6 @@ static int soc_tplg_dai_elems_load(struct soc_tplg *tplg,
count = le32_to_cpu(hdr->count);
- if (tplg->pass != SOC_TPLG_PASS_BE_DAI)
return 0;- /* config the existing BE DAIs */ for (i = 0; i < count; i++) { dai = (struct snd_soc_tplg_dai *)tplg->pos;
@@ -2610,9 +2571,6 @@ static int soc_tplg_manifest_load(struct soc_tplg *tplg, bool abi_match; int ret = 0;
if (tplg->pass != SOC_TPLG_PASS_MANIFEST)
return 0;manifest = (struct snd_soc_tplg_manifest *)tplg->pos;
/* check ABI version by size, create a new manifest if abi not match */
On 5/13/20 3:40 AM, Pierre-Louis Bossart wrote:
On 5/12/20 1:23 PM, Keyon Jie wrote:
As we have check the 'pass' in the soc_elem_pass_load(), so no need to check it again in each specific elem_load function, at the same time, the tplg->pos will be reset to the next header base when the pass is mismatched, so the increasing of the tplg->pos in these cases made no sense. Here remove all of them.
Sorry Keyon, I am not comfortable with the changes. This started as a removal of a debug log and we ended-up changing the execution flow in one of the more complex parts of the ASoC framework. I spent 30mn looking at the code and I don't have a feeling all corner cases are covered. The bar for core changes is higher, it'd help to have additional explanations or evidence that your proposal was tested extensively. You may be right but we should be careful with topology changes.
Maybe I should send these 2 patches separately, this 2nd one is actually fix for a long existed issue and it is exposed to me when writing the 1st commit: 1. the check of "if (tplg->pass != SOC_TPLG_PASS_xxx" was good and it acted as the correct filter of the pass, but 2. the increment "tplg->pos += le32_to_cpu(hdr->size) + le32_to_cpu(hdr->payload_size);" was actually wrong when it the pass is unmatched in #1, it should be "tplg->pos += le32_to_cpu(hdr->payload_size);" as the "hdr->size" was already calculated in the upper layer in soc_tplg_load_header(): "tplg->pos = tplg->hdr_pos + sizeof(struct snd_soc_tplg_hdr);". 3. the exited mistake described in #2 didn't triggered any real problems as the "tplg->pos" will be reset to new value in soc_tplg_load_header() when the next header comes, so increasing the "tplg->pos" here at the end of the current header check actually make no sense, we can just remove them.
The code changes here might make you nervous but the logic here is not that complicated, if you rule out the issue I mentioned above in #2 and #3, the remained changes here is only removing the checks "if (tplg->pass != SOC_TPLG_PASS_xxx" as they are already covered by the 1st commit.
I have tested these changes with 3 typical SOF topology files (nocodec, tgl i2s + dsm, hda-generic) and it works all fine there and the correct header information are logged with the changes, agree we should test more extensively and widely, so tests on SST driver are very welcomed.
So in short, I did these changes together since I am already here, but if you and Mark preferred, I can split out the fix for the "tplg->pos" increment and send it ahead.
Thanks, ~Keyon
Signed-off-by: Keyon Jie yang.jie@linux.intel.com
sound/soc/soc-topology.c | 46 ++-------------------------------------- 1 file changed, 2 insertions(+), 44 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index b3dae319c108..682ffaa85e9e 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -246,8 +246,8 @@ static inline void soc_control_err(struct soc_tplg *tplg, } /* pass vendor data to component driver for processing */ -static int soc_tplg_vendor_load_(struct soc_tplg *tplg, - struct snd_soc_tplg_hdr *hdr) +static int soc_tplg_vendor_load(struct soc_tplg *tplg, + struct snd_soc_tplg_hdr *hdr) { int ret = 0; @@ -268,16 +268,6 @@ static int soc_tplg_vendor_load_(struct soc_tplg *tplg, return ret; } -/* pass vendor data to component driver for processing */ -static int soc_tplg_vendor_load(struct soc_tplg *tplg, - struct snd_soc_tplg_hdr *hdr) -{ - if (tplg->pass != SOC_TPLG_PASS_VENDOR) - return 0;
- return soc_tplg_vendor_load_(tplg, hdr); -}
/* optionally pass new dynamic widget to component driver. This is mainly for * external widgets where we can assign private data/ops */ static int soc_tplg_widget_load(struct soc_tplg *tplg, @@ -1127,12 +1117,6 @@ static int soc_tplg_kcontrol_elems_load(struct soc_tplg *tplg, int ret; int i; - if (tplg->pass != SOC_TPLG_PASS_MIXER) { - tplg->pos += le32_to_cpu(hdr->size) + - le32_to_cpu(hdr->payload_size); - return 0; - }
dev_dbg(tplg->dev, "ASoC: adding %d kcontrols at 0x%lx\n", hdr->count, soc_tplg_get_offset(tplg)); @@ -1204,14 +1188,6 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg, count = le32_to_cpu(hdr->count); - if (tplg->pass != SOC_TPLG_PASS_GRAPH) { - tplg->pos += - le32_to_cpu(hdr->size) + - le32_to_cpu(hdr->payload_size);
- return 0; - }
if (soc_tplg_check_elem_count(tplg, sizeof(struct snd_soc_tplg_dapm_graph_elem), count, le32_to_cpu(hdr->payload_size), "graph")) { @@ -1741,9 +1717,6 @@ static int soc_tplg_dapm_widget_elems_load(struct soc_tplg *tplg, count = le32_to_cpu(hdr->count); - if (tplg->pass != SOC_TPLG_PASS_WIDGET) - return 0;
dev_dbg(tplg->dev, "ASoC: adding %d DAPM widgets\n", count); for (i = 0; i < count; i++) { @@ -2101,9 +2074,6 @@ static int soc_tplg_pcm_elems_load(struct soc_tplg *tplg, count = le32_to_cpu(hdr->count); - if (tplg->pass != SOC_TPLG_PASS_PCM_DAI) - return 0;
/* check the element size and count */ pcm = (struct snd_soc_tplg_pcm *)tplg->pos; size = le32_to_cpu(pcm->size); @@ -2386,12 +2356,6 @@ static int soc_tplg_link_elems_load(struct soc_tplg *tplg, count = le32_to_cpu(hdr->count); - if (tplg->pass != SOC_TPLG_PASS_LINK) { - tplg->pos += le32_to_cpu(hdr->size) + - le32_to_cpu(hdr->payload_size); - return 0; - }
/* check the element size and count */ link = (struct snd_soc_tplg_link_config *)tplg->pos; size = le32_to_cpu(link->size); @@ -2528,9 +2492,6 @@ static int soc_tplg_dai_elems_load(struct soc_tplg *tplg, count = le32_to_cpu(hdr->count); - if (tplg->pass != SOC_TPLG_PASS_BE_DAI) - return 0;
/* config the existing BE DAIs */ for (i = 0; i < count; i++) { dai = (struct snd_soc_tplg_dai *)tplg->pos; @@ -2610,9 +2571,6 @@ static int soc_tplg_manifest_load(struct soc_tplg *tplg, bool abi_match; int ret = 0; - if (tplg->pass != SOC_TPLG_PASS_MANIFEST) - return 0;
manifest = (struct snd_soc_tplg_manifest *)tplg->pos; /* check ABI version by size, create a new manifest if abi not match */
On 5/13/20 10:36 AM, Keyon Jie wrote:
On 5/13/20 3:40 AM, Pierre-Louis Bossart wrote:
On 5/12/20 1:23 PM, Keyon Jie wrote:
As we have check the 'pass' in the soc_elem_pass_load(), so no need to check it again in each specific elem_load function, at the same time, the tplg->pos will be reset to the next header base when the pass is mismatched, so the increasing of the tplg->pos in these cases made no sense. Here remove all of them.
Sorry Keyon, I am not comfortable with the changes. This started as a removal of a debug log and we ended-up changing the execution flow in one of the more complex parts of the ASoC framework. I spent 30mn looking at the code and I don't have a feeling all corner cases are covered. The bar for core changes is higher, it'd help to have additional explanations or evidence that your proposal was tested extensively. You may be right but we should be careful with topology changes.
Maybe I should send these 2 patches separately, this 2nd one is actually fix for a long existed issue and it is exposed to me when writing the 1st commit:
- the check of "if (tplg->pass != SOC_TPLG_PASS_xxx" was good and it
acted as the correct filter of the pass, but 2. the increment "tplg->pos += le32_to_cpu(hdr->size) + le32_to_cpu(hdr->payload_size);" was actually wrong when it the pass is unmatched in #1, it should be "tplg->pos += le32_to_cpu(hdr->payload_size);" as the "hdr->size" was already calculated in the upper layer in soc_tplg_load_header(): "tplg->pos = tplg->hdr_pos + sizeof(struct snd_soc_tplg_hdr);". 3. the exited mistake described in #2 didn't triggered any real problems as the "tplg->pos" will be reset to new value in soc_tplg_load_header() when the next header comes, so increasing the "tplg->pos" here at the end of the current header check actually make no sense, we can just remove them.
The code changes here might make you nervous but the logic here is not that complicated, if you rule out the issue I mentioned above in #2 and #3, the remained changes here is only removing the checks "if (tplg->pass != SOC_TPLG_PASS_xxx" as they are already covered by the 1st commit.
I have tested these changes with 3 typical SOF topology files (nocodec, tgl i2s + dsm, hda-generic) and it works all fine there and the correct header information are logged with the changes, agree we should test more extensively and widely, so tests on SST driver are very welcomed.
+ Cezary from the SST driver side.
Hi Cezary, can you help to check this from the SST driver side?
Thanks, ~Keyon
So in short, I did these changes together since I am already here, but if you and Mark preferred, I can split out the fix for the "tplg->pos" increment and send it ahead.
Thanks, ~Keyon
Signed-off-by: Keyon Jie yang.jie@linux.intel.com
sound/soc/soc-topology.c | 46 ++-------------------------------------- 1 file changed, 2 insertions(+), 44 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index b3dae319c108..682ffaa85e9e 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -246,8 +246,8 @@ static inline void soc_control_err(struct soc_tplg *tplg, } /* pass vendor data to component driver for processing */ -static int soc_tplg_vendor_load_(struct soc_tplg *tplg, - struct snd_soc_tplg_hdr *hdr) +static int soc_tplg_vendor_load(struct soc_tplg *tplg, + struct snd_soc_tplg_hdr *hdr) { int ret = 0; @@ -268,16 +268,6 @@ static int soc_tplg_vendor_load_(struct soc_tplg *tplg, return ret; } -/* pass vendor data to component driver for processing */ -static int soc_tplg_vendor_load(struct soc_tplg *tplg, - struct snd_soc_tplg_hdr *hdr) -{ - if (tplg->pass != SOC_TPLG_PASS_VENDOR) - return 0;
- return soc_tplg_vendor_load_(tplg, hdr); -}
/* optionally pass new dynamic widget to component driver. This is mainly for * external widgets where we can assign private data/ops */ static int soc_tplg_widget_load(struct soc_tplg *tplg, @@ -1127,12 +1117,6 @@ static int soc_tplg_kcontrol_elems_load(struct soc_tplg *tplg, int ret; int i; - if (tplg->pass != SOC_TPLG_PASS_MIXER) { - tplg->pos += le32_to_cpu(hdr->size) + - le32_to_cpu(hdr->payload_size); - return 0; - }
dev_dbg(tplg->dev, "ASoC: adding %d kcontrols at 0x%lx\n", hdr->count, soc_tplg_get_offset(tplg)); @@ -1204,14 +1188,6 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg, count = le32_to_cpu(hdr->count); - if (tplg->pass != SOC_TPLG_PASS_GRAPH) { - tplg->pos += - le32_to_cpu(hdr->size) + - le32_to_cpu(hdr->payload_size);
- return 0; - }
if (soc_tplg_check_elem_count(tplg, sizeof(struct snd_soc_tplg_dapm_graph_elem), count, le32_to_cpu(hdr->payload_size), "graph")) { @@ -1741,9 +1717,6 @@ static int soc_tplg_dapm_widget_elems_load(struct soc_tplg *tplg, count = le32_to_cpu(hdr->count); - if (tplg->pass != SOC_TPLG_PASS_WIDGET) - return 0;
dev_dbg(tplg->dev, "ASoC: adding %d DAPM widgets\n", count); for (i = 0; i < count; i++) { @@ -2101,9 +2074,6 @@ static int soc_tplg_pcm_elems_load(struct soc_tplg *tplg, count = le32_to_cpu(hdr->count); - if (tplg->pass != SOC_TPLG_PASS_PCM_DAI) - return 0;
/* check the element size and count */ pcm = (struct snd_soc_tplg_pcm *)tplg->pos; size = le32_to_cpu(pcm->size); @@ -2386,12 +2356,6 @@ static int soc_tplg_link_elems_load(struct soc_tplg *tplg, count = le32_to_cpu(hdr->count); - if (tplg->pass != SOC_TPLG_PASS_LINK) { - tplg->pos += le32_to_cpu(hdr->size) + - le32_to_cpu(hdr->payload_size); - return 0; - }
/* check the element size and count */ link = (struct snd_soc_tplg_link_config *)tplg->pos; size = le32_to_cpu(link->size); @@ -2528,9 +2492,6 @@ static int soc_tplg_dai_elems_load(struct soc_tplg *tplg, count = le32_to_cpu(hdr->count); - if (tplg->pass != SOC_TPLG_PASS_BE_DAI) - return 0;
/* config the existing BE DAIs */ for (i = 0; i < count; i++) { dai = (struct snd_soc_tplg_dai *)tplg->pos; @@ -2610,9 +2571,6 @@ static int soc_tplg_manifest_load(struct soc_tplg *tplg, bool abi_match; int ret = 0; - if (tplg->pass != SOC_TPLG_PASS_MANIFEST) - return 0;
manifest = (struct snd_soc_tplg_manifest *)tplg->pos; /* check ABI version by size, create a new manifest if abi not match */
Hi Keyon,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on asoc/for-next] [also build test WARNING on v5.7-rc5 next-20200512] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Keyon-Jie/ASoC-topology-refine-and-... base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next reproduce: # apt-get install sparse # sparse version: v0.6.1-191-gc51a0382-dirty make ARCH=x86_64 allmodconfig make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag as appropriate Reported-by: kbuild test robot lkp@intel.com
sparse warnings: (new ones prefixed by >>)
sound/soc/soc-topology.c:2639:5: sparse: sparse: symbol 'elem_load' was not declared. Should it be static?
Please review and possibly fold the followup patch.
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Signed-off-by: kbuild test robot lkp@intel.com --- soc-topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index b3dae319c1086..70c0ff7ce13ff 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -2636,7 +2636,7 @@ static int soc_tplg_manifest_load(struct soc_tplg *tplg, return ret; }
-int (*elem_load[])(struct soc_tplg *, struct snd_soc_tplg_hdr *) = { +static int (*elem_load[])(struct soc_tplg *, struct snd_soc_tplg_hdr *) = { [SOC_TPLG_PASS_MANIFEST] = soc_tplg_manifest_load, [SOC_TPLG_PASS_VENDOR] = soc_tplg_vendor_load, [SOC_TPLG_PASS_MIXER] = soc_tplg_kcontrol_elems_load,
participants (3)
-
kbuild test robot -
Keyon Jie -
Pierre-Louis Bossart