Messages in this thread | | | Subject | Re: [PATCH 6/7] clk: scmi: Allocate CLK operations dynamically | From | Stephen Boyd <> | Date | Wed, 21 Feb 2024 21:44:14 -0800 |
| |
Quoting Cristian Marussi (2024-02-14 10:30:05) > diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c > index 5747b6d651f0..b91a0dbd2fe0 100644 > --- a/drivers/clk/clk-scmi.c > +++ b/drivers/clk/clk-scmi.c > @@ -158,51 +158,6 @@ static int scmi_clk_atomic_is_enabled(struct clk_hw *hw) > return !!enabled; > } > > -/* > - * We can provide enable/disable/is_enabled atomic callbacks only if the > - * underlying SCMI transport for an SCMI instance is configured to handle > - * SCMI commands in an atomic manner. > - * > - * When no SCMI atomic transport support is available we instead provide only > - * the prepare/unprepare API, as allowed by the clock framework when atomic > - * calls are not available. > - * > - * Two distinct sets of clk_ops are provided since we could have multiple SCMI > - * instances with different underlying transport quality, so they cannot be > - * shared. > - */ > -static const struct clk_ops scmi_clk_ops = { > - .recalc_rate = scmi_clk_recalc_rate, > - .round_rate = scmi_clk_round_rate, > - .set_rate = scmi_clk_set_rate, > - .prepare = scmi_clk_enable, > - .unprepare = scmi_clk_disable, > - .set_parent = scmi_clk_set_parent, > - .get_parent = scmi_clk_get_parent, > - .determine_rate = scmi_clk_determine_rate, > -}; > - > -static const struct clk_ops scmi_atomic_clk_ops = {
It's not great to move these function pointer structs out of RO memory to RW. I'm also not convinced that it's any better to construct them at runtime. Isn't there a constant set of possible clk configurations? Or why can't we simply add some failures to the clk_ops functions instead?
> - .recalc_rate = scmi_clk_recalc_rate, > - .round_rate = scmi_clk_round_rate, > - .set_rate = scmi_clk_set_rate, > - .enable = scmi_clk_atomic_enable, > - .disable = scmi_clk_atomic_disable, > - .is_enabled = scmi_clk_atomic_is_enabled,
| |