Messages in this thread | | | Date | Fri, 10 Apr 2015 15:36:10 +0200 | From | Sebastian Reichel <> | Subject | Re: [RFC, PATCH] HSI: cmt_speech: fix timestamp interface |
| |
Hi Arnd,
[CC'd Kai's new mail address]
On Fri, Apr 10, 2015 at 01:59:19PM +0200, Arnd Bergmann wrote: > The user interface for timestamps in the new cms_speech > driver is broken in multiple ways:
ow :( Thanks for the report.
> - The layout is incompatible between 32-bit and 64-bit user > space, because of the size differences in 'struct timespec'. > This means that the driver can not work when used with 32-bit > user space on a 64-bit kernel. > > - As there are plans to change 32-bit user space to use > a 64-bit time_t type in the future, it will also be > incompatible with new 32-bit user space. > > As the driver is not yet widely used, now would be a good time > to change it to just use a 64-bit nanosecond value, which also > happens to be more efficient. In order to make the layout > non-ambiguous, we have to also add extra padding so the 64-bit > value is naturally aligned. > > The change requires the respective update to user space tools > using it. If that is for some reason not possible any more, > another solution would be to replace the timespec structure > with two __u32 values to avoid incompatibilities later. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
I think updating to two __u32 values is the better solution here. While the driver is new in the mainline kernel it has been used for multiple years downstream in the Maemo/MeeGo/Mer communities and I would prefer to keep userspace compatibility.
-- Sebastian
> diff --git a/drivers/hsi/clients/cmt_speech.c b/drivers/hsi/clients/cmt_speech.c > index 4983529a9c6c..568dbc117f2e 100644 > --- a/drivers/hsi/clients/cmt_speech.c > +++ b/drivers/hsi/clients/cmt_speech.c > @@ -451,9 +451,7 @@ static void cs_hsi_read_on_control_complete(struct hsi_msg *msg) > dev_dbg(&hi->cl->device, "Read on control: %08X\n", cmd); > cs_release_cmd(msg); > if (hi->flags & CS_FEAT_TSTAMP_RX_CTRL) { > - struct timespec *tstamp = > - &hi->mmap_cfg->tstamp_rx_ctrl; > - do_posix_clock_monotonic_gettime(tstamp); > + hi->mmap_cfg->tstamp_rx_ctrl = ktime_get_ns(); > } > spin_unlock(&hi->lock); > > diff --git a/include/uapi/linux/hsi/cs-protocol.h b/include/uapi/linux/hsi/cs-protocol.h > index 4957bba57cbe..fce633c7c649 100644 > --- a/include/uapi/linux/hsi/cs-protocol.h > +++ b/include/uapi/linux/hsi/cs-protocol.h > @@ -90,12 +90,12 @@ struct cs_mmap_config_block { > __u32 tx_offsets[CS_MAX_BUFFERS]; > __u32 rx_ptr; > __u32 rx_ptr_boundary; > - __u32 reserved3[2]; > + __u32 reserved3[3]; > /* > * if enabled with CS_FEAT_TSTAMP_RX_CTRL, monotonic > * timestamp taken when the last control command was received > */ > - struct timespec tstamp_rx_ctrl; > + __u64 tstamp_rx_ctrl; > }; > > #define CS_IO_MAGIC 'C' [unhandled content-type:application/pgp-signature] | |