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
