Re: [PATCH] atomisp2: unify some ifdef cases caused by format changes

2017-03-08 Thread Mauro Carvalho Chehab 
Em Wed, 8 Mar 2017 14:55:44 +0100
Hans Verkuil  escreveu:

> On 08/03/17 14:45, Hans Verkuil wrote:
> > On 08/03/17 14:39, Greg KH wrote:  
> >> On Wed, Mar 08, 2017 at 01:49:23PM +0100, Hans Verkuil wrote:  
> >>> OK, so I discovered that these patches are for a driver added to 
> >>> linux-next
> >>> without it ever been cross-posted to linux-media.
> >>>
> >>> To be polite, I think that's rather impolite.  
> >>
> >> They were, but got rejected due to the size :(
> >>
> >> Mauro was cc:ed directly, he knew these were coming...
> >>
> >> I can take care of the cleanup patches for now, you don't have to review
> >> them if you don't want to.  
> >
> > Please do.
> >
> > For the next time if the patches are too large: at least post a message with
> > a link to a repo for people to look at. I would like to know what's going
> > on in staging/media, especially since I will do a lot of the reviewing (at
> > least if it is a V4L2 driver) when they want to move it out of staging.  
> 
> Same issue BTW with the bcm2835 driver. That too landed in staging without
> ever being posted to the linux-media mailinglist. Size is no excuse for that
> driver since it isn't that large.

This one got posted at media ML:
https://patchwork.linuxtv.org/project/linux-media/list/?state=*=2835

(patches 1/6 to 6/6)

Thanks,
Mauro

Re: [PATCH] atomisp2: unify some ifdef cases caused by format changes

2017-03-08 Thread Greg KH 
On Wed, Mar 08, 2017 at 01:49:23PM +0100, Hans Verkuil wrote:
> OK, so I discovered that these patches are for a driver added to linux-next
> without it ever been cross-posted to linux-media.
> 
> To be polite, I think that's rather impolite.

They were, but got rejected due to the size :(

Mauro was cc:ed directly, he knew these were coming...

I can take care of the cleanup patches for now, you don't have to review
them if you don't want to.

thanks,

greg k-h

Re: [PATCH] atomisp2: unify some ifdef cases caused by format changes

2017-03-08 Thread Greg KH 
On Wed, Mar 08, 2017 at 02:55:44PM +0100, Hans Verkuil wrote:
> On 08/03/17 14:45, Hans Verkuil wrote:
> > On 08/03/17 14:39, Greg KH wrote:
> > > On Wed, Mar 08, 2017 at 01:49:23PM +0100, Hans Verkuil wrote:
> > > > OK, so I discovered that these patches are for a driver added to 
> > > > linux-next
> > > > without it ever been cross-posted to linux-media.
> > > > 
> > > > To be polite, I think that's rather impolite.
> > > 
> > > They were, but got rejected due to the size :(
> > > 
> > > Mauro was cc:ed directly, he knew these were coming...
> > > 
> > > I can take care of the cleanup patches for now, you don't have to review
> > > them if you don't want to.
> > 
> > Please do.
> > 
> > For the next time if the patches are too large: at least post a message with
> > a link to a repo for people to look at. I would like to know what's going
> > on in staging/media, especially since I will do a lot of the reviewing (at
> > least if it is a V4L2 driver) when they want to move it out of staging.
> 
> Same issue BTW with the bcm2835 driver. That too landed in staging without
> ever being posted to the linux-media mailinglist. Size is no excuse for that
> driver since it isn't that large.
> 
> I'll handle cleanup patches for the bcm2835 driver since it is now in our 
> tree.

Nope, it got moved out as it didn't belong there yet :)

It's now in drivers/staging/vc04_services/ as all of the issues so far
aren't media ones, but vc04 api issues.  Once those get ironed out, then
the media people can have at it :)

thanks,

greg k-h

Re: [PATCH] atomisp2: unify some ifdef cases caused by format changes

2017-03-08 Thread Hans Verkuil 
On 08/03/17 14:55, Hans Verkuil wrote:
> On 08/03/17 14:45, Hans Verkuil wrote:
>> On 08/03/17 14:39, Greg KH wrote:
>>> On Wed, Mar 08, 2017 at 01:49:23PM +0100, Hans Verkuil wrote:
 OK, so I discovered that these patches are for a driver added to linux-next
 without it ever been cross-posted to linux-media.

 To be polite, I think that's rather impolite.
>>>
>>> They were, but got rejected due to the size :(
>>>
>>> Mauro was cc:ed directly, he knew these were coming...
>>>
>>> I can take care of the cleanup patches for now, you don't have to review
>>> them if you don't want to.
>>
>> Please do.
>>
>> For the next time if the patches are too large: at least post a message with
>> a link to a repo for people to look at. I would like to know what's going
>> on in staging/media, especially since I will do a lot of the reviewing (at
>> least if it is a V4L2 driver) when they want to move it out of staging.
> 
> Same issue BTW with the bcm2835 driver. That too landed in staging without
> ever being posted to the linux-media mailinglist. Size is no excuse for that
> driver since it isn't that large.

Sorry about the noise. This driver *was* cross-posted to us. Ignore this rant 
:-)

Hans

Re: [PATCH] atomisp2: unify some ifdef cases caused by format changes

2017-03-08 Thread Hans Verkuil 
On 08/03/17 15:20, Greg KH wrote:
> On Wed, Mar 08, 2017 at 02:55:44PM +0100, Hans Verkuil wrote:
>> On 08/03/17 14:45, Hans Verkuil wrote:
>>> On 08/03/17 14:39, Greg KH wrote:
 On Wed, Mar 08, 2017 at 01:49:23PM +0100, Hans Verkuil wrote:
> OK, so I discovered that these patches are for a driver added to 
> linux-next
> without it ever been cross-posted to linux-media.
>
> To be polite, I think that's rather impolite.

 They were, but got rejected due to the size :(

 Mauro was cc:ed directly, he knew these were coming...

 I can take care of the cleanup patches for now, you don't have to review
 them if you don't want to.
>>>
>>> Please do.
>>>
>>> For the next time if the patches are too large: at least post a message with
>>> a link to a repo for people to look at. I would like to know what's going
>>> on in staging/media, especially since I will do a lot of the reviewing (at
>>> least if it is a V4L2 driver) when they want to move it out of staging.
>>
>> Same issue BTW with the bcm2835 driver. That too landed in staging without
>> ever being posted to the linux-media mailinglist. Size is no excuse for that
>> driver since it isn't that large.
>>
>> I'll handle cleanup patches for the bcm2835 driver since it is now in our 
>> tree.
> 
> Nope, it got moved out as it didn't belong there yet :)
> 
> It's now in drivers/staging/vc04_services/ as all of the issues so far
> aren't media ones, but vc04 api issues.  Once those get ironed out, then
> the media people can have at it :)

OK, then I will ignore bcm2835 patches for now.

Good to know!

Hans

Re: [PATCH] atomisp2: unify some ifdef cases caused by format changes

2017-03-08 Thread Hans Verkuil 

On 08/03/17 14:45, Hans Verkuil wrote:

On 08/03/17 14:39, Greg KH wrote:

On Wed, Mar 08, 2017 at 01:49:23PM +0100, Hans Verkuil wrote:

OK, so I discovered that these patches are for a driver added to linux-next
without it ever been cross-posted to linux-media.

To be polite, I think that's rather impolite.


They were, but got rejected due to the size :(

Mauro was cc:ed directly, he knew these were coming...

I can take care of the cleanup patches for now, you don't have to review
them if you don't want to.


Please do.

For the next time if the patches are too large: at least post a message with
a link to a repo for people to look at. I would like to know what's going
on in staging/media, especially since I will do a lot of the reviewing (at
least if it is a V4L2 driver) when they want to move it out of staging.


Same issue BTW with the bcm2835 driver. That too landed in staging without
ever being posted to the linux-media mailinglist. Size is no excuse for that
driver since it isn't that large.

I'll handle cleanup patches for the bcm2835 driver since it is now in our tree.

Regards,

Hans

Re: [PATCH] atomisp2: unify some ifdef cases caused by format changes

2017-03-08 Thread Hans Verkuil 

On 08/03/17 14:39, Greg KH wrote:

On Wed, Mar 08, 2017 at 01:49:23PM +0100, Hans Verkuil wrote:

OK, so I discovered that these patches are for a driver added to linux-next
without it ever been cross-posted to linux-media.

To be polite, I think that's rather impolite.


They were, but got rejected due to the size :(

Mauro was cc:ed directly, he knew these were coming...

I can take care of the cleanup patches for now, you don't have to review
them if you don't want to.


Please do.

For the next time if the patches are too large: at least post a message with
a link to a repo for people to look at. I would like to know what's going
on in staging/media, especially since I will do a lot of the reviewing (at
least if it is a V4L2 driver) when they want to move it out of staging.

Thanks,

Hans

Re: [PATCH] atomisp2: unify some ifdef cases caused by format changes

2017-03-08 Thread Hans Verkuil 

OK, so I discovered that these patches are for a driver added to linux-next
without it ever been cross-posted to linux-media.

To be polite, I think that's rather impolite.

I'm now reviewing patches for a driver I haven't seen, so that makes me rather
unhappy.

Please cross-post staging/media drivers to linux-media!

Regards,

Hans

On 06/03/17 12:21, Alan Cox wrote:

The two drivers were originally merged by tools, and the tools didn't always
spot white space only changes. Fix a few of them found by zero-day and clean
up some more by hand.

Signed-off-by: Alan Cox 
---
 .../media/atomisp/pci/atomisp2/atomisp_cmd.c   |   29 ---
 .../media/atomisp/pci/atomisp2/atomisp_ioctl.c |   21 -
 .../media/atomisp/pci/atomisp2/atomisp_subdev.c|   81 
 .../media/atomisp/pci/atomisp2/atomisp_v4l2.c  |8 --
 4 files changed, 6 insertions(+), 133 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index e99f7b8..d9a5c24 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -1099,15 +1099,9 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, 
int error,
WARN_ON(!vb);
if (vb)
pipe->frame_config_id[vb->i] = frame->isp_config_id;
-#ifndef ISP2401
if (css_pipe_id == IA_CSS_PIPE_ID_CAPTURE &&
asd->pending_capture_request > 0) {
err = atomisp_css_offline_capture_configure(asd,
-#else
-   if (css_pipe_id == IA_CSS_PIPE_ID_CAPTURE) {
-   if (asd->pending_capture_request > 0) {
-   err = atomisp_css_offline_capture_configure(asd,
-#endif
asd->params.offline_parm.num_captures,
asd->params.offline_parm.skip_frames,
asd->params.offline_parm.offset);
@@ -1298,9 +1292,7 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int 
error,
 */
wake_up(>done);
}
-#ifndef ISP2401
-
-#else
+#ifdef ISP2401
atomic_set(>wdt_count, 0);
 #endif
/*
@@ -4995,26 +4987,15 @@ int atomisp_try_fmt(struct video_device *vdev, struct 
v4l2_format *f,
 {
struct atomisp_device *isp = video_get_drvdata(vdev);
struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
-#ifndef ISP2401
struct v4l2_subdev_pad_config pad_cfg;
-#else
-struct v4l2_subdev_pad_config pad_cfg;
-#endif
struct v4l2_subdev_format format = {
.which = V4L2_SUBDEV_FORMAT_TRY,
-#ifndef ISP2401
};
-#else
-   };
-#endif
+
struct v4l2_mbus_framefmt *snr_mbus_fmt = 
const struct atomisp_format_bridge *fmt;
struct atomisp_input_stream_info *stream_info =
-#ifndef ISP2401
(struct atomisp_input_stream_info *)snr_mbus_fmt->reserved;
-#else
-   (struct atomisp_input_stream_info *)snr_mbus_fmt->reserved;
-#endif
uint16_t stream_index;
int source_pad = atomisp_subdev_source_pad(vdev);
int ret;
@@ -5044,11 +5025,7 @@ int atomisp_try_fmt(struct video_device *vdev, struct 
v4l2_format *f,
snr_mbus_fmt->width, snr_mbus_fmt->height);

ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
-#ifndef ISP2401
   pad, set_fmt, _cfg, );
-#else
-   pad, set_fmt, _cfg, );
-#endif
if (ret)
return ret;

@@ -6454,9 +6431,7 @@ int atomisp_s_ae_window(struct atomisp_sub_device *asd,
struct atomisp_ae_window *arg)
 {
struct atomisp_device *isp = asd->isp;
-#ifndef ISP2401
/* Coverity CID 298071 - initialzize struct */
-#endif
struct v4l2_subdev_selection sel = { 0 };

sel.r.left = arg->x_left;
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c
index 1445279..6064bb8 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c
@@ -530,13 +530,8 @@ const struct atomisp_format_bridge 
*atomisp_get_format_bridge(
return NULL;
 }

-#ifndef ISP2401
 const struct atomisp_format_bridge *atomisp_get_format_bridge_from_mbus(
u32 mbus_code)
-#else
-const struct atomisp_format_bridge *atomisp_get_format_bridge_from_mbus(u32
-   
mbus_code)
-#endif
 {
unsigned int i;

@@ -1303,14 +1298,8 @@ static int atomisp_qbuf(struct file *file, void *fh, 
struct v4l2_buffer *buf)
/* this buffer will have a per-frame parameter */

[PATCH] atomisp2: unify some ifdef cases caused by format changes

2017-03-06 Thread Alan Cox 
The two drivers were originally merged by tools, and the tools didn't always
spot white space only changes. Fix a few of them found by zero-day and clean
up some more by hand.

Signed-off-by: Alan Cox 
---
 .../media/atomisp/pci/atomisp2/atomisp_cmd.c   |   29 ---
 .../media/atomisp/pci/atomisp2/atomisp_ioctl.c |   21 -
 .../media/atomisp/pci/atomisp2/atomisp_subdev.c|   81 
 .../media/atomisp/pci/atomisp2/atomisp_v4l2.c  |8 --
 4 files changed, 6 insertions(+), 133 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index e99f7b8..d9a5c24 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -1099,15 +1099,9 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, 
int error,
WARN_ON(!vb);
if (vb)
pipe->frame_config_id[vb->i] = frame->isp_config_id;
-#ifndef ISP2401
if (css_pipe_id == IA_CSS_PIPE_ID_CAPTURE &&
asd->pending_capture_request > 0) {
err = atomisp_css_offline_capture_configure(asd,
-#else
-   if (css_pipe_id == IA_CSS_PIPE_ID_CAPTURE) {
-   if (asd->pending_capture_request > 0) {
-   err = atomisp_css_offline_capture_configure(asd,
-#endif
asd->params.offline_parm.num_captures,
asd->params.offline_parm.skip_frames,
asd->params.offline_parm.offset);
@@ -1298,9 +1292,7 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int 
error,
 */
wake_up(>done);
}
-#ifndef ISP2401
-
-#else
+#ifdef ISP2401
atomic_set(>wdt_count, 0);
 #endif
/*
@@ -4995,26 +4987,15 @@ int atomisp_try_fmt(struct video_device *vdev, struct 
v4l2_format *f,
 {
struct atomisp_device *isp = video_get_drvdata(vdev);
struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
-#ifndef ISP2401
struct v4l2_subdev_pad_config pad_cfg;
-#else
-struct v4l2_subdev_pad_config pad_cfg;
-#endif
struct v4l2_subdev_format format = {
.which = V4L2_SUBDEV_FORMAT_TRY,
-#ifndef ISP2401
};
-#else
-   };
-#endif
+
struct v4l2_mbus_framefmt *snr_mbus_fmt = 
const struct atomisp_format_bridge *fmt;
struct atomisp_input_stream_info *stream_info =
-#ifndef ISP2401
(struct atomisp_input_stream_info *)snr_mbus_fmt->reserved;
-#else
-   (struct atomisp_input_stream_info *)snr_mbus_fmt->reserved;
-#endif
uint16_t stream_index;
int source_pad = atomisp_subdev_source_pad(vdev);
int ret;
@@ -5044,11 +5025,7 @@ int atomisp_try_fmt(struct video_device *vdev, struct 
v4l2_format *f,
snr_mbus_fmt->width, snr_mbus_fmt->height);
 
ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
-#ifndef ISP2401
   pad, set_fmt, _cfg, );
-#else
-   pad, set_fmt, _cfg, );
-#endif
if (ret)
return ret;
 
@@ -6454,9 +6431,7 @@ int atomisp_s_ae_window(struct atomisp_sub_device *asd,
struct atomisp_ae_window *arg)
 {
struct atomisp_device *isp = asd->isp;
-#ifndef ISP2401
/* Coverity CID 298071 - initialzize struct */
-#endif
struct v4l2_subdev_selection sel = { 0 };
 
sel.r.left = arg->x_left;
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c
index 1445279..6064bb8 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c
@@ -530,13 +530,8 @@ const struct atomisp_format_bridge 
*atomisp_get_format_bridge(
return NULL;
 }
 
-#ifndef ISP2401
 const struct atomisp_format_bridge *atomisp_get_format_bridge_from_mbus(
u32 mbus_code)
-#else
-const struct atomisp_format_bridge *atomisp_get_format_bridge_from_mbus(u32
-   
mbus_code)
-#endif
 {
unsigned int i;
 
@@ -1303,14 +1298,8 @@ static int atomisp_qbuf(struct file *file, void *fh, 
struct v4l2_buffer *buf)
/* this buffer will have a per-frame parameter */
pipe->frame_request_config_id[buf->index] = buf->reserved2 &
~ATOMISP_BUFFER_HAS_PER_FRAME_SETTING;
-#ifndef ISP2401
dev_dbg(isp->dev, "This buffer requires per_frame setting which 
has isp_config_id %d\n",
pipe->frame_request_config_id[buf->index]);
-#else
-   dev_dbg(isp->dev, "This buffer requires per_frame setting \