On 12/5/2018 8:00 AM, Georgi Djakov wrote:

Hi Evan,

On 12/1/18 02:39, Evan Green wrote:

quoted

On Tue, Nov 27, 2018 at 10:04 AM Georgi Djakov [off-list ref] wrote:

quoted

From: David Dai <redacted>

Introduce Qualcomm SDM845 specific provider driver using the
interconnect framework.

Signed-off-by: David Dai <redacted>
Signed-off-by: Georgi Djakov <redacted>
---
  .../bindings/interconnect/qcom,sdm845.txt     |  24 +
  drivers/interconnect/Kconfig                  |   5 +
  drivers/interconnect/Makefile                 |   1 +
  drivers/interconnect/qcom/Kconfig             |  13 +
  drivers/interconnect/qcom/Makefile            |   5 +
  drivers/interconnect/qcom/sdm845.c            | 836 ++++++++++++++++++
  .../dt-bindings/interconnect/qcom,sdm845.h    | 143 +++
  7 files changed, 1027 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,sdm845.txt
  create mode 100644 drivers/interconnect/qcom/Kconfig
  create mode 100644 drivers/interconnect/qcom/Makefile
  create mode 100644 drivers/interconnect/qcom/sdm845.c
  create mode 100644 include/dt-bindings/interconnect/qcom,sdm845.h

diff --git a/Documentation/devicetree/bindings/interconnect/qcom,sdm845.txt b/Documentation/devicetree/bindings/interconnect/qcom,sdm845.txt
new file mode 100644
index 000000000000..d45150e99665
--- /dev/null
+++ b/Documentation/devicetree/bindings/interconnect/qcom,sdm845.txt

@@ -0,0 +1,24 @@
+Qualcomm SDM845 Network-On-Chip interconnect driver binding
+-----------------------------------------------------------
+
+SDM845 interconnect providers support system bandwidth requirements through
+RPMh hardware accelerators known as Bus Clock Manager(BCM). The provider is able
+to communicate with the BCM through the Resource State Coordinator(RSC)
+associated with each execution environment. Provider nodes must reside within
+an RPMh device node pertaining to their RSC and each provider maps to
+a single RPMh resource.
+
+Required properties :
+- compatible : shall contain only one of the following:
+                       "qcom,sdm845-rsc-hlos"

I wonder if maybe hlos isn't necessary. Unless you somehow imagine
secure mode would have a device tree entry in here as well? Probably
not.

Ok, will remove it. David, please chime in if you have any concerns with
this.

No strong preferences in terms of naming, but need to make the 
distinction between this and potential other rsc types if we're to add 
additional provider drivers.

quoted

quoted

+- #interconnect-cells : should contain 1
+
+Examples:
+
+apps_rsc: rsc {
+               qnoc: qnoc-rsc-hlos {
+                       compatible = "qcom,sdm845-rsc-hlos";
+                       #interconnect-cells = <1>;
+               };
+};
+

...

quoted

diff --git a/drivers/interconnect/qcom/sdm845.c b/drivers/interconnect/qcom/sdm845.c
new file mode 100644
index 000000000000..1678de91ca52
--- /dev/null
+++ b/drivers/interconnect/qcom/sdm845.c

@@ -0,0 +1,836 @@

...

quoted

+
+static void tcs_list_gen(struct list_head *bcm_list,
+                        struct tcs_cmd *tcs_list, int *n)

We could make the prototype of this function be:

static void tcs_list_gen(struct list_head *bcm_list,
         struct tcs_cmd tcs_list[SDM845_MAX_VCD], int n[SDM845_MAX_VCD])

which would catch errors if somebody later passed in an array that
wasn't the right size, since we blindly memset below.

Yes, sounds good. I will try to optimize it.

Thanks,
Georgi

quoted

quoted

+{
+       struct qcom_icc_bcm *bcm;
+       bool commit;
+       size_t idx = 0, batch = 0, cur_vcd_size = 0;
+
+       memset(n, 0, sizeof(int) * SDM845_MAX_VCD);
+
+       list_for_each_entry(bcm, bcm_list, list) {
+               commit = false;
+               cur_vcd_size++;
+               if ((list_is_last(&bcm->list, bcm_list)) ||
+                   bcm->aux_data.vcd != list_next_entry(bcm, list)->aux_data.vcd) {
+                       commit = true;
+                       cur_vcd_size = 0;
+               }
+               tcs_cmd_gen(&tcs_list[idx], bcm->vote_x, bcm->vote_y,
+                           bcm->addr, commit);
+               idx++;
+               n[batch]++;
+               /*
+                * Batch the BCMs in such a way that we do not split them in
+                * multiple payloads when they are under the same VCD. This is
+                * to ensure that every BCM is committed since we only set the
+                * commit bit on the last BCM request of every VCD.
+                */
+               if (n[batch] >= MAX_RPMH_PAYLOAD) {
+                       if (!commit) {
+                               n[batch] -= cur_vcd_size;
+                               n[batch + 1] = cur_vcd_size;
+                       }
+                       batch++;
+               }
+       }
+}
+

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project