Hi Sakari, On 5/19/23 12:58, Sakari Ailus wrote: > Hi Hans, > > On Fri, May 19, 2023 at 10:55:42AM +0200, Hans de Goede wrote: <snip> >>> Although if the number of those drivers would be small, this could be just >>> undesirable but still somehow acceptable. And I wouldn't expect new sensors >>> to be paired with the IPU2 anymore. How many drivers there would be >>> roughly? I think I've seen ten-ish sensor drivers with the atomisp driver. >> >> About ten-ish drivers sounds right. >> >>> Isn't it possible to create a device for this purpose and use software >>> nodes for the GPIOs? I guess that would be a hack as well and you'd somehow >>> have to initialise this via other route than driver probe. >> >> This creates unsolvable probe-ordering problems. At a minimum we would >> need some check inside sensor drivers for them to return -EPROBE_DEFER >> until the GPIO mappings are added through some other means. Note that >> without the mappings gpiod_get will return -ENOENT, so we cannot just >> use its return value. > > They probably will already need this in order to make sure the atomisp > bridge has been initialized. The instantiation of the sensor i2c_clients and of the atomisp PCI device is independent of each other. In my other patch series I'm moving sensor registration for atomisp over to the v4l2-async framework like all other bridge/ISP drivers use so that atomisp no longer needs special sensor drivers. And AFAIK one of the reasons for having the v4l2-async framework is to avoid needing to have a specific probe order between bridge vs sensor drivers. > Could this code live in the atomisp bridge instead? That does not solve the probe ordering problem the sensor drivers need the GPIOs to enable the sensor and they all enable the sensor in their probe() to check the hw-id. The sensor's probe() function runs without any ordering guarantees vs the ISP/brige's probe() function. This is not an issue because at least during probe() the sensor drivers don't have any interactions with the bridge and any further access to the sensor-drivers is delayed until the v4l2-async notifier completion callback has run. The only way to work around the probe-ordering problem would be to delay checking the hw-id until the sensor gets linked to the bridge. Which would mean registering an async notifier from the sensors to catch binding from the sensor drivers and allowing the binding to fail. Delaying the hw-id check like this would be much more involved then the currently proposed solution and will likely cause other issues like the wrong driver binding when hw-vendors get the ACPI hw-id wrong (this is a known issue with audio-codecs, so chances are we will see this with sensors too). >> And if we need some changes anyways just adding the single line call >> to the new helper seems both the least invasive and the easiest. > > Simplest given the current patches, surely. But nothing to do with the > sensor drivers. I'd at least like to see relatively trivial ways to avoid > this researched first. There are no trivial ways to avoid this. Period, full stop. I have been looking into this for quite some time now and this really is the easiest solution. I cannot help but feel that you are blocking progress here purely because you have a very dt centric view. I've done my best to make the changes as non-invasive as possible and I must say I'm quite unhappy about the in my eyes unnecessary and unproductive push back here. I say unproductive because you have not offered any workable alternatives yet. Please keep in mind that the atomisp work I do is a volunteer / side project, so as such I have limited time to invest in this and I must say that your in my eyes unproductive unnecessary push back is not helping with my motivation for this. Also I'm basically fixing a mess here which Intel has left behind by never properly doing hw-enablement for this... Regards, Hans
