Re: [PATCH RESEND v2 2/8] media: uvc: Allow quirking by entity guid

[Ricardo Ribalda]

Hi Laurent

On Fri, 30 Dec 2022 at 14:40, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> Hi Ricardo,
>
> Thank you for the patch.
>
>
> On Fri, Dec 02, 2022 at 06:02:42PM +0100, Ricardo Ribalda wrote:
> > When an IP is shared by multiple devices its erratas will be shared by
> > all of them. Instead of creating a long list of device quirks, or
> > waiting for the users to report errors in their hardware lets add a
> > routine to add quirks based on the entity guid.
>
> I'm not thrilled by this. An entity is not an "IP". Quirks are needed to
> handle issues with particular firmware versions on particular devices.
> The same entity GUID can be used by different devices running different
> firmware versions, some that would require a quirk and some that
> wouldn't.

Unfortunately there are ISPs that do not support firmware upgrading
that have an error on their firmware (or in this particular case a
different interpretation of the standard). Those ISPs are mounted in
different boards with a VID:PID that is chosen by the module
manufacturer.
In those cases we cannot get a list of the devices that are broken, we
could only get a sublist that we will keep updating indefinitely as
users keep reporting bugs (if they do so).

We are lucky enough that SunplusIT has been very active and provided
us a way to detect what hardware requires quirking. In those
situations where the vendor is on board and there is no upgrade
mechanism I think that this is a good compromise.

>
> > Tested-by: HungNien Chen <hn.chen@xxxxxxxxxxxxx>
> > Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> > ---
> >  drivers/media/usb/uvc/uvc_driver.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index 9c05776f11d1..c63ecfd4617d 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -1493,6 +1493,28 @@ static int uvc_parse_control(struct uvc_device *dev)
> >       return 0;
> >  }
> >
> > +static const struct uvc_entity_quirk {
> > +     u8 guid[16];
> > +     u32 quirks;
> > +} uvc_entity_quirk[] = {
> > +};
> > +
> > +static void uvc_entity_quirks(struct uvc_device *dev)
> > +{
> > +     struct uvc_entity *entity;
> > +     int i;
>
> unsigned int
>
> > +
> > +     list_for_each_entry(entity, &dev->entities, list) {
> > +             for (i = 0; i < ARRAY_SIZE(uvc_entity_quirk); i++) {
> > +                     if (memcmp(entity->guid, uvc_entity_quirk[i].guid,
> > +                                sizeof(entity->guid)) == 0) {
> > +                             dev->quirks |= uvc_entity_quirk[i].quirks;
> > +                             break;
> > +                     }
> > +             }
> > +     }
> > +}
> > +
> >  /* -----------------------------------------------------------------------------
> >   * Privacy GPIO
> >   */
> > @@ -2452,6 +2474,9 @@ static int uvc_probe(struct usb_interface *intf,
> >               goto error;
> >       }
> >
> > +     /* Apply entity based quirks */
> > +     uvc_entity_quirks(dev);
> > +
> >       dev_info(&dev->udev->dev, "Found UVC %u.%02x device %s (%04x:%04x)\n",
> >                dev->uvc_version >> 8, dev->uvc_version & 0xff,
> >                udev->product ? udev->product : "<unnamed>",
> >
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda