On 25-03-03 18:49:58, Marc Zyngier wrote:

quoted

quoted

quoted

+
+     pmu-a520 {
+             compatible = "arm,cortex-a520-pmu";
+             interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
+     };
+
+     pmu-a720 {
+             compatible = "arm,cortex-a720-pmu";
+             interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
+     };

This is wrong. The default configuration for PPIs is to expose the
*same* device on all CPUs. You must use PPI affinities for your PMUs.
Please see the GICv3 binding for the details.

We have discussed internally, we have not seen the benefits routing
different PPI interrupt to dedicated CPUs. Any use cases?

This isn't about changing the PPI. It is about matching CPUs with
their PMU. Here, you are saying "both PMU types are connected to all
the CPUs using PPI7".

That's obviously not the case.

quoted

I prefer changing pmu nodes as one generic Armv8 PMU node. Is it accepted?

No, that's not acceptable.

quoted

Or must I keep both pmu for A520 and A720, and add PPI affinities to
describe hardware well?

This is an established practice on all big-little systems: each PMU
node has an affinity that indicates which CPUs they are connected
to. For GICv3+, this is carried by the interrupt specifier.

Please look at existing SoCs supported, such as rk3399, for example.

I see. I will add ppi-partitions for gic-v3 node.

quoted

quoted

This will need to be bumped up to 4, and all the interrupt specifiers adjusted.

Depends on if PPI affinities is must.

Definitely a must, unless you want to completely remove all traces of
the PMU, which is of course silly, but a valid alternative.

I will change #interrupt-cells to 4, and applies to all interrupt
specifiers.

quoted

quoted

quoted

+             arm,no-tick-in-suspend;

Why do you need this? Is the HW so broken that you have implemented
the global counter in a power domain that isn't always on?

Not hardware broken, just arch timer will be powered off at cpu idle
and system suspend due to power consumption reason.

This is not about the timer. This is about the global counter. If your
counter stops ticking when you're in idle or suspended, your system is
broken and you need this property. If the timer (or more precisely the
comparator) is turned off because the CPU is off, then that's the
expected behaviour and you don't need this property.

I will delete this property.

-- 

Best regards,
Peter