[U-Boot] [PATCH v5] mmc: add generic mmc spi driver

Thomas Chou
2010-04-29T14:51:03Z

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