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

Move STM32 MCO configuration from Kconfig to dts #68846

Closed
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/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ zephyr_library_sources_ifdef(CONFIG_CLOCK_CONTROL_RPI_PICO clock_cont

if(CONFIG_CLOCK_CONTROL_STM32_CUBE)
zephyr_library_sources_ifdef(CONFIG_CLOCK_STM32_MUX clock_stm32_mux.c)
zephyr_library_sources_ifdef(CONFIG_CLOCK_STM32_MCO clock_stm32_mco.c)
if(CONFIG_SOC_SERIES_STM32MP1X)
zephyr_library_sources(clock_stm32_ll_mp1.c)
elseif(CONFIG_SOC_SERIES_STM32H7X)
Expand Down
7 changes: 7 additions & 0 deletions drivers/clock_control/Kconfig.stm32
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ config CLOCK_STM32_MUX
for a specific domain. For instance per_ck clock on STM32H7 or
CLK48 clock

config CLOCK_STM32_MCO
bool "STM32 clock MCO driver"
default y
depends on DT_HAS_ST_STM32_MCO_CLOCK_ENABLED
help
Enable driver for STM32 clock MCO clock output.

# Micro-controller Clock output configuration options

choice
Expand Down
78 changes: 78 additions & 0 deletions drivers/clock_control/clock_stm32_mco.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Copyright (c) 2024 Benjamin Björnsson <[email protected]>
*
* SPDX-License-Identifier: Apache-2.0
*/

#define DT_DRV_COMPAT st_stm32_mco_clock

#include <zephyr/arch/common/sys_bitops.h>
#include <zephyr/device.h>
#include <zephyr/devicetree.h>
#include <zephyr/drivers/clock_control/stm32_clock_control.h>
#include <zephyr/drivers/pinctrl.h>

#include <stdint.h>

struct clock_control_stm32_mco_config {
uint32_t base;
const struct stm32_pclken *pclken;
size_t pclk_len;
const struct pinctrl_dev_config *pcfg;
};

static int clock_control_stm32_mco_init(const struct device *dev)
{
const struct clock_control_stm32_mco_config *config = dev->config;

for (int i = 0; i < config->pclk_len; i++) {
sys_clear_bits(config->base + STM32_CLOCK_REG_GET(config->pclken->enr),
STM32_CLOCK_MASK_GET(config->pclken[i].enr)
<< STM32_CLOCK_SHIFT_GET(config->pclken[i].enr));
sys_set_bits(config->base + STM32_CLOCK_REG_GET(config->pclken->enr),
STM32_CLOCK_VAL_GET(config->pclken[i].enr)
<< STM32_CLOCK_SHIFT_GET(config->pclken[i].enr));
}
Comment on lines +24 to +35
Copy link
Member

Choose a reason for hiding this comment

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

Altenatively, this could remain a routine of the clock_control diver (called before clock configuretion) and as such, reuse stm32_clock_control_on internal calls.


return 0;
}

#define STM32_MCO_INIT(inst) \
PINCTRL_DT_INST_DEFINE(inst); \
\
static const struct stm32_pclken pclken_##inst[] = STM32_DT_INST_CLOCKS(inst); \
\
static struct clock_control_stm32_mco_config clk_cfg_##inst = { \
.base = DT_REG_ADDR(DT_INST_CLOCKS_CTLR(inst)), \
.pclken = pclken_##inst, \
.pclk_len = DT_INST_NUM_CLOCKS(inst), \
.pcfg = PINCTRL_DT_INST_DEV_CONFIG_GET(inst), \
}; \
\
DEVICE_DT_INST_DEFINE(inst, &clock_control_stm32_mco_init, \
NULL, NULL, &clk_cfg_##inst, \
PRE_KERNEL_1, CONFIG_CLOCK_CONTROL_INIT_PRIORITY, \
NULL);

DT_INST_FOREACH_STATUS_OKAY(STM32_MCO_INIT)

#define GET_DEV(node_id) DEVICE_DT_GET(node_id),

static int stm32_mco_pinctrl_init(void)
{
const struct device *dev_list[] = {DT_FOREACH_STATUS_OKAY(st_stm32_mco_clock, GET_DEV)};
int list_len = ARRAY_SIZE(dev_list);

for (int i = 0; i < list_len; i++) {
const struct clock_control_stm32_mco_config *config = dev_list[i]->config;
int res = pinctrl_apply_state(config->pcfg, PINCTRL_STATE_DEFAULT);
if (res < 0) {

Check warning on line 69 in drivers/clock_control/clock_stm32_mco.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LINE_SPACING

drivers/clock_control/clock_stm32_mco.c:69 Missing a blank line after declarations
return res;
}
}

return 0;
}

/* Need to be initialised after GPIO driver */
SYS_INIT(stm32_mco_pinctrl_init, POST_KERNEL, CONFIG_KERNEL_INIT_PRIORITY_DEVICE);
10 changes: 10 additions & 0 deletions dts/arm/st/c0/stm32c0.dtsi
Copy link
Member

Choose a reason for hiding this comment

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

Note on the bindings:

	clocks = <&rcc 0 MCO_DIV(3)>,
		 <&rcc 0 MCO_SEL(4)>;

If you're not actually using rcc as clock controller then you don't need to specify it, and you don't need to use same cells neither, so it can just be:

	clocks = <MCO_DIV(3)>,
		 <MCO_SEL(4)>;

Or even :

	clocks = <MCO_DIV(3) MCO_SEL(4)>;

or whatever finally. This would help to get rid of the initialization priority issue.

Copy link
Member

Choose a reason for hiding this comment

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

Thinking about it more, it would be interesting to reusing the existing format and specifying the clock source:

	clocks = <&rcc STM32_SRC_PLL_Q MCO_SEL(4)>,

This way, it is clearer for the user to know what the exact clock source is (or what value has been set 2 years ago w/o opening the RM). It would also prevent to set clocks source that don't exist.
Then for prescaler, 2 options:

  • Use a dedicated property:
        st,mco-prescaler = <MCO_DIV(3)>;
  • Reuse existing rcc phandle array with a specificier:
       <&rcc PRESCALER MCO_DIV(3)>;

Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,16 @@
clock-frequency = <DT_FREQ_K(32)>;
status = "disabled";
};

clk_mco: clk-mco {
compatible = "st,stm32-mco-clock";
status = "disabled";
};

clk_mco2: clk-mco2 {
compatible = "st,stm32-mco-clock";
status = "disabled";
};
};

soc {
Expand Down
16 changes: 16 additions & 0 deletions dts/bindings/clock/st,stm32-mco-clock.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Copyright (c) 2023 Benjamin Björnsson <[email protected]>
# SPDX-License-Identifier: Apache-2.0

description: STM32 MCO Clock

compatible: "st,stm32-mco-clock"

include: [pinctrl-device.yaml, base.yaml]

properties:

pinctrl-0:
required: true

pinctrl-names:
required: true
8 changes: 8 additions & 0 deletions include/zephyr/dt-bindings/clock/stm32c0_clock.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,21 @@
(((mask) & STM32_CLOCK_MASK_MASK) << STM32_CLOCK_MASK_SHIFT) | \
(((val) & STM32_CLOCK_VAL_MASK) << STM32_CLOCK_VAL_SHIFT))

/** @brief RCC_CFGRR register offset */
#define CFGR_REG 0x08

/** @brief RCC_CCIPR register offset */
#define CCIPR_REG 0x54

/** @brief RCC_CSR1 register offset */
#define CSR1_REG 0x5C

/** @brief Device domain clocks selection helpers */
/** CFGR devices */
#define MCO_SEL(val) STM32_CLOCK(val, 3, 24, CFGR_REG)
#define MCO_DIV(val) STM32_CLOCK(val, 3, 28, CFGR_REG)
#define MCO2_SEL(val) STM32_CLOCK(val, 3, 16, CFGR_REG)
#define MCO2_DIV(val) STM32_CLOCK(val, 3, 20, CFGR_REG)
/** CCIPR devices */
#define USART1_SEL(val) STM32_CLOCK(val, 3, 0, CCIPR_REG)
#define I2C1_SEL(val) STM32_CLOCK(val, 3, 12, CCIPR_REG)
Expand Down
2 changes: 1 addition & 1 deletion west.yml
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ manifest:
groups:
- hal
- name: hal_stm32
revision: 60c9634f61c697e1c310ec648d33529712806069
revision: pull/195/head
path: modules/hal/stm32
groups:
- hal
Expand Down
Loading