Re: [PATCH net-next 4/5] net: lan743x: Add support to SGMII 1G and 2.5G

Hi Andrew,

Thank you for review comments.

The 06/14/2022 23:13, Andrew Lunn wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

quoted

+/* MMD Device IDs */
+#define STD_DEVID                    (0x0)
+#define MMD_PMAPMD                   (0x1)
+#define MMD_PCS                              (0x3)
+#define MMD_ANEG                     (0x7)
+#define MMD_VSPEC1                   (0x1E)
+#define MMD_VSPEC2                   (0x1F)

Please use the values from include/uapi/mdio.h

Accepted. Will fix this.

quoted

+
+/* Vendor Specific SGMII MMD details */
+#define SR_MII_DEV_ID1                       0x0002
+#define SR_MII_DEV_ID2                       0x0003

MDIO_DEVID1 & MDIO_DEVID2

Not used these definitions. I will remote.

quoted

+#define SR_VSMMD_PCS_ID1             0x0004
+#define SR_VSMMD_PCS_ID2             0x0005
+#define SR_VSMMD_STS                 0x0008
+#define SR_VSMMD_CTRL                        0x0009
+
+#define SR_MII_CTRL                  0x0000
+#define SR_MII_CTRL_RST_             BIT(15)
+#define SR_MII_CTRL_LBE_             BIT(14)
+#define SR_MII_CTRL_SS13_            BIT(13)
+#define SR_MII_CTRL_AN_ENABLE_               BIT(12)
+#define SR_MII_CTRL_LPM_             BIT(11)
+#define SR_MII_CTRL_RESTART_AN_              BIT(9)
+#define SR_MII_CTRL_DUPLEX_MODE_     BIT(8)
+#define SR_MII_CTRL_SS6_             BIT(6)

These look like standard BMCR registers. Please use the values from
mii.h

Accepted. Will fix this.

quoted

+#define SR_MII_STS                   0x0001
+#define SR_MII_STS_ABL100T4_         BIT(15)
+#define SR_MII_STS_FD100ABL_         BIT(14)
+#define SR_MII_STS_HD100ABL_         BIT(13)
+#define SR_MII_STS_FD10ABL_          BIT(12)
+#define SR_MII_STS_HD10ABL_          BIT(11)
+#define SR_MII_STS_FD100T_           BIT(10)
+#define SR_MII_STS_HD100T_           BIT(9)
+#define SR_MII_STS_EXT_STS_ABL_              BIT(8)
+#define SR_MII_STS_UN_DIR_ABL_               BIT(7)
+#define SR_MII_STS_MF_PRE_SUP_               BIT(6)
+#define SR_MII_STS_AN_CMPL_          BIT(5)
+#define SR_MII_STS_RF_                       BIT(4)
+#define SR_MII_STS_AN_ABL_           BIT(3)
+#define SR_MII_STS_LINK_STS_         BIT(2)
+#define SR_MII_STS_EXT_REG_CAP_              BIT(0)

These look like BMSR.

Accepted. Will fix this.

It could even be, you can just use generic code for these.

   Andrew

-- 

Thanks,
Raju