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

From: Maxim Levitsky
Date: Sat Nov 02 2013 - 19:29:14 EST


On Wed, 2013-10-30 at 15:54 +0800, rogerable@xxxxxxxxxxx wrote:
> From: Roger Tseng <rogerable@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> ---
> 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@xxxxxxxxx>


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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/