Hi Hans On 04/11/2021 14:49, Hans de Goede wrote: > Hi Daniel, > > On 11/2/21 00:43, Daniel Scally wrote: >> Hi Hans >> >> On 01/11/2021 16:02, Hans de Goede wrote: > <snip> > >>>> Having looked at this yesterday evening I'm more and more convinced it's >>>> necessary. I hacked it into the ov8865 driver in the interim (just by >>>> calling i2c_acpi_new_device() in probe) and then worked on that dw9719 >>>> code you found [1] to turn it into an i2c driver (attached, though still >>>> needs a bit of work), which will successfully bind to the i2c client >>>> enumerated by that i2c_acpi_new_device() call. From there though it >>>> needs a way for the v4l2 subdev to be matched to the sensor's subdev. >>>> This can happen automatically by way of the lens-focus firmware property >>>> against the sensor - we currently build those in the cio2-bridge, so >>>> adding another software node for the VCM and creating a lens-focus >>>> property for the sensor's software_node with a pointer to the VCM's node >>>> seems like the best way to do that. >>> So besides prepping a v5 of my previous series, with update regulator >>> init-data for the VCM I've also been looking into this, attached are >>> the results. >>> >>> Some notes from initial testing: >>> >>> 1. The driver you attached will only successful probe if I insmod >>> it while streaming video from the sensor. So I think we need another >>> regulator or the clk for just the VCM too, I will investigate this >>> later this week. >> Oh really, I'll test that too; thanks for the patches. There's a couple >> of tweaks to the driver anyway, so hopefully be able to get it ironed out. > Ok, I've figured this out now, with the attached patch (which also > explains what is going on) as well as an updated tps68470_board_data.c > with updated regulator_init_data for the VCM (also attached), the driver > can now successfully talk to the VCM in probe() while we are NOT > streaming from the ov8865. > > Daniel, please feel free to squash this into your original dw9719 patch. > Nice thanks - I'll do that. > >>> 2. I need some help with all the fwnode link stuff (I'm not very familiar >>> with this). There seems to be a chicken and egg problem here though, >>> because the v4l2subdev for the VCM does not register because of async stuff >>> and if we add it to the "graph" then my idea to enumerate the VCMs >>> from the SSDB on the complete() callback won't work. But we can do this >>> on a per sensor basis instead from the cio2_notifier_bound() callback >>> instead I guess ? >> >> I think on top of your work in the cio2-bridge for patch 3 you can do this: >> >> >> 1. Create another software node against the cio2_sensor struct, with the >> name coming from the vcm_types array >> >> 2. Assign that software node to board_info.swnode in >> cio2_bridge_instantiate_vcm_i2c_client() >> >> 3. Add another entry to dev_properties for the sensor, that is named >> "lens-focus" and contains a reference to the software_node created in #2 >> just like the references to the sensor/cio2 nodes. >> >> >> This way when the sensor driver calls >> v4l2_async_register_subdev_sensor() it should create a notifier that >> looks for that VCM client to bind. I think then rather than putting >> anything in the .bound() / .complete() callbacks, we should modify core >> to do _something_ when async matching some subdevs. The something would >> depend on the kind of devices that match, for example with the sensor >> driver and the ipu3-cio2 driver, there's an entity whos function is >> MEDIA_ENT_F_VID_IF_BRIDGE matching to an entity whos function is >> MEDIA_ENT_F_CAM_SENSOR, and it seems to me that every scenario like that >> is going to result in media pad links being created. Similarly for our >> sensor that's a device with entity function MEDIA_ENT_F_LENS matching to >> MEDIA_ENT_F_CAM_SENSOR, and I think that in those cases we can create >> either an interface link or a new kind of link (maybe >> "MEDIA_LNK_FL_ANCILLARY_LINK" or something...) between the two to show >> that they form a single logical unit, which we can then report to libcamera. >> >> >> Hope that makes sense... > Maybe? I have not looked into this closely yet. I'll continue working on > this coming Tuesday. > > If you feel like tinkering I would not mind if you beat me to it this > weekend :) OTOH please enjoy your weekend doing whatever, I can continue > working on this during office-hours next week. I'll probably have some time to look at it over the next few days; I'll let you know how I get on. > > Regards, > > Hans