* [PATCH v1] iio: pressure: dps310: Use get_unaligned_beXX()
@ 2020-10-26 18:05 Andy Shevchenko
2020-10-29 15:25 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2020-10-26 18:05 UTC (permalink / raw)
To: Jonathan Cameron, linux-iio, Lars-Peter Clausen, Peter Meerwald-Stadler
Cc: Andy Shevchenko, Eddie James
This makes the driver code slightly easier to read.
Cc: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/iio/pressure/dps310.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
index 0730380ceb69..6a21cb5d3d37 100644
--- a/drivers/iio/pressure/dps310.c
+++ b/drivers/iio/pressure/dps310.c
@@ -127,7 +127,7 @@ static int dps310_get_coefs(struct dps310_data *data)
c0 = (coef[0] << 4) | (coef[1] >> 4);
data->c0 = sign_extend32(c0, 11);
- c1 = ((coef[1] & GENMASK(3, 0)) << 8) | coef[2];
+ c1 = get_unaligned_be16(&coef[1]) & GENMASK(11, 0);
data->c1 = sign_extend32(c1, 11);
/*
@@ -138,22 +138,22 @@ static int dps310_get_coefs(struct dps310_data *data)
c00 = (coef[3] << 12) | (coef[4] << 4) | (coef[5] >> 4);
data->c00 = sign_extend32(c00, 19);
- c10 = ((coef[5] & GENMASK(3, 0)) << 16) | (coef[6] << 8) | coef[7];
+ c10 = get_unaligned_be24(&coef[5]) & GENMASK(19, 0);
data->c10 = sign_extend32(c10, 19);
- c01 = (coef[8] << 8) | coef[9];
+ c01 = get_unaligned_be16(&coef[8]);
data->c01 = sign_extend32(c01, 15);
- c11 = (coef[10] << 8) | coef[11];
+ c11 = get_unaligned_be16(&coef[10]);
data->c11 = sign_extend32(c11, 15);
- c20 = (coef[12] << 8) | coef[13];
+ c20 = get_unaligned_be16(&coef[12]);
data->c20 = sign_extend32(c20, 15);
- c21 = (coef[14] << 8) | coef[15];
+ c21 = get_unaligned_be16(&coef[14]);
data->c21 = sign_extend32(c21, 15);
- c30 = (coef[16] << 8) | coef[17];
+ c30 = get_unaligned_be16(&coef[16]);
data->c30 = sign_extend32(c30, 15);
return 0;
@@ -323,7 +323,7 @@ static int dps310_read_pres_raw(struct dps310_data *data)
if (rc < 0)
goto done;
- raw = (val[0] << 16) | (val[1] << 8) | val[2];
+ raw = get_unaligned_be24(&val[0]);
data->pressure_raw = sign_extend32(raw, 23);
done:
@@ -342,7 +342,7 @@ static int dps310_read_temp_ready(struct dps310_data *data)
if (rc < 0)
return rc;
- raw = (val[0] << 16) | (val[1] << 8) | val[2];
+ raw = get_unaligned_be24(&val[0]);
data->temp_raw = sign_extend32(raw, 23);
return 0;
--
2.28.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v1] iio: pressure: dps310: Use get_unaligned_beXX()
2020-10-26 18:05 [PATCH v1] iio: pressure: dps310: Use get_unaligned_beXX() Andy Shevchenko
@ 2020-10-29 15:25 ` Jonathan Cameron
2020-10-29 17:13 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2020-10-29 15:25 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-iio, Lars-Peter Clausen, Peter Meerwald-Stadler, Eddie James
On Mon, 26 Oct 2020 20:05:15 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> This makes the driver code slightly easier to read.
>
> Cc: Eddie James <eajames@linux.ibm.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Hi Andy,
I am a bit in 2 minds about this one.
I'm not 100% sure on the register arrangement here, but it 'looks' like
a be64 followed by some smaller registers.
It isn't documented like that in the datasheet though, so representing
it as such might confuse people...
So, we 'could' use a structure with a __be64 and some __be16s then all
would be aligned, but it would be hard to align with the datasheet.
Unfortunately what you have here is also rather hard to align with
the datasheet...
Jonathan
> ---
> drivers/iio/pressure/dps310.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
> index 0730380ceb69..6a21cb5d3d37 100644
> --- a/drivers/iio/pressure/dps310.c
> +++ b/drivers/iio/pressure/dps310.c
> @@ -127,7 +127,7 @@ static int dps310_get_coefs(struct dps310_data *data)
> c0 = (coef[0] << 4) | (coef[1] >> 4);
> data->c0 = sign_extend32(c0, 11);
>
> - c1 = ((coef[1] & GENMASK(3, 0)) << 8) | coef[2];
> + c1 = get_unaligned_be16(&coef[1]) & GENMASK(11, 0);
> data->c1 = sign_extend32(c1, 11);
>
> /*
> @@ -138,22 +138,22 @@ static int dps310_get_coefs(struct dps310_data *data)
> c00 = (coef[3] << 12) | (coef[4] << 4) | (coef[5] >> 4);
> data->c00 = sign_extend32(c00, 19);
>
> - c10 = ((coef[5] & GENMASK(3, 0)) << 16) | (coef[6] << 8) | coef[7];
> + c10 = get_unaligned_be24(&coef[5]) & GENMASK(19, 0);
> data->c10 = sign_extend32(c10, 19);
>
> - c01 = (coef[8] << 8) | coef[9];
> + c01 = get_unaligned_be16(&coef[8]);
> data->c01 = sign_extend32(c01, 15);
>
> - c11 = (coef[10] << 8) | coef[11];
> + c11 = get_unaligned_be16(&coef[10]);
> data->c11 = sign_extend32(c11, 15);
>
> - c20 = (coef[12] << 8) | coef[13];
> + c20 = get_unaligned_be16(&coef[12]);
> data->c20 = sign_extend32(c20, 15);
>
> - c21 = (coef[14] << 8) | coef[15];
> + c21 = get_unaligned_be16(&coef[14]);
> data->c21 = sign_extend32(c21, 15);
>
> - c30 = (coef[16] << 8) | coef[17];
> + c30 = get_unaligned_be16(&coef[16]);
> data->c30 = sign_extend32(c30, 15);
>
> return 0;
> @@ -323,7 +323,7 @@ static int dps310_read_pres_raw(struct dps310_data *data)
> if (rc < 0)
> goto done;
>
> - raw = (val[0] << 16) | (val[1] << 8) | val[2];
> + raw = get_unaligned_be24(&val[0]);
> data->pressure_raw = sign_extend32(raw, 23);
>
> done:
> @@ -342,7 +342,7 @@ static int dps310_read_temp_ready(struct dps310_data *data)
> if (rc < 0)
> return rc;
>
> - raw = (val[0] << 16) | (val[1] << 8) | val[2];
> + raw = get_unaligned_be24(&val[0]);
> data->temp_raw = sign_extend32(raw, 23);
>
> return 0;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1] iio: pressure: dps310: Use get_unaligned_beXX()
2020-10-29 15:25 ` Jonathan Cameron
@ 2020-10-29 17:13 ` Andy Shevchenko
0 siblings, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2020-10-29 17:13 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, linux-iio, Lars-Peter Clausen,
Peter Meerwald-Stadler, Eddie James
On Thu, Oct 29, 2020 at 5:26 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 26 Oct 2020 20:05:15 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> > This makes the driver code slightly easier to read.
> >
> > Cc: Eddie James <eajames@linux.ibm.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> Hi Andy,
>
> I am a bit in 2 minds about this one.
>
> I'm not 100% sure on the register arrangement here, but it 'looks' like
> a be64 followed by some smaller registers.
> It isn't documented like that in the datasheet though, so representing
> it as such might confuse people...
>
> So, we 'could' use a structure with a __be64 and some __be16s then all
> would be aligned, but it would be hard to align with the datasheet.
>
> Unfortunately what you have here is also rather hard to align with
> the datasheet...
Thanks for the review. I just got it right now that the proper one
(and for previous similar patch) should be converting to the bitmap
API.
Because it seems to me that there are fields from a bigger bitmap
area. However, I might be mistaken.
I will drop this from my local branch.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-29 17:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-26 18:05 [PATCH v1] iio: pressure: dps310: Use get_unaligned_beXX() Andy Shevchenko
2020-10-29 15:25 ` Jonathan Cameron
2020-10-29 17:13 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).