[PATCH v2 5/5] clk: scmi: register scmi clocks with CCF
Etienne Carriere
etienne.carriere at linaro.org
Mon Mar 7 11:17:11 CET 2022
Hello Sean,
Thanks for the feedback.
Sorry I missed your post end Feb.
On Fri, 25 Feb 2022 at 07:33, Sean Anderson <seanga2 at gmail.com> wrote:
>
> Hi Etienne,
>
>
> On 2/21/22 3:22 AM, Etienne Carriere wrote:
> > Implements SCMI APIs to retrieve the number exposed SCMI clocks using
> > SCMI_PROTOCOL_ATTRIBUTES messages and the names of the clocks using
> > SCMI_CLOCK_ATTRIBUTES messages.
> >
> > This change updates sandbox SCMI clock test driver to manage these
> > 2 new message IDs.
> >
> > Cc: Lukasz Majewski <lukma at denx.de>
> > Cc: Sean Anderson <seanga2 at gmail.com>
> > Cc: Clement Leger <clement.leger at bootlin.com>
> > Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>
> > Reviewed-by: Patrick Delaunay <patrick.delaunay at foss.st.com>
> > Signed-off-by: Gabriel Fernandez <gabriel.fernandez at st.com>
> > Signed-off-by: Etienne Carriere <etienne.carriere at linaro.org>
> > ---
> > Changes since v1:
> > - Remove Series-links tag and apply R-b tag.
> > ---
> > drivers/clk/clk_scmi.c | 90 ++++++++++++++++++++++
> > drivers/firmware/scmi/sandbox-scmi_agent.c | 53 +++++++++++++
> > include/scmi_protocols.h | 43 +++++++++++
> > 3 files changed, 186 insertions(+)
> >
> > diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
> > index 42fbab0d21..57022685e2 100644
> > --- a/drivers/clk/clk_scmi.c
> > +++ b/drivers/clk/clk_scmi.c
> > @@ -11,6 +11,52 @@
> > #include <scmi_agent.h>
> > #include <scmi_protocols.h>
> > #include <asm/types.h>
> > +#include <linux/clk-provider.h>
> > +
> > +static int scmi_clk_get_num_clock(struct udevice *dev, size_t *num_clocks)
> > +{
> > + struct scmi_clk_protocol_attr_out out;
> > + struct scmi_msg msg = {
> > + .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> > + .message_id = SCMI_PROTOCOL_ATTRIBUTES,
> > + .out_msg = (u8 *)&out,
> > + .out_msg_sz = sizeof(out),
> > + };
> > + int ret;
> > +
> > + ret = devm_scmi_process_msg(dev, &msg);
> > + if (ret)
> > + return ret;
> > +
> > + *num_clocks = out.attributes & SCMI_CLK_PROTO_ATTR_COUNT_MASK;
> > +
> > + return 0;
> > +}
> > +
> > +static int scmi_clk_get_attibute(struct udevice *dev, int clkid, char **name)
>
> nit: scmi_clk_get_attribute
right!
>
> > +{
> > + struct scmi_clk_attribute_in in = {
> > + .clock_id = clkid,
> > + };
> > + struct scmi_clk_attribute_out out;
> > + struct scmi_msg msg = {
> > + .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> > + .message_id = SCMI_CLOCK_ATTRIBUTES,
> > + .in_msg = (u8 *)&in,
> > + .in_msg_sz = sizeof(in),
> > + .out_msg = (u8 *)&out,
> > + .out_msg_sz = sizeof(out),
> > + };
> > + int ret;
> > +
> > + ret = devm_scmi_process_msg(dev, &msg);
> > + if (ret)
> > + return ret;
> > +
> > + *name = out.clock_name;
> > +
> > + return 0;
> > +}
> >
> > static int scmi_clk_gate(struct clk *clk, int enable)
> > {
> > @@ -88,6 +134,49 @@ static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
> > return scmi_clk_get_rate(clk);
> > }
> >
> > +static int scmi_clk_probe(struct udevice *dev)
> > +{
> > + struct clk *clk;
> > + size_t num_clocks, i;
> > + int ret;
> > +
> > + if (!CONFIG_IS_ENABLED(CLK_CCF))
> > + return 0;
> > +
> > + /* register CCF children: CLK UCLASS, no probed again */
> > + if (device_get_uclass_id(dev->parent) == UCLASS_CLK)
> > + return 0;
> > +
> > + ret = scmi_clk_get_num_clock(dev, &num_clocks);
> > + if (ret)
> > + return ret;
> > +
> > + for (i = 0; i < num_clocks; i++) {
> > + char *name;
> > +
> > + if (!scmi_clk_get_attibute(dev, i, &name)) {
> > + char *clock_name = strdup(name);
> > +
> > + clk = kzalloc(sizeof(*clk), GFP_KERNEL);
> > + if (!clk || !clock_name)
> > + ret = -ENOMEM;
> > + else
> > + ret = clk_register(clk, dev->driver->name,
> > + clock_name, dev->name);
> > +
> > + if (ret) {
> > + free(clk);
> > + free(clock_name);
> > + return ret;
> > + }
> > +
> > + clk_dm(i, clk);
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
>
> So why not call scmi_clk_get_num_clock during probe() and then check against
> it in xlate? I don't really see why we need CCF. This also means you don't
> have a driver with copies of itself as children, and is lighter on memory.
There is no specific need for CCF for the scmi clock driver itself.
The goal of these changes is rather to allow platforms that do already
enable CCF (for an u-boot native clock driver)
to be able to also embed SCMI clocks support, for clocks controlled
from an external firmware.
Br,
etienne
>
> --Sean
>
> > static const struct clk_ops scmi_clk_ops = {
> > .enable = scmi_clk_enable,
> > .disable = scmi_clk_disable,
> > @@ -99,4 +188,5 @@ U_BOOT_DRIVER(scmi_clock) = {
> > .name = "scmi_clk",
> > .id = UCLASS_CLK,
> > .ops = &scmi_clk_ops,
> > + .probe = &scmi_clk_probe,
> > };
> > diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
> > index fc717a8aea..c555164d19 100644
> > --- a/drivers/firmware/scmi/sandbox-scmi_agent.c
> > +++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
> > @@ -114,6 +114,55 @@ static struct sandbox_scmi_voltd *get_scmi_voltd_state(uint domain_id)
> > * Sandbox SCMI agent ops
> > */
> >
> > +static int sandbox_scmi_clock_protocol_attribs(struct udevice *dev,
> > + struct scmi_msg *msg)
> > +{
> > + struct scmi_clk_protocol_attr_out *out = NULL;
> > +
> > + if (!msg->out_msg || msg->out_msg_sz < sizeof(*out))
> > + return -EINVAL;
> > +
> > + out = (struct scmi_clk_protocol_attr_out *)msg->out_msg;
> > + out->attributes = ARRAY_SIZE(scmi_clk);
> > + out->status = SCMI_SUCCESS;
> > +
> > + return 0;
> > +}
> > +
> > +static int sandbox_scmi_clock_attribs(struct udevice *dev, struct scmi_msg *msg)
> > +{
> > + struct scmi_clk_attribute_in *in = NULL;
> > + struct scmi_clk_attribute_out *out = NULL;
> > + struct sandbox_scmi_clk *clk_state = NULL;
> > + int ret;
> > +
> > + if (!msg->in_msg || msg->in_msg_sz < sizeof(*in) ||
> > + !msg->out_msg || msg->out_msg_sz < sizeof(*out))
> > + return -EINVAL;
> > +
> > + in = (struct scmi_clk_attribute_in *)msg->in_msg;
> > + out = (struct scmi_clk_attribute_out *)msg->out_msg;
> > +
> > + clk_state = get_scmi_clk_state(in->clock_id);
> > + if (!clk_state) {
> > + dev_err(dev, "Unexpected clock ID %u\n", in->clock_id);
> > +
> > + out->status = SCMI_NOT_FOUND;
> > + } else {
> > + memset(out, 0, sizeof(*out));
> > +
> > + if (clk_state->enabled)
> > + out->attributes = 1;
> > +
> > + ret = snprintf(out->clock_name, sizeof(out->clock_name),
> > + "clk%u", in->clock_id);
> > + assert(ret > 0 && ret < sizeof(out->clock_name));
> > +
> > + out->status = SCMI_SUCCESS;
> > + }
> > +
> > + return 0;
> > +}
> > static int sandbox_scmi_clock_rate_set(struct udevice *dev,
> > struct scmi_msg *msg)
> > {
> > @@ -427,6 +476,10 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev,
> > switch (msg->protocol_id) {
> > case SCMI_PROTOCOL_ID_CLOCK:
> > switch (msg->message_id) {
> > + case SCMI_PROTOCOL_ATTRIBUTES:
> > + return sandbox_scmi_clock_protocol_attribs(dev, msg);
> > + case SCMI_CLOCK_ATTRIBUTES:
> > + return sandbox_scmi_clock_attribs(dev, msg);
> > case SCMI_CLOCK_RATE_SET:
> > return sandbox_scmi_clock_rate_set(dev, msg);
> > case SCMI_CLOCK_RATE_GET:
> > diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h
> > index ef26e72176..a220cb2a91 100644
> > --- a/include/scmi_protocols.h
> > +++ b/include/scmi_protocols.h
> > @@ -40,22 +40,65 @@ enum scmi_status_code {
> > SCMI_PROTOCOL_ERROR = -10,
> > };
> >
> > +/*
> > + * Generic message IDs
> > + */
> > +enum scmi_discovery_id {
> > + SCMI_PROTOCOL_VERSION = 0x0,
> > + SCMI_PROTOCOL_ATTRIBUTES = 0x1,
> > + SCMI_PROTOCOL_MESSAGE_ATTRIBUTES = 0x2,
> > +};
> > +
> > /*
> > * SCMI Clock Protocol
> > */
> >
> > enum scmi_clock_message_id {
> > + SCMI_CLOCK_ATTRIBUTES = 0x3,
> > SCMI_CLOCK_RATE_SET = 0x5,
> > SCMI_CLOCK_RATE_GET = 0x6,
> > SCMI_CLOCK_CONFIG_SET = 0x7,
> > };
> >
> > +#define SCMI_CLK_PROTO_ATTR_COUNT_MASK GENMASK(15, 0)
> > #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)
> >
> > +#define SCMI_CLOCK_NAME_LENGTH_MAX 16
> > +
> > +/**
> > + * struct scmi_clk_get_nb_out - Response for SCMI_PROTOCOL_ATTRIBUTES command
> > + * @status: SCMI command status
> > + * @attributes: Attributes of the clock protocol, mainly number of clocks exposed
> > + */
> > +struct scmi_clk_protocol_attr_out {
> > + s32 status;
> > + u32 attributes;
> > +};
> > +
> > +/**
> > + * struct scmi_clk_attribute_in - Message payload for SCMI_CLOCK_ATTRIBUTES command
> > + * @clock_id: SCMI clock ID
> > + */
> > +struct scmi_clk_attribute_in {
> > + u32 clock_id;
> > +};
> > +
> > +/**
> > + * struct scmi_clk_get_nb_out - Response payload for SCMI_CLOCK_ATTRIBUTES command
> > + * @status: SCMI command status
> > + * @attributes: clock attributes
> > + * @clock_name: name of the clock
> > + */
> > +struct scmi_clk_attribute_out {
> > + s32 status;
> > + u32 attributes;
> > + char clock_name[SCMI_CLOCK_NAME_LENGTH_MAX];
> > +};
> > +
> > /**
> > * struct scmi_clk_state_in - Message payload for CLOCK_CONFIG_SET command
> > * @clock_id: SCMI clock ID
> >
>
More information about the U-Boot
mailing list