* [PATCH 0/8] firmware: arm_scmi: trivial cleanups
@ 2018-05-09 17:07 Sudeep Holla
2018-05-09 17:07 ` [PATCH 1/8] firmware: arm_scmi: improve code readability using bitfield accessor macros Sudeep Holla
` (7 more replies)
0 siblings, 8 replies; 19+ messages in thread
From: Sudeep Holla @ 2018-05-09 17:07 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
This contains all of the trivial review comments that were not addressed
as the series was queued for v4.17 and were not critical.
Regards,
Sudeep
Sudeep Holla (8):
firmware: arm_scmi: improve code readability using bitfield accessor
macros
firmware: arm_scmi: fix kernel-docs documentation
firmware: arm_scmi: rename get_transition_latency and
add_opps_to_device
firmware: arm_scmi: rename scmi_xfer_{init,get,put}
firmware: arm_scmi: drop unused `con_priv` structure member
firmware: arm_scmi: remove unnecessary bitmap_zero
firmware: arm_scmi: improve exit paths and code readability
firmware: arm_scmi: simplify exit path by returning on error
drivers/cpufreq/scmi-cpufreq.c | 4 +-
drivers/firmware/arm_scmi/base.c | 43 +++++++-------
drivers/firmware/arm_scmi/bus.c | 22 ++++----
drivers/firmware/arm_scmi/clock.c | 24 ++++----
drivers/firmware/arm_scmi/common.h | 22 ++++----
drivers/firmware/arm_scmi/driver.c | 109 ++++++++++++++++++------------------
drivers/firmware/arm_scmi/perf.c | 38 ++++++-------
drivers/firmware/arm_scmi/power.c | 16 +++---
drivers/firmware/arm_scmi/sensors.c | 20 +++----
include/linux/scmi_protocol.h | 18 ++++--
10 files changed, 166 insertions(+), 150 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/8] firmware: arm_scmi: improve code readability using bitfield accessor macros 2018-05-09 17:07 [PATCH 0/8] firmware: arm_scmi: trivial cleanups Sudeep Holla @ 2018-05-09 17:07 ` Sudeep Holla 2018-05-17 8:14 ` Jonathan Cameron 2018-05-09 17:07 ` [PATCH 2/8] firmware: arm_scmi: fix kernel-docs documentation Sudeep Holla ` (6 subsequent siblings) 7 siblings, 1 reply; 19+ messages in thread From: Sudeep Holla @ 2018-05-09 17:07 UTC (permalink / raw) To: linux-arm-kernel By using FIELD_{FIT,GET,PREP} and GENMASK macro accessors we can avoid some clumpsy custom shifting and masking macros and also improve the code better readability. Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/firmware/arm_scmi/common.h | 9 +++++---- drivers/firmware/arm_scmi/driver.c | 31 ++++++++++++++----------------- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 0c30234f9098..e8f332c9c469 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -7,6 +7,7 @@ * Copyright (C) 2018 ARM Ltd. */ +#include <linux/bitfield.h> #include <linux/completion.h> #include <linux/device.h> #include <linux/errno.h> @@ -14,10 +15,10 @@ #include <linux/scmi_protocol.h> #include <linux/types.h> -#define PROTOCOL_REV_MINOR_BITS 16 -#define PROTOCOL_REV_MINOR_MASK ((1U << PROTOCOL_REV_MINOR_BITS) - 1) -#define PROTOCOL_REV_MAJOR(x) ((x) >> PROTOCOL_REV_MINOR_BITS) -#define PROTOCOL_REV_MINOR(x) ((x) & PROTOCOL_REV_MINOR_MASK) +#define PROTOCOL_REV_MINOR_MASK GENMASK(15, 0) +#define PROTOCOL_REV_MAJOR_MASK GENMASK(31, 16) +#define PROTOCOL_REV_MAJOR(x) (u16)(FIELD_GET(PROTOCOL_REV_MAJOR_MASK, (x))) +#define PROTOCOL_REV_MINOR(x) (u16)(FIELD_GET(PROTOCOL_REV_MINOR_MASK, (x))) #define MAX_PROTOCOLS_IMP 16 #define MAX_OPPS 16 diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 14b147135a0c..917786d91f55 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -29,16 +29,12 @@ #include "common.h" -#define MSG_ID_SHIFT 0 -#define MSG_ID_MASK 0xff -#define MSG_TYPE_SHIFT 8 -#define MSG_TYPE_MASK 0x3 -#define MSG_PROTOCOL_ID_SHIFT 10 -#define MSG_PROTOCOL_ID_MASK 0xff -#define MSG_TOKEN_ID_SHIFT 18 -#define MSG_TOKEN_ID_MASK 0x3ff -#define MSG_XTRACT_TOKEN(header) \ - (((header) >> MSG_TOKEN_ID_SHIFT) & MSG_TOKEN_ID_MASK) +#define MSG_ID_MASK GENMASK(7, 0) +#define MSG_TYPE_MASK GENMASK(9, 8) +#define MSG_PROTOCOL_ID_MASK GENMASK(17, 10) +#define MSG_TOKEN_ID_MASK GENMASK(27, 18) +#define MSG_XTRACT_TOKEN(hdr) FIELD_GET(MSG_TOKEN_ID_MASK, (hdr)) +#define MSG_TOKEN_MAX (MSG_XTRACT_TOKEN(MSG_TOKEN_ID_MASK) + 1) enum scmi_error_codes { SCMI_SUCCESS = 0, /* Success */ @@ -255,9 +251,9 @@ static void scmi_rx_callback(struct mbox_client *cl, void *m) */ static inline u32 pack_scmi_header(struct scmi_msg_hdr *hdr) { - return ((hdr->id & MSG_ID_MASK) << MSG_ID_SHIFT) | - ((hdr->seq & MSG_TOKEN_ID_MASK) << MSG_TOKEN_ID_SHIFT) | - ((hdr->protocol_id & MSG_PROTOCOL_ID_MASK) << MSG_PROTOCOL_ID_SHIFT); + return FIELD_PREP(MSG_ID_MASK, hdr->id) | + FIELD_PREP(MSG_TOKEN_ID_MASK, hdr->seq) | + FIELD_PREP(MSG_PROTOCOL_ID_MASK, hdr->protocol_id); } /** @@ -621,9 +617,9 @@ static int scmi_xfer_info_init(struct scmi_info *sinfo) struct scmi_xfers_info *info = &sinfo->minfo; /* Pre-allocated messages, no more than what hdr.seq can support */ - if (WARN_ON(desc->max_msg >= (MSG_TOKEN_ID_MASK + 1))) { - dev_err(dev, "Maximum message of %d exceeds supported %d\n", - desc->max_msg, MSG_TOKEN_ID_MASK + 1); + if (WARN_ON(desc->max_msg >= MSG_TOKEN_MAX)) { + dev_err(dev, "Maximum message of %d exceeds supported %ld\n", + desc->max_msg, MSG_TOKEN_MAX); return -EINVAL; } @@ -840,7 +836,8 @@ static int scmi_probe(struct platform_device *pdev) if (of_property_read_u32(child, "reg", &prot_id)) continue; - prot_id &= MSG_PROTOCOL_ID_MASK; + if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id)) + dev_err(dev, "Out of range protocol %d\n", prot_id); if (!scmi_is_protocol_implemented(handle, prot_id)) { dev_err(dev, "SCMI protocol %d not implemented\n", -- 2.7.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 1/8] firmware: arm_scmi: improve code readability using bitfield accessor macros 2018-05-09 17:07 ` [PATCH 1/8] firmware: arm_scmi: improve code readability using bitfield accessor macros Sudeep Holla @ 2018-05-17 8:14 ` Jonathan Cameron 0 siblings, 0 replies; 19+ messages in thread From: Jonathan Cameron @ 2018-05-17 8:14 UTC (permalink / raw) To: linux-arm-kernel On Wed, 9 May 2018 18:07:07 +0100 Sudeep Holla <sudeep.holla@arm.com> wrote: > By using FIELD_{FIT,GET,PREP} and GENMASK macro accessors we can avoid > some clumpsy custom shifting and masking macros and also improve the > code better readability. > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> Hi Sudeep, A minor comment inline. Jonathan > --- > drivers/firmware/arm_scmi/common.h | 9 +++++---- > drivers/firmware/arm_scmi/driver.c | 31 ++++++++++++++----------------- > 2 files changed, 19 insertions(+), 21 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h > index 0c30234f9098..e8f332c9c469 100644 > --- a/drivers/firmware/arm_scmi/common.h > +++ b/drivers/firmware/arm_scmi/common.h > @@ -7,6 +7,7 @@ > * Copyright (C) 2018 ARM Ltd. > */ > > +#include <linux/bitfield.h> > #include <linux/completion.h> > #include <linux/device.h> > #include <linux/errno.h> > @@ -14,10 +15,10 @@ > #include <linux/scmi_protocol.h> > #include <linux/types.h> > > -#define PROTOCOL_REV_MINOR_BITS 16 > -#define PROTOCOL_REV_MINOR_MASK ((1U << PROTOCOL_REV_MINOR_BITS) - 1) > -#define PROTOCOL_REV_MAJOR(x) ((x) >> PROTOCOL_REV_MINOR_BITS) > -#define PROTOCOL_REV_MINOR(x) ((x) & PROTOCOL_REV_MINOR_MASK) > +#define PROTOCOL_REV_MINOR_MASK GENMASK(15, 0) > +#define PROTOCOL_REV_MAJOR_MASK GENMASK(31, 16) > +#define PROTOCOL_REV_MAJOR(x) (u16)(FIELD_GET(PROTOCOL_REV_MAJOR_MASK, (x))) > +#define PROTOCOL_REV_MINOR(x) (u16)(FIELD_GET(PROTOCOL_REV_MINOR_MASK, (x))) > #define MAX_PROTOCOLS_IMP 16 > #define MAX_OPPS 16 > > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c > index 14b147135a0c..917786d91f55 100644 > --- a/drivers/firmware/arm_scmi/driver.c > +++ b/drivers/firmware/arm_scmi/driver.c > @@ -29,16 +29,12 @@ > > #include "common.h" > > -#define MSG_ID_SHIFT 0 > -#define MSG_ID_MASK 0xff > -#define MSG_TYPE_SHIFT 8 > -#define MSG_TYPE_MASK 0x3 > -#define MSG_PROTOCOL_ID_SHIFT 10 > -#define MSG_PROTOCOL_ID_MASK 0xff > -#define MSG_TOKEN_ID_SHIFT 18 > -#define MSG_TOKEN_ID_MASK 0x3ff > -#define MSG_XTRACT_TOKEN(header) \ > - (((header) >> MSG_TOKEN_ID_SHIFT) & MSG_TOKEN_ID_MASK) > +#define MSG_ID_MASK GENMASK(7, 0) > +#define MSG_TYPE_MASK GENMASK(9, 8) > +#define MSG_PROTOCOL_ID_MASK GENMASK(17, 10) > +#define MSG_TOKEN_ID_MASK GENMASK(27, 18) > +#define MSG_XTRACT_TOKEN(hdr) FIELD_GET(MSG_TOKEN_ID_MASK, (hdr)) > +#define MSG_TOKEN_MAX (MSG_XTRACT_TOKEN(MSG_TOKEN_ID_MASK) + 1) This feels a little odd. It's not the Max value, I think, but rather one more than it. I would set it to this -1 and use > than in the test below. > > enum scmi_error_codes { > SCMI_SUCCESS = 0, /* Success */ > @@ -255,9 +251,9 @@ static void scmi_rx_callback(struct mbox_client *cl, void *m) > */ > static inline u32 pack_scmi_header(struct scmi_msg_hdr *hdr) > { > - return ((hdr->id & MSG_ID_MASK) << MSG_ID_SHIFT) | > - ((hdr->seq & MSG_TOKEN_ID_MASK) << MSG_TOKEN_ID_SHIFT) | > - ((hdr->protocol_id & MSG_PROTOCOL_ID_MASK) << MSG_PROTOCOL_ID_SHIFT); > + return FIELD_PREP(MSG_ID_MASK, hdr->id) | > + FIELD_PREP(MSG_TOKEN_ID_MASK, hdr->seq) | > + FIELD_PREP(MSG_PROTOCOL_ID_MASK, hdr->protocol_id); > } > > /** > @@ -621,9 +617,9 @@ static int scmi_xfer_info_init(struct scmi_info *sinfo) > struct scmi_xfers_info *info = &sinfo->minfo; > > /* Pre-allocated messages, no more than what hdr.seq can support */ > - if (WARN_ON(desc->max_msg >= (MSG_TOKEN_ID_MASK + 1))) { > - dev_err(dev, "Maximum message of %d exceeds supported %d\n", > - desc->max_msg, MSG_TOKEN_ID_MASK + 1); > + if (WARN_ON(desc->max_msg >= MSG_TOKEN_MAX)) { > + dev_err(dev, "Maximum message of %d exceeds supported %ld\n", > + desc->max_msg, MSG_TOKEN_MAX); > return -EINVAL; > } > > @@ -840,7 +836,8 @@ static int scmi_probe(struct platform_device *pdev) > if (of_property_read_u32(child, "reg", &prot_id)) > continue; > > - prot_id &= MSG_PROTOCOL_ID_MASK; > + if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id)) > + dev_err(dev, "Out of range protocol %d\n", prot_id); > > if (!scmi_is_protocol_implemented(handle, prot_id)) { > dev_err(dev, "SCMI protocol %d not implemented\n", ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/8] firmware: arm_scmi: fix kernel-docs documentation 2018-05-09 17:07 [PATCH 0/8] firmware: arm_scmi: trivial cleanups Sudeep Holla 2018-05-09 17:07 ` [PATCH 1/8] firmware: arm_scmi: improve code readability using bitfield accessor macros Sudeep Holla @ 2018-05-09 17:07 ` Sudeep Holla 2018-05-17 8:30 ` Jonathan Cameron 2018-05-09 17:07 ` [PATCH 3/8] firmware: arm_scmi: rename get_transition_latency and add_opps_to_device Sudeep Holla ` (5 subsequent siblings) 7 siblings, 1 reply; 19+ messages in thread From: Sudeep Holla @ 2018-05-09 17:07 UTC (permalink / raw) To: linux-arm-kernel There are few missing descriptions for function parameters and structure members along with certain instances where excessive function parameters or structure members are described. This patch fixes all of those warnings. Reported-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/firmware/arm_scmi/base.c | 20 +++++++++-------- drivers/firmware/arm_scmi/common.h | 7 ++++-- drivers/firmware/arm_scmi/driver.c | 45 ++++++++++++++++++++------------------ include/linux/scmi_protocol.h | 8 +++++++ 4 files changed, 48 insertions(+), 32 deletions(-) diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c index 0d3806c0d432..c36ded9dbb83 100644 --- a/drivers/firmware/arm_scmi/base.c +++ b/drivers/firmware/arm_scmi/base.c @@ -26,7 +26,7 @@ struct scmi_msg_resp_base_attributes { * scmi_base_attributes_get() - gets the implementation details * that are associated with the base protocol. * - * @handle - SCMI entity handle + * @handle: SCMI entity handle * * Return: 0 on success, else appropriate SCMI error. */ @@ -50,14 +50,15 @@ static int scmi_base_attributes_get(const struct scmi_handle *handle) } scmi_one_xfer_put(handle, t); + return ret; } /** * scmi_base_vendor_id_get() - gets vendor/subvendor identifier ASCII string. * - * @handle - SCMI entity handle - * @sub_vendor - specify true if sub-vendor ID is needed + * @handle: SCMI entity handle + * @sub_vendor: specify true if sub-vendor ID is needed * * Return: 0 on success, else appropriate SCMI error. */ @@ -97,7 +98,7 @@ scmi_base_vendor_id_get(const struct scmi_handle *handle, bool sub_vendor) * implementation 32-bit version. The format of the version number is * vendor-specific * - * @handle - SCMI entity handle + * @handle: SCMI entity handle * * Return: 0 on success, else appropriate SCMI error. */ @@ -128,8 +129,8 @@ scmi_base_implementation_version_get(const struct scmi_handle *handle) * scmi_base_implementation_list_get() - gets the list of protocols it is * OSPM is allowed to access * - * @handle - SCMI entity handle - * @protocols_imp - pointer to hold the list of protocol identifiers + * @handle: SCMI entity handle + * @protocols_imp: pointer to hold the list of protocol identifiers * * Return: 0 on success, else appropriate SCMI error. */ @@ -173,15 +174,16 @@ static int scmi_base_implementation_list_get(const struct scmi_handle *handle, } while (loop_num_ret); scmi_one_xfer_put(handle, t); + return ret; } /** * scmi_base_discover_agent_get() - discover the name of an agent * - * @handle - SCMI entity handle - * @id - Agent identifier - * @name - Agent identifier ASCII string + * @handle: SCMI entity handle + * @id: Agent identifier + * @name: Agent identifier ASCII string * * An agent id of 0 is reserved to identify the platform itself. * Generally operating system is represented as "OSPM" diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index e8f332c9c469..0821662a4633 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -51,8 +51,11 @@ struct scmi_msg_resp_prot_version { * @id: The identifier of the command being sent * @protocol_id: The identifier of the protocol used to send @id command * @seq: The token to identify the message. when a message/command returns, - * the platform returns the whole message header unmodified including - * the token. + * the platform returns the whole message header unmodified including + * the token + * @status: Status of the transfer once it's complete + * @poll_completion: Indicate if the transfer needs to be polled for + * completion or interrupt mode is used */ struct scmi_msg_hdr { u8 id; diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 917786d91f55..6fee11f06a66 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -51,7 +51,7 @@ enum scmi_error_codes { SCMI_ERR_MAX }; -/* List of all SCMI devices active in system */ +/* List of all SCMI devices active in system */ static LIST_HEAD(scmi_list); /* Protection for the entire list */ static DEFINE_MUTEX(scmi_list_mutex); @@ -68,7 +68,6 @@ static DEFINE_MUTEX(scmi_list_mutex); struct scmi_xfers_info { struct scmi_xfer *xfer_block; unsigned long *xfer_alloc_table; - /* protect transfer allocation */ spinlock_t xfer_lock; }; @@ -94,6 +93,7 @@ struct scmi_desc { * @payload: Transmit/Receive mailbox channel payload area * @dev: Reference to device in the SCMI hierarchy corresponding to this * channel + * @handle: Pointer to SCMI entity handle */ struct scmi_chan_info { struct mbox_client cl; @@ -104,7 +104,7 @@ struct scmi_chan_info { }; /** - * struct scmi_info - Structure representing a SCMI instance + * struct scmi_info - Structure representing a SCMI instance * * @dev: Device pointer * @desc: SoC description for this instance @@ -113,9 +113,9 @@ struct scmi_chan_info { * implementation version and (sub-)vendor identification. * @minfo: Message info * @tx_idr: IDR object to map protocol id to channel info pointer - * @protocols_imp: list of protocols implemented, currently maximum of + * @protocols_imp: List of protocols implemented, currently maximum of * MAX_PROTOCOLS_IMP elements allocated by the base protocol - * @node: list head + * @node: List head * @users: Number of users of this instance */ struct scmi_info { @@ -221,9 +221,7 @@ static void scmi_rx_callback(struct mbox_client *cl, void *m) xfer_id = MSG_XTRACT_TOKEN(ioread32(&mem->msg_header)); - /* - * Are we even expecting this? - */ + /* Are we even expecting this? */ if (!test_bit(xfer_id, minfo->xfer_alloc_table)) { dev_err(dev, "message for %d is not expected!\n", xfer_id); return; @@ -248,6 +246,8 @@ static void scmi_rx_callback(struct mbox_client *cl, void *m) * * @hdr: pointer to header containing all the information on message id, * protocol id and sequence id. + * + * Return: 32-bit packed command header to be sent to the platform. */ static inline u32 pack_scmi_header(struct scmi_msg_hdr *hdr) { @@ -282,9 +282,9 @@ static void scmi_tx_prepare(struct mbox_client *cl, void *m) } /** - * scmi_one_xfer_get() - Allocate one message + * scmi_xfer_get() - Allocate one message * - * @handle: SCMI entity handle + * @handle: Pointer to SCMI entity handle * * Helper function which is used by various command functions that are * exposed to clients of this driver for allocating a message traffic event. @@ -326,8 +326,8 @@ static struct scmi_xfer *scmi_one_xfer_get(const struct scmi_handle *handle) /** * scmi_one_xfer_put() - Release a message * - * @minfo: transfer info pointer - * @xfer: message that was reserved by scmi_one_xfer_get + * @handle: Pointer to SCMI entity handle + * @xfer: message that was reserved by scmi_xfer_get * * This holds a spinlock to maintain integrity of internal data structures. */ @@ -374,12 +374,12 @@ static bool scmi_xfer_done_no_timeout(const struct scmi_chan_info *cinfo, /** * scmi_do_xfer() - Do one transfer * - * @info: Pointer to SCMI entity information + * @handle: Pointer to SCMI entity handle * @xfer: Transfer to initiate and wait for response * * Return: -ETIMEDOUT in case of no response, if transmit error, - * return corresponding error, else if all goes well, - * return 0. + * return corresponding error, else if all goes well, + * return 0. */ int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer) { @@ -438,9 +438,9 @@ int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer) /** * scmi_one_xfer_init() - Allocate and initialise one message * - * @handle: SCMI entity handle + * @handle: Pointer to SCMI entity handle * @msg_id: Message identifier - * @msg_prot_id: Protocol identifier for the message + * @prot_id: Protocol identifier for the message * @tx_size: transmit message size * @rx_size: receive message size * @p: pointer to the allocated and initialised message @@ -478,13 +478,16 @@ int scmi_one_xfer_init(const struct scmi_handle *handle, u8 msg_id, u8 prot_id, xfer->hdr.poll_completion = false; *p = xfer; + return 0; } /** * scmi_version_get() - command to get the revision of the SCMI entity * - * @handle: Handle to SCMI entity information + * @handle: Pointer to SCMI entity handle + * @protocol: Protocol identifier for the message + * @version: Holds returned version of protocol. * * Updates the SCMI information in the internal data structure. * @@ -541,7 +544,7 @@ scmi_is_protocol_implemented(const struct scmi_handle *handle, u8 prot_id) * @dev: pointer to device for which we want SCMI handle * * NOTE: The function does not track individual clients of the framework - * and is expected to be maintained by caller of SCMI protocol library. + * and is expected to be maintained by caller of SCMI protocol library. * scmi_handle_put must be balanced with successful scmi_handle_get * * Return: pointer to handle if successful, NULL on error @@ -572,7 +575,7 @@ struct scmi_handle *scmi_handle_get(struct device *dev) * @handle: handle acquired by scmi_handle_get * * NOTE: The function does not track individual clients of the framework - * and is expected to be maintained by caller of SCMI protocol library. + * and is expected to be maintained by caller of SCMI protocol library. * scmi_handle_put must be balanced with successful scmi_handle_get * * Return: 0 is successfully released @@ -595,7 +598,7 @@ int scmi_handle_put(const struct scmi_handle *handle) } static const struct scmi_desc scmi_generic_desc = { - .max_rx_timeout_ms = 30, /* we may increase this if required */ + .max_rx_timeout_ms = 30, /* We may increase this if required */ .max_msg = 20, /* Limited by MBOX_TX_QUEUE_LEN */ .max_msg_size = 128, }; diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index b458c87b866c..a171c1e293e8 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -189,6 +189,14 @@ struct scmi_sensor_ops { * @perf_ops: pointer to set of performance protocol operations * @clk_ops: pointer to set of clock protocol operations * @sensor_ops: pointer to set of sensor protocol operations + * @perf_priv: pointer to private data structure specific to performance + * protocol(for internal use only) + * @clk_priv: pointer to private data structure specific to clock + * protocol(for internal use only) + * @power_priv: pointer to private data structure specific to power + * protocol(for internal use only) + * @sensor_priv: pointer to private data structure specific to sensors + * protocol(for internal use only) */ struct scmi_handle { struct device *dev; -- 2.7.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/8] firmware: arm_scmi: fix kernel-docs documentation 2018-05-09 17:07 ` [PATCH 2/8] firmware: arm_scmi: fix kernel-docs documentation Sudeep Holla @ 2018-05-17 8:30 ` Jonathan Cameron 0 siblings, 0 replies; 19+ messages in thread From: Jonathan Cameron @ 2018-05-17 8:30 UTC (permalink / raw) To: linux-arm-kernel On Wed, 9 May 2018 18:07:08 +0100 Sudeep Holla <sudeep.holla@arm.com> wrote: > There are few missing descriptions for function parameters and structure > members along with certain instances where excessive function parameters > or structure members are described. > > This patch fixes all of those warnings. > > Reported-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> A few really minor points inline. The white space changes should be in a separate patch. It's not a big thing in a simple patch like this one, but it is good practice in general. With the white space changes pulled out of here. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Thanks, Jonathan > --- > drivers/firmware/arm_scmi/base.c | 20 +++++++++-------- > drivers/firmware/arm_scmi/common.h | 7 ++++-- > drivers/firmware/arm_scmi/driver.c | 45 ++++++++++++++++++++------------------ > include/linux/scmi_protocol.h | 8 +++++++ > 4 files changed, 48 insertions(+), 32 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c > index 0d3806c0d432..c36ded9dbb83 100644 > --- a/drivers/firmware/arm_scmi/base.c > +++ b/drivers/firmware/arm_scmi/base.c > @@ -26,7 +26,7 @@ struct scmi_msg_resp_base_attributes { > * scmi_base_attributes_get() - gets the implementation details > * that are associated with the base protocol. > * > - * @handle - SCMI entity handle > + * @handle: SCMI entity handle > * > * Return: 0 on success, else appropriate SCMI error. > */ > @@ -50,14 +50,15 @@ static int scmi_base_attributes_get(const struct scmi_handle *handle) > } > > scmi_one_xfer_put(handle, t); > + Unrelated change, please have these white space improvements in a separate patch. > return ret; > } > > /** > * scmi_base_vendor_id_get() - gets vendor/subvendor identifier ASCII string. > * > - * @handle - SCMI entity handle > - * @sub_vendor - specify true if sub-vendor ID is needed > + * @handle: SCMI entity handle > + * @sub_vendor: specify true if sub-vendor ID is needed > * > * Return: 0 on success, else appropriate SCMI error. > */ > @@ -97,7 +98,7 @@ scmi_base_vendor_id_get(const struct scmi_handle *handle, bool sub_vendor) > * implementation 32-bit version. The format of the version number is > * vendor-specific > * > - * @handle - SCMI entity handle > + * @handle: SCMI entity handle > * > * Return: 0 on success, else appropriate SCMI error. > */ > @@ -128,8 +129,8 @@ scmi_base_implementation_version_get(const struct scmi_handle *handle) > * scmi_base_implementation_list_get() - gets the list of protocols it is > * OSPM is allowed to access > * > - * @handle - SCMI entity handle > - * @protocols_imp - pointer to hold the list of protocol identifiers > + * @handle: SCMI entity handle > + * @protocols_imp: pointer to hold the list of protocol identifiers > * > * Return: 0 on success, else appropriate SCMI error. > */ > @@ -173,15 +174,16 @@ static int scmi_base_implementation_list_get(const struct scmi_handle *handle, > } while (loop_num_ret); > > scmi_one_xfer_put(handle, t); > + Another unrelated change. > return ret; > } > > /** > * scmi_base_discover_agent_get() - discover the name of an agent > * > - * @handle - SCMI entity handle > - * @id - Agent identifier > - * @name - Agent identifier ASCII string > + * @handle: SCMI entity handle > + * @id: Agent identifier > + * @name: Agent identifier ASCII string > * > * An agent id of 0 is reserved to identify the platform itself. > * Generally operating system is represented as "OSPM" > diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h > index e8f332c9c469..0821662a4633 100644 > --- a/drivers/firmware/arm_scmi/common.h > +++ b/drivers/firmware/arm_scmi/common.h > @@ -51,8 +51,11 @@ struct scmi_msg_resp_prot_version { > * @id: The identifier of the command being sent > * @protocol_id: The identifier of the protocol used to send @id command > * @seq: The token to identify the message. when a message/command returns, > - * the platform returns the whole message header unmodified including > - * the token. > + * the platform returns the whole message header unmodified including > + * the token > + * @status: Status of the transfer once it's complete > + * @poll_completion: Indicate if the transfer needs to be polled for > + * completion or interrupt mode is used > */ > struct scmi_msg_hdr { > u8 id; > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c > index 917786d91f55..6fee11f06a66 100644 > --- a/drivers/firmware/arm_scmi/driver.c > +++ b/drivers/firmware/arm_scmi/driver.c > @@ -51,7 +51,7 @@ enum scmi_error_codes { > SCMI_ERR_MAX > }; > > -/* List of all SCMI devices active in system */ > +/* List of all SCMI devices active in system */ > static LIST_HEAD(scmi_list); > /* Protection for the entire list */ > static DEFINE_MUTEX(scmi_list_mutex); > @@ -68,7 +68,6 @@ static DEFINE_MUTEX(scmi_list_mutex); > struct scmi_xfers_info { > struct scmi_xfer *xfer_block; > unsigned long *xfer_alloc_table; > - /* protect transfer allocation */ > spinlock_t xfer_lock; > }; > > @@ -94,6 +93,7 @@ struct scmi_desc { > * @payload: Transmit/Receive mailbox channel payload area > * @dev: Reference to device in the SCMI hierarchy corresponding to this > * channel > + * @handle: Pointer to SCMI entity handle > */ > struct scmi_chan_info { > struct mbox_client cl; > @@ -104,7 +104,7 @@ struct scmi_chan_info { > }; > > /** > - * struct scmi_info - Structure representing a SCMI instance > + * struct scmi_info - Structure representing a SCMI instance Nitpick: an SCMI > * > * @dev: Device pointer > * @desc: SoC description for this instance > @@ -113,9 +113,9 @@ struct scmi_chan_info { > * implementation version and (sub-)vendor identification. > * @minfo: Message info > * @tx_idr: IDR object to map protocol id to channel info pointer > - * @protocols_imp: list of protocols implemented, currently maximum of > + * @protocols_imp: List of protocols implemented, currently maximum of > * MAX_PROTOCOLS_IMP elements allocated by the base protocol > - * @node: list head > + * @node: List head > * @users: Number of users of this instance > */ > struct scmi_info { > @@ -221,9 +221,7 @@ static void scmi_rx_callback(struct mbox_client *cl, void *m) > > xfer_id = MSG_XTRACT_TOKEN(ioread32(&mem->msg_header)); > > - /* > - * Are we even expecting this? > - */ > + /* Are we even expecting this? */ Technically this is a different issue, but as it's comment related you 'could' put it in the same patch if the description mentions it. > if (!test_bit(xfer_id, minfo->xfer_alloc_table)) { > dev_err(dev, "message for %d is not expected!\n", xfer_id); > return; > @@ -248,6 +246,8 @@ static void scmi_rx_callback(struct mbox_client *cl, void *m) > * > * @hdr: pointer to header containing all the information on message id, > * protocol id and sequence id. > + * > + * Return: 32-bit packed command header to be sent to the platform. > */ > static inline u32 pack_scmi_header(struct scmi_msg_hdr *hdr) > { > @@ -282,9 +282,9 @@ static void scmi_tx_prepare(struct mbox_client *cl, void *m) > } > > /** > - * scmi_one_xfer_get() - Allocate one message > + * scmi_xfer_get() - Allocate one message > * > - * @handle: SCMI entity handle > + * @handle: Pointer to SCMI entity handle > * > * Helper function which is used by various command functions that are > * exposed to clients of this driver for allocating a message traffic event. > @@ -326,8 +326,8 @@ static struct scmi_xfer *scmi_one_xfer_get(const struct scmi_handle *handle) > /** > * scmi_one_xfer_put() - Release a message > * > - * @minfo: transfer info pointer > - * @xfer: message that was reserved by scmi_one_xfer_get > + * @handle: Pointer to SCMI entity handle > + * @xfer: message that was reserved by scmi_xfer_get > * > * This holds a spinlock to maintain integrity of internal data structures. > */ > @@ -374,12 +374,12 @@ static bool scmi_xfer_done_no_timeout(const struct scmi_chan_info *cinfo, > /** > * scmi_do_xfer() - Do one transfer > * > - * @info: Pointer to SCMI entity information > + * @handle: Pointer to SCMI entity handle > * @xfer: Transfer to initiate and wait for response > * > * Return: -ETIMEDOUT in case of no response, if transmit error, > - * return corresponding error, else if all goes well, > - * return 0. > + * return corresponding error, else if all goes well, > + * return 0. > */ > int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer) > { > @@ -438,9 +438,9 @@ int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer) > /** > * scmi_one_xfer_init() - Allocate and initialise one message > * > - * @handle: SCMI entity handle > + * @handle: Pointer to SCMI entity handle > * @msg_id: Message identifier > - * @msg_prot_id: Protocol identifier for the message > + * @prot_id: Protocol identifier for the message > * @tx_size: transmit message size > * @rx_size: receive message size > * @p: pointer to the allocated and initialised message > @@ -478,13 +478,16 @@ int scmi_one_xfer_init(const struct scmi_handle *handle, u8 msg_id, u8 prot_id, > xfer->hdr.poll_completion = false; > > *p = xfer; > + Same comment. > return 0; > } > > /** > * scmi_version_get() - command to get the revision of the SCMI entity > * > - * @handle: Handle to SCMI entity information > + * @handle: Pointer to SCMI entity handle > + * @protocol: Protocol identifier for the message > + * @version: Holds returned version of protocol. > * > * Updates the SCMI information in the internal data structure. > * > @@ -541,7 +544,7 @@ scmi_is_protocol_implemented(const struct scmi_handle *handle, u8 prot_id) > * @dev: pointer to device for which we want SCMI handle > * > * NOTE: The function does not track individual clients of the framework > - * and is expected to be maintained by caller of SCMI protocol library. > + * and is expected to be maintained by caller of SCMI protocol library. > * scmi_handle_put must be balanced with successful scmi_handle_get > * > * Return: pointer to handle if successful, NULL on error > @@ -572,7 +575,7 @@ struct scmi_handle *scmi_handle_get(struct device *dev) > * @handle: handle acquired by scmi_handle_get > * > * NOTE: The function does not track individual clients of the framework > - * and is expected to be maintained by caller of SCMI protocol library. > + * and is expected to be maintained by caller of SCMI protocol library. > * scmi_handle_put must be balanced with successful scmi_handle_get > * > * Return: 0 is successfully released > @@ -595,7 +598,7 @@ int scmi_handle_put(const struct scmi_handle *handle) > } > > static const struct scmi_desc scmi_generic_desc = { > - .max_rx_timeout_ms = 30, /* we may increase this if required */ > + .max_rx_timeout_ms = 30, /* We may increase this if required */ > .max_msg = 20, /* Limited by MBOX_TX_QUEUE_LEN */ > .max_msg_size = 128, > }; > diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h > index b458c87b866c..a171c1e293e8 100644 > --- a/include/linux/scmi_protocol.h > +++ b/include/linux/scmi_protocol.h > @@ -189,6 +189,14 @@ struct scmi_sensor_ops { > * @perf_ops: pointer to set of performance protocol operations > * @clk_ops: pointer to set of clock protocol operations > * @sensor_ops: pointer to set of sensor protocol operations > + * @perf_priv: pointer to private data structure specific to performance > + * protocol(for internal use only) > + * @clk_priv: pointer to private data structure specific to clock > + * protocol(for internal use only) > + * @power_priv: pointer to private data structure specific to power > + * protocol(for internal use only) > + * @sensor_priv: pointer to private data structure specific to sensors > + * protocol(for internal use only) > */ > struct scmi_handle { > struct device *dev; ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/8] firmware: arm_scmi: rename get_transition_latency and add_opps_to_device 2018-05-09 17:07 [PATCH 0/8] firmware: arm_scmi: trivial cleanups Sudeep Holla 2018-05-09 17:07 ` [PATCH 1/8] firmware: arm_scmi: improve code readability using bitfield accessor macros Sudeep Holla 2018-05-09 17:07 ` [PATCH 2/8] firmware: arm_scmi: fix kernel-docs documentation Sudeep Holla @ 2018-05-09 17:07 ` Sudeep Holla 2018-05-10 8:29 ` Rafael J. Wysocki 2018-05-17 8:32 ` Jonathan Cameron 2018-05-09 17:07 ` [PATCH 4/8] firmware: arm_scmi: rename scmi_xfer_{init,get,put} Sudeep Holla ` (4 subsequent siblings) 7 siblings, 2 replies; 19+ messages in thread From: Sudeep Holla @ 2018-05-09 17:07 UTC (permalink / raw) To: linux-arm-kernel Most of the scmi code follows the suggestion from Greg KH on a totally different thread[0] to have the subsystem name first, followed by the noun and finally the verb with couple of these exceptions. This patch fixes them so that all the functions names are aligned to on practice. [0] https://www.spinics.net/lists/arm-kernel/msg583673.html Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/cpufreq/scmi-cpufreq.c | 4 ++-- drivers/firmware/arm_scmi/perf.c | 10 +++++----- include/linux/scmi_protocol.h | 10 +++++----- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c index b4dbc77459b6..50b1551ba894 100644 --- a/drivers/cpufreq/scmi-cpufreq.c +++ b/drivers/cpufreq/scmi-cpufreq.c @@ -117,7 +117,7 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy) return -ENODEV; } - ret = handle->perf_ops->add_opps_to_device(handle, cpu_dev); + ret = handle->perf_ops->device_opps_add(handle, cpu_dev); if (ret) { dev_warn(cpu_dev, "failed to add opps to the device\n"); return ret; @@ -164,7 +164,7 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy) /* SCMI allows DVFS request for any domain from any CPU */ policy->dvfs_possible_from_any_cpu = true; - latency = handle->perf_ops->get_transition_latency(handle, cpu_dev); + latency = handle->perf_ops->transition_latency_get(handle, cpu_dev); if (!latency) latency = CPUFREQ_ETERNAL; diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index 987c64d19801..611ab08e6174 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -349,8 +349,8 @@ static int scmi_dev_domain_id(struct device *dev) return clkspec.args[0]; } -static int scmi_dvfs_add_opps_to_device(const struct scmi_handle *handle, - struct device *dev) +static int scmi_dvfs_device_opps_add(const struct scmi_handle *handle, + struct device *dev) { int idx, ret, domain; unsigned long freq; @@ -383,7 +383,7 @@ static int scmi_dvfs_add_opps_to_device(const struct scmi_handle *handle, return 0; } -static int scmi_dvfs_get_transition_latency(const struct scmi_handle *handle, +static int scmi_dvfs_transition_latency_get(const struct scmi_handle *handle, struct device *dev) { struct perf_dom_info *dom; @@ -432,8 +432,8 @@ static struct scmi_perf_ops perf_ops = { .level_set = scmi_perf_level_set, .level_get = scmi_perf_level_get, .device_domain_id = scmi_dev_domain_id, - .get_transition_latency = scmi_dvfs_get_transition_latency, - .add_opps_to_device = scmi_dvfs_add_opps_to_device, + .transition_latency_get = scmi_dvfs_transition_latency_get, + .device_opps_add = scmi_dvfs_device_opps_add, .freq_set = scmi_dvfs_freq_set, .freq_get = scmi_dvfs_freq_get, }; diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index a171c1e293e8..f4c9fc0fc755 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -85,8 +85,8 @@ struct scmi_clk_ops { * @level_set: sets the performance level of a domain * @level_get: gets the performance level of a domain * @device_domain_id: gets the scmi domain id for a given device - * @get_transition_latency: gets the DVFS transition latency for a given device - * @add_opps_to_device: adds all the OPPs for a given device + * @transition_latency_get: gets the DVFS transition latency for a given device + * @device_opps_add: adds all the OPPs for a given device * @freq_set: sets the frequency for a given device using sustained frequency * to sustained performance level mapping * @freq_get: gets the frequency for a given device using sustained frequency @@ -102,10 +102,10 @@ struct scmi_perf_ops { int (*level_get)(const struct scmi_handle *handle, u32 domain, u32 *level, bool poll); int (*device_domain_id)(struct device *dev); - int (*get_transition_latency)(const struct scmi_handle *handle, + int (*transition_latency_get)(const struct scmi_handle *handle, struct device *dev); - int (*add_opps_to_device)(const struct scmi_handle *handle, - struct device *dev); + int (*device_opps_add)(const struct scmi_handle *handle, + struct device *dev); int (*freq_set)(const struct scmi_handle *handle, u32 domain, unsigned long rate, bool poll); int (*freq_get)(const struct scmi_handle *handle, u32 domain, -- 2.7.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/8] firmware: arm_scmi: rename get_transition_latency and add_opps_to_device 2018-05-09 17:07 ` [PATCH 3/8] firmware: arm_scmi: rename get_transition_latency and add_opps_to_device Sudeep Holla @ 2018-05-10 8:29 ` Rafael J. Wysocki 2018-05-10 9:53 ` Sudeep Holla 2018-05-17 8:32 ` Jonathan Cameron 1 sibling, 1 reply; 19+ messages in thread From: Rafael J. Wysocki @ 2018-05-10 8:29 UTC (permalink / raw) To: linux-arm-kernel On Wed, May 9, 2018 at 7:07 PM, Sudeep Holla <sudeep.holla@arm.com> wrote: > Most of the scmi code follows the suggestion from Greg KH on a totally > different thread[0] to have the subsystem name first, followed by the > noun and finally the verb with couple of these exceptions. > > This patch fixes them so that all the functions names are aligned to > on practice. > > [0] https://www.spinics.net/lists/arm-kernel/msg583673.html > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> ACK for the cpufreq driver changes and I'm assuming this to go in via the arch. > --- > drivers/cpufreq/scmi-cpufreq.c | 4 ++-- > drivers/firmware/arm_scmi/perf.c | 10 +++++----- > include/linux/scmi_protocol.h | 10 +++++----- > 3 files changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c > index b4dbc77459b6..50b1551ba894 100644 > --- a/drivers/cpufreq/scmi-cpufreq.c > +++ b/drivers/cpufreq/scmi-cpufreq.c > @@ -117,7 +117,7 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy) > return -ENODEV; > } > > - ret = handle->perf_ops->add_opps_to_device(handle, cpu_dev); > + ret = handle->perf_ops->device_opps_add(handle, cpu_dev); > if (ret) { > dev_warn(cpu_dev, "failed to add opps to the device\n"); > return ret; > @@ -164,7 +164,7 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy) > /* SCMI allows DVFS request for any domain from any CPU */ > policy->dvfs_possible_from_any_cpu = true; > > - latency = handle->perf_ops->get_transition_latency(handle, cpu_dev); > + latency = handle->perf_ops->transition_latency_get(handle, cpu_dev); > if (!latency) > latency = CPUFREQ_ETERNAL; > > diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c > index 987c64d19801..611ab08e6174 100644 > --- a/drivers/firmware/arm_scmi/perf.c > +++ b/drivers/firmware/arm_scmi/perf.c > @@ -349,8 +349,8 @@ static int scmi_dev_domain_id(struct device *dev) > return clkspec.args[0]; > } > > -static int scmi_dvfs_add_opps_to_device(const struct scmi_handle *handle, > - struct device *dev) > +static int scmi_dvfs_device_opps_add(const struct scmi_handle *handle, > + struct device *dev) > { > int idx, ret, domain; > unsigned long freq; > @@ -383,7 +383,7 @@ static int scmi_dvfs_add_opps_to_device(const struct scmi_handle *handle, > return 0; > } > > -static int scmi_dvfs_get_transition_latency(const struct scmi_handle *handle, > +static int scmi_dvfs_transition_latency_get(const struct scmi_handle *handle, > struct device *dev) > { > struct perf_dom_info *dom; > @@ -432,8 +432,8 @@ static struct scmi_perf_ops perf_ops = { > .level_set = scmi_perf_level_set, > .level_get = scmi_perf_level_get, > .device_domain_id = scmi_dev_domain_id, > - .get_transition_latency = scmi_dvfs_get_transition_latency, > - .add_opps_to_device = scmi_dvfs_add_opps_to_device, > + .transition_latency_get = scmi_dvfs_transition_latency_get, > + .device_opps_add = scmi_dvfs_device_opps_add, > .freq_set = scmi_dvfs_freq_set, > .freq_get = scmi_dvfs_freq_get, > }; > diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h > index a171c1e293e8..f4c9fc0fc755 100644 > --- a/include/linux/scmi_protocol.h > +++ b/include/linux/scmi_protocol.h > @@ -85,8 +85,8 @@ struct scmi_clk_ops { > * @level_set: sets the performance level of a domain > * @level_get: gets the performance level of a domain > * @device_domain_id: gets the scmi domain id for a given device > - * @get_transition_latency: gets the DVFS transition latency for a given device > - * @add_opps_to_device: adds all the OPPs for a given device > + * @transition_latency_get: gets the DVFS transition latency for a given device > + * @device_opps_add: adds all the OPPs for a given device > * @freq_set: sets the frequency for a given device using sustained frequency > * to sustained performance level mapping > * @freq_get: gets the frequency for a given device using sustained frequency > @@ -102,10 +102,10 @@ struct scmi_perf_ops { > int (*level_get)(const struct scmi_handle *handle, u32 domain, > u32 *level, bool poll); > int (*device_domain_id)(struct device *dev); > - int (*get_transition_latency)(const struct scmi_handle *handle, > + int (*transition_latency_get)(const struct scmi_handle *handle, > struct device *dev); > - int (*add_opps_to_device)(const struct scmi_handle *handle, > - struct device *dev); > + int (*device_opps_add)(const struct scmi_handle *handle, > + struct device *dev); > int (*freq_set)(const struct scmi_handle *handle, u32 domain, > unsigned long rate, bool poll); > int (*freq_get)(const struct scmi_handle *handle, u32 domain, > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/8] firmware: arm_scmi: rename get_transition_latency and add_opps_to_device 2018-05-10 8:29 ` Rafael J. Wysocki @ 2018-05-10 9:53 ` Sudeep Holla 0 siblings, 0 replies; 19+ messages in thread From: Sudeep Holla @ 2018-05-10 9:53 UTC (permalink / raw) To: linux-arm-kernel On Thu, May 10, 2018 at 10:29:48AM +0200, Rafael J. Wysocki wrote: > On Wed, May 9, 2018 at 7:07 PM, Sudeep Holla <sudeep.holla@arm.com> wrote: > > Most of the scmi code follows the suggestion from Greg KH on a totally > > different thread[0] to have the subsystem name first, followed by the > > noun and finally the verb with couple of these exceptions. > > > > This patch fixes them so that all the functions names are aligned to > > on practice. > > > > [0] https://www.spinics.net/lists/arm-kernel/msg583673.html > > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > ACK for the cpufreq driver changes and I'm assuming this to go in via the arch. > Thanks, yes I plan to send it via ARM-SoC. -- Regards, Sudeep ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/8] firmware: arm_scmi: rename get_transition_latency and add_opps_to_device 2018-05-09 17:07 ` [PATCH 3/8] firmware: arm_scmi: rename get_transition_latency and add_opps_to_device Sudeep Holla 2018-05-10 8:29 ` Rafael J. Wysocki @ 2018-05-17 8:32 ` Jonathan Cameron 1 sibling, 0 replies; 19+ messages in thread From: Jonathan Cameron @ 2018-05-17 8:32 UTC (permalink / raw) To: linux-arm-kernel On Wed, 9 May 2018 18:07:09 +0100 Sudeep Holla <sudeep.holla@arm.com> wrote: > Most of the scmi code follows the suggestion from Greg KH on a totally > different thread[0] to have the subsystem name first, followed by the > noun and finally the verb with couple of these exceptions. > > This patch fixes them so that all the functions names are aligned to > on practice. > > [0] https://www.spinics.net/lists/arm-kernel/msg583673.html > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> Sensible tidy up, for what it's worth Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/cpufreq/scmi-cpufreq.c | 4 ++-- > drivers/firmware/arm_scmi/perf.c | 10 +++++----- > include/linux/scmi_protocol.h | 10 +++++----- > 3 files changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c > index b4dbc77459b6..50b1551ba894 100644 > --- a/drivers/cpufreq/scmi-cpufreq.c > +++ b/drivers/cpufreq/scmi-cpufreq.c > @@ -117,7 +117,7 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy) > return -ENODEV; > } > > - ret = handle->perf_ops->add_opps_to_device(handle, cpu_dev); > + ret = handle->perf_ops->device_opps_add(handle, cpu_dev); > if (ret) { > dev_warn(cpu_dev, "failed to add opps to the device\n"); > return ret; > @@ -164,7 +164,7 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy) > /* SCMI allows DVFS request for any domain from any CPU */ > policy->dvfs_possible_from_any_cpu = true; > > - latency = handle->perf_ops->get_transition_latency(handle, cpu_dev); > + latency = handle->perf_ops->transition_latency_get(handle, cpu_dev); > if (!latency) > latency = CPUFREQ_ETERNAL; > > diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c > index 987c64d19801..611ab08e6174 100644 > --- a/drivers/firmware/arm_scmi/perf.c > +++ b/drivers/firmware/arm_scmi/perf.c > @@ -349,8 +349,8 @@ static int scmi_dev_domain_id(struct device *dev) > return clkspec.args[0]; > } > > -static int scmi_dvfs_add_opps_to_device(const struct scmi_handle *handle, > - struct device *dev) > +static int scmi_dvfs_device_opps_add(const struct scmi_handle *handle, > + struct device *dev) > { > int idx, ret, domain; > unsigned long freq; > @@ -383,7 +383,7 @@ static int scmi_dvfs_add_opps_to_device(const struct scmi_handle *handle, > return 0; > } > > -static int scmi_dvfs_get_transition_latency(const struct scmi_handle *handle, > +static int scmi_dvfs_transition_latency_get(const struct scmi_handle *handle, > struct device *dev) > { > struct perf_dom_info *dom; > @@ -432,8 +432,8 @@ static struct scmi_perf_ops perf_ops = { > .level_set = scmi_perf_level_set, > .level_get = scmi_perf_level_get, > .device_domain_id = scmi_dev_domain_id, > - .get_transition_latency = scmi_dvfs_get_transition_latency, > - .add_opps_to_device = scmi_dvfs_add_opps_to_device, > + .transition_latency_get = scmi_dvfs_transition_latency_get, > + .device_opps_add = scmi_dvfs_device_opps_add, > .freq_set = scmi_dvfs_freq_set, > .freq_get = scmi_dvfs_freq_get, > }; > diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h > index a171c1e293e8..f4c9fc0fc755 100644 > --- a/include/linux/scmi_protocol.h > +++ b/include/linux/scmi_protocol.h > @@ -85,8 +85,8 @@ struct scmi_clk_ops { > * @level_set: sets the performance level of a domain > * @level_get: gets the performance level of a domain > * @device_domain_id: gets the scmi domain id for a given device > - * @get_transition_latency: gets the DVFS transition latency for a given device > - * @add_opps_to_device: adds all the OPPs for a given device > + * @transition_latency_get: gets the DVFS transition latency for a given device > + * @device_opps_add: adds all the OPPs for a given device > * @freq_set: sets the frequency for a given device using sustained frequency > * to sustained performance level mapping > * @freq_get: gets the frequency for a given device using sustained frequency > @@ -102,10 +102,10 @@ struct scmi_perf_ops { > int (*level_get)(const struct scmi_handle *handle, u32 domain, > u32 *level, bool poll); > int (*device_domain_id)(struct device *dev); > - int (*get_transition_latency)(const struct scmi_handle *handle, > + int (*transition_latency_get)(const struct scmi_handle *handle, > struct device *dev); > - int (*add_opps_to_device)(const struct scmi_handle *handle, > - struct device *dev); > + int (*device_opps_add)(const struct scmi_handle *handle, > + struct device *dev); > int (*freq_set)(const struct scmi_handle *handle, u32 domain, > unsigned long rate, bool poll); > int (*freq_get)(const struct scmi_handle *handle, u32 domain, ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/8] firmware: arm_scmi: rename scmi_xfer_{init,get,put} 2018-05-09 17:07 [PATCH 0/8] firmware: arm_scmi: trivial cleanups Sudeep Holla ` (2 preceding siblings ...) 2018-05-09 17:07 ` [PATCH 3/8] firmware: arm_scmi: rename get_transition_latency and add_opps_to_device Sudeep Holla @ 2018-05-09 17:07 ` Sudeep Holla 2018-05-17 8:38 ` Jonathan Cameron 2018-05-09 17:07 ` [PATCH 5/8] firmware: arm_scmi: drop unused `con_priv` structure member Sudeep Holla ` (3 subsequent siblings) 7 siblings, 1 reply; 19+ messages in thread From: Sudeep Holla @ 2018-05-09 17:07 UTC (permalink / raw) To: linux-arm-kernel Just after the initial patches were queued, Jonathan Cameron mentioned that scmi_one_xfer_{get_put} were not very clear and suggested to use scmi_xfer_{alloc,free}. While I agree to some extent, the reason not to have alloc/free as these are preallocated buffers and these functions just returns a reference to free slot in that preallocated array. However it was agreed to drop "_one" as it's implicit that we are always dealing with one slot anyways. This patch updates the name accordingly dropping "_one" in both {get,put} functions. Also scmi_one_xfer_init is renamed as scmi_xfer_get_init to reflect the fact that it gets the free slots and then initialise it. Reported-by: Jonathan Cameron <jonathan.cameron@huawei.com> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/firmware/arm_scmi/base.c | 23 +++++++++++++---------- drivers/firmware/arm_scmi/clock.c | 24 ++++++++++++------------ drivers/firmware/arm_scmi/common.h | 4 ++-- drivers/firmware/arm_scmi/driver.c | 20 ++++++++++---------- drivers/firmware/arm_scmi/perf.c | 28 ++++++++++++++-------------- drivers/firmware/arm_scmi/power.c | 16 ++++++++-------- drivers/firmware/arm_scmi/sensors.c | 20 ++++++++++---------- 7 files changed, 69 insertions(+), 66 deletions(-) diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c index c36ded9dbb83..9dff33ea6416 100644 --- a/drivers/firmware/arm_scmi/base.c +++ b/drivers/firmware/arm_scmi/base.c @@ -37,7 +37,7 @@ static int scmi_base_attributes_get(const struct scmi_handle *handle) struct scmi_msg_resp_base_attributes *attr_info; struct scmi_revision_info *rev = handle->version; - ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES, + ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES, SCMI_PROTOCOL_BASE, 0, sizeof(*attr_info), &t); if (ret) return ret; @@ -49,7 +49,7 @@ static int scmi_base_attributes_get(const struct scmi_handle *handle) rev->num_agents = attr_info->num_agents; } - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -81,7 +81,7 @@ scmi_base_vendor_id_get(const struct scmi_handle *handle, bool sub_vendor) size = ARRAY_SIZE(rev->vendor_id); } - ret = scmi_one_xfer_init(handle, cmd, SCMI_PROTOCOL_BASE, 0, size, &t); + ret = scmi_xfer_get_init(handle, cmd, SCMI_PROTOCOL_BASE, 0, size, &t); if (ret) return ret; @@ -89,7 +89,8 @@ scmi_base_vendor_id_get(const struct scmi_handle *handle, bool sub_vendor) if (!ret) memcpy(vendor_id, t->rx.buf, size); - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); + return ret; } @@ -110,7 +111,7 @@ scmi_base_implementation_version_get(const struct scmi_handle *handle) struct scmi_xfer *t; struct scmi_revision_info *rev = handle->version; - ret = scmi_one_xfer_init(handle, BASE_DISCOVER_IMPLEMENT_VERSION, + ret = scmi_xfer_get_init(handle, BASE_DISCOVER_IMPLEMENT_VERSION, SCMI_PROTOCOL_BASE, 0, sizeof(*impl_ver), &t); if (ret) return ret; @@ -121,7 +122,8 @@ scmi_base_implementation_version_get(const struct scmi_handle *handle) rev->impl_ver = le32_to_cpu(*impl_ver); } - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); + return ret; } @@ -144,7 +146,7 @@ static int scmi_base_implementation_list_get(const struct scmi_handle *handle, u32 tot_num_ret = 0, loop_num_ret; struct device *dev = handle->dev; - ret = scmi_one_xfer_init(handle, BASE_DISCOVER_LIST_PROTOCOLS, + ret = scmi_xfer_get_init(handle, BASE_DISCOVER_LIST_PROTOCOLS, SCMI_PROTOCOL_BASE, sizeof(*num_skip), 0, &t); if (ret) return ret; @@ -173,7 +175,7 @@ static int scmi_base_implementation_list_get(const struct scmi_handle *handle, tot_num_ret += loop_num_ret; } while (loop_num_ret); - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -196,7 +198,7 @@ static int scmi_base_discover_agent_get(const struct scmi_handle *handle, int ret; struct scmi_xfer *t; - ret = scmi_one_xfer_init(handle, BASE_DISCOVER_AGENT, + ret = scmi_xfer_get_init(handle, BASE_DISCOVER_AGENT, SCMI_PROTOCOL_BASE, sizeof(__le32), SCMI_MAX_STR_SIZE, &t); if (ret) @@ -208,7 +210,8 @@ static int scmi_base_discover_agent_get(const struct scmi_handle *handle, if (!ret) memcpy(name, t->rx.buf, SCMI_MAX_STR_SIZE); - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); + return ret; } diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c index e6f17825db79..3874666a8a14 100644 --- a/drivers/firmware/arm_scmi/clock.c +++ b/drivers/firmware/arm_scmi/clock.c @@ -77,7 +77,7 @@ static int scmi_clock_protocol_attributes_get(const struct scmi_handle *handle, struct scmi_xfer *t; struct scmi_msg_resp_clock_protocol_attributes *attr; - ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES, + ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES, SCMI_PROTOCOL_CLOCK, 0, sizeof(*attr), &t); if (ret) return ret; @@ -90,7 +90,7 @@ static int scmi_clock_protocol_attributes_get(const struct scmi_handle *handle, ci->max_async_req = attr->max_async_req; } - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -101,7 +101,7 @@ static int scmi_clock_attributes_get(const struct scmi_handle *handle, struct scmi_xfer *t; struct scmi_msg_resp_clock_attributes *attr; - ret = scmi_one_xfer_init(handle, CLOCK_ATTRIBUTES, SCMI_PROTOCOL_CLOCK, + ret = scmi_xfer_get_init(handle, CLOCK_ATTRIBUTES, SCMI_PROTOCOL_CLOCK, sizeof(clk_id), sizeof(*attr), &t); if (ret) return ret; @@ -115,7 +115,7 @@ static int scmi_clock_attributes_get(const struct scmi_handle *handle, else clk->name[0] = '\0'; - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -132,7 +132,7 @@ scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, struct scmi_msg_clock_describe_rates *clk_desc; struct scmi_msg_resp_clock_describe_rates *rlist; - ret = scmi_one_xfer_init(handle, CLOCK_DESCRIBE_RATES, + ret = scmi_xfer_get_init(handle, CLOCK_DESCRIBE_RATES, SCMI_PROTOCOL_CLOCK, sizeof(*clk_desc), 0, &t); if (ret) return ret; @@ -186,7 +186,7 @@ scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, clk->list.num_rates = tot_rate_cnt; err: - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -196,7 +196,7 @@ scmi_clock_rate_get(const struct scmi_handle *handle, u32 clk_id, u64 *value) int ret; struct scmi_xfer *t; - ret = scmi_one_xfer_init(handle, CLOCK_RATE_GET, SCMI_PROTOCOL_CLOCK, + ret = scmi_xfer_get_init(handle, CLOCK_RATE_GET, SCMI_PROTOCOL_CLOCK, sizeof(__le32), sizeof(u64), &t); if (ret) return ret; @@ -211,7 +211,7 @@ scmi_clock_rate_get(const struct scmi_handle *handle, u32 clk_id, u64 *value) *value |= (u64)le32_to_cpu(*(pval + 1)) << 32; } - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -222,7 +222,7 @@ static int scmi_clock_rate_set(const struct scmi_handle *handle, u32 clk_id, struct scmi_xfer *t; struct scmi_clock_set_rate *cfg; - ret = scmi_one_xfer_init(handle, CLOCK_RATE_SET, SCMI_PROTOCOL_CLOCK, + ret = scmi_xfer_get_init(handle, CLOCK_RATE_SET, SCMI_PROTOCOL_CLOCK, sizeof(*cfg), 0, &t); if (ret) return ret; @@ -235,7 +235,7 @@ static int scmi_clock_rate_set(const struct scmi_handle *handle, u32 clk_id, ret = scmi_do_xfer(handle, t); - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -246,7 +246,7 @@ scmi_clock_config_set(const struct scmi_handle *handle, u32 clk_id, u32 config) struct scmi_xfer *t; struct scmi_clock_set_config *cfg; - ret = scmi_one_xfer_init(handle, CLOCK_CONFIG_SET, SCMI_PROTOCOL_CLOCK, + ret = scmi_xfer_get_init(handle, CLOCK_CONFIG_SET, SCMI_PROTOCOL_CLOCK, sizeof(*cfg), 0, &t); if (ret) return ret; @@ -257,7 +257,7 @@ scmi_clock_config_set(const struct scmi_handle *handle, u32 clk_id, u32 config) ret = scmi_do_xfer(handle, t); - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 0821662a4633..41b03e46cca8 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -95,9 +95,9 @@ struct scmi_xfer { struct completion done; }; -void scmi_one_xfer_put(const struct scmi_handle *h, struct scmi_xfer *xfer); +void scmi_xfer_put(const struct scmi_handle *h, struct scmi_xfer *xfer); int scmi_do_xfer(const struct scmi_handle *h, struct scmi_xfer *xfer); -int scmi_one_xfer_init(const struct scmi_handle *h, u8 msg_id, u8 prot_id, +int scmi_xfer_get_init(const struct scmi_handle *h, u8 msg_id, u8 prot_id, size_t tx_size, size_t rx_size, struct scmi_xfer **p); int scmi_handle_put(const struct scmi_handle *handle); struct scmi_handle *scmi_handle_get(struct device *dev); diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 6fee11f06a66..33d2b78af3ff 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -295,7 +295,7 @@ static void scmi_tx_prepare(struct mbox_client *cl, void *m) * * Return: 0 if all went fine, else corresponding error. */ -static struct scmi_xfer *scmi_one_xfer_get(const struct scmi_handle *handle) +static struct scmi_xfer *scmi_xfer_get(const struct scmi_handle *handle) { u16 xfer_id; struct scmi_xfer *xfer; @@ -324,14 +324,14 @@ static struct scmi_xfer *scmi_one_xfer_get(const struct scmi_handle *handle) } /** - * scmi_one_xfer_put() - Release a message + * scmi_xfer_put() - Release a message * * @handle: Pointer to SCMI entity handle * @xfer: message that was reserved by scmi_xfer_get * * This holds a spinlock to maintain integrity of internal data structures. */ -void scmi_one_xfer_put(const struct scmi_handle *handle, struct scmi_xfer *xfer) +void scmi_xfer_put(const struct scmi_handle *handle, struct scmi_xfer *xfer) { unsigned long flags; struct scmi_info *info = handle_to_scmi_info(handle); @@ -436,7 +436,7 @@ int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer) } /** - * scmi_one_xfer_init() - Allocate and initialise one message + * scmi_xfer_get_init() - Allocate and initialise one message * * @handle: Pointer to SCMI entity handle * @msg_id: Message identifier @@ -445,13 +445,13 @@ int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer) * @rx_size: receive message size * @p: pointer to the allocated and initialised message * - * This function allocates the message using @scmi_one_xfer_get and + * This function allocates the message using @scmi_xfer_get and * initialise the header. * * Return: 0 if all went fine with @p pointing to message, else * corresponding error. */ -int scmi_one_xfer_init(const struct scmi_handle *handle, u8 msg_id, u8 prot_id, +int scmi_xfer_get_init(const struct scmi_handle *handle, u8 msg_id, u8 prot_id, size_t tx_size, size_t rx_size, struct scmi_xfer **p) { int ret; @@ -464,7 +464,7 @@ int scmi_one_xfer_init(const struct scmi_handle *handle, u8 msg_id, u8 prot_id, tx_size > info->desc->max_msg_size) return -ERANGE; - xfer = scmi_one_xfer_get(handle); + xfer = scmi_xfer_get(handle); if (IS_ERR(xfer)) { ret = PTR_ERR(xfer); dev_err(dev, "failed to get free message slot(%d)\n", ret); @@ -500,7 +500,7 @@ int scmi_version_get(const struct scmi_handle *handle, u8 protocol, __le32 *rev_info; struct scmi_xfer *t; - ret = scmi_one_xfer_init(handle, PROTOCOL_VERSION, protocol, 0, + ret = scmi_xfer_get_init(handle, PROTOCOL_VERSION, protocol, 0, sizeof(*version), &t); if (ret) return ret; @@ -511,7 +511,7 @@ int scmi_version_get(const struct scmi_handle *handle, u8 protocol, *version = le32_to_cpu(*rev_info); } - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -539,7 +539,7 @@ scmi_is_protocol_implemented(const struct scmi_handle *handle, u8 prot_id) } /** - * scmi_handle_get() - Get the SCMI handle for a device + * scmi_handle_get() - Get the SCMI handle for a device * * @dev: pointer to device for which we want SCMI handle * diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index 611ab08e6174..2a219b1261b1 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -115,7 +115,7 @@ static int scmi_perf_attributes_get(const struct scmi_handle *handle, struct scmi_xfer *t; struct scmi_msg_resp_perf_attributes *attr; - ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES, + ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES, SCMI_PROTOCOL_PERF, 0, sizeof(*attr), &t); if (ret) return ret; @@ -133,7 +133,7 @@ static int scmi_perf_attributes_get(const struct scmi_handle *handle, pi->stats_size = le32_to_cpu(attr->stats_size); } - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -145,7 +145,7 @@ scmi_perf_domain_attributes_get(const struct scmi_handle *handle, u32 domain, struct scmi_xfer *t; struct scmi_msg_resp_perf_domain_attributes *attr; - ret = scmi_one_xfer_init(handle, PERF_DOMAIN_ATTRIBUTES, + ret = scmi_xfer_get_init(handle, PERF_DOMAIN_ATTRIBUTES, SCMI_PROTOCOL_PERF, sizeof(domain), sizeof(*attr), &t); if (ret) @@ -171,7 +171,7 @@ scmi_perf_domain_attributes_get(const struct scmi_handle *handle, u32 domain, memcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE); } - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -194,7 +194,7 @@ scmi_perf_describe_levels_get(const struct scmi_handle *handle, u32 domain, struct scmi_msg_perf_describe_levels *dom_info; struct scmi_msg_resp_perf_describe_levels *level_info; - ret = scmi_one_xfer_init(handle, PERF_DESCRIBE_LEVELS, + ret = scmi_xfer_get_init(handle, PERF_DESCRIBE_LEVELS, SCMI_PROTOCOL_PERF, sizeof(*dom_info), 0, &t); if (ret) return ret; @@ -237,7 +237,7 @@ scmi_perf_describe_levels_get(const struct scmi_handle *handle, u32 domain, } while (num_returned && num_remaining); perf_dom->opp_count = tot_opp_cnt; - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); sort(perf_dom->opp, tot_opp_cnt, sizeof(*opp), opp_cmp_func, NULL); return ret; @@ -250,7 +250,7 @@ static int scmi_perf_limits_set(const struct scmi_handle *handle, u32 domain, struct scmi_xfer *t; struct scmi_perf_set_limits *limits; - ret = scmi_one_xfer_init(handle, PERF_LIMITS_SET, SCMI_PROTOCOL_PERF, + ret = scmi_xfer_get_init(handle, PERF_LIMITS_SET, SCMI_PROTOCOL_PERF, sizeof(*limits), 0, &t); if (ret) return ret; @@ -262,7 +262,7 @@ static int scmi_perf_limits_set(const struct scmi_handle *handle, u32 domain, ret = scmi_do_xfer(handle, t); - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -273,7 +273,7 @@ static int scmi_perf_limits_get(const struct scmi_handle *handle, u32 domain, struct scmi_xfer *t; struct scmi_perf_get_limits *limits; - ret = scmi_one_xfer_init(handle, PERF_LIMITS_GET, SCMI_PROTOCOL_PERF, + ret = scmi_xfer_get_init(handle, PERF_LIMITS_GET, SCMI_PROTOCOL_PERF, sizeof(__le32), 0, &t); if (ret) return ret; @@ -288,7 +288,7 @@ static int scmi_perf_limits_get(const struct scmi_handle *handle, u32 domain, *min_perf = le32_to_cpu(limits->min_level); } - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -299,7 +299,7 @@ static int scmi_perf_level_set(const struct scmi_handle *handle, u32 domain, struct scmi_xfer *t; struct scmi_perf_set_level *lvl; - ret = scmi_one_xfer_init(handle, PERF_LEVEL_SET, SCMI_PROTOCOL_PERF, + ret = scmi_xfer_get_init(handle, PERF_LEVEL_SET, SCMI_PROTOCOL_PERF, sizeof(*lvl), 0, &t); if (ret) return ret; @@ -311,7 +311,7 @@ static int scmi_perf_level_set(const struct scmi_handle *handle, u32 domain, ret = scmi_do_xfer(handle, t); - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -321,7 +321,7 @@ static int scmi_perf_level_get(const struct scmi_handle *handle, u32 domain, int ret; struct scmi_xfer *t; - ret = scmi_one_xfer_init(handle, PERF_LEVEL_GET, SCMI_PROTOCOL_PERF, + ret = scmi_xfer_get_init(handle, PERF_LEVEL_GET, SCMI_PROTOCOL_PERF, sizeof(u32), sizeof(u32), &t); if (ret) return ret; @@ -333,7 +333,7 @@ static int scmi_perf_level_get(const struct scmi_handle *handle, u32 domain, if (!ret) *level = le32_to_cpu(*(__le32 *)t->rx.buf); - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } diff --git a/drivers/firmware/arm_scmi/power.c b/drivers/firmware/arm_scmi/power.c index 087c2876cdf2..cfa033b05aed 100644 --- a/drivers/firmware/arm_scmi/power.c +++ b/drivers/firmware/arm_scmi/power.c @@ -63,7 +63,7 @@ static int scmi_power_attributes_get(const struct scmi_handle *handle, struct scmi_xfer *t; struct scmi_msg_resp_power_attributes *attr; - ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES, + ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES, SCMI_PROTOCOL_POWER, 0, sizeof(*attr), &t); if (ret) return ret; @@ -78,7 +78,7 @@ static int scmi_power_attributes_get(const struct scmi_handle *handle, pi->stats_size = le32_to_cpu(attr->stats_size); } - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -90,7 +90,7 @@ scmi_power_domain_attributes_get(const struct scmi_handle *handle, u32 domain, struct scmi_xfer *t; struct scmi_msg_resp_power_domain_attributes *attr; - ret = scmi_one_xfer_init(handle, POWER_DOMAIN_ATTRIBUTES, + ret = scmi_xfer_get_init(handle, POWER_DOMAIN_ATTRIBUTES, SCMI_PROTOCOL_POWER, sizeof(domain), sizeof(*attr), &t); if (ret) @@ -109,7 +109,7 @@ scmi_power_domain_attributes_get(const struct scmi_handle *handle, u32 domain, memcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE); } - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -120,7 +120,7 @@ scmi_power_state_set(const struct scmi_handle *handle, u32 domain, u32 state) struct scmi_xfer *t; struct scmi_power_set_state *st; - ret = scmi_one_xfer_init(handle, POWER_STATE_SET, SCMI_PROTOCOL_POWER, + ret = scmi_xfer_get_init(handle, POWER_STATE_SET, SCMI_PROTOCOL_POWER, sizeof(*st), 0, &t); if (ret) return ret; @@ -132,7 +132,7 @@ scmi_power_state_set(const struct scmi_handle *handle, u32 domain, u32 state) ret = scmi_do_xfer(handle, t); - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -142,7 +142,7 @@ scmi_power_state_get(const struct scmi_handle *handle, u32 domain, u32 *state) int ret; struct scmi_xfer *t; - ret = scmi_one_xfer_init(handle, POWER_STATE_GET, SCMI_PROTOCOL_POWER, + ret = scmi_xfer_get_init(handle, POWER_STATE_GET, SCMI_PROTOCOL_POWER, sizeof(u32), sizeof(u32), &t); if (ret) return ret; @@ -153,7 +153,7 @@ scmi_power_state_get(const struct scmi_handle *handle, u32 domain, u32 *state) if (!ret) *state = le32_to_cpu(*(__le32 *)t->rx.buf); - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c index bbb469fea0ed..27f2092b9882 100644 --- a/drivers/firmware/arm_scmi/sensors.c +++ b/drivers/firmware/arm_scmi/sensors.c @@ -79,7 +79,7 @@ static int scmi_sensor_attributes_get(const struct scmi_handle *handle, struct scmi_xfer *t; struct scmi_msg_resp_sensor_attributes *attr; - ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES, + ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES, SCMI_PROTOCOL_SENSOR, 0, sizeof(*attr), &t); if (ret) return ret; @@ -95,7 +95,7 @@ static int scmi_sensor_attributes_get(const struct scmi_handle *handle, si->reg_size = le32_to_cpu(attr->reg_size); } - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -108,7 +108,7 @@ static int scmi_sensor_description_get(const struct scmi_handle *handle, struct scmi_xfer *t; struct scmi_msg_resp_sensor_description *buf; - ret = scmi_one_xfer_init(handle, SENSOR_DESCRIPTION_GET, + ret = scmi_xfer_get_init(handle, SENSOR_DESCRIPTION_GET, SCMI_PROTOCOL_SENSOR, sizeof(__le32), 0, &t); if (ret) return ret; @@ -150,7 +150,7 @@ static int scmi_sensor_description_get(const struct scmi_handle *handle, */ } while (num_returned && num_remaining); - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -162,7 +162,7 @@ scmi_sensor_configuration_set(const struct scmi_handle *handle, u32 sensor_id) struct scmi_xfer *t; struct scmi_msg_set_sensor_config *cfg; - ret = scmi_one_xfer_init(handle, SENSOR_CONFIG_SET, + ret = scmi_xfer_get_init(handle, SENSOR_CONFIG_SET, SCMI_PROTOCOL_SENSOR, sizeof(*cfg), 0, &t); if (ret) return ret; @@ -173,7 +173,7 @@ scmi_sensor_configuration_set(const struct scmi_handle *handle, u32 sensor_id) ret = scmi_do_xfer(handle, t); - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -185,7 +185,7 @@ static int scmi_sensor_trip_point_set(const struct scmi_handle *handle, struct scmi_xfer *t; struct scmi_msg_set_sensor_trip_point *trip; - ret = scmi_one_xfer_init(handle, SENSOR_TRIP_POINT_SET, + ret = scmi_xfer_get_init(handle, SENSOR_TRIP_POINT_SET, SCMI_PROTOCOL_SENSOR, sizeof(*trip), 0, &t); if (ret) return ret; @@ -198,7 +198,7 @@ static int scmi_sensor_trip_point_set(const struct scmi_handle *handle, ret = scmi_do_xfer(handle, t); - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } @@ -209,7 +209,7 @@ static int scmi_sensor_reading_get(const struct scmi_handle *handle, struct scmi_xfer *t; struct scmi_msg_sensor_reading_get *sensor; - ret = scmi_one_xfer_init(handle, SENSOR_READING_GET, + ret = scmi_xfer_get_init(handle, SENSOR_READING_GET, SCMI_PROTOCOL_SENSOR, sizeof(*sensor), sizeof(u64), &t); if (ret) @@ -227,7 +227,7 @@ static int scmi_sensor_reading_get(const struct scmi_handle *handle, *value |= (u64)le32_to_cpu(*(pval + 1)) << 32; } - scmi_one_xfer_put(handle, t); + scmi_xfer_put(handle, t); return ret; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/8] firmware: arm_scmi: rename scmi_xfer_{init,get,put} 2018-05-09 17:07 ` [PATCH 4/8] firmware: arm_scmi: rename scmi_xfer_{init,get,put} Sudeep Holla @ 2018-05-17 8:38 ` Jonathan Cameron 0 siblings, 0 replies; 19+ messages in thread From: Jonathan Cameron @ 2018-05-17 8:38 UTC (permalink / raw) To: linux-arm-kernel On Wed, 9 May 2018 18:07:10 +0100 Sudeep Holla <sudeep.holla@arm.com> wrote: > Just after the initial patches were queued, Jonathan Cameron mentioned > that scmi_one_xfer_{get_put} were not very clear and suggested to use > scmi_xfer_{alloc,free}. While I agree to some extent, the reason not to > have alloc/free as these are preallocated buffers and these functions > just returns a reference to free slot in that preallocated array. > However it was agreed to drop "_one" as it's implicit that we are always > dealing with one slot anyways. > > This patch updates the name accordingly dropping "_one" in both {get,put} > functions. Also scmi_one_xfer_init is renamed as scmi_xfer_get_init to > reflect the fact that it gets the free slots and then initialise it. > > Reported-by: Jonathan Cameron <jonathan.cameron@huawei.com> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> Hi Sudeep, Mostly fine, but one block of comment change that is unrelated has snuck into this patch. I think it should be in the earlier kernel-doc fix up patch. You have also been a little inconsistent on whether to add blank lines before return ret statements. This is really trivial so up to you on whether to fix that up in v2. With the doc line moved to the other patch. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/firmware/arm_scmi/base.c | 23 +++++++++++++---------- > drivers/firmware/arm_scmi/clock.c | 24 ++++++++++++------------ > drivers/firmware/arm_scmi/common.h | 4 ++-- > drivers/firmware/arm_scmi/driver.c | 20 ++++++++++---------- > drivers/firmware/arm_scmi/perf.c | 28 ++++++++++++++-------------- > drivers/firmware/arm_scmi/power.c | 16 ++++++++-------- > drivers/firmware/arm_scmi/sensors.c | 20 ++++++++++---------- > 7 files changed, 69 insertions(+), 66 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c > index c36ded9dbb83..9dff33ea6416 100644 > --- a/drivers/firmware/arm_scmi/base.c > +++ b/drivers/firmware/arm_scmi/base.c > @@ -37,7 +37,7 @@ static int scmi_base_attributes_get(const struct scmi_handle *handle) > struct scmi_msg_resp_base_attributes *attr_info; > struct scmi_revision_info *rev = handle->version; > > - ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES, > + ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES, > SCMI_PROTOCOL_BASE, 0, sizeof(*attr_info), &t); > if (ret) > return ret; > @@ -49,7 +49,7 @@ static int scmi_base_attributes_get(const struct scmi_handle *handle) > rev->num_agents = attr_info->num_agents; > } > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > > return ret; > } > @@ -81,7 +81,7 @@ scmi_base_vendor_id_get(const struct scmi_handle *handle, bool sub_vendor) > size = ARRAY_SIZE(rev->vendor_id); > } > > - ret = scmi_one_xfer_init(handle, cmd, SCMI_PROTOCOL_BASE, 0, size, &t); > + ret = scmi_xfer_get_init(handle, cmd, SCMI_PROTOCOL_BASE, 0, size, &t); > if (ret) > return ret; > > @@ -89,7 +89,8 @@ scmi_base_vendor_id_get(const struct scmi_handle *handle, bool sub_vendor) > if (!ret) > memcpy(vendor_id, t->rx.buf, size); > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > + > return ret; > } > > @@ -110,7 +111,7 @@ scmi_base_implementation_version_get(const struct scmi_handle *handle) > struct scmi_xfer *t; > struct scmi_revision_info *rev = handle->version; > > - ret = scmi_one_xfer_init(handle, BASE_DISCOVER_IMPLEMENT_VERSION, > + ret = scmi_xfer_get_init(handle, BASE_DISCOVER_IMPLEMENT_VERSION, > SCMI_PROTOCOL_BASE, 0, sizeof(*impl_ver), &t); > if (ret) > return ret; > @@ -121,7 +122,8 @@ scmi_base_implementation_version_get(const struct scmi_handle *handle) > rev->impl_ver = le32_to_cpu(*impl_ver); > } > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > + > return ret; > } > > @@ -144,7 +146,7 @@ static int scmi_base_implementation_list_get(const struct scmi_handle *handle, > u32 tot_num_ret = 0, loop_num_ret; > struct device *dev = handle->dev; > > - ret = scmi_one_xfer_init(handle, BASE_DISCOVER_LIST_PROTOCOLS, > + ret = scmi_xfer_get_init(handle, BASE_DISCOVER_LIST_PROTOCOLS, > SCMI_PROTOCOL_BASE, sizeof(*num_skip), 0, &t); > if (ret) > return ret; > @@ -173,7 +175,7 @@ static int scmi_base_implementation_list_get(const struct scmi_handle *handle, > tot_num_ret += loop_num_ret; > } while (loop_num_ret); > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > > return ret; > } > @@ -196,7 +198,7 @@ static int scmi_base_discover_agent_get(const struct scmi_handle *handle, > int ret; > struct scmi_xfer *t; > > - ret = scmi_one_xfer_init(handle, BASE_DISCOVER_AGENT, > + ret = scmi_xfer_get_init(handle, BASE_DISCOVER_AGENT, > SCMI_PROTOCOL_BASE, sizeof(__le32), > SCMI_MAX_STR_SIZE, &t); > if (ret) > @@ -208,7 +210,8 @@ static int scmi_base_discover_agent_get(const struct scmi_handle *handle, > if (!ret) > memcpy(name, t->rx.buf, SCMI_MAX_STR_SIZE); > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > + I actually agree that it is sensible to drop this extra line in here as you are working on the related code (and it improves readability a tiny bit). However, if you are going to do it, do them all. There are other places below that would benefit from this little change. > return ret; > } > > diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c > index e6f17825db79..3874666a8a14 100644 > --- a/drivers/firmware/arm_scmi/clock.c > +++ b/drivers/firmware/arm_scmi/clock.c > @@ -77,7 +77,7 @@ static int scmi_clock_protocol_attributes_get(const struct scmi_handle *handle, > struct scmi_xfer *t; > struct scmi_msg_resp_clock_protocol_attributes *attr; > > - ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES, > + ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES, > SCMI_PROTOCOL_CLOCK, 0, sizeof(*attr), &t); > if (ret) > return ret; > @@ -90,7 +90,7 @@ static int scmi_clock_protocol_attributes_get(const struct scmi_handle *handle, > ci->max_async_req = attr->max_async_req; > } > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -101,7 +101,7 @@ static int scmi_clock_attributes_get(const struct scmi_handle *handle, > struct scmi_xfer *t; > struct scmi_msg_resp_clock_attributes *attr; > > - ret = scmi_one_xfer_init(handle, CLOCK_ATTRIBUTES, SCMI_PROTOCOL_CLOCK, > + ret = scmi_xfer_get_init(handle, CLOCK_ATTRIBUTES, SCMI_PROTOCOL_CLOCK, > sizeof(clk_id), sizeof(*attr), &t); > if (ret) > return ret; > @@ -115,7 +115,7 @@ static int scmi_clock_attributes_get(const struct scmi_handle *handle, > else > clk->name[0] = '\0'; > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -132,7 +132,7 @@ scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > struct scmi_msg_clock_describe_rates *clk_desc; > struct scmi_msg_resp_clock_describe_rates *rlist; > > - ret = scmi_one_xfer_init(handle, CLOCK_DESCRIBE_RATES, > + ret = scmi_xfer_get_init(handle, CLOCK_DESCRIBE_RATES, > SCMI_PROTOCOL_CLOCK, sizeof(*clk_desc), 0, &t); > if (ret) > return ret; > @@ -186,7 +186,7 @@ scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > clk->list.num_rates = tot_rate_cnt; > > err: > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -196,7 +196,7 @@ scmi_clock_rate_get(const struct scmi_handle *handle, u32 clk_id, u64 *value) > int ret; > struct scmi_xfer *t; > > - ret = scmi_one_xfer_init(handle, CLOCK_RATE_GET, SCMI_PROTOCOL_CLOCK, > + ret = scmi_xfer_get_init(handle, CLOCK_RATE_GET, SCMI_PROTOCOL_CLOCK, > sizeof(__le32), sizeof(u64), &t); > if (ret) > return ret; > @@ -211,7 +211,7 @@ scmi_clock_rate_get(const struct scmi_handle *handle, u32 clk_id, u64 *value) > *value |= (u64)le32_to_cpu(*(pval + 1)) << 32; > } > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -222,7 +222,7 @@ static int scmi_clock_rate_set(const struct scmi_handle *handle, u32 clk_id, > struct scmi_xfer *t; > struct scmi_clock_set_rate *cfg; > > - ret = scmi_one_xfer_init(handle, CLOCK_RATE_SET, SCMI_PROTOCOL_CLOCK, > + ret = scmi_xfer_get_init(handle, CLOCK_RATE_SET, SCMI_PROTOCOL_CLOCK, > sizeof(*cfg), 0, &t); > if (ret) > return ret; > @@ -235,7 +235,7 @@ static int scmi_clock_rate_set(const struct scmi_handle *handle, u32 clk_id, > > ret = scmi_do_xfer(handle, t); > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -246,7 +246,7 @@ scmi_clock_config_set(const struct scmi_handle *handle, u32 clk_id, u32 config) > struct scmi_xfer *t; > struct scmi_clock_set_config *cfg; > > - ret = scmi_one_xfer_init(handle, CLOCK_CONFIG_SET, SCMI_PROTOCOL_CLOCK, > + ret = scmi_xfer_get_init(handle, CLOCK_CONFIG_SET, SCMI_PROTOCOL_CLOCK, > sizeof(*cfg), 0, &t); > if (ret) > return ret; > @@ -257,7 +257,7 @@ scmi_clock_config_set(const struct scmi_handle *handle, u32 clk_id, u32 config) > > ret = scmi_do_xfer(handle, t); > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h > index 0821662a4633..41b03e46cca8 100644 > --- a/drivers/firmware/arm_scmi/common.h > +++ b/drivers/firmware/arm_scmi/common.h > @@ -95,9 +95,9 @@ struct scmi_xfer { > struct completion done; > }; > > -void scmi_one_xfer_put(const struct scmi_handle *h, struct scmi_xfer *xfer); > +void scmi_xfer_put(const struct scmi_handle *h, struct scmi_xfer *xfer); > int scmi_do_xfer(const struct scmi_handle *h, struct scmi_xfer *xfer); > -int scmi_one_xfer_init(const struct scmi_handle *h, u8 msg_id, u8 prot_id, > +int scmi_xfer_get_init(const struct scmi_handle *h, u8 msg_id, u8 prot_id, > size_t tx_size, size_t rx_size, struct scmi_xfer **p); > int scmi_handle_put(const struct scmi_handle *handle); > struct scmi_handle *scmi_handle_get(struct device *dev); > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c > index 6fee11f06a66..33d2b78af3ff 100644 > --- a/drivers/firmware/arm_scmi/driver.c > +++ b/drivers/firmware/arm_scmi/driver.c > @@ -295,7 +295,7 @@ static void scmi_tx_prepare(struct mbox_client *cl, void *m) > * > * Return: 0 if all went fine, else corresponding error. > */ > -static struct scmi_xfer *scmi_one_xfer_get(const struct scmi_handle *handle) > +static struct scmi_xfer *scmi_xfer_get(const struct scmi_handle *handle) > { > u16 xfer_id; > struct scmi_xfer *xfer; > @@ -324,14 +324,14 @@ static struct scmi_xfer *scmi_one_xfer_get(const struct scmi_handle *handle) > } > > /** > - * scmi_one_xfer_put() - Release a message > + * scmi_xfer_put() - Release a message > * > * @handle: Pointer to SCMI entity handle > * @xfer: message that was reserved by scmi_xfer_get > * > * This holds a spinlock to maintain integrity of internal data structures. > */ > -void scmi_one_xfer_put(const struct scmi_handle *handle, struct scmi_xfer *xfer) > +void scmi_xfer_put(const struct scmi_handle *handle, struct scmi_xfer *xfer) > { > unsigned long flags; > struct scmi_info *info = handle_to_scmi_info(handle); > @@ -436,7 +436,7 @@ int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer) > } > > /** > - * scmi_one_xfer_init() - Allocate and initialise one message > + * scmi_xfer_get_init() - Allocate and initialise one message > * > * @handle: Pointer to SCMI entity handle > * @msg_id: Message identifier > @@ -445,13 +445,13 @@ int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer) > * @rx_size: receive message size > * @p: pointer to the allocated and initialised message > * > - * This function allocates the message using @scmi_one_xfer_get and > + * This function allocates the message using @scmi_xfer_get and > * initialise the header. > * > * Return: 0 if all went fine with @p pointing to message, else > * corresponding error. > */ > -int scmi_one_xfer_init(const struct scmi_handle *handle, u8 msg_id, u8 prot_id, > +int scmi_xfer_get_init(const struct scmi_handle *handle, u8 msg_id, u8 prot_id, > size_t tx_size, size_t rx_size, struct scmi_xfer **p) > { > int ret; > @@ -464,7 +464,7 @@ int scmi_one_xfer_init(const struct scmi_handle *handle, u8 msg_id, u8 prot_id, > tx_size > info->desc->max_msg_size) > return -ERANGE; > > - xfer = scmi_one_xfer_get(handle); > + xfer = scmi_xfer_get(handle); > if (IS_ERR(xfer)) { > ret = PTR_ERR(xfer); > dev_err(dev, "failed to get free message slot(%d)\n", ret); > @@ -500,7 +500,7 @@ int scmi_version_get(const struct scmi_handle *handle, u8 protocol, > __le32 *rev_info; > struct scmi_xfer *t; > > - ret = scmi_one_xfer_init(handle, PROTOCOL_VERSION, protocol, 0, > + ret = scmi_xfer_get_init(handle, PROTOCOL_VERSION, protocol, 0, > sizeof(*version), &t); > if (ret) > return ret; > @@ -511,7 +511,7 @@ int scmi_version_get(const struct scmi_handle *handle, u8 protocol, > *version = le32_to_cpu(*rev_info); > } > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -539,7 +539,7 @@ scmi_is_protocol_implemented(const struct scmi_handle *handle, u8 prot_id) > } > > /** > - * scmi_handle_get() - Get the SCMI handle for a device > + * scmi_handle_get() - Get the SCMI handle for a device This wants to be in the earlier documentation fix patch not here. > * > * @dev: pointer to device for which we want SCMI handle > * > diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c > index 611ab08e6174..2a219b1261b1 100644 > --- a/drivers/firmware/arm_scmi/perf.c > +++ b/drivers/firmware/arm_scmi/perf.c > @@ -115,7 +115,7 @@ static int scmi_perf_attributes_get(const struct scmi_handle *handle, > struct scmi_xfer *t; > struct scmi_msg_resp_perf_attributes *attr; > > - ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES, > + ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES, > SCMI_PROTOCOL_PERF, 0, sizeof(*attr), &t); > if (ret) > return ret; > @@ -133,7 +133,7 @@ static int scmi_perf_attributes_get(const struct scmi_handle *handle, > pi->stats_size = le32_to_cpu(attr->stats_size); > } > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -145,7 +145,7 @@ scmi_perf_domain_attributes_get(const struct scmi_handle *handle, u32 domain, > struct scmi_xfer *t; > struct scmi_msg_resp_perf_domain_attributes *attr; > > - ret = scmi_one_xfer_init(handle, PERF_DOMAIN_ATTRIBUTES, > + ret = scmi_xfer_get_init(handle, PERF_DOMAIN_ATTRIBUTES, > SCMI_PROTOCOL_PERF, sizeof(domain), > sizeof(*attr), &t); > if (ret) > @@ -171,7 +171,7 @@ scmi_perf_domain_attributes_get(const struct scmi_handle *handle, u32 domain, > memcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE); > } > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -194,7 +194,7 @@ scmi_perf_describe_levels_get(const struct scmi_handle *handle, u32 domain, > struct scmi_msg_perf_describe_levels *dom_info; > struct scmi_msg_resp_perf_describe_levels *level_info; > > - ret = scmi_one_xfer_init(handle, PERF_DESCRIBE_LEVELS, > + ret = scmi_xfer_get_init(handle, PERF_DESCRIBE_LEVELS, > SCMI_PROTOCOL_PERF, sizeof(*dom_info), 0, &t); > if (ret) > return ret; > @@ -237,7 +237,7 @@ scmi_perf_describe_levels_get(const struct scmi_handle *handle, u32 domain, > } while (num_returned && num_remaining); > > perf_dom->opp_count = tot_opp_cnt; > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > > sort(perf_dom->opp, tot_opp_cnt, sizeof(*opp), opp_cmp_func, NULL); > return ret; > @@ -250,7 +250,7 @@ static int scmi_perf_limits_set(const struct scmi_handle *handle, u32 domain, > struct scmi_xfer *t; > struct scmi_perf_set_limits *limits; > > - ret = scmi_one_xfer_init(handle, PERF_LIMITS_SET, SCMI_PROTOCOL_PERF, > + ret = scmi_xfer_get_init(handle, PERF_LIMITS_SET, SCMI_PROTOCOL_PERF, > sizeof(*limits), 0, &t); > if (ret) > return ret; > @@ -262,7 +262,7 @@ static int scmi_perf_limits_set(const struct scmi_handle *handle, u32 domain, > > ret = scmi_do_xfer(handle, t); > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -273,7 +273,7 @@ static int scmi_perf_limits_get(const struct scmi_handle *handle, u32 domain, > struct scmi_xfer *t; > struct scmi_perf_get_limits *limits; > > - ret = scmi_one_xfer_init(handle, PERF_LIMITS_GET, SCMI_PROTOCOL_PERF, > + ret = scmi_xfer_get_init(handle, PERF_LIMITS_GET, SCMI_PROTOCOL_PERF, > sizeof(__le32), 0, &t); > if (ret) > return ret; > @@ -288,7 +288,7 @@ static int scmi_perf_limits_get(const struct scmi_handle *handle, u32 domain, > *min_perf = le32_to_cpu(limits->min_level); > } > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -299,7 +299,7 @@ static int scmi_perf_level_set(const struct scmi_handle *handle, u32 domain, > struct scmi_xfer *t; > struct scmi_perf_set_level *lvl; > > - ret = scmi_one_xfer_init(handle, PERF_LEVEL_SET, SCMI_PROTOCOL_PERF, > + ret = scmi_xfer_get_init(handle, PERF_LEVEL_SET, SCMI_PROTOCOL_PERF, > sizeof(*lvl), 0, &t); > if (ret) > return ret; > @@ -311,7 +311,7 @@ static int scmi_perf_level_set(const struct scmi_handle *handle, u32 domain, > > ret = scmi_do_xfer(handle, t); > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -321,7 +321,7 @@ static int scmi_perf_level_get(const struct scmi_handle *handle, u32 domain, > int ret; > struct scmi_xfer *t; > > - ret = scmi_one_xfer_init(handle, PERF_LEVEL_GET, SCMI_PROTOCOL_PERF, > + ret = scmi_xfer_get_init(handle, PERF_LEVEL_GET, SCMI_PROTOCOL_PERF, > sizeof(u32), sizeof(u32), &t); > if (ret) > return ret; > @@ -333,7 +333,7 @@ static int scmi_perf_level_get(const struct scmi_handle *handle, u32 domain, > if (!ret) > *level = le32_to_cpu(*(__le32 *)t->rx.buf); > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > diff --git a/drivers/firmware/arm_scmi/power.c b/drivers/firmware/arm_scmi/power.c > index 087c2876cdf2..cfa033b05aed 100644 > --- a/drivers/firmware/arm_scmi/power.c > +++ b/drivers/firmware/arm_scmi/power.c > @@ -63,7 +63,7 @@ static int scmi_power_attributes_get(const struct scmi_handle *handle, > struct scmi_xfer *t; > struct scmi_msg_resp_power_attributes *attr; > > - ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES, > + ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES, > SCMI_PROTOCOL_POWER, 0, sizeof(*attr), &t); > if (ret) > return ret; > @@ -78,7 +78,7 @@ static int scmi_power_attributes_get(const struct scmi_handle *handle, > pi->stats_size = le32_to_cpu(attr->stats_size); > } > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -90,7 +90,7 @@ scmi_power_domain_attributes_get(const struct scmi_handle *handle, u32 domain, > struct scmi_xfer *t; > struct scmi_msg_resp_power_domain_attributes *attr; > > - ret = scmi_one_xfer_init(handle, POWER_DOMAIN_ATTRIBUTES, > + ret = scmi_xfer_get_init(handle, POWER_DOMAIN_ATTRIBUTES, > SCMI_PROTOCOL_POWER, sizeof(domain), > sizeof(*attr), &t); > if (ret) > @@ -109,7 +109,7 @@ scmi_power_domain_attributes_get(const struct scmi_handle *handle, u32 domain, > memcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE); > } > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -120,7 +120,7 @@ scmi_power_state_set(const struct scmi_handle *handle, u32 domain, u32 state) > struct scmi_xfer *t; > struct scmi_power_set_state *st; > > - ret = scmi_one_xfer_init(handle, POWER_STATE_SET, SCMI_PROTOCOL_POWER, > + ret = scmi_xfer_get_init(handle, POWER_STATE_SET, SCMI_PROTOCOL_POWER, > sizeof(*st), 0, &t); > if (ret) > return ret; > @@ -132,7 +132,7 @@ scmi_power_state_set(const struct scmi_handle *handle, u32 domain, u32 state) > > ret = scmi_do_xfer(handle, t); > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -142,7 +142,7 @@ scmi_power_state_get(const struct scmi_handle *handle, u32 domain, u32 *state) > int ret; > struct scmi_xfer *t; > > - ret = scmi_one_xfer_init(handle, POWER_STATE_GET, SCMI_PROTOCOL_POWER, > + ret = scmi_xfer_get_init(handle, POWER_STATE_GET, SCMI_PROTOCOL_POWER, > sizeof(u32), sizeof(u32), &t); > if (ret) > return ret; > @@ -153,7 +153,7 @@ scmi_power_state_get(const struct scmi_handle *handle, u32 domain, u32 *state) > if (!ret) > *state = le32_to_cpu(*(__le32 *)t->rx.buf); > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c > index bbb469fea0ed..27f2092b9882 100644 > --- a/drivers/firmware/arm_scmi/sensors.c > +++ b/drivers/firmware/arm_scmi/sensors.c > @@ -79,7 +79,7 @@ static int scmi_sensor_attributes_get(const struct scmi_handle *handle, > struct scmi_xfer *t; > struct scmi_msg_resp_sensor_attributes *attr; > > - ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES, > + ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES, > SCMI_PROTOCOL_SENSOR, 0, sizeof(*attr), &t); > if (ret) > return ret; > @@ -95,7 +95,7 @@ static int scmi_sensor_attributes_get(const struct scmi_handle *handle, > si->reg_size = le32_to_cpu(attr->reg_size); > } > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -108,7 +108,7 @@ static int scmi_sensor_description_get(const struct scmi_handle *handle, > struct scmi_xfer *t; > struct scmi_msg_resp_sensor_description *buf; > > - ret = scmi_one_xfer_init(handle, SENSOR_DESCRIPTION_GET, > + ret = scmi_xfer_get_init(handle, SENSOR_DESCRIPTION_GET, > SCMI_PROTOCOL_SENSOR, sizeof(__le32), 0, &t); > if (ret) > return ret; > @@ -150,7 +150,7 @@ static int scmi_sensor_description_get(const struct scmi_handle *handle, > */ > } while (num_returned && num_remaining); > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -162,7 +162,7 @@ scmi_sensor_configuration_set(const struct scmi_handle *handle, u32 sensor_id) > struct scmi_xfer *t; > struct scmi_msg_set_sensor_config *cfg; > > - ret = scmi_one_xfer_init(handle, SENSOR_CONFIG_SET, > + ret = scmi_xfer_get_init(handle, SENSOR_CONFIG_SET, > SCMI_PROTOCOL_SENSOR, sizeof(*cfg), 0, &t); > if (ret) > return ret; > @@ -173,7 +173,7 @@ scmi_sensor_configuration_set(const struct scmi_handle *handle, u32 sensor_id) > > ret = scmi_do_xfer(handle, t); > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -185,7 +185,7 @@ static int scmi_sensor_trip_point_set(const struct scmi_handle *handle, > struct scmi_xfer *t; > struct scmi_msg_set_sensor_trip_point *trip; > > - ret = scmi_one_xfer_init(handle, SENSOR_TRIP_POINT_SET, > + ret = scmi_xfer_get_init(handle, SENSOR_TRIP_POINT_SET, > SCMI_PROTOCOL_SENSOR, sizeof(*trip), 0, &t); > if (ret) > return ret; > @@ -198,7 +198,7 @@ static int scmi_sensor_trip_point_set(const struct scmi_handle *handle, > > ret = scmi_do_xfer(handle, t); > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > > @@ -209,7 +209,7 @@ static int scmi_sensor_reading_get(const struct scmi_handle *handle, > struct scmi_xfer *t; > struct scmi_msg_sensor_reading_get *sensor; > > - ret = scmi_one_xfer_init(handle, SENSOR_READING_GET, > + ret = scmi_xfer_get_init(handle, SENSOR_READING_GET, > SCMI_PROTOCOL_SENSOR, sizeof(*sensor), > sizeof(u64), &t); > if (ret) > @@ -227,7 +227,7 @@ static int scmi_sensor_reading_get(const struct scmi_handle *handle, > *value |= (u64)le32_to_cpu(*(pval + 1)) << 32; > } > > - scmi_one_xfer_put(handle, t); > + scmi_xfer_put(handle, t); > return ret; > } > ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 5/8] firmware: arm_scmi: drop unused `con_priv` structure member 2018-05-09 17:07 [PATCH 0/8] firmware: arm_scmi: trivial cleanups Sudeep Holla ` (3 preceding siblings ...) 2018-05-09 17:07 ` [PATCH 4/8] firmware: arm_scmi: rename scmi_xfer_{init,get,put} Sudeep Holla @ 2018-05-09 17:07 ` Sudeep Holla 2018-05-17 8:41 ` Jonathan Cameron 2018-05-09 17:07 ` [PATCH 6/8] firmware: arm_scmi: remove unnecessary bitmap_zero Sudeep Holla ` (2 subsequent siblings) 7 siblings, 1 reply; 19+ messages in thread From: Sudeep Holla @ 2018-05-09 17:07 UTC (permalink / raw) To: linux-arm-kernel Initially con_priv was supposedly used for transport specific data when the SCMI driver had an abstraction to communicate with different mailbox controllers. But after some discussions, the idea was dropped but this variable slipped through the cracks. This patch gets rid of this unused variable. Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/firmware/arm_scmi/common.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 41b03e46cca8..937a930ce87d 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -86,9 +86,7 @@ struct scmi_msg { * buffer for the rx path as we use for the tx path. * @done: completion event */ - struct scmi_xfer { - void *con_priv; struct scmi_msg_hdr hdr; struct scmi_msg tx; struct scmi_msg rx; -- 2.7.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 5/8] firmware: arm_scmi: drop unused `con_priv` structure member 2018-05-09 17:07 ` [PATCH 5/8] firmware: arm_scmi: drop unused `con_priv` structure member Sudeep Holla @ 2018-05-17 8:41 ` Jonathan Cameron 0 siblings, 0 replies; 19+ messages in thread From: Jonathan Cameron @ 2018-05-17 8:41 UTC (permalink / raw) To: linux-arm-kernel On Wed, 9 May 2018 18:07:11 +0100 Sudeep Holla <sudeep.holla@arm.com> wrote: > Initially con_priv was supposedly used for transport specific data when > the SCMI driver had an abstraction to communicate with different mailbox > controllers. But after some discussions, the idea was dropped but this > variable slipped through the cracks. > > This patch gets rid of this unused variable. > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> Curious that it never had any docs :) I'm glad we didn't add them two patches earlier to remove them here as that would have been silly. Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/firmware/arm_scmi/common.h | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h > index 41b03e46cca8..937a930ce87d 100644 > --- a/drivers/firmware/arm_scmi/common.h > +++ b/drivers/firmware/arm_scmi/common.h > @@ -86,9 +86,7 @@ struct scmi_msg { > * buffer for the rx path as we use for the tx path. > * @done: completion event > */ > - > struct scmi_xfer { > - void *con_priv; > struct scmi_msg_hdr hdr; > struct scmi_msg tx; > struct scmi_msg rx; ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 6/8] firmware: arm_scmi: remove unnecessary bitmap_zero 2018-05-09 17:07 [PATCH 0/8] firmware: arm_scmi: trivial cleanups Sudeep Holla ` (4 preceding siblings ...) 2018-05-09 17:07 ` [PATCH 5/8] firmware: arm_scmi: drop unused `con_priv` structure member Sudeep Holla @ 2018-05-09 17:07 ` Sudeep Holla 2018-05-17 8:43 ` Jonathan Cameron 2018-05-09 17:07 ` [PATCH 7/8] firmware: arm_scmi: improve exit paths and code readability Sudeep Holla 2018-05-09 17:07 ` [PATCH 8/8] firmware: arm_scmi: simplify exit path by returning on error Sudeep Holla 7 siblings, 1 reply; 19+ messages in thread From: Sudeep Holla @ 2018-05-09 17:07 UTC (permalink / raw) To: linux-arm-kernel kcalloc zeros the memory and it's totally unnecessary to zero the bitmap again using bitmap_zero. This patch just drops the unnecessary use of the bitmap_zero in the context. Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/firmware/arm_scmi/driver.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 33d2b78af3ff..4087d6c50ecd 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -636,8 +636,6 @@ static int scmi_xfer_info_init(struct scmi_info *sinfo) if (!info->xfer_alloc_table) return -ENOMEM; - bitmap_zero(info->xfer_alloc_table, desc->max_msg); - /* Pre-initialize the buffer pointer to pre-allocated buffers */ for (i = 0, xfer = info->xfer_block; i < desc->max_msg; i++, xfer++) { xfer->rx.buf = devm_kcalloc(dev, sizeof(u8), desc->max_msg_size, -- 2.7.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 6/8] firmware: arm_scmi: remove unnecessary bitmap_zero 2018-05-09 17:07 ` [PATCH 6/8] firmware: arm_scmi: remove unnecessary bitmap_zero Sudeep Holla @ 2018-05-17 8:43 ` Jonathan Cameron 0 siblings, 0 replies; 19+ messages in thread From: Jonathan Cameron @ 2018-05-17 8:43 UTC (permalink / raw) To: linux-arm-kernel On Wed, 9 May 2018 18:07:12 +0100 Sudeep Holla <sudeep.holla@arm.com> wrote: > kcalloc zeros the memory and it's totally unnecessary to zero the bitmap > again using bitmap_zero. This patch just drops the unnecessary use of > the bitmap_zero in the context. > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/firmware/arm_scmi/driver.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c > index 33d2b78af3ff..4087d6c50ecd 100644 > --- a/drivers/firmware/arm_scmi/driver.c > +++ b/drivers/firmware/arm_scmi/driver.c > @@ -636,8 +636,6 @@ static int scmi_xfer_info_init(struct scmi_info *sinfo) > if (!info->xfer_alloc_table) > return -ENOMEM; > > - bitmap_zero(info->xfer_alloc_table, desc->max_msg); > - > /* Pre-initialize the buffer pointer to pre-allocated buffers */ > for (i = 0, xfer = info->xfer_block; i < desc->max_msg; i++, xfer++) { > xfer->rx.buf = devm_kcalloc(dev, sizeof(u8), desc->max_msg_size, ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 7/8] firmware: arm_scmi: improve exit paths and code readability 2018-05-09 17:07 [PATCH 0/8] firmware: arm_scmi: trivial cleanups Sudeep Holla ` (5 preceding siblings ...) 2018-05-09 17:07 ` [PATCH 6/8] firmware: arm_scmi: remove unnecessary bitmap_zero Sudeep Holla @ 2018-05-09 17:07 ` Sudeep Holla 2018-05-17 9:13 ` Jonathan Cameron 2018-05-09 17:07 ` [PATCH 8/8] firmware: arm_scmi: simplify exit path by returning on error Sudeep Holla 7 siblings, 1 reply; 19+ messages in thread From: Sudeep Holla @ 2018-05-09 17:07 UTC (permalink / raw) To: linux-arm-kernel The existing code intends the good path to reduce the code which is so uncommon. It's obvious to have more readable code with a goto used for the error path. This patch adds more appropriate error paths and makes code more readable. It also moves a error logging outside the scope of locking. Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/firmware/arm_scmi/bus.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c index f2760a596c28..472c88ae1c0f 100644 --- a/drivers/firmware/arm_scmi/bus.c +++ b/drivers/firmware/arm_scmi/bus.c @@ -125,13 +125,13 @@ scmi_device_create(struct device_node *np, struct device *parent, int protocol) int id, retval; struct scmi_device *scmi_dev; - id = ida_simple_get(&scmi_bus_id, 1, 0, GFP_KERNEL); - if (id < 0) - return NULL; - scmi_dev = kzalloc(sizeof(*scmi_dev), GFP_KERNEL); if (!scmi_dev) - goto no_mem; + return NULL; + + id = ida_simple_get(&scmi_bus_id, 1, 0, GFP_KERNEL); + if (id < 0) + goto free_mem; scmi_dev->id = id; scmi_dev->protocol_id = protocol; @@ -141,13 +141,15 @@ scmi_device_create(struct device_node *np, struct device *parent, int protocol) dev_set_name(&scmi_dev->dev, "scmi_dev.%d", id); retval = device_register(&scmi_dev->dev); - if (!retval) - return scmi_dev; + if (retval) + goto put_dev; + return scmi_dev; +put_dev: put_device(&scmi_dev->dev); - kfree(scmi_dev); -no_mem: ida_simple_remove(&scmi_bus_id, id); +free_mem: + kfree(scmi_dev); return NULL; } @@ -171,9 +173,9 @@ int scmi_protocol_register(int protocol_id, scmi_prot_init_fn_t fn) spin_lock(&protocol_lock); ret = idr_alloc(&scmi_protocols, fn, protocol_id, protocol_id + 1, GFP_ATOMIC); + spin_unlock(&protocol_lock); if (ret != protocol_id) pr_err("unable to allocate SCMI idr slot, err %d\n", ret); - spin_unlock(&protocol_lock); return ret; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 7/8] firmware: arm_scmi: improve exit paths and code readability 2018-05-09 17:07 ` [PATCH 7/8] firmware: arm_scmi: improve exit paths and code readability Sudeep Holla @ 2018-05-17 9:13 ` Jonathan Cameron 0 siblings, 0 replies; 19+ messages in thread From: Jonathan Cameron @ 2018-05-17 9:13 UTC (permalink / raw) To: linux-arm-kernel On Wed, 9 May 2018 18:07:13 +0100 Sudeep Holla <sudeep.holla@arm.com> wrote: > The existing code intends the good path to reduce the code which is so > uncommon. It's obvious to have more readable code with a goto used for > the error path. This patch adds more appropriate error paths and makes > code more readable. It also moves a error logging outside the scope of > locking. > > Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> It could be argued the lock move is unrelated and should be in a separate patch... Not particularly important though. Some comments inline, but I'm happy with how you did this. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/firmware/arm_scmi/bus.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c > index f2760a596c28..472c88ae1c0f 100644 > --- a/drivers/firmware/arm_scmi/bus.c > +++ b/drivers/firmware/arm_scmi/bus.c > @@ -125,13 +125,13 @@ scmi_device_create(struct device_node *np, struct device *parent, int protocol) > int id, retval; > struct scmi_device *scmi_dev; > > - id = ida_simple_get(&scmi_bus_id, 1, 0, GFP_KERNEL); > - if (id < 0) > - return NULL; > - > scmi_dev = kzalloc(sizeof(*scmi_dev), GFP_KERNEL); > if (!scmi_dev) > - goto no_mem; > + return NULL; > + > + id = ida_simple_get(&scmi_bus_id, 1, 0, GFP_KERNEL); > + if (id < 0) > + goto free_mem; I guess this reorder was to match what is done in remove? Personally I would have reordered remove as that was a one line change, but it really doesn't matter. > > scmi_dev->id = id; > scmi_dev->protocol_id = protocol; > @@ -141,13 +141,15 @@ scmi_device_create(struct device_node *np, struct device *parent, int protocol) > dev_set_name(&scmi_dev->dev, "scmi_dev.%d", id); > > retval = device_register(&scmi_dev->dev); > - if (!retval) > - return scmi_dev; > + if (retval) > + goto put_dev; > > + return scmi_dev; If you are respinning I'd add a blank line here for readability. > +put_dev: > put_device(&scmi_dev->dev); > - kfree(scmi_dev); > -no_mem: > ida_simple_remove(&scmi_bus_id, id); > +free_mem: > + kfree(scmi_dev); > return NULL; > } > > @@ -171,9 +173,9 @@ int scmi_protocol_register(int protocol_id, scmi_prot_init_fn_t fn) > spin_lock(&protocol_lock); > ret = idr_alloc(&scmi_protocols, fn, protocol_id, protocol_id + 1, > GFP_ATOMIC); > + spin_unlock(&protocol_lock); > if (ret != protocol_id) > pr_err("unable to allocate SCMI idr slot, err %d\n", ret); > - spin_unlock(&protocol_lock); > > return ret; > } ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 8/8] firmware: arm_scmi: simplify exit path by returning on error 2018-05-09 17:07 [PATCH 0/8] firmware: arm_scmi: trivial cleanups Sudeep Holla ` (6 preceding siblings ...) 2018-05-09 17:07 ` [PATCH 7/8] firmware: arm_scmi: improve exit paths and code readability Sudeep Holla @ 2018-05-09 17:07 ` Sudeep Holla 2018-05-17 9:14 ` Jonathan Cameron 7 siblings, 1 reply; 19+ messages in thread From: Sudeep Holla @ 2018-05-09 17:07 UTC (permalink / raw) To: linux-arm-kernel Yet another nasty indentation left out during code restructuring. It's must simpler to return on error instead of having unnecessary indentation. Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/firmware/arm_scmi/driver.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 4087d6c50ecd..e996395af5f2 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -687,11 +687,12 @@ static int scmi_remove(struct platform_device *pdev) list_del(&info->node); mutex_unlock(&scmi_list_mutex); - if (!ret) { - /* Safe to free channels since no more users */ - ret = idr_for_each(idr, scmi_mbox_free_channel, idr); - idr_destroy(&info->tx_idr); - } + if (ret) + return ret; + + /* Safe to free channels since no more users */ + ret = idr_for_each(idr, scmi_mbox_free_channel, idr); + idr_destroy(&info->tx_idr); return ret; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 8/8] firmware: arm_scmi: simplify exit path by returning on error 2018-05-09 17:07 ` [PATCH 8/8] firmware: arm_scmi: simplify exit path by returning on error Sudeep Holla @ 2018-05-17 9:14 ` Jonathan Cameron 0 siblings, 0 replies; 19+ messages in thread From: Jonathan Cameron @ 2018-05-17 9:14 UTC (permalink / raw) To: linux-arm-kernel On Wed, 9 May 2018 18:07:14 +0100 Sudeep Holla <sudeep.holla@arm.com> wrote: > Yet another nasty indentation left out during code restructuring. It's > must simpler to return on error instead of having unnecessary indentation. > > Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/firmware/arm_scmi/driver.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c > index 4087d6c50ecd..e996395af5f2 100644 > --- a/drivers/firmware/arm_scmi/driver.c > +++ b/drivers/firmware/arm_scmi/driver.c > @@ -687,11 +687,12 @@ static int scmi_remove(struct platform_device *pdev) > list_del(&info->node); > mutex_unlock(&scmi_list_mutex); > > - if (!ret) { > - /* Safe to free channels since no more users */ > - ret = idr_for_each(idr, scmi_mbox_free_channel, idr); > - idr_destroy(&info->tx_idr); > - } > + if (ret) > + return ret; > + > + /* Safe to free channels since no more users */ > + ret = idr_for_each(idr, scmi_mbox_free_channel, idr); > + idr_destroy(&info->tx_idr); > > return ret; > } ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2018-05-17 9:14 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-09 17:07 [PATCH 0/8] firmware: arm_scmi: trivial cleanups Sudeep Holla
2018-05-09 17:07 ` [PATCH 1/8] firmware: arm_scmi: improve code readability using bitfield accessor macros Sudeep Holla
2018-05-17 8:14 ` Jonathan Cameron
2018-05-09 17:07 ` [PATCH 2/8] firmware: arm_scmi: fix kernel-docs documentation Sudeep Holla
2018-05-17 8:30 ` Jonathan Cameron
2018-05-09 17:07 ` [PATCH 3/8] firmware: arm_scmi: rename get_transition_latency and add_opps_to_device Sudeep Holla
2018-05-10 8:29 ` Rafael J. Wysocki
2018-05-10 9:53 ` Sudeep Holla
2018-05-17 8:32 ` Jonathan Cameron
2018-05-09 17:07 ` [PATCH 4/8] firmware: arm_scmi: rename scmi_xfer_{init,get,put} Sudeep Holla
2018-05-17 8:38 ` Jonathan Cameron
2018-05-09 17:07 ` [PATCH 5/8] firmware: arm_scmi: drop unused `con_priv` structure member Sudeep Holla
2018-05-17 8:41 ` Jonathan Cameron
2018-05-09 17:07 ` [PATCH 6/8] firmware: arm_scmi: remove unnecessary bitmap_zero Sudeep Holla
2018-05-17 8:43 ` Jonathan Cameron
2018-05-09 17:07 ` [PATCH 7/8] firmware: arm_scmi: improve exit paths and code readability Sudeep Holla
2018-05-17 9:13 ` Jonathan Cameron
2018-05-09 17:07 ` [PATCH 8/8] firmware: arm_scmi: simplify exit path by returning on error Sudeep Holla
2018-05-17 9:14 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).