[PATCH 1/1] atmel-ssc: fix misuse of dev_dbg when requested ssc instance is not found

[PATCH 1/1] atmel-ssc: fix misuse of dev_dbg when requested ssc instance is not found

[Hans-Christian Egtvedt]

This patch replaces the dev_dbg(...) with a pr_err since the ssc pointer
is not valid when the id is not found in the list.

Signed-off-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>
---
 drivers/misc/atmel-ssc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c
index 6b35874..6cff1bb 100644
--- a/drivers/misc/atmel-ssc.c
+++ b/drivers/misc/atmel-ssc.c
@@ -35,7 +35,7 @@ struct ssc_device *ssc_request(unsigned int ssc_num)
 
 	if (!ssc_valid) {
 		spin_unlock(&user_lock);
-		dev_dbg(&ssc->pdev->dev, "could not find requested device\n");
+		pr_err("ssc: ssc%d platform device is missing\n", ssc_num);
 		return ERR_PTR(-ENODEV);
 	}
 
-- 
1.5.6.3

Re: [PATCH 1/1] atmel-ssc: fix misuse of dev_dbg when requested ssc instance is not found

[Hans-Christian Egtvedt]

On Mon, 19 Jan 2009 13:57:56 +0100
Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com> wrote:

Bump, or did I miss the merge window?

> This patch replaces the dev_dbg(...) with a pr_err since the ssc
> pointer is not valid when the id is not found in the list.
> 
> Signed-off-by: Hans-Christian Egtvedt
> <hans-christian.egtvedt@atmel.com> ---
>  drivers/misc/atmel-ssc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c
> index 6b35874..6cff1bb 100644
> --- a/drivers/misc/atmel-ssc.c
> +++ b/drivers/misc/atmel-ssc.c
> @@ -35,7 +35,7 @@ struct ssc_device *ssc_request(unsigned int ssc_num)
>  
>  	if (!ssc_valid) {
>  		spin_unlock(&user_lock);
> -		dev_dbg(&ssc->pdev->dev, "could not find requested device\n");
> +		pr_err("ssc: ssc%d platform device is missing\n", ssc_num);
>  		return ERR_PTR(-ENODEV);
>  	}
>  

-- 
Best regards,
Hans-Christian Egtvedt

Re: [PATCH 1/1] atmel-ssc: fix misuse of dev_dbg when requested ssc instance is not found

[Andrew Morton]

On Thu, 29 Jan 2009 17:46:55 +0100
Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com> wrote:

> On Mon, 19 Jan 2009 13:57:56 +0100
> Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com> wrote:
> > This patch replaces the dev_dbg(...) with a pr_err since the ssc
> > pointer is not valid when the id is not found in the list.
> > 
> > Signed-off-by: Hans-Christian Egtvedt
> > <hans-christian.egtvedt@atmel.com> ---
> >  drivers/misc/atmel-ssc.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c
> > index 6b35874..6cff1bb 100644
> > --- a/drivers/misc/atmel-ssc.c
> > +++ b/drivers/misc/atmel-ssc.c
> > @@ -35,7 +35,7 @@ struct ssc_device *ssc_request(unsigned int ssc_num)
> >  
> >  	if (!ssc_valid) {
> >  		spin_unlock(&user_lock);
> > -		dev_dbg(&ssc->pdev->dev, "could not find requested device\n");
> > +		pr_err("ssc: ssc%d platform device is missing\n", ssc_num);
> >  		return ERR_PTR(-ENODEV);
> >  	}
> >  
> 
> Bump, or did I miss the merge window?
> 

(Top-posting repaired.  Please don't do that!  It makes it horrid to
reply to you)

(Suitable cc's added - this was why your patch got lost)

The patch seems reasonable but the changelog seems to be quite
misleading.  I did this:

   The ssc pointer is not valid when the id is not found in the
   list.  Convert the message from a debug one into an error message
   and avoid dereferencing the bad pointer.

OK?

Re: [PATCH 1/1] atmel-ssc: fix misuse of dev_dbg when requested ssc instance is not found

[Hans-Christian Egtvedt]

On Thu, 29 Jan 2009 15:50:42 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 29 Jan 2009 17:46:55 +0100
> Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com> wrote:
> 

<snipp>

> > Bump, or did I miss the merge window?
> > 
> 
> (Top-posting repaired.  Please don't do that!  It makes it horrid to
> reply to you)
> 

Sorry, I'll do a resend of the original email instead next time. I had
the idea about the initial email would be picked up.

> (Suitable cc's added - this was why your patch got lost)
> 

AFAICT the lkml is the place for the drivers/misc stuff. Should I be
looking in another file than MAINTAINERS?

> The patch seems reasonable but the changelog seems to be quite
> misleading.  I did this:
> 
>    The ssc pointer is not valid when the id is not found in the
>    list.  Convert the message from a debug one into an error message
>    and avoid dereferencing the bad pointer.
> 
> OK?

Sounds way better than my Norwegian English, thanks (-:

-- 
Best regards,
Hans-Christian Egtvedt

Re: [PATCH 1/1] atmel-ssc: fix misuse of dev_dbg when requested ssc instance is not found

[Andrew Morton]

On Fri, 30 Jan 2009 09:20:25 +0100 Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com> wrote:

> > (Suitable cc's added - this was why your patch got lost)
> > 
> 
> AFAICT the lkml is the place for the drivers/misc stuff. Should I be
> looking in another file than MAINTAINERS?

Lots of drivers aren't mentioned in MAINTAINERS.  It is pretty
important to cc the relevant maintainer if poss.  `git whatchanged' is
one way.  Or a bit of inspired guessing (atmel == Haavard, for example).

But the easy way is to cc me - I'll route the thing in the appropriate
direction.

Re: [PATCH 1/1] atmel-ssc: fix misuse of dev_dbg when requested ssc instance is not found

[Haavard Skinnemoen]

[CCs added]

Hans-Christian Egtvedt wrote:
> This patch replaces the dev_dbg(...) with a pr_err since the ssc pointer
> is not valid when the id is not found in the list.
> 
> Signed-off-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>

Acked-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>

...or should I just apply it to the avr32 tree?

Haavard

> ---
>  drivers/misc/atmel-ssc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c
> index 6b35874..6cff1bb 100644
> --- a/drivers/misc/atmel-ssc.c
> +++ b/drivers/misc/atmel-ssc.c
> @@ -35,7 +35,7 @@ struct ssc_device *ssc_request(unsigned int ssc_num)
>  
>  	if (!ssc_valid) {
>  		spin_unlock(&user_lock);
> -		dev_dbg(&ssc->pdev->dev, "could not find requested device\n");
> +		pr_err("ssc: ssc%d platform device is missing\n", ssc_num);
>  		return ERR_PTR(-ENODEV);
>  	}
>  

Re: [PATCH 1/1] atmel-ssc: fix misuse of dev_dbg when requested ssc instance is not found

[Andrew Morton]

On Fri, 30 Jan 2009 10:26:54 +0100 Haavard Skinnemoen <haavard.skinnemoen@atmel.com> wrote:

> [CCs added]
> 
> Hans-Christian Egtvedt wrote:
> > This patch replaces the dev_dbg(...) with a pr_err since the ssc pointer
> > is not valid when the id is not found in the list.
> > 
> > Signed-off-by: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>
> 
> Acked-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
> 
> ...or should I just apply it to the avr32 tree?
> 

Is OK, I tossed into onto my 2.6.29 pile.

Should it be backported to 2.6.28.x or earlier?

> 
> > ---
> >  drivers/misc/atmel-ssc.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c
> > index 6b35874..6cff1bb 100644
> > --- a/drivers/misc/atmel-ssc.c
> > +++ b/drivers/misc/atmel-ssc.c
> > @@ -35,7 +35,7 @@ struct ssc_device *ssc_request(unsigned int ssc_num)
> >  
> >  	if (!ssc_valid) {
> >  		spin_unlock(&user_lock);
> > -		dev_dbg(&ssc->pdev->dev, "could not find requested device\n");
> > +		pr_err("ssc: ssc%d platform device is missing\n", ssc_num);
> >  		return ERR_PTR(-ENODEV);
> >  	}
> >  

Re: [PATCH 1/1] atmel-ssc: fix misuse of dev_dbg when requested ssc instance is not found

[Hans-Christian Egtvedt]

On Fri, 30 Jan 2009 01:36:28 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Fri, 30 Jan 2009 10:26:54 +0100 Haavard Skinnemoen
> <haavard.skinnemoen@atmel.com> wrote:
> 
> > [CCs added]
> > 
> > Hans-Christian Egtvedt wrote:
> > > This patch replaces the dev_dbg(...) with a pr_err since the ssc
> > > pointer is not valid when the id is not found in the list.
> > > 
> > > Signed-off-by: Hans-Christian Egtvedt
> > > <hans-christian.egtvedt@atmel.com>
> > 
> > Acked-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
> > 
> > ...or should I just apply it to the avr32 tree?
> > 
> 
> Is OK, I tossed into onto my 2.6.29 pile.
> 
> Should it be backported to 2.6.28.x or earlier?
> 

It shoud apply clean to 2.6.28.x as well, but this bug will only
trigger when a user of the SSC peripheral tries to request a SSC which
is already requested or not added to the list at all. So in most cases
bad platform drivers setup, configuration error or bad code.

In kernel 2.6.29-rc3 there are three users:

1 sound/soc/atmel/sam9g20_wm8731.c        274 ssc = ssc_request(0);
2 sound/soc/atmel/playpaq_wm8510.c        399 ssc = ssc_request(0);
3 sound/spi/at73c213.c                    980 chip->ssc = ssc_request(board->ssc_id);

The two first are a bit suspicious since they hard code id 0.

My 0.02 € is to get it in for 2.6.29, but fine to leave out for earlier
kernels.

-- 
Best regards,
Hans-Christian Egtvedt

Re: [PATCH 1/1] atmel-ssc: fix misuse of dev_dbg when requested ssc instance is not found

[Mark Brown]

On Fri, Jan 30, 2009 at 10:50:57AM +0100, Hans-Christian Egtvedt wrote:

> 1 sound/soc/atmel/sam9g20_wm8731.c        274 ssc = ssc_request(0);
> 2 sound/soc/atmel/playpaq_wm8510.c        399 ssc = ssc_request(0);
> 3 sound/spi/at73c213.c                    980 chip->ssc = ssc_request(board->ssc_id);

> The two first are a bit suspicious since they hard code id 0.

They are ASoC board drivers so are already entirely specific to one
board (or family of boards).