On Thu, 30 Jan 2014, Daniel Vetter <daniel@xxxxxxxx> wrote: > How does this fit in with Thierry's dp aux helper improvements? I prefer > if we start to converge on that sooner than later ... Patch 1/4 actually fixes a potential bug in intel_dp_sink_crc(), which now detects failure only if the read returns 0, but plunges on with any negative error values. So we may want to queue that for fixes. Patch 2/4 could be dropped. I'd like to do 3/4 as a prep patch anyway, as it potentially changes behaviour, and makes conversion to the helpers more straightforward. Patch 4/4 could be dropped. To see how this would pan out, I'm almost done with the native aux conversion (did not look at the i2c-over-aux part yet). If we use those interfaces directly all over the place, the only annoyance is the potential for the same confusion that I've tried to avoid in this series. The only correct check for success is comparing the transfer function return value to the number of bytes that was to be transferred: if (drm_dp_dpcd_read(&intel_dp->dp_aux, offset, buffer, len) != len) but len may in fact be a fairly long #define like DP_DEVICE_SERVICE_IRQ_VECTOR. I do notice Thierry checks for ret < 0 in his code, although, IIUC, it's possible the transfer ends with fewer bytes transferred than requested. I'm beginning to feel like the functions should return and guarantee an error code < 0 if not all bytes were transferred, just for the ease of use. I mean, it's a nice feature to be able to make the distinction, but I can't fathom a practical situation where that would be necessary. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
