| Message ID | 1465897725-16213-1-git-send-email-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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");
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(-) -- 2.4.11 -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html