Re: [PATCH] media: pvrusb2: Convert timers to use timer_setup()

2017-10-25 Thread Mike Isely

Ack'ed (separate formal reply)

  -Mike

On Wed, 25 Oct 2017, Kees Cook wrote:

> Eek, sorry, this uses timer_setup_on_stack() which is only in -next.
> If you can Ack this, I can carry it in the timer tree.
> 
> Thanks!
> 
> -Kees
> 
> On Tue, Oct 24, 2017 at 5:22 PM, Kees Cook <keesc...@chromium.org> wrote:
> > In preparation for unconditionally passing the struct timer_list pointer to
> > all timer callbacks, switch to using the new timer_setup() and from_timer()
> > to pass the timer pointer explicitly.
> >
> > Cc: Mike Isely <is...@pobox.com>
> > Cc: Mauro Carvalho Chehab <mche...@kernel.org>
> > Cc: linux-media@vger.kernel.org
> > Signed-off-by: Kees Cook <keesc...@chromium.org>
> > ---
> >  drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 64 
> > ++---
> >  1 file changed, 36 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c 
> > b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> > index ad5b25b89699..8289ee482f49 100644
> > --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> > +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> > @@ -330,10 +330,10 @@ static void pvr2_hdw_state_log_state(struct pvr2_hdw 
> > *);
> >  static int pvr2_hdw_cmd_usbstream(struct pvr2_hdw *hdw,int runFl);
> >  static int pvr2_hdw_commit_setup(struct pvr2_hdw *hdw);
> >  static int pvr2_hdw_get_eeprom_addr(struct pvr2_hdw *hdw);
> > -static void pvr2_hdw_quiescent_timeout(unsigned long);
> > -static void pvr2_hdw_decoder_stabilization_timeout(unsigned long);
> > -static void pvr2_hdw_encoder_wait_timeout(unsigned long);
> > -static void pvr2_hdw_encoder_run_timeout(unsigned long);
> > +static void pvr2_hdw_quiescent_timeout(struct timer_list *);
> > +static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *);
> > +static void pvr2_hdw_encoder_wait_timeout(struct timer_list *);
> > +static void pvr2_hdw_encoder_run_timeout(struct timer_list *);
> >  static int pvr2_issue_simple_cmd(struct pvr2_hdw *,u32);
> >  static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> > unsigned int timeout,int probe_fl,
> > @@ -2373,18 +2373,15 @@ struct pvr2_hdw *pvr2_hdw_create(struct 
> > usb_interface *intf,
> > }
> > if (!hdw) goto fail;
> >
> > -   setup_timer(>quiescent_timer, pvr2_hdw_quiescent_timeout,
> > -   (unsigned long)hdw);
> > +   timer_setup(>quiescent_timer, pvr2_hdw_quiescent_timeout, 0);
> >
> > -   setup_timer(>decoder_stabilization_timer,
> > -   pvr2_hdw_decoder_stabilization_timeout,
> > -   (unsigned long)hdw);
> > +   timer_setup(>decoder_stabilization_timer,
> > +   pvr2_hdw_decoder_stabilization_timeout, 0);
> >
> > -   setup_timer(>encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> > -   (unsigned long)hdw);
> > +   timer_setup(>encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> > +   0);
> >
> > -   setup_timer(>encoder_run_timer, pvr2_hdw_encoder_run_timeout,
> > -   (unsigned long)hdw);
> > +   timer_setup(>encoder_run_timer, pvr2_hdw_encoder_run_timeout, 
> > 0);
> >
> > hdw->master_state = PVR2_STATE_DEAD;
> >
> > @@ -3539,10 +3536,16 @@ static void pvr2_ctl_read_complete(struct urb *urb)
> > complete(>ctl_done);
> >  }
> >
> > +struct hdw_timer {
> > +   struct timer_list timer;
> > +   struct pvr2_hdw *hdw;
> > +};
> >
> > -static void pvr2_ctl_timeout(unsigned long data)
> > +static void pvr2_ctl_timeout(struct timer_list *t)
> >  {
> > -   struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> > +   struct hdw_timer *timer = from_timer(timer, t, timer);
> > +   struct pvr2_hdw *hdw = timer->hdw;
> > +
> > if (hdw->ctl_write_pend_flag || hdw->ctl_read_pend_flag) {
> > hdw->ctl_timeout_flag = !0;
> > if (hdw->ctl_write_pend_flag)
> > @@ -3564,7 +3567,10 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> >  {
> > unsigned int idx;
> > int status = 0;
> > -   struct timer_list timer;
> > +   struct hdw_timer timer = {
> > +   .hdw = hdw,
> > +   };
> > +
> > if (!hdw->ctl_lock_held) {
> > pvr2_trace(PVR2_TRACE_ERROR_LEGS,
> >"Atte

Re: [PATCH] media: pvrusb2: Convert timers to use timer_setup()

2017-10-25 Thread Mike Isely

Acked-By: Mike Isely <is...@pobox.com>

On Tue, 24 Oct 2017, Kees Cook wrote:

> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
> 
> Cc: Mike Isely <is...@pobox.com>
> Cc: Mauro Carvalho Chehab <mche...@kernel.org>
> Cc: linux-media@vger.kernel.org
> Signed-off-by: Kees Cook <keesc...@chromium.org>
> ---
>  drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 64 
> ++---
>  1 file changed, 36 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c 
> b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> index ad5b25b89699..8289ee482f49 100644
> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> @@ -330,10 +330,10 @@ static void pvr2_hdw_state_log_state(struct pvr2_hdw *);
>  static int pvr2_hdw_cmd_usbstream(struct pvr2_hdw *hdw,int runFl);
>  static int pvr2_hdw_commit_setup(struct pvr2_hdw *hdw);
>  static int pvr2_hdw_get_eeprom_addr(struct pvr2_hdw *hdw);
> -static void pvr2_hdw_quiescent_timeout(unsigned long);
> -static void pvr2_hdw_decoder_stabilization_timeout(unsigned long);
> -static void pvr2_hdw_encoder_wait_timeout(unsigned long);
> -static void pvr2_hdw_encoder_run_timeout(unsigned long);
> +static void pvr2_hdw_quiescent_timeout(struct timer_list *);
> +static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *);
> +static void pvr2_hdw_encoder_wait_timeout(struct timer_list *);
> +static void pvr2_hdw_encoder_run_timeout(struct timer_list *);
>  static int pvr2_issue_simple_cmd(struct pvr2_hdw *,u32);
>  static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
>   unsigned int timeout,int probe_fl,
> @@ -2373,18 +2373,15 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface 
> *intf,
>   }
>   if (!hdw) goto fail;
>  
> - setup_timer(>quiescent_timer, pvr2_hdw_quiescent_timeout,
> - (unsigned long)hdw);
> + timer_setup(>quiescent_timer, pvr2_hdw_quiescent_timeout, 0);
>  
> - setup_timer(>decoder_stabilization_timer,
> - pvr2_hdw_decoder_stabilization_timeout,
> - (unsigned long)hdw);
> + timer_setup(>decoder_stabilization_timer,
> + pvr2_hdw_decoder_stabilization_timeout, 0);
>  
> - setup_timer(>encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> - (unsigned long)hdw);
> + timer_setup(>encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> + 0);
>  
> - setup_timer(>encoder_run_timer, pvr2_hdw_encoder_run_timeout,
> - (unsigned long)hdw);
> + timer_setup(>encoder_run_timer, pvr2_hdw_encoder_run_timeout, 0);
>  
>   hdw->master_state = PVR2_STATE_DEAD;
>  
> @@ -3539,10 +3536,16 @@ static void pvr2_ctl_read_complete(struct urb *urb)
>   complete(>ctl_done);
>  }
>  
> +struct hdw_timer {
> + struct timer_list timer;
> + struct pvr2_hdw *hdw;
> +};
>  
> -static void pvr2_ctl_timeout(unsigned long data)
> +static void pvr2_ctl_timeout(struct timer_list *t)
>  {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct hdw_timer *timer = from_timer(timer, t, timer);
> + struct pvr2_hdw *hdw = timer->hdw;
> +
>   if (hdw->ctl_write_pend_flag || hdw->ctl_read_pend_flag) {
>   hdw->ctl_timeout_flag = !0;
>   if (hdw->ctl_write_pend_flag)
> @@ -3564,7 +3567,10 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
>  {
>   unsigned int idx;
>   int status = 0;
> - struct timer_list timer;
> + struct hdw_timer timer = {
> + .hdw = hdw,
> + };
> +
>   if (!hdw->ctl_lock_held) {
>   pvr2_trace(PVR2_TRACE_ERROR_LEGS,
>  "Attempted to execute control transfer without 
> lock!!");
> @@ -3621,8 +3627,8 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
>   hdw->ctl_timeout_flag = 0;
>   hdw->ctl_write_pend_flag = 0;
>   hdw->ctl_read_pend_flag = 0;
> - setup_timer(, pvr2_ctl_timeout, (unsigned long)hdw);
> - timer.expires = jiffies + timeout;
> + timer_setup_on_stack(, pvr2_ctl_timeout, 0);
> + timer.timer.expires = jiffies + timeout;
>  
>   if (write_len && write_data) {
>   hdw->cmd_debug_state = 2;
> @@ -3677,7 +3683,7 @@ status);
>   }
>  
>   /* Start timer */
> - add_timer();
> + add_timer();
>  
>   /* Now wait for all I/O to complete */

Re: [PATCH 05/24] media: v4l2-dev: convert VFL_TYPE_* into an enum

2017-10-09 Thread Mike Isely

Acked-By: Mike Isely <is...@pobox.com>

On Mon, 9 Oct 2017, Mauro Carvalho Chehab wrote:

> Using enums makes easier to document, as it can use kernel-doc
> markups. It also allows cross-referencing, with increases the
> kAPI readability.
> 
> Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
> ---
>  Documentation/media/kapi/v4l2-dev.rst | 17 ++---
>  drivers/media/pci/cx88/cx88-blackbird.c   |  3 +-
>  drivers/media/pci/cx88/cx88-video.c   | 10 +++---
>  drivers/media/pci/cx88/cx88.h |  4 +--
>  drivers/media/pci/saa7134/saa7134-video.c |  2 ++
>  drivers/media/usb/cx231xx/cx231xx-video.c |  2 ++
>  drivers/media/usb/pvrusb2/pvrusb2-v4l2.c  |  2 ++
>  drivers/media/usb/tm6000/tm6000-video.c   |  2 ++
>  drivers/media/v4l2-core/v4l2-dev.c| 10 +++---
>  include/media/v4l2-dev.h  | 59 
> +--
>  include/media/v4l2-mediabus.h | 30 
>  11 files changed, 98 insertions(+), 43 deletions(-)
> 
> diff --git a/Documentation/media/kapi/v4l2-dev.rst 
> b/Documentation/media/kapi/v4l2-dev.rst
> index b29aa616c267..7bb0505b60f1 100644
> --- a/Documentation/media/kapi/v4l2-dev.rst
> +++ b/Documentation/media/kapi/v4l2-dev.rst
> @@ -196,11 +196,18 @@ device.
>  Which device is registered depends on the type argument. The following
>  types exist:
>  
> -- ``VFL_TYPE_GRABBER``: ``/dev/videoX`` for video input/output devices
> -- ``VFL_TYPE_VBI``: ``/dev/vbiX`` for vertical blank data (i.e. closed 
> captions, teletext)
> -- ``VFL_TYPE_RADIO``: ``/dev/radioX`` for radio tuners
> -- ``VFL_TYPE_SDR``: ``/dev/swradioX`` for Software Defined Radio tuners
> -- ``VFL_TYPE_TOUCH``: ``/dev/v4l-touchX`` for touch sensors
> +==    
> ==
> +:c:type:`vfl_devnode_type` Device nameUsage
> +==    
> ==
> +``VFL_TYPE_GRABBER``   ``/dev/videoX``   for video input/output 
> devices
> +``VFL_TYPE_VBI``   ``/dev/vbiX`` for vertical blank data 
> (i.e.
> +  closed captions, teletext)
> +``VFL_TYPE_RADIO`` ``/dev/radioX``   for radio tuners
> +``VFL_TYPE_SUBDEV````/dev/v4l-subdevX``  for V4L2 subdevices
> +``VFL_TYPE_SDR``   ``/dev/swradioX`` for Software Defined Radio
> +  (SDR) tuners
> +``VFL_TYPE_TOUCH`` ``/dev/v4l-touchX``   for touch sensors
> +==    
> ==
>  
>  The last argument gives you a certain amount of control over the device
>  device node number used (i.e. the X in ``videoX``). Normally you will pass -1
> diff --git a/drivers/media/pci/cx88/cx88-blackbird.c 
> b/drivers/media/pci/cx88/cx88-blackbird.c
> index e3101f04941c..0e0952e60795 100644
> --- a/drivers/media/pci/cx88/cx88-blackbird.c
> +++ b/drivers/media/pci/cx88/cx88-blackbird.c
> @@ -805,8 +805,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
>  
>   strcpy(cap->driver, "cx88_blackbird");
>   sprintf(cap->bus_info, "PCI:%s", pci_name(dev->pci));
> - cx88_querycap(file, core, cap);
> - return 0;
> + return cx88_querycap(file, core, cap);
>  }
>  
>  static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
> diff --git a/drivers/media/pci/cx88/cx88-video.c 
> b/drivers/media/pci/cx88/cx88-video.c
> index 7d25ecd4404b..9be682cdb644 100644
> --- a/drivers/media/pci/cx88/cx88-video.c
> +++ b/drivers/media/pci/cx88/cx88-video.c
> @@ -806,8 +806,8 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void 
> *priv,
>   return 0;
>  }
>  
> -void cx88_querycap(struct file *file, struct cx88_core *core,
> -struct v4l2_capability *cap)
> +int cx88_querycap(struct file *file, struct cx88_core *core,
> +   struct v4l2_capability *cap)
>  {
>   struct video_device *vdev = video_devdata(file);
>  
> @@ -825,11 +825,14 @@ void cx88_querycap(struct file *file, struct cx88_core 
> *core,
>   case VFL_TYPE_VBI:
>   cap->device_caps |= V4L2_CAP_VBI_CAPTURE;
>   break;
> + default:
> + return -EINVAL;
>   }
>   cap->capabilities = cap->device_caps | V4L2_CAP_VIDEO_CAPTURE |
>   V4L2_CAP_VBI_CAPTURE | V4L2_CAP_DEVICE_CAPS;
>   if (core->board.radio.type == CX88_RADIO)
>   cap->capabilities |= V4L2_CAP_RADIO;
> + return 0;
>  }
>  EXPORT_SYMBOL(cx88_querycap);
>

Re: usb/media/pvrusb2: warning in pvr2_send_request_ex/usb_submit_urb

2017-09-20 Thread Mike Isely

What you have here is way beyond just feeding random crap in via the 
syscall interface.  To cause this you have to fake the presence of a 
pvrusb2 compatible *hardware* USB device and then lie about its endpoint 
configuration.  Is that really a concern here?  Are we now saying that 
any kernel driver which talks via USB must now also specifically verify 
the exact expected USB endpoint configuration?  Where does that end?  
How about the vendor-specific RPC protocol that the hardware actually 
implements over the bulk endpoint?  It's likely that the pvrusb2 driver 
may be making assumptions about the expected responses over that 
protocol.

Please realize that I'm not dismissing this.  I can see some merit in 
this.  But I'm just a bit surprised that now we're going this far.  Is 
this really the intention?  You're talking about code 
(pvrusb2_send_request_ex()) that hasn't changed in about 10 years.  
With this level of paranoia there's got to be a pretty target-rich 
environment over the set of kernel-supported USB devices.

To take this another step, wouldn't that same level of paranoia be a 
concern for any externally connected PCI-Express device?  Because that's 
another external way into the computer that involves very non-trivial 
and very hardware-centric protocols.  Thunderbolt devices would be an 
example of this.

  -Mike


On Wed, 20 Sep 2017, Andrey Konovalov wrote:

> Hi!
> 
> I've got the following report while fuzzing the kernel with syzkaller.
> 
> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).
> 
> There seems to be no check on endpoint type before submitting bulk urb
> in pvr2_send_request_ex().
> 
> usb 1-1: New USB device found, idVendor=2040, idProduct=7500
> usb 1-1: New USB device strings: Mfr=0, Product=255, SerialNumber=0
> usb 1-1: Product: a
> gadgetfs: configuration #6
> pvrusb2: Hardware description: WinTV HVR-1950 Model 750xx
> usb 1-1: BOGUS urb xfer, pipe 3 != type 1
> [ cut here ]
> WARNING: CPU: 1 PID: 2713 at drivers/usb/core/urb.c:449
> usb_submit_urb+0xf8a/0x11d0
> Modules linked in:
> CPU: 1 PID: 2713 Comm: pvrusb2-context Not tainted
> 4.14.0-rc1-42251-gebb2c2437d80 #210
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> task: 88006b7a18c0 task.stack: 880069978000
> RIP: 0010:usb_submit_urb+0xf8a/0x11d0 drivers/usb/core/urb.c:448
> RSP: 0018:88006997f990 EFLAGS: 00010286
> RAX: 0029 RBX: 880063661900 RCX: 
> RDX: 0029 RSI: 86876d60 RDI: ed000d32ff24
> RBP: 88006997fa90 R08: 11000d32fdca R09: 
> R10:  R11:  R12: 11000d32ff39
> R13: 0001 R14: 0003 R15: 880068bbed68
> FS:  () GS:88006c60() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 01032000 CR3: 6a0ff000 CR4: 06f0
> Call Trace:
>  pvr2_send_request_ex+0xa57/0x1d80 
> drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3645
>  pvr2_hdw_check_firmware drivers/media/usb/pvrusb2/pvrusb2-hdw.c:1812
>  pvr2_hdw_setup_low drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2107
>  pvr2_hdw_setup drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2250
>  pvr2_hdw_initialize+0x548/0x3c10 drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2327
>  pvr2_context_check drivers/media/usb/pvrusb2/pvrusb2-context.c:118
>  pvr2_context_thread_func+0x361/0x8c0
> drivers/media/usb/pvrusb2/pvrusb2-context.c:167
>  kthread+0x3a1/0x470 kernel/kthread.c:231
>  ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
> Code: 48 8b 85 30 ff ff ff 48 8d b8 98 00 00 00 e8 ee 82 89 fe 45 89
> e8 44 89 f1 4c 89 fa 48 89 c6 48 c7 c7 40 c0 ea 86 e8 30 1b dc fc <0f>
> ff e9 9b f7 ff ff e8 aa 95 25 fd e9 80 f7 ff ff e8 50 74 f3
> ---[ end trace 6919030503719da6 ]---
> 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8


Re: [PATCH v2 5/6] media/usb/pvrusb2: Support for V4L2_CTRL_WHICH_DEF_VAL

2015-10-29 Thread Mike Isely

Looks good to me (still), including now the change I had previously 
suggested.  For the record, the ack still applies.  (I guess you can 
consider this to be an ack of the ack...)

  -Mike


On Thu, 29 Oct 2015, Ricardo Ribalda Delgado wrote:

> This driver does not use the control infrastructure.
> Add support for the new field which on structure
>  v4l2_ext_controls
> 
> Acked-by: Mike Isely <is...@pobox.com>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.riba...@gmail.com>
> ---
>  drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c 
> b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
> index 1c5f85bf7ed4..81f788b7b242 100644
> --- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
> +++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
> @@ -628,6 +628,7 @@ static int pvr2_g_ext_ctrls(struct file *file, void *priv,
>   struct pvr2_v4l2_fh *fh = file->private_data;
>   struct pvr2_hdw *hdw = fh->channel.mc_head->hdw;
>   struct v4l2_ext_control *ctrl;
> + struct pvr2_ctrl *cptr;
>   unsigned int idx;
>   int val;
>   int ret;
> @@ -635,8 +636,15 @@ static int pvr2_g_ext_ctrls(struct file *file, void 
> *priv,
>   ret = 0;
>   for (idx = 0; idx < ctls->count; idx++) {
>   ctrl = ctls->controls + idx;
> - ret = pvr2_ctrl_get_value(
> - pvr2_hdw_get_ctrl_v4l(hdw, ctrl->id), );
> + cptr = pvr2_hdw_get_ctrl_v4l(hdw, ctrl->id);
> + if (cptr) {
> + if (ctls->which == V4L2_CTRL_WHICH_DEF_VAL)
> + pvr2_ctrl_get_def(cptr, );
> + else
> + ret = pvr2_ctrl_get_value(cptr, );
> + } else
> + ret = -EINVAL;
> +
>   if (ret) {
>   ctls->error_idx = idx;
>   return ret;
> @@ -658,6 +666,10 @@ static int pvr2_s_ext_ctrls(struct file *file, void 
> *priv,
>   unsigned int idx;
>   int ret;
>  
> + /* Default value cannot be changed */
> + if (ctls->which == V4L2_CTRL_WHICH_DEF_VAL)
> + return -EINVAL;
> +
>   ret = 0;
>   for (idx = 0; idx < ctls->count; idx++) {
>   ctrl = ctls->controls + idx;
> 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line "unsubscribe linux-media" 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 07/10] media/usb/pvrusb2: Support for V4L2_CTRL_WHICH_DEF_VAL

2015-08-21 Thread Mike Isely

The code you've added is carefully checking the return pointer from 
pvr2_hdw_get_ctrl_v4l() yet the original code did not operate this way.  
The result is that now there's this unbalanced effect where it appears 
that the validity of the pvr2_ctrl instance is only checked on one side 
of the if-statement.  I would recommend instead to elevate the call to 
pvr2_hdw_get_ctrl_v4l() out of the if-statement - since in both cases 
it's being called the same way both times.  Then do the validity check 
in that one spot and that simplifies the if-statement all the way down 
to choosing between pvr2_ctrl_get_value() vs pvr2_ctrl_get_def().

It's not a correctness comment; what you have should work fine.  So I'm 
ack'ing this in any case:

Acked-By: Mike Isely is...@pobox.com

But you can do the above pretty easily  safely, and simplify it a bit 
further.

  -Mike


On Fri, 21 Aug 2015, Ricardo Ribalda Delgado wrote:

 This driver does not use the control infrastructure.
 Add support for the new field which on structure
  v4l2_ext_controls
 
 Signed-off-by: Ricardo Ribalda Delgado ricardo.riba...@gmail.com
 ---
  drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 17 -
  1 file changed, 16 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c 
 b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
 index 1c5f85bf7ed4..43b2f2214798 100644
 --- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
 +++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
 @@ -628,6 +628,7 @@ static int pvr2_g_ext_ctrls(struct file *file, void *priv,
   struct pvr2_v4l2_fh *fh = file-private_data;
   struct pvr2_hdw *hdw = fh-channel.mc_head-hdw;
   struct v4l2_ext_control *ctrl;
 + struct pvr2_ctrl *cptr;
   unsigned int idx;
   int val;
   int ret;
 @@ -635,8 +636,18 @@ static int pvr2_g_ext_ctrls(struct file *file, void 
 *priv,
   ret = 0;
   for (idx = 0; idx  ctls-count; idx++) {
   ctrl = ctls-controls + idx;
 - ret = pvr2_ctrl_get_value(
 + if (ctls-which == V4L2_CTRL_WHICH_DEF_VAL) {
 + cptr = pvr2_hdw_get_ctrl_v4l(hdw, ctrl-id);
 + if (cptr)
 + pvr2_ctrl_get_def(cptr, val);
 + else
 + ret = -EINVAL;
 +
 +
 + } else
 + ret = pvr2_ctrl_get_value(
   pvr2_hdw_get_ctrl_v4l(hdw, ctrl-id), val);
 +
   if (ret) {
   ctls-error_idx = idx;
   return ret;
 @@ -658,6 +669,10 @@ static int pvr2_s_ext_ctrls(struct file *file, void 
 *priv,
   unsigned int idx;
   int ret;
  
 + /* Default value cannot be changed */
 + if (ctls-which == V4L2_CTRL_WHICH_DEF_VAL)
 + return -EINVAL;
 +
   ret = 0;
   for (idx = 0; idx  ctls-count; idx++) {
   ctrl = ctls-controls + idx;
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] [media] [pvrusb2]: remove dead retry cmd code

2015-01-24 Thread Mike Isely

Sorry been asleep at the wheel here.  I'll take a look.

Please realize that the code path being talked about here HAS worked - 
because the encoder does tend to fail and this is how the driver 
recovers.

  -Mike


On Fri, 16 Jan 2015, Hans Verkuil wrote:

 On 01/16/2015 12:29 PM, Haim Daniel wrote:
  It looks that if (try_count  20) continue jumps to end of the  do ...
  while(0) loop and goes out.
 
 Ah, you are right. But that is obviously not what was intended, so just 
 removing
 it is not a proper 'fix'.
 
 Mike, can you take a look at this?
 
 Regards,
 
   Hans
 
  
  --hd.
  On Fri, 2015-01-16 at 11:57 +0100, Hans Verkuil wrote:
  On 01/05/2015 11:38 PM, Haim Daniel wrote:
  In case a command is timed out, current flow sets the retry_flag
  and does nothing.
 
  Really? That's not how I read the code: it retries up to 20 times before
  bailing out.
 
  Perhaps you missed the if (try_count  20) continue; line?
 
  Regards,
 
 Hans
 
 
  Signed-off-by: Haim Daniel haim.dan...@gmail.com
  ---
   drivers/media/usb/pvrusb2/pvrusb2-encoder.c | 15 +--
   1 file changed, 1 insertion(+), 14 deletions(-)
 
  diff --git a/drivers/media/usb/pvrusb2/pvrusb2-encoder.c 
  b/drivers/media/usb/pvrusb2/pvrusb2-encoder.c
  index f7702ae..02028aa 100644
  --- a/drivers/media/usb/pvrusb2/pvrusb2-encoder.c
  +++ b/drivers/media/usb/pvrusb2/pvrusb2-encoder.c
  @@ -145,8 +145,6 @@ static int pvr2_encoder_cmd(void *ctxt,
u32 *argp)
   {
unsigned int poll_count;
  - unsigned int try_count = 0;
  - int retry_flag;
int ret = 0;
unsigned int idx;
/* These sizes look to be limited by the FX2 firmware implementation */
  @@ -213,8 +211,6 @@ static int pvr2_encoder_cmd(void *ctxt,
break;
}
   
  - retry_flag = 0;
  - try_count++;
ret = 0;
wrData[0] = 0;
wrData[1] = cmd;
  @@ -245,11 +241,9 @@ static int pvr2_encoder_cmd(void *ctxt,
}
if (rdData[0]  (poll_count  1000)) continue;
if (!rdData[0]) {
  - retry_flag = !0;
pvr2_trace(
PVR2_TRACE_ERROR_LEGS,
  - Encoder timed out waiting for us
  - ; arranging to retry);
  + Encoder timed out waiting for us);
} else {
pvr2_trace(
PVR2_TRACE_ERROR_LEGS,
  @@ -269,13 +263,6 @@ static int pvr2_encoder_cmd(void *ctxt,
ret = -EBUSY;
break;
}
  - if (retry_flag) {
  - if (try_count  20) continue;
  - pvr2_trace(
  - PVR2_TRACE_ERROR_LEGS,
  - Too many retries...);
  - ret = -EBUSY;
  - }
if (ret) {
del_timer_sync(hdw-encoder_run_timer);
hdw-state_encoder_ok = 0;
 
 
  
  
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 02/14] pvrusb2: fix sparse warning

2013-10-04 Thread Mike Isely

Acked-by: Mike Isely is...@pobox.com

  -Mike

On Fri, 4 Oct 2013, Hans Verkuil wrote:

 From: Hans Verkuil hans.verk...@cisco.com
 
 drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2871:13: warning: symbol 
 'pvr2_hdw_get_detected_std' was not declared. Should it be static?
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 Cc: Mike Isely is...@pobox.com
 ---
  drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c 
 b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
 index c4d51d7..ea05f67 100644
 --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
 +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
 @@ -2868,7 +2868,7 @@ static void pvr2_subdev_set_control(struct pvr2_hdw 
 *hdw, int id,
   pvr2_subdev_set_control(hdw, id, #lab, (hdw)-lab##_val); \
   }
  
 -v4l2_std_id pvr2_hdw_get_detected_std(struct pvr2_hdw *hdw)
 +static v4l2_std_id pvr2_hdw_get_detected_std(struct pvr2_hdw *hdw)
  {
   v4l2_std_id std;
   std = (v4l2_std_id)hdw-std_mask_avail;
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [REVIEWv2 PATCH 07/12] pvrusb2: use v4l2_dev instead of the deprecated parent field.

2013-06-12 Thread Mike Isely

Acked-By: Mike Isely is...@pobox.com

  -Mike

On Wed, 12 Jun 2013, Hans Verkuil wrote:

 From: Hans Verkuil hans.verk...@cisco.com
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 ---
  drivers/media/usb/pvrusb2/pvrusb2-hdw.c  |4 
  drivers/media/usb/pvrusb2/pvrusb2-hdw.h  |4 
  drivers/media/usb/pvrusb2/pvrusb2-v4l2.c |7 ---
  3 files changed, 12 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c 
 b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
 index d329209..c4d51d7 100644
 --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
 +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
 @@ -2704,6 +2704,10 @@ static void pvr2_hdw_remove_usb_stuff(struct pvr2_hdw 
 *hdw)
   pvr2_hdw_render_useless(hdw);
  }
  
 +void pvr2_hdw_set_v4l2_dev(struct pvr2_hdw *hdw, struct video_device *vdev)
 +{
 + vdev-v4l2_dev = hdw-v4l2_dev;
 +}
  
  /* Destroy hardware interaction structure */
  void pvr2_hdw_destroy(struct pvr2_hdw *hdw)
 diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.h 
 b/drivers/media/usb/pvrusb2/pvrusb2-hdw.h
 index 1a135cf..4184707 100644
 --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.h
 +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.h
 @@ -22,6 +22,7 @@
  
  #include linux/usb.h
  #include linux/videodev2.h
 +#include media/v4l2-dev.h
  #include pvrusb2-io.h
  #include pvrusb2-ctrl.h
  
 @@ -138,6 +139,9 @@ const char *pvr2_hdw_get_device_identifier(struct 
 pvr2_hdw *);
  /* Called when hardware has been unplugged */
  void pvr2_hdw_disconnect(struct pvr2_hdw *);
  
 +/* Sets v4l2_dev of a video_device struct */
 +void pvr2_hdw_set_v4l2_dev(struct pvr2_hdw *, struct video_device *);
 +
  /* Get the number of defined controls */
  unsigned int pvr2_hdw_get_ctrl_count(struct pvr2_hdw *);
  
 diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c 
 b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
 index 82f619b..d77069e 100644
 --- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
 +++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
 @@ -31,6 +31,7 @@
  #include linux/videodev2.h
  #include linux/module.h
  #include media/v4l2-dev.h
 +#include media/v4l2-device.h
  #include media/v4l2-common.h
  #include media/v4l2-ioctl.h
  
 @@ -870,8 +871,8 @@ static void pvr2_v4l2_dev_destroy(struct pvr2_v4l2_dev 
 *dip)
  static void pvr2_v4l2_dev_disassociate_parent(struct pvr2_v4l2_dev *dip)
  {
   if (!dip) return;
 - if (!dip-devbase.parent) return;
 - dip-devbase.parent = NULL;
 + if (!dip-devbase.v4l2_dev-dev) return;
 + dip-devbase.v4l2_dev-dev = NULL;
   device_move(dip-devbase.dev, NULL, DPM_ORDER_NONE);
  }
  
 @@ -1321,7 +1322,7 @@ static void pvr2_v4l2_dev_init(struct pvr2_v4l2_dev 
 *dip,
   if (nr_ptr  (unit_number = 0)  (unit_number  PVR_NUM)) {
   mindevnum = nr_ptr[unit_number];
   }
 - dip-devbase.parent = usbdev-dev;
 + pvr2_hdw_set_v4l2_dev(hdw, dip-devbase);
   if ((video_register_device(dip-devbase,
  dip-v4l_type, mindevnum)  0) 
   (video_register_device(dip-devbase,
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 33/68] [media] pvrusb2: get rid of warning: no previous prototype

2012-10-27 Thread Mike Isely

Acked-By: Mike Isely is...@pobox.com

On Sat, 27 Oct 2012, Mauro Carvalho Chehab wrote:

 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c:199:5: warning: no previous 
 prototype for 'pvr2_s_std' [-Wmissing-prototypes]
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c:368:5: warning: no previous 
 prototype for 'pvr2_s_frequency' [-Wmissing-prototypes]
 
 Cc: Mike Isely is...@pobox.com
 Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
 ---
  drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c 
 b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
 index db249ca..6930676 100644
 --- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
 +++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
 @@ -196,7 +196,7 @@ static int pvr2_g_std(struct file *file, void *priv, 
 v4l2_std_id *std)
   return ret;
  }
  
 -int pvr2_s_std(struct file *file, void *priv, v4l2_std_id *std)
 +static int pvr2_s_std(struct file *file, void *priv, v4l2_std_id *std)
  {
   struct pvr2_v4l2_fh *fh = file-private_data;
   struct pvr2_hdw *hdw = fh-channel.mc_head-hdw;
 @@ -365,7 +365,7 @@ static int pvr2_s_tuner(struct file *file, void *priv, 
 struct v4l2_tuner *vt)
   vt-audmode);
  }
  
 -int pvr2_s_frequency(struct file *file, void *priv, struct v4l2_frequency 
 *vf)
 +static int pvr2_s_frequency(struct file *file, void *priv, struct 
 v4l2_frequency *vf)
  {
   struct pvr2_v4l2_fh *fh = file-private_data;
   struct pvr2_hdw *hdw = fh-channel.mc_head-hdw;
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] pvr2: fix minor storage

2012-10-25 Thread Mike Isely

Completely agree!  Thanks for spotting that one.

Signed-off-by: Mike Isely is...@pobox.com

  -Mike


On Thu, 25 Oct 2012, Alan Cox wrote:

 From: Alan Cox a...@linux.intel.com
 
 This should have break statements in it.
 
 Signed-off-by: Alan Cox a...@linux.intel.com
 ---
 
  drivers/media/usb/pvrusb2/pvrusb2-hdw.c |6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c 
 b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
 index fb828ba..299751a 100644
 --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
 +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
 @@ -3563,9 +3563,9 @@ void pvr2_hdw_v4l_store_minor_number(struct pvr2_hdw 
 *hdw,
enum pvr2_v4l_type index,int v)
  {
   switch (index) {
 - case pvr2_v4l_type_video: hdw-v4l_minor_number_video = v;
 - case pvr2_v4l_type_vbi: hdw-v4l_minor_number_vbi = v;
 - case pvr2_v4l_type_radio: hdw-v4l_minor_number_radio = v;
 + case pvr2_v4l_type_video: hdw-v4l_minor_number_video = v;break;
 + case pvr2_v4l_type_vbi: hdw-v4l_minor_number_vbi = v;break;
 + case pvr2_v4l_type_radio: hdw-v4l_minor_number_radio = v;break;
   default: break;
   }
  }
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: Core + Radio profile

2012-08-22 Thread Mike Isely
On Wed, 22 Aug 2012, Mauro Carvalho Chehab wrote:

 Em 22-08-2012 07:11, Hans Verkuil escreveu:
  I've added some more core profile requirements.
 
 
  Streaming I/O is not supported by radio nodes.
 
   Hmm... pvrusb2/ivtv? Ok, it makes sense to move it to use the alsa
 mpeg API there. If we're enforcing it, we should deprecate the current way
 there, and make it use ALSA.

I am unaware of any ALSA MPEG API.  It's entirely likely that this is 
because I haven't been paying attention.  Nevertheless, can you please 
point me at any documentation on this so I can get up to speed?

Currently the pvrusb2 driver does not attempt to perform any processing 
or filtering of the data stream, so radio data is just the same mpeg 
stream as video (but without any real embedded video data).  If I have 
to get into the business of processing the MPEG data in order to adhere 
to this proposal, then that will be a very big deal for this driver.

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] pvrusb2: Declare MODULE_FIRMWARE usage

2012-07-26 Thread Mike Isely

Acked-By: Mike Isely is...@pobox.com

  -Mike


On Thu, 26 Jul 2012, Tim Gardner wrote:

 Cc: Mike Isely is...@pobox.com
 Cc: Mauro Carvalho Chehab mche...@infradead.org
 Cc: linux-media@vger.kernel.org
 Signed-off-by: Tim Gardner tim.gard...@canonical.com
 ---
  drivers/media/video/pvrusb2/pvrusb2-devattr.c |   17 -
  1 file changed, 12 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-devattr.c 
 b/drivers/media/video/pvrusb2/pvrusb2-devattr.c
 index d8c8982..adc501d3 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-devattr.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-devattr.c
 @@ -54,8 +54,9 @@ static const struct pvr2_device_client_desc 
 pvr2_cli_29xxx[] = {
   { .module_id = PVR2_CLIENT_ID_DEMOD },
  };
  
 +#define PVR2_FIRMWARE_29xxx v4l-pvrusb2-29xxx-01.fw
  static const char *pvr2_fw1_names_29xxx[] = {
 - v4l-pvrusb2-29xxx-01.fw,
 + PVR2_FIRMWARE_29xxx,
  };
  
  static const struct pvr2_device_desc pvr2_device_29xxx = {
 @@ -87,8 +88,9 @@ static const struct pvr2_device_client_desc 
 pvr2_cli_24xxx[] = {
   { .module_id = PVR2_CLIENT_ID_DEMOD },
  };
  
 +#define PVR2_FIRMWARE_24xxx v4l-pvrusb2-24xxx-01.fw
  static const char *pvr2_fw1_names_24xxx[] = {
 - v4l-pvrusb2-24xxx-01.fw,
 + PVR2_FIRMWARE_24xxx,
  };
  
  static const struct pvr2_device_desc pvr2_device_24xxx = {
 @@ -369,8 +371,9 @@ static const struct pvr2_device_client_desc 
 pvr2_cli_73xxx[] = {
 .i2c_address_list = \x42},
  };
  
 +#define PVR2_FIRMWARE_73xxx v4l-pvrusb2-73xxx-01.fw
  static const char *pvr2_fw1_names_73xxx[] = {
 - v4l-pvrusb2-73xxx-01.fw,
 + PVR2_FIRMWARE_73xxx,
  };
  
  static const struct pvr2_device_desc pvr2_device_73xxx = {
 @@ -475,8 +478,9 @@ static const struct pvr2_dvb_props pvr2_751xx_dvb_props = 
 {
  };
  #endif
  
 +#define PVR2_FIRMWARE_75xxx v4l-pvrusb2-73xxx-01.fw
  static const char *pvr2_fw1_names_75xxx[] = {
 - v4l-pvrusb2-73xxx-01.fw,
 + PVR2_FIRMWARE_75xxx,
  };
  
  static const struct pvr2_device_desc pvr2_device_750xx = {
 @@ -556,7 +560,10 @@ struct usb_device_id pvr2_device_table[] = {
  };
  
  MODULE_DEVICE_TABLE(usb, pvr2_device_table);
 -
 +MODULE_FIRMWARE(PVR2_FIRMWARE_29xxx);
 +MODULE_FIRMWARE(PVR2_FIRMWARE_24xxx);
 +MODULE_FIRMWARE(PVR2_FIRMWARE_73xxx);
 +MODULE_FIRMWARE(PVR2_FIRMWARE_75xxx);
  
  /*
Stuff for Emacs to see, in order to encourage consistent editing style:
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[GIT PULL FOR 3.5] pvrusb2 driver updates

2012-05-04 Thread Mike Isely

Mauro:

Please pull - this includes a long-awaited change courtesy of Hans 
Verkuil which finally transitions the driver to video_ioctl2.

  -Mike Isely


The following changes since commit a1ac5dc28d2b4ca78e183229f7c595ffd725241c:

  [media] gspca - sn9c20x: Change the exposure setting of Omnivision sensors 
(2012-05-03 15:29:56 -0300)

are available in the git repository at:
  git://git.linuxtv.org/mcisely/pvrusb2-20120504.git pvrusb2-merge-20120504

Hans Verkuil (1):
  pvrusb2: convert to video_ioctl2

Mike Isely (9):
  pvrusb2: Stop statically initializing reserved struct fields to zero
  pvrusb2: Clean up pvr2_hdw_get_detected_std()
  pvrusb2: Implement querystd for videodev_ioctl2
  pvrusb2: Transform video standard detection result into read-only control 
ID
  pvrusb2: Fix truncated video standard names (trivial)
  pvrusb2: Base available video standards on what hardware supports
  pvrusb2: Trivial tweak to get rid of some redundant dereferences
  pvrusb2: Get rid of obsolete code for video standard enumeration
  pvrusb2: For querystd, start with list of hardware-supported standards

 drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h |6 +-
 drivers/media/video/pvrusb2/pvrusb2-hdw.c  |  193 +---
 drivers/media/video/pvrusb2/pvrusb2-hdw.h  |9 +-
 drivers/media/video/pvrusb2/pvrusb2-v4l2.c | 1343 ++--
 4 files changed, 735 insertions(+), 816 deletions(-)

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 5/8] [media] pvrusb2: initialize standards mask before detecting standard

2011-10-05 Thread Mike Isely

Mauro:

With the line you've just added, then the  = arg assignment in the 
immediate prior line is effectively dead code.  Try this instead:

case VIDIOC_QUERYSTD:
{
-   v4l2_std_id *std = arg;
+   v4l2_std_id *std = V4L2_STD_ALL;
ret = pvr2_hdw_get_detected_std(hdw, std);
break;
}

  -Mike


On Tue, 4 Oct 2011, Mauro Carvalho Chehab wrote:

 Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
 ---
  drivers/media/video/pvrusb2/pvrusb2-v4l2.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-v4l2.c 
 b/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
 index 0d029da..ce7ac45 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
 @@ -230,6 +230,7 @@ static long pvr2_v4l2_do_ioctl(struct file *file, 
 unsigned int cmd, void *arg)
   case VIDIOC_QUERYSTD:
   {
   v4l2_std_id *std = arg;
 + *std = V4L2_STD_ALL;
   ret = pvr2_hdw_get_detected_std(hdw, std);
   break;
   }
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 5/8] [media] pvrusb2: initialize standards mask before detecting standard

2011-10-05 Thread Mike Isely


On Wed, 5 Oct 2011, Mauro Carvalho Chehab wrote:

 Em 05-10-2011 11:00, Mike Isely escreveu:
  
  Mauro:
  
  With the line you've just added, then the  = arg assignment in the
  immediate prior line is effectively dead code.  Try this instead:
 
 Look better:
 
 v4l2_std_id *std = arg;
   + *std = V4L2_STD_ALL;
 
 The above code is creating a pointer 'std' of the type 'v4l2_std_id', and
 initializing the pointer with the void *arg.

Oh yeah, you're absolutely right.  I got visually tricked by the well 
known confusing C initialization syntax when dealing with pointers!  
I've got to not respond to stuff like this in the morning until I've 
finished waking up.  Duh...

 
 Then, it is doing an indirect reference to the pointer, filling its
 contents with V4L2_STD_ALL value.
 
 The code above is sane (and, btw, it works). After those patches, the
 detection code will detect PAL/M or NTSC/M depending on the channel I
 tune here (my cable operator broadcasts some channels with one format,
 and others with the other one). Before this patch and the msp3400, it
 would return a mask with PAL/M and PAL/60 or a mask with all NTSC/M formats.

Regarding your first version of the patch:

Acked-By: Mike Isely is...@pobox.com

  -Mike

 
 Regards,
 Mauro.
 
  
  case VIDIOC_QUERYSTD:
  {
  -   v4l2_std_id *std = arg;
  +   v4l2_std_id *std = V4L2_STD_ALL;
  ret = pvr2_hdw_get_detected_std(hdw, std);
  break;
  }
  
 -Mike
  
  
  On Tue, 4 Oct 2011, Mauro Carvalho Chehab wrote:
  
   Signed-off-by: Mauro Carvalho Chehabmche...@redhat.com
   ---
 drivers/media/video/pvrusb2/pvrusb2-v4l2.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
   
   diff --git a/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
   b/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
   index 0d029da..ce7ac45 100644
   --- a/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
   +++ b/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
   @@ -230,6 +230,7 @@ static long pvr2_v4l2_do_ioctl(struct file *file,
   unsigned int cmd, void *arg)
 case VIDIOC_QUERYSTD:
 {
 v4l2_std_id *std = arg;
   + *std = V4L2_STD_ALL;
 ret = pvr2_hdw_get_detected_std(hdw, std);
 break;
 }
   
  
 
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] [media] pvrusb2: implement VIDIOC_QUERYSTD

2011-10-03 Thread Mike Isely

Acked-By: Mike Isely is...@pobox.com

  -Mike

On Mon, 3 Oct 2011, Mauro Carvalho Chehab wrote:

 Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
 ---
  drivers/media/video/pvrusb2/pvrusb2-hdw.c  |7 +++
  drivers/media/video/pvrusb2/pvrusb2-hdw.h  |3 +++
  drivers/media/video/pvrusb2/pvrusb2-v4l2.c |7 +++
  3 files changed, 17 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.c 
 b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
 index e98d382..5a6f24d 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-hdw.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
 @@ -2993,6 +2993,13 @@ static void pvr2_subdev_set_control(struct pvr2_hdw 
 *hdw, int id,
   pvr2_subdev_set_control(hdw, id, #lab, (hdw)-lab##_val); \
   }
  
 +int pvr2_hdw_get_detected_std(struct pvr2_hdw *hdw, v4l2_std_id *std)
 +{
 + v4l2_device_call_all(hdw-v4l2_dev, 0,
 +  video, querystd, std);
 + return 0;
 +}
 +
  /* Execute whatever commands are required to update the state of all the
 sub-devices so that they match our current control values. */
  static void pvr2_subdev_update(struct pvr2_hdw *hdw)
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.h 
 b/drivers/media/video/pvrusb2/pvrusb2-hdw.h
 index d7753ae..6654658 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-hdw.h
 +++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.h
 @@ -214,6 +214,9 @@ struct pvr2_stream *pvr2_hdw_get_video_stream(struct 
 pvr2_hdw *);
  int pvr2_hdw_get_stdenum_value(struct pvr2_hdw *hdw,struct v4l2_standard 
 *std,
  unsigned int idx);
  
 +/* Get the detected video standard */
 +int pvr2_hdw_get_detected_std(struct pvr2_hdw *hdw, v4l2_std_id *std);
 +
  /* Enable / disable retrieval of CPU firmware or prom contents.  This must
 be enabled before pvr2_hdw_cpufw_get() will function.  Note that doing
 this may prevent the device from running (and leaving this mode may
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-v4l2.c 
 b/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
 index e27f8ab..0d029da 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
 @@ -227,6 +227,13 @@ static long pvr2_v4l2_do_ioctl(struct file *file, 
 unsigned int cmd, void *arg)
   break;
   }
  
 + case VIDIOC_QUERYSTD:
 + {
 + v4l2_std_id *std = arg;
 + ret = pvr2_hdw_get_detected_std(hdw, std);
 + break;
 + }
 +
   case VIDIOC_G_STD:
   {
   int val = 0;
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFCv6 PATCH 04/10] pvrusb2: fix g/s_tuner support.

2011-06-19 Thread Mike Isely

I understand that this patch would not have been need had the pvrusb2 
driver been using videodev_ioctl2.  This is a situation that I'm going 
to (finally) remedy ASAP.  In the mean time...

Acked-By: Mike Isely is...@pobox.com

  -Mike


On Tue, 14 Jun 2011, Hans Verkuil wrote:

 From: Hans Verkuil hans.verk...@cisco.com
 
 The tuner-core subdev requires that the type field of v4l2_tuner is
 filled in correctly. This is done in v4l2-ioctl.c, but pvrusb2 doesn't
 use that yet, so we have to do it manually based on whether the current
 input is radio or not.
 
 Tested with my pvrusb2.
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 ---
  drivers/media/video/pvrusb2/pvrusb2-hdw.c |4 
  1 files changed, 4 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.c 
 b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
 index 9d0dd08..e98d382 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-hdw.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
 @@ -3046,6 +3046,8 @@ static void pvr2_subdev_update(struct pvr2_hdw *hdw)
   if (hdw-input_dirty || hdw-audiomode_dirty || hdw-force_dirty) {
   struct v4l2_tuner vt;
   memset(vt, 0, sizeof(vt));
 + vt.type = (hdw-input_val == PVR2_CVAL_INPUT_RADIO) ?
 + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
   vt.audmode = hdw-audiomode_val;
   v4l2_device_call_all(hdw-v4l2_dev, 0, tuner, s_tuner, vt);
   }
 @@ -5171,6 +5173,8 @@ void pvr2_hdw_status_poll(struct pvr2_hdw *hdw)
  {
   struct v4l2_tuner *vtp = hdw-tuner_signal_info;
   memset(vtp, 0, sizeof(*vtp));
 + vtp-type = (hdw-input_val == PVR2_CVAL_INPUT_RADIO) ?
 + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
   hdw-tuner_signal_stale = 0;
   /* Note: There apparently is no replacement for VIDIOC_CROPCAP
  using v4l2-subdev - therefore we can't support that AT ALL right
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/6] [media] pvrusb2: check for allocation failures

2011-03-26 Thread Mike Isely
I'll look at the surrounding code and see what makes sense there. Having 
an error leg for allocation failures is a useful thing.


-Mike


Dan Carpenter wrote:

On Fri, Mar 25, 2011 at 11:33:36PM -0500, Mike Isely wrote:
  

Acked-By: Mike Isely is...@pobox.com




I'd need to reformat this one to get it to apply... :/  It doesn't
actually fix the bug so it's not worth it.

regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  



--

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/6] [media] pvrusb2: white space changes

2011-03-25 Thread Mike Isely

I vehemently object to this scale of disruption to the pvrusb2 driver 
source code purely to move around a bunch of braces and whitespace.  
ESPECIALLY the massive ridiculous changes having to do with if-statement 
syntax!

Nacked-By: Mike Isely is...@pobox.com


On Sat, 26 Mar 2011, Dan Carpenter wrote:

 * Broke up if statements so that the condition and the body are on
   separate lines.
 * Added spaces around commas and other operator characters.
 * Removed extra blank lines.
 * Added blank lines after declarations.
 * Changed C99 comments into kernel style.
 * Fixed checkpatch complaints where { char was on its own line but it
   wasn't the start of a function.
 
 Signed-off-by: Dan Carpenter erro...@gmail.com
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-std.c 
 b/drivers/media/video/pvrusb2/pvrusb2-std.c
 index ca9f83a..a5d4867 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-std.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-std.c
 @@ -28,39 +28,38 @@ struct std_name {
   v4l2_std_id id;
  };
  
 -
  #define CSTD_PAL \
 - (V4L2_STD_PAL_B| \
 -  V4L2_STD_PAL_B1| \
 -  V4L2_STD_PAL_G| \
 -  V4L2_STD_PAL_H| \
 -  V4L2_STD_PAL_I| \
 -  V4L2_STD_PAL_D| \
 -  V4L2_STD_PAL_D1| \
 -  V4L2_STD_PAL_K| \
 -  V4L2_STD_PAL_M| \
 -  V4L2_STD_PAL_N| \
 -  V4L2_STD_PAL_Nc| \
 + (V4L2_STD_PAL_B  | \
 +  V4L2_STD_PAL_B1 | \
 +  V4L2_STD_PAL_G  | \
 +  V4L2_STD_PAL_H  | \
 +  V4L2_STD_PAL_I  | \
 +  V4L2_STD_PAL_D  | \
 +  V4L2_STD_PAL_D1 | \
 +  V4L2_STD_PAL_K  | \
 +  V4L2_STD_PAL_M  | \
 +  V4L2_STD_PAL_N  | \
 +  V4L2_STD_PAL_Nc | \
V4L2_STD_PAL_60)
  
  #define CSTD_NTSC \
 - (V4L2_STD_NTSC_M| \
 -  V4L2_STD_NTSC_M_JP| \
 -  V4L2_STD_NTSC_M_KR| \
 + (V4L2_STD_NTSC_M| \
 +  V4L2_STD_NTSC_M_JP | \
 +  V4L2_STD_NTSC_M_KR | \
V4L2_STD_NTSC_443)
  
  #define CSTD_ATSC \
 - (V4L2_STD_ATSC_8_VSB| \
 + (V4L2_STD_ATSC_8_VSB | \
V4L2_STD_ATSC_16_VSB)
  
  #define CSTD_SECAM \
 - (V4L2_STD_SECAM_B| \
 -  V4L2_STD_SECAM_D| \
 -  V4L2_STD_SECAM_G| \
 -  V4L2_STD_SECAM_H| \
 -  V4L2_STD_SECAM_K| \
 -  V4L2_STD_SECAM_K1| \
 -  V4L2_STD_SECAM_L| \
 + (V4L2_STD_SECAM_B  | \
 +  V4L2_STD_SECAM_D  | \
 +  V4L2_STD_SECAM_G  | \
 +  V4L2_STD_SECAM_H  | \
 +  V4L2_STD_SECAM_K  | \
 +  V4L2_STD_SECAM_K1 | \
 +  V4L2_STD_SECAM_L  | \
V4L2_STD_SECAM_LC)
  
  #define TSTD_B   (V4L2_STD_PAL_B|V4L2_STD_SECAM_B)
 @@ -82,39 +81,40 @@ struct std_name {
  
  /* Mapping of standard bits to color system */
  static const struct std_name std_groups[] = {
 - {PAL,CSTD_PAL},
 - {NTSC,CSTD_NTSC},
 - {SECAM,CSTD_SECAM},
 - {ATSC,CSTD_ATSC},
 + {PAL,   CSTD_PAL},
 + {NTSC,  CSTD_NTSC},
 + {SECAM, CSTD_SECAM},
 + {ATSC,  CSTD_ATSC},
  };
  
  /* Mapping of standard bits to modulation system */
  static const struct std_name std_items[] = {
 - {B,TSTD_B},
 - {B1,TSTD_B1},
 - {D,TSTD_D},
 - {D1,TSTD_D1},
 - {G,TSTD_G},
 - {H,TSTD_H},
 - {I,TSTD_I},
 - {K,TSTD_K},
 - {K1,TSTD_K1},
 - {L,TSTD_L},
 - {LC,V4L2_STD_SECAM_LC},
 - {M,TSTD_M},
 - {Mj,V4L2_STD_NTSC_M_JP},
 - {443,V4L2_STD_NTSC_443},
 - {Mk,V4L2_STD_NTSC_M_KR},
 - {N,TSTD_N},
 - {Nc,TSTD_Nc},
 - {60,TSTD_60},
 - {8VSB,V4L2_STD_ATSC_8_VSB},
 - {16VSB,V4L2_STD_ATSC_16_VSB},
 + {B, TSTD_B},
 + {B1,TSTD_B1},
 + {D, TSTD_D},
 + {D1,TSTD_D1},
 + {G, TSTD_G},
 + {H, TSTD_H},
 + {I, TSTD_I},
 + {K, TSTD_K},
 + {K1,TSTD_K1},
 + {L, TSTD_L},
 + {LC,V4L2_STD_SECAM_LC},
 + {M, TSTD_M},
 + {Mj,V4L2_STD_NTSC_M_JP},
 + {443,   V4L2_STD_NTSC_443},
 + {Mk,V4L2_STD_NTSC_M_KR},
 + {N, TSTD_N},
 + {Nc,TSTD_Nc},
 + {60,TSTD_60},
 + {8VSB,  V4L2_STD_ATSC_8_VSB},
 + {16VSB, V4L2_STD_ATSC_16_VSB},
  };
  
 -
 -// Search an array of std_name structures and return a pointer to the
 -// element with the matching name.
 +/*
 + * Search an array of std_name structures and return a pointer to the
 + * element with the matching name.
 + */
  static const struct std_name *find_std_name(const struct std_name *arrPtr,
   unsigned int arrSize,
   const char *bufPtr,
 @@ -122,16 +122,18 @@ static const struct std_name *find_std_name(const 
 struct std_name *arrPtr,
  {
   unsigned int idx;
   const struct std_name *p;
 +
   for (idx = 0; idx  arrSize; idx++) {
   p = arrPtr + idx;
 - if (strlen(p-name) != bufSize) continue;
 - if (!memcmp(bufPtr,p-name,bufSize)) return p;
 + if (strlen(p-name) != bufSize)
 + continue;
 + if (!memcmp(bufPtr, p-name, bufSize))
 + return p

Re: [PATCH 2/6] [media] pvrusb2: fix remaining checkpatch.pl complaints

2011-03-25 Thread Mike Isely

I am OK with the #include change, but NOT the if-statement change.  But 
since it's bundled into one patch...

Nacked-By: Mike Isely is...@pobox.com


On Sat, 26 Mar 2011, Dan Carpenter wrote:

 * Include linux/string.h instead of asm/string.h.
 * Remove unneeded curly braces.
 
 Signed-off-by: Dan Carpenter erro...@gmail.com
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-std.c 
 b/drivers/media/video/pvrusb2/pvrusb2-std.c
 index a5d4867..370a9ab 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-std.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-std.c
 @@ -20,7 +20,7 @@
  
  #include pvrusb2-std.h
  #include pvrusb2-debug.h
 -#include asm/string.h
 +#include linux/string.h
  #include linux/slab.h
  
  struct std_name {
 @@ -294,9 +294,8 @@ static struct v4l2_standard *match_std(v4l2_std_id id)
   unsigned int idx;
  
   for (idx = 0; idx  generic_standards_cnt; idx++) {
 - if (generic_standards[idx].id  id) {
 + if (generic_standards[idx].id  id)
   return generic_standards + idx;
 - }
   }
   return NULL;
  }
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/6] [media] pvrusb2: check for allocation failures

2011-03-25 Thread Mike Isely

Acked-By: Mike Isely is...@pobox.com

On Sat, 26 Mar 2011, Dan Carpenter wrote:

 This function returns NULL on failure so lets do that if kzalloc()
 fails.  There is a separate problem that the caller for this function
 doesn't check for errors...
 
 Signed-off-by: Dan Carpenter erro...@gmail.com
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-std.c 
 b/drivers/media/video/pvrusb2/pvrusb2-std.c
 index 370a9ab..b214f77 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-std.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-std.c
 @@ -388,6 +388,9 @@ struct v4l2_standard *pvr2_std_create_enum(unsigned int 
 *countptr,
  
   stddefs = kzalloc(sizeof(struct v4l2_standard) * std_cnt,
 GFP_KERNEL);
 + if (!stddefs)
 + return NULL;
 +
   for (idx = 0; idx  std_cnt; idx++)
   stddefs[idx].index = idx;
  
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/6] [media] pvrusb2: fix camel case variables

2011-03-25 Thread Mike Isely

It not worth this scale of source code disruption to the source code 
just to rename a bunch of variables.  I'm sorry, but...

Nacked-By: Mike Isely is...@pobox.com


On Sat, 26 Mar 2011, Dan Carpenter wrote:

 This patch renames some variables to bring them more in line with
 kernel CodingStyle.
 
 arrPtr  = arr
 arrSize = arr_size
 bufPtr  = buf
 bufSize = buf_size
 
 Signed-off-by: Dan Carpenter erro...@gmail.com
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-std.c 
 b/drivers/media/video/pvrusb2/pvrusb2-std.c
 index b214f77..d5a679f 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-std.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-std.c
 @@ -115,26 +115,26 @@ static const struct std_name std_items[] = {
   * Search an array of std_name structures and return a pointer to the
   * element with the matching name.
   */
 -static const struct std_name *find_std_name(const struct std_name *arrPtr,
 - unsigned int arrSize,
 - const char *bufPtr,
 - unsigned int bufSize)
 +static const struct std_name *find_std_name(const struct std_name *arr,
 + unsigned int arr_size,
 + const char *buf,
 + unsigned int buf_size)
  {
   unsigned int idx;
   const struct std_name *p;
  
 - for (idx = 0; idx  arrSize; idx++) {
 - p = arrPtr + idx;
 - if (strlen(p-name) != bufSize)
 + for (idx = 0; idx  arr_size; idx++) {
 + p = arr + idx;
 + if (strlen(p-name) != buf_size)
   continue;
 - if (!memcmp(bufPtr, p-name, bufSize))
 + if (!memcmp(buf, p-name, buf_size))
   return p;
   }
   return NULL;
  }
  
 -int pvr2_std_str_to_id(v4l2_std_id *idPtr, const char *bufPtr,
 -unsigned int bufSize)
 +int pvr2_std_str_to_id(v4l2_std_id *idPtr, const char *buf,
 +unsigned int buf_size)
  {
   v4l2_std_id id = 0;
   v4l2_std_id cmsk = 0;
 @@ -144,27 +144,27 @@ int pvr2_std_str_to_id(v4l2_std_id *idPtr, const char 
 *bufPtr,
   char ch;
   const struct std_name *sp;
  
 - while (bufSize) {
 + while (buf_size) {
   if (!mMode) {
   cnt = 0;
 - while ((cnt  bufSize)  (bufPtr[cnt] != '-'))
 + while ((cnt  buf_size)  (buf[cnt] != '-'))
   cnt++;
 - if (cnt = bufSize)
 + if (cnt = buf_size)
   return 0; /* No more characters */
   sp = find_std_name(std_groups, ARRAY_SIZE(std_groups),
 -bufPtr, cnt);
 +buf, cnt);
   if (!sp)
   return 0; /* Illegal color system name */
   cnt++;
 - bufPtr += cnt;
 - bufSize -= cnt;
 + buf += cnt;
 + buf_size -= cnt;
   mMode = !0;
   cmsk = sp-id;
   continue;
   }
   cnt = 0;
 - while (cnt  bufSize) {
 - ch = bufPtr[cnt];
 + while (cnt  buf_size) {
 + ch = buf[cnt];
   if (ch == ';') {
   mMode = 0;
   break;
 @@ -174,7 +174,7 @@ int pvr2_std_str_to_id(v4l2_std_id *idPtr, const char 
 *bufPtr,
   cnt++;
   }
   sp = find_std_name(std_items, ARRAY_SIZE(std_items),
 -bufPtr, cnt);
 +buf, cnt);
   if (!sp)
   return 0; /* Illegal modulation system ID */
   t = sp-id  cmsk;
 @@ -182,10 +182,10 @@ int pvr2_std_str_to_id(v4l2_std_id *idPtr, const char 
 *bufPtr,
   return 0; /* Specific color + modulation system
illegal */
   id |= t;
 - if (cnt  bufSize)
 + if (cnt  buf_size)
   cnt++;
 - bufPtr += cnt;
 - bufSize -= cnt;
 + buf += cnt;
 + buf_size -= cnt;
   }
  
   if (idPtr)
 @@ -193,7 +193,7 @@ int pvr2_std_str_to_id(v4l2_std_id *idPtr, const char 
 *bufPtr,
   return !0;
  }
  
 -unsigned int pvr2_std_id_to_str(char *bufPtr, unsigned int bufSize,
 +unsigned int pvr2_std_id_to_str(char *buf, unsigned int buf_size,
   v4l2_std_id id)
  {
   unsigned int idx1, idx2;
 @@ -212,26 +212,26 @@ unsigned int pvr2_std_id_to_str(char *bufPtr, unsigned 
 int bufSize,
   continue

Re: [PATCH 5/5] [media] pvrusb2: delete generic_standards_cnt

2011-03-25 Thread Mike Isely

Are you actually serious about this?  Well it's a small change...

Acked-By: Mike Isely is...@pobox.com


On Sat, 26 Mar 2011, Dan Carpenter wrote:

 The generic_standards_cnt define is only used in one place and it's
 more readable to just call ARRAY_SIZE(generic_standards) directly.
 
 Signed-off-by: Dan Carpenter erro...@gmail.com
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-std.c 
 b/drivers/media/video/pvrusb2/pvrusb2-std.c
 index d5a679f..9bebc08 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-std.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-std.c
 @@ -287,13 +287,11 @@ static struct v4l2_standard generic_standards[] = {
   }
  };
  
 -#define generic_standards_cnt ARRAY_SIZE(generic_standards)
 -
  static struct v4l2_standard *match_std(v4l2_std_id id)
  {
   unsigned int idx;
  
 - for (idx = 0; idx  generic_standards_cnt; idx++) {
 + for (idx = 0; idx  ARRAY_SIZE(generic_standards); idx++) {
   if (generic_standards[idx].id  id)
   return generic_standards + idx;
   }
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/6] [media] pvrusb2: replace !0 with 1

2011-03-25 Thread Mike Isely

That's an opinion which I as the driver author disagree with.  Strongly.  
How hard is it to read not false?

Nacked-By: Mike Isely is...@pobox.com


On Sat, 26 Mar 2011, Dan Carpenter wrote:

 Using !0 is less readable than just saying 1.
 
 Signed-off-by: Dan Carpenter erro...@gmail.com
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-std.c 
 b/drivers/media/video/pvrusb2/pvrusb2-std.c
 index 9bebc08..ca4f67b 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-std.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-std.c
 @@ -158,7 +158,7 @@ int pvr2_std_str_to_id(v4l2_std_id *idPtr, const char 
 *buf,
   cnt++;
   buf += cnt;
   buf_size -= cnt;
 - mMode = !0;
 + mMode = 1;
   cmsk = sp-id;
   continue;
   }
 @@ -190,7 +190,7 @@ int pvr2_std_str_to_id(v4l2_std_id *idPtr, const char 
 *buf,
  
   if (idPtr)
   *idPtr = id;
 - return !0;
 + return 1;
  }
  
  unsigned int pvr2_std_id_to_str(char *buf, unsigned int buf_size,
 @@ -217,10 +217,10 @@ unsigned int pvr2_std_id_to_str(char *buf, unsigned int 
 buf_size,
   buf_size -= c2;
   buf += c2;
   }
 - cfl = !0;
 + cfl = 1;
   c2 = scnprintf(buf, buf_size,
  %s-, gp-name);
 - gfl = !0;
 + gfl = 1;
   } else {
   c2 = scnprintf(buf, buf_size, /);
   }
 @@ -315,7 +315,7 @@ static int pvr2_std_fill(struct v4l2_standard *std, 
 v4l2_std_id id)
   std-name[bcnt] = 0;
   pvr2_trace(PVR2_TRACE_STD, Set up standard idx=%u name=%s,
  std-index, std-name);
 - return !0;
 + return 1;
  }
  
  /*
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: compilation warnings/errors

2011-03-13 Thread Mike Isely
On Fri, 11 Mar 2011, Mike Isely wrote:

 On Fri, 11 Mar 2011, Mauro Carvalho Chehab wrote:
 
  /home/mchehab/new_build/v4l/pvrusb2-v4l2.c: In function 
  'pvr2_v4l2_do_ioctl':
  /home/mchehab/new_build/v4l/pvrusb2-v4l2.c:798:23: warning: variable 'cap' 
  set but not used [-Wunused-but-set-variable]
 
 I will look into these.  I'm a little puzzled right now since silly 
 stuff like this usually doesn't get by me.  Unfortunately I can't look 
 at it right this minute.  Expect to hear from me on Sunday.

I looked at these two warnings.  It's dead code that should be removed.  
Amazingly enough, this particular bit of crap has been in the driver, 
unnoticed, since 2008!

I have a pull request coming for more pvrusb2 patches, probably in a few 
more hours, once I'm done testing.  A fix for this will be in the patch 
set.

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[GIT PATCHES FOR 2.6.39] pvrusb2 driver fixes / improvements

2011-03-13 Thread Mike Isely

Mauro:

Please pull the following patches.  Note also that the Implement 
support for Terratec Grabster AV400 is not as big of a change as it 
might sound.  The work to implement that really amounted to just some 
extra table entries, plus those changes have been out in the wild via 
the standalone pvrusb2 driver for quite some time.  Getting that into 
the kernel is long overdue.

  -Mike


The following changes since commit 41f3becb7bef489f9e8c35284dd88a1ff59b190c:
  Hans Verkuil (1):
[media] V4L DocBook: update V4L2 version

are available in the git repository at:

  git://git.linuxtv.org/mcisely/pvrusb2-dev.git pvrusb2-merge-2

Mike Isely (2):
  pvrusb2: Implement support for Terratec Grabster AV400
  pvrusb2: Remove dead code

Xiaochen Wang (1):
  pvrusb2: check kmalloc return value

 drivers/media/video/pvrusb2/pvrusb2-cx2584x-v4l.c |   18 +++
 drivers/media/video/pvrusb2/pvrusb2-devattr.c |   24 +
 drivers/media/video/pvrusb2/pvrusb2-hdw.c |   24 ++---
 drivers/media/video/pvrusb2/pvrusb2-v4l2.c|2 -
 4 files changed, 58 insertions(+), 10 deletions(-)

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: compilation warnings/errors

2011-03-11 Thread Mike Isely
On Fri, 11 Mar 2011, Mauro Carvalho Chehab wrote:

 /home/mchehab/new_build/v4l/pvrusb2-v4l2.c: In function 'pvr2_v4l2_do_ioctl':
 /home/mchehab/new_build/v4l/pvrusb2-v4l2.c:798:23: warning: variable 'cap' 
 set but not used [-Wunused-but-set-variable]

I will look into these.  I'm a little puzzled right now since silly 
stuff like this usually doesn't get by me.  Unfortunately I can't look 
at it right this minute.  Expect to hear from me on Sunday.

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[GIT PULL FOR 2.6.39] pvrusb2 driver

2011-02-20 Thread Mike Isely

Mauro,

[Note: This is my first real attempt at using git to get changes pulled, 
so please let me know if I missed a step.  These changes are all 
relatively minor and have been sitting around for while.  There will be 
more to follow once I'm sure I am doing this process correctly...  
-Mike Isely]


The following changes since commit 5ed4bbdae09d207d141759e013a0f3c24ae76ecc:
  Mauro Carvalho Chehab (1):
[media] tuner-core: Don't touch at standby during tuner_lookup

are available in the git repository at:

  git://git.linuxtv.org/mcisely/pvrusb2-dev.git pvrusb2-merge-1

Mike Isely (5):
  pvrusb2: Handle change of mode before handling change of video standard
  pvrusb2: Minor cosmetic code tweak
  pvrusb2: Fix a few missing default control values, for cropping
  pvrusb2: Minor VBI tweak to help potential CC support
  pvrusb2: Use sysfs_attr_init() where appropriate

Servaas Vandenberghe (1):
  pvrusb2: width and height maximum values.

 drivers/media/video/pvrusb2/pvrusb2-hdw.c   |   60 +++
 drivers/media/video/pvrusb2/pvrusb2-sysfs.c |9 
 2 files changed, 43 insertions(+), 26 deletions(-)

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] ir-kbd-i2c: improve remote behavior with z8 behind usb

2011-01-21 Thread Mike Isely

The pvrusb2 change is obviously trivial so I have no issue with it.

Acked-By: Mike Isely is...@pobox.com

Note the spelling of my last name Isely not Isley.  A good way to 
remember is to think of the normal word wisely and just drop the 
leading w.  (And yes, is...@isely.net and is...@pobox.com lead to the 
same inbox.)

  -Mike


On Thu, 20 Jan 2011, Jarod Wilson wrote:

 Add the same are you ready? i2c_master_send() poll command to
 get_key_haup_xvr found in lirc_zilog, which is apparently seen in
 the Windows driver for the PVR-150 w/a z8. This stabilizes what is
 received from both the HD-PVR and HVR-1950, even with their polling
 intervals at the default of 100, thus the removal of the custom
 260ms polling_interval in pvrusb2-i2c-core.c.
 
 CC: Andy Walls awa...@md.metrocast.net
 CC: Mike Isley is...@isley.net
 Signed-off-by: Jarod Wilson ja...@redhat.com
 ---
  drivers/media/video/ir-kbd-i2c.c   |   13 +
  drivers/media/video/pvrusb2/pvrusb2-i2c-core.c |1 -
  2 files changed, 13 insertions(+), 1 deletions(-)
 
 diff --git a/drivers/media/video/ir-kbd-i2c.c 
 b/drivers/media/video/ir-kbd-i2c.c
 index d2b20ad..a221ad6 100644
 --- a/drivers/media/video/ir-kbd-i2c.c
 +++ b/drivers/media/video/ir-kbd-i2c.c
 @@ -128,6 +128,19 @@ static int get_key_haup(struct IR_i2c *ir, u32 *ir_key, 
 u32 *ir_raw)
  
  static int get_key_haup_xvr(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
  {
 + int ret;
 + unsigned char buf[1] = { 0 };
 +
 + /*
 +  * This is the same apparent are you ready? poll command observed
 +  * watching Windows driver traffic and implemented in lirc_zilog. With
 +  * this added, we get far saner remote behavior with z8 chips on usb
 +  * connected devices, even with the default polling interval of 100ms.
 +  */
 + ret = i2c_master_send(ir-c, buf, 1);
 + if (ret != 1)
 + return (ret  0) ? ret : -EINVAL;
 +
   return get_key_haup_common (ir, ir_key, ir_raw, 6, 3);
  }
  
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c 
 b/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c
 index ccc8849..451ecd4 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c
 @@ -597,7 +597,6 @@ static void pvr2_i2c_register_ir(struct pvr2_hdw *hdw)
   init_data-internal_get_key_func = IR_KBD_GET_KEY_HAUP_XVR;
   init_data-type  = RC_TYPE_RC5;
   init_data-name  = hdw-hdw_desc-description;
 - init_data-polling_interval  = 260; /* ms From lirc_zilog */
   /* IR Receiver */
   info.addr  = 0x71;
   info.platform_data = init_data;
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] ir-kbd-i2c: improve remote behavior with z8 behind usb

2011-01-21 Thread Mike Isely

On Fri, 21 Jan 2011, Mike Isely wrote:

 
 Note the spelling of my last name Isely not Isley.  A good way to 
 remember is to think of the normal word wisely and just drop the 
 leading w.  (And yes, is...@isely.net and is...@pobox.com lead to the 
 same inbox.)

And of course having said that, I then failed to fix the cc list.  
Sorry about that.  D'Oh!!!

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] ir-kbd-i2c: improve remote behavior with z8 behind usb

2011-01-21 Thread Mike Isely
On Fri, 21 Jan 2011, Jarod Wilson wrote:

 On Fri, Jan 21, 2011 at 10:31:42AM -0600, Mike Isely wrote:
  
  The pvrusb2 change is obviously trivial so I have no issue with it.
  
  Acked-By: Mike Isely is...@pobox.com
  
  Note the spelling of my last name Isely not Isley.  A good way to 
  remember is to think of the normal word wisely and just drop the 
  leading w.  (And yes, is...@isely.net and is...@pobox.com lead to the 
  same inbox.)
 
 Thanks Mike, apologies about the misspelling, I didn't catch it until
 after I hit send. I had the Isley Brothers in my head. :)

No problem.  It's a very common mistake.  And no, I'm not related to 
them.  For the record, I generally don't get concerned about the 
spelling of my name, unless the error causes problems (e.g. lost e-mail) 
or the error gets propagated to a large list where it might multiply...

Anyway, sorry also about taking this thread off topic.  Enough said...

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

2011-01-19 Thread Mike Isely
On Wed, 19 Jan 2011, Andy Walls wrote:

 On Wed, 2011-01-19 at 00:20 -0500, Jarod Wilson wrote:
 
 
   Not working with
  lirc_zilog yet, it fails to load, due to an -EIO ret to one of the
  i2c_master_send() calls in lirc_zilog during probe of the TX side. Haven't
  looked into it any more than that yet.
 
 Well technically lirc_zilog doesn't probe anymore.  It relies on the
 bridge driver telling it the truth.

The bridge driver (pvrusb2) still does one probe if it's a 24xxx device: 
It probes 0x71 in order to determine if it is dealing with an MCE 
variant device.  Hauppauge did not change the USB ID when they released 
the 24xxx MCE variant (which has the IR blaster, thus the zilog device).  
The only way to tell the two devices apart is by discovering the 
existence of the zilog device - and the bridge driver needs to do this 
in order to properly disable its emulated I2C IR receiver which would 
otherwise be needed for the non-MCE device.

Based on the discussion here, could that probe be a source of trouble on 
the 24XXX MCE device?

This probing behavior does not happen for HVR-1950 (or HVR-1900) since 
there's only one possible IR configuration there.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

2011-01-19 Thread Mike Isely
On Wed, 19 Jan 2011, Andy Walls wrote:

 On Wed, 2011-01-19 at 13:40 +0100, Jean Delvare wrote:
  On Wed, 19 Jan 2011 07:21:58 -0500, Andy Walls wrote:
   For debugging, you might want to hack in a probe of address 0x70 for
   your HVR-1950, to ensure the Tx side responds in the bridge driver. 
  
  ... keeping in mind that the Z8 doesn't seem to like quick writes, so
  short reads should be used for probing purpose.
  
 
 Noted.  Thanks.
 
 Actually, I think that might be due to the controller in the USB
 connected devices (hdpvr and pvrusb2).  The PCI connected devices, like
 cx18 cards, don't have a problem with the Z8, the default I2C probe
 method, and i2c-algo-bit.
 (A good example of why only bridge drivers should do any required
 probing.)
 
 
 Looking at the code in pvrusb2, it appears to already use a 0 length
 read for a probe:
 
 http://git.linuxtv.org/media_tree.git?a=blob;f=drivers/media/video/pvrusb2/pvrusb2-i2c-core.c;h=ccc884948f34b385563ccbf548c5f80b33cd4f08;hb=refs/heads/staging/for_2.6.38-rc1#l542

Yes but that function is used in two places: (1) If a bus scan is 
performed during initialization (normally it isn't), and (2) it is 
called once ONLY for a 24xxx device (targeting 0x71) in order to 
determine if it is dealing with the MCE variant.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

2011-01-19 Thread Mike Isely

On Wed, 19 Jan 2011, Andy Walls wrote:

   [...]

 
 So the HVR-1950 only has Z8's capable of both Tx and Rx?  No HVR-1950
 has an Rx only Z8 unit?

As far as I know, that is indeed the case - Tx and Rx always.

It's the older 24xxx devices where there could be a difference, and 
that's why the probe only takes place there.  (And in the receive-only 
24xxx configuration it's not a Z8 but something wierd that is only 
accessible through FX2 commands not via I2C, which is why the bridge 
driver emulates the older I2C chip, making IR reception behave like the 
original 29xxx device.)

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

2011-01-19 Thread Mike Isely
On Wed, 19 Jan 2011, Jean Delvare wrote:

 Hi Andy,
 
 On Sun, 16 Jan 2011 14:20:49 -0500, Andy Walls wrote:
  3. I hear from Jean, or whomever really cares about ir-kbd-i2c, if
  adding some new fields for struct IR_i2c_init_data is acceptable.
  Specifically, I'd like to add a transceiver_lock mutex, a transceiver
  reset callback, and a data pointer for that reset callback.
  (Only lirc_zilog would use the reset callback and data pointer.)
 
 Adding fields to these structures is perfectly fine, if you need to do
 that, just go on.
 
 But I'm a little confused about the names you chose,
 ir_transceiver_lock and transceiver_lock. These seem too
 TX-oriented for a mutex that is supposed to synchronize TX and RX
 access. It's particularly surprising for the ir-kbd-i2c driver, which
 as far as I know only supports RX. The name xcvr_lock you used for
 lirc_zilog seems more appropriate.

Actually the term transceiver is normally understood to mean both 
directions.  Otherwise it would be receiver or transmitter.  
Another screwy as aspect of english, and I say this as a native english 
speaker.  The term xcvr is usually just considered to be shorthand for 
transceiver.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

2011-01-19 Thread Mike Isely
On Wed, 19 Jan 2011, Jarod Wilson wrote:

 On Jan 19, 2011, at 8:20 AM, Mike Isely wrote:
 
  This probing behavior does not happen for HVR-1950 (or HVR-1900) since 
  there's only one possible IR configuration there.
 
 Just to be 100% clear, the device I'm poking it is definitely an
 HVR-1950, using ir_scheme PVR2_IR_SCHEME_ZILOG, so the probe bits
 shouldn't coming into play with anything I'm doing. Only just now
 started looking at the pvrusb2 code. Wow, there's a LOT of it. ;)

Yes, and yes :-)

The standalone driver version (which is loaded with ifdef's that allow 
compilation back to 2.6.11) makes the in-kernel driver look small by 
comparison.

There is a fair degree of compartmentalization between the modules.  
The roadmap to what it does for just HVR-1950 you can find by first 
looking at the declarations in pvrusb2-devattr.h and then the 
device-specific configurations in pvrusb2-devattr.c.  From there you can 
usually grep your way around to see how those configuration bits affect 
the rest of the driver.  Most of the really fun stuff is in 
pvrusb2-hdw.c.  Pretty much everything else supports or uses that 
central component.

The actual stuff which deals with I2C is not that large.  Beyond making 
the access possible at all, the driver largely just tries to stay out of 
the way of external logic that needs to reach the bus.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH] pvrusb2: Provide more information about IR units to lirc_zilog and ir-kbd-i2c

2011-01-16 Thread Mike Isely

Andy:

Is the IR_i2c_init_data struct instance required to remain around for 
the life of the driver's registration and is that why you stuffed it 
into the pvr2_hdw struct?  Second: If the first question is yes, then is 
that struct considered to be read-only once it is set up and passed 
through to the i2c device registration function?  In other words, could 
that structure be a const static initialized at compile time, perhaps 
as part of a table definition?

I believe I follow this and it looks good.  The concept looks very 
simple and it's nice that the changes are really only in a single spot.  
Just thinking ahead about making the setup table-driven and not 
requiring data segment storage.

  -Mike


Acked-By: Mike Isely is...@pobox.com

On Sun, 16 Jan 2011, Andy Walls wrote:

 
 When registering an IR Rx device with the I2C subsystem, provide more detailed
 information about the IR device and default remote configuration for the IR
 driver modules.
 
 Also explicitly register any IR Tx device with the I2C subsystem.
 
 Signed-off-by: Andy Walls awa...@md.metrocast.net
 Cc: Mike Isely is...@isely.net
 
 --
 Mike,
 
 As discussed on IRC, this patch will enable lirc_zilog to bind to Zilog
 Z8 IR units on devices supported by pvrusb2.
 
 Please review and comment.  This patch could have been written a number
 of ways.  The way I chose was very direct: hard-coding information in a
 single function.
 
 A git branch with this change, and the updated lirc_zilog, is here:
 
   git://linuxtv.org/awalls/media_tree.git z8-pvrusb2
 
   
 http://git.linuxtv.org/awalls/media_tree.git?a=shortlog;h=refs/heads/z8-pvrusb2
 
 Regards,
 Andy
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h 
 b/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h
 index ac94a8b..305e6aa 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h
 +++ b/drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h
 @@ -40,6 +40,7 @@
  #include pvrusb2-io.h
  #include media/v4l2-device.h
  #include media/cx2341x.h
 +#include media/ir-kbd-i2c.h
  #include pvrusb2-devattr.h
  
  /* Legal values for PVR2_CID_HSM */
 @@ -202,6 +203,7 @@ struct pvr2_hdw {
  
   /* IR related */
   unsigned int ir_scheme_active; /* IR scheme as seen from the outside */
 + struct IR_i2c_init_data ir_init_data; /* params passed to IR modules */
  
   /* Frequency table */
   unsigned int freqTable[FREQTABLE_SIZE];
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c 
 b/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c
 index 7cbe18c..ccc8849 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c
 @@ -19,6 +19,7 @@
   */
  
  #include linux/i2c.h
 +#include media/ir-kbd-i2c.h
  #include pvrusb2-i2c-core.h
  #include pvrusb2-hdw-internal.h
  #include pvrusb2-debug.h
 @@ -48,13 +49,6 @@ module_param_named(disable_autoload_ir_video, 
 pvr2_disable_ir_video,
  MODULE_PARM_DESC(disable_autoload_ir_video,
1=do not try to autoload ir_video IR receiver);
  
 -/* Mapping of IR schemes to known I2C addresses - if any */
 -static const unsigned char ir_video_addresses[] = {
 - [PVR2_IR_SCHEME_ZILOG] = 0x71,
 - [PVR2_IR_SCHEME_29XXX] = 0x18,
 - [PVR2_IR_SCHEME_24XXX] = 0x18,
 -};
 -
  static int pvr2_i2c_write(struct pvr2_hdw *hdw, /* Context */
 u8 i2c_addr,  /* I2C address we're talking to */
 u8 *data, /* Data to write */
 @@ -574,26 +568,56 @@ static void do_i2c_scan(struct pvr2_hdw *hdw)
  static void pvr2_i2c_register_ir(struct pvr2_hdw *hdw)
  {
   struct i2c_board_info info;
 - unsigned char addr = 0;
 + struct IR_i2c_init_data *init_data = hdw-ir_init_data;
   if (pvr2_disable_ir_video) {
   pvr2_trace(PVR2_TRACE_INFO,
  Automatic binding of ir_video has been disabled.);
   return;
   }
 - if (hdw-ir_scheme_active  ARRAY_SIZE(ir_video_addresses)) {
 - addr = ir_video_addresses[hdw-ir_scheme_active];
 - }
 - if (!addr) {
 + memset(info, 0, sizeof(struct i2c_board_info));
 + switch (hdw-ir_scheme_active) {
 + case PVR2_IR_SCHEME_24XXX: /* FX2-controlled IR */
 + case PVR2_IR_SCHEME_29XXX: /* Original 29xxx device */
 + init_data-ir_codes  = RC_MAP_HAUPPAUGE_NEW;
 + init_data-internal_get_key_func = IR_KBD_GET_KEY_HAUP;
 + init_data-type  = RC_TYPE_RC5;
 + init_data-name  = hdw-hdw_desc-description;
 + init_data-polling_interval  = 100; /* ms From ir-kbd-i2c */
 + /* IR Receiver */
 + info.addr  = 0x18;
 + info.platform_data = init_data;
 + strlcpy(info.type, ir_video, I2C_NAME_SIZE);
 + pvr2_trace(PVR2_TRACE_INFO, Binding %s to i2c address 0x%02x.,
 +info.type

Re: [RFC PATCH] pvrusb2: Provide more information about IR units to lirc_zilog and ir-kbd-i2c

2011-01-16 Thread Mike Isely
On Sun, 16 Jan 2011, Andy Walls wrote:

 On Sun, 2011-01-16 at 20:27 -0600, Mike Isely wrote:

   [,,,]

 
 Right now, yes.  In the near future, I need to use to to pass 3
 non-const items though:
 
 1. A struct mutex *transceiver_lock so that the bridge driver can pass
 a mutex to multiple modules accessing the Z8.  That would be a per
 device instance mutex, instantiated and initialized by the bridge
 driver.  The use case where this would be needed is a setup where
 ir-kbd-i2c handles Z8 IR Rx and lirc_zilog handles only Z8 IR Tx of the
 same chip.
 
 2. A bridge driver provided void (*reset_ir_chip)(struct i2c_adapter
 *adap),  or maybe void (*reset_ir_chip)(void *priv), callback to
 reset the transceiver chip when it gets hung.  The original lirc_pvr150
 module had some hard coded reset function names and calls in it, but
 they were removed with the rename to lirc_zilog and move into the
 kernel.  I'd like to get that ability back.
 
 3. A bridge driver provided private data structure for the void *priv
 argument of the aforementioned reset callback.  This would also be a per
 device object instantiated and initialized by the bridge driver. 
 

I follow.  Makes sense.

Something to consider, perhaps for the future:  Seems like what you have 
here amounts to some configuration data which will always be read-only, 
and other data which maps to the context in which the driver is being 
used (e.g. mutex instance, callback private context pointer, etc).  
That configuration data, if packed up into its own struct, could then be 
squirreled away at compile-time by the bridge driver and provided as 
part of a single table lookup.  This only makes sense if there are a lot 
of configuration bits - but here I count 6 different items.


 
  I believe I follow this and it looks good.  The concept looks very 
  simple and it's nice that the changes are really only in a single spot.  
  Just thinking ahead about making the setup table-driven and not 
  requiring data segment storage.
 
 With the patch right now it could be constant, I think.  You would have
 to use some generic name, like pvrusb2 IR, instead of
 hdw-hdw_desc-description though.
 
 For my future plans, if you don't provide a reset callback and don't
 wish to provide a mutex, then yes you can keep it constant.
 
 I suspect not providing a reset callback may be OK.
 
 Not providing a mutex is also OK but it imposes a limitation: only one
 IR module should be allowed to use the Z8 chip.  That means
 only lirc_zilog for IR Tx/Rx with Rx through LIRC, or
 only ir-kbd-i2c for IR Rx through the the Linux input subsystem.

For the future, I have no problem providing a reset callback.  And given 
what you've said, I see no reason to do anything here which would 
constrain what you're trying to accomplish.  But if down the road you do 
set up a separate configuration struct which this context struct might 
point to, then I'd like to update the pvrusb2 driver to take advantage 
of it.  But this is no big deal for now.

 
-Mike
  
  
  Acked-By: Mike Isely is...@pobox.com
 
 Thanks.  I'll pull this into my Z8 branch then.

You're welcome.

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Volunteers needed: BKL removal: replace .ioctl by .unlocked_ioctl

2010-12-18 Thread Mike Isely

I'll take care of the pvrusb2 driver.  How soon does this need to be 
completed?

  -Mike


On Sat, 18 Dec 2010, Hans Verkuil wrote:

 On Saturday, December 18, 2010 12:31:26 Hans Verkuil wrote:
  Driver list:
  
  saa7146 (Hans Verkuil)
  mem2mem_testdev (Pawel Osciak or Marek Szyprowski)
  cx23885 (Steve Toth)
  cx18-alsa (Andy Walls)
  omap24xxcam (Sakari Ailus or David Cohen)
  au0828 (Janne Grunau)
  cpia2 (Andy Walls or Hans Verkuil)
  cx231xx (Mauro Carvalho Chehab)
  davinci (Muralidharan Karicheri)
  saa6588 (Hans Verkuil)
  pvrusb2 (Mike Isely)
  usbvision (Hans Verkuil)
  s5p-fimc (Sylwester Nawrocki)
  fsl-viu (Anatolij Gustschin)
  tlg2300 (Mauro Carvalho Chehab)
  zr364xx (Hans de Goede)
  soc_camera (Guennadi Liakhovetski)
  usbvideo/vicam (Hans de Goede)
  s2255drv (Pete Eberlein)
  bttv (Mauro Carvalho Chehab)
  stk-webcam (Hans de Goede)
  se401 (Hans de Goede)
  si4713-i2c (Hans Verkuil)
  dsbr100 (Hans Verkuil)
 
 Oops, si4713-i2c and saa6588 are subdevs, so those two can be removed from
 this list.
 
 Regards,
 
   Hans
 
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 16/16] v4l: Remove module_name argument to the v4l2_i2c_new_subdev* functions

2010-10-03 Thread Mike Isely

For just the pvrusb2 part of the patch series below

Acked-By: Mike Isely is...@pobox.com


On Fri, 24 Sep 2010, Laurent Pinchart wrote:

 The argument isn't used anymore by the functions, remote it.
 
 Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 ---
  drivers/media/radio/radio-si4713.c|2 +-
  drivers/media/video/au0828/au0828-cards.c |4 ++--
  drivers/media/video/bt8xx/bttv-cards.c|   22 +++---
  drivers/media/video/cafe_ccic.c   |2 +-
  drivers/media/video/cx18/cx18-i2c.c   |8 
  drivers/media/video/cx231xx/cx231xx-cards.c   |4 ++--
  drivers/media/video/cx23885/cx23885-cards.c   |2 +-
  drivers/media/video/cx23885/cx23885-video.c   |4 ++--
  drivers/media/video/cx88/cx88-cards.c |9 -
  drivers/media/video/cx88/cx88-video.c |7 +++
  drivers/media/video/davinci/vpfe_capture.c|1 -
  drivers/media/video/davinci/vpif_capture.c|1 -
  drivers/media/video/davinci/vpif_display.c|2 +-
  drivers/media/video/em28xx/em28xx-cards.c |   18 +-
  drivers/media/video/fsl-viu.c |2 +-
  drivers/media/video/ivtv/ivtv-i2c.c   |   22 +-
  drivers/media/video/mxb.c |   12 ++--
  drivers/media/video/pvrusb2/pvrusb2-hdw.c |6 ++
  drivers/media/video/saa7134/saa7134-cards.c   |8 
  drivers/media/video/saa7134/saa7134-core.c|4 ++--
  drivers/media/video/sh_vou.c  |2 +-
  drivers/media/video/soc_camera.c  |2 +-
  drivers/media/video/usbvision/usbvision-i2c.c |6 +++---
  drivers/media/video/v4l2-common.c |   15 +--
  drivers/media/video/vino.c|4 ++--
  drivers/media/video/zoran/zoran_card.c|5 ++---
  drivers/staging/go7007/go7007-driver.c|2 +-
  drivers/staging/tm6000/tm6000-cards.c |4 ++--
  include/media/v4l2-common.h   |   16 ++--
  29 files changed, 88 insertions(+), 108 deletions(-)
 
 diff --git a/drivers/media/radio/radio-si4713.c 
 b/drivers/media/radio/radio-si4713.c
 index 045b10f..d49c215 100644
 --- a/drivers/media/radio/radio-si4713.c
 +++ b/drivers/media/radio/radio-si4713.c
 @@ -291,7 +291,7 @@ static int radio_si4713_pdriver_probe(struct 
 platform_device *pdev)
   goto unregister_v4l2_dev;
   }
  
 - sd = v4l2_i2c_new_subdev_board(rsdev-v4l2_dev, adapter, NULL,
 + sd = v4l2_i2c_new_subdev_board(rsdev-v4l2_dev, adapter,
   pdata-subdev_board_info, NULL);
   if (!sd) {
   dev_err(pdev-dev, Cannot get v4l2 subdevice\n);
 diff --git a/drivers/media/video/au0828/au0828-cards.c 
 b/drivers/media/video/au0828/au0828-cards.c
 index 0453816..01be89f 100644
 --- a/drivers/media/video/au0828/au0828-cards.c
 +++ b/drivers/media/video/au0828/au0828-cards.c
 @@ -212,7 +212,7 @@ void au0828_card_setup(struct au0828_dev *dev)
  be abstracted out if we ever need to support a different
  demod) */
   sd = v4l2_i2c_new_subdev(dev-v4l2_dev, dev-i2c_adap,
 - NULL, au8522, 0x8e  1, NULL);
 + au8522, 0x8e  1, NULL);
   if (sd == NULL)
   printk(KERN_ERR analog subdev registration failed\n);
   }
 @@ -221,7 +221,7 @@ void au0828_card_setup(struct au0828_dev *dev)
   if (dev-board.tuner_type != TUNER_ABSENT) {
   /* Load the tuner module, which does the attach */
   sd = v4l2_i2c_new_subdev(dev-v4l2_dev, dev-i2c_adap,
 - NULL, tuner, dev-board.tuner_addr, NULL);
 + tuner, dev-board.tuner_addr, NULL);
   if (sd == NULL)
   printk(KERN_ERR tuner subdev registration fail\n);
  
 diff --git a/drivers/media/video/bt8xx/bttv-cards.c 
 b/drivers/media/video/bt8xx/bttv-cards.c
 index 87d8b00..49efcf6 100644
 --- a/drivers/media/video/bt8xx/bttv-cards.c
 +++ b/drivers/media/video/bt8xx/bttv-cards.c
 @@ -3529,7 +3529,7 @@ void __devinit bttv_init_card2(struct bttv *btv)
   struct v4l2_subdev *sd;
  
   sd = v4l2_i2c_new_subdev(btv-c.v4l2_dev,
 - btv-c.i2c_adap, NULL, saa6588, 0, addrs);
 + btv-c.i2c_adap, saa6588, 0, addrs);
   btv-has_saa6588 = (sd != NULL);
   }
  
 @@ -3554,7 +3554,7 @@ void __devinit bttv_init_card2(struct bttv *btv)
   };
  
   btv-sd_msp34xx = v4l2_i2c_new_subdev(btv-c.v4l2_dev,
 - btv-c.i2c_adap, NULL, msp3400, 0, addrs);
 + btv-c.i2c_adap, msp3400, 0, addrs);
   if (btv-sd_msp34xx)
   return;
   goto no_audio;
 @@ -3568,7 +3568,7 @@ void __devinit bttv_init_card2

Re: [PATCH 07/16] pvrusb2: Don't use module names to load I2C modules

2010-10-03 Thread Mike Isely

Acked-By: Mike Isely is...@pobox.com

On Fri, 24 Sep 2010, Laurent Pinchart wrote:

 With the v4l2_i2c_new_subdev* functions now supporting loading modules
 based on modaliases, replace the hardcoded module name passed to those
 functions by NULL.
 
 All corresponding I2C modules have been checked, and all of them include
 a module aliases table with names corresponding to what the pvrusb2
 driver uses.
 
 Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 ---
  drivers/media/video/pvrusb2/pvrusb2-hdw.c |   11 ++-
  1 files changed, 2 insertions(+), 9 deletions(-)
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.c 
 b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
 index 70ea578..bef2027 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-hdw.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
 @@ -2082,20 +2082,13 @@ static int pvr2_hdw_load_subdev(struct pvr2_hdw *hdw,
   return -EINVAL;
   }
  
 - /* Note how the 2nd and 3rd arguments are the same for
 -  * v4l2_i2c_new_subdev().  Why?
 -  * Well the 2nd argument is the module name to load, while the 3rd
 -  * argument is documented in the framework as being the chipid -
 -  * and every other place where I can find examples of this, the
 -  * chipid appears to just be the module name again.  So here we
 -  * just do the same thing. */
   if (i2ccnt == 1) {
   pvr2_trace(PVR2_TRACE_INIT,
  Module ID %u:
   Setting up with specified i2c address 0x%x,
  mid, i2caddr[0]);
   sd = v4l2_i2c_new_subdev(hdw-v4l2_dev, hdw-i2c_adap,
 -  fname, fname,
 +  NULL, fname,
i2caddr[0], NULL);
   } else {
   pvr2_trace(PVR2_TRACE_INIT,
 @@ -2103,7 +2096,7 @@ static int pvr2_hdw_load_subdev(struct pvr2_hdw *hdw,
   Setting up with address probe list,
  mid);
   sd = v4l2_i2c_new_subdev(hdw-v4l2_dev, hdw-i2c_adap,
 - fname, fname,
 + NULL, fname,
   0, i2caddr);
   }
  
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] V4L/DVB: pvrusb2: remove unneeded NULL checks

2010-08-19 Thread Mike Isely

Based on the surrounding code (the unconditional dereference), I agree 
that this particular bit of coding paranoia is not doing much good.

Acked-by: Mike Isely is...@pobox.com

On Thu, 19 Aug 2010, Dan Carpenter wrote:

 We dereference maskptr unconditionally at the start of the function
 and also inside the call to parse_tlist() towards the end of the
 function.  This function is called from store_val_any() and it always
 passes a non-NULL pointer. 
 
 Signed-off-by: Dan Carpenter erro...@gmail.com
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-ctrl.c 
 b/drivers/media/video/pvrusb2/pvrusb2-ctrl.c
 index 1b992b8..55ea914 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-ctrl.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-ctrl.c
 @@ -513,7 +513,7 @@ int pvr2_ctrl_sym_to_value(struct pvr2_ctrl *cptr,
   if (ret = 0) {
   ret = pvr2_ctrl_range_check(cptr,*valptr);
   }
 - if (maskptr) *maskptr = ~0;
 + *maskptr = ~0;
   } else if (cptr-info-type == pvr2_ctl_bool) {
   ret = parse_token(ptr,len,valptr,boolNames,
 ARRAY_SIZE(boolNames));
 @@ -522,7 +522,7 @@ int pvr2_ctrl_sym_to_value(struct pvr2_ctrl *cptr,
   } else if (ret == 0) {
   *valptr = (*valptr  1) ? !0 : 0;
   }
 - if (maskptr) *maskptr = 1;
 + *maskptr = 1;
   } else if (cptr-info-type == pvr2_ctl_enum) {
   ret = parse_token(
   ptr,len,valptr,
 @@ -531,7 +531,7 @@ int pvr2_ctrl_sym_to_value(struct pvr2_ctrl *cptr,
   if (ret = 0) {
   ret = pvr2_ctrl_range_check(cptr,*valptr);
   }
 - if (maskptr) *maskptr = ~0;
 + *maskptr = ~0;
   } else if (cptr-info-type == pvr2_ctl_bitmask) {
   ret = parse_tlist(
   ptr,len,maskptr,valptr,
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] media: video: pvrusb2: remove custom hex_to_bin()

2010-07-27 Thread Mike Isely

Andy:

Acked-By: Mike Isely is...@pobox.com

  -Mike


On Tue, 27 Jul 2010, Andy Shevchenko wrote:

 Signed-off-by: Andy Shevchenko andy.shevche...@gmail.com
 Cc: Mike Isely is...@pobox.com
 ---
  drivers/media/video/pvrusb2/pvrusb2-debugifc.c |   14 ++
  1 files changed, 2 insertions(+), 12 deletions(-)
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-debugifc.c 
 b/drivers/media/video/pvrusb2/pvrusb2-debugifc.c
 index e9b11e1..4279ebb 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-debugifc.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-debugifc.c
 @@ -94,8 +94,6 @@ static int debugifc_parse_unsigned_number(const char 
 *buf,unsigned int count,
 u32 *num_ptr)
  {
   u32 result = 0;
 - u32 val;
 - int ch;
   int radix = 10;
   if ((count = 2)  (buf[0] == '0') 
   ((buf[1] == 'x') || (buf[1] == 'X'))) {
 @@ -107,17 +105,9 @@ static int debugifc_parse_unsigned_number(const char 
 *buf,unsigned int count,
   }
  
   while (count--) {
 - ch = *buf++;
 - if ((ch = '0')  (ch = '9')) {
 - val = ch - '0';
 - } else if ((ch = 'a')  (ch = 'f')) {
 - val = ch - 'a' + 10;
 - } else if ((ch = 'A')  (ch = 'F')) {
 - val = ch - 'A' + 10;
 - } else {
 + int val = hex_to_bin(*buf++);
 + if (val  0 || val = radix)
   return -EINVAL;
 - }
 - if (val = radix) return -EINVAL;
   result *= radix;
   result += val;
   }
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Status of the patches under review at LMML (60 patches)

2010-07-07 Thread Mike Isely
On Wed, 7 Jul 2010, Sven Barth wrote:

 Hi!
 
 Am 06.07.2010 15:06, schrieb Mauro Carvalho Chehab:
  == Waiting for Mike Iselyis...@isely.net  review ==
 
  Apr,25 2010: Problem with cx25840 and Terratec Grabster AV400
 http://patchwork.kernel.org/patch/94960
 
 Is Mike really the maintainer of the cx25840 module and not only of the
 pvrusb2 one? If he's not the maintainer you should contact the real one, cause
 I don't think that Mike can help much regarding patches for the cx25840 in
 that case.
 
 Also I might need to adjust the patch cause of the recent changes that
 happened there the last few months. (I don't know when I'll find time for
 this...)
 
 Regards,
 Sven

No I am definitely not the maintainer of that module, and my knowledge 
of its inner workings (though improved a lot lately) is still not very 
good.

All I can do here is verify that it doesn't break the pvrusb2 driver 
(which is what I was planning on doing).

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Status of the patches under review at LMML (60 patches)

2010-07-07 Thread Mike Isely
On Tue, 6 Jul 2010, Mauro Carvalho Chehab wrote:

 This is the summary of the patches that are currently under review at 
 Linux Media Mailing List linux-media@vger.kernel.org.
 Each patch is represented by its submission date, the subject (up to 70
 chars) and the patchwork link (if submitted via email).
 
 P.S.: This email is c/c to the developers where some action is expected.
   If you were copied, please review the patches, acking/nacking or
   submitting an update.
 

   [...]


   == Waiting for Mike Isely is...@isely.net review == 
 
 Apr,25 2010: Problem with cx25840 and Terratec Grabster AV400 
   http://patchwork.kernel.org/patch/94960
 

These are cx25840 patches and I'm not the maintainer of that module.  I 
can't really speak to the correctness of the changes.  Best I can do is 
to try the patch with a few pvrusb2-driven devices here that use the 
cx25840 module.  I've done that now (HVR-1950 and PVR-USB2 model 24012) 
and everything continues to work fine.  Note, this part of the patch:

int hw_fix = state-pvr150_workaround;
-
-   if (std == V4L2_STD_NTSC_M_JP) {
+   if (std == V4L2_STD_NTSC_M_JP) {
/* Japan uses EIAJ audio standard */
cx25840_write(client, 0x808, hw_fix ? 0x2f : 0xf7);
} else if (std == V4L2_STD_NTSC_M_KR) {

is a whitespace-only change which introduces a bogus tab and messes up 
the indentation of that opening if-statement.  It should probably be 
removed from the patch.  Other than that, you have my ack:

Acked-By: Mike Isely is...@pobox.com

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [git:v4l-dvb/other] V4L/DVB: drivers/media/video/pvrusb2: Add missing mutex_unlock

2010-07-03 Thread Mike Isely
On Sat, 3 Jul 2010, Douglas Schilling Landgraf wrote:

 Hello Mike,
 
 Mike Isely wrote:
  Mauro:
  
  FYI, I posted an Acked-By: Mike Isely is...@pobox.com weeks ago, back on
  27-May, immediately after the patch was posted.  It's a great catch, and the
  bug has been there since basically the beginning of the driver.  Was I ever
  supposed to see any kind of reaction to that ack (e.g. having the Acked-By
  added to the patch)?  I had posted it in reply to the original patch, copied
  back to the patch author, to lkml, to linux-media, kernel-janitors, and
  Mauro.
  
-Mike
 
 It seems my mistake since I have added CC instead of Acked-by, sorry.
 This happened because usually I add CC to the authors of drivers when I took
 patches from patchwork and I wanna notify them. In your case, I missed the
 acked-by.
 
 Mauro, if possible, could you please replace CC to the correct Acked-by before
 submit this patch to Linus?
 

Hmm, going through my old e-mail now I can see that the patch was picked 
up for -mm on 1-Jun.  At that time I was marked as a CC: for the patch - 
which I'd expect as the driver maintainer.  But no Acked-By: was 
showing.  Maybe that's when the ack got missed.

Obviously I have no issue with this patch.  My only real concern is that 
nobody thinks I might have been ignoring it.  Thanks for following up.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Subject: Composite input from OnAir Creator - use as security camera

2010-05-26 Thread Mike Isely
 doesn't seem to be in Lenny.
 
 Months (years?) ago, I did manage to get MythTV to work with analog signals
 but the reception was horrible so I gave up.  Watching TV is not a high
 priority and I never installed the amplified roof antenna.  I never figured
 out how to select the composite or S-video inputs from within MythTV.
 
 o xawtv
 
 xawtv doesn't have a norm for NTSC (only PAL and SECAM) so even though I can
 select the composite input:  no picture.  It also says:
 
 v4l-conf had some trouble, trying to continue anyway
 Warning: Cannot convert string -*-ledfixed-medium-r-*--39-*-*-*-c-*-*-* to 
 type FontStruct
 v4l2: read: rc=262144/size=442368
 
 So,  what else can I try?  Any hints welcome.  Thanks.
 
 A.
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 5/17] drivers/media/video/pvrusb2: Add missing mutex_unlock

2010-05-26 Thread Mike Isely

I looked through my revision history and that bug has been there in the 
driver source since at least May 2005, long before it was ever merged 
into the kernel.  Wow, what a great catch.  Thanks!

Acked-By: Mike Isely is...@pobox.com

  -Mike


On Wed, 26 May 2010, Julia Lawall wrote:

 From: Julia Lawall ju...@diku.dk
 
 Add a mutex_unlock missing on the error path.  In the other functions in
 the same file the locks and unlocks of this mutex appear to be balanced,
 so it would seem that the same should hold in this case.
 
 The semantic match that finds this problem is as follows:
 (http://coccinelle.lip6.fr/)
 
 // smpl
 @@
 expression E1;
 @@
 
 * mutex_lock(E1,...);
   +... when != E1
   if (...) {
 ... when != E1
 *   return ...;
   }
   ...+
 * mutex_unlock(E1,...);
 // /smpl
 
 Signed-off-by: Julia Lawall ju...@diku.dk
 
 ---
  drivers/media/video/pvrusb2/pvrusb2-ioread.c |5 -
  1 file changed, 4 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-ioread.c 
 b/drivers/media/video/pvrusb2/pvrusb2-ioread.c
 index b482478..bba6115 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-ioread.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-ioread.c
 @@ -223,7 +223,10 @@ int pvr2_ioread_setup(struct pvr2_ioread *cp,struct 
 pvr2_stream *sp)
   pvr2_ioread_setup (setup) id=%p,cp);
   pvr2_stream_kill(sp);
   ret = pvr2_stream_set_buffer_count(sp,BUFFER_COUNT);
 - if (ret  0) return ret;
 + if (ret  0) {
 + mutex_unlock(cp-mutex);
 + return ret;
 + }
   for (idx = 0; idx  BUFFER_COUNT; idx++) {
   bp = pvr2_stream_get_buffer(sp,idx);
   pvr2_buffer_set_buffer(bp,
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches

2010-05-21 Thread Mike Isely

Mauro:

You are reading too much into that comment.

I never said it was valid to do what had been done, only that for the 
longest time this is what the driver did and it never caused a problem 
that I was made aware of.  What I said there was correct, that this is 
what the driver had been doing in the past, that it's definitely causing 
a problem now and thus that is why this patch exists.

I'd really rather you not mess with my comment.  Probably too late 
however.

  -Mike


On Fri, 21 May 2010, Mauro Carvalho Chehab wrote:

 Mike Isely wrote:
  Please from http://linuxtv.org/hg/~mcisely/pvrusb2-patches for the
  following pvrusb2 driver fixes / improvements:
  
  - pvrusb2: Minor debug code fixup
  - pvrusb2: Fix Gotview hardware support
  - pvrusb2: Avoid using stack allocated buffers when performing USB I/O
 
 Your comment for this patch is wrong:
 
   pvrusb2: The pvrusb2 driver has for the longest time used a (tiny)
   stack allocated buffer for some of its I/O with the hardware.
   Apparently later kernels don't like this behavior and trap it at
   run-time, causing nasty complaints to the kernel log.  This trivial
   change fixes the one case in the driver where this had been happening.
 
 It were never valid to use stack for DMA, as kernel provides
 no warranty that the stack would be on a page that can do DMA.
 In a matter of fact, as most x86 USB drivers accept DMA at the first
 3Gb of RAM space, this bug is generally not noticed on i386/x86_64
 archs. Yet, if your machine has more than 3Gb, there are some chances that
 the stack would be at the HIGHMEM area, where DMA is not supported by the
 processor.
 
 As this is a common error, newer kernels have some instrumentation support to
 warn about such troubles.
 
 I'll be fixing the comment.
 
 
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches

2010-05-21 Thread Mike Isely
On Fri, 21 May 2010, Mauro Carvalho Chehab wrote:

 Mike Isely wrote:
  On Fri, 21 May 2010, Mauro Carvalho Chehab wrote:
  
  Mike Isely wrote:
  Mauro:
 
  You are reading too much into that comment.
 
  I never said it was valid to do what had been done, only that for the 
  longest time this is what the driver did and it never caused a problem 
  that I was made aware of.  What I said there was correct, that this is 
  what the driver had been doing in the past, that it's definitely causing 
  a problem now and thus that is why this patch exists.
  As I said, this is not right:
 Apparently later kernels don't like this behavior
  
  Mauro:
  
  That statement was in reference to the fact that previously the problem 
  had gone undetected, but now later kernels can notice and complain about 
  this, thus later kernels don't like this behavior.
  
  We can debate that perhaps the statement can be worded better, but that 
  doesn't make it *wrong*.
  
 
 Calling 2.6.12 kernel as later kernels doesn't seem right to me (that was
 about the kernel were em28xx driver were introduced).

The point when the em28xx driver appeared has nothing to do with this.

The point when the kernel started complaining about the use of a stack 
based USB I/O buffers is the relevant point, which was not back in 
2.6.12.  I learned of this behavior (that is, receiving warnings about 
the usage) as being new in the 2.6.34 timeframe, the point when a user 
pointed out the complaint message in his kernel log; at that time I had 
not yet tested against that kernel version.


 
  
  It is not later kernels. DMA over stack were never supported. Your driver
  had a bug that you didn't noticed for long time, probably because nobody
  reported you this issue, since it appears only on some non-Intel archs and
  on i386 with more than 3.12 Gb of RAM, and when the stack happens to be 
  after
  the first 3.12 Gb (with is a somewhat rare condition).
  
  I understand your point perfectly that this was never right or valid.  
  In fact, I also understood that point long before you decided to explain 
  it to me here - after all my realization of this problem in the driver 
  is why I wrote the patch in the first place.  Absolutely no argument 
  there about the importance of the change.
  
  None of that however justifies putting words into an author's mouth, 
  which is effectively what you did by replacing that commit comment.
  
 
 First of all, it is clearly stated at the patch that the description
 were changed and by whom:
   [mche...@redhat.com: fix patch description]
 
 Second, it is an usual upstream practice to fix descriptions where needed.
 The patch description is a precious resource for people that are seeking
 for similar bugs, and for those that are trying to understand some code.
 So, it is important to not send broken/incomplete comments to kernel,
 or comments that may have a dubious interpretation. So, subsystem maintainers
 frequently need to fix comments.
 
 Anyway, to end this discussion, I can simply revert the patch from the 
 staging tree, waiting for a new patch from you with a fixed comment.
 
 Also, if you prefer, next time, I won't apply any patch from you if I
 found a bad comment without your ack, even if it means that you'll
 probably loose a merge window.

Leave it as is.  What's done is done.  Your replaced comment is of 
course still correct.  I would just appreciate some better sensitivity 
in the future about replacing authors' comments, especially since in 
this case your interpretation of my original comment was wrong.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches

2010-05-21 Thread Mike Isely
On Fri, 21 May 2010, Mauro Carvalho Chehab wrote:

 Mike Isely wrote:

   [snip]

  The point when the kernel started complaining about the use of a stack 
  based USB I/O buffers is the relevant point, which was not back in 
  2.6.12.  I learned of this behavior (that is, receiving warnings about 
  the usage) as being new in the 2.6.34 timeframe, the point when a user 
  pointed out the complaint message in his kernel log; at that time I had 
  not yet tested against that kernel version.
 
 Older kernels also complain if the stack were actually out of the
 DMA range and you try to program DMA there. Yet, only now we've seen
 consumer PC's with lots of RAM.

One of my test targets is a PC in 32 bit mode with 4GB of RAM; strange 
then that I've never seen the kernel complain there.  The bad buffer has 
been in the driver for even longer than that and nobody raised the issue 
to me until about a month ago (the fix was created back then but for 
other reasons that you already know it didn't become available in -hg 
until last week).


 
  Leave it as is.  What's done is done.  Your replaced comment is of 
  course still correct. I would just appreciate some better sensitivity 
  in the future about replacing authors' comments, especially since in 
  this case your interpretation of my original comment was wrong.
 
 Ok.

Thanks.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches

2010-05-16 Thread Mike Isely

Please from http://linuxtv.org/hg/~mcisely/pvrusb2-patches for the
following pvrusb2 driver fixes / improvements:

- pvrusb2: Minor debug code fixup
- pvrusb2: Fix Gotview hardware support
- pvrusb2: Avoid using stack allocated buffers when performing USB I/O
- pvrusb2: New feature to mark specific hardware support as experimental
- pvrusb2: Fix kernel oops at device unregistration
- pvrusb2: Fix missing header include
- pvrusb2: Fix USB parent device reference count
- pvrusb2: Fix minor internal array allocation
- pvrusb2: Fix kernel oops on device tear-down
- pvrusb2: Call sysfs_attr_init() appropriately...

 pvrusb2-devattr.c |1
 pvrusb2-devattr.h |5 
 pvrusb2-hdw.c |   26 +
 pvrusb2-main.c|4 +--
 pvrusb2-sysfs.c   |   64 +++---
 pvrusb2-v4l2.c|   16 ++---
 6 files changed, 107 insertions(+), 9 deletions(-)

These are primarily a collection of stability fixes.

Thanks,

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Problem with cx25840 and Terratec Grabster AV400

2010-04-24 Thread Mike Isely
On Sat, 24 Apr 2010, Sven Barth wrote:

 On 24.04.2010 19:13, Mike Isely wrote:
  
  Actually the support in the pvrusb2 driver was never really completed.
  But since I don't have a sample of the hardware here I went on ahead and
  merged what was there so that it could get exposure and the remaining
  problems sorted out.
  
 -Mike
  
 
 Hi!
 
 Although you never really completed that support for the AV400 it runs pretty
 well once you've touched the cx25840 source. I'm using it for months now and
 it runs better than it did with Windows (I sometimes had troubles with audio
 there which led to an out of sync audio track).

Unfortunately I can't really say it is supported in the pvrusb2 driver 
until it actually works well enough that a user doesn't have to hack 
driver source (pvrusb2 or otherwise).  Otherwise I'm just going to get 
inundated with help requests for this.  Not having a sample of the 
device here I'm handicapped from debugging such issues.

I've just made a change to the pvrusb2 driver to allow for the ability 
to mark a piece of hardware (such as this device) as experimental.  
Such devices will generate a warning in the kernel log upon 
initialization.  The experimental marker doesn't impact the ability to 
use the device; it just triggers the warning message.  Once we know the 
device is working acceptably well enough, the marker can be turned off.  
This should help avoid misleading others about whether or not the 
pvrusb2 driver fully supports a particular piece of hardware.


 
 I wrote the last mail, because I want to sort out why the cx25837 chip in my
 device is behaving differently than expected by the corresponding driver and
 to remove the need to patch the v4l sources manually.
 Once I don't need to fear that the next system update breaks the device again
 (because cx25840.ko is overwritten), I'm more then willed to help you to
 complete the support for my device in your driver (feature testing, etc).

We definitely need to do this.


 
 Regards,
 Sven
 
 PS: Did you read my mail from last December?
 http://www.isely.net/pipermail/pvrusb2/2009-December/002716.html

Yeah, I saw it back then, and then I probably got distracted away :-(

The key issue is that your hardware doesn't seem to work until you make 
those two changes to the v4l-dvb cx25840 driver.  Obviously one can't 
just make those changes without understanding the implications for other 
users of the driver.  I (or someone expert at the cx25840 module) needs 
to study that patch and understand what is best to do for the driver.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Problem with cx25840 and Terratec Grabster AV400

2010-04-24 Thread Mike Isely
On Sat, 24 Apr 2010, Sven Barth wrote:

 Hi!
 
 On 24.04.2010 22:24, Mike Isely wrote:
  On Sat, 24 Apr 2010, Sven Barth wrote:
   
   Hi!
   
   Although you never really completed that support for the AV400 it runs
   pretty
   well once you've touched the cx25840 source. I'm using it for months now
   and
   it runs better than it did with Windows (I sometimes had troubles with
   audio
   there which led to an out of sync audio track).
  
  Unfortunately I can't really say it is supported in the pvrusb2 driver
  until it actually works well enough that a user doesn't have to hack
  driver source (pvrusb2 or otherwise).  Otherwise I'm just going to get
  inundated with help requests for this.  Not having a sample of the
  device here I'm handicapped from debugging such issues.
  
 
 I don't want to have this hacking as much as you do. But currently it's the
 only way that works for me (I'm really glad that it has come that far ^^)...
 I'll try to help here as good as I can (and time permits) to solve this issue.

I understand.


 
  I've just made a change to the pvrusb2 driver to allow for the ability
  to mark a piece of hardware (such as this device) as experimental.
  Such devices will generate a warning in the kernel log upon
  initialization.  The experimental marker doesn't impact the ability to
  use the device; it just triggers the warning message.  Once we know the
  device is working acceptably well enough, the marker can be turned off.
  This should help avoid misleading others about whether or not the
  pvrusb2 driver fully supports a particular piece of hardware.
  
 
 No offense intended, but do you really think that people will read that?
 Normal users (using Ubuntu, etc) don't really care whether their device is
 marked as experimental or not... they just want it to work and thus can go to
 great lengths to disturb the developers working on their driver...

No offense taken.  Not a problem.  But I felt it was at least important 
enough for the driver to document this fact.  For those who use the 
device who are capable of attempting some hacking - those people WILL 
see the message and hopefully that will encourage such folks to contact 
the author (me) for assistance in further stabilizing the device.

The intent wasn't for the flag to be any excuse not to work on it - I 
just want to leave a marker indicating that the driver is not expected 
to be fully working (or supported) at this time.


 
   PS: Did you read my mail from last December?
   http://www.isely.net/pipermail/pvrusb2/2009-December/002716.html
  
  Yeah, I saw it back then, and then I probably got distracted away :-(
 
 I know that problem pretty well. ^^ I was only curious.

Spending a lot of time today catching up on stuff like this.  Just 
smoked out two kernel oopses in the driver today as well.


 
  
  The key issue is that your hardware doesn't seem to work until you make
  those two changes to the v4l-dvb cx25840 driver.  Obviously one can't
  just make those changes without understanding the implications for other
  users of the driver.  I (or someone expert at the cx25840 module) needs
  to study that patch and understand what is best to do for the driver.
  
 -Mike
  
  
 

 It would be interesting to know why the v4l devs disabled the audio routing
 for cx2583x chips and whether it was intended that a cx25837 chip gets the
 same treatment as a e.g. cx25836.

I wish I could provide specific information about that :-(


 And those implications you're talking about is the reason why I wrote here:
 I want to check whether there is a better or more correct way than to disable
 those checks (it works here, because I have only that one device that contains
 a cx2583x chip...).

 Just a thought: can it be that my chip's audio routing isn't set to the
 correct value after initialization and thus it needs to be set at least once,
 while all other chips default to a working routing after initialization? Could
 be a design mistake done by Terratec...

There is no one correct audio routing.  And by audio routing I mean 
the wiring between the chip and the various audio inputs that feed it.  
The choice for how to route all this is up to the vendor of the device.  
In many cases there is a common reference design that the vendor starts 
from, in which case such routing will be more common across devices.  
But that's just luck really.  The cx25840 driver provides an API to 
things like the pvrusb2 driver to select the proper routing based on 
that bridge driver's knowledge of the surrounding hardware.  This is one 
of the areas that have to be worked on when porting to a new device.  
The PVR2_ROUTING_SCHEME_ enumeration in the pvrusb2 driver is part 
of this.

With that all said I haven't looked closely enough at your patch to the 
cx25840 module so I'm only assuming that we're talking about the same 
thing here.  I have a funny feeling that you're hitting on something 
else however.  I need to look at this more

Re: [PATCH] device_attributes: add sysfs_attr_init() for dynamic attributes

2010-04-10 Thread Mike Isely

Acked-By: Mike Isely is...@pobox.com

(in the context of the pvrusb2 driver related changes)

  -Mike

On Mon, 22 Mar 2010, Wolfram Sang wrote:

 Made necessary by 6992f5334995af474c2b58d010d08bc597f0f2fe.
 
 Found by this semantic patch:
 
 @ init @
 type T;
 identifier A;
 @@
 
 T {
 ...
 struct device_attribute A;
 ...
 };
 
 @ main extends init @
 expression E;
 statement S;
 identifier err;
 T *name;
 @@
 
 ... when != sysfs_attr_init(name-A.attr);
 (
 +   sysfs_attr_init(name-A.attr);
 if (device_create_file(E, name-A))
 S
 |
 +   sysfs_attr_init(name-A.attr);
 err = device_create_file(E, name-A);
 )
 
 While reviewing, I put the initialization to apropriate places.
 
 Signed-off-by: Wolfram Sang w.s...@pengutronix.de
 Cc: Eric W. Biederman ebied...@xmission.com
 Cc: Greg KH gre...@suse.de
 Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
 Cc: Mike Isely is...@pobox.com
 Cc: Mauro Carvalho Chehab mche...@infradead.org
 Cc: Sujith Thomas sujith.tho...@intel.com
 Cc: Matthew Garrett m...@redhat.com
 ---
 
 The thermal-sys.c-part should fix bugs #15548 and #15584.
 
  drivers/macintosh/windfarm_core.c   |1 +
  drivers/media/video/pvrusb2/pvrusb2-sysfs.c |8 
  drivers/platform/x86/intel_menlow.c |1 +
  drivers/thermal/thermal_sys.c   |1 +
  drivers/video/fsl-diu-fb.c  |1 +
  5 files changed, 12 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/macintosh/windfarm_core.c 
 b/drivers/macintosh/windfarm_core.c
 index 419795f..f447642 100644
 --- a/drivers/macintosh/windfarm_core.c
 +++ b/drivers/macintosh/windfarm_core.c
 @@ -209,6 +209,7 @@ int wf_register_control(struct wf_control *new_ct)
   kref_init(new_ct-ref);
   list_add(new_ct-link, wf_controls);
  
 + sysfs_attr_init(new_ct-attr.attr);
   new_ct-attr.attr.name = new_ct-name;
   new_ct-attr.attr.mode = 0644;
   new_ct-attr.show = wf_show_control;
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-sysfs.c 
 b/drivers/media/video/pvrusb2/pvrusb2-sysfs.c
 index 6c23456..71f5056 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-sysfs.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-sysfs.c
 @@ -423,10 +423,12 @@ static void pvr2_sysfs_add_debugifc(struct pvr2_sysfs 
 *sfp)
  
   dip = kzalloc(sizeof(*dip),GFP_KERNEL);
   if (!dip) return;
 + sysfs_attr_init(dip-attr_debugcmd.attr);
   dip-attr_debugcmd.attr.name = debugcmd;
   dip-attr_debugcmd.attr.mode = S_IRUGO|S_IWUSR|S_IWGRP;
   dip-attr_debugcmd.show = debugcmd_show;
   dip-attr_debugcmd.store = debugcmd_store;
 + sysfs_attr_init(dip-attr_debuginfo.attr);
   dip-attr_debuginfo.attr.name = debuginfo;
   dip-attr_debuginfo.attr.mode = S_IRUGO;
   dip-attr_debuginfo.show = debuginfo_show;
 @@ -644,6 +646,7 @@ static void class_dev_create(struct pvr2_sysfs *sfp,
   return;
   }
  
 + sysfs_attr_init(sfp-attr_v4l_minor_number.attr);
   sfp-attr_v4l_minor_number.attr.name = v4l_minor_number;
   sfp-attr_v4l_minor_number.attr.mode = S_IRUGO;
   sfp-attr_v4l_minor_number.show = v4l_minor_number_show;
 @@ -658,6 +661,7 @@ static void class_dev_create(struct pvr2_sysfs *sfp,
   sfp-v4l_minor_number_created_ok = !0;
   }
  
 + sysfs_attr_init(sfp-attr_v4l_radio_minor_number.attr);
   sfp-attr_v4l_radio_minor_number.attr.name = v4l_radio_minor_number;
   sfp-attr_v4l_radio_minor_number.attr.mode = S_IRUGO;
   sfp-attr_v4l_radio_minor_number.show = v4l_radio_minor_number_show;
 @@ -672,6 +676,7 @@ static void class_dev_create(struct pvr2_sysfs *sfp,
   sfp-v4l_radio_minor_number_created_ok = !0;
   }
  
 + sysfs_attr_init(sfp-attr_unit_number.attr);
   sfp-attr_unit_number.attr.name = unit_number;
   sfp-attr_unit_number.attr.mode = S_IRUGO;
   sfp-attr_unit_number.show = unit_number_show;
 @@ -685,6 +690,7 @@ static void class_dev_create(struct pvr2_sysfs *sfp,
   sfp-unit_number_created_ok = !0;
   }
  
 + sysfs_attr_init(sfp-attr_bus_info.attr);
   sfp-attr_bus_info.attr.name = bus_info_str;
   sfp-attr_bus_info.attr.mode = S_IRUGO;
   sfp-attr_bus_info.show = bus_info_show;
 @@ -699,6 +705,7 @@ static void class_dev_create(struct pvr2_sysfs *sfp,
   sfp-bus_info_created_ok = !0;
   }
  
 + sysfs_attr_init(sfp-attr_hdw_name.attr);
   sfp-attr_hdw_name.attr.name = device_hardware_type;
   sfp-attr_hdw_name.attr.mode = S_IRUGO;
   sfp-attr_hdw_name.show = hdw_name_show;
 @@ -713,6 +720,7 @@ static void class_dev_create(struct pvr2_sysfs *sfp,
   sfp-hdw_name_created_ok = !0;
   }
  
 + sysfs_attr_init(sfp-attr_hdw_desc.attr);
   sfp-attr_hdw_desc.attr.name = device_hardware_description;
   sfp-attr_hdw_desc.attr.mode = S_IRUGO;
   sfp-attr_hdw_desc.show = hdw_desc_show

Re: [PATCH 09/11] pvrusb2-v4l2: Rename dev_info to pdi

2010-04-10 Thread Mike Isely

Acked-By: Mike Isely is...@pobox.com

  -Mike


On Mon, 5 Apr 2010, Joe Perches wrote:

 There is a macro called dev_info that prints struct device specific
 information.  Having variables with the same name can be confusing and
 prevents conversion of the macro to a function.
 
 Rename the existing dev_info variables to something else in preparation
 to converting the dev_info macro to a function.
 
 Signed-off-by: Joe Perches j...@perches.com
 ---
  drivers/media/video/pvrusb2/pvrusb2-v4l2.c |   22 +++---
  1 files changed, 11 insertions(+), 11 deletions(-)
 
 diff --git a/drivers/media/video/pvrusb2/pvrusb2-v4l2.c 
 b/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
 index cc8ddb2..ba32c91 100644
 --- a/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
 @@ -48,7 +48,7 @@ struct pvr2_v4l2_dev {
  
  struct pvr2_v4l2_fh {
   struct pvr2_channel channel;
 - struct pvr2_v4l2_dev *dev_info;
 + struct pvr2_v4l2_dev *pdi;
   enum v4l2_priority prio;
   struct pvr2_ioread *rhp;
   struct file *file;
 @@ -161,7 +161,7 @@ static long pvr2_v4l2_do_ioctl(struct file *file, 
 unsigned int cmd, void *arg)
  {
   struct pvr2_v4l2_fh *fh = file-private_data;
   struct pvr2_v4l2 *vp = fh-vhead;
 - struct pvr2_v4l2_dev *dev_info = fh-dev_info;
 + struct pvr2_v4l2_dev *pdi = fh-pdi;
   struct pvr2_hdw *hdw = fh-channel.mc_head-hdw;
   long ret = -EINVAL;
  
 @@ -563,14 +563,14 @@ static long pvr2_v4l2_do_ioctl(struct file *file, 
 unsigned int cmd, void *arg)
  
   case VIDIOC_STREAMON:
   {
 - if (!fh-dev_info-stream) {
 + if (!fh-pdi-stream) {
   /* No stream defined for this node.  This means
  that we're not currently allowed to stream from
  this node. */
   ret = -EPERM;
   break;
   }
 - ret = pvr2_hdw_set_stream_type(hdw,dev_info-config);
 + ret = pvr2_hdw_set_stream_type(hdw,pdi-config);
   if (ret  0) return ret;
   ret = pvr2_hdw_set_streaming(hdw,!0);
   break;
 @@ -578,7 +578,7 @@ static long pvr2_v4l2_do_ioctl(struct file *file, 
 unsigned int cmd, void *arg)
  
   case VIDIOC_STREAMOFF:
   {
 - if (!fh-dev_info-stream) {
 + if (!fh-pdi-stream) {
   /* No stream defined for this node.  This means
  that we're not currently allowed to stream from
  this node. */
 @@ -1031,7 +1031,7 @@ static int pvr2_v4l2_open(struct file *file)
   }
  
   init_waitqueue_head(fhp-wait_data);
 - fhp-dev_info = dip;
 + fhp-pdi = dip;
  
   pvr2_trace(PVR2_TRACE_STRUCT,Creating pvr_v4l2_fh id=%p,fhp);
   pvr2_channel_init(fhp-channel,vp-channel.mc_head);
 @@ -1112,7 +1112,7 @@ static int pvr2_v4l2_iosetup(struct pvr2_v4l2_fh *fh)
   struct pvr2_hdw *hdw;
   if (fh-rhp) return 0;
  
 - if (!fh-dev_info-stream) {
 + if (!fh-pdi-stream) {
   /* No stream defined for this node.  This means that we're
  not currently allowed to stream from this node. */
   return -EPERM;
 @@ -1121,21 +1121,21 @@ static int pvr2_v4l2_iosetup(struct pvr2_v4l2_fh *fh)
   /* First read() attempt.  Try to claim the stream and start
  it... */
   if ((ret = pvr2_channel_claim_stream(fh-channel,
 -  fh-dev_info-stream)) != 0) {
 +  fh-pdi-stream)) != 0) {
   /* Someone else must already have it */
   return ret;
   }
  
 - fh-rhp = pvr2_channel_create_mpeg_stream(fh-dev_info-stream);
 + fh-rhp = pvr2_channel_create_mpeg_stream(fh-pdi-stream);
   if (!fh-rhp) {
   pvr2_channel_claim_stream(fh-channel,NULL);
   return -ENOMEM;
   }
  
   hdw = fh-channel.mc_head-hdw;
 - sp = fh-dev_info-stream-stream;
 + sp = fh-pdi-stream-stream;
   pvr2_stream_set_callback(sp,(pvr2_stream_callback)pvr2_v4l2_notify,fh);
 - pvr2_hdw_set_stream_type(hdw,fh-dev_info-config);
 + pvr2_hdw_set_stream_type(hdw,fh-pdi-config);
   if ((ret = pvr2_hdw_set_streaming(hdw,!0))  0) return ret;
   return pvr2_ioread_set_enabled(fh-rhp,!0);
  }
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-07 Thread Mike Isely
On Thu, 8 Apr 2010, hermann pitton wrote:

 Hi,
 
 Am Mittwoch, den 07.04.2010, 20:50 +0200 schrieb Lars Hanisch:
  Am 06.04.2010 16:33, schrieb Mike Isely:
 [snip]
  
   Mike, do you know of anyone actively using that additional information?
  
   Yes.
  
   The VDR project at one time implemented a plugin to directly interface
   to the pvrusb2 driver in this manner.  I do not know if it is still
   being used since I don't maintain that plugin.
  
Just FYI:
The PVR USB2 device is now handled by the pvrinput-plugin, which uses 
  only ioctls. The old pvrusb2-plugin is obsolete.
  
http://projects.vdr-developer.org/projects/show/plg-pvrinput

Lars:

Thanks for letting me know about that - until this message I had no idea 
if VDR was still using that interface.


  
  Regards,
  Lars.
 
 [snip]
 
 thanks Lars.
 
 Mike is really caring and went out for even any most obscure tuner bit
 to help to improve such stuff in the past, when we have been without any
 data sheets.

Hermann:

You might have me confused with Mike Krufky there - he's the one who did 
so much of the tuner driver overhauling in v4l-dvb in the past.


 
 To open second, maybe third and even forth ways for apps to use a
 device, likely going out of sync soon, does only load maintenance work
 without real gain.

Well it was an experiment at the time to see how well such a concept 
would work.  I had done it in a way to minimize maintenance load going 
forward.  On both counts I feel the interface actually has done very 
well, nonstandard though it may be.

I still get the general impression that the user community really has 
liked the sysfs interface, but the developers never really got very fond 
of it :-(


 
 We should stay sharp to discover something others don't want to let us
 know about. All other ideas about markets are illusions. Or?
 
 So, debugfs sounds much better than sysfs for my taste.
 
 Any app and any driver, going out of sync on the latter, will remind us
 that backward compat _must always be guaranteed_  ...
 
 Or did change anything on that and is sysfs excluded from that rule?

Backwards compatibility is very important and thus any kind of new 
interface deserves a lot of forethought to ensure that choices are made 
in the present that people will regret in the future.  Making an 
interface self-describing is one way that helps with compatibility: if 
the app can discover on its own how to use the interface then it can 
adapt to interface changes in the future.  I think a lot of people get 
their brains so wrapped around the ioctl-way of doing things and then 
they try to map that concept into a sysfs-like (or debugfs-like) 
abstraction that they don't see how to naturally take advantage of what 
is possible there.

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-07 Thread Mike Isely
On Wed, 7 Apr 2010, Hans Verkuil wrote:

   [...]

 
 Perhaps we should just not do this in sysfs at all but in debugfs? We have a
 lot more freedom there. No requirement of one-value-per-file, and if we need
 to we can change things in the future. It would actually be easier to issue
 ioctl commands to a driver from debugfs since we have a proper struct file
 there.
 
 It could be implemented as a separate module that can be loaded if debugfs is
 enabled and suddenly you have all this extra debug functionality.
 
 I admit, I would really enjoy writing something like this. I just don't want
 to do this in sysfs as that makes it too 'official' so to speak. In other 
 words,
 mainline applications should not use sysfs, but home-grown scripts are free to
 use it as far as I am concerned.
 
 How much of a problem would that be for you, Mike? On the one hand users have
 to mount debugfs, but on the other hand it will be consistent for all drivers
 that use the control framework. And you should be able to ditch a substantial
 amount of code :-)

Adding a debugfs interface that can be used by all V4L drivers is 
obviously a concept I would not have any problem with.

However that does not necessarily mean that I would agree with eventual 
removal of the pvrusb2 driver's existing sysfs interface.  That would 
depend on whether or not doing such a thing loses functionality and what 
the driver's user community would think about it.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Mike Isely
 is that it is (within limits) self-describing.  For each 
control you get not only the ability to get (and usually set) its value, 
but you can inspect its limits, learn what the default value is, 
retrieve a description (though being english only that has limited 
utility), and discover its type.  The sysfs interface in the driver 
tries to use symbolic values where possible, including when setting / 
clearing bits in a bit mask.  The fact that such information is there 
helps when writing, say, a generic dialog-based wrapper to provide a TUI 
(Text User Interface) over the interface without having to encode too 
much specific information in the wrapper itself.  Thus if new controls 
get added (or changed) later then such things just automatically adapt.  
Of course this isn't a perfect thing, but it helps.

 
 Comments? Ideas? Once we commit to something it is there for a long time to
 come since sysfs is after all a public API (although it seems to be more
 changable than ioctls).

I have found it useful over a long period of time.

I also suggest that if such an interface is defined in the general case 
that it should include some introspection capabilities, which will make 
it easier (or even possible) to evolve the interface over time while 
minimizing backwards compatibility issues.

If such a generic interface were made available, I could see an argument 
to remove the sysfs interface from the pvrusb2 driver.  HOWEVER there is 
a community using it, and also unless the generic interface were a 
complete replacement, the pvrusb2 piece will probably have to stay.  
(Note: the sysfs interface in the pvrusb2 driver is already a CONFIG_XX 
parameter so it's easy to deselect it when building the driver.)

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Mike Isely
On Tue, 6 Apr 2010, Hans Verkuil wrote:

   [...]

 
 One thing that might be useful is to prefix the name with the control class
 name. E.g. hue becomes user_hue and audio_crc becomes mpeg_audio_crc. It would
 groups them better. Or one could make a controls/user and controls/mpeg
 directory. That might not be such a bad idea actually.

I agree with grouping in concept, and using subdirectories is not a bad 
thing.  Probably however you'd want to ensure that in the end all the 
controls end up logically at the same depth in the tree.


   [...]

 
 An in between solution would be to add _type files. So you would have 
 'hue' and 'hue_type'. 'cat hue_type' would give something like:
 
 int 0 255 1 128 0x Hue
 
 In other words 'type min max step flags name'.

There was I thought at some point in the past a kernel policy that sysfs 
controls were supposed to limit themselves to one value per node.

 
 And for menu controls like stream_type (hmm, that would become 
 stream_type_type...) you would get:
 
 menu 0 5 1 0 Stream Type
 MPEG-2 Program Stream
 
 MPEG-1 System Stream
 MPEG-2 DVD-compatible Stream
 MPEG-1 VCD-compatible Stream
 MPEG-2 SVCD-compatible Stream
 
 Note the empty line to denote the unsupported menu item (transport stream).
 
 This would give the same information with just a single extra file. Still not
 sure whether it is worth it though.

Just remember that the more complex / subtle you make the node contents, 
then the more parsing will be required for any program that tries to use 
it.  I also think it's probably a bad idea for example to define a 
format where the whitespace conveys additional information.  The case 
where I've seen whitespace as part of the syntax actually work cleanly 
is in Python.


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Mike Isely
On Tue, 6 Apr 2010, Devin Heitmueller wrote:

   [...]

 
 I tend to agree with Hans.  We've already got *too many* interfaces
 that do the same thing.  The testing matrix is already a nightmare -
 V4L1 versus V4L2, mmap() versus read(), legacy controls versus
 extended controls, and don't get even me started on VBI.
 
 We should be working to make drivers and interfaces simpler, with
 *fewer* ways of doing the same thing.  The flexibility of providing
 yet another interface via sysfs compared to just calling v4l2-ctl just
 isn't worth the extra testing overhead.  We've already got too much
 stuff that needs to be fixed and not enough good developers to warrant
 making the code more complicated with little tangible benefit.

If another API (e.g. sysfs) is defined and it is specifically NOT 
permitted to be a complete set, then one can ultimately end up with 
situations where in order to effectively use a driver then multiple APIs 
*must* be used by the application.  That's even worse.

This situation already exists in the pvrusb2 driver and it's not because 
of sysfs - it's because of V4L and DVB.  When the pvrusb2 driver is used 
to handle a hybrid device (such as the HVR-1950) one has to use both the 
DVB and V4L APIs in order to effectively operate the device.  This is 
because both APIs provide something not available in the other.  And 
this really sucks if all the user wants to do is stream mpeg, darn it! 
And I don't care if it is digital or analog.  I think that situation is 
very wrong; given that the HVR-1950 can spit out mpeg in either mode the 
user shouldn't be forced to make his application choice based on which 
mode he wants.  There's only ONE application out there that allows the 
user to operate an HVR-1950 without being forced to deal with this: 
MythTV, and that's because, well, MythTV implements both APIs: V4L and 
DVB.

I really, really dislike situations that arise where multiple APIs are 
*required* to operate a device, when really there should just be one 
API.  That said, if multiple APIs are to be exported by the driver 
interface, then such APIs really should be as complete as possible in 
order to avoid potential problems later where because of previous 
limiting choices of API design now multiple APIs become required.

I agree that testing against multiple APIs can be a pain and a drain on 
effort.  But that has not happened with the pvrusb2 driver.  It should 
be possible to implement the API in a way that minimizes further 
thrashing due to driver changes.  The pvrusb2 sysfs implementation there 
is programmatically created when the driver comes up.  The code which 
implements that interface really doesn't have any logic specific to 
particular API functions; it is just a reflection of what is internally 
in the driver.  If new knobs are added to the pvrusb2 driver, then the 
knob automatically appears in the sysfs interface.  If you were to go 
through the change history of the pvrusb2-sysfs.c module, all you're 
really going to find are changes caused by the sysfs class environment 
itself (i.e. when struct class was morphed into struct device), not the 
driver or its functionality.



 
 And nobody I've talked to who writes applications that work with V4L
 has been screaming OMG, if only V4L had a sysfs interface to manage
 controls!

The experience I've seen with users and the pvrusb2 interface is that 
once they discover the sysfs API, the response is in fact very positive.  
Most users of the driver had no concept that such a thing was even 
possible until they were exposed to it.  Now that's not to say that we 
should all be screaming for this - but if people didn't really 
understand what was possible, then how could they ask for it?

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Mike Isely
On Tue, 6 Apr 2010, Markus Rechberger wrote:

   [...]

 
 how about security permissions? while you can easily change the
 permission levels for nodes in /dev you can't do this so easily with
 sysfs entries.
 I don't really think this is needed at all some applications will
 start to use ioctl some other apps might
 go for sysfs.. this makes the API a little bit whacko

This is an excellent point.  I should have brought this up sooner.

The driver has control over the modes of the nodes in sysfs.  The driver 
does NOT have control over the owner / group of those nodes.  It is 
possible to change the owner / group from userspace, and I *think* it's 
possible to create a udev rule to do this, but honestly I have not 
investigated this possibility so I don't fully know.

This is one serious potential drawback to using sysfs as a driver API.

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Mike Isely
On Tue, 6 Apr 2010, Laurent Pinchart wrote:

 Hi Andy,
 
 On Tuesday 06 April 2010 13:06:18 Andy Walls wrote:
  On Tue, 2010-04-06 at 08:37 +0200, Hans Verkuil wrote:
 
 [snip]
 
   Again, I still don't know whether we should do this. It is dangerously
   seductive because it would be so trivial to implement.
  
  It's like watching ships run aground on a shallow sandbar that all the
  locals know about.  The waters off of 'Point /sys' are full of usability
  shipwrecks.  I don't know if it's some siren's song, the lack of a light
  house, or just strange currents that deceive even seasoned
  navigators
  
  Let the user run 'v4l2-ctl -d /dev/videoN -L' to learn about the control
  metatdata.  It's not as easy as typing 'cat', but the user base using
  sysfs in an interactive shell or shell script should also know how to
  use v4l2-ctl.  In embedded systems, the final system deployment should
  not need the control metadata available from sysfs in a command shell
  anyway.
 
 I fully agree with this. If we push the idea one step further, why do we need 
 to expose controls in sysfs at all ?

I have found it useful to have the sysfs interface within the pvrusb2 
driver.

If it is going to take a lot of work to specifically craft a sysfs 
interface that exports the V4L API, then it will probably be a pain to 
maintain going forward.  By a lot of work I mean that each V4L API 
function would have to be explicitly coded for in this interface, thus 
as the V4L API evolves over time then extra work must be expended each 
time to keep the sysfs interface in step.  If that is to be the case 
then it may not be worth it.

In the pvrusb2 driver this has not been the case because the code I 
wrote which implements the sysfs interface for the driver does this 
programmatically.  That is, there is nothing in the pvrusb2-sysfs.c 
module which is specific to a particular function.  Instead, when the 
module initializes it is able to enumerate the API on its own and 
generate the appropriate interface for each control it finds.  Thus as 
the pvrusb2 driver's implementation has evolved over time, the sysfs 
implementation has simply continues to do its job, automatically 
reflecting internal changes without any extra work in that module's 
code.  I don't know if that same strategy could be done in the V4L core.  
If it could, then this would probably alleviate a lot of concerns about 
testing / maintenance going forward.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: OnAir USB HDTV Creator

2010-02-26 Thread Mike Isely
On Fri, 26 Feb 2010, Dean wrote:

 I am trying to use an 'OnAir USB HDTV Creator' (from autumnwave.com).  
 According to
 http://www.linuxtv.org/wiki/index.php/OnAir_USB_HDTV_Creator
 This device is supported, however it's not working for me.  Following the 
 instructions at above link, I tried this:
 modprobe pvrusb2 initusbreset=0
 
 The result:
 FATAL: Error inserting pvrusb2 
 (/lib/modules/2.6.31.12-desktop586-1mnb/kernel/drivers/media/video/pvrusb2/pvrusb2.ko.gz):
  Unknown symbol in module, or unknown parameter (see dmesg)

Dean:

The initusbreset module option no longer exists.  That's why your 
modprobe command is failing.  That feature was removed from the driver, 
due to a change in USB stack behavior that started with the 2.6.27 
kernel.  (The resolution hinted at in the wiki page was in fact just 
removal of the feature.)

So you need to not use initusbreset=0.  The advice in the wiki is over 
a year out of date.

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches

2010-02-21 Thread Mike Isely

Please from http://linuxtv.org/hg/~mcisely/pvrusb2-patches for the
following pvrusb2 driver fixes / improvements:

- pvrusb2: Enforce a 300msec stabilization interval during stream strart
- pvrusb2: Reduce encoder quiet period
- pvrusb2: Adjust 300msec digitizer wait to be more selective

 pvrusb2-hdw-internal.h |   12 -
 pvrusb2-hdw.c  |   61 -
 pvrusb2-hdw.h  |1
 3 files changed, 61 insertions(+), 13 deletions(-)

These fixes improve mpeg streaming startup stability for any
pvrusb2-driven device which contains an saa7115 video digitizer.

Thanks,

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Do any drivers access the cx25840 module in an atomic context?

2010-01-18 Thread Mike Isely
On Mon, 18 Jan 2010, Andy Walls wrote:

 On Mon, 2010-01-18 at 14:18 -0600, Mike Isely wrote:
  On Mon, 18 Jan 2010, Andy Walls wrote: 
   Any definitive confirmation anyone can give on any of these drivers
   would be helpful and would save me some time.
 
 
 Mike,
 
 Great!  Thank you for the answers.

You're welcome.


 
 If you would indulge one more (compound) question:
 
 Looking at the I2C master implementation in pvrusb2, it looks like it
 would be OK for me to combine the i2c_master_write() and
 i2c_master_read() in cx25840_read() into a single 2 msg i2c_transfer()
 without the pvrusb2 driver having a problem.
 
 a. Is that correct?

Yes, that is correct.


 b. Is there a limit on the combined payload, such that a the
 cx25840_read4() would not work as a combined i2c_transfer() ?

There is an overall limit on the size of the I2C transfer.  This is due 
to the underlying firmware on pvrusb2-support devices.  Essentially the 
entire outgoing transfer plus a few bytes of overhead has to fit inside 
a single 64 byte URB.  This also limits the atomic read size to roughly 
64 bytes as well (the URB size on the returned data).  You should be 
able to reliably write up to 48 bytes at a time, perhaps even a little 
more.

This issue caused a problem for the cx25840 module a few years back when 
it used to do firmware downloads with large (e.g. 1024 byte or larger) 
single I2C transfers.  Hans told me then it was that large because it 
allowed the ivtv driver to run more efficiently, but we cut it back to 
48 bytes since it triggered problems with I2C adapters (e.g. pvrusb2) 
which could not handle such larger transfers at all.

The pvrusb2 driver's I2C adapter is really just a proxy for the I2C 
implementation in the device at the far end of the USB cable.  So it 
works at a higher level than one might normally expect - it operates at 
the transfer level, no bit-banging.  The communications protocol 
required by the hardware limits the I2C transfers to be either a write 
of some size, or an atomic write followed by a read of various sizes.  
The pvrusb2 implementation looks at the incoming transfers and tries to 
map them as best it can over what the device protocol allows.  
Generally this means that if it is possible it will do the right thing.  
In the specific case you mentioned above, the result should in fact be 
exactly what you need.  (I'm saying that without having looked at that 
area of code for quite a while, but it's what I remember being in my 
head when I did that part..)

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL for 2.6.32] V4L/DVB updates

2009-12-01 Thread Mike Isely
On Tue, 1 Dec 2009, Mauro Carvalho Chehab wrote:

 Mike Isely wrote:
  On Mon, 30 Nov 2009, Mauro Carvalho Chehab wrote:
  
  Em Sat, 28 Nov 2009 14:33:30 -0600 (CST)
  Mike Isely is...@isely.net escreveu:
 
  Mauro:
 
  I had also posted up two high priority pvrusb2 patches that should 
  really be cherry-picked for 2.6.32.  You've already pulled them into 
  v4l/dvb and I did mark them as high priority at the time.
 
  These patches enable use of FX2 microcontroller firmware that is 16KB in 
  size.  Hauppauge is no longer shipping 8KB firmware for HVR-1950 and 
  HVR-1900 and without these changes then those devices won't work AT ALL 
  in kernel 2.6.32.
 
  You can find these within the v4l-dvb Mercurial repository here:
 
  Changeset 13495:87c3853fe2b3 
  Subject: pvrusb2: Support 16KB FX2 firmware
  http://linuxtv.org/hg/v4l-dvb/rev/87c3853fe2b3
 
  Changeset 13500:d4c418d4b25c
  Subject: pvrusb2: Fix lingering 16KB FX2 Firmware issues
  http://linuxtv.org/hg/v4l-dvb/rev/d4c418d4b25c
 
  I do not believe these patches have any ordering dependencies with other 
  patches, though between the two the second one technically should come 
  after the first.
  
  
  
  There are. Picking just those patches broke compilation.
  
  Mauro:
  
  Please forward to me the compilation errors.  Right now I am just not 
  seeing how a patch this trivial could have any compilation dependencies.  
  And unfortunately I will not be able to reproduce your build setup until 
  at least Tuesday night.  I must be blind.
  
  
  Also, it seemed too late for adding support for newer boards/firmware when 
  Linus
  is about to release a kernel.
  
  This is not a new feature.  It's a bug fix due to something that 
  Hauppauge recently did.  Hauppauge is NO LONGER officially distributing 
  FX2 firmware with their hardware which the driver can use.  This simply 
  prevents any new HVR-1950 / HVR-1900 users from working under Linux.  
  This fixes breakage for previously working hardware.  The root cause is 
  simple - the firmware blob is just larger now - and the fix is trivial.  
  It absolutely needs to go in.  In fact, this should go back to a 
  2.6.31.x and a 2.6.27.x release as well, though in those cases I have to 
  figure out if driver source code is still close enough for the same 
  patches to still work.
  
  I am sorry this is showing up late for you.  There are multiple reasons 
  for this.  However I did mark these patches as high priority, 
  following your v4l-dvb changeset process.  I did comment on the pull 
  request that these were important but I guess I needed to also 
  specifically call these out in the pull request text as well.
  
  If these don't get in now as part of the official 2.6.32 release, these 
  absolutely need to be queued for 2.6.32.1.
 
 We are very late for 2.6.32. I'm not sure if are there still time for it.
 
 I'll seek for some time during this week to add those patches at the upstream
 tree and removing them from the development tree and see what compilation
 issues arise.

Mauro:

Thanks.

Guess I also really need to get up to speed on git, finally...

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL for 2.6.32] V4L/DVB updates

2009-11-28 Thread Mike Isely
On Sat, 28 Nov 2009, Hans Verkuil wrote:

 On Friday 27 November 2009 22:40:01 Stefan Lippers-Hollmann wrote:
  Hi
  
  On Friday 27 November 2009, Mauro Carvalho Chehab wrote:
   Hi Linus,
   
   Please pull from:
   
   ssh://master.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-2.6.git 
   for_linus
   
   For the following drivers and building fixes:
   
  - radio-gemtek-pci: fix double mutex_lock
  - v4l: add more missing linux/sched.h includes
  - soc-camera: properly initialise the device object when reusing
  - soc-camera: sh_mobile_ceu_camera: call pm_runtime_disable
  - em28xx: fix Reddo DVB-C USB TV Box GPIO
  - davinci: remove stray duplicate config pointer
  - SMS_SIANO_MDTV should depend on HAS_DMA
  - cxusb: Fix hang on DViCO FusionHDTV DVB-T Dual Digital 4 (rev 1)
  - sh_mobile_ceu_camera: fix compile warning
  - Fix wrong parameter order in memset
  [...]
  
  Please consider cherry picking the following two patches from Hans 
  Verkuil [1]:
  - add the missing s2250-loader.h
  - s2250 mutex patch 
 
 Good catch. Mauro, can you please add these two as well for 2.6.32? I'm sure
 I marked these two as high-prio patches. This staging driver is actively
 being used and developed so this regression should be fixed.
 

Mauro:

I had also posted up two high priority pvrusb2 patches that should 
really be cherry-picked for 2.6.32.  You've already pulled them into 
v4l/dvb and I did mark them as high priority at the time.

These patches enable use of FX2 microcontroller firmware that is 16KB in 
size.  Hauppauge is no longer shipping 8KB firmware for HVR-1950 and 
HVR-1900 and without these changes then those devices won't work AT ALL 
in kernel 2.6.32.

You can find these within the v4l-dvb Mercurial repository here:

Changeset 13495:87c3853fe2b3 
Subject: pvrusb2: Support 16KB FX2 firmware
http://linuxtv.org/hg/v4l-dvb/rev/87c3853fe2b3

Changeset 13500:d4c418d4b25c
Subject: pvrusb2: Fix lingering 16KB FX2 Firmware issues
http://linuxtv.org/hg/v4l-dvb/rev/d4c418d4b25c

I do not believe these patches have any ordering dependencies with other 
patches, though between the two the second one technically should come 
after the first.

Thanks,

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-20091124

2009-11-24 Thread Mike Isely

Mauro:

Please from http://linuxtv.org/hg/~mcisely/pvrusb2-20091124 for the
following pvrusb2 driver fixes / improvements:

- pvrusb2: Support 16KB FX2 firmware
- pvrusb2: Support manual extraction of 16KB FX2 firmware
- pvrusb2: Shorten device hardware description text to work around V4L 
shortcoming
- pvrusb2: Bind I2C address 0x71 for Zilog IR devices
- pvrusb2: Cosmetic tweak to minimize size_t exposure
- pvrusb2: Fix lingering 16KB FX2 Firmware issues

 pvrusb2-debugifc.c |   14 +-
 pvrusb2-devattr.c  |   13 -
 pvrusb2-devattr.h  |1 +
 pvrusb2-hdw.c  |   44 +---
 pvrusb2-hdw.h  |2 +-
 pvrusb2-i2c-core.c |1 +
 6 files changed, 49 insertions(+), 26 deletions(-)

The 16KB fixes are pretty important - Hauppauge has updated the FX2
firmware for HVR-1950 and HVR-1900 devices such that it is 16KB in
size now.  Unfortunately the driver for years had been enforcing the
size to be 8KB which made it unable to load the newer firmware.  This
causes a problem for new users of the driver since the driver CD from
Hauppauge contains this newer firmware.  With the 16KB fixes this
problem is solved.  They are marked high priority.

Thanks,

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-20091011

2009-10-29 Thread Mike Isely
On Thu, 29 Oct 2009, Mauro Carvalho Chehab wrote:

 Em Sun, 11 Oct 2009 22:53:14 -0500 (CDT)
 Mike Isely is...@isely.net escreveu:
 
  
  Mauro:
  
  Please from http://linuxtv.org/hg/~mcisely/pvrusb2-20091011 for a few 
  various pvrusb2 fixes / improvements.  No critical bug fixes here, just 
  a bunch of little things:
  
  - pvrusb2: Make more info available to udev
  - pvrusb2: Soften encoder warning message
  - pvrusb2: Improve diagnostic info on driver initialization failure
  - pvrusb2: Report hardware description to kernel log upon initialization
  - pvrusb2: Add hardware description to debuginfo output
  - pvrusb2: Fix redundant message on driver initialization failure (missing 
  break)
  - pvrusb2: Cosmetic kernel log tweak
 
 Committed.
 
 Please fix the existing CodingStyle erros added on this changeset on a next 
 pull request:

We've had this discussion many times in the past.  I am not going to 
have that debate yet again.  These will NOT be changed.

  -Mike


 
 ERROR: trailing statements should be on next line
 #26: FILE: linux/drivers/media/video/pvrusb2/pvrusb2-v4l2.c:919:
 +   if (!dip) return;
 +   if (!dip) return;
 ERROR: trailing statements should be on next line
 #27: FILE: linux/drivers/media/video/pvrusb2/pvrusb2-v4l2.c:920:
 +   if (!dip-devbase.parent) return;
 +   if (!dip-devbase.parent) return;
 
 
 
 Cheers,
 Mauro
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-20091011

2009-10-11 Thread Mike Isely

Mauro:

Please from http://linuxtv.org/hg/~mcisely/pvrusb2-20091011 for a few 
various pvrusb2 fixes / improvements.  No critical bug fixes here, just 
a bunch of little things:

- pvrusb2: Make more info available to udev
- pvrusb2: Soften encoder warning message
- pvrusb2: Improve diagnostic info on driver initialization failure
- pvrusb2: Report hardware description to kernel log upon initialization
- pvrusb2: Add hardware description to debuginfo output
- pvrusb2: Fix redundant message on driver initialization failure (missing 
break)
- pvrusb2: Cosmetic kernel log tweak

 pvrusb2-debugifc.c |3 +++
 pvrusb2-encoder.c  |5 -
 pvrusb2-hdw-internal.h |1 +
 pvrusb2-hdw.c  |   33 -
 pvrusb2-v4l2.c |   21 -
 5 files changed, 52 insertions(+), 11 deletions(-)

  -Mike



-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: How to make my device work with linux?

2009-10-01 Thread Mike Isely
On Thu, 1 Oct 2009, Wellington Terumi Uemura wrote:

 I was looking around to find that there is a driver for that Fujitsu
 MB86A16 inside the Linux Mantis Driver project, Fujitsu MB86A16
 DVB-S/DSS DC Receiver driver made by Manu Abraham
 http://www.verbraak.org/wiki/index.php/Linux_Mantis_driver.
 
 I've done a few tests with usbsnoop and other tools but USB sniffer
 doesn't see any valid command, jut a bunch of bytes that makes no
 sense:
 http://www.isely.net/pvrusb2/firmware.html#FX2

What you've pointed at here is a page that describes using a trick with 
the pvrusb2 driver to suck an image of the FX2 firmware out of the FX2 
processor itself.  That won't work in your case however since it 
requires that the pvrusb2 driver already be talking to the chip.  The 
procedure documented at that link is really about firmware extraction 
not reverse-engineering the data link protocol between the FX2 and the 
host.

 
 I will try my luck compiling that Fujitsu driver, but my best guess is
 that without a proper I/O from that FX2 it will end up with nothing at
 all.

It's that data link protocol that you need to understand.  Please 
realize that the FX2 is just an 8051 microcontroller which happens to 
have a fairly interesting USB device interface resident on the same 
silicon.  Beyond that, the chip's behavior is really up to whatever the 
firmware does.  For pvrusb2-driven devices that firmware's behavior is 
pretty well understood.  That driver also benefits from the fact that 
essentially all USB hosted analog (and some hybrid) capture cards with 
an mpeg encoder and an FX2 all are derivations from a reference design 
by a single vendor.  That reference design included reference 
firmware, which each manufacturer of course tweaked a bit.  For that 
reason, all those different devices tend to implement a similar enough 
data link protocol that the pvrusb2 driver can handle them all with the 
same implementation.

The problem is, we don't know if any of that is true for your device.  
You are dealing with a digital-only capture device so it can't be based 
on that same reference design.  It is entirely sensible that the FX2 
firmware was set up in that case with similar requirements in mind so 
the result *might* be similar in behavior.  But it really isn't known.  
So when you scan documentation for other drivers (e.g. pvrusb2) you must 
really look at it all with a rather large helping of scepticism.

Mike Krufky mentions a driver for the TDA18271 and he's right.  There is 
one - because the pvrusb2 driver also relies on that when driving an 
HVR-1950 capture device which happens to use that same part.  But that 
isn't the driver you need.  What you need is a bridge driver that can 
implement the host side of the data link protocol implemented by your 
device's FX2.  That is what the pvrusb2 driver does for the capture 
devices it handles.  With the proper bridge driver set up, then the 
TDA18271 sub-device driver can ride over that data link to establish 
communications with its hardware in the device.  THEN you'll be on the 
way to having something working.

I know that none of the about is the answer you're looking for.  But 
perhaps it will lead you in the right direction.  It is entirely 
possible that there is another bridge driver out there which can handle 
this part, but I can't help you there.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: How to make my device work with linux?

2009-10-01 Thread Mike Isely
On Thu, 1 Oct 2009, Devin Heitmueller wrote:

 On Thu, Oct 1, 2009 at 6:03 PM, Wellington Terumi Uemura
 wellingtonuem...@gmail.com wrote:
  It's not the answer that I was looking for but looks like the thing is
  much more complex than just compile and run drivers, this gives me
  another perspective, like a dead end.
 
  Thank you Mike.
 
 Well, it's certainly possible to get it to work if you're willing to
 make the investment.  It's just one of those situations where you
 realize quickly that you're going to have to be prepared to do *way*
 more work than just adding a new board profile.  Just because there
 are drivers for the chips on your device doesn't mean that it is
 trivial to get working.
 
 Cheers,
 
 Devin
 

And actually I wasn't intending on totally discouraging you either.  
But you do need to see the perspective of what you're trying to do 
otherwise you may just get frustrated.

Things aren't hopeless.  The cxusb module in DVB might be something you 
should look at.  I guess it depends on how deep you wish to dive here.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] s2255drv: Don't conditionalize video buffer completion on waiting processes

2009-09-23 Thread Mike Isely
# HG changeset patch
# User Mike Isely is...@pobox.com
# Date 1253739604 18000
# Node ID 522a74147753ba59c7f45e368439928090a286f2
# Parent  e349075171ddf939381fad432c23c1269abc4899
s2255drv: Don't conditionalize video buffer completion on waiting processes

From: Mike Isely is...@pobox.com

The s2255 driver had logic which aborted processing of a video frame
if there was no process waiting on the video buffer in question.  That
simply doesn't work when the application is doing things in an
asynchronous manner.  If the application went to the trouble to queue
the buffer in the first place, then the driver should always attempt
to complete it - even if the application at that moment has its
attention turned elsewhere.  Applications which always blocked waiting
for I/O on the capture device would not have been affected by this.
Applications which *mostly* blocked waiting for I/O on the capture
device probably only would have been somewhat affected (frame lossage,
at a rate which goes up as the application blocks less).  Applications
which never blocked on the capture device (e.g. polling only) however
would never have been able to receive any video frames, since in that
case this is anyone waiting on this? check on the buffer never would
have evalutated true.  This patch just deletes that harmful check
against the buffer's wait queue.

Priority: high

Signed-off-by: Mike Isely is...@pobox.com

diff -r e349075171dd -r 522a74147753 linux/drivers/media/video/s2255drv.c
--- a/linux/drivers/media/video/s2255drv.c  Mon Sep 21 10:42:22 2009 -0500
+++ b/linux/drivers/media/video/s2255drv.c  Wed Sep 23 16:00:04 2009 -0500
@@ -599,11 +599,6 @@
buf = list_entry(dma_q-active.next,
 struct s2255_buffer, vb.queue);
 
-   if (!waitqueue_active(buf-vb.done)) {
-   /* no one active */
-   rc = -1;
-   goto unlock;
-   }
list_del(buf-vb.queue);
do_gettimeofday(buf-vb.ts);
dprintk(100, [%p/%d] wakeup\n, buf, buf-vb.i);


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] s2255drv: Don't conditionalize video buffer completion on waiting processes

2009-09-23 Thread Mike Isely
On Wed, 23 Sep 2009, dean wrote:

 This seems ok.  This portion of code was based on vivi.c, so that might be
 checked also.

Yes, after seeing the mention of vivi in this driver I looked at vivi.c 
and saw the same construct there.  Though I'm willing to bet that it's 
just as incorrect there as it was here, I haven't tested or otherwise 
used vivi so I wasn't prepared to recommend a patch for it as well.

Probably vivi should be fixed, since it is after all intended as a model 
for other v4l driver developers.  (And are there any other drivers based 
on vivi which have inherited this bug as well?)

  -Mike


 
 
 
 Mike Isely wrote:
  # HG changeset patch
  # User Mike Isely is...@pobox.com
  # Date 1253739604 18000
  # Node ID 522a74147753ba59c7f45e368439928090a286f2
  # Parent  e349075171ddf939381fad432c23c1269abc4899
  s2255drv: Don't conditionalize video buffer completion on waiting processes
  
  From: Mike Isely is...@pobox.com
  
  The s2255 driver had logic which aborted processing of a video frame
  if there was no process waiting on the video buffer in question.  That
  simply doesn't work when the application is doing things in an
  asynchronous manner.  If the application went to the trouble to queue
  the buffer in the first place, then the driver should always attempt
  to complete it - even if the application at that moment has its
  attention turned elsewhere.  Applications which always blocked waiting
  for I/O on the capture device would not have been affected by this.
  Applications which *mostly* blocked waiting for I/O on the capture
  device probably only would have been somewhat affected (frame lossage,
  at a rate which goes up as the application blocks less).  Applications
  which never blocked on the capture device (e.g. polling only) however
  would never have been able to receive any video frames, since in that
  case this is anyone waiting on this? check on the buffer never would
  have evalutated true.  This patch just deletes that harmful check
  against the buffer's wait queue.
  
  Priority: high
  
  Signed-off-by: Mike Isely is...@pobox.com
  
  diff -r e349075171dd -r 522a74147753 linux/drivers/media/video/s2255drv.c
  --- a/linux/drivers/media/video/s2255drv.c  Mon Sep 21 10:42:22 2009 -0500
  +++ b/linux/drivers/media/video/s2255drv.c  Wed Sep 23 16:00:04 2009 -0500
  @@ -599,11 +599,6 @@
  buf = list_entry(dma_q-active.next,
   struct s2255_buffer, vb.queue);
   -  if (!waitqueue_active(buf-vb.done)) {
  -   /* no one active */
  -   rc = -1;
  -   goto unlock;
  -   }
  list_del(buf-vb.queue);
  do_gettimeofday(buf-vb.ts);
  dprintk(100, [%p/%d] wakeup\n, buf, buf-vb.i);
  
  

 
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] bttv: Fix potential out-of-order field processing

2009-09-21 Thread Mike Isely

Mauro:

You can also directly pull this from: 
http://linuxtv.org/hg/~mcisely/bttv-patches/

Sorry about the excessively long commit description, but I felt it 
important to fully explain this somewhat subtle problem for what is 
otherwise a mature driver.  The actual patch is tiny.

  -Mike


# HG changeset patch
# User Mike Isely is...@pobox.com
# Date 1253545748 18000
# Node ID 760a8bc4028014493ccbbe85d0c9e8c91873fc23
# Parent  29e4ba1a09bcf9a03a653b2124929f5359fef772
bttv: Fix potential out-of-order field processing

From: Mike Isely is...@pobox.com

There is a subtle interaction in the bttv driver which can result in
fields being repeatedly processed out of order.  This is a problem
specifically when running in V4L2_FIELD_ALTERNATE mode (probably the
most common case).

1. The determination of which fields are associated with which buffers
happens in videobuf, before the bttv driver gets a chance to queue the
corresponding DMA.  Thus by the point when the DMA is queued for a
given buffer, the algorithm has to do the queuing based on the
buffer's already assigned field type - not based on which field is
next in the video stream.

2. The driver normally tries to queue both the top and bottom fields
at the same time (see bttv_irq_next_video()).  It tries to sort out
top vs bottom by looking at the field type for the next 2 available
buffers and assigning them appropriately.

3. However the bttv driver *always* actually processes the top field
first.  There's even an interrupt set aside for specifically
recognizing when the top field has been processed so that it can be
marked done even while the bottom field is still being DMAed.

Given all of the above, if one gets into a situation where
bttv_irq_next_video() gets entered when the first available buffer has
been pre-associated as a bottom field, then the function is going to
process the buffers out of order.  That first available buffer will be
put into the bottom field slot and the buffer after that will be put
into the top field slot.  Problem is, since the top field is always
processed first by the driver, then that second buffer (the one after
the first available buffer) will be the first one to be finished.
Because of the strict fifo handling of all video buffers, then that
top field won't be seen by the app until after the bottom field is
also processed.  Worse still, the app will get back the
chronologically later bottom field first, *before* the top field is
received.  The buffer's timestamps will even be backwards.

While not fatal to most TV apps, this behavior can subtlely degrade
userspace deinterlacing (probably will cause jitter).  That's probably
why it has gone unnoticed.  But it will also cause serious problems if
the app in question discards all but the latest received buffer (a
latency minimizing tactic) - causing one field to only ever be
displayed since the other is now always late.  Unfortunately once you
get into this state, you're stuck this way - because having consumed
two buffers, now the next time around the first available buffer
will again be a bottom field and the same thing happens.

How can we get into this state?  In a perfect world, where there's
always a few free buffers queued to the driver, it should be
impossible.  However if something disrupts streaming, e.g. if the
userspace app can't queue free buffers fast enough for a moment due
perhaps to a CPU scheduling glitch, then the driver can get
momentarily starved and some number of fields will be dropped.  That's
OK.  But if an odd number of fields get dropped, then that first
available buffer might be the bottom field and now we're stuck...

This patch fixes that problem by deliberately only setting up a single
field for one frame if we don't get a top field as the first available
buffer.  By purposely skipping the other field, then we only handle a
single buffer thus bringing things back into proper sync (i.e. top
field first) for the next frame.  To do this we just drop the few
lines in bttv_irq_next_video() that attempt to set up the second
buffer when that second buffer isn't for the bottom field.

This is definitely a problem in when in V4L2_FIELD_ALTERNATE mode.  In
the other modes this change either has no effect or doesn't harm
things any further anyway.

Priority: high

Signed-off-by: Mike Isely is...@pobox.com

diff -r 29e4ba1a09bc -r 760a8bc40280 
linux/drivers/media/video/bt8xx/bttv-driver.c
--- a/linux/drivers/media/video/bt8xx/bttv-driver.c Sat Sep 19 09:45:22 
2009 -0300
+++ b/linux/drivers/media/video/bt8xx/bttv-driver.c Mon Sep 21 10:09:08 
2009 -0500
@@ -3828,11 +3828,34 @@
if (!V4L2_FIELD_HAS_BOTH(item-vb.field) 
(item-vb.queue.next != btv-capture)) {
item = list_entry(item-vb.queue.next, struct 
bttv_buffer, vb.queue);
+   /* Mike Isely is...@pobox.com - Only check
+* and set up the bottom field in the logic

[PATCH] bttv: Fix reversed polarity error when switching video standard

2009-09-21 Thread Mike Isely

Mauro:

You can also directly pull this from: 
http://linuxtv.org/hg/~mcisely/bttv-patches/

Again, another longer-than usual commit description here.  Same reason 
as before.  This bug is a little less subtle than the other one - and 
the patch is even smaller (one character).

  -Mike


# HG changeset patch
# User Mike Isely is...@pobox.com
# Date 1253547742 18000
# Node ID e349075171ddf939381fad432c23c1269abc4899
# Parent  760a8bc4028014493ccbbe85d0c9e8c91873fc23
bttv: Fix reversed polarity error when switching video standard

From: Mike Isely is...@pobox.com

The bttv driver function which handles switching of the video standard
(set_tvnorm() in bttv-driver.c) includes a check which can optionally
also reset the cropping configuration to a default value.  It is
optional based on a comparison of the cropcap parameters of the
previous vs the newly requested video standard.  The comparison is
being done with a memcmp(), a function which only returns a true value
if the comparison actually fails.

This if-statement appears to have been written to assume wrong
memcmp() semantics.  That is, it was re-initializing the cropping
configuration only if the new video standard did NOT have different
cropcap values.  That doesn't make any sense.  One definitely should
reset things if the cropcap parameters are different - if there's any
comparison to made at all.

The effect of this problem was that a transition from, say, PAL to
NTSC would leave in place old cropping setup that made sense for the
PAL geometry but not for NTSC.  If the application doesn't care about
cropping it also won't try to reset the cropping configuration,
resulting in an improperly cropped video frame.  In the case I was
testing this actually caused black video frames to be displayed.

Another interesting effect of this bug is that if one does something
which does NOT change the video standard and this function is run,
then the cropping setup gets reset anyway - again because of the
backwards comparison.  It turns out that just running anything which
merely opens and closes the video device node (e.g. v4l-info) will
cause this to happen.  One can argue that simply opening the device
node and not doing anything to it should not mess with any of its
state - but because of this behavior, any TV app which does such
things (e.g. xawtv) probably therefore doesn't see the problem.

The solution is to fix the sense of the if-statement.  It's easy to
see how this mistake could have been made given how memcmp() works.
The patch is therefore removal of a single ! character from the
if-statement in set_tvnorm in bttv-driver.c.

Priority: high

Signed-off-by: Mike Isely is...@pobox.com

diff -r 760a8bc40280 -r e349075171dd 
linux/drivers/media/video/bt8xx/bttv-driver.c
--- a/linux/drivers/media/video/bt8xx/bttv-driver.c Mon Sep 21 10:09:08 
2009 -0500
+++ b/linux/drivers/media/video/bt8xx/bttv-driver.c Mon Sep 21 10:42:22 
2009 -0500
@@ -1322,7 +1322,7 @@
 
tvnorm = bttv_tvnorms[norm];
 
-   if (!memcmp(bttv_tvnorms[btv-tvnorm].cropcap, tvnorm-cropcap,
+   if (memcmp(bttv_tvnorms[btv-tvnorm].cropcap, tvnorm-cropcap,
sizeof (tvnorm-cropcap))) {
bttv_crop_reset(btv-crop[0], norm);
btv-crop[1] = btv-crop[0]; /* current = default */

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [linuxtv-commits] [hg:v4l-dvb] cx25840: fix determining the firmware name

2009-09-07 Thread Mike Isely
On Mon, 7 Sep 2009, Mauro Carvalho Chehab wrote:


   [...]

 
 I remember I asked both Mikes (Michael Krufky and Mike Isely) on March for 
 some
 tests with the new firmware. I'm not sure if they had some time for testing 
 it.

Yes, I remember this exchange.  At the time you had mentioned only the 3 
files with identical MD5 sums (not that other one with the dadb79... 
sum).  Here's the message I sent in response at that time:

CUT_HERE

From is...@isely.net Tue Mar 31 21:17:58 2009
From: Mike Isely is...@isely.net
To: Mauro Carvalho Chehab mche...@infradead.org
Cc: Hans Verkuil hverk...@xs4all.nl, Michael Krufky mkru...@linuxtv.org, 
Mike Isely at pobox is...@pobox.com
Date: Tue, 31 Mar 2009 21:17:54 -0500 (CDT)
Subject: Re: Conexant firmwares
Reply-To: Mike Isely is...@pobox.com

On Tue, 24 Mar 2009, Mike Isely wrote:

 On Mon, 23 Mar 2009, Mauro Carvalho Chehab wrote:
 
  Hi Hans and Mike I.,
  
  Conexant officially sent us their firmwares. Due to that, I've added the
  firmwares at linuxtv.org and included at the get_dvb_firmware script. 
  
  I'll also make sure that they'll be added at kernel-firmware -git and
  included in Fedora.
  
  As Michael K. noticed, the cx25840 firmwares are identical, for 3 different
  models:
  
  a9f8f5d901a7fb42f552e1ee6384f3bb  cx23885 firmware/v4l-cx23885-enc.fw
  a9f8f5d901a7fb42f552e1ee6384f3bb  cx25840 firmwares/v4l-cx231xx-avcore-01.fw
  a9f8f5d901a7fb42f552e1ee6384f3bb  cx25840 firmwares/v4l-cx23885-avcore-01.fw
  
  Could you please test if those firmwares work well for the ivtv and pvrusb2
  models? 
  
  All those firmwares are available at linuxtv.org downloads/firmware dir.
  
 
 I imagine that they will all work just fine.  But I'll let you know once 
 I've positively verified that fact.
 
   -Mike

Yes, it works fine.  I tested an HVR-1950 and a PVR-USB2 24xxx series 
model.  Both work with the new firmware.

Given that the 3 firmware images are identical, then what's the point of 
there being 3 different names?

  -Mike

/CUT_HERE


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-misc

2009-08-07 Thread Mike Isely

Acked-By: Mike Isely is...@pobox.com

  -Mike

On Fri, 7 Aug 2009, Hans Verkuil wrote:

 Hi Mauro,
 
 Please pull from http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-misc for the 
 following:
 
 - pvrusb2: fix compile warning
 - cx24113: fix mips compiler warning
 - hdpvr: add missing initialization of current_norm
 - v4l2-ioctl: fix G_STD and G_PARM default handlers
 - v4l2-ctl: fix get/set-parm bugs and add get/set-output-parm support
 
 Thanks,
 
 Hans
 
 diffstat:
  linux/drivers/media/dvb/frontends/cx24113.c   |6 +
  linux/drivers/media/video/hdpvr/hdpvr-video.c |2
  linux/drivers/media/video/pvrusb2/pvrusb2-audio.c |5 -
  linux/drivers/media/video/v4l2-ioctl.c|   15 ++-
  v4l2-apps/util/v4l2-ctl.cpp   |   98 
 +-
  5 files changed, 101 insertions(+), 25 deletions(-)
 
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: offering bounty for GPL'd dual em28xx support

2009-07-29 Thread Mike Isely
On Wed, 22 Jul 2009, Mauro Carvalho Chehab wrote:

 Em Wed, 22 Jul 2009 11:06:12 -0400
 Devin Heitmueller dheitmuel...@kernellabs.com escreveu:
 
  On Wed, Jul 22, 2009 at 11:01 AM, Jelle de
  Jongjelledej...@powercraft.nl wrote:
   Funky timing of those mails :D.
  
   I saw only after sending my mail that Steve was talking about analog and
   that this is indeed different. Dual analog tuner support should be
   possible right? Maybe with some other analog usb chipsets? I don't know
   what the usb blocksize is or if they are isochronous transfers or bulk
   or control.
  
   I assume the video must be uncompressed transferred over usb because the
   decoding chip is on the usb device is not capable of doing compression
   encoding after the analog video decoding? Are there usb devices that do
   such tricks?
  
  There were older devices that did compression, mainly designed to fit
  the stream inside of 12Mbps USB.  However, they required onboard RAM
  to buffer the frame which added considerable cost (in addition to the
  overhead of doing the compression), and as a result pretty much all of
  the USB 2.0 designs I have seen do not do any on-chip compression.
  
  The example which comes to mind is the Hauppauge Win-TV USB which uses
  the usbvision chipset.
 
 pvrusb2 also has compression, provided by an external mpeg encoder. Those
 devices are USB 2.0
 

I know this is a fairly old thread, but I've been away on vacation and 
I'm catching up on e-mail right now.  So forgive me if this has already 
been answered...

The Hauppauge Win-TV PVR-USB2 is the most well-known device in this 
category and it's what the pvrusb2 driver originally targeted.  This 
device uses a dedicated mpeg encoder chip within the device, so the 
video stream coming from the hardware is actually compressed video (mpeg 
format, mostly DVD-style mpeg2).  The question of USB 1.1 vs USB 2.0 is 
actually due to the device's microcontroller (the bridge chip) not the 
mpeg encoder.  In the pvrusb2 case, that controller is a Cypress FX2 
which includes an on-chip USB 2.0 high-speed device interface.  But the 
mpeg encoder actually doesn't REQUIRE USB 2.0 high-speed.  The default 
encoder settings configured by the pvrusb2 driver actually work quite 
well over USB 1.1, since the resulting video stream requires 
significantly less bandwidth than the 12Mbps that USB 1.1 can 
theoretically supply.  I've actually successfully tested such a 
configuration here.  The hardware works fine over USB 1.1.

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-dev

2009-06-20 Thread Mike Isely

[sorry, reposted with corrected subject tag]

Mauro:

Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2-dev for a
number of pvrusb2 driver fixes.  The first two in particular are
high-priority critical fixes that clean up a nasty regression
involving analog capture on the HVR-1950 and the older 24xxx model
series.  It would be good to see those at least backported to a
2.6.30.x release.

- pvrusb2: Fix hardware scaling when used with cx25840
- pvrusb2: Re-fix hardware scaling on video standard change
- pvrusb2: Change initial default frequency setting
- pvrusb2: Improve handling of routing schemes
- pvrusb2: De-obfuscate code which handles routing schemes

 pvrusb2-audio.c   |   14 ++-
 pvrusb2-cs53l32a.c|   26 +++--
 pvrusb2-cx2584x-v4l.c |   39 +---
 pvrusb2-hdw.c |   60 --
 pvrusb2-video-v4l.c   |   37 +-
 5 files changed, 98 insertions(+), 78 deletions(-)


  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: s5h1411_readreg: readreg error (ret == -5)

2009-06-12 Thread Mike Isely

I am unable to reproduce the s5h1411 error here.

However my HVR-1950 loads the s5h1409 module - NOT s5h1411.ko; I wonder 
if Hauppauge has changed chips on newer devices and so you're running a 
different tuner module.  That would explain the different behavior.  
Unfortunately it also means it will be very difficult for me to track 
the problem down here since I don't have that device variant.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: s5h1411_readreg: readreg error (ret == -5)

2009-06-12 Thread Mike Isely

Well now I feel like an idiot.  Thanks for pointing that out in my own 
code :-)

Still digging through this.

  -Mike


On Fri, 12 Jun 2009, Andy Walls wrote:

 On Fri, 2009-06-12 at 15:33 -0500, Mike Isely wrote:
  I am unable to reproduce the s5h1411 error here.
  
  However my HVR-1950 loads the s5h1409 module - NOT s5h1411.ko; I wonder 
  if Hauppauge has changed chips on newer devices and so you're running a 
  different tuner module.
 
 The digital demodulator driver to use is hardcoded in pvrusb2-devattr.c:
 
 static const struct pvr2_dvb_props pvr2_750xx_dvb_props = {
 .frontend_attach = pvr2_s5h1409_attach,
 .tuner_attach= pvr2_tda18271_8295_attach,
 };
 
 static const struct pvr2_dvb_props pvr2_751xx_dvb_props = {
 .frontend_attach = pvr2_s5h1411_attach,
 .tuner_attach= pvr2_tda18271_8295_attach,
 };
 ...
 static const struct pvr2_device_desc pvr2_device_750xx = {
 .description = WinTV HVR-1950 Model Category 750xx,
 ...
 .dvb_props = pvr2_750xx_dvb_props,
 #endif
 };
 ...
 static const struct pvr2_device_desc pvr2_device_751xx = {
 .description = WinTV HVR-1950 Model Category 751xx,
 ...
 .dvb_props = pvr2_751xx_dvb_props,
 #endif
 };
 
 
That would explain the different behavior.  
  Unfortunately it also means it will be very difficult for me to track 
  the problem down here since I don't have that device variant.
 
 If you have more than 1 HVR-1950, maybe one is a 751xx variant.
 
 When I ran into I2C errors often, it was because of PCI bus errors
 screwing up the bit banging.  Obviously, that's not the case here.
 
 -Andy
 
-Mike
 
 
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: s5h1411_readreg: readreg error (ret == -5)

2009-06-11 Thread Mike Isely
On Thu, 11 Jun 2009, Steven Toth wrote:

 Mike Isely wrote:
  On Sun, 7 Jun 2009, Roger wrote:
  
   From looking at linux/drivers/media/dvb/frontends/s5h1411.c,  The
   s5h1411_readreg wants to see 2 but is getting -5 from the i2c bus.
   
   --- Snip ---
   
   s5h1411_readreg: readreg error (ret == -5)
   pvrusb2: unregistering DVB devices
   device: 'dvb0.net0': device_unregister
   
   --- Snip ---
   
   What exactly does this mean?
  
  Roger:
  
  It means that the module attempted an I2C transfer and the transfer failed.
  The I2C adapter within the pvrusb2 driver will return either the number of
  bytes that it transferred or a failure code.  The failure code, as is normal
  convention in the kernel, will be a negated errno value.  Thus the expected
  value of 2 would be the fact that it probably tried a 2 byte transfer, while
  the actual value returned of -5 indicate an EIO error, which is what the
  pvrusb2 driver will return when the underlying I2C transaction has failed.
  
  Of course the real question is not that it failed but why it failed.  And
  for that I unfortunately do not have an answer.  It's possible that the
  s5h1411 driver did something that the chip didn't like and the chip
  responded by going deaf on the I2C bus.  More than a few I2C-driven parts
  can behave this way.  It's also possible that the part might have been busy
  and unable to respond - but usually in that case the driver for such a part
  will be written with this in mind and will know how / when to communicate
  with the hardware.
 
 Roger:
 
 Another possibility, although I don't know the PVRUSB2 driver too well, the
 s5h1411 is being held in reset when the driver unloads _AFTER_ the last active
 use was analog video (assuming the s5h1411 is floated in reset as the FX2
 input port might be shared with the analog encoder)

Good point.  The pvrusb2 driver is not currently doing anything specific 
- or at least deliberate - via the FX2 to move that part in/out of 
reset.  (Of course, I am issuing FX2 commands to shift modes and that 
might in turn be triggering other things.)  But even if I did do 
something specific, what kind of impact is that likely to do on the 
corresponding, blissfully ignorant, driver?

This actually drives towards a larger issue - the pvrusb2 driver works 
with various V4L-only sub-devices, e.g. cx25840, which have no relevance 
in digital mode but I can't really control when that corresponding 
driver is enabled / disabled.  So if I have to take an extra step to 
physically disable a chip when in digital mode, then this might impact 
the sub-driver which otherwise is going to have no clue what is really 
going on.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: s5h1411_readreg: readreg error (ret == -5)

2009-06-10 Thread Mike Isely
On Sun, 7 Jun 2009, Roger wrote:

 From looking at linux/drivers/media/dvb/frontends/s5h1411.c,  The
 s5h1411_readreg wants to see 2 but is getting -5 from the i2c bus.
 
 --- Snip ---
 
 s5h1411_readreg: readreg error (ret == -5)
 pvrusb2: unregistering DVB devices
 device: 'dvb0.net0': device_unregister
 
 --- Snip ---
 
 What exactly does this mean?

Roger:

It means that the module attempted an I2C transfer and the transfer 
failed.  The I2C adapter within the pvrusb2 driver will return either 
the number of bytes that it transferred or a failure code.  The failure 
code, as is normal convention in the kernel, will be a negated errno 
value.  Thus the expected value of 2 would be the fact that it probably 
tried a 2 byte transfer, while the actual value returned of -5 indicate 
an EIO error, which is what the pvrusb2 driver will return when the 
underlying I2C transaction has failed.

Of course the real question is not that it failed but why it failed.  
And for that I unfortunately do not have an answer.  It's possible that 
the s5h1411 driver did something that the chip didn't like and the chip 
responded by going deaf on the I2C bus.  More than a few I2C-driven 
parts can behave this way.  It's also possible that the part might have 
been busy and unable to respond - but usually in that case the driver 
for such a part will be written with this in mind and will know how / 
when to communicate with the hardware.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PULL] http://kernellabs.com/hg/~stoth/tda10048-merge/

2009-05-21 Thread Mike Isely

I see no issues here with the pvrusb2 part of it...

Acked-by: Mike Isely is...@pobox.com

On Wed, 20 May 2009, Steven Toth wrote:

 Mauro,
 
 Please pull from http://kernellabs.com/hg/~stoth/tda10048-merge/
 
-  TDA10048: Ensure the I/F changes during DVB-T 6/7/8 bandwidth changes.
-  cx23885: Ensure we specify I/F's for all bandwidths
-  pvrusb2: Ensure we specify I/F's for all bandwidths
-  TDA10048: Missing two I/F's / Pll combinations from the PLL table
-  cx23885: fix tda10048 IF frequencies for the Hauppauge WinTV-HVR1210
 
  dvb/frontends/tda10048.c|  188 +---
  dvb/frontends/tda10048.h|6
  video/cx23885/cx23885-dvb.c |8
  video/pvrusb2/pvrusb2-devattr.c |4
  4 files changed, 139 insertions(+), 67 deletions(-)
 
 This was regression tested on the following products. HVR-1900, HVR-1200 and
 HVR-1700.
 
 Thanks
 
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/8] ir-kbd-i2c conversion to the new i2c binding model (v3)

2009-05-17 Thread Mike Isely
On Thu, 14 May 2009, Jean Delvare wrote:

 On Thu, 14 May 2009 21:25:02 +0200, Oldřich Jedlička wrote:
  On Wednesday 13 of May 2009 at 21:45:59, Jean Delvare wrote:
   Hi all,
  
   Here comes an update of my conversion of ir-kbd-i2c to the new i2c
   binding model. I've split it into 8 pieces for easier review. Firstly
   there is 1 preliminary patch:
  
  
  Hi Jean,
  
  works for me, as usual :-) I've used the all-in-one patch and the 
  up-to-date 
  v4l-dvb tree (compiled yesterday for completeness).
 
 Oldrich, thanks a lot for testing and reporting, this is very
 appreciated.
 

Jean:

I tried the all-in-one patch here on a PVR-USB2 24xxx model (slightly 
older v4l-dvb repo and 2.6.27.13 vanilla kernel) and it worked fine.  
I'll add an acked-by to the corresponding (trivial) pvrusb2 patch that 
you've posted.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

Re: [PATCH 8/8] pvrusb2: Instantiate ir_video I2C device by default

2009-05-17 Thread Mike Isely
On Wed, 13 May 2009, Jean Delvare wrote:

 Now that the ir-kbd-i2c driver has been converted to a new-style i2c
 driver, we can instantiate the ir_video I2C device by default. The
 pvr2_disable_ir_video is kept to disable the IR receiver, either
 because the user doesn't use it, or for debugging purpose.
 
 Signed-off-by: Jean Delvare kh...@linux-fr.org
 Cc: Mike Isely is...@pobox.com

Acked-by: Mike Isely is...@pobox.com

 ---
  linux/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 --- v4l-dvb.orig/linux/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c 
 2009-05-13 18:05:54.0 +0200
 +++ v4l-dvb/linux/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c  
 2009-05-13 18:06:32.0 +0200
 @@ -43,7 +43,7 @@ static int ir_mode[PVR_NUM] = { [0 ... P
  module_param_array(ir_mode, int, NULL, 0444);
  MODULE_PARM_DESC(ir_mode,specify: 0=disable IR reception, 1=normal IR);
  
 -static int pvr2_disable_ir_video = 1;
 +static int pvr2_disable_ir_video;
  module_param_named(disable_autoload_ir_video, pvr2_disable_ir_video,
  int, S_IRUGO|S_IWUSR);
  MODULE_PARM_DESC(disable_autoload_ir_video,
 
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-dev

2009-05-12 Thread Mike Isely
On Mon, 11 May 2009, Mauro Carvalho Chehab wrote:

 Em Mon, 11 May 2009 22:09:26 -0300
 Mauro Carvalho Chehab mche...@infradead.org escreveu:
 
  Em Sat, 9 May 2009 16:49:31 -0500 (CDT)
  Mike Isely is...@isely.net escreveu:
  
   
   Mauro:
   
   Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2-dev for various 
   pvrusb2 changesets.  Several have to do with IR as previously discussed 
   with Jean Delvare.  He's waiting for these changes.  Other stuff is more 
   of a miscellaneous / cleanup nature.
 
 Hmm... this one failed when importing on -git:
 
 Changeset: 11749
 From: Greg Kroah-Hartman  gre...@suse.de
 Commiter: Mike Isely is...@pobox.com
 Date: Fri May 01 22:36:33 2009 -0500
 Subject: pvrusb2: remove driver_data direct access of struct device
 
 In the near future, the driver core is going to not allow direct access
 to the driver_data pointer in struct device.  Instead, the functions
 dev_get_drvdata() and dev_set_drvdata() should be used.  These functions
 have been around since the beginning, so are backwards compatible with
 all older kernel versions.
 
 Priority: normal
 
 Cc: Mauro Carvalho Chehab mche...@infradead.org
 Cc: linux-media@vger.kernel.org
 
 $ patch -p1 -i 11749.patch
 patching file drivers/media/video/pvrusb2/pvrusb2-sysfs.c
 Reversed (or previously applied) patch detected!  Assume -R? [n] 
 
 It seems that you've got a Greg's patch, removed the parts that touch on other
 files, applied on your tree and asked me to merge it. Please, never do it,
 since this will cause merge problems when exporting the patches to git. Next
 time, just reply with an acked-by, and let Patchwork to add your ack on the
 original patch.
 

I in fact did what you are asking for here (i.e. wait for it to show up 
on its own) before for another change that got rid of usb_settoggle().  
It took a LONG time - WEEKS - for that fix to get back into v4l-dvb via 
the mechanism you just described.  And this had the effect of breaking 
the v4l-dvb repository for a period of time when the kernel mainline 
then unpublished the usb_settoggle() function.  I do NOT like to see 
that happen.  That caused me to decide not to rely on what you are now 
telling me to do.

I also disagree with this for another reason.  What happens if, say, 
Greg generates a patch that I need before I can proceed with other 
changes?  Do I just sit around and wait for it to trickle back before I 
can continue?  That seems wrong.  In addition in the past when there 
have been pvrusb2 changes generated from upstream you have asked if I 
was planning on pulling them in myself - which I've done in the past.

It seems wrong on its face to tell me that I can't go get a patch that 
affects my driver.

AND...  In the case of the remove usb_settoggle() patch, I *DID* in 
fact add my acked-by to that patch.  Greg dutifully took note of this 
and ensured it was part of the git patch.  However when it got back into 
v4l-dvb, the acked-by attribution was missing.  I complained about this 
to you and your response was that this was a fault of the pathway / 
mechanism and that I should basically accept this.  So now you're 
telling me to do this anyway?

Whatever.

  -Mike



-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-dev

2009-05-09 Thread Mike Isely

Mauro:

Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2-dev for various 
pvrusb2 changesets.  Several have to do with IR as previously discussed 
with Jean Delvare.  He's waiting for these changes.  Other stuff is more 
of a miscellaneous / cleanup nature.

  -Mike


- pvrusb2: Select, track, and report IR scheme in use with the device
- pvrusb2: Update to work with upcoming ir_video changes in v4l-dvb core
- pvrusb2: Set ir_video autoloading to default disabled
- pvrusb2: Bump up version advertised through v4l interface
- pvrusb2: Don't use the internal i2c client list
- pvrusb2: remove driver_data direct access of struct device
- pvrusb2: Allocate a routing ID for future support of Terratec Grabster AV400

 pvrusb2-devattr.c  |1 
 pvrusb2-devattr.h  |   23 +-
 pvrusb2-hdw-internal.h |3 +
 pvrusb2-hdw.c  |   78 -
 pvrusb2-i2c-core.c |   53 +++--
 pvrusb2-sysfs.c|   22 ++---
 pvrusb2-v4l2.c |2 -
 7 files changed, 106 insertions(+), 76 deletions(-)

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PULL] http://linuxtv.org/hg/~stoth/tda10048

2009-05-05 Thread Mike Isely

Nice catch.  Thanks.

  -Mike


On Tue, 5 May 2009, Steven Toth wrote:

 In addition to this:
 
 Please pull from http://www.linuxtv.org/hg/~stoth/tda10048
 
-  tda10048: Added option to block i2c gate control from other drivers.
-  pvrusb2: Ensure the PVRUSB2 disabled the i2c gate on the tda10048.
 
  dvb/frontends/tda10048.c|  222 +++-
  dvb/frontends/tda10048.h|   17 +
  video/cx23885/cx23885-dvb.c |4
  video/pvrusb2/pvrusb2-devattr.c |3
  4 files changed, 244 insertions(+), 2 deletions(-)
 
 This fixes a bug in the current PVRUSB2 tree where DVB-T is not working due
 to multiple repeaters upsteam of the tuner on the same i2c bus.
 
 - Steve
 
 Steven Toth wrote:
  Mauro,
  
  Please pull from http://www.linuxtv.org/hg/~stoth/tda10048
  
 -  tda10048: Add ability to select I/F at attach time.
 -  cx23885: For tda10048 boards ensure we specify the I/F
 -  pvrusb2: Ensure we specify the I/F at attach time.
  
   dvb/frontends/tda10048.c|  219 +++-
   dvb/frontends/tda10048.h|   14 +
   video/cx23885/cx23885-dvb.c |4
   video/pvrusb2/pvrusb2-devattr.c |2
   4 files changed, 237 insertions(+), 2 deletions(-)
  
  The TDA10048 used to have a hard-coded I/F, I've improved this to support
  different I/F's and ensured that all current bridge drivers specify their
  needs.
  
  Regards,
  
  - Steve
  
 
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [cron job] v4l-dvb daily build 2.6.22 and up: ERRORS, 2.6.16-2.6.21: ERRORS

2009-05-01 Thread Mike Isely
On Fri, 1 May 2009, Alexey Klimov wrote:

 Hello,
 
 On Mon, Apr 20, 2009 at 3:59 AM, Mike Isely is...@isely.net wrote:

   [...]

 
  So the kernel already has this; it just needs to be pulled back into
  v4l-dvb.  It's an obvious trivial thing for now and I've acked it there.
  Obviously we're getting had here because you're compiling against a
  kernel snapshot that's been changed but v4l-dvb doesn't have the
  corresponding change in its local copy of the pvrusb2 driver.  Part of
  the fun of synchronizing changes from different trees :-(
 
 Well, good to know that this thing is already fixed.
 I'm very sorry for the mess.

No apology needed.  Really - this mess wasn't caused by you.  If 
anything I should have just immediately pulled that patch into hg and 
not waited for it to trickle back to Mauro.  That would have avoided the 
error.  So, all I can say is that I'm sorry you had to hit this!

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

Re: [PATCH 2/6] ir-kbd-i2c: Switch to the new-style device binding model

2009-05-01 Thread Mike Isely

Jean:

I have another idea that I think you'll like.  I'm putting the finishing 
touches on the patch right now.

What I have implements correct ir_video loading for the pvrusb2 driver.  
It also includes a lookup table (though with only 1 entry right now) to 
determine the proper I2C address and I use i2c_new_device() now instead 
of i2c_new_probed_device().  I've also renamed things to be ir_video 
everywhere instead of ir_kbd_i2c as was discussed.

In particular the disable option is there like before, but now it's 
called disable_autoload_ir_video.

So far this is exactly what I was saying before.  But I'm also making 
one more change: the disable_autoload_ir_video option default value will 
- for now - be 1, i.e. everything above I just described starts off 
disabled.  This means that the entire patch can be pulled _right_ _now_ 
without breaking anything at all, because the outward behavior is still 
unchanged.

Then, when you're ready to bring your stuff in, all you need to do is 
include a 1-line change to pvrusb2-i2c-core.c to switch the default 
value of disable_autoload_ir_video back to 0, which now enables all the 
corresponding pvrusb2 changes that you need to avoid any breakage inside 
my driver.  Just to be absolutely crystal clear, here's the change you 
will be able to do once these changes are in:

diff -r 8d2e1361520c linux/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c
--- a/linux/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c  Fri May 01 
20:23:39 2009 -0500
+++ b/linux/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c  Fri May 01 
20:24:23 2009 -0500
@@ -43,7 +43,7 @@
 module_param_array(ir_mode, int, NULL, 0444);
 MODULE_PARM_DESC(ir_mode,specify: 0=disable IR reception, 1=normal IR);
 
+static int pvr2_disable_ir_video;
-static int pvr2_disable_ir_video = 1;
 module_param_named(disable_autoload_ir_video, pvr2_disable_ir_video,
   int, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(disable_autoload_ir_video,

So with all this, what I am setting up right now will cause no harm to 
the existing situation, requires no actual messing with module options, 
and once you're reading, just include the 1-line change above and you're 
set.  There's no race here, no gap in IR handling.

  -Mike


On Thu, 23 Apr 2009, Mike Isely wrote:

 
 Hi Jean,
 
 I had actually written out a longer, detailed, point-by-point reply 
 earlier today, but before I could finish it I got interrupted with a 
 crisis.  And then another.  And that's kind of how my day went.  Now I'm 
 finally back to this, but I have another e-mail debacle to immediately 
 deal with (thank you pobox.com, not!) so I'm tossing that unfinished 
 lengthy reply.  I think I can sum this whole thing up very simply.  
 Here's what I think should be happening:
 
 1a. Your existing IR changeset, minus the pvrusb2 changes, should be 
 merged.
 
 1b. In parallel with (1a) I modify my pvrusb2 changeset to use the right 
 module name and I adjust the driver option name to match.
 
 2. My pvrusb2 changeset, with changes from (1b) is merged.
 
 3. Andy's proposed change for ir_kbd_i2c to support additional IR 
 devices is investigated and probably merged.
 
 4. I test the changed ir_kbd_i2c against additional pvrusb2 devices 
 known not to work previously with ir_kbd_i2c.  If they work, then I will 
 create a pvrusb2 patch to load ir_video in those cases as well as the 
 cases already set up (which still won't cover all possible 
 pvrusb2-driven devices).
 
 I think this satisfies the remaining issues, except that from between 
 steps 1 and 2 ir_kbd_i2c won't work with the pvrusb2 driver.  But you 
 know, I'm OK with that.  I expect (2) to happen within a few days after 
 (1) so the impact should be minimal.  It certainly won't escape into the 
 kernel tree that way.  In addition, my impression from the community of 
 pvrusb2 users is that the preferred IR strategy is lirc anyway, and it's 
 a foregone conclusion that they are all going to be, uh, impacted once 
 your compatibility parts are gone from i2c.  From where I'm sitting the 
 gap from (1) to (2) is trivial compared to that impending mess - 
 realize I'm not complaining since after all (a) it really falls to the 
 lirc developers to fix that, (b) you do need to get your changes in, and 
 (c) there's little I can do for lirc there except to keep it working as 
 long as possible with the pvrusb2 driver.  I'm just pointing out that 
 I'm OK with the step 1 - 2 gap for the pvrusb2 driver because it's 
 small and will be nothing compared to what's about to happen with lirc.
 
 If you still don't like any of that, well, then I give up.  In that 
 case, then merge your changes with the pvrusb2 changes included.  I 
 won't ack them, but I'll just deal with it later.  Because while your 
 changes might keep ir_kbd_i2c going, they will also immediately break 
 the more-useful and apparently more-used lirc (by always binding 
 ir_kbd_i2c even in cases in the pvrusb2 driver where it's known

Re: [PATCH] media: remove driver_data direct access of struct device

2009-05-01 Thread Mike Isely

Acked-By: Mike Isely is...@pobox.com

Note #1: I am just acking the pvrusb2 part of this.

Note #2: I am immediately pulling the pvrusb2 part of these changes into 
that driver.

  -Mike


On Thu, 30 Apr 2009, Greg Kroah-Hartman wrote:

 From: Greg Kroah-Hartman gre...@suse.de
 
 In the near future, the driver core is going to not allow direct access
 to the driver_data pointer in struct device.  Instead, the functions
 dev_get_drvdata() and dev_set_drvdata() should be used.  These functions
 have been around since the beginning, so are backwards compatible with
 all older kernel versions.
 
 
 Cc: Mauro Carvalho Chehab mche...@infradead.org
 Cc: linux-media@vger.kernel.org
 Signed-off-by: Greg Kroah-Hartman gre...@suse.de
 
 ---
  drivers/media/dvb/firewire/firedtv-1394.c   |4 ++--
  drivers/media/dvb/firewire/firedtv-dvb.c|2 +-
  drivers/media/video/pvrusb2/pvrusb2-sysfs.c |   22 +++---
  3 files changed, 14 insertions(+), 14 deletions(-)
 
 --- a/drivers/media/dvb/firewire/firedtv-1394.c
 +++ b/drivers/media/dvb/firewire/firedtv-1394.c
 @@ -225,7 +225,7 @@ fail_free:
  
  static int node_remove(struct device *dev)
  {
 - struct firedtv *fdtv = dev-driver_data;
 + struct firedtv *fdtv = dev_get_drvdata(dev);
  
   fdtv_dvb_unregister(fdtv);
  
 @@ -242,7 +242,7 @@ static int node_remove(struct device *de
  
  static int node_update(struct unit_directory *ud)
  {
 - struct firedtv *fdtv = ud-device.driver_data;
 + struct firedtv *fdtv = dev_get_drvdata(ud-device);
  
   if (fdtv-isochannel = 0)
   cmp_establish_pp_connection(fdtv, fdtv-subunit,
 --- a/drivers/media/dvb/firewire/firedtv-dvb.c
 +++ b/drivers/media/dvb/firewire/firedtv-dvb.c
 @@ -268,7 +268,7 @@ struct firedtv *fdtv_alloc(struct device
   if (!fdtv)
   return NULL;
  
 - dev-driver_data= fdtv;
 + dev_set_drvdata(dev, fdtv);
   fdtv-device= dev;
   fdtv-isochannel= -1;
   fdtv-voltage   = 0xff;
 --- a/drivers/media/video/pvrusb2/pvrusb2-sysfs.c
 +++ b/drivers/media/video/pvrusb2/pvrusb2-sysfs.c
 @@ -539,7 +539,7 @@ static void class_dev_destroy(struct pvr
sfp-attr_unit_number);
   }
   pvr2_sysfs_trace(Destroying class_dev id=%p,sfp-class_dev);
 - sfp-class_dev-driver_data = NULL;
 + dev_set_drvdata(sfp-class_dev, NULL);
   device_unregister(sfp-class_dev);
   sfp-class_dev = NULL;
  }
 @@ -549,7 +549,7 @@ static ssize_t v4l_minor_number_show(str
struct device_attribute *attr, char *buf)
  {
   struct pvr2_sysfs *sfp;
 - sfp = (struct pvr2_sysfs *)class_dev-driver_data;
 + sfp = dev_get_drvdata(class_dev);
   if (!sfp) return -EINVAL;
   return scnprintf(buf,PAGE_SIZE,%d\n,
pvr2_hdw_v4l_get_minor_number(sfp-channel.hdw,
 @@ -561,7 +561,7 @@ static ssize_t bus_info_show(struct devi
struct device_attribute *attr, char *buf)
  {
   struct pvr2_sysfs *sfp;
 - sfp = (struct pvr2_sysfs *)class_dev-driver_data;
 + sfp = dev_get_drvdata(class_dev);
   if (!sfp) return -EINVAL;
   return scnprintf(buf,PAGE_SIZE,%s\n,
pvr2_hdw_get_bus_info(sfp-channel.hdw));
 @@ -572,7 +572,7 @@ static ssize_t hdw_name_show(struct devi
struct device_attribute *attr, char *buf)
  {
   struct pvr2_sysfs *sfp;
 - sfp = (struct pvr2_sysfs *)class_dev-driver_data;
 + sfp = dev_get_drvdata(class_dev);
   if (!sfp) return -EINVAL;
   return scnprintf(buf,PAGE_SIZE,%s\n,
pvr2_hdw_get_type(sfp-channel.hdw));
 @@ -583,7 +583,7 @@ static ssize_t hdw_desc_show(struct devi
struct device_attribute *attr, char *buf)
  {
   struct pvr2_sysfs *sfp;
 - sfp = (struct pvr2_sysfs *)class_dev-driver_data;
 + sfp = dev_get_drvdata(class_dev);
   if (!sfp) return -EINVAL;
   return scnprintf(buf,PAGE_SIZE,%s\n,
pvr2_hdw_get_desc(sfp-channel.hdw));
 @@ -595,7 +595,7 @@ static ssize_t v4l_radio_minor_number_sh
  char *buf)
  {
   struct pvr2_sysfs *sfp;
 - sfp = (struct pvr2_sysfs *)class_dev-driver_data;
 + sfp = dev_get_drvdata(class_dev);
   if (!sfp) return -EINVAL;
   return scnprintf(buf,PAGE_SIZE,%d\n,
pvr2_hdw_v4l_get_minor_number(sfp-channel.hdw,
 @@ -607,7 +607,7 @@ static ssize_t unit_number_show(struct d
   struct device_attribute *attr, char *buf)
  {
   struct pvr2_sysfs *sfp;
 - sfp = (struct pvr2_sysfs *)class_dev-driver_data;
 + sfp = dev_get_drvdata(class_dev);
   if (!sfp) return -EINVAL;
   return scnprintf(buf,PAGE_SIZE,%d\n,
pvr2_hdw_get_unit_number(sfp-channel.hdw));
 @@ -635,7 +635,7 @@ static void class_dev_create(struct pvr2

Re: [PATCH] pvrusb2: Don't use the internal i2c client list

2009-04-30 Thread Mike Isely
On Thu, 30 Apr 2009, Jean Delvare wrote:

 The i2c core used to maintain a list of client for each adapter. This
 is a duplication of what the driver core already does, so this list
 will be removed as part of a future cleanup. Anyone using this list
 must stop doing so.
 
 For pvrusb2, I propose the following change, which should lead to an
 equally informative output. The only difference is that i2c clients
 which are not a v4l2 subdev won't show up, but I guess this case is
 not supposed to happen anyway.

It will happen for anything i2c used by v4l which itself is not really a 
part of v4l.  That would include, uh, lirc.

I will review and test this first chance I get which should be tomorrow.

  -Mike


 
 Signed-off-by: Jean Delvare kh...@linux-fr.org
 Cc: Mike Isely is...@pobox.com
 ---
 Mike, can you please review and test this patch? Thanks.
 
  linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c |   56 
 +--
  1 file changed, 13 insertions(+), 43 deletions(-)
 
 --- v4l-dvb.orig/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c  
 2009-04-30 16:52:32.0 +0200
 +++ v4l-dvb/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c   2009-04-30 
 17:20:37.0 +0200
 @@ -4920,65 +4920,35 @@ static unsigned int pvr2_hdw_report_clie
   unsigned int tcnt = 0;
   unsigned int ccnt;
   struct i2c_client *client;
 - struct list_head *item;
 - void *cd;
   const char *p;
   unsigned int id;
  
 - ccnt = scnprintf(buf, acnt, Associated v4l2-subdev drivers:);
 + ccnt = scnprintf(buf, acnt, Associated v4l2-subdev drivers and I2C 
 clients:\n);
   tcnt += ccnt;
   v4l2_device_for_each_subdev(sd, hdw-v4l2_dev) {
   id = sd-grp_id;
   p = NULL;
   if (id  ARRAY_SIZE(module_names)) p = module_names[id];
   if (p) {
 - ccnt = scnprintf(buf + tcnt, acnt - tcnt,  %s, p);
 + ccnt = scnprintf(buf + tcnt, acnt - tcnt,   %s:, p);
   tcnt += ccnt;
   } else {
   ccnt = scnprintf(buf + tcnt, acnt - tcnt,
 -   (unknown id=%u), id);
 +(unknown id=%u):, id);
   tcnt += ccnt;
   }
 - }
 - ccnt = scnprintf(buf + tcnt, acnt - tcnt, \n);
 - tcnt += ccnt;
 -
 - ccnt = scnprintf(buf + tcnt, acnt - tcnt, I2C clients:\n);
 - tcnt += ccnt;
 -
 - mutex_lock(hdw-i2c_adap.clist_lock);
 - list_for_each(item, hdw-i2c_adap.clients) {
 - client = list_entry(item, struct i2c_client, list);
 - ccnt = scnprintf(buf + tcnt, acnt - tcnt,
 -%s: i2c=%02x, client-name, client-addr);
 - tcnt += ccnt;
 - cd = i2c_get_clientdata(client);
 - v4l2_device_for_each_subdev(sd, hdw-v4l2_dev) {
 - if (cd == sd) {
 - id = sd-grp_id;
 - p = NULL;
 - if (id  ARRAY_SIZE(module_names)) {
 - p = module_names[id];
 - }
 - if (p) {
 - ccnt = scnprintf(buf + tcnt,
 -  acnt - tcnt,
 -   subdev=%s, p);
 - tcnt += ccnt;
 - } else {
 - ccnt = scnprintf(buf + tcnt,
 -  acnt - tcnt,
 -   subdev= id %u),
 -  id);
 - tcnt += ccnt;
 - }
 - break;
 - }
 + client = v4l2_get_subdevdata(sd);
 + if (client) {
 + ccnt = scnprintf(buf + tcnt, acnt - tcnt,
 +   %s @ %02x\n, client-name,
 +  client-addr);
 + tcnt += ccnt;
 + } else {
 + ccnt = scnprintf(buf + tcnt, acnt - tcnt,
 +   no i2c client\n);
 + tcnt += ccnt;
   }
 - ccnt = scnprintf(buf + tcnt, acnt - tcnt, \n);
 - tcnt += ccnt;
   }
 - mutex_unlock(hdw-i2c_adap.clist_lock);
   return tcnt;
  }
  
 
 
 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/6] ir-kbd-i2c: Switch to the new-style device binding model

2009-04-23 Thread Mike Isely

Hi Jean,

I had actually written out a longer, detailed, point-by-point reply 
earlier today, but before I could finish it I got interrupted with a 
crisis.  And then another.  And that's kind of how my day went.  Now I'm 
finally back to this, but I have another e-mail debacle to immediately 
deal with (thank you pobox.com, not!) so I'm tossing that unfinished 
lengthy reply.  I think I can sum this whole thing up very simply.  
Here's what I think should be happening:

1a. Your existing IR changeset, minus the pvrusb2 changes, should be 
merged.

1b. In parallel with (1a) I modify my pvrusb2 changeset to use the right 
module name and I adjust the driver option name to match.

2. My pvrusb2 changeset, with changes from (1b) is merged.

3. Andy's proposed change for ir_kbd_i2c to support additional IR 
devices is investigated and probably merged.

4. I test the changed ir_kbd_i2c against additional pvrusb2 devices 
known not to work previously with ir_kbd_i2c.  If they work, then I will 
create a pvrusb2 patch to load ir_video in those cases as well as the 
cases already set up (which still won't cover all possible 
pvrusb2-driven devices).

I think this satisfies the remaining issues, except that from between 
steps 1 and 2 ir_kbd_i2c won't work with the pvrusb2 driver.  But you 
know, I'm OK with that.  I expect (2) to happen within a few days after 
(1) so the impact should be minimal.  It certainly won't escape into the 
kernel tree that way.  In addition, my impression from the community of 
pvrusb2 users is that the preferred IR strategy is lirc anyway, and it's 
a foregone conclusion that they are all going to be, uh, impacted once 
your compatibility parts are gone from i2c.  From where I'm sitting the 
gap from (1) to (2) is trivial compared to that impending mess - 
realize I'm not complaining since after all (a) it really falls to the 
lirc developers to fix that, (b) you do need to get your changes in, and 
(c) there's little I can do for lirc there except to keep it working as 
long as possible with the pvrusb2 driver.  I'm just pointing out that 
I'm OK with the step 1 - 2 gap for the pvrusb2 driver because it's 
small and will be nothing compared to what's about to happen with lirc.

If you still don't like any of that, well, then I give up.  In that 
case, then merge your changes with the pvrusb2 changes included.  I 
won't ack them, but I'll just deal with it later.  Because while your 
changes might keep ir_kbd_i2c going, they will also immediately break 
the more-useful and apparently more-used lirc (by always binding 
ir_kbd_i2c even in cases in the pvrusb2 driver where it's known that it 
can't even work but lirc would have) which as far as I'm concerned is 
far worse than the step 1 - 2 gap above.  But it's just another gap and 
I'll push another pvrusb2 changeset on top to clean it up.  So just do 
whatever you think is best right now, if you disagree with the sequence 
above.

Now I will go off and deal with the steamer that pobox.com has just 
handed me :-(

  -Mike


-- 

Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [cron job] v4l-dvb daily build 2.6.22 and up: ERRORS, 2.6.16-2.6.21: ERRORS

2009-04-19 Thread Mike Isely
On Mon, 20 Apr 2009, Alexey Klimov wrote:

   [...]

 When trying to compile v4l-dvb tree under 2.6.30-rc2 (up-to-date) i
 have such error with pvr2 module:
 
   CC [M]  /w/new/v4l-dvb/v4l/pvrusb2-hdw.o
 /w/new/v4l-dvb/v4l/pvrusb2-hdw.c: In function 'pvr2_upload_firmware1':
 /w/new/v4l-dvb/v4l/pvrusb2-hdw.c:1474: error: implicit declaration of
 function 'usb_settoggle'
 /w/new/v4l-dvb/v4l/pvrusb2-hdw.c: In function 'pvr2_hdw_load_modules':
 /w/new/v4l-dvb/v4l/pvrusb2-hdw.c:2133: warning: format not a string
 literal and no format arguments
 make[3]: *** [/w/new/v4l-dvb/v4l/pvrusb2-hdw.o] Error 1
 make[2]: *** [_module_/w/new/v4l-dvb/v4l] Error 2
 
 It's probably due to this git commit:
 http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=3444b26afa145148951112534f298bdc554ec789
 
 I don't have idea how to fix it fast and correctly.

This might explain things a bit.  The following thread took place on 
linux-usb on 7-April:

quote

On Tue, 7 Apr 2009, Greg KH wrote:

 On Tue, Apr 07, 2009 at 05:31:55PM +, David Vrabel wrote:
  Wireless USB endpoint state has a sequence number and a current
  window and not just a single toggle bit.  So allow HCDs to provide a
  endpoint_reset method and call this or clear the software toggles as
  required (after a clear halt).
 
  usb_settoggle() and friends are then HCD internal and are moved into
  core/hcd.h.

 You remove this api, yet the pvrusb2 driver used it, and you don't seem
 to have resolved the issue where it was calling it:

  diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.c 
  b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
  index fa304e5..b86682d 100644
  --- a/drivers/media/video/pvrusb2/pvrusb2-hdw.c
  +++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
  @@ -1418,7 +1418,6 @@ static int pvr2_upload_firmware1(struct pvr2_hdw *hdw)
  return ret;
  }
 
  -   usb_settoggle(hdw-usb_dev, 0  0xf, !(0  USB_DIR_IN), 0);
  usb_clear_halt(hdw-usb_dev, usb_sndbulkpipe(hdw-usb_dev, 0  0x7f));
 
  pipe = usb_sndctrlpipe(hdw-usb_dev, 0);

 Should usb_reset_endpoint() be called here instead?


Speaking as the maintainer of that driver, I'm OK with accepting this
as-is for now.  This is a sequence that should not interfere with normal
driver operation.  I will look at this further a little later (not
likely before the merge window closes) and if this change turns out to
cause a problem I'll make a follow-up fix upstream.

Acked-By: Mike Isely is...@pobox.com

  -Mike

/quote

So the kernel already has this; it just needs to be pulled back into 
v4l-dvb.  It's an obvious trivial thing for now and I've acked it there.  
Obviously we're getting had here because you're compiling against a 
kernel snapshot that's been changed but v4l-dvb doesn't have the 
corresponding change in its local copy of the pvrusb2 driver.  Part of 
the fun of synchronizing changes from different trees :-(

Mauro:  If you just want to take this as-is (or find the git commit and 
pull it down), I'm fine.  Otherwise I'll set up a repo that you can pull 
from with this single-line change.  The above changeset also has the 
following attributions:

From: David Vrabel david.vra...@csr.com
Signed-off-by: David Vrabel david.vra...@csr.com

  -Mike



-- 

Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/6] ir-kbd-i2c: Switch to the new-style device binding model

2009-04-17 Thread Mike Isely

I thought we were going to leave the pvrusb2 driver out of this since 
I've already got a change ready that also includes additional logic to 
take into account the properties of the hardware device (i.e. only 
activate ir-kbd-i2c when we know it has a chance of working).

  -Mike


On Fri, 17 Apr 2009, Jean Delvare wrote:

 Let card drivers probe for IR receiver devices and instantiate them if
 found. Ultimately it would be better if we could stop probing
 completely, but I suspect this won't be possible for all card types.
 
 There's certainly room for cleanups. For example, some drivers are
 sharing I2C adapter IDs, so they also had to share the list of I2C
 addresses being probed for an IR receiver. Now that each driver
 explicitly says which addresses should be probed, maybe some addresses
 can be dropped from some drivers.
 
 Also, the special cases in saa7134-i2c should probably be handled on a
 per-board basis. This would be more efficient and less risky than always
 probing extra addresses on all boards. I'll give it a try later.
 
 Signed-off-by: Jean Delvare kh...@linux-fr.org
 Cc: Mauro Carvalho Chehab mche...@infradead.org
 Cc: Hans Verkuil hverk...@xs4all.nl
 Cc: Andy Walls awa...@radix.net
 Cc: Mike Isely is...@pobox.com
 ---
  linux/drivers/media/video/bt8xx/bttv-i2c.c   |   21 +
  linux/drivers/media/video/cx231xx/cx231xx-cards.c|   11 
  linux/drivers/media/video/cx231xx/cx231xx-i2c.c  |3 
  linux/drivers/media/video/cx231xx/cx231xx.h  |1 
  linux/drivers/media/video/cx23885/cx23885-i2c.c  |   12 +
  linux/drivers/media/video/cx88/cx88-i2c.c|   13 +
  linux/drivers/media/video/em28xx/em28xx-cards.c  |   20 +
  linux/drivers/media/video/em28xx/em28xx-i2c.c|3 
  linux/drivers/media/video/em28xx/em28xx-input.c  |6 
  linux/drivers/media/video/em28xx/em28xx.h|1 
  linux/drivers/media/video/ir-kbd-i2c.c   |  200 
 +++---
  linux/drivers/media/video/ivtv/ivtv-i2c.c|   31 ++
  linux/drivers/media/video/pvrusb2/pvrusb2-i2c-core.c |   24 ++
  linux/drivers/media/video/saa7134/saa7134-i2c.c  |3 
  linux/drivers/media/video/saa7134/saa7134-input.c|   86 ++-
  linux/drivers/media/video/saa7134/saa7134.h  |1 
  linux/include/media/ir-kbd-i2c.h |2 
  17 files changed, 244 insertions(+), 194 deletions(-)

   [...]

-- 

Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] Anticipating lirc breakage

2009-04-08 Thread Mike Isely
On Tue, 7 Apr 2009, Jean Delvare wrote:

 I'll rework my patch set to implement strategy #1 and post it when I'm
 done. As far as I can see this should be very similar to my original
 attempt, with just ir_video devices instead or ir-kbd devices, and
 also fixes for the minor issues that have been reported.
 
 Do you want me to include pvrusb2 in my new patch set, or should I still
 leave it to you?

If you were to include pvrusb2, you would also need the changeset which 
expands the IR scheme handling to make it possible to correctly select 
when to bind the driver.  But Mauro hasn't pulled that so you can't 
easily build on it right now.  And as I understand it, the only real 
impact in the second changeset in the pending series is just the name of 
the module (i.e. change ir-kbd to ir_video).

It's extra work for you to do this.  So just let me deal with it.  If my 
understanding above is correct, I'll just fix the second patch and the 
pvrusb2 driver should be ready to go for this.

  -Mike


-- 

Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   >