Jordan Crouse wrote:
> Add SD support to the AU1200 MMC driver. This can
> be added post 2.6.15, I'm just sending them out today so the various
> maintainers can get them queued up.
>
> Signed-off-by: Jordan Crouse <jordan.crouse@xxxxxxx>
> ---
>
> drivers/mmc/au1xmmc.c | 124 ++++++++++++++++++++++++++++---------------------
> 1 files changed, 71 insertions(+), 53 deletions(-)
>
Russell is still the maintainer of the MMC layer. :)
I still have some comments though.
> @@ -124,8 +132,8 @@ static inline void IRQ_OFF(struct au1xmm
> static inline void SEND_STOP(struct au1xmmc_host *host)
> {
>
> - /* We know the value of CONFIG2, so avoid a read we don't need */
> - u32 mask = SD_CONFIG2_EN;
> + /* Penalty box for Jordan - NEVER ASSUME! */
> + u32 mask = au_readl(HOST_CONFIG2(host));
>
> WARN_ON(host->status != HOST_S_DATA);
> host->status = HOST_S_STOP;
This comment will be terribly confusing to anyone reading your code.
> @@ -196,7 +207,11 @@ static int au1xmmc_send_command(struct a
>
> switch(cmd->flags) {
> case MMC_RSP_R1:
> - mmccmd |= SD_CMD_RT_1;
> + if (cmd->opcode == 0x03 && host->mmc->mode == MMC_MODE_SD)
> + mmccmd |= SD_CMD_RT_6;
> + else
> + mmccmd |= SD_CMD_RT_1;
> +
> break;
> case MMC_RSP_R1B:
> mmccmd |= SD_CMD_RT_1B;
No, no, no! Even if this wasn't already fixed in the current kernel you
never hack around bugs in other parts of the kernel, you fix them!
> @@ -504,8 +519,8 @@ static void au1xmmc_cmd_complete(struct
> r[3] = au_readl(host->iobase + SD_RESP0);
>
> /* The CRC is omitted from the response, so really we only got
> - * 120 bytes, but the engine expects 128 bits, so we have to shift
> - * things up
> + * 120 bytes, but the engine expects 128 bits, so we have to
> + * shift things up
> */
>
> for(i = 0; i < 4; i++) {
s/bytes/bits/
> @@ -611,7 +635,7 @@ au1xmmc_prepare_data(struct au1xmmc_host
>
> int len = (datalen > sg_len) ? sg_len : datalen;
>
> - if (i == host->dma.len - 1)
> + if (i == (host->dma.len - 1))
> flags = DDMA_FLAGS_IE;
>
> if (host->flags & HOST_F_XMIT){
broken indentation.
> @@ -627,23 +651,11 @@ au1xmmc_prepare_data(struct au1xmmc_host
> len, flags);
> }
>
> - if (!ret)
> + if (ret == 0)
> goto dataerr;
>
> datalen -= len;
> }
> - }
> - else {
> - host->pio.index = 0;
> - host->pio.offset = 0;
> - host->pio.len = datalen;
> -
> - if (host->flags & HOST_F_XMIT)
> - IRQ_ON(host, SD_CONFIG_TH);
> - else
> - IRQ_ON(host, SD_CONFIG_NE);
> - //IRQ_ON(host, SD_CONFIG_RA|SD_CONFIG_RF);
> - }
>
> return MMC_ERR_NONE;
>
Here aswell. And it seems the block above will get the wrong indentation.
Rgds
Pierre
