Hi Stephen,

On Tue, Jan 14, 2014 at 08:57:53PM +0000, Stephen Boyd wrote:

On 01/13/14 03:52, Will Deacon wrote:

quoted

I also don't think that's the right solution, based on the above. It's
actually pretty hard to work out what's the right thing to do here...

Yes it doesn't seem like the right solution.

quoted

We *could* have a per-cpu pointer to the cpu_pmu_pointer, but then we'd
need to update the IRQ handlers, including things like the CCI PMU which
really doesn't care about per-cpu stuff. So after all this, the shim we have
around the IRQ handler for the U8500 SPI workarounds might be the right
thing after all -- it allows us to consolidate the conversion of a pcpu
pointer into the relevant instance (actually any instance, since they'd all
point at the same thing) for the current CPU.

What do you think to having that shim throw away the second level pcpu
pointer in the case of a PPI? (probably means we need to revisit that
renaming again).

Ok I think I understand what you're getting at. We pass a per-cpu
pointer to the cpu_pmu pointer as the dev_id argument to the PPI irq
handler, and then we check to see if the irq is per-cpu inside the
armpmu_dispatch_irq() function and throw away the second level of
pointer, i.e.

static irqreturn_t armpmu_dispatch_irq(int irq, void *dev)
{
        struct arm_pmu *armpmu;
        struct platform_device *plat_device;
        struct arm_pmu_platdata *plat;

        if (irq_is_percpu(irq))
                dev = *(struct arm_pmu_cpu **)dev;
        armpmu = dev;
        plat_device = armpmu->plat_device;
        plat = dev_get_platdata(&plat_device->dev);

        if (plat && plat->handle_irq)
                return plat->handle_irq(irq, dev, armpmu->handle_irq);
        else
                return armpmu->handle_irq(irq, dev);
}

Yup, that's what I was trying to explain (badly). Thanks.

We still need to make a per-cpu variable to hold the pointer, and assign
it during cpu_pmu_init like this patch does. Hopefully that is ok.

I think that's ok. The percpu code in genirq requires a pcpu token (for good
reason) and the irq handler needs to get at the pmu structure. The
alternative is adding pointers from something like the pmu_hw_events to the
arm_pmu, but I think that's more ugly.

Will