Hi,
On 7/14/2020 3:47 AM, Doug Anderson wrote:
Hi,
On Mon, Jun 22, 2020 at 2:33 AM Maulik Shah <mkshah@xxxxxxxxxxxxxx> wrote:
Clear previous kernel's configuration during init by resetting
all interrupts in enable bank to zero.
Signed-off-by: Maulik Shah <mkshah@xxxxxxxxxxxxxx>
---
drivers/irqchip/qcom-pdc.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 8beb6f7..11a9d3a 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -19,6 +19,7 @@
#include <linux/slab.h>
#include <linux/types.h>
+#define PDC_MAX_IRQS_PER_REG 32
#define PDC_MAX_IRQS 168
#define PDC_MAX_GPIO_IRQS 256
@@ -339,6 +340,7 @@ static const struct irq_domain_ops qcom_pdc_gpio_ops = {
static int pdc_setup_pin_mapping(struct device_node *np)
{
int ret, n;
+ u32 reg, max_regs, max_pins = 0;
n = of_property_count_elems_of_size(np, "qcom,pdc-ranges", sizeof(u32));
if (n <= 0 || n % 3)
@@ -367,8 +369,19 @@ static int pdc_setup_pin_mapping(struct device_node *np)
&pdc_region[n].cnt);
if (ret)
return ret;
+ max_pins += pdc_region[n].cnt;
}
+ if (max_pins > PDC_MAX_IRQS)
+ return -EINVAL;
+
+ max_regs = max_pins / PDC_MAX_IRQS_PER_REG;
+ if (max_pins % PDC_MAX_IRQS_PER_REG)
+ max_regs++;
nit: max_regs = DIV_ROUND_UP(max_pins, PDC_MAX_IRQS_PER_REG)
+ for (reg = 0; reg < max_regs; reg++)
+ pdc_reg_write(IRQ_ENABLE_BANK, reg, 0);
This doesn't feel correct to me, but maybe I'm misunderstanding the
hardware (I don't think I have access to a reference manual). Looking
at the example in the bindings, I see:
qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
In that example we have mappings for PDC ports:
0 - 93 (count = 94)
94 - 108 (count = 15)
115 - 121 (count = 7)
Notice the slight discontinuity there. I presume that discontinuity
is normal / allowed? If so, if there is enough of it then I think
your math could be wrong, though with the example you get lucky and it
works out OK. It's easy to see the problem with a slightly different
example: Imagine that you had this:
0 - 33 (count = 34)
94 - 108 (count = 15)
115 - 121 (count = 7)
...now max_pins = 56 and max_regs = 2. So you'll init reg 0 and 1.
...but (IIUC) you actually should be initting 0, 1, 2, and 3.
Right, Thanks for cacthing this. I will fix in next revision.
Thanks,
Maulik
I have no idea what might be in those discontinuous ranges and if it's
always OK to clear, but (assuming it is) one fix is to put your
clearing loop _inside_ the other "for" loop in this function, AKA:
for (reg = pdc_region[n].pin_base / PDC_MAX_IRQS_PER_REG;
reg < DIV_ROUND_UP(pdc_region[n].pin_base + pdc_region[n].cnt),
PDC_MAX_IRQS_PER_REG)
reg++)
...or another option is to keep track of the max "pin_base + cnt" and
loop from 0 to there? I just don't know your hardware well enough to
tell which would be right.
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
