SPI: API interface with bits per SPI frame

[ccli8]

Description of defect

SPI class supports 4-16 bits per SPI frame according to SPI::format(...).

/** Configure the data transmission format.
     *
     *  @param bits Number of bits per SPI frame (4 - 16).
     *  @param mode Clock polarity and phase mode (0 - 3).
     *
     */
    void format(int bits, int mode = 0);

This SPI::write(int value) can support arbitrary bits per SPI frame with data type int.

/** Write to the SPI Slave and return the response.
     *  @param value Data to be sent to the SPI slave.
     */
    virtual int write(int value);

But this SPI::write(const char *tx_buffer, ...) can just support <=8 bits per SPI frame due to data type char. Why not design it to support arbitrary bits per SPI frame like above?

/** Write to the SPI Slave and obtain the response.
     *
     *  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.
     *
     *  @param tx_buffer Pointer to the byte-array of data to write to the device.
     *  @param tx_length Number of bytes to write, may be zero.
     *  @param rx_buffer Pointer to the byte-array of data to read from the device.
     *  @param rx_length Number of bytes to read, may be zero.
     *  @return
     *      The number of bytes written and read from the device. This is
     *      maximum of tx_length and rx_length.
     */
    virtual int write(const char *tx_buffer, int tx_length, char *rx_buffer, int rx_length);

A relevant question with bits per SPI frame: With Type being template, would the unit of tx_length/rx_length be Type instead of byte?

/** Start non-blocking SPI transfer using 8bit buffers.
     *
     * This function locks the deep sleep until any event has occurred.
     *
     * @param tx_buffer The TX buffer with data to be transferred. If NULL is passed,
     *                  the default SPI value is sent.
     * @param tx_length The length of TX buffer in bytes.
     * @param rx_buffer The RX buffer which is used for received data. If NULL is passed,
     *                  received data are ignored.
     * @param rx_length The length of RX buffer in bytes.
     * @param callback  The event callback function.
     * @param event     The event mask of events to modify. @see spi_api.h for SPI events.
     *
     * @return Operation result.
     * @retval 0 If the transfer has started.
     * @retval -1 If SPI peripheral is busy.
     */
    template<typename Type>
    int transfer(const Type *tx_buffer, int tx_length, Type *rx_buffer, int rx_length, const event_callback_t &callback, int event = SPI_EVENT_COMPLETE)
    {

Target(s) affected by this defect ?

N/A

Toolchain(s) (name and version) displaying this defect ?

N/A

What version of Mbed-os are you using (tag or sha) ?

mbed-os-6.0.0-alpha1

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

N/A

How is this defect reproduced ?

N/A

[0xc0170]

This write method operates on 8bit buffers but uses internally write functionality that should follow what format method set. This is not clear from the docs, what should happen in this case. We should review and clarify. This is a good candidate for feature-hal-spec-spi we already have. have you looked at it if it fixes this problem?

cc @ARMmbed/mbed-os-hal

[mprse]

In the new HAL API on the feature branch, spi_transfer uses void* type to specify the rx/tx buffer and follows the configured format:

mbed-os/hal/spi_api.h

Lines 233 to 242 in f88b7cb

	* Transfer data blocking until the end of the operation.
	*
	* @param obj SPI periheral.
	* @param tx Buffer so send.
	* @param tx_len Number of symbol in tx.
	* @param rx Buffer to store received symbols.
	* @param rx_len Number of symbol to store in rx.
	* @param fill_symbol Symbol to send if tx_len is less than rx_len.
	*/
	uint32_t spi_transfer(spi_t *obj, const void *tx, uint32_t tx_len, void *rx, uint32_t rx_len, const void *fill_symbol);

More details about new SPI HAL API can be found here:
https://github.com/ARMmbed/mbed-os/blob/feature-hal-spec-spi/docs/design-documents/hal/0000-spi-overhaul.md#the-new-api

Unfortunately, at the moment there are no plans to merge SPI feature branch into master.

[ccli8]

More details about new SPI HAL API can be found here:
https://github.com/ARMmbed/mbed-os/blob/feature-hal-spec-spi/docs/design-documents/hal/0000-spi-overhaul.md#the-new-api

Yes. The new SPI HAL API can address my issue. But as mentioned above @mprse, it will not be merged soon. It is overhaul. Actually, I create the ticket to address SPI::write(const char *tx_buffer, ...) because I want to optimize spi_master_block_write(...) and substitute it for bad performance spi_master_write(...) in case of successive data transfer. #12454 is associated with it. But the approach is obstructed due to char data type in SPI::write(const char *tx_buffer, ...)/spi_master_block_write(...). Now that the new SPI HAL API is not to release soon, would you re-design SPI::write(const char *tx_buffer, ...)/spi_master_block_write(...) first to enable arbitrary bits per frame as short-term solution?

[ciarmcom]

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers.
Internal Jira reference: https://jira.arm.com/browse/IOTOSM-2193