linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] spi/pl022: make sure the chip_info dev pointer is set
@ 2010-09-30  9:40 Linus Walleij
  2010-09-30 14:33 ` Grant Likely
  0 siblings, 1 reply; 3+ messages in thread
From: Linus Walleij @ 2010-09-30  9:40 UTC (permalink / raw)
  To: Grant Likely, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
  Cc: Linus Walleij, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

We forgot to set the pointer back to the device if the user
supplied all device settings, leading to crashing debug prints
as controller data is verified.

Signed-off-by: Linus Walleij <linus.walleij-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>
---
 drivers/spi/amba-pl022.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/spi/amba-pl022.c b/drivers/spi/amba-pl022.c
index 4c37c4e..90a7e5d 100644
--- a/drivers/spi/amba-pl022.c
+++ b/drivers/spi/amba-pl022.c
@@ -1574,6 +1574,8 @@ static int pl022_setup(struct spi_device *spi)
 		chip_info->duplex = SSP_MICROWIRE_CHANNEL_FULL_DUPLEX;
 		chip_info->cs_control = null_cs_control;
 	} else {
+		/* Users should not set this field */
+		chip_info->dev = &spi->dev;
 		dev_dbg(&spi->dev,
 			"using user supplied controller_data settings\n");
 	}
-- 
1.6.3.3


------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] spi/pl022: make sure the chip_info dev pointer is set
  2010-09-30  9:40 [PATCH] spi/pl022: make sure the chip_info dev pointer is set Linus Walleij
@ 2010-09-30 14:33 ` Grant Likely
  2010-10-01 10:37   ` Linus Walleij
  0 siblings, 1 reply; 3+ messages in thread
From: Grant Likely @ 2010-09-30 14:33 UTC (permalink / raw)
  To: Linus Walleij; +Cc: spi-devel-general, linux-arm-kernel

On Thu, Sep 30, 2010 at 6:40 PM, Linus Walleij
<linus.walleij@stericsson.com> wrote:
> We forgot to set the pointer back to the device if the user
> supplied all device settings, leading to crashing debug prints
> as controller data is verified.
>
> Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
> ---
>  drivers/spi/amba-pl022.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/spi/amba-pl022.c b/drivers/spi/amba-pl022.c
> index 4c37c4e..90a7e5d 100644
> --- a/drivers/spi/amba-pl022.c
> +++ b/drivers/spi/amba-pl022.c
> @@ -1574,6 +1574,8 @@ static int pl022_setup(struct spi_device *spi)
>                chip_info->duplex = SSP_MICROWIRE_CHANNEL_FULL_DUPLEX;
>                chip_info->cs_control = null_cs_control;
>        } else {
> +               /* Users should not set this field */
> +               chip_info->dev = &spi->dev;
>                dev_dbg(&spi->dev,
>                        "using user supplied controller_data settings\n");
>        }

Nack.  data passed to a driver via the controller_data must be
immutable as far as the device driver is concerned.  I realize that
this has been rather sloppy in the past; but I'm enforcing it now.
controller_data is static data passed to the driver, and the 'dev'
member of pl022_config_chip needs to be removed.

So, instead of setting the dev pointer here; this code should *remove*
the dev pointer from the structure and use spi->dev directly instead.
I've looked at the driver, and I don't see any significant locations
using chip_info that cannot get the chip_info directly from the struct
spi_device.

g.


> --
> 1.6.3.3
>
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] spi/pl022: make sure the chip_info dev pointer is set
  2010-09-30 14:33 ` Grant Likely
@ 2010-10-01 10:37   ` Linus Walleij
  0 siblings, 0 replies; 3+ messages in thread
From: Linus Walleij @ 2010-10-01 10:37 UTC (permalink / raw)
  To: Grant Likely
  Cc: spi-devel-general@lists.sourceforge.net,
	linux-arm-kernel@lists.infradead.org

Grant Likely wrote:

>>        } else {
>> +               /* Users should not set this field */
>> +               chip_info->dev = &spi->dev;
>>                dev_dbg(&spi->dev,
>>                        "using user supplied controller_data settings\n");
>>        }
> 
> Nack.  data passed to a driver via the controller_data must be
> immutable as far as the device driver is concerned.

I posted another patch removing it instead, that's obviously the
right thing to do...

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-10-01 10:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-30  9:40 [PATCH] spi/pl022: make sure the chip_info dev pointer is set Linus Walleij
2010-09-30 14:33 ` Grant Likely
2010-10-01 10:37   ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).