On 25-03-03 18:49:58, Marc Zyngier wrote:
> > > > +
> > > > + 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.
>
> > I prefer changing pmu nodes as one generic Armv8 PMU node. Is it accepted?
>
> No, that's not acceptable.
>
> > 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.
> > >
> > > 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.
>
> > > > + 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
