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

Marcelo Schmitt marcelo.schmitt1 em gmail.com
Sáb Maio 23 17:46:08 -03 2026


On 05/20, 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.
> 
> Signed-off-by: Gabriel Braga Lagrotaria <gabrielblo em ime.usp.br>
> Co-developed-by: Ricardo H H Kojo <ricardo.kojo em ime.usp.br>
> Signed-off-by: Ricardo H H Kojo <ricardo.kojo em ime.usp.br>
Choose appropriate tag name string.

> Co-developed-by: Ellian Carlos <elliancarlos em gmail.com>
> Signed-off-by: Ellian Carlos <elliancarlos em gmail.com>
> ---
>  drivers/iio/light/veml6030.c | 87 +++++++++++++-----------------------
>  1 file changed, 32 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
> index 3d11e4736330..315d59ae8c9d 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;
What about enlarging struct veml6030_chip instead of creating struct
veml6030_hw_init_config_desc? The gain options seem to be chip type dependent,
right?

>  	int (*hw_init)(struct iio_dev *indio_dev, struct device *dev);
>  	int (*set_info)(struct iio_dev *indio_dev);
>  };
> @@ -969,55 +978,13 @@ static int veml6030_regfield_init(struct iio_dev *indio_dev)
>   * 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");
> -
> -	return ret;
> -}
> +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,
So these could be added to veml6030_chip.

> +};
>  
>  /*
>   * Set ALS gain to 1/8, integration time to 100 ms, ALS and WHITE
> @@ -1026,13 +993,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,
These can be added to veml6035_chip.

> +};
> +
> +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;
If going to extend struct veml603x_chip, then we can get the local config data
from the chip info struct.
	const struct veml6030_chip *chip = data->chip;

and of course a slight update to the usage below.
>  
> -	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 +1018,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");
>  
> @@ -1174,6 +1149,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,
This will not be needed.

>  	.hw_init = veml6030_hw_init,
>  	.set_info = veml6030_set_info,
>  };
> @@ -1185,7 +1161,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,
Neither these.
>  	.set_info = veml6030_set_info,
>  };
>  
> -- 
> 2.54.0
> 
> -- 
> kernel mailing list
> kernel em lists.ime.usp.br
> https://lists.ime.usp.br/listinfo/kernel


Mais detalhes sobre a lista de discussão kernel