Hi Maíra,
thanks for working on this.
Am 28.07.25 um 14:35 schrieb Maíra Canal:
Currently, when we prepare or unprepare RPi's clocks, we don't actually
enable/disable the firmware clock. This means that
`clk_disable_unprepare()` doesn't actually change the clock state at
all, nor does it lowers the clock rate.
From the Mailbox Property Interface documentation [1], we can see that
we should use `RPI_FIRMWARE_SET_CLOCK_STATE` to set the clock state
off/on. Therefore, use `RPI_FIRMWARE_SET_CLOCK_STATE` to create a
prepare and an unprepare hook for RPi's firmware clock.
As now the clocks are actually turned off, some of them are now marked
with CLK_IGNORE_UNUSED or CLK_IS_CRITICAL, as those are required since
early boot or are required during reboot.
Link: https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface
[1]
Fixes: 93d2725affd6 ("clk: bcm: rpi: Discover the firmware clocks")
could you please explain from user perspective, which issue is fixed by
this patch?
Why does this needs to be backported?
Signed-off-by: Maíra Canal <mca...@igalia.com>
---
drivers/clk/bcm/clk-raspberrypi.c | 41 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/bcm/clk-raspberrypi.c
b/drivers/clk/bcm/clk-raspberrypi.c
index
8e4fde03ed232b464165f524d27744b4ced93a60..a2bd5040283a2f456760bd685e696b423985cac0
100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -68,6 +68,7 @@ struct raspberrypi_clk_variant {
char *clkdev;
unsigned long min_rate;
bool minimize;
+ u32 flags;
};
static struct raspberrypi_clk_variant
@@ -75,6 +76,7 @@ raspberrypi_clk_variants[RPI_FIRMWARE_NUM_CLK_ID] = {
[RPI_FIRMWARE_ARM_CLK_ID] = {
.export = true,
.clkdev = "cpu0",
+ .flags = CLK_IGNORE_UNUSED,
},
[RPI_FIRMWARE_CORE_CLK_ID] = {
.export = true,
@@ -90,6 +92,7 @@ raspberrypi_clk_variants[RPI_FIRMWARE_NUM_CLK_ID] = {
* always use the minimum the drivers will let us.
*/
.minimize = true,
+ .flags = CLK_IGNORE_UNUSED,
},
[RPI_FIRMWARE_M2MC_CLK_ID] = {
.export = true,
@@ -115,6 +118,7 @@ raspberrypi_clk_variants[RPI_FIRMWARE_NUM_CLK_ID] = {
* drivers will let us.
*/
.minimize = true,
+ .flags = CLK_IGNORE_UNUSED,
},
[RPI_FIRMWARE_V3D_CLK_ID] = {
.export = true,
@@ -127,6 +131,7 @@ raspberrypi_clk_variants[RPI_FIRMWARE_NUM_CLK_ID] = {
[RPI_FIRMWARE_HEVC_CLK_ID] = {
.export = true,
.minimize = true,
+ .flags = CLK_IGNORE_UNUSED,
},
[RPI_FIRMWARE_ISP_CLK_ID] = {
.export = true,
@@ -135,6 +140,7 @@ raspberrypi_clk_variants[RPI_FIRMWARE_NUM_CLK_ID] = {
[RPI_FIRMWARE_PIXEL_BVB_CLK_ID] = {
.export = true,
.minimize = true,
+ .flags = CLK_IS_CRITICAL,
},
[RPI_FIRMWARE_VEC_CLK_ID] = {
.export = true,
@@ -259,7 +265,40 @@ static int raspberrypi_fw_dumb_determine_rate(struct
clk_hw *hw,
return 0;
}
+static int raspberrypi_fw_prepare(struct clk_hw *hw)
+{
+ const struct raspberrypi_clk_data *data = clk_hw_to_data(hw);
+ struct raspberrypi_clk *rpi = data->rpi;
+ u32 state = RPI_FIRMWARE_STATE_ENABLE_BIT;
+ int ret;
+
+ ret = raspberrypi_clock_property(rpi->firmware, data,
+ RPI_FIRMWARE_SET_CLOCK_STATE, &state);
+ if (ret)
+ dev_err(rpi->dev, "Failed to set clock %d state to on: %d",
+ data->id, ret);
I suggest to use dev_err_ratelimited for prepare/unprepare, otherwise
this could spam the kernel log.
Furthermore i wouldn't recommend to log some magic clock id. How about
using clk_hw_get_name(hw) instead?
Don't we need a newline character at the end?
+
+ return ret;
+}
+
+static void raspberrypi_fw_unprepare(struct clk_hw *hw)
+{
+ const struct raspberrypi_clk_data *data = clk_hw_to_data(hw);
+ struct raspberrypi_clk *rpi = data->rpi;
+ u32 state = 0;
+ int ret;
+
+ ret = raspberrypi_clock_property(rpi->firmware, data,
+ RPI_FIRMWARE_SET_CLOCK_STATE, &state);
+ if (ret)
+ dev_err(rpi->dev, "Failed to set clock %d state to off: %d",
+ data->id, ret);
see above
Best regards
+}
+
+
static const struct clk_ops raspberrypi_firmware_clk_ops = {
+ .prepare = raspberrypi_fw_prepare,
+ .unprepare = raspberrypi_fw_unprepare,
.is_prepared = raspberrypi_fw_is_prepared,
.recalc_rate = raspberrypi_fw_get_rate,
.determine_rate = raspberrypi_fw_dumb_determine_rate,
@@ -289,7 +328,7 @@ static struct clk_hw *raspberrypi_clk_register(struct
raspberrypi_clk *rpi,
if (!init.name)
return ERR_PTR(-ENOMEM);
init.ops = &raspberrypi_firmware_clk_ops;
- init.flags = CLK_GET_RATE_NOCACHE;
+ init.flags = variant->flags | CLK_GET_RATE_NOCACHE;
data->hw.init = &init;
