[PATCH] ASoC: topology: log all headers being parsed

[Keyon Jie]

On 5/12/20 2:38 AM, Pierre-Louis Bossart wrote:
> Kind reminder to always CC: maintainers....

Thanks for reminding.

> 
> On 5/9/20 3:22 AM, Keyon Jie wrote:
>> The check (tplg->pass == le32_to_cpu(hdr->type)) makes no sense as it is
>> comparing two different enums, remove the check and log all headers that
>> being parsed.
> 
> It's true that the TYPE_MIXER should probably not be compared to 
> TPLG_PASS_MANIFEST, but one would think that e.g. the TYPE_MIXER is 
> handled in the TPLG_PASS_MIXER, no? and likewise that in the 
> TPLG_PASS_MANIFEST all TPLG_TYPE_MANIFEST are handled?

Yes, all TYPE_MIXER/ENUM/BYTES are handled in the TPLG_PASS_MIXER, and 
in the TPLG_PASS_MANIFEST all TPLG_TYPE_MANIFEST are handled, but all 
these are guaranteed and more details are logged inside the 
corresponding xxx_load(), at here it is only checking the validity of 
the header, I think we can log all headers for debug purpose? Imagine 
that there might be some headers won't be parsed by any xxx_load().

> 
> Shouldn't you add a switch case to reconcile the two lists instead of 
> removing the test altogether?

I can add a function to reconcile the two list, but since we already 
have "tplg->pass != SOC_TPLG_PASS_xxx" check in each xxx_load(), do you 
suggest to moving the logging inside these xxx_load() to reduce the 
redundant check?

Thanks,
~Keyon

> 
> #define SND_SOC_TPLG_TYPE_MIXER        1
> #define SND_SOC_TPLG_TYPE_BYTES        2
> #define SND_SOC_TPLG_TYPE_ENUM        3
> #define SND_SOC_TPLG_TYPE_DAPM_GRAPH    4
> #define SND_SOC_TPLG_TYPE_DAPM_WIDGET    5
> #define SND_SOC_TPLG_TYPE_DAI_LINK    6
> #define SND_SOC_TPLG_TYPE_PCM        7
> #define SND_SOC_TPLG_TYPE_MANIFEST    8
> #define SND_SOC_TPLG_TYPE_CODEC_LINK    9
> #define SND_SOC_TPLG_TYPE_BACKEND_LINK    10
> #define SND_SOC_TPLG_TYPE_PDATA        11
> #define SND_SOC_TPLG_TYPE_DAI        12
> 
> #define SOC_TPLG_PASS_MANIFEST        0
> #define SOC_TPLG_PASS_VENDOR        1
> #define SOC_TPLG_PASS_MIXER        2
> #define SOC_TPLG_PASS_WIDGET        3
> #define SOC_TPLG_PASS_PCM_DAI        4
> #define SOC_TPLG_PASS_GRAPH        5
> #define SOC_TPLG_PASS_PINS        6
> #define SOC_TPLG_PASS_BE_DAI        7
> #define SOC_TPLG_PASS_LINK        8
> 
> 
>>
>> Signed-off-by: Keyon Jie <yang.jie at linux.intel.com>
>> ---
>>   sound/soc/soc-topology.c | 9 ++++-----
>>   1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
>> index 49875978a1ce..58cf5a12af3f 100644
>> --- a/sound/soc/soc-topology.c
>> +++ b/sound/soc/soc-topology.c
>> @@ -2685,11 +2685,10 @@ static int soc_valid_header(struct soc_tplg 
>> *tplg,
>>           return -EINVAL;
>>       }
>> -    if (tplg->pass == le32_to_cpu(hdr->type))
>> -        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);
>> +    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 1;
>>   }
>>