On 9/30/2024 5:08 AM, Karol Kolacinski wrote:
From: Jacob Keller <[email protected]>

The PFTSYN_SEM, PFINT_TSYN_MSK and PFHH_SEM registers are defined in the
datasheet as "PF scoped". PF scoped registers appear in the datasheet to
have a different offset per PF. This is misleading. Each PF has its own
scope of the register, and accessing any of the offsets on a given PF
will read the PF-appropriate register. There is no reason to calculate
different offsets when reading and writing to these registers.

The original code implementing access to the semaphore registers failed
to understand this nature of PF-scoped registers and included additional
offset calculations. Remove these.

This can be tested with direct access to the registers to show that each
PF sees its own scoped register:

   user@system:~ice$ for i in {0..7}; do sudo readpci -q -s 17:00.0 \
                     -a $((0x88880 + 4*i)); done
   0x88880 == 0x00000001
   0x88884 == 0x00000001
   0x88888 == 0x00000001
   0x8888c == 0x00000001
   0x88890 == 0x00000001
   0x88894 == 0x00000001
   0x88898 == 0x00000001
   0x8889c == 0x00000001
   user@system:~ice$ for i in {0..7}; do sudo readpci -q -s 17:00.0 \
                     -a $((0x88880 + 4*i)) -w 0 ; done
   0x88880 == 0x00000000
   0x88884 == 0x00000000
   0x88888 == 0x00000000
   0x8888c == 0x00000000
   0x88890 == 0x00000000
   0x88894 == 0x00000000
   0x88898 == 0x00000000
   0x8889c == 0x00000000
   user@system:~ice$ for i in {0..7}; do sudo readpci -q -s 17:00.0 \
                     -a $((0x88880 + 4*i)); done
   0x88880 == 0x00000001
   0x88884 == 0x00000001
   0x88888 == 0x00000001
   0x8888c == 0x00000001
   0x88890 == 0x00000001
   0x88894 == 0x00000001
   0x88898 == 0x00000001
   0x8889c == 0x00000001

Additionally, you can quickly tell that the PF-offset doesn't matter
because its not an included parameter of the auto-generated register
header file. Other parameters which do matter get generated as part of
the offset macros.

Fix the uses of PFTSYN_SEM, PFINT_TSYN_MSK and PFHH_SEM to stop doing
the unnecessary offset calculation.

This also sounds like -next material rather than a bug fix.

Thanks,
Tony

Fixes: 7d606a1e2d05 ("ice: unify logic for programming PFINT_TSYN_MSK")
Fixes: 03cb4473be92 ("ice: add low level PTP clock access functions")
Fixes: 13a64f0b9894 ("ice: support crosstimestamping on E822 devices if 
supported")
Reviewed-by: Przemek Kitszel <[email protected]>
Reviewed-by: Arkadiusz Kubalewski <[email protected]>
Signed-off-by: Jacob Keller <[email protected]>
Signed-off-by: Karol Kolacinski <[email protected]>

Reply via email to