Re: [PATCH v9 04/10] drivers: qcom: rpmh: add RPMH helper functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux