Hi Sakari,
On 2/12/24 10:19, Sakari Ailus wrote:
> The MEI CSI device is created as MEI boots up. This often takes place
> after the IPU6 driver probes, in which case the IPU6 driver returned
> -EPROBE_DEFER. The MEI CSI driver also returns -EPROBE_DEFER if the
> firmware nodes created by the IPU bridge (via IPU6 driver) aren't in
> place.
>
> If no other drivers are being probed after this point, neither IPU6 nor
> MEI CSI drivers will be re-probed. Address this (hopefully temporarily) by
> polling MEI CSI device in the IPU bridge initialisation.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> drivers/media/pci/intel/ipu-bridge.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
> index b2cf80d62ba2..45c39bd93d74 100644
> --- a/drivers/media/pci/intel/ipu-bridge.c
> +++ b/drivers/media/pci/intel/ipu-bridge.c
> @@ -4,6 +4,7 @@
> #include <linux/acpi.h>
> #include <linux/device.h>
> #include <linux/i2c.h>
> +#include <linux/iopoll.h>
> #include <linux/mei_cl_bus.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> @@ -138,17 +139,21 @@ static struct device *ipu_bridge_get_ivsc_csi_dev(struct acpi_device *adev)
> /* IVSC device on platform bus */
> dev = bus_find_device(&platform_bus_type, NULL, adev,
> ipu_bridge_match_ivsc_dev);
> - if (dev) {
> - snprintf(name, sizeof(name), "%s-%pUl", dev_name(dev), &uuid);
> + if (!dev)
> + return NULL;
>
> - csi_dev = device_find_child_by_name(dev, name);
> + snprintf(name, sizeof(name), "%s-%pUl", dev_name(dev), &uuid);
>
> - put_device(dev);
> + /*
> + * FIXME: instantiate MEI CSI software nodes outside the IPU bridge (or
> + * call IPU bridge from MEI CSI). Wait up to 60 seconds here.
> + */
> + read_poll_timeout(device_find_child_by_name, csi_dev, csi_dev,
> + 20000, 60000000, false, dev, name);
>
> - return csi_dev;
> - }
> + put_device(dev);
>
> - return NULL;
> + return csi_dev;
> }
>
> static int ipu_bridge_check_ivsc_dev(struct ipu_sensor *sensor,
Hmm, ok so the issue here is that the MEI CSI device's creation does
not trigger a running of the work to re-probe deferred devices
because the probe() for the MEI CSI device's driver does not succeed.
And at this point nothing else is getting probed, so the IPU6
driver's probe() will never get called again, causing things
to not work.
So 3 options:
1. Instantiate MEI CSI software nodes outside the IPU bridge
this seems undesirable since the nodes point to each other
so doing this will be tricky I think.
2. Call ipu_bridge_init from the MEI CSI driver, this seems
like the best option to me. The MEI CSI driver can lookup
the PCI device for the bridge and call ipu_bridge_init()
on it.
ipu_bridge_init() is intended to get called only once
even if the driver is unbound + rebound. So this should
be ok, but we would need to:
a) Add a staitc mutex in ipu_bridge.c which is locked for
the duration of ipu_bridge_init()
b) Move the checks which ensure that ipu_bridge_init()
gets called only once to inside ipu_bridge_init(), this
part may be tricky when also taking chromebooks which don't
need ipu_bridge_init() at all into account. Although that
would only be a problem if those chromebooks also have
an ivsc chip.
3. In the -EPROBE_DEFER path of the MEI CSI driver force
deferred probing to run, this would require:
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 85152537dbf1..3836ac02332d 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -194,6 +194,7 @@ void driver_deferred_probe_trigger(void)
*/
queue_work(system_unbound_wq, &deferred_probe_work);
}
+EXPORT_SYMBOL_GPL(driver_deferred_probe_trigger);
/**
* device_block_probing() - Block/defer device's probes
And a matching .h change and then we can just call
driver_deferred_probe_trigger() in the -EPROBE_DEFER
path of the MEI CSI driver.
This option would be my preferred solution.
I think we can sell exporting this to Greg KH,
otherwise we can also call:
platform_create_bundle() to create + successfully
probe() a fake platform device and then immediately
afterwards unregister the plat-dev + plat-drv again.
But it would be much better if we can just call
driver_deferred_probe_trigger().
Note if you decide to add:
EXPORT_SYMBOL_GPL(driver_deferred_probe_trigger);
it is important that the commit message properly
explains the problem (with a step by step what
happens in time which leads to the case where
no deferred-probes are running while one should
run for our purpose).
I would be happy to review the commit message before
you submit such a patch to Greg.
Regards,
Hans
