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

2021-06-23 Thread 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é  
> wrote:
>>
>> On 6/23/21 10:39 AM, Bin Meng wrote:
>>> On Wed, Jun 23, 2021 at 4:30 PM Cédric Le Goater  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.



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

2021-06-23 Thread 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  wrote:
>>
>> On 6/23/21 10:39 AM, Bin Meng wrote:
>>> On Wed, Jun 23, 2021 at 4:30 PM Cédric Le Goater  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

2021-06-23 Thread Bin Meng
On Wed, Jun 23, 2021 at 4:55 PM Cédric Le Goater  wrote:
>
> On 6/23/21 10:39 AM, Bin Meng wrote:
> > On Wed, Jun 23, 2021 at 4:30 PM Cédric Le Goater  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

2021-06-23 Thread Bin Meng
On Wed, Jun 23, 2021 at 4:52 PM 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  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

2021-06-23 Thread 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  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

2021-06-23 Thread 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  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

2021-06-23 Thread 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  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 
>> ---
>>  hw/sd/sd.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
> 
> Regards,
> Bin
> 




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

2021-06-23 Thread Bin Meng
On Wed, Jun 23, 2021 at 4:30 PM Cédric Le Goater  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 
> ---
>  hw/sd/sd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Regards,
Bin



[PATCH] sd: mmc: Fix SET_BLOCK_COUNT command argument

2021-06-23 Thread Cédric Le Goater
The number of blocks is defined in the lower bits [15:0]

Signed-off-by: Cédric Le Goater 
---
 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 & 0x;
 return sd_r1;
 
 default:
-- 
2.31.1