| Message ID | 20200420185310.6630-3-dmurphy@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,1/3] net: phy: Add a generic phy file for TI generic PHYs | expand |
On 20. 04. 20 20:53, Dan Murphy wrote: > Port the DP83869 kernel driver. > > Signed-off-by: Dan Murphy <dmurphy at ti.com> > --- > drivers/net/phy/Kconfig | 4 + > drivers/net/phy/Makefile | 1 + > drivers/net/phy/dp83869.c | 440 +++++++++++++++++++++++++++ > drivers/net/phy/ti_phy_init.c | 4 + > drivers/net/phy/ti_phy_init.h | 1 + > include/dt-bindings/net/ti-dp83869.h | 42 +++ > 6 files changed, 492 insertions(+) > create mode 100644 drivers/net/phy/dp83869.c > create mode 100644 include/dt-bindings/net/ti-dp83869.h > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index e366f10afc59..5f14bdba0a3b 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -252,6 +252,10 @@ config PHY_DP83867 > select PHY_TI > bool "Texas Instruments Ethernet DP83867 PHY support" > > +config PHY_DP83869 > + select PHY_TI > + bool "Texas Instruments Ethernet DP83869 PHY support" isn't this a little bit short? I expect checkpatch should be reporting issue with it. > + > config PHY_VITESSE > bool "Vitesse Ethernet PHYs support" > > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > index 9c6d31718c00..f8797319154f 100644 > --- a/drivers/net/phy/Makefile > +++ b/drivers/net/phy/Makefile > @@ -27,6 +27,7 @@ obj-$(CONFIG_PHY_SMSC) += smsc.o > obj-$(CONFIG_PHY_TERANETICS) += teranetics.o > obj-$(CONFIG_PHY_TI) += ti_phy_init.o > obj-$(CONFIG_PHY_DP83867) += dp83867.o > +obj-$(CONFIG_PHY_DP83869) += dp83869.o > obj-$(CONFIG_PHY_XILINX) += xilinx_phy.o > obj-$(CONFIG_PHY_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o > obj-$(CONFIG_PHY_VITESSE) += vitesse.o > diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c > new file mode 100644 > index 000000000000..1eaaea20b37a > --- /dev/null > +++ b/drivers/net/phy/dp83869.c > @@ -0,0 +1,440 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Driver for the Texas Instruments DP83869 PHY > + * Copyright (C) 2019 Texas Instruments Inc. 2020? > + */ > + > +#include <common.h> > +#include <phy.h> > +#include <dm/devres.h> > +#include <linux/compat.h> > +#include <malloc.h> > + > +#include <dm.h> > + > +#include <dt-bindings/net/ti-dp83869.h> > + > +#include "ti_phy_init.h" > + > +#define DP83869_PHY_ID 0x2000a0f1 > +#define DP83869_DEVADDR 0x1f > + > +#define MII_DP83869_PHYCTRL 0x10 > +#define MII_DP83869_MICR 0x12 > +#define MII_DP83869_ISR 0x13 > +#define DP83869_CTRL 0x1f > +#define DP83869_CFG4 0x1e > + > +/* Extended Registers */ > +#define DP83869_GEN_CFG3 0x0031 > +#define DP83869_RGMIICTL 0x0032 > +#define DP83869_STRAP_STS1 0x006e > +#define DP83869_RGMIIDCTL 0x0086 > +#define DP83869_IO_MUX_CFG 0x0170 > +#define DP83869_OP_MODE 0x01df > +#define DP83869_FX_CTRL 0x0c00 > + > +#define DP83869_SW_RESET BIT(15) > +#define DP83869_SW_RESTART BIT(14) > + > +/* MICR Interrupt bits */ > +#define MII_DP83869_MICR_AN_ERR_INT_EN BIT(15) > +#define MII_DP83869_MICR_SPEED_CHNG_INT_EN BIT(14) > +#define MII_DP83869_MICR_DUP_MODE_CHNG_INT_EN BIT(13) > +#define MII_DP83869_MICR_PAGE_RXD_INT_EN BIT(12) > +#define MII_DP83869_MICR_AUTONEG_COMP_INT_EN BIT(11) > +#define MII_DP83869_MICR_LINK_STS_CHNG_INT_EN BIT(10) > +#define MII_DP83869_MICR_FALSE_CARRIER_INT_EN BIT(8) > +#define MII_DP83869_MICR_SLEEP_MODE_CHNG_INT_EN BIT(4) > +#define MII_DP83869_MICR_WOL_INT_EN BIT(3) > +#define MII_DP83869_MICR_XGMII_ERR_INT_EN BIT(2) > +#define MII_DP83869_MICR_POL_CHNG_INT_EN BIT(1) > +#define MII_DP83869_MICR_JABBER_INT_EN BIT(0) > + > +#define MII_DP83869_BMCR_DEFAULT (BMCR_ANENABLE | \ > + BMCR_FULLDPLX | \ > + BMCR_SPEED1000) > + > +#define MII_DP83869_FIBER_ADVERTISE (ADVERTISE_CSMA | \ > + ADVERTISE_1000XHALF | \ > + ADVERTISE_1000XFULL | \ > + ADVERTISE_1000XPAUSE | \ > + ADVERTISE_1000XPSE_ASYM) > + > +/* This is the same bit mask as the BMCR so re-use the BMCR default */ > +#define DP83869_FX_CTRL_DEFAULT MII_DP83869_BMCR_DEFAULT > + > +/* CFG1 bits */ > +#define DP83869_CFG1_DEFAULT (ADVERTISE_1000HALF | \ > + ADVERTISE_1000FULL | \ > + CTL1000_AS_MASTER) > + > +/* RGMIICTL bits */ > +#define DP83869_RGMII_TX_CLK_DELAY_EN BIT(1) > +#define DP83869_RGMII_RX_CLK_DELAY_EN BIT(0) > + > +/* STRAP_STS1 bits */ > +#define DP83869_STRAP_OP_MODE_MASK GENMASK(2, 0) > +#define DP83869_STRAP_STS1_RESERVED BIT(11) > +#define DP83869_STRAP_MIRROR_ENABLED BIT(12) > + > +/* PHYCTRL bits */ > +#define DP83869_RX_FIFO_SHIFT 12 > +#define DP83869_TX_FIFO_SHIFT 14 > + > +/* PHY_CTRL lower bytes 0x48 are declared as reserved */ > +#define DP83869_PHY_CTRL_DEFAULT 0x48 > +#define DP83869_PHYCR_FIFO_DEPTH_MASK GENMASK(15, 12) > +#define DP83869_PHYCR_RESERVED_MASK BIT(11) > + > +/* RGMIIDCTL bits */ > +#define DP83869_RGMII_TX_CLK_DELAY_SHIFT 4 > + > +/* IO_MUX_CFG bits */ > +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_CTRL 0x1f > + > +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_MAX 0x0 > +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_MIN 0x1f > +#define DP83869_IO_MUX_CFG_CLK_O_SEL_MASK (0x1f << 8) > +#define DP83869_IO_MUX_CFG_CLK_O_SEL_SHIFT 8 > + > +/* CFG3 bits */ > +#define DP83869_CFG3_PORT_MIRROR_EN BIT(0) > + > +/* CFG4 bits */ > +#define DP83869_INT_OE BIT(7) > + > +/* OP MODE */ > +#define DP83869_OP_MODE_MII BIT(5) > +#define DP83869_SGMII_RGMII_BRIDGE BIT(6) > + > +enum { > + DP83869_PORT_MIRRORING_KEEP, > + DP83869_PORT_MIRRORING_EN, > + DP83869_PORT_MIRRORING_DIS, > +}; We have met with this in the kernel. Greg was asking us to write exact value to all enum entries. > + > +struct dp83869_private { > + int tx_fifo_depth; > + int rx_fifo_depth; > + int io_impedance; > + int port_mirroring; > + bool rxctrl_strap_quirk; > + int clk_output_sel; > + int mode; > +}; I would prefer kernel-doc format to make reading easier. Also tx_fifo_depth doesn't look like signed type. I would make them unsigned. > + > +static int dp83869_config_port_mirroring(struct phy_device *phydev) > +{ > + struct dp83869_private *dp83869 = phydev->priv; > + > + if (dp83869->port_mirroring == DP83869_PORT_MIRRORING_EN) > + return phy_set_bits_mmd(phydev, DP83869_DEVADDR, > + DP83869_GEN_CFG3, > + DP83869_CFG3_PORT_MIRROR_EN); > + else you can remove this else. > + return phy_clear_bits_mmd(phydev, DP83869_DEVADDR, > + DP83869_GEN_CFG3, > + DP83869_CFG3_PORT_MIRROR_EN); > +} > + > +static int dp83869_set_strapped_mode(struct phy_device *phydev) > +{ > + struct dp83869_private *dp83869 = phydev->priv; > + u16 val; > + > + val = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_STRAP_STS1); > + if (val < 0) > + return val; > + > + dp83869->mode = val & DP83869_STRAP_OP_MODE_MASK; > + > + return 0; > +} > + > +#ifdef CONFIG_OF_MDIO Isn't there a way to remove these? if (IS_ENABLED(CONFIG_OF_MDIO)) ... > +static int dp83869_of_init(struct phy_device *phydev) > +{ > + struct dp83869_private *dp83869 = phydev->priv; > + struct device *dev = &phydev->mdio.dev; > + struct device_node *of_node = dev->of_node; > + int ret; > + > + if (!of_node) > + return -ENODEV; didn't go deep to this but can this happen? > + > + dp83869->io_impedance = -EINVAL; I would prefer to group it together. It means move this before dt reading. > + > + /* Optional configuration */ > + ret = of_property_read_u32(of_node, "ti,clk-output-sel", > + &dp83869->clk_output_sel); > + if (ret || dp83869->clk_output_sel > DP83869_CLK_O_SEL_REF_CLK) > + dp83869->clk_output_sel = DP83869_CLK_O_SEL_REF_CLK; > + > + ret = of_property_read_u32(of_node, "ti,op-mode", &dp83869->mode); > + if (ret == 0) { !ret > + if (dp83869->mode < DP83869_RGMII_COPPER_ETHERNET || > + dp83869->mode > DP83869_SGMII_COPPER_ETHERNET) > + return -EINVAL; > + } else { > + ret = dp83869_set_strapped_mode(phydev); > + if (ret) > + return ret; > + } > + > + if (of_property_read_bool(of_node, "ti,max-output-impedance")) > + dp83869->io_impedance = DP83869_IO_MUX_CFG_IO_IMPEDANCE_MAX; > + else if (of_property_read_bool(of_node, "ti,min-output-impedance")) > + dp83869->io_impedance = DP83869_IO_MUX_CFG_IO_IMPEDANCE_MIN; > + > + if (of_property_read_bool(of_node, "enet-phy-lane-swap")) { > + dp83869->port_mirroring = DP83869_PORT_MIRRORING_EN; > + } else { > + /* If the lane swap is not in the DT then check the straps */ > + ret = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_STRAP_STS1); > + if (ret < 0) > + return ret; newline here please. > + if (ret & DP83869_STRAP_MIRROR_ENABLED) > + dp83869->port_mirroring = DP83869_PORT_MIRRORING_EN; > + else > + dp83869->port_mirroring = DP83869_PORT_MIRRORING_DIS; > + } > + > + if (of_property_read_u32(of_node, "rx-fifo-depth", > + &dp83869->rx_fifo_depth)) > + dp83869->rx_fifo_depth = DP83869_PHYCR_FIFO_DEPTH_4_B_NIB; > + > + if (of_property_read_u32(of_node, "tx-fifo-depth", > + &dp83869->tx_fifo_depth)) > + dp83869->tx_fifo_depth = DP83869_PHYCR_FIFO_DEPTH_4_B_NIB; > + > + return ret; > +} > +#else > +static int dp83869_of_init(struct phy_device *phydev) > +{ > + return dp83869_set_strapped_mode(phydev); > +} > +#endif /* CONFIG_OF_MDIO */ > + > +static int dp83869_configure_rgmii(struct phy_device *phydev, > + struct dp83869_private *dp83869) > +{ > + int ret = 0, val; > + > + if (phy_interface_is_rgmii(phydev)) { > + val = phy_read(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL); > + if (val < 0) > + return val; > + > + val &= ~DP83869_PHYCR_FIFO_DEPTH_MASK; > + val |= (dp83869->tx_fifo_depth << DP83869_TX_FIFO_SHIFT); > + val |= (dp83869->rx_fifo_depth << DP83869_RX_FIFO_SHIFT); > + > + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL, val); > + if (ret) > + return ret; > + } > + > + if (dp83869->io_impedance >= 0) > + ret = phy_set_bits_mmd(phydev, DP83869_DEVADDR, > + DP83869_IO_MUX_CFG, > + dp83869->io_impedance & > + DP83869_IO_MUX_CFG_IO_IMPEDANCE_CTRL); > + return ret; > +} > + > +static int dp83869_config_aneg(struct phy_device *phydev) > +{ > + struct dp83869_private *dp83869 = phydev->priv; > + int err; > + > + if (dp83869->mode == DP83869_RGMII_1000_BASE) { > + err = phy_write(phydev, MDIO_DEVAD_NONE, MII_ADVERTISE, > + MII_DP83869_FIBER_ADVERTISE); > + if (err) > + return err; > + > + err = phy_write(phydev, MDIO_DEVAD_NONE, DP83869_CTRL, > + DP83869_SW_RESTART); > + if (err) > + return err; > + > + return genphy_restart_aneg(phydev); > + } else { you can remove this else. > + return genphy_config_aneg(phydev); > + } > +} > + > +static int dp83869_configure_mode(struct phy_device *phydev, > + struct dp83869_private *dp83869) > +{ > + int phy_ctrl_val; > + int ret; > + > + if (dp83869->mode < DP83869_RGMII_COPPER_ETHERNET || > + dp83869->mode > DP83869_SGMII_COPPER_ETHERNET) > + return -EINVAL; > + > + /* Below init sequence for each operational mode is defined in > + * section 9.4.8 of the datasheet. > + */ > + ret = phy_write_mmd(phydev, DP83869_DEVADDR, DP83869_OP_MODE, > + dp83869->mode); > + if (ret) > + return ret; > + > + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, MII_DP83869_BMCR_DEFAULT); > + if (ret) > + return ret; > + > + phy_ctrl_val = (dp83869->rx_fifo_depth << DP83869_RX_FIFO_SHIFT | > + dp83869->tx_fifo_depth << DP83869_TX_FIFO_SHIFT | > + DP83869_PHY_CTRL_DEFAULT); () are useless. > + > + switch (dp83869->mode) { > + case DP83869_RGMII_COPPER_ETHERNET: > + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL, > + phy_ctrl_val); > + if (ret) > + return ret; > + > + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_CTRL1000, > + DP83869_CFG1_DEFAULT); > + if (ret) > + return ret; > + > + ret = dp83869_configure_rgmii(phydev, dp83869); > + if (ret) > + return ret; > + break; > + case DP83869_RGMII_SGMII_BRIDGE: > + ret = phy_set_bits_mmd(phydev, DP83869_DEVADDR, DP83869_OP_MODE, > + DP83869_SGMII_RGMII_BRIDGE, > + DP83869_SGMII_RGMII_BRIDGE); > + if (ret) > + return ret; > + > + ret = phy_write_mmd(phydev, DP83869_DEVADDR, > + DP83869_FX_CTRL, DP83869_FX_CTRL_DEFAULT); > + if (ret) > + return ret; > + nit: remove this newline - it is not consistent. > + break; > + case DP83869_1000M_MEDIA_CONVERT: > + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL, > + phy_ctrl_val); > + if (ret) > + return ret; > + > + ret = phy_write_mmd(phydev, DP83869_DEVADDR, > + DP83869_FX_CTRL, DP83869_FX_CTRL_DEFAULT); > + if (ret) > + return ret; > + break; > + case DP83869_100M_MEDIA_CONVERT: > + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL, > + phy_ctrl_val); > + if (ret) > + return ret; > + break; > + case DP83869_SGMII_COPPER_ETHERNET: > + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL, > + phy_ctrl_val); > + if (ret) > + return ret; > + > + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_CTRL1000, DP83869_CFG1_DEFAULT); > + if (ret) > + return ret; > + > + ret = phy_write_mmd(phydev, DP83869_DEVADDR, > + DP83869_FX_CTRL, DP83869_FX_CTRL_DEFAULT); > + if (ret) > + return ret; > + ditto. > + break; > + case DP83869_RGMII_100_BASE: > + case DP83869_RGMII_1000_BASE: Does it mean that there is not need to anything to configure? Comment about it would be good. > + break; > + default: > + return -EINVAL; > + } > + > + return ret; > +} > + > +static int dp83869_phy_reset(struct phy_device *phydev) > +{ > + int ret; > + > + ret = phy_write(phydev, MDIO_DEVAD_NONE,DP83869_CTRL, DP83869_SW_RESET); > + if (ret < 0) > + return ret; > + > + udelay(200); comment about this delay would be necessary. Just explain where that 200us is coming from. > + > + return 0; > +} > + > +static int dp83869_config_init(struct phy_device *phydev) > +{ > + struct dp83869_private *dp83869 = phydev->priv; > + int ret; > + > + ret = dp83869_phy_reset(phydev); > + if (ret) > + return ret; > + > + ret = dp83869_configure_mode(phydev, dp83869); > + if (ret) > + return ret; > + > + if (dp83869->port_mirroring != DP83869_PORT_MIRRORING_KEEP) > + dp83869_config_port_mirroring(phydev); > + > + /* Clock output selection if muxing property is set */ > + if (dp83869->clk_output_sel != DP83869_CLK_O_SEL_REF_CLK) > + ret = phy_set_bits_mmd(phydev, DP83869_DEVADDR, > + DP83869_IO_MUX_CFG, > + dp83869->clk_output_sel << > + DP83869_IO_MUX_CFG_CLK_O_SEL_SHIFT); > + > + dp83869_config_aneg(phydev); You are not checking return value from this function. > + > + return ret; > +} > + > +static int dp83869_probe(struct phy_device *phydev) > +{ > + struct dp83869_private *dp83869; > + int ret; > + > + dp83869 = kzalloc(sizeof(*dp83869), GFP_KERNEL); > + if (!dp83869) > + return -ENOMEM; > + > + phydev->priv = dp83869; > + > + ret = dp83869_of_init(phydev); > + if (ret) > + return ret; > + > + return dp83869_config_init(phydev); > +} > + > +static struct phy_driver dp83869_driver = { > + .name = "TI DP83869", > + .uid = DP83869_PHY_ID, > + .mask = 0xfffffff0, > + .features = PHY_GBIT_FEATURES, > + .probe = dp83869_probe, > + .config = &dp83869_config_init, > + .startup = &genphy_startup, > + .shutdown = &genphy_shutdown, > +}; > + > +int phy_dp83869_init(void) > +{ > + phy_register(&dp83869_driver); > + return 0; > +} > diff --git a/drivers/net/phy/ti_phy_init.c b/drivers/net/phy/ti_phy_init.c > index 11c4c166b2f5..cb261b86b05d 100644 > --- a/drivers/net/phy/ti_phy_init.c > +++ b/drivers/net/phy/ti_phy_init.c > @@ -92,6 +92,10 @@ int phy_ti_init(void) > phy_dp83867_init(); > #endif > > +#ifdef CONFIG_PHY_DP83869 > + phy_dp83869_init(); > +#endif > + > #ifdef CONFIG_PHY_TI_GENERIC > phy_register(&dp83822_driver); > phy_register(&dp83825s_driver); > diff --git a/drivers/net/phy/ti_phy_init.h b/drivers/net/phy/ti_phy_init.h > index 309da2aacccb..f94ff6291a3f 100644 > --- a/drivers/net/phy/ti_phy_init.h > +++ b/drivers/net/phy/ti_phy_init.h > @@ -12,5 +12,6 @@ > #define _TI_GEN_PHY_H > > int phy_dp83867_init(void); > +int phy_dp83869_init(void); > > #endif /* _TI_GEN_PHY_H */ > diff --git a/include/dt-bindings/net/ti-dp83869.h b/include/dt-bindings/net/ti-dp83869.h > new file mode 100644 > index 000000000000..218b1a64e975 > --- /dev/null > +++ b/include/dt-bindings/net/ti-dp83869.h > @@ -0,0 +1,42 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Device Tree constants for the Texas Instruments DP83869 PHY > + * > + * Author: Dan Murphy <dmurphy at ti.com> > + * > + * Copyright: (C) 2019 Texas Instruments, Inc. > + */ > + > +#ifndef _DT_BINDINGS_TI_DP83869_H > +#define _DT_BINDINGS_TI_DP83869_H > + > +/* PHY CTRL bits */ > +#define DP83869_PHYCR_FIFO_DEPTH_3_B_NIB 0x00 > +#define DP83869_PHYCR_FIFO_DEPTH_4_B_NIB 0x01 > +#define DP83869_PHYCR_FIFO_DEPTH_6_B_NIB 0x02 > +#define DP83869_PHYCR_FIFO_DEPTH_8_B_NIB 0x03 > + > +/* IO_MUX_CFG - Clock output selection */ > +#define DP83869_CLK_O_SEL_CHN_A_RCLK 0x0 > +#define DP83869_CLK_O_SEL_CHN_B_RCLK 0x1 > +#define DP83869_CLK_O_SEL_CHN_C_RCLK 0x2 > +#define DP83869_CLK_O_SEL_CHN_D_RCLK 0x3 > +#define DP83869_CLK_O_SEL_CHN_A_RCLK_DIV5 0x4 > +#define DP83869_CLK_O_SEL_CHN_B_RCLK_DIV5 0x5 > +#define DP83869_CLK_O_SEL_CHN_C_RCLK_DIV5 0x6 > +#define DP83869_CLK_O_SEL_CHN_D_RCLK_DIV5 0x7 > +#define DP83869_CLK_O_SEL_CHN_A_TCLK 0x8 > +#define DP83869_CLK_O_SEL_CHN_B_TCLK 0x9 > +#define DP83869_CLK_O_SEL_CHN_C_TCLK 0xa > +#define DP83869_CLK_O_SEL_CHN_D_TCLK 0xb > +#define DP83869_CLK_O_SEL_REF_CLK 0xc > + > +#define DP83869_RGMII_COPPER_ETHERNET 0x00 > +#define DP83869_RGMII_1000_BASE 0x01 > +#define DP83869_RGMII_100_BASE 0x02 > +#define DP83869_RGMII_SGMII_BRIDGE 0x03 > +#define DP83869_1000M_MEDIA_CONVERT 0x04 > +#define DP83869_100M_MEDIA_CONVERT 0x05 > +#define DP83869_SGMII_COPPER_ETHERNET 0x06 > + > +#endif > Thanks, Michal
Michal On 4/21/20 3:23 AM, Michal Simek wrote: > On 20. 04. 20 20:53, Dan Murphy wrote: >> Port the DP83869 kernel driver. >> >> Signed-off-by: Dan Murphy <dmurphy at ti.com> >> --- >> drivers/net/phy/Kconfig | 4 + >> drivers/net/phy/Makefile | 1 + >> drivers/net/phy/dp83869.c | 440 +++++++++++++++++++++++++++ >> drivers/net/phy/ti_phy_init.c | 4 + >> drivers/net/phy/ti_phy_init.h | 1 + >> include/dt-bindings/net/ti-dp83869.h | 42 +++ >> 6 files changed, 492 insertions(+) >> create mode 100644 drivers/net/phy/dp83869.c >> create mode 100644 include/dt-bindings/net/ti-dp83869.h >> >> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig >> index e366f10afc59..5f14bdba0a3b 100644 >> --- a/drivers/net/phy/Kconfig >> +++ b/drivers/net/phy/Kconfig >> @@ -252,6 +252,10 @@ config PHY_DP83867 >> select PHY_TI >> bool "Texas Instruments Ethernet DP83867 PHY support" >> >> +config PHY_DP83869 >> + select PHY_TI >> + bool "Texas Instruments Ethernet DP83869 PHY support" > isn't this a little bit short? I expect checkpatch should be reporting > issue with it. Yes but I was following other config flags in this file. > > >> + >> config PHY_VITESSE >> bool "Vitesse Ethernet PHYs support" >> >> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile >> index 9c6d31718c00..f8797319154f 100644 >> --- a/drivers/net/phy/Makefile >> +++ b/drivers/net/phy/Makefile >> @@ -27,6 +27,7 @@ obj-$(CONFIG_PHY_SMSC) += smsc.o >> obj-$(CONFIG_PHY_TERANETICS) += teranetics.o >> obj-$(CONFIG_PHY_TI) += ti_phy_init.o >> obj-$(CONFIG_PHY_DP83867) += dp83867.o >> +obj-$(CONFIG_PHY_DP83869) += dp83869.o >> obj-$(CONFIG_PHY_XILINX) += xilinx_phy.o >> obj-$(CONFIG_PHY_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o >> obj-$(CONFIG_PHY_VITESSE) += vitesse.o >> diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c >> new file mode 100644 >> index 000000000000..1eaaea20b37a >> --- /dev/null >> +++ b/drivers/net/phy/dp83869.c >> @@ -0,0 +1,440 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* Driver for the Texas Instruments DP83869 PHY >> + * Copyright (C) 2019 Texas Instruments Inc. > 2020? This driver was developed in 2019 and added to the 5.4 kernel so not sure we should update the copyright when side porting. At the very least I will need to add 2019-20 > >> + */ >> + >> +#include <common.h> >> +#include <phy.h> >> +#include <dm/devres.h> >> +#include <linux/compat.h> >> +#include <malloc.h> >> + >> +#include <dm.h> >> + >> +#include <dt-bindings/net/ti-dp83869.h> >> + >> +#include "ti_phy_init.h" >> + >> +#define DP83869_PHY_ID 0x2000a0f1 >> +#define DP83869_DEVADDR 0x1f >> + >> +#define MII_DP83869_PHYCTRL 0x10 >> +#define MII_DP83869_MICR 0x12 >> +#define MII_DP83869_ISR 0x13 >> +#define DP83869_CTRL 0x1f >> +#define DP83869_CFG4 0x1e >> + >> +/* Extended Registers */ >> +#define DP83869_GEN_CFG3 0x0031 >> +#define DP83869_RGMIICTL 0x0032 >> +#define DP83869_STRAP_STS1 0x006e >> +#define DP83869_RGMIIDCTL 0x0086 >> +#define DP83869_IO_MUX_CFG 0x0170 >> +#define DP83869_OP_MODE 0x01df >> +#define DP83869_FX_CTRL 0x0c00 >> + >> +#define DP83869_SW_RESET BIT(15) >> +#define DP83869_SW_RESTART BIT(14) >> + >> +/* MICR Interrupt bits */ >> +#define MII_DP83869_MICR_AN_ERR_INT_EN BIT(15) >> +#define MII_DP83869_MICR_SPEED_CHNG_INT_EN BIT(14) >> +#define MII_DP83869_MICR_DUP_MODE_CHNG_INT_EN BIT(13) >> +#define MII_DP83869_MICR_PAGE_RXD_INT_EN BIT(12) >> +#define MII_DP83869_MICR_AUTONEG_COMP_INT_EN BIT(11) >> +#define MII_DP83869_MICR_LINK_STS_CHNG_INT_EN BIT(10) >> +#define MII_DP83869_MICR_FALSE_CARRIER_INT_EN BIT(8) >> +#define MII_DP83869_MICR_SLEEP_MODE_CHNG_INT_EN BIT(4) >> +#define MII_DP83869_MICR_WOL_INT_EN BIT(3) >> +#define MII_DP83869_MICR_XGMII_ERR_INT_EN BIT(2) >> +#define MII_DP83869_MICR_POL_CHNG_INT_EN BIT(1) >> +#define MII_DP83869_MICR_JABBER_INT_EN BIT(0) >> + >> +#define MII_DP83869_BMCR_DEFAULT (BMCR_ANENABLE | \ >> + BMCR_FULLDPLX | \ >> + BMCR_SPEED1000) >> + >> +#define MII_DP83869_FIBER_ADVERTISE (ADVERTISE_CSMA | \ >> + ADVERTISE_1000XHALF | \ >> + ADVERTISE_1000XFULL | \ >> + ADVERTISE_1000XPAUSE | \ >> + ADVERTISE_1000XPSE_ASYM) >> + >> +/* This is the same bit mask as the BMCR so re-use the BMCR default */ >> +#define DP83869_FX_CTRL_DEFAULT MII_DP83869_BMCR_DEFAULT >> + >> +/* CFG1 bits */ >> +#define DP83869_CFG1_DEFAULT (ADVERTISE_1000HALF | \ >> + ADVERTISE_1000FULL | \ >> + CTL1000_AS_MASTER) >> + >> +/* RGMIICTL bits */ >> +#define DP83869_RGMII_TX_CLK_DELAY_EN BIT(1) >> +#define DP83869_RGMII_RX_CLK_DELAY_EN BIT(0) >> + >> +/* STRAP_STS1 bits */ >> +#define DP83869_STRAP_OP_MODE_MASK GENMASK(2, 0) >> +#define DP83869_STRAP_STS1_RESERVED BIT(11) >> +#define DP83869_STRAP_MIRROR_ENABLED BIT(12) >> + >> +/* PHYCTRL bits */ >> +#define DP83869_RX_FIFO_SHIFT 12 >> +#define DP83869_TX_FIFO_SHIFT 14 >> + >> +/* PHY_CTRL lower bytes 0x48 are declared as reserved */ >> +#define DP83869_PHY_CTRL_DEFAULT 0x48 >> +#define DP83869_PHYCR_FIFO_DEPTH_MASK GENMASK(15, 12) >> +#define DP83869_PHYCR_RESERVED_MASK BIT(11) >> + >> +/* RGMIIDCTL bits */ >> +#define DP83869_RGMII_TX_CLK_DELAY_SHIFT 4 >> + >> +/* IO_MUX_CFG bits */ >> +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_CTRL 0x1f >> + >> +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_MAX 0x0 >> +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_MIN 0x1f >> +#define DP83869_IO_MUX_CFG_CLK_O_SEL_MASK (0x1f << 8) >> +#define DP83869_IO_MUX_CFG_CLK_O_SEL_SHIFT 8 >> + >> +/* CFG3 bits */ >> +#define DP83869_CFG3_PORT_MIRROR_EN BIT(0) >> + >> +/* CFG4 bits */ >> +#define DP83869_INT_OE BIT(7) >> + >> +/* OP MODE */ >> +#define DP83869_OP_MODE_MII BIT(5) >> +#define DP83869_SGMII_RGMII_BRIDGE BIT(6) >> + >> +enum { >> + DP83869_PORT_MIRRORING_KEEP, >> + DP83869_PORT_MIRRORING_EN, >> + DP83869_PORT_MIRRORING_DIS, >> +}; > We have met with this in the kernel. Greg was asking us to write exact > value to all enum entries. > Hmm. Can you give me a reference?? I am not doubting you but I would like to read that guidance. That reference will help with an debate I am having in the kernel. >> + >> +struct dp83869_private { >> + int tx_fifo_depth; >> + int rx_fifo_depth; >> + int io_impedance; >> + int port_mirroring; >> + bool rxctrl_strap_quirk; >> + int clk_output_sel; >> + int mode; >> +}; > I would prefer kernel-doc format to make reading easier. > Also tx_fifo_depth doesn't look like signed type. I would make them > unsigned. > > >> + >> +static int dp83869_config_port_mirroring(struct phy_device *phydev) >> +{ >> + struct dp83869_private *dp83869 = phydev->priv; >> + >> + if (dp83869->port_mirroring == DP83869_PORT_MIRRORING_EN) >> + return phy_set_bits_mmd(phydev, DP83869_DEVADDR, >> + DP83869_GEN_CFG3, >> + DP83869_CFG3_PORT_MIRROR_EN); >> + else > you can remove this else. Ack > >> + return phy_clear_bits_mmd(phydev, DP83869_DEVADDR, >> + DP83869_GEN_CFG3, >> + DP83869_CFG3_PORT_MIRROR_EN); >> +} >> + >> +static int dp83869_set_strapped_mode(struct phy_device *phydev) >> +{ >> + struct dp83869_private *dp83869 = phydev->priv; >> + u16 val; >> + >> + val = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_STRAP_STS1); >> + if (val < 0) >> + return val; >> + >> + dp83869->mode = val & DP83869_STRAP_OP_MODE_MASK; >> + >> + return 0; >> +} >> + >> +#ifdef CONFIG_OF_MDIO > Isn't there a way to remove these? > > if (IS_ENABLED(CONFIG_OF_MDIO)) > ... I looked at the 83867 it uses #if defined(CONFIG_DM_ETH) so I can change this >> +static int dp83869_of_init(struct phy_device *phydev) >> +{ >> + struct dp83869_private *dp83869 = phydev->priv; >> + struct device *dev = &phydev->mdio.dev; >> + struct device_node *of_node = dev->of_node; >> + int ret; >> + >> + if (!of_node) >> + return -ENODEV; > didn't go deep to this but can this happen? Does every device in the uBoot tree use the DT or do some still use board files? > >> + >> + dp83869->io_impedance = -EINVAL; > I would prefer to group it together. It means move this before dt reading. > >> + >> + /* Optional configuration */ >> + ret = of_property_read_u32(of_node, "ti,clk-output-sel", >> + &dp83869->clk_output_sel); >> + if (ret || dp83869->clk_output_sel > DP83869_CLK_O_SEL_REF_CLK) >> + dp83869->clk_output_sel = DP83869_CLK_O_SEL_REF_CLK; >> + >> + ret = of_property_read_u32(of_node, "ti,op-mode", &dp83869->mode); >> + if (ret == 0) { > !ret Ack > >> + if (dp83869->mode < DP83869_RGMII_COPPER_ETHERNET || >> + dp83869->mode > DP83869_SGMII_COPPER_ETHERNET) >> + return -EINVAL; >> + } else { >> + ret = dp83869_set_strapped_mode(phydev); >> + if (ret) >> + return ret; >> + } >> + >> + if (of_property_read_bool(of_node, "ti,max-output-impedance")) >> + dp83869->io_impedance = DP83869_IO_MUX_CFG_IO_IMPEDANCE_MAX; >> + else if (of_property_read_bool(of_node, "ti,min-output-impedance")) >> + dp83869->io_impedance = DP83869_IO_MUX_CFG_IO_IMPEDANCE_MIN; >> + >> + if (of_property_read_bool(of_node, "enet-phy-lane-swap")) { >> + dp83869->port_mirroring = DP83869_PORT_MIRRORING_EN; >> + } else { >> + /* If the lane swap is not in the DT then check the straps */ >> + ret = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_STRAP_STS1); >> + if (ret < 0) >> + return ret; > newline here please. Ack > >> + if (ret & DP83869_STRAP_MIRROR_ENABLED) >> + dp83869->port_mirroring = DP83869_PORT_MIRRORING_EN; >> + else >> + dp83869->port_mirroring = DP83869_PORT_MIRRORING_DIS; >> + } >> + >> + if (of_property_read_u32(of_node, "rx-fifo-depth", >> + &dp83869->rx_fifo_depth)) >> + dp83869->rx_fifo_depth = DP83869_PHYCR_FIFO_DEPTH_4_B_NIB; >> + >> + if (of_property_read_u32(of_node, "tx-fifo-depth", >> + &dp83869->tx_fifo_depth)) >> + dp83869->tx_fifo_depth = DP83869_PHYCR_FIFO_DEPTH_4_B_NIB; >> + >> + return ret; >> +} >> +#else >> +static int dp83869_of_init(struct phy_device *phydev) >> +{ >> + return dp83869_set_strapped_mode(phydev); >> +} >> +#endif /* CONFIG_OF_MDIO */ >> + >> +static int dp83869_configure_rgmii(struct phy_device *phydev, >> + struct dp83869_private *dp83869) >> +{ >> + int ret = 0, val; >> + >> + if (phy_interface_is_rgmii(phydev)) { >> + val = phy_read(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL); >> + if (val < 0) >> + return val; >> + >> + val &= ~DP83869_PHYCR_FIFO_DEPTH_MASK; >> + val |= (dp83869->tx_fifo_depth << DP83869_TX_FIFO_SHIFT); >> + val |= (dp83869->rx_fifo_depth << DP83869_RX_FIFO_SHIFT); >> + >> + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL, val); >> + if (ret) >> + return ret; >> + } >> + >> + if (dp83869->io_impedance >= 0) >> + ret = phy_set_bits_mmd(phydev, DP83869_DEVADDR, >> + DP83869_IO_MUX_CFG, >> + dp83869->io_impedance & >> + DP83869_IO_MUX_CFG_IO_IMPEDANCE_CTRL); >> + return ret; >> +} >> + >> +static int dp83869_config_aneg(struct phy_device *phydev) >> +{ >> + struct dp83869_private *dp83869 = phydev->priv; >> + int err; >> + >> + if (dp83869->mode == DP83869_RGMII_1000_BASE) { >> + err = phy_write(phydev, MDIO_DEVAD_NONE, MII_ADVERTISE, >> + MII_DP83869_FIBER_ADVERTISE); >> + if (err) >> + return err; >> + >> + err = phy_write(phydev, MDIO_DEVAD_NONE, DP83869_CTRL, >> + DP83869_SW_RESTART); >> + if (err) >> + return err; >> + >> + return genphy_restart_aneg(phydev); >> + } else { > you can remove this else. Ack > >> + return genphy_config_aneg(phydev); >> + } >> +} >> + >> +static int dp83869_configure_mode(struct phy_device *phydev, >> + struct dp83869_private *dp83869) >> +{ >> + int phy_ctrl_val; >> + int ret; >> + >> + if (dp83869->mode < DP83869_RGMII_COPPER_ETHERNET || >> + dp83869->mode > DP83869_SGMII_COPPER_ETHERNET) >> + return -EINVAL; >> + >> + /* Below init sequence for each operational mode is defined in >> + * section 9.4.8 of the datasheet. >> + */ >> + ret = phy_write_mmd(phydev, DP83869_DEVADDR, DP83869_OP_MODE, >> + dp83869->mode); >> + if (ret) >> + return ret; >> + >> + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, MII_DP83869_BMCR_DEFAULT); >> + if (ret) >> + return ret; >> + >> + phy_ctrl_val = (dp83869->rx_fifo_depth << DP83869_RX_FIFO_SHIFT | >> + dp83869->tx_fifo_depth << DP83869_TX_FIFO_SHIFT | >> + DP83869_PHY_CTRL_DEFAULT); > () are useless. Ack > >> + >> + switch (dp83869->mode) { >> + case DP83869_RGMII_COPPER_ETHERNET: >> + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL, >> + phy_ctrl_val); >> + if (ret) >> + return ret; >> + >> + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_CTRL1000, >> + DP83869_CFG1_DEFAULT); >> + if (ret) >> + return ret; >> + >> + ret = dp83869_configure_rgmii(phydev, dp83869); >> + if (ret) >> + return ret; >> + break; >> + case DP83869_RGMII_SGMII_BRIDGE: >> + ret = phy_set_bits_mmd(phydev, DP83869_DEVADDR, DP83869_OP_MODE, >> + DP83869_SGMII_RGMII_BRIDGE, >> + DP83869_SGMII_RGMII_BRIDGE); >> + if (ret) >> + return ret; >> + >> + ret = phy_write_mmd(phydev, DP83869_DEVADDR, >> + DP83869_FX_CTRL, DP83869_FX_CTRL_DEFAULT); >> + if (ret) >> + return ret; >> + > nit: remove this newline - it is not consistent. Ack > >> + break; >> + case DP83869_1000M_MEDIA_CONVERT: >> + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL, >> + phy_ctrl_val); >> + if (ret) >> + return ret; >> + >> + ret = phy_write_mmd(phydev, DP83869_DEVADDR, >> + DP83869_FX_CTRL, DP83869_FX_CTRL_DEFAULT); >> + if (ret) >> + return ret; >> + break; >> + case DP83869_100M_MEDIA_CONVERT: >> + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL, >> + phy_ctrl_val); >> + if (ret) >> + return ret; >> + break; >> + case DP83869_SGMII_COPPER_ETHERNET: >> + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL, >> + phy_ctrl_val); >> + if (ret) >> + return ret; >> + >> + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_CTRL1000, DP83869_CFG1_DEFAULT); >> + if (ret) >> + return ret; >> + >> + ret = phy_write_mmd(phydev, DP83869_DEVADDR, >> + DP83869_FX_CTRL, DP83869_FX_CTRL_DEFAULT); >> + if (ret) >> + return ret; >> + > ditto. Ack > >> + break; >> + case DP83869_RGMII_100_BASE: >> + case DP83869_RGMII_1000_BASE: > Does it mean that there is not need to anything to configure? > Comment about it would be good. I will add a comment as one of these is actually a fiber connection which I have not figured out how uBoot handles fiber connections yet. > >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + return ret; >> +} >> + >> +static int dp83869_phy_reset(struct phy_device *phydev) >> +{ >> + int ret; >> + >> + ret = phy_write(phydev, MDIO_DEVAD_NONE,DP83869_CTRL, DP83869_SW_RESET); >> + if (ret < 0) >> + return ret; >> + >> + udelay(200); > comment about this delay would be necessary. Just explain where that > 200us is coming from. Ack. > >> + >> + return 0; >> +} >> + >> +static int dp83869_config_init(struct phy_device *phydev) >> +{ >> + struct dp83869_private *dp83869 = phydev->priv; >> + int ret; >> + >> + ret = dp83869_phy_reset(phydev); >> + if (ret) >> + return ret; >> + >> + ret = dp83869_configure_mode(phydev, dp83869); >> + if (ret) >> + return ret; >> + >> + if (dp83869->port_mirroring != DP83869_PORT_MIRRORING_KEEP) >> + dp83869_config_port_mirroring(phydev); >> + >> + /* Clock output selection if muxing property is set */ >> + if (dp83869->clk_output_sel != DP83869_CLK_O_SEL_REF_CLK) >> + ret = phy_set_bits_mmd(phydev, DP83869_DEVADDR, >> + DP83869_IO_MUX_CFG, >> + dp83869->clk_output_sel << >> + DP83869_IO_MUX_CFG_CLK_O_SEL_SHIFT); >> + >> + dp83869_config_aneg(phydev); > > You are not checking return value from this function. Ack Dan
On 21. 04. 20 14:04, Dan Murphy wrote: > Michal > > On 4/21/20 3:23 AM, Michal Simek wrote: >> On 20. 04. 20 20:53, Dan Murphy wrote: >>> Port the DP83869 kernel driver. >>> >>> Signed-off-by: Dan Murphy <dmurphy at ti.com> >>> --- >>> ? drivers/net/phy/Kconfig????????????? |?? 4 + >>> ? drivers/net/phy/Makefile???????????? |?? 1 + >>> ? drivers/net/phy/dp83869.c??????????? | 440 +++++++++++++++++++++++++++ >>> ? drivers/net/phy/ti_phy_init.c??????? |?? 4 + >>> ? drivers/net/phy/ti_phy_init.h??????? |?? 1 + >>> ? include/dt-bindings/net/ti-dp83869.h |? 42 +++ >>> ? 6 files changed, 492 insertions(+) >>> ? create mode 100644 drivers/net/phy/dp83869.c >>> ? create mode 100644 include/dt-bindings/net/ti-dp83869.h >>> >>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig >>> index e366f10afc59..5f14bdba0a3b 100644 >>> --- a/drivers/net/phy/Kconfig >>> +++ b/drivers/net/phy/Kconfig >>> @@ -252,6 +252,10 @@ config PHY_DP83867 >>> ????? select PHY_TI >>> ????? bool "Texas Instruments Ethernet DP83867 PHY support" >>> ? +config PHY_DP83869 >>> +??? select PHY_TI >>> +??? bool "Texas Instruments Ethernet DP83869 PHY support" >> isn't this a little bit short? I expect checkpatch should be reporting >> issue with it. > > Yes but I was following other config flags in this file. Just no reason to follow bad examples. :-) >> >> >>> + >>> ? config PHY_VITESSE >>> ????? bool "Vitesse Ethernet PHYs support" >>> ? diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile >>> index 9c6d31718c00..f8797319154f 100644 >>> --- a/drivers/net/phy/Makefile >>> +++ b/drivers/net/phy/Makefile >>> @@ -27,6 +27,7 @@ obj-$(CONFIG_PHY_SMSC) += smsc.o >>> ? obj-$(CONFIG_PHY_TERANETICS) += teranetics.o >>> ? obj-$(CONFIG_PHY_TI) += ti_phy_init.o >>> ? obj-$(CONFIG_PHY_DP83867) += dp83867.o >>> +obj-$(CONFIG_PHY_DP83869) += dp83869.o >>> ? obj-$(CONFIG_PHY_XILINX) += xilinx_phy.o >>> ? obj-$(CONFIG_PHY_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o >>> ? obj-$(CONFIG_PHY_VITESSE) += vitesse.o >>> diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c >>> new file mode 100644 >>> index 000000000000..1eaaea20b37a >>> --- /dev/null >>> +++ b/drivers/net/phy/dp83869.c >>> @@ -0,0 +1,440 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +/* Driver for the Texas Instruments DP83869 PHY >>> + * Copyright (C) 2019 Texas Instruments Inc. >> 2020? > > This driver was developed in 2019 and added to the 5.4 kernel so not > sure we should update the copyright when side porting. > > At the very least I will need to add 2019-20 yup. > >> >>> + */ >>> + >>> +#include <common.h> >>> +#include <phy.h> >>> +#include <dm/devres.h> >>> +#include <linux/compat.h> >>> +#include <malloc.h> >>> + >>> +#include <dm.h> >>> + >>> +#include <dt-bindings/net/ti-dp83869.h> >>> + >>> +#include "ti_phy_init.h" >>> + >>> +#define DP83869_PHY_ID??????? 0x2000a0f1 >>> +#define DP83869_DEVADDR??????? 0x1f >>> + >>> +#define MII_DP83869_PHYCTRL??? 0x10 >>> +#define MII_DP83869_MICR??? 0x12 >>> +#define MII_DP83869_ISR??????? 0x13 >>> +#define DP83869_CTRL??????? 0x1f >>> +#define DP83869_CFG4??????? 0x1e >>> + >>> +/* Extended Registers */ >>> +#define DP83869_GEN_CFG3??????? 0x0031 >>> +#define DP83869_RGMIICTL??? 0x0032 >>> +#define DP83869_STRAP_STS1??? 0x006e >>> +#define DP83869_RGMIIDCTL??? 0x0086 >>> +#define DP83869_IO_MUX_CFG??? 0x0170 >>> +#define DP83869_OP_MODE??????? 0x01df >>> +#define DP83869_FX_CTRL??????? 0x0c00 >>> + >>> +#define DP83869_SW_RESET??? BIT(15) >>> +#define DP83869_SW_RESTART??? BIT(14) >>> + >>> +/* MICR Interrupt bits */ >>> +#define MII_DP83869_MICR_AN_ERR_INT_EN??????? BIT(15) >>> +#define MII_DP83869_MICR_SPEED_CHNG_INT_EN??? BIT(14) >>> +#define MII_DP83869_MICR_DUP_MODE_CHNG_INT_EN??? BIT(13) >>> +#define MII_DP83869_MICR_PAGE_RXD_INT_EN??? BIT(12) >>> +#define MII_DP83869_MICR_AUTONEG_COMP_INT_EN??? BIT(11) >>> +#define MII_DP83869_MICR_LINK_STS_CHNG_INT_EN??? BIT(10) >>> +#define MII_DP83869_MICR_FALSE_CARRIER_INT_EN??? BIT(8) >>> +#define MII_DP83869_MICR_SLEEP_MODE_CHNG_INT_EN??? BIT(4) >>> +#define MII_DP83869_MICR_WOL_INT_EN??????? BIT(3) >>> +#define MII_DP83869_MICR_XGMII_ERR_INT_EN??? BIT(2) >>> +#define MII_DP83869_MICR_POL_CHNG_INT_EN??? BIT(1) >>> +#define MII_DP83869_MICR_JABBER_INT_EN??????? BIT(0) >>> + >>> +#define MII_DP83869_BMCR_DEFAULT??? (BMCR_ANENABLE | \ >>> +???????????????????? BMCR_FULLDPLX | \ >>> +???????????????????? BMCR_SPEED1000) >>> + >>> +#define MII_DP83869_FIBER_ADVERTISE??? (ADVERTISE_CSMA | \ >>> +???????????????????? ADVERTISE_1000XHALF | \ >>> +???????????????????? ADVERTISE_1000XFULL | \ >>> +???????????????????? ADVERTISE_1000XPAUSE | \ >>> +???????????????????? ADVERTISE_1000XPSE_ASYM) >>> + >>> +/* This is the same bit mask as the BMCR so re-use the BMCR default */ >>> +#define DP83869_FX_CTRL_DEFAULT??? MII_DP83869_BMCR_DEFAULT >>> + >>> +/* CFG1 bits */ >>> +#define DP83869_CFG1_DEFAULT??? (ADVERTISE_1000HALF | \ >>> +???????????????? ADVERTISE_1000FULL | \ >>> +???????????????? CTL1000_AS_MASTER) >>> + >>> +/* RGMIICTL bits */ >>> +#define DP83869_RGMII_TX_CLK_DELAY_EN??????? BIT(1) >>> +#define DP83869_RGMII_RX_CLK_DELAY_EN??????? BIT(0) >>> + >>> +/* STRAP_STS1 bits */ >>> +#define DP83869_STRAP_OP_MODE_MASK??????? GENMASK(2, 0) >>> +#define DP83869_STRAP_STS1_RESERVED??????? BIT(11) >>> +#define DP83869_STRAP_MIRROR_ENABLED??????? BIT(12) >>> + >>> +/* PHYCTRL bits */ >>> +#define DP83869_RX_FIFO_SHIFT??? 12 >>> +#define DP83869_TX_FIFO_SHIFT??? 14 >>> + >>> +/* PHY_CTRL lower bytes 0x48 are declared as reserved */ >>> +#define DP83869_PHY_CTRL_DEFAULT??? 0x48 >>> +#define DP83869_PHYCR_FIFO_DEPTH_MASK??? GENMASK(15, 12) >>> +#define DP83869_PHYCR_RESERVED_MASK??? BIT(11) >>> + >>> +/* RGMIIDCTL bits */ >>> +#define DP83869_RGMII_TX_CLK_DELAY_SHIFT??? 4 >>> + >>> +/* IO_MUX_CFG bits */ >>> +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_CTRL??? 0x1f >>> + >>> +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_MAX??? 0x0 >>> +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_MIN??? 0x1f >>> +#define DP83869_IO_MUX_CFG_CLK_O_SEL_MASK??? (0x1f << 8) >>> +#define DP83869_IO_MUX_CFG_CLK_O_SEL_SHIFT??? 8 >>> + >>> +/* CFG3 bits */ >>> +#define DP83869_CFG3_PORT_MIRROR_EN????????????? BIT(0) >>> + >>> +/* CFG4 bits */ >>> +#define DP83869_INT_OE??? BIT(7) >>> + >>> +/* OP MODE */ >>> +#define DP83869_OP_MODE_MII??????????? BIT(5) >>> +#define DP83869_SGMII_RGMII_BRIDGE??????? BIT(6) >>> + >>> +enum { >>> +??? DP83869_PORT_MIRRORING_KEEP, >>> +??? DP83869_PORT_MIRRORING_EN, >>> +??? DP83869_PORT_MIRRORING_DIS, >>> +}; >> We have met with this in the kernel. Greg was asking us to write exact >> value to all enum entries. >> > Hmm. Can you give me a reference?? I am not doubting you but I would > like to read that guidance. > > That reference will help with an debate I am having in the kernel. Take a look at this thread. https://lore.kernel.org/linux-arm-kernel/20200318125003.GA2727094 at kroah.com/ <snip> >>> + >>> +#ifdef CONFIG_OF_MDIO >> Isn't there a way to remove these? >> >> if (IS_ENABLED(CONFIG_OF_MDIO)) >> ... > I looked at the 83867 it uses #if defined(CONFIG_DM_ETH) so I can change > this There are a lot of places which should be update/done better. >>> +static int dp83869_of_init(struct phy_device *phydev) >>> +{ >>> +??? struct dp83869_private *dp83869 = phydev->priv; >>> +??? struct device *dev = &phydev->mdio.dev; >>> +??? struct device_node *of_node = dev->of_node; >>> +??? int ret; >>> + >>> +??? if (!of_node) >>> +??????? return -ENODEV; >> didn't go deep to this but can this happen? > > Does every device in the uBoot tree use the DT or do some still use > board files? IIRC ethernet phys are not based on driver model that's why devices are not created for it and I am not quite sure if platdata are supported. I think question is what way you use. If you use just OF_MDIO/DM_ETH then Kconfig should have dependencies. And if someone else want to run it without it (which is IMHO unlikely) then they can update the driver self. >> >>> + >>> +??? dp83869->io_impedance = -EINVAL; >> I would prefer to group it together. It means move this before dt >> reading. >> No reaction on this line that's why just commenting it that you spot it. Thanks, Michal
Michal On 4/21/20 7:39 AM, Michal Simek wrote: > On 21. 04. 20 14:04, Dan Murphy wrote: >> Michal >> >> On 4/21/20 3:23 AM, Michal Simek wrote: >>> On 20. 04. 20 20:53, Dan Murphy wrote: >>>> Port the DP83869 kernel driver. >>>> >>>> Signed-off-by: Dan Murphy <dmurphy at ti.com> >>>> --- >>>> ? drivers/net/phy/Kconfig????????????? |?? 4 + >>>> ? drivers/net/phy/Makefile???????????? |?? 1 + >>>> ? drivers/net/phy/dp83869.c??????????? | 440 +++++++++++++++++++++++++++ >>>> ? drivers/net/phy/ti_phy_init.c??????? |?? 4 + >>>> ? drivers/net/phy/ti_phy_init.h??????? |?? 1 + >>>> ? include/dt-bindings/net/ti-dp83869.h |? 42 +++ >>>> ? 6 files changed, 492 insertions(+) >>>> ? create mode 100644 drivers/net/phy/dp83869.c >>>> ? create mode 100644 include/dt-bindings/net/ti-dp83869.h >>>> >>>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig >>>> index e366f10afc59..5f14bdba0a3b 100644 >>>> --- a/drivers/net/phy/Kconfig >>>> +++ b/drivers/net/phy/Kconfig >>>> @@ -252,6 +252,10 @@ config PHY_DP83867 >>>> ????? select PHY_TI >>>> ????? bool "Texas Instruments Ethernet DP83867 PHY support" >>>> ? +config PHY_DP83869 >>>> +??? select PHY_TI >>>> +??? bool "Texas Instruments Ethernet DP83869 PHY support" >>> isn't this a little bit short? I expect checkpatch should be reporting >>> issue with it. >> Yes but I was following other config flags in this file. > Just no reason to follow bad examples. :-) True. I will update the examples for the TI PHYs. > > > >>> >>>> + >>>> ? config PHY_VITESSE >>>> ????? bool "Vitesse Ethernet PHYs support" >>>> ? diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile >>>> index 9c6d31718c00..f8797319154f 100644 >>>> --- a/drivers/net/phy/Makefile >>>> +++ b/drivers/net/phy/Makefile >>>> @@ -27,6 +27,7 @@ obj-$(CONFIG_PHY_SMSC) += smsc.o >>>> ? obj-$(CONFIG_PHY_TERANETICS) += teranetics.o >>>> ? obj-$(CONFIG_PHY_TI) += ti_phy_init.o >>>> ? obj-$(CONFIG_PHY_DP83867) += dp83867.o >>>> +obj-$(CONFIG_PHY_DP83869) += dp83869.o >>>> ? obj-$(CONFIG_PHY_XILINX) += xilinx_phy.o >>>> ? obj-$(CONFIG_PHY_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o >>>> ? obj-$(CONFIG_PHY_VITESSE) += vitesse.o >>>> diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c >>>> new file mode 100644 >>>> index 000000000000..1eaaea20b37a >>>> --- /dev/null >>>> +++ b/drivers/net/phy/dp83869.c >>>> @@ -0,0 +1,440 @@ >>>> +// SPDX-License-Identifier: GPL-2.0 >>>> +/* Driver for the Texas Instruments DP83869 PHY >>>> + * Copyright (C) 2019 Texas Instruments Inc. >>> 2020? >> This driver was developed in 2019 and added to the 5.4 kernel so not >> sure we should update the copyright when side porting. >> >> At the very least I will need to add 2019-20 > yup. Ack > >>>> + */ >>>> + >>>> +#include <common.h> >>>> +#include <phy.h> >>>> +#include <dm/devres.h> >>>> +#include <linux/compat.h> >>>> +#include <malloc.h> >>>> + >>>> +#include <dm.h> >>>> + >>>> +#include <dt-bindings/net/ti-dp83869.h> >>>> + >>>> +#include "ti_phy_init.h" >>>> + >>>> +#define DP83869_PHY_ID??????? 0x2000a0f1 >>>> +#define DP83869_DEVADDR??????? 0x1f >>>> + >>>> +#define MII_DP83869_PHYCTRL??? 0x10 >>>> +#define MII_DP83869_MICR??? 0x12 >>>> +#define MII_DP83869_ISR??????? 0x13 >>>> +#define DP83869_CTRL??????? 0x1f >>>> +#define DP83869_CFG4??????? 0x1e >>>> + >>>> +/* Extended Registers */ >>>> +#define DP83869_GEN_CFG3??????? 0x0031 >>>> +#define DP83869_RGMIICTL??? 0x0032 >>>> +#define DP83869_STRAP_STS1??? 0x006e >>>> +#define DP83869_RGMIIDCTL??? 0x0086 >>>> +#define DP83869_IO_MUX_CFG??? 0x0170 >>>> +#define DP83869_OP_MODE??????? 0x01df >>>> +#define DP83869_FX_CTRL??????? 0x0c00 >>>> + >>>> +#define DP83869_SW_RESET??? BIT(15) >>>> +#define DP83869_SW_RESTART??? BIT(14) >>>> + >>>> +/* MICR Interrupt bits */ >>>> +#define MII_DP83869_MICR_AN_ERR_INT_EN??????? BIT(15) >>>> +#define MII_DP83869_MICR_SPEED_CHNG_INT_EN??? BIT(14) >>>> +#define MII_DP83869_MICR_DUP_MODE_CHNG_INT_EN??? BIT(13) >>>> +#define MII_DP83869_MICR_PAGE_RXD_INT_EN??? BIT(12) >>>> +#define MII_DP83869_MICR_AUTONEG_COMP_INT_EN??? BIT(11) >>>> +#define MII_DP83869_MICR_LINK_STS_CHNG_INT_EN??? BIT(10) >>>> +#define MII_DP83869_MICR_FALSE_CARRIER_INT_EN??? BIT(8) >>>> +#define MII_DP83869_MICR_SLEEP_MODE_CHNG_INT_EN??? BIT(4) >>>> +#define MII_DP83869_MICR_WOL_INT_EN??????? BIT(3) >>>> +#define MII_DP83869_MICR_XGMII_ERR_INT_EN??? BIT(2) >>>> +#define MII_DP83869_MICR_POL_CHNG_INT_EN??? BIT(1) >>>> +#define MII_DP83869_MICR_JABBER_INT_EN??????? BIT(0) >>>> + >>>> +#define MII_DP83869_BMCR_DEFAULT??? (BMCR_ANENABLE | \ >>>> +???????????????????? BMCR_FULLDPLX | \ >>>> +???????????????????? BMCR_SPEED1000) >>>> + >>>> +#define MII_DP83869_FIBER_ADVERTISE??? (ADVERTISE_CSMA | \ >>>> +???????????????????? ADVERTISE_1000XHALF | \ >>>> +???????????????????? ADVERTISE_1000XFULL | \ >>>> +???????????????????? ADVERTISE_1000XPAUSE | \ >>>> +???????????????????? ADVERTISE_1000XPSE_ASYM) >>>> + >>>> +/* This is the same bit mask as the BMCR so re-use the BMCR default */ >>>> +#define DP83869_FX_CTRL_DEFAULT??? MII_DP83869_BMCR_DEFAULT >>>> + >>>> +/* CFG1 bits */ >>>> +#define DP83869_CFG1_DEFAULT??? (ADVERTISE_1000HALF | \ >>>> +???????????????? ADVERTISE_1000FULL | \ >>>> +???????????????? CTL1000_AS_MASTER) >>>> + >>>> +/* RGMIICTL bits */ >>>> +#define DP83869_RGMII_TX_CLK_DELAY_EN??????? BIT(1) >>>> +#define DP83869_RGMII_RX_CLK_DELAY_EN??????? BIT(0) >>>> + >>>> +/* STRAP_STS1 bits */ >>>> +#define DP83869_STRAP_OP_MODE_MASK??????? GENMASK(2, 0) >>>> +#define DP83869_STRAP_STS1_RESERVED??????? BIT(11) >>>> +#define DP83869_STRAP_MIRROR_ENABLED??????? BIT(12) >>>> + >>>> +/* PHYCTRL bits */ >>>> +#define DP83869_RX_FIFO_SHIFT??? 12 >>>> +#define DP83869_TX_FIFO_SHIFT??? 14 >>>> + >>>> +/* PHY_CTRL lower bytes 0x48 are declared as reserved */ >>>> +#define DP83869_PHY_CTRL_DEFAULT??? 0x48 >>>> +#define DP83869_PHYCR_FIFO_DEPTH_MASK??? GENMASK(15, 12) >>>> +#define DP83869_PHYCR_RESERVED_MASK??? BIT(11) >>>> + >>>> +/* RGMIIDCTL bits */ >>>> +#define DP83869_RGMII_TX_CLK_DELAY_SHIFT??? 4 >>>> + >>>> +/* IO_MUX_CFG bits */ >>>> +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_CTRL??? 0x1f >>>> + >>>> +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_MAX??? 0x0 >>>> +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_MIN??? 0x1f >>>> +#define DP83869_IO_MUX_CFG_CLK_O_SEL_MASK??? (0x1f << 8) >>>> +#define DP83869_IO_MUX_CFG_CLK_O_SEL_SHIFT??? 8 >>>> + >>>> +/* CFG3 bits */ >>>> +#define DP83869_CFG3_PORT_MIRROR_EN????????????? BIT(0) >>>> + >>>> +/* CFG4 bits */ >>>> +#define DP83869_INT_OE??? BIT(7) >>>> + >>>> +/* OP MODE */ >>>> +#define DP83869_OP_MODE_MII??????????? BIT(5) >>>> +#define DP83869_SGMII_RGMII_BRIDGE??????? BIT(6) >>>> + >>>> +enum { >>>> +??? DP83869_PORT_MIRRORING_KEEP, >>>> +??? DP83869_PORT_MIRRORING_EN, >>>> +??? DP83869_PORT_MIRRORING_DIS, >>>> +}; >>> We have met with this in the kernel. Greg was asking us to write exact >>> value to all enum entries. >>> >> Hmm. Can you give me a reference?? I am not doubting you but I would >> like to read that guidance. >> >> That reference will help with an debate I am having in the kernel. > Take a look at this thread. > https://lore.kernel.org/linux-arm-kernel/20200318125003.GA2727094 at kroah.com/ Thank you > <snip> > >>>> + >>>> +#ifdef CONFIG_OF_MDIO >>> Isn't there a way to remove these? >>> >>> if (IS_ENABLED(CONFIG_OF_MDIO)) >>> ... >> I looked at the 83867 it uses #if defined(CONFIG_DM_ETH) so I can change >> this > There are a lot of places which should be update/done better. Are you inferring that this is not correct either? > > >>>> +static int dp83869_of_init(struct phy_device *phydev) >>>> +{ >>>> +??? struct dp83869_private *dp83869 = phydev->priv; >>>> +??? struct device *dev = &phydev->mdio.dev; >>>> +??? struct device_node *of_node = dev->of_node; >>>> +??? int ret; >>>> + >>>> +??? if (!of_node) >>>> +??????? return -ENODEV; >>> didn't go deep to this but can this happen? >> Does every device in the uBoot tree use the DT or do some still use >> board files? > IIRC ethernet phys are not based on driver model that's why devices are > not created for it and I am not quite sure if platdata are supported. > > I think question is what way you use. If you use just OF_MDIO/DM_ETH > then Kconfig should have dependencies. And if someone else want to run > it without it (which is IMHO unlikely) then they can update the driver self. Well technically some phys like this one may not need DT at all if strapped in hardware. >>>> + >>>> +??? dp83869->io_impedance = -EINVAL; >>> I would prefer to group it together. It means move this before dt >>> reading. >>> > No reaction on this line that's why just commenting it that you spot it. I had to look at it in detail.? Not sure why this was set there. This should be removed Dan > Thanks, > Michal > > >
Hi Dan I am interested to see the mainline support for the DP83869 phy. Is there any progress on this patch or are there any blocker? Am Di., 21. Apr. 2020 um 16:35 Uhr schrieb Dan Murphy <dmurphy@ti.com>: > > Michal > > On 4/21/20 7:39 AM, Michal Simek wrote: > > On 21. 04. 20 14:04, Dan Murphy wrote: > >> Michal > >> > >> On 4/21/20 3:23 AM, Michal Simek wrote: > >>> On 20. 04. 20 20:53, Dan Murphy wrote: > >>>> Port the DP83869 kernel driver. > >>>> > >>>> Signed-off-by: Dan Murphy <dmurphy@ti.com> > >>>> --- > >>>> drivers/net/phy/Kconfig | 4 + > >>>> drivers/net/phy/Makefile | 1 + > >>>> drivers/net/phy/dp83869.c | 440 +++++++++++++++++++++++++++ > >>>> drivers/net/phy/ti_phy_init.c | 4 + > >>>> drivers/net/phy/ti_phy_init.h | 1 + > >>>> include/dt-bindings/net/ti-dp83869.h | 42 +++ > >>>> 6 files changed, 492 insertions(+) > >>>> create mode 100644 drivers/net/phy/dp83869.c > >>>> create mode 100644 include/dt-bindings/net/ti-dp83869.h > >>>> > >>>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > >>>> index e366f10afc59..5f14bdba0a3b 100644 > >>>> --- a/drivers/net/phy/Kconfig > >>>> +++ b/drivers/net/phy/Kconfig > >>>> @@ -252,6 +252,10 @@ config PHY_DP83867 > >>>> select PHY_TI > >>>> bool "Texas Instruments Ethernet DP83867 PHY support" > >>>> +config PHY_DP83869 > >>>> + select PHY_TI > >>>> + bool "Texas Instruments Ethernet DP83869 PHY support" > >>> isn't this a little bit short? I expect checkpatch should be reporting > >>> issue with it. > >> Yes but I was following other config flags in this file. > > Just no reason to follow bad examples. :-) > > True. > > I will update the examples for the TI PHYs. > > > > > > > > > >>> > >>>> + > >>>> config PHY_VITESSE > >>>> bool "Vitesse Ethernet PHYs support" > >>>> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > >>>> index 9c6d31718c00..f8797319154f 100644 > >>>> --- a/drivers/net/phy/Makefile > >>>> +++ b/drivers/net/phy/Makefile > >>>> @@ -27,6 +27,7 @@ obj-$(CONFIG_PHY_SMSC) += smsc.o > >>>> obj-$(CONFIG_PHY_TERANETICS) += teranetics.o > >>>> obj-$(CONFIG_PHY_TI) += ti_phy_init.o > >>>> obj-$(CONFIG_PHY_DP83867) += dp83867.o > >>>> +obj-$(CONFIG_PHY_DP83869) += dp83869.o > >>>> obj-$(CONFIG_PHY_XILINX) += xilinx_phy.o > >>>> obj-$(CONFIG_PHY_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o > >>>> obj-$(CONFIG_PHY_VITESSE) += vitesse.o > >>>> diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c > >>>> new file mode 100644 > >>>> index 000000000000..1eaaea20b37a > >>>> --- /dev/null > >>>> +++ b/drivers/net/phy/dp83869.c > >>>> @@ -0,0 +1,440 @@ > >>>> +// SPDX-License-Identifier: GPL-2.0 > >>>> +/* Driver for the Texas Instruments DP83869 PHY > >>>> + * Copyright (C) 2019 Texas Instruments Inc. > >>> 2020? > >> This driver was developed in 2019 and added to the 5.4 kernel so not > >> sure we should update the copyright when side porting. > >> > >> At the very least I will need to add 2019-20 > > yup. > > > Ack > > > > >>>> + */ > >>>> + > >>>> +#include <common.h> > >>>> +#include <phy.h> > >>>> +#include <dm/devres.h> > >>>> +#include <linux/compat.h> > >>>> +#include <malloc.h> > >>>> + > >>>> +#include <dm.h> > >>>> + > >>>> +#include <dt-bindings/net/ti-dp83869.h> > >>>> + > >>>> +#include "ti_phy_init.h" > >>>> + > >>>> +#define DP83869_PHY_ID 0x2000a0f1 > >>>> +#define DP83869_DEVADDR 0x1f > >>>> + > >>>> +#define MII_DP83869_PHYCTRL 0x10 > >>>> +#define MII_DP83869_MICR 0x12 > >>>> +#define MII_DP83869_ISR 0x13 > >>>> +#define DP83869_CTRL 0x1f > >>>> +#define DP83869_CFG4 0x1e > >>>> + > >>>> +/* Extended Registers */ > >>>> +#define DP83869_GEN_CFG3 0x0031 > >>>> +#define DP83869_RGMIICTL 0x0032 > >>>> +#define DP83869_STRAP_STS1 0x006e > >>>> +#define DP83869_RGMIIDCTL 0x0086 > >>>> +#define DP83869_IO_MUX_CFG 0x0170 > >>>> +#define DP83869_OP_MODE 0x01df > >>>> +#define DP83869_FX_CTRL 0x0c00 > >>>> + > >>>> +#define DP83869_SW_RESET BIT(15) > >>>> +#define DP83869_SW_RESTART BIT(14) > >>>> + > >>>> +/* MICR Interrupt bits */ > >>>> +#define MII_DP83869_MICR_AN_ERR_INT_EN BIT(15) > >>>> +#define MII_DP83869_MICR_SPEED_CHNG_INT_EN BIT(14) > >>>> +#define MII_DP83869_MICR_DUP_MODE_CHNG_INT_EN BIT(13) > >>>> +#define MII_DP83869_MICR_PAGE_RXD_INT_EN BIT(12) > >>>> +#define MII_DP83869_MICR_AUTONEG_COMP_INT_EN BIT(11) > >>>> +#define MII_DP83869_MICR_LINK_STS_CHNG_INT_EN BIT(10) > >>>> +#define MII_DP83869_MICR_FALSE_CARRIER_INT_EN BIT(8) > >>>> +#define MII_DP83869_MICR_SLEEP_MODE_CHNG_INT_EN BIT(4) > >>>> +#define MII_DP83869_MICR_WOL_INT_EN BIT(3) > >>>> +#define MII_DP83869_MICR_XGMII_ERR_INT_EN BIT(2) > >>>> +#define MII_DP83869_MICR_POL_CHNG_INT_EN BIT(1) > >>>> +#define MII_DP83869_MICR_JABBER_INT_EN BIT(0) > >>>> + > >>>> +#define MII_DP83869_BMCR_DEFAULT (BMCR_ANENABLE | \ > >>>> + BMCR_FULLDPLX | \ > >>>> + BMCR_SPEED1000) > >>>> + > >>>> +#define MII_DP83869_FIBER_ADVERTISE (ADVERTISE_CSMA | \ > >>>> + ADVERTISE_1000XHALF | \ > >>>> + ADVERTISE_1000XFULL | \ > >>>> + ADVERTISE_1000XPAUSE | \ > >>>> + ADVERTISE_1000XPSE_ASYM) > >>>> + > >>>> +/* This is the same bit mask as the BMCR so re-use the BMCR default */ > >>>> +#define DP83869_FX_CTRL_DEFAULT MII_DP83869_BMCR_DEFAULT > >>>> + > >>>> +/* CFG1 bits */ > >>>> +#define DP83869_CFG1_DEFAULT (ADVERTISE_1000HALF | \ > >>>> + ADVERTISE_1000FULL | \ > >>>> + CTL1000_AS_MASTER) > >>>> + > >>>> +/* RGMIICTL bits */ > >>>> +#define DP83869_RGMII_TX_CLK_DELAY_EN BIT(1) > >>>> +#define DP83869_RGMII_RX_CLK_DELAY_EN BIT(0) > >>>> + > >>>> +/* STRAP_STS1 bits */ > >>>> +#define DP83869_STRAP_OP_MODE_MASK GENMASK(2, 0) > >>>> +#define DP83869_STRAP_STS1_RESERVED BIT(11) > >>>> +#define DP83869_STRAP_MIRROR_ENABLED BIT(12) > >>>> + > >>>> +/* PHYCTRL bits */ > >>>> +#define DP83869_RX_FIFO_SHIFT 12 > >>>> +#define DP83869_TX_FIFO_SHIFT 14 > >>>> + > >>>> +/* PHY_CTRL lower bytes 0x48 are declared as reserved */ > >>>> +#define DP83869_PHY_CTRL_DEFAULT 0x48 > >>>> +#define DP83869_PHYCR_FIFO_DEPTH_MASK GENMASK(15, 12) > >>>> +#define DP83869_PHYCR_RESERVED_MASK BIT(11) > >>>> + > >>>> +/* RGMIIDCTL bits */ > >>>> +#define DP83869_RGMII_TX_CLK_DELAY_SHIFT 4 > >>>> + > >>>> +/* IO_MUX_CFG bits */ > >>>> +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_CTRL 0x1f > >>>> + > >>>> +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_MAX 0x0 > >>>> +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_MIN 0x1f > >>>> +#define DP83869_IO_MUX_CFG_CLK_O_SEL_MASK (0x1f << 8) > >>>> +#define DP83869_IO_MUX_CFG_CLK_O_SEL_SHIFT 8 > >>>> + > >>>> +/* CFG3 bits */ > >>>> +#define DP83869_CFG3_PORT_MIRROR_EN BIT(0) > >>>> + > >>>> +/* CFG4 bits */ > >>>> +#define DP83869_INT_OE BIT(7) > >>>> + > >>>> +/* OP MODE */ > >>>> +#define DP83869_OP_MODE_MII BIT(5) > >>>> +#define DP83869_SGMII_RGMII_BRIDGE BIT(6) > >>>> + > >>>> +enum { > >>>> + DP83869_PORT_MIRRORING_KEEP, > >>>> + DP83869_PORT_MIRRORING_EN, > >>>> + DP83869_PORT_MIRRORING_DIS, > >>>> +}; > >>> We have met with this in the kernel. Greg was asking us to write exact > >>> value to all enum entries. > >>> > >> Hmm. Can you give me a reference? I am not doubting you but I would > >> like to read that guidance. > >> > >> That reference will help with an debate I am having in the kernel. > > Take a look at this thread. > > https://lore.kernel.org/linux-arm-kernel/20200318125003.GA2727094@kroah.com/ > Thank you > > <snip> > > > >>>> + > >>>> +#ifdef CONFIG_OF_MDIO > >>> Isn't there a way to remove these? > >>> > >>> if (IS_ENABLED(CONFIG_OF_MDIO)) > >>> ... > >> I looked at the 83867 it uses #if defined(CONFIG_DM_ETH) so I can change > >> this > > There are a lot of places which should be update/done better. > Are you inferring that this is not correct either? > > > > > >>>> +static int dp83869_of_init(struct phy_device *phydev) > >>>> +{ > >>>> + struct dp83869_private *dp83869 = phydev->priv; > >>>> + struct device *dev = &phydev->mdio.dev; > >>>> + struct device_node *of_node = dev->of_node; > >>>> + int ret; > >>>> + > >>>> + if (!of_node) > >>>> + return -ENODEV; > >>> didn't go deep to this but can this happen? > >> Does every device in the uBoot tree use the DT or do some still use > >> board files? > > IIRC ethernet phys are not based on driver model that's why devices are > > not created for it and I am not quite sure if platdata are supported. > > > > I think question is what way you use. If you use just OF_MDIO/DM_ETH > > then Kconfig should have dependencies. And if someone else want to run > > it without it (which is IMHO unlikely) then they can update the driver self. > > Well technically some phys like this one may not need DT at all if > strapped in hardware. > > > >>>> + > >>>> + dp83869->io_impedance = -EINVAL; > >>> I would prefer to group it together. It means move this before dt > >>> reading. > >>> > > No reaction on this line that's why just commenting it that you spot it. > > I had to look at it in detail. Not sure why this was set there. This > should be removed > > Dan > > > > Thanks, > > Michal > > > > > > -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index e366f10afc59..5f14bdba0a3b 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -252,6 +252,10 @@ config PHY_DP83867 select PHY_TI bool "Texas Instruments Ethernet DP83867 PHY support" +config PHY_DP83869 + select PHY_TI + bool "Texas Instruments Ethernet DP83869 PHY support" + config PHY_VITESSE bool "Vitesse Ethernet PHYs support" diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index 9c6d31718c00..f8797319154f 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_PHY_SMSC) += smsc.o obj-$(CONFIG_PHY_TERANETICS) += teranetics.o obj-$(CONFIG_PHY_TI) += ti_phy_init.o obj-$(CONFIG_PHY_DP83867) += dp83867.o +obj-$(CONFIG_PHY_DP83869) += dp83869.o obj-$(CONFIG_PHY_XILINX) += xilinx_phy.o obj-$(CONFIG_PHY_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o obj-$(CONFIG_PHY_VITESSE) += vitesse.o diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c new file mode 100644 index 000000000000..1eaaea20b37a --- /dev/null +++ b/drivers/net/phy/dp83869.c @@ -0,0 +1,440 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Driver for the Texas Instruments DP83869 PHY + * Copyright (C) 2019 Texas Instruments Inc. + */ + +#include <common.h> +#include <phy.h> +#include <dm/devres.h> +#include <linux/compat.h> +#include <malloc.h> + +#include <dm.h> + +#include <dt-bindings/net/ti-dp83869.h> + +#include "ti_phy_init.h" + +#define DP83869_PHY_ID 0x2000a0f1 +#define DP83869_DEVADDR 0x1f + +#define MII_DP83869_PHYCTRL 0x10 +#define MII_DP83869_MICR 0x12 +#define MII_DP83869_ISR 0x13 +#define DP83869_CTRL 0x1f +#define DP83869_CFG4 0x1e + +/* Extended Registers */ +#define DP83869_GEN_CFG3 0x0031 +#define DP83869_RGMIICTL 0x0032 +#define DP83869_STRAP_STS1 0x006e +#define DP83869_RGMIIDCTL 0x0086 +#define DP83869_IO_MUX_CFG 0x0170 +#define DP83869_OP_MODE 0x01df +#define DP83869_FX_CTRL 0x0c00 + +#define DP83869_SW_RESET BIT(15) +#define DP83869_SW_RESTART BIT(14) + +/* MICR Interrupt bits */ +#define MII_DP83869_MICR_AN_ERR_INT_EN BIT(15) +#define MII_DP83869_MICR_SPEED_CHNG_INT_EN BIT(14) +#define MII_DP83869_MICR_DUP_MODE_CHNG_INT_EN BIT(13) +#define MII_DP83869_MICR_PAGE_RXD_INT_EN BIT(12) +#define MII_DP83869_MICR_AUTONEG_COMP_INT_EN BIT(11) +#define MII_DP83869_MICR_LINK_STS_CHNG_INT_EN BIT(10) +#define MII_DP83869_MICR_FALSE_CARRIER_INT_EN BIT(8) +#define MII_DP83869_MICR_SLEEP_MODE_CHNG_INT_EN BIT(4) +#define MII_DP83869_MICR_WOL_INT_EN BIT(3) +#define MII_DP83869_MICR_XGMII_ERR_INT_EN BIT(2) +#define MII_DP83869_MICR_POL_CHNG_INT_EN BIT(1) +#define MII_DP83869_MICR_JABBER_INT_EN BIT(0) + +#define MII_DP83869_BMCR_DEFAULT (BMCR_ANENABLE | \ + BMCR_FULLDPLX | \ + BMCR_SPEED1000) + +#define MII_DP83869_FIBER_ADVERTISE (ADVERTISE_CSMA | \ + ADVERTISE_1000XHALF | \ + ADVERTISE_1000XFULL | \ + ADVERTISE_1000XPAUSE | \ + ADVERTISE_1000XPSE_ASYM) + +/* This is the same bit mask as the BMCR so re-use the BMCR default */ +#define DP83869_FX_CTRL_DEFAULT MII_DP83869_BMCR_DEFAULT + +/* CFG1 bits */ +#define DP83869_CFG1_DEFAULT (ADVERTISE_1000HALF | \ + ADVERTISE_1000FULL | \ + CTL1000_AS_MASTER) + +/* RGMIICTL bits */ +#define DP83869_RGMII_TX_CLK_DELAY_EN BIT(1) +#define DP83869_RGMII_RX_CLK_DELAY_EN BIT(0) + +/* STRAP_STS1 bits */ +#define DP83869_STRAP_OP_MODE_MASK GENMASK(2, 0) +#define DP83869_STRAP_STS1_RESERVED BIT(11) +#define DP83869_STRAP_MIRROR_ENABLED BIT(12) + +/* PHYCTRL bits */ +#define DP83869_RX_FIFO_SHIFT 12 +#define DP83869_TX_FIFO_SHIFT 14 + +/* PHY_CTRL lower bytes 0x48 are declared as reserved */ +#define DP83869_PHY_CTRL_DEFAULT 0x48 +#define DP83869_PHYCR_FIFO_DEPTH_MASK GENMASK(15, 12) +#define DP83869_PHYCR_RESERVED_MASK BIT(11) + +/* RGMIIDCTL bits */ +#define DP83869_RGMII_TX_CLK_DELAY_SHIFT 4 + +/* IO_MUX_CFG bits */ +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_CTRL 0x1f + +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_MAX 0x0 +#define DP83869_IO_MUX_CFG_IO_IMPEDANCE_MIN 0x1f +#define DP83869_IO_MUX_CFG_CLK_O_SEL_MASK (0x1f << 8) +#define DP83869_IO_MUX_CFG_CLK_O_SEL_SHIFT 8 + +/* CFG3 bits */ +#define DP83869_CFG3_PORT_MIRROR_EN BIT(0) + +/* CFG4 bits */ +#define DP83869_INT_OE BIT(7) + +/* OP MODE */ +#define DP83869_OP_MODE_MII BIT(5) +#define DP83869_SGMII_RGMII_BRIDGE BIT(6) + +enum { + DP83869_PORT_MIRRORING_KEEP, + DP83869_PORT_MIRRORING_EN, + DP83869_PORT_MIRRORING_DIS, +}; + +struct dp83869_private { + int tx_fifo_depth; + int rx_fifo_depth; + int io_impedance; + int port_mirroring; + bool rxctrl_strap_quirk; + int clk_output_sel; + int mode; +}; + +static int dp83869_config_port_mirroring(struct phy_device *phydev) +{ + struct dp83869_private *dp83869 = phydev->priv; + + if (dp83869->port_mirroring == DP83869_PORT_MIRRORING_EN) + return phy_set_bits_mmd(phydev, DP83869_DEVADDR, + DP83869_GEN_CFG3, + DP83869_CFG3_PORT_MIRROR_EN); + else + return phy_clear_bits_mmd(phydev, DP83869_DEVADDR, + DP83869_GEN_CFG3, + DP83869_CFG3_PORT_MIRROR_EN); +} + +static int dp83869_set_strapped_mode(struct phy_device *phydev) +{ + struct dp83869_private *dp83869 = phydev->priv; + u16 val; + + val = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_STRAP_STS1); + if (val < 0) + return val; + + dp83869->mode = val & DP83869_STRAP_OP_MODE_MASK; + + return 0; +} + +#ifdef CONFIG_OF_MDIO +static int dp83869_of_init(struct phy_device *phydev) +{ + struct dp83869_private *dp83869 = phydev->priv; + struct device *dev = &phydev->mdio.dev; + struct device_node *of_node = dev->of_node; + int ret; + + if (!of_node) + return -ENODEV; + + dp83869->io_impedance = -EINVAL; + + /* Optional configuration */ + ret = of_property_read_u32(of_node, "ti,clk-output-sel", + &dp83869->clk_output_sel); + if (ret || dp83869->clk_output_sel > DP83869_CLK_O_SEL_REF_CLK) + dp83869->clk_output_sel = DP83869_CLK_O_SEL_REF_CLK; + + ret = of_property_read_u32(of_node, "ti,op-mode", &dp83869->mode); + if (ret == 0) { + if (dp83869->mode < DP83869_RGMII_COPPER_ETHERNET || + dp83869->mode > DP83869_SGMII_COPPER_ETHERNET) + return -EINVAL; + } else { + ret = dp83869_set_strapped_mode(phydev); + if (ret) + return ret; + } + + if (of_property_read_bool(of_node, "ti,max-output-impedance")) + dp83869->io_impedance = DP83869_IO_MUX_CFG_IO_IMPEDANCE_MAX; + else if (of_property_read_bool(of_node, "ti,min-output-impedance")) + dp83869->io_impedance = DP83869_IO_MUX_CFG_IO_IMPEDANCE_MIN; + + if (of_property_read_bool(of_node, "enet-phy-lane-swap")) { + dp83869->port_mirroring = DP83869_PORT_MIRRORING_EN; + } else { + /* If the lane swap is not in the DT then check the straps */ + ret = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_STRAP_STS1); + if (ret < 0) + return ret; + if (ret & DP83869_STRAP_MIRROR_ENABLED) + dp83869->port_mirroring = DP83869_PORT_MIRRORING_EN; + else + dp83869->port_mirroring = DP83869_PORT_MIRRORING_DIS; + } + + if (of_property_read_u32(of_node, "rx-fifo-depth", + &dp83869->rx_fifo_depth)) + dp83869->rx_fifo_depth = DP83869_PHYCR_FIFO_DEPTH_4_B_NIB; + + if (of_property_read_u32(of_node, "tx-fifo-depth", + &dp83869->tx_fifo_depth)) + dp83869->tx_fifo_depth = DP83869_PHYCR_FIFO_DEPTH_4_B_NIB; + + return ret; +} +#else +static int dp83869_of_init(struct phy_device *phydev) +{ + return dp83869_set_strapped_mode(phydev); +} +#endif /* CONFIG_OF_MDIO */ + +static int dp83869_configure_rgmii(struct phy_device *phydev, + struct dp83869_private *dp83869) +{ + int ret = 0, val; + + if (phy_interface_is_rgmii(phydev)) { + val = phy_read(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL); + if (val < 0) + return val; + + val &= ~DP83869_PHYCR_FIFO_DEPTH_MASK; + val |= (dp83869->tx_fifo_depth << DP83869_TX_FIFO_SHIFT); + val |= (dp83869->rx_fifo_depth << DP83869_RX_FIFO_SHIFT); + + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL, val); + if (ret) + return ret; + } + + if (dp83869->io_impedance >= 0) + ret = phy_set_bits_mmd(phydev, DP83869_DEVADDR, + DP83869_IO_MUX_CFG, + dp83869->io_impedance & + DP83869_IO_MUX_CFG_IO_IMPEDANCE_CTRL); + return ret; +} + +static int dp83869_config_aneg(struct phy_device *phydev) +{ + struct dp83869_private *dp83869 = phydev->priv; + int err; + + if (dp83869->mode == DP83869_RGMII_1000_BASE) { + err = phy_write(phydev, MDIO_DEVAD_NONE, MII_ADVERTISE, + MII_DP83869_FIBER_ADVERTISE); + if (err) + return err; + + err = phy_write(phydev, MDIO_DEVAD_NONE, DP83869_CTRL, + DP83869_SW_RESTART); + if (err) + return err; + + return genphy_restart_aneg(phydev); + } else { + return genphy_config_aneg(phydev); + } +} + +static int dp83869_configure_mode(struct phy_device *phydev, + struct dp83869_private *dp83869) +{ + int phy_ctrl_val; + int ret; + + if (dp83869->mode < DP83869_RGMII_COPPER_ETHERNET || + dp83869->mode > DP83869_SGMII_COPPER_ETHERNET) + return -EINVAL; + + /* Below init sequence for each operational mode is defined in + * section 9.4.8 of the datasheet. + */ + ret = phy_write_mmd(phydev, DP83869_DEVADDR, DP83869_OP_MODE, + dp83869->mode); + if (ret) + return ret; + + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, MII_DP83869_BMCR_DEFAULT); + if (ret) + return ret; + + phy_ctrl_val = (dp83869->rx_fifo_depth << DP83869_RX_FIFO_SHIFT | + dp83869->tx_fifo_depth << DP83869_TX_FIFO_SHIFT | + DP83869_PHY_CTRL_DEFAULT); + + switch (dp83869->mode) { + case DP83869_RGMII_COPPER_ETHERNET: + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL, + phy_ctrl_val); + if (ret) + return ret; + + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_CTRL1000, + DP83869_CFG1_DEFAULT); + if (ret) + return ret; + + ret = dp83869_configure_rgmii(phydev, dp83869); + if (ret) + return ret; + break; + case DP83869_RGMII_SGMII_BRIDGE: + ret = phy_set_bits_mmd(phydev, DP83869_DEVADDR, DP83869_OP_MODE, + DP83869_SGMII_RGMII_BRIDGE, + DP83869_SGMII_RGMII_BRIDGE); + if (ret) + return ret; + + ret = phy_write_mmd(phydev, DP83869_DEVADDR, + DP83869_FX_CTRL, DP83869_FX_CTRL_DEFAULT); + if (ret) + return ret; + + break; + case DP83869_1000M_MEDIA_CONVERT: + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL, + phy_ctrl_val); + if (ret) + return ret; + + ret = phy_write_mmd(phydev, DP83869_DEVADDR, + DP83869_FX_CTRL, DP83869_FX_CTRL_DEFAULT); + if (ret) + return ret; + break; + case DP83869_100M_MEDIA_CONVERT: + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL, + phy_ctrl_val); + if (ret) + return ret; + break; + case DP83869_SGMII_COPPER_ETHERNET: + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83869_PHYCTRL, + phy_ctrl_val); + if (ret) + return ret; + + ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_CTRL1000, DP83869_CFG1_DEFAULT); + if (ret) + return ret; + + ret = phy_write_mmd(phydev, DP83869_DEVADDR, + DP83869_FX_CTRL, DP83869_FX_CTRL_DEFAULT); + if (ret) + return ret; + + break; + case DP83869_RGMII_100_BASE: + case DP83869_RGMII_1000_BASE: + break; + default: + return -EINVAL; + } + + return ret; +} + +static int dp83869_phy_reset(struct phy_device *phydev) +{ + int ret; + + ret = phy_write(phydev, MDIO_DEVAD_NONE,DP83869_CTRL, DP83869_SW_RESET); + if (ret < 0) + return ret; + + udelay(200); + + return 0; +} + +static int dp83869_config_init(struct phy_device *phydev) +{ + struct dp83869_private *dp83869 = phydev->priv; + int ret; + + ret = dp83869_phy_reset(phydev); + if (ret) + return ret; + + ret = dp83869_configure_mode(phydev, dp83869); + if (ret) + return ret; + + if (dp83869->port_mirroring != DP83869_PORT_MIRRORING_KEEP) + dp83869_config_port_mirroring(phydev); + + /* Clock output selection if muxing property is set */ + if (dp83869->clk_output_sel != DP83869_CLK_O_SEL_REF_CLK) + ret = phy_set_bits_mmd(phydev, DP83869_DEVADDR, + DP83869_IO_MUX_CFG, + dp83869->clk_output_sel << + DP83869_IO_MUX_CFG_CLK_O_SEL_SHIFT); + + dp83869_config_aneg(phydev); + + return ret; +} + +static int dp83869_probe(struct phy_device *phydev) +{ + struct dp83869_private *dp83869; + int ret; + + dp83869 = kzalloc(sizeof(*dp83869), GFP_KERNEL); + if (!dp83869) + return -ENOMEM; + + phydev->priv = dp83869; + + ret = dp83869_of_init(phydev); + if (ret) + return ret; + + return dp83869_config_init(phydev); +} + +static struct phy_driver dp83869_driver = { + .name = "TI DP83869", + .uid = DP83869_PHY_ID, + .mask = 0xfffffff0, + .features = PHY_GBIT_FEATURES, + .probe = dp83869_probe, + .config = &dp83869_config_init, + .startup = &genphy_startup, + .shutdown = &genphy_shutdown, +}; + +int phy_dp83869_init(void) +{ + phy_register(&dp83869_driver); + return 0; +} diff --git a/drivers/net/phy/ti_phy_init.c b/drivers/net/phy/ti_phy_init.c index 11c4c166b2f5..cb261b86b05d 100644 --- a/drivers/net/phy/ti_phy_init.c +++ b/drivers/net/phy/ti_phy_init.c @@ -92,6 +92,10 @@ int phy_ti_init(void) phy_dp83867_init(); #endif +#ifdef CONFIG_PHY_DP83869 + phy_dp83869_init(); +#endif + #ifdef CONFIG_PHY_TI_GENERIC phy_register(&dp83822_driver); phy_register(&dp83825s_driver); diff --git a/drivers/net/phy/ti_phy_init.h b/drivers/net/phy/ti_phy_init.h index 309da2aacccb..f94ff6291a3f 100644 --- a/drivers/net/phy/ti_phy_init.h +++ b/drivers/net/phy/ti_phy_init.h @@ -12,5 +12,6 @@ #define _TI_GEN_PHY_H int phy_dp83867_init(void); +int phy_dp83869_init(void); #endif /* _TI_GEN_PHY_H */ diff --git a/include/dt-bindings/net/ti-dp83869.h b/include/dt-bindings/net/ti-dp83869.h new file mode 100644 index 000000000000..218b1a64e975 --- /dev/null +++ b/include/dt-bindings/net/ti-dp83869.h @@ -0,0 +1,42 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Device Tree constants for the Texas Instruments DP83869 PHY + * + * Author: Dan Murphy <dmurphy at ti.com> + * + * Copyright: (C) 2019 Texas Instruments, Inc. + */ + +#ifndef _DT_BINDINGS_TI_DP83869_H +#define _DT_BINDINGS_TI_DP83869_H + +/* PHY CTRL bits */ +#define DP83869_PHYCR_FIFO_DEPTH_3_B_NIB 0x00 +#define DP83869_PHYCR_FIFO_DEPTH_4_B_NIB 0x01 +#define DP83869_PHYCR_FIFO_DEPTH_6_B_NIB 0x02 +#define DP83869_PHYCR_FIFO_DEPTH_8_B_NIB 0x03 + +/* IO_MUX_CFG - Clock output selection */ +#define DP83869_CLK_O_SEL_CHN_A_RCLK 0x0 +#define DP83869_CLK_O_SEL_CHN_B_RCLK 0x1 +#define DP83869_CLK_O_SEL_CHN_C_RCLK 0x2 +#define DP83869_CLK_O_SEL_CHN_D_RCLK 0x3 +#define DP83869_CLK_O_SEL_CHN_A_RCLK_DIV5 0x4 +#define DP83869_CLK_O_SEL_CHN_B_RCLK_DIV5 0x5 +#define DP83869_CLK_O_SEL_CHN_C_RCLK_DIV5 0x6 +#define DP83869_CLK_O_SEL_CHN_D_RCLK_DIV5 0x7 +#define DP83869_CLK_O_SEL_CHN_A_TCLK 0x8 +#define DP83869_CLK_O_SEL_CHN_B_TCLK 0x9 +#define DP83869_CLK_O_SEL_CHN_C_TCLK 0xa +#define DP83869_CLK_O_SEL_CHN_D_TCLK 0xb +#define DP83869_CLK_O_SEL_REF_CLK 0xc + +#define DP83869_RGMII_COPPER_ETHERNET 0x00 +#define DP83869_RGMII_1000_BASE 0x01 +#define DP83869_RGMII_100_BASE 0x02 +#define DP83869_RGMII_SGMII_BRIDGE 0x03 +#define DP83869_1000M_MEDIA_CONVERT 0x04 +#define DP83869_100M_MEDIA_CONVERT 0x05 +#define DP83869_SGMII_COPPER_ETHERNET 0x06 + +#endif
Port the DP83869 kernel driver. Signed-off-by: Dan Murphy <dmurphy at ti.com> --- drivers/net/phy/Kconfig | 4 + drivers/net/phy/Makefile | 1 + drivers/net/phy/dp83869.c | 440 +++++++++++++++++++++++++++ drivers/net/phy/ti_phy_init.c | 4 + drivers/net/phy/ti_phy_init.h | 1 + include/dt-bindings/net/ti-dp83869.h | 42 +++ 6 files changed, 492 insertions(+) create mode 100644 drivers/net/phy/dp83869.c create mode 100644 include/dt-bindings/net/ti-dp83869.h