Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers: spi_nrfx_spim: Add clock requests for fast SPIM instances #82994

Merged
merged 3 commits into from
Dec 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions drivers/clock_control/clock_control_nrf2_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ int nrf_clock_control_request_sync(const struct device *dev,

err = k_sem_take(&req.sem, timeout);
if (err < 0) {
nrf_clock_control_cancel_or_release(dev, spec, &req.cli);
return err;
}

Expand Down
108 changes: 105 additions & 3 deletions drivers/spi/spi_nrfx_spim.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <zephyr/cache.h>
#include <zephyr/pm/device.h>
#include <zephyr/pm/device_runtime.h>
#include <zephyr/drivers/clock_control/nrf_clock_control.h>
#include <zephyr/drivers/pinctrl.h>
#include <zephyr/mem_mgmt/mem_attr.h>
#include <soc.h>
Expand Down Expand Up @@ -42,6 +43,16 @@ LOG_MODULE_REGISTER(spi_nrfx_spim, CONFIG_SPI_LOG_LEVEL);
#define SPI_BUFFER_IN_RAM 1
#endif

#if defined(CONFIG_CLOCK_CONTROL_NRF2_GLOBAL_HSFLL) && \
(defined(CONFIG_HAS_HW_NRF_SPIM120) || \
defined(CONFIG_HAS_HW_NRF_SPIM121))
#define SPIM_REQUESTS_CLOCK(idx) UTIL_OR(IS_EQ(idx, 120), \
IS_EQ(idx, 121))
#define USE_CLOCK_REQUESTS 1
#else
#define SPIM_REQUESTS_CLOCK(idx) 0
#endif

struct spi_nrfx_data {
struct spi_context ctx;
const struct device *dev;
Expand All @@ -57,6 +68,9 @@ struct spi_nrfx_data {
uint8_t ppi_ch;
uint8_t gpiote_ch;
#endif
#ifdef USE_CLOCK_REQUESTS
bool clock_requested;
#endif
};

struct spi_nrfx_config {
Expand All @@ -74,10 +88,59 @@ struct spi_nrfx_config {
#ifdef CONFIG_DCACHE
uint32_t mem_attr;
#endif
#ifdef USE_CLOCK_REQUESTS
const struct device *clk_dev;
struct nrf_clock_spec clk_spec;
#endif
};

static void event_handler(const nrfx_spim_evt_t *p_event, void *p_context);

static inline int request_clock(const struct device *dev)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't the compiler complain about unused dev parameter when USE_CLOCK_REQUESTS is undefined? same for release_clock

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it may complain, depending on the enabled warnings. I'll add ARG_UNUSED() macro calls.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

{
#ifdef USE_CLOCK_REQUESTS
struct spi_nrfx_data *dev_data = dev->data;
const struct spi_nrfx_config *dev_config = dev->config;
int error;

if (!dev_config->clk_dev) {
return 0;
}

error = nrf_clock_control_request_sync(
dev_config->clk_dev, &dev_config->clk_spec,
K_MSEC(CONFIG_SPI_COMPLETION_TIMEOUT_TOLERANCE));
if (error < 0) {
LOG_ERR("Failed to request clock: %d", error);
return error;
}

dev_data->clock_requested = true;
#else
ARG_UNUSED(dev);
#endif

return 0;
}

static inline void release_clock(const struct device *dev)
{
#ifdef USE_CLOCK_REQUESTS
struct spi_nrfx_data *dev_data = dev->data;
const struct spi_nrfx_config *dev_config = dev->config;

if (!dev_data->clock_requested) {
return;
}

dev_data->clock_requested = false;

nrf_clock_control_release(dev_config->clk_dev, &dev_config->clk_spec);
#else
ARG_UNUSED(dev);
#endif
}

static inline void finalize_spi_transaction(const struct device *dev, bool deactivate_cs)
{
struct spi_nrfx_data *dev_data = dev->data;
Expand All @@ -92,6 +155,10 @@ static inline void finalize_spi_transaction(const struct device *dev, bool deact
nrfy_spim_disable(reg);
}

if (!IS_ENABLED(CONFIG_PM_DEVICE_RUNTIME)) {
release_clock(dev);
}

pm_device_runtime_put_async(dev, K_NO_WAIT);
}

Expand Down Expand Up @@ -467,6 +534,11 @@ static int transceive(const struct device *dev,
spi_context_lock(&dev_data->ctx, asynchronous, cb, userdata, spi_cfg);

error = configure(dev, spi_cfg);

if (error == 0 && !IS_ENABLED(CONFIG_PM_DEVICE_RUNTIME)) {
error = request_clock(dev);
}

if (error == 0) {
dev_data->busy = true;

Expand Down Expand Up @@ -518,6 +590,8 @@ static int transceive(const struct device *dev,
} else if (error) {
finalize_spi_transaction(dev, true);
}
} else {
pm_device_runtime_put(dev);
}

spi_context_release(&dev_data->ctx, error);
Expand Down Expand Up @@ -575,7 +649,7 @@ static DEVICE_API(spi, spi_nrfx_driver_api) = {
.release = spi_nrfx_release,
};

static void spim_resume(const struct device *dev)
static int spim_resume(const struct device *dev)
{
const struct spi_nrfx_config *dev_config = dev->config;

Expand All @@ -587,6 +661,8 @@ static void spim_resume(const struct device *dev)
#ifdef CONFIG_SOC_NRF54H20_GPD
nrf_gpd_retain_pins_set(dev_config->pcfg, false);
#endif

return IS_ENABLED(CONFIG_PM_DEVICE_RUNTIME) ? request_clock(dev) : 0;
}

static void spim_suspend(const struct device *dev)
Expand All @@ -599,6 +675,10 @@ static void spim_suspend(const struct device *dev)
dev_data->initialized = false;
}

if (IS_ENABLED(CONFIG_PM_DEVICE_RUNTIME)) {
release_clock(dev);
}

#ifdef CONFIG_SOC_NRF54H20_GPD
nrf_gpd_retain_pins_set(dev_config->pcfg, true);
#endif
Expand All @@ -609,7 +689,7 @@ static void spim_suspend(const struct device *dev)
static int spim_nrfx_pm_action(const struct device *dev, enum pm_device_action action)
{
if (action == PM_DEVICE_ACTION_RESUME) {
spim_resume(dev);
return spim_resume(dev);
} else if (IS_ENABLED(CONFIG_PM_DEVICE) && (action == PM_DEVICE_ACTION_SUSPEND)) {
spim_suspend(dev);
} else {
Expand Down Expand Up @@ -685,6 +765,21 @@ static int spi_nrfx_init(const struct device *dev)
(0))), \
(0))

/* Fast instances depend on the global HSFLL clock controller (as they need
* to request the highest frequency from it to operate correctly), so they
* must be initialized after that controller driver, hence the default SPI
* initialization priority may be too early for them.
*/
#if defined(CONFIG_CLOCK_CONTROL_NRF2_GLOBAL_HSFLL_INIT_PRIORITY) && \
CONFIG_SPI_INIT_PRIORITY < CONFIG_CLOCK_CONTROL_NRF2_GLOBAL_HSFLL_INIT_PRIORITY
#define SPIM_INIT_PRIORITY(idx) \
COND_CODE_1(SPIM_REQUESTS_CLOCK(idx), \
(UTIL_INC(CONFIG_CLOCK_CONTROL_NRF2_GLOBAL_HSFLL_INIT_PRIORITY)), \
(CONFIG_SPI_INIT_PRIORITY))
#else
#define SPIM_INIT_PRIORITY(idx) CONFIG_SPI_INIT_PRIORITY
#endif

#define SPI_NRFX_SPIM_DEFINE(idx) \
NRF_DT_CHECK_NODE_HAS_PINCTRL_SLEEP(SPIM(idx)); \
static void irq_connect##idx(void) \
Expand Down Expand Up @@ -735,6 +830,13 @@ static int spi_nrfx_init(const struct device *dev)
.wake_gpiote = WAKE_GPIOTE_INSTANCE(SPIM(idx)), \
IF_ENABLED(CONFIG_DCACHE, \
(.mem_attr = SPIM_GET_MEM_ATTR(idx),)) \
IF_ENABLED(USE_CLOCK_REQUESTS, \
(.clk_dev = SPIM_REQUESTS_CLOCK(idx) \
? DEVICE_DT_GET(DT_CLOCKS_CTLR(SPIM(idx))) \
: NULL, \
.clk_spec = { \
.frequency = NRF_CLOCK_CONTROL_FREQUENCY_MAX, \
},)) \
}; \
BUILD_ASSERT(!SPIM_HAS_PROP(idx, wake_gpios) || \
!(DT_GPIO_FLAGS(SPIM(idx), wake_gpios) & GPIO_ACTIVE_LOW),\
Expand All @@ -745,7 +847,7 @@ static int spi_nrfx_init(const struct device *dev)
PM_DEVICE_DT_GET(SPIM(idx)), \
&spi_##idx##_data, \
&spi_##idx##z_config, \
POST_KERNEL, CONFIG_SPI_INIT_PRIORITY, \
POST_KERNEL, SPIM_INIT_PRIORITY(idx), \
&spi_nrfx_driver_api)

#define SPIM_MEMORY_SECTION(idx) \
Expand Down
2 changes: 2 additions & 0 deletions dts/common/nordic/nrf54h20.dtsi
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,7 @@
power-domains = <&gpd NRF_GPD_FAST_ACTIVE1>;
easydma-maxcnt-bits = <15>;
interrupts = <230 NRF_DEFAULT_IRQ_PRIORITY>;
clocks = <&hsfll120>;
max-frequency = <DT_FREQ_M(32)>;
#address-cells = <1>;
#size-cells = <0>;
Expand All @@ -674,6 +675,7 @@
status = "disabled";
easydma-maxcnt-bits = <15>;
interrupts = <231 NRF_DEFAULT_IRQ_PRIORITY>;
clocks = <&hsfll120>;
power-domains = <&gpd NRF_GPD_FAST_ACTIVE1>;
max-frequency = <DT_FREQ_M(32)>;
#address-cells = <1>;
Expand Down
Loading