Hi Richard, > Thanks a lot for your review-comments firstly. :) You are welcome :) > > > +/* Abort type definition in the command register */ > > > +#define SDHCI_CMD_ABORTCMD 0xC0 > > > > Won't that belong into sd.h (unless I misunderstood your last mail)? > This is the bit definitions of the ABORTCMD CMD-TYPE on the bit6~7 of CMD register. > Here is the definition of the CMD register derived from SDHC spec. FYI. > D15 D14 D13 D08 D07 D06 D05 D04 D03 D02 D01 D00 > Rsvd Command Index Command Type Data Present Select Command Index Check Enable Command CRC Check Enable Rsvd Response Type Select Ack, I found that, too. Exactly because it is in the standard, I thought this should rather go into sd.h than sdhci-esdhc-imx.c. Would be a seperate patch, though. > > > > > +/* VENDOR SPEC register */ > > > +#define SDHCI_VENDOR_SPEC 0xC0 > > > + > > > +/* > > > + * The CMDTYPE of the CMD register(offset 0xE) should be set to > > > > Check spaces. > I used the <kernel_dir>/./scripts/checkpatch.pl script to check the patches, and didn't find that there are issues about the spaces. > Can you tell me what's kinds of spaces issue should be fixed? Space before opening brace -> "register (offset ..." > No, we can't keep it enabled all the time. > This bit should be set to '1'/clear to '0' at the begin/end of the transfer. > Unfortunately, We can't use it to fix CMD12 issue either, this bit is only used to fix SDIO Multi-BLK NO INT case. Ok, thanks for checking. > IC guy insist that the CMD12 case is not a bug refer to the SD HOST controller spec, the bit7-6 should be > Set to 11b when the abort CMD is issued. That's a flaw in the core then? Need to investigate that. > > Hmm, to me, just using cpu_is_mx53() is more readable than introducing > > another layer of flags/quirks. > Hi Wolfram: > I discussed it with Richard Zhao before sending out these V3 patches. > As we know that there is not only mx53 has this issue, maybe some following SOCs have this issue too. > So we make a decision that we introduce another flags/quirks to declare it for all those SOCs that required this > mechanism in the end. Seems I am outnumbered on this matter, so OK. I just got a bit afraid of that approach seeing it didn't scale very well with sdhci.c. > > > + > > > +static struct sdhci_ops sdhci_esdhc_ops; > > > + > > > > Move them to the front. But I did this already, so no worries :) I will > > ping Chris to merge my series, so we will have something better to > > develop on. > > > Thanks.:) He pulled the changes now, so please rebase your patches against mmc-next. There is already a write_le-function now, but this should be not too hard, hopefully. Keep in mind that you don't need to cast void*. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
Attachment:
signature.asc
Description: Digital signature
