On 16.04.2019 07:39, kavyasree.kotag...@microchip.com wrote:
> The VSC8514 PHY is a 4-ports PHY that is 10/100/1000BASE-T, 100BASE-FX,
> 1000BASE-X, can communicate with the MAC via QSGMII.
> The MAC interface protocol for each port within QSGMII can
> be either 1000BASE-X or SGMII, if the QSGMII MAC that the VSC8514 is
> connecting to supports this functionality.
> VSC8514 also supports SGMII MAC-side autonegotiation on each individual
> port, downshifting, can set the blinking pattern of each of its 4 LEDs,
> SyncE, 1000BASE-T Ring Resiliency as well as HP Auto-MDIX detection.
> 
> This adds support for 10BASE-T, 100BASE-TX, and 1000BASE-T,
> QSGMII link with the MAC, downshifting, HP Auto-MDIX detection
> and blinking pattern for its 4 LEDs.
> 
> The GPIO register bank is a set of registers that are common to all PHYs
> in the package. So any modification in any register of this bank affects
> all PHYs of the package.
> 
> If the PHYs haven't been reset before booting the Linux kernel and were
> configured to use interrupts for e.g. link status updates, it is
> required to clear the interrupts mask register of all PHYs before being
> able to use interrupts with any PHY. The first PHY of the package that
> will be init will take care of clearing all PHYs interrupts mask
> registers. Thus, we need to keep track of the init sequence in the
> package, if it's already been done or if it's to be done.
> 
> Most of the init sequence of a PHY of the package is common to all PHYs
> in the package, thus we use the SMI broadcast feature which enables us
> to propagate a write in one register of one PHY to all PHYs in the same
> package.
> 
> Signed-off-by: Kavya Sree Kotagiri <kavyasree.kotag...@microchip.com>
> Signed-off-by: Quentin Schulz <quentin.sch...@bootlin.com>
> Co-developed-by: Quentin Schulz <quentin.sch...@bootlin.com>
> ---
> 
> Changes in v5:
> - Added return statements in functions calling vsc85xx_csr_ctrl_phy_read().
> - Added comments in vsc85xx_csr_ctrl_phy_read() and 
> vsc85xx_csr_ctrl_phy_write(). 
> 
> Changes in v4:
> - Removed features and aneg_done settings.
> 
> Changes in v3:
> - Used BIT(x) instead of hex values.
> - Replaced magic numbers with constants.
> - Handled delays and timeouts.
> - Added comments where needed.
> 
> Changes in v2:
> - Sorted variable declarations.
> 
>  drivers/net/phy/Kconfig |   2 +-
>  drivers/net/phy/mscc.c  | 454 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 455 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 520657945b82..89085e87ecab 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -397,7 +397,7 @@ config MICROCHIP_T1_PHY
>  config MICROSEMI_PHY
>       tristate "Microsemi PHYs"
>       ---help---
> -       Currently supports VSC8530, VSC8531, VSC8540 and VSC8541 PHYs
> +       Currently supports VSC8514, VSC8530, VSC8531, VSC8540 and VSC8541 PHYs
>  
>  config NATIONAL_PHY
>       tristate "National Semiconductor PHYs"
> diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
> index db50efb30df5..3a32007dcb51 100644
> --- a/drivers/net/phy/mscc.c
> +++ b/drivers/net/phy/mscc.c
> @@ -85,12 +85,38 @@ enum rgmii_rx_clock_delay {
>  #define LED_MODE_SEL_MASK(x)           (GENMASK(3, 0) << LED_MODE_SEL_POS(x))
>  #define LED_MODE_SEL(x, mode)                  (((mode) << 
> LED_MODE_SEL_POS(x)) & LED_MODE_SEL_MASK(x))
>  
> +#define MSCC_EXT_PAGE_CSR_CNTL_17           17
> +#define MSCC_EXT_PAGE_CSR_CNTL_18           18
> +
> +#define MSCC_EXT_PAGE_CSR_CNTL_19           19
> +#define MSCC_PHY_CSR_CNTL_19_REG_ADDR(x)    (x)
> +#define MSCC_PHY_CSR_CNTL_19_TARGET(x)      ((x) << 12)
> +#define MSCC_PHY_CSR_CNTL_19_READ           BIT(14)
> +#define MSCC_PHY_CSR_CNTL_19_CMD            BIT(15)
> +
> +#define MSCC_EXT_PAGE_CSR_CNTL_20           20
> +#define MSCC_PHY_CSR_CNTL_20_TARGET(x)      (x)
> +
> +#define PHY_MCB_TARGET                    0x07
> +#define PHY_MCB_S6G_WRITE                 BIT(31)
> +#define PHY_MCB_S6G_READ                  BIT(30)
> +
> +#define PHY_S6G_SYS_RST_POS            31
> +#define PHY_S6G_ENA_LANE_POS           18
> +#define PHY_S6G_ENA_LOOP_POS           8
> +#define PHY_S6G_QRATE_POS              6
> +#define PHY_S6G_IF_MODE_POS            4
> +#define PHY_S6G_PLL_ENA_OFFS_POS       21
> +#define PHY_S6G_PLL_FSM_CTRL_DATA_POS          8
> +#define PHY_S6G_PLL_FSM_ENA_POS                7
> +
>  #define MSCC_EXT_PAGE_ACCESS           31
>  #define MSCC_PHY_PAGE_STANDARD                 0x0000 /* Standard registers 
> */
>  #define MSCC_PHY_PAGE_EXTENDED                 0x0001 /* Extended registers 
> */
>  #define MSCC_PHY_PAGE_EXTENDED_2       0x0002 /* Extended reg - page 2 */
>  #define MSCC_PHY_PAGE_EXTENDED_3       0x0003 /* Extended reg - page 3 */
>  #define MSCC_PHY_PAGE_EXTENDED_4       0x0004 /* Extended reg - page 4 */
> +#define MSCC_PHY_PAGE_CSR_CNTL                 MSCC_PHY_PAGE_EXTENDED_4
>  /* Extended reg - GPIO; this is a bank of registers that are shared for all 
> PHYs
>   * in the same package.
>   */
> @@ -216,6 +242,7 @@ enum rgmii_rx_clock_delay {
>  #define MSCC_PHY_TR_MSB                        18
>  
>  /* Microsemi PHY ID's */
> +#define PHY_ID_VSC8514                         0x00070670
>  #define PHY_ID_VSC8530                         0x00070560
>  #define PHY_ID_VSC8531                         0x00070570
>  #define PHY_ID_VSC8540                         0x00070760
> @@ -1742,6 +1769,385 @@ static int vsc8584_did_interrupt(struct phy_device 
> *phydev)
>       return (rc < 0) ? 0 : rc & MII_VSC85XX_INT_MASK_MASK;
>  }
>  
> +static int vsc8514_config_pre_init(struct phy_device *phydev)
> +{
> +     /* These are the settings to override the silicon default
> +      * values to handle hardware performance of PHY. They
> +      * are set at Power-On state and remain until PHY Reset.
> +      */
> +     const struct reg_val pre_init1[] = {
> +             {0x0f90, 0x00688980},
> +             {0x0786, 0x00000003},
> +             {0x07fa, 0x0050100f},
> +             {0x0f82, 0x0012b002},
> +             {0x1686, 0x00000004},
> +             {0x168c, 0x00d2c46f},
> +             {0x17a2, 0x00000620},
> +             {0x16a0, 0x00eeffdd},
> +             {0x16a6, 0x00071448},
> +             {0x16a4, 0x0013132f},
> +             {0x16a8, 0x00000000},
> +             {0x0ffc, 0x00c0a028},
> +             {0x0fe8, 0x0091b06c},
> +             {0x0fea, 0x00041600},
> +             {0x0f80, 0x00fffaff},
> +             {0x0fec, 0x00901809},
> +             {0x0ffe, 0x00b01007},
> +             {0x16b0, 0x00eeff00},
> +             {0x16b2, 0x00007000},
> +             {0x16b4, 0x00000814},
> +     };
> +     unsigned int i;
> +     u16 reg;
> +
> +     phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD);
> +
> +     /* all writes below are broadcasted to all PHYs in the same package */
> +     reg = phy_base_read(phydev, MSCC_PHY_EXT_CNTL_STATUS);
> +     reg |= SMI_BROADCAST_WR_EN;
> +     phy_base_write(phydev, MSCC_PHY_EXT_CNTL_STATUS, reg);
> +
> +     phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TEST);
> +
> +     reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8);
> +     reg |= BIT(15);
> +     phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg);
> +
> +     phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TR);
> +
> +     for (i = 0; i < ARRAY_SIZE(pre_init1); i++)
> +             vsc8584_csr_write(phydev, pre_init1[i].reg, pre_init1[i].val);
> +
> +     phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TEST);
> +
> +     reg = phy_base_read(phydev, MSCC_PHY_TEST_PAGE_8);
> +     reg &= ~BIT(15);
> +     phy_base_write(phydev, MSCC_PHY_TEST_PAGE_8, reg);
> +
> +     phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD);
> +
> +     reg = phy_base_read(phydev, MSCC_PHY_EXT_CNTL_STATUS);
> +     reg &= ~SMI_BROADCAST_WR_EN;
> +     phy_base_write(phydev, MSCC_PHY_EXT_CNTL_STATUS, reg);
> +
> +     return 0;
> +}
> +
> +static u32 vsc85xx_csr_ctrl_phy_read(struct phy_device *phydev,
> +                                  u32 target, u32 reg)
> +{
> +     unsigned long deadline;
> +     u32 val, val_l, val_h;
> +
> +     phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_CSR_CNTL);
> +
> +     /* 6-bit Target_ID is split between MSCC_EXT_PAGE_CSR_CNTL_20 and
> +      * MSCC_EXT_PAGE_CSR_CNTL_19 registers. Target_ID[5:2] maps to
> +      * bits[3:0] of MSCC_EXT_PAGE_CSR_CNTL_20 and  Target_ID[1:0]
> +      * maps to bits[13:12] of MSCC_EXT_PAGE_CSR_CNTL_19.
> +      */
> +
> +     /* Setup the Target ID */
> +     phy_base_write(phydev, MSCC_EXT_PAGE_CSR_CNTL_20,
> +                    MSCC_PHY_CSR_CNTL_20_TARGET(target >> 2));
> +
> +     /* Trigger CSR Action - Read into the CSR's */
> +     phy_base_write(phydev, MSCC_EXT_PAGE_CSR_CNTL_19,
> +                    MSCC_PHY_CSR_CNTL_19_CMD | MSCC_PHY_CSR_CNTL_19_READ |
> +                    MSCC_PHY_CSR_CNTL_19_REG_ADDR(reg) |
> +                    MSCC_PHY_CSR_CNTL_19_TARGET(target & 0x3));
> +
> +     /* Wait for register access*/
> +     deadline = jiffies + msecs_to_jiffies(PROC_CMD_NCOMPLETED_TIMEOUT_MS);
> +     do {
> +             usleep_range(500, 1000);
> +             val = phy_base_read(phydev, MSCC_EXT_PAGE_CSR_CNTL_19);
> +     } while (time_before(jiffies, deadline) &&
> +             !(val & MSCC_PHY_CSR_CNTL_19_CMD));
> +
> +     if (!(val & MSCC_PHY_CSR_CNTL_19_CMD))
> +             return -ETIMEDOUT;

The new series addresses half of the review comments, the other half is still
open. Like this one here, you can't return -ETIMEDOUT if your function has
return type u32. Please check the review comments again.

Reply via email to