On Wed, 2025-07-23 at 11:06 -0400, Sasha Levin wrote:
On Wed, Jul 23, 2025 at 06:48:08AM -0700, srinivas pandruvada wrote:
On Wed, 2025-07-23 at 12:11 +0530, Harshit Mogalapalli wrote:
- /* Check if the ENABLE bit was actually changed */
- ret = rapl_read_data_raw(rd, PL1_ENABLE, true, &val);
- if (ret) {
cpus_read_unlock();
return ret;
- }
Shouldn't this be rapl_read_data_raw(rd, PL1_ENABLE, false, &val); ?
Correct. This will result in additional call to rapl_unit_xlate(), but since for primitive PL1_ENABLE, the unit is ARBITRARY_UNIT, this will not translate and return the same value.
I was thinking we need to call rapl_unit_xlate() so we won't just get whatever the raw value is, but yes - since it's ARBITRARY_UNIT then it won't really do much.
I guess in this case it doesn't really matter if we pass true or false here?
correct, you just save one additional call.
Thanks, Srinivas