linux-mips
[Top] [All Lists]

Re: [PATCH 2/2] SPI: ralink: add Ralink SoC spi driver

To: John Crispin <blogic@openwrt.org>
Subject: Re: [PATCH 2/2] SPI: ralink: add Ralink SoC spi driver
From: Mark Brown <broonie@kernel.org>
Date: Tue, 13 Aug 2013 19:58:02 +0100
Cc: Gabor Juhos <juhosg@openwrt.org>, linux-spi@vger.kernel.org, linux-mips@linux-mips.org
In-reply-to: <520A7E67.1060604@openwrt.org>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-id: linux-mips <linux-mips.eddie.linux-mips.org>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-software: Ecartis version 1.0.0
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <1376074288-29302-1-git-send-email-blogic@openwrt.org> <1376074288-29302-2-git-send-email-blogic@openwrt.org> <20130811132642.GB6427@sirena.org.uk> <520A7E67.1060604@openwrt.org>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Aug 13, 2013 at 08:43:51PM +0200, John Crispin wrote:

> >There is presumably a maximum transfer size here from the FIFO that is
> >holding the data?

> The hardware is not running in DMA/IRQ mode and hence it can only
> read/write 1 byte at a time.

OK, then the code looks buggy since it does all the Tx then all the Rx
so a bidirectional transfer should fail.  I'd expect Tx and Rx to be
part of the same loop in this case.

> >Set min_speed_hz in the spi_master and the core will check this for you.

> it seems that min_speed is not handled by the core yet. I saw
> several drivers do minimum speed testing. I am leaving this code in
> the driver until there is a generic minimum speed check

Or add the check to the core...

> >clk_prepare_enable(), and it'd be nice to use runtime PM and enable the
> >clock only when doing transfers though that's not essential.

> The clock is free running and always running.

It's still nice to turn it off for power, and very cheap to implement.

Attachment: signature.asc
Description: Digital signature

<Prev in Thread] Current Thread [Next in Thread>