| Message ID | 20200728091057.6.I27a5605d9cb3ff643ef040c4ef932a30df9a8fac@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | sx9310 iio driver updates | expand |
On Tue, Jul 28, 2020 at 6:15 PM Daniel Campello <campello@chromium.org> wrote: > > Use __aligned(8) to ensure that the timestamp is correctly aligned > when we call push_to_buffers > > Signed-off-by: Daniel Campello <campello@chromium.org> > --- > > drivers/iio/proximity/sx9310.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c > index de52afd7c13333..fb5c16f2aa6b1a 100644 > --- a/drivers/iio/proximity/sx9310.c > +++ b/drivers/iio/proximity/sx9310.c > @@ -131,8 +131,8 @@ struct sx9310_data { > */ > bool prox_stat[SX9310_NUM_CHANNELS]; > bool trigger_enabled; > - __be16 buffer[SX9310_NUM_CHANNELS + > - 4]; /* 64-bit data + 64-bit timestamp */ > + /* 64-bit data + 64-bit timestamp buffer */ > + __be16 buffer[SX9310_NUM_CHANNELS + 4] __aligned(8); If the data amount (channels) is always the same, please, use struct approach. Otherwise put a comment explaining dynamic data.
On Tue, Jul 28, 2020 at 12:11 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Tue, Jul 28, 2020 at 6:15 PM Daniel Campello <campello@chromium.org> wrote: > > > > Use __aligned(8) to ensure that the timestamp is correctly aligned > > when we call push_to_buffers > > > > Signed-off-by: Daniel Campello <campello@chromium.org> > > --- > > > > drivers/iio/proximity/sx9310.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c > > index de52afd7c13333..fb5c16f2aa6b1a 100644 > > --- a/drivers/iio/proximity/sx9310.c > > +++ b/drivers/iio/proximity/sx9310.c > > @@ -131,8 +131,8 @@ struct sx9310_data { > > */ > > bool prox_stat[SX9310_NUM_CHANNELS]; > > bool trigger_enabled; > > - __be16 buffer[SX9310_NUM_CHANNELS + > > - 4]; /* 64-bit data + 64-bit timestamp */ > > + /* 64-bit data + 64-bit timestamp buffer */ > > + __be16 buffer[SX9310_NUM_CHANNELS + 4] __aligned(8); > > If the data amount (channels) is always the same, please, use struct approach. > Otherwise put a comment explaining dynamic data. I'm not sure what you mean here. I have a comment above for the size of the array. > > -- > With Best Regards, > Andy Shevchenko
On Wed, Jul 29, 2020 at 12:26 AM Daniel Campello <campello@google.com> wrote: > On Tue, Jul 28, 2020 at 12:11 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Tue, Jul 28, 2020 at 6:15 PM Daniel Campello <campello@chromium.org> wrote: ... > > > - __be16 buffer[SX9310_NUM_CHANNELS + > > > - 4]; /* 64-bit data + 64-bit timestamp */ > > > + /* 64-bit data + 64-bit timestamp buffer */ > > > + __be16 buffer[SX9310_NUM_CHANNELS + 4] __aligned(8); > > > > If the data amount (channels) is always the same, please, use struct approach. > > Otherwise put a comment explaining dynamic data. > I'm not sure what you mean here. I have a comment above for the size > of the array. Here [1] was a discussion about commenting on the dynamic amount of data [see the cover letter and replies to it] in the buffer and the struct approach [e.g. very first patch in the series]. [1]: https://lore.kernel.org/linux-iio/MN2PR12MB43905A2256F98BB5EFCE7DD3C4770@MN2PR12MB4390.namprd12.prod.outlook.com/T/
diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c index de52afd7c13333..fb5c16f2aa6b1a 100644 --- a/drivers/iio/proximity/sx9310.c +++ b/drivers/iio/proximity/sx9310.c @@ -131,8 +131,8 @@ struct sx9310_data { */ bool prox_stat[SX9310_NUM_CHANNELS]; bool trigger_enabled; - __be16 buffer[SX9310_NUM_CHANNELS + - 4]; /* 64-bit data + 64-bit timestamp */ + /* 64-bit data + 64-bit timestamp buffer */ + __be16 buffer[SX9310_NUM_CHANNELS + 4] __aligned(8); /* Remember enabled channels and sample rate during suspend. */ unsigned int suspend_ctrl0; struct completion completion; @@ -339,7 +339,7 @@ static int sx9310_read_prox_data(struct sx9310_data *data, if (ret < 0) return ret; - return regmap_bulk_read(data->regmap, chan->address, val, 2); + return regmap_bulk_read(data->regmap, chan->address, val, sizeof(*val)); } /*
Use __aligned(8) to ensure that the timestamp is correctly aligned when we call push_to_buffers Signed-off-by: Daniel Campello <campello@chromium.org> --- drivers/iio/proximity/sx9310.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)