On Tue, Feb 21, 2017 at 05:19:58PM +0530, Anurup M wrote:
On Monday 20 February 2017 04:59 PM, Mark Rutland wrote:If there's a particular case whre we'd see the overflow bit set for an
On Sun, Feb 19, 2017 at 01:51:22PM -0500, Anurup M wrote:Here I ignore if the event does not exist. I have seen it is handled
+ /* Clear the IRQ status flag */Do we expect to take interrupts for an event which does not exist?
+ hisi_djtag_writereg(module_id, MN1_BANK_SELECT,
+ MN1_INTC_REG_OFF, (1 << bit_pos), client);
+
+ /* Get the corresponding event struct */
+ event = mn_pmu->hw_perf_events[bit_pos];
+ if (!event)
+ continue;
in arm_pmu and other reference
implementations to ignore if there is no event.
The event is cleared in .del. So if .del is called before the IRQ
handler, this check is required right?
Please comment.
event, please add a comment describing that case here.
[...]
You'll need to elaborate on that a bit further; I don't understand.There is a defect in the mbigen hardware to handle the IRQ mapping+static int hisi_mn_init_irqs_fdt(struct device *dev,Surely we expect a specific number of interrupts?
+ struct hisi_pmu *mn_pmu)
+{
+ struct hisi_mn_data *mn_data = mn_pmu->hwmod_data;
+ struct hisi_djtag_client *client = mn_data->client;
+ int irq = -1, num_irqs, i;
+
+ num_irqs = of_irq_count(dev->of_node);
+ for (i = 0; i < num_irqs; i++) {Why are we throwing these away?
+ irq = of_irq_get(dev->of_node, i);
+ if (irq < 0)
+ dev_info(dev, "No IRQ resource!\n");
+ }
+I don't understand this comment.
+ if (irq < 0)
+ return 0;
+
+ /* The last entry in the IRQ list to be chosen
+ * This is as per mbigen-v2 IRQ mapping
+ */
+ return hisi_mn_init_irq(irq, mn_pmu, client);
Why do we only use the list IRQ?
What does this have to do with the mbigen?
No ordering requirement was described in the DT binding.
for MN.
Due to this the IRQ property
of MN is made as a list and we read all IRQs and use only the last one.
I shall mention it in the comment and also add note in the DT bindings.
If the interrupts aren't usable, there's arguably not much point listing
them in the DT.
Regardless, the order of the list *must* be specified in the DT binding.
Thanks,
Mark.