diff mbox series

[v2,06/10] iio: pressure: mprls0025pa.c remove dangerous defaults

Message ID 20231224143500.10940-7-petre.rodan@subdimension.ro (mailing list archive)
State Changes Requested
Headers show
Series changes to mprls0025pa | expand

Commit Message

Petre Rodan Dec. 24, 2023, 2:34 p.m. UTC
This driver supports 32*3 combinations of fixed ranges and transfer
functions, plus custom ranges.

So statistically a user has more than 99% chance that the provided
default configuration will generate invalid pressure readings if the
bindings are not initialized and the driver is instantiated via sysfs.

The current patch removes this loophole making sure the driver loads
only if the dt has been initialized.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
Signed-off-by: Andreas Klinger <ak@it-klinger.de>
---
 drivers/iio/pressure/mprls0025pa.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

--
2.41.0

Comments

Jonathan Cameron Dec. 26, 2023, 4:39 p.m. UTC | #1
On Sun, 24 Dec 2023 16:34:51 +0200
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> This driver supports 32*3 combinations of fixed ranges and transfer
> functions, plus custom ranges.
> 
> So statistically a user has more than 99% chance that the provided
> default configuration will generate invalid pressure readings if the
> bindings are not initialized and the driver is instantiated via sysfs.

I guess 99% is strong enough that it's unlikely we will break anyone so
fair enough to drop this attempt to guess the values.

> 
> The current patch removes this loophole making sure the driver loads
> only if the dt has been initialized.

Drop the reference to DT. It's correctly using generic firmware accessors
so should work fine with ACPI PRP0001 for example as well as DT.

Usually we just refer to the need for firmware properties.
There are lots of places they could be coming from.

> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> Signed-off-by: Andreas Klinger <ak@it-klinger.de>
> ---
>  drivers/iio/pressure/mprls0025pa.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/pressure/mprls0025pa.c b/drivers/iio/pressure/mprls0025pa.c
> index 233cc1dc38ad..63c46592956f 100644
> --- a/drivers/iio/pressure/mprls0025pa.c
> +++ b/drivers/iio/pressure/mprls0025pa.c
> @@ -375,11 +375,8 @@ static int mpr_probe(struct i2c_client *client)
>  				"honeywell,transfer-function %d invalid\n",
>  								data->function);
>  	} else {

I'd prefer the condition flipped even though patch will be more noisy.

	if (!dev_fwnode(dev))
		return dev_err_probe()...

	...

As end result will be more readable.

> -		/* when loaded as i2c device we need to use default values */
> -		dev_notice(dev, "firmware node not found; using defaults\n");
> -		data->pmin = 0;
> -		data->pmax = 172369; /* 25 psi */
> -		data->function = MPR_FUNCTION_A;
> +		return dev_err_probe(dev, -EINVAL,
> +				  "driver needs to be initialized in the dt\n");
>  	}
> 
>  	data->outmin = mpr_func_spec[data->function].output_min;
> --
> 2.41.0
>
diff mbox series

Patch

diff --git a/drivers/iio/pressure/mprls0025pa.c b/drivers/iio/pressure/mprls0025pa.c
index 233cc1dc38ad..63c46592956f 100644
--- a/drivers/iio/pressure/mprls0025pa.c
+++ b/drivers/iio/pressure/mprls0025pa.c
@@ -375,11 +375,8 @@  static int mpr_probe(struct i2c_client *client)
 				"honeywell,transfer-function %d invalid\n",
 								data->function);
 	} else {
-		/* when loaded as i2c device we need to use default values */
-		dev_notice(dev, "firmware node not found; using defaults\n");
-		data->pmin = 0;
-		data->pmax = 172369; /* 25 psi */
-		data->function = MPR_FUNCTION_A;
+		return dev_err_probe(dev, -EINVAL,
+				  "driver needs to be initialized in the dt\n");
 	}

 	data->outmin = mpr_func_spec[data->function].output_min;