Re: [PATCH 4/5] i3c: dw: Do not use OF match data as a quirk bitmask

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing error check for platform_get_irq() which breaks probe deferral.
--

--- Patch [4]: [PATCH 4/5] i3c: dw: Do not use OF match data as a quirk bitmask ---
commit b46fa3977d02ec78544c1f987ffbf99b39c47f0d
Author: Akhil R <[email protected]>

i3c: dw: Do not use OF match data as a quirk bitmask

This commit updates dw_i3c_common_probe() to explicitly check for a match
in the driver's ACPI table instead of merely testing for an ACPI companion.
This prevents OF match data from being incorrectly cast to a quirk bitmask.

Closes: https://lore.kernel.org/all/[email protected]/

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 3816a50a52cc5..17e1dd4fb5f3d 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c

[ ... ]

> @@ -1618,9 +1620,10 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
>  
>  	master->dev = &pdev->dev;
>  
> -	if (has_acpi_companion(&pdev->dev)) {
> -		quirks = (unsigned long)device_get_match_data(&pdev->dev);
> -	} else if (pdev->dev.of_node) {
> +	acpi_id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev);
> +	if (acpi_id) {
> +		quirks = acpi_id->driver_data;
> +	} else {
>  		drvdata = device_get_match_data(&pdev->dev);
>  		if (drvdata)
>  			quirks = drvdata->flags;

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing
dw_i3c_common_probe(), I noticed an existing issue with how the IRQ is
fetched later in the same function:

	irq = platform_get_irq(pdev, 0);
	ret = devm_request_irq(&pdev->dev, irq,
			       dw_i3c_master_irq_handler, 0,
			       dev_name(&pdev->dev), master);

Does this code fail to handle deferred probing properly?

If platform_get_irq() returns a negative error like -EPROBE_DEFER, it is
passed directly as the irq parameter to devm_request_irq(). Because the irq
parameter is an unsigned integer, the negative value is cast to a large
number. This causes request_threaded_irq() to reject it as out of bounds
and return -EINVAL.

Could this permanently break initialization on systems that require probe
deferral by masking the original deferral error code?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4

-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c