* [PATCH v2] iio:gyro:mpu3050 Treat otp value as a __le64 and use FIELD_GET() to break up
@ 2020-11-29 18:44 Jonathan Cameron
2020-11-29 20:37 ` Linus Walleij
2020-11-30 11:00 ` Andy Shevchenko
0 siblings, 2 replies; 3+ messages in thread
From: Jonathan Cameron @ 2020-11-29 18:44 UTC (permalink / raw)
To: linux-iio; +Cc: Jonathan Cameron, Andy Shevchenko, Linus Walleij
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Inspired by Andy Shevchenko's proposal to use get_unaligned_leXX().
The whole one time programable memory is treated as a single 64bit
little endian value. Thus we can avoid a lot of messy handling
of fields overlapping byte boundaries by just loading and manipulating
it as an __le64 converted to a u64. That lets us just use FIELD_GET()
and GENMASK() to extract the values desired.
Note only build tested. We need to use GENMASK_ULL and %llX formatters
to account for the larger types used in computing the various fields.
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20201128185156.428327-1-jic23@kernel.org
---
Changes since v1:
* Fix 32 bit builds by using GENMASK_ULL (thanks to Linus + Andy)
drivers/iio/gyro/mpu3050-core.c | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index 00e58060968c..dfa31a23500f 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -13,6 +13,7 @@
* TODO: add support for setting up the low pass 3dB frequency.
*/
+#include <linux/bitfield.h>
#include <linux/bitops.h>
#include <linux/delay.h>
#include <linux/err.h>
@@ -784,7 +785,8 @@ static int mpu3050_read_mem(struct mpu3050 *mpu3050,
static int mpu3050_hw_init(struct mpu3050 *mpu3050)
{
int ret;
- u8 otp[8];
+ __le64 otp_le;
+ u64 otp;
/* Reset */
ret = regmap_update_bits(mpu3050->map,
@@ -815,29 +817,31 @@ static int mpu3050_hw_init(struct mpu3050 *mpu3050)
MPU3050_MEM_USER_BANK |
MPU3050_MEM_OTP_BANK_0),
0,
- sizeof(otp),
- otp);
+ sizeof(otp_le),
+ (u8 *)&otp_le);
if (ret)
return ret;
/* This is device-unique data so it goes into the entropy pool */
- add_device_randomness(otp, sizeof(otp));
+ add_device_randomness(&otp_le, sizeof(otp_le));
+
+ otp = le64_to_cpu(otp_le);
dev_info(mpu3050->dev,
- "die ID: %04X, wafer ID: %02X, A lot ID: %04X, "
- "W lot ID: %03X, WP ID: %01X, rev ID: %02X\n",
+ "die ID: %04llX, wafer ID: %02llX, A lot ID: %04llX, "
+ "W lot ID: %03llX, WP ID: %01llX, rev ID: %02llX\n",
/* Die ID, bits 0-12 */
- (otp[1] << 8 | otp[0]) & 0x1fff,
+ FIELD_GET(GENMASK_ULL(12, 0), otp),
/* Wafer ID, bits 13-17 */
- ((otp[2] << 8 | otp[1]) & 0x03e0) >> 5,
+ FIELD_GET(GENMASK_ULL(17, 13), otp),
/* A lot ID, bits 18-33 */
- ((otp[4] << 16 | otp[3] << 8 | otp[2]) & 0x3fffc) >> 2,
+ FIELD_GET(GENMASK_ULL(33, 18), otp),
/* W lot ID, bits 34-45 */
- ((otp[5] << 8 | otp[4]) & 0x3ffc) >> 2,
+ FIELD_GET(GENMASK_ULL(45, 34), otp),
/* WP ID, bits 47-49 */
- ((otp[6] << 8 | otp[5]) & 0x0380) >> 7,
+ FIELD_GET(GENMASK_ULL(49, 47), otp),
/* rev ID, bits 50-55 */
- otp[6] >> 2);
+ FIELD_GET(GENMASK_ULL(55, 50), otp));
return 0;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio:gyro:mpu3050 Treat otp value as a __le64 and use FIELD_GET() to break up
2020-11-29 18:44 [PATCH v2] iio:gyro:mpu3050 Treat otp value as a __le64 and use FIELD_GET() to break up Jonathan Cameron
@ 2020-11-29 20:37 ` Linus Walleij
2020-11-30 11:00 ` Andy Shevchenko
1 sibling, 0 replies; 3+ messages in thread
From: Linus Walleij @ 2020-11-29 20:37 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Jonathan Cameron, Andy Shevchenko
On Sun, Nov 29, 2020 at 7:47 PM Jonathan Cameron <jic23@kernel.org> wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Inspired by Andy Shevchenko's proposal to use get_unaligned_leXX().
>
> The whole one time programable memory is treated as a single 64bit
> little endian value. Thus we can avoid a lot of messy handling
> of fields overlapping byte boundaries by just loading and manipulating
> it as an __le64 converted to a u64. That lets us just use FIELD_GET()
> and GENMASK() to extract the values desired.
>
> Note only build tested. We need to use GENMASK_ULL and %llX formatters
> to account for the larger types used in computing the various fields.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Link: https://lore.kernel.org/r/20201128185156.428327-1-jic23@kernel.org
> ---
>
> Changes since v1:
> * Fix 32 bit builds by using GENMASK_ULL (thanks to Linus + Andy)
Easy to test since I'm playing with one of these right now!
Before:
[ 2.677957] mpu3050-i2c 2-0068: found MPU-3050 part no: 6, version: 14
[ 2.695905] mpu3050-i2c 2-0068: die ID: 05A1, wafer ID: 12, A lot
ID: 0000, W lot ID: 662, WP ID: 3, rev ID: 1A
After:
[ 2.667944] mpu3050-i2c 2-0068: found MPU-3050 part no: 6, version: 14
[ 2.686969] mpu3050-i2c 2-0068: die ID: 05A1, wafer ID: 12, A lot
ID: 0000, W lot ID: 662, WP ID: 3, rev ID: 1A
And works too.
Tested-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
I used the same approach in the Yamaha driver and it looks good
there too.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio:gyro:mpu3050 Treat otp value as a __le64 and use FIELD_GET() to break up
2020-11-29 18:44 [PATCH v2] iio:gyro:mpu3050 Treat otp value as a __le64 and use FIELD_GET() to break up Jonathan Cameron
2020-11-29 20:37 ` Linus Walleij
@ 2020-11-30 11:00 ` Andy Shevchenko
1 sibling, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2020-11-30 11:00 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Jonathan Cameron, Linus Walleij
On Sun, Nov 29, 2020 at 06:44:59PM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Inspired by Andy Shevchenko's proposal to use get_unaligned_leXX().
>
> The whole one time programable memory is treated as a single 64bit
> little endian value. Thus we can avoid a lot of messy handling
> of fields overlapping byte boundaries by just loading and manipulating
> it as an __le64 converted to a u64. That lets us just use FIELD_GET()
> and GENMASK() to extract the values desired.
>
> Note only build tested. We need to use GENMASK_ULL and %llX formatters
> to account for the larger types used in computing the various fields.
LGTM now, thanks!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Link: https://lore.kernel.org/r/20201128185156.428327-1-jic23@kernel.org
> ---
>
> Changes since v1:
> * Fix 32 bit builds by using GENMASK_ULL (thanks to Linus + Andy)
>
> drivers/iio/gyro/mpu3050-core.c | 28 ++++++++++++++++------------
> 1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> index 00e58060968c..dfa31a23500f 100644
> --- a/drivers/iio/gyro/mpu3050-core.c
> +++ b/drivers/iio/gyro/mpu3050-core.c
> @@ -13,6 +13,7 @@
> * TODO: add support for setting up the low pass 3dB frequency.
> */
>
> +#include <linux/bitfield.h>
> #include <linux/bitops.h>
> #include <linux/delay.h>
> #include <linux/err.h>
> @@ -784,7 +785,8 @@ static int mpu3050_read_mem(struct mpu3050 *mpu3050,
> static int mpu3050_hw_init(struct mpu3050 *mpu3050)
> {
> int ret;
> - u8 otp[8];
> + __le64 otp_le;
> + u64 otp;
>
> /* Reset */
> ret = regmap_update_bits(mpu3050->map,
> @@ -815,29 +817,31 @@ static int mpu3050_hw_init(struct mpu3050 *mpu3050)
> MPU3050_MEM_USER_BANK |
> MPU3050_MEM_OTP_BANK_0),
> 0,
> - sizeof(otp),
> - otp);
> + sizeof(otp_le),
> + (u8 *)&otp_le);
> if (ret)
> return ret;
>
> /* This is device-unique data so it goes into the entropy pool */
> - add_device_randomness(otp, sizeof(otp));
> + add_device_randomness(&otp_le, sizeof(otp_le));
> +
> + otp = le64_to_cpu(otp_le);
>
> dev_info(mpu3050->dev,
> - "die ID: %04X, wafer ID: %02X, A lot ID: %04X, "
> - "W lot ID: %03X, WP ID: %01X, rev ID: %02X\n",
> + "die ID: %04llX, wafer ID: %02llX, A lot ID: %04llX, "
> + "W lot ID: %03llX, WP ID: %01llX, rev ID: %02llX\n",
> /* Die ID, bits 0-12 */
> - (otp[1] << 8 | otp[0]) & 0x1fff,
> + FIELD_GET(GENMASK_ULL(12, 0), otp),
> /* Wafer ID, bits 13-17 */
> - ((otp[2] << 8 | otp[1]) & 0x03e0) >> 5,
> + FIELD_GET(GENMASK_ULL(17, 13), otp),
> /* A lot ID, bits 18-33 */
> - ((otp[4] << 16 | otp[3] << 8 | otp[2]) & 0x3fffc) >> 2,
> + FIELD_GET(GENMASK_ULL(33, 18), otp),
> /* W lot ID, bits 34-45 */
> - ((otp[5] << 8 | otp[4]) & 0x3ffc) >> 2,
> + FIELD_GET(GENMASK_ULL(45, 34), otp),
> /* WP ID, bits 47-49 */
> - ((otp[6] << 8 | otp[5]) & 0x0380) >> 7,
> + FIELD_GET(GENMASK_ULL(49, 47), otp),
> /* rev ID, bits 50-55 */
> - otp[6] >> 2);
> + FIELD_GET(GENMASK_ULL(55, 50), otp));
>
> return 0;
> }
> --
> 2.29.2
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-30 11:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-29 18:44 [PATCH v2] iio:gyro:mpu3050 Treat otp value as a __le64 and use FIELD_GET() to break up Jonathan Cameron
2020-11-29 20:37 ` Linus Walleij
2020-11-30 11:00 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox