From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wolfgang Wegner <wolfgang@leila.ping.de>
Date: Fri, 23 Apr 2010 10:20:34 +0200
Subject: [U-Boot] [PATCH] mmc: new legacy MMC/SPI driver
In-Reply-To: <1271993307-10361-1-git-send-email-vapier@gentoo.org>
References: <1271913394-12958-1-git-send-email-thomas@wytron.com.tw>
	<1271993307-10361-1-git-send-email-vapier@gentoo.org>
Message-ID: <20100423082034.GM20047@leila.ping.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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