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

> +     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

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.

> +     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.
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to