[PATCH] phy: adin: add readext and writeext support for mdio cmd

The adin phy has extended registers that can be accessed using adin_ext_read and adin_ext_write. These registers can be read directly using the mdio command using readext and writext. For example:
=> mdio rx ethernet@428a0000 0xff23 Reading from bus ethernet@428a0000 PHY at address 0: 65315 - 0xe01
Signed-off-by: Nate Drude nate.d@variscite.com --- drivers/net/phy/adin.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index fb9f1e4c70d..d29210adffc 100644 --- a/drivers/net/phy/adin.c +++ b/drivers/net/phy/adin.c @@ -155,6 +155,18 @@ static int adin_ext_write(struct phy_device *phydev, const u32 regnum, const u16 return phy_write(phydev, MDIO_DEVAD_NONE, ADIN1300_EXT_REG_DATA, val); }
+static int adin_extread(struct phy_device *phydev, int addr, int devaddr, + int regnum) +{ + return adin_ext_read(phydev, regnum); +} + +static int adin_extwrite(struct phy_device *phydev, int addr, + int devaddr, int regnum, u16 val) +{ + return adin_ext_write(phydev, regnum, val); +} + static int adin_config_clk_out(struct phy_device *phydev) { ofnode node = phy_get_ofnode(phydev); @@ -260,4 +272,6 @@ U_BOOT_PHY_DRIVER(ADIN1300) = { .config = adin1300_config, .startup = genphy_startup, .shutdown = genphy_shutdown, + .readext = adin_extread, + .writeext = adin_extwrite, };

On 7/11/23 23:59, Nate Drude wrote:
The adin phy has extended registers that can be accessed using adin_ext_read and adin_ext_write. These registers can be read directly using the mdio command using readext and writext. For example:
=> mdio rx ethernet@428a0000 0xff23 Reading from bus ethernet@428a0000 PHY at address 0: 65315 - 0xe01
Signed-off-by: Nate Drude nate.d@variscite.com
Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org

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 nate.d@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. + 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 + * + * Copyright 2023 Variscite Ltd. + * Copyright 2023 MaxLinear Inc. + * Author: Nate Drude nate.d@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 }, + { 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; + } + } + + 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); + + 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; + 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__); + 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); + if (ofnode_read_u32(node, propname, &val)) + continue; + + /* 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, "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__, 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); + + if (ret < 0) + return ret; + + /* Configure LEDs */ + ret = mxl8611x_led_cfg(phydev); + + 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); + return mxl8611x_config(phydev); +} + +static int mxl86111_config(struct phy_device *phydev) +{ + printf("MXL86111 PHY detected at addr %d\n", phydev->addr); + return mxl8611x_config(phydev); +} + +U_BOOT_PHY_DRIVER(mxl86110) = { + .name = "MXL86110", + .uid = PHY_ID_MXL86110, + .mask = 0xffffffff, + .features = PHY_GBIT_FEATURES, + .config = mxl86110_config, + .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, +};

The MXL8611X driver has custom bindings for configuring the LEDs and RGMII internal delays. This patch adds the documentation and defines necessary to configure it from the device tree.
Signed-off-by: Nate Drude nate.d@variscite.com --- .../net/phy/mxl-8611x.txt | 37 +++++++++++++++++++ include/dt-bindings/net/mxl-8611x.h | 27 ++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 doc/device-tree-bindings/net/phy/mxl-8611x.txt create mode 100644 include/dt-bindings/net/mxl-8611x.h
diff --git a/doc/device-tree-bindings/net/phy/mxl-8611x.txt b/doc/device-tree-bindings/net/phy/mxl-8611x.txt new file mode 100644 index 00000000000..462fdf61666 --- /dev/null +++ b/doc/device-tree-bindings/net/phy/mxl-8611x.txt @@ -0,0 +1,37 @@ +* MaxLinear MXL8611x PHY Device Tree binding + +Required properties: +- reg: PHY address + +Optional properties: +- mxl-8611x,ledN_cfg: Register configuration for COM_EXT_LED0_CFG, + COM_EXT_LED1_CFG, and COM_EXT_LED2_CFG +- mxl-8611x,rx-internal-delay-ps: RGMII RX Clock Delay used only when PHY operates + in RGMII mode with internal delay (phy-mode is 'rgmii-id' or + 'rgmii-rxid') in pico-seconds. +- mxl-8611x,tx-internal-delay-ps-100m: RGMII TX Clock Delay used only when PHY operates + in 10/100M RGMII mode with internal delay (phy-mode is 'rgmii-id' or + 'rgmii-txid') in pico-seconds. +- mxl-8611x,tx-internal-delay-ps-1g: RGMII TX Clock Delay used only when PHY operates + in 1G RGMII mode with internal delay (phy-mode is 'rgmii-id' or + 'rgmii-txid') in pico-seconds. + +Example: + + ethernet-phy@0 { + reg = <0>; + + mxl-8611x,led0_cfg = <( + MXL8611X_LEDX_CFG_LINK_UP_RX_ACT_ON | + MXL8611X_LEDX_CFG_LINK_UP_TX_ACT_ON | + MXL8611X_LEDX_CFG_TRAFFIC_ACT_BLINK_IND + )>; + mxl-8611x,led1_cfg = <( + MXL8611X_LEDX_CFG_LINK_UP_10MB_ON | + MXL8611X_LEDX_CFG_LINK_UP_100MB_ON | + MXL8611X_LEDX_CFG_LINK_UP_1GB_ON + )>; + mxl-8611x,rx-internal-delay-ps = <0>; + mxl-8611x,tx-internal-delay-ps-100m = <2250>; + mxl-8611x,tx-internal-delay-ps-1g = <150>; + }; diff --git a/include/dt-bindings/net/mxl-8611x.h b/include/dt-bindings/net/mxl-8611x.h new file mode 100644 index 00000000000..cb0ec0f8bd0 --- /dev/null +++ b/include/dt-bindings/net/mxl-8611x.h @@ -0,0 +1,27 @@ +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */ +/* + * Device Tree constants for MaxLinear MXL8611x PHYs + * + * Copyright 2023 Variscite Ltd. + * Copyright 2023 MaxLinear Inc. + */ + +#ifndef _DT_BINDINGS_MXL_8611X_H +#define _DT_BINDINGS_MXL_8611X_H + +#define MXL8611X_LEDX_CFG_TRAFFIC_ACT_BLINK_IND (1 << 13) +#define MXL8611X_LEDX_CFG_LINK_UP_FULL_DUPLEX_ON (1 << 12) +#define MXL8611X_LEDX_CFG_LINK_UP_HALF_DUPLEX_ON (1 << 11) +#define MXL8611X_LEDX_CFG_LINK_UP_TX_ACT_ON (1 << 10) +#define MXL8611X_LEDX_CFG_LINK_UP_RX_ACT_ON (1 << 9) +#define MXL8611X_LEDX_CFG_LINK_UP_TX_ON (1 << 8) +#define MXL8611X_LEDX_CFG_LINK_UP_RX_ON (1 << 7) +#define MXL8611X_LEDX_CFG_LINK_UP_1GB_ON (1 << 6) +#define MXL8611X_LEDX_CFG_LINK_UP_100MB_ON (1 << 5) +#define MXL8611X_LEDX_CFG_LINK_UP_10MB_ON (1 << 4) +#define MXL8611X_LEDX_CFG_LINK_UP_COLLISION (1 << 3) +#define MXL8611X_LEDX_CFG_LINK_UP_1GB_BLINK (1 << 2) +#define MXL8611X_LEDX_CFG_LINK_UP_100MB_BLINK (1 << 1) +#define MXL8611X_LEDX_CFG_LINK_UP_10MB_BLINK (1 << 0) + +#endif

On 7/12/23 22:50, Nate Drude wrote:
The MXL8611X driver has custom bindings for configuring the LEDs and RGMII internal delays. This patch adds the documentation and defines necessary to configure it from the device tree.
Signed-off-by: Nate Drude nate.d@variscite.com
Is this binding document picked from Linux ?
If so, include commit ID in the commit message.
If not, why ?

Hi Marek,
On 7/12/23 5:16 PM, Marek Vasut wrote:
[You don't often get email from marek.vasut@mailbox.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
On 7/12/23 22:50, Nate Drude wrote:
The MXL8611X driver has custom bindings for configuring the LEDs and RGMII internal delays. This patch adds the documentation and defines necessary to configure it from the device tree.
Signed-off-by: Nate Drude nate.d@variscite.com
Is this binding document picked from Linux ?
If so, include commit ID in the commit message.
If not, why ?
There is no driver pushed to Linux yet. There is a vendor driver that is not ready for mainline.
The vendor driver does not support any device tree configuration. Thus, these bindings are new and are aligned with the Variscite kernel, which includes a patched version of the vendor driver: https://github.com/varigit/linux-imx/compare/fb80db2fc3ba1ccb3c7503495c4544e...
The defines in mxl-8611x.h are copied from the vendor driver.
Thanks, Nate

On 7/13/23 02:12, Nate Drude wrote:
Hi Marek,
On 7/12/23 5:16 PM, Marek Vasut wrote:
[You don't often get email from marek.vasut@mailbox.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
On 7/12/23 22:50, Nate Drude wrote:
The MXL8611X driver has custom bindings for configuring the LEDs and RGMII internal delays. This patch adds the documentation and defines necessary to configure it from the device tree.
Signed-off-by: Nate Drude nate.d@variscite.com
Is this binding document picked from Linux ?
If so, include commit ID in the commit message.
If not, why ?
There is no driver pushed to Linux yet. There is a vendor driver that is not ready for mainline.
The vendor driver does not support any device tree configuration. Thus, these bindings are new and are aligned with the Variscite kernel, which includes a patched version of the vendor driver: https://github.com/varigit/linux-imx/compare/fb80db2fc3ba1ccb3c7503495c4544e...
The defines in mxl-8611x.h are copied from the vendor driver.
Please send those bindings to devicetree@ so they get reviewed by DT maintainers too. Otherwise U-Boot might end up using divergent bindings from what is going to land in Linux. Then add a link into the patch to the submission at lore.kernel.org .
Thanks

On 7/12/23 3:50 PM, Nate Drude wrote:
The MXL8611X driver has custom bindings for configuring the LEDs and RGMII internal delays. This patch adds the documentation and defines necessary to configure it from the device tree.
Signed-off-by: Nate Drude nate.d@variscite.com
.../net/phy/mxl-8611x.txt | 37 +++++++++++++++++++ include/dt-bindings/net/mxl-8611x.h | 27 ++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 doc/device-tree-bindings/net/phy/mxl-8611x.txt create mode 100644 include/dt-bindings/net/mxl-8611x.h
diff --git a/doc/device-tree-bindings/net/phy/mxl-8611x.txt b/doc/device-tree-bindings/net/phy/mxl-8611x.txt new file mode 100644 index 00000000000..462fdf61666 --- /dev/null +++ b/doc/device-tree-bindings/net/phy/mxl-8611x.txt @@ -0,0 +1,37 @@ +* MaxLinear MXL8611x PHY Device Tree binding
+Required properties: +- reg: PHY address
+Optional properties: +- mxl-8611x,ledN_cfg: Register configuration for COM_EXT_LED0_CFG, + COM_EXT_LED1_CFG, and COM_EXT_LED2_CFG +- mxl-8611x,rx-internal-delay-ps: RGMII RX Clock Delay used only when PHY operates + in RGMII mode with internal delay (phy-mode is 'rgmii-id' or + 'rgmii-rxid') in pico-seconds. +- mxl-8611x,tx-internal-delay-ps-100m: RGMII TX Clock Delay used only when PHY operates + in 10/100M RGMII mode with internal delay (phy-mode is 'rgmii-id' or + 'rgmii-txid') in pico-seconds. +- mxl-8611x,tx-internal-delay-ps-1g: RGMII TX Clock Delay used only when PHY operates + in 1G RGMII mode with internal delay (phy-mode is 'rgmii-id' or + 'rgmii-txid') in pico-seconds.
+Example:
+ ethernet-phy@0 { + reg = <0>;
+ mxl-8611x,led0_cfg = <( + MXL8611X_LEDX_CFG_LINK_UP_RX_ACT_ON | + MXL8611X_LEDX_CFG_LINK_UP_TX_ACT_ON | + MXL8611X_LEDX_CFG_TRAFFIC_ACT_BLINK_IND + )>; + mxl-8611x,led1_cfg = <( + MXL8611X_LEDX_CFG_LINK_UP_10MB_ON | + MXL8611X_LEDX_CFG_LINK_UP_100MB_ON | + MXL8611X_LEDX_CFG_LINK_UP_1GB_ON + )>; + mxl-8611x,rx-internal-delay-ps = <0>; + mxl-8611x,tx-internal-delay-ps-100m = <2250>; + mxl-8611x,tx-internal-delay-ps-1g = <150>; + }; diff --git a/include/dt-bindings/net/mxl-8611x.h b/include/dt-bindings/net/mxl-8611x.h new file mode 100644 index 00000000000..cb0ec0f8bd0 --- /dev/null +++ b/include/dt-bindings/net/mxl-8611x.h @@ -0,0 +1,27 @@ +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */ +/*
- Device Tree constants for MaxLinear MXL8611x PHYs
- Copyright 2023 Variscite Ltd.
- Copyright 2023 MaxLinear Inc.
- */
+#ifndef _DT_BINDINGS_MXL_8611X_H +#define _DT_BINDINGS_MXL_8611X_H
+#define MXL8611X_LEDX_CFG_TRAFFIC_ACT_BLINK_IND (1 << 13) +#define MXL8611X_LEDX_CFG_LINK_UP_FULL_DUPLEX_ON (1 << 12) +#define MXL8611X_LEDX_CFG_LINK_UP_HALF_DUPLEX_ON (1 << 11) +#define MXL8611X_LEDX_CFG_LINK_UP_TX_ACT_ON (1 << 10) +#define MXL8611X_LEDX_CFG_LINK_UP_RX_ACT_ON (1 << 9) +#define MXL8611X_LEDX_CFG_LINK_UP_TX_ON (1 << 8) +#define MXL8611X_LEDX_CFG_LINK_UP_RX_ON (1 << 7) +#define MXL8611X_LEDX_CFG_LINK_UP_1GB_ON (1 << 6) +#define MXL8611X_LEDX_CFG_LINK_UP_100MB_ON (1 << 5) +#define MXL8611X_LEDX_CFG_LINK_UP_10MB_ON (1 << 4) +#define MXL8611X_LEDX_CFG_LINK_UP_COLLISION (1 << 3) +#define MXL8611X_LEDX_CFG_LINK_UP_1GB_BLINK (1 << 2) +#define MXL8611X_LEDX_CFG_LINK_UP_100MB_BLINK (1 << 1) +#define MXL8611X_LEDX_CFG_LINK_UP_10MB_BLINK (1 << 0)
+#endif
I mistakenly sent the mxl-8611x patch series In-Reply-To my previous adin patch.
Is this okay, or should I resend the mxl-8611x patches?
Thanks, Nate

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 nate.d@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@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@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, +};

Hi Marek,
On 7/12/23 5:15 PM, Marek Vasut wrote:
[You don't often get email from marek.vasut@mailbox.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
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 nate.d@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@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@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
Thanks for your review. I will make these changes and send a v2 of the patches.
Sincerely, Nate

On Tue, Jul 11, 2023 at 04:59:15PM -0500, Nate Drude wrote:
The adin phy has extended registers that can be accessed using adin_ext_read and adin_ext_write. These registers can be read directly using the mdio command using readext and writext. For example:
=> mdio rx ethernet@428a0000 0xff23 Reading from bus ethernet@428a0000 PHY at address 0: 65315 - 0xe01
Signed-off-by: Nate Drude nate.d@variscite.com Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org
Applied to u-boot/master, thanks!
participants (3)
-
Marek Vasut
-
Nate Drude
-
Tom Rini