[PATCH] [media] rc: ite-cir: Initialise ite_dev::rdev earlier

[PATCH] [media] rc: ite-cir: Initialise ite_dev::rdev earlier

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1202 bytes --]

ite_dev::rdev is currently initialised in ite_probe() after
rc_register_device() returns.  If a newly registered device is opened
quickly enough, we may enable interrupts and try to use ite_dev::rdev
before it has been initialised.  Move it up to the earliest point we
can, right after calling rc_allocate_device().

References: http://bugs.debian.org/684441
Reported-and-tested-by: YunQiang Su <wzssyqa@gmail.com>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/ite-cir.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
index 0e49c99..c06992e 100644
--- a/drivers/media/rc/ite-cir.c
+++ b/drivers/media/rc/ite-cir.c
@@ -1473,6 +1473,7 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 	rdev = rc_allocate_device();
 	if (!rdev)
 		goto failure;
+	itdev->rdev = rdev;
 
 	ret = -ENODEV;
 
@@ -1604,7 +1605,6 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 	if (ret)
 		goto failure;
 
-	itdev->rdev = rdev;
 	ite_pr(KERN_NOTICE, "driver has been successfully loaded\n");
 
 	return 0;


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH v2] [media] rc: ite-cir: Initialise ite_dev::rdev earlier

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1306 bytes --]

ite_dev::rdev is currently initialised in ite_probe() after
rc_register_device() returns.  If a newly registered device is opened
quickly enough, we may enable interrupts and try to use ite_dev::rdev
before it has been initialised.  Move it up to the earliest point we
can, right after calling rc_allocate_device().

References: http://bugs.debian.org/684441
Reported-and-tested-by: YunQiang Su <wzssyqa@gmail.com>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Cc: stable@vger.kernel.org
---
Unlike the previous version, this will apply cleanly to the media
staging/for_v3.6 branch.

Ben.

 drivers/media/rc/ite-cir.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
index 36fe5a3..24c77a4 100644
--- a/drivers/media/rc/ite-cir.c
+++ b/drivers/media/rc/ite-cir.c
@@ -1473,6 +1473,7 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 	rdev = rc_allocate_device();
 	if (!rdev)
 		goto failure;
+	itdev->rdev = rdev;
 
 	ret = -ENODEV;
 
@@ -1604,7 +1605,6 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 	if (ret)
 		goto failure3;
 
-	itdev->rdev = rdev;
 	ite_pr(KERN_NOTICE, "driver has been successfully loaded\n");
 
 	return 0;


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH v2] [media] rc: ite-cir: Initialise ite_dev::rdev earlier

[YunQiang Su]

[-- Attachment #1: Type: text/plain, Size: 1741 bytes --]

Is it commit the the current 3.5 package in experimental?
I upgrade kernel from the version I build with this patch to the one
new coming to experimental.
And it panic again every morning, while the screens are not the same
between days.

On Mon, Aug 20, 2012 at 7:32 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
> ite_dev::rdev is currently initialised in ite_probe() after
> rc_register_device() returns.  If a newly registered device is opened
> quickly enough, we may enable interrupts and try to use ite_dev::rdev
> before it has been initialised.  Move it up to the earliest point we
> can, right after calling rc_allocate_device().
>
> References: http://bugs.debian.org/684441
> Reported-and-tested-by: YunQiang Su <wzssyqa@gmail.com>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> Cc: stable@vger.kernel.org
> ---
> Unlike the previous version, this will apply cleanly to the media
> staging/for_v3.6 branch.
>
> Ben.
>
>  drivers/media/rc/ite-cir.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
> index 36fe5a3..24c77a4 100644
> --- a/drivers/media/rc/ite-cir.c
> +++ b/drivers/media/rc/ite-cir.c
> @@ -1473,6 +1473,7 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
>         rdev = rc_allocate_device();
>         if (!rdev)
>                 goto failure;
> +       itdev->rdev = rdev;
>
>         ret = -ENODEV;
>
> @@ -1604,7 +1605,6 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
>         if (ret)
>                 goto failure3;
>
> -       itdev->rdev = rdev;
>         ite_pr(KERN_NOTICE, "driver has been successfully loaded\n");
>
>         return 0;
>



-- 
YunQiang Su

[-- Attachment #2: 3.jpg --]
[-- Type: image/jpeg, Size: 110393 bytes --]

[-- Attachment #3: 4.jpg --]
[-- Type: image/jpeg, Size: 102674 bytes --]

Re: [PATCH v2] [media] rc: ite-cir: Initialise ite_dev::rdev earlier

[Luis Henriques]

On Mon, Aug 20, 2012 at 12:32:27AM +0100, Ben Hutchings wrote:
> ite_dev::rdev is currently initialised in ite_probe() after
> rc_register_device() returns.  If a newly registered device is opened
> quickly enough, we may enable interrupts and try to use ite_dev::rdev
> before it has been initialised.  Move it up to the earliest point we
> can, right after calling rc_allocate_device().

I believe this is the same bug:

https://bugzilla.kernel.org/show_bug.cgi?id=46391

And the bug is present in other IR devices as well.

I've sent a proposed fix:

http://marc.info/?l=linux-kernel&m=134590803109050&w=2

Cheers,
--
Luis

> 
> References: http://bugs.debian.org/684441 Reported-and-tested-by:
> YunQiang Su <wzssyqa@gmail.com> Signed-off-by: Ben Hutchings
> <ben@decadent.org.uk> Cc: stable@vger.kernel.org --- Unlike the
> previous version, this will apply cleanly to the media
> staging/for_v3.6 branch.
> 
> Ben.
> 
>  drivers/media/rc/ite-cir.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
> index 36fe5a3..24c77a4 100644
> --- a/drivers/media/rc/ite-cir.c
> +++ b/drivers/media/rc/ite-cir.c
> @@ -1473,6 +1473,7 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
>  	rdev = rc_allocate_device();
>  	if (!rdev)
>  		goto failure;
> +	itdev->rdev = rdev;
>  
>  	ret = -ENODEV;
>  
> @@ -1604,7 +1605,6 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
>  	if (ret)
>  		goto failure3;
>  
> -	itdev->rdev = rdev;
>  	ite_pr(KERN_NOTICE, "driver has been successfully loaded\n");
>  
>  	return 0;
> 

Re: [PATCH v2] [media] rc: ite-cir: Initialise ite_dev::rdev earlier

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2128 bytes --]

On Tue, 2012-08-28 at 12:44 +0100, Luis Henriques wrote:
> On Mon, Aug 20, 2012 at 12:32:27AM +0100, Ben Hutchings wrote:
> > ite_dev::rdev is currently initialised in ite_probe() after
> > rc_register_device() returns.  If a newly registered device is opened
> > quickly enough, we may enable interrupts and try to use ite_dev::rdev
> > before it has been initialised.  Move it up to the earliest point we
> > can, right after calling rc_allocate_device().
> 
> I believe this is the same bug:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=46391
> 
> And the bug is present in other IR devices as well.
> 
> I've sent a proposed fix:
> 
> http://marc.info/?l=linux-kernel&m=134590803109050&w=2

It might be a worthwhile fix.  But it doesn't fix this bug - after that
patch, the driver will still enable its IRQ before initialising
ite_dev::rdev.

Ben.

> Cheers,
> --
> Luis
> 
> > 
> > References: http://bugs.debian.org/684441 Reported-and-tested-by:
> > YunQiang Su <wzssyqa@gmail.com> Signed-off-by: Ben Hutchings
> > <ben@decadent.org.uk> Cc: stable@vger.kernel.org --- Unlike the
> > previous version, this will apply cleanly to the media
> > staging/for_v3.6 branch.
> > 
> > Ben.
> > 
> >  drivers/media/rc/ite-cir.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
> > index 36fe5a3..24c77a4 100644
> > --- a/drivers/media/rc/ite-cir.c
> > +++ b/drivers/media/rc/ite-cir.c
> > @@ -1473,6 +1473,7 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
> >  	rdev = rc_allocate_device();
> >  	if (!rdev)
> >  		goto failure;
> > +	itdev->rdev = rdev;
> >  
> >  	ret = -ENODEV;
> >  
> > @@ -1604,7 +1605,6 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
> >  	if (ret)
> >  		goto failure3;
> >  
> > -	itdev->rdev = rdev;
> >  	ite_pr(KERN_NOTICE, "driver has been successfully loaded\n");
> >  
> >  	return 0;
> > 
> 

-- 
Ben Hutchings
It is a miracle that curiosity survives formal education. - Albert Einstein

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH v2] [media] rc: ite-cir: Initialise ite_dev::rdev earlier

[Luis Henriques]

On Tue, Aug 28, 2012 at 10:09:55AM -0700, Ben Hutchings wrote:
> On Tue, 2012-08-28 at 12:44 +0100, Luis Henriques wrote:
> > On Mon, Aug 20, 2012 at 12:32:27AM +0100, Ben Hutchings wrote:
> > > ite_dev::rdev is currently initialised in ite_probe() after
> > > rc_register_device() returns.  If a newly registered device is opened
> > > quickly enough, we may enable interrupts and try to use ite_dev::rdev
> > > before it has been initialised.  Move it up to the earliest point we
> > > can, right after calling rc_allocate_device().
> > 
> > I believe this is the same bug:
> > 
> > https://bugzilla.kernel.org/show_bug.cgi?id=46391
> > 
> > And the bug is present in other IR devices as well.
> > 
> > I've sent a proposed fix:
> > 
> > http://marc.info/?l=linux-kernel&m=134590803109050&w=2
> 
> It might be a worthwhile fix.  But it doesn't fix this bug - after that
> patch, the driver will still enable its IRQ before initialising
> ite_dev::rdev.

You're absolutely right, sorry for the noise.  I should have taken a
closer look at your patch.

Cheers,
--
Luis

> 
> Ben.
> 
> > Cheers,
> > --
> > Luis
> > 
> > > 
> > > References: http://bugs.debian.org/684441 Reported-and-tested-by:
> > > YunQiang Su <wzssyqa@gmail.com> Signed-off-by: Ben Hutchings
> > > <ben@decadent.org.uk> Cc: stable@vger.kernel.org --- Unlike the
> > > previous version, this will apply cleanly to the media
> > > staging/for_v3.6 branch.
> > > 
> > > Ben.
> > > 
> > >  drivers/media/rc/ite-cir.c |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
> > > index 36fe5a3..24c77a4 100644
> > > --- a/drivers/media/rc/ite-cir.c
> > > +++ b/drivers/media/rc/ite-cir.c
> > > @@ -1473,6 +1473,7 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
> > >  	rdev = rc_allocate_device();
> > >  	if (!rdev)
> > >  		goto failure;
> > > +	itdev->rdev = rdev;
> > >  
> > >  	ret = -ENODEV;
> > >  
> > > @@ -1604,7 +1605,6 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
> > >  	if (ret)
> > >  		goto failure3;
> > >  
> > > -	itdev->rdev = rdev;
> > >  	ite_pr(KERN_NOTICE, "driver has been successfully loaded\n");
> > >  
> > >  	return 0;
> > > 
> > 
> 
> -- 
> Ben Hutchings
> It is a miracle that curiosity survives formal education. - Albert Einstein

Re: [PATCH v2] [media] rc: ite-cir: Initialise ite_dev::rdev earlier

[YunQiang Su]

I upgrade my system yesterday, and this morning, it panic always, even
I boot another system
first and reboot.

Both the kernel in testing and experimental have this problem.

On Wed, Aug 29, 2012 at 4:48 AM, Luis Henriques
<luis.henriques@canonical.com> wrote:
> On Tue, Aug 28, 2012 at 10:09:55AM -0700, Ben Hutchings wrote:
>> On Tue, 2012-08-28 at 12:44 +0100, Luis Henriques wrote:
>> > On Mon, Aug 20, 2012 at 12:32:27AM +0100, Ben Hutchings wrote:
>> > > ite_dev::rdev is currently initialised in ite_probe() after
>> > > rc_register_device() returns.  If a newly registered device is opened
>> > > quickly enough, we may enable interrupts and try to use ite_dev::rdev
>> > > before it has been initialised.  Move it up to the earliest point we
>> > > can, right after calling rc_allocate_device().
>> >
>> > I believe this is the same bug:
>> >
>> > https://bugzilla.kernel.org/show_bug.cgi?id=46391
>> >
>> > And the bug is present in other IR devices as well.
>> >
>> > I've sent a proposed fix:
>> >
>> > http://marc.info/?l=linux-kernel&m=134590803109050&w=2
>>
>> It might be a worthwhile fix.  But it doesn't fix this bug - after that
>> patch, the driver will still enable its IRQ before initialising
>> ite_dev::rdev.
>
> You're absolutely right, sorry for the noise.  I should have taken a
> closer look at your patch.
>
> Cheers,
> --
> Luis
>
>>
>> Ben.
>>
>> > Cheers,
>> > --
>> > Luis
>> >
>> > >
>> > > References: http://bugs.debian.org/684441 Reported-and-tested-by:
>> > > YunQiang Su <wzssyqa@gmail.com> Signed-off-by: Ben Hutchings
>> > > <ben@decadent.org.uk> Cc: stable@vger.kernel.org --- Unlike the
>> > > previous version, this will apply cleanly to the media
>> > > staging/for_v3.6 branch.
>> > >
>> > > Ben.
>> > >
>> > >  drivers/media/rc/ite-cir.c |    2 +-
>> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
>> > > index 36fe5a3..24c77a4 100644
>> > > --- a/drivers/media/rc/ite-cir.c
>> > > +++ b/drivers/media/rc/ite-cir.c
>> > > @@ -1473,6 +1473,7 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
>> > >   rdev = rc_allocate_device();
>> > >   if (!rdev)
>> > >           goto failure;
>> > > + itdev->rdev = rdev;
>> > >
>> > >   ret = -ENODEV;
>> > >
>> > > @@ -1604,7 +1605,6 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
>> > >   if (ret)
>> > >           goto failure3;
>> > >
>> > > - itdev->rdev = rdev;
>> > >   ite_pr(KERN_NOTICE, "driver has been successfully loaded\n");
>> > >
>> > >   return 0;
>> > >
>> >
>>
>> --
>> Ben Hutchings
>> It is a miracle that curiosity survives formal education. - Albert Einstein



-- 
YunQiang Su