Hi Andy, On Fri, Nov 08, 2024 at 06:28:05PM +0200, Andy Shevchenko wrote: > On Fri, Nov 08, 2024 at 04:06:39PM +0000, Sakari Ailus wrote: > > On Fri, Nov 08, 2024 at 04:50:24PM +0200, Andy Shevchenko wrote: > > > The driver of OmniVision OV7251 expects "enable" pin instead of "reset". > > > Remap "reset" to "enable" and update polarity. > > > > > > In particular, the Linux kernel can't load the camera sensor > > > driver on Microsoft Surface Book without this change: > > > > > > ov7251 i2c-INT347E:00: supply vdddo not found, using dummy regulator > > > ov7251 i2c-INT347E:00: supply vddd not found, using dummy regulator > > > ov7251 i2c-INT347E:00: supply vdda not found, using dummy regulator > > > ov7251 i2c-INT347E:00: cannot get enable gpio > > > ov7251 i2c-INT347E:00: probe with driver ov7251 failed with error -2 > > ... > > > Should this be cc'd to stable? I guess it's not exactly a fix in the driver > > but a BIOS bug, but it can be worked around in the driver. :-) > > It's everything, but a BIOS bug, it's DT bug and whoever first introduced that > GPIO in the driver. Even in the DT present in kernel the pin was referred as How is that a DT (binding?) bug? > CAM_RST_N, which is exactly how this patch names it. > > OTOH it's a fix to the driver that never worked for ACPI case, so there never > was a regression to fix. It probably worked just fine, just not with that Surface Book. The polarity of the enable gpio appears to be set wrong in devm_gpiod_get() call. I can post a patch but cannot test it. Similarly, you should actually set the flags to GPIOD_OUT_HIGH as reset should be enabled here -- it's disabled only in power_on() as part of the power-on sequence. -- Regards, Sakari Ailus
