drivers/periph/spi: add 4MHZ to available speeds #4889
Conversation
LudwigKnuepfer
added
Type: enhancement
|
I've added a default case in all switch/case that did not have one so that the code compiles. I did not try to guess what would be the correct way to handle the new speed, even where it seemed obvious. Let me know if you think that should be made differently ! |
OlegHahm
added
the
Community: Hack'n'ACK candidate
|
@dkm the change itself looks ok to me. But honestly I don't understand your motivation behind it. For the |
| @@ -65,6 +65,8 @@ int spi_init_master(spi_t dev, spi_conf_t conf, spi_speed_t speed) | |||
| case SPI_SPEED_1MHZ: | |||
| f_baud = 1000000; | |||
| break; | |||
| case SPI_SPEED_4MHZ: | |||
| return -1; | |||
There was a problem hiding this comment.
Why do you need this besides of the default -1 some lines below?
|
I also don't get the need behind this PR. What is the benefit of allowing 4MHz specifically and why is 5MHz not sufficient? When talking on the mailing list I was under the impression, that the missing speed value was quite different from the existing values - sorry for not clarifying this before hand! To make sure we are on the same page: the speed values for SPI are not meant to be dead on - but rather meant as 'in the range of'. So 5MHz might as well be implemented with 4MHz for a particular CPU. The reason to understand them as 'in the range of' is simply, that most MCUs support only a very rough set of pre-scalers to set the actual SPI clock... So introducing (very) fine grained values for SPI speed values is not really practicable for platform independent interface... |
|
To be more precise : I'm abusing the SPI for generating a bit sequence suitable for a 1 wire protocol. This protocol has some timing details, thus the need for a particular SPI speed. I can close this PR and maintain a private patch if that's not to be merged. |
|
I see... Let's try to find a way to work this out. Which CPU are you using? We should be able to find a way to let you select the SPI speed you need for your platform without having to go through the SPI interface I think. |
|
lm4f120: SPI drivers not yet in master, it's part of a PR I'm planning to push when riot's SPI driver has been refactored. |
|
@haukepetersen would you take this over? IMO it should be ok to add the speed value to the interface and acts as "don't care" if not available for a device driver. But I assume you have a stronger opinion :-) ? |
|
Yes, I do! :-) |
|
@dkm: so I take it the |
|
@haukepetersen : yes, you can write the desired speed in a register. I don't need a very precise 4Mhz as the LEDs accept a range of timings. I could do the maths to know more precisely the range of freq that would work with my code, but I'm not even sure that would really make sense. I really thought that would be trivial to get this merged. If that's too much a burden, I can really maintain my own SPI driver with extra speed. (FYI, this is the rationale behind my code https://productize.be/driving-ws2812-programmable-rgb-leds-using-hardware-spi/ ) |
|
Interesting LED thingy, I am still trying to get mine to run :-) Looking at the timing table, why don't you just clock the thing with 1MHz? The timings sure allow for this and it should still be fast enough to update a significant number of LEDs in a decent time?!
In general yes, but we are trying really hard to keep the core interfaces as clean as possible, so I try to understand first if there are hard reasons for certain changes of if there might be other ways to solve this... |
|
@dkm This is another relevant article along the same idea |
|
@haukepetersen sure I could drive it at 1MHz, but i've already some issues with interrupts (in order for the ws2812 to work correctly, there should be no pause in the tx sequence)... The higher, the better :). Same goes for using the UART, it could work, but I'm afraid the mcu will spend most of the time doing the tx. I would love using the DMA, but there is no API for that yet in RIOT :) |
yes, there is, it's the already existing one... All you need to do is to implement int using DMA and youre good :-) |
miri64
removed
the
CI: ready for build
|
Any progress? |
|
I would like to close this. @dkm: would you be ok with that on base of the discussion above? |
|
@haukepetersen sure, we can close it :) |
|
thanks! |
This follows a previous exchange on the mailing-list.
Allows for SPI to be driven at 4Mhz.