| 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: <fbb40619-ec21-ab88-42e2-afa71bed3412@gmail.com>
Date: Tue, 16 Apr 2019 20:45:06 +0200
From: Heiner Kallweit <hkallweit1@...il.com>
To: Kavyasree.Kotagiri@...rochip.com, andrew@...n.ch,
davem@...emloft.net
Cc: netdev@...r.kernel.org, quentin.schulz@...tlin.com
Subject: Re: [PATCH v5 1/2] net: phy: mscc: add support for VSC8514 PHY.
On 16.04.2019 07:39, Kavyasree.Kotagiri@...rochip.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.kotagiri@...rochip.com>
> Signed-off-by: Quentin Schulz <quentin.schulz@...tlin.com>
> Co-developed-by: Quentin Schulz <quentin.schulz@...tlin.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.
Powered by blists - more mailing lists