From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id BE1B3EB64DA for ; Wed, 12 Jul 2023 22:16:07 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id B2EFF86B32; Thu, 13 Jul 2023 00:16:05 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=reject dis=none) header.from=mailbox.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; secure) header.d=mailbox.org header.i=@mailbox.org header.b="k5mKRqU/"; dkim=pass (2048-bit key) header.d=mailbox.org header.i=@mailbox.org header.b="PJSdhGmT"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 2935386B58; Thu, 13 Jul 2023 00:16:04 +0200 (CEST) Received: from mout-p-102.mailbox.org (mout-p-102.mailbox.org [80.241.56.152]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 0673E869F7 for ; Thu, 13 Jul 2023 00:16:01 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=reject dis=none) header.from=mailbox.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=marek.vasut@mailbox.org Received: from smtp102.mailbox.org (smtp102.mailbox.org [10.196.197.102]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-102.mailbox.org (Postfix) with ESMTPS id 4R1XBS59DMz9skX; Thu, 13 Jul 2023 00:16:00 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mailbox.org; s=mail20150812; t=1689200160; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=mhJHlQPsFpF4EtIbAWE1V4eQR6ztbmLruFy4hcrgI40=; b=k5mKRqU/vf2wWgxffgj1d4twmjYw1TBPDB+84/2lJgMKyRrICnjBZMrn5Ck4ems385s4Pm +bXGwZzHeYrx3zmxB0FxO02cy7Cv0DC70bFhBdKiUsAJMUs/PEQEQUwxN2r3NONMDEH3kS eXQV60Ezy8PmoptPMrP6hklyQRy/Pdrjd84RmHjIe88t3AgcALBHGxwt7SFphsxtQnVpIL pX8X6hSt1oEmUu8nYsIIm4UWw60FW/8BgI+uyzwB1Yg5SlkCJm5JSUoG9Kbsy4XyCcWQUF QMtrtGLdgaScjQ7TbseP9ifX/EqxCs0RiGeajjVttqtZanrLPDeknSptle4iPA== Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mailbox.org; s=mail20150812; t=1689200158; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=mhJHlQPsFpF4EtIbAWE1V4eQR6ztbmLruFy4hcrgI40=; b=PJSdhGmTKebokXCIvMnf+tRz2+h7oL//n6pxSWG+eVj+FANdhTMHLhNExnGSmSULq0Z5y/ fkTO6nt9/Bwd2KQE6UUsbc2VjsMKaP9kLUaOywK3ubWQxfVfaH5rxZgxTnmy1B70DZ70S4 DlcAIY9J015lzLaR8ybUh9AAQ31qNcjcirbvxwWLGGs0wVNTz3u8+ghDHnmgTCH/jo1sJz ETukZKuENPZriHV+1iOgUBxREm/HQD3N60BP/xJEOcOmQDNwB4XeVnyRIE5wYNWEy3cy3l sijyVdN8HpivS51PvtGK5OQU7JieyUtZ/yfaWaottSTWkhYZarqZE4Gv9k0Abw== Date: Thu, 13 Jul 2023 00:15:56 +0200 MIME-Version: 1.0 Subject: Re: [PATCH 1/2] phy: mxl-8611x: add driver for MaxLinear mxl-8611x PHYs Content-Language: en-US To: Nate Drude , Joe Hershberger , Ramon Fried , Michal Simek , Marek Vasut , u-boot@lists.denx.de Cc: Eran Matityahu References: <05c50729-f70d-6cb0-bf8b-95008e6cce50@variscite.com> <7434d330-6c84-5fee-71a3-f207967c8854@variscite.com> From: Marek Vasut In-Reply-To: <7434d330-6c84-5fee-71a3-f207967c8854@variscite.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-MBO-RS-META: t6fejzscm5pn8wrisbaunttyfp7ztpp6 X-MBO-RS-ID: f1503736a6d3e93be69 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean On 7/12/23 22:48, Nate Drude wrote: > The MxL86110 is a low power Ethernet PHY transceiver integrated > circuit following the IEEE 802.3 [1] standard. It offers a > cost-optimized solution that is well-suited for routers, > switches, and home gateways. It performs data transmission on > an Ethernet twisted pair copper cable of category CAT5e or higher. > The MxL86110 supports 1000, 100, and 10 Mbit/s data rates. > > The current driver implementation supports: > - configuring rgmii from the device tree > - configuring the LEDs from the device tree > - reading extended registers using the mdio command, e.g.: >     => mdio rx ethernet@428a0000 0xa00c >     Reading from bus ethernet@428a0000 >     PHY at address 0: >     40972 - 0x2600 > > Signed-off-by: Nate Drude > --- >  drivers/net/phy/Kconfig     |   5 + >  drivers/net/phy/Makefile    |   1 + >  drivers/net/phy/mxl-8611x.c | 271 ++++++++++++++++++++++++++++++++++++ >  3 files changed, 277 insertions(+) >  create mode 100644 drivers/net/phy/mxl-8611x.c > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index 24158776f52..0fc0fb2c5bf 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -220,6 +220,11 @@ config PHY_MICREL_KSZ8XXX > >  endif # PHY_MICREL > > +config PHY_MXL8611X > +    bool "MaxLinear MXL8611X Ethernet PHYs" > +    help > +        Add support for configuring MaxLinear MXL8611X Ethernet PHYs. Probably better just "Support for MaxLinear MXL8611X Ethernet PHYs." in the help text. >  config PHY_MSCC >      bool "Microsemi Corp Ethernet PHYs support" > > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > index 85d17f109cd..b3f4d75936c 100644 > --- a/drivers/net/phy/Makefile > +++ b/drivers/net/phy/Makefile > @@ -24,6 +24,7 @@ obj-$(CONFIG_PHY_MARVELL_10G) += marvell10g.o >  obj-$(CONFIG_PHY_MICREL_KSZ8XXX) += micrel_ksz8xxx.o >  obj-$(CONFIG_PHY_MICREL_KSZ90X1) += micrel_ksz90x1.o >  obj-$(CONFIG_PHY_MESON_GXL) += meson-gxl.o > +obj-$(CONFIG_PHY_MXL8611X) += mxl-8611x.o >  obj-$(CONFIG_PHY_NATSEMI) += natsemi.o >  obj-$(CONFIG_PHY_NXP_C45_TJA11XX) += nxp-c45-tja11xx.o >  obj-$(CONFIG_PHY_NXP_TJA11XX) += nxp-tja11xx.o > diff --git a/drivers/net/phy/mxl-8611x.c b/drivers/net/phy/mxl-8611x.c > new file mode 100644 > index 00000000000..467edc5bb5f > --- /dev/null > +++ b/drivers/net/phy/mxl-8611x.c > @@ -0,0 +1,271 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/** > + *  Driver for MaxLinear MXL861100 Ethernet PHY 611x ... you have either 1 or 0 duplicated above. > + * Copyright 2023 Variscite Ltd. > + * Copyright 2023 MaxLinear Inc. > + * Author: Nate Drude > + */ > +#include > +#include > +#include > +#include > + > +/* PHY IDs */ > +#define PHY_ID_MXL86110        0xC1335580 > +#define PHY_ID_MXL86111        0xC1335588 > + > +/* required to access extended registers */ > +#define MXL8611X_EXTD_REG_ADDR_OFFSET                0x1E > +#define MXL8611X_EXTD_REG_ADDR_DATA                0x1F > + > +/* RGMII register */ > +#define MXL8611X_EXT_RGMII_CFG1_REG                0xA003 > +#define MXL8611X_EXT_RGMII_CFG1_NO_DELAY            0 > + > +#define MXL8611X_EXT_RGMII_CFG1_RX_DELAY_MASK            GENMASK(13, 10) > +#define MXL8611X_EXT_RGMII_CFG1_TX_1G_DELAY_MASK        GENMASK(3, 0) > +#define MXL8611X_EXT_RGMII_CFG1_TX_10MB_100MB_DELAY_MASK    GENMASK(7, 4) > + > +/* LED registers and defines */ > +#define MXL8611X_LED0_CFG_REG                    0xA00C > +#define MXL8611X_LED1_CFG_REG                    0xA00D > +#define MXL8611X_LED2_CFG_REG                    0xA00E > + > +/** > + * struct mxl8611x_cfg_reg_map - map a config value to aregister value > + * @cfg        value in device configuration > + * @reg        value in the register > + */ > +struct mxl8611x_cfg_reg_map { > +    int cfg; > +    int reg; > +}; > + > +static const struct mxl8611x_cfg_reg_map mxl8611x_rgmii_delays[] = { > +    { 0, 0 }, > +    { 150, 1 }, Is this like n * 150 expanded into a table here ? Please drop. > +    { 300, 2 }, > +    { 450, 3 }, > +    { 600, 4 }, > +    { 750, 5 }, > +    { 900, 6 }, > +    { 1050, 7 }, > +    { 1200, 8 }, > +    { 1350, 9 }, > +    { 1500, 10 }, > +    { 1650, 11 }, > +    { 1800, 12 }, > +    { 1950, 13 }, > +    { 2100, 14 }, > +    { 2250, 15 }, > +    { 0, 0 } // Marks the end of the array > +}; > + > +static int mxl8611x_lookup_reg_value(const struct mxl8611x_cfg_reg_map > *tbl, > +                     const int cfg, int *reg) > +{ > +    size_t i; > + > +    for (i = 0; i == 0 || tbl[i].cfg; i++) { > +        if (tbl[i].cfg == cfg) { > +            *reg = tbl[i].reg; > +            return 0; > +        } > +    } value / 150 instead of this ? > +    return -EINVAL; > +} > + > +static u16 mxl8611x_ext_read(struct phy_device *phydev, const u32 regnum) > +{ > +    u16 val; > + > +    phy_write(phydev, MDIO_DEVAD_NONE, MXL8611X_EXTD_REG_ADDR_OFFSET, > regnum); > +    val = phy_read(phydev, MDIO_DEVAD_NONE, MXL8611X_EXTD_REG_ADDR_DATA); Is this some phy_read_mmd() reimplementation here ? > +    debug("%s: MXL86110@0x%x 0x%x=0x%x\n", __func__, phydev->addr, > regnum, val); > + > +    return val; > +} > + > +static int mxl8611x_ext_write(struct phy_device *phydev, const u32 > regnum, const u16 val) > +{ > +    debug("%s: MXL86110@0x%x 0x%x=0x%x\n", __func__, phydev->addr, > regnum, val); > + > +    phy_write(phydev, MDIO_DEVAD_NONE, MXL8611X_EXTD_REG_ADDR_OFFSET, > regnum); > + > +    return phy_write(phydev, MDIO_DEVAD_NONE, > MXL8611X_EXTD_REG_ADDR_DATA, val); > +} > + > +static int mxl8611x_extread(struct phy_device *phydev, int addr, int > devaddr, > +                   int regnum) > +{ > +    return mxl8611x_ext_read(phydev, regnum); > +} > + > +static int mxl8611x_extwrite(struct phy_device *phydev, int addr, > +                int devaddr, int regnum, u16 val) > +{ > +    return mxl8611x_ext_write(phydev, regnum, val); > +} > + > +static int mxl8611x_led_cfg(struct phy_device *phydev) > +{ > +    int ret = 0; No need to init ret to 0 here, it is inited in snprintf below. > +    int i; > +    char propname[25]; > +    u32 val; > + > +    ofnode node = phy_get_ofnode(phydev); > + > +    if (!ofnode_valid(node)) { > +        printf("%s: failed to get node\n", __func__); Try 'dev_err(phydev->dev,' instead of the printf()s > +        return -EINVAL; > +    } > + > +    /* Loop through three the LED registers */ > +    for (i = 0; i < 3; i++) { > +        /* Read property from device tree */ > +        ret = snprintf(propname, 25, "mxl-8611x,led%d_cfg", i); Instead of hardcoding 25, use 'sizeof(propname)' . > +        if (ofnode_read_u32(node, propname, &val)) > +            continue; It might be simpler to call this thrice instead of a loop here: ofnode_read_u32_default(); ...ext_write(); And just initialize the registers with default values of the prop is missing in DT. And you avoid the snprintf() too. > +        /* Update PHY LED register */ > +        mxl8611x_ext_write(phydev, MXL8611X_LED0_CFG_REG + i, val); > +    } > + > +    return 0; > +} > + > +static int mxl8611x_rgmii_cfg_of_delay(struct phy_device *phydev, const > char *property, int *val) > +{ > +    u32 of_val; > +    int ret; > + > +    ofnode node = phy_get_ofnode(phydev); > + > +    if (!ofnode_valid(node)) { > +        printf("%s: failed to get node\n", __func__); > +        return -EINVAL; > +    } > + > +    /* Get property from dts */ > +    ret = ofnode_read_u32(node, property, &of_val); > +    if (ret) > +        return ret; > + > +    /* Convert delay in ps to register value */ > +    ret = mxl8611x_lookup_reg_value(mxl8611x_rgmii_delays, of_val, val); > +    if (ret) > +        printf("%s: Error: %s = %d is invalid, using default value\n", > +               __func__, property, of_val); > + > +    return ret; > +} > + > +static int mxl8611x_rgmii_cfg(struct phy_device *phydev) > +{ > +    u32 val = 0; > +    int rxdelay, txdelay_100m, txdelay_1g; > + > +    /* Get rgmii register value */ > +    val = mxl8611x_ext_read(phydev, MXL8611X_EXT_RGMII_CFG1_REG); > + > +    /* Get RGMII Rx Delay Selection from device tree or rgmii register */ > +    if (mxl8611x_rgmii_cfg_of_delay(phydev, > "mxl-8611x,rx-internal-delay-ps", &rxdelay)) > +        rxdelay = FIELD_GET(MXL8611X_EXT_RGMII_CFG1_RX_DELAY_MASK, val); > + > +    /* Get Fast Ethernet RGMII Tx Clock Delay Selection from device > tree or rgmii register */ > +    if (mxl8611x_rgmii_cfg_of_delay(phydev, Use this form please, fix globally: ret = operation(); if (ret) handle_error(); or ret = operation(); if (ret) goto err_handler; > "mxl-8611x,tx-internal-delay-ps-100m", > +                    &txdelay_100m)) > +        txdelay_100m = > FIELD_GET(MXL8611X_EXT_RGMII_CFG1_TX_10MB_100MB_DELAY_MASK, val); > + > +    /* Get Gigabit Ethernet RGMII Tx Clock Delay Selection from device > tree or rgmii register */ > +    if (mxl8611x_rgmii_cfg_of_delay(phydev, > "mxl-8611x,tx-internal-delay-ps-1g", &txdelay_1g)) > +        txdelay_1g = > FIELD_GET(MXL8611X_EXT_RGMII_CFG1_TX_1G_DELAY_MASK, val); > + > +    switch (phydev->interface) { > +    case PHY_INTERFACE_MODE_RGMII: > +        val = MXL8611X_EXT_RGMII_CFG1_NO_DELAY; > +        break; > +    case PHY_INTERFACE_MODE_RGMII_RXID: > +        val = (val & ~MXL8611X_EXT_RGMII_CFG1_RX_DELAY_MASK) | > +            FIELD_PREP(MXL8611X_EXT_RGMII_CFG1_RX_DELAY_MASK, rxdelay); > +        break; > +    case PHY_INTERFACE_MODE_RGMII_TXID: > +        val = (val & ~MXL8611X_EXT_RGMII_CFG1_TX_10MB_100MB_DELAY_MASK) | > + > FIELD_PREP(MXL8611X_EXT_RGMII_CFG1_TX_10MB_100MB_DELAY_MASK, txdelay_100m); > +        val = (val & ~MXL8611X_EXT_RGMII_CFG1_TX_1G_DELAY_MASK) | > +            FIELD_PREP(MXL8611X_EXT_RGMII_CFG1_TX_1G_DELAY_MASK, > txdelay_1g); > +        break; > +    case PHY_INTERFACE_MODE_RGMII_ID: > +        val = (val & ~MXL8611X_EXT_RGMII_CFG1_RX_DELAY_MASK) | > +            FIELD_PREP(MXL8611X_EXT_RGMII_CFG1_RX_DELAY_MASK, rxdelay); > +        val = (val & ~MXL8611X_EXT_RGMII_CFG1_TX_10MB_100MB_DELAY_MASK) | > + > FIELD_PREP(MXL8611X_EXT_RGMII_CFG1_TX_10MB_100MB_DELAY_MASK, txdelay_100m); > +        val = (val & ~MXL8611X_EXT_RGMII_CFG1_TX_1G_DELAY_MASK) | > +            FIELD_PREP(MXL8611X_EXT_RGMII_CFG1_TX_1G_DELAY_MASK, > txdelay_1g); > +        break; > +    default: > +        printf("%s: Error: Unsupported rgmii mode %d\n", __func__, dev_err() > phydev->interface); > +        return -EINVAL; > +    } > + > +    return mxl8611x_ext_write(phydev, MXL8611X_EXT_RGMII_CFG1_REG, val); > +} > + > +static int mxl8611x_config(struct phy_device *phydev) > +{ > +    int ret; > + > +    /* Configure rgmii */ > +    ret = mxl8611x_rgmii_cfg(phydev); > + Drop newline > +    if (ret < 0) > +        return ret; > + > +    /* Configure LEDs */ > +    ret = mxl8611x_led_cfg(phydev); > + Drop newline > +    if (ret < 0) > +        return ret; > + > +    return genphy_config(phydev); > +} > + > +static int mxl86110_config(struct phy_device *phydev) > +{ > +    printf("MXL86110 PHY detected at addr %d\n", phydev->addr); Drop the printf > +    return mxl8611x_config(phydev); > +} > + > +static int mxl86111_config(struct phy_device *phydev) > +{ > +    printf("MXL86111 PHY detected at addr %d\n", phydev->addr); Drop the printf > +    return mxl8611x_config(phydev); > +} > + > +U_BOOT_PHY_DRIVER(mxl86110) = { > +    .name = "MXL86110", > +    .uid = PHY_ID_MXL86110, > +    .mask = 0xffffffff, > +    .features = PHY_GBIT_FEATURES, > +    .config = mxl86110_config, This can be unified into single PHY driver , see printf above, just tweak the .mask accordingly. > +    .startup = genphy_startup, > +    .shutdown = genphy_shutdown, > +    .readext = mxl8611x_extread, > +    .writeext = mxl8611x_extwrite, > +}; > + > +U_BOOT_PHY_DRIVER(mxl86111) = { > +    .name = "MXL86111", > +    .uid = PHY_ID_MXL86111, > +    .mask = 0xffffffff, > +    .features = PHY_GBIT_FEATURES, > +    .config = mxl86111_config, > +    .startup = genphy_startup, > +    .shutdown = genphy_shutdown, > +    .readext = mxl8611x_extread, > +    .writeext = mxl8611x_extwrite, > +}; -- Best regards, Marek Vasut