Hi, On Fri, 2023-10-20 at 16:39 +0200, Hans de Goede wrote: > Hi, > > On 10/20/23 12:47, Andreas Helbech Kleist wrote: > > I've only tried it on my WIP IPU4 hack of this driver > > Oh a IPU4 version of this driver would be very interesting, > I was actually thinking about looking into this myself > because I have a Chuwi Hi13 tablet which uses the IPU4 and > it would be nice to get the camera working there (and on > other IPU4 using devices). > > Can you give some more info on your IPU4 work, e.g. : > > 1. Does it work? Not yet. The intel-ipu4 module is working to the extent I'm able to test it without the isys-module. Buttress authentication succeeds, and I have very similar mmiotraces between the new driver and the old 4.19 based driver I'm comparing it to. The isys-module is very much work in progress. I'm learning the v4l2 along the way, so there is a steep learning curve. > 2. Which sensor are you using it with ? A custom tc358748 driver that pretends to be a sensor that delivers RGB data. I tried with the tc358746 driver in-tree, but figured it would be simpler to start out with a custom driver, since we have an old driver that has a list of the exact register writes needed to setup the device. > 3. Which device are you testing on ? An endoscopic system from Ambu A/S (the company I work for). See my colleague Claus' description of the device here: https://lore.kernel.org/all/471df7ffdf34b73d186c429a366cfee62963015f.camel@xxxxxxxxx/ > 4. Do you have a public git branch with this somewhere ? Not yet, but I will share when I have something working. I could push the main driver part if anyone is interested. The isys driver is not worth sharing yet. > I'm afraid that I won't have time to look into this > myself anytime soon, but I'm very interested in this. It would be great to have somebody else testing this out when I get a bit further. > > > but I think it > > is correct for IPU6 as well. The reason is that isys_driver is an > > auxiliary_driver, so I don't think > > > > MODULE_DEVICE_TABLE(pci, isys_pci_tbl); > > > > has any effect. The PCI probe happens in ipu6_pci_probe in ipu6.c > > (because it has a pci_device_id table as well), and the isys_driver > > is > > probed indirectly by ipu6-bus.c. > > So the MODULE_DEVICE_TABLE(pci, isys_pci_tbl) indeed does not > belong in this auxbus driver, instead it should use some sort > of auxbus MODULE_DEVICE_TABLE() to autoload based on its > auxbus modalias. > > But it does have an effect, modprobe will load both the main > ipu6 driver registering the aux devices as well as this driver > based on the modalias of the PCI device because with this > MODULE_DEVICE_TABLE(pci, isys_pci_tbl); statement both drivers > match that PCI modalias. All right. But since the main driver contains the same table, I don't think there's any need to have it here? > But the correct thing to do here would be to switch to > an auxbus based MODULE_DEVICE_TABLE() for the isys driver. The isys_driver already has an auxiliary_device_id table. I'm not sure if that's what you mean? >From the bottom of ipu6-isys.c in PATCH 10/15: +static const struct auxiliary_device_id ipu6_isys_id_table[] = { + { + .name = "intel_ipu6.isys", + .driver_data = (kernel_ulong_t)&ipu6_isys_auxdrv_data, + }, +}; + +static struct auxiliary_driver isys_driver = { + .name = IPU6_ISYS_NAME, + .probe = isys_probe, + .remove = isys_remove, + .id_table = ipu6_isys_id_table, + .driver = { + .pm = &isys_pm_ops, + }, +}; + +module_auxiliary_driver(isys_driver); > > Regards, > > Hans > > > >
