diff mbox series

[U-Boot,v4,1/4] mmc: mmc_spi: Use SPI_XFER_BEGIN and SPI_XFER_END flags

Message ID 20190716042120.32548-2-anup.patel@wdc.com
State Superseded
Delegated to: Peng Fan
Headers show
Series SiFive SPI MMC Support | expand

Commit Message

Anup Patel July 16, 2019, 4:22 a.m. UTC
Most DM based SPI host controller drivers use SPI_XFER_BEGIN and
SPI_XFER_END flags to enable/disable slave chip select.

This patch extends MMC SPI driver to pass SPI_XFER_BEGIN flag when
MMC command is send at start and pass SPI_XFER_END flag using a
dummy transfer (of bitlen = 0) at the end of MMC command.

Suggested-by: Jagan Teki <jagan@amarulasolutions.com>
Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 drivers/mmc/mmc_spi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Jagan Teki July 16, 2019, 7:35 a.m. UTC | #1
On Tue, Jul 16, 2019 at 9:52 AM Anup Patel <Anup.Patel@wdc.com> wrote:
>
> Most DM based SPI host controller drivers use SPI_XFER_BEGIN and
> SPI_XFER_END flags to enable/disable slave chip select.
>
> This patch extends MMC SPI driver to pass SPI_XFER_BEGIN flag when
> MMC command is send at start and pass SPI_XFER_END flag using a
> dummy transfer (of bitlen = 0) at the end of MMC command.
>
> Suggested-by: Jagan Teki <jagan@amarulasolutions.com>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>  drivers/mmc/mmc_spi.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c
> index f3d687ae80..350812a04b 100644
> --- a/drivers/mmc/mmc_spi.c
> +++ b/drivers/mmc/mmc_spi.c
> @@ -84,7 +84,7 @@ static int mmc_spi_sendcmd(struct udevice *dev,
>         cmdo[4] = cmdarg >> 8;
>         cmdo[5] = cmdarg;
>         cmdo[6] = (crc7(0, &cmdo[1], 5) << 1) | 0x01;
> -       ret = dm_spi_xfer(dev, sizeof(cmdo) * 8, cmdo, NULL, 0);
> +       ret = dm_spi_xfer(dev, sizeof(cmdo) * 8, cmdo, NULL, SPI_XFER_BEGIN);
>         if (ret)
>                 return ret;
>
> @@ -360,6 +360,8 @@ static int dm_mmc_spi_request(struct udevice *dev, struct mmc_cmd *cmd,
>         }
>
>  done:
> +       dm_spi_xfer(dev, 0, NULL, NULL, SPI_XFER_END);
> +

Look like this driver has several dm_spi_xfer in their won sequence,
have you tried of sending command and buf in same spi_xfer call so
dout and din can be NULL based on the read and write? if ie possible
we can manage the transfer flags in single sub call based on data_len
like drivers/mtd/spi/sf.c does.
Anup Patel July 16, 2019, 8:08 a.m. UTC | #2
On Tue, Jul 16, 2019 at 1:05 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Tue, Jul 16, 2019 at 9:52 AM Anup Patel <Anup.Patel@wdc.com> wrote:
> >
> > Most DM based SPI host controller drivers use SPI_XFER_BEGIN and
> > SPI_XFER_END flags to enable/disable slave chip select.
> >
> > This patch extends MMC SPI driver to pass SPI_XFER_BEGIN flag when
> > MMC command is send at start and pass SPI_XFER_END flag using a
> > dummy transfer (of bitlen = 0) at the end of MMC command.
> >
> > Suggested-by: Jagan Teki <jagan@amarulasolutions.com>
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > ---
> >  drivers/mmc/mmc_spi.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c
> > index f3d687ae80..350812a04b 100644
> > --- a/drivers/mmc/mmc_spi.c
> > +++ b/drivers/mmc/mmc_spi.c
> > @@ -84,7 +84,7 @@ static int mmc_spi_sendcmd(struct udevice *dev,
> >         cmdo[4] = cmdarg >> 8;
> >         cmdo[5] = cmdarg;
> >         cmdo[6] = (crc7(0, &cmdo[1], 5) << 1) | 0x01;
> > -       ret = dm_spi_xfer(dev, sizeof(cmdo) * 8, cmdo, NULL, 0);
> > +       ret = dm_spi_xfer(dev, sizeof(cmdo) * 8, cmdo, NULL, SPI_XFER_BEGIN);
> >         if (ret)
> >                 return ret;
> >
> > @@ -360,6 +360,8 @@ static int dm_mmc_spi_request(struct udevice *dev, struct mmc_cmd *cmd,
> >         }
> >
> >  done:
> > +       dm_spi_xfer(dev, 0, NULL, NULL, SPI_XFER_END);
> > +
>
> Look like this driver has several dm_spi_xfer in their won sequence,
> have you tried of sending command and buf in same spi_xfer call so
> dout and din can be NULL based on the read and write? if ie possible
> we can manage the transfer flags in single sub call based on data_len
> like drivers/mtd/spi/sf.c does.

MMC SPI protocol is very different compared to SPI serial flash.

We have to poll for expected byte pattern when waiting for transfer/command
to complete. We don't know in-advance how many bytes we will have to read.

Regards,
Anup
Jagan Teki July 16, 2019, 8:26 a.m. UTC | #3
On Tue, Jul 16, 2019 at 1:38 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Tue, Jul 16, 2019 at 1:05 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > On Tue, Jul 16, 2019 at 9:52 AM Anup Patel <Anup.Patel@wdc.com> wrote:
> > >
> > > Most DM based SPI host controller drivers use SPI_XFER_BEGIN and
> > > SPI_XFER_END flags to enable/disable slave chip select.
> > >
> > > This patch extends MMC SPI driver to pass SPI_XFER_BEGIN flag when
> > > MMC command is send at start and pass SPI_XFER_END flag using a
> > > dummy transfer (of bitlen = 0) at the end of MMC command.
> > >
> > > Suggested-by: Jagan Teki <jagan@amarulasolutions.com>
> > > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > > ---
> > >  drivers/mmc/mmc_spi.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c
> > > index f3d687ae80..350812a04b 100644
> > > --- a/drivers/mmc/mmc_spi.c
> > > +++ b/drivers/mmc/mmc_spi.c
> > > @@ -84,7 +84,7 @@ static int mmc_spi_sendcmd(struct udevice *dev,
> > >         cmdo[4] = cmdarg >> 8;
> > >         cmdo[5] = cmdarg;
> > >         cmdo[6] = (crc7(0, &cmdo[1], 5) << 1) | 0x01;
> > > -       ret = dm_spi_xfer(dev, sizeof(cmdo) * 8, cmdo, NULL, 0);
> > > +       ret = dm_spi_xfer(dev, sizeof(cmdo) * 8, cmdo, NULL, SPI_XFER_BEGIN);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -360,6 +360,8 @@ static int dm_mmc_spi_request(struct udevice *dev, struct mmc_cmd *cmd,
> > >         }
> > >
> > >  done:
> > > +       dm_spi_xfer(dev, 0, NULL, NULL, SPI_XFER_END);
> > > +
> >
> > Look like this driver has several dm_spi_xfer in their won sequence,
> > have you tried of sending command and buf in same spi_xfer call so
> > dout and din can be NULL based on the read and write? if ie possible
> > we can manage the transfer flags in single sub call based on data_len
> > like drivers/mtd/spi/sf.c does.
>
> MMC SPI protocol is very different compared to SPI serial flash.
>
> We have to poll for expected byte pattern when waiting for transfer/command
> to complete. We don't know in-advance how many bytes we will have to read.

Yes, so we begin the transfer and end if successful is it? apart from
this we also need to provide claim and release calls from here,
so-that the sifive spi can handle them accordingly otherwise using
sifive spi may not work with flash.
Anup Patel July 16, 2019, 8:52 a.m. UTC | #4
On Tue, Jul 16, 2019 at 1:56 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Tue, Jul 16, 2019 at 1:38 PM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Tue, Jul 16, 2019 at 1:05 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > On Tue, Jul 16, 2019 at 9:52 AM Anup Patel <Anup.Patel@wdc.com> wrote:
> > > >
> > > > Most DM based SPI host controller drivers use SPI_XFER_BEGIN and
> > > > SPI_XFER_END flags to enable/disable slave chip select.
> > > >
> > > > This patch extends MMC SPI driver to pass SPI_XFER_BEGIN flag when
> > > > MMC command is send at start and pass SPI_XFER_END flag using a
> > > > dummy transfer (of bitlen = 0) at the end of MMC command.
> > > >
> > > > Suggested-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > > > ---
> > > >  drivers/mmc/mmc_spi.c | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c
> > > > index f3d687ae80..350812a04b 100644
> > > > --- a/drivers/mmc/mmc_spi.c
> > > > +++ b/drivers/mmc/mmc_spi.c
> > > > @@ -84,7 +84,7 @@ static int mmc_spi_sendcmd(struct udevice *dev,
> > > >         cmdo[4] = cmdarg >> 8;
> > > >         cmdo[5] = cmdarg;
> > > >         cmdo[6] = (crc7(0, &cmdo[1], 5) << 1) | 0x01;
> > > > -       ret = dm_spi_xfer(dev, sizeof(cmdo) * 8, cmdo, NULL, 0);
> > > > +       ret = dm_spi_xfer(dev, sizeof(cmdo) * 8, cmdo, NULL, SPI_XFER_BEGIN);
> > > >         if (ret)
> > > >                 return ret;
> > > >
> > > > @@ -360,6 +360,8 @@ static int dm_mmc_spi_request(struct udevice *dev, struct mmc_cmd *cmd,
> > > >         }
> > > >
> > > >  done:
> > > > +       dm_spi_xfer(dev, 0, NULL, NULL, SPI_XFER_END);
> > > > +
> > >
> > > Look like this driver has several dm_spi_xfer in their won sequence,
> > > have you tried of sending command and buf in same spi_xfer call so
> > > dout and din can be NULL based on the read and write? if ie possible
> > > we can manage the transfer flags in single sub call based on data_len
> > > like drivers/mtd/spi/sf.c does.
> >
> > MMC SPI protocol is very different compared to SPI serial flash.
> >
> > We have to poll for expected byte pattern when waiting for transfer/command
> > to complete. We don't know in-advance how many bytes we will have to read.
>
> Yes, so we begin the transfer and end if successful is it? apart from
> this we also need to provide claim and release calls from here,
> so-that the sifive spi can handle them accordingly otherwise using
> sifive spi may not work with flash.

The claim and release functions are already called at beginning and
end of dm_mmc_spi_request() respectively.
Jagan Teki July 16, 2019, 11:26 a.m. UTC | #5
On Tue, Jul 16, 2019 at 2:22 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Tue, Jul 16, 2019 at 1:56 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > On Tue, Jul 16, 2019 at 1:38 PM Anup Patel <anup@brainfault.org> wrote:
> > >
> > > On Tue, Jul 16, 2019 at 1:05 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > > >
> > > > On Tue, Jul 16, 2019 at 9:52 AM Anup Patel <Anup.Patel@wdc.com> wrote:
> > > > >
> > > > > Most DM based SPI host controller drivers use SPI_XFER_BEGIN and
> > > > > SPI_XFER_END flags to enable/disable slave chip select.
> > > > >
> > > > > This patch extends MMC SPI driver to pass SPI_XFER_BEGIN flag when
> > > > > MMC command is send at start and pass SPI_XFER_END flag using a
> > > > > dummy transfer (of bitlen = 0) at the end of MMC command.
> > > > >
> > > > > Suggested-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > > > > ---
> > > > >  drivers/mmc/mmc_spi.c | 4 +++-
> > > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c
> > > > > index f3d687ae80..350812a04b 100644
> > > > > --- a/drivers/mmc/mmc_spi.c
> > > > > +++ b/drivers/mmc/mmc_spi.c
> > > > > @@ -84,7 +84,7 @@ static int mmc_spi_sendcmd(struct udevice *dev,
> > > > >         cmdo[4] = cmdarg >> 8;
> > > > >         cmdo[5] = cmdarg;
> > > > >         cmdo[6] = (crc7(0, &cmdo[1], 5) << 1) | 0x01;
> > > > > -       ret = dm_spi_xfer(dev, sizeof(cmdo) * 8, cmdo, NULL, 0);
> > > > > +       ret = dm_spi_xfer(dev, sizeof(cmdo) * 8, cmdo, NULL, SPI_XFER_BEGIN);
> > > > >         if (ret)
> > > > >                 return ret;
> > > > >
> > > > > @@ -360,6 +360,8 @@ static int dm_mmc_spi_request(struct udevice *dev, struct mmc_cmd *cmd,
> > > > >         }
> > > > >
> > > > >  done:
> > > > > +       dm_spi_xfer(dev, 0, NULL, NULL, SPI_XFER_END);
> > > > > +
> > > >
> > > > Look like this driver has several dm_spi_xfer in their won sequence,
> > > > have you tried of sending command and buf in same spi_xfer call so
> > > > dout and din can be NULL based on the read and write? if ie possible
> > > > we can manage the transfer flags in single sub call based on data_len
> > > > like drivers/mtd/spi/sf.c does.
> > >
> > > MMC SPI protocol is very different compared to SPI serial flash.
> > >
> > > We have to poll for expected byte pattern when waiting for transfer/command
> > > to complete. We don't know in-advance how many bytes we will have to read.
> >
> > Yes, so we begin the transfer and end if successful is it? apart from
> > this we also need to provide claim and release calls from here,
> > so-that the sifive spi can handle them accordingly otherwise using
> > sifive spi may not work with flash.
>
> The claim and release functions are already called at beginning and
> end of dm_mmc_spi_request() respectively.

I'm saying from spi driver, since we don't have dedicated bus enable
bit the driver can make use of cs handling. so the claim can return
always 0 in this case.
diff mbox series

Patch

diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c
index f3d687ae80..350812a04b 100644
--- a/drivers/mmc/mmc_spi.c
+++ b/drivers/mmc/mmc_spi.c
@@ -84,7 +84,7 @@  static int mmc_spi_sendcmd(struct udevice *dev,
 	cmdo[4] = cmdarg >> 8;
 	cmdo[5] = cmdarg;
 	cmdo[6] = (crc7(0, &cmdo[1], 5) << 1) | 0x01;
-	ret = dm_spi_xfer(dev, sizeof(cmdo) * 8, cmdo, NULL, 0);
+	ret = dm_spi_xfer(dev, sizeof(cmdo) * 8, cmdo, NULL, SPI_XFER_BEGIN);
 	if (ret)
 		return ret;
 
@@ -360,6 +360,8 @@  static int dm_mmc_spi_request(struct udevice *dev, struct mmc_cmd *cmd,
 	}
 
 done:
+	dm_spi_xfer(dev, 0, NULL, NULL, SPI_XFER_END);
+
 	dm_spi_release_bus(dev);
 
 	return ret;