[PATCH V2] spi/pl022: Fix range checking for bits per word

[PATCH V2] spi/pl022: Fix range checking for bits per word

[Vinit Shenoy]

pl022 ssp controller supports word lengths from 4 to 16 (or 32) bits.
Currently implemented checks were incorrect. It has following check

if (pl022->vendor->max_bpw >= 32)

which must be checking for <=.

Also error print message is incorrect, that prints "range is from 1 to
16".

Fix both these issues.

Signed-off-by: Vinit Shenoy <vinit.shenoy-qxv4g6HH51o@public.gmane.org>
---
V1->V2:
- Fixed the check:
	if (pl022->vendor->max_bpw >= 32)
- Re-written complete range check logic.

 drivers/spi/spi-pl022.c |   25 +++++++++----------------
 1 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index 09c925a..1ead49d 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -1823,9 +1823,12 @@ static int pl022_setup(struct spi_device *spi)
 	} else
 		chip->cs_control = chip_info->cs_control;
 
-	if (bits <= 3) {
-		/* PL022 doesn't support less than 4-bits */
+	/* Check bits per word with vendor specific range */
+	if ((bits <= 3) || (bits > pl022->vendor->max_bpw)) {
 		status = -ENOTSUPP;
+		dev_err(&spi->dev, "illegal data size for this controller!\n");
+		dev_err(&spi->dev, "This controller can only handle 4 <= n <= %d bit words\n",
+				pl022->vendor->max_bpw);
 		goto err_config_params;
 	} else if (bits <= 8) {
 		dev_dbg(&spi->dev, "4 <= n <=8 bits per word\n");
@@ -1838,20 +1841,10 @@ static int pl022_setup(struct spi_device *spi)
 		chip->read = READING_U16;
 		chip->write = WRITING_U16;
 	} else {
-		if (pl022->vendor->max_bpw >= 32) {
-			dev_dbg(&spi->dev, "17 <= n <= 32 bits per word\n");
-			chip->n_bytes = 4;
-			chip->read = READING_U32;
-			chip->write = WRITING_U32;
-		} else {
-			dev_err(&spi->dev,
-				"illegal data size for this controller!\n");
-			dev_err(&spi->dev,
-				"a standard pl022 can only handle "
-				"1 <= n <= 16 bit words\n");
-			status = -ENOTSUPP;
-			goto err_config_params;
-		}
+		dev_dbg(&spi->dev, "17 <= n <= 32 bits per word\n");
+		chip->n_bytes = 4;
+		chip->read = READING_U32;
+		chip->write = WRITING_U32;
 	}
 
 	/* Now Initialize all register settings required for this chip */
-- 
1.7.3.4


------------------------------------------------------------------------------
Better than sec? Nothing is better than sec when it comes to
monitoring Big Data applications. Try Boundary one-second 
resolution app monitoring today. Free.
http://p.sf.net/sfu/Boundary-dev2dev

Re: [PATCH V2] spi/pl022: Fix range checking for bits per word

[Linus Walleij]

On Tue, Apr 17, 2012 at 9:10 AM, Vinit Shenoy <vinit.shenoy-qxv4g6HH51o@public.gmane.org> wrote:

> pl022 ssp controller supports word lengths from 4 to 16 (or 32) bits.
> Currently implemented checks were incorrect. It has following check
>
> if (pl022->vendor->max_bpw >= 32)
>
> which must be checking for <=.
>
> Also error print message is incorrect, that prints "range is from 1 to
> 16".
>
> Fix both these issues.
>
> Signed-off-by: Vinit Shenoy <vinit.shenoy-qxv4g6HH51o@public.gmane.org>
> ---
> V1->V2:
> - Fixed the check:
>        if (pl022->vendor->max_bpw >= 32)
> - Re-written complete range check logic.

Looks correct to me:
Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Yours,
Linus Walleij

------------------------------------------------------------------------------
Better than sec? Nothing is better than sec when it comes to
monitoring Big Data applications. Try Boundary one-second 
resolution app monitoring today. Free.
http://p.sf.net/sfu/Boundary-dev2dev

Re: [PATCH V2] spi/pl022: Fix range checking for bits per word

[Grant Likely]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 909 bytes --]

On Tue, 17 Apr 2012 11:29:37 +0200, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Apr 17, 2012 at 9:10 AM, Vinit Shenoy <vinit.shenoy-qxv4g6HH51o@public.gmane.org> wrote:
> 
> > pl022 ssp controller supports word lengths from 4 to 16 (or 32) bits.
> > Currently implemented checks were incorrect. It has following check
> >
> > if (pl022->vendor->max_bpw >= 32)
> >
> > which must be checking for <=.
> >
> > Also error print message is incorrect, that prints "range is from 1 to
> > 16".
> >
> > Fix both these issues.
> >
> > Signed-off-by: Vinit Shenoy <vinit.shenoy-qxv4g6HH51o@public.gmane.org>
> > ---
> > V1->V2:
> > - Fixed the check:
> >        if (pl022->vendor->max_bpw >= 32)
> > - Re-written complete range check logic.
> 
> Looks correct to me:
> Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Applied, thanks

g.



[-- Attachment #2: Type: text/plain, Size: 274 bytes --]

------------------------------------------------------------------------------
Better than sec? Nothing is better than sec when it comes to
monitoring Big Data applications. Try Boundary one-second 
resolution app monitoring today. Free.
http://p.sf.net/sfu/Boundary-dev2dev

[-- Attachment #3: Type: text/plain, Size: 210 bytes --]

_______________________________________________
spi-devel-general mailing list
spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Re: [PATCH V2] spi/pl022: Fix range checking for bits per word

[Shubhrajyoti Datta]

Hi Vinit,

Looks OK to me ,

On Tue, Apr 17, 2012 at 12:40 PM, Vinit Shenoy <vinit.shenoy-qxv4g6HH51o@public.gmane.org> wrote:
> pl022 ssp controller supports word lengths from 4 to 16 (or 32) bits.
> Currently implemented checks were incorrect. It has following check
>
> if (pl022->vendor->max_bpw >= 32)
>
> which must be checking for <=.
>
> Also error print message is incorrect, that prints "range is from 1 to
> 16".
>
> Fix both these issues.

Thanks ,

>
> Signed-off-by: Vinit Shenoy <vinit.shenoy-qxv4g6HH51o@public.gmane.org>
> ---
> V1->V2:
> - Fixed the check:
>        if (pl022->vendor->max_bpw >= 32)
> - Re-written complete range check logic.
I think the patches could be split as one is a fix the other corrects the
error msg.

Nit:
Also the changlogs spell the issue and not the solution.
I think that could be improved.
Other than  that feel free to add
Reviewed-by: Shubhrajyoti D <shubhrajyoti-l0cyMroinI0@public.gmane.org>

If you like.
>
>  drivers/spi/spi-pl022.c |   25 +++++++++----------------
>  1 files changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
> index 09c925a..1ead49d 100644
> --- a/drivers/spi/spi-pl022.c
> +++ b/drivers/spi/spi-pl022.c
> @@ -1823,9 +1823,12 @@ static int pl022_setup(struct spi_device *spi)
>        } else
>                chip->cs_control = chip_info->cs_control;
>
> -       if (bits <= 3) {
> -               /* PL022 doesn't support less than 4-bits */
> +       /* Check bits per word with vendor specific range */
> +       if ((bits <= 3) || (bits > pl022->vendor->max_bpw)) {
>                status = -ENOTSUPP;
> +               dev_err(&spi->dev, "illegal data size for this controller!\n");
> +               dev_err(&spi->dev, "This controller can only handle 4 <= n <= %d bit words\n",
> +                               pl022->vendor->max_bpw);
>                goto err_config_params;
>        } else if (bits <= 8) {
>                dev_dbg(&spi->dev, "4 <= n <=8 bits per word\n");
> @@ -1838,20 +1841,10 @@ static int pl022_setup(struct spi_device *spi)
>                chip->read = READING_U16;
>                chip->write = WRITING_U16;
>        } else {
> -               if (pl022->vendor->max_bpw >= 32) {
> -                       dev_dbg(&spi->dev, "17 <= n <= 32 bits per word\n");
> -                       chip->n_bytes = 4;
> -                       chip->read = READING_U32;
> -                       chip->write = WRITING_U32;
> -               } else {
> -                       dev_err(&spi->dev,
> -                               "illegal data size for this controller!\n");
> -                       dev_err(&spi->dev,
> -                               "a standard pl022 can only handle "
> -                               "1 <= n <= 16 bit words\n");
> -                       status = -ENOTSUPP;
> -                       goto err_config_params;
> -               }
> +               dev_dbg(&spi->dev, "17 <= n <= 32 bits per word\n");
> +               chip->n_bytes = 4;
> +               chip->read = READING_U32;
> +               chip->write = WRITING_U32;
>        }
>
>        /* Now Initialize all register settings required for this chip */
> --
> 1.7.3.4
>

------------------------------------------------------------------------------
Better than sec? Nothing is better than sec when it comes to
monitoring Big Data applications. Try Boundary one-second 
resolution app monitoring today. Free.
http://p.sf.net/sfu/Boundary-dev2dev