Hello Simon and all,
On Sun, 26 Jul 2020 at 16:54, Simon Glass sjg@chromium.org wrote:
Hi Etienne,
On Fri, 17 Jul 2020 at 09:43, Etienne Carriere etienne.carriere@linaro.org wrote:
This change introduces a clock driver for SCMI agent devices. When SCMI agent and SCMI clock drivers are enabled, SCMI agent binds a clock device for each SCMI clock protocol devices enabled in the FDT.
SCMI clock driver is embedded upon CONFIG_CLK_SCMI=y. If enabled, CONFIG_SCMI_AGENT is also enabled.
SCMI Clock protocol is defined in the SCMI specification [1].
Links: [1] https://developer.arm.com/architectures/system-architectures/software-standa...
That doesn't seem to work for me (404 error)
I think that Arm site was broken when you tried. Works fine and this seems to be the official Arm URL for the SCMI specification.
Signed-off-by: Etienne Carriere etienne.carriere@linaro.org
drivers/clk/Kconfig | 8 +++ drivers/clk/Makefile | 1 + drivers/clk/clk_scmi.c | 152 ++++++++++++++++++++++++++++++++++++++++ drivers/firmware/scmi.c | 3 + 4 files changed, 164 insertions(+) create mode 100644 drivers/clk/clk_scmi.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 82cb1874e19..234d6035202 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -152,6 +152,14 @@ config CLK_CDCE9XX Enable the clock synthesizer driver for CDCE913/925/937/949 series of chips.
+config CLK_SCMI
bool "Enable SCMI clock driver"select SCMI_FIRMWAREperhaps 'depends on' would be better?
Ok
helpEnable this option if you want to support clock devices exposedby a SCMI agent based on SCMI clock protocol communicationwith a SCMI server.source "drivers/clk/analogbits/Kconfig" source "drivers/clk/at91/Kconfig" source "drivers/clk/exynos/Kconfig" diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index d9119545810..76bba77d1f0 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -31,6 +31,7 @@ obj-$(CONFIG_CLK_K210) += kendryte/ obj-$(CONFIG_CLK_MPC83XX) += mpc83xx_clk.o obj-$(CONFIG_CLK_OWL) += owl/ obj-$(CONFIG_CLK_RENESAS) += renesas/ +obj-$(CONFIG_CLK_SCMI) += clk_scmi.o obj-$(CONFIG_CLK_SIFIVE) += sifive/ obj-$(CONFIG_ARCH_SUNXI) += sunxi/ obj-$(CONFIG_CLK_STM32F) += clk_stm32f.o diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c new file mode 100644 index 00000000000..efe64a6a38f --- /dev/null +++ b/drivers/clk/clk_scmi.c @@ -0,0 +1,152 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2019-2020 Linaro Limited
- */
+#include <common.h> +#include <clk-uclass.h> +#include <dm.h> +#include <scmi.h> +#include <asm/types.h>
+enum scmi_clock_message_id {
SCMI_CLOCK_RATE_SET = 0x5,SCMI_CLOCK_RATE_GET = 0x6,SCMI_CLOCK_CONFIG_SET = 0x7,+};
+#define SCMI_CLK_RATE_ASYNC_NOTIFY BIT(0) +#define SCMI_CLK_RATE_ASYNC_NORESP (BIT(0) | BIT(1)) +#define SCMI_CLK_RATE_ROUND_DOWN 0 +#define SCMI_CLK_RATE_ROUND_UP BIT(2) +#define SCMI_CLK_RATE_ROUND_CLOSEST BIT(3)
+struct scmi_clk_state_in {
u32 clock_id;u32 attributes;+};
+struct scmi_clk_state_out {
s32 status;+};
+static int scmi_clk_gate(struct clk *clk, int enable) +{
struct scmi_clk_state_in in = {.clock_id = clk->id,.attributes = enable,};struct scmi_clk_state_out out;struct scmi_msg scmi_msg = {.protocol_id = SCMI_PROTOCOL_ID_CLOCK,.message_id = SCMI_CLOCK_CONFIG_SET,.in_msg = (u8 *)&in,.in_msg_sz = sizeof(in),.out_msg = (u8 *)&out,.out_msg_sz = sizeof(out),};int rc;Again please use 'ret'.
Sure, will do.
rc = scmi_send_and_process_msg(clk->dev->parent, &scmi_msg);if (rc)return rc;return scmi_to_linux_errno(out.status);+}
+static int scmi_clk_enable(struct clk *clk) +{
return scmi_clk_gate(clk, 1);+}
+static int scmi_clk_disable(struct clk *clk) +{
return scmi_clk_gate(clk, 0);+}
+struct scmi_clk_rate_get_in {
u32 clock_id;+};
+struct scmi_clk_rate_get_out {
s32 status;u32 rate_lsb;u32 rate_msb;+};
+static ulong scmi_clk_get_rate(struct clk *clk) +{
struct scmi_clk_rate_get_in in = {.clock_id = clk->id,};struct scmi_clk_rate_get_out out;struct scmi_msg scmi_msg = {.protocol_id = SCMI_PROTOCOL_ID_CLOCK,.message_id = SCMI_CLOCK_RATE_GET,.in_msg = (u8 *)&in,.in_msg_sz = sizeof(in),.out_msg = (u8 *)&out,.out_msg_sz = sizeof(out),};int rc;rc = scmi_send_and_process_msg(clk->dev->parent, &scmi_msg);if (rc)return 0;Why not rc?
True, thanks.
rc = scmi_to_linux_errno(out.status);if (rc)return 0;return (ulong)(((u64)out.rate_msb << 32) | out.rate_lsb);This is odd. Can you use ulong instead of u64 to avoid two casts?
A ulong would fit on a 64bit machine but not on a 32bit as compiler will likely complain that the '<< 32' operation is meaningless.
+}
+struct scmi_clk_rate_set_in {
struct comment. I think these need to go in some sort of protocol header.
These structures are used only by this very SCMI clock driver hence I don't think it deserves exposure in a shared header file.
u32 clock_id;u32 flags;u32 rate_lsb;u32 rate_msb;+};
+struct scmi_clk_rate_set_out {
s32 status;+};
+static ulong scmi_clk_set_rate(struct clk *clk, ulong rate) +{
struct scmi_clk_rate_set_in in = {.clock_id = clk->id,.flags = SCMI_CLK_RATE_ASYNC_NORESP |SCMI_CLK_RATE_ROUND_CLOSEST,.rate_lsb = (u32)rate,.rate_msb = (u32)((u64)rate >> 32),};struct scmi_clk_rate_set_out out;struct scmi_msg scmi_msg = {.protocol_id = SCMI_PROTOCOL_ID_CLOCK,.message_id = SCMI_CLOCK_RATE_SET,.in_msg = (u8 *)&in,.in_msg_sz = sizeof(in),.out_msg = (u8 *)&out,.out_msg_sz = sizeof(out),};int rc;rc = scmi_send_and_process_msg(clk->dev->parent, &scmi_msg);if (rc)return 0;return scmi_to_linux_errno(out.status);+}
+static const struct clk_ops scmi_clk_ops = {
.enable = scmi_clk_enable,.disable = scmi_clk_disable,.get_rate = scmi_clk_get_rate,.set_rate = scmi_clk_set_rate,+};
+U_BOOT_DRIVER(scmi_clock) = {
.name = "scmi_clk",.id = UCLASS_CLK,.ops = &scmi_clk_ops,+}; diff --git a/drivers/firmware/scmi.c b/drivers/firmware/scmi.c index fa8a91c3f3d..9f06718df51 100644 --- a/drivers/firmware/scmi.c +++ b/drivers/firmware/scmi.c @@ -399,6 +399,9 @@ static int scmi_bind(struct udevice *dev) continue;
switch (protocol_id) {
case SCMI_PROTOCOL_ID_CLOCK:drv = DM_GET_DRIVER(scmi_clock);break; default: dev_info(dev, "Ignore unsupported SCMI protocol %u\n", protocol_id);-- 2.17.1
Can you add a sandbox test for this?
Ok I'm looking at that.
Regards, Simon