[PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53
Wolfram Sang
w.sang at pengutronix.de
Tue Mar 1 06:31:15 EST 2011
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/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110301/acca923e/attachment.sig>
More information about the linux-arm-kernel
mailing list