Re: [PATCH] arm64: topology: Don't support AMU without cpufreq

On 09-07-20, 13:46, Ionela Voinescu wrote:

I saw this case during FVP testing, although I acknowledge the 'virtual'
part of that platform [1]. But allowing this does enable AMU testing on
an AEM FVP.

In kernel, we only support things that are in mainline, else we don't
care about them. That's the general rule. And yeah I understand that
this is early support for a new hardware, and so it is better to add
code for things we are sure about.

While I completely understand the reasoning behind avoiding to introduce
large changes for small corner-case gains,

I think even that is fine, if there is a problem to be solved it needs
to be solved, big or small doesn't really matter. Just that it needs
to be there in mainline.

the arguments for this
support was:
 - (1) AMUs are a new feature and it will take some time until we see the
   real usecases. That's always the case with early support for a
   feature - we want to add it early to enable its use and testing, but
   it will take some time to establish the true usecases.

Exactly, and so people normally prefer to keep things simple until the
time the needs arises for the same. A patch can be added later, its no
big deal. But it should be added when we need it.

 - (2) It literally needed 2 lines of code + the weak cpufreq function
   to support this.

Yeah, small or big doesn't really matter.

Given that I can't guarantee what hardware will or won't do, and given
that AMUs are an optional feature, I controlled the only thing I could:
the software :). By not making assumptions about the hardware, I ensured
that the code does not break the interaction between cpufreq use or AMU
use for frequency invariance.

This will be nicer in the new code as the control will be at CPU level,
rather than policy level.

I won't try to force you to remove this piece and will leave it for
you to decide.

But, I don't see a future system in mainline which uses AMU but
doesn't have cpufreq for all its CPUs. And so I won't have kept code
for that, even if it is just 2 lines. We can always add it back when
required.

Thanks for the review again Ionela.

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel