diff mbox series

[v2,5/6] media: ccs: Better separate CCS static data access

Message ID 20231113134458.1423754-6-sakari.ailus@linux.intel.com
State New
Headers show
Series None | expand

Commit Message

Sakari Ailus Nov. 13, 2023, 1:44 p.m. UTC
Separate CCS static data read-only register access in ccs-reg-access.c by
naming them differently.

The code in this file generally deals with reading and writing registers
where as static data (when it comes to ccs_static_read_only()) contains
the read-only register values but no hardware registers are accessed in
that case.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../driver-api/media/drivers/ccs/mk-ccs-regs  |  2 +-
 drivers/media/i2c/ccs/ccs-reg-access.c        | 20 +++++++++----------
 2 files changed, 11 insertions(+), 11 deletions(-)

Comments

Sakari Ailus Nov. 13, 2023, 4 p.m. UTC | #1
On Mon, Nov 13, 2023 at 04:16:18PM +0200, Laurent Pinchart wrote:
> On Mon, Nov 13, 2023 at 02:08:49PM +0000, Sakari Ailus wrote:
> > On Mon, Nov 13, 2023 at 03:57:10PM +0200, Laurent Pinchart wrote:
> > > Hi Sakari,
> > > 
> > > Thank you for the patch.
> > > 
> > > On Mon, Nov 13, 2023 at 03:44:57PM +0200, Sakari Ailus wrote:
> > > > Separate CCS static data read-only register access in ccs-reg-access.c by
> > > > naming them differently.
> > > 
> > > "naming the access functions differently"
> > > 
> > > > The code in this file generally deals with reading and writing registers
> > > > where as static data (when it comes to ccs_static_read_only()) contains
> > > > the read-only register values but no hardware registers are accessed in
> > > > that case.
> > > > 
> > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > ---
> > > >  .../driver-api/media/drivers/ccs/mk-ccs-regs  |  2 +-
> > > >  drivers/media/i2c/ccs/ccs-reg-access.c        | 20 +++++++++----------
> > > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/Documentation/driver-api/media/drivers/ccs/mk-ccs-regs b/Documentation/driver-api/media/drivers/ccs/mk-ccs-regs
> > > > index 01252ee6062b..3d3152b45821 100755
> > > > --- a/Documentation/driver-api/media/drivers/ccs/mk-ccs-regs
> > > > +++ b/Documentation/driver-api/media/drivers/ccs/mk-ccs-regs
> > > > @@ -136,7 +136,7 @@ if (! defined $kernel) {
> > > >  
> > > >  print $H "#define CCS_FL_FLOAT_IREAL	" . flag_str(\$flag, \$all_flags) . "\n";
> > > >  print $H "#define CCS_FL_IREAL		" . flag_str(\$flag, \$all_flags) . "\n";
> > > > -print $H "#define CCS_BUILD_BUG \
> > > > +print $H "#define CCS_BUILD_BUG \\
> > > 
> > > This doesn't seem related.
> > 
> > Oops. This was meant to go to the previous patch. I'll address this in
> > v3...
> > 
> > > 
> > > >  	BUILD_BUG_ON(~CCI_REG_PRIVATE_MASK & ($all_flags))\n"
> > > >      if defined $kernel;
> > > >  
> > > > 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,
> > > 
> > > "static read only" sounds weird when interpreting "read" as a verb. And
> > > if "read" is not a verb, you're missing a verb :-) Maybe
> > > __ccs_read_state_data() would be a better name ? Same below.
> > 
> > Why state_data? I prefer to use terms used in the spec, not come up with
> > new ones.
> 
> Sorry, I mean __ccs_read_static_data(). I'm working on a patch related
> to V4L2 subdev state at the moment :-)
> 
> > There are also other blocks in static data beseides read-only register
> > values.
> > 
> > How about ccs_static_data_read_only_reg(), would that be more informative
> > in your opinion?
> 
> ccs_static_data_read_ro_reg() would work for me. I interpret
> ccs_static_data_read_only_reg() as "read just the register and nothing
> else".

ccs_static_data_read_ro_reg() seems reasonable, I'll use it in v3.

There will be more similarly named functions in the future.
diff mbox series

Patch

diff --git a/Documentation/driver-api/media/drivers/ccs/mk-ccs-regs b/Documentation/driver-api/media/drivers/ccs/mk-ccs-regs
index 01252ee6062b..3d3152b45821 100755
--- a/Documentation/driver-api/media/drivers/ccs/mk-ccs-regs
+++ b/Documentation/driver-api/media/drivers/ccs/mk-ccs-regs
@@ -136,7 +136,7 @@  if (! defined $kernel) {
 
 print $H "#define CCS_FL_FLOAT_IREAL	" . flag_str(\$flag, \$all_flags) . "\n";
 print $H "#define CCS_FL_IREAL		" . flag_str(\$flag, \$all_flags) . "\n";
-print $H "#define CCS_BUILD_BUG \
+print $H "#define CCS_BUILD_BUG \\
 	BUILD_BUG_ON(~CCI_REG_PRIVATE_MASK & ($all_flags))\n"
     if defined $kernel;
 
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;
 	}