Hi Martina, On Fri, Jun 11, 2021 at 10:06:00AM +0100, Krasteva, Martina wrote: > Hi Sakari, > > Thanks for the review > > On 6/10/2021 10:27 PM, Sakari Ailus wrote: > > Hi Martina, > > > > On Thu, May 27, 2021 at 03:21:39PM +0100, Martina Krasteva wrote: > > > From: Martina Krasteva <martinax.krasteva@xxxxxxxxx> > > > > > > Patch series contains Sony IMX335, Sony IMX412 and OmniVision OV9282 > > > camera sensor drivers and respective binding documentation. > > > > > > v1->v2: > > > - define maxItems for reset-gpios in dt binding document > > > - make sure the device is powered off on remove > > > - use pm_runtime_resume_and_get() instead of pm_runtime_get_sync() > > > > > > v1: https://patchwork.kernel.org/project/linux-media/list/?series=458115 > > Thanks for the update. > > > > The patches seem fine, but I noticed one problem: the analogue gain is only > > updated when exposure is set. This is a bug. > > > > Most drivers do not try to synchronise setting analogue gain and exposure > > to the same frame. Do you need that? Alternatively the control framework > > would probably need to be amended a little --- something that would have > > other benefits, too. > > Analog gain and exposure are "clustered". If I understand correctly, when > several controls are in a cluster and one/several of them are set/get from > userspace only the first control ops is called - V4L2_CID_EXPOSURE in my > case. > > Analog gain can be set explicitly, exposure control ops will be called with > analog gain new value and current exp. value. > Then I could have checked the is_new flag, so I can write the reg value just > to the updated control, not to both. > > In my case the userspace provides gain and exposure settings in sync so > cluster is used to mirror that behavior. I missed that, indeed that's fine then. -- Regards, Sakari Ailus
