Patchwork [U-Boot] AT91:mmc:fix multiple read/write error

login
register
mail settings
Submitter elen.song
Date July 12, 2011, 10:17 a.m.
Message ID <1310465827-9599-1-git-send-email-elen.song@atmel.com>
Download mbox | patch
Permalink /patch/104336/
State Accepted
Commit c310fc840472a36e4b9d2505830e9dc8d458d63c
Delegated to: Andy Fleming
Headers show

Comments

elen.song - July 12, 2011, 10:17 a.m.
According to datasheet,set block count before multiple read/write.

Signed-off-by: elen.song <elen.song@atmel.com>
---
 drivers/mmc/atmel_mci.h     |    9 ++++++++-
 drivers/mmc/gen_atmel_mci.c |    4 ++++
 2 files changed, 12 insertions(+), 1 deletions(-)
Reinhard Meyer - Aug. 2, 2011, 6:52 a.m.
Dear Andy, dear Elen,
> According to datasheet,set block count before multiple read/write.
> 
> Signed-off-by: elen.song <elen.song@atmel.com>
> ---
>  drivers/mmc/atmel_mci.h     |    9 ++++++++-
>  drivers/mmc/gen_atmel_mci.c |    4 ++++
>  2 files changed, 12 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/atmel_mci.h b/drivers/mmc/atmel_mci.h
> index 90ab6a8..3095d22 100644
> --- a/drivers/mmc/atmel_mci.h
> +++ b/drivers/mmc/atmel_mci.h
> @@ -36,7 +36,7 @@ typedef struct atmel_mci {
>  	u32	sdcr;	/* 0x0c */
>  	u32	argr;	/* 0x10 */
>  	u32	cmdr;	/* 0x14 */
> -	u32	_18;	/* 0x18 */
> +	u32	blkr;	/* 0x18 */
>  	u32	_1c;	/* 0x1c */
>  	u32	rspr;	/* 0x20 */
>  	u32	rspr1;	/* 0x24 */
> @@ -67,6 +67,7 @@ typedef struct atmel_mci {
>  #define MMCI_SDCR				0x000c
>  #define MMCI_ARGR				0x0010
>  #define MMCI_CMDR				0x0014
> +#define MMCI_BLKR				0x0018
>  #define MMCI_RSPR				0x0020
>  #define MMCI_RSPR1				0x0024
>  #define MMCI_RSPR2				0x0028
> @@ -140,6 +141,12 @@ typedef struct atmel_mci {
>  #define MMCI_TRTYP_OFFSET			19
>  #define MMCI_TRTYP_SIZE				2
>  
> +/* Bitfields in BLKR */
> +#define MMCI_BCNT_OFFSET			0
> +#define MMCI_BCNT_SIZE				16
> +#define MMCI_BLKLEN_OFFSET			16
> +#define MMCI_BLKLEN_SIZE			16
> +
>  /* Bitfields in RSPRx */
>  #define MMCI_RSP_OFFSET				0
>  #define MMCI_RSP_SIZE				32
> diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
> index f346b24..d217574 100644
> --- a/drivers/mmc/gen_atmel_mci.c
> +++ b/drivers/mmc/gen_atmel_mci.c
> @@ -183,6 +183,10 @@ mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>  	/* Figure out the transfer arguments */
>  	cmdr = mci_encode_cmd(cmd, data, &error_flags);
>  
> +	if (data)
> +		writel(MMCI_BF(BCNT, data->blocks) |
> +			MMCI_BF(BLKLEN,	mmc->read_bl_len), &mci->blkr);
> +
>  	/* Send the command */
>  	writel(cmd->cmdarg, &mci->argr);
>  	writel(cmdr, &mci->cmdr);

I find it highly irregular that this patch has made it into mainline!

1. We have had a discussion about that I think the approach in this
patch is wrong. My concerns have not been addressed yet.

2. This is clearly an ATMEL domain/custodian patch.

3. I was not put on CC by Elen.

4. Andy snapped it away and applied it without sending an "Applied to"
message.


Best Regards,
Reinhard
Andy Fleming - Aug. 2, 2011, 2:05 p.m.
> I find it highly irregular that this patch has made it into mainline!
>
> 1. We have had a discussion about that I think the approach in this
> patch is wrong. My concerns have not been addressed yet.


I have seen no such discussion. I saw no comments on this patch, so I
applied it. In an idea world, I would read all of the u-boot list
messages, but in reality I filter heavily, and often only look for
messages in the same thread as the patch when deciding whether to
apply (it was also a fairly simple-seeming patch).


>
> 2. This is clearly an ATMEL domain/custodian patch.


It's also clearly an MMC domain/custodian patch, as the changes rest
solely within the MMC directory.


>
> 3. I was not put on CC by Elen.


Elen should have put you on CC, I agree.


>
> 4. Andy snapped it away and applied it without sending an "Applied to"
> message.

I should have sent the message, but it wouldn't have affected things,
much. The gap between application and pull request tends to be fairly
short. Could you point me to the discussion, so I can see your
objections? I'd be happy to revert the patch.

Patch

diff --git a/drivers/mmc/atmel_mci.h b/drivers/mmc/atmel_mci.h
index 90ab6a8..3095d22 100644
--- a/drivers/mmc/atmel_mci.h
+++ b/drivers/mmc/atmel_mci.h
@@ -36,7 +36,7 @@  typedef struct atmel_mci {
 	u32	sdcr;	/* 0x0c */
 	u32	argr;	/* 0x10 */
 	u32	cmdr;	/* 0x14 */
-	u32	_18;	/* 0x18 */
+	u32	blkr;	/* 0x18 */
 	u32	_1c;	/* 0x1c */
 	u32	rspr;	/* 0x20 */
 	u32	rspr1;	/* 0x24 */
@@ -67,6 +67,7 @@  typedef struct atmel_mci {
 #define MMCI_SDCR				0x000c
 #define MMCI_ARGR				0x0010
 #define MMCI_CMDR				0x0014
+#define MMCI_BLKR				0x0018
 #define MMCI_RSPR				0x0020
 #define MMCI_RSPR1				0x0024
 #define MMCI_RSPR2				0x0028
@@ -140,6 +141,12 @@  typedef struct atmel_mci {
 #define MMCI_TRTYP_OFFSET			19
 #define MMCI_TRTYP_SIZE				2
 
+/* Bitfields in BLKR */
+#define MMCI_BCNT_OFFSET			0
+#define MMCI_BCNT_SIZE				16
+#define MMCI_BLKLEN_OFFSET			16
+#define MMCI_BLKLEN_SIZE			16
+
 /* Bitfields in RSPRx */
 #define MMCI_RSP_OFFSET				0
 #define MMCI_RSP_SIZE				32
diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c
index f346b24..d217574 100644
--- a/drivers/mmc/gen_atmel_mci.c
+++ b/drivers/mmc/gen_atmel_mci.c
@@ -183,6 +183,10 @@  mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 	/* Figure out the transfer arguments */
 	cmdr = mci_encode_cmd(cmd, data, &error_flags);
 
+	if (data)
+		writel(MMCI_BF(BCNT, data->blocks) |
+			MMCI_BF(BLKLEN,	mmc->read_bl_len), &mci->blkr);
+
 	/* Send the command */
 	writel(cmd->cmdarg, &mci->argr);
 	writel(cmdr, &mci->cmdr);