Messages in this thread | | | Date | Tue, 10 Oct 2023 10:35:09 +0100 | From | Sudeep Holla <> | Subject | Re: [RFC] firmware: arm_scmi: clock: add fixed clock attribute support |
| |
On Tue, Oct 10, 2023 at 10:22:03AM +0100, Cristian Marussi wrote: > On Tue, Oct 10, 2023 at 10:12:23AM +0100, Sudeep Holla wrote: > > On Tue, Oct 10, 2023 at 10:29:11AM +0800, Peng Fan (OSS) wrote: > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > There are clocks: > > > system critical, not allow linux to disable, change rate > > > allow linux to get rate, because some periphals will use the frequency > > > to configure periphals. > > > > > > So introduce an attribute to indicated FIXED clock > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > --- > > > drivers/clk/clk-scmi.c | 6 ++++++ > > > drivers/firmware/arm_scmi/clock.c | 5 ++++- > > > include/linux/scmi_protocol.h | 1 + > > > 3 files changed, 11 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c > > > index 8cbe24789c24..a539a35bd45a 100644 > > > --- a/drivers/clk/clk-scmi.c > > > +++ b/drivers/clk/clk-scmi.c > > > @@ -182,6 +182,10 @@ static const struct clk_ops scmi_clk_ops = { > > > .determine_rate = scmi_clk_determine_rate, > > > }; > > > > > > +static const struct clk_ops scmi_fixed_rate_clk_ops = { > > > + .recalc_rate = scmi_clk_recalc_rate, > > > +}; > > > + > > > static const struct clk_ops scmi_atomic_clk_ops = { > > > .recalc_rate = scmi_clk_recalc_rate, > > > .round_rate = scmi_clk_round_rate, > > > @@ -293,6 +297,8 @@ static int scmi_clocks_probe(struct scmi_device *sdev) > > > if (is_atomic && > > > sclk->info->enable_latency <= atomic_threshold) > > > scmi_ops = &scmi_atomic_clk_ops; > > > + else if (sclk->info->rate_fixed) > > > + scmi_ops = &scmi_fixed_rate_clk_ops; > > > else > > > scmi_ops = &scmi_clk_ops; > > > > > > diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c > > > index ddaef34cd88b..8c52db539e54 100644 > > > --- a/drivers/firmware/arm_scmi/clock.c > > > +++ b/drivers/firmware/arm_scmi/clock.c > > > @@ -46,6 +46,7 @@ struct scmi_msg_resp_clock_attributes { > > > #define SUPPORTS_RATE_CHANGE_REQUESTED_NOTIF(x) ((x) & BIT(30)) > > > #define SUPPORTS_EXTENDED_NAMES(x) ((x) & BIT(29)) > > > #define SUPPORTS_PARENT_CLOCK(x) ((x) & BIT(28)) > > > +#define SUPPORTS_FIXED_RATE_CLOCK(x) ((x) & BIT(27)) > > > > I don't see this in the specification, am I missing something ? > > > > And why do we need it. Can't this be discrete clock with only one clock > > rate ? Or step clock with both lowest and highest the same and step being 0. > > At-least I don't see the need to change the spec for this and hence no need > > to assign any attribute bit-field to represent the same. > > > > No this is not in the spec, it would require a spec change. > > My understanding is that they have clocks that CAN have more than one rate BUT > such clock cannot be changed by Linux, only other agents can > enable/disable/set_rate BUT they still want to be able to query the > current rate for configuration purposes. >
Fair enough. As I mentioned to Peng on the other thread, it is *not a fixed* clock. It is read only for this agent.
-- Regards, Sudeep
| |