Re: [PATCH net-next 1/5] net: dsa: mv88e6xxx: rename TAI definitions according to core

[Vladimir Oltean <olteanv@gmail.com>]

On Mon, Sep 15, 2025 at 02:06:15PM +0100, Russell King (Oracle) wrote:
> The TAI_EVENT_STATUS and TAI_CFG definitions are only used for the
> 88E6352-family of TAI implementations. Rename them as such, and
> remove the TAI_EVENT_TIME_* definitions that are unused (although
> we read them as a block.)
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
> diff --git a/drivers/net/dsa/mv88e6xxx/ptp.h b/drivers/net/dsa/mv88e6xxx/ptp.h
> index b3fd177d67e3..67deb2f0fddb 100644
> --- a/drivers/net/dsa/mv88e6xxx/ptp.h
> +++ b/drivers/net/dsa/mv88e6xxx/ptp.h
> @@ -16,19 +16,19 @@
>  #include "chip.h"
>  
>  /* Offset 0x00: TAI Global Config */
> -#define MV88E6XXX_TAI_CFG			0x00
> -#define MV88E6XXX_TAI_CFG_CAP_OVERWRITE		0x8000
> -#define MV88E6XXX_TAI_CFG_CAP_CTR_START		0x4000
> -#define MV88E6XXX_TAI_CFG_EVREQ_FALLING		0x2000
> -#define MV88E6XXX_TAI_CFG_TRIG_ACTIVE_LO	0x1000
> -#define MV88E6XXX_TAI_CFG_IRL_ENABLE		0x0400
> -#define MV88E6XXX_TAI_CFG_TRIG_IRQ_EN		0x0200
> -#define MV88E6XXX_TAI_CFG_EVREQ_IRQ_EN		0x0100
> -#define MV88E6XXX_TAI_CFG_TRIG_LOCK		0x0080
> -#define MV88E6XXX_TAI_CFG_BLOCK_UPDATE		0x0008
> -#define MV88E6XXX_TAI_CFG_MULTI_PTP		0x0004
> -#define MV88E6XXX_TAI_CFG_TRIG_MODE_ONESHOT	0x0002
> -#define MV88E6XXX_TAI_CFG_TRIG_ENABLE		0x0001
> +#define MV88E6352_TAI_CFG			0x00
> +#define MV88E6352_TAI_CFG_CAP_OVERWRITE		0x8000
> +#define MV88E6352_TAI_CFG_CAP_CTR_START		0x4000
> +#define MV88E6352_TAI_CFG_EVREQ_FALLING		0x2000
> +#define MV88E6352_TAI_CFG_TRIG_ACTIVE_LO	0x1000
> +#define MV88E6352_TAI_CFG_IRL_ENABLE		0x0400
> +#define MV88E6352_TAI_CFG_TRIG_IRQ_EN		0x0200
> +#define MV88E6352_TAI_CFG_EVREQ_IRQ_EN		0x0100
> +#define MV88E6352_TAI_CFG_TRIG_LOCK		0x0080
> +#define MV88E6352_TAI_CFG_BLOCK_UPDATE		0x0008
> +#define MV88E6352_TAI_CFG_MULTI_PTP		0x0004
> +#define MV88E6352_TAI_CFG_TRIG_MODE_ONESHOT	0x0002
> +#define MV88E6352_TAI_CFG_TRIG_ENABLE		0x0001
>  
>  /* Offset 0x01: Timestamp Clock Period (ps) */
>  #define MV88E6XXX_TAI_CLOCK_PERIOD		0x01
> @@ -53,18 +53,16 @@
>  #define MV88E6XXX_TAI_IRL_COMP_PS		0x08
>  
>  /* Offset 0x09: Event Status */
> -#define MV88E6XXX_TAI_EVENT_STATUS		0x09
> -#define MV88E6XXX_TAI_EVENT_STATUS_ERROR	0x0200
> -#define MV88E6XXX_TAI_EVENT_STATUS_VALID	0x0100
> -#define MV88E6XXX_TAI_EVENT_STATUS_CTR_MASK	0x00ff
> -
>  /* Offset 0x0A/0x0B: Event Time */

Was it intentional to keep the comment for a register with removed
definitions, and this placement for it? It looks like this (confusing
to me):

/* Offset 0x09: Event Status */
/* Offset 0x0A/0x0B: Event Time */
#define MV88E6352_TAI_EVENT_STATUS		0x09

> -#define MV88E6XXX_TAI_EVENT_TIME_LO		0x0a
> -#define MV88E6XXX_TAI_EVENT_TYPE_HI		0x0b
> +#define MV88E6352_TAI_EVENT_STATUS		0x09
> +#define MV88E6352_TAI_EVENT_STATUS_CAP_TRIG	0x4000
> +#define MV88E6352_TAI_EVENT_STATUS_ERROR	0x0200
> +#define MV88E6352_TAI_EVENT_STATUS_VALID	0x0100
> +#define MV88E6352_TAI_EVENT_STATUS_CTR_MASK	0x00ff
>  
>  /* Offset 0x0E/0x0F: PTP Global Time */
> -#define MV88E6XXX_TAI_TIME_LO			0x0e
> -#define MV88E6XXX_TAI_TIME_HI			0x0f
> +#define MV88E6352_TAI_TIME_LO			0x0e
> +#define MV88E6352_TAI_TIME_HI			0x0f
>  
>  /* Offset 0x10/0x11: Trig Generation Time */
>  #define MV88E6XXX_TAI_TRIG_TIME_LO		0x10
> @@ -101,8 +99,8 @@
>  #define MV88E6XXX_PTP_GC_INT_STATUS		0x08
>  
>  /* Offset 0x9/0xa: Global Time */
> -#define MV88E6XXX_PTP_GC_TIME_LO		0x09
> -#define MV88E6XXX_PTP_GC_TIME_HI		0x0A
> +#define MV88E6165_PTP_GC_TIME_LO		0x09
> +#define MV88E6165_PTP_GC_TIME_HI		0x0A
>  
>  /* 6165 Per Port Registers */
>  /* Offset 0: Arrival Time 0 Status */
> -- 
> 2.47.3
>