diff mbox

[U-Boot] mx28: fix i.MX28 spi driver

Message ID CAOMZO5B5z6urOb7jx83c46tG_3V2TZ9LjpPJ9iVbDF9w-pEMzg@mail.gmail.com
State Changes Requested
Headers show

Commit Message

Fabio Estevam Jan. 14, 2012, 6:54 p.m. UTC
On Sat, Jan 14, 2012 at 4:53 PM, Fabio Estevam <festevam@gmail.com> wrote:

> Actually I meant this:

One more time ;-)

Comments

Matthias Fuchs Jan. 14, 2012, 8:09 p.m. UTC | #1
This cannot work. I do not understand what you are trying to achieve with this.

Matthias

On 01/14/2012 07:54 PM, Fabio Estevam wrote:
> On Sat, Jan 14, 2012 at 4:53 PM, Fabio Estevam <festevam@gmail.com> wrote:
> 
>> Actually I meant this:
> 
> One more time ;-)
> 
> --- a/drivers/spi/mxs_spi.c
> +++ b/drivers/spi/mxs_spi.c
> @@ -130,13 +130,10 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
>         const char *tx = dout;
>         char *rx = din;
> 
> -       if (bitlen == 0)
> -               return 0;
> -
>         if (!rx && !tx)
>                 return 0;
> 
> -       if (flags & SPI_XFER_BEGIN)
> +       if ((flags & SPI_XFER_BEGIN) && len)
>                 mxs_spi_start_xfer(ssp_regs);
> 
>         while (len--) {
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Fabio Estevam Jan. 14, 2012, 8:14 p.m. UTC | #2
On Sat, Jan 14, 2012 at 6:09 PM, Matthias Fuchs <matthias.fuchs@esd.eu> wrote:
> This cannot work. I do not understand what you are trying to achieve with this.

I would like to avoid the extra dummy read that your patch proposes.
Matthias Fuchs Jan. 15, 2012, 12:10 p.m. UTC | #3
On 01/14/2012 09:14 PM, Fabio Estevam wrote:
> On Sat, Jan 14, 2012 at 6:09 PM, Matthias Fuchs <matthias.fuchs@esd.eu> wrote:
>> This cannot work. I do not understand what you are trying to achieve with this.
> 
> I would like to avoid the extra dummy read that your patch proposes.
That's what I also tried. But from the ref manual I got no idea.
When we do not find a hy to deassert the chip select manually, we cannot
avoid this read.

It seems that the mainline linux only implements a GPIO based SPI driver.
So we cannot take it for reference.

I'm wonderng how this works on the Denx' M28EVK.

Matthias

> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Fabio Estevam Jan. 15, 2012, 3:28 p.m. UTC | #4
On Sun, Jan 15, 2012 at 10:10 AM, Matthias Fuchs <matthias.fuchs@esd.eu> wrote:

> That's what I also tried. But from the ref manual I got no idea.
> When we do not find a hy to deassert the chip select manually, we cannot
> avoid this read.

I was assuming that the chip select deassertion was correctly being
handled by the
mxs_spi_end_xfer function.

This function handles the LOCK_CS and IGNORE_CRC bits that are
responsible for chip select assertion/deassertion.

From the MX28 Reference Manual:

"IGNORE_CRC bit: In SPI/SSI modes: When set to 1, deassert the chip
select (SSn) pin after the command is executed."

Then I tought that mxs_spi_end_xfer should be also called in the case
where bitlen==0. In current code this does not happen.

> It seems that the mainline linux only implements a GPIO based SPI driver.
> So we cannot take it for reference.

There is no mxs spi driver in the mainline kernel yet.
Matthias Fuchs Jan. 15, 2012, 6:36 p.m. UTC | #5
On 01/15/2012 04:28 PM, Fabio Estevam wrote:
> On Sun, Jan 15, 2012 at 10:10 AM, Matthias Fuchs <matthias.fuchs@esd.eu> wrote:
> 
>> That's what I also tried. But from the ref manual I got no idea.
>> When we do not find a hy to deassert the chip select manually, we cannot
>> avoid this read.
> 
> I was assuming that the chip select deassertion was correctly being
> handled by the
> mxs_spi_end_xfer function.
> 
> This function handles the LOCK_CS and IGNORE_CRC bits that are
> responsible for chip select assertion/deassertion.
> 
> From the MX28 Reference Manual:
> 
> "IGNORE_CRC bit: In SPI/SSI modes: When set to 1, deassert the chip
> select (SSn) pin after the command is executed."
Exactly. That's the point. It says "... after the command ...". So you need 
a transfer after calling mxs_spi_end_xfer(). That's why the current code calls
it before transferring the final byte. 

Matthias

> 
> Then I tought that mxs_spi_end_xfer should be also called in the case
> where bitlen==0. In current code this does not happen.
> 
>> It seems that the mainline linux only implements a GPIO based SPI driver.
>> So we cannot take it for reference.
> 
> There is no mxs spi driver in the mainline kernel yet.
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
diff mbox

Patch

--- a/drivers/spi/mxs_spi.c
+++ b/drivers/spi/mxs_spi.c
@@ -130,13 +130,10 @@  int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
        const char *tx = dout;
        char *rx = din;

-       if (bitlen == 0)
-               return 0;
-
        if (!rx && !tx)
                return 0;

-       if (flags & SPI_XFER_BEGIN)
+       if ((flags & SPI_XFER_BEGIN) && len)
                mxs_spi_start_xfer(ssp_regs);

        while (len--) {