* [PATCH 1/4] iio: magnetometer: ak8974: support AMI306 variant
2017-08-17 13:56 [PATCH 0/4] iio: magnetometer: ak8974: support AMI306 Michał Mirosław
@ 2017-08-17 13:56 ` Michał Mirosław
2017-08-20 10:19 ` Jonathan Cameron
2017-08-17 13:56 ` [PATCH 3/4] iio: magnetometer: ak8974: mark INT_CLEAR as precious Michał Mirosław
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Michał Mirosław @ 2017-08-17 13:56 UTC (permalink / raw)
To: Linus Walleij, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler
Cc: linux-iio
Add support for AMI306 magnetometer - very similar to AMI305.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/iio/magnetometer/Kconfig | 4 +-
drivers/iio/magnetometer/ak8974.c | 90 ++++++++++++++++++++++++++++++---------
2 files changed, 72 insertions(+), 22 deletions(-)
diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
index 421ad90a5fbe..ed9d776d01af 100644
--- a/drivers/iio/magnetometer/Kconfig
+++ b/drivers/iio/magnetometer/Kconfig
@@ -13,8 +13,8 @@ config AK8974
select IIO_BUFFER
select IIO_TRIGGERED_BUFFER
help
- Say yes here to build support for Asahi Kasei AK8974 or
- AMI305 I2C-based 3-axis magnetometer chips.
+ Say yes here to build support for Asahi Kasei AK8974, AMI305 or
+ AMI306 I2C-based 3-axis magnetometer chips.
To compile this driver as a module, choose M here: the module
will be called ak8974.
diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
index e13370dc9b1c..76091da20a0c 100644
--- a/drivers/iio/magnetometer/ak8974.c
+++ b/drivers/iio/magnetometer/ak8974.c
@@ -36,7 +36,7 @@
* and MSB is at the next higher address.
*/
-/* These registers are common for AK8974 and AMI305 */
+/* These registers are common for AK8974 and AMI30x */
#define AK8974_SELFTEST 0x0C
#define AK8974_SELFTEST_IDLE 0x55
#define AK8974_SELFTEST_OK 0xAA
@@ -44,6 +44,7 @@
#define AK8974_INFO 0x0D
#define AK8974_WHOAMI 0x0F
+#define AK8974_WHOAMI_VALUE_AMI306 0x46
#define AK8974_WHOAMI_VALUE_AMI305 0x47
#define AK8974_WHOAMI_VALUE_AK8974 0x48
@@ -73,6 +74,35 @@
#define AK8974_TEMP 0x31
#define AMI305_TEMP 0x60
+/* AMI306-specific control register */
+#define AMI306_CTRL4 0x5C
+
+/* AMI306 factory calibration data */
+
+/* fine axis sensitivity */
+#define AMI306_FINEOUTPUT_X 0x90
+#define AMI306_FINEOUTPUT_Y 0x92
+#define AMI306_FINEOUTPUT_Z 0x94
+
+/* axis sensitivity */
+#define AMI306_SENS_X 0x96
+#define AMI306_SENS_Y 0x98
+#define AMI306_SENS_Z 0x9A
+
+/* axis cross-interference */
+#define AMI306_GAIN_PARA_XZ 0x9C
+#define AMI306_GAIN_PARA_XY 0x9D
+#define AMI306_GAIN_PARA_YZ 0x9E
+#define AMI306_GAIN_PARA_YX 0x9F
+#define AMI306_GAIN_PARA_ZY 0xA0
+#define AMI306_GAIN_PARA_ZX 0xA1
+
+/* offset at ZERO magnetic field */
+#define AMI306_OFFZERO_X 0xF8
+#define AMI306_OFFZERO_Y 0xFA
+#define AMI306_OFFZERO_Z 0xFC
+
+
#define AK8974_INT_X_HIGH BIT(7) /* Axis over +threshold */
#define AK8974_INT_Y_HIGH BIT(6)
#define AK8974_INT_Z_HIGH BIT(5)
@@ -158,6 +188,26 @@ struct ak8974 {
static const char ak8974_reg_avdd[] = "avdd";
static const char ak8974_reg_dvdd[] = "dvdd";
+static int ak8974_get_u16_val(struct ak8974 *ak8974, u8 reg, u16 *val)
+{
+ int ret;
+ __le16 bulk;
+
+ ret = regmap_bulk_read(ak8974->map, reg, &bulk, 2);
+ if (ret)
+ return ret;
+ *val = le16_to_cpu(bulk);
+
+ return 0;
+}
+
+static int ak8974_set_u16_val(struct ak8974 *ak8974, u8 reg, u16 val)
+{
+ __le16 bulk = cpu_to_le16(val);
+
+ return regmap_bulk_write(ak8974->map, reg, &bulk, 2);
+}
+
static int ak8974_set_power(struct ak8974 *ak8974, bool mode)
{
int ret;
@@ -209,6 +259,12 @@ static int ak8974_configure(struct ak8974 *ak8974)
ret = regmap_write(ak8974->map, AK8974_CTRL3, 0);
if (ret)
return ret;
+ if (ak8974->variant == AK8974_WHOAMI_VALUE_AMI306) {
+ /* magic from datasheet: set high-speed measurement mode */
+ ret = ak8974_set_u16_val(ak8974, AMI306_CTRL4, 0xA07E);
+ if (ret)
+ return ret;
+ }
ret = regmap_write(ak8974->map, AK8974_INT_CTRL, AK8974_INT_CTRL_POL);
if (ret)
return ret;
@@ -388,19 +444,6 @@ static int ak8974_selftest(struct ak8974 *ak8974)
return 0;
}
-static int ak8974_get_u16_val(struct ak8974 *ak8974, u8 reg, u16 *val)
-{
- int ret;
- __le16 bulk;
-
- ret = regmap_bulk_read(ak8974->map, reg, &bulk, 2);
- if (ret)
- return ret;
- *val = le16_to_cpu(bulk);
-
- return 0;
-}
-
static int ak8974_detect(struct ak8974 *ak8974)
{
unsigned int whoami;
@@ -413,9 +456,13 @@ static int ak8974_detect(struct ak8974 *ak8974)
if (ret)
return ret;
+ name = "ami305";
+
switch (whoami) {
+ case AK8974_WHOAMI_VALUE_AMI306:
+ name = "ami306";
+ /* fall-through */
case AK8974_WHOAMI_VALUE_AMI305:
- name = "ami305";
ret = regmap_read(ak8974->map, AMI305_VER, &fw);
if (ret)
return ret;
@@ -602,9 +649,11 @@ static bool ak8974_writeable_reg(struct device *dev, unsigned int reg)
case AMI305_OFFSET_Y + 1:
case AMI305_OFFSET_Z:
case AMI305_OFFSET_Z + 1:
- if (ak8974->variant == AK8974_WHOAMI_VALUE_AMI305)
- return true;
- return false;
+ return ak8974->variant == AK8974_WHOAMI_VALUE_AMI305 ||
+ ak8974->variant == AK8974_WHOAMI_VALUE_AMI306;
+ case AMI306_CTRL4:
+ case AMI306_CTRL4 + 1:
+ return ak8974->variant == AK8974_WHOAMI_VALUE_AMI306;
default:
return false;
}
@@ -678,7 +727,7 @@ static int ak8974_probe(struct i2c_client *i2c,
ret = ak8974_detect(ak8974);
if (ret) {
- dev_err(&i2c->dev, "neither AK8974 nor AMI305 found\n");
+ dev_err(&i2c->dev, "neither AK8974 nor AMI30x found\n");
goto power_off;
}
@@ -827,6 +876,7 @@ static const struct dev_pm_ops ak8974_dev_pm_ops = {
static const struct i2c_device_id ak8974_id[] = {
{"ami305", 0 },
+ {"ami306", 0 },
{"ak8974", 0 },
{}
};
@@ -850,7 +900,7 @@ static struct i2c_driver ak8974_driver = {
};
module_i2c_driver(ak8974_driver);
-MODULE_DESCRIPTION("AK8974 and AMI305 3-axis magnetometer driver");
+MODULE_DESCRIPTION("AK8974 and AMI30x 3-axis magnetometer driver");
MODULE_AUTHOR("Samu Onkalo");
MODULE_AUTHOR("Linus Walleij");
MODULE_LICENSE("GPL v2");
--
2.11.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] iio: magnetometer: ak8974: support AMI306 variant
2017-08-17 13:56 ` [PATCH 1/4] iio: magnetometer: ak8974: support AMI306 variant Michał Mirosław
@ 2017-08-20 10:19 ` Jonathan Cameron
0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2017-08-20 10:19 UTC (permalink / raw)
To: Michał Mirosław
Cc: Linus Walleij, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, linux-iio
On Thu, 17 Aug 2017 15:56:10 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> Add support for AMI306 magnetometer - very similar to AMI305.
>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/iio/magnetometer/Kconfig | 4 +-
> drivers/iio/magnetometer/ak8974.c | 90 ++++++++++++++++++++++++++++++---------
> 2 files changed, 72 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
> index 421ad90a5fbe..ed9d776d01af 100644
> --- a/drivers/iio/magnetometer/Kconfig
> +++ b/drivers/iio/magnetometer/Kconfig
> @@ -13,8 +13,8 @@ config AK8974
> select IIO_BUFFER
> select IIO_TRIGGERED_BUFFER
> help
> - Say yes here to build support for Asahi Kasei AK8974 or
> - AMI305 I2C-based 3-axis magnetometer chips.
> + Say yes here to build support for Asahi Kasei AK8974, AMI305 or
> + AMI306 I2C-based 3-axis magnetometer chips.
>
> To compile this driver as a module, choose M here: the module
> will be called ak8974.
> diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
> index e13370dc9b1c..76091da20a0c 100644
> --- a/drivers/iio/magnetometer/ak8974.c
> +++ b/drivers/iio/magnetometer/ak8974.c
> @@ -36,7 +36,7 @@
> * and MSB is at the next higher address.
> */
>
> -/* These registers are common for AK8974 and AMI305 */
> +/* These registers are common for AK8974 and AMI30x */
> #define AK8974_SELFTEST 0x0C
> #define AK8974_SELFTEST_IDLE 0x55
> #define AK8974_SELFTEST_OK 0xAA
> @@ -44,6 +44,7 @@
> #define AK8974_INFO 0x0D
>
> #define AK8974_WHOAMI 0x0F
> +#define AK8974_WHOAMI_VALUE_AMI306 0x46
> #define AK8974_WHOAMI_VALUE_AMI305 0x47
> #define AK8974_WHOAMI_VALUE_AK8974 0x48
>
> @@ -73,6 +74,35 @@
> #define AK8974_TEMP 0x31
> #define AMI305_TEMP 0x60
>
> +/* AMI306-specific control register */
> +#define AMI306_CTRL4 0x5C
> +
> +/* AMI306 factory calibration data */
> +
> +/* fine axis sensitivity */
> +#define AMI306_FINEOUTPUT_X 0x90
> +#define AMI306_FINEOUTPUT_Y 0x92
> +#define AMI306_FINEOUTPUT_Z 0x94
> +
> +/* axis sensitivity */
> +#define AMI306_SENS_X 0x96
> +#define AMI306_SENS_Y 0x98
> +#define AMI306_SENS_Z 0x9A
> +
> +/* axis cross-interference */
> +#define AMI306_GAIN_PARA_XZ 0x9C
> +#define AMI306_GAIN_PARA_XY 0x9D
> +#define AMI306_GAIN_PARA_YZ 0x9E
> +#define AMI306_GAIN_PARA_YX 0x9F
> +#define AMI306_GAIN_PARA_ZY 0xA0
> +#define AMI306_GAIN_PARA_ZX 0xA1
> +
> +/* offset at ZERO magnetic field */
> +#define AMI306_OFFZERO_X 0xF8
> +#define AMI306_OFFZERO_Y 0xFA
> +#define AMI306_OFFZERO_Z 0xFC
> +
> +
> #define AK8974_INT_X_HIGH BIT(7) /* Axis over +threshold */
> #define AK8974_INT_Y_HIGH BIT(6)
> #define AK8974_INT_Z_HIGH BIT(5)
> @@ -158,6 +188,26 @@ struct ak8974 {
> static const char ak8974_reg_avdd[] = "avdd";
> static const char ak8974_reg_dvdd[] = "dvdd";
>
> +static int ak8974_get_u16_val(struct ak8974 *ak8974, u8 reg, u16 *val)
> +{
> + int ret;
> + __le16 bulk;
> +
> + ret = regmap_bulk_read(ak8974->map, reg, &bulk, 2);
> + if (ret)
> + return ret;
> + *val = le16_to_cpu(bulk);
> +
> + return 0;
> +}
> +
> +static int ak8974_set_u16_val(struct ak8974 *ak8974, u8 reg, u16 val)
> +{
> + __le16 bulk = cpu_to_le16(val);
> +
> + return regmap_bulk_write(ak8974->map, reg, &bulk, 2);
> +}
> +
> static int ak8974_set_power(struct ak8974 *ak8974, bool mode)
> {
> int ret;
> @@ -209,6 +259,12 @@ static int ak8974_configure(struct ak8974 *ak8974)
> ret = regmap_write(ak8974->map, AK8974_CTRL3, 0);
> if (ret)
> return ret;
> + if (ak8974->variant == AK8974_WHOAMI_VALUE_AMI306) {
> + /* magic from datasheet: set high-speed measurement mode */
> + ret = ak8974_set_u16_val(ak8974, AMI306_CTRL4, 0xA07E);
> + if (ret)
> + return ret;
> + }
> ret = regmap_write(ak8974->map, AK8974_INT_CTRL, AK8974_INT_CTRL_POL);
> if (ret)
> return ret;
> @@ -388,19 +444,6 @@ static int ak8974_selftest(struct ak8974 *ak8974)
> return 0;
> }
>
> -static int ak8974_get_u16_val(struct ak8974 *ak8974, u8 reg, u16 *val)
> -{
> - int ret;
> - __le16 bulk;
> -
> - ret = regmap_bulk_read(ak8974->map, reg, &bulk, 2);
> - if (ret)
> - return ret;
> - *val = le16_to_cpu(bulk);
> -
> - return 0;
> -}
> -
> static int ak8974_detect(struct ak8974 *ak8974)
> {
> unsigned int whoami;
> @@ -413,9 +456,13 @@ static int ak8974_detect(struct ak8974 *ak8974)
> if (ret)
> return ret;
>
> + name = "ami305";
> +
> switch (whoami) {
> + case AK8974_WHOAMI_VALUE_AMI306:
> + name = "ami306";
> + /* fall-through */
> case AK8974_WHOAMI_VALUE_AMI305:
> - name = "ami305";
> ret = regmap_read(ak8974->map, AMI305_VER, &fw);
> if (ret)
> return ret;
> @@ -602,9 +649,11 @@ static bool ak8974_writeable_reg(struct device *dev, unsigned int reg)
> case AMI305_OFFSET_Y + 1:
> case AMI305_OFFSET_Z:
> case AMI305_OFFSET_Z + 1:
> - if (ak8974->variant == AK8974_WHOAMI_VALUE_AMI305)
> - return true;
> - return false;
> + return ak8974->variant == AK8974_WHOAMI_VALUE_AMI305 ||
> + ak8974->variant == AK8974_WHOAMI_VALUE_AMI306;
> + case AMI306_CTRL4:
> + case AMI306_CTRL4 + 1:
> + return ak8974->variant == AK8974_WHOAMI_VALUE_AMI306;
> default:
> return false;
> }
> @@ -678,7 +727,7 @@ static int ak8974_probe(struct i2c_client *i2c,
>
> ret = ak8974_detect(ak8974);
> if (ret) {
> - dev_err(&i2c->dev, "neither AK8974 nor AMI305 found\n");
> + dev_err(&i2c->dev, "neither AK8974 nor AMI30x found\n");
> goto power_off;
> }
>
> @@ -827,6 +876,7 @@ static const struct dev_pm_ops ak8974_dev_pm_ops = {
>
> static const struct i2c_device_id ak8974_id[] = {
> {"ami305", 0 },
> + {"ami306", 0 },
> {"ak8974", 0 },
> {}
> };
> @@ -850,7 +900,7 @@ static struct i2c_driver ak8974_driver = {
> };
> module_i2c_driver(ak8974_driver);
>
> -MODULE_DESCRIPTION("AK8974 and AMI305 3-axis magnetometer driver");
> +MODULE_DESCRIPTION("AK8974 and AMI30x 3-axis magnetometer driver");
> MODULE_AUTHOR("Samu Onkalo");
> MODULE_AUTHOR("Linus Walleij");
> MODULE_LICENSE("GPL v2");
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] iio: magnetometer: ak8974: mark INT_CLEAR as precious
2017-08-17 13:56 [PATCH 0/4] iio: magnetometer: ak8974: support AMI306 Michał Mirosław
2017-08-17 13:56 ` [PATCH 1/4] iio: magnetometer: ak8974: support AMI306 variant Michał Mirosław
@ 2017-08-17 13:56 ` Michał Mirosław
2017-08-17 13:56 ` [PATCH 2/4] iio: magnetometer: ak8974: add_device_randomness(fw + serial) Michał Mirosław
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Michał Mirosław @ 2017-08-17 13:56 UTC (permalink / raw)
To: Linus Walleij, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler
Cc: linux-iio
Reading INT_CLEAR has side effects - disallow reading it via debugfs.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/iio/magnetometer/ak8974.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
index ab204f8560e3..d8c45206ba00 100644
--- a/drivers/iio/magnetometer/ak8974.c
+++ b/drivers/iio/magnetometer/ak8974.c
@@ -662,11 +662,17 @@ static bool ak8974_writeable_reg(struct device *dev, unsigned int reg)
}
}
+static bool ak8974_precious_reg(struct device *dev, unsigned int reg)
+{
+ return reg == AK8974_INT_CLEAR;
+}
+
static const struct regmap_config ak8974_regmap_config = {
.reg_bits = 8,
.val_bits = 8,
.max_register = 0xff,
.writeable_reg = ak8974_writeable_reg,
+ .precious_reg = ak8974_precious_reg,
};
static int ak8974_probe(struct i2c_client *i2c,
--
2.11.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/4] iio: magnetometer: ak8974: add_device_randomness(fw + serial)
2017-08-17 13:56 [PATCH 0/4] iio: magnetometer: ak8974: support AMI306 Michał Mirosław
2017-08-17 13:56 ` [PATCH 1/4] iio: magnetometer: ak8974: support AMI306 variant Michał Mirosław
2017-08-17 13:56 ` [PATCH 3/4] iio: magnetometer: ak8974: mark INT_CLEAR as precious Michał Mirosław
@ 2017-08-17 13:56 ` Michał Mirosław
2017-08-20 10:22 ` Jonathan Cameron
2017-08-17 13:56 ` [PATCH 4/4] iio: magnetometer: ak8974: debug AMI306 calibration data Michał Mirosław
2017-08-18 21:42 ` [PATCH 0/4] iio: magnetometer: ak8974: support AMI306 Linus Walleij
4 siblings, 1 reply; 13+ messages in thread
From: Michał Mirosław @ 2017-08-17 13:56 UTC (permalink / raw)
To: Linus Walleij, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler
Cc: linux-iio
Mix device-specific data into randomness pool.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/iio/magnetometer/ak8974.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
index 76091da20a0c..ab204f8560e3 100644
--- a/drivers/iio/magnetometer/ak8974.c
+++ b/drivers/iio/magnetometer/ak8974.c
@@ -20,6 +20,7 @@
#include <linux/mutex.h>
#include <linux/delay.h>
#include <linux/bitops.h>
+#include <linux/random.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <linux/pm_runtime.h>
@@ -466,10 +467,12 @@ static int ak8974_detect(struct ak8974 *ak8974)
ret = regmap_read(ak8974->map, AMI305_VER, &fw);
if (ret)
return ret;
+ add_device_randomness(&fw, sizeof(fw));
fw &= 0x7f; /* only bits 0 thru 6 valid */
ret = ak8974_get_u16_val(ak8974, AMI305_SN, &sn);
if (ret)
return ret;
+ add_device_randomness(&sn, sizeof(sn));
dev_info(&ak8974->i2c->dev,
"detected %s, FW ver %02x, S/N: %04x\n",
name, fw, sn);
--
2.11.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] iio: magnetometer: ak8974: add_device_randomness(fw + serial)
2017-08-17 13:56 ` [PATCH 2/4] iio: magnetometer: ak8974: add_device_randomness(fw + serial) Michał Mirosław
@ 2017-08-20 10:22 ` Jonathan Cameron
2017-08-20 11:10 ` Linus Walleij
0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2017-08-20 10:22 UTC (permalink / raw)
To: Michał Mirosław
Cc: Linus Walleij, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, linux-iio
On Thu, 17 Aug 2017 15:56:11 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> Mix device-specific data into randomness pool.
>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Hmm. I wondered about the firmware version as a source of randomness, but
will leave it in here.
Applied to the togreg branch of iio.git and pushed out as testing.
Thanks,
Jonathan
> ---
> drivers/iio/magnetometer/ak8974.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
> index 76091da20a0c..ab204f8560e3 100644
> --- a/drivers/iio/magnetometer/ak8974.c
> +++ b/drivers/iio/magnetometer/ak8974.c
> @@ -20,6 +20,7 @@
> #include <linux/mutex.h>
> #include <linux/delay.h>
> #include <linux/bitops.h>
> +#include <linux/random.h>
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
> #include <linux/pm_runtime.h>
> @@ -466,10 +467,12 @@ static int ak8974_detect(struct ak8974 *ak8974)
> ret = regmap_read(ak8974->map, AMI305_VER, &fw);
> if (ret)
> return ret;
> + add_device_randomness(&fw, sizeof(fw));
The firmware version seems rather to guessable to really count as randomness.
presumably it would constant for a particular batch of chips.
> fw &= 0x7f; /* only bits 0 thru 6 valid */
> ret = ak8974_get_u16_val(ak8974, AMI305_SN, &sn);
> if (ret)
> return ret;
> + add_device_randomness(&sn, sizeof(sn));
This is better. The serial number should be hard to guess on a particular
part.
Jonathan
> dev_info(&ak8974->i2c->dev,
> "detected %s, FW ver %02x, S/N: %04x\n",
> name, fw, sn);
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] iio: magnetometer: ak8974: add_device_randomness(fw + serial)
2017-08-20 10:22 ` Jonathan Cameron
@ 2017-08-20 11:10 ` Linus Walleij
2017-08-20 14:18 ` Jonathan Cameron
0 siblings, 1 reply; 13+ messages in thread
From: Linus Walleij @ 2017-08-20 11:10 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Michał Mirosław, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, linux-iio
On Sun, Aug 20, 2017 at 12:22 PM, Jonathan Cameron <jic23@kernel.org> wrote=
:
> On Thu, 17 Aug 2017 15:56:11 +0200
> Micha=C5=82 Miros=C5=82aw <mirq-linux@rere.qmqm.pl> wrote:
>> Signed-off-by: Micha=C5=82 Miros=C5=82aw <mirq-linux@rere.qmqm.pl>
>
> Hmm. I wondered about the firmware version as a source of randomness, but
> will leave it in here.
(...)
>> + add_device_randomness(&fw, sizeof(fw));
>
> The firmware version seems rather to guessable to really count as randomn=
ess.
> presumably it would constant for a particular batch of chips.
That is true, this should go, not for security reasons but because it doesn=
't
add much device-unique randomness.
Nothing we add with add_device_randomness() actually affects the entropy
pool trust, it is just icing making it - maybe - even more random, so addin=
g
256 zeroes is fine for the trust, just pointless and taking time for no goo=
d.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] iio: magnetometer: ak8974: add_device_randomness(fw + serial)
2017-08-20 11:10 ` Linus Walleij
@ 2017-08-20 14:18 ` Jonathan Cameron
0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2017-08-20 14:18 UTC (permalink / raw)
To: Linus Walleij
Cc: Michał Mirosław, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, linux-iio
On Sun, 20 Aug 2017 13:10:58 +0200
Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sun, Aug 20, 2017 at 12:22 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> > On Thu, 17 Aug 2017 15:56:11 +0200
> > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> >> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> >
> > Hmm. I wondered about the firmware version as a source of randomness, but
> > will leave it in here.
> (...)
> >> + add_device_randomness(&fw, sizeof(fw));
> >
> > The firmware version seems rather to guessable to really count as randomness.
> > presumably it would constant for a particular batch of chips.
>
> That is true, this should go, not for security reasons but because it doesn't
> add much device-unique randomness.
Dropped the fw one...
J
>
> Nothing we add with add_device_randomness() actually affects the entropy
> pool trust, it is just icing making it - maybe - even more random, so adding
> 256 zeroes is fine for the trust, just pointless and taking time for no good.
>
> Yours,
> Linus Walleij
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] iio: magnetometer: ak8974: debug AMI306 calibration data
2017-08-17 13:56 [PATCH 0/4] iio: magnetometer: ak8974: support AMI306 Michał Mirosław
` (2 preceding siblings ...)
2017-08-17 13:56 ` [PATCH 2/4] iio: magnetometer: ak8974: add_device_randomness(fw + serial) Michał Mirosław
@ 2017-08-17 13:56 ` Michał Mirosław
2017-08-20 10:25 ` Jonathan Cameron
2017-08-18 21:42 ` [PATCH 0/4] iio: magnetometer: ak8974: support AMI306 Linus Walleij
4 siblings, 1 reply; 13+ messages in thread
From: Michał Mirosław @ 2017-08-17 13:56 UTC (permalink / raw)
To: Linus Walleij, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler
Cc: linux-iio
Use AMI306 calibration data for randomness and debugging.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/iio/magnetometer/ak8974.c | 41 +++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
index d8c45206ba00..b3221b34606b 100644
--- a/drivers/iio/magnetometer/ak8974.c
+++ b/drivers/iio/magnetometer/ak8974.c
@@ -445,6 +445,20 @@ static int ak8974_selftest(struct ak8974 *ak8974)
return 0;
}
+static void ak8974_read_calib_data(struct ak8974 *ak8974, unsigned int reg,
+ __le16 *tab, size_t tab_size)
+{
+ int ret = regmap_bulk_read(ak8974->map, reg, tab, tab_size);
+ if (ret) {
+ memset(tab, 0xFF, tab_size);
+ dev_warn(&ak8974->i2c->dev,
+ "can't read calibration data (regs %u..%u): %d\n",
+ reg, reg + tab_size - 1, ret);
+ } else {
+ add_device_randomness(tab, tab_size);
+ }
+}
+
static int ak8974_detect(struct ak8974 *ak8974)
{
unsigned int whoami;
@@ -490,6 +504,33 @@ static int ak8974_detect(struct ak8974 *ak8974)
ak8974->name = name;
ak8974->variant = whoami;
+ if (whoami == AK8974_WHOAMI_VALUE_AMI306) {
+ __le16 fab_data1[9], fab_data2[3];
+ int i;
+
+ ak8974_read_calib_data(ak8974, AMI306_FINEOUTPUT_X,
+ fab_data1, sizeof(fab_data1));
+ ak8974_read_calib_data(ak8974, AMI306_OFFZERO_X,
+ fab_data2, sizeof(fab_data2));
+
+ for (i = 0; i < 3; ++i) {
+ static const char axis[3] = "XYZ";
+ static const char pgaxis[6] = "ZYZXYX";
+ unsigned offz = le16_to_cpu(fab_data2[i]) & 0x7F;
+ unsigned fine = le16_to_cpu(fab_data1[i]);
+ unsigned sens = le16_to_cpu(fab_data1[i + 3]);
+ unsigned pgain1 = le16_to_cpu(fab_data1[i + 6]);
+ unsigned pgain2 = pgain1 >> 8;
+
+ pgain1 &= 0xFF;
+
+ dev_info(&ak8974->i2c->dev,
+ "factory calibration for axis %c: offz=%u sens=%u fine=%u pga%c=%u pga%c=%u\n",
+ axis[i], offz, sens, fine, pgaxis[i * 2],
+ pgain1, pgaxis[i * 2 + 1], pgain2);
+ }
+ }
+
return 0;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] iio: magnetometer: ak8974: debug AMI306 calibration data
2017-08-17 13:56 ` [PATCH 4/4] iio: magnetometer: ak8974: debug AMI306 calibration data Michał Mirosław
@ 2017-08-20 10:25 ` Jonathan Cameron
2017-08-20 10:28 ` Jonathan Cameron
0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2017-08-20 10:25 UTC (permalink / raw)
To: Michał Mirosław
Cc: Linus Walleij, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, linux-iio
On Thu, 17 Aug 2017 15:56:12 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> Use AMI306 calibration data for randomness and debugging.
>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/iio/magnetometer/ak8974.c | 41 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
> index d8c45206ba00..b3221b34606b 100644
> --- a/drivers/iio/magnetometer/ak8974.c
> +++ b/drivers/iio/magnetometer/ak8974.c
> @@ -445,6 +445,20 @@ static int ak8974_selftest(struct ak8974 *ak8974)
> return 0;
> }
>
> +static void ak8974_read_calib_data(struct ak8974 *ak8974, unsigned int reg,
> + __le16 *tab, size_t tab_size)
> +{
> + int ret = regmap_bulk_read(ak8974->map, reg, tab, tab_size);
> + if (ret) {
> + memset(tab, 0xFF, tab_size);
> + dev_warn(&ak8974->i2c->dev,
> + "can't read calibration data (regs %u..%u): %d\n",
> + reg, reg + tab_size - 1, ret);
> + } else {
> + add_device_randomness(tab, tab_size);
> + }
> +}
> +
> static int ak8974_detect(struct ak8974 *ak8974)
> {
> unsigned int whoami;
> @@ -490,6 +504,33 @@ static int ak8974_detect(struct ak8974 *ak8974)
> ak8974->name = name;
> ak8974->variant = whoami;
>
> + if (whoami == AK8974_WHOAMI_VALUE_AMI306) {
> + __le16 fab_data1[9], fab_data2[3];
> + int i;
> +
> + ak8974_read_calib_data(ak8974, AMI306_FINEOUTPUT_X,
> + fab_data1, sizeof(fab_data1));
> + ak8974_read_calib_data(ak8974, AMI306_OFFZERO_X,
> + fab_data2, sizeof(fab_data2));
> +
> + for (i = 0; i < 3; ++i) {
> + static const char axis[3] = "XYZ";
> + static const char pgaxis[6] = "ZYZXYX";
> + unsigned offz = le16_to_cpu(fab_data2[i]) & 0x7F;
> + unsigned fine = le16_to_cpu(fab_data1[i]);
> + unsigned sens = le16_to_cpu(fab_data1[i + 3]);
> + unsigned pgain1 = le16_to_cpu(fab_data1[i + 6]);
> + unsigned pgain2 = pgain1 >> 8;
> +
> + pgain1 &= 0xFF;
> +
> + dev_info(&ak8974->i2c->dev,
> + "factory calibration for axis %c: offz=%u sens=%u fine=%u pga%c=%u pga%c=%u\n",
> + axis[i], offz, sens, fine, pgaxis[i * 2],
> + pgain1, pgaxis[i * 2 + 1], pgain2);
> + }
> + }
> +
> return 0;
> }
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] iio: magnetometer: ak8974: debug AMI306 calibration data
2017-08-20 10:25 ` Jonathan Cameron
@ 2017-08-20 10:28 ` Jonathan Cameron
2017-08-20 14:40 ` Jonathan Cameron
0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2017-08-20 10:28 UTC (permalink / raw)
To: Michał Mirosław
Cc: Linus Walleij, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, linux-iio
On Sun, 20 Aug 2017 11:25:52 +0100
Jonathan Cameron <jic23@kernel.org> wrote:
> On Thu, 17 Aug 2017 15:56:12 +0200
> Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
>
> > Use AMI306 calibration data for randomness and debugging.
> >
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Applied to the togreg branch of iio.git and pushed out as testing for the
> autobuilders to play with it.
>
> Thanks,
>
> Jonathan
> > ---
> > drivers/iio/magnetometer/ak8974.c | 41 +++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 41 insertions(+)
> >
> > diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
> > index d8c45206ba00..b3221b34606b 100644
> > --- a/drivers/iio/magnetometer/ak8974.c
> > +++ b/drivers/iio/magnetometer/ak8974.c
> > @@ -445,6 +445,20 @@ static int ak8974_selftest(struct ak8974 *ak8974)
> > return 0;
> > }
> >
> > +static void ak8974_read_calib_data(struct ak8974 *ak8974, unsigned int reg,
> > + __le16 *tab, size_t tab_size)
> > +{
> > + int ret = regmap_bulk_read(ak8974->map, reg, tab, tab_size);
> > + if (ret) {
> > + memset(tab, 0xFF, tab_size);
> > + dev_warn(&ak8974->i2c->dev,
> > + "can't read calibration data (regs %u..%u): %d\n",
> > + reg, reg + tab_size - 1, ret);
Slight tweak made to avoid the warning:
drivers/iio/magnetometer/ak8974.c: In function ‘ak8974_read_calib_data’:
drivers/iio/magnetometer/ak8974.c:455:45: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 4 has type ‘size_t {aka long unsigned int}’ [-Wformat=]
"can't read calibration data (regs %u..%u): %d\n",
~^
%lu
reg, reg + tab_size - 1, ret);
~~~~~~~~~~~~~~~~~~
Did the obvious ;)
Jonathan
> > + } else {
> > + add_device_randomness(tab, tab_size);
> > + }
> > +}
> > +
> > static int ak8974_detect(struct ak8974 *ak8974)
> > {
> > unsigned int whoami;
> > @@ -490,6 +504,33 @@ static int ak8974_detect(struct ak8974 *ak8974)
> > ak8974->name = name;
> > ak8974->variant = whoami;
> >
> > + if (whoami == AK8974_WHOAMI_VALUE_AMI306) {
> > + __le16 fab_data1[9], fab_data2[3];
> > + int i;
> > +
> > + ak8974_read_calib_data(ak8974, AMI306_FINEOUTPUT_X,
> > + fab_data1, sizeof(fab_data1));
> > + ak8974_read_calib_data(ak8974, AMI306_OFFZERO_X,
> > + fab_data2, sizeof(fab_data2));
> > +
> > + for (i = 0; i < 3; ++i) {
> > + static const char axis[3] = "XYZ";
> > + static const char pgaxis[6] = "ZYZXYX";
> > + unsigned offz = le16_to_cpu(fab_data2[i]) & 0x7F;
> > + unsigned fine = le16_to_cpu(fab_data1[i]);
> > + unsigned sens = le16_to_cpu(fab_data1[i + 3]);
> > + unsigned pgain1 = le16_to_cpu(fab_data1[i + 6]);
> > + unsigned pgain2 = pgain1 >> 8;
> > +
> > + pgain1 &= 0xFF;
> > +
> > + dev_info(&ak8974->i2c->dev,
> > + "factory calibration for axis %c: offz=%u sens=%u fine=%u pga%c=%u pga%c=%u\n",
> > + axis[i], offz, sens, fine, pgaxis[i * 2],
> > + pgain1, pgaxis[i * 2 + 1], pgain2);
> > + }
> > + }
> > +
> > return 0;
> > }
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] iio: magnetometer: ak8974: debug AMI306 calibration data
2017-08-20 10:28 ` Jonathan Cameron
@ 2017-08-20 14:40 ` Jonathan Cameron
0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2017-08-20 14:40 UTC (permalink / raw)
To: Michał Mirosław
Cc: Linus Walleij, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, linux-iio
On Sun, 20 Aug 2017 11:28:29 +0100
Jonathan Cameron <jic23@jic23.retrosnub.co.uk> wrote:
> On Sun, 20 Aug 2017 11:25:52 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > On Thu, 17 Aug 2017 15:56:12 +0200
> > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> >
> > > Use AMI306 calibration data for randomness and debugging.
> > >
> > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > Applied to the togreg branch of iio.git and pushed out as testing for the
> > autobuilders to play with it.
> >
> > Thanks,
> >
> > Jonathan
> > > ---
> > > drivers/iio/magnetometer/ak8974.c | 41 +++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 41 insertions(+)
> > >
> > > diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
> > > index d8c45206ba00..b3221b34606b 100644
> > > --- a/drivers/iio/magnetometer/ak8974.c
> > > +++ b/drivers/iio/magnetometer/ak8974.c
> > > @@ -445,6 +445,20 @@ static int ak8974_selftest(struct ak8974 *ak8974)
> > > return 0;
> > > }
> > >
> > > +static void ak8974_read_calib_data(struct ak8974 *ak8974, unsigned int reg,
> > > + __le16 *tab, size_t tab_size)
> > > +{
> > > + int ret = regmap_bulk_read(ak8974->map, reg, tab, tab_size);
> > > + if (ret) {
> > > + memset(tab, 0xFF, tab_size);
> > > + dev_warn(&ak8974->i2c->dev,
> > > + "can't read calibration data (regs %u..%u): %d\n",
> > > + reg, reg + tab_size - 1, ret);
> Slight tweak made to avoid the warning:
>
> drivers/iio/magnetometer/ak8974.c: In function ‘ak8974_read_calib_data’:
> drivers/iio/magnetometer/ak8974.c:455:45: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 4 has type ‘size_t {aka long unsigned int}’ [-Wformat=]
> "can't read calibration data (regs %u..%u): %d\n",
> ~^
> %lu
> reg, reg + tab_size - 1, ret);
> ~~~~~~~~~~~~~~~~~~
>
> Did the obvious ;)
Lets try that again. %zu is the right option for size_t to
deal with how it changes type across architectures.
Anyhow, updated (again).
Jonathan
>
> Jonathan
>
> > > + } else {
> > > + add_device_randomness(tab, tab_size);
> > > + }
> > > +}
> > > +
> > > static int ak8974_detect(struct ak8974 *ak8974)
> > > {
> > > unsigned int whoami;
> > > @@ -490,6 +504,33 @@ static int ak8974_detect(struct ak8974 *ak8974)
> > > ak8974->name = name;
> > > ak8974->variant = whoami;
> > >
> > > + if (whoami == AK8974_WHOAMI_VALUE_AMI306) {
> > > + __le16 fab_data1[9], fab_data2[3];
> > > + int i;
> > > +
> > > + ak8974_read_calib_data(ak8974, AMI306_FINEOUTPUT_X,
> > > + fab_data1, sizeof(fab_data1));
> > > + ak8974_read_calib_data(ak8974, AMI306_OFFZERO_X,
> > > + fab_data2, sizeof(fab_data2));
> > > +
> > > + for (i = 0; i < 3; ++i) {
> > > + static const char axis[3] = "XYZ";
> > > + static const char pgaxis[6] = "ZYZXYX";
> > > + unsigned offz = le16_to_cpu(fab_data2[i]) & 0x7F;
> > > + unsigned fine = le16_to_cpu(fab_data1[i]);
> > > + unsigned sens = le16_to_cpu(fab_data1[i + 3]);
> > > + unsigned pgain1 = le16_to_cpu(fab_data1[i + 6]);
> > > + unsigned pgain2 = pgain1 >> 8;
> > > +
> > > + pgain1 &= 0xFF;
> > > +
> > > + dev_info(&ak8974->i2c->dev,
> > > + "factory calibration for axis %c: offz=%u sens=%u fine=%u pga%c=%u pga%c=%u\n",
> > > + axis[i], offz, sens, fine, pgaxis[i * 2],
> > > + pgain1, pgaxis[i * 2 + 1], pgain2);
> > > + }
> > > + }
> > > +
> > > return 0;
> > > }
> > >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] iio: magnetometer: ak8974: support AMI306
2017-08-17 13:56 [PATCH 0/4] iio: magnetometer: ak8974: support AMI306 Michał Mirosław
` (3 preceding siblings ...)
2017-08-17 13:56 ` [PATCH 4/4] iio: magnetometer: ak8974: debug AMI306 calibration data Michał Mirosław
@ 2017-08-18 21:42 ` Linus Walleij
4 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2017-08-18 21:42 UTC (permalink / raw)
To: Michał Mirosław
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, linux-iio
On Thu, Aug 17, 2017 at 3:56 PM, Micha=C5=82 Miros=C5=82aw
<mirq-linux@rere.qmqm.pl> wrote:
> This series adds support for AMI306 magnetometer. The device is very
> similar to already supported AMI305.
All four patches:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Very clear and to the point. Nice to see some wider use of this :)
> Calibration data is not used
> yet as I still need to figure out what it means.
Does your datasheet tell you how to convert to ISO units? Right now
the sensor only suppport raw data and it would be nice to support
IIO_CHAN_INFO_SCALE like e.g. ak8975.c does.
I bet the same scaling applies as to the other variants.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 13+ messages in thread