[U-Boot] [PATCH 5/8] net: phy: dp83867: rework delay rgmii delay handling
Joe Hershberger
joe.hershberger at ni.com
Tue Nov 19 21:54:43 UTC 2019
On Mon, Nov 18, 2019 at 3:26 PM Grygorii Strashko
<grygorii.strashko at ti.com> wrote:
>
> Based on commit c11669a2757e ("net: phy: dp83867: Rework delay rgmii delay
> handling") of mainline linux kernel.
>
> The current code is assuming the reset default of the delay control
> register was to have delay disabled. This is what the datasheet shows as
> the register's initial value. However, that's not actually true: the
> default is controlled by the PHY's pin strapping.
>
> This patch:
> - insures the other direction's delay is disabled If the interface mode is
> selected as RX or TX delay only
> - validates the delay values and fail if they are not in range
> - checks if the board is strapped to have a delay and is configured to use
> "rgmii" mode and warning is generated that "rgmii-id" should have been
> used.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko at ti.com>
Nit below...
Acked-by: Joe Hershberger <joe.hershberger at ni.com>
> ---
> drivers/net/phy/dp83867.c | 76 ++++++++++++++++++++++++++++++++-------
> 1 file changed, 64 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> index cd3c1c596a..1721f6892b 100644
> --- a/drivers/net/phy/dp83867.c
> +++ b/drivers/net/phy/dp83867.c
> @@ -25,6 +25,7 @@
> #define DP83867_CFG4 0x0031
> #define DP83867_RGMIICTL 0x0032
> #define DP83867_STRAP_STS1 0x006E
> +#define DP83867_STRAP_STS2 0x006f
> #define DP83867_RGMIIDCTL 0x0086
> #define DP83867_IO_MUX_CFG 0x0170
>
> @@ -52,6 +53,13 @@
> /* STRAP_STS1 bits */
> #define DP83867_STRAP_STS1_RESERVED BIT(11)
>
> +/* STRAP_STS2 bits */
> +#define DP83867_STRAP_STS2_CLK_SKEW_TX_MASK GENMASK(6, 4)
> +#define DP83867_STRAP_STS2_CLK_SKEW_TX_SHIFT 4
> +#define DP83867_STRAP_STS2_CLK_SKEW_RX_MASK GENMASK(2, 0)
> +#define DP83867_STRAP_STS2_CLK_SKEW_RX_SHIFT 0
> +#define DP83867_STRAP_STS2_CLK_SKEW_NONE BIT(2)
> +
> /* PHY CTRL bits */
> #define DP83867_PHYCR_FIFO_DEPTH_SHIFT 14
> #define DP83867_PHYCR_RESERVED_MASK BIT(11)
> @@ -63,7 +71,9 @@
> #define DP83867_PHYCTRL_TXFIFO_SHIFT 14
>
> /* RGMIIDCTL bits */
> +#define DP83867_RGMII_TX_CLK_DELAY_MAX 0xf
> #define DP83867_RGMII_TX_CLK_DELAY_SHIFT 4
> +#define DP83867_RGMII_RX_CLK_DELAY_MAX 0xf
>
> /* CFG2 bits */
> #define MII_DP83867_CFG2_SPEEDOPT_10EN 0x0040
> @@ -74,8 +84,6 @@
> #define MII_DP83867_CFG2_MASK 0x003F
>
> /* User setting - can be taken from DTS */
> -#define DEFAULT_RX_ID_DELAY DP83867_RGMIIDCTL_2_25_NS
> -#define DEFAULT_TX_ID_DELAY DP83867_RGMIIDCTL_2_75_NS
> #define DEFAULT_FIFO_DEPTH DP83867_PHYCR_FIFO_DEPTH_4_B_NIB
>
> /* IO_MUX_CFG bits */
> @@ -98,8 +106,8 @@ enum {
> };
>
> struct dp83867_private {
> - int rx_id_delay;
> - int tx_id_delay;
> + u32 rx_id_delay;
> + u32 tx_id_delay;
> int fifo_depth;
> int io_impedance;
> bool rxctrl_strap_quirk;
> @@ -168,13 +176,55 @@ static int dp83867_of_init(struct phy_device *phydev)
>
> if (ofnode_read_bool(node, "ti,dp83867-rxctrl-strap-quirk"))
> dp83867->rxctrl_strap_quirk = true;
> - dp83867->rx_id_delay = ofnode_read_u32_default(node,
> - "ti,rx-internal-delay",
> - DEFAULT_RX_ID_DELAY);
>
> - dp83867->tx_id_delay = ofnode_read_u32_default(node,
> - "ti,tx-internal-delay",
> - DEFAULT_TX_ID_DELAY);
> + /* Existing behavior was to use default pin strapping delay in rgmii
Is this copied out of Linux verbatim? If not, please change the
comment block format.
> + * mode, but rgmii should have meant no delay. Warn existing users.
> + */
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> + u16 val = phy_read_mmd(phydev, DP83867_DEVADDR,
> + DP83867_STRAP_STS2);
> + u16 txskew = (val & DP83867_STRAP_STS2_CLK_SKEW_TX_MASK) >>
> + DP83867_STRAP_STS2_CLK_SKEW_TX_SHIFT;
> + u16 rxskew = (val & DP83867_STRAP_STS2_CLK_SKEW_RX_MASK) >>
> + DP83867_STRAP_STS2_CLK_SKEW_RX_SHIFT;
> +
> + if (txskew != DP83867_STRAP_STS2_CLK_SKEW_NONE ||
> + rxskew != DP83867_STRAP_STS2_CLK_SKEW_NONE)
> + pr_warn("PHY has delays via pin strapping, but phy-mode = 'rgmii'\n"
> + "Should be 'rgmii-id' to use internal delays\n");
> + }
> +
> + /* RX delay *must* be specified if internal delay of RX is used. */
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
> + ret = ofnode_read_u32(node, "ti,rx-internal-delay",
> + &dp83867->rx_id_delay);
> + if (ret) {
> + pr_debug("ti,rx-internal-delay must be specified\n");
> + return ret;
> + }
> + if (dp83867->rx_id_delay > DP83867_RGMII_RX_CLK_DELAY_MAX) {
> + pr_debug("ti,rx-internal-delay value of %u out of range\n",
> + dp83867->rx_id_delay);
> + return -EINVAL;
> + }
> + }
> +
> + /* TX delay *must* be specified if internal delay of RX is used. */
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> + ret = ofnode_read_u32(node, "ti,tx-internal-delay",
> + &dp83867->tx_id_delay);
> + if (ret) {
> + debug("ti,tx-internal-delay must be specified\n");
> + return ret;
> + }
> + if (dp83867->tx_id_delay > DP83867_RGMII_TX_CLK_DELAY_MAX) {
> + pr_debug("ti,tx-internal-delay value of %u out of range\n",
> + dp83867->tx_id_delay);
> + return -EINVAL;
> + }
> + }
>
> dp83867->fifo_depth = ofnode_read_u32_default(node, "ti,fifo-depth",
> DEFAULT_FIFO_DEPTH);
> @@ -191,8 +241,8 @@ static int dp83867_of_init(struct phy_device *phydev)
> {
> struct dp83867_private *dp83867 = phydev->priv;
>
> - dp83867->rx_id_delay = DEFAULT_RX_ID_DELAY;
> - dp83867->tx_id_delay = DEFAULT_TX_ID_DELAY;
> + dp83867->rx_id_delay = DP83867_RGMIIDCTL_2_25_NS;
> + dp83867->tx_id_delay = DP83867_RGMIIDCTL_2_75_NS;
> dp83867->fifo_depth = DEFAULT_FIFO_DEPTH;
> dp83867->io_impedance = -EINVAL;
>
> @@ -281,6 +331,8 @@ static int dp83867_config(struct phy_device *phydev)
> val = phy_read_mmd(phydev, DP83867_DEVADDR,
> DP83867_RGMIICTL);
>
> + val &= ~(DP83867_RGMII_TX_CLK_DELAY_EN |
> + DP83867_RGMII_RX_CLK_DELAY_EN);
> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
> val |= (DP83867_RGMII_TX_CLK_DELAY_EN |
> DP83867_RGMII_RX_CLK_DELAY_EN);
> --
> 2.17.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
More information about the U-Boot
mailing list