Re: [PATCH V3 12/14] i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI

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

Pre-existing issues:
- [High] NULL pointer dereference in proxy PM callbacks when child device i=
s unbound
--

--- Patch [12]: [PATCH V3 12/14] i3c: mipi-i3c-hci-pci: Propagate I3C wakeu=
p requirements to PCI ---
commit da80b1fc5d708734df314262ab3cf3703c943f07
Author: Adrian Hunter <[email protected]>
Subject: i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI

This patch aligns the PCI wakeup state with the requirements of the I3C
devices it serves. It checks during suspend if any HCI instance has a
wakeup-enabled I3C device, and arms the PCI function's wakeup accordingly.

> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers=
/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index 5a9e2a43eff8d..2b3bf6fa74f29 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
[ ... ]
> @@ -272,6 +274,13 @@ static bool mipi_i3c_hci_pci_is_mfd(struct device *d=
ev)
>  	return dev_is_platform(dev) && mfd_get_cell(to_platform_device(dev));
>  }
> =20
> +static bool mipi_i3c_hci_pci_any_wakeup_enabled(struct device *dev)
> +{
> +	struct i3c_hci *hci =3D dev_get_drvdata(dev);
> +
> +	return i3c_master_any_wakeup_enabled(&hci->master);

[Severity: High]
This is a pre-existing issue, but can this sequence result in a NULL pointer
dereference if the child driver is unbound?

If the mipi-i3c-hci platform driver fails to probe or is manually unbound
via sysfs, dev_get_drvdata(dev) will return NULL.

During system suspend, the PCI parent driver iterates over its child
platform devices in mipi_i3c_hci_pci_suspend(). If the BIOS left the bus
active (or the hardware hung during driver removal and failed to clear
the HC_CONTROL_BUS_ENABLE register), mipi_i3c_hci_pci_is_operational()
will return true.

This causes mipi_i3c_hci_pci_suspend_instance() to call
i3c_hci_rpm_suspend() and this newly added function, both of which will
dereference the NULL hci pointer.

Is it possible to add a NULL check here or in the caller before proceeding
with the suspend operations on the child device?

> +}
> +
>  static int mipi_i3c_hci_pci_suspend_instance(struct device *dev, void *d=
ata)
>  {
>  	struct mipi_i3c_hci_pci_pm_data *pm_data =3D data;

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260804133810.1849=
[email protected]?part=3D12