Re: [PATCH] media: rc: cec devices do not have a lirc chardev

2018-11-14 Thread Torbjorn Jansson

On 2018-10-22 15:17, Sean Young wrote:

On Mon, Oct 22, 2018 at 01:28:42PM +0100, Sean Young wrote:

On Mon, Oct 22, 2018 at 12:30:29PM +0100, Hans Verkuil wrote:

On 10/22/2018 11:14 AM, Sean Young wrote:

Would you be able to test the following patch please?


Sean,

I think you should be able to test this with the vivid driver. Load the vivid 
driver,
run:

cec-ctl --tv; cec-ctl -d1 --playback

Then:

cec-ctl -d1 -t0 --user-control-pressed ui-cmd=F5


Ah, thanks. That will help with testing/reproducing.
  

That said, I tried this, but it doesn't crash for me, but perhaps I need to run
some RC command first...


Hmm I think those commands should be enough. It probably needs
CONFIG_DEBUG_SPINLOCK to detect the uninitialized spinlock. I'm trying it now.


Yes, that turned out to work. With CONFIG_DEBUG_SPINLOCK on, it goes bang
every time. With the patch, the problem goes away.

Without CONFIG_DEBUG_SPINLOCK we're going into undefined behaviour, so
Torbjorn you're only seeing the oops occassionally (and which is why it has
not been observed or reported before).

Thanks,

Sean



FYI i have tested this patch for a while now and have not seen any more kernel 
oops.

So it appears to be working.



Re: [PATCH] media: rc: cec devices do not have a lirc chardev

2018-10-22 Thread Sean Young
On Mon, Oct 22, 2018 at 01:28:42PM +0100, Sean Young wrote:
> On Mon, Oct 22, 2018 at 12:30:29PM +0100, Hans Verkuil wrote:
> > On 10/22/2018 11:14 AM, Sean Young wrote:
> > > Would you be able to test the following patch please?
> > 
> > Sean,
> > 
> > I think you should be able to test this with the vivid driver. Load the 
> > vivid driver,
> > run:
> > 
> > cec-ctl --tv; cec-ctl -d1 --playback
> > 
> > Then:
> > 
> > cec-ctl -d1 -t0 --user-control-pressed ui-cmd=F5
> 
> Ah, thanks. That will help with testing/reproducing.
>  
> > That said, I tried this, but it doesn't crash for me, but perhaps I need to 
> > run
> > some RC command first...
> 
> Hmm I think those commands should be enough. It probably needs
> CONFIG_DEBUG_SPINLOCK to detect the uninitialized spinlock. I'm trying it now.

Yes, that turned out to work. With CONFIG_DEBUG_SPINLOCK on, it goes bang
every time. With the patch, the problem goes away.

Without CONFIG_DEBUG_SPINLOCK we're going into undefined behaviour, so 
Torbjorn you're only seeing the oops occassionally (and which is why it has
not been observed or reported before).

Thanks,

Sean


Re: [PATCH] media: rc: cec devices do not have a lirc chardev

2018-10-22 Thread Torbjorn Jansson

On 2018-10-22 14:28, Sean Young wrote:

On Mon, Oct 22, 2018 at 12:30:29PM +0100, Hans Verkuil wrote:

On 10/22/2018 11:14 AM, Sean Young wrote:

On Mon, Oct 22, 2018 at 11:44:22AM +0200, Torbjorn Jansson wrote:

On 2018-10-22 10:59, Sean Young wrote:

On Sat, Oct 20, 2018 at 11:12:16PM +0200, Hans Verkuil wrote:

Hi Sean,

Can you take a look at this, it appears to be an RC issue, see my analysis 
below.

On 10/20/2018 03:26 PM, Torbjorn Jansson wrote:

Hello

i'm using the pulse8 usb cec adapter to control my tv.
i have a few scripts that poll the power status of my tv and after a while it 
stops working returning errors when trying to check if tv is on or off.
this i think matches a kernel oops i'm seeing that i suspect is related to this.

i have sometimes been able to recover from this problem by completely cutting 
power to my tv and also unplugging the usb cec adapter.
i have a feeling that the tv is at least partly to blame for cec-ctl not 
working but in any case there shouldn't be a kernel oops.


also every now and then i see this in dmesg:
cec cec0: transmit: failed 05
cec cec0: transmit: failed 06
but that doesn't appear to do any harm as far as i can tell.

any idea whats causing the oops?

the ops:

BUG: unable to handle kernel NULL pointer dereference at 0038
PGD 0 P4D 0
Oops:  [#1] SMP PTI
CPU: 9 PID: 27687 Comm: kworker/9:2 Tainted: P   OE 
4.18.12-200.fc28.x86_64 #1
Hardware name: Supermicro C7X99-OCE-F/C7X99-OCE-F, BIOS 2.1a 06/15/2018
Workqueue: events pulse8_irq_work_handler [pulse8_cec]
RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0 [rc_core]


Huh. ir_lirc_scancode_event() calls spin_lock_irqsave(&dev->lirc_fh_lock, 
flags);

The spinlock dev->lirc_fh_lock is initialized in ir_lirc_register(), which is 
called
from rc_register_device(), except when the protocol is CEC:

  /* Ensure that the lirc kfifo is setup before we start the thread */
  if (dev->allowed_protocols != RC_PROTO_BIT_CEC) {
  rc = ir_lirc_register(dev);
  if (rc < 0)
  goto out_rx;
  }

So it looks like ir_lirc_scancode_event() fails because dev->lirc_fh_lock was 
never
initialized.

Could this be fall-out from the lirc changes you did not too long ago?


Yes, this is broken. My bad, sorry. I think this must have been broken since
v4.16. I can write a patch but I don't have a patch but I'm on the train
to ELCE in Edinburgh now, with no hardware to test on.


Sean



the kernel oops have been happening for a while now.
yesterday when i checked my logs i can see them at least back a couple of
months when i was running 4.17

also my scripts to poll status of my tv and turn it on/off works for a while
so it doesn't crash right away.
maybe it only crashes when i send cec command to turn on/off tv and only
polling for status is no problem.


i think i have a separate issue too because i had problems even before the
kernel oopses started.
but i suspect this is caused by my tv locking up the cec bus because
unplugging power to tv for a few minutes (i must wait or it will still be
just as broken) and then back used to resolve the cec errors from my
scripts.



Would you be able to test the following patch please?


Sean,

I think you should be able to test this with the vivid driver. Load the vivid 
driver,
run:

cec-ctl --tv; cec-ctl -d1 --playback

Then:

cec-ctl -d1 -t0 --user-control-pressed ui-cmd=F5


Ah, thanks. That will help with testing/reproducing.
  

That said, I tried this, but it doesn't crash for me, but perhaps I need to run
some RC command first...


Hmm I think those commands should be enough. It probably needs
CONFIG_DEBUG_SPINLOCK to detect the uninitialized spinlock. I'm trying it now.

Thanks,

Sean



FYI the commands i run is as follows.

getting status, this is run frequently several times per minute (once every 10 
or 15 seconds):

cec-ctl --to=0 --give-device-power-status

then when i want to power on or off this is run:
cec-ctl --to=0 --image-view-on
or
cec-ctl --to=0 --standby


it usually takes day or two before i get a kernel oops.
but i haven't studied super closely when exactly the oops happens in relation 
to what commands i sent since by now I'm used to it not working most of the time.




Re: [PATCH] media: rc: cec devices do not have a lirc chardev

2018-10-22 Thread Sean Young
On Mon, Oct 22, 2018 at 12:30:29PM +0100, Hans Verkuil wrote:
> On 10/22/2018 11:14 AM, Sean Young wrote:
> > On Mon, Oct 22, 2018 at 11:44:22AM +0200, Torbjorn Jansson wrote:
> >> On 2018-10-22 10:59, Sean Young wrote:
> >>> On Sat, Oct 20, 2018 at 11:12:16PM +0200, Hans Verkuil wrote:
>  Hi Sean,
> 
>  Can you take a look at this, it appears to be an RC issue, see my 
>  analysis below.
> 
>  On 10/20/2018 03:26 PM, Torbjorn Jansson wrote:
> > Hello
> >
> > i'm using the pulse8 usb cec adapter to control my tv.
> > i have a few scripts that poll the power status of my tv and after a 
> > while it stops working returning errors when trying to check if tv is 
> > on or off.
> > this i think matches a kernel oops i'm seeing that i suspect is related 
> > to this.
> >
> > i have sometimes been able to recover from this problem by completely 
> > cutting power to my tv and also unplugging the usb cec adapter.
> > i have a feeling that the tv is at least partly to blame for cec-ctl 
> > not working but in any case there shouldn't be a kernel oops.
> >
> >
> > also every now and then i see this in dmesg:
> > cec cec0: transmit: failed 05
> > cec cec0: transmit: failed 06
> > but that doesn't appear to do any harm as far as i can tell.
> >
> > any idea whats causing the oops?
> >
> > the ops:
> >
> > BUG: unable to handle kernel NULL pointer dereference at 
> > 0038
> > PGD 0 P4D 0
> > Oops:  [#1] SMP PTI
> > CPU: 9 PID: 27687 Comm: kworker/9:2 Tainted: P   OE 
> > 4.18.12-200.fc28.x86_64 #1
> > Hardware name: Supermicro C7X99-OCE-F/C7X99-OCE-F, BIOS 2.1a 06/15/2018
> > Workqueue: events pulse8_irq_work_handler [pulse8_cec]
> > RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0 [rc_core]
> 
>  Huh. ir_lirc_scancode_event() calls 
>  spin_lock_irqsave(&dev->lirc_fh_lock, flags);
> 
>  The spinlock dev->lirc_fh_lock is initialized in ir_lirc_register(), 
>  which is called
>  from rc_register_device(), except when the protocol is CEC:
> 
>   /* Ensure that the lirc kfifo is setup before we start the 
>  thread */
>   if (dev->allowed_protocols != RC_PROTO_BIT_CEC) {
>   rc = ir_lirc_register(dev);
>   if (rc < 0)
>   goto out_rx;
>   }
> 
>  So it looks like ir_lirc_scancode_event() fails because 
>  dev->lirc_fh_lock was never
>  initialized.
> 
>  Could this be fall-out from the lirc changes you did not too long ago?
> >>>
> >>> Yes, this is broken. My bad, sorry. I think this must have been broken 
> >>> since
> >>> v4.16. I can write a patch but I don't have a patch but I'm on the train
> >>> to ELCE in Edinburgh now, with no hardware to test on.
> >>>
> >>>
> >>> Sean
> >>>
> >>
> >> the kernel oops have been happening for a while now.
> >> yesterday when i checked my logs i can see them at least back a couple of
> >> months when i was running 4.17
> >>
> >> also my scripts to poll status of my tv and turn it on/off works for a 
> >> while
> >> so it doesn't crash right away.
> >> maybe it only crashes when i send cec command to turn on/off tv and only
> >> polling for status is no problem.
> >>
> >>
> >> i think i have a separate issue too because i had problems even before the
> >> kernel oopses started.
> >> but i suspect this is caused by my tv locking up the cec bus because
> >> unplugging power to tv for a few minutes (i must wait or it will still be
> >> just as broken) and then back used to resolve the cec errors from my
> >> scripts.
> > 
> > 
> > Would you be able to test the following patch please?
> 
> Sean,
> 
> I think you should be able to test this with the vivid driver. Load the vivid 
> driver,
> run:
> 
> cec-ctl --tv; cec-ctl -d1 --playback
> 
> Then:
> 
> cec-ctl -d1 -t0 --user-control-pressed ui-cmd=F5

Ah, thanks. That will help with testing/reproducing.
 
> That said, I tried this, but it doesn't crash for me, but perhaps I need to 
> run
> some RC command first...

Hmm I think those commands should be enough. It probably needs
CONFIG_DEBUG_SPINLOCK to detect the uninitialized spinlock. I'm trying it now.

Thanks,

Sean


Re: [PATCH] media: rc: cec devices do not have a lirc chardev

2018-10-22 Thread Hans Verkuil
On 10/22/2018 11:14 AM, Sean Young wrote:
> On Mon, Oct 22, 2018 at 11:44:22AM +0200, Torbjorn Jansson wrote:
>> On 2018-10-22 10:59, Sean Young wrote:
>>> On Sat, Oct 20, 2018 at 11:12:16PM +0200, Hans Verkuil wrote:
>>>> Hi Sean,
>>>>
>>>> Can you take a look at this, it appears to be an RC issue, see my analysis 
>>>> below.
>>>>
>>>> On 10/20/2018 03:26 PM, Torbjorn Jansson wrote:
>>>>> Hello
>>>>>
>>>>> i'm using the pulse8 usb cec adapter to control my tv.
>>>>> i have a few scripts that poll the power status of my tv and after a 
>>>>> while it stops working returning errors when trying to check if tv is on 
>>>>> or off.
>>>>> this i think matches a kernel oops i'm seeing that i suspect is related 
>>>>> to this.
>>>>>
>>>>> i have sometimes been able to recover from this problem by completely 
>>>>> cutting power to my tv and also unplugging the usb cec adapter.
>>>>> i have a feeling that the tv is at least partly to blame for cec-ctl not 
>>>>> working but in any case there shouldn't be a kernel oops.
>>>>>
>>>>>
>>>>> also every now and then i see this in dmesg:
>>>>> cec cec0: transmit: failed 05
>>>>> cec cec0: transmit: failed 06
>>>>> but that doesn't appear to do any harm as far as i can tell.
>>>>>
>>>>> any idea whats causing the oops?
>>>>>
>>>>> the ops:
>>>>>
>>>>> BUG: unable to handle kernel NULL pointer dereference at 0038
>>>>> PGD 0 P4D 0
>>>>> Oops:  [#1] SMP PTI
>>>>> CPU: 9 PID: 27687 Comm: kworker/9:2 Tainted: P   OE 
>>>>> 4.18.12-200.fc28.x86_64 #1
>>>>> Hardware name: Supermicro C7X99-OCE-F/C7X99-OCE-F, BIOS 2.1a 06/15/2018
>>>>> Workqueue: events pulse8_irq_work_handler [pulse8_cec]
>>>>> RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0 [rc_core]
>>>>
>>>> Huh. ir_lirc_scancode_event() calls spin_lock_irqsave(&dev->lirc_fh_lock, 
>>>> flags);
>>>>
>>>> The spinlock dev->lirc_fh_lock is initialized in ir_lirc_register(), which 
>>>> is called
>>>> from rc_register_device(), except when the protocol is CEC:
>>>>
>>>>  /* Ensure that the lirc kfifo is setup before we start the thread 
>>>> */
>>>>  if (dev->allowed_protocols != RC_PROTO_BIT_CEC) {
>>>>  rc = ir_lirc_register(dev);
>>>>  if (rc < 0)
>>>>  goto out_rx;
>>>>  }
>>>>
>>>> So it looks like ir_lirc_scancode_event() fails because dev->lirc_fh_lock 
>>>> was never
>>>> initialized.
>>>>
>>>> Could this be fall-out from the lirc changes you did not too long ago?
>>>
>>> Yes, this is broken. My bad, sorry. I think this must have been broken since
>>> v4.16. I can write a patch but I don't have a patch but I'm on the train
>>> to ELCE in Edinburgh now, with no hardware to test on.
>>>
>>>
>>> Sean
>>>
>>
>> the kernel oops have been happening for a while now.
>> yesterday when i checked my logs i can see them at least back a couple of
>> months when i was running 4.17
>>
>> also my scripts to poll status of my tv and turn it on/off works for a while
>> so it doesn't crash right away.
>> maybe it only crashes when i send cec command to turn on/off tv and only
>> polling for status is no problem.
>>
>>
>> i think i have a separate issue too because i had problems even before the
>> kernel oopses started.
>> but i suspect this is caused by my tv locking up the cec bus because
>> unplugging power to tv for a few minutes (i must wait or it will still be
>> just as broken) and then back used to resolve the cec errors from my
>> scripts.
> 
> 
> Would you be able to test the following patch please?

Sean,

I think you should be able to test this with the vivid driver. Load the vivid 
driver,
run:

cec-ctl --tv; cec-ctl -d1 --playback

Then:

cec-ctl -d1 -t0 --user-control-pressed ui-cmd=F5

That said, I tried this, but it doesn't crash for me, but perhaps I need to run
some RC command first...

Regards,

Hans

> 
> Thanks,
> 
> Sean
> ---
> From 1b8b20b6

[PATCH] media: rc: cec devices do not have a lirc chardev

2018-10-22 Thread Sean Young
On Mon, Oct 22, 2018 at 11:44:22AM +0200, Torbjorn Jansson wrote:
> On 2018-10-22 10:59, Sean Young wrote:
> > On Sat, Oct 20, 2018 at 11:12:16PM +0200, Hans Verkuil wrote:
> > > Hi Sean,
> > > 
> > > Can you take a look at this, it appears to be an RC issue, see my 
> > > analysis below.
> > > 
> > > On 10/20/2018 03:26 PM, Torbjorn Jansson wrote:
> > > > Hello
> > > > 
> > > > i'm using the pulse8 usb cec adapter to control my tv.
> > > > i have a few scripts that poll the power status of my tv and after a 
> > > > while it stops working returning errors when trying to check if tv is 
> > > > on or off.
> > > > this i think matches a kernel oops i'm seeing that i suspect is related 
> > > > to this.
> > > > 
> > > > i have sometimes been able to recover from this problem by completely 
> > > > cutting power to my tv and also unplugging the usb cec adapter.
> > > > i have a feeling that the tv is at least partly to blame for cec-ctl 
> > > > not working but in any case there shouldn't be a kernel oops.
> > > > 
> > > > 
> > > > also every now and then i see this in dmesg:
> > > > cec cec0: transmit: failed 05
> > > > cec cec0: transmit: failed 06
> > > > but that doesn't appear to do any harm as far as i can tell.
> > > > 
> > > > any idea whats causing the oops?
> > > > 
> > > > the ops:
> > > > 
> > > > BUG: unable to handle kernel NULL pointer dereference at 
> > > > 0038
> > > > PGD 0 P4D 0
> > > > Oops:  [#1] SMP PTI
> > > > CPU: 9 PID: 27687 Comm: kworker/9:2 Tainted: P   OE 
> > > > 4.18.12-200.fc28.x86_64 #1
> > > > Hardware name: Supermicro C7X99-OCE-F/C7X99-OCE-F, BIOS 2.1a 06/15/2018
> > > > Workqueue: events pulse8_irq_work_handler [pulse8_cec]
> > > > RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0 [rc_core]
> > > 
> > > Huh. ir_lirc_scancode_event() calls spin_lock_irqsave(&dev->lirc_fh_lock, 
> > > flags);
> > > 
> > > The spinlock dev->lirc_fh_lock is initialized in ir_lirc_register(), 
> > > which is called
> > > from rc_register_device(), except when the protocol is CEC:
> > > 
> > >  /* Ensure that the lirc kfifo is setup before we start the 
> > > thread */
> > >  if (dev->allowed_protocols != RC_PROTO_BIT_CEC) {
> > >  rc = ir_lirc_register(dev);
> > >  if (rc < 0)
> > >  goto out_rx;
> > >  }
> > > 
> > > So it looks like ir_lirc_scancode_event() fails because dev->lirc_fh_lock 
> > > was never
> > > initialized.
> > > 
> > > Could this be fall-out from the lirc changes you did not too long ago?
> > 
> > Yes, this is broken. My bad, sorry. I think this must have been broken since
> > v4.16. I can write a patch but I don't have a patch but I'm on the train
> > to ELCE in Edinburgh now, with no hardware to test on.
> > 
> > 
> > Sean
> > 
> 
> the kernel oops have been happening for a while now.
> yesterday when i checked my logs i can see them at least back a couple of
> months when i was running 4.17
> 
> also my scripts to poll status of my tv and turn it on/off works for a while
> so it doesn't crash right away.
> maybe it only crashes when i send cec command to turn on/off tv and only
> polling for status is no problem.
> 
> 
> i think i have a separate issue too because i had problems even before the
> kernel oopses started.
> but i suspect this is caused by my tv locking up the cec bus because
> unplugging power to tv for a few minutes (i must wait or it will still be
> just as broken) and then back used to resolve the cec errors from my
> scripts.


Would you be able to test the following patch please?

Thanks,

Sean
---
>From 1b8b20b606b30c0e301c80e18af8d77194269bc1 Mon Sep 17 00:00:00 2001
From: Sean Young 
Date: Mon, 22 Oct 2018 10:01:50 +0100
Subject: [PATCH] media: rc: cec devices do not have a lirc chardev
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This fixes an oops in ir_lirc_scancode_event().

BUG: unable to handle kernel NULL pointer dereference at 0038
PGD 0 P4D 0
Oops:  [#1] SMP PTI
CPU: 9 PID: 27687 Comm: kworker/9:2 Tainted: P   OE 
4.18.12-200.fc28.x86_64 #1
Hardware name: Supermicro C7X99-OCE-F/C7X99-OCE