SPI: BCM2835: clock divider can be a multiple of 2

Commit Message

Martin Sperl March 19, 2015, 9:01 a.m. UTC

From: Martin Sperl <[e-mail]>

The official documentation is wrong in this respect.
Has been tested empirically for dividers 2-1024

Signed-off-by: Martin Sperl <[e-mail]>
---
 drivers/spi/spi-bcm2835.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Stephen Warren March 20, 2015, 5:06 a.m. UTC | #1

On 03/19/2015 03:01 AM, [e-mail] wrote:
> From: Martin Sperl <[e-mail]>
> 
> The official documentation is wrong in this respect.
> Has been tested empirically for dividers 2-1024

Do you have a link to a confirmation of this from the RPi Foundation or
Broadcom, even a forum post or something? How does the RPI Foundation's
downstream kernel restrict the divider?
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Martin Sperl March 20, 2015, 7:04 a.m. UTC | #2

> On 20.03.2015, at 06:06, Stephen Warren <[e-mail]> wrote:
> 
> On 03/19/2015 03:01 AM, [e-mail] wrote:
>> From: Martin Sperl <[e-mail]>
>> 
>> The official documentation is wrong in this respect.
>> Has been tested empirically for dividers 2-1024
> 
> Do you have a link to a confirmation of this from the RPi Foundation or
> Broadcom, even a forum post or something? How does the RPI Foundation's
> downstream kernel restrict the divider?

Unfortunately there are no official errata by the foundation.

Here the argument by a foundation official (Gordon Hollingworth)
why they do not (want to) provide updates:
http://www.raspberrypi.org/forums/viewtopic.php?p=447724#p447724

That is in a slightly different context, but still it also applies to
errata in general.

The foundation spi-bcm2708 driver also relies on the power-of 2 rule
following to the letter the documentation they provide.

For some more infos of experience please look here:
https://github.com/notro/spi-bcm2708/wiki
http://www.raspberrypi.org/forums/viewtopic.php?f=44&t=43442
http://elinux.org/BCM2835_datasheet_errata#p156

Notro and I have been running an out of tree driver for a long time
and that never showed any issues with this multiple of 2.

Some of these kernels/drivers have been shared with lots of people
for the use with TFT displays or CAN controllers.

I have measured this empirically for every divider between 2 to 1024
by doing some transfers, and measuring them with a SALEAE logic analyzer.

See here: for the analysis:
https://github.com/msperl/spi-bcm2835/issues/8

You can also fetch the raw data and csv exports here:
https://github.com/msperl/spi-bcm2835/blob/patch_the_upstream/images/

Note that the descriptions of how to do polled, interrupt driven and 
DMA driven SPI transfers (page 158) is also not completely accurate.
It seems to show some "procedures", but these are not optimized and
some are inconsistent: e.g: this limitation of 16/12 byte transfers
while the register documentation says 16 words not bytes - which means 
64 bytes can enter the FIFO buffer (which is also the limit when
the HW signals that it is full via BCM2835_SPI_CS_TXD)

This will be one of the next patches for improvement of the driver.

Martin

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Noralf Trønnes March 20, 2015, 2:25 p.m. UTC | #3

Den 20.03.2015 08:04, skrev Martin Sperl:
>> On 20.03.2015, at 06:06, Stephen Warren <[e-mail]> wrote:
>>
>> On 03/19/2015 03:01 AM, [e-mail] wrote:
>>> From: Martin Sperl <[e-mail]>
>>>
>>> The official documentation is wrong in this respect.
>>> Has been tested empirically for dividers 2-1024
>> Do you have a link to a confirmation of this from the RPi Foundation or
>> Broadcom, even a forum post or something? How does the RPI Foundation's
>> downstream kernel restrict the divider?
> Unfortunately there are no official errata by the foundation.
>
> Here the argument by a foundation official (Gordon Hollingworth)
> why they do not (want to) provide updates:
> http://www.raspberrypi.org/forums/viewtopic.php?p=447724#p447724
>
> That is in a slightly different context, but still it also applies to
> errata in general.
>
> The foundation spi-bcm2708 driver also relies on the power-of 2 rule
> following to the letter the documentation they provide.
>
> For some more infos of experience please look here:
> https://github.com/notro/spi-bcm2708/wiki
> http://www.raspberrypi.org/forums/viewtopic.php?f=44&t=43442
> http://elinux.org/BCM2835_datasheet_errata#p156
>
> Notro and I have been running an out of tree driver for a long time
> and that never showed any issues with this multiple of 2.
>
> Some of these kernels/drivers have been shared with lots of people
> for the use with TFT displays or CAN controllers.
>
> I have measured this empirically for every divider between 2 to 1024
> by doing some transfers, and measuring them with a SALEAE logic analyzer.
>
> See here: for the analysis:
> https://github.com/msperl/spi-bcm2835/issues/8
>
> You can also fetch the raw data and csv exports here:
> https://github.com/msperl/spi-bcm2835/blob/patch_the_upstream/images/
>
> Note that the descriptions of how to do polled, interrupt driven and
> DMA driven SPI transfers (page 158) is also not completely accurate.
> It seems to show some "procedures", but these are not optimized and
> some are inconsistent: e.g: this limitation of 16/12 byte transfers
> while the register documentation says 16 words not bytes - which means
> 64 bytes can enter the FIFO buffer (which is also the limit when
> the HW signals that it is full via BCM2835_SPI_CS_TXD)
>
> This will be one of the next patches for improvement of the driver.

The out-of-tree github.com/notro/spi-bcm2708.c is used in numerous
installations since may 2013. I just heard of a 50,000 order for a
particular SPI TFT display.
This driver has no restriction on CDIV except for boundary checking:
         cdiv = DIV_ROUND_UP(bus_hz, hz);

Many of these displays also have a touch controller on the same bus.
AFAIK the most common default speeds used with displays are: 
16,24,32,48,64,80,128MHz
Default transmit buffer size is 4k. The touch controller is set to 2MHz.
I haven't heard any reports of this driver being used for reading large 
buffers.

To my knowledge there hasn't been any issues with lifting this CDIV 
restriction.


Regards,
Noralf Trønnes

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mark Brown March 23, 2015, 6:53 p.m. UTC | #4

On Thu, Mar 19, 2015 at 09:01:52AM +0000, [e-mail] wrote:
> From: Martin Sperl <[e-mail]>
> 
> The official documentation is wrong in this respect.
> Has been tested empirically for dividers 2-1024

Applied, thanks.

Patch

diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index e1dea40..779e3a8 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -192,8 +192,9 @@  static int bcm2835_spi_start_transfer(
 	if (spi_hz >= clk_hz / 2) {
 		cdiv = 2; /* clk_hz/2 is the fastest we can go */
 	} else if (spi_hz) {
-		/* CDIV must be a power of two */
-		cdiv = roundup_pow_of_two(DIV_ROUND_UP(clk_hz, spi_hz));
+		/* CDIV must be a multiple of two */
+		cdiv = DIV_ROUND_UP(clk_hz, spi_hz);
+		cdiv += (cdiv % 2);
 
 		if (cdiv >= 65536)
 			cdiv = 0; /* 0 is the slowest we can go */