Re: st_pressure: lps22: unused default data

[Michael Nosthoff <posted@heine.so>]

On 2018-03-08 14:54, Michael Nosthoff wrote:
> Hi,

Sorry for answering my own mail, but I got a bit further. See below

> I have an LPS22 sensor which i used sucessfully with Kernel 4.9. Upon
> switching to 4.14 it was not loading properly anymore. This failure
> occured:
> 
> [    3.827094] iio iio:device1: DRDY on pdata not valid.
> [    3.832578] st-press-i2c: probe of 0-005c failed with error -22
> 
> So I started digging. It started working again when I add this entry
> to my devicetree:
> 
> st,drdy-int-pin = <1>;
> 
> So it seems the default value seems to be not set in 4.14 but was in 
> 4.9
> 
> I came to the conclusion that the problem happens in
> drivers/iio/pressure/st_pressure_core.c in
> 
> st_press_common_probe:
> 
> 
> struct st_sensors_platform_data *pdata =
> 	(struct st_sensors_platform_data *)press_data->dev->platform_data;
> ...
> if (!pdata && press_data->sensor_settings->drdy_irq.addr)
> 	pdata =	(struct st_sensors_platform_data *)&default_press_pdata;
> 
> err = st_sensors_init_sensor(indio_dev, 
> press_data->dev->platform_data);
> 
> 
> If pdata is not set (which is correct in my case) it gets assigned a
> default value. But in the call to st_sensors_init_sensor it's still
> using the data stored in press_data.
> 
> Okay, simple solution would be to pass pdata here as well. But I'm a
> bit confused about the commit [1] which introduced this. pdata is not
> used anymore in this function, so it seems completely useless to set it
> there. And so I don't understand what the commit tries to achieve at 
> all
> besides never using the default data.
> 

Somebody seems also to have noticed in the accel driver. See this commit 
[2]
and decided to remove the default initialization completely. Which makes 
sense
as this code was not doing anything.

I guess the what the intial commit was trying to achieve was to not 
overwrite
the pointer in dev->platform_data which i can imagine might be a problem 
when
using it as a module. But maybe forgot to alter the following usage of 
the data.

So I would suggest to revert commit [2] and pass pdata to 
st_sensors_init_sensor
in both cases (st_pressure and st_accel).

Does this make sense?



> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/iio/pressure/st_pressure_core.c?id=7383d44b84c94aaca4bf695a6bd8a69f2295ef1a
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/iio/accel/st_accel_core.c?id=585ed27d06151f98e39238298f43ee261314ae74
-- 
Regards,
Michael