Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings

[Benjamin GAIGNARD]

On 1/31/20 12:22 AM, Dave Airlie wrote:
>>>> hi-actually yes, we should probably be using this instead of just dropping
>>>> this. Also, I didn't write this code originally I just refactored a bunch
>>>> of
>>>> it - Dave Airlied is the original author, but the original version of this
>>>> code was written ages ago. tbh, I think it's a safe bet to say that they
>>>> probably did mean to use this but forgot to and no one noticed until now.
>>> Hi,
>>>
>>> Any clue about how to use crc value ? Does it have to be checked
>>> against something else ?
>>> If crc are not matching what should we do of the data copied just before ?
>> We should be able to just take the CRC value from the sideband message and
>> then generate our own CRC value using the sideband message contents, and check
>> if the two are equal. If they aren't, something went wrong and we didn't
>> receive the message properly.
>>
>> Now as to what we should do when we have CRC mismatches? That's a bit more
>> difficult. If you have access to the DP MST spec, I suppose a place to start
>> figuring that out would be checking if there's a way for us to request that a
>> branch device resend whatever message it sent previously. If there isn't, I
>> guess we should just print an error in dmesg (possibly with a hexdump of the
>> failed message as well) and not forward the message to the driver. Not sure of
>> any better way of handling it then that
> Yeah I think this reflects what I wanted to do, I've no memory of a
> retransmit option in the spec, but I've away from it for a while. But
> we'd want to compare the CRC with what we got to make sure the are the
> same.

Hmm, that far more complex than just fix compilation warnings :)

I will split the patch in two:

- one for of all other warnings, hopefully it can get reviewed

- one for this crc4 variable. Does checking crc value and print an error 
should be acceptable ?

Something like:

if (crc4 != msg->chunk[msg->curchunk_len - 1])

 ÂÂÂ print_hex_dump(KERN_DEBUG, "wrong crc", DUMP_PREFIX_NONE, 16, 1, 
msg->chunk, msg->curchunk_len, false);


Benjamin


>
> Dave.