[PATCH] iio: imu: st_lsm6dsx: remove invalid gain value for LSM9DS1

[PATCH] iio: imu: st_lsm6dsx: remove invalid gain value for LSM9DS1

[Lorenzo Bianconi]

Get rid of invalid sensitivity value for LSM9DS1 gyro sensor

Fixes: 687a60feb9c6 ("iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9ds1")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index fd152fff0a8c..c85c8be3a024 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -151,10 +151,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 					.addr = 0x10,
 					.mask = GENMASK(4, 3),
 				},
-				.fs_avl[0] = { IIO_DEGREE_TO_RAD(245), 0x0 },
-				.fs_avl[1] = { IIO_DEGREE_TO_RAD(500), 0x1 },
-				.fs_avl[2] = { IIO_DEGREE_TO_RAD(0), 0x2 },
-				.fs_avl[3] = { IIO_DEGREE_TO_RAD(2000), 0x3 },
+				.fs_avl[0] = {  IIO_DEGREE_TO_RAD(245), 0x0 },
+				.fs_avl[1] = {  IIO_DEGREE_TO_RAD(500), 0x1 },
+				.fs_avl[2] = { IIO_DEGREE_TO_RAD(2000), 0x3 },
 			},
 		},
 	},
@@ -1196,13 +1195,19 @@ static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev,
 					    char *buf)
 {
 	struct st_lsm6dsx_sensor *sensor = iio_priv(dev_get_drvdata(dev));
+	const struct st_lsm6dsx_fs_table_entry *fs_table;
 	enum st_lsm6dsx_sensor_id id = sensor->id;
 	struct st_lsm6dsx_hw *hw = sensor->hw;
 	int i, len = 0;
 
-	for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++)
+	fs_table = &hw->settings->fs_table[id];
+	for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++) {
+		if (!fs_table->fs_avl[i].gain)
+			break;
+
 		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06u ",
-				 hw->settings->fs_table[id].fs_avl[i].gain);
+				 fs_table->fs_avl[i].gain);
+	}
 	buf[len - 1] = '\n';
 
 	return len;
-- 
2.21.0

Re: [PATCH] iio: imu: st_lsm6dsx: remove invalid gain value for LSM9DS1

[Jonathan Cameron]

On Tue, 27 Aug 2019 10:26:35 +0200
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> Get rid of invalid sensitivity value for LSM9DS1 gyro sensor
> 
> Fixes: 687a60feb9c6 ("iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9ds1")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
The zero degree scale is certainly odd otherwise, so good to tidy
this up.

Applied to the togreg branch of iio.git.

Thanks,

Jonathan

> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index fd152fff0a8c..c85c8be3a024 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -151,10 +151,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  					.addr = 0x10,
>  					.mask = GENMASK(4, 3),
>  				},
> -				.fs_avl[0] = { IIO_DEGREE_TO_RAD(245), 0x0 },
> -				.fs_avl[1] = { IIO_DEGREE_TO_RAD(500), 0x1 },
> -				.fs_avl[2] = { IIO_DEGREE_TO_RAD(0), 0x2 },
> -				.fs_avl[3] = { IIO_DEGREE_TO_RAD(2000), 0x3 },
> +				.fs_avl[0] = {  IIO_DEGREE_TO_RAD(245), 0x0 },
> +				.fs_avl[1] = {  IIO_DEGREE_TO_RAD(500), 0x1 },
> +				.fs_avl[2] = { IIO_DEGREE_TO_RAD(2000), 0x3 },
>  			},
>  		},
>  	},
> @@ -1196,13 +1195,19 @@ static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev,
>  					    char *buf)
>  {
>  	struct st_lsm6dsx_sensor *sensor = iio_priv(dev_get_drvdata(dev));
> +	const struct st_lsm6dsx_fs_table_entry *fs_table;
>  	enum st_lsm6dsx_sensor_id id = sensor->id;
>  	struct st_lsm6dsx_hw *hw = sensor->hw;
>  	int i, len = 0;
>  
> -	for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++)
> +	fs_table = &hw->settings->fs_table[id];
> +	for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++) {
> +		if (!fs_table->fs_avl[i].gain)
> +			break;
> +
>  		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06u ",
> -				 hw->settings->fs_table[id].fs_avl[i].gain);
> +				 fs_table->fs_avl[i].gain);
> +	}
>  	buf[len - 1] = '\n';
>  
>  	return len;

[BUG] Re: [PATCH] iio: imu: st_lsm6dsx: remove invalid gain value for LSM9DS1

[Martin Kepplinger]

On 27.08.19 22:08, Jonathan Cameron wrote:
> On Tue, 27 Aug 2019 10:26:35 +0200
> Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> 
>> Get rid of invalid sensitivity value for LSM9DS1 gyro sensor
>>
>> Fixes: 687a60feb9c6 ("iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9ds1")
>> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> The zero degree scale is certainly odd otherwise, so good to tidy
> this up.
> 
> Applied to the togreg branch of iio.git.
> 

Hi Jon,

you have applied this too quickly. I've left that zero value in there
for a reason, see below:


> Thanks,
> 
> Jonathan
> 
>> ---
>>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 17 +++++++++++------
>>  1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> index fd152fff0a8c..c85c8be3a024 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> @@ -151,10 +151,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>>  					.addr = 0x10,
>>  					.mask = GENMASK(4, 3),
>>  				},
>> -				.fs_avl[0] = { IIO_DEGREE_TO_RAD(245), 0x0 },
>> -				.fs_avl[1] = { IIO_DEGREE_TO_RAD(500), 0x1 },
>> -				.fs_avl[2] = { IIO_DEGREE_TO_RAD(0), 0x2 },
>> -				.fs_avl[3] = { IIO_DEGREE_TO_RAD(2000), 0x3 },
>> +				.fs_avl[0] = {  IIO_DEGREE_TO_RAD(245), 0x0 },
>> +				.fs_avl[1] = {  IIO_DEGREE_TO_RAD(500), 0x1 },
>> +				.fs_avl[2] = { IIO_DEGREE_TO_RAD(2000), 0x3 },
>>  			},
>>  		},
>>  	},
>> @@ -1196,13 +1195,19 @@ static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev,
>>  					    char *buf)
>>  {
>>  	struct st_lsm6dsx_sensor *sensor = iio_priv(dev_get_drvdata(dev));
>> +	const struct st_lsm6dsx_fs_table_entry *fs_table;
>>  	enum st_lsm6dsx_sensor_id id = sensor->id;
>>  	struct st_lsm6dsx_hw *hw = sensor->hw;
>>  	int i, len = 0;
>>  
>> -	for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++)
>> +	fs_table = &hw->settings->fs_table[id];
>> +	for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++) {
>> +		if (!fs_table->fs_avl[i].gain)
>> +			break;

You Ooops here and it's pretty obvious! You don't have
ST_LSM6DSX_FS_LIST_SIZE number of elements in the array anymore, but you
try to access it (the 4th).

I suggest reverting this (if not able to delete it entirely) and start
over in case this "invalid" value thing hurts and needs to get fixed.

I any case, there _is_ something we should do because it's not too
obvious what constraints the st_lsm6dsx_sensor_settings struct
definition has. It should be mostly clear when looking at the header but
a few inline comments might help.

thanks,

                          martin



>> +
>>  		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06u ",
>> -				 hw->settings->fs_table[id].fs_avl[i].gain);
>> +				 fs_table->fs_avl[i].gain);
>> +	}
>>  	buf[len - 1] = '\n';
>>  
>>  	return len;
> 

[PATCH] Revert "iio: imu: st_lsm6dsx: remove invalid gain value for LSM9DS1"

[Martin Kepplinger]

The st_lsm6dsx_sensor_settings struct's fs_avl[] array is expected to
be of size 4. We can't define it to be of size 3 and try to access
the 4th element below.

Commit c8d4066c7246 ("iio: imu: st_lsm6dsx: remove invalid gain value
for LSM9DS1") which is a "cleanup" change, results in an Ooops here
so revert it.

Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 2d3495560136..499745424c1e 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -151,9 +151,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 					.addr = 0x10,
 					.mask = GENMASK(4, 3),
 				},
-				.fs_avl[0] = {  IIO_DEGREE_TO_RAD(245), 0x0 },
-				.fs_avl[1] = {  IIO_DEGREE_TO_RAD(500), 0x1 },
-				.fs_avl[2] = { IIO_DEGREE_TO_RAD(2000), 0x3 },
+				.fs_avl[0] = { IIO_DEGREE_TO_RAD(245), 0x0 },
+				.fs_avl[1] = { IIO_DEGREE_TO_RAD(500), 0x1 },
+				.fs_avl[2] = { IIO_DEGREE_TO_RAD(0), 0x2 },
+				.fs_avl[3] = { IIO_DEGREE_TO_RAD(2000), 0x3 },
 			},
 		},
 	},
@@ -1195,19 +1196,13 @@ static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev,
 					    char *buf)
 {
 	struct st_lsm6dsx_sensor *sensor = iio_priv(dev_get_drvdata(dev));
-	const struct st_lsm6dsx_fs_table_entry *fs_table;
 	enum st_lsm6dsx_sensor_id id = sensor->id;
 	struct st_lsm6dsx_hw *hw = sensor->hw;
 	int i, len = 0;
 
-	fs_table = &hw->settings->fs_table[id];
-	for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++) {
-		if (!fs_table->fs_avl[i].gain)
-			break;
-
+	for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++)
 		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06u ",
-				 fs_table->fs_avl[i].gain);
-	}
+				 hw->settings->fs_table[id].fs_avl[i].gain);
 	buf[len - 1] = '\n';
 
 	return len;
-- 
2.20.1

Re: [BUG] Re: [PATCH] iio: imu: st_lsm6dsx: remove invalid gain value for LSM9DS1

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 2299 bytes --]

> On 27.08.19 22:08, Jonathan Cameron wrote:
> > On Tue, 27 Aug 2019 10:26:35 +0200
> > Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> > 
> >> Get rid of invalid sensitivity value for LSM9DS1 gyro sensor
> >>
> >> Fixes: 687a60feb9c6 ("iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9ds1")
> >> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > The zero degree scale is certainly odd otherwise, so good to tidy
> > this up.
> > 
> > Applied to the togreg branch of iio.git.
> > 
> 
> Hi Jon,
> 
> you have applied this too quickly. I've left that zero value in there
> for a reason, see below:
> 
> 
> > Thanks,
> > 
> > Jonathan
> > 
> >> ---

[...]

> 
> You Ooops here and it's pretty obvious! You don't have
> ST_LSM6DSX_FS_LIST_SIZE number of elements in the array anymore, but you
> try to access it (the 4th).

Hi Martin,

according to pahole (x86_64): 

struct st_lsm6dsx_fs {
	 [...]
	 /* size: 8, cachelines: 1, members: 2 *
};

struct st_lsm6dsx_fs_table_entry {
	[...]
	struct st_lsm6dsx_fs fs_avl[4];                  /*     4    32 */
	/* size: 36, cachelines: 1, members: 2 */
};

struct st_lsm6dsx_settings {
	[...]
	struct st_lsm6dsx_fs_table_entry fs_table[2];    /*   284    72 */
	/* size: 464, cachelines: 8, members: 14 */
};

struct st_lsm6dsx_fs_table_entry in st_lsm6dsx_settings will always have 4
elements for fs_avl array and since the array is defined as static the
uninitialized elements are set to 0.

Could you please share the ops you are getting?

Regards,
Lorenzo

> 
> I suggest reverting this (if not able to delete it entirely) and start
> over in case this "invalid" value thing hurts and needs to get fixed.
> 
> I any case, there _is_ something we should do because it's not too
> obvious what constraints the st_lsm6dsx_sensor_settings struct
> definition has. It should be mostly clear when looking at the header but
> a few inline comments might help.
> 
> thanks,
> 
>                           martin
> 
> 
> 
> >> +
> >>  		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06u ",
> >> -				 hw->settings->fs_table[id].fs_avl[i].gain);
> >> +				 fs_table->fs_avl[i].gain);
> >> +	}
> >>  	buf[len - 1] = '\n';
> >>  
> >>  	return len;
> > 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [BUG] Re: [PATCH] iio: imu: st_lsm6dsx: remove invalid gain value for LSM9DS1

[Martin Kepplinger]

On 29.08.19 10:37, Lorenzo Bianconi wrote:
>> On 27.08.19 22:08, Jonathan Cameron wrote:
>>> On Tue, 27 Aug 2019 10:26:35 +0200
>>> Lorenzo Bianconi <lorenzo@kernel.org> wrote:
>>>
>>>> Get rid of invalid sensitivity value for LSM9DS1 gyro sensor
>>>>
>>>> Fixes: 687a60feb9c6 ("iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9ds1")
>>>> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
>>> The zero degree scale is certainly odd otherwise, so good to tidy
>>> this up.
>>>
>>> Applied to the togreg branch of iio.git.
>>>
>>
>> Hi Jon,
>>
>> you have applied this too quickly. I've left that zero value in there
>> for a reason, see below:
>>
>>
>>> Thanks,
>>>
>>> Jonathan
>>>
>>>> ---
> 
> [...]
> 
>>
>> You Ooops here and it's pretty obvious! You don't have
>> ST_LSM6DSX_FS_LIST_SIZE number of elements in the array anymore, but you
>> try to access it (the 4th).
> 
> Hi Martin,
> 
> according to pahole (x86_64): 
> 
> struct st_lsm6dsx_fs {
> 	 [...]
> 	 /* size: 8, cachelines: 1, members: 2 *
> };
> 
> struct st_lsm6dsx_fs_table_entry {
> 	[...]
> 	struct st_lsm6dsx_fs fs_avl[4];                  /*     4    32 */
> 	/* size: 36, cachelines: 1, members: 2 */
> };
> 
> struct st_lsm6dsx_settings {
> 	[...]
> 	struct st_lsm6dsx_fs_table_entry fs_table[2];    /*   284    72 */
> 	/* size: 464, cachelines: 8, members: 14 */
> };
> 
> struct st_lsm6dsx_fs_table_entry in st_lsm6dsx_settings will always have 4
> elements for fs_avl array and since the array is defined as static the
> uninitialized elements are set to 0.
> 
> Could you please share the ops you are getting?
> 

How this oops during startup can look like is appended below. I know
that exactly this change causes it. Can you test this too please?

Given the cleanup nature of this patch, I think it's best to revert it
in case of any doubt.

thanks,

                          martin



Feb 14 10:12:00 pureos kernel: [    4.056014] Data abort info:
Feb 14 10:12:00 pureos kernel: [    4.061361] bq25890-charger 0-006b:
Capacity for 4184000 is 99%
Feb 14 10:12:00 pureos kernel: [    4.064751]   ISV = 0, ISS = 0x00000004
Feb 14 10:12:00 pureos kernel: [    4.070942]   CM = 0, WnR = 0
Feb 14 10:12:00 pureos kernel: [    4.076588] user pgtable: 4k pages,
48-bit VAs, pgdp=00000000e4fa6000
Feb 14 10:12:00 pureos kernel: [    4.086256] [0000000000000018]
pgd=0000000000000000
Feb 14 10:12:00 pureos kernel: [    4.100224] Internal error: Oops:
96000004 [#1] PREEMPT SMP
Feb 14 10:12:00 pureos kernel: [    4.101985] Goodix-TS 2-005d: ID 5688,
version: 0100
Feb 14 10:12:00 pureos kernel: [    4.106038] Modules linked in:
crct10dif_ce(+) st_magn_i2c(+) st_lsm6dsx_i2c(+) st_magn st_lsm6dsx
snd_soc_sgtl5000(+) st_sensors_i2c st_sensors tcpci
industrialio_triggered_buffer vcnl4000 kfifo_buf snd_soc_simple_card
snd_soc_fsl_sai tcpm ghash_ce bq25890_charger snd_soc_gtm601
snd_soc_simple_card_utils imx_pcm_dma goodix(+) roles snd_soc_core typec
sha2_ce snd_pcm_dmaengine snd_pcm imx_sdma imx2_wdt sha1_ce snvs_pwrkey
gpio_vibra qoriq_thermal virt_dma snd_timer watchdog snd soundcore
usb_f_acm u_serial usb_f_rndis g_multi usb_f_mass_storage u_ether
libcomposite ip_tables x_tables ipv6 nf_defrag_ipv6 xhci_plat_hcd
xhci_hcd usbcore dwc3 ulpi udc_core usb_common phy_fsl_imx8mq_usb
Feb 14 10:12:00 pureos kernel: [    4.113252] Goodix-TS 2-005d: Direct
firmware load for goodix_5688_cfg.bin failed with error -2
Feb 14 10:12:00 pureos kernel: [    4.174094] CPU: 2 PID: 341 Comm:
systemd-udevd Tainted: G        W         5.3.0-rc2-gc652c7f46913 #141
Feb 14 10:12:00 pureos kernel: [    4.174096] Hardware name: Purism
Librem 5 devkit (DT)
Feb 14 10:12:00 pureos kernel: [    4.174100] pstate: 80000005 (Nzcv
daif -PAN -UAO)
Feb 14 10:12:00 pureos kernel: [    4.174113] pc :
st_lsm6dsx_i2c_probe+0x18/0x80 [st_lsm6dsx_i2c]
Feb 14 10:12:00 pureos kernel: [    4.174124] lr :
i2c_device_probe+0x1f0/0x2b8
Feb 14 10:12:00 pureos kernel: [    4.174126] sp : ffff8000a4837970
Feb 14 10:12:00 pureos kernel: [    4.174128] x29: ffff8000a4837970 x28:
0000000000000000
Feb 14 10:12:00 pureos kernel: [    4.174132] x27: ffff000010b70000 x26:
ffff8000a4837d68
Feb 14 10:12:00 pureos kernel: [    4.174135] x25: ffff000010860000 x24:
ffff000008a94038
Feb 14 10:12:00 pureos kernel: [    4.174139] x23: ffff000008a94038 x22:
ffff000008a94000
Feb 14 10:12:00 pureos kernel: [    4.174142] x21: ffff8000a55a0400 x20:
ffff000008a92000
Feb 14 10:12:00 pureos kernel: [    4.174146] x19: ffff8000a55a0400 x18:
ffffffffffffffff
Feb 14 10:12:00 pureos kernel: [    4.174149] x17: 0000000000000000 x16:
0000000000000000
Feb 14 10:12:00 pureos kernel: [    4.174152] x15: 0000000000040000 x14:
00000000fffffff0
Feb 14 10:12:00 pureos kernel: [    4.174156] x13: 0000000000000001 x12:
0000000000000000
Feb 14 10:12:00 pureos kernel: [    4.174159] x11: 0000000000000000 x10:
0101010101010101
Feb 14 10:12:00 pureos kernel: [    4.174162] x9 : fffffffffffffffc x8 :
0000000000000008
Feb 14 10:12:00 pureos kernel: [    4.174166] x7 : 0000000000000004 x6 :
1e0e1a00f2ade4ef
Feb 14 10:12:00 pureos kernel: [    4.174169] x5 : 6f642d72001a0e1e x4 :
8080808000000000
Feb 14 10:12:00 pureos kernel: [    4.174173] x3 : 0000000000000000 x2 :
ffff000008a93000
Feb 14 10:12:00 pureos kernel: [    4.193031] driver: 'Goodix-TS':
driver_bound: bound to device '2-005d'
Feb 14 10:12:00 pureos kernel: [    4.193105] x1 : 0000000000000000 x0 :
ffff8000a55a0400
Feb 14 10:12:00 pureos kernel: [    4.199343] bus: 'i2c': really_probe:
bound device 2-005d to driver Goodix-TS
Feb 14 10:12:00 pureos kernel: [    4.203445] Call trace:
Feb 14 10:12:00 pureos kernel: [    4.203456]
st_lsm6dsx_i2c_probe+0x18/0x80 [st_lsm6dsx_i2c]
Feb 14 10:12:00 pureos kernel: [    4.203462]  i2c_device_probe+0x1f0/0x2b8
Feb 14 10:12:00 pureos kernel: [    4.203468]  really_probe+0x168/0x368
Feb 14 10:12:00 pureos kernel: [    4.203471]
driver_probe_device.part.2+0x10c/0x128
Feb 14 10:12:00 pureos kernel: [    4.203475]
device_driver_attach+0x74/0xa0
Feb 14 10:12:00 pureos kernel: [    4.203478]  __driver_attach+0x84/0x130
Feb 14 10:12:00 pureos kernel: [    4.203481]  bus_for_each_dev+0x68/0xc8
Feb 14 10:12:00 pureos kernel: [    4.203484]  driver_attach+0x20/0x28
Feb 14 10:12:00 pureos kernel: [    4.203487]  bus_add_driver+0xd4/0x1f8
Feb 14 10:12:00 pureos kernel: [    4.203490]  driver_register+0x60/0x110
Feb 14 10:12:00 pureos kernel: [    4.203497]  i2c_register_driver+0x44/0x98
Feb 14 10:12:00 pureos kernel: [    4.216908] devices_kset: Moving sound
to end of list
Feb 14 10:12:00 pureos kernel: [    4.217738]
st_lsm6dsx_driver_init+0x1c/0x1000 [st_lsm6dsx_i2c]
Feb 14 10:12:00 pureos kernel: [    4.217744]  do_one_initcall+0x58/0x1a8
Feb 14 10:12:00 pureos kernel: [    4.217749]  do_init_module+0x54/0x1d4
Feb 14 10:12:00 pureos kernel: [    4.217752]  load_module+0x1998/0x1c40
Feb 14 10:12:00 pureos kernel: [    4.217755]
__se_sys_finit_module+0xc0/0xd8
Feb 14 10:12:00 pureos kernel: [    4.217761]
__arm64_sys_finit_module+0x14/0x20
Feb 14 10:12:00 pureos kernel: [    4.223515] PM: Moving platform:sound
to end of list
Feb 14 10:12:00 pureos kernel: [    4.228823]
el0_svc_common.constprop.0+0xb0/0x168
Feb 14 10:12:00 pureos kernel: [    4.228827]  el0_svc_handler+0x18/0x20
Feb 14 10:12:00 pureos kernel: [    4.228830]  el0_svc+0x8/0xc
Feb 14 10:12:00 pureos kernel: [    4.228838] Code: d2800003 910003fd
a90153f3 aa0003f3 (f9400c34)
Feb 14 10:12:00 pureos kernel: [    4.228843] ---[ end trace
4933f7b108d54662 ]---

Re: [BUG] Re: [PATCH] iio: imu: st_lsm6dsx: remove invalid gain value for LSM9DS1

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 14587 bytes --]

> On 29.08.19 10:37, Lorenzo Bianconi wrote:
> >> On 27.08.19 22:08, Jonathan Cameron wrote:
> >>> On Tue, 27 Aug 2019 10:26:35 +0200
> >>> Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> >>>
> >>>> Get rid of invalid sensitivity value for LSM9DS1 gyro sensor
> >>>>
> >>>> Fixes: 687a60feb9c6 ("iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9ds1")
> >>>> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> >>> The zero degree scale is certainly odd otherwise, so good to tidy
> >>> this up.
> >>>
> >>> Applied to the togreg branch of iio.git.
> >>>

[...]

> > struct st_lsm6dsx_fs_table_entry in st_lsm6dsx_settings will always have 4
> > elements for fs_avl array and since the array is defined as static the
> > uninitialized elements are set to 0.
> > 
> > Could you please share the ops you are getting?
> > 
> 
> How this oops during startup can look like is appended below. I know
> that exactly this change causes it. Can you test this too please?

I did it but I have no issues

> 
> Given the cleanup nature of this patch, I think it's best to revert it
> in case of any doubt.
> 
> thanks,
> 
>                           martin
> 

is it the full ops? It seems some parts are missing.
Are you running some userspace aps reading in_anglvel_scale_available or reading/writing in in_anglvel_scale?
Could you please double check if the following patch helps? (just compiled)

Regards,
Lorenzo

---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  1 +
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 28 +++++++++++++-------
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c |  3 ++-
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index 5e3cd96b0059..a6cd736fd273 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -100,6 +100,7 @@ struct st_lsm6dsx_fs {
 struct st_lsm6dsx_fs_table_entry {
 	struct st_lsm6dsx_reg reg;
 	struct st_lsm6dsx_fs fs_avl[ST_LSM6DSX_FS_LIST_SIZE];
+	int len;
 };
 
 /**
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 2d3495560136..37a2aa211757 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -145,6 +145,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.fs_avl[1] = { IIO_G_TO_M_S_2(122), 0x2 },
 				.fs_avl[2] = { IIO_G_TO_M_S_2(244), 0x3 },
 				.fs_avl[3] = { IIO_G_TO_M_S_2(732), 0x1 },
+				.len = 4,
 			},
 			[ST_LSM6DSX_ID_GYRO] = {
 				.reg = {
@@ -154,6 +155,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.fs_avl[0] = {  IIO_DEGREE_TO_RAD(245), 0x0 },
 				.fs_avl[1] = {  IIO_DEGREE_TO_RAD(500), 0x1 },
 				.fs_avl[2] = { IIO_DEGREE_TO_RAD(2000), 0x3 },
+				.len = 3,
 			},
 		},
 	},
@@ -215,6 +217,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.fs_avl[1] = { IIO_G_TO_M_S_2(122), 0x2 },
 				.fs_avl[2] = { IIO_G_TO_M_S_2(244), 0x3 },
 				.fs_avl[3] = { IIO_G_TO_M_S_2(488), 0x1 },
+				.len = 4,
 			},
 			[ST_LSM6DSX_ID_GYRO] = {
 				.reg = {
@@ -225,6 +228,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 },
 				.fs_avl[2] = { IIO_DEGREE_TO_RAD(35000), 0x2 },
 				.fs_avl[3] = { IIO_DEGREE_TO_RAD(70000), 0x3 },
+				.len = 4,
 			},
 		},
 		.decimator = {
@@ -327,6 +331,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.fs_avl[1] = { IIO_G_TO_M_S_2(122), 0x2 },
 				.fs_avl[2] = { IIO_G_TO_M_S_2(244), 0x3 },
 				.fs_avl[3] = { IIO_G_TO_M_S_2(488), 0x1 },
+				.len = 4,
 			},
 			[ST_LSM6DSX_ID_GYRO] = {
 				.reg = {
@@ -337,6 +342,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 },
 				.fs_avl[2] = { IIO_DEGREE_TO_RAD(35000), 0x2 },
 				.fs_avl[3] = { IIO_DEGREE_TO_RAD(70000), 0x3 },
+				.len = 4,
 			},
 		},
 		.decimator = {
@@ -448,6 +454,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.fs_avl[1] = { IIO_G_TO_M_S_2(122), 0x2 },
 				.fs_avl[2] = { IIO_G_TO_M_S_2(244), 0x3 },
 				.fs_avl[3] = { IIO_G_TO_M_S_2(488), 0x1 },
+				.len = 4,
 			},
 			[ST_LSM6DSX_ID_GYRO] = {
 				.reg = {
@@ -458,6 +465,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 },
 				.fs_avl[2] = { IIO_DEGREE_TO_RAD(35000), 0x2 },
 				.fs_avl[3] = { IIO_DEGREE_TO_RAD(70000), 0x3 },
+				.len = 4,
 			},
 		},
 		.decimator = {
@@ -563,6 +571,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.fs_avl[1] = { IIO_G_TO_M_S_2(122), 0x2 },
 				.fs_avl[2] = { IIO_G_TO_M_S_2(244), 0x3 },
 				.fs_avl[3] = { IIO_G_TO_M_S_2(488), 0x1 },
+				.len = 4,
 			},
 			[ST_LSM6DSX_ID_GYRO] = {
 				.reg = {
@@ -573,6 +582,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 },
 				.fs_avl[2] = { IIO_DEGREE_TO_RAD(35000), 0x2 },
 				.fs_avl[3] = { IIO_DEGREE_TO_RAD(70000), 0x3 },
+				.len = 4,
 			},
 		},
 		.batch = {
@@ -693,6 +703,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.fs_avl[1] = { IIO_G_TO_M_S_2(122), 0x2 },
 				.fs_avl[2] = { IIO_G_TO_M_S_2(244), 0x3 },
 				.fs_avl[3] = { IIO_G_TO_M_S_2(488), 0x1 },
+				.len = 4,
 			},
 			[ST_LSM6DSX_ID_GYRO] = {
 				.reg = {
@@ -703,6 +714,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 },
 				.fs_avl[2] = { IIO_DEGREE_TO_RAD(35000), 0x2 },
 				.fs_avl[3] = { IIO_DEGREE_TO_RAD(70000), 0x3 },
+				.len = 4,
 			},
 		},
 		.batch = {
@@ -800,6 +812,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.fs_avl[1] = { IIO_G_TO_M_S_2(122), 0x2 },
 				.fs_avl[2] = { IIO_G_TO_M_S_2(244), 0x3 },
 				.fs_avl[3] = { IIO_G_TO_M_S_2(488), 0x1 },
+				.len = 4,
 			},
 			[ST_LSM6DSX_ID_GYRO] = {
 				.reg = {
@@ -810,6 +823,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 },
 				.fs_avl[2] = { IIO_DEGREE_TO_RAD(35000), 0x2 },
 				.fs_avl[3] = { IIO_DEGREE_TO_RAD(70000), 0x3 },
+				.len = 4,
 			},
 		},
 		.batch = {
@@ -933,11 +947,12 @@ static int st_lsm6dsx_set_full_scale(struct st_lsm6dsx_sensor *sensor,
 	int i, err;
 
 	fs_table = &sensor->hw->settings->fs_table[sensor->id];
-	for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++)
+	for (i = 0; i < fs_table->len; i++) {
 		if (fs_table->fs_avl[i].gain == gain)
 			break;
+	}
 
-	if (i == ST_LSM6DSX_FS_LIST_SIZE)
+	if (i == fs_table->len)
 		return -EINVAL;
 
 	data = ST_LSM6DSX_SHIFT_VAL(fs_table->fs_avl[i].val,
@@ -1196,18 +1211,13 @@ static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev,
 {
 	struct st_lsm6dsx_sensor *sensor = iio_priv(dev_get_drvdata(dev));
 	const struct st_lsm6dsx_fs_table_entry *fs_table;
-	enum st_lsm6dsx_sensor_id id = sensor->id;
 	struct st_lsm6dsx_hw *hw = sensor->hw;
 	int i, len = 0;
 
-	fs_table = &hw->settings->fs_table[id];
-	for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++) {
-		if (!fs_table->fs_avl[i].gain)
-			break;
-
+	fs_table = &hw->settings->fs_table[sensor->id];
+	for (i = 0; i < fs_table->len; i++)
 		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06u ",
 				 fs_table->fs_avl[i].gain);
-	}
 	buf[len - 1] = '\n';
 
 	return len;
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
index 66fbcd94642d..dcf349278e8f 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
@@ -61,6 +61,7 @@ static const struct st_lsm6dsx_ext_dev_settings st_lsm6dsx_ext_dev_table[] = {
 				.gain = 1500,
 				.val = 0x0,
 			}, /* 1500 uG/LSB */
+			.len = 1,
 		},
 		.temp_comp = {
 			.addr = 0x60,
@@ -555,7 +556,7 @@ static ssize_t st_lsm6dsx_shub_scale_avail(struct device *dev,
 	int i, len = 0;
 
 	settings = sensor->ext_info.settings;
-	for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++) {
+	for (i = 0; i < settings->fs_table.len; i++) {
 		u16 val = settings->fs_table.fs_avl[i].gain;
 
 		if (val > 0)
-- 
2.21.0


> 
> 
> Feb 14 10:12:00 pureos kernel: [    4.056014] Data abort info:
> Feb 14 10:12:00 pureos kernel: [    4.061361] bq25890-charger 0-006b:
> Capacity for 4184000 is 99%
> Feb 14 10:12:00 pureos kernel: [    4.064751]   ISV = 0, ISS = 0x00000004
> Feb 14 10:12:00 pureos kernel: [    4.070942]   CM = 0, WnR = 0
> Feb 14 10:12:00 pureos kernel: [    4.076588] user pgtable: 4k pages,
> 48-bit VAs, pgdp=00000000e4fa6000
> Feb 14 10:12:00 pureos kernel: [    4.086256] [0000000000000018]
> pgd=0000000000000000
> Feb 14 10:12:00 pureos kernel: [    4.100224] Internal error: Oops:
> 96000004 [#1] PREEMPT SMP
> Feb 14 10:12:00 pureos kernel: [    4.101985] Goodix-TS 2-005d: ID 5688,
> version: 0100
> Feb 14 10:12:00 pureos kernel: [    4.106038] Modules linked in:
> crct10dif_ce(+) st_magn_i2c(+) st_lsm6dsx_i2c(+) st_magn st_lsm6dsx
> snd_soc_sgtl5000(+) st_sensors_i2c st_sensors tcpci
> industrialio_triggered_buffer vcnl4000 kfifo_buf snd_soc_simple_card
> snd_soc_fsl_sai tcpm ghash_ce bq25890_charger snd_soc_gtm601
> snd_soc_simple_card_utils imx_pcm_dma goodix(+) roles snd_soc_core typec
> sha2_ce snd_pcm_dmaengine snd_pcm imx_sdma imx2_wdt sha1_ce snvs_pwrkey
> gpio_vibra qoriq_thermal virt_dma snd_timer watchdog snd soundcore
> usb_f_acm u_serial usb_f_rndis g_multi usb_f_mass_storage u_ether
> libcomposite ip_tables x_tables ipv6 nf_defrag_ipv6 xhci_plat_hcd
> xhci_hcd usbcore dwc3 ulpi udc_core usb_common phy_fsl_imx8mq_usb
> Feb 14 10:12:00 pureos kernel: [    4.113252] Goodix-TS 2-005d: Direct
> firmware load for goodix_5688_cfg.bin failed with error -2
> Feb 14 10:12:00 pureos kernel: [    4.174094] CPU: 2 PID: 341 Comm:
> systemd-udevd Tainted: G        W         5.3.0-rc2-gc652c7f46913 #141
> Feb 14 10:12:00 pureos kernel: [    4.174096] Hardware name: Purism
> Librem 5 devkit (DT)
> Feb 14 10:12:00 pureos kernel: [    4.174100] pstate: 80000005 (Nzcv
> daif -PAN -UAO)
> Feb 14 10:12:00 pureos kernel: [    4.174113] pc :
> st_lsm6dsx_i2c_probe+0x18/0x80 [st_lsm6dsx_i2c]
> Feb 14 10:12:00 pureos kernel: [    4.174124] lr :
> i2c_device_probe+0x1f0/0x2b8
> Feb 14 10:12:00 pureos kernel: [    4.174126] sp : ffff8000a4837970
> Feb 14 10:12:00 pureos kernel: [    4.174128] x29: ffff8000a4837970 x28:
> 0000000000000000
> Feb 14 10:12:00 pureos kernel: [    4.174132] x27: ffff000010b70000 x26:
> ffff8000a4837d68
> Feb 14 10:12:00 pureos kernel: [    4.174135] x25: ffff000010860000 x24:
> ffff000008a94038
> Feb 14 10:12:00 pureos kernel: [    4.174139] x23: ffff000008a94038 x22:
> ffff000008a94000
> Feb 14 10:12:00 pureos kernel: [    4.174142] x21: ffff8000a55a0400 x20:
> ffff000008a92000
> Feb 14 10:12:00 pureos kernel: [    4.174146] x19: ffff8000a55a0400 x18:
> ffffffffffffffff
> Feb 14 10:12:00 pureos kernel: [    4.174149] x17: 0000000000000000 x16:
> 0000000000000000
> Feb 14 10:12:00 pureos kernel: [    4.174152] x15: 0000000000040000 x14:
> 00000000fffffff0
> Feb 14 10:12:00 pureos kernel: [    4.174156] x13: 0000000000000001 x12:
> 0000000000000000
> Feb 14 10:12:00 pureos kernel: [    4.174159] x11: 0000000000000000 x10:
> 0101010101010101
> Feb 14 10:12:00 pureos kernel: [    4.174162] x9 : fffffffffffffffc x8 :
> 0000000000000008
> Feb 14 10:12:00 pureos kernel: [    4.174166] x7 : 0000000000000004 x6 :
> 1e0e1a00f2ade4ef
> Feb 14 10:12:00 pureos kernel: [    4.174169] x5 : 6f642d72001a0e1e x4 :
> 8080808000000000
> Feb 14 10:12:00 pureos kernel: [    4.174173] x3 : 0000000000000000 x2 :
> ffff000008a93000
> Feb 14 10:12:00 pureos kernel: [    4.193031] driver: 'Goodix-TS':
> driver_bound: bound to device '2-005d'
> Feb 14 10:12:00 pureos kernel: [    4.193105] x1 : 0000000000000000 x0 :
> ffff8000a55a0400
> Feb 14 10:12:00 pureos kernel: [    4.199343] bus: 'i2c': really_probe:
> bound device 2-005d to driver Goodix-TS
> Feb 14 10:12:00 pureos kernel: [    4.203445] Call trace:
> Feb 14 10:12:00 pureos kernel: [    4.203456]
> st_lsm6dsx_i2c_probe+0x18/0x80 [st_lsm6dsx_i2c]
> Feb 14 10:12:00 pureos kernel: [    4.203462]  i2c_device_probe+0x1f0/0x2b8
> Feb 14 10:12:00 pureos kernel: [    4.203468]  really_probe+0x168/0x368
> Feb 14 10:12:00 pureos kernel: [    4.203471]
> driver_probe_device.part.2+0x10c/0x128
> Feb 14 10:12:00 pureos kernel: [    4.203475]
> device_driver_attach+0x74/0xa0
> Feb 14 10:12:00 pureos kernel: [    4.203478]  __driver_attach+0x84/0x130
> Feb 14 10:12:00 pureos kernel: [    4.203481]  bus_for_each_dev+0x68/0xc8
> Feb 14 10:12:00 pureos kernel: [    4.203484]  driver_attach+0x20/0x28
> Feb 14 10:12:00 pureos kernel: [    4.203487]  bus_add_driver+0xd4/0x1f8
> Feb 14 10:12:00 pureos kernel: [    4.203490]  driver_register+0x60/0x110
> Feb 14 10:12:00 pureos kernel: [    4.203497]  i2c_register_driver+0x44/0x98
> Feb 14 10:12:00 pureos kernel: [    4.216908] devices_kset: Moving sound
> to end of list
> Feb 14 10:12:00 pureos kernel: [    4.217738]
> st_lsm6dsx_driver_init+0x1c/0x1000 [st_lsm6dsx_i2c]
> Feb 14 10:12:00 pureos kernel: [    4.217744]  do_one_initcall+0x58/0x1a8
> Feb 14 10:12:00 pureos kernel: [    4.217749]  do_init_module+0x54/0x1d4
> Feb 14 10:12:00 pureos kernel: [    4.217752]  load_module+0x1998/0x1c40
> Feb 14 10:12:00 pureos kernel: [    4.217755]
> __se_sys_finit_module+0xc0/0xd8
> Feb 14 10:12:00 pureos kernel: [    4.217761]
> __arm64_sys_finit_module+0x14/0x20
> Feb 14 10:12:00 pureos kernel: [    4.223515] PM: Moving platform:sound
> to end of list
> Feb 14 10:12:00 pureos kernel: [    4.228823]
> el0_svc_common.constprop.0+0xb0/0x168
> Feb 14 10:12:00 pureos kernel: [    4.228827]  el0_svc_handler+0x18/0x20
> Feb 14 10:12:00 pureos kernel: [    4.228830]  el0_svc+0x8/0xc
> Feb 14 10:12:00 pureos kernel: [    4.228838] Code: d2800003 910003fd
> a90153f3 aa0003f3 (f9400c34)
> Feb 14 10:12:00 pureos kernel: [    4.228843] ---[ end trace
> 4933f7b108d54662 ]---
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [BUG] Re: [PATCH] iio: imu: st_lsm6dsx: remove invalid gain value for LSM9DS1

[Martin Kepplinger]

On 30.08.19 09:23, Lorenzo Bianconi wrote:
>> On 29.08.19 10:37, Lorenzo Bianconi wrote:
>>>> On 27.08.19 22:08, Jonathan Cameron wrote:
>>>>> On Tue, 27 Aug 2019 10:26:35 +0200
>>>>> Lorenzo Bianconi <lorenzo@kernel.org> wrote:
>>>>>
>>>>>> Get rid of invalid sensitivity value for LSM9DS1 gyro sensor
>>>>>>
>>>>>> Fixes: 687a60feb9c6 ("iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9ds1")
>>>>>> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
>>>>> The zero degree scale is certainly odd otherwise, so good to tidy
>>>>> this up.
>>>>>
>>>>> Applied to the togreg branch of iio.git.
>>>>>
> 
> [...]
> 
>>> struct st_lsm6dsx_fs_table_entry in st_lsm6dsx_settings will always have 4
>>> elements for fs_avl array and since the array is defined as static the
>>> uninitialized elements are set to 0.
>>>
>>> Could you please share the ops you are getting?
>>>
>>
>> How this oops during startup can look like is appended below. I know
>> that exactly this change causes it. Can you test this too please?
> 
> I did it but I have no issues
> 
>>
>> Given the cleanup nature of this patch, I think it's best to revert it
>> in case of any doubt.
>>
>> thanks,
>>
>>                           martin
>>
> 
> is it the full ops? It seems some parts are missing.
> Are you running some userspace aps reading in_anglvel_scale_available or reading/writing in in_anglvel_scale?
> Could you please double check if the following patch helps? (just compiled)
> 

it does not, and you're right, the problem should be somewhere else.
I've yet to debug it further.

thanks,

                        martin

Re: [BUG] Re: [PATCH] iio: imu: st_lsm6dsx: remove invalid gain value for LSM9DS1

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 1992 bytes --]

> On 30.08.19 09:23, Lorenzo Bianconi wrote:
> >> On 29.08.19 10:37, Lorenzo Bianconi wrote:
> >>>> On 27.08.19 22:08, Jonathan Cameron wrote:
> >>>>> On Tue, 27 Aug 2019 10:26:35 +0200
> >>>>> Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> >>>>>
> >>>>>> Get rid of invalid sensitivity value for LSM9DS1 gyro sensor
> >>>>>>
> >>>>>> Fixes: 687a60feb9c6 ("iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9ds1")
> >>>>>> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> >>>>> The zero degree scale is certainly odd otherwise, so good to tidy
> >>>>> this up.
> >>>>>
> >>>>> Applied to the togreg branch of iio.git.
> >>>>>
> > 
> > [...]
> > 
> >>> struct st_lsm6dsx_fs_table_entry in st_lsm6dsx_settings will always have 4
> >>> elements for fs_avl array and since the array is defined as static the
> >>> uninitialized elements are set to 0.
> >>>
> >>> Could you please share the ops you are getting?
> >>>
> >>
> >> How this oops during startup can look like is appended below. I know
> >> that exactly this change causes it. Can you test this too please?
> > 
> > I did it but I have no issues
> > 
> >>
> >> Given the cleanup nature of this patch, I think it's best to revert it
> >> in case of any doubt.
> >>
> >> thanks,
> >>
> >>                           martin
> >>
> > 
> > is it the full ops? It seems some parts are missing.
> > Are you running some userspace aps reading in_anglvel_scale_available or reading/writing in in_anglvel_scale?
> > Could you please double check if the following patch helps? (just compiled)
> > 
> 
> it does not, and you're right, the problem should be somewhere else.
> I've yet to debug it further.
> 
> thanks,

Looking at the previous patch I spotted an issue (not related to the one you
are facing)..actually we can set device gain to 0 forcing to 0 sensor output.
I will post a formal patch to fix it.

Regards,
Lorenzo

> 
>                         martin
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [BUG] Re: [PATCH] iio: imu: st_lsm6dsx: remove invalid gain value for LSM9DS1

[Jonathan Cameron]

On Sat, 31 Aug 2019 11:32:24 +0200
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> > On 30.08.19 09:23, Lorenzo Bianconi wrote:  
> > >> On 29.08.19 10:37, Lorenzo Bianconi wrote:  
> > >>>> On 27.08.19 22:08, Jonathan Cameron wrote:  
> > >>>>> On Tue, 27 Aug 2019 10:26:35 +0200
> > >>>>> Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> > >>>>>  
> > >>>>>> Get rid of invalid sensitivity value for LSM9DS1 gyro sensor
> > >>>>>>
> > >>>>>> Fixes: 687a60feb9c6 ("iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9ds1")
> > >>>>>> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>  
> > >>>>> The zero degree scale is certainly odd otherwise, so good to tidy
> > >>>>> this up.
> > >>>>>
> > >>>>> Applied to the togreg branch of iio.git.
> > >>>>>  
> > > 
> > > [...]
> > >   
> > >>> struct st_lsm6dsx_fs_table_entry in st_lsm6dsx_settings will always have 4
> > >>> elements for fs_avl array and since the array is defined as static the
> > >>> uninitialized elements are set to 0.
> > >>>
> > >>> Could you please share the ops you are getting?
> > >>>  
> > >>
> > >> How this oops during startup can look like is appended below. I know
> > >> that exactly this change causes it. Can you test this too please?  
> > > 
> > > I did it but I have no issues
> > >   
> > >>
> > >> Given the cleanup nature of this patch, I think it's best to revert it
> > >> in case of any doubt.
> > >>
> > >> thanks,
> > >>
> > >>                           martin
> > >>  
> > > 
> > > is it the full ops? It seems some parts are missing.
> > > Are you running some userspace aps reading in_anglvel_scale_available or reading/writing in in_anglvel_scale?
> > > Could you please double check if the following patch helps? (just compiled)
> > >   
> > 
> > it does not, and you're right, the problem should be somewhere else.
> > I've yet to debug it further.
> > 
> > thanks,  
> 
> Looking at the previous patch I spotted an issue (not related to the one you
> are facing)..actually we can set device gain to 0 forcing to 0 sensor output.
> I will post a formal patch to fix it.
> 

Rather than messing around with a pull request that has already gone
to Greg, I'm going to leave this as it is for now.

The trace doesn't make a lot of sense to me and whilst a bit messy (as fixed
up by Lorenzo's follow up) I can't see why things would crash.

So needs debugging and that isn't a problem given we are only looking
at putting this support into rc1.  Not ideal, but there is time
to work out what is wrong and fix it up!

Thanks,

Jonathan


> Regards,
> Lorenzo
> 
> > 
> >                         martin
> > 
> >   

Re: [BUG] Re: [PATCH] iio: imu: st_lsm6dsx: remove invalid gain value for LSM9DS1

[Martin Kepplinger]

On 01.09.19 16:50, Jonathan Cameron wrote:

[...]

> 
> Rather than messing around with a pull request that has already gone
> to Greg, I'm going to leave this as it is for now.
> 
> The trace doesn't make a lot of sense to me and whilst a bit messy (as fixed
> up by Lorenzo's follow up) I can't see why things would crash.
> 
> So needs debugging and that isn't a problem given we are only looking
> at putting this support into rc1.  Not ideal, but there is time
> to work out what is wrong and fix it up!
> 
> Thanks,
> 
> Jonathan
> 

for the record, I found the problem I've had here. It's been introduced
with the DEV_NAME change and I sent a fix (and question):

https://lore.kernel.org/linux-iio/20190903051802.22716-1-martin.kepplinger@puri.sm/T/#u

thanks,

                                   martin

Re: [BUG] Re: [PATCH] iio: imu: st_lsm6dsx: remove invalid gain value for LSM9DS1

[Jonathan Cameron]

On Tue, 3 Sep 2019 07:22:20 +0200
Martin Kepplinger <martink@posteo.de> wrote:

> On 01.09.19 16:50, Jonathan Cameron wrote:
> 
> [...]
> 
> > 
> > Rather than messing around with a pull request that has already gone
> > to Greg, I'm going to leave this as it is for now.
> > 
> > The trace doesn't make a lot of sense to me and whilst a bit messy (as fixed
> > up by Lorenzo's follow up) I can't see why things would crash.
> > 
> > So needs debugging and that isn't a problem given we are only looking
> > at putting this support into rc1.  Not ideal, but there is time
> > to work out what is wrong and fix it up!
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> 
> for the record, I found the problem I've had here. It's been introduced
> with the DEV_NAME change and I sent a fix (and question):
> 
> https://lore.kernel.org/linux-iio/20190903051802.22716-1-martin.kepplinger@puri.sm/T/#u
> 

Doh. This one is entirely my fault as I messed up that last minute edit :(

Greg, this bug is in my outstanding pull request. If you would prefer to
pick it from lore and add my Acked-by that's great.
If not I can send this after rc1. 

Thanks,

Jonathan