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 <mach/rpm-[8660|8064|8960]>: 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@xxxxxxxxxxxxxxx
> Fixes: 58e214382bdd ("mfd: qcom-rpm: Driver for the Qualcomm RPM")
> Cc: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>
> Reviewed-by: Björn Andersson <bjorn.andersson@xxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html