Hi Anshul,
On Thu, 30 Oct 2025 18:33:22 +0530, Anshul Dalal wrote:
> +char k3_get_speed_grade(void)
> +{
> + u32 efuse_val = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID);
> + u32 efuse_speed = (efuse_val & JTAG_DEV_SPEED_MASK) >>
> + JTAG_DEV_SPEED_SHIFT;
> +
> + char speed_grade = ('A' - 1) + efuse_speed;
> +
> + /* Speed grades for AM62a are not sequential */
> + switch (efuse_speed) {
> + case 'T':
> + return 'V';
> + case 'V':
> + return 'T';
> + default:
> + return speed_grade;
> + }
> +}
I think the T/V swap here never actually takes effect.
JTAG_DEV_SPEED_MASK is GENMASK(10, 6), so after the shift efuse_speed is
just the raw 5-bit field value, i.e. somewhere in 0..31. The switch then
compares that raw value against the ASCII character literals 'T' (84) and
'V' (86), which are well outside 0..31. Those two case labels are
therefore unreachable and the switch always falls through to default,
returning the plain sequential decode. The swap described by the comment
never happens.
In practice this is harmless today: k3_get_speed_grade()'s only consumer
is the A53 clock-rate fixup, and grades S/T/U/V all map to the same
frequency in am62a_map, so swapping T and V can't change the result
either way. So this is more of a correctness/readability fix than an
active bug.
I believe you want to switch on the decoded letter rather than the raw
efuse value:
/* Speed grades for AM62a are not sequential */
switch (speed_grade) {
case 'T':
return 'V';
case 'V':
return 'T';
default:
return speed_grade;
}
Since this series has already been merged (it's in mainline as of the
2025-11-12 pull), this would need a follow-up fix on top rather than a
respin -- happy to send one if that's easier for you.
Thanks,
Jonathan
