Re: [PATCH 2/4] soundwire: cdns: fix check for number of messages

[Srinivas Kandagatla <srinivas.kandagatla@linaro.org>]

On 28/03/2019 14:03, Pierre-Louis Bossart wrote:
>> ---
>>   drivers/soundwire/cadence_master.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/soundwire/cadence_master.c 
>> b/drivers/soundwire/cadence_master.c
>> index cb6a331f448a..d41adbc57918 100644
>> --- a/drivers/soundwire/cadence_master.c
>> +++ b/drivers/soundwire/cadence_master.c
>> @@ -434,7 +434,7 @@ cdns_xfer_msg_defer(struct sdw_bus *bus,
>>       int cmd = 0, ret;
>>       /* for defer only 1 message is supported */
>> -    if (msg->len > 1)
> 
> I am not sure this is a typo.
> 

Am bit confused with defer data-structure itself.

I was assuming that defer->length is number of sdw messages in the 
structure. If its same as message lenght then sdw_msg->len and 
sdw_defer->length are totally redundant. May be we should get rid of 
lenght field in defer to avoid confusion?

> IRRC the hardware only defer e.g. a single write that can perform bank 
> switches and writes at specific times indicated with an SSP. What's more 
okay got it.
Does that mean that candence controller can only support 1 byte defer 
transfers?

> is that the defer structure is actually modified below this code (not 
> shown in the diff) to set defer>length = msg->len, so testing before the 
> value is set looks more suspicious than the current code.

removing the length field in defer should get rid of such instances.

--srini

> 
> Vinod, does this ring a bell?
> 
> 
>> +    if (defer->length > 1)
>>           return -ENOTSUPP;
>>       ret = cdns_prep_msg(cdns, msg, &cmd);
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel