drivers: spi: make SPI dt-spec macros compatible with C++

As of today it is not possible to use SPI dt-spec macros in C++,
something known and documented. The main reason is because `cs` property
is initialized using a compound literal, something not supported in C++.
This PR takes another approach, that is to not make `cs` a pointer but a
struct member. This way, we can perform a regular initialization, at the
cost of using extra memory for unused delay/pin/flags if `cs` is not
used.

Fixes #56572

Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
This commit is contained in:
Gerard Marull-Paretas
2023-04-05 17:03:52 +02:00
committed by Carles Cufí
parent 1cf1994686
commit 3f2c2d4130
16 changed files with 62 additions and 97 deletions

View File

@@ -198,18 +198,18 @@ static bool bt_spi_handle_vendor_evt(uint8_t *rxmsg)
static int configure_cs(void)
{
/* Configure pin as output and set to active */
return gpio_pin_configure_dt(&bus.config.cs->gpio, GPIO_OUTPUT_ACTIVE);
return gpio_pin_configure_dt(&bus.config.cs.gpio, GPIO_OUTPUT_ACTIVE);
}
static void kick_cs(void)
{
gpio_pin_set_dt(&bus.config.cs->gpio, 0);
gpio_pin_set_dt(&bus.config.cs->gpio, 1);
gpio_pin_set_dt(&bus.config.cs.gpio, 0);
gpio_pin_set_dt(&bus.config.cs.gpio, 1);
}
static void release_cs(void)
{
gpio_pin_set_dt(&bus.config.cs->gpio, 0);
gpio_pin_set_dt(&bus.config.cs.gpio, 0);
}
static bool irq_pin_high(void)

View File

@@ -220,7 +220,7 @@ static int fpga_ice40_load_gpio(const struct device *dev, uint32_t *image_ptr, u
const struct fpga_ice40_config *config = dev->config;
/* prepare masks */
cs = BIT(config->bus.config.cs->gpio.pin);
cs = BIT(config->bus.config.cs.gpio.pin);
clk = BIT(config->clk.pin);
pico = BIT(config->pico.pin);
creset = BIT(config->creset.pin);
@@ -241,7 +241,7 @@ static int fpga_ice40_load_gpio(const struct device *dev, uint32_t *image_ptr, u
LOG_DBG("Initializing GPIO");
ret = gpio_pin_configure_dt(&config->cdone, GPIO_INPUT) ||
gpio_pin_configure_dt(&config->creset, GPIO_OUTPUT_HIGH) ||
gpio_pin_configure_dt(&config->bus.config.cs->gpio, GPIO_OUTPUT_HIGH) ||
gpio_pin_configure_dt(&config->bus.config.cs.gpio, GPIO_OUTPUT_HIGH) ||
gpio_pin_configure_dt(&config->clk, GPIO_OUTPUT_HIGH) ||
gpio_pin_configure_dt(&config->pico, GPIO_OUTPUT_HIGH);
__ASSERT(ret == 0, "Failed to initialize GPIO: %d", ret);
@@ -297,7 +297,7 @@ static int fpga_ice40_load_gpio(const struct device *dev, uint32_t *image_ptr, u
unlock:
(void)gpio_pin_configure_dt(&config->creset, GPIO_OUTPUT_HIGH);
(void)gpio_pin_configure_dt(&config->bus.config.cs->gpio, GPIO_OUTPUT_HIGH);
(void)gpio_pin_configure_dt(&config->bus.config.cs.gpio, GPIO_OUTPUT_HIGH);
(void)gpio_pin_configure_dt(&config->clk, GPIO_DISCONNECTED);
(void)gpio_pin_configure_dt(&config->pico, GPIO_DISCONNECTED);
#ifdef CONFIG_PINCTRL
@@ -339,7 +339,7 @@ static int fpga_ice40_load_spi(const struct device *dev, uint32_t *image_ptr, ui
LOG_DBG("Initializing GPIO");
ret = gpio_pin_configure_dt(&config->cdone, GPIO_INPUT) ||
gpio_pin_configure_dt(&config->creset, GPIO_OUTPUT_HIGH) ||
gpio_pin_configure_dt(&config->bus.config.cs->gpio, GPIO_OUTPUT_HIGH);
gpio_pin_configure_dt(&config->bus.config.cs.gpio, GPIO_OUTPUT_HIGH);
__ASSERT(ret == 0, "Failed to initialize GPIO: %d", ret);
LOG_DBG("Set CRESET low");
@@ -350,7 +350,7 @@ static int fpga_ice40_load_spi(const struct device *dev, uint32_t *image_ptr, ui
}
LOG_DBG("Set SPI_CS low");
ret = gpio_pin_configure_dt(&config->bus.config.cs->gpio, GPIO_OUTPUT_LOW);
ret = gpio_pin_configure_dt(&config->bus.config.cs.gpio, GPIO_OUTPUT_LOW);
if (ret < 0) {
LOG_ERR("failed to set SPI_CS low: %d", ret);
goto unlock;
@@ -373,7 +373,7 @@ static int fpga_ice40_load_spi(const struct device *dev, uint32_t *image_ptr, ui
k_busy_wait(config->config_delay_us);
LOG_DBG("Set SPI_CS high");
ret = gpio_pin_configure_dt(&config->bus.config.cs->gpio, GPIO_OUTPUT_HIGH);
ret = gpio_pin_configure_dt(&config->bus.config.cs.gpio, GPIO_OUTPUT_HIGH);
if (ret < 0) {
LOG_ERR("failed to set SPI_CS high: %d", ret);
goto unlock;
@@ -389,7 +389,7 @@ static int fpga_ice40_load_spi(const struct device *dev, uint32_t *image_ptr, ui
}
LOG_DBG("Set SPI_CS low");
ret = gpio_pin_configure_dt(&config->bus.config.cs->gpio, GPIO_OUTPUT_LOW);
ret = gpio_pin_configure_dt(&config->bus.config.cs.gpio, GPIO_OUTPUT_LOW);
if (ret < 0) {
LOG_ERR("failed to set SPI_CS low: %d", ret);
goto unlock;
@@ -405,7 +405,7 @@ static int fpga_ice40_load_spi(const struct device *dev, uint32_t *image_ptr, ui
}
LOG_DBG("Set SPI_CS high");
ret = gpio_pin_configure_dt(&config->bus.config.cs->gpio, GPIO_OUTPUT_HIGH);
ret = gpio_pin_configure_dt(&config->bus.config.cs.gpio, GPIO_OUTPUT_HIGH);
if (ret < 0) {
LOG_ERR("failed to set SPI_CS high: %d", ret);
goto unlock;
@@ -438,7 +438,7 @@ static int fpga_ice40_load_spi(const struct device *dev, uint32_t *image_ptr, ui
unlock:
(void)gpio_pin_configure_dt(&config->creset, GPIO_OUTPUT_HIGH);
(void)gpio_pin_configure_dt(&config->bus.config.cs->gpio, GPIO_OUTPUT_HIGH);
(void)gpio_pin_configure_dt(&config->bus.config.cs.gpio, GPIO_OUTPUT_HIGH);
#ifdef CONFIG_PINCTRL
(void)pinctrl_apply_state(config->pincfg, PINCTRL_STATE_DEFAULT);
#endif

View File

@@ -74,7 +74,7 @@ static bool spi_b91_config_cs(const struct device *dev,
const struct spi_b91_cfg *b91_config = SPI_CFG(dev);
/* software flow control */
if (config->cs) {
if (config->cs.gpio.port != NULL) {
/* disable all hardware CS pins */
spi_b91_hw_cs_disable(b91_config);
return true;
@@ -397,7 +397,7 @@ static int spi_b91_transceive(const struct device *dev,
spi_context_buffers_setup(&data->ctx, tx_bufs, rx_bufs, 1);
/* if cs is defined: software cs control, set active true */
if (config->cs) {
if (config->cs.gpio.port != NULL) {
spi_context_cs_control(&data->ctx, true);
}
@@ -405,7 +405,7 @@ static int spi_b91_transceive(const struct device *dev,
spi_b91_txrx(dev, txrx_len);
/* if cs is defined: software cs control, set active false */
if (config->cs) {
if (config->cs.gpio.port != NULL) {
spi_context_cs_control(&data->ctx, false);
}

View File

@@ -75,7 +75,7 @@ static int spi_cc13xx_cc26xx_configure(const struct device *dev,
return -EINVAL;
}
if (config->operation & SPI_CS_ACTIVE_HIGH && !config->cs) {
if (config->operation & SPI_CS_ACTIVE_HIGH && config->cs.gpio.port == NULL) {
LOG_ERR("Active high CS requires emulation through a GPIO line.");
return -EINVAL;
}

View File

@@ -236,18 +236,18 @@ static inline int spi_context_cs_configure_all(struct spi_context *ctx)
static inline void _spi_context_cs_control(struct spi_context *ctx,
bool on, bool force_off)
{
if (ctx->config && ctx->config->cs && ctx->config->cs->gpio.port) {
if (ctx->config && ctx->config->cs.gpio.port) {
if (on) {
gpio_pin_set_dt(&ctx->config->cs->gpio, 1);
k_busy_wait(ctx->config->cs->delay);
gpio_pin_set_dt(&ctx->config->cs.gpio, 1);
k_busy_wait(ctx->config->cs.delay);
} else {
if (!force_off &&
ctx->config->operation & SPI_HOLD_ON_CS) {
return;
}
k_busy_wait(ctx->config->cs->delay);
gpio_pin_set_dt(&ctx->config->cs->gpio, 0);
k_busy_wait(ctx->config->cs.delay);
gpio_pin_set_dt(&ctx->config->cs.gpio, 0);
}
}
}

View File

@@ -147,7 +147,7 @@ static int spi_gd32_configure(const struct device *dev,
/* Reset to hardware NSS mode. */
SPI_CTL0(cfg->reg) &= ~SPI_CTL0_SWNSSEN;
if (config->cs != NULL) {
if (config->cs.gpio.port != NULL) {
SPI_CTL0(cfg->reg) |= SPI_CTL0_SWNSSEN;
} else {
/*

View File

@@ -104,13 +104,9 @@ static inline int z_vrfy_spi_transceive(const struct device *dev,
}
memcpy(&config_copy, config, sizeof(*config));
if (config_copy.cs) {
const struct spi_cs_control *cs = config_copy.cs;
Z_OOPS(Z_SYSCALL_MEMORY_READ(cs, sizeof(*cs)));
if (cs->gpio.port) {
Z_OOPS(Z_SYSCALL_OBJ(cs->gpio.port, K_OBJ_DRIVER_GPIO));
}
if (config_copy.cs.gpio.port != NULL) {
Z_OOPS(Z_SYSCALL_OBJ(config_copy.cs.gpio.port,
K_OBJ_DRIVER_GPIO));
}
return copy_bufs_and_transceive((const struct device *)dev,

View File

@@ -541,7 +541,7 @@ static int spi_stm32_configure(const struct device *dev,
LL_SPI_DisableCRC(spi);
if (config->cs || !IS_ENABLED(CONFIG_SPI_STM32_USE_HW_SS)) {
if (config->cs.gpio.port != NULL || !IS_ENABLED(CONFIG_SPI_STM32_USE_HW_SS)) {
#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32h7_spi)
if (SPI_OP_MODE_GET(config->operation) == SPI_OP_MODE_MASTER) {
if (LL_SPI_GetNSSPolarity(spi) == LL_SPI_NSS_POLARITY_LOW)

View File

@@ -91,7 +91,7 @@ static int configure(const struct device *dev,
return -EINVAL;
}
if (spi_cfg->cs) {
if (spi_cfg->cs.gpio.port != NULL) {
LOG_ERR("CS control via GPIO is not supported");
return -EINVAL;
}

View File

@@ -109,7 +109,7 @@ int spi_oc_simple_transceive(const struct device *dev,
spi_oc_simple_configure(info, spi, config);
/* Set chip select */
if (config->cs) {
if (config->cs.gpio.port != NULL) {
spi_context_cs_control(&spi->ctx, true);
} else {
sys_write8(1 << config->slave, SPI_OC_SIMPLE_SPSS(info));
@@ -147,7 +147,7 @@ int spi_oc_simple_transceive(const struct device *dev,
}
/* Clear chip-select */
if (config->cs) {
if (config->cs.gpio.port != NULL) {
spi_context_cs_control(&spi->ctx, false);
} else {
sys_write8(0 << config->slave, SPI_OC_SIMPLE_SPSS(info));

View File

@@ -549,7 +549,7 @@ static int spi_pw_configure(const struct device *dev,
/* At this point, it's mandatory to set this on the context! */
spi->ctx.config = config;
if (!spi->ctx.config->cs) {
if (spi->ctx.config->cs.gpio.port == NULL) {
if (spi->cs_mode == CS_GPIO_MODE) {
LOG_DBG("cs gpio is NULL, switch to hw mode");
spi->cs_mode = CS_HW_MODE;

View File

@@ -216,7 +216,7 @@ static int spi_sifive_transceive(const struct device *dev,
* If the chip select configuration is not present, we'll ask the
* SPI peripheral itself to control the CS line
*/
if (config->cs == NULL) {
if (config->cs.gpio.port == NULL) {
hw_cs_control = true;
}

View File

@@ -197,8 +197,9 @@ struct spi_cs_control {
* @param spi_dev a SPI device node identifier
* @return #gpio_dt_spec struct corresponding with spi_dev's chip select
*/
#define SPI_CS_GPIOS_DT_SPEC_GET(spi_dev) \
GPIO_DT_SPEC_GET_BY_IDX(DT_BUS(spi_dev), cs_gpios, DT_REG_ADDR(spi_dev))
#define SPI_CS_GPIOS_DT_SPEC_GET(spi_dev) \
GPIO_DT_SPEC_GET_BY_IDX_OR(DT_BUS(spi_dev), cs_gpios, \
DT_REG_ADDR(spi_dev), {})
/**
* @brief Get a <tt>struct gpio_dt_spec</tt> for a SPI device's chip select pin
@@ -212,7 +213,6 @@ struct spi_cs_control {
#define SPI_CS_GPIOS_DT_SPEC_INST_GET(inst) \
SPI_CS_GPIOS_DT_SPEC_GET(DT_DRV_INST(inst))
#ifndef __cplusplus
/**
* @brief Initialize and get a pointer to a @p spi_cs_control from a
* devicetree node identifier
@@ -229,39 +229,27 @@ struct spi_cs_control {
* spidev: spi-device@0 { ... };
* };
*
* Assume that @p gpio0 follows the standard convention for specifying
* GPIOs, i.e. it has the following in its binding:
*
* gpio-cells:
* - pin
* - flags
*
* Example usage:
*
* struct spi_cs_control *ctrl =
* SPI_CS_CONTROL_PTR_DT(DT_NODELABEL(spidev), 2);
* struct spi_cs_control ctrl =
* SPI_CS_CONTROL_INIT(DT_NODELABEL(spidev), 2);
*
* This example is equivalent to:
*
* struct spi_cs_control *ctrl =
* &(struct spi_cs_control) {
* .gpio_dev = DEVICE_DT_GET(DT_NODELABEL(gpio0)),
* .delay = 2,
* .gpio_pin = 1,
* .gpio_dt_flags = GPIO_ACTIVE_LOW
* };
*
* This macro is not available in C++.
* struct spi_cs_control ctrl = {
* .gpio = SPI_CS_GPIOS_DT_SPEC_GET(DT_NODELABEL(spidev)),
* .delay = 2,
* };
*
* @param node_id Devicetree node identifier for a device on a SPI bus
* @param delay_ The @p delay field to set in the @p spi_cs_control
* @return a pointer to the @p spi_cs_control structure
*/
#define SPI_CS_CONTROL_PTR_DT(node_id, delay_) \
(&(struct spi_cs_control) { \
#define SPI_CS_CONTROL_INIT(node_id, delay_) \
{ \
.gpio = SPI_CS_GPIOS_DT_SPEC_GET(node_id), \
.delay = (delay_), \
})
}
/**
* @brief Get a pointer to a @p spi_cs_control from a devicetree node
@@ -272,15 +260,12 @@ struct spi_cs_control {
* Therefore, @p DT_DRV_COMPAT must already be defined before using
* this macro.
*
* This macro is not available in C++.
*
* @param inst Devicetree node instance number
* @param delay_ The @p delay field to set in the @p spi_cs_control
* @return a pointer to the @p spi_cs_control structure
*/
#define SPI_CS_CONTROL_PTR_DT_INST(inst, delay_) \
SPI_CS_CONTROL_PTR_DT(DT_DRV_INST(inst), delay_)
#endif
/**
* @brief SPI controller configuration structure
@@ -319,10 +304,9 @@ struct spi_config {
uint16_t slave;
#endif /* CONFIG_SPI_EXTENDED_MODES */
const struct spi_cs_control *cs;
struct spi_cs_control cs;
};
#ifndef __cplusplus
/**
* @brief Structure initializer for spi_config from devicetree
*
@@ -334,8 +318,6 @@ struct spi_config {
* SPI_CS_CONTROL_PTR_DT(). The @p gpio_dev value pointed to by this
* structure must be checked using device_is_ready() before use.
*
* This macro is not available in C++.
*
* @param node_id Devicetree node identifier for the SPI device whose
* struct spi_config to create an initializer for
* @param operation_ the desired @p operation field in the struct spi_config
@@ -349,10 +331,7 @@ struct spi_config {
DT_PROP(node_id, duplex) | \
DT_PROP(node_id, frame_format), \
.slave = DT_REG_ADDR(node_id), \
.cs = COND_CODE_1( \
DT_SPI_DEV_HAS_CS_GPIOS(node_id), \
(SPI_CS_CONTROL_PTR_DT(node_id, delay_)), \
(NULL)), \
.cs = SPI_CS_CONTROL_INIT(node_id, delay_), \
}
/**
@@ -361,8 +340,6 @@ struct spi_config {
* This is equivalent to
* <tt>SPI_CONFIG_DT(DT_DRV_INST(inst), operation_, delay_)</tt>.
*
* This macro is not available in C++.
*
* @param inst Devicetree instance number
* @param operation_ the desired @p operation field in the struct spi_config
* @param delay_ the desired @p delay field in the struct spi_config's
@@ -370,7 +347,6 @@ struct spi_config {
*/
#define SPI_CONFIG_DT_INST(inst, operation_, delay_) \
SPI_CONFIG_DT(DT_DRV_INST(inst), operation_, delay_)
#endif
/**
* @brief Complete SPI DT information
@@ -383,7 +359,6 @@ struct spi_dt_spec {
struct spi_config config;
};
#ifndef __cplusplus
/**
* @brief Structure initializer for spi_dt_spec from devicetree
*
@@ -396,8 +371,6 @@ struct spi_dt_spec {
* @ref device_is_ready checks.
* @deprecated Use @ref spi_is_ready_dt instead.
*
* This macro is not available in C++.
*
* @param node_id Devicetree node identifier for the SPI device whose
* struct spi_dt_spec to create an initializer for
* @param operation_ the desired @p operation field in the struct spi_config
@@ -416,8 +389,6 @@ struct spi_dt_spec {
* This is equivalent to
* <tt>SPI_DT_SPEC_GET(DT_DRV_INST(inst), operation_, delay_)</tt>.
*
* This macro is not available in C++.
*
* @param inst Devicetree instance number
* @param operation_ the desired @p operation field in the struct spi_config
* @param delay_ the desired @p delay field in the struct spi_config's
@@ -425,7 +396,6 @@ struct spi_dt_spec {
*/
#define SPI_DT_SPEC_INST_GET(inst, operation_, delay_) \
SPI_DT_SPEC_GET(DT_DRV_INST(inst), operation_, delay_)
#endif
/**
* @brief SPI buffer structure
@@ -532,8 +502,8 @@ static inline bool spi_is_ready(const struct spi_dt_spec *spec)
return false;
}
/* Validate CS gpio port is ready, if it is used */
if (spec->config.cs &&
!device_is_ready(spec->config.cs->gpio.port)) {
if (spec->config.cs.gpio.port != NULL &&
!device_is_ready(spec->config.cs.gpio.port)) {
return false;
}
return true;
@@ -554,8 +524,8 @@ static inline bool spi_is_ready_dt(const struct spi_dt_spec *spec)
return false;
}
/* Validate CS gpio port is ready, if it is used */
if (spec->config.cs &&
!device_is_ready(spec->config.cs->gpio.port)) {
if (spec->config.cs.gpio.port != NULL &&
!device_is_ready(spec->config.cs.gpio.port)) {
return false;
}
return true;

View File

@@ -288,14 +288,13 @@ static bool background_transfer(const struct device *spi_dev)
{
static const uint8_t tx_buffer[] = "Nordic Semiconductor";
static uint8_t rx_buffer[sizeof(tx_buffer)];
static const struct spi_cs_control spi_dev_cs_ctrl = {
.gpio = GPIO_DT_SPEC_GET(SPI_DEV_NODE, cs_gpios),
};
static const struct spi_config spi_dev_cfg = {
.operation = SPI_OP_MODE_MASTER | SPI_WORD_SET(8) |
SPI_TRANSFER_MSB,
.frequency = 1000000,
.cs = &spi_dev_cs_ctrl
.cs = {
.gpio = GPIO_DT_SPEC_GET(SPI_DEV_NODE, cs_gpios),
},
};
static const struct spi_buf tx_buf = {
.buf = (void *)tx_buffer,

View File

@@ -27,7 +27,7 @@ void test_basic_write_9bit_words(const struct device *dev,
config.frequency = 125000;
config.operation = SPI_OP_MODE_MASTER | SPI_WORD_SET(9);
config.slave = 0;
config.cs = cs;
config.cs = *cs;
uint16_t buff[5] = { 0x0101, 0x00ff, 0x00a5, 0x0000, 0x0102};
int len = 5 * sizeof(buff[0]);
@@ -54,7 +54,7 @@ void test_9bit_loopback_partial(const struct device *dev,
config.frequency = 125000;
config.operation = SPI_OP_MODE_MASTER | SPI_WORD_SET(9);
config.slave = 0;
config.cs = cs;
config.cs = *cs;
enum { datacount = 5 };
uint16_t buff[datacount] = { 0x0101, 0x0102, 0x0003, 0x0004, 0x0105};
@@ -93,7 +93,7 @@ void test_8bit_xfer(const struct device *dev, struct spi_cs_control *cs)
config.frequency = 1000000;
config.operation = SPI_OP_MODE_MASTER | SPI_WORD_SET(8);
config.slave = 0;
config.cs = cs;
config.cs = *cs;
enum { datacount = 5 };
uint8_t buff[datacount] = { 0x01, 0x02, 0x03, 0x04, 0x05};

View File

@@ -18,21 +18,21 @@ ZTEST(spi_dt_spec, test_dt_spec)
SPI_DT_SPEC_GET(DT_NODELABEL(test_spi_dev_cs), 0, 0);
LOG_DBG("spi_cs.bus = %p", spi_cs.bus);
LOG_DBG("spi_cs.config.cs->gpio.port = %p", spi_cs.config.cs->gpio.port);
LOG_DBG("spi_cs.config.cs->gpio.pin = %u", spi_cs.config.cs->gpio.pin);
LOG_DBG("spi_cs.config.cs.gpio.port = %p", spi_cs.config.cs.gpio.port);
LOG_DBG("spi_cs.config.cs.gpio.pin = %u", spi_cs.config.cs.gpio.pin);
zassert_equal(spi_cs.bus, DEVICE_DT_GET(DT_NODELABEL(test_spi_cs)), "");
zassert_equal(spi_cs.config.cs->gpio.port, DEVICE_DT_GET(DT_NODELABEL(test_gpio)), "");
zassert_equal(spi_cs.config.cs->gpio.pin, 0x10, "");
zassert_equal(spi_cs.config.cs.gpio.port, DEVICE_DT_GET(DT_NODELABEL(test_gpio)), "");
zassert_equal(spi_cs.config.cs.gpio.pin, 0x10, "");
const struct spi_dt_spec spi_no_cs =
SPI_DT_SPEC_GET(DT_NODELABEL(test_spi_dev_no_cs), 0, 0);
LOG_DBG("spi_no_cs.bus = %p", spi_no_cs.bus);
LOG_DBG("spi_no_cs.config.cs = %p", spi_no_cs.config.cs);
LOG_DBG("spi_no_cs.config.cs.gpio.port = %p", spi_no_cs.config.cs.gpio.port);
zassert_equal(spi_no_cs.bus, DEVICE_DT_GET(DT_NODELABEL(test_spi_no_cs)), "");
zassert_is_null(spi_no_cs.config.cs, "");
zassert_is_null(spi_no_cs.config.cs.gpio.port, "");
}
ZTEST_SUITE(spi_dt_spec, NULL, NULL, NULL, NULL, NULL);