sailus, Hi I just read your latest reply in the email-thread about where to do the GPIO mapping. After thinking more about it I'm ok with doing the mapping on a per sensor-type basis in the INT3472 code as long as no DMI quirks are necessary.
If DMI quirks are necessary at the INT3472 level and could be avoided with some more generic workaround in the sensor driver, then I would still like to see such a workaround in the sensor driver, reading your latest email I believe we are on the same page on that ?
For now / for the ov7251 case this is not relevant but it is the main reason for my hesitation about solving this in the INT3472 code.
hansg: what are the cases you envision where DMI-specific quirks would be needed in the INT3472 driver, but not in sensor drivers ?
I can imagine machine-specific quirks being needed, but I don't see why that would depend on which driver the code is located in
hansg: I'd like to revisit this topic if we come across that.
I presume it would require an actual bug, even in Windows specific bits. It is unlikely DMI checks would be needed as presumably it works on Windows without them.
See the mt9m114 example for something which I'm worried about: https://github.com/jwrdegoede/linux-sunxi/commit/e43be8f19b5913a2e4bd715e7eef88fd909a2d1d
hansg: The reason we have acpi_graph_ignore_port() in drivers/acpi/mipi-disco-img.c is really because of an actual bug. And that's the only example I know DMI checks have been necessary when it comes to cameras.
The issue is that if a standard camera connector is used with say both powerdown and reset GPIOs on the connector, but the sensor has only one of the 2 GPIOs then Windows will happily toggle both GPIOs on power on/off since that is (I believe) handled by a separate power-sequencing driver which binds to the INT3472 device
hansg: I don't like that mt9m114 patch :-)
(unsurprisingly I suppose)
hansg: My suggestion was a (virtual) GPIO-controlled GPIO device handling multiple GPIOs. :-)
No amount of mapping is going to get Linux to toggle both GPIOs as Windows does. So if we have 2 different mt9m114 camera modules on such a standard connector, 1 using the powerdown pin 1 using the reset pin then things will not work.
That'd avoid any driver changes to this driver albeit it'd require a new driver, which of course can be used to solve similar problems elsewhere.
hansg: your issue is not specific to the mt9m114 camera sensor. if we fix it there, we will need the same code in all camera sensor drivers. that's not the way to go
sailus, interesting suggestion...
pinchartl, this is only an issue on sensors with only 1 of powerdown / reset many sensors have both. With that said you have a valid point.
So my plan for the mt9m114 is to see which pin is actually the one we need on the few atomisp devices which use it (Asus T100TA and probably also T100HA) 
and see if they are consistent in this so we can do a mapping solution.
you mentioned a "standard connector". what connector is that ?
Anyways the only reason I brought up the mt9m114 thing is to explain that sailus' "I presume it would require an actual bug, even in Windows specific bits" statement is not correct. Because windows seems to have a separate power-sequencing driver which just bangs all GPIOs described in the DSM, which is quite different from how Linux does things so we are going to have some impedance matching issues here even with mapping
hansg: Right, that may well be. It's difficult to predict the future. Custom things sometimes tend to get done on that side.
but that's not machine-specific. this can be solved in a central location, for instance with a one-GPIO-controls-multiple-GPIOs driver as Sakari mentioned, without DMI matches, can't it ?
pinchartl, it is not really a doucmented standardized connector, it is more that on some of the atomisp tablets there is a small zif connector which offers a standard set of signals / voltages from the reference design and then different batches of the tablet may use different sensors on the same PCB / connector.
But we haven't needed them yet and so I still would be surprised if we couldn't find a reasonable way to do things without DMI checks.
pinchartl, yes sailus suggestion should work, but adds another layer / more complexity. I expect the cases where we actually hit an issue like this to be small and only impacting one or a few sensor drivers at which point it might be better to solve it in the sensor drivers.
I really really don"t want to see these kind of hacks in sensor drivers
they just don't belong there
pinchartl, I could say the same for the platfrom-drivers-x86 code and then we just both sit there on our high pedestals and users loose because they have non working hw
hansg: That's and outcome I'd also prefer to avoid.
Sakari proposed a technical solution to solve this problem. users don't lose anything with that
I'm trying to be pragmatic here and except the re-mapping in the INT3472 code even though IMHO the DT binding clearly is wrong in the ov7521, it would be nice if next time things are not solvable cleanly on the INT3472 side some pragmatism is offered on the drivers/media/i2c side
Sakari's solution requires a significant amount of effort over a sensor driver level solution and manpower is something we have a big shortage of. Remember perfect is the enemy of good.
We have a single int3472 driver that is used on ACPI only, plus some dozens of sensor drivers that need to work in a large number of different environments. The more convoluted the firmware handling the sensor drivers becomes, the worse it will be. And I'd prefer it not to get any worse. I've been fixing more power management issues in sensor drivers than I need lately.
So as long as we can reasonably keep these things out of the sensor drivers, then that is my fairly firm preference.
sailus, I agree, note those power-management bugs are completely unrelated to the sensor drivers being used on ACPI systems though.
I'd be happy to change my opinion if we got no new sensor drivers anymore but that doesn't seem to be the case, quite the contrary.
They're not entirely unrelated.
sensor drivers are too complex today already. we need to simplify them, not make them worse
and yes, it's also a "I trade my pain for yours" type of issue. what makes your life easier makes mine more painful
a virtual GPIO driver will require a bit more work, but will then make everybody's life better
I don't see is as not being pragmatic
I agree with trying to keep kludges in the int3472 driver where possible. All I'm asking for is if there is a hard way to fix something on the int3472 side and an easy way to fix it on the sensor side that fixing things on the sensor driver side will be considered then and not dismissed beforehand
it's not like we're asking for V4L2 to be rewritten from scratch
(I'd like that though, but it's a different story :-))
if we run into a hard problem, we can certainly discuss it to find a solution
hansg: I'm fine discussing this topic again when we encounter such cases. It's the complexity and maintenance burden I'm concerned in sensor drivers as we have lots of them and they could be used anywhere, in principle.
I'm happy review int3472 driver patches if you think that could be of any help.
sailus, I understand your worries there.
sailus: happy, or willing ? :-)
Well, maybe willing is the right word. :-) But still.
Ok, so for now lets move forward with the int3472 remapping code. I would like to see the initial implementation be a bit more generic and not limited to only being able to remap what is called "reset" at the INT3472 level. I'll reply to the patch about this.
sailus, I'm ok with having to do the int3472 reviews myself, but if you are offering to do reviews it would be great if you can review "[PATCH v7] media: i2c: Add Omnivision OV02C10 sensor driver" that patch is the only thing keeping cameras from workin in Fedora 41 on all Dell XPS 9x40 models
I already did a cursory review of v6 resulting in this v7 and I helped Heimer clean up the out of tree version before Heimer posted v1
s/helped/coached/
In an ideal world we would have the original int3472 driver only, without Windows specific ACPI extensions that now lives in clk_and_regulator.c and tps68470.c. With DisCo for Imaging implemented in ACPI tables we'll get closer to that but when that's going to happen I don't know.
hansg: https://lore.kernel.org/linux-media/20241220132419.1027206-1-sakari.ailus@linux.intel.com/
IOW we're about to change the APIs for sensor drivers and keeping backwards compatibility will be a major pain for older drivers. That's why I haven't paid much attention to new sensor drivers recently but obviously we'll need to get new drivers in if merging the API change will get much further delayed.
I can certainly check that driver though.
sailus, I understand where you are coming from, but looking at the delays in getting the IPU6 CSI2 code upstream because of streams / metadata considerations I do not think that delaying sensor drivers for this reason is a good idea.
These internal reworks and/or API changes tend to take longer then expected and in the mean time it would be nice to have people hw supported in the mainline kernel. E.g. these Dell XPS models are close to a year old now.
I guess this is also a sensor by sensor case thing, e.g. the t4ka3 sensor which I'm also involved in is for more exotic / old hw so that one can wait.
hansg: I thought we could get these changes sooner. Obviously if we don't get that soon in, then the advantages there are provided by a small delay is no longer justified by the harms. In that case we'll just have a longer list of drivers with some compatibility code.
ok, I understand.
hansg: Let's see how this looks like when we have rc1 in the media tree. This is a big change we've been working on, I can say for almost 10 years if you count all the new features (streams in particular) that are going to be enabled.
ok
I have to go downtown to pick up a parcel and then I'm of to lunch, so I'm going offline now, bye.
hansg: sailus: it seems I missed your chit-chat here. Do we have any archives available to read (yes, I read an email with the conclusion, but I am curious to read the discussion in full)?
_0andriy_, hi, I have just send you the discussion from my IRC clients scrollback buffer
Thanks!
_0andriy_: https://linuxtv.org/irc/oftc/log/linux-media