[U-Boot] [RFC PATCH 1/2] net: phy: ti: Allow the driver to be more configurable

[Dan Murphy]

On 03/31/2016 07:42 AM, Dan Murphy wrote:
> Not all devices use the same internal delay or fifo depth.
> Add the ability to set the internal delay for rx or tx and the
> fifo depth via the config file.  If the value is not set in the
> config file then set the delay to the default.

This patch also aligns the uboot driver with the kernel driver.
And when DTS data is added the data_init api can be used
to populate the DTS data to the data structure.

>
> Signed-off-by: Dan Murphy <dmurphy at ti.com>
> ---
>  drivers/net/phy/ti.c                 | 71 ++++++++++++++++++++++++++++++++----
>  include/dt-bindings/net/ti-dp83867.h | 35 ++++++++++++++++++
>  2 files changed, 98 insertions(+), 8 deletions(-)
>  create mode 100644 include/dt-bindings/net/ti-dp83867.h
>
> diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c
> index c3912d5..6da0523 100644
> --- a/drivers/net/phy/ti.c
> +++ b/drivers/net/phy/ti.c
> @@ -6,6 +6,9 @@
>   */
>  #include <common.h>
>  #include <phy.h>
> +#include <linux/compat.h>
> +
> +#include <dt-bindings/net/ti-dp83867.h>
>  
>  /* TI DP83867 */
>  #define DP83867_DEVADDR		0x1f
> @@ -57,6 +60,17 @@
>  #define MII_MMD_CTRL_INCR_RDWT	0x8000 /* post increment on reads & writes */
>  #define MII_MMD_CTRL_INCR_ON_WT	0xC000 /* post increment on writes only */
>  
> +/* User setting - can be taken from DTS */
> +#define DEFAULT_RX_ID_DELAY	8
> +#define DEFAULT_TX_ID_DELAY	0xa
> +#define DEFAULT_FIFO_DEPTH	1
> +
> +struct dp83867_private {
> +	int rx_id_delay;
> +	int tx_id_delay;
> +	int fifo_depth;
> +};
> +
>  /**
>   * phy_read_mmd_indirect - reads data from the MMD registers
>   * @phydev: The PHY device bus
> @@ -134,16 +148,53 @@ static inline bool phy_interface_is_rgmii(struct phy_device *phydev)
>  		phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID;
>  }
>  
> -/* User setting - can be taken from DTS */
> -#define RX_ID_DELAY	8
> -#define TX_ID_DELAY	0xa
> -#define FIFO_DEPTH	1

Probably should use the #defines here instead of magic numbers.

> +/**
> + * dp83867_data_init - Convenience function for setting PHY specific data
> + * @phydev: the phy_device struct
> + */
> +static int dp83867_data_init(struct phy_device *phydev)
> +{
> +	struct dp83867_private *dp83867 = phydev->priv;
> +
> +#ifdef CONFIG_RGMII_RX_ID
> +	dp83867->rx_id_delay = CONFIG_RGMII_RX_ID;
> +#else
> +	dp83867->rx_id_delay = DEFAULT_RX_ID_DELAY;
> +#endif
> +
> +#ifdef CONFIG_RGMII_TX_ID
> +	dp83867->tx_id_delay = CONFIG_RGMII_TX_ID;
> +#else
> +	dp83867->tx_id_delay = DEFAULT_TX_ID_DELAY;
> +#endif
> +
> +#ifdef CONFIG_FIFO_DEPTH
> +	dp83867->fifo_depth = CONFIG_FIFO_DEPTH;
> +#else
> +	dp83867->fifo_depth = DEFAULT_FIFO_DEPTH;
> +#endif
> +	return 0;
> +}
>  
>  static int dp83867_config(struct phy_device *phydev)
>  {
> +	struct dp83867_private *dp83867;
>  	unsigned int val, delay;
>  	int ret;
>  
> +	if (!phydev->priv) {
> +		dp83867 = kzalloc(sizeof(*dp83867), GFP_KERNEL);
> +		if (!dp83867)
> +			return -ENOMEM;
> +
> +		phydev->priv = dp83867;
> +		ret = dp83867_data_init(phydev);
> +		if (ret)
> +			goto dp83867_write_error;
> +	} else {
> +		dp83867 = (struct dp83867_private *)phydev->priv;
> +	}
> +
>  	/* Restart the PHY.  */
>  	val = phy_read(phydev, MDIO_DEVAD_NONE, DP83867_CTRL);
>  	phy_write(phydev, MDIO_DEVAD_NONE, DP83867_CTRL,
> @@ -152,9 +203,9 @@ static int dp83867_config(struct phy_device *phydev)
>  	if (phy_interface_is_rgmii(phydev)) {
>  		ret = phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83867_PHYCTRL,
>  			(DP83867_MDI_CROSSOVER_AUTO << DP83867_MDI_CROSSOVER) |
> -			(FIFO_DEPTH << DP83867_PHYCR_FIFO_DEPTH_SHIFT));
> +			(dp83867->fifo_depth << DP83867_PHYCR_FIFO_DEPTH_SHIFT));
>  		if (ret)
> -			return ret;
> +			goto dp83867_write_error;
>  	}
>  
>  	if ((phydev->interface >= PHY_INTERFACE_MODE_RGMII_ID) &&
> @@ -175,8 +226,8 @@ static int dp83867_config(struct phy_device *phydev)
>  		phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
>  				       DP83867_DEVADDR, phydev->addr, val);
>  
> -		delay = (RX_ID_DELAY |
> -			 (TX_ID_DELAY << DP83867_RGMII_TX_CLK_DELAY_SHIFT));
> +		delay = (dp83867->rx_id_delay |
> +			 (dp83867->tx_id_delay << DP83867_RGMII_TX_CLK_DELAY_SHIFT));
>  
>  		phy_write_mmd_indirect(phydev, DP83867_RGMIIDCTL,
>  				       DP83867_DEVADDR, phydev->addr, delay);
> @@ -184,6 +235,10 @@ static int dp83867_config(struct phy_device *phydev)
>  
>  	genphy_config_aneg(phydev);
>  	return 0;
> +
> +dp83867_write_error:
> +	free(dp83867);
> +	return ret;
>  }
>  
>  static struct phy_driver DP83867_driver = {
> diff --git a/include/dt-bindings/net/ti-dp83867.h b/include/dt-bindings/net/ti-dp83867.h
> new file mode 100644
> index 0000000..5c592fb
> --- /dev/null
> +++ b/include/dt-bindings/net/ti-dp83867.h
> @@ -0,0 +1,35 @@
> +/*
> + * TI DP83867 PHY drivers
> + *
> + * SPDX-License-Identifier:	GPL-2.0
> + *
> + */
> +
> +#ifndef _DT_BINDINGS_TI_DP83867_H
> +#define _DT_BINDINGS_TI_DP83867_H
> +
> +/* PHY CTRL bits */
> +#define DP83867_PHYCR_FIFO_DEPTH_3_B_NIB	0x00
> +#define DP83867_PHYCR_FIFO_DEPTH_4_B_NIB	0x01
> +#define DP83867_PHYCR_FIFO_DEPTH_6_B_NIB	0x02
> +#define DP83867_PHYCR_FIFO_DEPTH_8_B_NIB	0x03
> +
> +/* RGMIIDCTL internal delay for rx and tx */
> +#define	DP83867_RGMIIDCTL_250_PS	0x0
> +#define	DP83867_RGMIIDCTL_500_PS	0x1
> +#define	DP83867_RGMIIDCTL_750_PS	0x2
> +#define	DP83867_RGMIIDCTL_1_NS		0x3
> +#define	DP83867_RGMIIDCTL_1_25_NS	0x4
> +#define	DP83867_RGMIIDCTL_1_50_NS	0x5
> +#define	DP83867_RGMIIDCTL_1_75_NS	0x6
> +#define	DP83867_RGMIIDCTL_2_00_NS	0x7
> +#define	DP83867_RGMIIDCTL_2_25_NS	0x8
> +#define	DP83867_RGMIIDCTL_2_50_NS	0x9
> +#define	DP83867_RGMIIDCTL_2_75_NS	0xa
> +#define	DP83867_RGMIIDCTL_3_00_NS	0xb
> +#define	DP83867_RGMIIDCTL_3_25_NS	0xc
> +#define	DP83867_RGMIIDCTL_3_50_NS	0xd
> +#define	DP83867_RGMIIDCTL_3_75_NS	0xe
> +#define	DP83867_RGMIIDCTL_4_00_NS	0xf
> +
> +#endif


-- 
------------------
Dan Murphy