Re: [PATCH 2/2] mmc: Test bus-width for old MMC devices

[Takashi Iwai]

At Tue, 14 Dec 2010 23:03:42 -0500,
zhangfei gao wrote:
> 
> On Wed, Nov 24, 2010 at 1:21 AM, Takashi Iwai <tiwai@xxxxxxx> wrote:
> > From: Aries Lee <arieslee@xxxxxxxxxxx>
> >
> > Some old MMC devices fail with the 4/8 bits the driver tries to use
> > exclusively. ÂThis patch adds a test for the given bus setup and falls
> > back to the lower bit mode (until 1-bit mode) when the test fails.
> >
> > [Major rework and refactoring by tiwai]
> >
> > Signed-off-by: Aries Lee <arieslee@xxxxxxxxxxx>
> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> > ---
> > Âdrivers/mmc/core/mmc.c   |  49 ++++++++++++---------
> > Âdrivers/mmc/core/mmc_ops.c | Â102 ++++++++++++++++++++++++++++++++++++++++++++
> > Âdrivers/mmc/core/mmc_ops.h | Â Â1 +
> > Â3 files changed, 131 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> > index e81e6fe..5d8b4b2 100644
> > --- a/drivers/mmc/core/mmc.c
> > +++ b/drivers/mmc/core/mmc.c
> > @@ -507,29 +507,36 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> > Â Â Â Â */
> > Â Â Â Âif ((card->csd.mmca_vsn >= CSD_SPEC_VER_4) &&
> > Â Â Â Â Â Â(host->caps & (MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA))) {
> > - Â Â Â Â Â Â Â unsigned ext_csd_bit, bus_width;
> > -
> > - Â Â Â Â Â Â Â if (host->caps & MMC_CAP_8_BIT_DATA) {
> > - Â Â Â Â Â Â Â Â Â Â Â ext_csd_bit = EXT_CSD_BUS_WIDTH_8;
> > - Â Â Â Â Â Â Â Â Â Â Â bus_width = MMC_BUS_WIDTH_8;
> > - Â Â Â Â Â Â Â } else {
> > - Â Â Â Â Â Â Â Â Â Â Â ext_csd_bit = EXT_CSD_BUS_WIDTH_4;
> > - Â Â Â Â Â Â Â Â Â Â Â bus_width = MMC_BUS_WIDTH_4;
> > + Â Â Â Â Â Â Â static unsigned ext_csd_bits[] = {
> > + Â Â Â Â Â Â Â Â Â Â Â EXT_CSD_BUS_WIDTH_8,
> > + Â Â Â Â Â Â Â Â Â Â Â EXT_CSD_BUS_WIDTH_4,
> > + Â Â Â Â Â Â Â Â Â Â Â EXT_CSD_BUS_WIDTH_1
> > + Â Â Â Â Â Â Â };
> > + Â Â Â Â Â Â Â static unsigned bus_widths[] = {
> > + Â Â Â Â Â Â Â Â Â Â Â MMC_BUS_WIDTH_8,
> > + Â Â Â Â Â Â Â Â Â Â Â MMC_BUS_WIDTH_4,
> > + Â Â Â Â Â Â Â Â Â Â Â MMC_BUS_WIDTH_1
> > + Â Â Â Â Â Â Â };
> > + Â Â Â Â Â Â Â unsigned idx;
> > +
> > + Â Â Â Â Â Â Â if (host->caps & MMC_CAP_8_BIT_DATA)
> > + Â Â Â Â Â Â Â Â Â Â Â idx = 0;
> > + Â Â Â Â Â Â Â else
> > + Â Â Â Â Â Â Â Â Â Â Â idx = 1;
> > + Â Â Â Â Â Â Â for (; idx < ARRAY_SIZE(bus_widths); idx++) {
> > + Â Â Â Â Â Â Â Â Â Â Â err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ÂEXT_CSD_BUS_WIDTH, ext_csd_bits[idx]);
> > + Â Â Â Â Â Â Â Â Â Â Â if (!err) {
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mmc_set_bus_width(card->host, bus_widths[idx]);
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â err = mmc_bus_test(card, bus_widths[idx]);
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (!err)
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
> > + Â Â Â Â Â Â Â Â Â Â Â }
> > Â Â Â Â Â Â Â Â}
> > -
> > - Â Â Â Â Â Â Â err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> > - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ÂEXT_CSD_BUS_WIDTH, ext_csd_bit);
> > -
> > - Â Â Â Â Â Â Â if (err && err != -EBADMSG)
> > - Â Â Â Â Â Â Â Â Â Â Â goto free_card;
> > -
> > Â Â Â Â Â Â Â Âif (err) {
> > - Â Â Â Â Â Â Â Â Â Â Â printk(KERN_WARNING "%s: switch to bus width %d "
> > - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â"failed\n", mmc_hostname(card->host),
> > - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â1 << bus_width);
> > - Â Â Â Â Â Â Â Â Â Â Â err = 0;
> > - Â Â Â Â Â Â Â } else {
> > - Â Â Â Â Â Â Â Â Â Â Â mmc_set_bus_width(card->host, bus_width);
> > + Â Â Â Â Â Â Â Â Â Â Â printk(KERN_WARNING "%s: switch to bus width failed\n",
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âmmc_hostname(card->host));
> > + Â Â Â Â Â Â Â Â Â Â Â goto free_card;
> > Â Â Â Â Â Â Â Â}
> > Â Â Â Â}
> >
> > diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> > index 326447c..f8f47f9 100644
> > --- a/drivers/mmc/core/mmc_ops.c
> > +++ b/drivers/mmc/core/mmc_ops.c
> > @@ -462,3 +462,105 @@ int mmc_send_status(struct mmc_card *card, u32 *status)
> > Â Â Â Âreturn 0;
> > Â}
> >
> > +#define MMC_CMD_BUS_TEST_W Â Â Â Â Â Â 19
> > +#define MMC_CMD_BUS_TEST_R Â Â Â Â Â Â 14
> > +
> > +static int
> > +mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
> > + Â Â Â Â Â Â Â Â u8 len)
> > +{
> > + Â Â Â struct mmc_request mrq;
> > + Â Â Â struct mmc_command cmd;
> > + Â Â Â struct mmc_data data;
> > + Â Â Â struct scatterlist sg;
> > + Â Â Â u8 *data_buf;
> > + Â Â Â u8 *test_buf;
> > + Â Â Â int i, err;
> > + Â Â Â static u8 testdata_8bit[8] = { 0x55, 0xaa, 0, 0, 0, 0, 0, 0 };
> > + Â Â Â static u8 testdata_4bit[4] = { 0x5a, 0, 0, 0 };
> > +
> > + Â Â Â /* dma onto stack is unsafe/nonportable, but callers to this
> > + Â Â Â Â* routine normally provide temporary on-stack buffers ...
> > + Â Â Â Â*/
> > + Â Â Â data_buf = kmalloc(len, GFP_KERNEL);
> > + Â Â Â if (!data_buf)
> > + Â Â Â Â Â Â Â return -ENOMEM;
> > +
> > + Â Â Â if (len == 8)
> > + Â Â Â Â Â Â Â test_buf = testdata_8bit;
> > + Â Â Â else if (len == 4)
> > + Â Â Â Â Â Â Â test_buf = testdata_4bit;
> > + Â Â Â else {
> > + Â Â Â Â Â Â Â printk(KERN_ERR "%s: Invaild bus_width %d\n",
> > + Â Â Â Â Â Â Â Â Â Â Âmmc_hostname(host), len);
> > + Â Â Â Â Â Â Â return -EINVAL;
> > + Â Â Â }
> > +
> > + Â Â Â if (opcode == MMC_CMD_BUS_TEST_W)
> > + Â Â Â Â Â Â Â memcpy(data_buf, test_buf, len);
> > +
> > + Â Â Â memset(&mrq, 0, sizeof(struct mmc_request));
> > + Â Â Â memset(&cmd, 0, sizeof(struct mmc_command));
> > + Â Â Â memset(&data, 0, sizeof(struct mmc_data));
> > +
> > + Â Â Â mrq.cmd = &cmd;
> > + Â Â Â mrq.data = &data;
> > + Â Â Â cmd.opcode = opcode;
> > + Â Â Â cmd.arg = 0;
> > +
> > + Â Â Â /* NOTE HACK: Âthe MMC_RSP_SPI_R1 is always correct here, but we
> > + Â Â Â Â* rely on callers to never use this with "native" calls for reading
> > + Â Â Â Â* CSD or CID. ÂNative versions of those commands use the R2 type,
> > + Â Â Â Â* not R1 plus a data block.
> > + Â Â Â Â*/
> > + Â Â Â cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> > +
> > + Â Â Â data.blksz = len;
> > + Â Â Â data.blocks = 1;
> > + Â Â Â if (opcode == MMC_CMD_BUS_TEST_R)
> > + Â Â Â Â Â Â Â data.flags = MMC_DATA_READ;
> > + Â Â Â else
> > + Â Â Â Â Â Â Â data.flags = MMC_DATA_WRITE;
> > +
> > + Â Â Â data.sg = &sg;
> > + Â Â Â data.sg_len = 1;
> > + Â Â Â sg_init_one(&sg, data_buf, len);
> > + Â Â Â mmc_wait_for_req(host, &mrq);
> > + Â Â Â err = 0;
> > + Â Â Â if (opcode == MMC_CMD_BUS_TEST_R) {
> > + Â Â Â Â Â Â Â for (i = 0; i < len / 4; i++)
> > + Â Â Â Â Â Â Â Â Â Â Â if ((test_buf[i] ^ data_buf[i]) != 0xff) {
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â err = -EIO;
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
> > + Â Â Â Â Â Â Â Â Â Â Â }
> > + Â Â Â }
> > + Â Â Â kfree(data_buf);
> > +
> > + Â Â Â if (cmd.error)
> > + Â Â Â Â Â Â Â return cmd.error;
> > + Â Â Â if (data.error)
> 
> 	if (data.error && (data.error != -EILSEQ))
> Could you add code here to ignore CRC error of CMD14.
> According to spec, CRC bits from card are optional in CMD14, and it is
> ignored by host.
> However some host still check and may get crc error here if card does not send.

Philip corrected my patch and added such a check (in sdhci.c side).
I'm going to resend the revised patch soon.


thanks,

Takashi
--
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/