Changes in v6:
Rebased [v5 08/10] on
https://lore.kernel.org/linux-arm-msm/[email protected]/#t
Changes in v5:
Drop [v4 11/11] because of a similar change is under review:
https://lore.kernel.org/linux-arm-msm/[email protected]/T/#t
Changes in v4:
In [v4 02/11], separated spurious interrupt handling.
In [v4 03/11], added Fixes tag for ("spmi: pmic-arb: do not ack and clear peripheral").
In [v4 11/11], updated the binding to address few warnings in "make dtbs_check"
Changes in v3:
Drop [v2 07/10] as this is no longer needed after this change:
50fc4c8cd240 ("spmi: spmi-pmic-arb: fix irq_set_type race condition")
In [v3 07/10], updated the author email to match with Signed-off-by.
In [v3 10/10], added the binding change in this series, and addressed issues in "make dt_binding_check"
Changes in v2:
In [v2 01/10], added code to handle spurious interrupt.
In [v2 03/10], adressed minor comments to update the code logic.
In [v2 04/10], minor update to detect spurious interrupt.
In [v2 05/10], added Fixes tag.
In [v2 07/10], added Fixes tag and updated commit text to explain the problem.
In [v2 08/10], added binding change to make interrupt properties as optional.
In [v2 09/10], updated to check presence of "interrupt-controller" property.
Abhijeet Dharmapurikar (1):
spmi: pmic-arb: add a print in cleanup_irq
Ashay Jaiswal (1):
spmi: pmic-arb: add support to dispatch interrupt based on IRQ status
David Collins (6):
spmi: pmic-arb: check apid against limits before calling irq handler
spmi: pmic-arb: correct duplicate APID to PPID mapping logic
spmi: pmic-arb: block access for invalid PMIC arbiter v5 SPMI writes
dt-bindings: spmi: spmi-pmic-arb: make interrupt properties as
optional
spmi: pmic-arb: make interrupt support optional
spmi: pmic-arb: increase SPMI transaction timeout delay
Fenglin Wu (1):
spmi: pmic-arb: handle spurious interrupt
Subbaraman Narayanamurthy (1):
spmi: pmic-arb: do not ack and clear peripheral interrupts in
cleanup_irq
.../bindings/spmi/qcom,spmi-pmic-arb.yaml | 3 -
drivers/spmi/spmi-pmic-arb.c | 136 +++++++++++++++------
2 files changed, 96 insertions(+), 43 deletions(-)
--
2.7.4
Call handle_bad_irq() when the summary interrupt is fired spuriously.
Signed-off-by: Fenglin Wu <[email protected]>
---
drivers/spmi/spmi-pmic-arb.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 5a99723..719bd73 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -605,10 +605,11 @@ static void cleanup_irq(struct spmi_pmic_arb *pmic_arb, u16 apid, int id)
irq_mask, ppid);
}
-static void periph_interrupt(struct spmi_pmic_arb *pmic_arb, u16 apid)
+static int periph_interrupt(struct spmi_pmic_arb *pmic_arb, u16 apid)
{
unsigned int irq;
u32 status, id;
+ int handled = 0;
u8 sid = (pmic_arb->apid_data[apid].ppid >> 8) & 0xF;
u8 per = pmic_arb->apid_data[apid].ppid & 0xFF;
@@ -623,7 +624,10 @@ static void periph_interrupt(struct spmi_pmic_arb *pmic_arb, u16 apid)
continue;
}
generic_handle_irq(irq);
+ handled++;
}
+
+ return handled;
}
static void pmic_arb_chained_irq(struct irq_desc *desc)
@@ -634,7 +638,7 @@ static void pmic_arb_chained_irq(struct irq_desc *desc)
int first = pmic_arb->min_apid >> 5;
int last = pmic_arb->max_apid >> 5;
u8 ee = pmic_arb->ee;
- u32 status, enable;
+ u32 status, enable, handled = 0;
int i, id, apid;
chained_irq_enter(chip, desc);
@@ -649,10 +653,14 @@ static void pmic_arb_chained_irq(struct irq_desc *desc)
enable = readl_relaxed(
ver_ops->acc_enable(pmic_arb, apid));
if (enable & SPMI_PIC_ACC_ENABLE_BIT)
- periph_interrupt(pmic_arb, apid);
+ if (periph_interrupt(pmic_arb, apid) != 0)
+ handled++;
}
}
+ if (handled == 0)
+ handle_bad_irq(desc);
+
chained_irq_exit(chip, desc);
}
--
2.7.4
From: David Collins <[email protected]>
Correct the way that duplicate PPID mappings are handled for PMIC
arbiter v5. The final APID mapped to a given PPID should be the
one which has write owner = APPS EE, if it exists, or if not
that, then the first APID mapped to the PPID, if it exists.
Fixes: 40f318f0ed67 ("spmi: pmic-arb: add support for HW version 5")
Signed-off-by: David Collins <[email protected]>
Signed-off-by: Fenglin Wu <[email protected]>
---
drivers/spmi/spmi-pmic-arb.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 56f2294..cf92abc 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -1031,7 +1031,8 @@ static int pmic_arb_read_apid_map_v5(struct spmi_pmic_arb *pmic_arb)
* version 5, there is more than one APID mapped to each PPID.
* The owner field for each of these mappings specifies the EE which is
* allowed to write to the APID. The owner of the last (highest) APID
- * for a given PPID will receive interrupts from the PPID.
+ * which has the IRQ owner bit set for a given PPID will receive
+ * interrupts from the PPID.
*/
for (i = 0; ; i++, apidd++) {
offset = pmic_arb->ver_ops->apid_map_offset(i);
@@ -1054,16 +1055,16 @@ static int pmic_arb_read_apid_map_v5(struct spmi_pmic_arb *pmic_arb)
apid = pmic_arb->ppid_to_apid[ppid] & ~PMIC_ARB_APID_VALID;
prev_apidd = &pmic_arb->apid_data[apid];
- if (valid && is_irq_ee &&
- prev_apidd->write_ee == pmic_arb->ee) {
+ if (!valid || apidd->write_ee == pmic_arb->ee) {
+ /* First PPID mapping or one for this EE */
+ pmic_arb->ppid_to_apid[ppid] = i | PMIC_ARB_APID_VALID;
+ } else if (valid && is_irq_ee &&
+ prev_apidd->write_ee == pmic_arb->ee) {
/*
* Duplicate PPID mapping after the one for this EE;
* override the irq owner
*/
prev_apidd->irq_ee = apidd->irq_ee;
- } else if (!valid || is_irq_ee) {
- /* First PPID mapping or duplicate for another EE */
- pmic_arb->ppid_to_apid[ppid] = i | PMIC_ARB_APID_VALID;
}
apidd->ppid = ppid;
--
2.7.4
From: Ashay Jaiswal <[email protected]>
Current implementation of SPMI arbiter dispatches interrupt based on the
Arbiter's accumulator status, in some cases the accumulator status may
remain zero and the interrupt remains un-handled. Add logic to dispatch
interrupts based Arbiter's IRQ status if the accumulator status is zero.
Signed-off-by: Ashay Jaiswal <[email protected]>
Signed-off-by: David Collins <[email protected]>
Signed-off-by: Fenglin Wu <[email protected]>
---
drivers/spmi/spmi-pmic-arb.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index e19eaec..56f2294 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -630,12 +630,18 @@ static void pmic_arb_chained_irq(struct irq_desc *desc)
u8 ee = pmic_arb->ee;
u32 status, enable, handled = 0;
int i, id, apid;
+ /* status based dispatch */
+ bool acc_valid = false;
+ u32 irq_status = 0;
chained_irq_enter(chip, desc);
for (i = first >> 5; i <= last >> 5; ++i) {
status = readl_relaxed(
ver_ops->owner_acc_status(pmic_arb, ee, i));
+ if (status)
+ acc_valid = true;
+
while (status) {
id = ffs(status) - 1;
status &= ~BIT(id);
@@ -653,6 +659,29 @@ static void pmic_arb_chained_irq(struct irq_desc *desc)
}
}
+ /* ACC_STATUS is empty but IRQ fired check IRQ_STATUS */
+ if (!acc_valid) {
+ for (i = first; i <= last; i++) {
+ /* skip if APPS is not irq owner */
+ if (pmic_arb->apid_data[i].irq_ee != pmic_arb->ee)
+ continue;
+
+ irq_status = readl_relaxed(
+ ver_ops->irq_status(pmic_arb, i));
+ if (irq_status) {
+ enable = readl_relaxed(
+ ver_ops->acc_enable(pmic_arb, i));
+ if (enable & SPMI_PIC_ACC_ENABLE_BIT) {
+ dev_dbg(&pmic_arb->spmic->dev,
+ "Dispatching IRQ for apid=%d status=%x\n",
+ i, irq_status);
+ if (periph_interrupt(pmic_arb, i) != 0)
+ handled++;
+ }
+ }
+ }
+ }
+
if (handled == 0)
handle_bad_irq(desc);
--
2.7.4
From: David Collins <[email protected]>
The system crashes due to an access permission violation when
writing to a PMIC peripheral which is not owned by the current
ee. Add a check for PMIC arbiter version 5 for such invalid
write requests and return an error instead of crashing the
system.
Signed-off-by: David Collins <[email protected]>
Signed-off-by: Fenglin Wu <[email protected]>
---
drivers/spmi/spmi-pmic-arb.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index cf92abc..39f25bc 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -1133,6 +1133,11 @@ static int pmic_arb_offset_v5(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
offset = 0x10000 * pmic_arb->ee + 0x80 * apid;
break;
case PMIC_ARB_CHANNEL_RW:
+ if (pmic_arb->apid_data[apid].write_ee != pmic_arb->ee) {
+ dev_err(&pmic_arb->spmic->dev, "disallowed SPMI write to sid=%u, addr=0x%04X\n",
+ sid, addr);
+ return -EPERM;
+ }
offset = 0x10000 * apid;
break;
}
--
2.7.4
From: David Collins <[email protected]>
Make all interrupt related properties as optional instead of
required. Some boards do not required PMIC IRQ support and it
isn't needed to handle SPMI bus transactions, so specify it as
optional.
Signed-off-by: David Collins <[email protected]>
Signed-off-by: Fenglin Wu <[email protected]>
---
Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml | 3 ---
1 file changed, 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
index 55d379c..fee4f0e 100644
--- a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
+++ b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
@@ -88,9 +88,6 @@ properties:
required:
- compatible
- reg-names
- - interrupts
- - interrupt-names
- - '#interrupt-cells'
- qcom,ee
- qcom,channel
--
2.7.4
On Thu, 17 Mar 2022 14:12:13 +0800, Fenglin Wu wrote:
> From: David Collins <[email protected]>
>
> Make all interrupt related properties as optional instead of
> required. Some boards do not required PMIC IRQ support and it
> isn't needed to handle SPMI bus transactions, so specify it as
> optional.
>
> Signed-off-by: David Collins <[email protected]>
> Signed-off-by: Fenglin Wu <[email protected]>
> ---
> Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml | 3 ---
> 1 file changed, 3 deletions(-)
>
Acked-by: Rob Herring <[email protected]>
> Changes in v6:
> Rebased [v5 08/10] on
> https://lore.kernel.org/linux-arm-msm/[email protected]/#t
Hi Stephen,
Can this patch series be reviewed? It would be good to know if something is
missing to keep this pending for a while.
Thanks,
Subbaraman