Re: [PATCH v2 2/2] media: i2c: add os02g10 image sensor driver

[Elgin Perumbilly]

Hi Laurent,                                                                     
                                                                                
>On Tue, May 05, 2026 at 02:42:21PM +0000, Elgin Perumbilly wrote:              
>> > On Tue, Apr 14, 2026 at 10:19:23AM +0000, Tarang Raval wrote:              
>> >> > On Tue, Apr 14, 2026 at 09:43:32AM +0000, Elgin Perumbilly wrote:       
>> >> > > > On Tue, Apr 14, 2026 at 02:19:45PM +0530, Elgin Perumbilly wrote:   
>> >> > > > > Add a v4l2 subdevice driver for the Omnivision os02g10 sensor.    
>> >> > > > >                                                                   
>> >> > > > > The Omnivision os02g10 is a CMOS image sensor with an active array size of
>> >> > > > > 1920 x 1080.                                                      
>> >> > > > >                                                                   
>> >> > > > > The following features are supported:                             
>> >> > > > > - Manual exposure an gain control support                         
>> >> > > > > - vblank/hblank control support                                   
>> >> > > > > - vflip/hflip control support                                     
>> >> > > > > - Test pattern control support                                    
>> >> > > > > - Supported resolution: 1920 x 1080 @ 30fps (SBGGR10)             
>> >> > > > >                                                                   
>> >> > > > > Signed-off-by: Elgin Perumbilly <elgin.perumbilly@xxxxxxxxxxxxxxxxx>
>> >> > > > > Reviewed-by: Tarang Raval <tarang.raval@xxxxxxxxxxxxxxxxx>        
>> >> > > > > ---                                                               
>> >> > > > >  MAINTAINERS                 |    1 +                             
>> >> > > > >  drivers/media/i2c/Kconfig   |   10 +                             
>> >> > > > >  drivers/media/i2c/Makefile  |    1 +                             
>> >> > > > >  drivers/media/i2c/os02g10.c | 1039 +++++++++++++++++++++++++++++++++++
>> >> > > > >  4 files changed, 1051 insertions(+)                              
>> >> > > > >  create mode 100644 drivers/media/i2c/os02g10.c                   
>> >> > >                                                                       
>> >> > > I have added a new function, os02g10_set_framefmt, which dynamically sets
>> >> > > the mode register.                                                    
>> >> > >                                                                       
>> >> > > Please let me know if I have missed anything or if further changes are
>> >> > > needed.                                                               
>> >> >                                                                         
>> >> > You also need to drop the supported_modes array, and implement support  
>> >> > for .set_selection().                                                   
>> >>                                                                           
>> >> Are you suggesting that we should drop the array below?                   
>> >                                                                            
>> > Correct.                                                                   
>> >                                                                            
>> >> static const struct os02g10_mode supported_modes[] = {                    
>> >>     {                                                                     
>> >>         .width = 1920,                                                    
>> >>         .height = 1080,                                                   
>> >>         .vts_def = 1246,                                                  
>> >>         .hts_def = 1082,                                                  
>> >>         .exp_def = 1100,                                                  
>> >>         .x_start = 2,                                                     
>> >>         .y_start = 6,                                                     
>> >>     },                                                                    
>> >> };                                                                        
>> >>                                                                           
>> >> If we remove this, how would we provide mode-specific parameters such as VTS?
>> >                                                                            
>> > Those should be computed by the driver based on the format and crop        
>> > rectangle configured by userspace.                                         
>>                                                                              
>> Could you please take a look at the latest v3 patch?                         
>                                                                               
>I'm short on time at the moment, but v3 is still in my inbox, I haven't        
>dropped it.                                                                    
>                                                                               
>--                                                                             
>Regards,                                                                       
>                                                                               
>Laurent Pinchart                                                               
                                                                                
Thanks for the update, no worries.                                              
                                                                                
Best Regards,                                                                   
Elgin