Re: [PATCH v1] iio: gyro: mpu3050: Use get_unaligned_leXX()

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 26 Oct 2020 19:53:40 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> This makes the driver code slightly easier to read.
> 
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

What you have is good, but could we do better?

Why not use an __le64 to grab the whole thing, convert it to a local
cpu endian u64 then use FIELD_GET or similar to pull out the right bits.
Would be fairly self documenting as well which is always nice.

Or am I missing something?

Jonathan


> ---
>  drivers/iio/gyro/mpu3050-core.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> index 00e58060968c..42b6ccb13568 100644
> --- a/drivers/iio/gyro/mpu3050-core.c
> +++ b/drivers/iio/gyro/mpu3050-core.c
> @@ -827,15 +827,15 @@ static int mpu3050_hw_init(struct mpu3050 *mpu3050)
>  		 "die ID: %04X, wafer ID: %02X, A lot ID: %04X, "
>  		 "W lot ID: %03X, WP ID: %01X, rev ID: %02X\n",
>  		 /* Die ID, bits 0-12 */
> -		 (otp[1] << 8 | otp[0]) & 0x1fff,
> +		 get_unaligned_le16(&otp[0]) & GENMASK(12, 0),
>  		 /* Wafer ID, bits 13-17 */
> -		 ((otp[2] << 8 | otp[1]) & 0x03e0) >> 5,
> +		 (get_unaligned_le16(&otp[1]) & GENMASK(9, 5)) >> 5,
>  		 /* A lot ID, bits 18-33 */
> -		 ((otp[4] << 16 | otp[3] << 8 | otp[2]) & 0x3fffc) >> 2,
> +		 (get_unaligned_le24(&otp[2]) & GENMASK(17, 2)) >> 2,
>  		 /* W lot ID, bits 34-45 */
> -		 ((otp[5] << 8 | otp[4]) & 0x3ffc) >> 2,
> +		 (get_unaligned_le16(&otp[4]) & GENMASK(13, 2)) >> 2,
>  		 /* WP ID, bits 47-49 */
> -		 ((otp[6] << 8 | otp[5]) & 0x0380) >> 7,
> +		 (get_unaligned_le16(&otp[5]) & GENMASK(9, 7)) >> 7,
>  		 /* rev ID, bits 50-55 */
>  		 otp[6] >> 2);
>