From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753335AbcFOIhp (ORCPT ); Wed, 15 Jun 2016 04:37:45 -0400 Received: from mail-wm0-f50.google.com ([74.125.82.50]:35353 "EHLO mail-wm0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753188AbcFOIhm (ORCPT ); Wed, 15 Jun 2016 04:37:42 -0400 Date: Wed, 15 Jun 2016 09:38:22 +0100 From: Lee Jones To: Linus Walleij Cc: linux-kernel@vger.kernel.org, Bjorn Andersson , Stephen Boyd , stable@vger.kernel.org Subject: Re: [PATCH v3] mfd: qcom_rpm: fix offset error for msm8660 Message-ID: <20160615083822.GA4948@dell> References: <1465945346-29298-1-git-send-email-linus.walleij@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1465945346-29298-1-git-send-email-linus.walleij@linaro.org> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 15 Jun 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 : all were using > offsets 3,11,15,23 and a select size of 4, except the MSM8660/APQ8060 > which was using offsets 3,11,19,27 and a select size of 7. > > All other platforms apart from msm8660 were affected by reading > excess registers, since 7 was hardcoded as the number of select > words, this patch makes also this part dynamic so we only write/read > as many select words as the platform actually use. > > Symptoms of this bug when using msm8660: the first RPM transaction > would work, but the next would stall or raise an error since the > previous transaction was not properly ACKed as the ACK words were > read at the wrong offset. > > Cc: stable@vger.kernel.org > Fixes: 58e214382bdd ("mfd: qcom-rpm: Driver for the Qualcomm RPM") > Cc: Stephen Boyd > Reviewed-by: Björn Andersson > Signed-off-by: Linus Walleij > --- > ChangeLog v2->v3: > - Fix the odd indentation pointed out by Björn, add reviewed-by. > ChangeLog v1->v2: > - Also augment the device data to hold the number of select words > as the was wrong on all other platforms: 4 vs 7. > --- > drivers/mfd/qcom_rpm.c | 50 ++++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 36 insertions(+), 14 deletions(-) Applied, thanks. > diff --git a/drivers/mfd/qcom_rpm.c b/drivers/mfd/qcom_rpm.c > index 1be47ad6441b..9364f88264e5 100644 > --- a/drivers/mfd/qcom_rpm.c > +++ b/drivers/mfd/qcom_rpm.c > @@ -34,7 +34,12 @@ 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; > + unsigned int sel_size; > }; > > struct qcom_rpm { > @@ -61,11 +66,7 @@ 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_MAX_SEL_SIZE 7 > > #define RPM_NOTIFICATION BIT(30) > #define RPM_REJECTED BIT(31) > @@ -157,6 +158,11 @@ 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, > + .sel_size = 4, > }; > > static const struct qcom_rpm_resource msm8660_rpm_resource_table[] = { > @@ -240,6 +246,11 @@ 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, > + .sel_size = 7, > }; > > static const struct qcom_rpm_resource msm8960_rpm_resource_table[] = { > @@ -322,6 +333,11 @@ 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, > + .sel_size = 4, > }; > > static const struct qcom_rpm_resource ipq806x_rpm_resource_table[] = { > @@ -362,6 +378,11 @@ 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, > + .sel_size = 4, > }; > > static const struct of_device_id qcom_rpm_of_match[] = { > @@ -380,7 +401,7 @@ int qcom_rpm_write(struct qcom_rpm *rpm, > { > const struct qcom_rpm_resource *res; > const struct qcom_rpm_data *data = rpm->data; > - u32 sel_mask[RPM_SELECT_SIZE] = { 0 }; > + u32 sel_mask[RPM_MAX_SEL_SIZE] = { 0 }; > int left; > int ret = 0; > int i; > @@ -398,12 +419,12 @@ int qcom_rpm_write(struct qcom_rpm *rpm, > writel_relaxed(buf[i], RPM_REQ_REG(rpm, res->target_id + i)); > > bitmap_set((unsigned long *)sel_mask, res->select_id, 1); > - for (i = 0; i < ARRAY_SIZE(sel_mask); i++) { > + for (i = 0; i < rpm->data->sel_size; 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)); > @@ -426,10 +447,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)); > - 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)); > + ack = readl_relaxed(RPM_CTRL_REG(rpm, rpm->data->ack_ctx_off)); > + for (i = 0; i < rpm->data->sel_size; i++) > + 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"); -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog