[PATCH v2 0/2] iio: light: veml6030: Generalize hw_init and cleanup naming

[PATCH v2 0/2] iio: light: veml6030: Generalize hw_init and cleanup naming

[Gabriel Braga Lagrotaria]

Hello,

This patchset refactors the veml6030 driver to improve support for device
variations and future-proof the codebase. The primary focus is to
deduplicate the hardware initialization logic and move away from wildcard
naming conventions.

The first patch removes the 'x' wildcard from the main chip structure,
renaming it to 'veml6030_chip_desc'. The second patch introduces a
configuration structure to handle device-specific values, allowing
veml6030 and veml6035 to share a single initialization path.

Changes in v2:
- Renamed 'struct veml603x_chip' to 'struct veml6030_chip_desc' to
avoid wildcard naming as requested.
- Refactored initialization logic to use a configuration descriptor,
removing the redundant veml6035_hw_init().

Gabriel Braga Lagrotaria (2):
  iio: light: veml6030: Remove 'x' wildcard usages in file
  iio: light: veml6030: Generalize hw_init for veml6030 and veml6035

 drivers/iio/light/veml6030.c | 102 +++++++++++++++--------------------
 1 file changed, 42 insertions(+), 60 deletions(-)

-- 
2.54.0

[PATCH v2 1/2] iio: light: veml6030: Remove 'x' wildcard usages in file

[Gabriel Braga Lagrotaria]

Remove use of 'x' wildcard at struct veml603x_chip, as it is not
particularly future-proof. Instead, use name veml6030_chip_desc.

Signed-off-by: Gabriel Braga Lagrotaria <gabrielblo@ime.usp.br>
Co-developed-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
Signed-off-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
Co-developed-by: Ellian Carlos <elliancarlos@gmail.com>
Signed-off-by: Ellian Carlos <elliancarlos@gmail.com>
---
 drivers/iio/light/veml6030.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
index 6bcacae3863c..3d11e4736330 100644
--- a/drivers/iio/light/veml6030.c
+++ b/drivers/iio/light/veml6030.c
@@ -82,7 +82,7 @@ struct veml6030_rf {
 	struct regmap_field *gain;
 };
 
-struct veml603x_chip {
+struct veml6030_chip_desc {
 	const char *name;
 	const struct iio_chan_spec *channels;
 	const int num_channels;
@@ -108,7 +108,7 @@ struct veml6030_data {
 	struct i2c_client *client;
 	struct regmap *regmap;
 	struct veml6030_rf rf;
-	const struct veml603x_chip *chip;
+	const struct veml6030_chip_desc *chip;
 	struct iio_gts gts;
 
 };
@@ -1167,7 +1167,7 @@ static int veml6030_runtime_resume(struct device *dev)
 static DEFINE_RUNTIME_DEV_PM_OPS(veml6030_pm_ops, veml6030_runtime_suspend,
 				 veml6030_runtime_resume, NULL);
 
-static const struct veml603x_chip veml6030_chip = {
+static const struct veml6030_chip_desc veml6030_chip = {
 	.name = "veml6030",
 	.channels = veml6030_channels,
 	.num_channels = ARRAY_SIZE(veml6030_channels),
@@ -1178,7 +1178,7 @@ static const struct veml603x_chip veml6030_chip = {
 	.set_info = veml6030_set_info,
 };
 
-static const struct veml603x_chip veml6035_chip = {
+static const struct veml6030_chip_desc veml6035_chip = {
 	.name = "veml6035",
 	.channels = veml6030_channels,
 	.num_channels = ARRAY_SIZE(veml6030_channels),
@@ -1189,7 +1189,7 @@ static const struct veml603x_chip veml6035_chip = {
 	.set_info = veml6030_set_info,
 };
 
-static const struct veml603x_chip veml7700_chip = {
+static const struct veml6030_chip_desc veml7700_chip = {
 	.name = "veml7700",
 	.channels = veml7700_channels,
 	.num_channels = ARRAY_SIZE(veml7700_channels),
-- 
2.54.0

[PATCH v2 2/2] iio: light: veml6030: Generalize hw_init for veml6030 and veml6035

[Gabriel Braga Lagrotaria]

Modify veml6030_hw_init() function to deduplicate the setup logic
for veml6030_hw_init() and veml6035_hw_init(), and remove
veml6035_hw_init().

Additionally, introduce struct veml6030_hw_init_config_desc to store and
pass the custom configuration values specific to each device variation.

Signed-off-by: Gabriel Braga Lagrotaria <gabrielblo@ime.usp.br>
Co-developed-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
Signed-off-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
Co-developed-by: Ellian Carlos <elliancarlos@gmail.com>
Signed-off-by: Ellian Carlos <elliancarlos@gmail.com>
---
 drivers/iio/light/veml6030.c | 92 +++++++++++++++---------------------
 1 file changed, 37 insertions(+), 55 deletions(-)

diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
index 3d11e4736330..132ef6136da8 100644
--- a/drivers/iio/light/veml6030.c
+++ b/drivers/iio/light/veml6030.c
@@ -82,6 +82,14 @@ struct veml6030_rf {
 	struct regmap_field *gain;
 };
 
+struct veml6030_hw_init_config_desc {
+	const struct iio_gain_sel_pair *gain_sel;
+	int gain_sel_size;
+	int gain_max_scale_int;
+	int gain_max_scale_nano;
+	unsigned int reg_als_conf_value;
+};
+
 struct veml6030_chip_desc {
 	const char *name;
 	const struct iio_chan_spec *channels;
@@ -89,6 +97,7 @@ struct veml6030_chip_desc {
 	const struct reg_field gain_rf;
 	const struct reg_field it_rf;
 	const int max_scale;
+	const struct veml6030_hw_init_config_desc *hw_init_cfg;
 	int (*hw_init)(struct iio_dev *indio_dev, struct device *dev);
 	int (*set_info)(struct iio_dev *indio_dev);
 };
@@ -963,61 +972,21 @@ static int veml6030_regfield_init(struct iio_dev *indio_dev)
 	return 0;
 }
 
+
 /*
  * Set ALS gain to 1/8, integration time to 100 ms, PSM to mode 2,
  * persistence to 1 x integration time and the threshold
  * interrupt disabled by default. First shutdown the sensor,
  * update registers and then power on the sensor.
  */
-static int veml6030_hw_init(struct iio_dev *indio_dev, struct device *dev)
-{
-	int ret, val;
-	struct veml6030_data *data = iio_priv(indio_dev);
-
-	ret = devm_iio_init_iio_gts(dev, 2, 150400000,
-				    veml6030_gain_sel, ARRAY_SIZE(veml6030_gain_sel),
-				    veml6030_it_sel, ARRAY_SIZE(veml6030_it_sel),
-				    &data->gts);
-	if (ret)
-		return dev_err_probe(dev, ret, "failed to init iio gts\n");
-
-	ret = veml6030_als_shut_down(data);
-	if (ret)
-		return dev_err_probe(dev, ret, "can't shutdown als\n");
-
-	ret = regmap_write(data->regmap, VEML6030_REG_ALS_CONF, 0x1001);
-	if (ret)
-		return dev_err_probe(dev, ret, "can't setup als configs\n");
-
-	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM,
-				 VEML6030_PSM | VEML6030_PSM_EN, 0x03);
-	if (ret)
-		return dev_err_probe(dev, ret, "can't setup default PSM\n");
-
-	ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, 0xFFFF);
-	if (ret)
-		return dev_err_probe(dev, ret, "can't setup high threshold\n");
-
-	ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, 0x0000);
-	if (ret)
-		return dev_err_probe(dev, ret, "can't setup low threshold\n");
-
-	ret = veml6030_als_pwr_on(data);
-	if (ret)
-		return dev_err_probe(dev, ret, "can't poweron als\n");
-
-	ret = devm_add_action_or_reset(dev, veml6030_als_shut_down_action, data);
-	if (ret < 0)
-		return ret;
-
-	/* Clear stale interrupt status bits if any during start */
-	ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &val);
-	if (ret < 0)
-		return dev_err_probe(dev, ret,
-				     "can't clear als interrupt status\n");
+static const struct veml6030_hw_init_config_desc veml6030_hw_init_config = {
+	.gain_sel = veml6030_gain_sel,
+	.gain_sel_size = ARRAY_SIZE(veml6030_gain_sel),
+	.gain_max_scale_int = 2,
+	.gain_max_scale_nano = 150400000,
+	.reg_als_conf_value = 0x1001,
+};
 
-	return ret;
-}
 
 /*
  * Set ALS gain to 1/8, integration time to 100 ms, ALS and WHITE
@@ -1026,13 +995,22 @@ static int veml6030_hw_init(struct iio_dev *indio_dev, struct device *dev)
  * threshold interrupt disabled by default. First shutdown the sensor,
  * update registers and then power on the sensor.
  */
-static int veml6035_hw_init(struct iio_dev *indio_dev, struct device *dev)
+static const struct veml6030_hw_init_config_desc veml6035_hw_init_config = {
+	.gain_sel = veml6035_gain_sel,
+	.gain_sel_size = ARRAY_SIZE(veml6035_gain_sel),
+	.gain_max_scale_int = 0,
+	.gain_max_scale_nano = 409600000,
+	.reg_als_conf_value = VEML6035_SENS | VEML6035_CHAN_EN | VEML6030_ALS_SD,
+};
+
+static int veml6030_hw_init(struct iio_dev *indio_dev, struct device *dev)
 {
 	int ret, val;
 	struct veml6030_data *data = iio_priv(indio_dev);
+	const struct veml6030_hw_init_config_desc *cfg = data->chip->hw_init_cfg;
 
-	ret = devm_iio_init_iio_gts(dev, 0, 409600000,
-				    veml6035_gain_sel, ARRAY_SIZE(veml6035_gain_sel),
+	ret = devm_iio_init_iio_gts(dev, cfg->gain_max_scale_int, cfg->gain_max_scale_nano,
+				    cfg->gain_sel, cfg->gain_sel_size,
 				    veml6030_it_sel, ARRAY_SIZE(veml6030_it_sel),
 				    &data->gts);
 	if (ret)
@@ -1042,8 +1020,7 @@ static int veml6035_hw_init(struct iio_dev *indio_dev, struct device *dev)
 	if (ret)
 		return dev_err_probe(dev, ret, "can't shutdown als\n");
 
-	ret = regmap_write(data->regmap, VEML6030_REG_ALS_CONF,
-			   VEML6035_SENS | VEML6035_CHAN_EN | VEML6030_ALS_SD);
+	ret = regmap_write(data->regmap, VEML6030_REG_ALS_CONF, cfg->reg_als_conf_value);
 	if (ret)
 		return dev_err_probe(dev, ret, "can't setup als configs\n");
 
@@ -1074,9 +1051,11 @@ static int veml6035_hw_init(struct iio_dev *indio_dev, struct device *dev)
 		return dev_err_probe(dev, ret,
 				     "can't clear als interrupt status\n");
 
-	return 0;
+	return ret;
 }
 
+
+
 static int veml6030_probe(struct i2c_client *client)
 {
 	int ret;
@@ -1167,6 +1146,7 @@ static int veml6030_runtime_resume(struct device *dev)
 static DEFINE_RUNTIME_DEV_PM_OPS(veml6030_pm_ops, veml6030_runtime_suspend,
 				 veml6030_runtime_resume, NULL);
 
+
 static const struct veml6030_chip_desc veml6030_chip = {
 	.name = "veml6030",
 	.channels = veml6030_channels,
@@ -1174,6 +1154,7 @@ static const struct veml6030_chip_desc veml6030_chip = {
 	.gain_rf = VEML6030_GAIN_RF,
 	.it_rf = VEML6030_IT_RF,
 	.max_scale = VEML6030_MAX_SCALE,
+	.hw_init_cfg = &veml6030_hw_init_config,
 	.hw_init = veml6030_hw_init,
 	.set_info = veml6030_set_info,
 };
@@ -1185,7 +1166,8 @@ static const struct veml6030_chip_desc veml6035_chip = {
 	.gain_rf = VEML6035_GAIN_RF,
 	.it_rf = VEML6030_IT_RF,
 	.max_scale = VEML6035_MAX_SCALE,
-	.hw_init = veml6035_hw_init,
+	.hw_init_cfg = &veml6035_hw_init_config,
+	.hw_init = veml6030_hw_init,
 	.set_info = veml6030_set_info,
 };
 
-- 
2.54.0

Re: [PATCH v2 2/2] iio: light: veml6030: Generalize hw_init for veml6030 and veml6035

[Maxwell Doose]

Hi there,

On Sat May 2, 2026 at 6:41 PM CDT, Gabriel Braga Lagrotaria <gabrielblo@ime.usp.br> wrote:
> Modify veml6030_hw_init() function to deduplicate the setup logic
> for veml6030_hw_init() and veml6035_hw_init(), and remove
> veml6035_hw_init().
>
> Additionally, introduce struct veml6030_hw_init_config_desc to store and
> pass the custom configuration values specific to each device variation.
>
> Signed-off-by: Gabriel Braga Lagrotaria <gabrielblo@ime.usp.br>
> Co-developed-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
> Signed-off-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
> Co-developed-by: Ellian Carlos <elliancarlos@gmail.com>
> Signed-off-by: Ellian Carlos <elliancarlos@gmail.com>
> ---
>  drivers/iio/light/veml6030.c | 92 +++++++++++++++---------------------
>  1 file changed, 37 insertions(+), 55 deletions(-)
>
[snip]

Just curious, how was this tested? It seems like you're modifying the
writing logic as well, and ordinarily patches like these don't get
merged unless properly tested. I haven't fully looked over it yet but I
just wanted some background before I start.

best regards,
maxwell

Re: [PATCH v2 1/2] iio: light: veml6030: Remove 'x' wildcard usages in file

[Joshua Crofts]

On Sun, 3 May 2026 at 01:44, Gabriel Braga Lagrotaria
<gabrielblo@ime.usp.br> wrote:
>
> Remove use of 'x' wildcard at struct veml603x_chip, as it is not
> particularly future-proof. Instead, use name veml6030_chip_desc.
>
> Signed-off-by: Gabriel Braga Lagrotaria <gabrielblo@ime.usp.br>
> Co-developed-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
> Signed-off-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
> Co-developed-by: Ellian Carlos <elliancarlos@gmail.com>
> Signed-off-by: Ellian Carlos <elliancarlos@gmail.com>
> ---
>  drivers/iio/light/veml6030.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
> index 6bcacae3863c..3d11e4736330 100644
> --- a/drivers/iio/light/veml6030.c
> +++ b/drivers/iio/light/veml6030.c
> @@ -82,7 +82,7 @@ struct veml6030_rf {
>         struct regmap_field *gain;
>  };
>
> -struct veml603x_chip {
> +struct veml6030_chip_desc {
>         const char *name;
>         const struct iio_chan_spec *channels;
>         const int num_channels;
> @@ -108,7 +108,7 @@ struct veml6030_data {
>         struct i2c_client *client;
>         struct regmap *regmap;
>         struct veml6030_rf rf;
> -       const struct veml603x_chip *chip;
> +       const struct veml6030_chip_desc *chip;
>         struct iio_gts gts;
>
>  };
> @@ -1167,7 +1167,7 @@ static int veml6030_runtime_resume(struct device *dev)
>  static DEFINE_RUNTIME_DEV_PM_OPS(veml6030_pm_ops, veml6030_runtime_suspend,
>                                  veml6030_runtime_resume, NULL);
>
> -static const struct veml603x_chip veml6030_chip = {
> +static const struct veml6030_chip_desc veml6030_chip = {
>         .name = "veml6030",
>         .channels = veml6030_channels,
>         .num_channels = ARRAY_SIZE(veml6030_channels),
> @@ -1178,7 +1178,7 @@ static const struct veml603x_chip veml6030_chip = {
>         .set_info = veml6030_set_info,
>  };
>
> -static const struct veml603x_chip veml6035_chip = {
> +static const struct veml6030_chip_desc veml6035_chip = {
>         .name = "veml6035",
>         .channels = veml6030_channels,
>         .num_channels = ARRAY_SIZE(veml6030_channels),
> @@ -1189,7 +1189,7 @@ static const struct veml603x_chip veml6035_chip = {
>         .set_info = veml6030_set_info,
>  };
>
> -static const struct veml603x_chip veml7700_chip = {
> +static const struct veml6030_chip_desc veml7700_chip = {
>         .name = "veml7700",
>         .channels = veml7700_channels,
>         .num_channels = ARRAY_SIZE(veml7700_channels),
> --
> 2.54.0
>
>

LGTM.

Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>

-- 
Kind regards

CJD

Re: [PATCH v2 2/2] iio: light: veml6030: Generalize hw_init for veml6030 and veml6035

[Andy Shevchenko]

On Sat, May 02, 2026 at 08:41:04PM -0300, Gabriel Braga Lagrotaria wrote:
> Modify veml6030_hw_init() function to deduplicate the setup logic
> for veml6030_hw_init() and veml6035_hw_init(), and remove
> veml6035_hw_init().
> 
> Additionally, introduce struct veml6030_hw_init_config_desc to store and
> pass the custom configuration values specific to each device variation.

...

> @@ -963,61 +972,21 @@ static int veml6030_regfield_init(struct iio_dev *indio_dev)
>  	return 0;
>  }
>  
> +

I can argue that this is a stray change, but I'm not objecting to having it.

>  /*
>   * Set ALS gain to 1/8, integration time to 100 ms, PSM to mode 2,
>   * persistence to 1 x integration time and the threshold
>   * interrupt disabled by default. First shutdown the sensor,
>   * update registers and then power on the sensor.
>   */

...

> +static const struct veml6030_hw_init_config_desc veml6030_hw_init_config = {
> +	.gain_sel = veml6030_gain_sel,
> +	.gain_sel_size = ARRAY_SIZE(veml6030_gain_sel),
> +	.gain_max_scale_int = 2,
> +	.gain_max_scale_nano = 150400000,
> +	.reg_als_conf_value = 0x1001,
> +};
>  
> -	return ret;
> -}
>  

Now here are two blank lines? One is enough.

...

> @@ -1074,9 +1051,11 @@ static int veml6035_hw_init(struct iio_dev *indio_dev, struct device *dev)
>  		return dev_err_probe(dev, ret,
>  				     "can't clear als interrupt status\n");
>  
> -	return 0;
> +	return ret;
>  }
>  
> +
> +

Ditto.

>  static int veml6030_probe(struct i2c_client *client)
>  {
>  	int ret;
> @@ -1167,6 +1146,7 @@ static int veml6030_runtime_resume(struct device *dev)
>  static DEFINE_RUNTIME_DEV_PM_OPS(veml6030_pm_ops, veml6030_runtime_suspend,
>  				 veml6030_runtime_resume, NULL);
>  
> +

Stray change.

>  static const struct veml6030_chip_desc veml6030_chip = {
>  	.name = "veml6030",
>  	.channels = veml6030_channels,

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] iio: light: veml6030: Generalize hw_init for veml6030 and veml6035

[Jonathan Cameron]

On Mon, 4 May 2026 17:14:56 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Sat, May 02, 2026 at 08:41:04PM -0300, Gabriel Braga Lagrotaria wrote:
> > Modify veml6030_hw_init() function to deduplicate the setup logic
> > for veml6030_hw_init() and veml6035_hw_init(), and remove
> > veml6035_hw_init().
> > 
> > Additionally, introduce struct veml6030_hw_init_config_desc to store and
> > pass the custom configuration values specific to each device variation.  
> 
> ...
> 
> > @@ -963,61 +972,21 @@ static int veml6030_regfield_init(struct iio_dev *indio_dev)
> >  	return 0;
> >  }
> >  
> > +  
> 
> I can argue that this is a stray change, but I'm not objecting to having it.

I am. Don't see a reason for two blank lines here.
There are subsystems where that is standard formatting, but it's not in IIO
and this driver doesn't do double blank lines anywhere else.

I guess Garbriel has been looking at other drivers where that style may
be in use.

Re: [PATCH v2 2/2] iio: light: veml6030: Generalize hw_init for veml6030 and veml6035

[Jonathan Cameron]

On Sat,  2 May 2026 20:41:04 -0300
Gabriel Braga Lagrotaria <gabrielblo@ime.usp.br> wrote:

> Modify veml6030_hw_init() function to deduplicate the setup logic
> for veml6030_hw_init() and veml6035_hw_init(), and remove
> veml6035_hw_init().
> 
> Additionally, introduce struct veml6030_hw_init_config_desc to store and
> pass the custom configuration values specific to each device variation.
> 
> Signed-off-by: Gabriel Braga Lagrotaria <gabrielblo@ime.usp.br>
> Co-developed-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
> Signed-off-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
> Co-developed-by: Ellian Carlos <elliancarlos@gmail.com>
> Signed-off-by: Ellian Carlos <elliancarlos@gmail.com>
Hi All,

A few additional comments from me.

> @@ -1074,9 +1051,11 @@ static int veml6035_hw_init(struct iio_dev *indio_dev, struct device *dev)
>  		return dev_err_probe(dev, ret,
>  				     "can't clear als interrupt status\n");
>  
> -	return 0;
> +	return ret;

Why?  If we can get here with anything that isn't 0 then fair enough, but I can't
see any change to make that now the case.  If we know it is zero then explicitly
returning that is good for readability.

>  }
>  
> +
> +
Please proofread your own patches for stuff like this.  It's very
easy to introduce stray changes as you develop your code so only
way to catch them is too look at the end result before sending it out.

Jonathan