[PATCH] iio: magnetometer: si7210: fix magnetic field measurement scale

[PATCH] iio: magnetometer: si7210: fix magnetic field measurement scale

[Antoni Pokusinski]

Applying the current scale value to the raw magnetic field measurements
gives the result in mT.

Fix the scale by increasing it 10 times, so that the final result after
applying the scale is in Gauss.

Fixes: cb29542a178f ("iio: magnetometer: si7210: add driver for Si7210")
Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
---
 drivers/iio/magnetometer/si7210.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/magnetometer/si7210.c b/drivers/iio/magnetometer/si7210.c
index 43b00d76505a..27e3feba7a0f 100644
--- a/drivers/iio/magnetometer/si7210.c
+++ b/drivers/iio/magnetometer/si7210.c
@@ -203,9 +203,9 @@ static int si7210_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SCALE:
 		*val = 0;
 		if (data->curr_scale == 20)
-			*val2 = 1250;
-		else /* data->curr_scale == 200 */
 			*val2 = 12500;
+		else /* data->curr_scale == 200 */
+			*val2 = 125000;
 		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_OFFSET:
 		*val = -16384;
@@ -274,9 +274,9 @@ static int si7210_write_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
-		if (val == 0 && val2 == 1250)
+		if (val == 0 && val2 == 12500)
 			scale = 20;
-		else if (val == 0 && val2 == 12500)
+		else if (val == 0 && val2 == 125000)
 			scale = 200;
 		else
 			return -EINVAL;

base-commit: aa61400ca17e264a4b597e3c0cda011c6b9b3bb5
-- 
2.25.1

Re: [PATCH] iio: magnetometer: si7210: fix magnetic field measurement scale

[Andy Shevchenko]

On Sun, Feb 02, 2025 at 11:07:10AM +0100, Antoni Pokusinski wrote:
> Applying the current scale value to the raw magnetic field measurements
> gives the result in mT.
> 
> Fix the scale by increasing it 10 times, so that the final result after
> applying the scale is in Gauss.

No objections against this change, just wondering since these are
the ABI changes (correct?) how should we really handle them in case
some of the user space stuff already relies on 'bad' values?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: magnetometer: si7210: fix magnetic field measurement scale

[Jonathan Cameron]

On Mon, 3 Feb 2025 11:26:27 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Sun, Feb 02, 2025 at 11:07:10AM +0100, Antoni Pokusinski wrote:
> > Applying the current scale value to the raw magnetic field measurements
> > gives the result in mT.
> > 
> > Fix the scale by increasing it 10 times, so that the final result after
> > applying the scale is in Gauss.  
> 
> No objections against this change, just wondering since these are
> the ABI changes (correct?) how should we really handle them in case
> some of the user space stuff already relies on 'bad' values?
> 
If it's broken wrt to the published ABI (and hence what other sensors
are exporting) then not a whole lot we can do other than
backport the fix.  This is one of the few cases where ABI backwards
compatibility comes second :(

Jonathan

Re: [PATCH] iio: magnetometer: si7210: fix magnetic field measurement scale

[Antoni Pokusinski]

On Mon, Feb 03, 2025 at 11:26:27AM +0200, Andy Shevchenko wrote:
> On Sun, Feb 02, 2025 at 11:07:10AM +0100, Antoni Pokusinski wrote:
> > Applying the current scale value to the raw magnetic field measurements
> > gives the result in mT.
> > 
> > Fix the scale by increasing it 10 times, so that the final result after
> > applying the scale is in Gauss.
> 
> No objections against this change, just wondering since these are
> the ABI changes (correct?) how should we really handle them in case
> some of the user space stuff already relies on 'bad' values?
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Just to clarify: yes, this patch makes the output values compliant to
ABI (Documentation/ABI/testing/sysfs-bus-iio). According to the ABI, the
output values for the magnetic field should be in Gauss, but during
the driver development I missed the fact that I return values in mT
instead... (1 Gauss = 0.1mT)

BTW I see that the patch cb29542a178f ("iio: magnetometer: si7210: add
driver for Si7210") is still on the "testing" branch only, so perhaps
it's still not too late to apply the fix?

Kind regards,
Antoni

Re: [PATCH] iio: magnetometer: si7210: fix magnetic field measurement scale

[Jonathan Cameron]

On Mon, 3 Feb 2025 20:39:20 +0100
Antoni Pokusinski <apokusinski01@gmail.com> wrote:

> On Mon, Feb 03, 2025 at 11:26:27AM +0200, Andy Shevchenko wrote:
> > On Sun, Feb 02, 2025 at 11:07:10AM +0100, Antoni Pokusinski wrote:  
> > > Applying the current scale value to the raw magnetic field measurements
> > > gives the result in mT.
> > > 
> > > Fix the scale by increasing it 10 times, so that the final result after
> > > applying the scale is in Gauss.  
> > 
> > No objections against this change, just wondering since these are
> > the ABI changes (correct?) how should we really handle them in case
> > some of the user space stuff already relies on 'bad' values?
> > 
> > -- 
> > With Best Regards,
> > Andy Shevchenko
> > 
> >   
> 
> Just to clarify: yes, this patch makes the output values compliant to
> ABI (Documentation/ABI/testing/sysfs-bus-iio). According to the ABI, the
> output values for the magnetic field should be in Gauss, but during
> the driver development I missed the fact that I return values in mT
> instead... (1 Gauss = 0.1mT)
> 
> BTW I see that the patch cb29542a178f ("iio: magnetometer: si7210: add
> driver for Si7210") is still on the "testing" branch only, so perhaps
> it's still not too late to apply the fix?
> 
Ah. I'd not registered that.  Definitely makes life easier.
This will just get queued up behind it and land upstream at the same
time.

Jonathan

> Kind regards,
> Antoni
> 
> 

Re: [PATCH] iio: magnetometer: si7210: fix magnetic field measurement scale

[Jonathan Cameron]

On Tue, 4 Feb 2025 19:24:54 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon, 3 Feb 2025 20:39:20 +0100
> Antoni Pokusinski <apokusinski01@gmail.com> wrote:
> 
> > On Mon, Feb 03, 2025 at 11:26:27AM +0200, Andy Shevchenko wrote:  
> > > On Sun, Feb 02, 2025 at 11:07:10AM +0100, Antoni Pokusinski wrote:    
> > > > Applying the current scale value to the raw magnetic field measurements
> > > > gives the result in mT.
> > > > 
> > > > Fix the scale by increasing it 10 times, so that the final result after
> > > > applying the scale is in Gauss.    
> > > 
> > > No objections against this change, just wondering since these are
> > > the ABI changes (correct?) how should we really handle them in case
> > > some of the user space stuff already relies on 'bad' values?
> > > 
> > > -- 
> > > With Best Regards,
> > > Andy Shevchenko
> > > 
> > >     
> > 
> > Just to clarify: yes, this patch makes the output values compliant to
> > ABI (Documentation/ABI/testing/sysfs-bus-iio). According to the ABI, the
> > output values for the magnetic field should be in Gauss, but during
> > the driver development I missed the fact that I return values in mT
> > instead... (1 Gauss = 0.1mT)
> > 
> > BTW I see that the patch cb29542a178f ("iio: magnetometer: si7210: add
> > driver for Si7210") is still on the "testing" branch only, so perhaps
> > it's still not too late to apply the fix?
> >   
> Ah. I'd not registered that.  Definitely makes life easier.
> This will just get queued up behind it and land upstream at the same
> time.
> 
> Jonathan
Applied.
> 
> > Kind regards,
> > Antoni
> > 
> >   
> 
>