Update SPI hal API #6782

ithinuel · 2018-05-01T11:35:52Z

Description

This PR bring a new and well defined SPI api at HAL level.

This disables SPI for all targets as tests needs to be added & all low-level drivers needs to be re-qualified against these tests.

Pull request type

[ ] Fix
[ ] Refactor
[ ] New target
[x] Feature
[ ] Breaking change

0xc0170 · 2018-05-02T10:16:12Z

@ithinuel Who should review this new specs?

@c1728p9 and @bulislaw or anyone else involved?

bulislaw

Thanks Wilfried that looks awesome!

bulislaw · 2018-05-02T11:39:31Z

hal/spi_api.h

 #if DEVICE_SPI
+/**
+ * \defgroup hal_new_spi Serial peripheral interface HAL API.


hal_new_spi -> hal_spi

bulislaw · 2018-05-02T11:44:35Z

hal/spi_api.h

-#else
-/** Non-asynch SPI HAL structure
+/**
+ * SPI object.


We'll need to make a note (highlight) of that in the PG.

bulislaw · 2018-05-02T11:46:25Z

hal/spi_api.h

 */
 typedef struct spi_s spi_t;

-#endif
+/**
+ * This structure groups all initialization parameters required by an SPI interface.


a SPI I think

bulislaw · 2018-05-02T11:47:38Z

hal/spi_api.h

+ * This structure groups all initialization parameters required by an SPI interface.
+ */
+typedef struct spi_init_t {
+    bool    is_master;  /**< True to configure the device in Master mode */


We need detailed description of all the fields, what happens when I set is_master to false? Will it be slave? what will happen if I give SS and set is_master to false? etc

bulislaw · 2018-05-02T11:48:25Z

hal/spi_api.h

+    PinName MCLK;
+
+    uint32_t fill_symbol; /**< only the n lower bits will be used. */
+    uint32_t clock_frequency; /**< MCLK frequency in Hz. */


Do we have requirements on range?

The supported clock frequency range highly depends on the device capability (PLL, clock divisors etc).

bulislaw · 2018-05-02T12:00:21Z

hal/spi_api.h

- *  The total number of bytes sent and received will be the maximum of
- *  tx_length and rx_length. The bytes written will be padded with the
- *  value 0xff.
+ * It sends `tx_count` symbols from the buffer pointed by `tx` or a place holder is `tx` is NULL.


r a place holder is tx is NULL -> r a place holder if tx is NULL

Also you are using place holder here and fill symbol before can we align please.

good catch 👍

bulislaw · 2018-05-02T12:05:03Z

hal/spi_api.h

 *
- * @param[in] obj The SPI peripheral to check
- * @return The module number
+ * @param[in] handle This.


You are confusing This you shouldn't use it in different contexts. First you refer it to the spi_t context and now to async. Can you reword all of the occurrences to spell out what exactly it is.

Indeed the object type is not the same. I'll reword that.

bulislaw · 2018-05-02T12:05:44Z

hal/spi_api.h

 /**
- * \defgroup AsynchSPI Asynchronous SPI Hardware Abstraction Layer
- * @{
+ * Frees the SPI instance.


Do we want to specify what does it mean? eg should it be clocked out?

I am not sure how clocking should be defined as if you only use masters then you could even clock the peripheral out between transaction where you might want to keep it enabled in slave mode.
Unless slave selection is handled externally to this API and then we can rely 100% on the spi_transfer to manage clocking of the peripheral.

bulislaw · 2018-05-02T12:07:05Z

hal/spi_api.h

- * @param[in] event     The logical OR of events to be registered
- * @param[in] handler   SPI interrupt handler
- * @param[in] hint      A suggestion for how to use DMA with this transfer
+/*


I don't get why do we have it, can't we reimplement the slave driver using the above? If not let's create a doxygen group spi_hal_deprecated or something like that.

we could implement spi_read and spi_write but not spi_data_available.
However, we would not need it by handling externally the SS event and triggering a spi_tranfer from there instead of polling on the peripheral to see if something is coming in.

bulislaw · 2018-05-02T12:07:56Z

hal/spi_api.h

+ * channel towards a slave (or a master).
+ * Two SPI instances using the same peripheral can be identified by their SS pin.
+ *
+ * # Defined behaviour


Can we copy the behaviours into each of the functions descriptions.

I tried to slightly rephrase them in each header to make them more readable.

0xc0170 · 2018-05-03T09:06:44Z

hal/spi_api.h

- * @param[in]  ssel The pin to use for SSEL
+    spi_mode_t  mode;   /**< Transmission mode. See spi_mode_t. */
+
+    PinName SS;         /**< Slave select pin. */


why are these pin names uppercase?

0xc0170 · 2018-05-03T09:09:15Z

hal/spi_api.h

+
+    bool clock_phase; /**< True if data line is valid when leaving active state. */
+    bool clock_polarity; /**< True if the clock's rest state is high (+Vcc). */
+    uint32_t word_length; /**< Length of a symbol in bit. */


why is this then uint32_t ? The maximum is 32 I would assume?

0xc0170 · 2018-05-03T09:17:10Z

hal/spi_api.h

 */
-void spi_format(spi_t *obj, int bits, int mode, int slave);
+typedef struct spi_transfer_args_t {


this first should be spi_transfer_args_s, however I see where this is going. But from the codebase, tags are named with suffix _s (see _api headers for instance).

Actually I switched to _t because doxygen was not linking them properly when I generated the doc locally. All references to spi_transfer_args_t would not work unless spi_transfer_args_s is used in documentation too, which I find misleading.
For consistency it is better to use either one or the other every where.
Also, if we decide to use _s for typdef struct identifier_s identifier_t;, for consistency, we would also be required to name enumerations with _e (or something similar) to distinguish typedef enum identifier_e identifier_t;.

Looking at enums, seems like no tags there just a name.

The doxygen issue - was not aware of it. If we keep both the same or leave it as it is now in HAL, this should be probably captured in the code style.

0xc0170 · 2018-05-03T09:22:05Z

hal/spi_api.h

+    SPI_EVENT_TYPE_ON_DONE, /**< The operation has completed successfully. */
+    SPI_EVENT_TYPE_ON_ABORT, /**< The operation has been aborted. */
+    SPI_EVENT_TYPE_ON_ERROR /**< An error occured. */
+} spi_even_type_t;


missing t in _even_ ?

0xc0170 · 2018-05-03T09:25:02Z

hal/spi_api.h

 */
+typedef enum spi_event_type_t {
+    SPI_EVENT_TYPE_ON_DONE, /**< The operation has completed successfully. */


Are we changing callbacks to on_done or on_abort - separate callbacks for each event? (previously events were bitmasked and a user could choose multiple).

0xc0170 · 2018-05-03T09:28:44Z

hal/spi_api.h

+typedef enum spi_event_type_t {
+    SPI_EVENT_TYPE_ON_DONE, /**< The operation has completed successfully. */
+    SPI_EVENT_TYPE_ON_ABORT, /**< The operation has been aborted. */
+    SPI_EVENT_TYPE_ON_ERROR /**< An error occured. */


If error happens, is there any hal function that could retrieve what has happened (mode fault, data loss) ? Or not needed?

There is no uniform way to implement that. Error cause & reporting vary too much across the devices.
Retrieving such information would have to be platform specific.

We used to have overflow, and noticed in CMSIS SPI driver they have this as well (as data loss) and also bus fault (for spi slave).

0xc0170 · 2018-05-03T09:29:23Z

hal/spi_api.h

 */
-uint8_t spi_get_module(spi_t *obj);
+spi_async_handle_t* spi_async_transfer(


async as a suffix (spi_transfer and spi_transfer_async) ?

This could be seen as a constructor for spi_async_handle_t. Thus all method taking an instance of spi_async_handle_t as its this is prefixed by spi_async_.

0xc0170 · 2018-05-03T09:34:58Z

hal/spi_api.h

 *
- * @param[in] obj The SPI peripheral to check
- * @return The module number
+ * @return A reference counted handle to the transaction.


What is this returning? spi_async_handle_t is specified as being implementation specific , why ? what is expected to be defined there? each async transfer generates unique handle?

What if SPI peripheral is already taken - already transfer registered (SPI_0 peripheral already in use)? What this returns?

where the peripheral is in use or not, this method creates and schedule a transaction to be processed as the peripheral becomes available.

This handle is used by the _abort method. If the user does not want to abort he can spi_async_free_handle() right away.
The resources attached to the handle (passed in spi_async_tranfer()) must stay allocated until completion of the transaction. This completion is notified by a call to the callback.

if for some reason the transaction cannot be scheduled (queue is full etc) this returns NULL and does not take ownership of the data. The callback is therefore not invoked.

I dont follow, not clear to me what spi_async_handle_t is the meaning and why is it passed nor how transactions should work (I would assume transactions are handled in the upper layer, this one is just getting data and handles it).

Queue? HAL driver does not have any queue , does it?

Hm, I see. I will rework that to make it clearer then.

0xc0170 · 2018-05-03T09:35:55Z

hal/spi_api.h

 */
+void spi_async_abort(spi_async_handle_t* handle);


is handle enough here? I would expect obj to be passed as well to be able to access internal SPI registers/drivers

It is up to the driver to keep track of which peripheral a scheduled transaction is attached to.

to abort, do anything with peripheral, we need "this" pointer. or this one is just touching handle data , nothing connected to peripheral? It might be answered by my above question

cmonr · 2018-05-07T16:04:23Z

@ithinuel Looks like this still needs some massaging. Travis isn't happy.

ithinuel · 2018-05-07T18:21:26Z

@cmonr LittleFS' test depends on SPI. I'm not sure if LittleFS should be completely disabled or should I raise an issue to make this test ran only if SPI is enabled ?

cmonr · 2018-05-07T19:06:16Z

Hmm, I could see this going either way since this would need to be dealt with before merging into master anyways.

@geky , thoughts?

Imo, disabling the test(s) for now would be fine since this is a feature branch, but you run the risk of things changing out from underneath the branch that makes re-merging a pita.

bmcdonnell-ionx · 2018-05-08T13:59:15Z

hal/spi_api.h

+ * \struct spi_async_handle_t
+ * This needs to be declared and defined by the low level device driver.
+ * It is used to eventually abort an async request before its completion. see spi_async_abort.
+ */


I think this would be easier to read with a linebreak after each function's comments.

bmcdonnell-ionx · 2018-05-08T14:00:15Z

hal/spi_api.h

 */
+typedef struct spi_init_s {
+    bool    is_master;  /**< True to configure the device in Master mode */
+    bool    msb_first;  /**< True to send/receive the most significant bit first. */


This being a C-library, you should add #include <stdbool.h>, right?

As well as #include <stdint.h> for uint32_t and #include "device.h" for PinName. Good catch !

bmcdonnell-ionx · 2018-05-08T15:09:08Z

Question

With this API, how would a user implement an asynchronous receiving SPI Slave? (Whether simplex or duplex, for simplicity I'm just speaking about the receiving aspect here.)

Observation, Suggestion

It looks to me like you'd have a thread that calls spi_async_transfer() repeatedly to capture data as they arrive. I'm inferring that the callback will only be executed max once per call to spi_async_transfer(). Is that right? Assuming so...

Data would likely be captured into a CircularBuffer. Then you're probably going to collect those data into messages, and act on those messages - which may take some time. While processing your received message, you don't want to miss any data, so then you need yet another thread to do the processing. So, now you need two threads - right?

I think you should include the ability to attach callbacks which will execute as interrupt handlers. Then in this scenario, the interrupt handler can capture data from your microcontroller SPI peripheral FIFO (e.g. into a circular buffer), and [you / the user / the SPISlave driver] only need to run one thread to handle messages.

My Implementation

... That's what I did in the project I described in #6504. I implemented it for LPC4088. The API is attached here for your reference. (In my project, I implemented my enhancements as separate files - an "extended" API, and an EnhancedSPISlave driver which uses it. I had thought per #6504 that I'd integrate those into Mbed OS for the PR.)

spi_api_extended.h.txt

I have an enum in my EnhancedSPISlave driver which mirrors the one in the API.

The tricky thing is that I think the interrupt conditions often vary from micro to micro. I imagined that you'd just add conditions to SpiIrqType, and #define some of them in/out according to the target.

ithinuel · 2018-05-08T16:02:54Z

With this API, how would a user implement an asynchronous receiving SPI Slave? (Whether simplex or duplex, for simplicity I'm just speaking about the receiving aspect here.)

In such case a user would, on SS assertion, schedule a reception passing a

spi_transfer_args_t transfer_args = {
    .tx = NULL, .tx_count = 0,
    .rx = the_rx_buffer,
    .rx_count = the_expected_rx_size_or_rx_size
};

The callback would be triggered when rx_count reaches 0 or SS is de-asserted.

It looks to me like you'd have a thread that calls spi_async_transfer() repeatedly to capture data as they arrive. I'm inferring that the callback will only be executed max once per call to spi_async_transfer(). Is that right? Assuming so...

That is a correct assumption.

While processing your received message, you don't want to miss any data, so then you need yet another thread to do the processing. So, now you need two threads - right?

Not necessarily. You can for example schedule two reception only transfers. Both callback would ping a semaphore that the duty thread is waiting on. This is in a way some double buffering.
Once the first transfer is completed (rx_count symbol is reached), [you/the user/the SPISlave class] can process and schedule another transfer (eventually a response) while the rest of frame/message is being received.

It is the responsibility of the hal implementation to pull data from the fifo (if there's any) and pushing them into the buffer (not circular). Whether it be using an interrupt, a DMA or even a thread polling on the reception register, this is implementation details.
The last triggered transaction would be completed by reaching the transaction requirements rx_count & tx_count. Some target may even be able to detect the end of the transaction when SS is de-asserted (if SS is controlled in hardware).
What we might eventually need is some arguments in the callback telling how many symbols were received/sent.

NB: I am preparing an update to this PR moving the async API to sai_api_async.h.
This will ease the understanding of how synchronous and asynchronous API are related.

TODO: Thoroughly detail Async behaviour.

bmcdonnell-ionx · 2018-05-08T16:33:31Z

@ithinuel,

The callback would be triggered when rx_count reaches 0 or SS is de-asserted.

What if the messages are streaming, the master is not using SS as a delimiter, and the message length is not known until the end of message is detected using some in-band means?

You can for example schedule two reception only transfers.

What if the message handling takes longer than two transfers? Or if the user [etc] wants to send a message with the second call, but doesn't know what that message will be until after the first? Or if the user wants to stream full duplex messages which are not simultaneously delimited?

ithinuel · 2018-05-08T17:11:23Z

@bmcdonnell-ionx
Nothing prevents the driver (C++) to schedule transfers 1 symbol at a time and to expose the interrupt context to the user.
The user could then do from here whatever (s)he wants.

bmcdonnell-ionx · 2018-05-08T17:24:24Z

@ithinuel,

Nothing prevents the driver (C++) to schedule transfers 1 symbol at a time and to expose the interrupt context to the user.

Do I understand correctly as follows? The API can only register one-shot interrupt handlers. This will generally be implemented internally as disabling the interrupt after every occurrence.

For a streaming mode of operation as discussed, you would require the driver or user to repeatedly register these one-shot interrupt handlers, which are disabled after every symbol by the API.

ithinuel · 2018-05-08T22:19:46Z

@bmcdonnell-ionx Sorry, I think I actually missed your point.

The aim of this API is to provide a way to offload the transmission and reception of the symbols to an interrupt or, even better, a DMA if available.

As the method used to process these transactions is a hal implementation details, the [driver(C++)/user] is unable to use the interrupt to process each symbol individually.

A slave could process a stream of data using large buffer thus inducing some latency. This is something to be expected when using a DMA. Even though some platform offer multiple interrupts from the DMA (1/4 full, half full etc) this is not portable/consistent enough across platforms to be reliably used in this API.

Do you have a concrete use case (a specific chip or set up) of a streamed SPI protocol that require live computation of the output that I could use ?

studavekar · 2018-05-09T04:32:15Z

Disabled littlefs tests

/morph build

mbed-ci · 2018-05-09T05:01:40Z

Build : FAILURE

Build number : 1957
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6782/

geky · 2018-05-09T13:28:48Z

@studavekar thanks!

bmcdonnell-ionx · 2018-05-09T15:21:38Z

@ithinuel,

As the method used to process these transactions is a hal implementation details, the [driver(C++)/user] is unable to use the interrupt to process each symbol individually.

I don't know that processing each symbol individually is important. I do think the ability to process the streams independently with messages of varying and unknown lengths is. I think it's desirable to be able to make it a dumb pipe as such.

A slave could process a stream of data using large buffer thus inducing some latency. This is something to be expected when using a DMA. Even though some platform offer multiple interrupts from the DMA (1/4 full, half full etc) this is not portable/consistent enough across platforms to be reliably used in this API.

This scheme of interrupting on chunks of data is part of what motivated me to design a streaming protocol for my project in the first place.

Another part is that the SPI peripheral on my target (LPC4088) uses "frames" which are delimited by the Slave Select (SS) signal. Frames are constant-width of (user-configurable) 4 to 16 bits each. The internal FIFOs are 8-frames each for Tx and Rx. This means that on this platform, SS is not usable for delimiting messages. Thus the dumb pipe / streaming protocol I devised (details below).

Do you have a concrete use case (a specific chip or set up) of a streamed SPI protocol that require live computation of the output that I could use ?

I'm not sure exactly what you mean by "concrete". I can describe what I did in my project in more detail.

The SPI master and slave devices (both microcontrollers) reside on separate boards, connected via cable, each running their own applications.

My EnhancedSPISlave runs on Mbed OS on the LPC4088, underclocked to 24 MHz. The SPI bus acts as a half-duplex dumb pipe, with 0xff as a fill character. Messages are in XML format (thus we have < / > delimiters). Messages consist of commands sent from the master, and responses from the slave. One command yields one response.

Timely responses are desirable, but it's not hard realtime. (Arguably not realtime at all.) By decoupling the inherent synchronization between Tx and Rx in the lower layer protocol, we don't have to have the lower layer (SPI) protocol coupled to the upper layer message timing.

On the slave side, my application attaches interrupt handlers to the driver, which:

Read received symbols from the micro's SPI peripheral Rx FIFO into a receiving CircularBuffer, and signal the processing thread that data is available.
Write data to the SPI peripheral Tx FIFO. It writes any symbols enqueued in the sending CircularBuffer, or fill characters when CircularBuffer::empty().

cmonr · 2018-06-04T16:17:26Z

@bulislaw @0xc0170 @deepikabhavnani @mprse @bmcdonnell-ionx @c1728p9 Looks like this is ready for a re-review!

Comments appear to be addressed.

c1728p9 · 2018-06-07T18:43:58Z

hal/spi_api.h

 */
-void spi_frequency(spi_t *obj, int hz);
+const spi_capabilities_t *const spi_get_capabilities(const spi_pins_t *pins);


Capabilities should be per device and not per pin.

At this level of abstraction, device have already been abstracted in favour of channel.
spi_pins_t identifies channel that can then internally be translated to device.

As capabilities are required before initialisation, it cannot rely on a spi_t channel instance.

cf https://github.com/ithinuel/mbed-os/blob/967bd308b01e6b04758678f73535d1fddd6bb262/hal/spi_api.h#L119

Why this function returns address of the spi_capabilities_t structure? In such scenario spi_get_capabilities() function would have to allocate special memory for the result on heap.
I think that the better option is to take address of the spi_capabilities_t structure as a parameter and fills the structure with proper data.

This is because the capabilities are not dynamic and should be defined as static structures.

const spi_capabilities_t my_capabilities_for_spi1 = { [...] }; const spi_capabilities_t my_capabilities_for_spi2= { [...] };

This method should identify the correct structure and return a pointer to this flash stored object.

c1728p9 · 2018-06-07T18:45:58Z

hal/spi_api.h

 */
-int  spi_slave_receive(spi_t *obj);
+void spi_init(spi_t *obj, const spi_init_t *init);


Is init the only way to set the frequency and other settings? This should support at least changing the frequency after init, and probably the other settings as well. I know the current SD card driver connects at 100KHz and then once reading the supported speed switches to a higher frequency.

Indeed, it was agreed with @bulislaw that as there is no use-case where frequency & settings are changed on-the-fly/mid transfer and that it only happens at the early stage of the channel lifetime it is fine to free the channel & re-initialise it.

It is unlikely to be a problem for the slave, neither electrically wise as pull-ups should be provided externally for the sake of startup/reset safety, nor software wise as such operations only happen while SS is not asserted .

mprse · 2018-06-11T11:54:10Z

hal/spi_api.h

+    bool clock_polarity; /**< True if the clock's rest state is high (+Vcc). */
+    bool continuous_mode; /**< True to use the continuous mode. */
+
+    spi_mode_t  mode; /**< Transmission mode. See spi_mode_t. */


The definition of the spi_mode_t type has been removed. It looks like mode field should be removed from spi_init_t.

bmcdonnell-ionx

Minor questions and suggestions.

bmcdonnell-ionx · 2018-06-11T19:45:25Z

hal/spi_async_api.h

+ * # Defined behaviours
+
+## Defined behaviours
+ * - A repeating transfer must not restart until the invokation of the completion callback has not returned.


Typo - "invocation" (c not k).
Here and again below.

bmcdonnell-ionx · 2018-06-11T19:49:31Z

hal/spi_async_api.h

+ * - Freeing buffers before freeing the associated transaction.
+ *
+ * # Lexicon
+ * ## `Completion`/`Cancelletion`/`Abortion`


Consider a different word/phrase in place of "abortion" (here and elsewhere), to avoid unrelated connotations. "Abort" or "aborting" are fine if the context fits, IMO.

bmcdonnell-ionx · 2018-06-11T19:57:34Z

hal/spi_async_api.h

+#include "hal/spi_api.h"
+
+#if DEVICE_SPI
+#if DEVICE_SPI_ASYNCH


I think DEVICE_SPI_ASYNCH has so far only been implemented on a very few targets. (I think I heard one vendor implemented it for their own product, but others didn't follow.) Should it remain as an optionally-implemented feature, or will it be required for all SPI implementations? If required, the #define should go away, right? If optional, will we see more targets implementing it?

Actually a lot of target do implement it.
All the stm32 family, and a lot of other vendors including SiLabs, Atmel, Nordic etc.
It is still optional as some device cannot implement it though.

New commits.

cmonr · 2018-06-21T15:27:21Z

@bulislaw @0xc0170 @deepikabhavnani @mprse @c1728p9 @bmcdonnell-ionx Mind re-reviewing?

deepikabhavnani

spi_format and spi_frequency API's are missing from new HAL header file.
SD specification states initialize device at 100-400Khz and later device switches to 25Mhz once initialization is complete. SD cards will not be accessible if frequency is not updated runtime.

Also how do you plan to support multiple SPI slave devices operating on different configuration. Whenever SPI driver acquires SPI bus it does frequency/format change for that particular peripheral.
https://github.com/ARMmbed/mbed-os/blob/master/drivers/SPI.cpp#L84

screamerbg · 2018-06-21T16:37:34Z

There's a huge Mbed partner impact from these changes that I cannot agree with. This is not a specification, but almost feel like a full SPI HAL rewrite. How is this going to be mitigated?
CC @maclobdell @toyowata @samchuarm @bentcooke

ithinuel · 2018-06-22T01:41:28Z

@deepikabhavnani Change in frequency (and/or format) are done between transfers when SS is de-asserted which is supported by this API.

Format and frequency selection in Slave mode on SS assertion are handled by hal implementation and does not require to be exposed and handled by higher layer. Some target like the MK66F supports multiple configuration in master mode. Other targets may support that in slave mode too.

maclobdell · 2018-06-25T15:16:21Z

Is there a design document or overview of the proposed architecture including a review of the customer/partner/developer impacts? Breaking changes generate quite a lot of difficulties and work, so they must be justified by benefits to our partner and developer ecosystems - and of course buy-in by those people.

cmonr · 2018-07-03T12:46:09Z

@ithinuel In aiming to keep this PR warm, are there any updates?

ithinuel · 2018-07-03T13:01:18Z

@cmonr I am prepare a design document (.md) that will be placed somewhere in the hal subdirectory.
This will serve as a kind of RFC material to explain the incentive behind these modifications and keep track of the various questions that are raised and answered.

cmonr · 2018-07-03T15:34:05Z

@ithinuel I'm not sure if you've ever experienced this, but once the conversation in a PR passes ~150 messages, users may start getting the GitHub Unicorn indicating that they could not load the page.

Feel free to consider migrating this to an issue or opening another PR. Afaik, there's no other current workaround.

deepikabhavnani · 2018-07-03T15:40:16Z

@ithinuel

Format and frequency selection in Slave mode on SS assertion are handled by hal implementation

I would be interested in understanding how this is done at HAL layer, do we save format / frequency of each slave (SPI instance) in HAL layer? We do not have any synchronization at HAL layer it was always done at driver level, how will we handle runtime frequency change?

Runtime frequency change is the requirement for SD driver.

ithinuel · 2018-07-11T15:53:38Z

Closing as per @cmonr comment.
A new one will soon be created.

ithinuel added 3 commits May 1, 2018 12:29

add new SPI hal API

cc5cd58

disable SPI on all targets

e04eae7

add missing "word length" parameter

5f9a804

0xc0170 added the needs: review label May 2, 2018

bulislaw reviewed May 2, 2018

View reviewed changes

bulislaw requested a review from c1728p9 May 2, 2018 12:08

0xc0170 added needs: work and removed needs: review labels May 2, 2018

0xc0170 requested changes May 3, 2018

View reviewed changes

ithinuel added 2 commits May 3, 2018 15:20

update according to reviews

0205765

reorder elements to reduce padding

bb175c3

cmonr mentioned this pull request May 8, 2018

spi_api bugfix - many targets #6365

Closed

5 tasks

bmcdonnell-ionx reviewed May 8, 2018

View reviewed changes

ithinuel changed the title ~~Update SPI peripheral~~ Update SPI hal API May 8, 2018

ithinuel added 2 commits May 8, 2018 17:18

move async spi api to its own header file. update the documentation.

91fb56b

TODO: Thoroughly detail Async behaviour.

disable travis tests for little FS as it depends on the SPI interface.

948ce0d

simplify the API according to latest review.

967bd30

cmonr added needs: review and removed needs: work labels Jun 4, 2018

c1728p9 previously requested changes Jun 7, 2018

View reviewed changes

mprse reviewed Jun 11, 2018

View reviewed changes

remove orphan config

ec4cb4e

bmcdonnell-ionx suggested changes Jun 11, 2018

View reviewed changes

cmonr added needs: work and removed needs: review labels Jun 12, 2018

ithinuel added 3 commits June 12, 2018 11:43

Move the configuration from init to transfer.

33d7f05

fix spelling and wording.

2d25a57

fix spi_init documentation

bb2a25f

deepikabhavnani suggested changes Jun 21, 2018

View reviewed changes

ithinuel mentioned this pull request Jun 26, 2018

How to call spi_free() #4957

Closed

ithinuel closed this Jul 11, 2018

0xc0170 removed the needs: work label Jul 11, 2018

ithinuel mentioned this pull request Aug 12, 2019

Reconcile mbed trace and greentea #9954

Closed

Update SPI hal API #6782

Update SPI hal API #6782

Conversation

ithinuel commented May 1, 2018

Description

Pull request type

0xc0170 commented May 2, 2018

bulislaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xc0170 May 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ithinuel May 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xc0170 May 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmonr commented May 7, 2018

ithinuel commented May 7, 2018

cmonr commented May 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmcdonnell-ionx commented May 8, 2018 • edited Loading

Question

Observation, Suggestion

My Implementation

ithinuel commented May 8, 2018

bmcdonnell-ionx commented May 8, 2018

ithinuel commented May 8, 2018 • edited Loading

bmcdonnell-ionx commented May 8, 2018

ithinuel commented May 8, 2018

studavekar commented May 9, 2018

mbed-ci commented May 9, 2018

geky commented May 9, 2018

bmcdonnell-ionx commented May 9, 2018

cmonr commented Jun 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ithinuel Jun 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmcdonnell-ionx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xc0170 May 3, 2018 •

edited

Loading

ithinuel May 3, 2018 •

edited

Loading

0xc0170 May 3, 2018 •

edited

Loading

bmcdonnell-ionx commented May 8, 2018 •

edited

Loading

ithinuel commented May 8, 2018 •

edited

Loading

ithinuel Jun 8, 2018 •

edited

Loading