Re: [PATCH v2 53/58] firewire: don't break long lines

2016-10-19 Thread Stefan Richter
On Oct 19 Mauro Carvalho Chehab wrote:
> Em Wed, 19 Oct 2016 08:03:01 +0900
> Takashi Sakamoto <o-taka...@sakamocchi.jp> escreveu:
> > A structure for firedtv (struct firedtv) has a member for a pointer to
> > struct device. In this case, we can use dev_dbg() for debug printing.
[...]
> Stefan,
> 
> Would the above be OK for you?

ACK.
-- 
Stefan Richter
-==- =-=- =-=--
http://arcgraph.de/sr/
--
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 53/58] firewire: don't break long lines

2016-10-19 Thread Stefan Richter
On Oct 19 Takashi Sakamoto wrote:
> --- a/drivers/media/firewire/firedtv-rc.c
> +++ b/drivers/media/firewire/firedtv-rc.c
> @@ -184,8 +184,9 @@ void fdtv_handle_rc(struct firedtv *fdtv, unsigned
> int code)
>   else if (code >= 0x4540 && code <= 0x4542)
>   code = oldtable[code - 0x4521];
>   else {
> - printk(KERN_DEBUG "firedtv: invalid key code 0x%04x "
> -"from remote control\n", code);
> + dev_dbg(fdtv->device,
> + "invalid key code 0x%04x from remote control\n",
> + code);
>   return;
>   }
> 

Yes, dev_XYZ(fdtv->device, ...) is better here and is already used this
way throughout the firedtv driver.  firedtv-rc.c somehow fell through the
cracks when firedtv was made to use dev_XYZ().

(On an unrelated note, this reminds me that I still need to take care of
Mauro's patches "Add a keymap for FireDTV board" and "firedtv: Port it to
use rc_core" from May 28, 2012.)
-- 
Stefan Richter
-==- =-=- =--==
http://arcgraph.de/sr/
--
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 03/57] [media] firewire: don't break long lines

2016-10-15 Thread Stefan Richter
On Oct 15 Takashi Sakamoto wrote:
> On Oct 15 2016 05:19, Mauro Carvalho Chehab wrote:
> > Due to the 80-cols checkpatch warnings, several strings
> > were broken into multiple lines. This is not considered
> > a good practice anymore, as it makes harder to grep for
> > strings at the source code. So, join those continuation
> > lines.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>  
> 
> I prefer this patch because of the same reason in patch comment.
> 
> Reviewed-by: Takashi Sakamoto <o-taka...@sakamocchi.jp>

Acked-by: Stefan Richter <stef...@s5r6.in-berlin.de>

> > ---
> >  drivers/media/firewire/firedtv-avc.c | 5 +++--
> >  drivers/media/firewire/firedtv-rc.c  | 5 +++--
> >  2 files changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/firewire/firedtv-avc.c 
> > b/drivers/media/firewire/firedtv-avc.c
> > index 251a556112a9..e04235ea23fb 100644
> > --- a/drivers/media/firewire/firedtv-avc.c
> > +++ b/drivers/media/firewire/firedtv-avc.c
> > @@ -1181,8 +1181,9 @@ int avc_ca_pmt(struct firedtv *fdtv, char *msg, int 
> > length)
> > if (es_info_length > 0) {
> > pmt_cmd_id = msg[read_pos++];
> > if (pmt_cmd_id != 1 && pmt_cmd_id != 4)
> > -   dev_err(fdtv->device, "invalid pmt_cmd_id %d "
> > -   "at stream level\n", pmt_cmd_id);
> > +   dev_err(fdtv->device,
> > +   "invalid pmt_cmd_id %d at stream 
> > level\n",
> > +   pmt_cmd_id);
> >  
> > if (es_info_length > sizeof(c->operand) - 4 -
> >  write_pos) {
> > diff --git a/drivers/media/firewire/firedtv-rc.c 
> > b/drivers/media/firewire/firedtv-rc.c
> > index f82d4a93feb3..babfb9cee20e 100644
> > --- a/drivers/media/firewire/firedtv-rc.c
> > +++ b/drivers/media/firewire/firedtv-rc.c
> > @@ -184,8 +184,9 @@ void fdtv_handle_rc(struct firedtv *fdtv, unsigned int 
> > code)
> > else if (code >= 0x4540 && code <= 0x4542)
> > code = oldtable[code - 0x4521];
> > else {
> > -   printk(KERN_DEBUG "firedtv: invalid key code 0x%04x "
> > -  "from remote control\n", code);
> > +   printk(KERN_DEBUG
> > +      "firedtv: invalid key code 0x%04x from remote control\n",
> > +  code);
> > return;
> > }  
> 
> 
> Regards
> 
> Takashi Sakamoto
> --
> 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



-- 
Stefan Richter
-==- =-=- -
http://arcgraph.de/sr/
--
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 00/57] don't break long lines on strings

2016-10-15 Thread Stefan Richter
On Oct 14 Mauro Carvalho Chehab wrote:
> There are lots of drivers on media that breaks long line strings in order to
> fit into 80 columns. This was an usual practice to make checkpatch happy.

This was practice even before checkpatch existed.
-- 
Stefan Richter
-==- =-=- -
http://arcgraph.de/sr/
--
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 18/26] [media] dvb: Get rid of typedev usage for enums

2015-06-08 Thread Stefan Richter
On Jun 08 Mauro Carvalho Chehab wrote:
 The DVB API was originally defined using typedefs. This is against
 Kernel CodingStyle, and there's no good usage here. While we can't
 remove its usage on userspace, we can avoid its usage in Kernelspace.
[...]

Acked-by: Stefan Richter stef...@s5r6.in-berlin.de (drivers/media/firewire/*)
-- 
Stefan Richter
-=-= -==- -=---
http://arcgraph.de/sr/
--
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:firewire:Remove unneeded function definition,avc_tuner_host2ca

2015-02-08 Thread Stefan Richter
On Feb 08 Nicholas Krause wrote:
 Removes the function,avc_tuner_host2ca and the ifdef marco statements
 around this function as there are no more callers of this function
 and therefor no need for it's definition anymore.
 
 Signed-off-by: Nicholas Krause xerofo...@gmail.com

Your changelog is incorrect.  If you had checked the history, you would
have learned that the function was added without callers in the first
place and never gained any callers since then.

How about first checking what the Host2CA AV/C command is for, and then
deciding on whether to add proper usage of it or to drop its implementation
from the source for good.

 ---
  drivers/media/firewire/firedtv-avc.c | 31 ---
  1 file changed, 31 deletions(-)
 
 diff --git a/drivers/media/firewire/firedtv-avc.c 
 b/drivers/media/firewire/firedtv-avc.c
 index 251a556..a7f2617 100644
 --- a/drivers/media/firewire/firedtv-avc.c
 +++ b/drivers/media/firewire/firedtv-avc.c
 @@ -912,37 +912,6 @@ void avc_remote_ctrl_work(struct work_struct *work)
   avc_register_remote_control(fdtv);
  }
  
 -#if 0 /* FIXME: unused */
 -int avc_tuner_host2ca(struct firedtv *fdtv)
 -{
 - struct avc_command_frame *c = (void *)fdtv-avc_data;
 - int ret;
 -
 - mutex_lock(fdtv-avc_mutex);
 -
 - c-ctype   = AVC_CTYPE_CONTROL;
 - c-subunit = AVC_SUBUNIT_TYPE_TUNER | fdtv-subunit;
 - c-opcode  = AVC_OPCODE_VENDOR;
 -
 - c-operand[0] = SFE_VENDOR_DE_COMPANYID_0;
 - c-operand[1] = SFE_VENDOR_DE_COMPANYID_1;
 - c-operand[2] = SFE_VENDOR_DE_COMPANYID_2;
 - c-operand[3] = SFE_VENDOR_OPCODE_HOST2CA;
 - c-operand[4] = 0; /* slot */
 - c-operand[5] = SFE_VENDOR_TAG_CA_APPLICATION_INFO; /* ca tag */
 - clear_operands(c, 6, 8);
 -
 - fdtv-avc_data_length = 12;
 - ret = avc_write(fdtv);
 -
 - /* FIXME: check response code? */
 -
 - mutex_unlock(fdtv-avc_mutex);
 -
 - return ret;
 -}
 -#endif
 -
  static int get_ca_object_pos(struct avc_response_frame *r)
  {
   int length = 1;



-- 
Stefan Richter
-=-= --=- -=---
http://arcgraph.de/sr/
--
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:firewire:Remove unneeded function definition,avc_tuner_host2ca in firedtv-avc.c

2015-02-08 Thread Stefan Richter
On Feb 08 nick wrote:
 On 2015-02-08 06:55 PM, Stefan Richter wrote:
  I still am missing research on the question whether or not the Common
  Interface serving part of the driver needs to send Host2CA commands.  If
  yes, we implement it and use the function.  If not, we remove the
  function.  As long as we are not sure, I prefer to leave the #if-0'd code
  where it is.  It documents how the command is formed, and we don't have
  any other documentation (except perhaps the git history).
[...]
 Stefan,
 I looked in the history with git log -p 154907957f939 and all I got 
 for this function was 
  Wed Feb 11 21:21:04 CET 2009
 firedtv: avc: header file cleanup
 
 Remove unused constants and declarations.
 Move privately used constants into .c files.

The function was added a few commits before this one, by firesat: update
isochronous interface, add CI support.

 Clearly this states to remove unused declarations and avc_tuner_host2ca is 
 unused.
 Can you explain to me then why it's still needed to be around if there no 
 callers
 of it?

See above; in this instance

#if 0
dead code
#endif

stands in for

/*
 * pseudo code
 */
-- 
Stefan Richter
-=-= --=- -=--=
http://arcgraph.de/sr/
--
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:firewire:Remove unneeded function definition,avc_tuner_host2ca in firedtv-avc.c

2015-02-08 Thread Stefan Richter
On Feb 08 Nicholas Krause wrote:
 Removes the unneeded function defintion,avc_tuner_host2ca in the file, 
 firedtv-avc. This
 function should have been removed during the refactoring of the firetv code 
 base during
 commit id,154907957f939 due to us removing unneeded definitions of functions 
 not called
 when moving private function defintions from firedtv-avc.h to the 
 file,firedtv-avc.c
 for the driver supporting firedtv enabled hardware respectfully.
 
 Signed-off-by: Nicholas Krause xerofo...@gmail.com

I still am missing research on the question whether or not the Common
Interface serving part of the driver needs to send Host2CA commands.  If
yes, we implement it and use the function.  If not, we remove the
function.  As long as we are not sure, I prefer to leave the #if-0'd code
where it is.  It documents how the command is formed, and we don't have
any other documentation (except perhaps the git history).

On a more general note, as others told you before, please stop sending
patches that were created without any research on your part.  Thank you.

 ---
  drivers/media/firewire/firedtv-avc.c | 31 ---
  1 file changed, 31 deletions(-)
 
 diff --git a/drivers/media/firewire/firedtv-avc.c 
 b/drivers/media/firewire/firedtv-avc.c
 index 251a556..a7f2617 100644
 --- a/drivers/media/firewire/firedtv-avc.c
 +++ b/drivers/media/firewire/firedtv-avc.c
 @@ -912,37 +912,6 @@ void avc_remote_ctrl_work(struct work_struct *work)
   avc_register_remote_control(fdtv);
  }
  
 -#if 0 /* FIXME: unused */
 -int avc_tuner_host2ca(struct firedtv *fdtv)
 -{
 - struct avc_command_frame *c = (void *)fdtv-avc_data;
 - int ret;
 -
 - mutex_lock(fdtv-avc_mutex);
 -
 - c-ctype   = AVC_CTYPE_CONTROL;
 - c-subunit = AVC_SUBUNIT_TYPE_TUNER | fdtv-subunit;
 - c-opcode  = AVC_OPCODE_VENDOR;
 -
 - c-operand[0] = SFE_VENDOR_DE_COMPANYID_0;
 - c-operand[1] = SFE_VENDOR_DE_COMPANYID_1;
 - c-operand[2] = SFE_VENDOR_DE_COMPANYID_2;
 - c-operand[3] = SFE_VENDOR_OPCODE_HOST2CA;
 - c-operand[4] = 0; /* slot */
 - c-operand[5] = SFE_VENDOR_TAG_CA_APPLICATION_INFO; /* ca tag */
 - clear_operands(c, 6, 8);
 -
 - fdtv-avc_data_length = 12;
 - ret = avc_write(fdtv);
 -
 - /* FIXME: check response code? */
 -
 - mutex_unlock(fdtv-avc_mutex);
 -
 - return ret;
 -}
 -#endif
 -
  static int get_ca_object_pos(struct avc_response_frame *r)
  {
   int length = 1;



-- 
Stefan Richter
-=-= --=- -=--=
http://arcgraph.de/sr/
--
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/1] [media] firewire: Deletion of an unnecessary check before the function call dvb_unregister_device

2014-11-20 Thread Stefan Richter
On Nov 20 SF Markus Elfring wrote:
 From: Markus Elfring elfr...@users.sourceforge.net
 Date: Thu, 20 Nov 2014 10:49:07 +0100
 
 The dvb_unregister_device() function tests whether its argument is NULL
 and then returns immediately. Thus the test around the call is not needed.
 
 This issue was detected by using the Coccinelle software.
 
 Signed-off-by: Markus Elfring elfr...@users.sourceforge.net

Reviewed-by: Stefan Richter stef...@s5r6.in-berlin.de

 ---
  drivers/media/firewire/firedtv-ci.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)
 
 diff --git a/drivers/media/firewire/firedtv-ci.c 
 b/drivers/media/firewire/firedtv-ci.c
 index e5ebdbf..e63f582 100644
 --- a/drivers/media/firewire/firedtv-ci.c
 +++ b/drivers/media/firewire/firedtv-ci.c
 @@ -253,6 +253,5 @@ int fdtv_ca_register(struct firedtv *fdtv)
  
  void fdtv_ca_release(struct firedtv *fdtv)
  {
 - if (fdtv-cadev)
 - dvb_unregister_device(fdtv-cadev);
 + dvb_unregister_device(fdtv-cadev);
  }



-- 
Stefan Richter
-=-- =-== =-=--
http://arcgraph.de/sr/
--
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] fix a warning on avr32 arch

2014-10-30 Thread Stefan Richter
On Oct 30 Mauro Carvalho Chehab wrote:
 on avr32 arch, those warnings happen:
   drivers/media/firewire/firedtv-fw.c: In function 'node_update':
   drivers/media/firewire/firedtv-fw.c:329: warning: comparison is always 
 true due to limited range of data type
 
 In this particular case, the signal is desired, as the isochannel
 var can be initalized with -1 inside the driver.
 
 So, change the type to s8, to avoid issues on archs where char
 is unsigned.
 
 Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com

Reviewed-by: Stefan Richter stef...@s5r6.in-berlin.de

 
 diff --git a/drivers/media/firewire/firedtv.h 
 b/drivers/media/firewire/firedtv.h
 index c2ba085e0d20..346a85be6de2 100644
 --- a/drivers/media/firewire/firedtv.h
 +++ b/drivers/media/firewire/firedtv.h
 @@ -96,7 +96,7 @@ struct firedtv {
  
   enum model_type type;
   charsubunit;
 - charisochannel;
 + s8  isochannel;
   struct fdtv_ir_context  *ir_context;
  
   fe_sec_voltage_tvoltage;

-- 
Stefan Richter
-=-- =-=- -
http://arcgraph.de/sr/
--
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] firewire: firedtv-avc: potential buffer overflow

2014-09-08 Thread Stefan Richter
On Sep 08 Dan Carpenter wrote:
 program_info_length is user controlled and can go up to 4095.  The
 operand[] array has 509 bytes so we need to add a limit here to prevent
 buffer overflows.
 
 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

Reviewed-by: Stefan Richter stef...@s5r6.in-berlin.de

Thank you.

 
 diff --git a/drivers/media/firewire/firedtv-avc.c 
 b/drivers/media/firewire/firedtv-avc.c
 index d1a1a13..ac17567 100644
 --- a/drivers/media/firewire/firedtv-avc.c
 +++ b/drivers/media/firewire/firedtv-avc.c
 @@ -1157,6 +1157,10 @@ int avc_ca_pmt(struct firedtv *fdtv, char *msg, int 
 length)
   if (pmt_cmd_id != 1  pmt_cmd_id != 4)
   dev_err(fdtv-device,
   invalid pmt_cmd_id %d\n, pmt_cmd_id);
 + if (program_info_length  sizeof(c-operand) - write_pos) {
 + ret = -EINVAL;
 + goto out;
 + }
  
   memcpy(c-operand[write_pos], msg[read_pos],
  program_info_length);
 @@ -1180,6 +1184,11 @@ int avc_ca_pmt(struct firedtv *fdtv, char *msg, int 
 length)
   dev_err(fdtv-device, invalid pmt_cmd_id %d 
   at stream level\n, pmt_cmd_id);
  
 + if (es_info_length  sizeof(c-operand) - write_pos) {
 + ret = -EINVAL;
 + goto out;
 + }
 +
   memcpy(c-operand[write_pos], msg[read_pos],
  es_info_length);
   read_pos += es_info_length;

-- 
Stefan Richter
-=-- =--= -=---
http://arcgraph.de/sr/
--
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] firewire: firedtv-avc: potential buffer overflow

2014-09-08 Thread Stefan Richter
On Sep 08 Stefan Richter wrote:
 On Sep 08 Dan Carpenter wrote:
  program_info_length is user controlled and can go up to 4095.  The
  operand[] array has 509 bytes so we need to add a limit here to prevent
  buffer overflows.
  
  Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
 
 Reviewed-by: Stefan Richter stef...@s5r6.in-berlin.de
 
 Thank you.

Oops, that was a bit too quick.  After the memcpy() accesses which you
protect, there are another four bytes written, still without checking
the bounds.

  
  diff --git a/drivers/media/firewire/firedtv-avc.c 
  b/drivers/media/firewire/firedtv-avc.c
  index d1a1a13..ac17567 100644
  --- a/drivers/media/firewire/firedtv-avc.c
  +++ b/drivers/media/firewire/firedtv-avc.c
  @@ -1157,6 +1157,10 @@ int avc_ca_pmt(struct firedtv *fdtv, char *msg, int 
  length)
  if (pmt_cmd_id != 1  pmt_cmd_id != 4)
  dev_err(fdtv-device,
  invalid pmt_cmd_id %d\n, pmt_cmd_id);
  +   if (program_info_length  sizeof(c-operand) - write_pos) {

So I suggest something like this instead:

+   if (program_info_length  sizeof(c-operand) - 4 - write_pos) {

  +   ret = -EINVAL;
  +   goto out;
  +   }
   
  memcpy(c-operand[write_pos], msg[read_pos],
 program_info_length);
  @@ -1180,6 +1184,11 @@ int avc_ca_pmt(struct firedtv *fdtv, char *msg, int 
  length)
  dev_err(fdtv-device, invalid pmt_cmd_id %d 
  at stream level\n, pmt_cmd_id);
   
  +   if (es_info_length  sizeof(c-operand) - write_pos) {

And likewise:

+   if (es_info_length  sizeof(c-operand) - 4 - 
write_pos) {

  +   ret = -EINVAL;
  +   goto out;
  +   }
  +
  memcpy(c-operand[write_pos], msg[read_pos],
 es_info_length);
  read_pos += es_info_length;

FYI, after this follows:

write_pos += es_info_length;
}
}
write_pos += 4; /* CRC */

c-operand[7] = 0x82;
c-operand[8] = (write_pos - 10)  8;
c-operand[9] = (write_pos - 10)  0xff;
c-operand[14] = write_pos - 15;

crc32_csum = crc32_be(0, c-operand[10], c-operand[12] - 1);
c-operand[write_pos - 4] = (crc32_csum  24)  0xff;
c-operand[write_pos - 3] = (crc32_csum  16)  0xff;
c-operand[write_pos - 2] = (crc32_csum   8)  0xff;
c-operand[write_pos - 1] = (crc32_csum   0)  0xff;
pad_operands(c, write_pos);

fdtv-avc_data_length = ALIGN(3 + write_pos, 4);
ret = avc_write(fdtv);


And pad_operands() is defined in the same source file as:

#define LAST_OPERAND (509 - 1)

static inline void clear_operands(struct avc_command_frame *c, int from, int to)
{
memset(c-operand[from], 0, to - from + 1);
}

static void pad_operands(struct avc_command_frame *c, int from)
{
int to = ALIGN(from, 4);

if (from = to  to = LAST_OPERAND)
clear_operands(c, from, to);
}

BTW, the calculation of to in pad_operands appears to be wrong, but this does
not affect Dan's patch.  I will send an extra patch for that.

Regards,
-- 
Stefan Richter
-=-- =--= -=---
http://arcgraph.de/sr/
--
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/3] femon: Display SNR in dB

2013-11-25 Thread Stefan Richter
On Nov 25 Stefan Richter wrote:
 On Nov 25 Jean Delvare wrote:
  Hi Manu,
  
  On Sun, 24 Nov 2013 22:51:33 +0530, Manu Abraham wrote:
   Sorry, that I came upon this patch quite late.
  
  No problem, better late than never! :)
  
   On Mon, Jun 3, 2013 at 8:51 PM, Jean Delvare kh...@linux-fr.org wrote:
SNR is supposed to be reported by the frontend drivers in dB, so print
it that way for drivers which implement it properly.
   
   Not all frontends do report report the SNR in dB. Well, You can say quite
   some frontends do report it that way.
  
  Last time I discussed this, I was told that this was the preferred way
  for frontends to report the SNR. I also referred to this document:
http://palosaari.fi/linux/v4l-dvb/snr_2012-05-21.txt
  I don't know now up-to-date it is by now, but back then it showed a
  significant number of frontends reporting in .1 dB already, including
  the ones I'm using right now (drx-3916k and drx-3913k.) With the
  current version of femon, femon -H reports it as 0%, which is quite
  useless. Thus my patch.
 [...]
 
 Hi,
 
 I inherited this in drivers/media/firewire/firedtv-fe.c:
 
 static int fdtv_read_snr(struct dvb_frontend *fe, u16 *snr)
 {
   struct firedtv *fdtv = fe-sec_priv;
   struct firedtv_tuner_status stat;
 
   if (avc_tuner_status(fdtv, stat))
   return -EINVAL;
 
   /* C/N[dB] = -10 * log10(snr / 65535) */
   *snr = stat.carrier_noise_ratio * 257;
   return 0;
 }
 
 As far as I understand, the comment should have been written with a FIXME
 prefix.
 
 I have no documentation and no personal manufacturer contact (and the
 devices are EOL).  All I know from the driver source is that we do get a 16
 bits wide carrier_noise_ratio.  So it appears to be something on a scale
 from 0x to 0x, and the comment makes it look like being on a linear
 scale originally.

Or I got it wrong and all the comment tries to tell that the value to be
returned in the snr argument is meant to be linear on a scale of 0...,
and the code tells that we get a linear value on a scale of 0...255 from
the firmware.  (Cc'ing Henrik who added the code and the comment.)

 I could cross-check with a Windows based TV viewer application what signal
 strength value is presented there to the user with DVB-T and DVB-S2
 incarnations of FireDTV devices.  Right now I don't remember how that
 application presents it (i.e. as percentage or dB or whatever...).
 When I looked at that application and at kaffeine some years ago, they
 displayed grossly different values.  I did not research back then whether
 the Linux driver or kaffeine or both treated it wrong.
 
 Any advice for the quoted kernel driver code?
 
 Thanks,

-- 
Stefan Richter
-=-===-= =-== ==--=
http://arcgraph.de/sr/
--
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/3] femon: Display SNR in dB

2013-11-25 Thread Stefan Richter
On Nov 25 Jean Delvare wrote:
 Hi Manu,
 
 On Sun, 24 Nov 2013 22:51:33 +0530, Manu Abraham wrote:
  Sorry, that I came upon this patch quite late.
 
 No problem, better late than never! :)
 
  On Mon, Jun 3, 2013 at 8:51 PM, Jean Delvare kh...@linux-fr.org wrote:
   SNR is supposed to be reported by the frontend drivers in dB, so print
   it that way for drivers which implement it properly.
  
  Not all frontends do report report the SNR in dB. Well, You can say quite
  some frontends do report it that way.
 
 Last time I discussed this, I was told that this was the preferred way
 for frontends to report the SNR. I also referred to this document:
   http://palosaari.fi/linux/v4l-dvb/snr_2012-05-21.txt
 I don't know now up-to-date it is by now, but back then it showed a
 significant number of frontends reporting in .1 dB already, including
 the ones I'm using right now (drx-3916k and drx-3913k.) With the
 current version of femon, femon -H reports it as 0%, which is quite
 useless. Thus my patch.
[...]

Hi,

I inherited this in drivers/media/firewire/firedtv-fe.c:

static int fdtv_read_snr(struct dvb_frontend *fe, u16 *snr)
{
struct firedtv *fdtv = fe-sec_priv;
struct firedtv_tuner_status stat;

if (avc_tuner_status(fdtv, stat))
return -EINVAL;

/* C/N[dB] = -10 * log10(snr / 65535) */
*snr = stat.carrier_noise_ratio * 257;
return 0;
}

As far as I understand, the comment should have been written with a FIXME
prefix.

I have no documentation and no personal manufacturer contact (and the
devices are EOL).  All I know from the driver source is that we do get a 16
bits wide carrier_noise_ratio.  So it appears to be something on a scale
from 0x to 0x, and the comment makes it look like being on a linear
scale originally.

I could cross-check with a Windows based TV viewer application what signal
strength value is presented there to the user with DVB-T and DVB-S2
incarnations of FireDTV devices.  Right now I don't remember how that
application presents it (i.e. as percentage or dB or whatever...).
When I looked at that application and at kaffeine some years ago, they
displayed grossly different values.  I did not research back then whether
the Linux driver or kaffeine or both treated it wrong.

Any advice for the quoted kernel driver code?

Thanks,
-- 
Stefan Richter
-=-===-= =-== ==--=
http://arcgraph.de/sr/
--
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] driver: firewire: remove unnecessary #if 0 code

2013-09-24 Thread Stefan Richter
On Sep 22 Govindarajulu Varadarajan wrote:
 Signed-off-by: Govindarajulu Varadarajan govindarajul...@gmail.com
 ---
  drivers/media/firewire/firedtv-avc.c | 41 
 
  1 file changed, 41 deletions(-)
 
 diff --git a/drivers/media/firewire/firedtv-avc.c 
 b/drivers/media/firewire/firedtv-avc.c
 index d1a1a13..786e273 100644
 --- a/drivers/media/firewire/firedtv-avc.c
 +++ b/drivers/media/firewire/firedtv-avc.c
 @@ -912,37 +912,6 @@ void avc_remote_ctrl_work(struct work_struct *work)
   avc_register_remote_control(fdtv);
  }
  
 -#if 0 /* FIXME: unused */
 -int avc_tuner_host2ca(struct firedtv *fdtv)
 -{
 - struct avc_command_frame *c = (void *)fdtv-avc_data;
 - int ret;
 -
 - mutex_lock(fdtv-avc_mutex);
 -
 - c-ctype   = AVC_CTYPE_CONTROL;
 - c-subunit = AVC_SUBUNIT_TYPE_TUNER | fdtv-subunit;
 - c-opcode  = AVC_OPCODE_VENDOR;
 -
 - c-operand[0] = SFE_VENDOR_DE_COMPANYID_0;
 - c-operand[1] = SFE_VENDOR_DE_COMPANYID_1;
 - c-operand[2] = SFE_VENDOR_DE_COMPANYID_2;
 - c-operand[3] = SFE_VENDOR_OPCODE_HOST2CA;
 - c-operand[4] = 0; /* slot */
 - c-operand[5] = SFE_VENDOR_TAG_CA_APPLICATION_INFO; /* ca tag */
 - clear_operands(c, 6, 8);
 -
 - fdtv-avc_data_length = 12;
 - ret = avc_write(fdtv);
 -
 - /* FIXME: check response code? */
 -
 - mutex_unlock(fdtv-avc_mutex);
 -
 - return ret;
 -}
 -#endif
 -
  static int get_ca_object_pos(struct avc_response_frame *r)
  {
   int length = 1;
 @@ -955,16 +924,6 @@ static int get_ca_object_pos(struct avc_response_frame 
 *r)
  
  static int get_ca_object_length(struct avc_response_frame *r)
  {
 -#if 0 /* FIXME: unused */
 - int size = 0;
 - int i;
 -
 - if (r-operand[7]  0x80)
 - for (i = 0; i  (r-operand[7]  0x7f); i++) {
 - size = 8;
 - size += r-operand[8 + i];
 - }
 -#endif
   return r-operand[7];
  }
  

Hi Henrik,

get_ca_object_pos() is used in avc_ca_get_mmi() when the AVC response
frame is inspected.  Do you know whether we should fix up this code for
support of length  127, or can we just delete this #if 0 block?
-- 
Stefan Richter
-=-===-= =--= ==---
http://arcgraph.de/sr/
--
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] net: add ETH_P_802_3_MIN

2013-03-21 Thread Stefan Richter
On Mar 21 Simon Horman wrote:
 Add a new constant ETH_P_802_3_MIN, the minimum ethernet type for
 an 802.3 frame. Frames with a lower value in the ethernet type field
 are Ethernet II.
 
 Also update all the users of this value that I could find to use the
 new constant.
 
 I anticipate adding some more users of this constant when
 adding MPLS support to Open vSwtich.
 
 As suggested by Jesse Gross.
 
 Compile tested only.
 
 Cc: Jesse Gross je...@nicira.com
 Cc: Stefan Richter stef...@s5r6.in-berlin.de
 Cc: Karsten Keil i...@linux-pingi.de
 Cc: Mauro Carvalho Chehab mche...@redhat.com
 Cc: linux1394-de...@lists.sourceforge.net
 Cc: linux-media@vger.kernel.org
 Cc: d...@openvswitch.org
 Signed-off-by: Simon Horman ho...@verge.net.au
 ---
  drivers/firewire/net.c   |2 +-

Acked-by: Stefan Richter stef...@s5r6.in-berlin.de

  drivers/isdn/i4l/isdn_net.c  |2 +-
  drivers/media/dvb-core/dvb_net.c |6 +++---
  drivers/net/ethernet/sun/niu.c   |2 +-
  drivers/net/plip/plip.c  |2 +-
  include/uapi/linux/if_ether.h|3 +++
  net/ethernet/eth.c   |2 +-
  net/openvswitch/datapath.c   |2 +-
  8 files changed, 12 insertions(+), 9 deletions(-)
 
 diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
 index 2b27bff..bd34ca1 100644
 --- a/drivers/firewire/net.c
 +++ b/drivers/firewire/net.c
 @@ -630,7 +630,7 @@ static int fwnet_finish_incoming_packet(struct net_device 
 *net,
   if (memcmp(eth-h_dest, net-dev_addr, net-addr_len))
   skb-pkt_type = PACKET_OTHERHOST;
   }
 - if (ntohs(eth-h_proto) = 1536) {
 + if (ntohs(eth-h_proto) = ETH_P_802_3_MIN) {
   protocol = eth-h_proto;
   } else {
   rawp = (u16 *)skb-data;
[...]

-- 
Stefan Richter
-=-===-= --== =-=-=
http://arcgraph.de/sr/
--
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] firedtv: add MAINTAINERS entry

2012-11-03 Thread Stefan Richter
There is currently discussion to add MAINTAINERS records for media
drivers that don't have one yet, possibly with 'orphan' or 'odd fixes'
status.  Here is a proper entry for the firedtv driver (for 1394
attached DVB STBs and 1394 attached DVB cards from Digital Everywhere).

The L: linux-media and T: linux-media.git lines in this entry are
redundant to what scripts/get_maintainer.pl would show automatically but
I added them for folks who read MAINTAINERS directly.  The (firedtv)
string is for those folks as well if they look for driver name rather
than file path.

The F: drivers/media/firewire/ pattern and the FireWire media drivers
title are currently synonymous with firedtv.  If more drivers get added
there, this can be revisited.

I don't have documentation or DVB-S2 devices to test, but I have DVB-C
and DVB-T devices for testing.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
 MAINTAINERS |8 
 1 file changed, 8 insertions(+)

--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3041,6 +3041,14 @@ T:   git git://git.alsa-project.org/alsa-k
 S: Maintained
 F: sound/firewire/
 
+FIREWIRE MEDIA DRIVERS (firedtv)
+M: Stefan Richter stef...@s5r6.in-berlin.de
+L: linux-media@vger.kernel.org
+L: linux1394-de...@lists.sourceforge.net
+T: git 
git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media.git
+S: Maintained
+F: drivers/media/firewire/
+
 FIREWIRE SBP-2 TARGET
 M: Chris Boot bo...@bootc.net
 L: linux-s...@vger.kernel.org



On Nov 02 Mauro Carvalho Chehab wrote:
 Em Thu, 1 Nov 2012 14:12:44 -0200
 Mauro Carvalho Chehab mche...@redhat.com escreveu:
 
  Em Thu, 1 Nov 2012 16:44:50 +0100
  Hans Verkuil hverk...@xs4all.nl escreveu:
  

(thread [media-workshop] Tentative Agenda for the November workshop)
   On Thu October 25 2012 19:27:01 Mauro Carvalho Chehab wrote:
I have an extra theme for discussions there: what should we do with the 
drivers
that don't have any MAINTAINERS entry.
[...]
It probably makes sense to mark them as Orphan (or, at least, have 
some
criteria to mark them as such). Perhaps before doing that, we could try
to see if are there any developer at the community with time and 
patience
to handle them.

This could of course be handled as part of the discussions about how to 
improve
the merge process, but I suspect that this could generate enough 
discussions
to be handled as a separate theme.
   
   Do we have a 'Maintainer-Light' category? I have a lot of hardware that I 
   can
   test. So while I wouldn't like to be marked as 'The Maintainer of driver 
   X'
   (since I simply don't have the time for that), I wouldn't mind being 
   marked as
   someone who can at least test patches if needed.
  
  There are several maintainance status there: 
  
  S: Status, one of the following:
 Supported:   Someone is actually paid to look after this.
 Maintained:  Someone actually looks after it.
 Odd Fixes:   It has a maintainer but they don't have time to do
  much other than throw the odd patch in. See below..
 Orphan:  No current maintainer [but maybe you could take the
  role as you write your new code].
 Obsolete:Old code. Something tagged obsolete generally means
  it has been replaced by a better system and you
  should be using that.
  
  (btw, I just realized that I should be changing the EDAC drivers I maintain
   to Supported; the media drivers I maintain should be kept as Maintained).
  
  I suspect that the maintainer-light category for those radio and similar
  old stuff is likely Odd Fixes.
  
There are some issues by not having a MAINTAINERS entry:
- patches may not flow into the driver maintainer;
- patches will likely be applied without tests/reviews or may
  stay for a long time queued;
- ./scripts/get_maintainer.pl at --no-git-fallback won't return
  any maintainer[1].

[1] Letting get_maintainer.pl is very time/CPU consuming. Letting it 
would 
delay a lot the patch review process, if applied for every patch, with
unreliable and doubtful results. I don't do it, due to the large volume
of patches, and because the 'other' results aren't typically the driver
maintainer.

An example of this is the results for a patch I just applied
(changeset 2866aed103b915ca8ba0ff76d5790caea4e62ced):

$ git show --pretty=email|./scripts/get_maintainer.pl
[...]
The driver author (Hongjun Chen hong-jun.c...@freescale.com) was not 
even
shown there, and the co-author got only 15% hit, while I got 100% and 
Hans
got 57%.

This happens not only to this driver. In a matter of fact, on most 
cases where
no MAINTAINERS entry exist, the driver's author gets a very small hit 
chance

Re: [RFC PATCH 0/3] Improve Kconfig selection for media devices

2012-05-28 Thread Stefan Richter
On May 27 Mauro Carvalho Chehab wrote:
 The Kconfig building system is improperly selecting some drivers,
 like analog TV tuners even when this is not required.
 
 Rearrange the Kconfig in a way to prevent that.
 
 Mauro Carvalho Chehab (3):
   media: reorganize the main Kconfig items
   media: Remove VIDEO_MEDIA Kconfig option
   media: only show V4L devices based on device type selection

On 1/3 media: reorganize the main Kconfig items:

a) I agree with Sylvester that the MEDIA_WEBCAM_SUPP variable, prompt
text, and help text should be worded a bit more general.  Wouldn't this
variable also cover industrial cameras and who knows what other kinds of
video inputs?  I also agree with Sylvester about the SUPP vs. SUPPORT
thing.

b) Small typo in the MEDIA_ANALOG_TV_SUPP help text:  of - or.

c) The RC_CORE_SUPP help text gives the impression that RC core is
always needed if there is hardware with an IR feature.  But the firedtv
driver is a case where the driver directly works on top of the input
subsystem rather than on RC core.  Maybe there are more such cases.
(Currently we don't ask whether FireDTV owners want IR support; we
silently build the IR part of firedtv in if CONFIG_INPUT is set, and
silently omit the IR part of firedtv if CONFIG_INPUT was disabled, which
requires CONFIG_EXPERT.)

How about turning the Remote Controller support option into merely a
filter for standalone IR and RF receivers and transmitters, whereas
Kconfig options in the analog and digital TV categories silently do
select RC_CORE if INPUT for combined tuner + IR/RF rx/tx hardware?
-- 
Stefan Richter
-=-===-- -=-= ===--
http://arcgraph.de/sr/
--
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 0/3] Improve Kconfig selection for media devices

2012-05-28 Thread Stefan Richter
On May 28 Mauro Carvalho Chehab wrote:
 Em 28-05-2012 06:48, Stefan Richter escreveu:
  c) The RC_CORE_SUPP help text gives the impression that RC core is
  always needed if there is hardware with an IR feature.  But the firedtv
  driver is a case where the driver directly works on top of the input
  subsystem rather than on RC core.  Maybe there are more such cases.
 
 All other drivers use RC_CORE, as we've replaced the existing implementations
 to use it, removing bad/inconsistent IR code implementations everywhere.
 The only driver left is firedtv.
[...]
 The right thing to do is to convert drivers/media/dvb/firewire/firedtv-rc.c
 to use rc-core. There are several issues with the current implementation:
 
   - IR keycode tables are hardcoded;
   - There is a magic to convert a 16 bits scancode (NEC protocol?)
 into a key;
   - There's no way to replace the existing table to an user-provided
 one;

There are two tables:  An old mapping and a new mapping.  The new mapping
is copied into a newly allocated writable array.  It should be possible to
overwrite this array by means of EVIOCSKEYCODE ioctls.

If I remember correctly, the firedtv driver sources came only with the old
mapping table when they were submitted for upstream merge.  When I helped
to clean up the driver, I noticed that the two FireDTV C/CI and T/CI (which
I newly purchased at the time as test devices) emitted entirely different
scan codes than what the sources suggested.  I suppose the original driver
sources were written against older firmware or maybe older hardware
revisions, possibly even prototype hardware.  We would have to get hold of
the original authors if we wanted to find out.

Anyway, I implemented the new scancode-keycode mapping in a way that
followed Dimitry's (?) review advice at that time, but left the old
immutable mapping in there as fallback if an old scancode was received.

If it is a burden, we could rip out the old table and see if anybody
complains.

   - The IR userspace tools won't work, as it doesn't export the
 needed sysfs nodes to report an IR.

But at least keypad/ keyboard related userspace should work.

 If you want, I can write a patch doing that, but I can't test it here, as
 I don't have a firedtv device.

I can test such a patch as spare time permits if you point me to particular
tools that I should test.
-- 
Stefan Richter
-=-===-- -=-= ===--
http://arcgraph.de/sr/
--
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] firedtv: Port it to use rc_core

2012-05-28 Thread Stefan Richter
On May 28 Stefan Richter wrote:
  +   idev-phys = /ir0;/* FIXME */  
 
 Something similar to drivers/media/dvb/dvb-usb/dvb-usb-remote.c::
 
   usb_make_path(d-udev, d-rc_phys, sizeof(d-rc_phys));
   strlcat(d-rc_phys, /ir0, sizeof(d-rc_phys));
 
 should be implemented for this, right?

PS:
The current input device looks like this:

/sys/devices/pci:00/:00:02.0/:02:00.0/:03:01.0/:04:00.0/fw7/fw7.0/input/input8/device
 - ../../../fw7.0

fw7.0 is dev_name(dev) in fdtv_register_rc() or dev_name(fdtv-device)
in general in firedtv.

The last numeric name before fw7, i.e. :04:00.0, is the name of the PCI
device of the FireWire controller.  fw7 is the name of the FireDTV node;
fw7.0 is the name of the (only) unit within the FireDTV node which
implements the DVB receiver and IR receiver.  What would be needed from
this?

FWIW, usb_make_path() results in usb-%s-%s % (usb_device.bus.bus_name,
usb_device.devpath).
-- 
Stefan Richter
-=-===-- -=-= ===--
http://arcgraph.de/sr/
--
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 v3.5-rc1] media updates for v3.5

2012-05-25 Thread Stefan Richter
On May 25 Mauro Carvalho Chehab wrote:
 A simple way to solve it seems to make those options dependent on 
 CONFIG_EXPERT.
 
 Not sure if all usual distributions disable it, but I guess most won't have
 EXPERT enabled.
 
 The enclosed patch does that. If nobody complains, I'll submit it together
 with the next git pull request.

I only want dvb-core and firedtv.  But when I switch off
CONFIG_MEDIA_TUNER_CUSTOMISE, suddenly also

  CC [M]  drivers/media/common/tuners/tuner-xc2028.o
  CC [M]  drivers/media/common/tuners/tuner-simple.o
  CC [M]  drivers/media/common/tuners/tuner-types.o
  CC [M]  drivers/media/common/tuners/mt20xx.o
  CC [M]  drivers/media/common/tuners/tda8290.o
  CC [M]  drivers/media/common/tuners/tea5767.o
  CC [M]  drivers/media/common/tuners/tea5761.o
  CC [M]  drivers/media/common/tuners/tda9887.o
  CC [M]  drivers/media/common/tuners/tda827x.o
  CC [M]  drivers/media/common/tuners/tda18271-maps.o
  CC [M]  drivers/media/common/tuners/tda18271-common.o
  CC [M]  drivers/media/common/tuners/tda18271-fe.o
  CC [M]  drivers/media/common/tuners/xc5000.o
  CC [M]  drivers/media/common/tuners/xc4000.o
  CC [M]  drivers/media/common/tuners/mc44s803.o
  LD [M]  drivers/media/common/tuners/tda18271.o

are built.  Why is that?

$ grep DVB .config
CONFIG_DVB_CORE=m
# CONFIG_DVB_NET is not set
CONFIG_DVB_MAX_ADAPTERS=8
# CONFIG_DVB_DYNAMIC_MINORS is not set
CONFIG_DVB_CAPTURE_DRIVERS=y
# CONFIG_DVB_BUDGET_CORE is not set
# CONFIG_DVB_TTUSB_BUDGET is not set
# CONFIG_DVB_TTUSB_DEC is not set
# CONFIG_DVB_B2C2_FLEXCOP is not set
# CONFIG_DVB_PLUTO2 is not set
CONFIG_DVB_FIREDTV=m
CONFIG_DVB_FIREDTV_INPUT=y
# CONFIG_DVB_PT1 is not set
# CONFIG_DVB_NGENE is not set
# CONFIG_DVB_DDBRIDGE is not set
# Supported DVB Frontends
# CONFIG_DVB_FE_CUSTOMISE is not set
# DVB-S (satellite) frontends
# DVB-T (terrestrial) frontends
# DVB-C (cable) frontends
# SEC control devices for DVB-S
# CONFIG_DVB_DUMMY_FE is not set

-- 
Stefan Richter
-=-===-- -=-= ==-=-
http://arcgraph.de/sr/
--
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 9/9] [media] firedtv: handle errors from dvb_net_init

2011-12-31 Thread Stefan Richter
On Dec 31 Jonathan Nieder wrote:
 It is not common for dvb_net_init to fail, but after the patch
 dvb_net_init: return -errno on error it can fail due to running out
 of memory.  Handle this.
 From an audit of dvb_net_init callers.
 
 Signed-off-by: Jonathan Nieder jrnie...@gmail.com

Reviewed-by: Stefan Richter stef...@s5r6.in-berlin.de

[...]
 --- a/drivers/media/dvb/firewire/firedtv-dvb.c
 +++ b/drivers/media/dvb/firewire/firedtv-dvb.c
 @@ -203,7 +203,9 @@ int fdtv_dvb_register(struct firedtv *fdtv, const
 char *name) if (err)
   goto fail_rem_frontend;
  
 - dvb_net_init(fdtv-adapter, fdtv-dvbnet, fdtv-demux.dmx);
 + err = dvb_net_init(fdtv-adapter, fdtv-dvbnet,
 fdtv-demux.dmx);
 + if (err)
 + goto fail_disconnect_frontend;
  
   fdtv_frontend_init(fdtv, name);
   err = dvb_register_frontend(fdtv-adapter, fdtv-fe);
 @@ -218,6 +220,7 @@ int fdtv_dvb_register(struct firedtv *fdtv, const
 char *name) 
  fail_net_release:
   dvb_net_release(fdtv-dvbnet);
 +fail_disconnect_frontend:
   fdtv-demux.dmx.close(fdtv-demux.dmx);
  fail_rem_frontend:
   fdtv-demux.dmx.remove_frontend(fdtv-demux.dmx,
 fdtv-frontend);

-- 
Stefan Richter
-=-==-== ==-- =
http://arcgraph.de/sr/
--
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 00/94] Only use DVBv5 internally on frontend drivers

2011-12-31 Thread Stefan Richter
On Dec 30 Mauro Carvalho Chehab wrote:
 Basically, changes all DVB frontend drivers to work directly with
 the DVBv5 structure.
[...]
 Test reports are welcome.
[...]
   [media] firedtv: convert set_fontend to use DVBv5 parameters
[...]
  drivers/media/dvb/firewire/firedtv-avc.c |   95 
  drivers/media/dvb/firewire/firedtv-fe.c  |   31 ++
  drivers/media/dvb/firewire/firedtv.h |4 +-

I briefly tested git://linuxtv.org/media_tree.gitstaging/for_v3.3
7c61d80a9bcf on top of v3.2-rc7 on a FireDTV-T/CI with kaffeine and on a
FireDTV-C/CI with kaffeine and smplayer and didn't notice any runtime problem.

Building fs/compat_ioctl.c failed for me though:
fs/compat_ioctl.c:1345:1: error: invalid application of ‘sizeof’ to incomplete 
type ‘struct dvb_frontend_parameters’ 
fs/compat_ioctl.c:1345:1: error: array type has incomplete element type
etc. pp.
-- 
Stefan Richter
-=-==-== ==-- =
http://arcgraph.de/sr/
--
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 v3 4/14] staging/media/as102: checkpatch fixes

2011-10-31 Thread Stefan Richter
 On Oct 30 Piotr Chmura wrote:
 Patch taken from http://kernellabs.com/hg/~dheitmueller/v4l-dvb-as102-2/

 Original source and comment:
 # HG changeset patch

By the way, the brand new git 1.7.8.rc0 features some HG support in git am:
https://code.google.com/p/git-core/source/detail?spec=svnbe3fa9125e708348c7baf04ebe9507a72a9d1800r=0cfd112032017ab68ed576f6bb5258452084ebf1

This converts the # User and # Date lines of HG patches into RFC 2822
From:  and Date:  lines which are then used as authorship metadata.
-- 
Stefan Richter
-=-==-== =-=- =
http://arcgraph.de/sr/
--
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 v3 4/14] staging/media/as102: checkpatch fixes

2011-10-30 Thread Stefan Richter
patch postings.
-- 
Stefan Richter
-=-==-== =-=- -
http://arcgraph.de/sr/
--
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: [RESEND PATCH 11/14] staging/media/as102: fix compile warning about unused function

2011-10-30 Thread Stefan Richter
On Oct 30 Sylwester Nawrocki wrote:
 On 10/18/2011 10:03 PM, Piotr Chmura wrote:
  Patch taken from http://kernellabs.com/hg/~dheitmueller/v4l-dvb-as102-2/
[...]
  +#if (LINUX_VERSION_CODE  KERNEL_VERSION(2, 6, 19))
 
 I was wondering, could such a conditional compilation be simply skipped when
 we are merging the driver into exactly known kernel version ?  
 For backports there are separate patches at media_build.git and I can't see
 such an approach used in any driver upstream.

Compatibility code is in fact not allowed anymore upstream.  But AFAIU,
this patch here does not have such a cleanup in its scope.  If the compat
removal isn't already included later on in Piotr's series, it will be done
later before this driver can be moved out of staging.
-- 
Stefan Richter
-=-==-== =-=- -
http://arcgraph.de/sr/
--
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: [RESEND PATCH 1/14] staging/media/as102: initial import from Abilis

2011-10-30 Thread Stefan Richter
On Oct 30 Piotr Chmura wrote:
  + * Note:
  + * - in AS102 SNR=MER
  + *   - the SNR will be returned in linear terms, i.e. not in dB
  + *   - the accuracy equals ±2dB for a SNR range from 4dB to 30dB
  + *   - the accuracy is2dB for SNR values outside this range
  + */
 
 I found another issue here.
 In this comment ± is from upper ASCII (0xF1). Should I change it into 
 sth. like +/- in this patch (1/14) or leave it and just resend without 
 Â (wasn't there in original patch, don't know where it came from) ?

Special characters can be used in comments, provided that they are UTF-8
encoded.  In case of names of persons or companies, it is very much
desirable to preserve special characters.  In case like this one on the
other hand, sticking with ASCII (the 7 bit character table) might not be
such a bad idea to keep things simple.  But since you are passing on a
patch from somebody else, the right thing to do is IMO to keep the special
characters that the author chose and only make sure that the file (and
the patch mailing) are UTF-8 encoded.
-- 
Stefan Richter
-=-==-== =-=- -
http://arcgraph.de/sr/
--
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/7] Staging submission: PCTV 74e driver (as102)

2011-10-16 Thread Stefan Richter
On Oct 15 Piotr Chmura wrote:
 Staging submission: PCTV 74e driver (as102)
 
 From: Devin Heitmuellerdheitmuel...@kernellabs.com
 
 pull as102 driver from
 
 This is driver for PCTV 74e DVB-T USB tuner, taken from [1],
 written by Devin Heitmueller using the GPL reference driver provided by 
 Abilis.
 
 The only change needed to compile it in current git tree [2]
 was changing calls usb_buffer_alloc() and usb_buffer_free() to
 usb_alloc_coherent() and usb_free_coherent(). It's included in this patch.
 
 Patch was tested by me on amd64.
 
 [1]http://kernellabs.com/hg/~dheitmueller/v4l-dvb-as102-2/
 [2] git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
 kernel-3.1.0-git9+
 
 Signed-off-by: Piotr Chmurachmoor...@poczta.onet.pl
 Cc: Devin Heitmuellerdheitmuel...@kernellabs.com
 Cc: Greg HKgre...@suse.de
 
 diff -Nur linux.clean/drivers/staging/as102/as102_drv.c 
 linux.as102_initial/drivers/staging/as102/as102_drv.c
 --- linux.clean/drivers/staging/as102/as102_drv.c 1970-01-01 
 01:00:00.0 +0100
 +++ linux.as102_initial/drivers/staging/as102/as102_drv.c 2011-10-14 
 17:55:02.0 +0200
 @@ -0,0 +1,360 @@
 +/*
 + * Abilis Systems Single DVB-T Receiver
 + * Copyright (C) 2008 Pierrick Hascoetpierrick.hasc...@abilis.com
 + * Copyright (C) 2010 Devin Heitmuellerdheitmuel...@kernellabs.com

Hi Piotr,

thanks for getting this going again.  - I have not yet looked through the
source but have some small remarks on the patch format.

  - In your changelogs and in the diffs, somehow the space between real
name and e-mail address got lost.

  - The repetition of the Subject: line as first line in the changelog is
unnecessary (and would cause an undesired duplication e.g. when git-am
is used, last time I checked).

  - AFAICT, author of patch 1/7 is not Devin but you.  Hence the From: line
right above the changelog is wrong.

  - The reference to the source hg tree is very helpful.  However, since
the as102 related history in there is very well laid out, it may be
beneficial to quote some of this prior history.  I suggest, include
the changelog of as102: import initial as102 driver,
http://kernellabs.com/hg/~dheitmueller/v4l-dvb-as102-2/rev/a78bda1e1a0b
(but mark it clearly as a quote from the out-of-tree repo), and include
a shortlog from this commit inclusive until the head commit inclusive.
(Note, the hg author field appears to be wrong; some of the changes
apparently need to be attributed to Pierrick Hascoet as author.)
This would IMO improve the picture of who contributed what when this
goes into mainline git history, even though the hg history needed to
be discarded.

  - A diffstat is always very nice to have in a patch posting.  Most tools
for patch generation make it easy to add, and it helps the recipients
of the patch posting.
(Also, a diffstat of all patches combined would be good to have in the
introductory PATCH 0/n posting, but not many developers take the time
to do so.)

Again, thanks for the effort and also thanks to Devin for making it
possible.
-- 
Stefan Richter
-=-==-== =-=- =
http://arcgraph.de/sr/
--
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: [RFCv2 PATCH 2/7] V4L menu: move legacy drivers into their own submenu.

2011-09-30 Thread Stefan Richter
On Sep 30 Hans Verkuil wrote:
 +menuconfig V4L_LEGACY_DRIVERS
 + bool V4L legacy devices
 + default n

default n is redundant.

 + ---help---
 +   Say Y here to enable support for these legacy drivers. These drivers
 +   are for old and obsure hardware (e.g. parallel port webcams, ISA
 +   drivers, niche hardware).

Perhaps add sentences like these which are commonly seen in such
menuconfig variables:

  This option alone does not add any kernel code.

  If you say N, all options in this submenu will be skipped and 
disabled.

There are obviously several already existing menuconfigs in the video section
which do not have these sentences; so maybe don't add the above, or add it
separately across the board, or whatever.  I find these sentences helpful when
running make oldconfig or the likes.
-- 
Stefan Richter
-=-==-== =--= -
http://arcgraph.de/sr/
--
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


Staging submission: PCTV 80e and PCTV 74e drivers (was Re: Problems cloning the git repostories)

2011-09-27 Thread Stefan Richter
Adding Cc: staging maintainer and mailinglist.

On Sep 26 Devin Heitmueller wrote:
 On Sun, Sep 25, 2011 at 11:25 PM, Mauro Carvalho Chehab
 mauroche...@gmail.com wrote:
  Want to see more device support upstream?  Optimize the process to
  make it easy for the people who know the hardware and how to write the
  drivers to get code upstream, and leave it to the janitors to work
  out the codingstyle issues.
 
  The process you've just described exists already since Sept, 2008.
  It is called:
         /drivers/staging
 
  In summary, if you don't have a couple hours to make your driver to
  match Kernel Coding Style, just send it as is to /drivers/staging, c/c
  me and Greg KH, and that's it.
 
 PULL http://kernellabs.com/hg/~dheitmueller/v4l-dvb-80e/
 PULL http://kernellabs.com/hg/~dheitmueller/v4l-dvb-as102-2/
 
 Have fun.
 
 The harder you make it to get code upstream, the more developers who
 will just say to hell with this.  And *that* is why there are
 thousands of lines of working drivers which various developers have in
 out-of-tree drivers.

Hi,

perhaps a kind developer over at devel@driverdev could extract patches for
staging out of the above mercurial repositories, and then folks can work
on mainline inclusion.  (Somebody who actually has such a device might be
most motivated to do it.)

No need to get angry that it hasn't happened yet. :-)  It's just a matter
of the right people joining the effort at the right time.

Thanks Devin for the offer,
-- 
Stefan Richter
-=-==-== =--= ==-==
http://arcgraph.de/sr/
--
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: Staging submission: PCTV 80e and PCTV 74e drivers (was Re: Problems cloning the git repostories)

2011-09-27 Thread Stefan Richter
On Sep 27 Greg KH wrote:
 On Tue, Sep 27, 2011 at 09:44:09AM +0200, Stefan Richter wrote:
  Adding Cc: staging maintainer and mailinglist.
  
  On Sep 26 Devin Heitmueller wrote:
   On Sun, Sep 25, 2011 at 11:25 PM, Mauro Carvalho Chehab
   mauroche...@gmail.com wrote:
In summary, if you don't have a couple hours to make your driver to
match Kernel Coding Style, just send it as is to /drivers/staging, c/c
me and Greg KH, and that's it.
   
   PULL http://kernellabs.com/hg/~dheitmueller/v4l-dvb-80e/
   PULL http://kernellabs.com/hg/~dheitmueller/v4l-dvb-as102-2/
 
 I can't do that as I need some commit messages in a format we can accept
 (i.e. your directory structure doesn't match what we need in the kernel
 tree from what I can tell.)
[...]
 As the drivers don't seem to be touched in way over a year, odds are the
 code isn't going to be able to build as-is, so it will require some
 changes for basic issues.
 
 And I'll glady accept patches for the staging tree.  Also note that
 we've just created a drivers/staging/media/ tree to house lots of
 different v4l drivers that are being worked on in the staging tree to
 help coordinate this type of work better.

The conversion into patches with proper changelog, fitting directory
structure, and basic build-ability in current staging is exactly
the first step for which a volunteer is sought (next would then be the
cleanup associated with staging-mainline transition); Devin noted that he
is not going to dedicate time for these types of tasks.  (I for one also
won't; still got plenty to do in some other drivers...)
-- 
Stefan Richter
-=-==-== =--= ==-==
http://arcgraph.de/sr/
--
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] [media] firedtv: change some -EFAULT returns to more fitting error codes

2011-07-06 Thread Stefan Richter
Mauro Carvalho Chehab wrote:
 I'm validating if all drivers are behaving equally with respect to the
 error codes returned to userspace, and double-checking with the API.
 
 On almost all places, -EFAULT code is used only to indicate when
 copy_from_user/copy_to_user fails. However, firedtv uses a lot of
 -EFAULT, where it seems to me that other error codes should be used
 instead (like -EIO for bus transfer errors and -EINVAL/-ERANGE for 
 invalid/out of range parameters).

This concerns only the CI (CAM) related code of firedtv of which I know
little.  Let's just pass through the error returns of lower level I/O
code where applicable, and -EACCES (permission denied) when a seemingly
valid but negative FCP response or an unknown-to-firedtv CA message is
received.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
Cc: Henrik Kurelid hen...@kurelid.se
---
Only tested on FireDTV-{C,T}/CI without CAM.

 drivers/media/dvb/firewire/firedtv-avc.c |2 
 drivers/media/dvb/firewire/firedtv-ci.c  |   34 +++
 2 files changed, 17 insertions(+), 19 deletions(-)

Index: b/drivers/media/dvb/firewire/firedtv-avc.c
===
--- a/drivers/media/dvb/firewire/firedtv-avc.c
+++ b/drivers/media/dvb/firewire/firedtv-avc.c
@@ -1208,7 +1208,7 @@ int avc_ca_pmt(struct firedtv *fdtv, cha
if (r-response != AVC_RESPONSE_ACCEPTED) {
dev_err(fdtv-device,
CA PMT failed with response 0x%x\n,
r-response);
-   ret = -EFAULT;
+   ret = -EACCES;
}
 out:
mutex_unlock(fdtv-avc_mutex);
Index: b/drivers/media/dvb/firewire/firedtv-ci.c
===
--- a/drivers/media/dvb/firewire/firedtv-ci.c
+++ b/drivers/media/dvb/firewire/firedtv-ci.c
@@ -45,11 +45,6 @@ static int fdtv_get_ca_flags(struct fire
return flags;
 }
 
-static int fdtv_ca_reset(struct firedtv *fdtv)
-{
-   return avc_ca_reset(fdtv) ? -EFAULT : 0;
-}
-
 static int fdtv_ca_get_caps(void *arg)
 {
struct ca_caps *cap = arg;
@@ -65,12 +60,14 @@ static int fdtv_ca_get_slot_info(struct
 {
struct firedtv_tuner_status stat;
struct ca_slot_info *slot = arg;
+   int err;
 
-   if (avc_tuner_status(fdtv, stat))
-   return -EFAULT;
+   err = avc_tuner_status(fdtv, stat);
+   if (err)
+   return err;
 
if (slot-num != 0)
-   return -EFAULT;
+   return -EACCES;
 
slot-type = CA_CI;
slot-flags = fdtv_get_ca_flags(stat);
@@ -81,21 +78,21 @@ static int fdtv_ca_app_info(struct fired
 {
struct ca_msg *reply = arg;
 
-   return avc_ca_app_info(fdtv, reply-msg, reply-length) ?
-EFAULT : 0;
+   return avc_ca_app_info(fdtv, reply-msg, reply-length);
 }
 
 static int fdtv_ca_info(struct firedtv *fdtv, void *arg)
 {
struct ca_msg *reply = arg;
 
-   return avc_ca_info(fdtv, reply-msg, reply-length) ? -EFAULT :
0;
+   return avc_ca_info(fdtv, reply-msg, reply-length);
 }
 
 static int fdtv_ca_get_mmi(struct firedtv *fdtv, void *arg)
 {
struct ca_msg *reply = arg;
 
-   return avc_ca_get_mmi(fdtv, reply-msg, reply-length) ?
-EFAULT : 0;
+   return avc_ca_get_mmi(fdtv, reply-msg, reply-length);
 }
 
 static int fdtv_ca_get_msg(struct firedtv *fdtv, void *arg)
@@ -111,14 +108,15 @@ static int fdtv_ca_get_msg(struct firedt
err = fdtv_ca_info(fdtv, arg);
break;
default:
-   if (avc_tuner_status(fdtv, stat))
-   err = -EFAULT;
-   else if (stat.ca_mmi == 1)
+   err = avc_tuner_status(fdtv, stat);
+   if (err)
+   break;
+   if (stat.ca_mmi == 1)
err = fdtv_ca_get_mmi(fdtv, arg);
else {
dev_info(fdtv-device, unhandled CA message
0x%08x\n, fdtv-ca_last_command);
-   err = -EFAULT;
+   err = -EACCES;
}
}
fdtv-ca_last_command = 0;
@@ -141,7 +139,7 @@ static int fdtv_ca_pmt(struct firedtv *f
data_length = msg-msg[3];
}
 
-   return avc_ca_pmt(fdtv, msg-msg[data_pos], data_length) ?
-EFAULT : 0;
+   return avc_ca_pmt(fdtv, msg-msg[data_pos], data_length);
 }
 
 static int fdtv_ca_send_msg(struct firedtv *fdtv, void *arg)
@@ -170,7 +168,7 @@ static int fdtv_ca_send_msg(struct fired
default:
dev_err(fdtv-device, unhandled CA message 0x%08x\n,
fdtv-ca_last_command);
-   err = -EFAULT;
+   err = -EACCES;
}
return err;
 }
@@ -184,7 +182,7 @@ static int fdtv_ca_ioctl(struct file *fi
 
switch (cmd) {
case CA_RESET:
-   err = fdtv_ca_reset(fdtv);
+   err = avc_ca_reset(fdtv);
break

[PATCH resend] [media] firedtv: change some -EFAULT returns to more fitting error codes

2011-07-06 Thread Stefan Richter
Mauro Carvalho Chehab wrote:
 I'm validating if all drivers are behaving equally with respect to the
 error codes returned to userspace, and double-checking with the API.
 
 On almost all places, -EFAULT code is used only to indicate when
 copy_from_user/copy_to_user fails. However, firedtv uses a lot of
 -EFAULT, where it seems to me that other error codes should be used
 instead (like -EIO for bus transfer errors and -EINVAL/-ERANGE for 
 invalid/out of range parameters).

This concerns only the CI (CAM) related code of firedtv of which I know
little.  Let's just pass through the error returns of lower level I/O
code where applicable, and -EACCES (permission denied) when a seemingly
valid but negative FCP response or an unknown-to-firedtv CA message is
received.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
Cc: Henrik Kurelid hen...@kurelid.se
---
Resent, hopefully without linewrap breakage this time.

 drivers/media/dvb/firewire/firedtv-avc.c |2 
 drivers/media/dvb/firewire/firedtv-ci.c  |   34 +++
 2 files changed, 17 insertions(+), 19 deletions(-)

Index: b/drivers/media/dvb/firewire/firedtv-avc.c
===
--- a/drivers/media/dvb/firewire/firedtv-avc.c
+++ b/drivers/media/dvb/firewire/firedtv-avc.c
@@ -1208,7 +1208,7 @@ int avc_ca_pmt(struct firedtv *fdtv, cha
if (r-response != AVC_RESPONSE_ACCEPTED) {
dev_err(fdtv-device,
CA PMT failed with response 0x%x\n, r-response);
-   ret = -EFAULT;
+   ret = -EACCES;
}
 out:
mutex_unlock(fdtv-avc_mutex);
Index: b/drivers/media/dvb/firewire/firedtv-ci.c
===
--- a/drivers/media/dvb/firewire/firedtv-ci.c
+++ b/drivers/media/dvb/firewire/firedtv-ci.c
@@ -45,11 +45,6 @@ static int fdtv_get_ca_flags(struct fire
return flags;
 }
 
-static int fdtv_ca_reset(struct firedtv *fdtv)
-{
-   return avc_ca_reset(fdtv) ? -EFAULT : 0;
-}
-
 static int fdtv_ca_get_caps(void *arg)
 {
struct ca_caps *cap = arg;
@@ -65,12 +60,14 @@ static int fdtv_ca_get_slot_info(struct
 {
struct firedtv_tuner_status stat;
struct ca_slot_info *slot = arg;
+   int err;
 
-   if (avc_tuner_status(fdtv, stat))
-   return -EFAULT;
+   err = avc_tuner_status(fdtv, stat);
+   if (err)
+   return err;
 
if (slot-num != 0)
-   return -EFAULT;
+   return -EACCES;
 
slot-type = CA_CI;
slot-flags = fdtv_get_ca_flags(stat);
@@ -81,21 +78,21 @@ static int fdtv_ca_app_info(struct fired
 {
struct ca_msg *reply = arg;
 
-   return avc_ca_app_info(fdtv, reply-msg, reply-length) ? -EFAULT : 0;
+   return avc_ca_app_info(fdtv, reply-msg, reply-length);
 }
 
 static int fdtv_ca_info(struct firedtv *fdtv, void *arg)
 {
struct ca_msg *reply = arg;
 
-   return avc_ca_info(fdtv, reply-msg, reply-length) ? -EFAULT : 0;
+   return avc_ca_info(fdtv, reply-msg, reply-length);
 }
 
 static int fdtv_ca_get_mmi(struct firedtv *fdtv, void *arg)
 {
struct ca_msg *reply = arg;
 
-   return avc_ca_get_mmi(fdtv, reply-msg, reply-length) ? -EFAULT : 0;
+   return avc_ca_get_mmi(fdtv, reply-msg, reply-length);
 }
 
 static int fdtv_ca_get_msg(struct firedtv *fdtv, void *arg)
@@ -111,14 +108,15 @@ static int fdtv_ca_get_msg(struct firedt
err = fdtv_ca_info(fdtv, arg);
break;
default:
-   if (avc_tuner_status(fdtv, stat))
-   err = -EFAULT;
-   else if (stat.ca_mmi == 1)
+   err = avc_tuner_status(fdtv, stat);
+   if (err)
+   break;
+   if (stat.ca_mmi == 1)
err = fdtv_ca_get_mmi(fdtv, arg);
else {
dev_info(fdtv-device, unhandled CA message 0x%08x\n,
 fdtv-ca_last_command);
-   err = -EFAULT;
+   err = -EACCES;
}
}
fdtv-ca_last_command = 0;
@@ -141,7 +139,7 @@ static int fdtv_ca_pmt(struct firedtv *f
data_length = msg-msg[3];
}
 
-   return avc_ca_pmt(fdtv, msg-msg[data_pos], data_length) ? -EFAULT : 0;
+   return avc_ca_pmt(fdtv, msg-msg[data_pos], data_length);
 }
 
 static int fdtv_ca_send_msg(struct firedtv *fdtv, void *arg)
@@ -170,7 +168,7 @@ static int fdtv_ca_send_msg(struct fired
default:
dev_err(fdtv-device, unhandled CA message 0x%08x\n,
fdtv-ca_last_command);
-   err = -EFAULT;
+   err = -EACCES;
}
return err;
 }
@@ -184,7 +182,7 @@ static int fdtv_ca_ioctl(struct file *fi
 
switch (cmd) {
case CA_RESET:
-   err = fdtv_ca_reset(fdtv);
+   err

Re: [RFC] Don't use linux/version.h anymore to indicate a per-driver version - Was: Re: [PATCH 03/37] Remove unneeded version.h includes from include/

2011-06-24 Thread Stefan Richter
On Jun 24 Mauro Carvalho Chehab wrote:
 Em 24-06-2011 10:54, Hans Verkuil escreveu:
  On Friday, June 24, 2011 15:45:59 Devin Heitmueller wrote:
  The versions are increased at the discretion of the driver maintainer,
  usually when there is some userland visible change in driver behavior.
   I assure you the application developers don't *want* to rely on such
  a mechanism, but there have definitely been cases in the past where
  there was no easy way to detect the behavior of the driver from
  userland.
 
  It lets application developers work around things like violations of
  the V4L2 standard which get fixed in newer revisions of the driver.
  It provides them the ability to put a hack in their code that says if
  (version  X) then this driver feature is broken and I shouldn't use
  it.
  
  Indeed. Ideally we shouldn't need it. But reality is different.
 
  What we have right now works and I see no compelling reason to change the
  behavior.
 
 A per-driver version only works if the user is running a vanilla kernel 
 without 
 any stable patches applied. 
 
 I doubt that this covers the large amount of the users: they'll either use an 
 stable patched kernel or a distribution-specific one. On both cases, the 
 driver
 version is not associated with a bug fix, as the driver maintainers just take
 care of increasing the driver version once per each new kernel version (when
 they care enough).
 
 Also, a git blame for the V4L2 drivers shows that only a few drivers have 
 their
 version increased as changes are applied there. So, relying on cap-version 
 has a minimal chance of working only with a few drivers, with vanilla *.0 
 kernels.

If the driver version is in fact an ABI version, then the driver author
should really increase it only when ABI behavior is changed (and only if
the behavior change can only be communicated by version number --- e.g.
addition of an ioctl is not among such reasons).  And the author should
commit behavior changing implementation and version number change in a
single changeset.

And anybody who backmerges such an ABI behavior change into another kernel
branch (stable, longterm, distro...) must backmerge the associated version
number change too.

Of course sometimes people realize this only after the fact.  Or driver
authors don't have a clear understanding of ABI versioning to begin with.
I am saying so because I had to learn it too; I certainly wasn't born
with an instinct knowledge how to do it properly.

(Disclaimer:  I have no stake in drivers/media/ ABIs.  But I am involved
in maintaining a userspace ABI elsewhere in drivers/firewire/, and one of
the userspace libraries that use this ABI.)
-- 
Stefan Richter
-=-==-== -==- ==---
http://arcgraph.de/sr/
--
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] Don't use linux/version.h anymore to indicate a per-driver version - Was: Re: [PATCH 03/37] Remove unneeded version.h includes from include/

2011-06-24 Thread Stefan Richter
On Jun 24 Devin Heitmueller wrote:
 Really, this is all about applications being able to jam a hack into
 their code that translates to don't call this ioctl() with some
 particular argument if it's driver W less than version X, because the
 driver had a bug that is likely to panic the guy's PC.  Sure, it's a
 crummy solution, but at this point it's the best that we have got.

The second best.  The best that we have got is that the user runs a fixed
kernel.

Anyway; if this is the only purpose that this interface version¹ serves,
then Mauro's subsystem-centralized solution has the benefit that it
eliminates mistakes due to oversight by individual driver authors.
Especially because the kind of implementation behavior changes that are
tracked by this type of version datum are sometimes just discovered or
documented in hindsight.  On the other hand, Mauro's solution is redundant
to the uname(2) syscall.

¹) Yes, it is still an ABI version, nothing less.  With all its backwards
and forwards compatibility ramifications.
-- 
Stefan Richter
-=-==-== -==- ==---
http://arcgraph.de/sr/
--
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] Don't use linux/version.h anymore to indicate a per-driver version - Was: Re: [PATCH 03/37] Remove unneeded version.h includes from include/

2011-06-24 Thread Stefan Richter
On Jun 24 Andy Walls wrote:
 I also use the driver version for troubleshooting problem with users.  I
 roughly know what wasn't working in what version of the cx18 and ivtv
 drivers.  If the end user can tell me the driver version (using v4l2-ctl
 --log-status) along with his symptoms, it makes my life easier.

Easier:
  I run Ubuntu 10.4.
  I run kernel 2.6.32.
One of these is usually already included in the first post or IRC message
from the user.

Separate driver versions are only needed on platforms where drivers are
not distributed by the operating system distributor, or driver source code
is not released within kernel source code.
-- 
Stefan Richter
-=-==-== -==- ==---
http://arcgraph.de/sr/
--
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] Don't use linux/version.h anymore to indicate a per-driver version - Was: Re: [PATCH 03/37] Remove unneeded version.h includes from include/

2011-06-24 Thread Stefan Richter
On Jun 24 Devin Heitmueller wrote:
 On Fri, Jun 24, 2011 at 5:20 PM, Stefan Richter
 stef...@s5r6.in-berlin.de wrote:
  Easier:
   I run Ubuntu 10.4.
   I run kernel 2.6.32.
  One of these is usually already included in the first post or IRC message
  from the user.
 
  Separate driver versions are only needed on platforms where drivers are
  not distributed by the operating system distributor, or driver source code
  is not released within kernel source code.
 
 Unfortunately, this doesn't work as all too often the user has Ubuntu
 10.1 but I installed the latest media_build tree a few months ago.
 Hence they are not necessarily on a particular binary release from a
 distro but rather have a mix of a distro's binary release and a
 v4l-dvb tree compiled from source.

If you release out-of-kernel-source driver sources for compilation against
binary kernels, and you have got users who go through this procedure, then
the user can for sure tell you the SCM version of the driver.

Besides, isn't this outdated practice in times where Joe Enduser can get
the very latest -rc kernel prepackaged on many distributions, including
ones like Ubuntu?

[Sorry, I'm getting perhaps a bit off-topic.]
-- 
Stefan Richter
-=-==-== -==- ==--=
http://arcgraph.de/sr/
--
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] firewire: octlet AT payloads can be stack-allocated

2011-04-22 Thread Stefan Richter
...provided that the allocation persists until the packet was sent out
to the bus.  But we do not need slab allocations anymore in order to
satisfy streaming DMA mapping constraints, thanks to commit da28947e7e36
firewire: ohci: avoid separate DMA mapping for small AT payloads.

(Besides, the slab-allocated buffers that firewire-core, firewire-sbp2,
and firedtv used to provide for 8-byte write and lock requests were
still not fully portable since they crossed cacheline boundaries or
shared a cacheline with unrelated CPU-accessed data.  snd-firewire-lib
got this aspect right by using an extra kmalloc/ kfree just for the
8-byte transaction buffer.)

This change replaces kmalloc'ed lock transaction scratch buffers in
firewire-core, firedtv, and snd-firewire-lib by local stack allocations.
The lifetime requirement of these allocations is fulfilled because the
call sites use the blocking fw_run_transaction API.

Perhaps the most notable result of the change is simpler locking because
there is no need to serialize usages of preallocated per-device buffers
anymore.  Also, allocations and deallocations are simpler.

firewire-sbp2's struct sbp2_orb.pointer buffer for 8-byte block write
requests on the other hand needs to remain slab-allocated in order to
keep the allocation around until end of AT DMA.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
Tested with firedtv, snd-firewire-speakers, sind-isight, and Coriander
on a kernel with CONFIG_DMA_API_DEBUG=y.

The patch could be split into three parts --- one for firedtv alone, one
for the firewire-core an snd-firewire-lib fw_iso_resource_manage related
changes, and one for the other changes in firewire-core.  But since the
second one necessarily affects both drivers/firewire/ and sound/firewire/
and because they all depend functionally on a change in linux1394-2.6.git
master, there is no much benefit in breaking this up.

 drivers/firewire/core-card.c |   16 
 drivers/firewire/core-cdev.c |4 +---
 drivers/firewire/core-iso.c  |   21 +++--
 drivers/firewire/core-transaction.c  |7 ---
 drivers/media/dvb/firewire/firedtv-avc.c |   15 +--
 include/linux/firewire.h |3 +--
 sound/firewire/cmp.c |3 +--
 sound/firewire/iso-resources.c   |   12 +++-
 sound/firewire/iso-resources.h   |1 -
 9 files changed, 30 insertions(+), 52 deletions(-)

Index: b/drivers/firewire/core-card.c
===
--- a/drivers/firewire/core-card.c
+++ b/drivers/firewire/core-card.c
@@ -258,8 +258,7 @@ static void allocate_broadcast_channel(s
 
if (!card-broadcast_channel_allocated) {
fw_iso_resource_manage(card, generation, 1ULL  31,
-  channel, bandwidth, true,
-  card-bm_transaction_data);
+  channel, bandwidth, true);
if (channel != 31) {
fw_notify(failed to allocate broadcast channel\n);
return;
@@ -294,6 +293,7 @@ static void bm_work(struct work_struct *
bool root_device_is_cmc;
bool irm_is_1394_1995_only;
bool keep_this_irm;
+   __be32 transaction_data[2];
 
spin_lock_irq(card-lock);
 
@@ -355,21 +355,21 @@ static void bm_work(struct work_struct *
goto pick_me;
}
 
-   card-bm_transaction_data[0] = cpu_to_be32(0x3f);
-   card-bm_transaction_data[1] = cpu_to_be32(local_id);
+   transaction_data[0] = cpu_to_be32(0x3f);
+   transaction_data[1] = cpu_to_be32(local_id);
 
spin_unlock_irq(card-lock);
 
rcode = fw_run_transaction(card, TCODE_LOCK_COMPARE_SWAP,
irm_id, generation, SCODE_100,
CSR_REGISTER_BASE + CSR_BUS_MANAGER_ID,
-   card-bm_transaction_data, 8);
+   transaction_data, 8);
 
if (rcode == RCODE_GENERATION)
/* Another bus reset, BM work has been rescheduled. */
goto out;
 
-   bm_id = be32_to_cpu(card-bm_transaction_data[0]);
+   bm_id = be32_to_cpu(transaction_data[0]);
 
spin_lock_irq(card-lock);
if (rcode == RCODE_COMPLETE  generation == card-generation)
@@ -490,11 +490,11 @@ static void bm_work(struct work_struct *
/*
 * Make sure that the cycle master sends cycle start packets.
 */
-   card-bm_transaction_data[0] = cpu_to_be32(CSR_STATE_BIT_CMSTR);
+   transaction_data[0] = cpu_to_be32(CSR_STATE_BIT_CMSTR);
rcode = fw_run_transaction(card, TCODE_WRITE_QUADLET_REQUEST

Re: firedtv and removal of old IEEE1394 stack

2011-02-09 Thread Stefan Richter
On Feb 09 Jan Hoogenraad wrote:
 For a problem description, and workaround, see:
 
 http://linuxtv.org/hg/~jhoogenraad/ubuntu-firedtv/

Do you mean
http://linuxtv.org/hg/~jhoogenraad/ubuntu-firedtv/rev/c8e14191e48d
Disable FIREDTV for debian/ubuntu distributions with bad header files?

I still don't see what the problem is.  If you have a kernel without
drivers/ieee1394/*, then you also must have a kernel .config without
CONFIG_IEEE1394.  Et voilà, firedtv builds fine (if CONFIG_FIREWIRE is y
or m).  So, please make sure that .config and kernel sources match.

IOW the workaround c8e14191e48d addresses the wrong issue.  Don't disable
CONFIG_DVB_FIREDTV; just make sure that the dependency of
CONFIG_DVB_FIREDTV_IEEE1394 on CONFIG_IEEE1394 is taken into account, like
in the mainline kernel's build system.

 and
 
 https://bugs.launchpad.net/ubuntu/+source/linux-kernel-headers/+bug/134222

Well, if you move arbitrary drivers/*/*.h files somewhere else where they
were never intended to be exported to, and supplant Kconfig by some
homegrewn ad hoc configuration builder, then you are of course on your own.
Still, my above comment on .config having to match the kernel sources
applies just as well and fully describes the problem and its solution. :-)
-- 
Stefan Richter
-=-==-== --=- -=--=
http://arcgraph.de/sr/
--
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: firedtv and removal of old IEEE1394 stack

2011-02-09 Thread Stefan Richter
On Feb 09 Stefan Richter wrote:
  https://bugs.launchpad.net/ubuntu/+source/linux-kernel-headers/+bug/134222

Correction:

Bug 134222 has *nothing* to do with the removal of the older ieee1394
stack.  The bug is about

  1. a defect during assembling the linux-kernel-headers package.
 The drivers/ieee1394/* files do not belong into such a package.
 They are driver source files, not exported kernel headers.

 Don't export kernel source files as linux-headers if they are not
 meant to be exported.

  2. the dvb backports relying on this broken package.  Tough luck.

 You want to build a kernelspace driver whose sources include other
 kernel sources?  Well, include these kernel sources, not some
 arbitrary userland source files.
-- 
Stefan Richter
-=-==-== --=- -=--=
http://arcgraph.de/sr/
--
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.39] Remove se401, usbvideo, dabusb, firedtv-1394 and VIDIOC_OLD

2011-02-06 Thread Stefan Richter
On Feb 05 Hans Verkuil wrote:
 The following changes since commit ffd14aab03dbb8bb1bac5284603835f94d833bd6:
   Devin Heitmueller (1):
 [media] au0828: fix VBI handling when in V4L2 streaming mode
 
 are available in the git repository at:
 
   ssh://linuxtv.org/git/hverkuil/media_tree.git v4l1
 
 Hans Verkuil (4):
   se401/usbvideo: remove last V4L1 drivers
   dabusb: remove obsolete driver
   firedtv: remove dependency on the deleted ieee1394 stack.
   v4l: removal of old, obsolete ioctls.

On commit f02c316436eef3baf349c489545edc7ade419ff6 firedtv: remove
dependency on the deleted ieee1394 stack.:

The diff is correct and runtime-tested it.  But, as discussed, the
changelog is wrong and the shortlog somewhat misleading.  I suggest
something along the lines of:

8

firedtv: remove obsolete ieee1394 backend code

drivers/ieee1394/ has been removed in Linux 2.6.37.  The corresponding
backend code in firedtv is no longer built in now and can be deleted.
Firedtv continues to work with drivers/firewire/.

Also, fix a Kconfig menu comment:  Removal of CONFIG_IEEE1394 made the
Supported FireWire (IEEE 1394) Adapters comment disappear; bring it back
with corrected dependency.

8

A minor note:  firedtv-dvb.c::fdtv_init() can now be shortened further,
and firedtv-fw.c::fdtv_fw_exit() can receive an __exit annotation.
However, these changes can wait for (or will be superseded by) a subsequent
simplification of firedtv which throws out the fdtv-backend abstraction.
I tend to think that the three parts of firedtv-fw.c (asynchronous I/O,
isochronous I/O, device probe/update/removal) can be moved into
firedtv-avc.c, -fe.c, and -dvb.c.  I will post something.

If you rewrite the changelog, you can add
Reviewed-by: Stefan Richter stef...@s5r6.in-berlin.de
if you like.
-- 
Stefan Richter
-=-==-== --=- --==-
http://arcgraph.de/sr/
--
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] firedtv: drop obsolete backend abstraction

2011-02-06 Thread Stefan Richter
Since the drivers/ieee1394/ backend was removed from firedtv, its I/O no
longer needs to be abstracted as exchangeable backend methods.

Also, ieee1394 variants of module and device probe and removal are no
longer there.  Move module probe and removal into firedtv-fw.c where
device probe and removal are implemented.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
Applies after Hans' firedtv: remove dependency on the deleted ieee1394
stack.,
http://git.linuxtv.org/hverkuil/media_tree.git?a=commitdiff;h=f02c316436eef3baf349c489545edc7ade419ff6

 drivers/media/dvb/firewire/firedtv-avc.c |   15 +-
 drivers/media/dvb/firewire/firedtv-dvb.c |  130 
 drivers/media/dvb/firewire/firedtv-fe.c  |8 
 drivers/media/dvb/firewire/firedtv-fw.c  |  146 +++
 drivers/media/dvb/firewire/firedtv.h |   31 ++---
 5 files changed, 140 insertions(+), 190 deletions(-)

Index: b/drivers/media/dvb/firewire/firedtv-avc.c
===
--- a/drivers/media/dvb/firewire/firedtv-avc.c
+++ b/drivers/media/dvb/firewire/firedtv-avc.c
@@ -241,8 +241,8 @@ static int avc_write(struct firedtv *fdt
if (unlikely(avc_debug))
debug_fcp(fdtv-avc_data, fdtv-avc_data_length);
 
-   err = fdtv-backend-write(fdtv, FCP_COMMAND_REGISTER,
-   fdtv-avc_data, fdtv-avc_data_length);
+   err = fdtv_write(fdtv, FCP_COMMAND_REGISTER,
+fdtv-avc_data, fdtv-avc_data_length);
if (err) {
dev_err(fdtv-device, FCP command write failed\n);
 
@@ -1322,7 +1322,7 @@ static int cmp_read(struct firedtv *fdtv
 
mutex_lock(fdtv-avc_mutex);
 
-   ret = fdtv-backend-read(fdtv, addr, data);
+   ret = fdtv_read(fdtv, addr, data);
if (ret  0)
dev_err(fdtv-device, CMP: read I/O error\n);
 
@@ -1340,7 +1340,7 @@ static int cmp_lock(struct firedtv *fdtv
/* data[] is stack-allocated and should not be DMA-mapped. */
memcpy(fdtv-avc_data, data, 8);
 
-   ret = fdtv-backend-lock(fdtv, addr, fdtv-avc_data);
+   ret = fdtv_lock(fdtv, addr, fdtv-avc_data);
if (ret  0)
dev_err(fdtv-device, CMP: lock I/O error\n);
else
@@ -1405,10 +1405,7 @@ repeat:
/* FIXME: this is for the worst case - optimize */
set_opcr_overhead_id(opcr, 0);
 
-   /*
-* FIXME: allocate isochronous channel and bandwidth at IRM
-* fdtv-backend-alloc_resources(fdtv, channels_mask, bw);
-*/
+   /* FIXME: allocate isochronous channel and bandwidth at IRM */
}
 
set_opcr_p2p_connections(opcr, get_opcr_p2p_connections(*opcr) + 1);
@@ -1424,8 +1421,6 @@ repeat:
/*
 * FIXME: if old_opcr.P2P_Connections  0,
 * deallocate isochronous channel and bandwidth at IRM
-* if (...)
-*  fdtv-backend-dealloc_resources(fdtv, channel, bw);
 */
 
if (++attempts  6) /* arbitrary limit */
Index: b/drivers/media/dvb/firewire/firedtv-dvb.c
===
--- a/drivers/media/dvb/firewire/firedtv-dvb.c
+++ b/drivers/media/dvb/firewire/firedtv-dvb.c
@@ -14,14 +14,9 @@
 #include linux/device.h
 #include linux/errno.h
 #include linux/kernel.h
-#include linux/mod_devicetable.h
 #include linux/module.h
 #include linux/mutex.h
-#include linux/slab.h
-#include linux/string.h
 #include linux/types.h
-#include linux/wait.h
-#include linux/workqueue.h
 
 #include dmxdev.h
 #include dvb_demux.h
@@ -166,11 +161,11 @@ int fdtv_stop_feed(struct dvb_demux_feed
 
 DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
 
-int fdtv_dvb_register(struct firedtv *fdtv)
+int fdtv_dvb_register(struct firedtv *fdtv, const char *name)
 {
int err;
 
-   err = dvb_register_adapter(fdtv-adapter, fdtv_model_names[fdtv-type],
+   err = dvb_register_adapter(fdtv-adapter, name,
   THIS_MODULE, fdtv-device, adapter_nr);
if (err  0)
goto fail_log;
@@ -210,7 +205,7 @@ int fdtv_dvb_register(struct firedtv *fd
 
dvb_net_init(fdtv-adapter, fdtv-dvbnet, fdtv-demux.dmx);
 
-   fdtv_frontend_init(fdtv);
+   fdtv_frontend_init(fdtv, name);
err = dvb_register_frontend(fdtv-adapter, fdtv-fe);
if (err)
goto fail_net_release;
@@ -248,122 +243,3 @@ void fdtv_dvb_unregister(struct firedtv 
dvb_dmx_release(fdtv-demux);
dvb_unregister_adapter(fdtv-adapter);
 }
-
-const char *fdtv_model_names[] = {
-   [FIREDTV_UNKNOWN] = unknown type,
-   [FIREDTV_DVB_S]   = FireDTV S/CI,
-   [FIREDTV_DVB_C]   = FireDTV C/CI,
-   [FIREDTV_DVB_T]   = FireDTV T/CI,
-   [FIREDTV_DVB_S2]  = FireDTV S2  ,
-};
-
-struct firedtv

Re: firedtv and removal of old IEEE1394 stack

2011-02-05 Thread Stefan Richter
On Feb 03 Hans Verkuil wrote:
 Hi Stefan,
 
 I discovered (somewhat to my surprise) that the IEEE1394 stack was removed
 from the kernel in 2.6.37. Your commit 
 66fa12c571d35e3cd62574c65f1785a460105397
 indicates that the ieee1394 firedtv code can be removed in an indepedent 
 commit.
 
 It seems that this was forgotten since the firedtv-1394.c source is still
 present.

It is not forgotten, just delayed. :-)

 Is it OK if I remove it? I assume that anything that depends on 
 DVB_FIREDTV_IEEE1394
 can be deleted.

This stuff can be removed indeed, and will be.  After that, some further
simplifications are possible since the backend abstraction is no longer
necessary.

 It would be nice to remove this since building the firedtv driver for older 
 kernels
 always gives problems on ubuntu due to some missing ieee1394 headers.

How so?  Then there is something wrong with the backported sources.  If
CONFIG_IEEE1394 is not defined, neither make nor gcc ever see anything
that includes ieee1394 headers.  Vice versa regarding CONFIG_FIREWIRE and
the newer firewire headers.
-- 
Stefan Richter
-=-==-== --=- --=-=
http://arcgraph.de/sr/
--
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.39] Remove se401, usbvideo, dabusb, firedtv-1394 and VIDIOC_OLD

2011-02-05 Thread Stefan Richter
On Feb 05 Hans Verkuil wrote:
 (Second attempt: fixes a link issue with firedtv and adds removal of the old 
 ioctls)
 
 This patch series removes the last V4L1 drivers (Yay!), the obsolete dabusb 
 driver,
 the ieee1394-stack part of the firedtv driver (the IEEE1394 stack was removed 
 in
 2.6.37), and the VIDIOC_*_OLD ioctls.
 
 Stefan, I went ahead with this since after further research I discovered that
 this driver hasn't been compiled at all since 2.6.37! The Kconfig had a
 dependency on IEEE1394, so when that config was removed, the driver no longer
 appeared in the config.
 
 I removed any remaining reference to IEEE1394 and changed the Kconfig 
 dependency
 to FIREWIRE. At least it compiles again :-)

Thanks for doing the firedtv cleanup.  However, the effect should just be
that of dead code elimination.  Was there any build problem that I missed?

AFAICS, firedtv builds and works fine in mainline 2.6.37(-rc) and
2.6.38(-rc).  From when I implemented the drivers/firewire/ backend of
firedtv, it should have been possible to build firedtv for a kernel with
one or both of drivers/{ieee1394,firewire}; controlled by whether
CONFIG_{IEEE1394,FIREWIRE} are defined or not.

I will have a look at your changes later.
-- 
Stefan Richter
-=-==-== --=- --=-=
http://arcgraph.de/sr/
--
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: firedtv and removal of old IEEE1394 stack

2011-02-05 Thread Stefan Richter
On Feb 05 Stefan Richter wrote:
 On Feb 03 Hans Verkuil wrote:
  It would be nice to remove this since building the firedtv driver for older 
  kernels
  always gives problems on ubuntu due to some missing ieee1394 headers.
 
 How so?  Then there is something wrong with the backported sources.

Or with the backports' build system perhaps.
-- 
Stefan Richter
-=-==-== --=- --=-=
http://arcgraph.de/sr/
--
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 update] firedtv: fix remote control with newer Xorg evdev

2011-01-17 Thread Stefan Richter
After a recent update of xf86-input-evdev and xorg-server, I noticed
that X11 applications did not receive keypresses from the FireDTV
infrared remote control anymore.  Instead, the Xorg log featured lots of

FireDTV remote control: dropping event due to full queue!

exclamations.  The Linux console did not have an issue with the
FireDTV's RC though.

The fix is to insert EV_SYN events after the key-down/-up events.
Dimitry notes that EV_SYN is also necessary between down and up,
otherwise userspace could combine their state.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
 drivers/media/dvb/firewire/firedtv-rc.c |9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Index: b/drivers/media/dvb/firewire/firedtv-rc.c
===
--- a/drivers/media/dvb/firewire/firedtv-rc.c
+++ b/drivers/media/dvb/firewire/firedtv-rc.c
@@ -172,7 +172,8 @@ void fdtv_unregister_rc(struct firedtv *
 
 void fdtv_handle_rc(struct firedtv *fdtv, unsigned int code)
 {
-   u16 *keycode = fdtv-remote_ctrl_dev-keycode;
+   struct input_dev *idev = fdtv-remote_ctrl_dev;
+   u16 *keycode = idev-keycode;
 
if (code = 0x0300  code = 0x031f)
code = keycode[code - 0x0300];
@@ -188,6 +189,8 @@ void fdtv_handle_rc(struct firedtv *fdtv
return;
}
 
-   input_report_key(fdtv-remote_ctrl_dev, code, 1);
-   input_report_key(fdtv-remote_ctrl_dev, code, 0);
+   input_report_key(idev, code, 1);
+   input_sync(idev);
+   input_report_key(idev, code, 0);
+   input_sync(idev);
 }


-- 
Stefan Richter
-=-==-== ---= =---=
http://arcgraph.de/sr/
--
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 incremental update] firedtv: fix remote control - addendum

2011-01-17 Thread Stefan Richter
Dimitry notes that EV_SYN is also necessary between down and up,
otherwise userspace could combine their state.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
Hi Mauro,
since you already pushed out the first version of firedtv: fix remote
control with newer Xorg evdev, here is the differential patch to the
updated version.  It's surely not super urgent though.

 drivers/media/dvb/firewire/firedtv-rc.c |1 +
 1 file changed, 1 insertion(+)

Index: b/drivers/media/dvb/firewire/firedtv-rc.c
===
--- a/drivers/media/dvb/firewire/firedtv-rc.c
+++ b/drivers/media/dvb/firewire/firedtv-rc.c
@@ -190,6 +190,7 @@ void fdtv_handle_rc(struct firedtv *fdtv
}
 
input_report_key(idev, code, 1);
+   input_sync(idev);
input_report_key(idev, code, 0);
input_sync(idev);
 }


-- 
Stefan Richter
-=-==-== ---= =---=
http://arcgraph.de/sr/
--
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] firedtv: fix remote control with newer Xorg evdev

2011-01-16 Thread Stefan Richter
After a recent update of xf86-input-evdev and xorg-server, I noticed
that X11 applications did not receive keypresses from the FireDTV
infrared remote control anymore.  Instead, the Xorg log featured lots of

FireDTV remote control: dropping event due to full queue!

exclamations.  The Linux console did not have an issue with the
FireDTV's RC though.

The fix is to insert EV_SYN events after the key-down/-up events.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
 drivers/media/dvb/firewire/firedtv-rc.c |8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

Index: b/drivers/media/dvb/firewire/firedtv-rc.c
===
--- a/drivers/media/dvb/firewire/firedtv-rc.c
+++ b/drivers/media/dvb/firewire/firedtv-rc.c
@@ -172,7 +172,8 @@ void fdtv_unregister_rc(struct firedtv *
 
 void fdtv_handle_rc(struct firedtv *fdtv, unsigned int code)
 {
-   u16 *keycode = fdtv-remote_ctrl_dev-keycode;
+   struct input_dev *idev = fdtv-remote_ctrl_dev;
+   u16 *keycode = idev-keycode;
 
if (code = 0x0300  code = 0x031f)
code = keycode[code - 0x0300];
@@ -188,6 +189,7 @@ void fdtv_handle_rc(struct firedtv *fdtv
return;
}
 
-   input_report_key(fdtv-remote_ctrl_dev, code, 1);
-   input_report_key(fdtv-remote_ctrl_dev, code, 0);
+   input_report_key(idev, code, 1);
+   input_report_key(idev, code, 0);
+   input_sync(idev);
 }


-- 
Stefan Richter
-=-==-== ---= =
http://arcgraph.de/sr/
--
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


drivers/media/IR/ir-keytable.c::ir_getkeycode - 'retval' may be used uninitialized

2010-10-31 Thread Stefan Richter
Commit 9f470095068e Input: media/IR - switch to using new keycode
interface added the following build warning:

drivers/media/IR/ir-keytable.c: In function 'ir_getkeycode':
drivers/media/IR/ir-keytable.c:363: warning: 'retval' may be used uninitialized 
in this function

It is due to an actual bug but I don't know the fix.
-- 
Stefan Richter
-=-==-=- =-=- =
http://arcgraph.de/sr/

--
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] dvb/firewire update

2010-10-13 Thread Stefan Richter
Hi Mauro,

please pull from the firedtv branch at

git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git 
firedtv

to receive the following update --- if you don't have it already in your
patch queue.  It adds a long asked for feature to FireWire sat
receivers.  Thanks.

Tommy Jonsson (1):
  V4L/DVB: firedtv: support for PSK8 for S2 devices. To watch HD.

 drivers/media/dvb/firewire/firedtv-avc.c |   30 +---
 drivers/media/dvb/firewire/firedtv-fe.c  |   36 -
 2 files changed, 60 insertions(+), 6 deletions(-)
-- 
Stefan Richter
-=-==-=- =-=- -==-=
http://arcgraph.de/sr/

--
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 update] V4L/DVB: firedtv: support for PSK8 for S2 devices. To watch HD.

2010-10-04 Thread Stefan Richter
Date: Sun, 12 Sep 2010 21:03:45 +0200
From: Tommy Jonsson quazz...@gmail.com

This is the first i have ever developed for linux, cant really wrap my
head around how to submit this..
Hope im sending this correctly, diff made with 'hg diff' from latest
hg clone http://linuxtv.org/hg/v4l-dvb;

It adds support for tuning with PSK8 modulation, pilot and rolloff
with the S2 versions of firedtv.

Signed-off-by: Tommy Jonsson quazz...@gmail.com
Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de (trivial 
simplification)
---
Update: resend with whitespace preserved, fe pointer does not have to be
put into function parameter lists, copyright notice removed (authorship
of smaller changes like this is tracked in the git changelog)

 drivers/media/dvb/firewire/firedtv-avc.c |   30 +--
 drivers/media/dvb/firewire/firedtv-fe.c  |   36 ++-
 2 files changed, 60 insertions(+), 6 deletions(-)

Index: b/drivers/media/dvb/firewire/firedtv-avc.c
===
--- a/drivers/media/dvb/firewire/firedtv-avc.c
+++ b/drivers/media/dvb/firewire/firedtv-avc.c
@@ -24,6 +24,8 @@
 #include linux/wait.h
 #include linux/workqueue.h
 
+#include dvb_frontend.h
+
 #include firedtv.h
 
 #define FCP_COMMAND_REGISTER   0xfb00ULL
@@ -368,10 +370,30 @@ static int avc_tuner_tuneqpsk(struct fir
c-operand[12] = 0;
 
if (fdtv-type == FIREDTV_DVB_S2) {
-   c-operand[13] = 0x1;
-   c-operand[14] = 0xff;
-   c-operand[15] = 0xff;
-
+   if (fdtv-fe.dtv_property_cache.delivery_system == SYS_DVBS2) {
+   switch (fdtv-fe.dtv_property_cache.modulation) {
+   case QAM_16:c-operand[13] = 0x1; break;
+   case QPSK:  c-operand[13] = 0x2; break;
+   case PSK_8: c-operand[13] = 0x3; break;
+   default:c-operand[13] = 0x2; break;
+   }
+   switch (fdtv-fe.dtv_property_cache.rolloff) {
+   case ROLLOFF_AUTO:  c-operand[14] = 0x2; break;
+   case ROLLOFF_35:c-operand[14] = 0x2; break;
+   case ROLLOFF_20:c-operand[14] = 0x0; break;
+   case ROLLOFF_25:c-operand[14] = 0x1; break;
+   /* case ROLLOFF_NONE:   c-operand[14] = 0xff; break; */
+   }
+   switch (fdtv-fe.dtv_property_cache.pilot) {
+   case PILOT_AUTO:c-operand[15] = 0x0; break;
+   case PILOT_OFF: c-operand[15] = 0x0; break;
+   case PILOT_ON:  c-operand[15] = 0x1; break;
+   }
+   } else {
+   c-operand[13] = 0x1;  /* auto modulation */
+   c-operand[14] = 0xff; /* disable rolloff */
+   c-operand[15] = 0xff; /* disable pilot */
+   }
return 16;
} else {
return 13;
Index: b/drivers/media/dvb/firewire/firedtv-fe.c
===
--- a/drivers/media/dvb/firewire/firedtv-fe.c
+++ b/drivers/media/dvb/firewire/firedtv-fe.c
@@ -155,6 +155,16 @@ static int fdtv_get_frontend(struct dvb_
return -EOPNOTSUPP;
 }
 
+static int fdtv_get_property(struct dvb_frontend *fe, struct dtv_property *tvp)
+{
+   return 0;
+}
+
+static int fdtv_set_property(struct dvb_frontend *fe, struct dtv_property *tvp)
+{
+   return 0;
+}
+
 void fdtv_frontend_init(struct firedtv *fdtv)
 {
struct dvb_frontend_ops *ops = fdtv-fe.ops;
@@ -166,6 +176,9 @@ void fdtv_frontend_init(struct firedtv *
ops-set_frontend   = fdtv_set_frontend;
ops-get_frontend   = fdtv_get_frontend;
 
+   ops-get_property   = fdtv_get_property;
+   ops-set_property   = fdtv_set_property;
+
ops-read_status= fdtv_read_status;
ops-read_ber   = fdtv_read_ber;
ops-read_signal_strength   = fdtv_read_signal_strength;
@@ -179,7 +192,6 @@ void fdtv_frontend_init(struct firedtv *
 
switch (fdtv-type) {
case FIREDTV_DVB_S:
-   case FIREDTV_DVB_S2:
fi-type= FE_QPSK;
 
fi-frequency_min   = 95;
@@ -188,7 +200,7 @@ void fdtv_frontend_init(struct firedtv *
fi-symbol_rate_min = 100;
fi-symbol_rate_max = 4000;
 
-   fi-caps= FE_CAN_INVERSION_AUTO |
+   fi-caps= FE_CAN_INVERSION_AUTO |
  FE_CAN_FEC_1_2|
  FE_CAN_FEC_2_3

[PATCH update2] V4L/DVB: firedtv: support for PSK8 for S2 devices. To watch HD.

2010-10-04 Thread Stefan Richter
Date: Sun, 12 Sep 2010 21:03:45 +0200
From: Tommy Jonsson quazz...@gmail.com

This is the first i have ever developed for linux, cant really wrap my
head around how to submit this..
Hope im sending this correctly, diff made with 'hg diff' from latest
hg clone http://linuxtv.org/hg/v4l-dvb;

It adds support for tuning with PSK8 modulation, pilot and rolloff
with the S2 versions of firedtv.

Signed-off-by: Tommy Jonsson quazz...@gmail.com
Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de (trivial 
simplification)
---
Sorry, missed two space-before-tab.

 drivers/media/dvb/firewire/firedtv-avc.c |   30 +--
 drivers/media/dvb/firewire/firedtv-fe.c  |   36 ++-
 2 files changed, 60 insertions(+), 6 deletions(-)

Index: b/drivers/media/dvb/firewire/firedtv-avc.c
===
--- a/drivers/media/dvb/firewire/firedtv-avc.c
+++ b/drivers/media/dvb/firewire/firedtv-avc.c
@@ -24,6 +24,8 @@
 #include linux/wait.h
 #include linux/workqueue.h
 
+#include dvb_frontend.h
+
 #include firedtv.h
 
 #define FCP_COMMAND_REGISTER   0xfb00ULL
@@ -368,10 +370,30 @@ static int avc_tuner_tuneqpsk(struct fir
c-operand[12] = 0;
 
if (fdtv-type == FIREDTV_DVB_S2) {
-   c-operand[13] = 0x1;
-   c-operand[14] = 0xff;
-   c-operand[15] = 0xff;
-
+   if (fdtv-fe.dtv_property_cache.delivery_system == SYS_DVBS2) {
+   switch (fdtv-fe.dtv_property_cache.modulation) {
+   case QAM_16:c-operand[13] = 0x1; break;
+   case QPSK:  c-operand[13] = 0x2; break;
+   case PSK_8: c-operand[13] = 0x3; break;
+   default:c-operand[13] = 0x2; break;
+   }
+   switch (fdtv-fe.dtv_property_cache.rolloff) {
+   case ROLLOFF_AUTO:  c-operand[14] = 0x2; break;
+   case ROLLOFF_35:c-operand[14] = 0x2; break;
+   case ROLLOFF_20:c-operand[14] = 0x0; break;
+   case ROLLOFF_25:c-operand[14] = 0x1; break;
+   /* case ROLLOFF_NONE:   c-operand[14] = 0xff; break; */
+   }
+   switch (fdtv-fe.dtv_property_cache.pilot) {
+   case PILOT_AUTO:c-operand[15] = 0x0; break;
+   case PILOT_OFF: c-operand[15] = 0x0; break;
+   case PILOT_ON:  c-operand[15] = 0x1; break;
+   }
+   } else {
+   c-operand[13] = 0x1;  /* auto modulation */
+   c-operand[14] = 0xff; /* disable rolloff */
+   c-operand[15] = 0xff; /* disable pilot */
+   }
return 16;
} else {
return 13;
Index: b/drivers/media/dvb/firewire/firedtv-fe.c
===
--- a/drivers/media/dvb/firewire/firedtv-fe.c
+++ b/drivers/media/dvb/firewire/firedtv-fe.c
@@ -155,6 +155,16 @@ static int fdtv_get_frontend(struct dvb_
return -EOPNOTSUPP;
 }
 
+static int fdtv_get_property(struct dvb_frontend *fe, struct dtv_property *tvp)
+{
+   return 0;
+}
+
+static int fdtv_set_property(struct dvb_frontend *fe, struct dtv_property *tvp)
+{
+   return 0;
+}
+
 void fdtv_frontend_init(struct firedtv *fdtv)
 {
struct dvb_frontend_ops *ops = fdtv-fe.ops;
@@ -166,6 +176,9 @@ void fdtv_frontend_init(struct firedtv *
ops-set_frontend   = fdtv_set_frontend;
ops-get_frontend   = fdtv_get_frontend;
 
+   ops-get_property   = fdtv_get_property;
+   ops-set_property   = fdtv_set_property;
+
ops-read_status= fdtv_read_status;
ops-read_ber   = fdtv_read_ber;
ops-read_signal_strength   = fdtv_read_signal_strength;
@@ -179,7 +192,6 @@ void fdtv_frontend_init(struct firedtv *
 
switch (fdtv-type) {
case FIREDTV_DVB_S:
-   case FIREDTV_DVB_S2:
fi-type= FE_QPSK;
 
fi-frequency_min   = 95;
@@ -188,7 +200,7 @@ void fdtv_frontend_init(struct firedtv *
fi-symbol_rate_min = 100;
fi-symbol_rate_max = 4000;
 
-   fi-caps= FE_CAN_INVERSION_AUTO |
+   fi-caps= FE_CAN_INVERSION_AUTO |
  FE_CAN_FEC_1_2|
  FE_CAN_FEC_2_3|
  FE_CAN_FEC_3_4|
@@ -198,6 +210,26 @@ void fdtv_frontend_init(struct firedtv *
  FE_CAN_QPSK;
break

Re: [PATCH] firedtv driver: support for PSK8 for S2 devices. To watch HD.

2010-09-15 Thread Stefan Richter
Tommy Jonsson wrote at linux-media:
 This is the first i have ever developed for linux, cant really wrap my
 head around how to submit this..
 Hope im sending this correctly, diff made with 'hg diff' from latest
 hg clone http://linuxtv.org/hg/v4l-dvb;
 
 It adds support for tuning with PSK8 modulation, pilot and rolloff
 with the S2 versions of firedtv.
 
 Signed-off-by: Tommy Jonsson quazz...@gmail.com
[...]

Excellent!  This has been on the wishlist of FireDTV/FloppyDTV-S2 owners for
quite some time.

The patch was a little bit mangled by the mail user agent, and there appear to
be some whitespace issues in it.  I will have a closer look at it later today
and repost the patch so that Mauro can apply it without manual intervention.
-- 
Stefan Richter
-=-==-=- =--= -
http://arcgraph.de/sr/
--
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] firedtv driver: support for PSK8 for S2 devices. To watch HD.

2010-09-15 Thread Stefan Richter
Tommy Jonsson wrote:
 --- a/linux/drivers/media/dvb/firewire/firedtv-avc.c  Fri Sep 03
 00:28:05 2010 -0300
 +++ b/linux/drivers/media/dvb/firewire/firedtv-avc.c  Sun Sep 12
 06:52:02 2010 +0200
[...]
 @@ -368,10 +369,30 @@
   c-operand[12] = 0;
 
   if (fdtv-type == FIREDTV_DVB_S2) {
 - c-operand[13] = 0x1;
 - c-operand[14] = 0xff;
 - c-operand[15] = 0xff;
 -
 + if (fe-dtv_property_cache.delivery_system == SYS_DVBS2) {
 + switch (fe-dtv_property_cache.modulation) {
 + case QAM_16:c-operand[13] = 0x1; break;
 + case QPSK:  c-operand[13] = 0x2; break;
 + case PSK_8: c-operand[13] = 0x3; break;
 + default:c-operand[13] = 0x2; break;
 + }
 + switch (fe-dtv_property_cache.rolloff) {
 + case ROLLOFF_AUTO:  c-operand[14] = 0x2; break;
 + case ROLLOFF_35:c-operand[14] = 0x2; break;
 + case ROLLOFF_20:c-operand[14] = 0x0; break;
 + case ROLLOFF_25:c-operand[14] = 0x1; break;
 + /* case ROLLOFF_NONE:   c-operand[14] = 0xff; break; */
 + }
 + switch (fe-dtv_property_cache.pilot) {
 + case PILOT_AUTO:c-operand[15] = 0x0; break;
 + case PILOT_OFF: c-operand[15] = 0x0; break;
 + case PILOT_ON:  c-operand[15] = 0x1; break;
 + }
 + } else {
 + c-operand[13] = 0x1;  /* auto modulation */
 + c-operand[14] = 0xff; /* disable rolloff */
 + c-operand[15] = 0xff; /* disable pilot */
 + }
   return 16;

Is it correct that there is no default: case for operand[14] and [15]?

   } else {
   return 13;
 @@ -548,7 +569,7 @@
   return 17 + add_pid_filter(fdtv, c-operand[17]);
  }
 
 -int avc_tuner_dsd(struct firedtv *fdtv,
 +int avc_tuner_dsd(struct dvb_frontend *fe, struct firedtv *fdtv,
 struct dvb_frontend_parameters *params)
  {
   struct avc_command_frame *c = (void *)fdtv-avc_data;

The frontend can be accessed via fdtv-fe also.  (I can change this together
with the whitespace things if you agree.)

 @@ -561,7 +582,7 @@
 
   switch (fdtv-type) {
   case FIREDTV_DVB_S:
 - case FIREDTV_DVB_S2: pos = avc_tuner_tuneqpsk(fdtv, params); break;
 + case FIREDTV_DVB_S2: pos = avc_tuner_tuneqpsk(fe, fdtv, params); break;
   case FIREDTV_DVB_C: pos = avc_tuner_dsd_dvb_c(fdtv, params); break;
   case FIREDTV_DVB_T: pos = avc_tuner_dsd_dvb_t(fdtv, params); break;
   default:
 diff -r 6e0befab696a linux/drivers/media/dvb/firewire/firedtv-fe.c
 --- a/linux/drivers/media/dvb/firewire/firedtv-fe.c   Fri Sep 03
 00:28:05 2010 -0300
 +++ b/linux/drivers/media/dvb/firewire/firedtv-fe.c   Sun Sep 12
 06:52:02 2010 +0200
[...]
 @@ -155,6 +156,17 @@
   return -EOPNOTSUPP;
  }
 
 +static int fdtv_get_property(struct dvb_frontend *fe,
 + struct dtv_property *tvp)
 +{
 + return 0;
 +}
 +static int fdtv_set_property(struct dvb_frontend *fe,
 + struct dtv_property *tvp)
 +{
 + return 0;
 +}
 +
  void fdtv_frontend_init(struct firedtv *fdtv)
  {
   struct dvb_frontend_ops *ops = fdtv-fe.ops;
 @@ -166,6 +178,9 @@
   ops-set_frontend   = fdtv_set_frontend;
   ops-get_frontend   = fdtv_get_frontend;
 
 + ops-get_property   = fdtv_get_property;
 + ops-set_property   = fdtv_set_property;
 +
   ops-read_status= fdtv_read_status;
   ops-read_ber   = fdtv_read_ber;
   ops-read_signal_strength   = fdtv_read_signal_strength;
[...]

(Hmm, note to self:  Can't DVB core provide empty default methods?)
-- 
Stefan Richter
-=-==-=- =--= -
http://arcgraph.de/sr/
--
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/8]reiserfs:stree.c Fix variable set but not used.

2010-06-14 Thread Stefan Richter
On 14 Jun, Justin P. Mattock wrote:
 On 06/14/2010 02:47 PM, Edward Shishkin wrote:
 Whitespaces should be removed.
 I recommend quilt package for managing patches:
 quilt refresh --strip-trailing-whitespace is your friend..
 
 o.k. I resent this.. fixed the whitespace(hopefully)
 and add your Acked to it.
 as for quilt I'll have to look into that..
 (using a lfs system, so if the sourcecode is easy
 to deal with(build), then it's a good but if it becomes
 a nightmare maybe not!!).

Since you appear to generate the patches with git, you can use git diff
--check [...] for some basic whitespace checks (additions of trailing
space, additions of space before tab).  For more extensive checks, try
git diff [...] | scripts/checkpatch.pl -.  Check this before you
commit.  If you committed already, git commit --amend [-a] [...] lets
you alter the very last commit of course.
-- 
Stefan Richter
-=-==-=- -==- -
http://arcgraph.de/sr/

--
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/3] IR: add core lirc device interface

2010-06-05 Thread Stefan Richter
Jon Smirl wrote:
 On Fri, Jun 4, 2010 at 5:17 PM, Jarod Wilson ja...@wilsonet.com wrote:
 Hrm, struct file_operations specifies an unsigned long for the ioctl
 args, so doesn't that mean we're pretty much stuck with only 32-bit
 for the ioctls?
 
 I haven't written an IOCTL in a while, but how would you pass a 64b
 memory address?

On architectures with 64 bits wide memory addresses, unsigned long
happens to be 64 bits wide too. :-)  IOW unsigned long is defined such
that casts from and to void * are lossless.

This is not universal though.  E.g. Microsoft compilers define unsigned
long always only as 32 bits wide AFAIK.
-- 
Stefan Richter
-=-==-=- -==- --=-=
http://arcgraph.de/sr/
--
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: post 2.6.34 bug: new code enabled by default

2010-05-26 Thread Stefan Richter
Mauro Carvalho Chehab wrote:
 Stefan Richter wrote:
[CONFIG_RC_MAP et al]
 Please leave the default of new options at N.

 (Unless this were a special case of new options that replaced older
 options and need to be migrated to 'on' per default in make oldconfig.
 I think this is not the case here.)
 
 This is the case here. Before the RC subsystem, the decoding for NEC and RC-5
 were done inside ir-core (at ir-functions). Also, all the keymap entries 
 (RC_MAP)
 were compiled in-kernel.

OK.  I happened to have a setup in which nothing of this was actually
used before.  (CONFIG_FIREDTV as only DVB tuner driver.)  --- Aha, it is
just a consequence of ir-core being enabled by default regardless if
needed, since 2.6.33:

config IR_CORE
tristate
depends on INPUT
default INPUT
-- 
Stefan Richter
-=-==-=- -=-= ==-=-
http://arcgraph.de/sr/
--
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: ideal DVB-C PCI/e card? [linux-media]

2010-05-25 Thread Stefan Richter
Jed wrote:
 On 24/05/10 4:18 AM, Stefan Richter wrote:
 Jed wrote:
 Ideally it'd be dual DVB-C, the only one I've found is more than dual
 DVB-C  is far too expensive.

 If you need two receivers but can only spare up to one PCI or PCIe slot,
 why not use two USB or FireWire attached receivers?

 FireWire ones seem to be out of production now though and weren't
 exactly on the cheap side.  OTOH one can drive up to 3 DVB FireWire
 receivers on a single FireWire bus; and for those who need even more
 there are dual link FireWire PCI and PCIe cards readily available.
 
 Thanks for offering your thoughts Stefan.
 Any specific recommendations?

 Ideally I want two or more dvb-c tuners in a pci/e form-factor.

 If there's FW or USB tuners that are mounted onto a PCI/e card, work
 well in Linux,  are relatively cheap, then I'd love to know!

I don't have an overview over USB tuners.

FireWire tuners are (or rather were) available as external boxes as well
as cards that could be mounted either in a PCI(e) slot --- but still had
to be connected to an internal or external FireWire port then --- or in
a floppy disk bay.  One tuner took up one slot or one bay.  Slot for CAM
included.

As I said, the FireWire tuners were _not_ cheap, compared to average USB
tuners or PCI tuners.  Maybe used one can be found to a somewhat better
price.  FireWire DVB tuners that were sold in the past by different
vendors were similar in hardware AFAIK, but only ones from Digital
Everywhere (called FireDTV and FloppyDTV) are supported under Linux
because DE supplied initial driver code and firmware information.

If you go for USB tuners, then I guess that you will also have to use
either external devices /or/ drive-bay mounted devices /or/ two PCI(e)
slots, since you wrote that you need CAMs --- and I doubt that there is
a cheap off-the-shelf solution that crams two CAM slots into a single
PCI slot or shares a CAM between tuners...  But as I said, I don't have
an overview.
-- 
Stefan Richter
-=-==-=- -=-= ==--=
http://arcgraph.de/sr/
--
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


post 2.6.34 bug: new code enabled by default

2010-05-25 Thread Stefan Richter
$ make oldconfig
...
  * Multimedia drivers
  *
  Compile Remote Controller keymap modules (RC_MAP) [M/n/?] (NEW) n
  Enable IR raw decoder for the NEC protocol (IR_NEC_DECODER) [M/n/?] (NEW) n
  Enable IR raw decoder for the RC-5 protocol (IR_RC5_DECODER) [M/n/?] (NEW) n
  Enable IR raw decoder for the RC6 protocol (IR_RC6_DECODER) [M/n/?] (NEW) n
  Enable IR raw decoder for the JVC protocol (IR_JVC_DECODER) [M/n/?] (NEW) n
  Enable IR raw decoder for the Sony protocol (IR_SONY_DECODER) [M/n/?] (NEW) n

Please leave the default of new options at N.

(Unless this were a special case of new options that replaced older
options and need to be migrated to 'on' per default in make oldconfig.
I think this is not the case here.)
-- 
Stefan Richter
-=-==-=- -=-= ==--=
http://arcgraph.de/sr/


--
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: ideal DVB-C PCI/e card? [linux-media]

2010-05-23 Thread Stefan Richter
Jed wrote:
 Ideally it'd be dual DVB-C, the only one I've found is more than dual
 DVB-C  is far too expensive.

If you need two receivers but can only spare up to one PCI or PCIe slot,
why not use two USB or FireWire attached receivers?

FireWire ones seem to be out of production now though and weren't
exactly on the cheap side.  OTOH one can drive up to 3 DVB FireWire
receivers on a single FireWire bus; and for those who need even more
there are dual link FireWire PCI and PCIe cards readily available.
-- 
Stefan Richter
-=-==-=- -=-= =-===
http://arcgraph.de/sr/
--
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: V4L-DVB drivers and BKL

2010-04-03 Thread Stefan Richter
Mauro Carvalho Chehab wrote:
 Hans Verkuil wrote:
 On the DVB side there seem to be only two sources that use the BKL:

 linux/drivers/media/dvb/bt8xx/dst_ca.c: lock_kernel();
 linux/drivers/media/dvb/bt8xx/dst_ca.c: unlock_kernel();
 linux/drivers/media/dvb/dvb-core/dvbdev.c:  lock_kernel();
 linux/drivers/media/dvb/dvb-core/dvbdev.c:  unlock_kernel();
 linux/drivers/media/dvb/dvb-core/dvbdev.c:  unlock_kernel();

 At first glance it doesn't seem too difficult to remove them, but I leave
 that to the DVB experts.
 
 The main issue is at dvbdev, since it is used by all devices. We need to get 
 rid
 of it.

Get rid of its lock_kernel in open and its locked ioctl, or of the
dvbdev 'library' itself?

 That's said, Stefan Richter sent a patch meant to reduce the issues with
 DVB. Unfortunately, I haven't seen any comments on it. It would be really 
 important
 to test his approach.

I will attempt to continue with this (convert more drivers if not all,
in a properly organized patch series for review), though it won't happen
overnight as I'm chronically short of spare time.  I.e. if others step
in, even better.
-- 
Stefan Richter
-=-==-=- -=-- ---==
http://arcgraph.de/sr/
--
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: V4L-DVB drivers and BKL

2010-04-01 Thread Stefan Richter
Hans Verkuil wrote:
 On the DVB side there seem to be only two sources that use the BKL:
 
 linux/drivers/media/dvb/bt8xx/dst_ca.c: lock_kernel();
 linux/drivers/media/dvb/bt8xx/dst_ca.c: unlock_kernel();

This is from an incomplete conversion from .ioctl to .unlocked_ioctl (no
conversion really, only a BKL push-down).

 linux/drivers/media/dvb/dvb-core/dvbdev.c:  lock_kernel();
 linux/drivers/media/dvb/dvb-core/dvbdev.c:  unlock_kernel();
 linux/drivers/media/dvb/dvb-core/dvbdev.c:  unlock_kernel();

This is from when the BKL was pushed down into drivers' open() methods.
To remove BKL from open(), check for possible races with module
insertion.  (A driver's module_init has to have set up everything that's
going to be used by open() before the char device is being registered.)

Apart from those two BKL uses in media/dvb/, there are also
  - remaining .ioctl that need to be checked for possible concurrency
issues, then converted to .unlocked_ioctl,
  - remaining .llseek uses (all implicit) which need to be checked
whether they should be no_llseek() (accompanied by nonseekable_open)
or generic_file_llseek() or default_llseek().
-- 
Stefan Richter
-=-==-=- -=-- =
http://arcgraph.de/sr/
--
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: V4L-DVB drivers and BKL

2010-04-01 Thread Stefan Richter
Stefan Richter wrote:
 linux/drivers/media/dvb/dvb-core/dvbdev.c:  lock_kernel();
 linux/drivers/media/dvb/dvb-core/dvbdev.c:  unlock_kernel();
 linux/drivers/media/dvb/dvb-core/dvbdev.c:  unlock_kernel();
 
 This is from when the BKL was pushed down into drivers' open() methods.
 To remove BKL from open(), check for possible races with module
 insertion.  (A driver's module_init has to have set up everything that's
 going to be used by open() before the char device is being registered.)

Last sentence was supposed to mean:  Before the char device is being
registered, a driver's module_init has to have set up everything that's
going to be used by openers of the file.

(Traditionally, the BKL serialized open() with module initialization,
which was not obvious to driver writers because it happened deep in the
core kernel.)
-- 
Stefan Richter
-=-==-=- -=-- =
http://arcgraph.de/sr/
--
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: V4L-DVB drivers and BKL

2010-04-01 Thread Stefan Richter
Hans Verkuil wrote:
 I just read on LWN that the core kernel guys are putting more effort into
 removing the BKL. We are still using it in our own drivers, mostly V4L.
 
 I added a BKL column to my driver list:
 
 http://www.linuxtv.org/wiki/index.php/V4L_framework_progress#Bridge_Drivers
 
 If you 'own' one of these drivers that still use BKL, then it would be nice
 if you can try and remove the use of the BKL from those drivers.
 
 The other part that needs to be done is to move from using the .ioctl file op
 to using .unlocked_ioctl. Very few drivers do that, but I suspect almost no
 driver actually needs to use .ioctl.

Also note that struct file_operations.llseek() grabs the BKL if .llseek
= default_llseek, or if .llseek == NULL  (struct file.f_mode 
FMODE_LSEEK) != 0.

I guess V4L/DVB character device file ABIs do not involve lseek() and
friends, do they?  If so, are the files flagged as non-seekable?

 On the DVB side there seem to be only two sources that use the BKL:
 
 linux/drivers/media/dvb/bt8xx/dst_ca.c: lock_kernel();
 linux/drivers/media/dvb/bt8xx/dst_ca.c: unlock_kernel();
 linux/drivers/media/dvb/dvb-core/dvbdev.c:  lock_kernel();
 linux/drivers/media/dvb/dvb-core/dvbdev.c:  unlock_kernel();
 linux/drivers/media/dvb/dvb-core/dvbdev.c:  unlock_kernel();
 
 At first glance it doesn't seem too difficult to remove them, but I leave
 that to the DVB experts.

As a dvb/firewire/firedtv user, I started to mess around with dvbdev and
firedtv:  https://patchwork.kernel.org/patch/88778/
-- 
Stefan Richter
-=-==-=- -=-- =
http://arcgraph.de/sr/
--
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 RFC v2] DVB: add dvb_generic_nonseekable_open, dvb_generic_unlocked_ioctl, use in firedtv

2010-03-28 Thread Stefan Richter
In order to remove Big Kernel Lock usages from the DVB subsystem,
we need to
  - provide .llseek file operations that do not grab the BKL (unlike
fs/read_write.c::default_llseek) or mark files as not seekable,
  - provide .unlocked_ioctl file operations.

Add two dvb_generic_ file operations for file interfaces which are not
seekable and, respectively, do not require the BKL in their ioctl
handlers.

Use them in one driver of which I am sure of that these are applicable.
(Affected code paths in firedtv-ci were not runtime-tested since I don't
have a CAM, but the frontend ioctls were of course runtime-tested.)

Notes:

  - The dvb-core internal dvb_usercopy() API is changed to match
.unlocked_ioctl() prototypes.

  - I suspect that all dvb_generic_open() users really want
nonseekable_open --- then we should simply change dvb_generic_open()
instead of adding dvb_generic_nonseekable_open() --- but I haven't
checked other users of dvb_generic_open whether they require
.llssek mehods other than fs/read_write.c::no_llseek.
Applies to:
drivers/media/dvb/ttpci/av7110.c
drivers/media/dvb/ttpci/av7110_av.c
drivers/media/dvb/ttpci/av7110_ca.c
drivers/media/dvb/dvb-core/dvb_net.c
drivers/media/dvb/dvb-core/dvb_frontend.c
drivers/media/dvb/dvb-core/dvb_ca_en50221.c

  - To be done by those who know the code:  Check all users of
struct dvb_device.kernel_ioctl whether they really need the BKL.
Convert to .unlocked_ioctl and remove .kernel_ioctl and the
temporarily introduced dvbdev.c::legacy_usercopy().
Applies to:
drivers/media/dvb/ttpci/av7110.c
drivers/media/dvb/ttpci/av7110_av.c
drivers/media/dvb/ttpci/av7110_ca.c
drivers/media/dvb/dvb-core/dvb_frontend.c

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---

Update:  Split dvb_usercopy into one that follows the .unlocked_ioctl
prototype form and another one that preserves the old form.

 drivers/media/dvb/dvb-core/dmxdev.c |   10 -
 drivers/media/dvb/dvb-core/dvb_ca_en50221.c |6 
 drivers/media/dvb/dvb-core/dvb_net.c|5 
 drivers/media/dvb/dvb-core/dvbdev.c |  190 +---
 drivers/media/dvb/dvb-core/dvbdev.h |   23 +-
 drivers/media/dvb/firewire/firedtv-ci.c |9 
 6 files changed, 148 insertions(+), 95 deletions(-)

Index: b/drivers/media/dvb/dvb-core/dmxdev.c
===
--- a/drivers/media/dvb/dvb-core/dmxdev.c
+++ b/drivers/media/dvb/dvb-core/dmxdev.c
@@ -963,8 +963,7 @@ dvb_demux_read(struct file *file, char _
return ret;
 }
 
-static int dvb_demux_do_ioctl(struct inode *inode, struct file *file,
- unsigned int cmd, void *parg)
+static long dvb_demux_do_ioctl(struct file *file, unsigned int cmd, void *parg)
 {
struct dmxdev_filter *dmxdevfilter = file-private_data;
struct dmxdev *dmxdev = dmxdevfilter-dev;
@@ -1087,7 +1086,7 @@ static int dvb_demux_do_ioctl(struct ino
 static int dvb_demux_ioctl(struct inode *inode, struct file *file,
   unsigned int cmd, unsigned long arg)
 {
-   return dvb_usercopy(inode, file, cmd, arg, dvb_demux_do_ioctl);
+   return dvb_usercopy(file, cmd, arg, dvb_demux_do_ioctl);
 }
 
 static unsigned int dvb_demux_poll(struct file *file, poll_table *wait)
@@ -1152,8 +1151,7 @@ static struct dvb_device dvbdev_demux = 
.fops = dvb_demux_fops
 };
 
-static int dvb_dvr_do_ioctl(struct inode *inode, struct file *file,
-   unsigned int cmd, void *parg)
+static long dvb_dvr_do_ioctl(struct file *file, unsigned int cmd, void *parg)
 {
struct dvb_device *dvbdev = file-private_data;
struct dmxdev *dmxdev = dvbdev-priv;
@@ -1179,7 +1177,7 @@ static int dvb_dvr_do_ioctl(struct inode
 static int dvb_dvr_ioctl(struct inode *inode, struct file *file,
 unsigned int cmd, unsigned long arg)
 {
-   return dvb_usercopy(inode, file, cmd, arg, dvb_dvr_do_ioctl);
+   return dvb_usercopy(file, cmd, arg, dvb_dvr_do_ioctl);
 }
 
 static unsigned int dvb_dvr_poll(struct file *file, poll_table *wait)
Index: b/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
===
--- a/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
@@ -1181,8 +1181,8 @@ static int dvb_ca_en50221_thread(void *d
  *
  * @return 0 on success, 0 on error.
  */
-static int dvb_ca_en50221_io_do_ioctl(struct inode *inode, struct file *file,
- unsigned int cmd, void *parg)
+static long dvb_ca_en50221_io_do_ioctl(struct file *file,
+  unsigned int cmd, void *parg)
 {
struct dvb_device *dvbdev = file-private_data;
struct dvb_ca_private *ca = dvbdev-priv;
@@ -1258,7 +1258,7 @@ static int dvb_ca_en50221_io_do_ioctl(st
 static int dvb_ca_en50221_io_ioctl

[PATCH RFC] DVB: add dvb_generic_nonseekable_open, dvb_generic_unlocked_ioctl, use in firedtv

2010-03-27 Thread Stefan Richter
In order to remove Big Kernel Lock usages from the DVB subsystem,
we need to
  - provide .llseek file operations that do not grab the BKL (unlike
fs/read_write.c::default_llseek) or mark files as not seekable,
  - provide .unlocked_ioctl file operations.

Add two dvb_generic_ file operations for file interfaces which are not
seekable and, respectively, do not require the BKL in their ioctl
handlers.

Use them in one driver of which I am sure of that these are applicable.
(Affected code paths were not runtime-tested since I don't have a CAM.)

Notes:

  - In order to maximize code reuse and minimize API churn, I pass a
dummy NULL struct inode * through dvb_usercopy() to
dvbdev-kernel_ioctl().  Very ugly; it may be better to convert
everything from .ioctl to .unlocked_ioctl in one go and remove the
inode pointer argument everywhere.

  - I suspect that actually all dvb_generic_open() users really want
nonseekable_open --- then we should simply change dvb_generic_open()
instead of adding dvb_generic_nonseekable_open() --- but I haven't
checked other users of dvb_generic_open whether they require
.llssek mehods other than fs/read_write.c::no_llseek.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---

This patch is motivated by Arnd's
bkl removal: make unlocked_ioctl mandatory
http://git.kernel.org/?p=linux/kernel/git/arnd/playground.git;a=blobdiff;f=drivers/media/dvb/firewire/firedtv-ci.c;h=7ab89035c101240a860d6c72bf8541a0fdf3aed9;hp=853e04b7cb361d45257f80dcafdcf121cd340c63;hb=05e7753338045e9ee3950b2da032c5e5774efa90;hpb=03165e1d096afb4b1d9cfccdad66eed038121cec
BKL removal: mark remaining users as 'depends on BKL'
http://git.kernel.org/?p=linux/kernel/git/arnd/playground.git;a=blobdiff;f=drivers/media/dvb/firewire/Kconfig;h=ba8938e26414c8c82c41677f87c1361ed8fcebd0;hp=4afa29256df110d3383b6b469762cefbb46436b6;hb=abb83d8fe5f8dcc8fca09bd9117429f73e1417e0;hpb=33c014b118f45516113d4b6823e40ea6f834dc6a


 drivers/media/dvb/dvb-core/dvbdev.c |   20 
 drivers/media/dvb/dvb-core/dvbdev.h |   11 +++
 drivers/media/dvb/firewire/firedtv-ci.c |4 ++--
 3 files changed, 29 insertions(+), 6 deletions(-)

Index: b/drivers/media/dvb/dvb-core/dvbdev.c
===
--- a/drivers/media/dvb/dvb-core/dvbdev.c
+++ b/drivers/media/dvb/dvb-core/dvbdev.c
@@ -135,6 +135,18 @@ int dvb_generic_open(struct inode *inode
 EXPORT_SYMBOL(dvb_generic_open);
 
 
+int dvb_generic_nonseekable_open(struct inode *inode, struct file *file)
+{
+   int retval = dvb_generic_open(inode, file);
+
+   if (retval == 0)
+   retval = nonseekable_open(inode, file);
+
+   return retval;
+}
+EXPORT_SYMBOL(dvb_generic_nonseekable_open);
+
+
 int dvb_generic_release(struct inode *inode, struct file *file)
 {
struct dvb_device *dvbdev = file-private_data;
@@ -170,6 +182,14 @@ int dvb_generic_ioctl(struct inode *inod
 EXPORT_SYMBOL(dvb_generic_ioctl);
 
 
+long dvb_generic_unlocked_ioctl(struct file *file,
+   unsigned int cmd, unsigned long arg)
+{
+   return dvb_generic_ioctl(NULL, file, cmd, arg);
+}
+EXPORT_SYMBOL(dvb_generic_unlocked_ioctl);
+
+
 static int dvbdev_get_free_id (struct dvb_adapter *adap, int type)
 {
u32 id = 0;
Index: b/drivers/media/dvb/dvb-core/dvbdev.h
===
--- a/drivers/media/dvb/dvb-core/dvbdev.h
+++ b/drivers/media/dvb/dvb-core/dvbdev.h
@@ -136,10 +136,13 @@ extern int dvb_register_device (struct d
 
 extern void dvb_unregister_device (struct dvb_device *dvbdev);
 
-extern int dvb_generic_open (struct inode *inode, struct file *file);
-extern int dvb_generic_release (struct inode *inode, struct file *file);
-extern int dvb_generic_ioctl (struct inode *inode, struct file *file,
- unsigned int cmd, unsigned long arg);
+extern int dvb_generic_open(struct inode *inode, struct file *file);
+extern int dvb_generic_nonseekable_open(struct inode *inode, struct file 
*file);
+extern int dvb_generic_release(struct inode *inode, struct file *file);
+extern int dvb_generic_ioctl(struct inode *inode, struct file *file,
+unsigned int cmd, unsigned long arg);
+extern long dvb_generic_unlocked_ioctl(struct file *file,
+  unsigned int cmd, unsigned long arg);
 
 /* we don't mess with video_usercopy() any more,
 we simply define out own dvb_usercopy(), which will hopefully become
Index: b/drivers/media/dvb/firewire/firedtv-ci.c
===
--- a/drivers/media/dvb/firewire/firedtv-ci.c
+++ b/drivers/media/dvb/firewire/firedtv-ci.c
@@ -217,8 +217,8 @@ static unsigned int fdtv_ca_io_poll(stru
 
 static const struct file_operations fdtv_ca_fops = {
.owner  = THIS_MODULE,
-   .ioctl  = dvb_generic_ioctl,
-   .open

Re: [PATCH] firedtv: add parameter to fake ca_system_ids in CA_INFO

2010-03-06 Thread Stefan Richter
Mauro Carvalho Chehab wrote:
 Stefan Richter wrote:
 
 The Digital Everywhere firmware have the shortcoming that ca_info_enq and
 ca_info are not supported. This means that we can never retrieve the correct
 ca_system_id to present in the CI message CA_INFO. Currently the driver uses
 the application id retrieved using app_info_req and app_info, but this id
 only match the correct ca_system_id as given in ca_info in some cases.
 This patch adds a parameter to the driver in order for the user to override
 what will be returned in the CA_INFO CI message. Up to four ca_system_ids can
 be specified.
 This is needed for users with CAMs that have different manufacturer id and
 ca_system_id and that uses applications that take this into account, like
 MythTV.
 
 This seems an ugly workaround. The better seems to patch MythTV to accept a 
 different
 CAM.

Ugly it is, for sure.  Can't comment on application-level solutions; if
thats the proper layer at which to address this, then that would be
preferable of course.

 +static int num_fake_ca_system_ids;
 ...
 +for (i = 0; i  num_fake_ca_system_ids; i++) {
 +app_info[4 + i * 2] =
 +(fake_ca_system_ids[i]  8)  0xff;
 ...
 
 NAK. If someone put an arbitrary high value for num_fake_ca_system_id's, it 
 will write outside
 the app_info array space, as the num_fake_ca_system_ids is not validated 
 against the size
 of app_info.

That's what I thought at first look at the patch too, but then I noticed
that inlcude/linux/moduleparam.h and kernel/params.c properly track
kparam_arry.max = ARRAY_SIZE(array).
http://lxr.linux.no/#linux+v2.6.33/include/linux/moduleparam.h#L62
http://lxr.linux.no/#linux+v2.6.33/include/linux/moduleparam.h#L213
http://lxr.linux.no/#linux+v2.6.33/kernel/params.c#L351
http://lxr.linux.no/#linux+v2.6.33/kernel/params.c#L296

So no danger here.

 Also, it makes no sense a negative value for this parameter.

I already posted an updated version of the patch which correctly defines
num_fake_ca_system_ids as an unsigned long.
-- 
Stefan Richter
-=-==-=- --== --==-
http://arcgraph.de/sr/
--
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] firedtv: add parameter to fake ca_system_ids in CA_INFO

2010-03-06 Thread Stefan Richter
Stefan Richter wrote:
 I already posted an updated version of the patch which correctly defines
 num_fake_ca_system_ids as an unsigned long.

err, unsigned int of course, as http://patchwork.kernel.org/patch/82912/.
-- 
Stefan Richter
-=-==-=- --== --==-
http://arcgraph.de/sr/
--
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] firedtv: correct version number and current/next in CA_PMT

2010-03-01 Thread Stefan Richter
Date: Tue, 21 Jul 2009 18:45:50 +0200
From: Henrik Kurelid hen...@kurelid.se

The version number in the CA_PMT message sent to the hardware was
alwaysed set to zero. This could cause problems if the PMT would
change during decryption of a channel since the new CA_PMT would have
the same version number as the old. The version number is now copied
from the original PMT.

Signed-off-by: Henrik Kurelid hen...@kurelid.se
Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---

This patch got stuck somehow on the long way upstream. :-)
Would be good to get into one of the next .34-rc releases.

 drivers/media/dvb/firewire/firedtv-avc.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/drivers/media/dvb/firewire/firedtv-avc.c
===
--- a/drivers/media/dvb/firewire/firedtv-avc.c
+++ b/drivers/media/dvb/firewire/firedtv-avc.c
@@ -1096,7 +1096,7 @@ int avc_ca_pmt(struct firedtv *fdtv, cha
 
c-operand[15] = msg[1]; /* Program number */
c-operand[16] = msg[2];
-   c-operand[17] = 0x01; /* Version number=0 + current/next=1 */
+   c-operand[17] = msg[3]; /* Version number and current/next */
c-operand[18] = 0x00; /* Section number=0 */
c-operand[19] = 0x00; /* Last section number=0 */
c-operand[20] = 0x1f; /* PCR_PID=1FFF */

-- 
Stefan Richter
-=-==-=- --== =
http://arcgraph.de/sr/

--
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] firedtv: add parameter to fake ca_system_ids in CA_INFO

2010-03-01 Thread Stefan Richter
Date: Sun, 4 Oct 2009 15:00:36 +0200
From: Henrik Kurelid hen...@kurelid.se

The Digital Everywhere firmware have the shortcoming that ca_info_enq and
ca_info are not supported. This means that we can never retrieve the correct
ca_system_id to present in the CI message CA_INFO. Currently the driver uses
the application id retrieved using app_info_req and app_info, but this id
only match the correct ca_system_id as given in ca_info in some cases.
This patch adds a parameter to the driver in order for the user to override
what will be returned in the CA_INFO CI message. Up to four ca_system_ids can
be specified.
This is needed for users with CAMs that have different manufacturer id and
ca_system_id and that uses applications that take this into account, like
MythTV.

Signed-off-by: Henrik Kurelid hen...@kurelid.se
Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de (rebased, format of 
comment)
---

Would be good to get into an .34-rc too.

 drivers/media/dvb/firewire/firedtv-avc.c |   31 ---
 1 file changed, 27 insertions(+), 4 deletions(-)

Index: b/drivers/media/dvb/firewire/firedtv-avc.c
===
--- a/drivers/media/dvb/firewire/firedtv-avc.c
+++ b/drivers/media/dvb/firewire/firedtv-avc.c
@@ -130,6 +130,20 @@ MODULE_PARM_DESC(debug, Verbose logging
, FCP payloads =  __stringify(AVC_DEBUG_FCP_PAYLOADS)
, or a combination, or all = -1));
 
+/*
+ * This is a workaround since there is no vendor specific command to retrieve
+ * ca_info using AVC. If this parameter is not used, ca_system_id will be
+ * filled with application_manufacturer from ca_app_info.
+ * Digital Everywhere have said that adding ca_info is on their TODO list.
+ */
+static int num_fake_ca_system_ids;
+static int fake_ca_system_ids[4] = { -1, -1, -1, -1 };
+module_param_array(fake_ca_system_ids, int, num_fake_ca_system_ids, 0644);
+MODULE_PARM_DESC(fake_ca_system_ids, If your CAM application manufacturer 
+does not have the same ca_system_id as your CAS, you can 
+override what ca_system_ids are presented to the 
+application by setting this field to an array of ids.);
+
 static const char *debug_fcp_ctype(unsigned int ctype)
 {
static const char *ctypes[] = {
@@ -977,7 +991,7 @@ int avc_ca_info(struct firedtv *fdtv, ch
 {
struct avc_command_frame *c = (void *)fdtv-avc_data;
struct avc_response_frame *r = (void *)fdtv-avc_data;
-   int pos, ret;
+   int i, pos, ret;
 
mutex_lock(fdtv-avc_mutex);
 
@@ -1004,9 +1018,18 @@ int avc_ca_info(struct firedtv *fdtv, ch
app_info[0] = (EN50221_TAG_CA_INFO  16)  0xff;
app_info[1] = (EN50221_TAG_CA_INFO   8)  0xff;
app_info[2] = (EN50221_TAG_CA_INFO   0)  0xff;
-   app_info[3] = 2;
-   app_info[4] = r-operand[pos + 0];
-   app_info[5] = r-operand[pos + 1];
+   if (num_fake_ca_system_ids == 0) {
+   app_info[3] = 2;
+   app_info[4] = r-operand[pos + 0];
+   app_info[5] = r-operand[pos + 1];
+   } else {
+   app_info[3] = num_fake_ca_system_ids * 2;
+   for (i = 0; i  num_fake_ca_system_ids; i++) {
+   app_info[4 + i * 2] =
+   (fake_ca_system_ids[i]  8)  0xff;
+   app_info[5 + i * 2] = fake_ca_system_ids[i]  0xff;
+   }
+   }
*len = app_info[3] + 4;
 out:
mutex_unlock(fdtv-avc_mutex);

-- 
Stefan Richter
-=-==-=- --== =
http://arcgraph.de/sr/

--
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] firedtv: add parameter to fake ca_system_ids in CA_INFO

2010-03-01 Thread Stefan Richter
 The Digital Everywhere firmware have the shortcoming that ca_info_enq and
 ca_info are not supported. This means that we can never retrieve the correct
 ca_system_id to present in the CI message CA_INFO. Currently the driver uses
 the application id retrieved using app_info_req and app_info, but this id
 only match the correct ca_system_id as given in ca_info in some cases.
 This patch adds a parameter to the driver in order for the user to override
 what will be returned in the CA_INFO CI message. Up to four ca_system_ids can
 be specified.
 This is needed for users with CAMs that have different manufacturer id and
 ca_system_id and that uses applications that take this into account, like
 MythTV.

This workaround is of course rather awkward.  Users who need this will
have a hard time to work out that this parameter needs to be set and
how.  Furthermore, the web site of Digital Everywhere says that hey
stopped manufacturing and ramped down support, hence it looks like this
issue will stay with us forever.

Isn't there a CA command that could be (mis)used for some kind of
probing of the CAM for correct system IDs?  E.g. a loop which retries a
kind of dummy operation with different system IDs until success,
initially during fdtv_dvb_register/ fdtv_ca_register?

(I don't know how CI works, and alas I don't have a CAM myself for
testing and am not very keen on getting one...)
-- 
Stefan Richter
-=-==-=- --== =
http://arcgraph.de/sr/

--
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 update] firedtv: add parameter to fake ca_system_ids in CA_INFO

2010-03-01 Thread Stefan Richter
From: Henrik Kurelid hen...@kurelid.se
Subject: firedtv: add parameter to fake ca_system_ids in CA_INFO

The Digital Everywhere firmware have the shortcoming that ca_info_enq and
ca_info are not supported. This means that we can never retrieve the correct
ca_system_id to present in the CI message CA_INFO. Currently the driver uses
the application id retrieved using app_info_req and app_info, but this id
only match the correct ca_system_id as given in ca_info in some cases.
This patch adds a parameter to the driver in order for the user to override
what will be returned in the CA_INFO CI message. Up to four ca_system_ids can
be specified.
This is needed for users with CAMs that have different manufacturer id and
ca_system_id and that uses applications that take this into account, like
MythTV.

Signed-off-by: Henrik Kurelid hen...@kurelid.se
Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de (rebased, type of 
num_fake_ca_system_ids, format of comment)
---

Update:  I found a sparse warning after the fact. num_fake_ca_system_ids
is meant to be unsigned, not signed.  Sorry that I missed it in my first
posting; it was hidden by an unrelated sparse error in
include/linux/skbuff.h which was introduced by commit 14d18a81.  I need
to have a word (again) with the author of this commit.

 drivers/media/dvb/firewire/firedtv-avc.c |   31 ---
 1 file changed, 27 insertions(+), 4 deletions(-)

Index: b/drivers/media/dvb/firewire/firedtv-avc.c
===
--- a/drivers/media/dvb/firewire/firedtv-avc.c
+++ b/drivers/media/dvb/firewire/firedtv-avc.c
@@ -130,6 +130,20 @@ MODULE_PARM_DESC(debug, Verbose logging
, FCP payloads =  __stringify(AVC_DEBUG_FCP_PAYLOADS)
, or a combination, or all = -1));
 
+/*
+ * This is a workaround since there is no vendor specific command to retrieve
+ * ca_info using AVC. If this parameter is not used, ca_system_id will be
+ * filled with application_manufacturer from ca_app_info.
+ * Digital Everywhere have said that adding ca_info is on their TODO list.
+ */
+static unsigned int num_fake_ca_system_ids;
+static int fake_ca_system_ids[4] = { -1, -1, -1, -1 };
+module_param_array(fake_ca_system_ids, int, num_fake_ca_system_ids, 0644);
+MODULE_PARM_DESC(fake_ca_system_ids, If your CAM application manufacturer 
+does not have the same ca_system_id as your CAS, you can 
+override what ca_system_ids are presented to the 
+application by setting this field to an array of ids.);
+
 static const char *debug_fcp_ctype(unsigned int ctype)
 {
static const char *ctypes[] = {
@@ -977,7 +991,7 @@ int avc_ca_info(struct firedtv *fdtv, ch
 {
struct avc_command_frame *c = (void *)fdtv-avc_data;
struct avc_response_frame *r = (void *)fdtv-avc_data;
-   int pos, ret;
+   int i, pos, ret;
 
mutex_lock(fdtv-avc_mutex);
 
@@ -1004,9 +1018,18 @@ int avc_ca_info(struct firedtv *fdtv, ch
app_info[0] = (EN50221_TAG_CA_INFO  16)  0xff;
app_info[1] = (EN50221_TAG_CA_INFO   8)  0xff;
app_info[2] = (EN50221_TAG_CA_INFO   0)  0xff;
-   app_info[3] = 2;
-   app_info[4] = r-operand[pos + 0];
-   app_info[5] = r-operand[pos + 1];
+   if (num_fake_ca_system_ids == 0) {
+   app_info[3] = 2;
+   app_info[4] = r-operand[pos + 0];
+   app_info[5] = r-operand[pos + 1];
+   } else {
+   app_info[3] = num_fake_ca_system_ids * 2;
+   for (i = 0; i  num_fake_ca_system_ids; i++) {
+   app_info[4 + i * 2] =
+   (fake_ca_system_ids[i]  8)  0xff;
+   app_info[5 + i * 2] = fake_ca_system_ids[i]  0xff;
+   }
+   }
*len = app_info[3] + 4;
 out:
mutex_unlock(fdtv-avc_mutex);

-- 
Stefan Richter
-=-==-=- --== =
http://arcgraph.de/sr/

--
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


How to add DVB-S2 support to firedtv?

2010-02-14 Thread Stefan Richter
Hi all,

what steps need to be taken to get DVB-S2 support into the firedtv
driver?  (The status is, as far as I understood:  FireDTV S2 and Floppy
DTV S2 devices recognize HD channels during channel scan but cannot tune
to them.  FireDTV C/CI DVB-C boxes however tune and play back HD
channels just fine.)

I suppose the frontend needs to be extended for s2api.  Was there a
respective conversion in another DVB driver that can serve as a good
coding example?

Is documentation from Digital Everywhere required regarding the
vendor-specific AV/C requests (LNB_CONTROL? TUNE_QPSK2?) or is the
current driver code enough to connect the dots?

Is the transport stream different from DVB-C HD streams so that changes
to the isochronous I/O part would be required?
-- 
Stefan Richter
-=-==-=- --=- -===-
http://arcgraph.de/sr/
--
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: [hg:v4l-dvb] firedtv: do not DMA-map stack addresses

2010-02-01 Thread Stefan Richter
 The patch number 14077 was added via Douglas Schilling Landgraf 
 dougsl...@redhat.com
 to http://linuxtv.org/hg/v4l-dvb master development tree.
[...]
 [dougsl...@redhat.com: patch backported to hg tree]

I don't know how you prefer to organize your trees, but:
In this particular case it could have been simpler if you had first
inserted an hg:v4l-dvb only patch which simply reverts the divergence of
firedtv in hg from mainline git.

This divergence was introduced by some kind of hg-git export mistake.
That's not a big issue but it may cause another mistake when the next
hg-git export happens.
-- 
Stefan Richter
-=-==-=- --=- =
http://arcgraph.de/sr/
--
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: [hg:v4l-dvb] firedtv: do not DMA-map stack addresses

2010-02-01 Thread Stefan Richter
Mauro Carvalho Chehab wrote:
 Hi Stefan,
[...]
 We've replaced our procedures on our trees recently. Until last year,
 I was applying the patches at -hg and then converting to -git.
 
 This year, we're just doing the reverse:
[...]

Ah, thanks for the heads-up.  This way my worries are unfounded. :-)
-- 
Stefan Richter
-=-==-=- --=- =
http://arcgraph.de/sr/
--
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] firedtv: add forgotten __exit annotation

2010-01-10 Thread Stefan Richter
Stefan Richter wrote on 2009-12-26:
 fdtv_fw_exit() is part of the firedtv driver's .exit.text section.
...
 --- linux-2.6.33-rc2.orig/drivers/media/dvb/firewire/firedtv-fw.c
 +++ linux-2.6.33-rc2/drivers/media/dvb/firewire/firedtv-fw.c
 @@ -332,7 +332,7 @@ int __init fdtv_fw_init(void)
   return driver_register(fdtv_driver.driver);
  }
  
 -void fdtv_fw_exit(void)
 +void __exit fdtv_fw_exit(void)
  {
   driver_unregister(fdtv_driver.driver);
   fw_core_remove_address_handler(fcp_handler);
 

This patch is bogus.  fdtv_fw_exit() is also called from firedtv's init.
-- 
Stefan Richter
-=-==-=- ---= -=-=-
http://arcgraph.de/sr/
--
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 v4l/dvb] firedtv: add forgotten __exit annotation

2009-12-25 Thread Stefan Richter
fdtv_fw_exit() is part of the firedtv driver's .exit.text section.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
 drivers/media/dvb/firewire/firedtv-fw.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.33-rc2/drivers/media/dvb/firewire/firedtv-fw.c
===
--- linux-2.6.33-rc2.orig/drivers/media/dvb/firewire/firedtv-fw.c
+++ linux-2.6.33-rc2/drivers/media/dvb/firewire/firedtv-fw.c
@@ -332,7 +332,7 @@ int __init fdtv_fw_init(void)
return driver_register(fdtv_driver.driver);
 }
 
-void fdtv_fw_exit(void)
+void __exit fdtv_fw_exit(void)
 {
driver_unregister(fdtv_driver.driver);
fw_core_remove_address_handler(fcp_handler);

-- 
Stefan Richter
-=-==--= ==-- ==-=-
http://arcgraph.de/sr/

--
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 v4l/dvb] firedtv: add missing NULL pointer check

2009-12-25 Thread Stefan Richter
If there is ever going to be a FireDTV or FloppyDTV firmware which does
not provide a minimal ASCII textual descriptor for Model_Id --- or if
the descriptor is provided indirectly in a descriptor directory ---
the ieee1394 variant of the device probe of firedtv would dereference a
NULL pointer.  The firewire variant of firedtv's device probe is not
affected.

The fix makes sure that such an unexpected firmware is safely recognized
by fdtv_alloc as an unknown firmware.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
 drivers/media/dvb/firewire/firedtv-1394.c |   10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Index: linux-2.6.33-rc2/drivers/media/dvb/firewire/firedtv-1394.c
===
--- linux-2.6.33-rc2.orig/drivers/media/dvb/firewire/firedtv-1394.c
+++ linux-2.6.33-rc2/drivers/media/dvb/firewire/firedtv-1394.c
@@ -193,9 +193,13 @@ static int node_probe(struct device *dev
int kv_len, err;
void *kv_str;
 
-   kv_len = (ud-model_name_kv-value.leaf.len - 2) * sizeof(quadlet_t);
-   kv_str = CSR1212_TEXTUAL_DESCRIPTOR_LEAF_DATA(ud-model_name_kv);
-
+   if (ud-model_name_kv) {
+   kv_len = (ud-model_name_kv-value.leaf.len - 2) * 4;
+   kv_str = 
CSR1212_TEXTUAL_DESCRIPTOR_LEAF_DATA(ud-model_name_kv);
+   } else {
+   kv_len = 0;
+   kv_str = NULL;
+   }
fdtv = fdtv_alloc(dev, fdtv_1394_backend, kv_str, kv_len);
if (!fdtv)
return -ENOMEM;

-- 
Stefan Richter
-=-==--= ==-- ==-=-
http://arcgraph.de/sr/

--
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] What are the goals for the architecture of an in-kernel IR system?

2009-11-28 Thread Stefan Richter
Jon Smirl wrote:
 There are two very basic things that we need to reach consensus on first.
 
 1) Unification with mouse/keyboard in evdev - put IR on equal footing.
 2) Specific tools (xmodmap, setkeycodes, etc or the LIRC ones) or
 generic tools (ls, mkdir, echo) for configuration

About 2:  If at all, there only needs to be a decision about pulse/space
to scancode converter configuration.  In contrast, scancode to keycode
converter configuration is already solved; the interface is
EVIOCSKEYCODE.  If you find the EVIOCSKEYCODE interface lacking, extend
it or submit an alternative --- but this does not affect LIRC and
whether to merge it in any way.

PS:  Drop your specific vs. generic tools terminology already.  Your
configfs based proposal requires specific tools as well, it's just
that they can be implemented in bash, among else.
-- 
Stefan Richter
-=-==--= =-== ===--
http://arcgraph.de/sr/
--
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] What are the goals for the architecture of an in-kernel IR system?

2009-11-28 Thread Stefan Richter
Jon Smirl wrote:
 If these drivers are for specific USB devices it is straight forward
 to turn them into kernel based drivers. If we are going for plug and
 play this needs to happen. All USB device drivers can be implemented
 in user space, but that doesn't mean you want to do that. Putting
 device drivers in the kernel subjects them to code inspection, they
 get shipped everywhere, they autoload when the device is inserted,
 they participate in suspend/resume, etc.

Huh?  Userspace implementations /can/ be code-reviewed (but they can't
crash your machine), they /can/ be and are shipped everywhere, they /do/
auto-load when the device is inserted.  And if there should be an issue
with power management (is there any?), then improve the ABI and libusb
can surely be improved.  I don't see why a device with a userspace
driver cannot be included in power management.
-- 
Stefan Richter
-=-==--= =-== ===--
http://arcgraph.de/sr/
--
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] What are the goals for the architecture of an in-kernel IR system?

2009-11-28 Thread Stefan Richter
Jon Smirl wrote:
 On Sat, Nov 28, 2009 at 1:17 PM, Stefan Richter
 stef...@s5r6.in-berlin.de wrote:
 Jon Smirl wrote:
 There are two very basic things that we need to reach consensus on first.

 1) Unification with mouse/keyboard in evdev - put IR on equal footing.
 2) Specific tools (xmodmap, setkeycodes, etc or the LIRC ones) or
 generic tools (ls, mkdir, echo) for configuration
 About 2:  If at all, there only needs to be a decision about pulse/space
 to scancode converter configuration.  In contrast, scancode to keycode
 converter configuration is already solved; the interface is
 EVIOCSKEYCODE.  If you find the EVIOCSKEYCODE interface lacking, extend
 it or submit an alternative --- but this does not affect LIRC and
 whether to merge it in any way.
 
 EVIOCSKEYCODE is lacking, first parameter is an INT. Some decoded IR
 codes are over 32b. Christoph posted an example that needs 128b. This
 is a problem with ioctls, they change size depending on platform and
 endianess.

No, they do not change size depending on platform and endianess if
basic rules are observed.  Defining compatible ioctls is not rocket
science.  Sure, int should not be used in ioctl arguments or other
binary interfaces.

(I never said EVIOCSKEYCODE was not lacking, I only said it exists
already.  When you talk about better scancode-to-keycode converter
configuration, then you are talking about EVIOCSKEYCODE, not about LIRC
or a hypothetic replacement of LIRC.  Ergo, the decision about whether
to merge LIRC or not is not blocked by this configuration interface issue.)

 Also, how do you create the devices for each remote? You would need to
 create these devices before being able to do EVIOCSKEYCODE to them.

The input subsystem creates devices on behalf of input drivers.  (Kernel
drivers, that is.  Userspace drivers are per se not affected.)

 PS:  Drop your specific vs. generic tools terminology already.  Your
 configfs based proposal requires specific tools as well, it's just
 that they can be implemented in bash, among else.
 
 The shell commands are the most generic tools in Unix.

The shell scripts are still special-purpose programs.

 udev already knows how to run shell scripts.
[...]

Udev can run any kind of program, compiled or interpreted.
-- 
Stefan Richter
-=-==--= =-== ===--
http://arcgraph.de/sr/
--
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] What are the goals for the architecture of an in-kernel IR system?

2009-11-28 Thread Stefan Richter
Jon Smirl wrote:
 On Sat, Nov 28, 2009 at 2:30 PM, Stefan Richter
 stef...@s5r6.in-berlin.de wrote:
 Jon Smirl wrote:
 If these drivers are for specific USB devices it is straight forward
 to turn them into kernel based drivers. If we are going for plug and
 play this needs to happen. All USB device drivers can be implemented
 in user space, but that doesn't mean you want to do that. Putting
 device drivers in the kernel subjects them to code inspection, they
 get shipped everywhere, they autoload when the device is inserted,
 they participate in suspend/resume, etc.
 Huh?  Userspace implementations /can/ be code-reviewed (but they can't
 crash your machine), they /can/ be and are shipped everywhere, they /do/
 auto-load when the device is inserted.  And if there should be an issue
 with power management (is there any?), then improve the ABI and libusb
 can surely be improved.  I don't see why a device with a userspace
 driver cannot be included in power management.
 
 If you want a micro-kernel there are plenty to pick from. Linux has
 chosen not to be a micro-kernel. The Linux model is device drivers in
 the kernel.

Total nonsense.  Neither am I arguing for a micro-kernel, nor are
userspace drivers alien to Linux.  Not at all.
-- 
Stefan Richter
-=-==--= =-== ===--
http://arcgraph.de/sr/
--
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] What are the goals for the architecture of an in-kernel IR system?

2009-11-28 Thread Stefan Richter
Jon Smirl wrote:
 On Sat, Nov 28, 2009 at 2:45 PM, Stefan Richter
 stef...@s5r6.in-berlin.de wrote:
 Jon Smirl wrote:
 Also, how do you create the devices for each remote? You would need to
 create these devices before being able to do EVIOCSKEYCODE to them.
 The input subsystem creates devices on behalf of input drivers.  (Kernel
 drivers, that is.  Userspace drivers are per se not affected.)
 
 We have one IR receiver device and multiple remotes. How does the
 input system know how many devices to create corresponding to how many
 remotes you have?

If several remotes are to be used on the same receiver, then they
necessarily need to generate different scancodes, don't they?  Otherwise
the input driver wouldn't be able to route their events to the
respective subdevice.  But if they do generate different scancodes,
there is no need to create subdevices just for EVIOCSKEYCODE's sake. (It
might still be desirable to have subdevices for other reasons perhaps.)
-- 
Stefan Richter
-=-==--= =-== ===--
http://arcgraph.de/sr/
--
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] What are the goals for the architecture of an in-kernel IR system?

2009-11-28 Thread Stefan Richter
Stefan Richter wrote:
 Jon Smirl wrote:
 We have one IR receiver device and multiple remotes. How does the
 input system know how many devices to create corresponding to how many
 remotes you have?
 
 If several remotes are to be used on the same receiver, then they
 necessarily need to generate different scancodes, don't they?  Otherwise
 the input driver wouldn't be able to route their events to the
 respective subdevice.  But if they do generate different scancodes,
 there is no need to create subdevices just for EVIOCSKEYCODE's sake. (It
 might still be desirable to have subdevices for other reasons perhaps.)

PS, forgot to add:  If there is a real need to initiate device creation
from userspace, then ioctl is not the way to go.
-- 
Stefan Richter
-=-==--= =-== ===--
http://arcgraph.de/sr/
--
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: [IR-RFC PATCH v4 0/6] In-kernel IR support using evdev

2009-11-26 Thread Stefan Richter
Jarod Wilson wrote:
 On 11/26/2009 08:34 PM, Jon Smirl wrote:
 Raw mode. There are three sysfs attributes - ir_raw, ir_carrier,
 ir_xmitter. Read from ir_raw to get the raw timing data from the IR
 device. Set carrier and active xmitters and then copy raw data to
 ir_raw to send. These attributes may be better on a debug switch. You
 would use raw mode when decoding a new protocol. After you figure out
 the new protocol, write an in-kernel encoder/decoder for it.
 
 Also neglected to recall there was raw IR data access too. However, a
 few things... One, this is, in some sense, cheating, as its not an input
 layer interface being used. :) Granted though, it *is* an existing
 kernel interface being used, instead of adding a new one. Two, there's
 no userspace to do anything with it at this time.

No; it is a new interface, just using an existing mechanism (sysfs). Not
all of sysfs in itself is an interface really; rather there is a number
of interfaces which are implemented by means of sysfs.

sysfs is primarily meant for simple textual attributes though, not for
I/O streams.
-- 
Stefan Richter
-=-==--= =-== ==-==
http://arcgraph.de/sr/
--
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] Should we create a raw input interface for IR's ? - Was: Re: [PATCH 1/3 v2] lirc core device driver infrastructure

2009-11-23 Thread Stefan Richter
Krzysztof Halasa wrote:
 Mauro Carvalho Chehab mche...@redhat.com writes:
 
 Event input has the advantage that the keystrokes will provide an unique
 representation that is independent of the device.
 
 This can hardly work as the only means, the remotes have different keys,
 the user almost always has to provide customized keyfunction mapping.

Modern input drivers in the mainline kernel have a scancode-to-keycode
translation table (or equivalent) which can be overwritten by userspace.
The mechanism to do that is the EVIOCSKEYCODE ioctl.

(This is no recommendation for lirc.  I have no idea whether a
pulse/space - scancode - keycode translation would be practical there.)
-- 
Stefan Richter
-=-==--= =-== =-===
http://arcgraph.de/sr/
--
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/6] DVB: firedtv: simplifications and a portability fix

2009-11-20 Thread Stefan Richter
On 18 Nov, Stefan Richter wrote:
 The following three patches are applicable after firedtv: port to new
 firewire core from 2009-11-08:
...
 The rest of this patch set additionally requires the latest firedtv as
 of 2.6.32-rc7:
...

I updated the firedtv branch at

git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git 
firedtv

now (based on v2.6.31 but having a merge from v2.6.32-rc8 in it due to
above mentioned requirement).

Mauro, please harvest the posted 4 + 6 patches from the mailing list, or
pull or cherry-pick them from linux1394-2.6.git firedtv.  Thanks.

Stefan Richter (11):
  firedtv: move remote control workqueue handling into rc source file
  firedtv: reform lock transaction backend call
  firedtv: add missing include, rename a constant
  firedtv: port to new firewire core
  firedtv: shrink buffer pointer table
  firedtv: packet requeuing is likely to succeed
  firedtv: remove an unnecessary function argument
  Merge tag 'v2.6.32-rc8' into firedtv
  firedtv: do not DMA-map stack addresses
  firedtv: remove check for interrupting signal
  firedtv: reduce memset()s

 drivers/media/dvb/firewire/Kconfig|7 +-
 drivers/media/dvb/firewire/Makefile   |1 +
 drivers/media/dvb/firewire/firedtv-1394.c |   42 +-
 drivers/media/dvb/firewire/firedtv-avc.c  |  566 +++--
 drivers/media/dvb/firewire/firedtv-dvb.c  |   16 +-
 drivers/media/dvb/firewire/firedtv-fw.c   |  376 ++
 drivers/media/dvb/firewire/firedtv-rc.c   |2 +
 drivers/media/dvb/firewire/firedtv.h  |   23 +-
 8 files changed, 746 insertions(+), 287 deletions(-)
-- 
Stefan Richter
-=-==--= =-== =-=-=
http://arcgraph.de/sr/

--
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 0/6] DVB: firedtv: simplifications and a portability fix

2009-11-18 Thread Stefan Richter
The following three patches are applicable after firedtv: port to new
firewire core from 2009-11-08:

[PATCH 1/6] firedtv: shrink buffer pointer table
[PATCH 2/6] firedtv: packet requeuing is likely to succeed
[PATCH 3/6] firedtv: remove an unnecessary function argument

The rest of this patch set additionally requires the latest firedtv as
of 2.6.32-rc7:

[PATCH 4/6] firedtv: do not DMA-map stack addresses
[PATCH 5/6] firedtv: remove check for interrupting signal
[PATCH 6/6] firedtv: reduce memset()s

 drivers/media/dvb/firewire/firedtv-1394.c |   13
 drivers/media/dvb/firewire/firedtv-avc.c  |  520 +++---
 drivers/media/dvb/firewire/firedtv-dvb.c  |1
 drivers/media/dvb/firewire/firedtv-fw.c   |   39 -
 drivers/media/dvb/firewire/firedtv.h  |8
 5 files changed, 306 insertions(+), 275 deletions(-)
-- 
Stefan Richter
-=-==--= =-== =--=-
http://arcgraph.de/sr/

--
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 1/6] firedtv: shrink buffer pointer table

2009-11-18 Thread Stefan Richter
Cache only addresses of whole pages, not of each buffer chunk.  Besides,
page addresses can be obtained by page_address() instead of kmap() since
they were allocated in lowmem.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
 drivers/media/dvb/firewire/firedtv-fw.c |   19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

Index: linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-fw.c
===
--- linux-2.6.32-rc7.orig/drivers/media/dvb/firewire/firedtv-fw.c
+++ linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-fw.c
@@ -6,9 +6,9 @@
 #include linux/errno.h
 #include linux/firewire.h
 #include linux/firewire-constants.h
-#include linux/highmem.h
 #include linux/kernel.h
 #include linux/list.h
+#include linux/mm.h
 #include linux/slab.h
 #include linux/spinlock.h
 #include linux/types.h
@@ -73,7 +73,7 @@ struct firedtv_receive_context {
struct fw_iso_buffer buffer;
int interrupt_packet;
int current_packet;
-   char *packets[N_PACKETS];
+   char *pages[N_PAGES];
 };
 
 static int queue_iso(struct firedtv_receive_context *ctx, int index)
@@ -100,7 +100,7 @@ static void handle_iso(struct fw_iso_con
struct firedtv *fdtv = data;
struct firedtv_receive_context *ctx = fdtv-backend_data;
__be32 *h, *h_end;
-   int i = ctx-current_packet, length, err;
+   int length, err, i = ctx-current_packet;
char *p, *p_end;
 
for (h = header, h_end = h + header_length / 4; h  h_end; h++) {
@@ -110,7 +110,8 @@ static void handle_iso(struct fw_iso_con
length = MAX_PACKET_SIZE;
}
 
-   p = ctx-packets[i];
+   p = ctx-pages[i / PACKETS_PER_PAGE]
+   + (i % PACKETS_PER_PAGE) * MAX_PACKET_SIZE;
p_end = p + length;
 
for (p += CIP_HEADER_SIZE + MPEG2_TS_HEADER_SIZE; p  p_end;
@@ -130,8 +131,7 @@ static int start_iso(struct firedtv *fdt
 {
struct firedtv_receive_context *ctx;
struct fw_device *device = device_of(fdtv);
-   char *p;
-   int i, j, k, err;
+   int i, err;
 
ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
if (!ctx)
@@ -153,11 +153,8 @@ static int start_iso(struct firedtv *fdt
ctx-interrupt_packet = 1;
ctx-current_packet = 0;
 
-   for (i = 0, k = 0; k  N_PAGES; k++) {
-   p = kmap(ctx-buffer.pages[k]);
-   for (j = 0; j  PACKETS_PER_PAGE  i  N_PACKETS; j++, i++)
-   ctx-packets[i] = p + j * MAX_PACKET_SIZE;
-   }
+   for (i = 0; i  N_PAGES; i++)
+   ctx-pages[i] = page_address(ctx-buffer.pages[i]);
 
for (i = 0; i  N_PACKETS; i++) {
err = queue_iso(ctx, i);

-- 
Stefan Richter
-=-==--= =-== =--=-
http://arcgraph.de/sr/

--
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 2/6] firedtv: packet requeuing is likely to succeed

2009-11-18 Thread Stefan Richter
Packet DMA buffers are queued either initially all at once (then, a
queueing failure will cause firedtv to release the DMA context as a
whole) or subsequently one by one as they recycled after use (then a
failure is extremely unlikely).  Therefore we can be a little less
cautious when counting at which packet buffer to set the interrupt flag.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
 drivers/media/dvb/firewire/firedtv-fw.c |   13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

Index: linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-fw.c
===
--- linux-2.6.32-rc7.orig/drivers/media/dvb/firewire/firedtv-fw.c
+++ linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-fw.c
@@ -79,19 +79,14 @@ struct firedtv_receive_context {
 static int queue_iso(struct firedtv_receive_context *ctx, int index)
 {
struct fw_iso_packet p;
-   int err;
 
p.payload_length = MAX_PACKET_SIZE;
-   p.interrupt = !(ctx-interrupt_packet  (IRQ_INTERVAL - 1));
+   p.interrupt = !(++ctx-interrupt_packet  (IRQ_INTERVAL - 1));
p.skip = 0;
p.header_length = ISO_HEADER_SIZE;
 
-   err = fw_iso_context_queue(ctx-context, p, ctx-buffer,
-  index * MAX_PACKET_SIZE);
-   if (!err)
-   ctx-interrupt_packet++;
-
-   return err;
+   return fw_iso_context_queue(ctx-context, p, ctx-buffer,
+   index * MAX_PACKET_SIZE);
 }
 
 static void handle_iso(struct fw_iso_context *context, u32 cycle,
@@ -150,7 +145,7 @@ static int start_iso(struct firedtv *fdt
if (err)
goto fail_context_destroy;
 
-   ctx-interrupt_packet = 1;
+   ctx-interrupt_packet = 0;
ctx-current_packet = 0;
 
for (i = 0; i  N_PAGES; i++)

-- 
Stefan Richter
-=-==--= =-== =--=-
http://arcgraph.de/sr/

--
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 3/6] firedtv: remove an unnecessary function argument

2009-11-18 Thread Stefan Richter
All read transactions initiated by firedtv are only quadlet-sized, hence
the backend-read call can be simplified a little.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
 drivers/media/dvb/firewire/firedtv-1394.c |4 ++--
 drivers/media/dvb/firewire/firedtv-avc.c  |8 
 drivers/media/dvb/firewire/firedtv-fw.c   |5 ++---
 drivers/media/dvb/firewire/firedtv.h  |2 +-
 4 files changed, 9 insertions(+), 10 deletions(-)

Index: linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-1394.c
===
--- linux-2.6.32-rc7.orig/drivers/media/dvb/firewire/firedtv-1394.c
+++ linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-1394.c
@@ -101,9 +101,9 @@ static int node_lock(struct firedtv *fdt
return ret;
 }
 
-static int node_read(struct firedtv *fdtv, u64 addr, void *data, size_t len)
+static int node_read(struct firedtv *fdtv, u64 addr, void *data)
 {
-   return hpsb_node_read(node_of(fdtv), addr, data, len);
+   return hpsb_node_read(node_of(fdtv), addr, data, 4);
 }
 
 static int node_write(struct firedtv *fdtv, u64 addr, void *data, size_t len)
Index: linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-avc.c
===
--- linux-2.6.32-rc7.orig/drivers/media/dvb/firewire/firedtv-avc.c
+++ linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-avc.c
@@ -1236,14 +1236,14 @@ int avc_ca_get_mmi(struct firedtv *fdtv,
 
 #define CMP_OUTPUT_PLUG_CONTROL_REG_0  0xf904ULL
 
-static int cmp_read(struct firedtv *fdtv, void *buf, u64 addr, size_t len)
+static int cmp_read(struct firedtv *fdtv, u64 addr, __be32 *data)
 {
int ret;
 
if (mutex_lock_interruptible(fdtv-avc_mutex))
return -EINTR;
 
-   ret = fdtv-backend-read(fdtv, addr, buf, len);
+   ret = fdtv-backend-read(fdtv, addr, data);
if (ret  0)
dev_err(fdtv-device, CMP: read I/O error\n);
 
@@ -1293,7 +1293,7 @@ int cmp_establish_pp_connection(struct f
int attempts = 0;
int ret;
 
-   ret = cmp_read(fdtv, opcr, opcr_address, 4);
+   ret = cmp_read(fdtv, opcr_address, opcr);
if (ret  0)
return ret;
 
@@ -1357,7 +1357,7 @@ void cmp_break_pp_connection(struct fire
u64 opcr_address = CMP_OUTPUT_PLUG_CONTROL_REG_0 + (plug  2);
int attempts = 0;
 
-   if (cmp_read(fdtv, opcr, opcr_address, 4)  0)
+   if (cmp_read(fdtv, opcr_address, opcr)  0)
return;
 
 repeat:
Index: linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-fw.c
===
--- linux-2.6.32-rc7.orig/drivers/media/dvb/firewire/firedtv-fw.c
+++ linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-fw.c
@@ -46,10 +46,9 @@ static int node_lock(struct firedtv *fdt
return node_req(fdtv, addr, data, 8, TCODE_LOCK_COMPARE_SWAP);
 }
 
-static int node_read(struct firedtv *fdtv, u64 addr, void *data, size_t len)
+static int node_read(struct firedtv *fdtv, u64 addr, void *data)
 {
-   return node_req(fdtv, addr, data, len, len == 4 ?
-   TCODE_READ_QUADLET_REQUEST : TCODE_READ_BLOCK_REQUEST);
+   return node_req(fdtv, addr, data, 4, TCODE_READ_QUADLET_REQUEST);
 }
 
 static int node_write(struct firedtv *fdtv, u64 addr, void *data, size_t len)
Index: linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv.h
===
--- linux-2.6.32-rc7.orig/drivers/media/dvb/firewire/firedtv.h
+++ linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv.h
@@ -74,7 +74,7 @@ struct firedtv;
 
 struct firedtv_backend {
int (*lock)(struct firedtv *fdtv, u64 addr, __be32 data[]);
-   int (*read)(struct firedtv *fdtv, u64 addr, void *data, size_t len);
+   int (*read)(struct firedtv *fdtv, u64 addr, void *data);
int (*write)(struct firedtv *fdtv, u64 addr, void *data, size_t len);
int (*start_iso)(struct firedtv *fdtv);
void (*stop_iso)(struct firedtv *fdtv);

-- 
Stefan Richter
-=-==--= =-== =--=-
http://arcgraph.de/sr/

--
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 4/6] firedtv: do not DMA-map stack addresses

2009-11-18 Thread Stefan Richter
This is a portability fix and reduces stack usage.

The DMA mapping API cannot map on-stack addresses, as explained in
Documentation/DMA-mapping.txt.  Convert the two cases of on-stack packet
payload buffers in firedtv (payload of write requests in avc_write and
of lock requests in cmp_lock) to slab-allocated memory.

We use the 512 bytes sized FCP frame buffer in struct firedtv for this
purpose.  Previously it held only incoming FCP responses, now it holds
pending FCP requests and is then overwriten by an FCP response from the
tuner subunit.  Ditto for CMP lock requests and responses.  Accesses to
the payload buffer are serialized by fdtv-avc_mutex.

As a welcome side effect, stack usage of the AV/C transaction functions
is reduced by 512 bytes.

Alas, avc_register_remote_control() is a special case:  It previously
did not wait for a response.  To fit better in with the other FCP
transactions, let it wait for an interim response.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
 drivers/media/dvb/firewire/firedtv-1394.c |9 +
 drivers/media/dvb/firewire/firedtv-avc.c  |  437 +-
 drivers/media/dvb/firewire/firedtv-dvb.c  |1 -
 drivers/media/dvb/firewire/firedtv-fw.c   |2 
 drivers/media/dvb/firewire/firedtv.h  |6 
 5 files changed, 264 insertions(+), 191 deletions(-)

Index: linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-1394.c
===
--- linux-2.6.32-rc7.orig/drivers/media/dvb/firewire/firedtv-1394.c
+++ linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-1394.c
@@ -90,13 +90,14 @@ static inline struct node_entry *node_of
return container_of(fdtv-device, struct unit_directory, device)-ne;
 }
 
-static int node_lock(struct firedtv *fdtv, u64 addr, __be32 data[])
+static int node_lock(struct firedtv *fdtv, u64 addr, void *data)
 {
+   quadlet_t *d = data;
int ret;
 
-   ret = hpsb_node_lock(node_of(fdtv), addr, EXTCODE_COMPARE_SWAP,
-   (__force quadlet_t *)data[1], (__force quadlet_t)data[0]);
-   data[0] = data[1];
+   ret = hpsb_node_lock(node_of(fdtv), addr,
+EXTCODE_COMPARE_SWAP, d[1], d[0]);
+   d[0] = d[1];
 
return ret;
 }
Index: linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-avc.c
===
--- linux-2.6.32-rc7.orig/drivers/media/dvb/firewire/firedtv-avc.c
+++ linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-avc.c
@@ -74,7 +74,6 @@
 #define EN50221_TAG_CA_INFO0x9f8031
 
 struct avc_command_frame {
-   int length;
u8 ctype;
u8 subunit;
u8 opcode;
@@ -82,7 +81,6 @@ struct avc_command_frame {
 };
 
 struct avc_response_frame {
-   int length;
u8 response;
u8 subunit;
u8 opcode;
@@ -202,78 +200,65 @@ static void debug_pmt(char *msg, int len
   16, 1, msg, length, false);
 }
 
-static int __avc_write(struct firedtv *fdtv,
-   const struct avc_command_frame *c, struct avc_response_frame *r)
+static int avc_write(struct firedtv *fdtv)
 {
int err, retry;
 
-   if (r)
-   fdtv-avc_reply_received = false;
+   fdtv-avc_reply_received = false;
 
for (retry = 0; retry  6; retry++) {
if (unlikely(avc_debug))
-   debug_fcp(c-ctype, c-length);
+   debug_fcp(fdtv-avc_data, fdtv-avc_data_length);
 
err = fdtv-backend-write(fdtv, FCP_COMMAND_REGISTER,
-  (void *)c-ctype, c-length);
+   fdtv-avc_data, fdtv-avc_data_length);
if (err) {
-   fdtv-avc_reply_received = true;
dev_err(fdtv-device, FCP command write failed\n);
+
return err;
}
 
-   if (!r)
-   return 0;
-
/*
 * AV/C specs say that answers should be sent within 150 ms.
 * Time out after 200 ms.
 */
if (wait_event_timeout(fdtv-avc_wait,
   fdtv-avc_reply_received,
-  msecs_to_jiffies(200)) != 0) {
-   r-length = fdtv-response_length;
-   memcpy(r-response, fdtv-response, r-length);
-
+  msecs_to_jiffies(200)) != 0)
return 0;
-   }
}
dev_err(fdtv-device, FCP response timed out\n);
+
return -ETIMEDOUT;
 }
 
-static int avc_write(struct firedtv *fdtv,
-   const struct avc_command_frame *c, struct avc_response_frame *r)
+static bool is_register_rc(struct avc_response_frame *r)
 {
-   int ret;
-
-   if (mutex_lock_interruptible(fdtv-avc_mutex))
-   return -EINTR;
-
-   ret

[PATCH 5/6] firedtv: remove check for interrupting signal

2009-11-18 Thread Stefan Richter
FCP transactions as well as CMP transactions were serialized with
mutex_lock_interruptible.  It is extremely unlikly though that a signal
will arrive while a concurrent process holds the mutex.  And even if one
does, the duration of a transaction is reasonably short (1.2 seconds if
all retries time out, usually much shorter).

Hence simplify the code to plain mutex_lock.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
 drivers/media/dvb/firewire/firedtv-avc.c |   51 ---
 1 file changed, 17 insertions(+), 34 deletions(-)

Index: linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-avc.c
===
--- linux-2.6.32-rc7.orig/drivers/media/dvb/firewire/firedtv-avc.c
+++ linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-avc.c
@@ -542,8 +542,7 @@ int avc_tuner_dsd(struct firedtv *fdtv,
struct avc_command_frame *c = (void *)fdtv-avc_data;
int ret;
 
-   if (mutex_lock_interruptible(fdtv-avc_mutex))
-   return -EINTR;
+   mutex_lock(fdtv-avc_mutex);
 
memset(c, 0, sizeof(*c));
 
@@ -584,8 +583,7 @@ int avc_tuner_set_pids(struct firedtv *f
if (pidc  16  pidc != 0xff)
return -EINVAL;
 
-   if (mutex_lock_interruptible(fdtv-avc_mutex))
-   return -EINTR;
+   mutex_lock(fdtv-avc_mutex);
 
memset(c, 0, sizeof(*c));
 
@@ -629,8 +627,7 @@ int avc_tuner_get_ts(struct firedtv *fdt
struct avc_command_frame *c = (void *)fdtv-avc_data;
int ret, sl;
 
-   if (mutex_lock_interruptible(fdtv-avc_mutex))
-   return -EINTR;
+   mutex_lock(fdtv-avc_mutex);
 
memset(c, 0, sizeof(*c));
 
@@ -670,8 +667,7 @@ int avc_identify_subunit(struct firedtv 
struct avc_response_frame *r = (void *)fdtv-avc_data;
int ret;
 
-   if (mutex_lock_interruptible(fdtv-avc_mutex))
-   return -EINTR;
+   mutex_lock(fdtv-avc_mutex);
 
memset(c, 0, sizeof(*c));
 
@@ -712,8 +708,7 @@ int avc_tuner_status(struct firedtv *fdt
struct avc_response_frame *r = (void *)fdtv-avc_data;
int length, ret;
 
-   if (mutex_lock_interruptible(fdtv-avc_mutex))
-   return -EINTR;
+   mutex_lock(fdtv-avc_mutex);
 
memset(c, 0, sizeof(*c));
 
@@ -795,8 +790,7 @@ int avc_lnb_control(struct firedtv *fdtv
struct avc_response_frame *r = (void *)fdtv-avc_data;
int i, j, k, ret;
 
-   if (mutex_lock_interruptible(fdtv-avc_mutex))
-   return -EINTR;
+   mutex_lock(fdtv-avc_mutex);
 
memset(c, 0, sizeof(*c));
 
@@ -844,8 +838,7 @@ int avc_register_remote_control(struct f
struct avc_command_frame *c = (void *)fdtv-avc_data;
int ret;
 
-   if (mutex_lock_interruptible(fdtv-avc_mutex))
-   return -EINTR;
+   mutex_lock(fdtv-avc_mutex);
 
memset(c, 0, sizeof(*c));
 
@@ -883,8 +876,7 @@ int avc_tuner_host2ca(struct firedtv *fd
struct avc_command_frame *c = (void *)fdtv-avc_data;
int ret;
 
-   if (mutex_lock_interruptible(fdtv-avc_mutex))
-   return -EINTR;
+   mutex_lock(fdtv-avc_mutex);
 
memset(c, 0, sizeof(*c));
 
@@ -943,8 +935,7 @@ int avc_ca_app_info(struct firedtv *fdtv
struct avc_response_frame *r = (void *)fdtv-avc_data;
int pos, ret;
 
-   if (mutex_lock_interruptible(fdtv-avc_mutex))
-   return -EINTR;
+   mutex_lock(fdtv-avc_mutex);
 
memset(c, 0, sizeof(*c));
 
@@ -986,8 +977,7 @@ int avc_ca_info(struct firedtv *fdtv, ch
struct avc_response_frame *r = (void *)fdtv-avc_data;
int pos, ret;
 
-   if (mutex_lock_interruptible(fdtv-avc_mutex))
-   return -EINTR;
+   mutex_lock(fdtv-avc_mutex);
 
memset(c, 0, sizeof(*c));
 
@@ -1028,8 +1018,7 @@ int avc_ca_reset(struct firedtv *fdtv)
struct avc_command_frame *c = (void *)fdtv-avc_data;
int ret;
 
-   if (mutex_lock_interruptible(fdtv-avc_mutex))
-   return -EINTR;
+   mutex_lock(fdtv-avc_mutex);
 
memset(c, 0, sizeof(*c));
 
@@ -1073,8 +1062,7 @@ int avc_ca_pmt(struct firedtv *fdtv, cha
if (unlikely(avc_debug  AVC_DEBUG_APPLICATION_PMT))
debug_pmt(msg, length);
 
-   if (mutex_lock_interruptible(fdtv-avc_mutex))
-   return -EINTR;
+   mutex_lock(fdtv-avc_mutex);
 
memset(c, 0, sizeof(*c));
 
@@ -1196,8 +1184,7 @@ int avc_ca_get_time_date(struct firedtv 
struct avc_response_frame *r = (void *)fdtv-avc_data;
int ret;
 
-   if (mutex_lock_interruptible(fdtv-avc_mutex))
-   return -EINTR;
+   mutex_lock(fdtv-avc_mutex);
 
memset(c, 0, sizeof(*c));
 
@@ -1233,8 +1220,7 @@ int avc_ca_enter_menu(struct firedtv *fd
struct avc_command_frame *c = (void *)fdtv-avc_data;
int ret;
 
-   if (mutex_lock_interruptible(fdtv-avc_mutex))
-   return

[PATCH 6/6] firedtv: reduce memset()s

2009-11-18 Thread Stefan Richter
Before each FCP transdaction, the entire 512 bytes of the FCP frame were
cleared, then values filled in.

Clear only the bytes between filled-in bytes and end of the
  - request frame, or
  - response frame if data from a larger response will be needed, or
  - whole frame if data from a variable length response will be taken.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
 drivers/media/dvb/firewire/firedtv-avc.c |  146 ++-
 1 file changed, 65 insertions(+), 81 deletions(-)

Index: linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-avc.c
===
--- linux-2.6.32-rc7.orig/drivers/media/dvb/firewire/firedtv-avc.c
+++ linux-2.6.32-rc7/drivers/media/dvb/firewire/firedtv-avc.c
@@ -87,6 +87,21 @@ struct avc_response_frame {
u8 operand[509];
 };
 
+#define LAST_OPERAND (509 - 1)
+
+static inline void clear_operands(struct avc_command_frame *c, int from, int 
to)
+{
+   memset(c-operand[from], 0, to - from + 1);
+}
+
+static void pad_operands(struct avc_command_frame *c, int from)
+{
+   int to = ALIGN(from, 4);
+
+   if (from = to  to = LAST_OPERAND)
+   clear_operands(c, from, to);
+}
+
 #define AVC_DEBUG_READ_DESCRIPTOR  0x0001
 #define AVC_DEBUG_DSIT 0x0002
 #define AVC_DEBUG_DSD  0x0004
@@ -303,8 +318,8 @@ static int add_pid_filter(struct firedtv
  * tuning command for setting the relative LNB frequency
  * (not supported by the AVC standard)
  */
-static void avc_tuner_tuneqpsk(struct firedtv *fdtv,
-  struct dvb_frontend_parameters *params)
+static int avc_tuner_tuneqpsk(struct firedtv *fdtv,
+ struct dvb_frontend_parameters *params)
 {
struct avc_command_frame *c = (void *)fdtv-avc_data;
 
@@ -356,14 +371,15 @@ static void avc_tuner_tuneqpsk(struct fi
c-operand[13] = 0x1;
c-operand[14] = 0xff;
c-operand[15] = 0xff;
-   fdtv-avc_data_length = 20;
+
+   return 16;
} else {
-   fdtv-avc_data_length = 16;
+   return 13;
}
 }
 
-static void avc_tuner_dsd_dvb_c(struct firedtv *fdtv,
-   struct dvb_frontend_parameters *params)
+static int avc_tuner_dsd_dvb_c(struct firedtv *fdtv,
+  struct dvb_frontend_parameters *params)
 {
struct avc_command_frame *c = (void *)fdtv-avc_data;
 
@@ -427,13 +443,11 @@ static void avc_tuner_dsd_dvb_c(struct f
c-operand[20] = 0x00;
c-operand[21] = 0x00;
 
-   /* Add PIDs to filter */
-   fdtv-avc_data_length =
-   ALIGN(22 + add_pid_filter(fdtv, c-operand[22]) + 3, 4);
+   return 22 + add_pid_filter(fdtv, c-operand[22]);
 }
 
-static void avc_tuner_dsd_dvb_t(struct firedtv *fdtv,
-   struct dvb_frontend_parameters *params)
+static int avc_tuner_dsd_dvb_t(struct firedtv *fdtv,
+  struct dvb_frontend_parameters *params)
 {
struct dvb_ofdm_parameters *ofdm = params-u.ofdm;
struct avc_command_frame *c = (void *)fdtv-avc_data;
@@ -531,32 +545,31 @@ static void avc_tuner_dsd_dvb_t(struct f
c-operand[15] = 0x00; /* network_ID[0] */
c-operand[16] = 0x00; /* network_ID[1] */
 
-   /* Add PIDs to filter */
-   fdtv-avc_data_length =
-   ALIGN(17 + add_pid_filter(fdtv, c-operand[17]) + 3, 4);
+   return 17 + add_pid_filter(fdtv, c-operand[17]);
 }
 
 int avc_tuner_dsd(struct firedtv *fdtv,
  struct dvb_frontend_parameters *params)
 {
struct avc_command_frame *c = (void *)fdtv-avc_data;
-   int ret;
+   int pos, ret;
 
mutex_lock(fdtv-avc_mutex);
 
-   memset(c, 0, sizeof(*c));
-
c-ctype   = AVC_CTYPE_CONTROL;
c-subunit = AVC_SUBUNIT_TYPE_TUNER | fdtv-subunit;
 
switch (fdtv-type) {
case FIREDTV_DVB_S:
-   case FIREDTV_DVB_S2: avc_tuner_tuneqpsk(fdtv, params); break;
-   case FIREDTV_DVB_C: avc_tuner_dsd_dvb_c(fdtv, params); break;
-   case FIREDTV_DVB_T: avc_tuner_dsd_dvb_t(fdtv, params); break;
+   case FIREDTV_DVB_S2: pos = avc_tuner_tuneqpsk(fdtv, params); break;
+   case FIREDTV_DVB_C: pos = avc_tuner_dsd_dvb_c(fdtv, params); break;
+   case FIREDTV_DVB_T: pos = avc_tuner_dsd_dvb_t(fdtv, params); break;
default:
BUG();
}
+   pad_operands(c, pos);
+
+   fdtv-avc_data_length = ALIGN(3 + pos, 4);
ret = avc_write(fdtv);
 #if 0
/*
@@ -585,8 +598,6 @@ int avc_tuner_set_pids(struct firedtv *f
 
mutex_lock(fdtv-avc_mutex);
 
-   memset(c, 0, sizeof(*c));
-
c-ctype   = AVC_CTYPE_CONTROL;
c-subunit = AVC_SUBUNIT_TYPE_TUNER | fdtv-subunit;
c-opcode  = AVC_OPCODE_DSD;
@@ -608,6 +619,7 @@ int avc_tuner_set_pids(struct firedtv *f

Re: [PATCH] firedtv: fix regression: tuning fails due to bogus error return

2009-11-13 Thread Stefan Richter
Stefan Richter wrote on 2009-10-17:
 Since 2.6.32(-rc1), DVB core checks the return value of
 dvb_frontend_ops.set_frontend.  Now it becomes apparent that firedtv
 always returned a bogus value from its set_frontend method.
 
 Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de

Hi Mauro,

when you committed this, you added CC: sta...@kernel.org.  This is not
necessary, 2.6.32-rc1 is the first kernel version with the regression
(see changelog).

Well, OK, the patch would become necessary in -stable if somebody was to
put the new set_frontend return code check in dvb core into -stable. But
I hope nobody is going to do that; it'd be a bad idea.

Furthermore...

 ---
  drivers/media/dvb/firewire/firedtv-avc.c |7 +--
  drivers/media/dvb/firewire/firedtv-fe.c  |8 +---
  2 files changed, 6 insertions(+), 9 deletions(-)
 
 Index: linux-2.6.32-rc5/drivers/media/dvb/firewire/firedtv-avc.c
 ===
 --- linux-2.6.32-rc5.orig/drivers/media/dvb/firewire/firedtv-avc.c
 +++ linux-2.6.32-rc5/drivers/media/dvb/firewire/firedtv-avc.c
 @@ -573,8 +573,11 @@ int avc_tuner_dsd(struct firedtv *fdtv,
  
   msleep(500);
  #if 0
 - /* FIXME: */
 - /* u8 *status was an out-parameter of avc_tuner_dsd, unused by caller */
 + /*
 +  * FIXME:
 +  * u8 *status was an out-parameter of avc_tuner_dsd, unused by caller
 +  * Check for AVC_RESPONSE_ACCEPTED here instead?
 +  */
   if (status)
   *status = r-operand[2];
  #endif

...the firedtv-avc.c part of the patch vanished when you committed it. I
guess this was accident, not deliberate --- otherwise a note in the
changelog below of my Signed-off-by would have been appreciated.

I will resubmit this missing comment change or a variation of it or even
a fix of the FIXME sometime in the future.

 Index: linux-2.6.32-rc5/drivers/media/dvb/firewire/firedtv-fe.c
 ===
 --- linux-2.6.32-rc5.orig/drivers/media/dvb/firewire/firedtv-fe.c
 +++ linux-2.6.32-rc5/drivers/media/dvb/firewire/firedtv-fe.c
 @@ -141,18 +141,12 @@ static int fdtv_read_uncorrected_blocks(
   return -EOPNOTSUPP;
  }
  
 -#define ACCEPTED 0x9
 -
  static int fdtv_set_frontend(struct dvb_frontend *fe,
struct dvb_frontend_parameters *params)
  {
   struct firedtv *fdtv = fe-sec_priv;
  
 - /* FIXME: avc_tuner_dsd never returns ACCEPTED. Check status? */
 - if (avc_tuner_dsd(fdtv, params) != ACCEPTED)
 - return -EINVAL;
 - else
 - return 0; /* not sure of this... */
 + return avc_tuner_dsd(fdtv, params);
  }
  
  static int fdtv_get_frontend(struct dvb_frontend *fe,
 

-- 
Stefan Richter
-=-==--= =-== -==-=
http://arcgraph.de/sr/
--
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] firedtv: fix regression: tuning fails due to bogus error return

2009-11-13 Thread Stefan Richter
Stefan Richter wrote:
 ...the firedtv-avc.c part of the patch vanished when you committed it.

It was still there in the linuxtv-commits message of the respective
Mercurial commit, BTW.  Something wrong with the hg to git workflow?
-- 
Stefan Richter
-=-==--= =-== -==-=
http://arcgraph.de/sr/
--
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 0/4] DVB: firedtv: port to new firewire driver stack

2009-11-08 Thread Stefan Richter
The following patch series adapts the firedtv driver for
FireWire-attached DVB boxes and cards to the newer firewire-core kernel
API.  The driver will continue to work with the older ieee1394 kernel
API as well.  Which of the two IEEE 1394 stacks will be used depends on
which one was configured at build time.  If both were built, firedtv
transparently uses the stack which is bound to the FireWire controller.

This patch series depends on changes to firewire-core which were merged
into Linux 2.6.31.

There is some non-essential functionality yet to implement:
  - Finish FCP support in firewire-core so that more than one AV/C
device can be used on the same FireWire bus at the same time.
  - Enhance firedtv to use firewire-core's channel and bandwidth
allocation function for proper cooperation with non-FireDTV AV/C
devices on the same bus.

Following as reply:
[PATCH 1/4] firedtv: move remote control workqueue handling into rc source file
[PATCH 2/4] firedtv: reform lock transaction backend call
[PATCH 3/4] firedtv: add missing include, rename a constant
[PATCH 4/4] firedtv: port to new firewire core

 drivers/media/dvb/firewire/Kconfig|7
 drivers/media/dvb/firewire/Makefile   |1
 drivers/media/dvb/firewire/firedtv-1394.c |   37 +-
 drivers/media/dvb/firewire/firedtv-avc.c  |   50 +-
 drivers/media/dvb/firewire/firedtv-dvb.c  |   15
 drivers/media/dvb/firewire/firedtv-fw.c   |  385 ++
 drivers/media/dvb/firewire/firedtv-rc.c   |2
 drivers/media/dvb/firewire/firedtv.h  |   17
 8 files changed, 471 insertions(+), 43 deletions(-)
-- 
Stefan Richter
-=-==--= =-== -=---
http://arcgraph.de/sr/

--
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 1/4] firedtv: move remote control workqueue handling into rc source file

2009-11-08 Thread Stefan Richter
Preparation for the port of firedtv to the firewire-core kernel API:
Canceling of the remote control workqueue job is factored into
firedtv-rc.c.  Plus trivial whitespace change.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
 drivers/media/dvb/firewire/firedtv-1394.c |5 +++--
 drivers/media/dvb/firewire/firedtv-rc.c   |2 ++
 2 files changed, 5 insertions(+), 2 deletions(-)

Index: linux-2.6.31.4/drivers/media/dvb/firewire/firedtv-1394.c
===
--- linux-2.6.31.4.orig/drivers/media/dvb/firewire/firedtv-1394.c
+++ linux-2.6.31.4/drivers/media/dvb/firewire/firedtv-1394.c
@@ -212,6 +212,7 @@ static int node_probe(struct device *dev
goto fail;
 
avc_register_remote_control(fdtv);
+
return 0;
 fail:
spin_lock_irq(node_list_lock);
@@ -220,6 +221,7 @@ fail:
fdtv_unregister_rc(fdtv);
 fail_free:
kfree(fdtv);
+
return err;
 }
 
@@ -233,10 +235,9 @@ static int node_remove(struct device *de
list_del(fdtv-list);
spin_unlock_irq(node_list_lock);
 
-   cancel_work_sync(fdtv-remote_ctrl_work);
fdtv_unregister_rc(fdtv);
-
kfree(fdtv);
+
return 0;
 }
 
Index: linux-2.6.31.4/drivers/media/dvb/firewire/firedtv-rc.c
===
--- linux-2.6.31.4.orig/drivers/media/dvb/firewire/firedtv-rc.c
+++ linux-2.6.31.4/drivers/media/dvb/firewire/firedtv-rc.c
@@ -14,6 +14,7 @@
 #include linux/kernel.h
 #include linux/string.h
 #include linux/types.h
+#include linux/workqueue.h
 
 #include firedtv.h
 
@@ -163,6 +164,7 @@ fail:
 
 void fdtv_unregister_rc(struct firedtv *fdtv)
 {
+   cancel_work_sync(fdtv-remote_ctrl_work);
kfree(fdtv-remote_ctrl_dev-keycode);
input_unregister_device(fdtv-remote_ctrl_dev);
 }

-- 
Stefan Richter
-=-==--= =-== -=---
http://arcgraph.de/sr/

--
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 2/4] firedtv: reform lock transaction backend call

2009-11-08 Thread Stefan Richter
Preparation for the port of firedtv to the firewire-core kernel API:
The fdtv-backend-lock() hook and thus the CMP code is slightly changed
to better fit with the new API.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
 drivers/media/dvb/firewire/firedtv-1394.c |   11 -
 drivers/media/dvb/firewire/firedtv-avc.c  |   50 --
 drivers/media/dvb/firewire/firedtv.h  |2 
 3 files changed, 37 insertions(+), 26 deletions(-)

Index: linux-2.6.31.4/drivers/media/dvb/firewire/firedtv-1394.c
===
--- linux-2.6.31.4.orig/drivers/media/dvb/firewire/firedtv-1394.c
+++ linux-2.6.31.4/drivers/media/dvb/firewire/firedtv-1394.c
@@ -87,10 +87,15 @@ static inline struct node_entry *node_of
return container_of(fdtv-device, struct unit_directory, device)-ne;
 }
 
-static int node_lock(struct firedtv *fdtv, u64 addr, void *data, __be32 arg)
+static int node_lock(struct firedtv *fdtv, u64 addr, __be32 data[])
 {
-   return hpsb_node_lock(node_of(fdtv), addr, EXTCODE_COMPARE_SWAP, data,
- (__force quadlet_t)arg);
+   int ret;
+
+   ret = hpsb_node_lock(node_of(fdtv), addr, EXTCODE_COMPARE_SWAP,
+   (__force quadlet_t *)data[1], (__force quadlet_t)data[0]);
+   data[0] = data[1];
+
+   return ret;
 }
 
 static int node_read(struct firedtv *fdtv, u64 addr, void *data, size_t len)
Index: linux-2.6.31.4/drivers/media/dvb/firewire/firedtv-avc.c
===
--- linux-2.6.31.4.orig/drivers/media/dvb/firewire/firedtv-avc.c
+++ linux-2.6.31.4/drivers/media/dvb/firewire/firedtv-avc.c
@@ -1255,14 +1255,14 @@ static int cmp_read(struct firedtv *fdtv
return ret;
 }
 
-static int cmp_lock(struct firedtv *fdtv, void *data, u64 addr, __be32 arg)
+static int cmp_lock(struct firedtv *fdtv, u64 addr, __be32 data[])
 {
int ret;
 
if (mutex_lock_interruptible(fdtv-avc_mutex))
return -EINTR;
 
-   ret = fdtv-backend-lock(fdtv, addr, data, arg);
+   ret = fdtv-backend-lock(fdtv, addr, data);
if (ret  0)
dev_err(fdtv-device, CMP: lock I/O error\n);
 
@@ -1292,25 +1292,25 @@ static inline void set_opcr(__be32 *opcr
 
 int cmp_establish_pp_connection(struct firedtv *fdtv, int plug, int channel)
 {
-   __be32 old_opcr, opcr;
+   __be32 old_opcr, opcr[2];
u64 opcr_address = CMP_OUTPUT_PLUG_CONTROL_REG_0 + (plug  2);
int attempts = 0;
int ret;
 
-   ret = cmp_read(fdtv, opcr, opcr_address, 4);
+   ret = cmp_read(fdtv, opcr, opcr_address, 4);
if (ret  0)
return ret;
 
 repeat:
-   if (!get_opcr_online(opcr)) {
+   if (!get_opcr_online(*opcr)) {
dev_err(fdtv-device, CMP: output offline\n);
return -EBUSY;
}
 
-   old_opcr = opcr;
+   old_opcr = *opcr;
 
-   if (get_opcr_p2p_connections(opcr)) {
-   if (get_opcr_channel(opcr) != channel) {
+   if (get_opcr_p2p_connections(*opcr)) {
+   if (get_opcr_channel(*opcr) != channel) {
dev_err(fdtv-device, CMP: cannot change channel\n);
return -EBUSY;
}
@@ -1318,11 +1318,11 @@ repeat:
 
/* We don't allocate isochronous resources. */
} else {
-   set_opcr_channel(opcr, channel);
-   set_opcr_data_rate(opcr, 2); /* S400 */
+   set_opcr_channel(opcr, channel);
+   set_opcr_data_rate(opcr, 2); /* S400 */
 
/* FIXME: this is for the worst case - optimize */
-   set_opcr_overhead_id(opcr, 0);
+   set_opcr_overhead_id(opcr, 0);
 
/*
 * FIXME: allocate isochronous channel and bandwidth at IRM
@@ -1330,13 +1330,16 @@ repeat:
 */
}
 
-   set_opcr_p2p_connections(opcr, get_opcr_p2p_connections(opcr) + 1);
+   set_opcr_p2p_connections(opcr, get_opcr_p2p_connections(*opcr) + 1);
 
-   ret = cmp_lock(fdtv, opcr, opcr_address, old_opcr);
+   opcr[1] = *opcr;
+   opcr[0] = old_opcr;
+
+   ret = cmp_lock(fdtv, opcr_address, opcr);
if (ret  0)
return ret;
 
-   if (old_opcr != opcr) {
+   if (old_opcr != *opcr) {
/*
 * FIXME: if old_opcr.P2P_Connections  0,
 * deallocate isochronous channel and bandwidth at IRM
@@ -1354,27 +1357,30 @@ repeat:
 
 void cmp_break_pp_connection(struct firedtv *fdtv, int plug, int channel)
 {
-   __be32 old_opcr, opcr;
+   __be32 old_opcr, opcr[2];
u64 opcr_address = CMP_OUTPUT_PLUG_CONTROL_REG_0 + (plug  2);
int attempts = 0;
 
-   if (cmp_read(fdtv, opcr, opcr_address, 4)  0)
+   if (cmp_read(fdtv, opcr, opcr_address, 4)  0)
return;
 
 repeat:
-   if (!get_opcr_online(opcr

[PATCH 3/4] firedtv: add missing include, rename a constant

2009-11-08 Thread Stefan Richter
Add #include dvb_demux.h for dvb_dmx_swfilter_packets().  This was
already indirectly included via firedtv.h, but don't rely on it.

The 4 bytes which were referred to as FIREWIRE_HEADER_SIZE are actually
the source packet header from IEC 61883-4 (MPEG2-TS data transmission
over 1394), not e.g. the IEEE 1394 isochronous packet header.  So choose
a more precise name.

Also, express the payload size as a preprocessor constant too.

Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
 drivers/media/dvb/firewire/firedtv-1394.c |   15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

Index: linux-2.6.31.4/drivers/media/dvb/firewire/firedtv-1394.c
===
--- linux-2.6.31.4.orig/drivers/media/dvb/firewire/firedtv-1394.c
+++ linux-2.6.31.4/drivers/media/dvb/firewire/firedtv-1394.c
@@ -26,13 +26,16 @@
 #include iso.h
 #include nodemgr.h
 
+#include dvb_demux.h
+
 #include firedtv.h
 
 static LIST_HEAD(node_list);
 static DEFINE_SPINLOCK(node_list_lock);
 
-#define FIREWIRE_HEADER_SIZE   4
-#define CIP_HEADER_SIZE8
+#define CIP_HEADER_SIZE8
+#define MPEG2_TS_HEADER_SIZE   4
+#define MPEG2_TS_SOURCE_PACKET_SIZE(4 + 188)
 
 static void rawiso_activity_cb(struct hpsb_iso *iso)
 {
@@ -62,20 +65,20 @@ static void rawiso_activity_cb(struct hp
buf = dma_region_i(iso-data_buf, unsigned char,
iso-infos[packet].offset + CIP_HEADER_SIZE);
count = (iso-infos[packet].len - CIP_HEADER_SIZE) /
-   (188 + FIREWIRE_HEADER_SIZE);
+   MPEG2_TS_SOURCE_PACKET_SIZE;
 
/* ignore empty packet */
if (iso-infos[packet].len = CIP_HEADER_SIZE)
continue;
 
while (count--) {
-   if (buf[FIREWIRE_HEADER_SIZE] == 0x47)
+   if (buf[MPEG2_TS_HEADER_SIZE] == 0x47)
dvb_dmx_swfilter_packets(fdtv-demux,
-   buf[FIREWIRE_HEADER_SIZE], 1);
+   buf[MPEG2_TS_HEADER_SIZE], 1);
else
dev_err(fdtv-device,
skipping invalid packet\n);
-   buf += 188 + FIREWIRE_HEADER_SIZE;
+   buf += MPEG2_TS_SOURCE_PACKET_SIZE;
}
}
 out:

-- 
Stefan Richter
-=-==--= =-== -=---
http://arcgraph.de/sr/

--
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   >