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
