linux-kernel - Re: [PATCH v1 3/3] iio: hid-sensor: Use aligned data type for timestamp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230828170928.5264dadf@jic23-huawei>
Date:   Mon, 28 Aug 2023 17:09:28 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Lorenzo Bianconi <lorenzo@...nel.org>,
        Philipp Jungkamp <p.jungkamp@....net>,
        Peter Zijlstra <peterz@...radead.org>,
        linux-input@...r.kernel.org, linux-iio@...r.kernel.org,
        linux-kernel@...r.kernel.org, Jiri Kosina <jikos@...nel.org>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        Lars-Peter Clausen <lars@...afoo.de>
Subject: Re: [PATCH v1 3/3] iio: hid-sensor: Use aligned data type for
 timestamp

On Tue, 15 Aug 2023 18:40:27 +0300
Andy Shevchenko <andriy.shevchenko@...ux.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@...ux.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;