Re: [PATCH v4 3/5] iio: pressure: dps310: add per-chip data

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 16 Oct 2026 18:35:31 +0300
Paulius Zaleckas <paulius.zaleckas@gmail.com> wrote:

> Move the coefficient count, the TMP_CFG startup value and the IIO device
> name into a per-chip structure, passed through the I2C and ACPI match
> data. This prepares for the Zilltek ZTS8032, which has two more
> coefficients and a different TMP_CFG layout.
> 
> No functional change.
> 
> Assisted-by: LLM
> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Paulius Zaleckas <paulius.zaleckas@gmail.com>
A few little things in line.

Sorry I'm late to the game - too much travel and catch up recently!

Jonathan

> ---
>  drivers/iio/pressure/dps310.c | 36 +++++++++++++++++++++++++++--------
>  1 file changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
> index 1d0b3232db9e..f2a8698e1e30 100644
> --- a/drivers/iio/pressure/dps310.c
> +++ b/drivers/iio/pressure/dps310.c
> @@ -62,6 +62,9 @@
>  #define DPS310_RESET		0x0c
>  #define  DPS310_RESET_MAGIC	0x09
>  #define DPS310_COEF_BASE	0x10
> +#define DPS310_COEF_LEN		18
> +
> +#define MAX_COEF_LEN		18
>  
>  /* Make sure sleep time is <= 30ms for usleep_range */
>  #define DPS310_POLL_SLEEP_US(t)		min(30000, (t) / 8)
> @@ -86,10 +89,23 @@ static const int scale_factors[] = {
>  	2088960,
>  };
>  
> +struct dps310_chip_info {
> +	const char *name;
> +	u8 tmp_cfg;	/* TMP_CFG at startup: sensor the coefficients are for */

I had to look at the datasheet for the dps310 to figure out what this
meant.  Can we instead use a field for exactly what it is within TMP_CFG?

	bool coef_for_ext_sensor;


> +	u8 coef_len;
> +};
> +
> +static const struct dps310_chip_info dps310_chip_info = {
> +	.name = "dps310",
> +	.tmp_cfg = DPS310_TMP_EXT,
> +	.coef_len = DPS310_COEF_LEN,
> +};
> +
>  struct dps310_data {
>  	struct i2c_client *client;
>  	struct regmap *regmap;
>  	struct mutex lock;	/* Lock for sequential HW access functions */
> +	const struct dps310_chip_info *info;
>  
>  	s32 c0, c1;
>  	s32 c00, c10, c20, c30, c01, c11, c21;
> @@ -149,13 +165,13 @@ static const struct iio_chan_spec dps310_channels[] = {
>  static int dps310_get_coefs(struct dps310_data *data)
>  {
>  	int rc;
> -	u8 coef[18];
> +	u8 coef[MAX_COEF_LEN];
>  	u32 c0, c1;
>  	u32 c00, c10, c20, c30, c01, c11, c21;
>  
>  	/* Read all sensor calibration coefficients from the COEF registers. */
>  	rc = regmap_bulk_read(data->regmap, DPS310_COEF_BASE, coef,
> -			      sizeof(coef));
> +			      data->info->coef_len);
>  	if (rc < 0)
>  		return rc;
>  
> @@ -252,10 +268,10 @@ static int dps310_startup(struct dps310_data *data)
>  		return rc;
>  
>  	/*
> -	 * Set up external (MEMS) temperature sensor in single sample, one
> -	 * measurement per second mode
> +	 * Set up the temperature sensor the coefficients are for in single
> +	 * sample, one measurement per second mode
>  	 */
> -	rc = regmap_write(data->regmap, DPS310_TMP_CFG, DPS310_TMP_EXT);
> +	rc = regmap_write(data->regmap, DPS310_TMP_CFG, data->info->tmp_cfg);
With change above this would then ideally use appropriate FIELD_PREP()
calls to write 3 fields such that we don't need the comment at all. Something
like
	rc = regmap_write(data->regmap, DPS310_TMP_CFG,
			  FIELD_PREP(DPS310_TMP_PRC_BITS, DPS310_TMP_PRC_1) |
			  FIELD_PREP(DPS310_TMP_RATE_BITS, DPS310_TMP_RATE_1SPS) |
 			  FIELD_PREP(DPS310_TMP_EXT,
				     data->info->coef_for_ext_sensor ? 1 : 0));
(with a few defines to make that work).
BTW I love that the PRC values are defined as 'optional, and may not be relevant'.
Huh? What does that mean. I guess - don't use them!

>  	if (rc)
>  		return rc;
>  
> @@ -979,7 +995,11 @@ static int dps310_probe(struct i2c_client *client)
>  	data->client = client;
>  	mutex_init(&data->lock);
>  
> -	iio->name = DPS310_DEV_NAME;
> +	data->info = i2c_get_match_data(client);
> +	if (!data->info)
> +		return -ENODATA;
> +
> +	iio->name = data->info->name;
>  	iio->channels = dps310_channels;
>  	iio->num_channels = ARRAY_SIZE(dps310_channels);
>  	iio->info = &dps310_info;
> @@ -1018,13 +1038,13 @@ static int dps310_probe(struct i2c_client *client)
>  }
>  
>  static const struct i2c_device_id dps310_id[] = {
> -	{ .name = DPS310_DEV_NAME },
> +	{ .name = "dps310", .driver_data = (kernel_ulong_t)&dps310_chip_info },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, dps310_id);
>  
>  static const struct acpi_device_id dps310_acpi_match[] = {
> -	{ "IFX3100" },
> +	{ .id = "IFX3100", .driver_data = (kernel_ulong_t)&dps310_chip_info },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(acpi, dps310_acpi_match);