Re: [PATCH] media: ipu-bridge: do not use the CVS device lookup for IVSC

[Sakari Ailus]

Hi Sergey,

On Wed, Sep 02, 2026 at 03:57:25PM +0100, Sergey Zagursky wrote:
> Sakari Ailus wrote:
> > I can confirm there's indeed an issue here. But considering the list
> > contains the CVS device HIDs, doesn't it mean you're returning NULL here
> > for CVS, i.e. not for IVSC?
> 
> acpi_match_device_ids() returns 0 on a match, not a boolean:
> 
>   int acpi_match_device_ids(struct acpi_device *device,
>                             const struct acpi_device_id *ids)
>   {
>         return __acpi_match_device(device, ids, NULL, NULL, NULL) ?
>                0 : -ENOENT;
>   }

Ah, I missed you used acpi_match_device_ids(), not
acpi_match_acpi_device(). v1 is still better than v2 though.

Can you match IVSC IDs instead? They're more stable as I don't expect new
ones are created anymore, which isn't the case for CVS.

This is ok as a fix that is easily backported, but I think parsing CVS /
IVSC should be separated so we wouldn't need a CVS fallback case at all.
That's not necessarily part of the same set though.

> 
> so the bare "if (acpi_match_device_ids(adev, cvs_acpi_ids))" is true when
> adev is *not* in the list, which is the IVSC case. Both spellings are in
> tree, e.g. drivers/acpi/scan.c:1800 uses the negated form for "matched"
> and drivers/acpi/x86/utils.c:206 the bare one for "did not match".
> 
> On this machine adev is INTC10CF, which is not in cvs_acpi_ids[], so the
> early return is taken, the IPU6 probe fails with -ENODEV and is retried
> once the IVSC device exists. With the polarity you read, IVSC would fall
> through to the lookup that returns the driverless INTC10CF:00 platform
> device and the camera would stay dead. It does come up, so the code
> behaves as the changelog describes.
> 
> That said, you had to stop and ask, which says enough about how it reads.
> v2 wraps the match in a named helper so the polarity is visible at the
> call site:
> 
>   static bool ipu_bridge_is_cvs_dev(struct acpi_device *adev)
>   {
>           return !acpi_match_device_ids(adev, cvs_acpi_ids);
>   }
> 
>   if (!ipu_bridge_is_cvs_dev(adev))
>           return NULL;
> 
> No functional change, so I rebuilt it but did not boot it again; the
> functional test in v1 stands:
> 
>   https://lore.kernel.org/linux-media/20260902145440.1786297-1-gvozdoder@xxxxxxxxx/
> 
> Thanks for the quick review.

-- 
Kidn regards,

Sakari Ailus