Re: [Y2038] [PATCH v2 9/9] [media] omap3isp: support 64-bit version of omap3isp_stat_data
Hi Arnd, On Monday 09 November 2015 21:30:41 Arnd Bergmann wrote: > On Monday 09 November 2015 22:09:26 Laurent Pinchart wrote: > > On Thursday 17 September 2015 23:19:40 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. > > > > We plan to design a new interface to handle statistics in V4L2. That API > > should support proper 64-bit timestamps out of the box, and will be > > implemented by the OMAP3 ISP driver. Userspace should then move to it. I > > wonder if it's worth it to fix the existing VIDIOC_OMAP3ISP_STAT_REQ ioctl > > given that I expect it to have a handful of users at most. > > We still need to do something to the driver. The alternative would > be to make the existing ioctl command optional at kernel compile-time > so we can still build the driver once we remove the 'struct timeval' > definition. That patch would add slightly less complexity here > but also lose functionality. > > As my patch here depends on the struct v4l2_timeval I introduced in > an earlier patch of the series, we will have to change it anyways, > but I'd prefer to keep the basic idea. Let's get back to this one > after the v4l_buffer replacement work is done. Fine with me. I'll mark the patch as "Obsoleted" in patchwork in the meantime. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Y2038] [PATCH v2 9/9] [media] omap3isp: support 64-bit version of omap3isp_stat_data
On Monday 09 November 2015 22:09:26 Laurent Pinchart wrote: > Hi Arnd, > > Thank you for the patch. > > On Thursday 17 September 2015 23:19:40 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. > > We plan to design a new interface to handle statistics in V4L2. That API > should support proper 64-bit timestamps out of the box, and will be > implemented by the OMAP3 ISP driver. Userspace should then move to it. I > wonder if it's worth it to fix the existing VIDIOC_OMAP3ISP_STAT_REQ ioctl > given that I expect it to have a handful of users at most. We still need to do something to the driver. The alternative would be to make the existing ioctl command optional at kernel compile-time so we can still build the driver once we remove the 'struct timeval' definition. That patch would add slightly less complexity here but also lose functionality. As my patch here depends on the struct v4l2_timeval I introduced in an earlier patch of the series, we will have to change it anyways, but I'd prefer to keep the basic idea. Let's get back to this one after the v4l_buffer replacement work is done. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 9/9] [media] omap3isp: support 64-bit version of omap3isp_stat_data
Hi Arnd, Thank you for the patch. On Thursday 17 September 2015 23:19:40 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. We plan to design a new interface to handle statistics in V4L2. That API should support proper 64-bit timestamps out of the box, and will be implemented by the OMAP3 ISP driver. Userspace should then move to it. I wonder if it's worth it to fix the existing VIDIOC_OMAP3ISP_STAT_REQ ioctl given that I expect it to have a handful of users at most. > 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 | 18 -- > drivers/media/platform/omap3isp/ispstat.h | 2 ++ > include/uapi/linux/omap3isp.h | 19 +++ > 6 files changed, 43 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/omap3isp/isph3a_aewb.c > b/drivers/media/platform/omap3isp/isph3a_aewb.c index > ccaf92f39236..2d567725608f 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 > 92937f7eecef..2ac02371318b 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 > 7138b043a4aa..669b97b079ee 100644 > --- a/drivers/media/platform/omap3isp/isphist.c > +++ b/drivers/media/platform/omap3isp/isphist.c > @@ -436,6 +436,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 > fa96e330c563..3d70332422b5 100644 > --- a/drivers/media/platform/omap3isp/ispstat.c > +++ b/drivers/media/platform/omap3isp/ispstat.c > @@ -496,8 +496,7 @@ int omap3isp_stat_request_statistics(struct ispstat > *stat, return PTR_ERR(buf); > } > > - data->ts.tv_sec = buf->ts.tv_sec; > - data->ts.tv_usec = buf->ts.tv_usec; > + data->ts = buf->ts; > data->config_counter = buf->config_counter; > data->frame_number = buf->frame_number; > data->buf_size = buf->buf_size; > @@ -509,6 +508,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
[PATCH v2 9/9] [media] omap3isp: support 64-bit version of omap3isp_stat_data
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. 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 | 18 -- drivers/media/platform/omap3isp/ispstat.h | 2 ++ include/uapi/linux/omap3isp.h | 19 +++ 6 files changed, 43 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/omap3isp/isph3a_aewb.c b/drivers/media/platform/omap3isp/isph3a_aewb.c index ccaf92f39236..2d567725608f 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 92937f7eecef..2ac02371318b 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 7138b043a4aa..669b97b079ee 100644 --- a/drivers/media/platform/omap3isp/isphist.c +++ b/drivers/media/platform/omap3isp/isphist.c @@ -436,6 +436,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 fa96e330c563..3d70332422b5 100644 --- a/drivers/media/platform/omap3isp/ispstat.c +++ b/drivers/media/platform/omap3isp/ispstat.c @@ -496,8 +496,7 @@ int omap3isp_stat_request_statistics(struct ispstat *stat, return PTR_ERR(buf); } - data->ts.tv_sec = buf->ts.tv_sec; - data->ts.tv_usec = buf->ts.tv_usec; + data->ts = buf->ts; data->config_counter = buf->config_counter; data->frame_number = buf->frame_number; data->buf_size = buf->buf_size; @@ -509,6 +508,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 7b4f136567a3..b19ea6c8f733 100644 --- a/drivers/media/platform/omap3isp/ispstat.h +++ b/drivers/media/platform/omap3isp/ispstat.h @@ -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 *st