[U-Boot] [uboot] [PATCH 1/7] drivers: net: cpsw: Add readinf of DT phy-handle node

[U-Boot] [uboot] [PATCH 1/7] drivers: net: cpsw: Add readinf of DT phy-handle node

[Dan Murphy]

Add the ability to read the phy-handle node of the
cpsw slave.  Upon reading this handle the phy-id
can be stored based on the reg node in the DT.

The phy-handle also needs to be stored and passed
to the phy to access any phy data that is available.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/net/cpsw.c | 18 ++++++++++++++++--
 include/cpsw.h     |  1 +
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
index 7104754..8ee33a6 100644
--- a/drivers/net/cpsw.c
+++ b/drivers/net/cpsw.c
@@ -965,6 +965,9 @@ static int cpsw_phy_init(struct cpsw_priv *priv, struct cpsw_slave *slave)
 	phydev->supported &= supported;
 	phydev->advertising = phydev->supported;
 
+	if (slave->data->phy_of_handle)
+		phydev->dev->of_offset = slave->data->phy_of_handle;
+
 	priv->phydev = phydev;
 	phy_config(phydev);
 
@@ -1217,8 +1220,19 @@ static int cpsw_eth_ofdata_to_platdata(struct udevice *dev)
 			if (phy_mode)
 				priv->data.slave_data[slave_index].phy_if =
 					phy_get_interface_by_name(phy_mode);
-			fdtdec_get_int_array(fdt, subnode, "phy_id", phy_id, 2);
-			priv->data.slave_data[slave_index].phy_addr = phy_id[1];
+
+			priv->data.slave_data[slave_index].phy_of_handle =
+				fdtdec_lookup_phandle(fdt, subnode, "phy-handle");
+
+			if (priv->data.slave_data[slave_index].phy_of_handle) {
+				priv->data.slave_data[slave_index].phy_addr =
+					fdtdec_get_int(gd->fdt_blob,
+						priv->data.slave_data[slave_index].phy_of_handle,
+						"reg", -1);
+			} else {
+				fdtdec_get_int_array(fdt, subnode, "phy_id", phy_id, 2);
+				priv->data.slave_data[slave_index].phy_addr = phy_id[1];
+			}
 			slave_index++;
 		}
 
diff --git a/include/cpsw.h b/include/cpsw.h
index cf1d30b..ba9d068 100644
--- a/include/cpsw.h
+++ b/include/cpsw.h
@@ -21,6 +21,7 @@ struct cpsw_slave_data {
 	u32		sliver_reg_ofs;
 	int		phy_addr;
 	int		phy_if;
+	int 		phy_of_handle;
 };
 
 enum {
-- 
2.8.0.rc3

[U-Boot] [uboot] [PATCH 2/7] net: zynq_gem: Add the passing of the phy-handle node

[Dan Murphy]

Add the ability to pass the phy-handle node offset
to the phy driver.  This allows the phy driver
to access the DT subnode's data and parse accordingly.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/net/zynq_gem.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
index aec8077..722927a 100644
--- a/drivers/net/zynq_gem.c
+++ b/drivers/net/zynq_gem.c
@@ -179,6 +179,7 @@ struct zynq_gem_priv {
 	struct zynq_gem_regs *iobase;
 	phy_interface_t interface;
 	struct phy_device *phydev;
+	int phy_of_handle;
 	struct mii_dev *bus;
 };
 
@@ -352,6 +353,10 @@ static int zynq_phy_init(struct udevice *dev)
 	priv->phydev->supported = supported | ADVERTISED_Pause |
 				  ADVERTISED_Asym_Pause;
 	priv->phydev->advertising = priv->phydev->supported;
+
+	if (priv->phy_of_handle > 0)
+		priv->phydev->dev->of_offset = priv->phy_of_handle;
+
 	phy_config(priv->phydev);
 
 	return 0;
@@ -684,10 +689,11 @@ static int zynq_gem_ofdata_to_platdata(struct udevice *dev)
 	priv->emio = 0;
 	priv->phyaddr = -1;
 
-	offset = fdtdec_lookup_phandle(gd->fdt_blob, dev->of_offset,
+	priv->phy_of_handle = fdtdec_lookup_phandle(gd->fdt_blob, dev->of_offset,
 				       "phy-handle");
-	if (offset > 0)
-		priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
+	if (priv->phy_of_handle > 0)
+		priv->phyaddr = fdtdec_get_int(gd->fdt_blob,
+					priv->phy_of_handle, "reg", -1);
 
 	phy_mode = fdt_getprop(gd->fdt_blob, dev->of_offset, "phy-mode", NULL);
 	if (phy_mode)
-- 
2.8.0.rc3

[U-Boot] [uboot] [PATCH 2/7] net: zynq_gem: Add the passing of the phy-handle node

[Mugunthan V N]

On Friday 08 April 2016 09:38 PM, Dan Murphy wrote:
> Add the ability to pass the phy-handle node offset
> to the phy driver.  This allows the phy driver
> to access the DT subnode's data and parse accordingly.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

[U-Boot] [uboot] [PATCH 2/7] net: zynq_gem: Add the passing of the phy-handle node

[Michal Simek]

On 8.4.2016 18:08, Dan Murphy wrote:
> Add the ability to pass the phy-handle node offset
> to the phy driver.  This allows the phy driver
> to access the DT subnode's data and parse accordingly.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  drivers/net/zynq_gem.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
> index aec8077..722927a 100644
> --- a/drivers/net/zynq_gem.c
> +++ b/drivers/net/zynq_gem.c
> @@ -179,6 +179,7 @@ struct zynq_gem_priv {
>  	struct zynq_gem_regs *iobase;
>  	phy_interface_t interface;
>  	struct phy_device *phydev;
> +	int phy_of_handle;
>  	struct mii_dev *bus;
>  };
>  
> @@ -352,6 +353,10 @@ static int zynq_phy_init(struct udevice *dev)
>  	priv->phydev->supported = supported | ADVERTISED_Pause |
>  				  ADVERTISED_Asym_Pause;
>  	priv->phydev->advertising = priv->phydev->supported;
> +
> +	if (priv->phy_of_handle > 0)
> +		priv->phydev->dev->of_offset = priv->phy_of_handle;
> +
>  	phy_config(priv->phydev);
>  
>  	return 0;
> @@ -684,10 +689,11 @@ static int zynq_gem_ofdata_to_platdata(struct udevice *dev)
>  	priv->emio = 0;
>  	priv->phyaddr = -1;
>  
> -	offset = fdtdec_lookup_phandle(gd->fdt_blob, dev->of_offset,
> +	priv->phy_of_handle = fdtdec_lookup_phandle(gd->fdt_blob, dev->of_offset,
>  				       "phy-handle");

This line is longer than 80 chars - checkpatch is failing on it.


> -	if (offset > 0)
> -		priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
> +	if (priv->phy_of_handle > 0)
> +		priv->phyaddr = fdtdec_get_int(gd->fdt_blob,
> +					priv->phy_of_handle, "reg", -1);
>  
>  	phy_mode = fdt_getprop(gd->fdt_blob, dev->of_offset, "phy-mode", NULL);
>  	if (phy_mode)
> 

You have to remove offset too.
drivers/net/zynq_gem.c:683:6: warning: unused variable ?offset?
[-Wunused-variable]
  int offset = 0;
      ^

With changes I have done myself it looks that it is working. I will
retest next version.

Thanks,
Michal

[U-Boot] [uboot] [PATCH v4 3/7] net: phy: dp83867: Add device tree bindings and documentation

[Dan Murphy]

Add the device tree bindings and the accompanying documentation
for the TI DP83867 Giga bit ethernet phy driver.

The original document was from:
    [commit 2a10154abcb75ad0d7b6bfea6210ac743ec60897 from the Linux kernel]

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v3- Update the bindings to the reflect the implementation

 doc/device-tree-bindings/net/ti,dp83867.txt | 25 +++++++++++++++++++++
 include/dt-bindings/net/ti-dp83867.h        | 35 +++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)
 create mode 100644 doc/device-tree-bindings/net/ti,dp83867.txt
 create mode 100644 include/dt-bindings/net/ti-dp83867.h

diff --git a/doc/device-tree-bindings/net/ti,dp83867.txt b/doc/device-tree-bindings/net/ti,dp83867.txt
new file mode 100644
index 0000000..cb77fdf
--- /dev/null
+++ b/doc/device-tree-bindings/net/ti,dp83867.txt
@@ -0,0 +1,25 @@
+* Texas Instruments - dp83867 Giga bit ethernet phy
+
+Required properties:
+	- reg - The ID number for the phy, usually a small integer
+	- ti,rx-internal-delay - RGMII Recieve Clock Delay - see dt-bindings/net/ti-dp83867.h
+		for applicable values
+	- ti,tx-internal-delay - RGMII Transmit Clock Delay - see dt-bindings/net/ti-dp83867.h
+		for applicable values
+	- ti,fifo-depth - Transmitt FIFO depth- see dt-bindings/net/ti-dp83867.h
+		for applicable values
+
+Default child nodes are standard Ethernet PHY device
+nodes as described in doc/devicetree/bindings/net/ethernet.txt
+
+Example:
+
+	ethernet-phy at 0 {
+		reg = <0>;
+		ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
+		ti,tx-internal-delay = <DP83867_RGMIIDCTL_2_75_NS>;
+		ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
+	};
+
+Datasheet can be found:
+http://www.ti.com/product/DP83867IR/datasheet
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
-- 
2.8.0.rc3

[U-Boot] [uboot] [PATCH v4 3/7] net: phy: dp83867: Add device tree bindings and documentation

[Mugunthan V N]

On Friday 08 April 2016 09:38 PM, Dan Murphy wrote:
> Add the device tree bindings and the accompanying documentation
> for the TI DP83867 Giga bit ethernet phy driver.
> 
> The original document was from:
>     [commit 2a10154abcb75ad0d7b6bfea6210ac743ec60897 from the Linux kernel]
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

[U-Boot] [uboot] [PATCH v4 3/7] net: phy: dp83867: Add device tree bindings and documentation

[Michal Simek]

On 8.4.2016 18:08, Dan Murphy wrote:
> Add the device tree bindings and the accompanying documentation
> for the TI DP83867 Giga bit ethernet phy driver.
> 
> The original document was from:
>     [commit 2a10154abcb75ad0d7b6bfea6210ac743ec60897 from the Linux kernel]
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v3- Update the bindings to the reflect the implementation
> 
>  doc/device-tree-bindings/net/ti,dp83867.txt | 25 +++++++++++++++++++++
>  include/dt-bindings/net/ti-dp83867.h        | 35 +++++++++++++++++++++++++++++
>  2 files changed, 60 insertions(+)
>  create mode 100644 doc/device-tree-bindings/net/ti,dp83867.txt
>  create mode 100644 include/dt-bindings/net/ti-dp83867.h
> 
> diff --git a/doc/device-tree-bindings/net/ti,dp83867.txt b/doc/device-tree-bindings/net/ti,dp83867.txt
> new file mode 100644
> index 0000000..cb77fdf
> --- /dev/null
> +++ b/doc/device-tree-bindings/net/ti,dp83867.txt
> @@ -0,0 +1,25 @@
> +* Texas Instruments - dp83867 Giga bit ethernet phy
> +
> +Required properties:
> +	- reg - The ID number for the phy, usually a small integer
> +	- ti,rx-internal-delay - RGMII Recieve Clock Delay - see dt-bindings/net/ti-dp83867.h
> +		for applicable values
> +	- ti,tx-internal-delay - RGMII Transmit Clock Delay - see dt-bindings/net/ti-dp83867.h
> +		for applicable values
> +	- ti,fifo-depth - Transmitt FIFO depth- see dt-bindings/net/ti-dp83867.h
> +		for applicable values
> +
> +Default child nodes are standard Ethernet PHY device
> +nodes as described in doc/devicetree/bindings/net/ethernet.txt
> +
> +Example:
> +
> +	ethernet-phy at 0 {
> +		reg = <0>;
> +		ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
> +		ti,tx-internal-delay = <DP83867_RGMIIDCTL_2_75_NS>;
> +		ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
> +	};
> +
> +Datasheet can be found:
> +http://www.ti.com/product/DP83867IR/datasheet
> 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
> + *

I would remove this one line above.

> + */
> +
> +#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
> 

It looks weird that you have in one patch #define<space or tab>name.

Please sync it.

Thanks,
Michal

[U-Boot] [uboot] [PATCH v4 4/7] net: phy: ti: Allow the driver to be more configurable

[Dan Murphy]

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 devicetree.  If the value is not set in the
devicetree then set the delay to the default.

If devicetree is not used then use the default defines within the
driver.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v4 - Rebased the driver and changed default values to -1

 drivers/net/phy/ti.c | 85 +++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 75 insertions(+), 10 deletions(-)

diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c
index 937426b..922f0a4 100644
--- a/drivers/net/phy/ti.c
+++ b/drivers/net/phy/ti.c
@@ -6,6 +6,14 @@
  */
 #include <common.h>
 #include <phy.h>
+#include <linux/compat.h>
+#include <malloc.h>
+
+#include <fdtdec.h>
+#include <dm.h>
+#include <dt-bindings/net/ti-dp83867.h>
+
+DECLARE_GLOBAL_DATA_PTR;
 
 /* TI DP83867 */
 #define DP83867_DEVADDR		0x1f
@@ -71,6 +79,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	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
+
+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
@@ -148,16 +167,58 @@ 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
+#if defined(CONFIG_DM_ETH)
+/**
+ * dp83867_data_init - Convenience function for setting PHY specific data
+ *
+ * @phydev: the phy_device struct
+ */
+static int dp83867_of_init(struct phy_device *phydev)
+{
+	struct dp83867_private *dp83867 = phydev->priv;
+	struct udevice *dev = phydev->dev;
+
+	dp83867->rx_id_delay = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
+				 "ti,rx_internal_delay", -1);
+
+	dp83867->tx_id_delay = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
+				 "ti,tx_internal_delay", -1);
+
+	dp83867->fifo_depth = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
+				 "ti,fifo_depth", -1);
+
+	return 0;
+}
+#else
+static int dp83867_of_init(struct phy_device *phydev)
+{
+	dp83867->rx_id_delay = DEFAULT_RX_ID_DELAY;
+	dp83867->tx_id_delay = DEFAULT_TX_ID_DELAY;
+	dp83867->fifo_depth = DEFAULT_FIFO_DEPTH;
+
+	return 0;
+}
+#endif
 
 static int dp83867_config(struct phy_device *phydev)
 {
+	struct dp83867_private *dp83867;
 	unsigned int val, delay, cfg2;
 	int ret;
 
+	if (!phydev->priv) {
+		dp83867 = kzalloc(sizeof(*dp83867), GFP_KERNEL);
+		if (!dp83867)
+			return -ENOMEM;
+
+		phydev->priv = dp83867;
+		ret = dp83867_of_init(phydev);
+		if (ret)
+			goto err_out;
+	} 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,
@@ -166,9 +227,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 err_out;
 	} else {
 		phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR,
 			  (BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000));
@@ -189,8 +250,8 @@ static int dp83867_config(struct phy_device *phydev)
 			  DP83867_PHYCTRL_SGMIIEN |
 			  (DP83867_MDI_CROSSOVER_MDIX <<
 			  DP83867_MDI_CROSSOVER) |
-			  (FIFO_DEPTH << DP83867_PHYCTRL_RXFIFO_SHIFT) |
-			  (FIFO_DEPTH  << DP83867_PHYCTRL_TXFIFO_SHIFT));
+			  (dp83867->fifo_depth << DP83867_PHYCTRL_RXFIFO_SHIFT) |
+			  (dp83867->fifo_depth  << DP83867_PHYCTRL_TXFIFO_SHIFT));
 		phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83867_BISCR, 0x0);
 	}
 
@@ -212,8 +273,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);
@@ -221,6 +282,10 @@ static int dp83867_config(struct phy_device *phydev)
 
 	genphy_config_aneg(phydev);
 	return 0;
+
+err_out:
+	kfree(dp83867);
+	return ret;
 }
 
 static struct phy_driver DP83867_driver = {
-- 
2.8.0.rc3

[U-Boot] [uboot] [PATCH v4 4/7] net: phy: ti: Allow the driver to be more configurable

[Mugunthan V N]

On Friday 08 April 2016 09:38 PM, 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 devicetree.  If the value is not set in the
> devicetree then set the delay to the default.
> 
> If devicetree is not used then use the default defines within the
> driver.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

[U-Boot] [uboot] [PATCH v4 4/7] net: phy: ti: Allow the driver to be more configurable

[Michal Simek]

On 8.4.2016 18:08, 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 devicetree.  If the value is not set in the
> devicetree then set the delay to the default.
> 
> If devicetree is not used then use the default defines within the
> driver.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v4 - Rebased the driver and changed default values to -1
> 
>  drivers/net/phy/ti.c | 85 +++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 75 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c
> index 937426b..922f0a4 100644
> --- a/drivers/net/phy/ti.c
> +++ b/drivers/net/phy/ti.c
> @@ -6,6 +6,14 @@
>   */
>  #include <common.h>
>  #include <phy.h>
> +#include <linux/compat.h>
> +#include <malloc.h>
> +
> +#include <fdtdec.h>
> +#include <dm.h>
> +#include <dt-bindings/net/ti-dp83867.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
>  
>  /* TI DP83867 */
>  #define DP83867_DEVADDR		0x1f
> @@ -71,6 +79,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	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
> +
> +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
> @@ -148,16 +167,58 @@ 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
> +#if defined(CONFIG_DM_ETH)
> +/**
> + * dp83867_data_init - Convenience function for setting PHY specific data
> + *
> + * @phydev: the phy_device struct
> + */
> +static int dp83867_of_init(struct phy_device *phydev)
> +{
> +	struct dp83867_private *dp83867 = phydev->priv;
> +	struct udevice *dev = phydev->dev;
> +
> +	dp83867->rx_id_delay = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
> +				 "ti,rx_internal_delay", -1);
> +
> +	dp83867->tx_id_delay = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
> +				 "ti,tx_internal_delay", -1);
> +
> +	dp83867->fifo_depth = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
> +				 "ti,fifo_depth", -1);
> +

These names are still incompatible with binding you have.
There are dashes instead of underscores.


> +	return 0;
> +}
> +#else
> +static int dp83867_of_init(struct phy_device *phydev)
> +{
> +	dp83867->rx_id_delay = DEFAULT_RX_ID_DELAY;
> +	dp83867->tx_id_delay = DEFAULT_TX_ID_DELAY;
> +	dp83867->fifo_depth = DEFAULT_FIFO_DEPTH;
> +
> +	return 0;
> +}
> +#endif
>  
>  static int dp83867_config(struct phy_device *phydev)
>  {
> +	struct dp83867_private *dp83867;
>  	unsigned int val, delay, cfg2;
>  	int ret;
>  
> +	if (!phydev->priv) {
> +		dp83867 = kzalloc(sizeof(*dp83867), GFP_KERNEL);
> +		if (!dp83867)
> +			return -ENOMEM;
> +
> +		phydev->priv = dp83867;
> +		ret = dp83867_of_init(phydev);
> +		if (ret)
> +			goto err_out;
> +	} 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,
> @@ -166,9 +227,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 err_out;
>  	} else {
>  		phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR,
>  			  (BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000));
> @@ -189,8 +250,8 @@ static int dp83867_config(struct phy_device *phydev)
>  			  DP83867_PHYCTRL_SGMIIEN |
>  			  (DP83867_MDI_CROSSOVER_MDIX <<
>  			  DP83867_MDI_CROSSOVER) |
> -			  (FIFO_DEPTH << DP83867_PHYCTRL_RXFIFO_SHIFT) |
> -			  (FIFO_DEPTH  << DP83867_PHYCTRL_TXFIFO_SHIFT));
> +			  (dp83867->fifo_depth << DP83867_PHYCTRL_RXFIFO_SHIFT) |
> +			  (dp83867->fifo_depth  << DP83867_PHYCTRL_TXFIFO_SHIFT));

                                              ^^
Here are 2 spaces instead of 1

>  		phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83867_BISCR, 0x0);
>  	}
>  
> @@ -212,8 +273,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);
> @@ -221,6 +282,10 @@ static int dp83867_config(struct phy_device *phydev)
>  
>  	genphy_config_aneg(phydev);
>  	return 0;
> +
> +err_out:
> +	kfree(dp83867);
> +	return ret;
>  }
>  
>  static struct phy_driver DP83867_driver = {
> 

This patch is breaking 80 chars limit reported by checkpatch.

Thanks,
Michal

[U-Boot] [uboot] [PATCH v4 4/7] net: phy: ti: Allow the driver to be more configurable

[Dan Murphy]

On 04/11/2016 02:08 AM, Michal Simek wrote:
> On 8.4.2016 18:08, 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 devicetree.  If the value is not set in the
>> devicetree then set the delay to the default.
>>
>> If devicetree is not used then use the default defines within the
>> driver.
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>>
>> v4 - Rebased the driver and changed default values to -1
>>
>>  drivers/net/phy/ti.c | 85 +++++++++++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 75 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c
>> index 937426b..922f0a4 100644
>> --- a/drivers/net/phy/ti.c
>> +++ b/drivers/net/phy/ti.c
>> @@ -6,6 +6,14 @@
>>   */
>>  #include <common.h>
>>  #include <phy.h>
>> +#include <linux/compat.h>
>> +#include <malloc.h>
>> +
>> +#include <fdtdec.h>
>> +#include <dm.h>
>> +#include <dt-bindings/net/ti-dp83867.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>>  
>>  /* TI DP83867 */
>>  #define DP83867_DEVADDR		0x1f
>> @@ -71,6 +79,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	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
>> +
>> +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
>> @@ -148,16 +167,58 @@ 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
>> +#if defined(CONFIG_DM_ETH)
>> +/**
>> + * dp83867_data_init - Convenience function for setting PHY specific data
>> + *
>> + * @phydev: the phy_device struct
>> + */
>> +static int dp83867_of_init(struct phy_device *phydev)
>> +{
>> +	struct dp83867_private *dp83867 = phydev->priv;
>> +	struct udevice *dev = phydev->dev;
>> +
>> +	dp83867->rx_id_delay = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
>> +				 "ti,rx_internal_delay", -1);
>> +
>> +	dp83867->tx_id_delay = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
>> +				 "ti,tx_internal_delay", -1);
>> +
>> +	dp83867->fifo_depth = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
>> +				 "ti,fifo_depth", -1);
>> +
> These names are still incompatible with binding you have.
> There are dashes instead of underscores.

OK fixed in new version.

>
>
>> +	return 0;
>> +}
>> +#else
>> +static int dp83867_of_init(struct phy_device *phydev)
>> +{
>> +	dp83867->rx_id_delay = DEFAULT_RX_ID_DELAY;
>> +	dp83867->tx_id_delay = DEFAULT_TX_ID_DELAY;
>> +	dp83867->fifo_depth = DEFAULT_FIFO_DEPTH;
>> +
>> +	return 0;
>> +}
>> +#endif
>>  
>>  static int dp83867_config(struct phy_device *phydev)
>>  {
>> +	struct dp83867_private *dp83867;
>>  	unsigned int val, delay, cfg2;
>>  	int ret;
>>  
>> +	if (!phydev->priv) {
>> +		dp83867 = kzalloc(sizeof(*dp83867), GFP_KERNEL);
>> +		if (!dp83867)
>> +			return -ENOMEM;
>> +
>> +		phydev->priv = dp83867;
>> +		ret = dp83867_of_init(phydev);
>> +		if (ret)
>> +			goto err_out;
>> +	} 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,
>> @@ -166,9 +227,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 err_out;
>>  	} else {
>>  		phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR,
>>  			  (BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000));
>> @@ -189,8 +250,8 @@ static int dp83867_config(struct phy_device *phydev)
>>  			  DP83867_PHYCTRL_SGMIIEN |
>>  			  (DP83867_MDI_CROSSOVER_MDIX <<
>>  			  DP83867_MDI_CROSSOVER) |
>> -			  (FIFO_DEPTH << DP83867_PHYCTRL_RXFIFO_SHIFT) |
>> -			  (FIFO_DEPTH  << DP83867_PHYCTRL_TXFIFO_SHIFT));
>> +			  (dp83867->fifo_depth << DP83867_PHYCTRL_RXFIFO_SHIFT) |
>> +			  (dp83867->fifo_depth  << DP83867_PHYCTRL_TXFIFO_SHIFT));
>                                               ^^
> Here are 2 spaces instead of 1

Removed

>
>>  		phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83867_BISCR, 0x0);
>>  	}
>>  
>> @@ -212,8 +273,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);
>> @@ -221,6 +282,10 @@ static int dp83867_config(struct phy_device *phydev)
>>  
>>  	genphy_config_aneg(phydev);
>>  	return 0;
>> +
>> +err_out:
>> +	kfree(dp83867);
>> +	return ret;
>>  }
>>  
>>  static struct phy_driver DP83867_driver = {
>>
> This patch is breaking 80 chars limit reported by checkpatch.


80 char limit is not an Error it is a Warning.
Well this is up to the maintainer.  This is the way the driver is written in the kernel.

I want to keep the uboot file as close to the way I wrote it in the kernel.  The same
warnings appear in the kernel code and this was accepted

Dan

>
> Thanks,
> Michal


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

[U-Boot] [uboot] [PATCH v4 4/7] net: phy: ti: Allow the driver to be more configurable

[Michal Simek]

On 11.4.2016 14:01, Dan Murphy wrote:
> On 04/11/2016 02:08 AM, Michal Simek wrote:
>> On 8.4.2016 18:08, 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 devicetree.  If the value is not set in the
>>> devicetree then set the delay to the default.
>>>
>>> If devicetree is not used then use the default defines within the
>>> driver.
>>>
>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>> ---
>>>
>>> v4 - Rebased the driver and changed default values to -1
>>>
>>>  drivers/net/phy/ti.c | 85 +++++++++++++++++++++++++++++++++++++++++++++-------
>>>  1 file changed, 75 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c
>>> index 937426b..922f0a4 100644
>>> --- a/drivers/net/phy/ti.c
>>> +++ b/drivers/net/phy/ti.c
>>> @@ -6,6 +6,14 @@
>>>   */
>>>  #include <common.h>
>>>  #include <phy.h>
>>> +#include <linux/compat.h>
>>> +#include <malloc.h>
>>> +
>>> +#include <fdtdec.h>
>>> +#include <dm.h>
>>> +#include <dt-bindings/net/ti-dp83867.h>
>>> +
>>> +DECLARE_GLOBAL_DATA_PTR;
>>>  
>>>  /* TI DP83867 */
>>>  #define DP83867_DEVADDR		0x1f
>>> @@ -71,6 +79,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	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
>>> +
>>> +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
>>> @@ -148,16 +167,58 @@ 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
>>> +#if defined(CONFIG_DM_ETH)
>>> +/**
>>> + * dp83867_data_init - Convenience function for setting PHY specific data
>>> + *
>>> + * @phydev: the phy_device struct
>>> + */
>>> +static int dp83867_of_init(struct phy_device *phydev)
>>> +{
>>> +	struct dp83867_private *dp83867 = phydev->priv;
>>> +	struct udevice *dev = phydev->dev;
>>> +
>>> +	dp83867->rx_id_delay = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
>>> +				 "ti,rx_internal_delay", -1);
>>> +
>>> +	dp83867->tx_id_delay = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
>>> +				 "ti,tx_internal_delay", -1);
>>> +
>>> +	dp83867->fifo_depth = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
>>> +				 "ti,fifo_depth", -1);
>>> +
>> These names are still incompatible with binding you have.
>> There are dashes instead of underscores.
> 
> OK fixed in new version.
> 
>>
>>
>>> +	return 0;
>>> +}
>>> +#else
>>> +static int dp83867_of_init(struct phy_device *phydev)
>>> +{
>>> +	dp83867->rx_id_delay = DEFAULT_RX_ID_DELAY;
>>> +	dp83867->tx_id_delay = DEFAULT_TX_ID_DELAY;
>>> +	dp83867->fifo_depth = DEFAULT_FIFO_DEPTH;
>>> +
>>> +	return 0;
>>> +}
>>> +#endif
>>>  
>>>  static int dp83867_config(struct phy_device *phydev)
>>>  {
>>> +	struct dp83867_private *dp83867;
>>>  	unsigned int val, delay, cfg2;
>>>  	int ret;
>>>  
>>> +	if (!phydev->priv) {
>>> +		dp83867 = kzalloc(sizeof(*dp83867), GFP_KERNEL);
>>> +		if (!dp83867)
>>> +			return -ENOMEM;
>>> +
>>> +		phydev->priv = dp83867;
>>> +		ret = dp83867_of_init(phydev);
>>> +		if (ret)
>>> +			goto err_out;
>>> +	} 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,
>>> @@ -166,9 +227,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 err_out;
>>>  	} else {
>>>  		phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR,
>>>  			  (BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000));
>>> @@ -189,8 +250,8 @@ static int dp83867_config(struct phy_device *phydev)
>>>  			  DP83867_PHYCTRL_SGMIIEN |
>>>  			  (DP83867_MDI_CROSSOVER_MDIX <<
>>>  			  DP83867_MDI_CROSSOVER) |
>>> -			  (FIFO_DEPTH << DP83867_PHYCTRL_RXFIFO_SHIFT) |
>>> -			  (FIFO_DEPTH  << DP83867_PHYCTRL_TXFIFO_SHIFT));
>>> +			  (dp83867->fifo_depth << DP83867_PHYCTRL_RXFIFO_SHIFT) |
>>> +			  (dp83867->fifo_depth  << DP83867_PHYCTRL_TXFIFO_SHIFT));
>>                                               ^^
>> Here are 2 spaces instead of 1
> 
> Removed
> 
>>
>>>  		phy_write(phydev, MDIO_DEVAD_NONE, MII_DP83867_BISCR, 0x0);
>>>  	}
>>>  
>>> @@ -212,8 +273,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);
>>> @@ -221,6 +282,10 @@ static int dp83867_config(struct phy_device *phydev)
>>>  
>>>  	genphy_config_aneg(phydev);
>>>  	return 0;
>>> +
>>> +err_out:
>>> +	kfree(dp83867);
>>> +	return ret;
>>>  }
>>>  
>>>  static struct phy_driver DP83867_driver = {
>>>
>> This patch is breaking 80 chars limit reported by checkpatch.
> 
> 
> 80 char limit is not an Error it is a Warning.
> Well this is up to the maintainer.  This is the way the driver is written in the kernel.
> 
> I want to keep the uboot file as close to the way I wrote it in the kernel.  The same
> warnings appear in the kernel code and this was accepted

ok. No problem with it.

Thanks,
Michal

[U-Boot] [uboot] [PATCH v4 4/7] net: phy: ti: Allow the driver to be more configurable

[Dan Murphy]

Michal

On 04/11/2016 07:11 AM, Michal Simek wrote:
> On 11.4.2016 14:01, Dan Murphy wrote:
>> On 04/11/2016 02:08 AM, Michal Simek wrote:
>>> On 8.4.2016 18:08, 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 devicetree.  If the value is not set in the
>>>> devicetree then set the delay to the default.
>>>>
>>>> If devicetree is not used then use the default defines within the
>>>> driver.
>>>>
>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>>> ---
>>>>
>>>> v4 - Rebased the driver and changed default values to -1
>>>>
>>>>  drivers/net/phy/ti.c | 85 +++++++++++++++++++++++++++++++++++++++++++++-------
>>>>  1 file changed, 75 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c
>>>> index 937426b..922f0a4 100644
>>>> --- a/drivers/net/phy/ti.c
>>>> +++ b/drivers/net/phy/ti.c
>>>> @@ -6,6 +6,14 @@
>>>>   */
>>>>  #include <common.h>
>>>>  #include <phy.h>
>>>> +#include <linux/compat.h>
>>>> +#include <malloc.h>
>>>> +
>>>> +#include <fdtdec.h>
>>>> +#include <dm.h>
>>>> +#include <dt-bindings/net/ti-dp83867.h>
>>>> +
>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>>  
>>>>  /* TI DP83867 */
>>>>  #define DP83867_DEVADDR		0x1f
>>>> @@ -71,6 +79,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	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
>>>> +
>>>> +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
>>>> @@ -148,16 +167,58 @@ 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
>>>> +#if defined(CONFIG_DM_ETH)
>>>> +/**
>>>> + * dp83867_data_init - Convenience function for setting PHY specific data
>>>> + *
>>>> + * @phydev: the phy_device struct
>>>> + */
>>>> +static int dp83867_of_init(struct phy_device *phydev)
>>>> +{
>>>> +	struct dp83867_private *dp83867 = phydev->priv;
>>>> +	struct udevice *dev = phydev->dev;
>>>> +
>>>> +	dp83867->rx_id_delay = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
>>>> +				 "ti,rx_internal_delay", -1);
>>>> +
>>>> +	dp83867->tx_id_delay = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
>>>> +				 "ti,tx_internal_delay", -1);
>>>> +
>>>> +	dp83867->fifo_depth = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
>>>> +				 "ti,fifo_depth", -1);
>>>> +
>>> These names are still incompatible with binding you have.
>>> There are dashes instead of underscores.
>> OK fixed in new version.

FYI I am changing these to the documented as it is in the kernel.
Changing the "_" to "-" so your dt file needs to be updated

<snip>

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

[U-Boot] [uboot] [PATCH 5/7] net: phy: Move is_rgmii helper to phy.h

[Dan Murphy]

Move the phy_interface_is_rgmii to the phy.h
file for all phy's to be able to use the API.

This now aligns with the Linux kernel based on
commit e463d88c36d42211aa72ed76d32fb8bf37820ef1

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/net/phy/ti.c | 11 -----------
 include/phy.h        | 11 +++++++++++
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c
index 922f0a4..d503eef 100644
--- a/drivers/net/phy/ti.c
+++ b/drivers/net/phy/ti.c
@@ -156,17 +156,6 @@ void phy_write_mmd_indirect(struct phy_device *phydev, int prtad,
 	phy_write(phydev, addr, MII_MMD_DATA, data);
 }
 
-/**
- * phy_interface_is_rgmii - Convenience function for testing if a PHY interface
- * is RGMII (all variants)
- * @phydev: the phy_device struct
- */
-static inline bool phy_interface_is_rgmii(struct phy_device *phydev)
-{
-	return phydev->interface >= PHY_INTERFACE_MODE_RGMII &&
-		phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID;
-}
-
 #if defined(CONFIG_DM_ETH)
 /**
  * dp83867_data_init - Convenience function for setting PHY specific data
diff --git a/include/phy.h b/include/phy.h
index 21459a8..7b2d1ff 100644
--- a/include/phy.h
+++ b/include/phy.h
@@ -277,6 +277,17 @@ int get_phy_id(struct mii_dev *bus, int addr, int devad, u32 *phy_id);
  */
 int phy_get_interface_by_name(const char *str);
 
+/**
+ * phy_interface_is_rgmii - Convenience function for testing if a PHY interface
+ * is RGMII (all variants)
+ * @phydev: the phy_device struct
+ */
+static inline bool phy_interface_is_rgmii(struct phy_device *phydev)
+{
+	return phydev->interface >= PHY_INTERFACE_MODE_RGMII &&
+		phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID;
+}
+
 /* PHY UIDs for various PHYs that are referenced in external code */
 #define PHY_UID_CS4340  0x13e51002
 #define PHY_UID_TN2020	0x00a19410
-- 
2.8.0.rc3

[U-Boot] [uboot] [PATCH 5/7] net: phy: Move is_rgmii helper to phy.h

[Mugunthan V N]

On Friday 08 April 2016 09:38 PM, Dan Murphy wrote:
> Move the phy_interface_is_rgmii to the phy.h
> file for all phy's to be able to use the API.
> 
> This now aligns with the Linux kernel based on
> commit e463d88c36d42211aa72ed76d32fb8bf37820ef1
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

[U-Boot] [uboot] [PATCH 5/7] net: phy: Move is_rgmii helper to phy.h

[Michal Simek]

On 8.4.2016 18:08, Dan Murphy wrote:
> Move the phy_interface_is_rgmii to the phy.h
> file for all phy's to be able to use the API.
> 
> This now aligns with the Linux kernel based on
> commit e463d88c36d42211aa72ed76d32fb8bf37820ef1
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  drivers/net/phy/ti.c | 11 -----------
>  include/phy.h        | 11 +++++++++++
>  2 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c
> index 922f0a4..d503eef 100644
> --- a/drivers/net/phy/ti.c
> +++ b/drivers/net/phy/ti.c
> @@ -156,17 +156,6 @@ void phy_write_mmd_indirect(struct phy_device *phydev, int prtad,
>  	phy_write(phydev, addr, MII_MMD_DATA, data);
>  }
>  
> -/**
> - * phy_interface_is_rgmii - Convenience function for testing if a PHY interface
> - * is RGMII (all variants)
> - * @phydev: the phy_device struct
> - */
> -static inline bool phy_interface_is_rgmii(struct phy_device *phydev)
> -{
> -	return phydev->interface >= PHY_INTERFACE_MODE_RGMII &&
> -		phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID;
> -}
> -
>  #if defined(CONFIG_DM_ETH)
>  /**
>   * dp83867_data_init - Convenience function for setting PHY specific data
> diff --git a/include/phy.h b/include/phy.h
> index 21459a8..7b2d1ff 100644
> --- a/include/phy.h
> +++ b/include/phy.h
> @@ -277,6 +277,17 @@ int get_phy_id(struct mii_dev *bus, int addr, int devad, u32 *phy_id);
>   */
>  int phy_get_interface_by_name(const char *str);
>  
> +/**
> + * phy_interface_is_rgmii - Convenience function for testing if a PHY interface
> + * is RGMII (all variants)
> + * @phydev: the phy_device struct
> + */
> +static inline bool phy_interface_is_rgmii(struct phy_device *phydev)
> +{
> +	return phydev->interface >= PHY_INTERFACE_MODE_RGMII &&
> +		phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID;
> +}
> +
>  /* PHY UIDs for various PHYs that are referenced in external code */
>  #define PHY_UID_CS4340  0x13e51002
>  #define PHY_UID_TN2020	0x00a19410
> 

Reviewed-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

[U-Boot] [uboot] [PATCH 6/7] net: phy: Add phy_interface_is_sgmii to phy.h

[Dan Murphy]

Add a helper to phy.h to identify whether the
phy is configured for SGMII all variables.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 include/phy.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/phy.h b/include/phy.h
index 7b2d1ff..ef3eb51 100644
--- a/include/phy.h
+++ b/include/phy.h
@@ -288,6 +288,17 @@ static inline bool phy_interface_is_rgmii(struct phy_device *phydev)
 		phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID;
 }
 
+/**
+ * phy_interface_is_sgmii - Convenience function for testing if a PHY interface
+ * is SGMII (all variants)
+ * @phydev: the phy_device struct
+ */
+static inline bool phy_interface_is_sgmii(struct phy_device *phydev)
+{
+	return phydev->interface >= PHY_INTERFACE_MODE_SGMII &&
+		phydev->interface <= PHY_INTERFACE_MODE_QSGMII;
+}
+
 /* PHY UIDs for various PHYs that are referenced in external code */
 #define PHY_UID_CS4340  0x13e51002
 #define PHY_UID_TN2020	0x00a19410
-- 
2.8.0.rc3

[U-Boot] [uboot] [PATCH 6/7] net: phy: Add phy_interface_is_sgmii to phy.h

[Mugunthan V N]

On Friday 08 April 2016 09:38 PM, Dan Murphy wrote:
> Add a helper to phy.h to identify whether the
> phy is configured for SGMII all variables.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

[U-Boot] [uboot] [PATCH 6/7] net: phy: Add phy_interface_is_sgmii to phy.h

[Michal Simek]

On 8.4.2016 18:08, Dan Murphy wrote:
> Add a helper to phy.h to identify whether the
> phy is configured for SGMII all variables.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  include/phy.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/include/phy.h b/include/phy.h
> index 7b2d1ff..ef3eb51 100644
> --- a/include/phy.h
> +++ b/include/phy.h
> @@ -288,6 +288,17 @@ static inline bool phy_interface_is_rgmii(struct phy_device *phydev)
>  		phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID;
>  }
>  
> +/**
> + * phy_interface_is_sgmii - Convenience function for testing if a PHY interface
> + * is SGMII (all variants)
> + * @phydev: the phy_device struct
> + */
> +static inline bool phy_interface_is_sgmii(struct phy_device *phydev)
> +{
> +	return phydev->interface >= PHY_INTERFACE_MODE_SGMII &&
> +		phydev->interface <= PHY_INTERFACE_MODE_QSGMII;
> +}
> +
>  /* PHY UIDs for various PHYs that are referenced in external code */
>  #define PHY_UID_CS4340  0x13e51002
>  #define PHY_UID_TN2020	0x00a19410
> 

Reviewed-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

[U-Boot] [uboot] [PATCH 7/7] net: phy: dp83867: Add SGMII helper for configuration

[Dan Murphy]

The code assumed that if the interface is not RGMII configured
then it must be SGMII configured.  This device has the ability
to support most of the MII interfaces.  Therefore add the
helper for SGMII and only configure the device if the interface is
configured for SGMII.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/net/phy/ti.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c
index d503eef..4402c52 100644
--- a/drivers/net/phy/ti.c
+++ b/drivers/net/phy/ti.c
@@ -219,7 +219,7 @@ static int dp83867_config(struct phy_device *phydev)
 			(dp83867->fifo_depth << DP83867_PHYCR_FIFO_DEPTH_SHIFT));
 		if (ret)
 			goto err_out;
-	} else {
+	} else if (phy_interface_is_sgmii(phydev)) {
 		phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR,
 			  (BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000));
 
-- 
2.8.0.rc3

[U-Boot] [uboot] [PATCH 7/7] net: phy: dp83867: Add SGMII helper for configuration

[Mugunthan V N]

On Friday 08 April 2016 09:38 PM, Dan Murphy wrote:
> The code assumed that if the interface is not RGMII configured
> then it must be SGMII configured.  This device has the ability
> to support most of the MII interfaces.  Therefore add the
> helper for SGMII and only configure the device if the interface is
> configured for SGMII.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

[U-Boot] [uboot] [PATCH 7/7] net: phy: dp83867: Add SGMII helper for configuration

[Michal Simek]

On 8.4.2016 18:08, Dan Murphy wrote:
> The code assumed that if the interface is not RGMII configured
> then it must be SGMII configured.  This device has the ability
> to support most of the MII interfaces.  Therefore add the
> helper for SGMII and only configure the device if the interface is
> configured for SGMII.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  drivers/net/phy/ti.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c
> index d503eef..4402c52 100644
> --- a/drivers/net/phy/ti.c
> +++ b/drivers/net/phy/ti.c
> @@ -219,7 +219,7 @@ static int dp83867_config(struct phy_device *phydev)
>  			(dp83867->fifo_depth << DP83867_PHYCR_FIFO_DEPTH_SHIFT));
>  		if (ret)
>  			goto err_out;
> -	} else {
> +	} else if (phy_interface_is_sgmii(phydev)) {
>  		phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR,
>  			  (BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000));
>  
> 

Reviewed-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

[U-Boot] [uboot] [PATCH 1/7] drivers: net: cpsw: Add readinf of DT phy-handle node

[Mugunthan V N]

On Friday 08 April 2016 09:38 PM, Dan Murphy wrote:
> Add the ability to read the phy-handle node of the
> cpsw slave.  Upon reading this handle the phy-id
> can be stored based on the reg node in the DT.
> 
> The phy-handle also needs to be stored and passed
> to the phy to access any phy data that is available.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

[U-Boot] [uboot] [PATCH 1/7] drivers: net: cpsw: Add readinf of DT phy-handle node

[Michal Simek]

On 8.4.2016 18:08, Dan Murphy wrote:
> Add the ability to read the phy-handle node of the
> cpsw slave.  Upon reading this handle the phy-id
> can be stored based on the reg node in the DT.
> 
> The phy-handle also needs to be stored and passed
> to the phy to access any phy data that is available.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  drivers/net/cpsw.c | 18 ++++++++++++++++--
>  include/cpsw.h     |  1 +
>  2 files changed, 17 insertions(+), 2 deletions(-)


Just a generic node. There are couple of new patches in this series
which is great but please also label them as v4 (or new v5).

Mixing version is not good solution and I had to apply all patches twice
because of this.

Please look at patman to handle versions for you.

Thanks,
Michal