Noralf Trønnes wrote: > Peter, please have a look at this diff and see if I'm on the right track > here: https://gist.github.com/notro/a43a93a3aa0cc75d930890b7b254fc0a Yes that's exactly what I meant; this way the possibility for contradicting sizes is eliminated by protocol and not just by implementation - very nice! Some more comments, sorry if this is just because of ongoing work: Perhaps the functions taking usb_device + ifnum could take usb_interface instead - but I don't know if that would simplify or complicate things. Alan mentioned this idea in similar circumstances in another thread. I don't feel strongly, but perhaps it's cleaner. gud_usb_control_msg() now seems almost redundant, maybe it could be removed. In gud_usb_set() if NULL == buf then that's passed to usb_control_msg() along with len, which likely crashes if len > 0, so it may be good to check or enforce that, maybe with else len=0; before the gud_usb_transfer() call. Finally a small style note that I'd personally change a few if (ret > 0) { blocks to have one indent level less and do each check right away, e.g. in gud_connector_get_modes(): ret = gud_usb_get() if (ret % EDID_LENGTH) { drm_err(); } else if (ret > 0) { edid_ctx.len = ret; edid = drm_do_get_edid(); } and later on in the function by the display modes one indent level could be saved with a goto: if (ret <= 0) goto out; but obviously no huge deal. In general it's really helpful for device development to see error messages when the device behaves incorrectly, the "Invalid .. size" errors are great examples of this, but e.g. gud_get_display_descriptor() returns -EIO without a message. Maybe there are opportunities for further helpful error messages? Thanks a lot and kind regards //Peter _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
