spi-atmel broken in next

spi-atmel broken in next

[Joachim Eastwood]

Hi,

spi-atmel is now broken for all spi transfers in linux-next. This is
caused by commit "spi: make sure all transfer has proper speed set".
The reason is the following code in atmel_spi_transfer(...)

 849  /* FIXME implement these protocol options!! */
 850  if (xfer->speed_hz) {
 851      dev_dbg(&spi->dev, "no protocol options yet\n");
 852      return -ENOPROTOOPT;
 853  }

Now that xfer->speed_hz is always set all spi transfers will return
with ENOPROTOOPT.

Not quite sure how to best fix it. Just removing the whole test
makes it work again but I guess we should do something better
with speed_hz here.

One little question; Is a per transfer speed setting really sensible?
As far as I can see from grepping the kernel source it's not
used by device drivers.


regards
Joachim Eastwood

------------------------------------------------------------------------------
The Go Parallel Website, sponsored by Intel - in partnership with Geeknet, 
is your hub for all things parallel software development, from weekly thought 
leadership blogs to news, videos, case studies, tutorials, tech docs, 
whitepapers, evaluation guides, and opinion stories. Check out the most 
recent posts - join the conversation now. http://goparallel.sourceforge.net/

Re: spi-atmel broken in next

[Grant Likely]

On Sun, Feb 17, 2013 at 11:15 AM, Joachim  Eastwood <manabian-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi,
>
> spi-atmel is now broken for all spi transfers in linux-next. This is
> caused by commit "spi: make sure all transfer has proper speed set".
> The reason is the following code in atmel_spi_transfer(...)
>
>  849  /* FIXME implement these protocol options!! */
>  850  if (xfer->speed_hz) {
>  851      dev_dbg(&spi->dev, "no protocol options yet\n");
>  852      return -ENOPROTOOPT;
>  853  }
>
> Now that xfer->speed_hz is always set all spi transfers will return
> with ENOPROTOOPT.
>
> Not quite sure how to best fix it. Just removing the whole test
> makes it work again but I guess we should do something better
> with speed_hz here.

Probably the best fix is to check instead if xfer->speed_hz is less
than the bus speed. If it is then you can fail. If it is not, then
continue.

g.

------------------------------------------------------------------------------
The Go Parallel Website, sponsored by Intel - in partnership with Geeknet, 
is your hub for all things parallel software development, from weekly thought 
leadership blogs to news, videos, case studies, tutorials, tech docs, 
whitepapers, evaluation guides, and opinion stories. Check out the most 
recent posts - join the conversation now. http://goparallel.sourceforge.net/

[PATCH] spi/atmel: fix speed_hz check in atmel_spi_transfer()

[Joachim Eastwood]

atmel_spi_transfer() would check speed_hz and fail if
the speed was changed in the transfer. After commit
"spi: make sure all transfer has proper speed set"
this would happen on all transfers.

Change speed_hz check to only fail if a lower speed
than max is requested.

Signed-off-by: Joachim Eastwood <manabian-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
Hi Grant,

Fix for spi-atmel as per your suggestion; check if
speed_hz is less than bus max speed.

regards
Joachim Eastwood

 drivers/spi/spi-atmel.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 656d137..80f5867 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -847,8 +847,8 @@ static int atmel_spi_transfer(struct spi_device *spi, struct spi_message *msg)
 		}
 
 		/* FIXME implement these protocol options!! */
-		if (xfer->speed_hz) {
-			dev_dbg(&spi->dev, "no protocol options yet\n");
+		if (xfer->speed_hz < spi->max_speed_hz) {
+			dev_dbg(&spi->dev, "can't change speed in transfer\n");
 			return -ENOPROTOOPT;
 		}
 
-- 
1.8.0


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_feb

Re: [PATCH] spi/atmel: fix speed_hz check in atmel_spi_transfer()

[Nicolas Ferre]

On 02/19/2013 10:44 PM, Joachim Eastwood :
> atmel_spi_transfer() would check speed_hz and fail if
> the speed was changed in the transfer. After commit
> "spi: make sure all transfer has proper speed set"
> this would happen on all transfers.
> 
> Change speed_hz check to only fail if a lower speed
> than max is requested.
> 
> Signed-off-by: Joachim Eastwood <manabian-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Acked-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>

Thanks for spotting this Joachim.

Bye,

> ---
> Hi Grant,
> 
> Fix for spi-atmel as per your suggestion; check if
> speed_hz is less than bus max speed.
> 
> regards
> Joachim Eastwood
> 
>  drivers/spi/spi-atmel.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index 656d137..80f5867 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -847,8 +847,8 @@ static int atmel_spi_transfer(struct spi_device *spi, struct spi_message *msg)
>  		}
>  
>  		/* FIXME implement these protocol options!! */
> -		if (xfer->speed_hz) {
> -			dev_dbg(&spi->dev, "no protocol options yet\n");
> +		if (xfer->speed_hz < spi->max_speed_hz) {
> +			dev_dbg(&spi->dev, "can't change speed in transfer\n");
>  			return -ENOPROTOOPT;
>  		}
>  
> 


-- 
Nicolas Ferre

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_feb

Re: [PATCH] spi/atmel: fix speed_hz check in atmel_spi_transfer()

[Grant Likely]

On Wed, 20 Feb 2013 08:51:48 +0100, Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> wrote:
> On 02/19/2013 10:44 PM, Joachim Eastwood :
> > atmel_spi_transfer() would check speed_hz and fail if
> > the speed was changed in the transfer. After commit
> > "spi: make sure all transfer has proper speed set"
> > this would happen on all transfers.
> > 
> > Change speed_hz check to only fail if a lower speed
> > than max is requested.
> > 
> > Signed-off-by: Joachim Eastwood <manabian-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> Acked-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> 
> Thanks for spotting this Joachim.

Applied, thanks.

g.

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_feb