All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Crosthwaite <crosthwaitepeter@gmail.com>
To: Andrew Baumann <Andrew.Baumann@microsoft.com>
Cc: Igor Mitsyanko <i.mitsyanko@gmail.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 2/3] sdhci: don't raise a command index error for an unexpected response
Date: Sun, 20 Dec 2015 13:44:38 -0800	[thread overview]
Message-ID: <CAPokK=r1iXFVw2NDwsL35vNV1Z-oRo-_GKZBj5o2MdW1GbVu1Q@mail.gmail.com> (raw)
In-Reply-To: <1450295260-10980-3-git-send-email-Andrew.Baumann@microsoft.com>

On Wed, Dec 16, 2015 at 11:47 AM, Andrew Baumann
<Andrew.Baumann@microsoft.com> wrote:
> This deletes a block of code that raised a command index error if a
> command returned response data, but the guest did not set the
> appropriate bits in the response register to handle such a response. I
> cannot find any documentation that suggests the controller should
> behave in this way, the error code doesn't make sense (command index
> error is defined for the case where the index in a response does not
> match that of the issued command), and in at least one case (CMD23
> issued by UEFI on Raspberry Pi 2), actual hardware does not do this.
>
> Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>

Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

All I can think of, is the original author is assuming that any
mismatch in response length can only occur on a mismatch of commands.
If we really need _CMDIDX it should be done directly as a check of the
command index in the response.

Regards,
Peter

> ---
>  hw/sd/sdhci.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index bc39d48..dd83e89 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -243,9 +243,6 @@ static void sdhci_send_command(SDHCIState *s)
>              (s->cmdreg & SDHC_CMD_RESPONSE) == SDHC_CMD_RSP_WITH_BUSY) {
>              s->norintsts |= SDHC_NIS_TRSCMP;
>          }
> -    } else if (rlen != 0 && (s->errintstsen & SDHC_EISEN_CMDIDX)) {
> -        s->errintsts |= SDHC_EIS_CMDIDX;
> -        s->norintsts |= SDHC_NIS_ERR;
>      }
>
>      if (s->norintstsen & SDHC_NISEN_CMDCMP) {
> --
> 2.5.3
>

  reply	other threads:[~2015-12-20 21:44 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-16 19:47 [Qemu-devel] [PATCH 0/3] sdhci patches to enable Raspberry Pi Andrew Baumann
2015-12-16 19:47 ` [Qemu-devel] [PATCH 1/3] sd: sdhci: Delete over-zealous power check Andrew Baumann
2015-12-16 20:00   ` Peter Crosthwaite
2015-12-16 20:28     ` Andrew Baumann
2015-12-16 19:47 ` [Qemu-devel] [PATCH 2/3] sdhci: don't raise a command index error for an unexpected response Andrew Baumann
2015-12-20 21:44   ` Peter Crosthwaite [this message]
2015-12-16 19:47 ` [Qemu-devel] [PATCH 3/3] sdhci: add optional quirk property to disable card insertion/removal interrupts Andrew Baumann
2015-12-20 22:34   ` Peter Crosthwaite

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPokK=r1iXFVw2NDwsL35vNV1Z-oRo-_GKZBj5o2MdW1GbVu1Q@mail.gmail.com' \
    --to=crosthwaitepeter@gmail.com \
    --cc=Andrew.Baumann@microsoft.com \
    --cc=i.mitsyanko@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.