Skip to content

Commit 377e6bd

Browse files
decsnykartben
authored andcommitted
spi_mcux_lpspi: More precise configuration checks
These changes: * Fix the check of the word size to be more useful - check min frame size instead of max - check for min word size requirement - add a clarifying comment about what the word size represents in hardware since the nomenclature from zephyr does not match the nxp references * Add a clarifying comment about half duplex being supported by hardware * Add LPSPI_ namespace to defines * Change chip select error message to be more clear about the problem * Move the check of the clock device being ready to the lpspi init, instead of checking it every time on configure. It probably also makes more sense to not ready the lpspi device if the clock is not ready. * Move the bare-metal configuration of bit fields AFTER the SDK Init call. * Return the proper error code if clock control call errors. Signed-off-by: Declan Snyder <[email protected]>
1 parent 1022a89 commit 377e6bd

File tree

1 file changed

+28
-17
lines changed

1 file changed

+28
-17
lines changed

drivers/spi/spi_mcux_lpspi.c

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ LOG_MODULE_REGISTER(spi_mcux_lpspi, CONFIG_SPI_LOG_LEVEL);
2929
/* If any hardware revisions change this, make it into a DT property.
3030
* DONT'T make #ifdefs here by platform.
3131
*/
32-
#define CHIP_SELECT_COUNT 4
33-
#define MAX_DATA_WIDTH 4096
32+
#define LPSPI_CHIP_SELECT_COUNT 4
33+
#define LPSPI_MIN_FRAME_SIZE_BITS 8
3434

3535
/* Required by DEVICE_MMIO_NAMED_* macros */
3636
#define DEV_CFG(_dev) ((const struct spi_mcux_config *)(_dev)->config)
@@ -161,29 +161,35 @@ static int spi_mcux_configure(const struct device *dev, const struct spi_config
161161
uint32_t word_size = SPI_WORD_SIZE_GET(spi_cfg->operation);
162162
lpspi_master_config_t master_config;
163163
uint32_t clock_freq;
164+
int ret;
164165

165166
if (spi_cfg->operation & SPI_HALF_DUPLEX) {
167+
/* the IP DOES support half duplex, need to implement driver support */
166168
LOG_ERR("Half-duplex not supported");
167169
return -ENOTSUP;
168170
}
169171

170-
if (spi_cfg->slave > CHIP_SELECT_COUNT) {
171-
LOG_ERR("Slave %d is greater than %d", spi_cfg->slave, CHIP_SELECT_COUNT);
172+
if (word_size < 8 || (word_size % 32 == 1)) {
173+
/* Zephyr word size == hardware FRAME size (not word size)
174+
* Max frame size: 4096 bits
175+
* (zephyr field is 6 bit wide for max 64 bit size, no need to check)
176+
* Min frame size: 8 bits.
177+
* Minimum hardware word size is 2. Since this driver is intended to work
178+
* for 32 bit platforms, and 64 bits is max size, then only 33 and 1 are invalid.
179+
*/
180+
LOG_ERR("Word size %d not allowed", word_size);
172181
return -EINVAL;
173182
}
174183

175-
if (word_size > MAX_DATA_WIDTH) {
176-
LOG_ERR("Word size %d is greater than %d", word_size, MAX_DATA_WIDTH);
184+
if (spi_cfg->slave > LPSPI_CHIP_SELECT_COUNT) {
185+
LOG_ERR("Peripheral %d select exceeds max %d", spi_cfg->slave,
186+
LPSPI_CHIP_SELECT_COUNT - 1);
177187
return -EINVAL;
178188
}
179189

180-
if (!device_is_ready(config->clock_dev)) {
181-
LOG_ERR("clock control device not ready");
182-
return -ENODEV;
183-
}
184-
185-
if (clock_control_get_rate(config->clock_dev, config->clock_subsys, &clock_freq)) {
186-
return -EINVAL;
190+
ret = clock_control_get_rate(config->clock_dev, config->clock_subsys, &clock_freq);
191+
if (ret) {
192+
return ret;
187193
}
188194

189195
if (data->ctx.config != NULL) {
@@ -200,10 +206,6 @@ static int spi_mcux_configure(const struct device *dev, const struct spi_config
200206
}
201207
}
202208

203-
if (IS_ENABLED(CONFIG_DEBUG)) {
204-
base->CR |= LPSPI_CR_DBGEN_MASK;
205-
}
206-
207209
data->ctx.config = spi_cfg;
208210

209211
LPSPI_MasterGetDefaultConfig(&master_config);
@@ -227,6 +229,10 @@ static int spi_mcux_configure(const struct device *dev, const struct spi_config
227229
LPSPI_MasterTransferCreateHandle(base, &data->handle, spi_mcux_master_callback, data);
228230
LPSPI_SetDummyData(base, 0);
229231

232+
if (IS_ENABLED(CONFIG_DEBUG)) {
233+
base->CR |= LPSPI_CR_DBGEN_MASK;
234+
}
235+
230236
return 0;
231237
}
232238

@@ -751,6 +757,11 @@ static int spi_mcux_init(const struct device *dev)
751757

752758
data->dev = dev;
753759

760+
if (!device_is_ready(config->clock_dev)) {
761+
LOG_ERR("clock control device not ready");
762+
return -ENODEV;
763+
}
764+
754765
if (IS_ENABLED(CONFIG_SPI_MCUX_LPSPI_DMA) && lpspi_inst_has_dma(data)) {
755766
err = lpspi_dma_devs_ready(data);
756767
}

0 commit comments

Comments
 (0)