| 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
| ||
|
Message-ID: <e7beb143-3c0d-4faf-8602-0cca63dd3b10@broadcom.com>
Date: Mon, 6 May 2024 12:25:36 -0700
From: Florian Fainelli <florian.fainelli@...adcom.com>
To: Kamil Horák - 2N <kamilh@...s.com>,
florian.fainelli@...adcom.com, bcm-kernel-feedback-list@...adcom.com,
andrew@...n.ch, hkallweit1@...il.com
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/3] net: phy: bcm54811: Add LRE registers definitions
On 5/6/24 07:40, Kamil Horák - 2N wrote:
> Add the definitions of LRE registers for Broadcom BCM5481x PHY
Missing Signed-off-by tag.
> ---
> include/linux/brcmphy.h | 91 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 90 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
> index 1394ba302367..9c0b78c1b6fb 100644
> --- a/include/linux/brcmphy.h
> +++ b/include/linux/brcmphy.h
> @@ -270,16 +270,105 @@
> #define BCM5482_SSD_SGMII_SLAVE 0x15 /* SGMII Slave Register */
> #define BCM5482_SSD_SGMII_SLAVE_EN 0x0002 /* Slave mode enable */
> #define BCM5482_SSD_SGMII_SLAVE_AD 0x0001 /* Slave auto-detection */
> +#define BCM5482_SSD_SGMII_SLAVE_AD 0x0001 /* Slave auto-detection */
> +
> +/* BroadR-Reach LRE Registers. */
> +#define MII_BCM54XX_LRECR 0x00 /* LRE Control Register */
> +#define MII_BCM54XX_LRESR 0x01 /* LRE Status Register */
> +#define MII_BCM54XX_LREPHYSID1 0x02 /* LRE PHYS ID 1 */
> +#define MII_BCM54XX_LREPHYSID2 0x03 /* LRE PHYS ID 2 */
> +#define MII_BCM54XX_LREANAA 0x04 /* LDS Auto-Negotiation Advertised Ability */
> +#define MII_BCM54XX_LREANAC 0x05 /* LDS Auto-Negotiation Advertised Control */
> +#define MII_BCM54XX_LREANPT 0x06 /* LDS Ability Next Page Transmit */
> +#define MII_BCM54XX_LRELPA 0x07 /* LDS Link Partner Ability */
> +#define MII_BCM54XX_LRELPNPM 0x08 /* LDS Link Partner Next Page Message */
> +#define MII_BCM54XX_LRELPNPC 0x09 /* LDS Link Partner Next Page Control */
> +#define MII_BCM54XX_LRELDSE 0x0a /* LDS Expansion Register */
> +#define MII_BCM54XX_LREES 0x0f /* LRE Extended Status */
> +
> +/* LRE control register. */
> +#define LRECR_RESET 0x8000 /* Reset to default state */
> +#define LRECR_LOOPBACK 0x4000 /* Internal Loopback */
> +#define LRECR_LDSRES 0x2000 /* Restart LDS Process */
> +#define LRECR_LDSEN 0x1000 /* LDS Enable */
> +#define LRECR_PDOWN 0x0800 /* Enable low power state */
> +#define LRECR_ISOLATE 0x0400 /* Isolate data paths from MII */
> +#define LRECR_SPEED100 0x0200 /* Select 100 Mbps */
> +#define LRECR_SPEED10 0x0000 /* Select 10 Mbps */
> +#define LRECR_4PAIRS 0x0020 /* Select 4 Pairs */
> +#define LRECR_2PAIRS 0x0010 /* Select 2 Pairs */
> +#define LRECR_1PAIR 0x0000 /* Select 1 Pair */
> +#define LRECR_MASTER 0x0008 /* Force Master when LDS disabled */
> +#define LRECR_SLAVE 0x0000 /* Force Slave when LDS disabled */
So previous register had definitions ordered from MSB to LSB...
> +
> +/* LRE status register. */
> +#define LRESR_ERCAP 0x0001 /* Ext-reg capability */
and here we have LSB to MSB, seems like you chose MSB to LSB, so we
should stick to that.
> +#define LRESR_JCD 0x0002 /* Jabber detected */
> +#define LRESR_LSTATUS 0x0004 /* Link status */
> +#define LRESR_LDSABILITY 0x0008 /* Can do LDS */
Comment should be LDS auto-negotiation capable, other comments are fine.
> +#define LRESR_8023 0x0010 /* Has IEEE 802.3 Support */
> +#define LRESR_LDSCOMPLETE 0x0020 /* LDS Auto-negotiation complete */
> +#define LRESR_MFPS 0x0040 /* Can suppress Management Frames Preamble */
> +#define LRESR_RESV 0x0080 /* Unused... */
> +#define LRESR_ESTATEN 0x0100 /* Extended Status in R15 */
> +#define LRESR_10_1PAIR 0x0200 /* Can do 10Mbps 1 Pair */
> +#define LRESR_10_2PAIR 0x0400 /* Can do 10Mbps 2 Pairs */
> +#define LRESR_100_2PAIR 0x0800 /* Can do 100Mbps 2 Pairs */
> +#define LRESR_100_4PAIR 0x1000 /* Can do 100Mbps 4 Pairs */
> +#define LRESR_100_1PAIR 0x2000 /* Can do 100Mbps 1 Pair */
> +
> +/* LDS Auto-Negotiation Advertised Ability. */
> +#define LREANAA_PAUSE_ASYM 0x8000 /* Can pause asymmetrically */
> +#define LREANAA_PAUSE 0x4000 /* Can pause */
> +#define LREANAA_100_1PAIR 0x0020 /* Can do 100Mbps 1 Pair */
> +#define LREANAA_100_4PAIR 0x0010 /* Can do 100Mbps 4 Pair */
> +#define LREANAA_100_2PAIR 0x0008 /* Can do 100Mbps 2 Pair */
> +#define LREANAA_10_2PAIR 0x0004 /* Can do 10Mbps 2 Pair */
> +#define LREANAA_10_1PAIR 0x0002 /* Can do 10Mbps 1 Pair */
> +
> +#define LRE_ADVERTISE_FULL (LREANAA_100_1PAIR | LREANAA_100_4PAIR | \
> + LREANAA_100_2PAIR | LREANAA_10_2PAIR | \
> + LREANAA_10_1PAIR)
> +
> +#define LRE_ADVERTISE_ALL LRE_ADVERTISE_FULL
> +
> +/* LDS Link Partner Ability. */
> +#define LRELPA_PAUSE_ASYM 0x8000 /* Supports asymmetric pause */
> +#define LRELPA_PAUSE 0x4000 /* Supports pause capability */
> +#define LRELPA_100_1PAIR 0x0020 /* 100Mbps 1 Pair capable */
> +#define LRELPA_100_4PAIR 0x0010 /* 100Mbps 4 Pair capable */
> +#define LRELPA_100_2PAIR 0x0008 /* 100Mbps 2 Pair capable */
> +#define LRELPA_10_2PAIR 0x0004 /* 10Mbps 2 Pair capable */
> +#define LRELPA_10_1PAIR 0x0002 /* 10Mbps 1 Pair capable */
> +
> +/* LDS Expansion register. */
> +#define LDSE_DOWNGRADE 0x8000 /* Can do LDS Speed Downgrade */
> +#define LDSE_MASTER 0x4000 /* Master / Slave */
> +#define LDSE_PAIRS_MASK 0x3000 /* Pair Count Mask */
Please define LDSE_PAIRS_SHIFT and make the LDSE_%dPAIRS left shift by 12.
> +#define LDSE_4PAIRS 0x2000 /* 4 Pairs Connection */
> +#define LDSE_2PAIRS 0x1000 /* 2 Pairs Connection */
> +#define LDSE_1PAIR 0x0000 /* 1 Pair Connection */
> +#define LDSE_CABLEN_MASK 0x0FFF /* Cable Length Mask */
>
> /* BCM54810 Registers */
> #define BCM54810_EXP_BROADREACH_LRE_MISC_CTL (MII_BCM54XX_EXP_SEL_ER + 0x90)
> #define BCM54810_EXP_BROADREACH_LRE_MISC_CTL_EN (1 << 0)
> #define BCM54810_SHD_CLK_CTL 0x3
> #define BCM54810_SHD_CLK_CTL_GTXCLK_EN (1 << 9)
> +#define BCM54810_SHD_SCR3_TRDDAPD 0x0100
Unrelated change?
> +
> +/* BCM54811 Registers */
> +#define BCM54811_EXP_BROADREACH_LRE_OVERLAY_CTL (MII_BCM54XX_EXP_SEL_ER + 0x9A)
> +/* Access Control Override Enable */
> +#define BCM54811_EXP_BROADREACH_LRE_OVERLAY_CTL_EN BIT(15)
> +/* Access Control Override Value */
> +#define BCM54811_EXP_BROADREACH_LRE_OVERLAY_CTL_OVERRIDE_VAL BIT(14)
> +/* Access Control Value */
> +#define BCM54811_EXP_BROADREACH_LRE_OVERLAY_CTL_VAL BIT(13)
This register is not documented in my datasheet, but register 0x0E is, I
am fine with using that one though.
>
> /* BCM54612E Registers */
> #define BCM54612E_EXP_SPARE0 (MII_BCM54XX_EXP_SEL_ETC + 0x34)
> -#define BCM54612E_LED4_CLK125OUT_EN (1 << 1)
> +#define BCM54612E_LED4_CLK125OUT_EN BIT(1)
Unrelated change, and one that is arguably inconsistent with the
definitions you are adding, I would stick with the style you used, since
there are precedents for that in brcmphy.h.
--
Florian
Download attachment "smime.p7s" of type "application/pkcs7-signature" (4221 bytes)
Powered by blists - more mailing lists