| Message ID | 20230815154027.12468-3-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v1,1/3] types: Complement the aligned types with signed 64-bit one | expand |
On Tue, 15 Aug 2023 18:40:27 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > Use aligned_s64 for the timestamp field. > > Note, the actual data is signed, hence with this we also amend that. > While at it, drop redundant __alignment directive. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/iio/accel/hid-sensor-accel-3d.c | 3 +-- > drivers/iio/gyro/hid-sensor-gyro-3d.c | 2 +- > drivers/iio/humidity/hid-sensor-humidity.c | 2 +- > drivers/iio/light/hid-sensor-als.c | 2 +- > drivers/iio/orientation/hid-sensor-incl-3d.c | 2 +- > drivers/iio/orientation/hid-sensor-rotation.c | 4 ++-- > drivers/iio/position/hid-sensor-custom-intel-hinge.c | 2 +- > drivers/iio/pressure/hid-sensor-press.c | 2 +- > drivers/iio/temperature/hid-sensor-temperature.c | 2 +- > 9 files changed, 10 insertions(+), 11 deletions(-) > > diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c > index 5eac7ea19993..f739589564c5 100644 > --- a/drivers/iio/accel/hid-sensor-accel-3d.c > +++ b/drivers/iio/accel/hid-sensor-accel-3d.c > @@ -25,10 +25,9 @@ struct accel_3d_state { > struct hid_sensor_hub_callbacks callbacks; > struct hid_sensor_common common_attributes; > struct hid_sensor_hub_attribute_info accel[ACCEL_3D_CHANNEL_MAX]; > - /* Ensure timestamp is naturally aligned */ Comment is still true and no more or less obvious than it was with the old code I think so I don't really see why it should be removed with this change. > struct { > u32 accel_val[3]; > - s64 timestamp __aligned(8); > + aligned_s64 timestamp; > } scan; > int scale_pre_decml; > int scale_post_decml; ... > diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c > index a033699910e8..864ecbcad26e 100644 > --- a/drivers/iio/orientation/hid-sensor-rotation.c > +++ b/drivers/iio/orientation/hid-sensor-rotation.c > @@ -19,8 +19,8 @@ struct dev_rot_state { > struct hid_sensor_common common_attributes; > struct hid_sensor_hub_attribute_info quaternion; > struct { > - s32 sampled_vals[4] __aligned(16); > - u64 timestamp __aligned(8); > + s32 sampled_vals[4]; Hmm. I can't immediately recall why that aligned(16) is therebut it's not related to the rest of this change so I'm not sure we should touch it. I don't think we ever required quaternions to be aligned as a whole so this does feel odd but in the category of something we should be careful touching or at very least do it in a different patch where it stands out. > + aligned_s64 timestamp; > } scan; > int scale_pre_decml; > int scale_post_decml; > diff --git a/drivers/iio/position/hid-sensor-custom-intel-hinge.c b/drivers/iio/position/hid-sensor-custom-intel-hinge.c > index 07c30d217255..48005b568dd9 100644 > --- a/drivers/iio/position/hid-sensor-custom-intel-hinge.c > +++ b/drivers/iio/position/hid-sensor-custom-intel-hinge.c > @@ -39,7 +39,7 @@ struct hinge_state { > const char *labels[CHANNEL_SCAN_INDEX_MAX]; > struct { > u32 hinge_val[3]; > - u64 timestamp __aligned(8); > + aligned_s64 timestamp; > } scan; > > int scale_pre_decml; > diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c > index a9215eb32d70..a964c7b65402 100644 > --- a/drivers/iio/pressure/hid-sensor-press.c > +++ b/drivers/iio/pressure/hid-sensor-press.c > @@ -24,7 +24,7 @@ struct press_state { > struct hid_sensor_hub_attribute_info press_attr; > struct { > u32 press_data; > - u64 timestamp __aligned(8); > + aligned_s64 timestamp; > } scan; > int scale_pre_decml; > int scale_post_decml; > diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c > index d40f235af1d4..32f4b13fd554 100644 > --- a/drivers/iio/temperature/hid-sensor-temperature.c > +++ b/drivers/iio/temperature/hid-sensor-temperature.c > @@ -18,7 +18,7 @@ struct temperature_state { > struct hid_sensor_hub_attribute_info temperature_attr; > struct { > s32 temperature_data; > - u64 timestamp __aligned(8); > + aligned_s64 timestamp; > } scan; > int scale_pre_decml; > int scale_post_decml;
On Mon, Aug 28, 2023 at 05:09:28PM +0100, Jonathan Cameron wrote: > On Tue, 15 Aug 2023 18:40:27 +0300 > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: ... > > - /* Ensure timestamp is naturally aligned */ > > Comment is still true and no more or less obvious than it was with the old code > I think so I don't really see why it should be removed with this change. Doesn't the prefix "aligned" make this comment somehow redundant? > > struct { > > u32 accel_val[3]; > > - s64 timestamp __aligned(8); > > + aligned_s64 timestamp; > > } scan; ... > > - s32 sampled_vals[4] __aligned(16); > > + s32 sampled_vals[4]; > > Hmm. I can't immediately recall why that aligned(16) is therebut > it's not related to the rest of this change so I'm not sure we should touch > it. I don't think we ever required quaternions to be aligned as a whole so > this does feel odd but in the category of something we should be careful > touching or at very least do it in a different patch where it stands out. I have checked the code and find nothing that justifies this, I can split it to a separate patch, though. Note, among ISH HID drivers it's the only one with a such... > > + aligned_s64 timestamp; > > } scan;
diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c index 5eac7ea19993..f739589564c5 100644 --- a/drivers/iio/accel/hid-sensor-accel-3d.c +++ b/drivers/iio/accel/hid-sensor-accel-3d.c @@ -25,10 +25,9 @@ struct accel_3d_state { struct hid_sensor_hub_callbacks callbacks; struct hid_sensor_common common_attributes; struct hid_sensor_hub_attribute_info accel[ACCEL_3D_CHANNEL_MAX]; - /* Ensure timestamp is naturally aligned */ struct { u32 accel_val[3]; - s64 timestamp __aligned(8); + aligned_s64 timestamp; } scan; int scale_pre_decml; int scale_post_decml; diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c index 698c50da1f10..a7050a6328d6 100644 --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c @@ -27,7 +27,7 @@ struct gyro_3d_state { struct hid_sensor_hub_attribute_info gyro[GYRO_3D_CHANNEL_MAX]; struct { u32 gyro_val[GYRO_3D_CHANNEL_MAX]; - u64 timestamp __aligned(8); + aligned_s64 timestamp; } scan; int scale_pre_decml; int scale_post_decml; diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c index fa0fe404a70a..0e484b78b735 100644 --- a/drivers/iio/humidity/hid-sensor-humidity.c +++ b/drivers/iio/humidity/hid-sensor-humidity.c @@ -18,7 +18,7 @@ struct hid_humidity_state { struct hid_sensor_hub_attribute_info humidity_attr; struct { s32 humidity_data; - u64 timestamp __aligned(8); + aligned_s64 timestamp; } scan; int scale_pre_decml; int scale_post_decml; diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c index eb1aedad7edc..8a906d95edd4 100644 --- a/drivers/iio/light/hid-sensor-als.c +++ b/drivers/iio/light/hid-sensor-als.c @@ -27,7 +27,7 @@ struct als_state { struct hid_sensor_hub_attribute_info als_illum; struct { u32 illum[CHANNEL_SCAN_INDEX_MAX]; - u64 timestamp __aligned(8); + aligned_s64 timestamp; } scan; int scale_pre_decml; int scale_post_decml; diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c index ba5b581d5b25..3e5f2c58dfa9 100644 --- a/drivers/iio/orientation/hid-sensor-incl-3d.c +++ b/drivers/iio/orientation/hid-sensor-incl-3d.c @@ -29,7 +29,7 @@ struct incl_3d_state { struct hid_sensor_hub_attribute_info incl[INCLI_3D_CHANNEL_MAX]; struct { u32 incl_val[INCLI_3D_CHANNEL_MAX]; - u64 timestamp __aligned(8); + aligned_s64 timestamp; } scan; int scale_pre_decml; int scale_post_decml; diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c index a033699910e8..864ecbcad26e 100644 --- a/drivers/iio/orientation/hid-sensor-rotation.c +++ b/drivers/iio/orientation/hid-sensor-rotation.c @@ -19,8 +19,8 @@ struct dev_rot_state { struct hid_sensor_common common_attributes; struct hid_sensor_hub_attribute_info quaternion; struct { - s32 sampled_vals[4] __aligned(16); - u64 timestamp __aligned(8); + s32 sampled_vals[4]; + aligned_s64 timestamp; } scan; int scale_pre_decml; int scale_post_decml; diff --git a/drivers/iio/position/hid-sensor-custom-intel-hinge.c b/drivers/iio/position/hid-sensor-custom-intel-hinge.c index 07c30d217255..48005b568dd9 100644 --- a/drivers/iio/position/hid-sensor-custom-intel-hinge.c +++ b/drivers/iio/position/hid-sensor-custom-intel-hinge.c @@ -39,7 +39,7 @@ struct hinge_state { const char *labels[CHANNEL_SCAN_INDEX_MAX]; struct { u32 hinge_val[3]; - u64 timestamp __aligned(8); + aligned_s64 timestamp; } scan; int scale_pre_decml; diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c index a9215eb32d70..a964c7b65402 100644 --- a/drivers/iio/pressure/hid-sensor-press.c +++ b/drivers/iio/pressure/hid-sensor-press.c @@ -24,7 +24,7 @@ struct press_state { struct hid_sensor_hub_attribute_info press_attr; struct { u32 press_data; - u64 timestamp __aligned(8); + aligned_s64 timestamp; } scan; int scale_pre_decml; int scale_post_decml; diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c index d40f235af1d4..32f4b13fd554 100644 --- a/drivers/iio/temperature/hid-sensor-temperature.c +++ b/drivers/iio/temperature/hid-sensor-temperature.c @@ -18,7 +18,7 @@ struct temperature_state { struct hid_sensor_hub_attribute_info temperature_attr; struct { s32 temperature_data; - u64 timestamp __aligned(8); + aligned_s64 timestamp; } scan; int scale_pre_decml; int scale_post_decml;
Use aligned_s64 for the timestamp field. Note, the actual data is signed, hence with this we also amend that. While at it, drop redundant __alignment directive. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/iio/accel/hid-sensor-accel-3d.c | 3 +-- drivers/iio/gyro/hid-sensor-gyro-3d.c | 2 +- drivers/iio/humidity/hid-sensor-humidity.c | 2 +- drivers/iio/light/hid-sensor-als.c | 2 +- drivers/iio/orientation/hid-sensor-incl-3d.c | 2 +- drivers/iio/orientation/hid-sensor-rotation.c | 4 ++-- drivers/iio/position/hid-sensor-custom-intel-hinge.c | 2 +- drivers/iio/pressure/hid-sensor-press.c | 2 +- drivers/iio/temperature/hid-sensor-temperature.c | 2 +- 9 files changed, 10 insertions(+), 11 deletions(-)