On 6/8/2021 3:17 PM, Rikard Falkeborn wrote:

On Mon, Jun 07, 2021 at 03:09:11PM -0400, Liang, Kan wrote:

quoted

On 6/5/2021 11:56 AM, Rikard Falkeborn wrote:

quoted

These are not modified, so make them const to allow the compiler to put
them in read-only memory.

For most of the cases, yes, but the ops are modified for the TGL/RKL. We may
want to create a read-only ops for the TGL/RKL as below, and make both
skl_uncore_msr_ops and rkl_uncore_msr_ops const as well.

Thanks for the feedback.

Yes, that was the odd one out, the commit message could have been
clearer. What I meant above was "Constify all intel_uncore_ops that are
not modified", which was all but skl_uncore_msr_ops. So the patch is
correct, but in order to also constify skl_uncore_msr_ops, we could do
as you suggested. Is it worth doing?

Since the patch set changes the intel_uncore_ops *ops to const, I think 
it's better to change them all at once. It will bring confusion if some 
of them are read-only but others can be modified.

And if so, in a V2 of this patch,
or as a follow-up patch?

Either is fine for me.

Thanks,
Kan