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
