* [Qemu-devel] [PATCH] hw/sd/bcm2835_sdhost: Fix PIO mode writes
@ 2018-07-16 22:23 Guenter Roeck
2018-07-17 1:20 ` Philippe Mathieu-Daudé
2018-07-19 10:38 ` Peter Maydell
0 siblings, 2 replies; 4+ messages in thread
From: Guenter Roeck @ 2018-07-16 22:23 UTC (permalink / raw)
To: Peter Maydell; +Cc: Philippe Mathieu-Daudé, qemu-devel, Guenter Roeck
Writes in PIO mode have two requirements:
- A data interrupt must be generated after a write command has been
issued to indicate that the chip is ready to receive data.
- A block interrupt must be generated after each block to indicate
that the chip is ready to receive the next data block.
Rearrange the code to make this happen. Tested on raspi3 (in PIO mode)
and raspi2 (in DMA mode).
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
hw/sd/bcm2835_sdhost.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/hw/sd/bcm2835_sdhost.c b/hw/sd/bcm2835_sdhost.c
index 4df4de7..1b760b2 100644
--- a/hw/sd/bcm2835_sdhost.c
+++ b/hw/sd/bcm2835_sdhost.c
@@ -179,9 +179,11 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s)
uint32_t value = 0;
int n;
int is_read;
+ int is_write;
is_read = (s->cmd & SDCMD_READ_CMD) != 0;
- if (s->datacnt != 0 && (!is_read || sdbus_data_ready(&s->sdbus))) {
+ is_write = (s->cmd & SDCMD_WRITE_CMD) != 0;
+ if (s->datacnt != 0 && (is_write || sdbus_data_ready(&s->sdbus))) {
if (is_read) {
n = 0;
while (s->datacnt && s->fifo_len < BCM2835_SDHOST_FIFO_LEN) {
@@ -201,8 +203,11 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s)
if (n != 0) {
bcm2835_sdhost_fifo_push(s, value);
s->status |= SDHSTS_DATA_FLAG;
+ if (s->config & SDHCFG_DATA_IRPT_EN) {
+ s->status |= SDHSTS_SDIO_IRPT;
+ }
}
- } else { /* write */
+ } else if (is_write) { /* write */
n = 0;
while (s->datacnt > 0 && (s->fifo_len > 0 || n > 0)) {
if (n == 0) {
@@ -223,11 +228,18 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s)
s->edm &= ~SDEDM_FSM_MASK;
s->edm |= SDEDM_FSM_DATAMODE;
trace_bcm2835_sdhost_edm_change("datacnt 0", s->edm);
-
- if ((s->cmd & SDCMD_WRITE_CMD) &&
+ }
+ if (is_write) {
+ /* set block interrupt at end of each block transfer */
+ if (s->hbct && s->datacnt % s->hbct == 0 &&
(s->config & SDHCFG_BLOCK_IRPT_EN)) {
s->status |= SDHSTS_BLOCK_IRPT;
}
+ /* set data interrupt after each transfer */
+ s->status |= SDHSTS_DATA_FLAG;
+ if (s->config & SDHCFG_DATA_IRPT_EN) {
+ s->status |= SDHSTS_SDIO_IRPT;
+ }
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] hw/sd/bcm2835_sdhost: Fix PIO mode writes
2018-07-16 22:23 [Qemu-devel] [PATCH] hw/sd/bcm2835_sdhost: Fix PIO mode writes Guenter Roeck
@ 2018-07-17 1:20 ` Philippe Mathieu-Daudé
2018-07-17 15:56 ` Guenter Roeck
2018-07-19 10:38 ` Peter Maydell
1 sibling, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-07-17 1:20 UTC (permalink / raw)
To: Guenter Roeck, Clement Deschamps, Luc Michel; +Cc: Peter Maydell, qemu-devel
Cc'ing ANTField folks who might have the specs.
On 07/16/2018 07:23 PM, Guenter Roeck wrote:
> Writes in PIO mode have two requirements:
>
> - A data interrupt must be generated after a write command has been
> issued to indicate that the chip is ready to receive data.
> - A block interrupt must be generated after each block to indicate
> that the chip is ready to receive the next data block.
>
> Rearrange the code to make this happen. Tested on raspi3 (in PIO mode)
> and raspi2 (in DMA mode).
Note to reviewers, this seems a follow-up of:
http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg03397.html
(This patch makes more sens after reading this previous thread).
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> hw/sd/bcm2835_sdhost.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/hw/sd/bcm2835_sdhost.c b/hw/sd/bcm2835_sdhost.c
> index 4df4de7..1b760b2 100644
> --- a/hw/sd/bcm2835_sdhost.c
> +++ b/hw/sd/bcm2835_sdhost.c
> @@ -179,9 +179,11 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s)
> uint32_t value = 0;
> int n;
> int is_read;
> + int is_write;
>
> is_read = (s->cmd & SDCMD_READ_CMD) != 0;
> - if (s->datacnt != 0 && (!is_read || sdbus_data_ready(&s->sdbus))) {
> + is_write = (s->cmd & SDCMD_WRITE_CMD) != 0;
> + if (s->datacnt != 0 && (is_write || sdbus_data_ready(&s->sdbus))) {
> if (is_read) {
> n = 0;
> while (s->datacnt && s->fifo_len < BCM2835_SDHOST_FIFO_LEN) {
> @@ -201,8 +203,11 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s)
> if (n != 0) {
> bcm2835_sdhost_fifo_push(s, value);
> s->status |= SDHSTS_DATA_FLAG;
> + if (s->config & SDHCFG_DATA_IRPT_EN) {
> + s->status |= SDHSTS_SDIO_IRPT;
> + }
> }
> - } else { /* write */
> + } else if (is_write) { /* write */
> n = 0;
> while (s->datacnt > 0 && (s->fifo_len > 0 || n > 0)) {
> if (n == 0) {
> @@ -223,11 +228,18 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s)
> s->edm &= ~SDEDM_FSM_MASK;
> s->edm |= SDEDM_FSM_DATAMODE;
> trace_bcm2835_sdhost_edm_change("datacnt 0", s->edm);
> -
> - if ((s->cmd & SDCMD_WRITE_CMD) &&
> + }
> + if (is_write) {
> + /* set block interrupt at end of each block transfer */
> + if (s->hbct && s->datacnt % s->hbct == 0 &&
> (s->config & SDHCFG_BLOCK_IRPT_EN)) {
> s->status |= SDHSTS_BLOCK_IRPT;
> }
> + /* set data interrupt after each transfer */
> + s->status |= SDHSTS_DATA_FLAG;
> + if (s->config & SDHCFG_DATA_IRPT_EN) {
> + s->status |= SDHSTS_SDIO_IRPT;
> + }
> }
> }
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] hw/sd/bcm2835_sdhost: Fix PIO mode writes
2018-07-17 1:20 ` Philippe Mathieu-Daudé
@ 2018-07-17 15:56 ` Guenter Roeck
0 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2018-07-17 15:56 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Clement Deschamps, Luc Michel, Peter Maydell, qemu-devel
On Mon, Jul 16, 2018 at 10:20:37PM -0300, Philippe Mathieu-Daudé wrote:
> Cc'ing ANTField folks who might have the specs.
>
> On 07/16/2018 07:23 PM, Guenter Roeck wrote:
> > Writes in PIO mode have two requirements:
> >
> > - A data interrupt must be generated after a write command has been
> > issued to indicate that the chip is ready to receive data.
> > - A block interrupt must be generated after each block to indicate
> > that the chip is ready to receive the next data block.
> >
> > Rearrange the code to make this happen. Tested on raspi3 (in PIO mode)
> > and raspi2 (in DMA mode).
>
> Note to reviewers, this seems a follow-up of:
> http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg03397.html
> (This patch makes more sens after reading this previous thread).
>
Not sure about making more sense, but the other patch may be required
as prerequisite. I noticed multiple divide by zero errors in the Linux
mmc code without it.
Guenter
> >
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> > hw/sd/bcm2835_sdhost.c | 20 ++++++++++++++++----
> > 1 file changed, 16 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/sd/bcm2835_sdhost.c b/hw/sd/bcm2835_sdhost.c
> > index 4df4de7..1b760b2 100644
> > --- a/hw/sd/bcm2835_sdhost.c
> > +++ b/hw/sd/bcm2835_sdhost.c
> > @@ -179,9 +179,11 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s)
> > uint32_t value = 0;
> > int n;
> > int is_read;
> > + int is_write;
> >
> > is_read = (s->cmd & SDCMD_READ_CMD) != 0;
> > - if (s->datacnt != 0 && (!is_read || sdbus_data_ready(&s->sdbus))) {
> > + is_write = (s->cmd & SDCMD_WRITE_CMD) != 0;
> > + if (s->datacnt != 0 && (is_write || sdbus_data_ready(&s->sdbus))) {
> > if (is_read) {
> > n = 0;
> > while (s->datacnt && s->fifo_len < BCM2835_SDHOST_FIFO_LEN) {
> > @@ -201,8 +203,11 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s)
> > if (n != 0) {
> > bcm2835_sdhost_fifo_push(s, value);
> > s->status |= SDHSTS_DATA_FLAG;
> > + if (s->config & SDHCFG_DATA_IRPT_EN) {
> > + s->status |= SDHSTS_SDIO_IRPT;
> > + }
> > }
> > - } else { /* write */
> > + } else if (is_write) { /* write */
> > n = 0;
> > while (s->datacnt > 0 && (s->fifo_len > 0 || n > 0)) {
> > if (n == 0) {
> > @@ -223,11 +228,18 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s)
> > s->edm &= ~SDEDM_FSM_MASK;
> > s->edm |= SDEDM_FSM_DATAMODE;
> > trace_bcm2835_sdhost_edm_change("datacnt 0", s->edm);
> > -
> > - if ((s->cmd & SDCMD_WRITE_CMD) &&
> > + }
> > + if (is_write) {
> > + /* set block interrupt at end of each block transfer */
> > + if (s->hbct && s->datacnt % s->hbct == 0 &&
> > (s->config & SDHCFG_BLOCK_IRPT_EN)) {
> > s->status |= SDHSTS_BLOCK_IRPT;
> > }
> > + /* set data interrupt after each transfer */
> > + s->status |= SDHSTS_DATA_FLAG;
> > + if (s->config & SDHCFG_DATA_IRPT_EN) {
> > + s->status |= SDHSTS_SDIO_IRPT;
> > + }
> > }
> > }
> >
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/sd/bcm2835_sdhost: Fix PIO mode writes
2018-07-16 22:23 [Qemu-devel] [PATCH] hw/sd/bcm2835_sdhost: Fix PIO mode writes Guenter Roeck
2018-07-17 1:20 ` Philippe Mathieu-Daudé
@ 2018-07-19 10:38 ` Peter Maydell
1 sibling, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2018-07-19 10:38 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Philippe Mathieu-Daudé, QEMU Developers
On 16 July 2018 at 23:23, Guenter Roeck <linux@roeck-us.net> wrote:
> Writes in PIO mode have two requirements:
>
> - A data interrupt must be generated after a write command has been
> issued to indicate that the chip is ready to receive data.
> - A block interrupt must be generated after each block to indicate
> that the chip is ready to receive the next data block.
>
> Rearrange the code to make this happen. Tested on raspi3 (in PIO mode)
> and raspi2 (in DMA mode).
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
As you say, all still a bit guesswork, but this doesn't break
my previously-working raspi3 kernel and it looks plausible
(especially the separation of "is_read" and "is_write" rather
than assuming everything not a read is a write). Applied
to target-arm.next for the next 3.0 rc.
thanks
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-07-19 10:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-16 22:23 [Qemu-devel] [PATCH] hw/sd/bcm2835_sdhost: Fix PIO mode writes Guenter Roeck
2018-07-17 1:20 ` Philippe Mathieu-Daudé
2018-07-17 15:56 ` Guenter Roeck
2018-07-19 10:38 ` Peter Maydell
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).