Hi Mike,
On 04/28/2010 12:49 AM, Mike Frysinger wrote:
> On Monday 26 April 2010 23:27:16 Thomas Chou wrote:
>
>> --- /dev/null
>> +++ b/common/cmd_mmc_spi.c
>> +struct mmc *mmc = NULL;
>>
> useless assignment since mmc gets set shortly after
>
OK.
>
>> +struct mmc_spi_priv *priv;
>> +
>> +if (argc< 2)
>> +goto usage;
>> +
>> +dev_num = simple_strtoul(argv[1],&endp, 0);
>> +if (*endp != 0)
>> +goto usage;
>> +mmc = find_mmc_device(dev_num);
>> +if (!mmc || strcmp(mmc->name, "MMC_SPI"))
>> +goto usage;
>>
> too many indents on the goto
>
The tabs were interfered by the '+' . It is correct when applied.
> also, this looks like it bails if no existing mmc spi device is found ? i
> thought the point of this command was to create new instances on the fly.
> this looks like it just reprograms the settings for an existing device.
>
>
OK. Now v4 patch creates a MMC dev on the fly.
>> +if (bus != priv->bus || cs != priv->cs ||
>> +speed != priv->speed || mode != priv->speed) {
>> +priv->bus = bus;
>> +priv->cs = cs;
>> +priv->speed = speed;
>> +priv->mode = mode;
>> +if (priv->slave) {
>> +free(priv->slave);
>> +priv->slave = NULL;
>> +}
>>
> you shouldnt be poking the spi internals like this. use the common spi funcs
> to free/alloc the slave device.
>
OK.
Best regards,
Thomas
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
