[U-Boot] [PATCH] mmc: new legacy MMC/SPI driver

[Wolfgang Wegner]

Hi Mike,

On Thu, Apr 22, 2010 at 11:28:27PM -0400, Mike Frysinger wrote:

> +static short mmc_spi_init_card(struct mmc_spi_dev *pdev)
> +{
> +	unsigned short cntr = 0;
> +
> +	/* for making init process beeing silent */
> +	init_mode = 1;
> +	/* save length of log for external usage */
> +	pdev->log_len = LOG_LEN;
> +
> +	/* 10 bytes(80 cycles) with CS de-asserted */
> +	mmc_spi_dummy_clocks(pdev, 10);
> +	pdev->doassert();
> +	if (send_cmd_and_wait(pdev, GO_IDLE_STATE, 0, R1_IDLE_STATE, MMC_INIT_TIMEOUT))
> +		return 1;
> +	pdev->deassert();
> +	/* Send One Byte Delay */
> +	if (pdev->write(Null_Word, 1, pdev->priv_data) < 0)
> +		return 1;
> +	pdev->doassert();
[...]

Is there any reason to use doassert() and deassert() except to generate
the "dummy" cycles needed to initialize the card? Which driver does
_need_ an explicit cs_activate() and cs_deactivate() outside the
write() and read() functions?

CS control is done automatically by most drivers. According to
include/spi.h, the _board_code_ is supposed to supply spi_cs_[de]activate()
to be used by the driver in case the hardware can not control the chip
selects automatically. The documentation is not clear about it, but
from my point of view spi_cs_[de]activate() are not part of the
SPI API.

I think we should not clutter client drivers with such implementation
specific things.

I have to clean up my code and try to migrate to your newer code base,
but I did implement a modified version using SPI_XFER_BEGIN,
spi_setup_slave() (to clear the CS) and SPI_CS_HIGH to achieve the
CS switching in a way compliant with e.g. the coldfire SPI driver.
(Although IIRC I had to add SPI_CS_HIGH there, too)

Regards,
Wolfgang