| Message ID | 20190708154730.16643-12-sudeep.holla@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | firmware: arm_scmi: Add support for Rx, async commands and delayed response | expand |
Quoting Sudeep Holla (2019-07-08 08:47:30) > CLOCK_PROTOCOL_ATTRIBUTES provides attributes to indicate the maximum > number of pending asynchronous clock rate changes supported by the > platform. If it's non-zero, then we should be able to use asynchronous > clock rate set for any clocks until the maximum limit is reached. > > Keeping the current count of pending asynchronous clock set rate > requests, we can decide if we can you asynchronous request for the This last part of the sentence doesn't read properly. Please rewrite. > incoming/new request. > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- > drivers/firmware/arm_scmi/clock.c | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c > index dd215bd11a58..70044b7c812e 100644 > --- a/drivers/firmware/arm_scmi/clock.c > +++ b/drivers/firmware/arm_scmi/clock.c > @@ -221,21 +222,35 @@ static int scmi_clock_rate_set(const struct scmi_handle *handle, u32 clk_id, > u64 rate) > { > int ret; > + u32 flags = 0; > struct scmi_xfer *t; > struct scmi_clock_set_rate *cfg; > + struct clock_info *ci = handle->clk_priv; > > ret = scmi_xfer_get_init(handle, CLOCK_RATE_SET, SCMI_PROTOCOL_CLOCK, > sizeof(*cfg), 0, &t); > if (ret) > return ret; > > + if (ci->max_async_req) { > + if (atomic_inc_return(&ci->cur_async_req) < ci->max_async_req) > + flags |= CLOCK_SET_ASYNC; > + else > + atomic_dec(&ci->cur_async_req); Can this be combined with the atomic_dec() below and done after either transfer? > + } > + > cfg = t->tx.buf; > - cfg->flags = cpu_to_le32(0); > + cfg->flags = cpu_to_le32(flags); > cfg->id = cpu_to_le32(clk_id); > cfg->value_low = cpu_to_le32(rate & 0xffffffff); > cfg->value_high = cpu_to_le32(rate >> 32); > > - ret = scmi_do_xfer(handle, t); > + if (flags & CLOCK_SET_ASYNC) { > + ret = scmi_do_xfer_with_response(handle, t); > + atomic_dec(&ci->cur_async_req); > + } else { > + ret = scmi_do_xfer(handle, t); > + } I mean putting the atomic_dec() here. > > scmi_xfer_put(handle, t); > return ret;
On Mon, Jul 22, 2019 at 02:29:53PM -0700, Stephen Boyd wrote: > Quoting Sudeep Holla (2019-07-08 08:47:30) > > CLOCK_PROTOCOL_ATTRIBUTES provides attributes to indicate the maximum > > number of pending asynchronous clock rate changes supported by the > > platform. If it's non-zero, then we should be able to use asynchronous > > clock rate set for any clocks until the maximum limit is reached. > > > > Keeping the current count of pending asynchronous clock set rate > > requests, we can decide if we can you asynchronous request for the > > This last part of the sentence doesn't read properly. Please rewrite. > Will fix. > > incoming/new request. > > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > --- > > drivers/firmware/arm_scmi/clock.c | 21 ++++++++++++++++++--- > > 1 file changed, 18 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c > > index dd215bd11a58..70044b7c812e 100644 > > --- a/drivers/firmware/arm_scmi/clock.c > > +++ b/drivers/firmware/arm_scmi/clock.c > > @@ -221,21 +222,35 @@ static int scmi_clock_rate_set(const struct scmi_handle *handle, u32 clk_id, > > u64 rate) > > { > > int ret; > > + u32 flags = 0; > > struct scmi_xfer *t; > > struct scmi_clock_set_rate *cfg; > > + struct clock_info *ci = handle->clk_priv; > > > > ret = scmi_xfer_get_init(handle, CLOCK_RATE_SET, SCMI_PROTOCOL_CLOCK, > > sizeof(*cfg), 0, &t); > > if (ret) > > return ret; > > > > + if (ci->max_async_req) { > > + if (atomic_inc_return(&ci->cur_async_req) < ci->max_async_req) > > + flags |= CLOCK_SET_ASYNC; > > + else > > + atomic_dec(&ci->cur_async_req); > > Can this be combined with the atomic_dec() below and done after either > transfer? > Yes but cleaner. > > + } > > + > > cfg = t->tx.buf; > > - cfg->flags = cpu_to_le32(0); > > + cfg->flags = cpu_to_le32(flags); > > cfg->id = cpu_to_le32(clk_id); > > cfg->value_low = cpu_to_le32(rate & 0xffffffff); > > cfg->value_high = cpu_to_le32(rate >> 32); > > > > - ret = scmi_do_xfer(handle, t); > > + if (flags & CLOCK_SET_ASYNC) { > > + ret = scmi_do_xfer_with_response(handle, t); > > + atomic_dec(&ci->cur_async_req); > > + } else { > > + ret = scmi_do_xfer(handle, t); > > + } > > I mean putting the atomic_dec() here. > Understood and done locally, will post as v2. -- Regards, Sudeep
diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c index dd215bd11a58..70044b7c812e 100644 --- a/drivers/firmware/arm_scmi/clock.c +++ b/drivers/firmware/arm_scmi/clock.c @@ -56,7 +56,7 @@ struct scmi_msg_resp_clock_describe_rates { struct scmi_clock_set_rate { __le32 flags; #define CLOCK_SET_ASYNC BIT(0) -#define CLOCK_SET_DELAYED BIT(1) +#define CLOCK_SET_IGNORE_RESP BIT(1) #define CLOCK_SET_ROUND_UP BIT(2) #define CLOCK_SET_ROUND_AUTO BIT(3) __le32 id; @@ -67,6 +67,7 @@ struct scmi_clock_set_rate { struct clock_info { int num_clocks; int max_async_req; + atomic_t cur_async_req; struct scmi_clock_info *clk; }; @@ -221,21 +222,35 @@ static int scmi_clock_rate_set(const struct scmi_handle *handle, u32 clk_id, u64 rate) { int ret; + u32 flags = 0; struct scmi_xfer *t; struct scmi_clock_set_rate *cfg; + struct clock_info *ci = handle->clk_priv; ret = scmi_xfer_get_init(handle, CLOCK_RATE_SET, SCMI_PROTOCOL_CLOCK, sizeof(*cfg), 0, &t); if (ret) return ret; + if (ci->max_async_req) { + if (atomic_inc_return(&ci->cur_async_req) < ci->max_async_req) + flags |= CLOCK_SET_ASYNC; + else + atomic_dec(&ci->cur_async_req); + } + cfg = t->tx.buf; - cfg->flags = cpu_to_le32(0); + cfg->flags = cpu_to_le32(flags); cfg->id = cpu_to_le32(clk_id); cfg->value_low = cpu_to_le32(rate & 0xffffffff); cfg->value_high = cpu_to_le32(rate >> 32); - ret = scmi_do_xfer(handle, t); + if (flags & CLOCK_SET_ASYNC) { + ret = scmi_do_xfer_with_response(handle, t); + atomic_dec(&ci->cur_async_req); + } else { + ret = scmi_do_xfer(handle, t); + } scmi_xfer_put(handle, t); return ret;
CLOCK_PROTOCOL_ATTRIBUTES provides attributes to indicate the maximum number of pending asynchronous clock rate changes supported by the platform. If it's non-zero, then we should be able to use asynchronous clock rate set for any clocks until the maximum limit is reached. Keeping the current count of pending asynchronous clock set rate requests, we can decide if we can you asynchronous request for the incoming/new request. Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/firmware/arm_scmi/clock.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) -- 2.17.1