* [PATCH] mfd: qcom_rpm: fix offset error for msm8660
@ 2016-06-14 9:48 Linus Walleij
2016-06-14 18:51 ` Stephen Boyd
2016-06-14 19:38 ` Bjorn Andersson
0 siblings, 2 replies; 4+ messages in thread
From: Linus Walleij @ 2016-06-14 9:48 UTC (permalink / raw)
To: lee.jones, linux-kernel, Bjorn Andersson, Stephen Boyd
Cc: Linus Walleij, stable
The RPM in MSM8660/APQ8060 has different offsets to the selector
ACK and request context ACK registers. Make all these register
offsets part of the per-SoC data and assign the right values.
The bug was found by verifying backwards to the vendor tree in
the out-of-tree files <mach/rpm-[8660|8064|8960]>: all were using
offsets 3,11,15,23 except the MSM8660/APQ8060 which was using
offsets 3,11,19,27.
Cc: stable@vger.kernel.org
Fixes: 58e214382bdd ("mfd: qcom-rpm: Driver for the Qualcomm RPM")
Cc: Björn Andersson <bjorn.andersson@linaro.org>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mfd/qcom_rpm.c | 37 +++++++++++++++++++++++++++----------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/drivers/mfd/qcom_rpm.c b/drivers/mfd/qcom_rpm.c
index 4f589cf75549..82cc7986e1ca 100644
--- a/drivers/mfd/qcom_rpm.c
+++ b/drivers/mfd/qcom_rpm.c
@@ -34,7 +34,11 @@ struct qcom_rpm_resource {
struct qcom_rpm_data {
u32 version;
const struct qcom_rpm_resource *resource_table;
- unsigned n_resources;
+ unsigned int n_resources;
+ unsigned int req_ctx_off;
+ unsigned int req_sel_off;
+ unsigned int ack_ctx_off;
+ unsigned int ack_sel_off;
};
struct qcom_rpm {
@@ -61,10 +65,6 @@ struct qcom_rpm {
#define RPM_REQUEST_TIMEOUT (5 * HZ)
-#define RPM_REQUEST_CONTEXT 3
-#define RPM_REQ_SELECT 11
-#define RPM_ACK_CONTEXT 15
-#define RPM_ACK_SELECTOR 23
#define RPM_SELECT_SIZE 7
#define RPM_NOTIFICATION BIT(30)
@@ -155,6 +155,10 @@ static const struct qcom_rpm_data apq8064_template = {
.version = 3,
.resource_table = apq8064_rpm_resource_table,
.n_resources = ARRAY_SIZE(apq8064_rpm_resource_table),
+ .req_ctx_off = 3,
+ .req_sel_off = 11,
+ .ack_ctx_off = 15,
+ .ack_sel_off = 23,
};
static const struct qcom_rpm_resource msm8660_rpm_resource_table[] = {
@@ -238,6 +242,10 @@ static const struct qcom_rpm_data msm8660_template = {
.version = 2,
.resource_table = msm8660_rpm_resource_table,
.n_resources = ARRAY_SIZE(msm8660_rpm_resource_table),
+ .req_ctx_off = 3,
+ .req_sel_off = 11,
+ .ack_ctx_off = 19,
+ .ack_sel_off = 27,
};
static const struct qcom_rpm_resource msm8960_rpm_resource_table[] = {
@@ -320,6 +328,10 @@ static const struct qcom_rpm_data msm8960_template = {
.version = 3,
.resource_table = msm8960_rpm_resource_table,
.n_resources = ARRAY_SIZE(msm8960_rpm_resource_table),
+ .req_ctx_off = 3,
+ .req_sel_off = 11,
+ .ack_ctx_off = 15,
+ .ack_sel_off = 23,
};
static const struct qcom_rpm_resource ipq806x_rpm_resource_table[] = {
@@ -360,6 +372,10 @@ static const struct qcom_rpm_data ipq806x_template = {
.version = 3,
.resource_table = ipq806x_rpm_resource_table,
.n_resources = ARRAY_SIZE(ipq806x_rpm_resource_table),
+ .req_ctx_off = 3,
+ .req_sel_off = 11,
+ .ack_ctx_off = 15,
+ .ack_sel_off = 23,
};
static const struct of_device_id qcom_rpm_of_match[] = {
@@ -398,10 +414,10 @@ int qcom_rpm_write(struct qcom_rpm *rpm,
bitmap_set((unsigned long *)sel_mask, res->select_id, 1);
for (i = 0; i < ARRAY_SIZE(sel_mask); i++) {
writel_relaxed(sel_mask[i],
- RPM_CTRL_REG(rpm, RPM_REQ_SELECT + i));
+ RPM_CTRL_REG(rpm, rpm->data->req_sel_off + i));
}
- writel_relaxed(BIT(state), RPM_CTRL_REG(rpm, RPM_REQUEST_CONTEXT));
+ writel_relaxed(BIT(state), RPM_CTRL_REG(rpm, rpm->data->req_ctx_off));
reinit_completion(&rpm->ack);
regmap_write(rpm->ipc_regmap, rpm->ipc_offset, BIT(rpm->ipc_bit));
@@ -424,10 +440,11 @@ static irqreturn_t qcom_rpm_ack_interrupt(int irq, void *dev)
u32 ack;
int i;
- ack = readl_relaxed(RPM_CTRL_REG(rpm, RPM_ACK_CONTEXT));
+ ack = readl_relaxed(RPM_CTRL_REG(rpm, rpm->data->ack_ctx_off));
for (i = 0; i < RPM_SELECT_SIZE; i++)
- writel_relaxed(0, RPM_CTRL_REG(rpm, RPM_ACK_SELECTOR + i));
- writel(0, RPM_CTRL_REG(rpm, RPM_ACK_CONTEXT));
+ writel_relaxed(0,
+ RPM_CTRL_REG(rpm, rpm->data->ack_sel_off + i));
+ writel(0, RPM_CTRL_REG(rpm, rpm->data->ack_ctx_off));
if (ack & RPM_NOTIFICATION) {
dev_warn(rpm->dev, "ignoring notification!\n");
--
2.4.11
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] mfd: qcom_rpm: fix offset error for msm8660
2016-06-14 9:48 [PATCH] mfd: qcom_rpm: fix offset error for msm8660 Linus Walleij
@ 2016-06-14 18:51 ` Stephen Boyd
2016-06-14 19:38 ` Linus Walleij
2016-06-14 19:38 ` Bjorn Andersson
1 sibling, 1 reply; 4+ messages in thread
From: Stephen Boyd @ 2016-06-14 18:51 UTC (permalink / raw)
To: Linus Walleij; +Cc: lee.jones, linux-kernel, Bjorn Andersson, stable
On 06/14, Linus Walleij wrote:
> The RPM in MSM8660/APQ8060 has different offsets to the selector
> ACK and request context ACK registers. Make all these register
> offsets part of the per-SoC data and assign the right values.
>
> The bug was found by verifying backwards to the vendor tree in
> the out-of-tree files <mach/rpm-[8660|8064|8960]>: all were using
> offsets 3,11,15,23 except the MSM8660/APQ8060 which was using
> offsets 3,11,19,27.
>
> Cc: stable@vger.kernel.org
> Fixes: 58e214382bdd ("mfd: qcom-rpm: Driver for the Qualcomm RPM")
> Cc: Björn Andersson <bjorn.andersson@linaro.org>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Good catch! That macro maze is fun.
> ---
> drivers/mfd/qcom_rpm.c | 37 +++++++++++++++++++++++++++----------
> 1 file changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mfd/qcom_rpm.c b/drivers/mfd/qcom_rpm.c
> index 4f589cf75549..82cc7986e1ca 100644
> --- a/drivers/mfd/qcom_rpm.c
> +++ b/drivers/mfd/qcom_rpm.c
> @@ -34,7 +34,11 @@ struct qcom_rpm_resource {
> struct qcom_rpm_data {
> u32 version;
> const struct qcom_rpm_resource *resource_table;
> - unsigned n_resources;
> + unsigned int n_resources;
> + unsigned int req_ctx_off;
> + unsigned int req_sel_off;
> + unsigned int ack_ctx_off;
> + unsigned int ack_sel_off;
> };
>
> struct qcom_rpm {
> @@ -61,10 +65,6 @@ struct qcom_rpm {
>
> #define RPM_REQUEST_TIMEOUT (5 * HZ)
>
> -#define RPM_REQUEST_CONTEXT 3
> -#define RPM_REQ_SELECT 11
> -#define RPM_ACK_CONTEXT 15
> -#define RPM_ACK_SELECTOR 23
> #define RPM_SELECT_SIZE 7
The RPM_SELECT_SIZE is 7 on 8660, but now you've pointed out that
otherwise the size is 4. I think you've uncovered another bug.
>
> #define RPM_NOTIFICATION BIT(30)
> @@ -398,10 +414,10 @@ int qcom_rpm_write(struct qcom_rpm *rpm,
> bitmap_set((unsigned long *)sel_mask, res->select_id, 1);
> for (i = 0; i < ARRAY_SIZE(sel_mask); i++) {
> writel_relaxed(sel_mask[i],
> - RPM_CTRL_REG(rpm, RPM_REQ_SELECT + i));
> + RPM_CTRL_REG(rpm, rpm->data->req_sel_off + i));
Here we write from 0 to ARRAY_SIZE(sel_mask) which is 7. That
would mean we write into the ack context that starts at 15 (we
start writing at req_sel_off which is always 11). Oops.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] mfd: qcom_rpm: fix offset error for msm8660
2016-06-14 18:51 ` Stephen Boyd
@ 2016-06-14 19:38 ` Linus Walleij
0 siblings, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2016-06-14 19:38 UTC (permalink / raw)
To: Stephen Boyd; +Cc: Lee Jones, linux-kernel, Bjorn Andersson, stable
On Tue, Jun 14, 2016 at 8:51 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 06/14, Linus Walleij wrote:
>> -#define RPM_REQUEST_CONTEXT 3
>> -#define RPM_REQ_SELECT 11
>> -#define RPM_ACK_CONTEXT 15
>> -#define RPM_ACK_SELECTOR 23
>> #define RPM_SELECT_SIZE 7
>
> The RPM_SELECT_SIZE is 7 on 8660, but now you've pointed out that
> otherwise the size is 4. I think you've uncovered another bug.
OMG you're right. I'll send a v2 fixing that too.
>> - RPM_CTRL_REG(rpm, RPM_REQ_SELECT + i));
>> + RPM_CTRL_REG(rpm, rpm->data->req_sel_off + i));
>
> Here we write from 0 to ARRAY_SIZE(sel_mask) which is 7. That
> would mean we write into the ack context that starts at 15 (we
> start writing at req_sel_off which is always 11). Oops.
Argh well it seems it didn't hurt so far but let's have it fixed.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mfd: qcom_rpm: fix offset error for msm8660
2016-06-14 9:48 [PATCH] mfd: qcom_rpm: fix offset error for msm8660 Linus Walleij
2016-06-14 18:51 ` Stephen Boyd
@ 2016-06-14 19:38 ` Bjorn Andersson
1 sibling, 0 replies; 4+ messages in thread
From: Bjorn Andersson @ 2016-06-14 19:38 UTC (permalink / raw)
To: Linus Walleij; +Cc: lee.jones, linux-kernel, Stephen Boyd, stable
On Tue 14 Jun 02:48 PDT 2016, Linus Walleij wrote:
> The RPM in MSM8660/APQ8060 has different offsets to the selector
> ACK and request context ACK registers. Make all these register
> offsets part of the per-SoC data and assign the right values.
>
> The bug was found by verifying backwards to the vendor tree in
> the out-of-tree files <mach/rpm-[8660|8064|8960]>: all were using
> offsets 3,11,15,23 except the MSM8660/APQ8060 which was using
> offsets 3,11,19,27.
>
> Cc: stable@vger.kernel.org
> Fixes: 58e214382bdd ("mfd: qcom-rpm: Driver for the Qualcomm RPM")
> Cc: Björn Andersson <bjorn.andersson@linaro.org>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Thanks for finding this, sorry for not getting it right in the first
place.
With the note about the odd indentation below, and a wish that you fix
the sel_mask length
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> drivers/mfd/qcom_rpm.c | 37 +++++++++++++++++++++++++++----------
> 1 file changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mfd/qcom_rpm.c b/drivers/mfd/qcom_rpm.c
[..]
> @@ -155,6 +155,10 @@ static const struct qcom_rpm_data apq8064_template = {
> .version = 3,
> .resource_table = apq8064_rpm_resource_table,
> .n_resources = ARRAY_SIZE(apq8064_rpm_resource_table),
> + .req_ctx_off = 3,
> + .req_sel_off = 11,
Odd indentation of 11, here and below.
> + .ack_ctx_off = 15,
> + .ack_sel_off = 23,
> };
>
> static const struct qcom_rpm_resource msm8660_rpm_resource_table[] = {
> @@ -238,6 +242,10 @@ static const struct qcom_rpm_data msm8660_template = {
> .version = 2,
> .resource_table = msm8660_rpm_resource_table,
> .n_resources = ARRAY_SIZE(msm8660_rpm_resource_table),
> + .req_ctx_off = 3,
> + .req_sel_off = 11,
> + .ack_ctx_off = 19,
> + .ack_sel_off = 27,
> };
>
> static const struct qcom_rpm_resource msm8960_rpm_resource_table[] = {
> @@ -320,6 +328,10 @@ static const struct qcom_rpm_data msm8960_template = {
> .version = 3,
> .resource_table = msm8960_rpm_resource_table,
> .n_resources = ARRAY_SIZE(msm8960_rpm_resource_table),
> + .req_ctx_off = 3,
> + .req_sel_off = 11,
> + .ack_ctx_off = 15,
> + .ack_sel_off = 23,
> };
>
> static const struct qcom_rpm_resource ipq806x_rpm_resource_table[] = {
> @@ -360,6 +372,10 @@ static const struct qcom_rpm_data ipq806x_template = {
> .version = 3,
> .resource_table = ipq806x_rpm_resource_table,
> .n_resources = ARRAY_SIZE(ipq806x_rpm_resource_table),
> + .req_ctx_off = 3,
> + .req_sel_off = 11,
> + .ack_ctx_off = 15,
> + .ack_sel_off = 23,
> };
>
> static const struct of_device_id qcom_rpm_of_match[] = {
> @@ -398,10 +414,10 @@ int qcom_rpm_write(struct qcom_rpm *rpm,
> bitmap_set((unsigned long *)sel_mask, res->select_id, 1);
> for (i = 0; i < ARRAY_SIZE(sel_mask); i++) {
As Stephen pointed out, this constant should be platform specific as
well. Would you mind fold that in, or maybe write a separate patch for
it?
> writel_relaxed(sel_mask[i],
> - RPM_CTRL_REG(rpm, RPM_REQ_SELECT + i));
> + RPM_CTRL_REG(rpm, rpm->data->req_sel_off + i));
> }
Regards,
Bjorn
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-06-14 19:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-14 9:48 [PATCH] mfd: qcom_rpm: fix offset error for msm8660 Linus Walleij
2016-06-14 18:51 ` Stephen Boyd
2016-06-14 19:38 ` Linus Walleij
2016-06-14 19:38 ` Bjorn Andersson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).