stm32h7: refactor and simplify clock helper functions

rockbox-gerrit-noreply--- via rockbox-cvs <[email protected]>
Newsgroups gmane.comp.systems.archos.rockbox.cvs
Message-ID <[email protected]> 
commit 386be9dfcc72a0c2b3786a7e1ee8ce55676c93bc
Author: Aidan MacDonald <[email protected]>
Date:   Fri Jan 16 14:17:02 2026 +0000

    stm32h7: refactor and simplify clock helper functions
    
    The clock helpers are only used for leaf clocks of single
    peripherals, which don't benefit from reference counting.
    
    Change-Id: Ica5685e7bc0fce621ae46f758f0ad0b1dcfb2789

diff --git a/firmware/SOURCES b/firmware/SOURCES
index 6be052de85..9279307af5 100644
--- a/firmware/SOURCES
+++ b/firmware/SOURCES
@@ -2024,7 +2024,6 @@ target/arm/rk27xx/ihifi2/audio-ihifi800.c
 target/arm/stm32/crt0-stm32h7.S
 target/arm/stm32/vectors-stm32h7.S
 target/arm/stm32/adc-stm32h7.c
-target/arm/stm32/clock-stm32h7.c
 target/arm/stm32/debug-stm32h7.c
 target/arm/stm32/gpio-stm32h7.c
 target/arm/stm32/i2c-stm32h7.c
diff --git a/firmware/target/arm/stm32/clock-stm32h7.c b/firmware/target/arm/stm32/clock-stm32h7.c
deleted file mode 100644
index ec1ff106b2..0000000000
--- a/firmware/target/arm/stm32/clock-stm32h7.c
+++ /dev/null
@@ -1,65 +0,0 @@
-/***************************************************************************
- *             __________               __   ___.
- *   Open      \______   \ ____   ____ |  | _\_ |__   _______  ___
- *   Source     |       _//  _ \_/ ___\|  |/ /| __ \ /  _ \  \/  /
- *   Jukebox    |    |   (  <_> )  \___|    < | \_\ (  <_> > <  <
- *   Firmware   |____|_  /\____/ \___  >__|_ \|___  /\____/__/\_ \
- *                     \/            \/     \/    \/            \/
- * $Id$
- *
- * Copyright (C) 2025 Aidan MacDonald
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version 2
- * of the License, or (at your option) any later version.
- *
- * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
- * KIND, either express or implied.
- *
- ****************************************************************************/
-#include "clock-stm32h7.h"
-#include "mutex.h"
-#include "panic.h"
-#include <stdint.h>
-
-struct stm_clock_state
-{
-    struct mutex mutex;
-    uint8_t refcount[STM_NUM_CLOCKS];
-};
-
-static struct stm_clock_state stm_clocks;
-
-void stm_clock_init(void)
-{
-    mutex_init(&stm_clocks.mutex);
-
-    stm_target_clock_init();
-}
-
-void stm_clock_enable(enum stm_clock clock)
-{
-    mutex_lock(&stm_clocks.mutex);
-
-    if (stm_clocks.refcount[clock] == UINT8_MAX)
-        panicf("%s: clock %d overflow", __func__, (int)clock);
-
-    if (stm_clocks.refcount[clock]++ == 0)
-        stm_target_clock_enable(clock, true);
-
-    mutex_unlock(&stm_clocks.mutex);
-}
-
-void stm_clock_disable(enum stm_clock clock)
-{
-    mutex_lock(&stm_clocks.mutex);
-
-    if (stm_clocks.refcount[clock] == 0)
-        panicf("%s: clock %d underflow", __func__, (int)clock);
-
-    if (--stm_clocks.refcount[clock] == 0)
-        stm_target_clock_enable(clock, false);
-
-    mutex_unlock(&stm_clocks.mutex);
-}
diff --git a/firmware/target/arm/stm32/clock-stm32h7.h b/firmware/target/arm/stm32/clock-stm32h7.h
index 7e78d30ecf..e6dbd44c53 100644
--- a/firmware/target/arm/stm32/clock-stm32h7.h
+++ b/firmware/target/arm/stm32/clock-stm32h7.h
@@ -25,18 +25,15 @@
 #include <stdbool.h>
 #include <stddef.h>
 
-enum stm_clock
+struct stm32_clock
 {
-    STM_CLOCK_SPI1_KER,
-    STM_CLOCK_SPI2_KER,
-    STM_CLOCK_SPI3_KER,
-    STM_CLOCK_SPI4_KER,
-    STM_CLOCK_SPI5_KER,
-    STM_CLOCK_SPI6_KER,
-    STM_CLOCK_LTDC_KER,
-    STM_CLOCK_SDMMC1_KER,
-    STM_CLOCK_SDMMC2_KER,
-    STM_NUM_CLOCKS,
+    uint32_t frequency;
+
+    uint32_t en_reg;
+    uint32_t en_bit;
+
+    uint32_t lpen_reg;
+    uint32_t lpen_bit;
 };
 
 /*
@@ -47,43 +44,43 @@ enum stm_clock
 void stm_target_clock_init(void) INIT_ATTR;
 
 /*
- * Callback to be implemented by the target when the hardware
- * clock needs to be turned on or off. Clocks are internally
- * reference counted so only the first / last user will change
- * the hardware state.
- *
- * Only clocks that are actually used need to be implemented,
- * and unless otherwise noted it is allowed for enable/disable
- * to be a no-op if the clock is always enabled.
+ * Called by system_init() to setup target clocks.
  */
-void stm_target_clock_enable(enum stm_clock clock, bool enable);
+static inline void stm_clock_init(void)
+{
+    stm_target_clock_init();
+}
 
 /*
- * Callback to return a specific clock's frequency. For most
- * peripherals the frequency must be known at initialization
- * and not change afterwards; see peripheral drivers for the
- * details, as their exact requirements may vary.
+ * Enables a clock by setting its enable bits in the RCC.
  */
-size_t stm_target_clock_get_frequency(enum stm_clock clock);
+static inline void stm32_clock_enable(const struct stm32_clock *clk)
+{
+    if (clk->en_reg)
+        *(volatile uint32_t *)clk->en_reg |= clk->en_bit;
 
-/*
- * Called from system_init(). Sets up internal book-keeping
- * and then calls stm_target_clock_init().
- */
-void stm_clock_init(void) INIT_ATTR;
+    if (clk->lpen_reg)
+        *(volatile uint32_t *)clk->lpen_reg |= clk->lpen_bit;
+}
 
 /*
- * Enable or disable a clock. Not safe to call from an IRQ handler.
+ * Disables a clock in the RCC.
  */
-void stm_clock_enable(enum stm_clock clock);
-void stm_clock_disable(enum stm_clock clock);
+static inline void stm32_clock_disable(const struct stm32_clock *clk)
+{
+    if (clk->en_reg)
+        *(volatile uint32_t *)clk->en_reg &= ~clk->en_bit;
+
+    if (clk->lpen_reg)
+        *(volatile uint32_t *)clk->lpen_reg &= ~clk->lpen_bit;
+}
 
 /*
  * Get a clock's frequency in Hz.
  */
-static inline size_t stm_clock_get_frequency(enum stm_clock clock)
+static inline uint32_t stm32_clock_get_frequency(const struct stm32_clock *clk)
 {
-    return stm_target_clock_get_frequency(clock);
+    return clk->frequency;
 }
 
 #endif /* __CLOCK_STM32H7_H__ */
diff --git a/firmware/target/arm/stm32/echoplayer/clock-echoplayer.c b/firmware/target/arm/stm32/echoplayer/clock-echoplayer.c
index fcf26ae3dc..2ff829bfd8 100644
--- a/firmware/target/arm/stm32/echoplayer/clock-echoplayer.c
+++ b/firmware/target/arm/stm32/echoplayer/clock-echoplayer.c
@@ -180,43 +180,26 @@ void stm_target_clock_init(void)
     init_periph_clock();
 }
 
-void stm_target_clock_enable(enum stm_clock clock, bool enable)
-{
-    switch (clock)
-    {
-    case STM_CLOCK_SPI5_KER:
-        reg_writef(RCC_APB2ENR, SPI5EN(enable));
-        reg_writef(RCC_APB2LPENR, SPI5EN(enable));
-        break;
-
-    case STM_CLOCK_LTDC_KER:
-        reg_writef(RCC_APB3ENR, LTDCEN(enable));
-        reg_writef(RCC_APB3LPENR, LTDCEN(enable));
-        break;
-
-    case STM_CLOCK_SDMMC1_KER:
-        reg_writef(RCC_AHB3ENR, SDMMC1EN(enable));
-        reg_writef(RCC_AHB3LPENR, SDMMC1EN(enable));
-        break;
-
-    default:
-        panicf("%s: unsupported clock %d", __func__, (int)clock);
-        break;
-    }
-}
-
-size_t stm_target_clock_get_frequency(enum stm_clock clock)
-{
-    switch (clock)
-    {
-    case STM_CLOCK_SPI5_KER:
-        return STM32_HSE_FREQ;
-
-    case STM_CLOCK_SDMMC1_KER:
-        return PLL1Q_FREQ;
-
-    default:
-        panicf("%s: unsupported clock %d", __func__, (int)clock);
-        return 0;
-    }
-}
+const struct stm32_clock sdmmc1_ker_clock = {
+    .frequency = PLL1Q_FREQ,
+    .en_reg = ITA_RCC_AHB3ENR,
+    .en_bit = BM_RCC_AHB3ENR_SDMMC1EN,
+    .lpen_reg = ITA_RCC_AHB3LPENR,
+    .lpen_bit = BM_RCC_AHB3LPENR_SDMMC1EN,
+};
+
+const struct stm32_clock ltdc_ker_clock = {
+    .frequency = LCD_DOTCLOCK_FREQ,
+    .en_reg = ITA_RCC_APB3ENR,
+    .en_bit = BM_RCC_APB3ENR_LTDCEN,
+    .lpen_reg = ITA_RCC_APB3LPENR,
+    .lpen_bit = BM_RCC_APB3ENR_LTDCEN,
+};
+
+const struct stm32_clock spi5_ker_clock = {
+    .frequency = STM32_HSE_FREQ,
+    .en_reg = ITA_RCC_APB2ENR,
+    .en_bit = BM_RCC_APB2ENR_SPI5EN,
+    .lpen_reg = ITA_RCC_APB2LPENR,
+    .lpen_bit = BM_RCC_APB2ENR_SPI5EN,
+};
diff --git a/firmware/target/arm/stm32/echoplayer/clock-echoplayer.h b/firmware/target/arm/stm32/echoplayer/clock-echoplayer.h
new file mode 100644
index 0000000000..cea54441c0
--- /dev/null
+++ b/firmware/target/arm/stm32/echoplayer/clock-echoplayer.h
@@ -0,0 +1,30 @@
+/***************************************************************************
+ *             __________               __   ___.
+ *   Open      \______   \ ____   ____ |  | _\_ |__   _______  ___
+ *   Source     |       _//  _ \_/ ___\|  |/ /| __ \ /  _ \  \/  /
+ *   Jukebox    |    |   (  <_> )  \___|    < | \_\ (  <_> > <  <
+ *   Firmware   |____|_  /\____/ \___  >__|_ \|___  /\____/__/\_ \
+ *                     \/            \/     \/    \/            \/
+ * $Id$
+ *
+ * Copyright (C) 2026 Aidan MacDonald
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ ****************************************************************************/
+#ifndef __CLOCK_ECHOPLAYER_H__
+#define __CLOCK_ECHOPLAYER_H__
+
+#include "clock-stm32h7.h"
+
+extern struct stm32_clock sdmmc1_ker_clock;
+extern struct stm32_clock ltdc_ker_clock;
+extern struct stm32_clock spi5_ker_clock;
+
+#endif /* __CLOCK_ECHOPLAYER_H__ */
diff --git a/firmware/target/arm/stm32/echoplayer/lcd-echoplayer.c b/firmware/target/arm/stm32/echoplayer/lcd-echoplayer.c
index ea2febeff6..947342b1fb 100644
--- a/firmware/target/arm/stm32/echoplayer/lcd-echoplayer.c
+++ b/firmware/target/arm/stm32/echoplayer/lcd-echoplayer.c
@@ -22,6 +22,7 @@
 #include "kernel.h"
 #include "lcd.h"
 #include "lcd-echoplayer.h"
+#include "clock-echoplayer.h"
 #include "nvic-arm.h"
 #include "spi-stm32h7.h"
 #include "gpio-stm32h7.h"
@@ -40,7 +41,7 @@
 
 struct stm_spi_config spi_cfg = {
     .instance = ITA_SPI5,
-    .clock = STM_CLOCK_SPI5_KER,
+    .clock = &spi5_ker_clock,
     .freq = LCD_SPI_FREQ,
     .mode = STM_SPIMODE_HALF_DUPLEX,
     .proto = STM_SPIPROTO_MOTOROLA,
@@ -61,7 +62,7 @@ struct stm_spi spi;
 static void init_ltdc(void)
 {
     /* Enable LTDC clock */
-    stm_clock_enable(STM_CLOCK_LTDC_KER);
+    stm32_clock_enable(&ltdc_ker_clock);
 
     /* Set timing parameters */
     const uint32_t hsw  = LCD_HSW - 1;
diff --git a/firmware/target/arm/stm32/echoplayer/sdmmc-echoplayer.c b/firmware/target/arm/stm32/echoplayer/sdmmc-echoplayer.c
index 5120cca473..7a192bc914 100644
--- a/firmware/target/arm/stm32/echoplayer/sdmmc-echoplayer.c
+++ b/firmware/target/arm/stm32/echoplayer/sdmmc-echoplayer.c
@@ -19,6 +19,7 @@
  *
  ****************************************************************************/
 #include "sdmmc_host.h"
+#include "clock-echoplayer.h"
 #include "sdmmc-stm32h7.h"
 #include "gpio-stm32h7.h"
 #include "nvic-arm.h"
@@ -95,7 +96,7 @@ static struct sdmmc_poll sdcard_poll;
 void sdmmc_host_target_init(void)
 {
     /* Initialize controller */
-    stm32h7_sdmmc_init(&sdmmc1_ctl, ITA_SDMMC1, STM_CLOCK_SDMMC1_KER,
+    stm32h7_sdmmc_init(&sdmmc1_ctl, ITA_SDMMC1, &sdmmc1_ker_clock,
                        stm32h7_reset_sdmmc1, NULL);
     nvic_enable_irq(NVIC_IRQN_SDMMC1);
 
diff --git a/firmware/target/arm/stm32/sdmmc-stm32h7.c b/firmware/target/arm/stm32/sdmmc-stm32h7.c
index 4147c0a95a..0fdad1156a 100644
--- a/firmware/target/arm/stm32/sdmmc-stm32h7.c
+++ b/firmware/target/arm/stm32/sdmmc-stm32h7.c
@@ -80,7 +80,7 @@ void stm32h7_reset_sdmmc1(void)
 
 void stm32h7_sdmmc_init(struct stm32h7_sdmmc_controller *ctl,
                         uint32_t instance,
-                        enum stm_clock clock,
+                        const struct stm32_clock *clock,
                         void (*reset_sdmmc)(void),
                         void (*vcc_enable)(bool))
 {
@@ -109,7 +109,7 @@ void stm32h7_sdmmc_set_power_enabled(void *controller, bool enabled)
         sleep(1);
 
         /* Bus clock is now needed, so enable kernel clock */
-        stm_clock_enable(ctl->clock);
+        stm32_clock_enable(ctl->clock);
 
         /* Configure bus parameters */
         stm32h7_sdmmc_set_bus_width(ctl, SDMMC_BUS_WIDTH_1BIT);
@@ -136,7 +136,7 @@ void stm32h7_sdmmc_set_power_enabled(void *controller, bool enabled)
          * and the bus is powered down; some quick testing shows this
          * seems to be true.
          */
-        stm_clock_disable(ctl->clock);
+        stm32_clock_disable(ctl->clock);
 
         /* Disable VCC */
         if (ctl->vcc_enable)
@@ -174,7 +174,7 @@ void stm32h7_sdmmc_set_bus_clock(void *controller, uint32_t clock)
     if (stm32h7_sdmmc_is_powered_off(ctl))
         return;
 
-    size_t ker_freq = stm_clock_get_frequency(ctl->clock);
+    size_t ker_freq = stm32_clock_get_frequency(ctl->clock);
     size_t bus_freq = get_sdmmc_bus_freq(clock);
     if (!bus_freq)
         panicf("%s", __func__);
diff --git a/firmware/target/arm/stm32/sdmmc-stm32h7.h b/firmware/target/arm/stm32/sdmmc-stm32h7.h
index 95f3ab0cbc..7aeed251e2 100644
--- a/firmware/target/arm/stm32/sdmmc-stm32h7.h
+++ b/firmware/target/arm/stm32/sdmmc-stm32h7.h
@@ -31,7 +31,7 @@ struct stm32h7_sdmmc_controller
     uint32_t regs;
 
     /* SDMMC kernel clock */
-    enum stm_clock clock;
+    const struct stm32_clock *clock;
 
     /* Callback to reset SDMMC instance in RCC */
     void (*reset_sdmmc)(void);
@@ -59,7 +59,7 @@ void stm32h7_reset_sdmmc1(void);
 
 void stm32h7_sdmmc_init(struct stm32h7_sdmmc_controller *controller,
                         uint32_t instance,
-                        enum stm_clock clock,
+                        const struct stm32_clock *clock,
                         void (*reset_sdmmc)(void),
                         void (*vcc_enable)(bool));
 
diff --git a/firmware/target/arm/stm32/spi-stm32h7.c b/firmware/target/arm/stm32/spi-stm32h7.c
index 941310f90d..fc4d87eafc 100644
--- a/firmware/target/arm/stm32/spi-stm32h7.c
+++ b/firmware/target/arm/stm32/spi-stm32h7.c
@@ -37,7 +37,7 @@ static void stm_spi_enable(struct stm_spi *spi, bool hd_tx, size_t size)
     if (tsize > TSIZE_MAX)
         panicf("%s: tsize > TSIZE_MAX", __func__);
 
-    stm_clock_enable(spi->clock);
+    stm32_clock_enable(spi->clock);
 
     if (spi->set_cs)
         spi->set_cs(spi, true);
@@ -59,7 +59,7 @@ static void stm_spi_disable(struct stm_spi *spi)
     if (spi->set_cs)
         spi->set_cs(spi, false);
 
-    stm_clock_disable(spi->clock);
+    stm32_clock_disable(spi->clock);
 }
 
 static uint32_t stm_spi_pack(const void **bufp, size_t *sizep)
@@ -110,7 +110,7 @@ static void stm_spi_unpack(void **bufp, size_t *sizep, uint32_t data)
 
 static uint32_t stm_spi_calc_mbr(const struct stm_spi_config *config)
 {
-    size_t ker_freq = stm_clock_get_frequency(config->clock);
+    size_t ker_freq = stm32_clock_get_frequency(config->clock);
     for (uint32_t mbr = 0; mbr <= 7; mbr++)
     {
         if (ker_freq / (2 << mbr) <= config->freq)
@@ -162,7 +162,7 @@ void stm_spi_init(struct stm_spi *spi,
         ftlevel *= 2;
     }
 
-    stm_clock_enable(spi->clock);
+    stm32_clock_enable(spi->clock);
 
     /* TODO: allow setting MBR here */
     reg_writelf(spi->regs, SPI_CFG1,
@@ -191,7 +191,7 @@ void stm_spi_init(struct stm_spi *spi,
                 MIDI(0),
                 MSSI(0));
 
-    stm_clock_disable(spi->clock);
+    stm32_clock_disable(spi->clock);
 }
 
 int stm_spi_xfer(struct stm_spi *spi, size_t size,
diff --git a/firmware/target/arm/stm32/spi-stm32h7.h b/firmware/target/arm/stm32/spi-stm32h7.h
index dbc6a2fdaa..a6b0ef11d0 100644
--- a/firmware/target/arm/stm32/spi-stm32h7.h
+++ b/firmware/target/arm/stm32/spi-stm32h7.h
@@ -58,7 +58,7 @@ struct stm_spi_config
      * such the kernel clock should not be changed after
      * the SPI peripheral is initialized.
      */
-    enum stm_clock clock;
+    const struct stm32_clock *clock;
     size_t freq;
 
     enum stm_spi_mode mode;
@@ -77,7 +77,7 @@ struct stm_spi_config
 struct stm_spi
 {
     uint32_t regs;
-    enum stm_clock clock;
+    const struct stm32_clock *clock;
     enum stm_spi_mode mode;
     stm_spi_set_cs_t set_cs;
     uint32_t frame_size;
-- 
rockbox-cvs mailing list
[email protected]
https://lists.haxx.se/mailman/listinfo/rockbox-cvs
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.