| Message ID | 20240212091917.342715-3-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IPU bridge improvements for IPU6 | expand |
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@linux.intel.com> > --- > 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
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,
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@linux.intel.com> --- drivers/media/pci/intel/ipu-bridge.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)