Hi Sakari, On 10/10/22 14:52, Sakari Ailus wrote: > Hi Benjamin, > > On Mon, Oct 10, 2022 at 02:11:46PM +0200, Benjamin MUGNIER wrote: > > ... > >>>>>>>>> I thought we did discuss dropping support for sensor synchronisation in >>>>>>>>> this version? >>>>>>>>> >>>>>>>> >>>>>>>> This properties affects strobing lights gpios polarities as you can see >>>>>>>> in vgxy61_update_gpios_strobe_polarity. If set to '1' all strobing gpios >>>>>>>> are inverted. This has nothing to do with the sensor synchronization. >>>>>>> >>>>>>> So this is for strobing a LED flash? It would be good to mention this in >>>>>>> DT bindings. >>>>>>> >>>>>>>> >>>>>>>> Now I realize this is poorly named, and I even forgot to document it in >>>>>>>> the device tree bindings file. I apologize. >>>>>>> >>>>>>> No problem. >>>>>>> >>>>>>>> >>>>>>>> I would like to rename it to 'st,strobe-polarity' since this is vendor >>>>>>>> specific and to better reflect that it affects strobing gpios. I'll make >>>>>>>> this change for v7 and document this in the bindings file too. Tell me if >>>>>>>> there is any issues with that. >>>>>>> >>>>>>> That name seems reasonable to me. Although, *if* this is actually usable as >>>>>>> a GPIO as the bindings suggest, then the GPIO flags would probably be a >>>>>>> better alternative. >>>>>>> >>>>>> >>>>>> If by GPIO flag you mean adding 'gpios' to the property, We could go with 'st,strobe-gpios-polarity', which in the end this leads to the same property name as it was in the dt bindings :) >>>>>> I'll add a bit of comments on the bindings. It seems to be the best choice. >>>>> >>>>> Is this a GPIO or is it not (e.g. strobe signal only)? >>>>> >>>>> For the latter the this should be fine. And "flash-leds" property should be >>>>> there as well I guess? >>>>> >>>> >>> >>> Please wrap the lines at around 74. Rewrapped now... >>> >> >> Done. Thank you. >> >>>> This property controls the polarity of and output GPIO connected to the >>>> sensor. This output GPIO is driven by the sensor firmware in order to >>>> illuminate the scene whenever necessary. I'm not sure this goes under the >>>> "flash-leds" category, as it only provides a signal with either "0" >>>> (don't illuminate) or a 1 (illuminate) ? The sensor controls the signal >>> >>> This is what sensors generally do. >>> >>>> following the programmed "strobe-mode" as you can see in >>>> vgxy61_strobe_mode according to the HDR mode. It does not have a >>>> max-microamp or timeout values as a flash I suppose, it is really a >>>> simple signal. >>> >>> Those are usually configured for the flash driver, not on the sensor. >>> >> >> Ok, I guess in this case there is no flash driver. Should I keep the >> 'st,strobe-gpios-polarity' property or are you aware of an already >> defined property for this behavior? > > So the LED is directly connected to this pin (perhaps in series with a > resistor)? That is an unusual solution. > Yes, the pin is connected to a transistor responsible of powering on and off the LEDs according to the pin value, so that they have correct voltage from another power supply. But yes that's basically it. Thanks a lot. -- Regards, Benjamin
