[PATCH] iio: magnetometer: mmc35240: refactor volatile and writeable reg functions

[PATCH] iio: magnetometer: mmc35240: refactor volatile and writeable reg functions

[Bianca Costa Galvão]

From: Bianca Costa Galvão <biancagalvao@usp.br>

Refactor 'mmc35240_is_volatile_reg' and 'mmc35240_is_writeable_reg' as they
share similar logic, returning the inverse of each other.

Introduce a new helper function 'mmc35240_reg_check' to generalize this
behavior, which is then used by both 'mmc35240_is_volatile_reg' and
'mmc35240_is_writeable_reg'.

Signed-off-by: Bianca Costa Galvão <biancagalvao@usp.br>
Co-developed-by: Júlia Calixto Rosa <juliacalixtorosa@usp.br>
Signed-off-by: Júlia Calixto Rosa <juliacalixtorosa@usp.br>
---
 drivers/iio/magnetometer/mmc35240.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/magnetometer/mmc35240.c b/drivers/iio/magnetometer/mmc35240.c
index e08a57cd6de2..07f58567e521 100644
--- a/drivers/iio/magnetometer/mmc35240.c
+++ b/drivers/iio/magnetometer/mmc35240.c
@@ -418,15 +418,14 @@ static const struct iio_info mmc35240_info = {
 	.attrs		= &mmc35240_attribute_group,
 };
 
+static bool mmc35240_reg_check(unsigned int reg)
+{
+	return reg == MMC35240_REG_CTRL0 || reg == MMC35240_REG_CTRL1;
+}
+
 static bool mmc35240_is_writeable_reg(struct device *dev, unsigned int reg)
 {
-	switch (reg) {
-	case MMC35240_REG_CTRL0:
-	case MMC35240_REG_CTRL1:
-		return true;
-	default:
-		return false;
-	}
+	return mmc35240_reg_check(reg);
 }
 
 static bool mmc35240_is_readable_reg(struct device *dev, unsigned int reg)
@@ -448,13 +447,7 @@ static bool mmc35240_is_readable_reg(struct device *dev, unsigned int reg)
 
 static bool mmc35240_is_volatile_reg(struct device *dev, unsigned int reg)
 {
-	switch (reg) {
-	case MMC35240_REG_CTRL0:
-	case MMC35240_REG_CTRL1:
-		return false;
-	default:
-		return true;
-	}
+	return !mmc35240_reg_check(reg);
 }
 
 static const struct reg_default mmc35240_reg_defaults[] = {
-- 
2.43.0

Re: [PATCH] iio: magnetometer: mmc35240: refactor volatile and writeable reg functions

[Jonathan Cameron]

On Sat, 26 Apr 2025 16:41:32 -0300
Bianca Costa Galvão <biancalvao@gmail.com> wrote:

> From: Bianca Costa Galvão <biancagalvao@usp.br>
> 
> Refactor 'mmc35240_is_volatile_reg' and 'mmc35240_is_writeable_reg' as they
> share similar logic, returning the inverse of each other.
> 
> Introduce a new helper function 'mmc35240_reg_check' to generalize this
> behavior, which is then used by both 'mmc35240_is_volatile_reg' and
> 'mmc35240_is_writeable_reg'.
> 
> Signed-off-by: Bianca Costa Galvão <biancagalvao@usp.br>
> Co-developed-by: Júlia Calixto Rosa <juliacalixtorosa@usp.br>
> Signed-off-by: Júlia Calixto Rosa <juliacalixtorosa@usp.br>
Hi Bianca, Júlia

The patch is mostly fine but I think the naming as
_reg_check() is not helpful as it doesn't indicate anything specific
is being checked.  I think ti would be cleaner to leave _is_writeable_reg() alone
and them make the implementation of _is_volatile_reg()

	return !mmc35240_is_writeable_reg(dev, reg);

Which will effectively document that all non writeable registers are volatile.

Jonathan

> ---
>  drivers/iio/magnetometer/mmc35240.c | 21 +++++++--------------
>  1 file changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/mmc35240.c b/drivers/iio/magnetometer/mmc35240.c
> index e08a57cd6de2..07f58567e521 100644
> --- a/drivers/iio/magnetometer/mmc35240.c
> +++ b/drivers/iio/magnetometer/mmc35240.c
> @@ -418,15 +418,14 @@ static const struct iio_info mmc35240_info = {
>  	.attrs		= &mmc35240_attribute_group,
>  };
>  
> +static bool mmc35240_reg_check(unsigned int reg)
> +{
> +	return reg == MMC35240_REG_CTRL0 || reg == MMC35240_REG_CTRL1;
> +}
> +
>  static bool mmc35240_is_writeable_reg(struct device *dev, unsigned int reg)
>  {
> -	switch (reg) {
> -	case MMC35240_REG_CTRL0:
> -	case MMC35240_REG_CTRL1:
> -		return true;
> -	default:
> -		return false;
> -	}
> +	return mmc35240_reg_check(reg);
>  }
>  
>  static bool mmc35240_is_readable_reg(struct device *dev, unsigned int reg)
> @@ -448,13 +447,7 @@ static bool mmc35240_is_readable_reg(struct device *dev, unsigned int reg)
>  
>  static bool mmc35240_is_volatile_reg(struct device *dev, unsigned int reg)
>  {
> -	switch (reg) {
> -	case MMC35240_REG_CTRL0:
> -	case MMC35240_REG_CTRL1:
> -		return false;
> -	default:
> -		return true;
> -	}
> +	return !mmc35240_reg_check(reg);
>  }
>  
>  static const struct reg_default mmc35240_reg_defaults[] = {