[PATCH] iio: temperature: mlx90632 Relax the compatibility check

[PATCH] iio: temperature: mlx90632 Relax the compatibility check

[Crt Mori]

Register EE_VERSION contains mixture of calibration information and DSP
version. So far, because calibrations were definite, the driver
compatibility depended on whole contents, but in the newer production
process the calibration part changes. Because of that, value in EE_VERSION
will be changed and to avoid that calibration value is same as DSP version
the MSB in calibration part was fixed to 1.
That means existing calibrations (medical and consumer) will now have
hex values (bits 8 to 15) of 83 and 84 respectively. Driver compatibility
should be based only on DSP version part of the EE_VERSION (bits 0 to 7)
register.

Signed-off-by: Crt Mori <cmo@melexis.com>
---
 drivers/iio/temperature/mlx90632.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/temperature/mlx90632.c b/drivers/iio/temperature/mlx90632.c
index 2243e8057ffc..2d54d9cac61d 100644
--- a/drivers/iio/temperature/mlx90632.c
+++ b/drivers/iio/temperature/mlx90632.c
@@ -81,6 +81,8 @@
 /* Magic constants */
 #define MLX90632_ID_MEDICAL	0x0105 /* EEPROM DSPv5 Medical device id */
 #define MLX90632_ID_CONSUMER	0x0205 /* EEPROM DSPv5 Consumer device id */
+#define MLX90632_DSP_VERSION	5 /* DSP version */
+#define MLX90632_DSP_MASK	GENMASK(7, 0) /* DSP version in EE_VERSION */
 #define MLX90632_RESET_CMD	0x0006 /* Reset sensor (address or global) */
 #define MLX90632_REF_12		12LL /**< ResCtrlRef value of Ch 1 or Ch 2 */
 #define MLX90632_REF_3		12LL /**< ResCtrlRef value of Channel 3 */
@@ -666,10 +668,13 @@ static int mlx90632_probe(struct i2c_client *client,
 	} else if (read == MLX90632_ID_CONSUMER) {
 		dev_dbg(&client->dev,
 			"Detected Consumer EEPROM calibration %x\n", read);
+	} else if ((read & MLX90632_DSP_MASK) == MLX90632_DSP_VERSION) {
+		dev_dbg(&client->dev,
+			"Detected Unknown EEPROM calibration %x\n", read);
 	} else {
 		dev_err(&client->dev,
-			"Wrong DSP version %x (expected %x or %x)\n",
-			read, MLX90632_DSPv5);
+			"Wrong DSP version %x (expected %x)\n",
+			read, MLX90632_DSP_VERSION);
 		return -EPROTONOSUPPORT;
 	}
 
-- 
2.20.1

Re: [PATCH] iio: temperature: mlx90632 Relax the compatibility check

[Jonathan Cameron]

On Thu, 23 May 2019 14:07:22 +0200
Crt Mori <cmo@melexis.com> wrote:

> Register EE_VERSION contains mixture of calibration information and DSP
> version. So far, because calibrations were definite, the driver
> compatibility depended on whole contents, but in the newer production
> process the calibration part changes. Because of that, value in EE_VERSION
> will be changed and to avoid that calibration value is same as DSP version
> the MSB in calibration part was fixed to 1.
> That means existing calibrations (medical and consumer) will now have
> hex values (bits 8 to 15) of 83 and 84 respectively. Driver compatibility
> should be based only on DSP version part of the EE_VERSION (bits 0 to 7)
> register.
> 
> Signed-off-by: Crt Mori <cmo@melexis.com>
Hi.

I'm going to take this via the slow path as you haven't called it out that
you want it applied as a fix (so for stable kernels).  Let me know if these
parts are in the wild and hence we should send it earlier.   We can do that
after it is in mainline anyway as a specific request to the stable maintainers.

Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to play with it.

Umm. This didn't actually apply to the current tree, so I did what I think
was intended by hand. Please take a look at the testing branch of iio.git
and check it is correct.

Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/iio/temperature/mlx90632.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/temperature/mlx90632.c b/drivers/iio/temperature/mlx90632.c
> index 2243e8057ffc..2d54d9cac61d 100644
> --- a/drivers/iio/temperature/mlx90632.c
> +++ b/drivers/iio/temperature/mlx90632.c
> @@ -81,6 +81,8 @@
>  /* Magic constants */
>  #define MLX90632_ID_MEDICAL	0x0105 /* EEPROM DSPv5 Medical device id */
>  #define MLX90632_ID_CONSUMER	0x0205 /* EEPROM DSPv5 Consumer device id */
> +#define MLX90632_DSP_VERSION	5 /* DSP version */
> +#define MLX90632_DSP_MASK	GENMASK(7, 0) /* DSP version in EE_VERSION */
>  #define MLX90632_RESET_CMD	0x0006 /* Reset sensor (address or global) */
>  #define MLX90632_REF_12		12LL /**< ResCtrlRef value of Ch 1 or Ch 2 */
>  #define MLX90632_REF_3		12LL /**< ResCtrlRef value of Channel 3 */
> @@ -666,10 +668,13 @@ static int mlx90632_probe(struct i2c_client *client,
>  	} else if (read == MLX90632_ID_CONSUMER) {
>  		dev_dbg(&client->dev,
>  			"Detected Consumer EEPROM calibration %x\n", read);
> +	} else if ((read & MLX90632_DSP_MASK) == MLX90632_DSP_VERSION) {
> +		dev_dbg(&client->dev,
> +			"Detected Unknown EEPROM calibration %x\n", read);
>  	} else {
>  		dev_err(&client->dev,
> -			"Wrong DSP version %x (expected %x or %x)\n",
> -			read, MLX90632_DSPv5);
> +			"Wrong DSP version %x (expected %x)\n",
> +			read, MLX90632_DSP_VERSION);
>  		return -EPROTONOSUPPORT;
>  	}
>  

Re: [PATCH] iio: temperature: mlx90632 Relax the compatibility check

[Crt Mori]

On Sun, 26 May 2019 at 18:48, Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Thu, 23 May 2019 14:07:22 +0200
> Crt Mori <cmo@melexis.com> wrote:
>
> > Register EE_VERSION contains mixture of calibration information and DSP
> > version. So far, because calibrations were definite, the driver
> > compatibility depended on whole contents, but in the newer production
> > process the calibration part changes. Because of that, value in EE_VERSION
> > will be changed and to avoid that calibration value is same as DSP version
> > the MSB in calibration part was fixed to 1.
> > That means existing calibrations (medical and consumer) will now have
> > hex values (bits 8 to 15) of 83 and 84 respectively. Driver compatibility
> > should be based only on DSP version part of the EE_VERSION (bits 0 to 7)
> > register.
> >
> > Signed-off-by: Crt Mori <cmo@melexis.com>
> Hi.
>
> I'm going to take this via the slow path as you haven't called it out that
> you want it applied as a fix (so for stable kernels).  Let me know if these
> parts are in the wild and hence we should send it earlier.   We can do that
> after it is in mainline anyway as a specific request to the stable maintainers.
Hi,
Since this is our change I did not think it warrants a fix label, but
if we could, we would be very happy, if this could go in for all
stable kernels. It is designed to be backwards compatible, so it
should not cause new problems.

> Applied to the togreg branch of iio.git and pushed out as testing for the
> autobuilders to play with it.
>
> Umm. This didn't actually apply to the current tree, so I did what I think
> was intended by hand. Please take a look at the testing branch of iio.git
> and check it is correct.
Sorry, but togreg branch does not even have temperature subdirectory
inside. Can you push testing to remote as well? Summary (git kernel
org) still has last update 8 days ago, or are you pushing somewhere
else?

>
> Applied to the togreg branch of iio.git and pushed out as testing for the
> autobuilders to play with it.
>
> Thanks,
>
> Jonathan
>
> > ---
> >  drivers/iio/temperature/mlx90632.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/temperature/mlx90632.c b/drivers/iio/temperature/mlx90632.c
> > index 2243e8057ffc..2d54d9cac61d 100644
> > --- a/drivers/iio/temperature/mlx90632.c
> > +++ b/drivers/iio/temperature/mlx90632.c
> > @@ -81,6 +81,8 @@
> >  /* Magic constants */
> >  #define MLX90632_ID_MEDICAL  0x0105 /* EEPROM DSPv5 Medical device id */
> >  #define MLX90632_ID_CONSUMER 0x0205 /* EEPROM DSPv5 Consumer device id */
> > +#define MLX90632_DSP_VERSION 5 /* DSP version */
> > +#define MLX90632_DSP_MASK    GENMASK(7, 0) /* DSP version in EE_VERSION */
> >  #define MLX90632_RESET_CMD   0x0006 /* Reset sensor (address or global) */
> >  #define MLX90632_REF_12              12LL /**< ResCtrlRef value of Ch 1 or Ch 2 */
> >  #define MLX90632_REF_3               12LL /**< ResCtrlRef value of Channel 3 */
> > @@ -666,10 +668,13 @@ static int mlx90632_probe(struct i2c_client *client,
> >       } else if (read == MLX90632_ID_CONSUMER) {
> >               dev_dbg(&client->dev,
> >                       "Detected Consumer EEPROM calibration %x\n", read);
> > +     } else if ((read & MLX90632_DSP_MASK) == MLX90632_DSP_VERSION) {
> > +             dev_dbg(&client->dev,
> > +                     "Detected Unknown EEPROM calibration %x\n", read);
> >       } else {
> >               dev_err(&client->dev,
> > -                     "Wrong DSP version %x (expected %x or %x)\n",
> > -                     read, MLX90632_DSPv5);
> > +                     "Wrong DSP version %x (expected %x)\n",
> > +                     read, MLX90632_DSP_VERSION);
> >               return -EPROTONOSUPPORT;
> >       }
> >
>

Re: [PATCH] iio: temperature: mlx90632 Relax the compatibility check

[Jonathan Cameron]

On Mon, 27 May 2019 09:40:53 +0200
Crt Mori <cmo@melexis.com> wrote:

> On Sun, 26 May 2019 at 18:48, Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Thu, 23 May 2019 14:07:22 +0200
> > Crt Mori <cmo@melexis.com> wrote:
> >  
> > > Register EE_VERSION contains mixture of calibration information and DSP
> > > version. So far, because calibrations were definite, the driver
> > > compatibility depended on whole contents, but in the newer production
> > > process the calibration part changes. Because of that, value in EE_VERSION
> > > will be changed and to avoid that calibration value is same as DSP version
> > > the MSB in calibration part was fixed to 1.
> > > That means existing calibrations (medical and consumer) will now have
> > > hex values (bits 8 to 15) of 83 and 84 respectively. Driver compatibility
> > > should be based only on DSP version part of the EE_VERSION (bits 0 to 7)
> > > register.
> > >
> > > Signed-off-by: Crt Mori <cmo@melexis.com>  
> > Hi.
> >
> > I'm going to take this via the slow path as you haven't called it out that
> > you want it applied as a fix (so for stable kernels).  Let me know if these
> > parts are in the wild and hence we should send it earlier.   We can do that
> > after it is in mainline anyway as a specific request to the stable maintainers.  
> Hi,
> Since this is our change I did not think it warrants a fix label, but
> if we could, we would be very happy, if this could go in for all
> stable kernels. It is designed to be backwards compatible, so it
> should not cause new problems.
Cool. I've moved it across to the fixes-togreg branch and should do a pull
request for that in a week or two at which point it'll get picked up
by the stable trees.

> 
> > Applied to the togreg branch of iio.git and pushed out as testing for the
> > autobuilders to play with it.
> >
> > Umm. This didn't actually apply to the current tree, so I did what I think
> > was intended by hand. Please take a look at the testing branch of iio.git
> > and check it is correct.  
> Sorry, but togreg branch does not even have temperature subdirectory
> inside. Can you push testing to remote as well? Summary (git kernel
> org) still has last update 8 days ago, or are you pushing somewhere
> else?
Umm. togreg should definitely have a temperature subdirectory.
I suspect this is the odd nature of the web interface for kernel.org.
For reasons I've never understood when you browse a 'tree' of a branch
other than master, it doesn't show the current head of that branch.
The easiest way to get there is to click on a particular commit and
then switch to the tree view.

For the other day it seems I hit an issue with the push and didn't notice.

Should be there now.

Jonathan

> 
> >
> > Applied to the togreg branch of iio.git and pushed out as testing for the
> > autobuilders to play with it.
> >
> > Thanks,
> >
> > Jonathan
> >  
> > > ---
> > >  drivers/iio/temperature/mlx90632.c | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/iio/temperature/mlx90632.c b/drivers/iio/temperature/mlx90632.c
> > > index 2243e8057ffc..2d54d9cac61d 100644
> > > --- a/drivers/iio/temperature/mlx90632.c
> > > +++ b/drivers/iio/temperature/mlx90632.c
> > > @@ -81,6 +81,8 @@
> > >  /* Magic constants */
> > >  #define MLX90632_ID_MEDICAL  0x0105 /* EEPROM DSPv5 Medical device id */
> > >  #define MLX90632_ID_CONSUMER 0x0205 /* EEPROM DSPv5 Consumer device id */
> > > +#define MLX90632_DSP_VERSION 5 /* DSP version */
> > > +#define MLX90632_DSP_MASK    GENMASK(7, 0) /* DSP version in EE_VERSION */
> > >  #define MLX90632_RESET_CMD   0x0006 /* Reset sensor (address or global) */
> > >  #define MLX90632_REF_12              12LL /**< ResCtrlRef value of Ch 1 or Ch 2 */
> > >  #define MLX90632_REF_3               12LL /**< ResCtrlRef value of Channel 3 */
> > > @@ -666,10 +668,13 @@ static int mlx90632_probe(struct i2c_client *client,
> > >       } else if (read == MLX90632_ID_CONSUMER) {
> > >               dev_dbg(&client->dev,
> > >                       "Detected Consumer EEPROM calibration %x\n", read);
> > > +     } else if ((read & MLX90632_DSP_MASK) == MLX90632_DSP_VERSION) {
> > > +             dev_dbg(&client->dev,
> > > +                     "Detected Unknown EEPROM calibration %x\n", read);
> > >       } else {
> > >               dev_err(&client->dev,
> > > -                     "Wrong DSP version %x (expected %x or %x)\n",
> > > -                     read, MLX90632_DSPv5);
> > > +                     "Wrong DSP version %x (expected %x)\n",
> > > +                     read, MLX90632_DSP_VERSION);
> > >               return -EPROTONOSUPPORT;
> > >       }
> > >  
> >  

Re: [PATCH] iio: temperature: mlx90632 Relax the compatibility check

[Crt Mori]

On Mon, 27 May 2019 at 12:00, Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 27 May 2019 09:40:53 +0200
> Crt Mori <cmo@melexis.com> wrote:
>
> > On Sun, 26 May 2019 at 18:48, Jonathan Cameron <jic23@kernel.org> wrote:
> > >
> > > On Thu, 23 May 2019 14:07:22 +0200
> > > Crt Mori <cmo@melexis.com> wrote:
> > >
> > > > Register EE_VERSION contains mixture of calibration information and DSP
> > > > version. So far, because calibrations were definite, the driver
> > > > compatibility depended on whole contents, but in the newer production
> > > > process the calibration part changes. Because of that, value in EE_VERSION
> > > > will be changed and to avoid that calibration value is same as DSP version
> > > > the MSB in calibration part was fixed to 1.
> > > > That means existing calibrations (medical and consumer) will now have
> > > > hex values (bits 8 to 15) of 83 and 84 respectively. Driver compatibility
> > > > should be based only on DSP version part of the EE_VERSION (bits 0 to 7)
> > > > register.
> > > >
> > > > Signed-off-by: Crt Mori <cmo@melexis.com>
> > > Hi.
> > >
> > > I'm going to take this via the slow path as you haven't called it out that
> > > you want it applied as a fix (so for stable kernels).  Let me know if these
> > > parts are in the wild and hence we should send it earlier.   We can do that
> > > after it is in mainline anyway as a specific request to the stable maintainers.
> > Hi,
> > Since this is our change I did not think it warrants a fix label, but
> > if we could, we would be very happy, if this could go in for all
> > stable kernels. It is designed to be backwards compatible, so it
> > should not cause new problems.
> Cool. I've moved it across to the fixes-togreg branch and should do a pull
> request for that in a week or two at which point it'll get picked up
> by the stable trees.
OK, thanks.

>
> >
> > > Applied to the togreg branch of iio.git and pushed out as testing for the
> > > autobuilders to play with it.
> > >
> > > Umm. This didn't actually apply to the current tree, so I did what I think
> > > was intended by hand. Please take a look at the testing branch of iio.git
> > > and check it is correct.
> > Sorry, but togreg branch does not even have temperature subdirectory
> > inside. Can you push testing to remote as well? Summary (git kernel
> > org) still has last update 8 days ago, or are you pushing somewhere
> > else?
> Umm. togreg should definitely have a temperature subdirectory.
> I suspect this is the odd nature of the web interface for kernel.org.
> For reasons I've never understood when you browse a 'tree' of a branch
> other than master, it doesn't show the current head of that branch.
> The easiest way to get there is to click on a particular commit and
> then switch to the tree view.

It did seem like a little fishy indeed. Was bugreport for this ever filed?

>
> For the other day it seems I hit an issue with the push and didn't notice.
>
> Should be there now.
OK, checked and it is as expected.

Crt
>
> Jonathan
>
> >
> > >
> > > Applied to the togreg branch of iio.git and pushed out as testing for the
> > > autobuilders to play with it.
> > >
> > > Thanks,
> > >
> > > Jonathan
> > >
> > > > ---
> > > >  drivers/iio/temperature/mlx90632.c | 9 +++++++--
> > > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/iio/temperature/mlx90632.c b/drivers/iio/temperature/mlx90632.c
> > > > index 2243e8057ffc..2d54d9cac61d 100644
> > > > --- a/drivers/iio/temperature/mlx90632.c
> > > > +++ b/drivers/iio/temperature/mlx90632.c
> > > > @@ -81,6 +81,8 @@
> > > >  /* Magic constants */
> > > >  #define MLX90632_ID_MEDICAL  0x0105 /* EEPROM DSPv5 Medical device id */
> > > >  #define MLX90632_ID_CONSUMER 0x0205 /* EEPROM DSPv5 Consumer device id */
> > > > +#define MLX90632_DSP_VERSION 5 /* DSP version */
> > > > +#define MLX90632_DSP_MASK    GENMASK(7, 0) /* DSP version in EE_VERSION */
> > > >  #define MLX90632_RESET_CMD   0x0006 /* Reset sensor (address or global) */
> > > >  #define MLX90632_REF_12              12LL /**< ResCtrlRef value of Ch 1 or Ch 2 */
> > > >  #define MLX90632_REF_3               12LL /**< ResCtrlRef value of Channel 3 */
> > > > @@ -666,10 +668,13 @@ static int mlx90632_probe(struct i2c_client *client,
> > > >       } else if (read == MLX90632_ID_CONSUMER) {
> > > >               dev_dbg(&client->dev,
> > > >                       "Detected Consumer EEPROM calibration %x\n", read);
> > > > +     } else if ((read & MLX90632_DSP_MASK) == MLX90632_DSP_VERSION) {
> > > > +             dev_dbg(&client->dev,
> > > > +                     "Detected Unknown EEPROM calibration %x\n", read);
> > > >       } else {
> > > >               dev_err(&client->dev,
> > > > -                     "Wrong DSP version %x (expected %x or %x)\n",
> > > > -                     read, MLX90632_DSPv5);
> > > > +                     "Wrong DSP version %x (expected %x)\n",
> > > > +                     read, MLX90632_DSP_VERSION);
> > > >               return -EPROTONOSUPPORT;
> > > >       }
> > > >
> > >
>