netdev - Re: [PATCH net-next 3/6] net: enetc: add definition for offset between eMAC and pMAC regs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y81kbbO+X21uVFMb@corigine.com>
Date:   Sun, 22 Jan 2023 17:29:33 +0100
From:   Simon Horman <simon.horman@...igine.com>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Xiaoliang Yang <xiaoliang.yang_1@....com>
Subject: Re: [PATCH net-next 3/6] net: enetc: add definition for offset
 between eMAC and pMAC regs

On Thu, Jan 19, 2023 at 06:04:28PM +0200, Vladimir Oltean wrote:
> This is a preliminary patch which replaces the hardcoded 0x1000 present
> in other PM1 (port MAC 1, aka pMAC) register definitions, which is an
> offset to the PM0 (port MAC 0, aka eMAC) equivalent register.
> This definition will be used in more places by future code.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> ---
>  .../net/ethernet/freescale/enetc/enetc_hw.h   | 104 +++++++++---------
>  1 file changed, 53 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> index cc8f1afdc3bc..5c88b3f2a095 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> @@ -226,6 +226,8 @@ enum enetc_bdr_type {TX, RX};
>  #define ENETC_MMCSR_ME		BIT(16)
>  #define ENETC_PTCMSDUR(n)	(0x2020 + (n) * 4) /* n = TC index [0..7] */
>  
> +#define ENETC_PMAC_OFFSET	0x1000
> +
>  #define ENETC_PM0_CMD_CFG	0x8008
>  #define ENETC_PM1_CMD_CFG	0x9008
>  #define ENETC_PM0_TX_EN		BIT(0)
> @@ -280,57 +282,57 @@ enum enetc_bdr_type {TX, RX};
>  /* Port MAC counters: Port MAC 0 corresponds to the eMAC and
>   * Port MAC 1 to the pMAC.
>   */
> -#define ENETC_PM_REOCT(mac)	(0x8100 + 0x1000 * (mac))
> -#define ENETC_PM_RALN(mac)	(0x8110 + 0x1000 * (mac))

...

> +#define ENETC_PM_REOCT(mac)	(0x8100 + ENETC_PMAC_OFFSET * (mac))
> +#define ENETC_PM_RALN(mac)	(0x8110 + ENETC_PMAC_OFFSET * (mac))

I'm not sure if it is an improvement, but did you consider something
like this? *completely untested*

#define ENETC_PM(mac, reg)	((reg) + ENETC_PMAC_OFFSET * (mac))
#define ENETC_PM_REOCT(mac)	ENETC_PM(mac, 0x8100)
#define ENETC_PM_RALN(mac)	ENETC_PM(mac, 0x8110)

...

Overall, the patchset looks good to me, FWIIW.