This patch makes two changes, both of which should be no-ops:
1. Make read_tcs_reg() / read_tcs_cmd() symmetric to write_tcs_reg() /
write_tcs_cmd().
2. Change the order of operations in the above functions to make it
more obvious to me what the math is doing. Specifically first you
want to find the right TCS, then the right register, and then
multiply by the command ID if necessary.
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v2: None
drivers/soc/qcom/rpmh-rsc.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index b71822131f59..b87b79f0347d 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -61,28 +61,33 @@
#define CMD_STATUS_ISSUED BIT(8)
#define CMD_STATUS_COMPL BIT(16)
-static u32 read_tcs_reg(struct rsc_drv *drv, int reg, int tcs_id, int cmd_id)
+static u32 read_tcs_cmd(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 +
+ return readl_relaxed(drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg +
RSC_DRV_CMD_OFFSET * cmd_id);
}
+static u32 read_tcs_reg(struct rsc_drv *drv, int reg, int tcs_id)
+{
+ return readl_relaxed(drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg);
+}
+
static void write_tcs_cmd(struct rsc_drv *drv, int reg, int tcs_id, int cmd_id,
u32 data)
{
- writel_relaxed(data, drv->tcs_base + reg + RSC_DRV_TCS_OFFSET * tcs_id +
+ writel_relaxed(data, drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg +
RSC_DRV_CMD_OFFSET * cmd_id);
}
static void write_tcs_reg(struct rsc_drv *drv, int reg, int tcs_id, u32 data)
{
- writel_relaxed(data, drv->tcs_base + reg + RSC_DRV_TCS_OFFSET * tcs_id);
+ writel_relaxed(data, drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg);
}
static void write_tcs_reg_sync(struct rsc_drv *drv, int reg, int tcs_id,
u32 data)
{
- writel(data, drv->tcs_base + reg + RSC_DRV_TCS_OFFSET * tcs_id);
+ writel(data, drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg);
for (;;) {
if (data == readl(drv->tcs_base + reg +
RSC_DRV_TCS_OFFSET * tcs_id))
@@ -94,7 +99,7 @@ static void write_tcs_reg_sync(struct rsc_drv *drv, int reg, int tcs_id,
static bool tcs_is_free(struct rsc_drv *drv, int tcs_id)
{
return !test_bit(tcs_id, drv->tcs_in_use) &&
- read_tcs_reg(drv, RSC_DRV_STATUS, tcs_id, 0);
+ read_tcs_reg(drv, RSC_DRV_STATUS, tcs_id);
}
static struct tcs_group *get_tcs_of_type(struct rsc_drv *drv, int type)
@@ -212,7 +217,7 @@ static irqreturn_t tcs_tx_done(int irq, void *p)
const struct tcs_request *req;
struct tcs_cmd *cmd;
- irq_status = read_tcs_reg(drv, RSC_DRV_IRQ_STATUS, 0, 0);
+ irq_status = read_tcs_reg(drv, RSC_DRV_IRQ_STATUS, 0);
for_each_set_bit(i, &irq_status, BITS_PER_LONG) {
req = get_req_from_tcs(drv, i);
@@ -226,7 +231,7 @@ static irqreturn_t tcs_tx_done(int irq, void *p)
u32 sts;
cmd = &req->cmds[j];
- sts = read_tcs_reg(drv, RSC_DRV_CMD_STATUS, i, j);
+ sts = read_tcs_cmd(drv, RSC_DRV_CMD_STATUS, i, j);
if (!(sts & CMD_STATUS_ISSUED) ||
((req->wait_for_compl || cmd->wait) &&
!(sts & CMD_STATUS_COMPL))) {
@@ -265,7 +270,7 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id,
cmd_msgid |= msg->wait_for_compl ? CMD_MSGID_RESP_REQ : 0;
cmd_msgid |= CMD_MSGID_WRITE;
- cmd_complete = read_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, tcs_id, 0);
+ cmd_complete = read_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, tcs_id);
for (i = 0, j = cmd_id; i < msg->num_cmds; i++, j++) {
cmd = &msg->cmds[i];
@@ -281,7 +286,7 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id,
}
write_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, tcs_id, cmd_complete);
- cmd_enable |= read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id, 0);
+ cmd_enable |= read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id);
write_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id, cmd_enable);
}
@@ -294,7 +299,7 @@ static void __tcs_trigger(struct rsc_drv *drv, int tcs_id)
* 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 = read_tcs_reg(drv, RSC_DRV_CONTROL, tcs_id);
enable &= ~TCS_AMC_MODE_TRIGGER;
write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
enable &= ~TCS_AMC_MODE_ENABLE;
@@ -319,10 +324,10 @@ static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs,
if (tcs_is_free(drv, tcs_id))
continue;
- curr_enabled = read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id, 0);
+ curr_enabled = read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id);
for_each_set_bit(j, &curr_enabled, MAX_CMDS_PER_TCS) {
- addr = read_tcs_reg(drv, RSC_DRV_CMD_ADDR, tcs_id, j);
+ addr = read_tcs_cmd(drv, RSC_DRV_CMD_ADDR, tcs_id, j);
for (k = 0; k < msg->num_cmds; k++) {
if (addr == msg->cmds[k].addr)
return -EBUSY;
--
2.25.1.481.gfbce0eb801-goog
Hi,
nit: can you please change as below, to mention its for TCS
drivers: qcom: rpmh-rsc: Clean code reading/writing TCS regs/cmds
Reviewed and tested.
Reviewed-by: Maulik Shah <[email protected]>
Tested-by: Maulik Shah <[email protected]>
Thanks,
Maulik
On 3/12/2020 4:43 AM, Douglas Anderson wrote:
> This patch makes two changes, both of which should be no-ops:
>
> 1. Make read_tcs_reg() / read_tcs_cmd() symmetric to write_tcs_reg() /
> write_tcs_cmd().
>
> 2. Change the order of operations in the above functions to make it
> more obvious to me what the math is doing. Specifically first you
> want to find the right TCS, then the right register, and then
> multiply by the command ID if necessary.
>
> Signed-off-by: Douglas Anderson<[email protected]>
> ---
>
> Changes in v2: None
>
> drivers/soc/qcom/rpmh-rsc.c | 31 ++++++++++++++++++-------------
> 1 file changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index b71822131f59..b87b79f0347d 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -61,28 +61,33 @@
> #define CMD_STATUS_ISSUED BIT(8)
> #define CMD_STATUS_COMPL BIT(16)
>
> -static u32 read_tcs_reg(struct rsc_drv *drv, int reg, int tcs_id, int cmd_id)
> +static u32 read_tcs_cmd(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 +
> + return readl_relaxed(drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg +
> RSC_DRV_CMD_OFFSET * cmd_id);
> }
>
> +static u32 read_tcs_reg(struct rsc_drv *drv, int reg, int tcs_id)
> +{
> + return readl_relaxed(drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg);
> +}
> +
> static void write_tcs_cmd(struct rsc_drv *drv, int reg, int tcs_id, int cmd_id,
> u32 data)
> {
> - writel_relaxed(data, drv->tcs_base + reg + RSC_DRV_TCS_OFFSET * tcs_id +
> + writel_relaxed(data, drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg +
> RSC_DRV_CMD_OFFSET * cmd_id);
> }
>
> static void write_tcs_reg(struct rsc_drv *drv, int reg, int tcs_id, u32 data)
> {
> - writel_relaxed(data, drv->tcs_base + reg + RSC_DRV_TCS_OFFSET * tcs_id);
> + writel_relaxed(data, drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg);
> }
>
> static void write_tcs_reg_sync(struct rsc_drv *drv, int reg, int tcs_id,
> u32 data)
> {
> - writel(data, drv->tcs_base + reg + RSC_DRV_TCS_OFFSET * tcs_id);
> + writel(data, drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg);
> for (;;) {
> if (data == readl(drv->tcs_base + reg +
> RSC_DRV_TCS_OFFSET * tcs_id))
> @@ -94,7 +99,7 @@ static void write_tcs_reg_sync(struct rsc_drv *drv, int reg, int tcs_id,
> static bool tcs_is_free(struct rsc_drv *drv, int tcs_id)
> {
> return !test_bit(tcs_id, drv->tcs_in_use) &&
> - read_tcs_reg(drv, RSC_DRV_STATUS, tcs_id, 0);
> + read_tcs_reg(drv, RSC_DRV_STATUS, tcs_id);
> }
>
> static struct tcs_group *get_tcs_of_type(struct rsc_drv *drv, int type)
> @@ -212,7 +217,7 @@ static irqreturn_t tcs_tx_done(int irq, void *p)
> const struct tcs_request *req;
> struct tcs_cmd *cmd;
>
> - irq_status = read_tcs_reg(drv, RSC_DRV_IRQ_STATUS, 0, 0);
> + irq_status = read_tcs_reg(drv, RSC_DRV_IRQ_STATUS, 0);
>
> for_each_set_bit(i, &irq_status, BITS_PER_LONG) {
> req = get_req_from_tcs(drv, i);
> @@ -226,7 +231,7 @@ static irqreturn_t tcs_tx_done(int irq, void *p)
> u32 sts;
>
> cmd = &req->cmds[j];
> - sts = read_tcs_reg(drv, RSC_DRV_CMD_STATUS, i, j);
> + sts = read_tcs_cmd(drv, RSC_DRV_CMD_STATUS, i, j);
> if (!(sts & CMD_STATUS_ISSUED) ||
> ((req->wait_for_compl || cmd->wait) &&
> !(sts & CMD_STATUS_COMPL))) {
> @@ -265,7 +270,7 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id,
> cmd_msgid |= msg->wait_for_compl ? CMD_MSGID_RESP_REQ : 0;
> cmd_msgid |= CMD_MSGID_WRITE;
>
> - cmd_complete = read_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, tcs_id, 0);
> + cmd_complete = read_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, tcs_id);
>
> for (i = 0, j = cmd_id; i < msg->num_cmds; i++, j++) {
> cmd = &msg->cmds[i];
> @@ -281,7 +286,7 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id,
> }
>
> write_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, tcs_id, cmd_complete);
> - cmd_enable |= read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id, 0);
> + cmd_enable |= read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id);
> write_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id, cmd_enable);
> }
>
> @@ -294,7 +299,7 @@ static void __tcs_trigger(struct rsc_drv *drv, int tcs_id)
> * 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 = read_tcs_reg(drv, RSC_DRV_CONTROL, tcs_id);
> enable &= ~TCS_AMC_MODE_TRIGGER;
> write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
> enable &= ~TCS_AMC_MODE_ENABLE;
> @@ -319,10 +324,10 @@ static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs,
> if (tcs_is_free(drv, tcs_id))
> continue;
>
> - curr_enabled = read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id, 0);
> + curr_enabled = read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id);
>
> for_each_set_bit(j, &curr_enabled, MAX_CMDS_PER_TCS) {
> - addr = read_tcs_reg(drv, RSC_DRV_CMD_ADDR, tcs_id, j);
> + addr = read_tcs_cmd(drv, RSC_DRV_CMD_ADDR, tcs_id, j);
> for (k = 0; k < msg->num_cmds; k++) {
> if (addr == msg->cmds[k].addr)
> return -EBUSY;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation