Messages in this thread | | | From | Doug Anderson <> | Date | Wed, 30 May 2018 14:49:31 -0700 | Subject | Re: [PATCH v9 04/10] drivers: qcom: rpmh: add RPMH helper functions |
| |
Hi,
On Thu, May 24, 2018 at 3:45 AM, Raju P L S S S N <rplsssn@codeaurora.org> wrote: > --- a/drivers/soc/qcom/rpmh-rsc.c > +++ b/drivers/soc/qcom/rpmh-rsc.c > @@ -61,6 +61,8 @@ > #define CMD_STATUS_ISSUED BIT(8) > #define CMD_STATUS_COMPL BIT(16) > > +LIST_HEAD(rsc_drv_list);
I still see no point of rsc_drv_list. Please remove it, AKA squash in <http://crosreview.com/1042883>.
I'm also still of the opinion that we should take something like <http://crosreview.com/1054646>, AKA "Get rid of the global array rpmh_rsc".
> +/** > + * __rpmh_write: send the RPMH request > + * > + * @dev: The device making the request > + * @state: Active/Sleep request type > + * @rpm_msg: The data that needs to be sent (cmds). > + */ > +static int __rpmh_write(const struct device *dev, enum rpmh_state state, > + struct rpmh_request *rpm_msg) > +{ > + struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev); > + > + if (IS_ERR(ctrlr)) > + return PTR_ERR(ctrlr); > + > + rpm_msg->msg.state = state; > + > + if (state != RPMH_ACTIVE_ONLY_STATE) > + return -EINVAL; > + > + WARN_ON(irqs_disabled()); > + > + return rpmh_rsc_send_data(ctrlr->drv, &rpm_msg->msg); > +}
You went too far in the removal of EXPORT_SYMBOL I think. This symbol needs to be exported because other code that could be compiled as a module might need to call into it. To explain:
* If two files that are always built-in to Linux need to call into each other: no need for EXPORT_SYMBOL.
* If two files that are always part of the same module need to call into each other: no need for EXPORT_SYMBOL.
* If one file that might be built-into a module needs to call another that's builtin to the kernel: need EXPORT_SYMBOL.
-Doug
| |