[PATCH] sd: mmc: Fix SET_BLOCK_COUNT command argument

[PATCH] sd: mmc: Fix SET_BLOCK_COUNT command argument

[Cédric Le Goater]

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(-)

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:
-- 
2.31.1

Re: [PATCH] sd: mmc: Fix SET_BLOCK_COUNT command argument

[Bin Meng]

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

Re: [PATCH] sd: mmc: Fix SET_BLOCK_COUNT command argument

[Philippe Mathieu-Daudé]

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
> 

Re: [PATCH] sd: mmc: Fix SET_BLOCK_COUNT command argument

[Cédric Le Goater]

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.

Re: [PATCH] sd: mmc: Fix SET_BLOCK_COUNT command argument

[Cédric Le Goater]

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. 

Re: [PATCH] sd: mmc: Fix SET_BLOCK_COUNT command argument

[Bin Meng]

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

Re: [PATCH] sd: mmc: Fix SET_BLOCK_COUNT command argument

[Bin Meng]

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

Re: [PATCH] sd: mmc: Fix SET_BLOCK_COUNT command argument

[Cédric Le Goater]

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.  
 

Re: [PATCH] sd: mmc: Fix SET_BLOCK_COUNT command argument

[Philippe Mathieu-Daudé]

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.