drivers/qcom: add RPMH communication support

Add controller driver for QCOM SoCs that have hardware based shared resource management. The hardware IP known as RSC (Resource State Coordinator) houses multiple Direct Resource Voter (DRV) for different execution levels. A DRV is a unique voter on the state of a shared resource. A Trigger Control Set (TCS) is a bunch of slots that can house multiple resource state requests, that when triggered will issue those requests through an internal bus to the Resource Power Manager Hardened (RPMH) blocks. These hardware blocks are capable of adjusting clocks, voltages etc. The resource state request from a DRV are aggregated along

with state requests from other processors in the SoC and the aggregate value is applied on the resource.

Some important aspects of the RPMH communication -
- Requests are <addr, value> with some header information
- Multiple requests (upto 16) may be sent through a TCS, at a time
- Requests in a TCS are sent in sequence
- Requests may be fire-n-forget or completion (response expected)
- Multiple TCS from the same DRV may be triggered simultaneously
- Cannot send a request if another reques for the same addr is in

progress from the same DRV
- When all the requests from a TCS are complete, an IRQ is raised
- The IRQ handler needs to clear the TCS before it is available for reuse
- TCS configuration is specific to a DRV
- Platform drivers may use DRV from different RSCs to make requests

Resource state requests made when CPUs are active are called 'active' state requests. Requests made when all the CPUs are powered down (idle state) are called 'sleep' state requests. They are matched by a corresponding 'wake' state requests which puts the resources back in to previously requested active state before resuming any CPU. TCSes are dedicated for each type of requests. Control TCS are used to provide specific information to the controller.

Signed-off-by Lina Iyer <[email protected]>

---
 drivers/soc/qcom/Kconfig                |  10 +
 drivers/soc/qcom/Makefile               |   1 +
 drivers/soc/qcom/rpmh-internal.h        |  87 +++++
 drivers/soc/qcom/rpmh-rsc.c             | 593 ++++++++++++++++++++++++++++++++
 include/dt-bindings/soc/qcom,rpmh-rsc.h |  14 +
 include/soc/qcom/tcs.h                  |  56 +++
 6 files changed, 761 insertions(+)
 create mode 100644 drivers/soc/qcom/rpmh-internal.h
 create mode 100644 drivers/soc/qcom/rpmh-rsc.c
 create mode 100644 include/dt-bindings/soc/qcom,rpmh-rsc.h
 create mode 100644 include/soc/qcom/tcs.h

drivers/soc/qcom/Kconfig +0 -0

index e050eb83341d..779f0d14748d 100644

drivers/soc/qcom/Kconfig +7 -0

+       help
+         Support for communication with the hardened-RPM blocks in
+         Qualcomm Technologies Inc (QTI) SoCs. RPMH communication uses an
+         internal bus to transmit state requests for shared resources. A set
+         of hardware components aggregate requests for these resources and
+         help apply the aggregated state on the resource.
+
 config QCOM_SMEM
        tristate "Qualcomm Shared Memory Manager (SMEM)"
        depends on ARCH_QCOM

drivers/soc/qcom/Makefile +0 -0

index dcebf2814e6d..e7d04f0e3616 100644

drivers/soc/qcom/rpmh-internal.h new file +10 -0

new file mode 100644
index 000000000000..12faec77c4f3
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ */
+
+
+#ifndef __RPM_INTERNAL_H__
+#define __RPM_INTERNAL_H__
+
+#include <soc/qcom/tcs.h>

drivers/soc/qcom/rpmh-internal.h new file +16 -0

+ */
+struct tcs_response {
+       struct rsc_drv *drv;
+       struct tcs_request *msg;
+       u32 m;
+       int err;
+       struct list_head list;
+};
+
+/**
+ * tcs_group: group of Trigger Command Sets for a request state
+ *
+ * @drv: the controller
+ * @type: type of the TCS in this group - active, sleep, wake
+ * @tcs_mask: mask of the TCSes relative to all the TCSes in the RSC
+ * @tcs_offset: start of the TCS group relative to the TCSes in the RSC

drivers/soc/qcom/rpmh-internal.h new file +6 -0

+ * @num_tcs: number of TCSes in this DRV
+ * @tasklet: handle responses, off-load work from IRQ handler
+ * @response_pending: list of responses that needs to be sent to caller
+ * @tcs: TCS groups
+ * @tcs_in_use: s/w state of the TCS
+ * @drv_lock: synchronize state of the controller

drivers/soc/qcom/rpmh-internal.h new file +2 -0

+
+#endif /* __RPM_INTERNAL_H__ */

drivers/soc/qcom/rpmh-rsc.c new file +3 -0

new file mode 100644
index 000000000000..6cb91a0114ee
@@ -0,0 +1,593 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.

drivers/soc/qcom/rpmh-rsc.c new file +38 -0

+#include <soc/qcom/tcs.h>
+#include <dt-bindings/soc/qcom,rpmh-rsc.h>
+
+#include "rpmh-internal.h"
+
+#define RSC_DRV_TCS_OFFSET             672
+#define RSC_DRV_CMD_OFFSET             20
+
+/* DRV Configuration Information Register */
+#define DRV_PRNT_CHLD_CONFIG           0x0C
+#define DRV_NUM_TCS_MASK               0x3F
+#define DRV_NUM_TCS_SHIFT              6
+#define DRV_NCPT_MASK                  0x1F
+#define DRV_NCPT_SHIFT                 27
+
+/* Register offsets */
+#define RSC_DRV_IRQ_ENABLE             0x00
+#define RSC_DRV_IRQ_STATUS             0x04
+#define RSC_DRV_IRQ_CLEAR              0x08
+#define RSC_DRV_CMD_WAIT_FOR_CMPL      0x10
+#define RSC_DRV_CONTROL                        0x14
+#define RSC_DRV_STATUS                 0x18
+#define RSC_DRV_CMD_ENABLE             0x1C
+#define RSC_DRV_CMD_MSGID              0x30
+#define RSC_DRV_CMD_ADDR               0x34
+#define RSC_DRV_CMD_DATA               0x38
+#define RSC_DRV_CMD_STATUS             0x3C
+#define RSC_DRV_CMD_RESP_DATA          0x40
+
+#define TCS_AMC_MODE_ENABLE            BIT(16)
+#define TCS_AMC_MODE_TRIGGER           BIT(24)
+
+/* TCS CMD register bit mask */
+#define CMD_MSGID_LEN                  8
+#define CMD_MSGID_RESP_REQ             BIT(8)
+#define CMD_MSGID_WRITE                        BIT(16)
+#define CMD_STATUS_ISSUED              BIT(8)
+#define CMD_STATUS_COMPL               BIT(16)

drivers/soc/qcom/rpmh-rsc.c new file +5 -0

+       if (!resp)
+               return ERR_PTR(-ENOMEM);
+
+       resp->drv = drv;
+       resp->msg = msg;

drivers/soc/qcom/rpmh-rsc.c new file +28 -0

+       tcs->responses[m - tcs->tcs_offset] = resp;
+
+       return resp;
+}
+
+static void free_response(struct tcs_response *resp)
+{
+       kfree(resp);
+}
+
+static struct tcs_response *get_response(struct rsc_drv *drv, u32 m)
+{
+       struct tcs_group *tcs = get_tcs_from_index(drv, m);
+
+       return tcs->responses[m - tcs->tcs_offset];
+}
+
+static u32 read_tcs_reg(struct rsc_drv *drv, int reg, int m, int n)
+{
+       return readl_relaxed(drv->tcs_base + reg + RSC_DRV_TCS_OFFSET * m +
+                           RSC_DRV_CMD_OFFSET * n);
+}
+
+static void write_tcs_reg(struct rsc_drv *drv, int reg, int m, int n,
+                               u32 data)
+{
+       writel_relaxed(data, drv->tcs_base + reg + RSC_DRV_TCS_OFFSET * m +
+                     RSC_DRV_CMD_OFFSET * n);

drivers/soc/qcom/rpmh-rsc.c new file +42 -0

+       }
+}
+
+static bool tcs_is_free(struct rsc_drv *drv, int m)
+{
+       return !atomic_read(&drv->tcs_in_use[m]) &&
+              read_tcs_reg(drv, RSC_DRV_STATUS, m, 0);
+}
+
+static struct tcs_group *get_tcs_of_type(struct rsc_drv *drv, int type)
+{
+       int i;
+       struct tcs_group *tcs;
+
+       for (i = 0; i < TCS_TYPE_NR; i++) {
+               if (type == drv->tcs[i].type)
+                       break;
+       }
+
+       if (i == TCS_TYPE_NR)
+               return ERR_PTR(-EINVAL);
+
+       tcs = &drv->tcs[i];
+       if (!tcs->num_tcs)
+               return ERR_PTR(-EINVAL);
+
+       return tcs;
+}
+
+static struct tcs_group *get_tcs_for_msg(struct rsc_drv *drv,
+                                       struct tcs_request *msg)
+{
+       int type;
+
+       switch (msg->state) {
+       case RPMH_ACTIVE_ONLY_STATE:
+               type = ACTIVE_TCS;
+               break;
+       default:
+               return ERR_PTR(-EINVAL);
+       }
+

drivers/soc/qcom/rpmh-rsc.c new file +19 -0

+
+       spin_lock_irqsave(&drv->drv_lock, flags);
+       INIT_LIST_HEAD(&resp->list);
+       list_add_tail(&resp->list, &drv->response_pending);
+       spin_unlock_irqrestore(&drv->drv_lock, flags);
+
+       tasklet_schedule(&drv->tasklet);
+}
+
+/**
+ * tcs_irq_handler: TX Done interrupt handler
+ */
+static irqreturn_t tcs_irq_handler(int irq, void *p)
+{
+       struct rsc_drv *drv = p;
+       int m, i;
+       u32 irq_status, sts;
+       struct tcs_response *resp;
+       struct tcs_cmd *cmd;

drivers/soc/qcom/rpmh-rsc.c new file +23 -0

+skip_resp:
+               /* Reclaim the TCS */
+               write_tcs_reg(drv, RSC_DRV_CMD_ENABLE, m, 0, 0);
+               write_tcs_reg(drv, RSC_DRV_IRQ_CLEAR, 0, 0, BIT(m));
+               atomic_set(&drv->tcs_in_use[m], 0);
+               send_tcs_response(resp);
+       }
+
+       return IRQ_HANDLED;
+}
+
+/**
+ * tcs_notify_tx_done: TX Done for requests that got a response
+ *
+ * @data: the tasklet argument
+ *
+ * Tasklet function to notify MBOX that we are done with the request.
+ * Handles all pending reponses whenever run.
+ */
+static void tcs_notify_tx_done(unsigned long data)
+{
+       struct rsc_drv *drv = (struct rsc_drv *)data;
+       struct tcs_response *resp;

drivers/soc/qcom/rpmh-rsc.c new file +44 -0

+               write_tcs_reg(drv, RSC_DRV_CMD_MSGID, m, j, msgid);
+               write_tcs_reg(drv, RSC_DRV_CMD_ADDR, m, j, cmd->addr);
+               write_tcs_reg(drv, RSC_DRV_CMD_DATA, m, j, cmd->data);
+       }
+
+       write_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, m, 0, cmd_complete);
+       cmd_enable |= read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, m, 0);
+       write_tcs_reg(drv, RSC_DRV_CMD_ENABLE, m, 0, cmd_enable);
+}
+
+static void __tcs_trigger(struct rsc_drv *drv, int m)
+{
+       u32 enable;
+
+       /*
+        * HW req: Clear the DRV_CONTROL and enable TCS again
+        * While clearing ensure that the AMC mode trigger is cleared
+        * and then the mode enable is cleared.
+        */
+       enable = read_tcs_reg(drv, RSC_DRV_CONTROL, m, 0);
+       enable &= ~TCS_AMC_MODE_TRIGGER;
+       write_tcs_reg_sync(drv, RSC_DRV_CONTROL, m, 0, enable);
+       enable &= ~TCS_AMC_MODE_ENABLE;
+       write_tcs_reg_sync(drv, RSC_DRV_CONTROL, m, 0, enable);
+
+       /* Enable the AMC mode on the TCS and then trigger the TCS */
+       enable = TCS_AMC_MODE_ENABLE;
+       write_tcs_reg_sync(drv, RSC_DRV_CONTROL, m, 0, enable);
+       enable |= TCS_AMC_MODE_TRIGGER;
+       write_tcs_reg_sync(drv, RSC_DRV_CONTROL, m, 0, enable);
+}
+
+static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs,
+                                struct tcs_request *msg)
+{
+       unsigned long curr_enabled;
+       u32 addr;
+       int i, j, k;
+       int m = tcs->tcs_offset;
+
+       for (i = 0; i < tcs->num_tcs; i++, m++) {
+               if (tcs_is_free(drv, m))
+                       continue;
+

drivers/soc/qcom/rpmh-rsc.c new file +5 -0

+
+       tcs = get_tcs_for_msg(drv, msg);
+       if (IS_ERR(tcs))
+               return PTR_ERR(tcs);
+

drivers/soc/qcom/rpmh-rsc.c new file +17 -0

+               ret = PTR_ERR(resp);
+               goto done_write;
+       }
+       resp->m = m;
+
+       atomic_set(&drv->tcs_in_use[m], 1);
+       __tcs_buffer_write(drv, m, 0, msg);
+       __tcs_trigger(drv, m);
+
+done_write:
+       spin_unlock_irqrestore(&tcs->tcs_lock, flags);
+       return ret;
+}
+
+/**
+ * rpmh_rsc_send_data: Validate the incoming message and write to the
+ * appropriate TCS block.

drivers/soc/qcom/rpmh-rsc.c new file +17 -0

+                                          msg->payload[0].addr);
+                       udelay(10);
+               }
+       } while (ret == -EBUSY);
+
+       return ret;
+}
+EXPORT_SYMBOL(rpmh_rsc_send_data);
+
+static int rpmh_probe_tcs_config(struct platform_device *pdev,
+                               struct rsc_drv *drv)
+{
+       struct tcs_type_config {
+               u32 type;
+               u32 n;
+       } tcs_cfg[TCS_TYPE_NR] = { { 0 } };
+       struct device_node *dn = pdev->dev.of_node;

drivers/soc/qcom/rpmh-rsc.c new file +6 -0

+       base = devm_ioremap_resource(&pdev->dev, res);
+       if (IS_ERR(base))
+               return PTR_ERR(base);
+
+       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "tcs");
+       if (!res)

drivers/soc/qcom/rpmh-rsc.c new file +24 -0

+       if (n != 2 * TCS_TYPE_NR)
+               return -EINVAL;
+
+       for (i = 0; i < TCS_TYPE_NR; i++) {
+               ret = of_property_read_u32_index(dn, "qcom,tcs-config",
+                                               i * 2, &tcs_cfg[i].type);
+               if (ret)
+                       return ret;
+               if (tcs_cfg[i].type >= TCS_TYPE_NR)
+                       return -EINVAL;
+
+               ret = of_property_read_u32_index(dn, "qcom,tcs-config",
+                                               i * 2 + 1, &tcs_cfg[i].n);
+               if (ret)
+                       return ret;
+               if (tcs_cfg[i].n > MAX_TCS_PER_TYPE)
+                       return -EINVAL;
+       }
+
+       for (i = 0; i < TCS_TYPE_NR; i++) {
+               tcs = &drv->tcs[tcs_cfg[i].type];
+               if (tcs->drv)
+                       return -EINVAL;
+               tcs->drv = drv;

drivers/soc/qcom/rpmh-rsc.c new file +34 -0

+                       return -EINVAL;
+
+               tcs->tcs_mask = ((1 << tcs->num_tcs) - 1) << st;
+               tcs->tcs_offset = st;
+               st += tcs->num_tcs;
+       }
+
+       drv->num_tcs = st;
+
+       return 0;
+}
+
+static int rpmh_rsc_probe(struct platform_device *pdev)
+{
+       struct device_node *dn = pdev->dev.of_node;
+       struct rsc_drv *drv;
+       int i, ret, irq;
+
+       drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
+       if (!drv)
+               return -ENOMEM;
+
+       ret = of_property_read_u32(dn, "qcom,drv-id", &drv->drv_id);
+       if (ret)
+               return ret;
+
+       drv->name = of_get_property(pdev->dev.of_node, "label", NULL);
+       if (!drv->name)
+               drv->name = dev_name(&pdev->dev);
+
+       ret = rpmh_probe_tcs_config(pdev, drv);
+       if (ret)
+               return ret;
+

drivers/soc/qcom/rpmh-rsc.c new file +9 -0

+                 .of_match_table = rpmh_drv_match,
+       },
+};
+
+static int __init rpmh_driver_init(void)
+{
+       return platform_driver_register(&rpmh_driver);
+}
+arch_initcall(rpmh_driver_init);

include/dt-bindings/soc/qcom,rpmh-rsc.h new file +14 -0

new file mode 100644
index 000000000000..868f998ea998
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef __DT_QCOM_RPMH_RSC_H__
+#define __DT_QCOM_RPMH_RSC_H__
+
+#define SLEEP_TCS      0
+#define WAKE_TCS       1
+#define ACTIVE_TCS     2
+#define CONTROL_TCS    3
+
+#endif /* __DT_QCOM_RPMH_RSC_H__ */

include/soc/qcom/tcs.h new file +28 -0

new file mode 100644
index 000000000000..9465f0560f7a
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef __SOC_QCOM_TCS_H__
+#define __SOC_QCOM_TCS_H__
+
+#define MAX_RPMH_PAYLOAD       16
+
+/**
+ * rpmh_state: state for the request
+ *
+ * RPMH_WAKE_ONLY_STATE:   Resume resource state to the value previously
+ *                         requested before the processor was powered down.
+ * RPMH_SLEEP_STATE:       State of the resource when the processor subsystem
+ *                         is powered down. There is no client using the
+ *                         resource actively.
+ * RPMH_ACTIVE_ONLY_STATE: Active or AMC mode requests. Resource state
+ *                         is aggregated immediately.
+ */
+enum rpmh_state {
+       RPMH_SLEEP_STATE,
+       RPMH_WAKE_ONLY_STATE,
+       RPMH_ACTIVE_ONLY_STATE,
+};
+
+/**

include/soc/qcom/tcs.h new file +3 -0

+};
+
+#endif /* __SOC_QCOM_TCS_H__ */

Quoting Lina Iyer (2018-03-02 08:43:08)

+#include <linux/module.h>

If the driver doesn't become tristate, this should become export.h instead of module.h

drivers/soc/qcom/rpmh-rsc.c line 139

Quoted 9 lines

drivers/soc/qcom/rpmh-rsc.c new file +9 -0

+
+static void write_tcs_reg_sync(struct rsc_drv *drv, int reg, int m, int n,
+                             u32 data)
+{
+       write_tcs_reg(drv, reg, m, n, data);
+       for (;;) {
+               if (data == read_tcs_reg(drv, reg, m, n))
+                       break;
+               udelay(1);

Hopefully this never gets stuck. Add a timeout?

drivers/soc/qcom/rpmh-rsc.c line 273

Quoted 3 lines

drivers/soc/qcom/rpmh-rsc.c new file +3 -0

+               list_del(&resp->list);
+               spin_unlock_irqrestore(&drv->drv_lock, flags);
+               free_response(resp);

But all this function does is free the structure? Will it do more later?

+ for_each_set_bit(j, &curr_enabled, MAX_CMDS_PER_TCS) {

drivers/soc/qcom/rpmh-rsc.c line 351

Quoted 7 lines

drivers/soc/qcom/rpmh-rsc.c new file +7 -0

+                       addr = read_tcs_reg(drv, RSC_DRV_CMD_ADDR, m, j);
+                       for (k = 0; k < msg->num_payload; k++) {
+                               if (addr == msg->payload[k].addr)
+                                       return -EBUSY;
+                       }
+               }
+       }

There isn't any way to do this in software only? Hopefully this isn't costly to read the TCS to see if something matches.

Hi Stephen,

I will address all the comments in the next spin of the patch. Here are some responses to the questions.

Quoted 2 lines · Stephen Boyd

Quoting Lina Iyer (2018-03-02 08:43:08)

[...]

Quoted 6 lines

+#include <linux/module.h>

If the driver doesn't become tristate, this should become export.h instead of module.h

MODULE_DEVICE_TABLE seems to need this.

Add device binding documentation for Qualcomm Technology Inc's RPMH RSC driver. The hardware block is used for communicating resource state

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt new file +19 -0

@@ -0,0 +1,131 @@
+RPMH RSC:
+------------
+
+Resource Power Manager Hardened (RPMH) is the mechanism for communicating with
+the hardened resource accelerators on Qualcomm SoCs. Requests to the resources
+can be written to the Trigger Command Set (TCS)  registers and using a (addr,
+val) pair and triggered. Messages in the TCS are then sent in sequence over an
+internal bus.
+
+The hardware block (Direct Resource Voter or DRV) is a part of the h/w entity
+(Resource State Coordinator a.k.a RSC) that can handle a multiple sleep and
+active/wake resource requests. Multiple such DRVs can exist in a SoC and can
+be written to from Linux. The structure of each DRV follows the same template
+with a few variations that are captured by the properties here.
+
+A TCS may be triggered from Linux or triggered by the F/W after all the CPUs
+have powered off to facilitate idle power saving. TCS could be classified as -
+
+       SLEEP,  /* Triggered by F/W */

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt new file +7 -0

+
+Properties:
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Should be "qcom,rpmh-rsc".

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt new file +27 -0

+       Value type: <string>
+       Definition: Maps the register specified in the reg property. Must be
+                   "drv" and "tcs".
+
+- interrupts:
+       Usage: required
+       Value type: <prop-encoded-interrupt>
+       Definition: The interrupt that trips when a message complete/response
+                  is received for this DRV from the accelerators.
+
+- qcom,drv-id:
+       Usage: required
+       Value type: <u32>
+       Definition: the id of the DRV in the RSC block.
+
+- qcom,tcs-config:
+       Usage: required
+       Value type: <prop-encoded-array>
+       Definition: the tuple defining the configuration of TCS.
+                   Must have 2 cells which describe each TCS type.
+                   <type number_of_tcs>.
+                   The order of the TCS must match the hardware
+                   configuration.
+       - Cell #1 (TCS Type): TCS types to be specified -
+               SLEEP_TCS
+               WAKE_TCS
+               ACTIVE_TCS

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt new file +10 -0

+
+Example 1:
+
+For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the
+register offsets for DRV2 start at 0D00, the register calculations are like
+this -
+First tuple: 0x179C0000 + 0x10000 * 2 = 0x179E0000
+Second tuple: 0x179E0000 + 0xD00 = 0x179E0D00
+
+       apps_rsc: rsc@179e000 {

Quoting Lina Iyer (2018-03-02 08:43:09)

Quoted 2 lines

Add device binding documentation for Qualcomm Technology Inc's RPMH RSC driver. The hardware block is used for communicating resource state

s/driver/hardware/

Quoted 23 lines

requests for shared resources.

Cc [email protected]

Signed-off-by Lina Iyer <[email protected]>

---

Changes in v2:
- Amend text to describe the registers in reg property
- Add reg-names for the registers
- Update examples to use GIC_SPI in interrupts instead of 0
- Rephrase incorrect description

Changes in v3:
- Fix unwanted capitalization
- Remove clients from the examples, this doc does not describe them
- Rephrase introductory paragraph
- Remove hardware specifics from DT bindings

---
 .../devicetree/bindings/arm/msm/rpmh-rsc.txt       | 131 +++++++++++++++++++++
 1 file changed, 131 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt

Quoted 5 lines

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt new file +0 -0

new file mode 100644
index 000000000000..afd3817cc615

Shouldn't this go into bindings/soc/qcom/ ?

+

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt line 29

Quoted 3 lines

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt new file +3 -0

+Requests can be made for the state of a resource, when the subsystem is active
+or idle. When all subsystems like Modem, GPU, CPU are idle, the resource state
+will be an aggregate of the sleep votes from each of those subsystem. Drivers

s/subsystem/subsystems/
s/Drivers/Clients/ ?

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt line 35

Quoted 6 lines

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt new file +6 -0

+may request a sleep value for their shared resources in addition to the active
+mode requests.
+
+Control requests are instance specific requests that may or may not reach an
+accelerator. Only one platform device in Linux can request a control channel
+on a DRV.

Not sure what this last sentence has to do with the DT binding. We generally try to avoid saying 'Linux' or 'driver' in DT binding documents, because they usually document hardware.

Quoted 11 lines

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt new file +11 -0

+
+Properties:
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Should be "qcom,rpmh-rsc".
+
+- reg:
+       Usage: required
+       Value type: <prop-encoded-array>

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt line 52

Quoted 6 lines

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt new file +6 -0

+       Definition: The first register specifies the base address of the DRV.
+                   The second register specifies the start address of the
+                   TCS.
+
+- reg-names:
+       Usage: required

optional?

Quoted 35 lines

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt new file +35 -0

+       Value type: <string>
+       Definition: Maps the register specified in the reg property. Must be
+                   "drv" and "tcs".
+
+- interrupts:
+       Usage: required
+       Value type: <prop-encoded-interrupt>
+       Definition: The interrupt that trips when a message complete/response
+                  is received for this DRV from the accelerators.
+
+- qcom,drv-id:
+       Usage: required
+       Value type: <u32>
+       Definition: the id of the DRV in the RSC block.
+
+- qcom,tcs-config:
+       Usage: required
+       Value type: <prop-encoded-array>
+       Definition: the tuple defining the configuration of TCS.
+                   Must have 2 cells which describe each TCS type.
+                   <type number_of_tcs>.
+                   The order of the TCS must match the hardware
+                   configuration.
+       - Cell #1 (TCS Type): TCS types to be specified -
+               SLEEP_TCS
+               WAKE_TCS
+               ACTIVE_TCS
+               CONTROL_TCS
+       - Cell #2 (Number of TCS): <u32>
+
+- label:
+       Usage: optional
+       Value type: <string>
+       Definition: Name for the RSC. The name would be used in trace logs.
+

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt line 89

Quoted 2 lines

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt new file +2 -0

+Drivers that want to use the RSC to communicate with RPMH must specify their
+bindings as child of the RSC controllers they wish to communicate with.

Is that going to work for drivers that want to talk to two or more RSCs? I suppose that they'll have to hook up into some sort of framework like clk or regulator and then drivers that want to use two RSCs for those things would be linked to those sub-device drivers with the normal clk or regulator bindings?

Quoted 10 lines

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt new file +10 -0

+
+Example 1:
+
+For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the
+register offsets for DRV2 start at 0D00, the register calculations are like
+this -
+First tuple: 0x179C0000 + 0x10000 * 2 = 0x179E0000
+Second tuple: 0x179E0000 + 0xD00 = 0x179E0D00
+
+       apps_rsc: rsc@179e000 {

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt line 109

Quoted 10 lines

Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt new file +10 -0

+               label = "apps_rsc";
+               compatible = "qcom,rpmh-rsc";
+               reg = <0x179e0000 0x10000>, <0x179e0d00 0x3000>;
+               reg-names = "drv", "tcs";
+               interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
+               qcom,drv-id = <2>;
+               qcom,tcs-config = <SLEEP_TCS   3>,
+                                 <WAKE_TCS    3>,
+                                 <ACTIVE_TCS  2>,
+                                 <CONTROL_TCS 1>;

Could qcom,tcs-config become something more like below?

	#qcom,sleep-tcs = <3>;
	#qcom,wake-tcs = <3>;
	#qcom,active-tcs = <2>;
	#qcom,control-tcs = <1>;

I don't really understand the binding design to have many cells with the *_TCS defines indicating what comes next.

Quoted 1 line

Quoted 1 line

drivers/soc/qcom/./trace-rpmh.h:29:3: error: implicit declaration of function '__assign_string'; did you mean '__assign_str'? [-Werror=implicit-function-declaration]

      __assign_string(name, d->name);
      ^

include/trace/trace_events.h:719:4: note: in definition of macro 'DECLARE_EVENT_CLASS'

Quoted 1 line · On Tue, 6 Mar 2018 13:38:06 +0800 kbuild test robot <[email protected]> wrote:

Quoted 1 line

Quoted 1 line

drivers/soc/qcom/./trace-rpmh.h:29:3: error: implicit declaration of function '__assign_string'; did you mean '__assign_str'? [-Werror=implicit-function-declaration]

Yes, you meant __assign_str(). I may have said __assign_string() in my comments, but I was doing it from memory, not actually compiling code.

Please make sure you test the code and make sure it builds before posting. And displays the tracepoint as you expect it to.

Quoted 5 lines · Steven Rostedt

Quoted 1 line · On Tue, 6 Mar 2018 13:38:06 +0800 kbuild test robot <[email protected]> wrote:

Quoted 1 line

Quoted 1 line

drivers/soc/qcom/./trace-rpmh.h:29:3: error: implicit declaration of function '__assign_string'; did you mean '__assign_str'? [-Werror=implicit-function-declaration]

Quoted 6 lines

Yes, you meant __assign_str(). I may have said __assign_string() in my comments, but I was doing it from memory, not actually compiling code.

Please make sure you test the code and make sure it builds before posting. And displays the tracepoint as you expect it to.

I compiled and checked for sparse. Didn't explictly test for this. Not sure why it did not fail for me.

Duh. FTRACE seems to have been disabled. Will fix and update.

drivers/soc/qcom/rpmh.c new file +12 -0

new file mode 100644
index 000000000000..d95ea3fa8b67
@@ -0,0 +1,257 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/atomic.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>

drivers/soc/qcom/rpmh.c new file +18 -0

+ *
+ * @dev: the platform device that is the owner
+ * @ctrlr: the controller associated with this client.
+ */
+struct rpmh_client {
+       struct device *dev;
+       struct rpmh_ctrlr *ctrlr;
+};
+
+static struct rpmh_ctrlr rpmh_rsc[RPMH_MAX_MBOXES];
+static DEFINE_MUTEX(rpmh_ctrlr_mutex);
+
+void rpmh_tx_done(struct tcs_request *msg, int r)
+{
+       struct rpmh_request *rpm_msg = container_of(msg,
+                                                  struct rpmh_request, msg);
+       atomic_t *wc = rpm_msg->wait_count;
+       struct completion *compl = rpm_msg->completion;

drivers/soc/qcom/rpmh.c new file +32 -0

+               complete(compl);
+}
+EXPORT_SYMBOL(rpmh_tx_done);
+
+/**
+ * wait_for_tx_done: Wait until the response is received.
+ *
+ * @rc: The RPMH client
+ * @compl: The completion object
+ * @addr: An addr that we sent in that request
+ * @data: The data for the address in that request
+ *
+ */
+static int wait_for_tx_done(struct rpmh_client *rc,
+                          struct completion *compl, u32 addr, u32 data)
+{
+       int ret;
+
+       ret = wait_for_completion_timeout(compl, RPMH_TIMEOUT);
+       if (ret)
+               dev_dbg(rc->dev,
+                      "RPMH response received addr=0x%x data=0x%x\n",
+                      addr, data);
+       else
+               dev_err(rc->dev,
+                       "RPMH response timeout addr=0x%x data=0x%x\n",
+                       addr, data);
+
+       return (ret > 0) ? 0 : -ETIMEDOUT;
+}
+
+/**

drivers/soc/qcom/rpmh.c new file +30 -0

+
+       rpm_msg->msg.state = state;
+
+       if (state == RPMH_ACTIVE_ONLY_STATE) {
+               WARN_ON(irqs_disabled());
+               ret = rpmh_rsc_send_data(rc->ctrlr->drv, &rpm_msg->msg);
+               if (!ret)
+                       dev_dbg(rc->dev,
+                              "RPMH request sent addr=0x%x, data=0x%x\n",
+                              rpm_msg->msg.payload[0].addr,
+                              rpm_msg->msg.payload[0].data);
+               else
+                       dev_warn(rc->dev,
+                               "Error in RPMH request addr=0x%x, data=0x%x\n",
+                               rpm_msg->msg.payload[0].addr,
+                               rpm_msg->msg.payload[0].data);
+       }
+
+       return ret;
+}
+
+/**
+ * rpmh_write: Write a set of RPMH commands and block until response
+ *
+ * @rc: The RPMh handle got from rpmh_get_dev_channel
+ * @state: Active/sleep set
+ * @cmd: The payload data
+ * @n: The number of elements in payload
+ *
+ * May sleep. Do not call from atomic contexts.

drivers/soc/qcom/rpmh.c new file +7 -0

+
+       memcpy(rpm_msg.cmd, cmd, n * sizeof(*cmd));
+       rpm_msg.msg.num_payload = n;
+
+       ret = __rpmh_write(rc, state, &rpm_msg);
+       if (ret)
+               return ret;

drivers/soc/qcom/rpmh.c new file +23 -0

+
+       if (!drv)
+               return ctrlr;
+
+       mutex_lock(&rpmh_ctrlr_mutex);
+       for (i = 0; i < RPMH_MAX_MBOXES; i++) {
+               if (rpmh_rsc[i].drv == drv) {
+                       ctrlr = &rpmh_rsc[i];
+                       goto unlock;
+               }
+       }
+
+       for (i = 0; i < RPMH_MAX_MBOXES; i++) {
+               if (rpmh_rsc[i].drv == NULL) {
+                       ctrlr = &rpmh_rsc[i];
+                       ctrlr->drv = drv;
+                       break;
+               }
+       }
+       WARN_ON(i == RPMH_MAX_MBOXES);
+unlock:
+       mutex_unlock(&rpmh_ctrlr_mutex);
+       return ctrlr;

drivers/soc/qcom/rpmh.c new file +5 -0

+{
+       struct rpmh_client *rc;
+
+       rc = kzalloc(sizeof(*rc), GFP_KERNEL);
+       if (!rc)

Quoting Lina Iyer (2018-03-02 08:43:11)

Quoted 22 lines

drivers/soc/qcom/rpmh.c new file +16 -0

new file mode 100644
index 000000000000..d95ea3fa8b67
@@ -0,0 +1,257 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/atomic.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/wait.h>
+

drivers/soc/qcom/rpmh.c line 22

Quoted 6 lines

drivers/soc/qcom/rpmh.c new file +6 -0

+#include <soc/qcom/rpmh.h>
+
+#include "rpmh-internal.h"
+
+#define RPMH_MAX_MBOXES                        2
+#define RPMH_TIMEOUT                   (10 * HZ)

Can this be in ms instead of HZ units?

Quoted 6 lines

drivers/soc/qcom/rpmh.c new file +6 -0

+
+/**
+ * rpmh_ctrlr: our representation of the controller
+ *
+ * @drv: the controller instance
+ */

drivers/soc/qcom/rpmh.c line 66

Quoted 6 lines

drivers/soc/qcom/rpmh.c new file +6 -0

+struct rpmh_ctrlr {
+       struct rsc_drv *drv;
+};
+
+/**
+ * rpmh_client: the client object

same kernel doc issues here.

Quoted 27 lines

drivers/soc/qcom/rpmh.c new file +27 -0

+ *
+ * @dev: the platform device that is the owner
+ * @ctrlr: the controller associated with this client.
+ */
+struct rpmh_client {
+       struct device *dev;
+       struct rpmh_ctrlr *ctrlr;
+};
+
+static struct rpmh_ctrlr rpmh_rsc[RPMH_MAX_MBOXES];
+static DEFINE_MUTEX(rpmh_ctrlr_mutex);
+
+void rpmh_tx_done(struct tcs_request *msg, int r)
+{
+       struct rpmh_request *rpm_msg = container_of(msg,
+                                                  struct rpmh_request, msg);
+       atomic_t *wc = rpm_msg->wait_count;
+       struct completion *compl = rpm_msg->completion;
+
+       rpm_msg->err = r;
+
+       if (r)
+               dev_err(rpm_msg->rc->dev,
+                      "RPMH TX fail in msg addr 0x%x, err=%d\n",
+                      rpm_msg->msg.payload[0].addr, r);
+
+       /* Signal the blocking thread we are done */

drivers/soc/qcom/rpmh.c line 94

Quoted 1 line

drivers/soc/qcom/rpmh.c new file +1 -0

+       if (wc && atomic_dec_and_test(wc) && compl)

I don't understand this whole thing. The atomic variable is always set to 1 in this patch, and then we will do a dec and test and then complete when that happens. There is the case where it isn't assigned, but then this logic doesn't happen at all. There must be some future code that uses this? Can you add the atomic counting stuff in that patch when we need to count more than one?

And then if that future happens, maybe consider changing from a count to a chained DMA list style type of thing, where each message has a single element that's written, but each message can have a 'wait' bit or not that would cause this code to call complete if it's there. Then drivers can wait on any certain part of the message completion (or multiple of them) without us having to do a count.

Quoted 36 lines

drivers/soc/qcom/rpmh.c new file +36 -0

+               complete(compl);
+}
+EXPORT_SYMBOL(rpmh_tx_done);
+
+/**
+ * wait_for_tx_done: Wait until the response is received.
+ *
+ * @rc: The RPMH client
+ * @compl: The completion object
+ * @addr: An addr that we sent in that request
+ * @data: The data for the address in that request
+ *
+ */
+static int wait_for_tx_done(struct rpmh_client *rc,
+                          struct completion *compl, u32 addr, u32 data)
+{
+       int ret;
+
+       ret = wait_for_completion_timeout(compl, RPMH_TIMEOUT);
+       if (ret)
+               dev_dbg(rc->dev,
+                      "RPMH response received addr=0x%x data=0x%x\n",
+                      addr, data);
+       else
+               dev_err(rc->dev,
+                       "RPMH response timeout addr=0x%x data=0x%x\n",
+                       addr, data);
+
+       return (ret > 0) ? 0 : -ETIMEDOUT;
+}
+
+/**
+ * __rpmh_write: send the RPMH request
+ *
+ * @rc: The RPMH client
+ * @state: Active/Sleep request type

drivers/soc/qcom/rpmh.c line 136

Quoted 6 lines

drivers/soc/qcom/rpmh.c new file +6 -0

+ * @rpm_msg: The data that needs to be sent (payload).
+ */
+static int __rpmh_write(struct rpmh_client *rc, enum rpmh_state state,
+                      struct rpmh_request *rpm_msg)
+{
+       int ret = -EFAULT;

Not sure -EFAULT is the right error value here. -EINVAL?

Quoted 34 lines

drivers/soc/qcom/rpmh.c new file +34 -0

+
+       rpm_msg->msg.state = state;
+
+       if (state == RPMH_ACTIVE_ONLY_STATE) {
+               WARN_ON(irqs_disabled());
+               ret = rpmh_rsc_send_data(rc->ctrlr->drv, &rpm_msg->msg);
+               if (!ret)
+                       dev_dbg(rc->dev,
+                              "RPMH request sent addr=0x%x, data=0x%x\n",
+                              rpm_msg->msg.payload[0].addr,
+                              rpm_msg->msg.payload[0].data);
+               else
+                       dev_warn(rc->dev,
+                               "Error in RPMH request addr=0x%x, data=0x%x\n",
+                               rpm_msg->msg.payload[0].addr,
+                               rpm_msg->msg.payload[0].data);
+       }
+
+       return ret;
+}
+
+/**
+ * rpmh_write: Write a set of RPMH commands and block until response
+ *
+ * @rc: The RPMh handle got from rpmh_get_dev_channel
+ * @state: Active/sleep set
+ * @cmd: The payload data
+ * @n: The number of elements in payload
+ *
+ * May sleep. Do not call from atomic contexts.
+ */
+int rpmh_write(struct rpmh_client *rc, enum rpmh_state state,
+             struct tcs_cmd *cmd, int n)
+{

drivers/soc/qcom/rpmh.c line 176

Quoted 6 lines

drivers/soc/qcom/rpmh.c new file +6 -0

+       DECLARE_COMPLETION_ONSTACK(compl);
+       atomic_t wait_count = ATOMIC_INIT(1);
+       DEFINE_RPMH_MSG_ONSTACK(rc, state, &compl, &wait_count, rpm_msg);
+       int ret;
+
+       if (IS_ERR_OR_NULL(rc) || !cmd || n <= 0 || n > MAX_RPMH_PAYLOAD)

How is rc IS_ERR_OR_NULL at this point?

Should n be unsigned then?

drivers/soc/qcom/rpmh.c line 179

Quoted 3 lines

drivers/soc/qcom/rpmh.c new file +3 -0

+               return -EINVAL;
+
+       might_sleep();

The wait_for_tx_done() would handle this might_sleep?

Quoted 11 lines

drivers/soc/qcom/rpmh.c new file +11 -0

+
+       memcpy(rpm_msg.cmd, cmd, n * sizeof(*cmd));
+       rpm_msg.msg.num_payload = n;
+
+       ret = __rpmh_write(rc, state, &rpm_msg);
+       if (ret)
+               return ret;
+
+       return wait_for_tx_done(rc, &compl, cmd[0].addr, cmd[0].data);
+}
+EXPORT_SYMBOL(rpmh_write);

drivers/soc/qcom/rpmh.c line 196

Quoted 6 lines

drivers/soc/qcom/rpmh.c new file +6 -0

+
+static struct rpmh_ctrlr *get_rpmh_ctrlr(struct platform_device *pdev)
+{
+       int i;
+       struct rsc_drv *drv = dev_get_drvdata(pdev->dev.parent);
+       struct rpmh_ctrlr *ctrlr = ERR_PTR(-EFAULT);

Not sure -EFAULT is the right error value here.

Quoted 23 lines

drivers/soc/qcom/rpmh.c new file +23 -0

+
+       if (!drv)
+               return ctrlr;
+
+       mutex_lock(&rpmh_ctrlr_mutex);
+       for (i = 0; i < RPMH_MAX_MBOXES; i++) {
+               if (rpmh_rsc[i].drv == drv) {
+                       ctrlr = &rpmh_rsc[i];
+                       goto unlock;
+               }
+       }
+
+       for (i = 0; i < RPMH_MAX_MBOXES; i++) {
+               if (rpmh_rsc[i].drv == NULL) {
+                       ctrlr = &rpmh_rsc[i];
+                       ctrlr->drv = drv;
+                       break;
+               }
+       }
+       WARN_ON(i == RPMH_MAX_MBOXES);
+unlock:
+       mutex_unlock(&rpmh_ctrlr_mutex);
+       return ctrlr;

drivers/soc/qcom/rpmh.c line 229

Quoted 10 lines

drivers/soc/qcom/rpmh.c new file +10 -0

+}
+
+/**
+ * rpmh_get_client: Get the RPMh handle
+ *
+ * @pdev: the platform device which needs to communicate with RPM
+ * accelerators
+ * May sleep.
+ */
+struct rpmh_client *rpmh_get_client(struct platform_device *pdev)

Given that the child devices are fairly well aware that they're rpmh device drivers, why do we need this set of APIs in a different file and also why do we need to have a client cookie design? It would make sense to have the cookie if the device hierarchy wasn't direct, but that doesn't seem to be the case here. Also it would make things easier to follow if the code was folded into the same C file. It looks like we may have two files exporting symbols to each other but not anywhere else.

Take a look at clk-rpm.c in clk/qcom and you'll see that we don't do any sort of client cookie. Instead, the parent device drv data has the pointer we want to get, and then the rpm APIs take that pointer.

Sleep and wake requests are sent when the application processor subsystem of the SoC is entering deep sleep states like in suspend. These requests help lower the system power requirements when the resources are not in use.

Sleep and wake requests are written to the TCS slots but are not triggered at the time of writing. The TCS are triggered by the firmware after the last of the CPUs has executed its WFI. Since these requests may come in different batches of requests, it is job of this controller

driver to find arrange the requests into the available TCSes.

Signed-off-by Lina Iyer <[email protected]>

---
 drivers/soc/qcom/rpmh-internal.h |   7 +++
 drivers/soc/qcom/rpmh-rsc.c      | 128 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 135 insertions(+)

drivers/soc/qcom/rpmh-internal.h +1 -0

index 1442a64ac4c5..65dfe1716265 100644
@@ -13,6 +13,7 @@
 #define MAX_CMDS_PER_TCS               16
 #define MAX_TCS_PER_TYPE               3
 #define MAX_TCS_NR                     (MAX_TCS_PER_TYPE * TCS_TYPE_NR)
+#define MAX_TCS_SLOTS                  (MAX_CMDS_PER_TCS * MAX_TCS_PER_TYPE)
 
 struct rsc_drv;
 

drivers/soc/qcom/rpmh-internal.h +7 -0

+               for (i = 0; i < msg->num_payload; i++)
+                       tcs->cmd_addr[slot + i] = msg->payload[i].addr;
+       }
+
+       offset = slot / tcs->ncpt;
+       *m = offset + tcs->tcs_offset;
+       *n = slot % tcs->ncpt;

drivers/soc/qcom/rpmh-internal.h +15 -0

+
+       tcs = get_tcs_for_msg(drv, msg);
+       if (IS_ERR(tcs))
+               return PTR_ERR(tcs);
+
+       spin_lock_irqsave(&tcs->tcs_lock, flags);
+       /* find the m-th TCS and the n-th position in the TCS to write to */
+       ret = find_slots(tcs, msg, &m, &n);
+       if (!ret)
+               __tcs_buffer_write(drv, m, n, msg);
+       spin_unlock_irqrestore(&tcs->tcs_lock, flags);
+
+       return ret;
+}
+

drivers/soc/qcom/rpmh-internal.h +4 -0

+                                           sizeof(u32) * tcs->num_tcs * ncpt,
+                                           GFP_KERNEL);
+               if (!tcs->cmd_addr)
+                       return -ENOMEM;

Quoting Lina Iyer (2018-03-02 08:43:12)

Quoted 9 lines

Sleep and wake requests are sent when the application processor subsystem of the SoC is entering deep sleep states like in suspend. These requests help lower the system power requirements when the resources are not in use.

Sleep and wake requests are written to the TCS slots but are not triggered at the time of writing. The TCS are triggered by the firmware after the last of the CPUs has executed its WFI. Since these requests may come in different batches of requests, it is job of this controller

it is the job?

Quoted 1 line

driver to find arrange the requests into the available TCSes.

find and arrange?

Quoted 19 lines

Signed-off-by Lina Iyer <[email protected]>

---
 drivers/soc/qcom/rpmh-internal.h |   7 +++
 drivers/soc/qcom/rpmh-rsc.c      | 128 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 135 insertions(+)

drivers/soc/qcom/rpmh-internal.h +1 -0

index 1442a64ac4c5..65dfe1716265 100644
@@ -13,6 +13,7 @@
 #define MAX_CMDS_PER_TCS               16
 #define MAX_TCS_PER_TYPE               3
 #define MAX_TCS_NR                     (MAX_TCS_PER_TYPE * TCS_TYPE_NR)
+#define MAX_TCS_SLOTS                  (MAX_CMDS_PER_TCS * MAX_TCS_PER_TYPE)
 
 struct rsc_drv;
 

drivers/soc/qcom/rpmh-internal.h struct tcs_response { · line 48

Quoted 5 lines

drivers/soc/qcom/rpmh-internal.h +1 -0

@@ -44,6 +45,8 @@ struct tcs_response {
  * @ncpt: number of commands in each TCS
  * @tcs_lock: lock for synchronizing this TCS writes
  * @responses: response objects for requests sent from each TCS
+ * @cmd_addr: flattened cache of cmds in sleep/wake TCS

Maybe 'cmds' or 'cmd_cache'?

Quoted 5 lines

drivers/soc/qcom/rpmh-internal.h +1 -0

+ * @slots: indicates which of @cmd_addr are occupied
  */
 struct tcs_group {
        struct rsc_drv *drv;

@@ -54,6 +57,9 @@ struct tcs_group {

drivers/soc/qcom/rpmh-internal.h struct tcs_group { · line 62

Quoted 6 lines

drivers/soc/qcom/rpmh-internal.h +3 -0

        int ncpt;
        spinlock_t tcs_lock;
        struct tcs_response *responses[MAX_TCS_PER_TYPE];
+       u32 *cmd_addr;
+       DECLARE_BITMAP(slots, MAX_TCS_SLOTS);
+

Drop the newline please.

drivers/soc/qcom/rpmh-internal.h int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg) · line 460

Quoted 8 lines

drivers/soc/qcom/rpmh-internal.h +1 -0

 };
 
 /**

@@ -450,6 +457,114 @@ int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg)
 }
 EXPORT_SYMBOL(rpmh_rsc_send_data);
 
+static int find_match(struct tcs_group *tcs, struct tcs_cmd *cmd, int len)

const tcs and cmd?

drivers/soc/qcom/rpmh-internal.h int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg) · line 462

Quoted 2 lines

drivers/soc/qcom/rpmh-internal.h +2 -0

+{
+       bool found = false;

Drop.

drivers/soc/qcom/rpmh-internal.h int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg) · line 466

Quoted 4 lines

drivers/soc/qcom/rpmh-internal.h +4 -0

+       int i = 0, j;
+
+       /* Check for already cached commands */
+       while ((i = find_next_bit(tcs->slots, MAX_TCS_SLOTS, i)) <

for_each_set_bit(... ?

drivers/soc/qcom/rpmh-internal.h int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg) · line 477

Quoted 11 lines

drivers/soc/qcom/rpmh-internal.h +11 -0

+             MAX_TCS_SLOTS) {
+               if (tcs->cmd_addr[i] != cmd[0].addr) {
+                       i++;
+                       continue;
+               }
+               /* sanity check to ensure the seq is same */
+               for (j = 1; j < len; j++) {
+                       WARN((tcs->cmd_addr[i + j] != cmd[j].addr),
+                           "Message does not match previous sequence.\n");
+                       return -EINVAL;
+               }

Can you fold the if and for loop together?

		for (j = 0; j < len; j++) {
			if (tcs->cmd_addr[i + j] != cmd[j].addr) {
				if (j == 0)
					break; /* Try another slot */
				WARN("Message doesn't match previous sequence\n");
				return -EINVAL;
			} else if (j == len - 1) {
				return i;
			}
		}
	}

	return -ENODATA;

drivers/soc/qcom/rpmh-internal.h int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg) · line 482

Quoted 5 lines

drivers/soc/qcom/rpmh-internal.h +5 -0

+               found = true;
+               break;
+       }
+
+       return found ? i : -1;

Is there space between slots? Just trying to understand how we differentiate two adjacent cmd buffers with the bitmap scheme if this loop is looking for free bits to find slots. Or maybe we need two bitmaps where one is the allocated region and the other is something indicating the start bit of a message

Quoted 6 lines

drivers/soc/qcom/rpmh-internal.h +6 -0

+}
+
+static int find_slots(struct tcs_group *tcs, struct tcs_request *msg,
+                    int *m, int *n)
+{
+       int slot, offset;

drivers/soc/qcom/rpmh-internal.h int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg) · line 494

Quoted 6 lines

drivers/soc/qcom/rpmh-internal.h +6 -0

+       int i = 0;
+
+       /* Find if we already have the msg in our TCS */
+       slot = find_match(tcs, msg->payload, msg->num_payload);
+       if (slot >= 0)
+               goto copy_data;

Shouldn't this goto skip setting the bits in tcs->slots?

Quoted 6 lines

drivers/soc/qcom/rpmh-internal.h +6 -0

+
+       /* Do over, until we can fit the full payload in a TCS */
+       do {
+               slot = bitmap_find_next_zero_area(tcs->slots, MAX_TCS_SLOTS,
+                                                i, msg->num_payload, 0);
+               if (slot == MAX_TCS_SLOTS)

drivers/soc/qcom/rpmh-internal.h int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg) · line 506

Quoted 6 lines

drivers/soc/qcom/rpmh-internal.h +6 -0

+                       break;
+               i += tcs->ncpt;
+       } while (slot + msg->num_payload - 1 >= i);
+
+       if (slot == MAX_TCS_SLOTS)
+               return -ENOMEM;

Would be nice to remove this duplicate condition somehow. Maybe a goto?

drivers/soc/qcom/rpmh-internal.h int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg) · line 511

Quoted 5 lines

drivers/soc/qcom/rpmh-internal.h +5 -0

+
+copy_data:
+       bitmap_set(tcs->slots, slot, msg->num_payload);
+       /* Copy the addresses of the resources over to the slots */
+       if (tcs->cmd_addr) {

find_match() above didn't check for tcs->cmd_addr. Does this ever happen to fail?

Quoted 11 lines

drivers/soc/qcom/rpmh-internal.h +11 -0

+               for (i = 0; i < msg->num_payload; i++)
+                       tcs->cmd_addr[slot + i] = msg->payload[i].addr;
+       }
+
+       offset = slot / tcs->ncpt;
+       *m = offset + tcs->tcs_offset;
+       *n = slot % tcs->ncpt;
+
+       return 0;
+}
+

drivers/soc/qcom/rpmh-internal.h int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg) · line 528

Quoted 6 lines

drivers/soc/qcom/rpmh-internal.h +6 -0

+static int tcs_ctrl_write(struct rsc_drv *drv, struct tcs_request *msg)
+{
+       struct tcs_group *tcs;
+       int m = 0, n = 0;
+       unsigned long flags;
+       int ret = 0;

Drop initial assignment please.

Quoted 15 lines

drivers/soc/qcom/rpmh-internal.h +15 -0

+
+       tcs = get_tcs_for_msg(drv, msg);
+       if (IS_ERR(tcs))
+               return PTR_ERR(tcs);
+
+       spin_lock_irqsave(&tcs->tcs_lock, flags);
+       /* find the m-th TCS and the n-th position in the TCS to write to */
+       ret = find_slots(tcs, msg, &m, &n);
+       if (!ret)
+               __tcs_buffer_write(drv, m, n, msg);
+       spin_unlock_irqrestore(&tcs->tcs_lock, flags);
+
+       return ret;
+}
+

[...]

drivers/soc/qcom/rpmh-internal.h static int rpmh_probe_tcs_config(struct platform_device *pdev, · line 656

Quoted 13 lines

drivers/soc/qcom/rpmh-internal.h +9 -0

@@ -530,6 +645,19 @@ static int rpmh_probe_tcs_config(struct platform_device *pdev,
                tcs->tcs_mask = ((1 << tcs->num_tcs) - 1) << st;
                tcs->tcs_offset = st;
                st += tcs->num_tcs;
+
+               /*
+                * Allocate memory to cache sleep and wake requests to
+                * avoid reading TCS register memory.
+                */
+               if (tcs->type == ACTIVE_TCS)
+                       continue;
+
+               tcs->cmd_addr = devm_kzalloc(&pdev->dev,

devm_kcalloc(&pdev->dev, tcs->num_tcs * ncpt, sizeof(u32) ?

Quoted 4 lines

drivers/soc/qcom/rpmh-internal.h +4 -0

+                                           sizeof(u32) * tcs->num_tcs * ncpt,
+                                           GFP_KERNEL);
+               if (!tcs->cmd_addr)
+                       return -ENOMEM;

Quoted 15 lines · Stephen Boyd

Quoting Lina Iyer (2018-03-02 08:43:12)

drivers/soc/qcom/rpmh-rsc.c int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg) · line 494

Quoted 10 lines

drivers/soc/qcom/rpmh-rsc.c +10 -0

+static int find_slots(struct tcs_group *tcs, struct tcs_request *msg,
+                    int *m, int *n)
+{
+       int slot, offset;
+       int i = 0;
+
+       /* Find if we already have the msg in our TCS */
+       slot = find_match(tcs, msg->payload, msg->num_payload);
+       if (slot >= 0)
+               goto copy_data;

Shouldn't this goto skip setting the bits in tcs->slots?

No, we overwrite what we found with this new data.

Quoted 6 lines

Quoted 6 lines

drivers/soc/qcom/rpmh-rsc.c +6 -0

+
+       /* Do over, until we can fit the full payload in a TCS */
+       do {
+               slot = bitmap_find_next_zero_area(tcs->slots, MAX_TCS_SLOTS,
+                                                i, msg->num_payload, 0);
+               if (slot == MAX_TCS_SLOTS)

Quoted 9 lines

drivers/soc/qcom/rpmh-rsc.c int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg) · line 506

Quoted 6 lines

drivers/soc/qcom/rpmh-rsc.c +6 -0

+                       break;
+               i += tcs->ncpt;
+       } while (slot + msg->num_payload - 1 >= i);
+
+       if (slot == MAX_TCS_SLOTS)
+               return -ENOMEM;

Would be nice to remove this duplicate condition somehow. Maybe a goto?

I would return instead of the break earlier instead of this here.

Quoted 9 lines

drivers/soc/qcom/rpmh-rsc.c int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg) · line 511

Quoted 5 lines

drivers/soc/qcom/rpmh-rsc.c +5 -0

+
+copy_data:
+       bitmap_set(tcs->slots, slot, msg->num_payload);
+       /* Copy the addresses of the resources over to the slots */
+       if (tcs->cmd_addr) {

find_match() above didn't check for tcs->cmd_addr. Does this ever happen to fail?

Not allocated for active TCSes. I should be checking for it there as well. Not sure how I didnt see a failure.

drivers/soc/qcom/rpmh-rsc.c +10 -0

index 34e780d9670f..e9f5a1f387fd 100644
@@ -170,6 +170,52 @@ static struct tcs_group *get_tcs_of_type(struct rsc_drv *drv, int type)
        return tcs;
 }
 
+/**
+ * rpmh_rsc_invalidate - Invalidate sleep and wake TCSes
+ *
+ * @drv: the mailbox controller
+ */
+int rpmh_rsc_invalidate(struct rsc_drv *drv)
+{
+       struct tcs_group *tcs;
+       int m, type, ret = 0;
+       int inv_types[] = { WAKE_TCS, SLEEP_TCS };

drivers/soc/qcom/rpmh-rsc.c +8 -0

+               if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS)) {
+                       spin_unlock_irqrestore(&tcs->tcs_lock, flags);
+                       continue;
+               }
+
+               /* Clear the enable register for each TCS of the type */
+               for (m = tcs->tcs_offset;
+                   m < tcs->tcs_offset + tcs->num_tcs; m++) {

Quoting Lina Iyer (2018-03-02 08:43:13)

Quoted 22 lines

drivers/soc/qcom/rpmh-rsc.c +14 -0

index 34e780d9670f..e9f5a1f387fd 100644
@@ -170,6 +170,52 @@ static struct tcs_group *get_tcs_of_type(struct rsc_drv *drv, int type)
        return tcs;
 }
 
+/**
+ * rpmh_rsc_invalidate - Invalidate sleep and wake TCSes
+ *
+ * @drv: the mailbox controller
+ */
+int rpmh_rsc_invalidate(struct rsc_drv *drv)
+{
+       struct tcs_group *tcs;
+       int m, type, ret = 0;
+       int inv_types[] = { WAKE_TCS, SLEEP_TCS };
+       unsigned long drv_flags, flags;
+
+       /* Lock the DRV and clear sleep and wake TCSes */
+       spin_lock_irqsave(&drv->drv_lock, drv_flags);

drivers/soc/qcom/rpmh-rsc.c static struct tcs_group *get_tcs_of_type(struct rsc_drv *drv, int type) · line 192

Quoted 6 lines

drivers/soc/qcom/rpmh-rsc.c +6 -0

+       for (type = 0; type < ARRAY_SIZE(inv_types); type++) {
+               tcs = get_tcs_of_type(drv, inv_types[type]);
+               if (IS_ERR(tcs))
+                       continue;
+
+               spin_lock_irqsave(&tcs->tcs_lock, flags);

Should just be a spin_lock() because we already irq saved a few lines above.

Quoted 8 lines

drivers/soc/qcom/rpmh-rsc.c +8 -0

+               if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS)) {
+                       spin_unlock_irqrestore(&tcs->tcs_lock, flags);
+                       continue;
+               }
+
+               /* Clear the enable register for each TCS of the type */
+               for (m = tcs->tcs_offset;
+                   m < tcs->tcs_offset + tcs->num_tcs; m++) {

drivers/soc/qcom/rpmh-rsc.c static struct tcs_group *get_tcs_of_type(struct rsc_drv *drv, int type) · line 210

Quoted 10 lines

drivers/soc/qcom/rpmh-rsc.c +10 -0

+                       if (!tcs_is_free(drv, m)) {
+                               spin_unlock_irqrestore(&tcs->tcs_lock, flags);
+                               ret = -EAGAIN;
+                               goto drv_unlock;
+                       }
+                       write_tcs_reg_sync(drv, RSC_DRV_CMD_ENABLE, m, 0, 0);
+                       /* Mark the TCS slots as free */
+                       bitmap_zero(tcs->slots, MAX_TCS_SLOTS);
+               }
+               spin_unlock_irqrestore(&tcs->tcs_lock, flags);

Maybe make another function called rpmh_tcs_invalidate(drv, enum TCS_FOO) and put this logic inside it? And then change from a for loop to two direct calls:

	lock()
	ret = rpmh_tcs_invalidate(drv, WAKE_TCS);
	if (!ret)
		ret = rpmh_tcs_invalidate(drv, SLEEP_TCS);
	unlock()