st_pressure: lps22: unused default data
Hi,
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.
Any Ideas?
Signature · 2 lines
Regards, Michael
Quoted 1 line · Michael Nosthoff
Hi,
Sorry for answering my own mail, but I got a bit further. See below
Quoted 33 lines
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 -22So 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.cin
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_sensorit's still using the data stored inpress_data.
Quoted 6 lines
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?
Signature · 2 lines
Regards, Michael
Quoted 61 lines · On Thu, 08 Mar 2018 15:19:39 +0100 Michael Nosthoff <[email protected]> wrote:
Quoted 1 line · Michael Nosthoff
Hi,
Sorry for answering my own mail, but I got a bit further. See below
Quoted 35 lines
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 -22So 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.cin
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_sensorit's still using the data stored inpress_data.Quoted 7 lines
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 indev->platform_datawhich i can imagine might be a problem when
using it as a module. But maybe forgot to alter the following usage of the data.
Quoted 6 lines
So I would suggest to revert commit [2] and pass pdata to
st_sensors_init_sensor
in both cases (st_pressureandst_accel).Does this make sense?
Yes, I was clearly dozing when Colin's patch came through and failed to notice that it should have been doing something with that pointer. Should always have been passed the pdata copy.
Thanks,
Jonathan