Patchwork [v3,12/14] hw/sd.c, hw/sd.h: add receive ready query routine to SD/MMC API

login
register
mail settings
Submitter Evgeny Voevodin
Date Dec. 12, 2011, 6:43 a.m.
Message ID <1323672206-11891-13-git-send-email-e.voevodin@samsung.com>
Download mbox | patch
Permalink /patch/130646/
State New
Headers show

Comments

Evgeny Voevodin - Dec. 12, 2011, 6:43 a.m.
From: Mitsyanko Igor <i.mitsyanko@samsung.com>

Data transfer direction between host controller and SD/MMC card is selected by
host controller configuration registers, but whether we actually need or need
not perform data transfer depends on type of last issued command. To avoid
memorization of which type of command host controller issued the last time, we
can use simple query procedures, to make sure that SD/MMC card is in the
right state. The only query routine currently presented in SD/MMC card
emulation is sd_data_ready(), this patch adds sd_receive_ready() routine.

Signed-off-by: Evgeny Voevodin <e.voevodin@samsung.com>
---
 hw/sd.c |    5 +++++
 hw/sd.h |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)
Peter Maydell - Dec. 13, 2011, 3:11 p.m.
On 12 December 2011 06:43, Evgeny Voevodin <e.voevodin@samsung.com> wrote:
> From: Mitsyanko Igor <i.mitsyanko@samsung.com>
>
> Data transfer direction between host controller and SD/MMC card is selected by
> host controller configuration registers, but whether we actually need or need
> not perform data transfer depends on type of last issued command. To avoid
> memorization of which type of command host controller issued the last time, we
> can use simple query procedures, to make sure that SD/MMC card is in the
> right state. The only query routine currently presented in SD/MMC card
> emulation is sd_data_ready(), this patch adds sd_receive_ready() routine.

I've thought about this a bit more and spent some time poking through
the standards docs. I'm kind of suspicious of both this function and
the existing sd_data_ready(). Real hardware doesn't have these signals,
both ends of the sd-controller-to-card connection just do their thing
and trust that the other end is in the state it is supposed to be.
If you (in the controller model) don't already know whether you are
OK to send/receive data then you haven't modelled some part of the
controller's state properly.

In your specific case you know whether a command is a data transfer
command by the Data Present Select bit that was written to the Command
Register. You know whether it's read or write by the Data Transfer
Direction Select bit in the Transfer Mode Register. So you don't need
to ask the sd card about these things. (The controller needs to not
blow up if the guest sets Data Present for a command that isn't a
data transfer command, and so on, but that should mostly just work
out without special code I think.)

-- PMM
Peter Maydell - Dec. 13, 2011, 4:22 p.m.
On 13 December 2011 15:11, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 12 December 2011 06:43, Evgeny Voevodin <e.voevodin@samsung.com> wrote:
>> From: Mitsyanko Igor <i.mitsyanko@samsung.com>
>>
>> Data transfer direction between host controller and SD/MMC card is selected by
>> host controller configuration registers, but whether we actually need or need
>> not perform data transfer depends on type of last issued command. To avoid
>> memorization of which type of command host controller issued the last time, we
>> can use simple query procedures, to make sure that SD/MMC card is in the
>> right state. The only query routine currently presented in SD/MMC card
>> emulation is sd_data_ready(), this patch adds sd_receive_ready() routine.
>
> I've thought about this a bit more and spent some time poking through
> the standards docs. I'm kind of suspicious of both this function and
> the existing sd_data_ready(). Real hardware doesn't have these signals,
> both ends of the sd-controller-to-card connection just do their thing
> and trust that the other end is in the state it is supposed to be.
> If you (in the controller model) don't already know whether you are
> OK to send/receive data then you haven't modelled some part of the
> controller's state properly.

Having thought even more :-), sd_data_ready() is actually modelling a
real hardware behaviour: "have we seen the start bit on the data line?".
So it is justifiable. I still don't think sd_receive_ready() is, though.

-- PMM
Mitsyanko Igor - Dec. 14, 2011, 8:29 a.m.
On 12/13/2011 08:22 PM, Peter Maydell wrote:
> On 13 December 2011 15:11, Peter Maydell<peter.maydell@linaro.org>  wrote:
>> On 12 December 2011 06:43, Evgeny Voevodin<e.voevodin@samsung.com>  wrote:
>>> From: Mitsyanko Igor<i.mitsyanko@samsung.com>
>>>
>>> Data transfer direction between host controller and SD/MMC card is selected by
>>> host controller configuration registers, but whether we actually need or need
>>> not perform data transfer depends on type of last issued command. To avoid
>>> memorization of which type of command host controller issued the last time, we
>>> can use simple query procedures, to make sure that SD/MMC card is in the
>>> right state. The only query routine currently presented in SD/MMC card
>>> emulation is sd_data_ready(), this patch adds sd_receive_ready() routine.
>>
>> I've thought about this a bit more and spent some time poking through
>> the standards docs. I'm kind of suspicious of both this function and
>> the existing sd_data_ready(). Real hardware doesn't have these signals,
>> both ends of the sd-controller-to-card connection just do their thing
>> and trust that the other end is in the state it is supposed to be.
>> If you (in the controller model) don't already know whether you are
>> OK to send/receive data then you haven't modelled some part of the
>> controller's state properly.
>
> Having thought even more :-), sd_data_ready() is actually modelling a
> real hardware behaviour: "have we seen the start bit on the data line?".
> So it is justifiable. I still don't think sd_receive_ready() is, though.
>
> -- PMM
>
>

I thought it's a good idea to protect user data from corruption because 
of inappropriate driver behaviour, even though it's obviously not a 
controller's concern. But after your comment about start bit it started 
to make sense to me that sd_data_ready() exists in current 
implementation, while other query functions don't :)

Patch

diff --git a/hw/sd.c b/hw/sd.c
index 10e26ad..b0a8557 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -1670,3 +1670,8 @@  void sd_enable(SDState *sd, int enable)
 {
     sd->enable = enable;
 }
+
+int sd_receive_ready(SDState *sd)
+{
+    return sd->state == sd_receivingdata_state;
+}
diff --git a/hw/sd.h b/hw/sd.h
index ac4b7c4..71ab781 100644
--- a/hw/sd.h
+++ b/hw/sd.h
@@ -75,5 +75,6 @@  uint8_t sd_read_data(SDState *sd);
 void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert);
 int sd_data_ready(SDState *sd);
 void sd_enable(SDState *sd, int enable);
+int sd_receive_ready(SDState *sd);
 
 #endif	/* __hw_sd_h */