Re: [PATCH 5/6] media: ccs: Better separate CCS static data access

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent,

On Fri, Nov 10, 2023 at 04:46:22PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Fri, Nov 10, 2023 at 11:47:04AM +0200, Sakari Ailus wrote:
> > Separate CCS static data read-only register access in ccs-reg-access.c.
> 
> Please explain why. I don't have enough information to review this
> patch.

You could read the spec. ;-)

I had to read the code to figure out what it was supposed to do, as it
wasn't apparent frome the function names: virtually all functions in the
file somehow deal with reading or writing registers. CCS static data is
used to store, among other things, values of read-only registers, and this
is what is being accessed here.

> 
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > ---
> >  drivers/media/i2c/ccs/ccs-reg-access.c | 20 ++++++++++----------
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/media/i2c/ccs/ccs-reg-access.c b/drivers/media/i2c/ccs/ccs-reg-access.c
> > index 25993445f4fe..652d705a2ef5 100644
> > --- a/drivers/media/i2c/ccs/ccs-reg-access.c
> > +++ b/drivers/media/i2c/ccs/ccs-reg-access.c
> > @@ -197,8 +197,8 @@ static int __ccs_read_addr(struct ccs_sensor *sensor, u32 reg, u32 *val,
> >  	return 0;
> >  }
> >  
> > -static int __ccs_read_data(struct ccs_reg *regs, size_t num_regs,
> > -			   u32 reg, u32 *val)
> > +static int __ccs_static_read_only(struct ccs_reg *regs, size_t num_regs,
> > +				  u32 reg, u32 *val)
> >  {
> >  	unsigned int width = ccs_reg_width(reg);
> >  	size_t i;
> > @@ -235,16 +235,16 @@ static int __ccs_read_data(struct ccs_reg *regs, size_t num_regs,
> >  	return -ENOENT;
> >  }
> >  
> > -static int ccs_read_data(struct ccs_sensor *sensor, u32 reg, u32 *val)
> > +static int ccs_static_read_only(struct ccs_sensor *sensor, u32 reg, u32 *val)
> >  {
> > -	if (!__ccs_read_data(sensor->sdata.sensor_read_only_regs,
> > -			     sensor->sdata.num_sensor_read_only_regs,
> > -			     reg, val))
> > +	if (!__ccs_static_read_only(sensor->sdata.sensor_read_only_regs,
> > +				    sensor->sdata.num_sensor_read_only_regs,
> > +				    reg, val))
> >  		return 0;
> >  
> > -	return __ccs_read_data(sensor->mdata.module_read_only_regs,
> > -			       sensor->mdata.num_module_read_only_regs,
> > -			       reg, val);
> > +	return __ccs_static_read_only(sensor->mdata.module_read_only_regs,
> > +				      sensor->mdata.num_module_read_only_regs,
> > +				      reg, val);
> >  }
> >  
> >  static int ccs_read_addr_raw(struct ccs_sensor *sensor, u32 reg, u32 *val,
> > @@ -253,7 +253,7 @@ static int ccs_read_addr_raw(struct ccs_sensor *sensor, u32 reg, u32 *val,
> >  	int rval;
> >  
> >  	if (data) {
> > -		rval = ccs_read_data(sensor, reg, val);
> > +		rval = ccs_static_read_only(sensor, reg, val);
> >  		if (!rval)
> >  			return 0;
> >  	}
> 

-- 
Regards,

Sakari Ailus




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux