All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Chou <thomas@wytron.com.tw>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v5] mmc: add generic mmc spi driver
Date: Thu, 29 Apr 2010 22:51:03 +0800	[thread overview]
Message-ID: <4BD99CD7.6030801@wytron.com.tw> (raw)
In-Reply-To: <v2j2acbd3e41004280821w1b5f5186mfd148220dc694e2d@mail.gmail.com>

On 04/28/2010 11:21 PM, Andy Fleming wrote:
>
>> +static int do_mmc_spi(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>> +{
>> +       int dev_num = -1;
>> +       uint bus;
>> +       uint cs;
>> +       uint speed;
>> +       uint mode;
>> +       char *endp;
>> +       struct mmc *mmc;
>> +       struct mmc_spi_priv *priv;
>> +
>> +       do {
>> +               mmc = find_mmc_device(++dev_num);
>> +       } while (mmc&&  strcmp(mmc->name, "MMC_SPI"));
>> +       if (!mmc) {
>> +               printf("Create MMC Device\n");
>> +               mmc = mmc_spi_init(CONFIG_MMC_SPI_BUS,
>> +                                  CONFIG_MMC_SPI_CS,
>> +                                  CONFIG_MMC_SPI_SPEED,
>> +                                  CONFIG_MMC_SPI_MODE);
>> +               if (!mmc) {
>> +                       printf("Failed to create MMC Device\n");
>> +                       return 1;
>> +               }
>> +               dev_num = mmc->block_dev.dev;
>> +       }
>>      
>
> I'm not sure I understand the logic behind this code.  The arguments
> to the command should be used to either find the already-existing bus,
> or to create a new one.  Unless I'm misunderstanding, this searches
> for the first MMC_SPI bus, and if it finds it, uses that, otherwise it
> creates a new one with the values specified in the config file.  Then
> it parses the command and overwrites the old parameters for the bus
> with new ones?  Why?  My instinct would be to create a separate
> instance of an MMC_SPI bus for each bus and chip select.  My SPI is
> rusty, so maybe chip-select should be configurable on a use-by-use
> basis.
>
> Certainly the current code will only use at most one MMC_SPI bus even
> if more are created, which seems wrong.
>    
Hi Mike,

Could you please give me some suggestion on the mmc_spi subcommand?

1. In v5 patch, I assumed a single changeable mmc_spi device is enough.

2. Andy suggested to create a new mmc_spi device for each bus and cs.

Either way is fine to me. Which one do you prefer?

Best regards,
Thomas

  parent reply	other threads:[~2010-04-29 14:51 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-23  2:53 [U-Boot] [PATCH] mmc: add generic mmc spi driver Thomas Chou
2010-04-23  3:35 ` Mike Frysinger
2010-04-23  4:04   ` Thomas Chou
2010-04-23  5:55     ` Thomas Chou
2010-04-25  6:56       ` Mike Frysinger
2010-04-26 14:37         ` Thomas Chou
2010-04-26 15:59           ` Mike Frysinger
2010-04-25  6:51     ` Mike Frysinger
2010-04-27  1:51 ` [U-Boot] [PATCH v2] " Thomas Chou
2010-04-27  3:27 ` [U-Boot] [PATCH v3] " Thomas Chou
2010-04-27 16:49   ` Mike Frysinger
2010-04-28  3:14     ` Thomas Chou
2010-04-28  2:50 ` [U-Boot] [PATCH v4] " Thomas Chou
2010-04-28  6:00 ` [U-Boot] [PATCH v5] " Thomas Chou
2010-04-28 15:21   ` Andy Fleming
2010-04-29  5:52     ` Thomas Chou
2010-04-29 14:51     ` Thomas Chou [this message]
2010-04-29 19:07       ` Mike Frysinger
2010-04-30  0:16         ` Thomas Chou
2010-05-03  0:54 ` [U-Boot] [PATCH 0/3] mmc: add mmc_spi driver Thomas Chou
2010-05-03  0:54 ` [U-Boot] [PATCH 1/3] lib: add crc7 from Linux Thomas Chou
2010-05-03  0:54 ` [U-Boot] [PATCH 2/3] mmc: add find_mmc_device_quiet that doesnt print not found message Thomas Chou
2010-05-06 22:14   ` Wolfgang Denk
2010-05-07  0:19     ` Thomas Chou
2010-05-07  0:51     ` [U-Boot] [PATCH 2/3 v2] mmc: control message print in find_mmc_device Thomas Chou
2010-05-19  4:31       ` Thomas Chou
2010-05-07  0:51     ` [U-Boot] [PATCH 3/3 v7] mmc: add generic mmc spi driver Thomas Chou
2010-05-03  0:54 ` [U-Boot] [PATCH 3/3 v6] " Thomas Chou
2010-05-19  4:37 ` [U-Boot] [PATCH 3/3 v8] " Thomas Chou
2010-05-28 22:44   ` Thomas Chou
2010-07-05  7:40   ` Mike Frysinger
2010-07-05 14:22     ` Thomas Chou
2010-07-05 20:11       ` Mike Frysinger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BD99CD7.6030801@wytron.com.tw \
    --to=thomas@wytron.com.tw \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.