Hi, On 3/31/21 1:03 PM, Laurent Pinchart wrote: > Hi Hans, > > On Wed, Mar 31, 2021 at 12:38:07PM +0200, Hans de Goede wrote: >> On 3/8/21 11:31 AM, Hans de Goede wrote: >>> From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> >>> >>> Some devices reference an output terminal as the source of extension >>> units. This is incorrect, as output terminals only have an input pin, >>> and thus can't be connected to any entity in the forward direction. The >>> resulting topology would cause issues when registering the media >>> controller graph. To avoid this problem, connect the extension unit to >>> the source of the output terminal instead. >>> >>> While at it, and while no device has been reported to be affected by >>> this issue, also handle forward scans where two output terminals would >>> be connected together, and skip the terminals found through such an >>> invalid connection. >>> >>> Reported-and-tested-by: John Nealy <jnealy3@xxxxxxxxx> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> >>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> >> >> Ping? This is a bug-fix which fixes a WARN triggering, causing many >> users to see a backtrace in their kernel-logs and reporting bugs about this: >> >> https://bugzilla.redhat.com/buglist.cgi?quicksearch=mc-entity.c >> >> Currently shows 12 open bugs for this and this is not counting all the >> ones which have already been triaged and matched as dups. >> >> As such it would be really good if we can get this merged and on >> its way to 5.12-rc# as a fix for 5.12 (and to be backported to the >> stable series). > > The patch is included in "[GIT PULL FOR v5.13] Miscellaneous changes". Ah I missed that. > I have no personal issue with it being merged in v5.12-rc, but technically > it's not a regression fix, is it ? I'll let Mauro decide what works best > for him. It is true that this is not a regression-fix but it is a bug-fix and for a bug which users are actively hitting. Regards, Hans > >>> --- >>> drivers/media/usb/uvc/uvc_driver.c | 32 ++++++++++++++++++++++++++++++ >>> 1 file changed, 32 insertions(+) >>> >>> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c >>> index 30ef2a3110f7..8df58f04dac6 100644 >>> --- a/drivers/media/usb/uvc/uvc_driver.c >>> +++ b/drivers/media/usb/uvc/uvc_driver.c >>> @@ -1716,6 +1716,31 @@ static int uvc_scan_chain_forward(struct uvc_video_chain *chain, >>> return -EINVAL; >>> } >>> >>> + /* >>> + * Some devices reference an output terminal as the >>> + * source of extension units. This is incorrect, as >>> + * output terminals only have an input pin, and thus >>> + * can't be connected to any entity in the forward >>> + * direction. The resulting topology would cause issues >>> + * when registering the media controller graph. To >>> + * avoid this problem, connect the extension unit to >>> + * the source of the output terminal instead. >>> + */ >>> + if (UVC_ENTITY_IS_OTERM(entity)) { >>> + struct uvc_entity *source; >>> + >>> + source = uvc_entity_by_id(chain->dev, >>> + entity->baSourceID[0]); >>> + if (!source) { >>> + uvc_dbg(chain->dev, DESCR, >>> + "Can't connect extension unit %u in chain\n", >>> + forward->id); >>> + break; >>> + } >>> + >>> + forward->baSourceID[0] = source->id; >>> + } >>> + >>> list_add_tail(&forward->chain, &chain->entities); >>> if (!found) >>> uvc_dbg_cont(PROBE, " (->"); >>> @@ -1735,6 +1760,13 @@ static int uvc_scan_chain_forward(struct uvc_video_chain *chain, >>> return -EINVAL; >>> } >>> >>> + if (UVC_ENTITY_IS_OTERM(entity)) { >>> + uvc_dbg(chain->dev, DESCR, >>> + "Unsupported connection between output terminals %u and %u\n", >>> + entity->id, forward->id); >>> + break; >>> + } >>> + >>> list_add_tail(&forward->chain, &chain->entities); >>> if (!found) >>> uvc_dbg_cont(PROBE, " (->"); >
