* [PATCH 1/6] docs: fix typo in lirc_device_interface.xml
2011-03-16 20:24 [PATCH 0/6] media: trivial IR fixes Jarod Wilson
@ 2011-03-16 20:24 ` Jarod Wilson
2011-03-16 20:24 ` [PATCH 2/6] imon: add more panel scancode mappings Jarod Wilson
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Jarod Wilson @ 2011-03-16 20:24 UTC (permalink / raw)
To: linux-media; +Cc: Jarod Wilson
Reported-by: Daniel Burr <dburr@topcon.com>
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
.../DocBook/v4l/lirc_device_interface.xml | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Documentation/DocBook/v4l/lirc_device_interface.xml b/Documentation/DocBook/v4l/lirc_device_interface.xml
index 68134c0..0e0453f 100644
--- a/Documentation/DocBook/v4l/lirc_device_interface.xml
+++ b/Documentation/DocBook/v4l/lirc_device_interface.xml
@@ -45,7 +45,7 @@ describing an IR signal are read from the chardev.</para>
<para>The data written to the chardev is a pulse/space sequence of integer
values. Pulses and spaces are only marked implicitly by their position. The
data must start and end with a pulse, therefore, the data must always include
-an unevent number of samples. The write function must block until the data has
+an uneven number of samples. The write function must block until the data has
been transmitted by the hardware.</para>
</section>
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/6] imon: add more panel scancode mappings
2011-03-16 20:24 [PATCH 0/6] media: trivial IR fixes Jarod Wilson
2011-03-16 20:24 ` [PATCH 1/6] docs: fix typo in lirc_device_interface.xml Jarod Wilson
@ 2011-03-16 20:24 ` Jarod Wilson
2011-03-16 20:24 ` [PATCH 3/6] ir-kbd-i2c: pass device code w/key in hauppauge case Jarod Wilson
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Jarod Wilson @ 2011-03-16 20:24 UTC (permalink / raw)
To: linux-media; +Cc: Jarod Wilson
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
drivers/media/rc/imon.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c
index e7dc6b4..f714e1a 100644
--- a/drivers/media/rc/imon.c
+++ b/drivers/media/rc/imon.c
@@ -277,12 +277,21 @@ static const struct {
u64 hw_code;
u32 keycode;
} imon_panel_key_table[] = {
- { 0x000000000f00ffeell, KEY_PROG1 }, /* Go */
+ { 0x000000000f00ffeell, KEY_MEDIA }, /* Go */
+ { 0x000000001200ffeell, KEY_UP },
+ { 0x000000001300ffeell, KEY_DOWN },
+ { 0x000000001400ffeell, KEY_LEFT },
+ { 0x000000001500ffeell, KEY_RIGHT },
+ { 0x000000001600ffeell, KEY_ENTER },
+ { 0x000000001700ffeell, KEY_ESC },
{ 0x000000001f00ffeell, KEY_AUDIO },
{ 0x000000002000ffeell, KEY_VIDEO },
{ 0x000000002100ffeell, KEY_CAMERA },
{ 0x000000002700ffeell, KEY_DVD },
{ 0x000000002300ffeell, KEY_TV },
+ { 0x000000002b00ffeell, KEY_EXIT },
+ { 0x000000002c00ffeell, KEY_SELECT },
+ { 0x000000002d00ffeell, KEY_MENU },
{ 0x000000000500ffeell, KEY_PREVIOUS },
{ 0x000000000700ffeell, KEY_REWIND },
{ 0x000000000400ffeell, KEY_STOP },
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/6] ir-kbd-i2c: pass device code w/key in hauppauge case
2011-03-16 20:24 [PATCH 0/6] media: trivial IR fixes Jarod Wilson
2011-03-16 20:24 ` [PATCH 1/6] docs: fix typo in lirc_device_interface.xml Jarod Wilson
2011-03-16 20:24 ` [PATCH 2/6] imon: add more panel scancode mappings Jarod Wilson
@ 2011-03-16 20:24 ` Jarod Wilson
2011-03-16 20:24 ` [PATCH 4/6] lirc: silence some compile warnings Jarod Wilson
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Jarod Wilson @ 2011-03-16 20:24 UTC (permalink / raw)
To: linux-media; +Cc: Jarod Wilson
The new hauppauge key tables use both device code button code.
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
drivers/media/video/ir-kbd-i2c.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/ir-kbd-i2c.c b/drivers/media/video/ir-kbd-i2c.c
index 672935f..3ab875d 100644
--- a/drivers/media/video/ir-kbd-i2c.c
+++ b/drivers/media/video/ir-kbd-i2c.c
@@ -108,7 +108,7 @@ static int get_key_haup_common(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw,
start, range, toggle, dev, code);
/* return key */
- *ir_key = code;
+ *ir_key = (dev << 8) | code;
*ir_raw = ircode;
return 1;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 4/6] lirc: silence some compile warnings
2011-03-16 20:24 [PATCH 0/6] media: trivial IR fixes Jarod Wilson
` (2 preceding siblings ...)
2011-03-16 20:24 ` [PATCH 3/6] ir-kbd-i2c: pass device code w/key in hauppauge case Jarod Wilson
@ 2011-03-16 20:24 ` Jarod Wilson
2011-03-16 20:24 ` [PATCH 5/6] lirc_zilog: error out if buffer read bytes != chunk size Jarod Wilson
2011-03-16 20:24 ` [PATCH 6/6] mceusb: topseed 0x0011 needs gen3 init for tx to work Jarod Wilson
5 siblings, 0 replies; 15+ messages in thread
From: Jarod Wilson @ 2011-03-16 20:24 UTC (permalink / raw)
To: linux-media; +Cc: Jarod Wilson
Both lirc_imon and lirc_sasem were causing gcc to complain about the
possible use of uninitialized variables.
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
drivers/staging/lirc/lirc_imon.c | 2 +-
drivers/staging/lirc/lirc_sasem.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/lirc/lirc_imon.c b/drivers/staging/lirc/lirc_imon.c
index 235cab0..4039eda 100644
--- a/drivers/staging/lirc/lirc_imon.c
+++ b/drivers/staging/lirc/lirc_imon.c
@@ -379,7 +379,7 @@ static ssize_t vfd_write(struct file *file, const char *buf,
struct imon_context *context;
const unsigned char vfd_packet6[] = {
0x01, 0x00, 0x00, 0x00, 0x00, 0xFF, 0xFF };
- int *data_buf;
+ int *data_buf = NULL;
context = file->private_data;
if (!context) {
diff --git a/drivers/staging/lirc/lirc_sasem.c b/drivers/staging/lirc/lirc_sasem.c
index 925eabe..63a438d 100644
--- a/drivers/staging/lirc/lirc_sasem.c
+++ b/drivers/staging/lirc/lirc_sasem.c
@@ -364,7 +364,7 @@ static ssize_t vfd_write(struct file *file, const char *buf,
int i;
int retval = 0;
struct sasem_context *context;
- int *data_buf;
+ int *data_buf = NULL;
context = (struct sasem_context *) file->private_data;
if (!context) {
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 5/6] lirc_zilog: error out if buffer read bytes != chunk size
2011-03-16 20:24 [PATCH 0/6] media: trivial IR fixes Jarod Wilson
` (3 preceding siblings ...)
2011-03-16 20:24 ` [PATCH 4/6] lirc: silence some compile warnings Jarod Wilson
@ 2011-03-16 20:24 ` Jarod Wilson
2011-03-17 0:07 ` Andy Walls
2011-03-16 20:24 ` [PATCH 6/6] mceusb: topseed 0x0011 needs gen3 init for tx to work Jarod Wilson
5 siblings, 1 reply; 15+ messages in thread
From: Jarod Wilson @ 2011-03-16 20:24 UTC (permalink / raw)
To: linux-media; +Cc: Jarod Wilson
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
drivers/staging/lirc/lirc_zilog.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c
index 407d4b4..5ada643 100644
--- a/drivers/staging/lirc/lirc_zilog.c
+++ b/drivers/staging/lirc/lirc_zilog.c
@@ -950,6 +950,10 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos)
ret = copy_to_user((void *)outbuf+written, buf,
rbuf->chunk_size);
written += rbuf->chunk_size;
+ } else {
+ zilog_error("Buffer read failed!\n");
+ ret = -EIO;
+ break;
}
}
}
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 5/6] lirc_zilog: error out if buffer read bytes != chunk size
2011-03-16 20:24 ` [PATCH 5/6] lirc_zilog: error out if buffer read bytes != chunk size Jarod Wilson
@ 2011-03-17 0:07 ` Andy Walls
2011-03-17 13:19 ` Jarod Wilson
0 siblings, 1 reply; 15+ messages in thread
From: Andy Walls @ 2011-03-17 0:07 UTC (permalink / raw)
To: Jarod Wilson; +Cc: linux-media
On Wed, 2011-03-16 at 16:24 -0400, Jarod Wilson wrote:
> Signed-off-by: Jarod Wilson <jarod@redhat.com>
> ---
> drivers/staging/lirc/lirc_zilog.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c
> index 407d4b4..5ada643 100644
> --- a/drivers/staging/lirc/lirc_zilog.c
> +++ b/drivers/staging/lirc/lirc_zilog.c
> @@ -950,6 +950,10 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos)
> ret = copy_to_user((void *)outbuf+written, buf,
> rbuf->chunk_size);
> written += rbuf->chunk_size;
> + } else {
> + zilog_error("Buffer read failed!\n");
> + ret = -EIO;
> + break;
No need to break, just let the non-0 ret value drop you out of the while
loop.
Regards,
Andy
> }
> }
> }
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 5/6] lirc_zilog: error out if buffer read bytes != chunk size
2011-03-17 0:07 ` Andy Walls
@ 2011-03-17 13:19 ` Jarod Wilson
2011-03-17 15:29 ` Andy Walls
0 siblings, 1 reply; 15+ messages in thread
From: Jarod Wilson @ 2011-03-17 13:19 UTC (permalink / raw)
To: Andy Walls; +Cc: linux-media
On Wed, Mar 16, 2011 at 08:07:22PM -0400, Andy Walls wrote:
> On Wed, 2011-03-16 at 16:24 -0400, Jarod Wilson wrote:
> > Signed-off-by: Jarod Wilson <jarod@redhat.com>
> > ---
> > drivers/staging/lirc/lirc_zilog.c | 4 ++++
> > 1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c
> > index 407d4b4..5ada643 100644
> > --- a/drivers/staging/lirc/lirc_zilog.c
> > +++ b/drivers/staging/lirc/lirc_zilog.c
> > @@ -950,6 +950,10 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos)
> > ret = copy_to_user((void *)outbuf+written, buf,
> > rbuf->chunk_size);
> > written += rbuf->chunk_size;
> > + } else {
> > + zilog_error("Buffer read failed!\n");
> > + ret = -EIO;
> > + break;
>
> No need to break, just let the non-0 ret value drop you out of the while
> loop.
Ah, indeed. I think I mindlessly copied what the tests just a few lines
above were doing without looking at the actual reason for them. I'll
remove that break from the patch here locally.
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 5/6] lirc_zilog: error out if buffer read bytes != chunk size
2011-03-17 13:19 ` Jarod Wilson
@ 2011-03-17 15:29 ` Andy Walls
2011-03-17 15:42 ` Jarod Wilson
0 siblings, 1 reply; 15+ messages in thread
From: Andy Walls @ 2011-03-17 15:29 UTC (permalink / raw)
To: Jarod Wilson; +Cc: linux-media
Jarod Wilson <jarod@redhat.com> wrote:
>On Wed, Mar 16, 2011 at 08:07:22PM -0400, Andy Walls wrote:
>> On Wed, 2011-03-16 at 16:24 -0400, Jarod Wilson wrote:
>> > Signed-off-by: Jarod Wilson <jarod@redhat.com>
>> > ---
>> > drivers/staging/lirc/lirc_zilog.c | 4 ++++
>> > 1 files changed, 4 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/drivers/staging/lirc/lirc_zilog.c
>b/drivers/staging/lirc/lirc_zilog.c
>> > index 407d4b4..5ada643 100644
>> > --- a/drivers/staging/lirc/lirc_zilog.c
>> > +++ b/drivers/staging/lirc/lirc_zilog.c
>> > @@ -950,6 +950,10 @@ static ssize_t read(struct file *filep, char
>*outbuf, size_t n, loff_t *ppos)
>> > ret = copy_to_user((void *)outbuf+written, buf,
>> > rbuf->chunk_size);
>> > written += rbuf->chunk_size;
>> > + } else {
>> > + zilog_error("Buffer read failed!\n");
>> > + ret = -EIO;
>> > + break;
>>
>> No need to break, just let the non-0 ret value drop you out of the
>while
>> loop.
>
>Ah, indeed. I think I mindlessly copied what the tests just a few lines
>above were doing without looking at the actual reason for them. I'll
>remove that break from the patch here locally.
>
>--
>Jarod Wilson
>jarod@redhat.com
You might also want to take a look at that test to ensure it doesn't break blocking read() behavior. (man 2 read). I'm swamped ATM and didn't look too hard.
It seems odd that the lirc buffer object can have data ready (the first branch of the big if() in the while() loop), and yet the read of that lirc buffer object fails.
-Andy
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 5/6] lirc_zilog: error out if buffer read bytes != chunk size
2011-03-17 15:29 ` Andy Walls
@ 2011-03-17 15:42 ` Jarod Wilson
2011-03-17 16:16 ` Andy Walls
0 siblings, 1 reply; 15+ messages in thread
From: Jarod Wilson @ 2011-03-17 15:42 UTC (permalink / raw)
To: Andy Walls; +Cc: linux-media
On Thu, Mar 17, 2011 at 11:29:08AM -0400, Andy Walls wrote:
> Jarod Wilson <jarod@redhat.com> wrote:
>
> >On Wed, Mar 16, 2011 at 08:07:22PM -0400, Andy Walls wrote:
> >> On Wed, 2011-03-16 at 16:24 -0400, Jarod Wilson wrote:
> >> > Signed-off-by: Jarod Wilson <jarod@redhat.com>
> >> > ---
> >> > drivers/staging/lirc/lirc_zilog.c | 4 ++++
> >> > 1 files changed, 4 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/drivers/staging/lirc/lirc_zilog.c
> >b/drivers/staging/lirc/lirc_zilog.c
> >> > index 407d4b4..5ada643 100644
> >> > --- a/drivers/staging/lirc/lirc_zilog.c
> >> > +++ b/drivers/staging/lirc/lirc_zilog.c
> >> > @@ -950,6 +950,10 @@ static ssize_t read(struct file *filep, char
> >*outbuf, size_t n, loff_t *ppos)
> >> > ret = copy_to_user((void *)outbuf+written, buf,
> >> > rbuf->chunk_size);
> >> > written += rbuf->chunk_size;
> >> > + } else {
> >> > + zilog_error("Buffer read failed!\n");
> >> > + ret = -EIO;
> >> > + break;
> >>
> >> No need to break, just let the non-0 ret value drop you out of the
> >while
> >> loop.
> >
> >Ah, indeed. I think I mindlessly copied what the tests just a few lines
> >above were doing without looking at the actual reason for them. I'll
> >remove that break from the patch here locally.
> >
> >--
> >Jarod Wilson
> >jarod@redhat.com
>
> You might also want to take a look at that test to ensure it doesn't break blocking read() behavior. (man 2 read). I'm swamped ATM and didn't look too hard.
>
> It seems odd that the lirc buffer object can have data ready (the first branch of the big if() in the while() loop), and yet the read of that lirc buffer object fails.
Generally, it shouldn't, but lirc_buffer_read uses kfifo underneath, and
in the pre-2.6.33 kfifo implementation, the retval from lirc_buffer_read
(as backported by way of media_build) is always 0, which is of course not
equal to chunk_size. So I think that in current kernels, this should never
trigger, and its partially just a note-to-self that this check will go
sideways when running on an older kernel, but not a bad check to have if
something really does go wrong.
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 5/6] lirc_zilog: error out if buffer read bytes != chunk size
2011-03-17 15:42 ` Jarod Wilson
@ 2011-03-17 16:16 ` Andy Walls
2011-03-17 19:08 ` Jarod Wilson
0 siblings, 1 reply; 15+ messages in thread
From: Andy Walls @ 2011-03-17 16:16 UTC (permalink / raw)
To: Jarod Wilson; +Cc: linux-media
Jarod Wilson <jarod@redhat.com> wrote:
>On Thu, Mar 17, 2011 at 11:29:08AM -0400, Andy Walls wrote:
>> Jarod Wilson <jarod@redhat.com> wrote:
>>
>> >On Wed, Mar 16, 2011 at 08:07:22PM -0400, Andy Walls wrote:
>> >> On Wed, 2011-03-16 at 16:24 -0400, Jarod Wilson wrote:
>> >> > Signed-off-by: Jarod Wilson <jarod@redhat.com>
>> >> > ---
>> >> > drivers/staging/lirc/lirc_zilog.c | 4 ++++
>> >> > 1 files changed, 4 insertions(+), 0 deletions(-)
>> >> >
>> >> > diff --git a/drivers/staging/lirc/lirc_zilog.c
>> >b/drivers/staging/lirc/lirc_zilog.c
>> >> > index 407d4b4..5ada643 100644
>> >> > --- a/drivers/staging/lirc/lirc_zilog.c
>> >> > +++ b/drivers/staging/lirc/lirc_zilog.c
>> >> > @@ -950,6 +950,10 @@ static ssize_t read(struct file *filep,
>char
>> >*outbuf, size_t n, loff_t *ppos)
>> >> > ret = copy_to_user((void *)outbuf+written, buf,
>> >> > rbuf->chunk_size);
>> >> > written += rbuf->chunk_size;
>> >> > + } else {
>> >> > + zilog_error("Buffer read failed!\n");
>> >> > + ret = -EIO;
>> >> > + break;
>> >>
>> >> No need to break, just let the non-0 ret value drop you out of the
>> >while
>> >> loop.
>> >
>> >Ah, indeed. I think I mindlessly copied what the tests just a few
>lines
>> >above were doing without looking at the actual reason for them. I'll
>> >remove that break from the patch here locally.
>> >
>> >--
>> >Jarod Wilson
>> >jarod@redhat.com
>>
>> You might also want to take a look at that test to ensure it doesn't
>break blocking read() behavior. (man 2 read). I'm swamped ATM and
>didn't look too hard.
>>
>> It seems odd that the lirc buffer object can have data ready (the
>first branch of the big if() in the while() loop), and yet the read of
>that lirc buffer object fails.
>
>Generally, it shouldn't, but lirc_buffer_read uses kfifo underneath,
>and
>in the pre-2.6.33 kfifo implementation, the retval from
>lirc_buffer_read
>(as backported by way of media_build) is always 0, which is of course
>not
>equal to chunk_size. So I think that in current kernels, this should
>never
>trigger, and its partially just a note-to-self that this check will go
>sideways when running on an older kernel, but not a bad check to have
>if
>something really does go wrong.
>
>--
>Jarod Wilson
>jarod@redhat.com
But the orignal intent of the check I put in was to avoid passing partial/junk data to userspace, and go around again to see if good data could be provided.
Your check bails when good data that might be sitting there still. That doesn't seem like a good trade for supporting backward compat for old kernels.
-Andy
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 5/6] lirc_zilog: error out if buffer read bytes != chunk size
2011-03-17 16:16 ` Andy Walls
@ 2011-03-17 19:08 ` Jarod Wilson
2011-03-18 0:50 ` Andy Walls
0 siblings, 1 reply; 15+ messages in thread
From: Jarod Wilson @ 2011-03-17 19:08 UTC (permalink / raw)
To: Andy Walls; +Cc: linux-media
On Thu, Mar 17, 2011 at 12:16:31PM -0400, Andy Walls wrote:
> Jarod Wilson <jarod@redhat.com> wrote:
>
> >On Thu, Mar 17, 2011 at 11:29:08AM -0400, Andy Walls wrote:
> >> Jarod Wilson <jarod@redhat.com> wrote:
> >>
> >> >On Wed, Mar 16, 2011 at 08:07:22PM -0400, Andy Walls wrote:
> >> >> On Wed, 2011-03-16 at 16:24 -0400, Jarod Wilson wrote:
> >> >> > Signed-off-by: Jarod Wilson <jarod@redhat.com>
> >> >> > ---
> >> >> > drivers/staging/lirc/lirc_zilog.c | 4 ++++
> >> >> > 1 files changed, 4 insertions(+), 0 deletions(-)
> >> >> >
> >> >> > diff --git a/drivers/staging/lirc/lirc_zilog.c
> >> >b/drivers/staging/lirc/lirc_zilog.c
> >> >> > index 407d4b4..5ada643 100644
> >> >> > --- a/drivers/staging/lirc/lirc_zilog.c
> >> >> > +++ b/drivers/staging/lirc/lirc_zilog.c
> >> >> > @@ -950,6 +950,10 @@ static ssize_t read(struct file *filep,
> >char
> >> >*outbuf, size_t n, loff_t *ppos)
> >> >> > ret = copy_to_user((void *)outbuf+written, buf,
> >> >> > rbuf->chunk_size);
> >> >> > written += rbuf->chunk_size;
> >> >> > + } else {
> >> >> > + zilog_error("Buffer read failed!\n");
> >> >> > + ret = -EIO;
> >> >> > + break;
> >> >>
> >> >> No need to break, just let the non-0 ret value drop you out of the
> >> >while
> >> >> loop.
> >> >
> >> >Ah, indeed. I think I mindlessly copied what the tests just a few
> >lines
> >> >above were doing without looking at the actual reason for them. I'll
> >> >remove that break from the patch here locally.
> >> >
> >> >--
> >> >Jarod Wilson
> >> >jarod@redhat.com
> >>
> >> You might also want to take a look at that test to ensure it doesn't
> >break blocking read() behavior. (man 2 read). I'm swamped ATM and
> >didn't look too hard.
> >>
> >> It seems odd that the lirc buffer object can have data ready (the
> >first branch of the big if() in the while() loop), and yet the read of
> >that lirc buffer object fails.
> >
> >Generally, it shouldn't, but lirc_buffer_read uses kfifo underneath,
> >and
> >in the pre-2.6.33 kfifo implementation, the retval from
> >lirc_buffer_read
> >(as backported by way of media_build) is always 0, which is of course
> >not
> >equal to chunk_size. So I think that in current kernels, this should
> >never
> >trigger, and its partially just a note-to-self that this check will go
> >sideways when running on an older kernel, but not a bad check to have
> >if
> >something really does go wrong.
>
> But the orignal intent of the check I put in was to avoid passing partial/junk data to userspace, and go around again to see if good data could be provided.
>
> Your check bails when good data that might be sitting there still. That doesn't seem like a good trade for supporting backward compat for old kernels.
Ah. Another thing I neglected to notice then. :)
Perhaps there should be a retry count check as well then, as otherwise,
its possible to get stuck in that loop forever (which is what was
happening on older kernels). Its conceivable that similar could happen on
a newer kernel for some reason.
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 5/6] lirc_zilog: error out if buffer read bytes != chunk size
2011-03-17 19:08 ` Jarod Wilson
@ 2011-03-18 0:50 ` Andy Walls
2011-03-22 20:39 ` Jarod Wilson
0 siblings, 1 reply; 15+ messages in thread
From: Andy Walls @ 2011-03-18 0:50 UTC (permalink / raw)
To: Jarod Wilson; +Cc: linux-media
On Thu, 2011-03-17 at 15:08 -0400, Jarod Wilson wrote:
> On Thu, Mar 17, 2011 at 12:16:31PM -0400, Andy Walls wrote:
> > Jarod Wilson <jarod@redhat.com> wrote:
> .
> >
> > But the orignal intent of the check I put in was to avoid passing
> partial/junk data to userspace, and go around again to see if good
> data could be provided.
> >
> > Your check bails when good data that might be sitting there still.
> That doesn't seem like a good trade for supporting backward compat for
> old kernels.
>
> Ah. Another thing I neglected to notice then. :)
>
> Perhaps there should be a retry count check as well then, as otherwise,
> its possible to get stuck in that loop forever (which is what was
> happening on older kernels). Its conceivable that similar could happen on
> a newer kernel for some reason.
Well, lets see,
>From the perspective of userspace & lircd:
1. A specification compliance failure for a corner case isn't too bad
(bailing out on junk and leaving good data behind)
2. An unrecoverable failure for any case is very bad (spinning/hanging
on a result that won't change)
3. Sending unitialized bytes out to userspace with copy_to_user() is
very bad.
(I recall the old code would do the copy to user and always tell
userspace it got a code whether it read anything out of the buffer or
not. IIRC, that leaked information off the stack.)
If the code as patched avoids the two very bad things (#2 and #3), then
the patch is OK by me.
Regards,
Andy
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/6] lirc_zilog: error out if buffer read bytes != chunk size
2011-03-18 0:50 ` Andy Walls
@ 2011-03-22 20:39 ` Jarod Wilson
0 siblings, 0 replies; 15+ messages in thread
From: Jarod Wilson @ 2011-03-22 20:39 UTC (permalink / raw)
To: Andy Walls; +Cc: linux-media
On Thu, Mar 17, 2011 at 08:50:33PM -0400, Andy Walls wrote:
> On Thu, 2011-03-17 at 15:08 -0400, Jarod Wilson wrote:
> > On Thu, Mar 17, 2011 at 12:16:31PM -0400, Andy Walls wrote:
> > > Jarod Wilson <jarod@redhat.com> wrote:
> > .
> > >
> > > But the orignal intent of the check I put in was to avoid passing
> > partial/junk data to userspace, and go around again to see if good
> > data could be provided.
> > >
> > > Your check bails when good data that might be sitting there still.
> > That doesn't seem like a good trade for supporting backward compat for
> > old kernels.
> >
> > Ah. Another thing I neglected to notice then. :)
> >
> > Perhaps there should be a retry count check as well then, as otherwise,
> > its possible to get stuck in that loop forever (which is what was
> > happening on older kernels). Its conceivable that similar could happen on
> > a newer kernel for some reason.
>
> Well, lets see,
>
> >From the perspective of userspace & lircd:
>
> 1. A specification compliance failure for a corner case isn't too bad
> (bailing out on junk and leaving good data behind)
>
> 2. An unrecoverable failure for any case is very bad (spinning/hanging
> on a result that won't change)
>
> 3. Sending unitialized bytes out to userspace with copy_to_user() is
> very bad.
> (I recall the old code would do the copy to user and always tell
> userspace it got a code whether it read anything out of the buffer or
> not. IIRC, that leaked information off the stack.)
>
>
> If the code as patched avoids the two very bad things (#2 and #3), then
> the patch is OK by me.
I *think* what I've got now should address both 2 and 3, with a very
minimal risk of leaving data behind, since it'll retry a couple of times
before bailing out of the loop, so it should be pretty unlikely we'd leave
any good data behind. But even if we do, like you said, this is just an IR
signal, the user can press the button on the remote again. :)
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 6/6] mceusb: topseed 0x0011 needs gen3 init for tx to work
2011-03-16 20:24 [PATCH 0/6] media: trivial IR fixes Jarod Wilson
` (4 preceding siblings ...)
2011-03-16 20:24 ` [PATCH 5/6] lirc_zilog: error out if buffer read bytes != chunk size Jarod Wilson
@ 2011-03-16 20:24 ` Jarod Wilson
5 siblings, 0 replies; 15+ messages in thread
From: Jarod Wilson @ 2011-03-16 20:24 UTC (permalink / raw)
To: linux-media; +Cc: Jarod Wilson
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
drivers/media/rc/mceusb.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index eedefce..044fb7a 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -261,7 +261,7 @@ static struct usb_device_id mceusb_dev_table[] = {
.driver_info = MCE_GEN2_TX_INV },
/* Topseed eHome Infrared Transceiver */
{ USB_DEVICE(VENDOR_TOPSEED, 0x0011),
- .driver_info = MCE_GEN2_TX_INV },
+ .driver_info = MCE_GEN3 },
/* Ricavision internal Infrared Transceiver */
{ USB_DEVICE(VENDOR_RICAVISION, 0x0010) },
/* Itron ione Libra Q-11 */
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread