lkml.org 
[lkml]   [2024]   [May]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 2/3] net: phy: bcm54811: Add LRE registers definitions
From
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

[unhandled content-type:application/pkcs7-signature]
\
 
 \ /
  Last update: 2024-05-27 18:17    [W:0.144 / U:0.744 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site