Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752540AbdLEAlO (ORCPT ); Mon, 4 Dec 2017 19:41:14 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:41182 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752162AbdLEAlL (ORCPT ); Mon, 4 Dec 2017 19:41:11 -0500 From: Laurent Pinchart To: Arnd Bergmann Cc: Mauro Carvalho Chehab , y2038@lists.linaro.org, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, Sakari Ailus Subject: Re: [PATCH 5/8] [media] omap3isp: support 64-bit version of omap3isp_stat_data Date: Tue, 05 Dec 2017 02:41:24 +0200 Message-ID: <1517434.pRS3rpdWG0@avalon> Organization: Ideas on Board Oy In-Reply-To: <20171127132027.1734806-5-arnd@arndb.de> References: <20171127132027.1734806-1-arnd@arndb.de> <20171127132027.1734806-5-arnd@arndb.de> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7568 Lines: 202 Hi Arnd, Thank you for the patch. I'll try to review this without too much delay. In the meantime, I'm CC'ing Sakari Ailus who might be faster than me :-) On Monday, 27 November 2017 15:19:57 EET Arnd Bergmann wrote: > C libraries with 64-bit time_t use an incompatible format for > struct omap3isp_stat_data. This changes the kernel code to > support either version, by moving over the normal handling > to the 64-bit variant, and adding compatiblity code to handle > the old binary format with the existing ioctl command code. > > Fortunately, the command code includes the size of the structure, > so the difference gets handled automatically. In the process of > eliminating the references to 'struct timeval' from the kernel, > I also change the way the timestamp is generated internally, > basically by open-coding the v4l2_get_timestamp() call. > > Signed-off-by: Arnd Bergmann > --- > drivers/media/platform/omap3isp/isph3a_aewb.c | 2 ++ > drivers/media/platform/omap3isp/isph3a_af.c | 2 ++ > drivers/media/platform/omap3isp/isphist.c | 2 ++ > drivers/media/platform/omap3isp/ispstat.c | 21 +++++++++++++++++++-- > drivers/media/platform/omap3isp/ispstat.h | 4 +++- > include/uapi/linux/omap3isp.h | 22 ++++++++++++++++++++++ > 6 files changed, 50 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/platform/omap3isp/isph3a_aewb.c > b/drivers/media/platform/omap3isp/isph3a_aewb.c index > d44626f20ac6..3c82dea4d375 100644 > --- a/drivers/media/platform/omap3isp/isph3a_aewb.c > +++ b/drivers/media/platform/omap3isp/isph3a_aewb.c > @@ -250,6 +250,8 @@ static long h3a_aewb_ioctl(struct v4l2_subdev *sd, > unsigned int cmd, void *arg) return omap3isp_stat_config(stat, arg); > case VIDIOC_OMAP3ISP_STAT_REQ: > return omap3isp_stat_request_statistics(stat, arg); > + case VIDIOC_OMAP3ISP_STAT_REQ_TIME32: > + return omap3isp_stat_request_statistics_time32(stat, arg); > case VIDIOC_OMAP3ISP_STAT_EN: { > unsigned long *en = arg; > return omap3isp_stat_enable(stat, !!*en); > diff --git a/drivers/media/platform/omap3isp/isph3a_af.c > b/drivers/media/platform/omap3isp/isph3a_af.c index > 99bd6cc21d86..4da25c84f0c6 100644 > --- a/drivers/media/platform/omap3isp/isph3a_af.c > +++ b/drivers/media/platform/omap3isp/isph3a_af.c > @@ -314,6 +314,8 @@ static long h3a_af_ioctl(struct v4l2_subdev *sd, > unsigned int cmd, void *arg) return omap3isp_stat_config(stat, arg); > case VIDIOC_OMAP3ISP_STAT_REQ: > return omap3isp_stat_request_statistics(stat, arg); > + case VIDIOC_OMAP3ISP_STAT_REQ_TIME32: > + return omap3isp_stat_request_statistics_time32(stat, arg); > case VIDIOC_OMAP3ISP_STAT_EN: { > int *en = arg; > return omap3isp_stat_enable(stat, !!*en); > diff --git a/drivers/media/platform/omap3isp/isphist.c > b/drivers/media/platform/omap3isp/isphist.c index > a4ed5d140d48..d4be3d0e06f9 100644 > --- a/drivers/media/platform/omap3isp/isphist.c > +++ b/drivers/media/platform/omap3isp/isphist.c > @@ -435,6 +435,8 @@ static long hist_ioctl(struct v4l2_subdev *sd, unsigned > int cmd, void *arg) return omap3isp_stat_config(stat, arg); > case VIDIOC_OMAP3ISP_STAT_REQ: > return omap3isp_stat_request_statistics(stat, arg); > + case VIDIOC_OMAP3ISP_STAT_REQ_TIME32: > + return omap3isp_stat_request_statistics_time32(stat, arg); > case VIDIOC_OMAP3ISP_STAT_EN: { > int *en = arg; > return omap3isp_stat_enable(stat, !!*en); > diff --git a/drivers/media/platform/omap3isp/ispstat.c > b/drivers/media/platform/omap3isp/ispstat.c index > 47cbc7e3d825..5967dfd0a9f7 100644 > --- a/drivers/media/platform/omap3isp/ispstat.c > +++ b/drivers/media/platform/omap3isp/ispstat.c > @@ -18,6 +18,7 @@ > #include > #include > #include > +#include > > #include "isp.h" > > @@ -237,7 +238,7 @@ static int isp_stat_buf_queue(struct ispstat *stat) > if (!stat->active_buf) > return STAT_NO_BUF; > > - v4l2_get_timestamp(&stat->active_buf->ts); > + ktime_get_ts64(&stat->active_buf->ts); > > stat->active_buf->buf_size = stat->buf_size; > if (isp_stat_buf_check_magic(stat, stat->active_buf)) { > @@ -500,7 +501,8 @@ int omap3isp_stat_request_statistics(struct ispstat > *stat, return PTR_ERR(buf); > } > > - data->ts = buf->ts; > + data->ts.tv_sec = buf->ts.tv_sec; > + data->ts.tv_usec = buf->ts.tv_nsec / NSEC_PER_USEC; > data->config_counter = buf->config_counter; > data->frame_number = buf->frame_number; > data->buf_size = buf->buf_size; > @@ -512,6 +514,21 @@ int omap3isp_stat_request_statistics(struct ispstat > *stat, return 0; > } > > +int omap3isp_stat_request_statistics_time32(struct ispstat *stat, > + struct omap3isp_stat_data_time32 *data) > +{ > + struct omap3isp_stat_data data64; > + int ret; > + > + ret = omap3isp_stat_request_statistics(stat, &data64); > + > + data->ts.tv_sec = data64.ts.tv_sec; > + data->ts.tv_usec = data64.ts.tv_usec; > + memcpy(&data->buf, &data64.buf, sizeof(*data) - sizeof(data->ts)); > + > + return ret; > +} > + > /* > * omap3isp_stat_config - Receives new statistic engine configuration. > * @new_conf: Pointer to config structure. > diff --git a/drivers/media/platform/omap3isp/ispstat.h > b/drivers/media/platform/omap3isp/ispstat.h index > 6d9b0244f320..923b38cfc682 100644 > --- a/drivers/media/platform/omap3isp/ispstat.h > +++ b/drivers/media/platform/omap3isp/ispstat.h > @@ -39,7 +39,7 @@ struct ispstat_buffer { > struct sg_table sgt; > void *virt_addr; > dma_addr_t dma_addr; > - struct timeval ts; > + struct timespec64 ts; > u32 buf_size; > u32 frame_number; > u16 config_counter; > @@ -130,6 +130,8 @@ struct ispstat_generic_config { > int omap3isp_stat_config(struct ispstat *stat, void *new_conf); > int omap3isp_stat_request_statistics(struct ispstat *stat, > struct omap3isp_stat_data *data); > +int omap3isp_stat_request_statistics_time32(struct ispstat *stat, > + struct omap3isp_stat_data_time32 *data); > int omap3isp_stat_init(struct ispstat *stat, const char *name, > const struct v4l2_subdev_ops *sd_ops); > void omap3isp_stat_cleanup(struct ispstat *stat); > diff --git a/include/uapi/linux/omap3isp.h b/include/uapi/linux/omap3isp.h > index 1a920145db04..87b55755f4ff 100644 > --- a/include/uapi/linux/omap3isp.h > +++ b/include/uapi/linux/omap3isp.h > @@ -55,6 +55,8 @@ > _IOWR('V', BASE_VIDIOC_PRIVATE + 5, struct omap3isp_h3a_af_config) > #define VIDIOC_OMAP3ISP_STAT_REQ \ > _IOWR('V', BASE_VIDIOC_PRIVATE + 6, struct omap3isp_stat_data) > +#define VIDIOC_OMAP3ISP_STAT_REQ_TIME32 \ > + _IOWR('V', BASE_VIDIOC_PRIVATE + 6, struct omap3isp_stat_data_time32) > #define VIDIOC_OMAP3ISP_STAT_EN \ > _IOWR('V', BASE_VIDIOC_PRIVATE + 7, unsigned long) > > @@ -165,7 +167,14 @@ struct omap3isp_h3a_aewb_config { > * @config_counter: Number of the configuration associated with the data. > */ > struct omap3isp_stat_data { > +#ifdef __KERNEL__ > + struct { > + __s64 tv_sec; > + __s64 tv_usec; > + } ts; > +#else > struct timeval ts; > +#endif > void __user *buf; > __u32 buf_size; > __u16 frame_number; > @@ -173,6 +182,19 @@ struct omap3isp_stat_data { > __u16 config_counter; > }; > > +#ifdef __KERNEL__ > +struct omap3isp_stat_data_time32 { > + struct { > + __s32 tv_sec; > + __s32 tv_usec; > + } ts; > + __u32 buf; > + __u32 buf_size; > + __u16 frame_number; > + __u16 cur_frame; > + __u16 config_counter; > +}; > +#endif > > /* Histogram related structs */ -- Regards, Laurent Pinchart