LinuxLists.cc - [PATCH v2 0/6] drivers/qcom: add additional functionality to RPMH

[permalink] [raw]

Subject: [PATCH v2 2/6] drivers: qcom: rpmh: export controller idle status

From: Lina Iyer <[email protected]>

Allow the controller state be read by platform drivers. This is useful
for PM drivers which can choose to disallow idle modes when the
controller is busy.

Signed-off-by: Lina Iyer <[email protected]>
Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
---
drivers/soc/qcom/rpmh.c | 13 +++++++++++++
include/soc/qcom/rpmh.h | 5 +++++
2 files changed, 18 insertions(+)

diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index c7beb68..2382276 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -511,3 +511,16 @@ int rpmh_invalidate(const struct device *dev)
return ret;
}
EXPORT_SYMBOL(rpmh_invalidate);
+
+/**
+ * rpmh_ctrlr_idle: Return the controller idle status
+ *
+ * @dev: the device making the request
+ */
+int rpmh_ctrlr_idle(const struct device *dev)
+{
+ struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
+
+ return rpmh_rsc_ctrlr_is_idle(ctrlr_to_drv(ctrlr));
+}
+EXPORT_SYMBOL(rpmh_ctrlr_idle);
diff --git a/include/soc/qcom/rpmh.h b/include/soc/qcom/rpmh.h
index 619e07c..d445322 100644
--- a/include/soc/qcom/rpmh.h
+++ b/include/soc/qcom/rpmh.h
@@ -24,6 +24,8 @@ int rpmh_write_batch(const struct device *dev, enum rpmh_state state,

int rpmh_invalidate(const struct device *dev);

+int rpmh_ctrlr_idle(const struct device *dev);
+
#else

static inline int rpmh_write(const struct device *dev, enum rpmh_state state,
@@ -46,6 +48,9 @@ static inline int rpmh_flush(const struct device *dev)
static inline int rpmh_invalidate(const struct device *dev)
{ return -ENODEV; }

+static inline int rpmh_ctrlr_idle(const struct device *dev)
+{ return -ENODEV; }
+
#endif /* CONFIG_QCOM_RPMH */

#endif /* __SOC_QCOM_RPMH_H__ */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project

2018-07-27 10:06:49

[permalink] [raw]

Subject: [PATCH v2 5/6] drivers: qcom: rpmh-rsc: write PDC data

From: Lina Iyer <[email protected]>

The Power Domain Controller can be programmed to wakeup the RSC and
setup the resources back in the active state, before the processor is
woken up by a timer interrupt. The wakeup value from the timer hardware
can be copied to the PDC which has its own timer and is in an always-on
power domain. Programming the wakeup value is done through a separate
register on the RSC.

Signed-off-by: Lina Iyer <[email protected]>
Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
---
Changes in v2:
- Remove unnecessary EXPORT_SYMBOL
---
drivers/soc/qcom/rpmh-internal.h | 3 +++
drivers/soc/qcom/rpmh-rsc.c | 35 +++++++++++++++++++++++++++++------
2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index 6cd2f78..f5359be 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -87,6 +87,7 @@ struct rpmh_ctrlr {
* Resource State Coordinator controller (RSC)
*
* @name: controller identifier
+ * @base: start address of the RSC's DRV registers
* @tcs_base: start address of the TCS registers in this controller
* @id: instance id in the controller (Direct Resource Voter)
* @num_tcs: number of TCSes in this DRV
@@ -97,6 +98,7 @@ struct rpmh_ctrlr {
*/
struct rsc_drv {
const char *name;
+ void __iomem *base;
void __iomem *tcs_base;
int id;
int num_tcs;
@@ -111,6 +113,7 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv,
const struct tcs_request *msg);
int rpmh_rsc_invalidate(struct rsc_drv *drv);
bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv);
+int rpmh_rsc_write_pdc_data(struct rsc_drv *drv, const struct tcs_request *msg);

void rpmh_tx_done(const struct tcs_request *msg, int r);

diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index 19616c9..c870477 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -60,6 +60,11 @@
#define CMD_STATUS_ISSUED BIT(8)
#define CMD_STATUS_COMPL BIT(16)

+/* PDC wakeup */
+#define RSC_PDC_DATA_SIZE 2
+#define RSC_PDC_DRV_DATA 0x38
+#define RSC_PDC_DATA_OFFSET 0x08
+
static u32 read_tcs_reg(struct rsc_drv *drv, int reg, int tcs_id, int cmd_id)
{
return readl_relaxed(drv->tcs_base + reg + RSC_DRV_TCS_OFFSET * tcs_id +
@@ -569,6 +574,25 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg)
return tcs_ctrl_write(drv, msg);
}

+int rpmh_rsc_write_pdc_data(struct rsc_drv *drv, const struct tcs_request *msg)
+{
+ int i;
+ void __iomem *addr = drv->base + RSC_PDC_DRV_DATA;
+
+ if (!msg || !msg->cmds || msg->num_cmds != RSC_PDC_DATA_SIZE)
+ return -EINVAL;
+
+ for (i = 0; i < msg->num_cmds; i++) {
+ /* Only data is write capable */
+ writel_relaxed(msg->cmds[i].data, addr);
+ trace_rpmh_send_msg(drv, RSC_PDC_DRV_DATA, i, 0,
+ &msg->cmds[i]);
+ addr += RSC_PDC_DATA_OFFSET;
+ }
+
+ return 0;
+}
+
static int rpmh_probe_tcs_config(struct platform_device *pdev,
struct rsc_drv *drv)
{
@@ -581,21 +605,20 @@ static int rpmh_probe_tcs_config(struct platform_device *pdev,
int i, ret, n, st = 0;
struct tcs_group *tcs;
struct resource *res;
- void __iomem *base;
char drv_id[10] = {0};

snprintf(drv_id, ARRAY_SIZE(drv_id), "drv-%d", drv->id);
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, drv_id);
- base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(base))
- return PTR_ERR(base);
+ drv->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(drv->base))
+ return PTR_ERR(drv->base);

ret = of_property_read_u32(dn, "qcom,tcs-offset", &offset);
if (ret)
return ret;
- drv->tcs_base = base + offset;
+ drv->tcs_base = drv->base + offset;

- config = readl_relaxed(base + DRV_PRNT_CHLD_CONFIG);
+ config = readl_relaxed(drv->base + DRV_PRNT_CHLD_CONFIG);

max_tcs = config;
max_tcs &= DRV_NUM_TCS_MASK << (DRV_NUM_TCS_SHIFT * drv->id);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project

2018-07-27 10:06:52

[permalink] [raw]

Subject: [PATCH v2 6/6] drivers: qcom: rpmh: write PDC data

From: Lina Iyer <[email protected]>

In addition to requests that are send to the remote processor, the
controller may allow certain data to be written to the controller for
use in specific cases like wakeup value when entering idle states.
Allow a pass through to write PDC data.

Signed-off-by: Lina Iyer <[email protected]>
Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
---
drivers/soc/qcom/rpmh.c | 28 ++++++++++++++++++++++++++++
include/soc/qcom/rpmh.h | 6 ++++++
2 files changed, 34 insertions(+)

diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index 0d276fd..f81488b 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -472,6 +472,34 @@ int rpmh_write_batch(const struct device *dev, enum rpmh_state state,
}
EXPORT_SYMBOL(rpmh_write_batch);

+/**
+ * rpmh_write_pdc_data: Write PDC data to the controller
+ *
+ * @dev: the device making the request
+ * @cmd: The payload data
+ * @n: The number of elements in payload
+ *
+ * Write PDC data to the controller. The messages are always sent async.
+ *
+ * May be called from atomic contexts.
+ */
+int rpmh_write_pdc_data(const struct device *dev,
+ const struct tcs_cmd *cmd, u32 n)
+{
+ DEFINE_RPMH_MSG_ONSTACK(dev, 0, NULL, rpm_msg);
+ struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
+
+ if (!n || n > MAX_RPMH_PAYLOAD)
+ return -EINVAL;
+
+ memcpy(rpm_msg.cmd, cmd, n * sizeof(*cmd));
+ rpm_msg.msg.num_cmds = n;
+ rpm_msg.msg.wait_for_compl = false;
+
+ return rpmh_rsc_write_pdc_data(ctrlr_to_drv(ctrlr), &rpm_msg.msg);
+}
+EXPORT_SYMBOL(rpmh_write_pdc_data);
+
static int is_req_valid(struct cache_req *req)
{
return (req->sleep_val != UINT_MAX &&
diff --git a/include/soc/qcom/rpmh.h b/include/soc/qcom/rpmh.h
index 018788d..d5e736e 100644
--- a/include/soc/qcom/rpmh.h
+++ b/include/soc/qcom/rpmh.h
@@ -28,6 +28,9 @@ int rpmh_write_batch(const struct device *dev, enum rpmh_state state,

int rpmh_mode_solver_set(const struct device *dev, bool enable);

+int rpmh_write_pdc_data(const struct device *dev,
+ const struct tcs_cmd *cmd, u32 n);
+
#else

static inline int rpmh_write(const struct device *dev, enum rpmh_state state,
@@ -56,6 +59,9 @@ static inline int rpmh_ctrlr_idle(const struct device *dev)
static inline int rpmh_mode_solver_set(const struct device *dev, bool enable)
{ return -ENODEV; }

+static inline int rpmh_write_pdc_data(const struct device *dev,
+ const struct tcs_cmd *cmd, u32 n)
+{ return -ENODEV; }
#endif /* CONFIG_QCOM_RPMH */

#endif /* __SOC_QCOM_RPMH_H__ */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project

2018-07-27 10:07:15

[permalink] [raw]

Subject: [PATCH v2 1/6] drivers: qcom: rpmh-rsc: return if the controller is idle

From: Lina Iyer <[email protected]>

Allow the controller status be queried. The controller is busy if it is
actively processing request.

Signed-off-by: Lina Iyer <[email protected]>
Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
---
Changes in v2:
- Remove unnecessary EXPORT_SYMBOL
---
drivers/soc/qcom/rpmh-internal.h | 1 +
drivers/soc/qcom/rpmh-rsc.c | 20 ++++++++++++++++++++
2 files changed, 21 insertions(+)

diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index a7bbbb6..4ff43bf 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -108,6 +108,7 @@ struct rsc_drv {
int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv,
const struct tcs_request *msg);
int rpmh_rsc_invalidate(struct rsc_drv *drv);
+bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv);

void rpmh_tx_done(const struct tcs_request *msg, int r);

diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index 33fe9f9..42d0041 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -496,6 +496,26 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg)
}

/**
+ * rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy.
+ *
+ * @drv: The controller
+ *
+ * Returns true if the TCSes are engaged in handling requests.
+ */
+bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv)
+{
+ int m;
+ struct tcs_group *tcs = get_tcs_of_type(drv, ACTIVE_TCS);
+
+ for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
+ if (!tcs_is_free(drv, m))
+ return false;
+ }
+
+ return true;
+}
+
+/**
* rpmh_rsc_write_ctrl_data: Write request to the controller
*
* @drv: the controller
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project

2018-07-27 10:07:18

[permalink] [raw]

Subject: [PATCH v2 3/6] drivers: qcom: rpmh: disallow active requests in solver mode

From: Lina Iyer <[email protected]>

Controllers may be in 'solver' state, where they could be in autonomous
mode executing low power modes for their hardware and as such are not
available for sending active votes. Device driver may notify RPMH API
that the controller is in solver mode and when in such mode, disallow
requests from platform drivers for state change using the RSC.

Signed-off-by: Lina Iyer <[email protected]>
Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
---
drivers/soc/qcom/rpmh-internal.h | 2 ++
drivers/soc/qcom/rpmh.c | 59 ++++++++++++++++++++++++++++++++++++++++
include/soc/qcom/rpmh.h | 5 ++++
3 files changed, 66 insertions(+)

diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index 4ff43bf..6cd2f78 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -72,12 +72,14 @@ struct rpmh_request {
* @cache_lock: synchronize access to the cache data
* @dirty: was the cache updated since flush
* @batch_cache: Cache sleep and wake requests sent as batch
+ * @in_solver_mode: Controller is busy in solver mode
*/
struct rpmh_ctrlr {
struct list_head cache;
spinlock_t cache_lock;
bool dirty;
struct list_head batch_cache;
+ bool in_solver_mode;
};

/**
diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index 2382276..0d276fd 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -5,6 +5,7 @@

#include <linux/atomic.h>
#include <linux/bug.h>
+#include <linux/delay.h>
#include <linux/interrupt.h>
#include <linux/jiffies.h>
#include <linux/kernel.h>
@@ -75,6 +76,50 @@ static struct rpmh_ctrlr *get_rpmh_ctrlr(const struct device *dev)
return &drv->client;
}

+static int check_ctrlr_state(struct rpmh_ctrlr *ctrlr, enum rpmh_state state)
+{
+ unsigned long flags;
+ int ret = 0;
+
+ /* Do not allow setting active votes when in solver mode */
+ spin_lock_irqsave(&ctrlr->cache_lock, flags);
+ if (ctrlr->in_solver_mode && state == RPMH_ACTIVE_ONLY_STATE)
+ ret = -EBUSY;
+ spin_unlock_irqrestore(&ctrlr->cache_lock, flags);
+
+ return ret;
+}
+
+/**
+ * rpmh_mode_solver_set: Indicate that the RSC controller hardware has
+ * been configured to be in solver mode
+ *
+ * @dev: the device making the request
+ * @enable: Boolean value indicating if the controller is in solver mode.
+ *
+ * When solver mode is enabled, passthru API will not be able to send wake
+ * votes, just awake and active votes.
+ */
+int rpmh_mode_solver_set(const struct device *dev, bool enable)
+{
+ struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
+ unsigned long flags;
+
+ for (;;) {
+ spin_lock_irqsave(&ctrlr->cache_lock, flags);
+ if (rpmh_rsc_ctrlr_is_idle(ctrlr_to_drv(ctrlr))) {
+ ctrlr->in_solver_mode = enable;
+ spin_unlock_irqrestore(&ctrlr->cache_lock, flags);
+ break;
+ }
+ spin_unlock_irqrestore(&ctrlr->cache_lock, flags);
+ udelay(10);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(rpmh_mode_solver_set);
+
void rpmh_tx_done(const struct tcs_request *msg, int r)
{
struct rpmh_request *rpm_msg = container_of(msg, struct rpmh_request,
@@ -231,8 +276,13 @@ int rpmh_write_async(const struct device *dev, enum rpmh_state state,
const struct tcs_cmd *cmd, u32 n)
{
struct rpmh_request *rpm_msg;
+ struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
int ret;

+ ret = check_ctrlr_state(ctrlr, state);
+ if (ret)
+ return ret;
+
rpm_msg = kzalloc(sizeof(*rpm_msg), GFP_ATOMIC);
if (!rpm_msg)
return -ENOMEM;
@@ -263,11 +313,16 @@ int rpmh_write(const struct device *dev, enum rpmh_state state,
{
DECLARE_COMPLETION_ONSTACK(compl);
DEFINE_RPMH_MSG_ONSTACK(dev, state, &compl, rpm_msg);
+ struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
int ret;

if (!cmd || !n || n > MAX_RPMH_PAYLOAD)
return -EINVAL;

+ ret = check_ctrlr_state(ctrlr, state);
+ if (ret)
+ return ret;
+
memcpy(rpm_msg.cmd, cmd, n * sizeof(*cmd));
rpm_msg.msg.num_cmds = n;

@@ -357,6 +412,10 @@ int rpmh_write_batch(const struct device *dev, enum rpmh_state state,
if (!cmd || !n)
return -EINVAL;

+ ret = check_ctrlr_state(ctrlr, state);
+ if (ret)
+ return ret;
+
while (n[count] > 0)
count++;
if (!count)
diff --git a/include/soc/qcom/rpmh.h b/include/soc/qcom/rpmh.h
index d445322..018788d 100644
--- a/include/soc/qcom/rpmh.h
+++ b/include/soc/qcom/rpmh.h
@@ -26,6 +26,8 @@ int rpmh_write_batch(const struct device *dev, enum rpmh_state state,

int rpmh_ctrlr_idle(const struct device *dev);

+int rpmh_mode_solver_set(const struct device *dev, bool enable);
+
#else

static inline int rpmh_write(const struct device *dev, enum rpmh_state state,
@@ -51,6 +53,9 @@ static inline int rpmh_invalidate(const struct device *dev)
static inline int rpmh_ctrlr_idle(const struct device *dev)
{ return -ENODEV; }

+static inline int rpmh_mode_solver_set(const struct device *dev, bool enable)
+{ return -ENODEV; }
+
#endif /* CONFIG_QCOM_RPMH */

#endif /* __SOC_QCOM_RPMH_H__ */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project

2018-07-27 10:07:57

[permalink] [raw]

Subject: [PATCH v2 4/6] drivers: qcom: rpmh-rsc: clear active mode configuration for waketcs

From: "Raju P.L.S.S.S.N" <[email protected]>

For RSCs that have sleep & wake TCS but no dedicated active TCS, wake
TCS can be re-purposed to send active requests. Once the active requests
are sent and response is received, the active mode configuration needs
to be cleared so that controller can use wake TCS for sending wake
requests in 'solver' state while executing low power modes.

Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
---
drivers/soc/qcom/rpmh-rsc.c | 77 +++++++++++++++++++++++++++++++--------------
1 file changed, 54 insertions(+), 23 deletions(-)

diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index 42d0041..19616c9 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -199,6 +199,42 @@ static const struct tcs_request *get_req_from_tcs(struct rsc_drv *drv,
return NULL;
}

+static void __tcs_trigger(struct rsc_drv *drv, int tcs_id, bool trigger)
+{
+ 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, tcs_id, 0);
+ enable &= ~TCS_AMC_MODE_TRIGGER;
+ write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
+ enable &= ~TCS_AMC_MODE_ENABLE;
+ write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
+
+ if (trigger) {
+ /* 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, tcs_id, enable);
+ enable |= TCS_AMC_MODE_TRIGGER;
+ write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
+ }
+}
+
+static inline void enable_tcs_irq(struct rsc_drv *drv, int tcs_id, bool enable)
+{
+ u32 data;
+
+ data = read_tcs_reg(drv, RSC_DRV_IRQ_ENABLE, 0, 0);
+ if (enable)
+ data |= BIT(tcs_id);
+ else
+ data &= ~BIT(tcs_id);
+ write_tcs_reg(drv, RSC_DRV_IRQ_ENABLE, 0, data);
+}
+
/**
* tcs_tx_done: TX Done interrupt handler
*/
@@ -235,6 +271,21 @@ static irqreturn_t tcs_tx_done(int irq, void *p)
}

trace_rpmh_tx_done(drv, i, req, err);
+
+ /*
+ * if wake tcs was re-purposed for sending active
+ * votes, clear AMC trigger & enable modes and
+ * disable interrupt for this TCS
+ */
+ if (!drv->tcs[ACTIVE_TCS].num_tcs) {
+ __tcs_trigger(drv, i, false);
+ /*
+ * Disable interrupt for this TCS to avoid being
+ * spammed with interrupts coming when the solver
+ * sends its wake votes.
+ */
+ enable_tcs_irq(drv, i, false);
+ }
skip:
/* Reclaim the TCS */
write_tcs_reg(drv, RSC_DRV_CMD_ENABLE, i, 0);
@@ -282,28 +333,6 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id,
write_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id, cmd_enable);
}

-static void __tcs_trigger(struct rsc_drv *drv, int tcs_id)
-{
- 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, tcs_id, 0);
- enable &= ~TCS_AMC_MODE_TRIGGER;
- write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
- enable &= ~TCS_AMC_MODE_ENABLE;
- write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, 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, tcs_id, enable);
- enable |= TCS_AMC_MODE_TRIGGER;
- write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
-}
-
static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs,
const struct tcs_request *msg)
{
@@ -374,10 +403,12 @@ static int tcs_write(struct rsc_drv *drv, const struct tcs_request *msg)

tcs->req[tcs_id - tcs->offset] = msg;
set_bit(tcs_id, drv->tcs_in_use);
+ if (msg->state == RPMH_ACTIVE_ONLY_STATE && tcs->type != ACTIVE_TCS)
+ enable_tcs_irq(drv, tcs_id, true);
spin_unlock(&drv->lock);

__tcs_buffer_write(drv, tcs_id, 0, msg);
- __tcs_trigger(drv, tcs_id);
+ __tcs_trigger(drv, tcs_id, true);

done_write:
spin_unlock_irqrestore(&tcs->lock, flags);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project

2018-09-05 20:08:59

[permalink] [raw]

Subject: Re: [PATCH v2 0/6] drivers/qcom: add additional functionality to RPMH

Stephen, Doug,

Would you have some time to take a look at this series?

Thanks,
Lina

On Fri, Jul 27 2018 at 04:04 -0600, Raju P L S S S N wrote:
>From: "Raju P.L.S.S.S.N" <[email protected]>
>
>Changes in v1:
>- Remove unnecessary EXPORT_SYMBOL in rpmh-rsc
>
>This set of patches add additional functionality to RPMH drivers[1].
>
>PM drivers can choose to disallow idle modes when RSC controller is busy
>sending or processing requests. The patches add necessary functions to
>query the controller status.
>
>The controllers may be in 'solver' state, where they could be in autonomous
>mode executing low power modes for their hardware and as such are not
>available for sending active votes. Functionality to get notified about
>such state and disallow requests for state change in that case is added
>in these patches.
>
>The Power Domain Controller can be programmed to wakeup the RSC and
>setup the resources back in the active state, before the processor is
>woken up by a timer interrupt. The wakeup value from the timer hardware
>can be copied to the PDC which has its own timer and is in an always-on
>power domain. Programming the wakeup value is done through a separate
>register on the RSC. Functions necessary to program wakeup is added in
>the patches.
>
>Please consider reviewing this patchset.
>
>v1:https://lkml.org/lkml/2018/7/19/213
>
>
>
>[1] https://lkml.org/lkml/2018/6/20/519
>
>Lina Iyer (5):
> drivers: qcom: rpmh-rsc: return if the controller is idle
> drivers: qcom: rpmh: export controller idle status
> drivers: qcom: rpmh: disallow active requests in solver mode
> drivers: qcom: rpmh-rsc: write PDC data
> drivers: qcom: rpmh: write PDC data
>
>Raju P.L.S.S.S.N (1):
> drivers: qcom: rpmh-rsc: clear active mode configuration for waketcs
>
> drivers/soc/qcom/rpmh-internal.h | 6 ++
> drivers/soc/qcom/rpmh-rsc.c | 132 ++++++++++++++++++++++++++++++---------
> drivers/soc/qcom/rpmh.c | 100 +++++++++++++++++++++++++++++
> include/soc/qcom/rpmh.h | 16 +++++
> 4 files changed, 225 insertions(+), 29 deletions(-)
>
>--
>The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
>

2018-09-11 22:39:39

[permalink] [raw]

Subject: Re: [PATCH v2 1/6] drivers: qcom: rpmh-rsc: return if the controller is idle

Hi Raju/Lina,

On Fri, Jul 27, 2018 at 03:34:44PM +0530, Raju P L S S S N wrote:
> From: Lina Iyer <[email protected]>
>
> Allow the controller status be queried. The controller is busy if it is
> actively processing request.
>
> Signed-off-by: Lina Iyer <[email protected]>
> Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
> ---
> Changes in v2:
> - Remove unnecessary EXPORT_SYMBOL
> ---
> drivers/soc/qcom/rpmh-internal.h | 1 +
> drivers/soc/qcom/rpmh-rsc.c | 20 ++++++++++++++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
> index a7bbbb6..4ff43bf 100644
> --- a/drivers/soc/qcom/rpmh-internal.h
> +++ b/drivers/soc/qcom/rpmh-internal.h
> @@ -108,6 +108,7 @@ struct rsc_drv {
> int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv,
> const struct tcs_request *msg);
> int rpmh_rsc_invalidate(struct rsc_drv *drv);
> +bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv);
>
> void rpmh_tx_done(const struct tcs_request *msg, int r);
>
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index 33fe9f9..42d0041 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -496,6 +496,26 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg)
> }
>
> /**
> + * rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy.
> + *
> + * @drv: The controller
> + *
> + * Returns true if the TCSes are engaged in handling requests.
> + */
> +bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv)
> +{
> + int m;
> + struct tcs_group *tcs = get_tcs_of_type(drv, ACTIVE_TCS);
> +
> + for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
> + if (!tcs_is_free(drv, m))
> + return false;
> + }
> +
> + return true;
> +}

This looks racy, tcs_write() could be running simultaneously and use
TCSes that were seen as free by _is_idle(). This could be fixed by
holding tcs->lock (assuming this doesn't cause lock ordering problems).
However even with this tcs_write() could run right after releasing the
lock, using TCSes and the caller of _is_idle() would consider the
controller to be idle.

2018-09-11 23:03:58

[permalink] [raw]

Subject: Re: [PATCH v2 3/6] drivers: qcom: rpmh: disallow active requests in solver mode

Hi Raju/Lina,

On Fri, Jul 27, 2018 at 03:34:46PM +0530, Raju P L S S S N wrote:
> From: Lina Iyer <[email protected]>
>
> Controllers may be in 'solver' state, where they could be in autonomous
> mode executing low power modes for their hardware and as such are not
> available for sending active votes. Device driver may notify RPMH API
> that the controller is in solver mode and when in such mode, disallow
> requests from platform drivers for state change using the RSC.
>
> Signed-off-by: Lina Iyer <[email protected]>
> Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
> ---
> drivers/soc/qcom/rpmh-internal.h | 2 ++
> drivers/soc/qcom/rpmh.c | 59 ++++++++++++++++++++++++++++++++++++++++
> include/soc/qcom/rpmh.h | 5 ++++
> 3 files changed, 66 insertions(+)
>
> diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
> index 4ff43bf..6cd2f78 100644
> --- a/drivers/soc/qcom/rpmh-internal.h
> +++ b/drivers/soc/qcom/rpmh-internal.h
> @@ -72,12 +72,14 @@ struct rpmh_request {
> * @cache_lock: synchronize access to the cache data
> * @dirty: was the cache updated since flush
> * @batch_cache: Cache sleep and wake requests sent as batch
> + * @in_solver_mode: Controller is busy in solver mode
> */
> struct rpmh_ctrlr {
> struct list_head cache;
> spinlock_t cache_lock;
> bool dirty;
> struct list_head batch_cache;
> + bool in_solver_mode;
> };
>
> /**
> diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
> index 2382276..0d276fd 100644
> --- a/drivers/soc/qcom/rpmh.c
> +++ b/drivers/soc/qcom/rpmh.c
> @@ -5,6 +5,7 @@
>
> #include <linux/atomic.h>
> #include <linux/bug.h>
> +#include <linux/delay.h>
> #include <linux/interrupt.h>
> #include <linux/jiffies.h>
> #include <linux/kernel.h>
> @@ -75,6 +76,50 @@ static struct rpmh_ctrlr *get_rpmh_ctrlr(const struct device *dev)
> return &drv->client;
> }
>
> +static int check_ctrlr_state(struct rpmh_ctrlr *ctrlr, enum rpmh_state state)
> +{
> + unsigned long flags;
> + int ret = 0;
> +
> + /* Do not allow setting active votes when in solver mode */
> + spin_lock_irqsave(&ctrlr->cache_lock, flags);
> + if (ctrlr->in_solver_mode && state == RPMH_ACTIVE_ONLY_STATE)
> + ret = -EBUSY;
> + spin_unlock_irqrestore(&ctrlr->cache_lock, flags);
> +
> + return ret;
> +}
> +
> +/**
> + * rpmh_mode_solver_set: Indicate that the RSC controller hardware has
> + * been configured to be in solver mode
> + *
> + * @dev: the device making the request
> + * @enable: Boolean value indicating if the controller is in solver mode.
> + *
> + * When solver mode is enabled, passthru API will not be able to send wake
> + * votes, just awake and active votes.
> + */
> +int rpmh_mode_solver_set(const struct device *dev, bool enable)
> +{
> + struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
> + unsigned long flags;
> +
> + for (;;) {
> + spin_lock_irqsave(&ctrlr->cache_lock, flags);
> + if (rpmh_rsc_ctrlr_is_idle(ctrlr_to_drv(ctrlr))) {
> + ctrlr->in_solver_mode = enable;

As commented on '[v2,1/6] drivers: qcom: rpmh-rsc: return if the
controller is idle', this seems potentially
racy. _is_idle() could report the controller as idle, even though some
TCSes are in use (after _is_idle() visited them).

Additional locking may be needed or a comment if this situation should
never happen on a sane system (I don't know enough about RPMh and its
clients to judge if this is the case).

2018-09-12 02:20:40

[permalink] [raw]

Subject: Re: [PATCH v2 1/6] drivers: qcom: rpmh-rsc: return if the controller is idle

On Tue, Sep 11 2018 at 16:39 -0600, Matthias Kaehlcke wrote:
>Hi Raju/Lina,
>
>On Fri, Jul 27, 2018 at 03:34:44PM +0530, Raju P L S S S N wrote:
>> From: Lina Iyer <[email protected]>
>>
>> Allow the controller status be queried. The controller is busy if it is
>> actively processing request.
>>
>> Signed-off-by: Lina Iyer <[email protected]>
>> Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
>> ---
>> Changes in v2:
>> - Remove unnecessary EXPORT_SYMBOL
>> ---
>> drivers/soc/qcom/rpmh-internal.h | 1 +
>> drivers/soc/qcom/rpmh-rsc.c | 20 ++++++++++++++++++++
>> 2 files changed, 21 insertions(+)
>>
>> diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
>> index a7bbbb6..4ff43bf 100644
>> --- a/drivers/soc/qcom/rpmh-internal.h
>> +++ b/drivers/soc/qcom/rpmh-internal.h
>> @@ -108,6 +108,7 @@ struct rsc_drv {
>> int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv,
>> const struct tcs_request *msg);
>> int rpmh_rsc_invalidate(struct rsc_drv *drv);
>> +bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv);
>>
>> void rpmh_tx_done(const struct tcs_request *msg, int r);
>>
>> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
>> index 33fe9f9..42d0041 100644
>> --- a/drivers/soc/qcom/rpmh-rsc.c
>> +++ b/drivers/soc/qcom/rpmh-rsc.c
>> @@ -496,6 +496,26 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg)
>> }
>>
>> /**
>> + * rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy.
>> + *
>> + * @drv: The controller
>> + *
>> + * Returns true if the TCSes are engaged in handling requests.
>> + */
>> +bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv)
>> +{
>> + int m;
>> + struct tcs_group *tcs = get_tcs_of_type(drv, ACTIVE_TCS);
>> +
>> + for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
>> + if (!tcs_is_free(drv, m))
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>
>This looks racy, tcs_write() could be running simultaneously and use
>TCSes that were seen as free by _is_idle(). This could be fixed by
>holding tcs->lock (assuming this doesn't cause lock ordering problems).
>However even with this tcs_write() could run right after releasing the
>lock, using TCSes and the caller of _is_idle() would consider the
>controller to be idle.

We could run this without the lock, since we are only reading a status.
Generally, this function is called from the idle code of the last CPU
and no CPU or active TCS request should be in progress, but if it were,
then this function would let the caller know we are not ready to do
idle. If there were no requests that were running at that time we read
the registers, we would not be making one after, since we are already
in the idle code and no requests are made there.

I understand, how it might appear racy, the context of the callling
function helps resolve that.

-- Lina

2018-09-12 02:23:57

[permalink] [raw]

Subject: Re: [PATCH v2 3/6] drivers: qcom: rpmh: disallow active requests in solver mode

On Tue, Sep 11 2018 at 17:02 -0600, Matthias Kaehlcke wrote:
>Hi Raju/Lina,
>
>On Fri, Jul 27, 2018 at 03:34:46PM +0530, Raju P L S S S N wrote:
>> From: Lina Iyer <[email protected]>
>>
>> Controllers may be in 'solver' state, where they could be in autonomous
>> mode executing low power modes for their hardware and as such are not
>> available for sending active votes. Device driver may notify RPMH API
>> that the controller is in solver mode and when in such mode, disallow
>> requests from platform drivers for state change using the RSC.
>>
>> Signed-off-by: Lina Iyer <[email protected]>
>> Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
>> ---
>> drivers/soc/qcom/rpmh-internal.h | 2 ++
>> drivers/soc/qcom/rpmh.c | 59 ++++++++++++++++++++++++++++++++++++++++
>> include/soc/qcom/rpmh.h | 5 ++++
>> 3 files changed, 66 insertions(+)
>>
>> diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
>> index 4ff43bf..6cd2f78 100644
>> --- a/drivers/soc/qcom/rpmh-internal.h
>> +++ b/drivers/soc/qcom/rpmh-internal.h
>> @@ -72,12 +72,14 @@ struct rpmh_request {
>> * @cache_lock: synchronize access to the cache data
>> * @dirty: was the cache updated since flush
>> * @batch_cache: Cache sleep and wake requests sent as batch
>> + * @in_solver_mode: Controller is busy in solver mode
>> */
>> struct rpmh_ctrlr {
>> struct list_head cache;
>> spinlock_t cache_lock;
>> bool dirty;
>> struct list_head batch_cache;
>> + bool in_solver_mode;
>> };
>>
>> /**
>> diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
>> index 2382276..0d276fd 100644
>> --- a/drivers/soc/qcom/rpmh.c
>> +++ b/drivers/soc/qcom/rpmh.c
>> @@ -5,6 +5,7 @@
>>
>> #include <linux/atomic.h>
>> #include <linux/bug.h>
>> +#include <linux/delay.h>
>> #include <linux/interrupt.h>
>> #include <linux/jiffies.h>
>> #include <linux/kernel.h>
>> @@ -75,6 +76,50 @@ static struct rpmh_ctrlr *get_rpmh_ctrlr(const struct device *dev)
>> return &drv->client;
>> }
>>
>> +static int check_ctrlr_state(struct rpmh_ctrlr *ctrlr, enum rpmh_state state)
>> +{
>> + unsigned long flags;
>> + int ret = 0;
>> +
>> + /* Do not allow setting active votes when in solver mode */
>> + spin_lock_irqsave(&ctrlr->cache_lock, flags);
>> + if (ctrlr->in_solver_mode && state == RPMH_ACTIVE_ONLY_STATE)
>> + ret = -EBUSY;
>> + spin_unlock_irqrestore(&ctrlr->cache_lock, flags);
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * rpmh_mode_solver_set: Indicate that the RSC controller hardware has
>> + * been configured to be in solver mode
>> + *
>> + * @dev: the device making the request
>> + * @enable: Boolean value indicating if the controller is in solver mode.
>> + *
>> + * When solver mode is enabled, passthru API will not be able to send wake
>> + * votes, just awake and active votes.
>> + */
>> +int rpmh_mode_solver_set(const struct device *dev, bool enable)
>> +{
>> + struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
>> + unsigned long flags;
>> +
>> + for (;;) {
>> + spin_lock_irqsave(&ctrlr->cache_lock, flags);
>> + if (rpmh_rsc_ctrlr_is_idle(ctrlr_to_drv(ctrlr))) {
>> + ctrlr->in_solver_mode = enable;
>
>As commented on '[v2,1/6] drivers: qcom: rpmh-rsc: return if the
>controller is idle', this seems potentially
>racy. _is_idle() could report the controller as idle, even though some
>TCSes are in use (after _is_idle() visited them).
>
>Additional locking may be needed or a comment if this situation should
>never happen on a sane system (I don't know enough about RPMh and its
>clients to judge if this is the case).
Hmm.. Forgot that we call from here. May be a lock might be helpful.

-- Lina

2018-09-12 21:51:55

[permalink] [raw]

Subject: Re: [PATCH v2 4/6] drivers: qcom: rpmh-rsc: clear active mode configuration for waketcs

On Fri, Jul 27, 2018 at 03:34:47PM +0530, Raju P L S S S N wrote:
> From: "Raju P.L.S.S.S.N" <[email protected]>
>
> For RSCs that have sleep & wake TCS but no dedicated active TCS, wake
> TCS can be re-purposed to send active requests. Once the active requests
> are sent and response is received, the active mode configuration needs
> to be cleared so that controller can use wake TCS for sending wake
> requests in 'solver' state while executing low power modes.
>
> Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
> ---
> drivers/soc/qcom/rpmh-rsc.c | 77 +++++++++++++++++++++++++++++++--------------
> 1 file changed, 54 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index 42d0041..19616c9 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -199,6 +199,42 @@ static const struct tcs_request *get_req_from_tcs(struct rsc_drv *drv,
> return NULL;
> }
>
> +static void __tcs_trigger(struct rsc_drv *drv, int tcs_id, bool trigger)
> +{
> + 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, tcs_id, 0);
> + enable &= ~TCS_AMC_MODE_TRIGGER;
> + write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
> + enable &= ~TCS_AMC_MODE_ENABLE;
> + write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
> +
> + if (trigger) {
> + /* 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, tcs_id, enable);
> + enable |= TCS_AMC_MODE_TRIGGER;
> + write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
> + }
> +}
> +
> +static inline void enable_tcs_irq(struct rsc_drv *drv, int tcs_id, bool enable)
> +{
> + u32 data;
> +
> + data = read_tcs_reg(drv, RSC_DRV_IRQ_ENABLE, 0, 0);
> + if (enable)
> + data |= BIT(tcs_id);
> + else
> + data &= ~BIT(tcs_id);
> + write_tcs_reg(drv, RSC_DRV_IRQ_ENABLE, 0, data);
> +}
> +
> /**
> * tcs_tx_done: TX Done interrupt handler
> */
> @@ -235,6 +271,21 @@ static irqreturn_t tcs_tx_done(int irq, void *p)
> }
>
> trace_rpmh_tx_done(drv, i, req, err);
> +
> + /*
> + * if wake tcs was re-purposed for sending active
> + * votes, clear AMC trigger & enable modes and
> + * disable interrupt for this TCS
> + */
> + if (!drv->tcs[ACTIVE_TCS].num_tcs) {
> + __tcs_trigger(drv, i, false);
> + /*
> + * Disable interrupt for this TCS to avoid being
> + * spammed with interrupts coming when the solver
> + * sends its wake votes.
> + */
> + enable_tcs_irq(drv, i, false);
> + }
> skip:
> /* Reclaim the TCS */
> write_tcs_reg(drv, RSC_DRV_CMD_ENABLE, i, 0);
> @@ -282,28 +333,6 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id,
> write_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id, cmd_enable);
> }
>
> -static void __tcs_trigger(struct rsc_drv *drv, int tcs_id)
> -{
> - 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, tcs_id, 0);
> - enable &= ~TCS_AMC_MODE_TRIGGER;
> - write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
> - enable &= ~TCS_AMC_MODE_ENABLE;
> - write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, 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, tcs_id, enable);
> - enable |= TCS_AMC_MODE_TRIGGER;
> - write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
> -}
> -
> static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs,
> const struct tcs_request *msg)
> {
> @@ -374,10 +403,12 @@ static int tcs_write(struct rsc_drv *drv, const struct tcs_request *msg)
>
> tcs->req[tcs_id - tcs->offset] = msg;
> set_bit(tcs_id, drv->tcs_in_use);
> + if (msg->state == RPMH_ACTIVE_ONLY_STATE && tcs->type != ACTIVE_TCS)
> + enable_tcs_irq(drv, tcs_id, true);

You might want to add a short comment here. In the context of this
patch it is (relatively) clear that the wake TCS is re-purposed for
active requests, and the interrupt is disabled by default for
non-active TCSes. However on it's own this isn't necessarily
evident without looking through the code.

> spin_unlock(&drv->lock);
>
> __tcs_buffer_write(drv, tcs_id, 0, msg);
> - __tcs_trigger(drv, tcs_id);
> + __tcs_trigger(drv, tcs_id, true);
>
> done_write:
> spin_unlock_irqrestore(&tcs->lock, flags);

Reviewed-by: Matthias Kaehlcke <[email protected]>

2018-09-12 22:29:50

[permalink] [raw]

Subject: Re: [PATCH v2 5/6] drivers: qcom: rpmh-rsc: write PDC data

On Fri, Jul 27, 2018 at 03:34:48PM +0530, Raju P L S S S N wrote:
> From: Lina Iyer <[email protected]>
>
> The Power Domain Controller can be programmed to wakeup the RSC and
> setup the resources back in the active state, before the processor is
> woken up by a timer interrupt. The wakeup value from the timer hardware
> can be copied to the PDC which has its own timer and is in an always-on
> power domain. Programming the wakeup value is done through a separate
> register on the RSC.
>
> Signed-off-by: Lina Iyer <[email protected]>
> Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
> ---
> Changes in v2:
> - Remove unnecessary EXPORT_SYMBOL
> ---
> drivers/soc/qcom/rpmh-internal.h | 3 +++
> drivers/soc/qcom/rpmh-rsc.c | 35 +++++++++++++++++++++++++++++------
> 2 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
> index 6cd2f78..f5359be 100644
> --- a/drivers/soc/qcom/rpmh-internal.h
> +++ b/drivers/soc/qcom/rpmh-internal.h
> @@ -87,6 +87,7 @@ struct rpmh_ctrlr {
> * Resource State Coordinator controller (RSC)
> *
> * @name: controller identifier
> + * @base: start address of the RSC's DRV registers
> * @tcs_base: start address of the TCS registers in this controller
> * @id: instance id in the controller (Direct Resource Voter)
> * @num_tcs: number of TCSes in this DRV
> @@ -97,6 +98,7 @@ struct rpmh_ctrlr {
> */
> struct rsc_drv {
> const char *name;
> + void __iomem *base;
> void __iomem *tcs_base;
> int id;
> int num_tcs;
> @@ -111,6 +113,7 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv,
> const struct tcs_request *msg);
> int rpmh_rsc_invalidate(struct rsc_drv *drv);
> bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv);
> +int rpmh_rsc_write_pdc_data(struct rsc_drv *drv, const struct tcs_request *msg);
>
> void rpmh_tx_done(const struct tcs_request *msg, int r);
>
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index 19616c9..c870477 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -60,6 +60,11 @@
> #define CMD_STATUS_ISSUED BIT(8)
> #define CMD_STATUS_COMPL BIT(16)
>
> +/* PDC wakeup */
> +#define RSC_PDC_DATA_SIZE 2
> +#define RSC_PDC_DRV_DATA 0x38
> +#define RSC_PDC_DATA_OFFSET 0x08
> +
> static u32 read_tcs_reg(struct rsc_drv *drv, int reg, int tcs_id, int cmd_id)
> {
> return readl_relaxed(drv->tcs_base + reg + RSC_DRV_TCS_OFFSET * tcs_id +
> @@ -569,6 +574,25 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg)
> return tcs_ctrl_write(drv, msg);
> }
>
> +int rpmh_rsc_write_pdc_data(struct rsc_drv *drv, const struct tcs_request *msg)
> +{
> + int i;
> + void __iomem *addr = drv->base + RSC_PDC_DRV_DATA;
> +
> + if (!msg || !msg->cmds || msg->num_cmds != RSC_PDC_DATA_SIZE)
> + return -EINVAL;

Is it really always exactly 2 (RSC_PDC_DATA_SIZE) commands?

> +
> + for (i = 0; i < msg->num_cmds; i++) {
> + /* Only data is write capable */
> + writel_relaxed(msg->cmds[i].data, addr);
> + trace_rpmh_send_msg(drv, RSC_PDC_DRV_DATA, i, 0,
> + &msg->cmds[i]);
> + addr += RSC_PDC_DATA_OFFSET;
> + }
> +
> + return 0;
> +}
> +
> static int rpmh_probe_tcs_config(struct platform_device *pdev,
> struct rsc_drv *drv)
> {
> @@ -581,21 +605,20 @@ static int rpmh_probe_tcs_config(struct platform_device *pdev,
> int i, ret, n, st = 0;
> struct tcs_group *tcs;
> struct resource *res;
> - void __iomem *base;
> char drv_id[10] = {0};
>
> snprintf(drv_id, ARRAY_SIZE(drv_id), "drv-%d", drv->id);
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, drv_id);
> - base = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(base))
> - return PTR_ERR(base);
> + drv->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(drv->base))
> + return PTR_ERR(drv->base);
>
> ret = of_property_read_u32(dn, "qcom,tcs-offset", &offset);
> if (ret)
> return ret;
> - drv->tcs_base = base + offset;
> + drv->tcs_base = drv->base + offset;
>
> - config = readl_relaxed(base + DRV_PRNT_CHLD_CONFIG);
> + config = readl_relaxed(drv->base + DRV_PRNT_CHLD_CONFIG);
>
> max_tcs = config;
> max_tcs &= DRV_NUM_TCS_MASK << (DRV_NUM_TCS_SHIFT * drv->id);

2018-09-12 22:34:20

[permalink] [raw]

Subject: Re: [PATCH v2 5/6] drivers: qcom: rpmh-rsc: write PDC data

On Wed, Sep 12 2018 at 16:28 -0600, Matthias Kaehlcke wrote:
>On Fri, Jul 27, 2018 at 03:34:48PM +0530, Raju P L S S S N wrote:
>> From: Lina Iyer <[email protected]>
>>
>> The Power Domain Controller can be programmed to wakeup the RSC and
>> setup the resources back in the active state, before the processor is
>> woken up by a timer interrupt. The wakeup value from the timer hardware
>> can be copied to the PDC which has its own timer and is in an always-on
>> power domain. Programming the wakeup value is done through a separate
>> register on the RSC.
>>
>> Signed-off-by: Lina Iyer <[email protected]>
>> Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
>> ---
>> Changes in v2:
>> - Remove unnecessary EXPORT_SYMBOL
>> ---
>
>> +int rpmh_rsc_write_pdc_data(struct rsc_drv *drv, const struct tcs_request *msg)
>> +{
>> + int i;
>> + void __iomem *addr = drv->base + RSC_PDC_DRV_DATA;
>> +
>> + if (!msg || !msg->cmds || msg->num_cmds != RSC_PDC_DATA_SIZE)
>> + return -EINVAL;
>
>Is it really always exactly 2 (RSC_PDC_DATA_SIZE) commands?
>
Yes, always.

2018-09-12 22:38:20

[permalink] [raw]

Subject: Re: [PATCH v2 5/6] drivers: qcom: rpmh-rsc: write PDC data

On Wed, Sep 12, 2018 at 04:33:56PM -0600, Lina Iyer wrote:
> On Wed, Sep 12 2018 at 16:28 -0600, Matthias Kaehlcke wrote:
> > On Fri, Jul 27, 2018 at 03:34:48PM +0530, Raju P L S S S N wrote:
> > > From: Lina Iyer <[email protected]>
> > >
> > > The Power Domain Controller can be programmed to wakeup the RSC and
> > > setup the resources back in the active state, before the processor is
> > > woken up by a timer interrupt. The wakeup value from the timer hardware
> > > can be copied to the PDC which has its own timer and is in an always-on
> > > power domain. Programming the wakeup value is done through a separate
> > > register on the RSC.
> > >
> > > Signed-off-by: Lina Iyer <[email protected]>
> > > Signed-off-by: Raju P.L.S.S.S.N <[email protected]>
> > > ---
> > > Changes in v2:
> > > - Remove unnecessary EXPORT_SYMBOL
> > > ---
> >
> > > +int rpmh_rsc_write_pdc_data(struct rsc_drv *drv, const struct tcs_request *msg)
> > > +{
> > > + int i;
> > > + void __iomem *addr = drv->base + RSC_PDC_DRV_DATA;
> > > +
> > > + if (!msg || !msg->cmds || msg->num_cmds != RSC_PDC_DATA_SIZE)
> > > + return -EINVAL;
> >
> > Is it really always exactly 2 (RSC_PDC_DATA_SIZE) commands?
> >
> Yes, always.

Thanks for the confirmation!

Reviewed-by: Matthias Kaehlcke <[email protected]>

2018-09-12 22:56:39