[PATCH 1/2] phy: mxl-8611x: add driver for MaxLinear mxl-8611x PHYs
Marek Vasut
marek.vasut at mailbox.org
Thu Jul 13 00:15:56 CEST 2023
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 at 428a0000 0xa00c
> Reading from bus ethernet at 428a0000
> PHY at address 0:
> 40972 - 0x2600
>
> Signed-off-by: Nate Drude <nate.d at variscite.com>
> ---
> 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 <nate.d at variscite.com>
> + */
> +#include <common.h>
> +#include <phy.h>
> +#include <linux/bitops.h>
> +#include <linux/bitfield.h>
> +
> +/* 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 at 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 at 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
More information about the U-Boot
mailing list