Messages in this thread |  | | Subject | Re: [PATCH v9 04/10] drivers: qcom: rpmh: add RPMH helper functions | From | Raju P L S S S N <> | Date | Mon, 11 Jun 2018 22:41:18 +0530 |
| |
Hi,
On 5/31/2018 3:19 AM, Doug Anderson wrote: > 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>.
Agree.
> > 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.
Thanks for the explanation Doug. Will address this.
Thanks, Raju
|  |