Re: [Intel-wired-lan] [PATCH iwl-net 4/5] ice: Remove unnecessary offset calculation for PF scoped registers

[Tony Nguyen <anthony.l.nguyen@intel.com>]

On 9/30/2024 5:08 AM, Karol Kolacinski wrote:
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> 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 <przemyslaw.kitszel@intel.com>
> Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>