On 4/20/2018 11:52 PM, Harry Wentland wrote: > On 2018-04-17 10:56 PM, Shirish S wrote: >> Currently the dm_dp_aux_transfer() does not parse >> the return value of dal_ddc_service_read_dpcd_data(), which also >> has a failure case. >> This patch captures the same and ensures the i2c operation status is >> sent appropriately to the drm framework. >> >> V2: Updated commit message. >> >> Signed-off-by: Shirish S <shirish.s at amd.com> >> --- >> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c >> index 782491e..7ac124d 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c >> @@ -115,7 +115,11 @@ static ssize_t dm_dp_aux_transfer(struct drm_dp_aux *aux, >> msg->address, >> msg->buffer, >> msg->size); >> - return read_bytes; >> + if (read_bytes != msg->size && >> + read_bytes >= DDC_RESULT_FAILED_OPERATION) > This doesn't look right. We shouldn't be returning the size or error code from the same function. This will not work if we submit a read 7 or 8 bytes and get an error. Agree, but hope you understood the issue, to re-iterate, in case of failure we reuturn a +ve number(7 or 8) back from dm_dp_aux_transfer() leading to edid read failures later. I have 2 suggestions: 1. change the "enum ddc_result {" to #defines of -ve error codes? or 2. make enum start with 129 (> than the max read_bytes possible)? let me know which one would be better? Thanks, Regards, Shirish S > > Harry > >> + return -EIO; >> + else >> + return read_bytes; >> case DP_AUX_I2C_WRITE: >> res = dal_ddc_service_write_dpcd_data( >> TO_DM_AUX(aux)->ddc_service, >>