Re: [PATCH v2 53/58] firewire: don't break long lines
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
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.
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)
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)
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
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
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/
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/
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/
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/
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
...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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
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.
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.
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/3] IR: add core lirc device interface
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
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]
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
$ 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]
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
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
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
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
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
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
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
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
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
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
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
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
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?
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
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
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
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
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?
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?
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?
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?
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?
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?
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
[PATCH 4/4] firedtv: port to new firewire core
The firedtv DVB driver will now work not only on top of the old ieee1394
driver stack but also on the new firewire driver stack.
Alongside to the firedtv-1394.c backend for driver binding and I/O, the
firedtv-fw.c backend is added. Depending on which of the two 1394
stacks is configured, one or the other or both backends will be built
into the firedtv driver.
This has been tested with a DVB-T and a DVB-C box on x86-64 and x86-32
together with a few different controllers (Agere FW323, a NEC chip, TI
TSB82AA2, TSB43AB22/A, VIA VT6306).
Signed-off-by: Stefan Richter stef...@s5r6.in-berlin.de
---
drivers/media/dvb/firewire/Kconfig|7 +
drivers/media/dvb/firewire/Makefile |1 +
drivers/media/dvb/firewire/firedtv-1394.c |6
drivers/media/dvb/firewire/firedtv-dvb.c | 15 +
drivers/media/dvb/firewire/firedtv-fw.c | 385 ++
drivers/media/dvb/firewire/firedtv.h | 15 +
6 files changed, 420 insertions(+), 9 deletions(-)
Index: linux-2.6.31.4/drivers/media/dvb/firewire/Kconfig
===
--- linux-2.6.31.4.orig/drivers/media/dvb/firewire/Kconfig
+++ linux-2.6.31.4/drivers/media/dvb/firewire/Kconfig
@@ -1,6 +1,6 @@
config DVB_FIREDTV
tristate FireDTV and FloppyDTV
- depends on DVB_CORE IEEE1394
+ depends on DVB_CORE (FIREWIRE || IEEE1394)
help
Support for DVB receivers from Digital Everywhere
which are connected via IEEE 1394 (FireWire).
@@ -13,8 +13,11 @@ config DVB_FIREDTV
if DVB_FIREDTV
+config DVB_FIREDTV_FIREWIRE
+ def_bool FIREWIRE = y || (FIREWIRE = m DVB_FIREDTV = m)
+
config DVB_FIREDTV_IEEE1394
- def_bool IEEE1394
+ def_bool IEEE1394 = y || (IEEE1394 = m DVB_FIREDTV = m)
config DVB_FIREDTV_INPUT
def_bool INPUT = y || (INPUT = m DVB_FIREDTV = m)
Index: linux-2.6.31.4/drivers/media/dvb/firewire/Makefile
===
--- linux-2.6.31.4.orig/drivers/media/dvb/firewire/Makefile
+++ linux-2.6.31.4/drivers/media/dvb/firewire/Makefile
@@ -1,6 +1,7 @@
obj-$(CONFIG_DVB_FIREDTV) += firedtv.o
firedtv-y := firedtv-avc.o firedtv-ci.o firedtv-dvb.o firedtv-fe.o
+firedtv-$(CONFIG_DVB_FIREDTV_FIREWIRE) += firedtv-fw.o
firedtv-$(CONFIG_DVB_FIREDTV_IEEE1394) += firedtv-1394.o
firedtv-$(CONFIG_DVB_FIREDTV_INPUT)+= firedtv-rc.o
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
@@ -1,5 +1,5 @@
/*
- * FireDTV driver (formerly known as FireSAT)
+ * FireDTV driver -- ieee1394 I/O backend
*
* Copyright (C) 2004 Andreas Monitzer a...@monitzer.com
* Copyright (C) 2007-2008 Ben Backx b...@bbackx.com
@@ -261,6 +261,7 @@ static int node_update(struct unit_direc
static struct hpsb_protocol_driver fdtv_driver = {
.name = firedtv,
+ .id_table = fdtv_id_table,
.update = node_update,
.driver = {
.probe = node_probe,
@@ -273,12 +274,11 @@ static struct hpsb_highlevel fdtv_highle
.fcp_request= fcp_request,
};
-int __init fdtv_1394_init(struct ieee1394_device_id id_table[])
+int __init fdtv_1394_init(void)
{
int ret;
hpsb_register_highlevel(fdtv_highlevel);
- fdtv_driver.id_table = id_table;
ret = hpsb_register_protocol(fdtv_driver);
if (ret) {
printk(KERN_ERR firedtv: failed to register protocol\n);
Index: linux-2.6.31.4/drivers/media/dvb/firewire/firedtv-dvb.c
===
--- linux-2.6.31.4.orig/drivers/media/dvb/firewire/firedtv-dvb.c
+++ linux-2.6.31.4/drivers/media/dvb/firewire/firedtv-dvb.c
@@ -297,7 +297,7 @@ struct firedtv *fdtv_alloc(struct device
#define AVC_UNIT_SPEC_ID_ENTRY 0x00a02d
#define AVC_SW_VERSION_ENTRY 0x010001
-static struct ieee1394_device_id fdtv_id_table[] = {
+const struct ieee1394_device_id fdtv_id_table[] = {
{
/* FloppyDTV S/CI and FloppyDTV S2 */
.match_flags= MATCH_FLAGS,
@@ -346,12 +346,23 @@ MODULE_DEVICE_TABLE(ieee1394, fdtv_id_ta
static int __init fdtv_init(void)
{
- return fdtv_1394_init(fdtv_id_table);
+ int ret;
+
+ ret = fdtv_fw_init();
+ if (ret 0)
+ return ret;
+
+ ret = fdtv_1394_init();
+ if (ret 0)
+ fdtv_fw_exit();
+
+ return ret;
}
static void __exit fdtv_exit(void)
{
fdtv_1394_exit();
+ fdtv_fw_exit();
}
module_init(fdtv_init);
Index: linux-2.6.31.4/drivers/media/dvb/firewire/firedtv-fw.c
===
--- /dev/null
+++ linux-2.6.31.4/drivers/media/dvb/firewire/firedtv-fw.c
@@ -0,0 +1,385 @@
+/*
+ * FireDTV
Re: + firedtv-fix-regression-tuning-fails-due-to-bogus-error-return.patch added to -mm tree
a...@linux-foundation.org wrote: The patch titled firedtv: fix regression: tuning fails due to bogus error return has been added to the -mm tree. Mauro, this one (v4l-dvb patch 13240) as well as firedtv: length field corrupt in ca2host if length127 (v4l-dvb patch 13237) are 2.6.32-rc material. The ca2host fix is possibly also good for 2.6.x.y, but this is something which Henrik knows better than me. -- 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
