diff mbox series

sd: mmc: Fix SET_BLOCK_COUNT command argument

Message ID 20210623083004.245894-1-clg@kaod.org
State New
Headers show
Series sd: mmc: Fix SET_BLOCK_COUNT command argument | expand

Commit Message

Cédric Le Goater June 23, 2021, 8:30 a.m. UTC
The number of blocks is defined in the lower bits [15:0]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Bin Meng June 23, 2021, 8:39 a.m. UTC | #1
On Wed, Jun 23, 2021 at 4:30 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> The number of blocks is defined in the lower bits [15:0]

I checked the physical spec v8.00 and it says bits [31:0] for CMD23 argument.

>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/sd/sd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Regards,
Bin
Philippe Mathieu-Daudé June 23, 2021, 8:52 a.m. UTC | #2
On 6/23/21 10:39 AM, Bin Meng wrote:
> On Wed, Jun 23, 2021 at 4:30 PM Cédric Le Goater <clg@kaod.org> wrote:
>>
>> The number of blocks is defined in the lower bits [15:0]
> 
> I checked the physical spec v8.00 and it says bits [31:0] for CMD23 argument.

Watch out, we only support 1-3:

enum SDPhySpecificationVersion {
    SD_PHY_SPECv1_10_VERS     = 1,
    SD_PHY_SPECv2_00_VERS     = 2,
    SD_PHY_SPECv3_01_VERS     = 3,
};

> 
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  hw/sd/sd.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
> 
> Regards,
> Bin
>
Cédric Le Goater June 23, 2021, 8:55 a.m. UTC | #3
On 6/23/21 10:39 AM, Bin Meng wrote:
> On Wed, Jun 23, 2021 at 4:30 PM Cédric Le Goater <clg@kaod.org> wrote:
>>
>> The number of blocks is defined in the lower bits [15:0]
> 
> I checked the physical spec v8.00 and it says bits [31:0] for CMD23 argument.

May be that's an eMMC thing. That's what I read from the specs :

 [31] Reliable Write Request
 [30:16] set to 0
 [15:0] number of blocks

Thanks,

C.
Cédric Le Goater June 23, 2021, 8:57 a.m. UTC | #4
On 6/23/21 10:52 AM, Philippe Mathieu-Daudé wrote:
> On 6/23/21 10:39 AM, Bin Meng wrote:
>> On Wed, Jun 23, 2021 at 4:30 PM Cédric Le Goater <clg@kaod.org> wrote:
>>>
>>> The number of blocks is defined in the lower bits [15:0]
>>
>> I checked the physical spec v8.00 and it says bits [31:0] for CMD23 argument.
> 
> Watch out, we only support 1-3:
> 
> enum SDPhySpecificationVersion {
>     SD_PHY_SPECv1_10_VERS     = 1,
>     SD_PHY_SPECv2_00_VERS     = 2,
>     SD_PHY_SPECv3_01_VERS     = 3,
> };
> 

Yes. block count was increased to 32-bit in v4 if I am correct. 

Any how, it is a bit more complex than the patch I sent which is fixing 
an issue I saw with eMMC.

Thanks,

C.
Bin Meng June 23, 2021, 9:11 a.m. UTC | #5
On Wed, Jun 23, 2021 at 4:52 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 6/23/21 10:39 AM, Bin Meng wrote:
> > On Wed, Jun 23, 2021 at 4:30 PM Cédric Le Goater <clg@kaod.org> wrote:
> >>
> >> The number of blocks is defined in the lower bits [15:0]
> >
> > I checked the physical spec v8.00 and it says bits [31:0] for CMD23 argument.
>
> Watch out, we only support 1-3:
>

Yes

> enum SDPhySpecificationVersion {
>     SD_PHY_SPECv1_10_VERS     = 1,
>     SD_PHY_SPECv2_00_VERS     = 2,
>     SD_PHY_SPECv3_01_VERS     = 3,
> };
>

However the physical sepc v8.00 should document any difference between
ver 3.0 and ver 8.0 if there are indeed any, but for CMD23 it does
not. So it means it's 32-bit since day 1.

To double check, I just downloaded the spec 3.01 and confirmed it's
still 32-bit.

Regards,
Bin
Bin Meng June 23, 2021, 9:12 a.m. UTC | #6
On Wed, Jun 23, 2021 at 4:55 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> On 6/23/21 10:39 AM, Bin Meng wrote:
> > On Wed, Jun 23, 2021 at 4:30 PM Cédric Le Goater <clg@kaod.org> wrote:
> >>
> >> The number of blocks is defined in the lower bits [15:0]
> >
> > I checked the physical spec v8.00 and it says bits [31:0] for CMD23 argument.
>
> May be that's an eMMC thing. That's what I read from the specs :
>
>  [31] Reliable Write Request
>  [30:16] set to 0
>  [15:0] number of blocks

I don't think we ever claimed eMMC support in QEMU. Did we?

Regards,
Bin
Cédric Le Goater June 23, 2021, 9:16 a.m. UTC | #7
On 6/23/21 11:12 AM, Bin Meng wrote:
> On Wed, Jun 23, 2021 at 4:55 PM Cédric Le Goater <clg@kaod.org> wrote:
>>
>> On 6/23/21 10:39 AM, Bin Meng wrote:
>>> On Wed, Jun 23, 2021 at 4:30 PM Cédric Le Goater <clg@kaod.org> wrote:
>>>>
>>>> The number of blocks is defined in the lower bits [15:0]
>>>
>>> I checked the physical spec v8.00 and it says bits [31:0] for CMD23 argument.
>>
>> May be that's an eMMC thing. That's what I read from the specs :
>>
>>  [31] Reliable Write Request
>>  [30:16] set to 0
>>  [15:0] number of blocks
> 
> I don't think we ever claimed eMMC support in QEMU. Did we?

Is that a question ? 

C.
Philippe Mathieu-Daudé June 23, 2021, 11:01 a.m. UTC | #8
On 6/23/21 11:11 AM, Bin Meng wrote:
> On Wed, Jun 23, 2021 at 4:52 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> On 6/23/21 10:39 AM, Bin Meng wrote:
>>> On Wed, Jun 23, 2021 at 4:30 PM Cédric Le Goater <clg@kaod.org> wrote:
>>>>
>>>> The number of blocks is defined in the lower bits [15:0]
>>>
>>> I checked the physical spec v8.00 and it says bits [31:0] for CMD23 argument.
>>
>> Watch out, we only support 1-3:
>>
> 
> Yes
> 
>> enum SDPhySpecificationVersion {
>>     SD_PHY_SPECv1_10_VERS     = 1,
>>     SD_PHY_SPECv2_00_VERS     = 2,
>>     SD_PHY_SPECv3_01_VERS     = 3,
>> };
>>
> 
> However the physical sepc v8.00 should document any difference between
> ver 3.0 and ver 8.0 if there are indeed any, but for CMD23 it does
> not. So it means it's 32-bit since day 1.
> 
> To double check, I just downloaded the spec 3.01 and confirmed it's
> still 32-bit.

OK, so patch is incorrect then.

Thanks,

Phil.
diff mbox series

Patch

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a73d80661a10..a2553a502edc 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1358,7 +1358,7 @@  static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         switch (sd->state) {
         case sd_transfer_state:
-            sd->multi_blk_cnt = req.arg;
+            sd->multi_blk_cnt = req.arg & 0xFFFF;
             return sd_r1;
 
         default: