Messages in this thread | | | Date | Wed, 23 Aug 2023 10:02:46 +0100 | From | Cristian Marussi <> | Subject | Re: [PATCH 1/6] firmware: arm_scmi: Simplify enable/disable Clock operations |
| |
On Tue, Aug 22, 2023 at 01:17:15PM -0700, Stephen Boyd wrote: > Quoting Cristian Marussi (2023-08-11 09:14:41) > > Add a param to Clock enable/disable operation to ask for atomic operation > > and remove _atomic version of such operations. >
Hi,
> Why? >
:D, given that the 2 flavours of SCMI enable/disable ops (and the upcoming state_get) just differ in their operating mode (atomic or not) and the Clock framework in turn wrap such calls into 4 related and explicitly named clk_ops (scmi_clock_enable/scmi_clock_atomic_enable etc) that hint at what is being done, seemed to me reasonable to reduce the churn and remove a bit of code wrappers in favour of a param.
> > > > No functional change. > > > > CC: Michael Turquette <mturquette@baylibre.com> > > CC: Stephen Boyd <sboyd@kernel.org> > > CC: linux-clk@vger.kernel.org > > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> > > --- > > drivers/clk/clk-scmi.c | 8 ++++---- > > drivers/firmware/arm_scmi/clock.c | 24 ++++++------------------ > > include/linux/scmi_protocol.h | 9 ++++----- > > 3 files changed, 14 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c > > index 2c7a830ce308..ff003083e592 100644 > > --- a/drivers/clk/clk-scmi.c > > +++ b/drivers/clk/clk-scmi.c > > @@ -78,28 +78,28 @@ static int scmi_clk_enable(struct clk_hw *hw) > > { > > struct scmi_clk *clk = to_scmi_clk(hw); > > > > - return scmi_proto_clk_ops->enable(clk->ph, clk->id); > > + return scmi_proto_clk_ops->enable(clk->ph, clk->id, false); > > } > > > > static void scmi_clk_disable(struct clk_hw *hw) > > { > > struct scmi_clk *clk = to_scmi_clk(hw); > > > > - scmi_proto_clk_ops->disable(clk->ph, clk->id); > > + scmi_proto_clk_ops->disable(clk->ph, clk->id, false); > > I enjoyed how it was before because I don't know what 'false' means > without looking at the ops now. >
Yes indeed, I can drop this and rework if you prefer to maintain the old API calls, but this would mean that whenever we'll add new atomic flavour to some new SCMI clk operations we'll have to add 2 ops instead of a parametrized one...this is what would happen also in this series with state_get (and what really triggered this refactor)
(and please consider that on the SCMI side, for testing purposes, I would prefer to expose always both atomic and non-atomic flavours even if NOT both actively used by the Clock framework...like state_get() that can only be atomic for Clock frmwk...)
Thanks, Cristian
| |