[PATCH] memstick: fix unreachable state in h_msb_read_page() in ms_block.c

[PATCH] memstick: fix unreachable state in h_msb_read_page() in ms_block.c

[rogerable]

From: Roger Tseng <rogerable@realtek.com>

In h_msb_read_page() in ms_block.c, flow never reaches case
MSB_RP_RECIVE_STATUS_REG. This causes error when MEMSTICK_INT_ERR is
encountered and status error bits are going to be examined, but the status will
never be copied back.

Fix it by transiting to MSB_RP_RECIVE_STATUS_REG right after
MSB_RP_SEND_READ_STATUS_REG.

Signed-off-by: Roger Tseng <rogerable@realtek.com>
---
 drivers/memstick/core/ms_block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index 08e7023..9188ef5 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -401,7 +401,7 @@ again:
 			sizeof(struct ms_status_register)))
 			return 0;
 
-		msb->state = MSB_RP_RECEIVE_OOB_READ;
+		msb->state = MSB_RP_RECIVE_STATUS_REG;
 		return 0;
 
 	case MSB_RP_RECIVE_STATUS_REG:
-- 
1.8.2

Re: [PATCH] memstick: fix unreachable state in h_msb_read_page() in ms_block.c

[Maxim Levitsky]

On Wed, 2013-10-30 at 15:54 +0800, rogerable@realtek.com wrote: 
> From: Roger Tseng <rogerable@realtek.com>
> 
> In h_msb_read_page() in ms_block.c, flow never reaches case
> MSB_RP_RECIVE_STATUS_REG. This causes error when MEMSTICK_INT_ERR is
> encountered and status error bits are going to be examined, but the status will
> never be copied back.
> 
> Fix it by transiting to MSB_RP_RECIVE_STATUS_REG right after
> MSB_RP_SEND_READ_STATUS_REG.
> 
> Signed-off-by: Roger Tseng <rogerable@realtek.com>
> ---
>  drivers/memstick/core/ms_block.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
> index 08e7023..9188ef5 100644
> --- a/drivers/memstick/core/ms_block.c
> +++ b/drivers/memstick/core/ms_block.c
> @@ -401,7 +401,7 @@ again:
>  			sizeof(struct ms_status_register)))
>  			return 0;
>  
> -		msb->state = MSB_RP_RECEIVE_OOB_READ;
> +		msb->state = MSB_RP_RECIVE_STATUS_REG;
>  		return 0;
>  
>  	case MSB_RP_RECIVE_STATUS_REG:

This is indeed a bug here. Probably added when I converted to use
#defines for states.

So,

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>


Btw, there is another  minor or major (pun inteded) bug out there in
this driver, which I still have no time to send patch for yet :-(

I accidentally didn't set the major and minor for the disk.
It happens to work as usually there is just one card reader out there,
and major is allocated dynamically.

The two lines in msb_init_disk,

msb->disk->major = major;
msb->disk->first_minor = disk_id << MS_BLOCK_PART_SHIFT;

Somehow fell through the cracks. I had this in older revision.

Feel free to add it, or maybe I find time to compile and test this and
send a patch.

You also welcome to write a driver for xD portion of the card reader
too.
I back then implemented all the generic support, and you only need to
expose an NAND driver (look at r852.c for my code).


-- 
Best regards,
        Maxim Levitsky