[PATCH 0/4] net: hix5hd2_gmac: add tx sg feature and reset/clock control signals

[PATCH 0/4] net: hix5hd2_gmac: add tx sg feature and reset/clock control signals

[Dongpo Li]

The "hix5hd2" is SoC name, add the generic ethernet driver name.
The "hisi-gemac-v1" is the basic version and "hisi-gemac-v2" adds
the SG/TXCSUM/TSO/UFO features.
This patch set only adds the SG(scatter-gather) driver for transmitting,
the drivers of other features will be submitted later.

Add the MAC reset control signals and clock signals.
As a result, the hix5hd2 ethernet clock can be abstracted as
gate clock instead of self defined complex clock.

Dongpo Li (2):
  clk: hix5hd2: change ethernet clock type
  ARM: dts: hix5hd2: add gmac clock and reset property

Li Dongpo (2):
  net: hix5hd2_gmac: add tx scatter-gather feature
  net: hix5hd2_gmac: add reset control and clock signals

 .../bindings/net/hisilicon-hix5hd2-gmac.txt        |  25 +-
 arch/arm/boot/dts/hisi-x5hd2-dkb.dts               |   2 +
 arch/arm/boot/dts/hisi-x5hd2.dtsi                  |  15 +-
 drivers/clk/hisilicon/clk-hix5hd2.c                |  72 +----
 drivers/net/ethernet/hisilicon/hix5hd2_gmac.c      | 353 +++++++++++++++++++--
 include/dt-bindings/clock/hix5hd2-clock.h          |   6 +-
 6 files changed, 381 insertions(+), 92 deletions(-)

-- 
2.8.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] net: hix5hd2_gmac: add tx scatter-gather feature

[Dongpo Li]

From: Li Dongpo <lidongpo-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>

The "hix5hd2" is SoC name, add the generic ethernet driver name.
The "hisi-gemac-v1" is the basic version and "hisi-gemac-v2" adds
the SG/TXCSUM/TSO/UFO features.
This patch only adds the SG(scatter-gather) driver for transmitting,
the drivers of other features will be submitted later.

Signed-off-by: Dongpo Li <lidongpo-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
---
 .../bindings/net/hisilicon-hix5hd2-gmac.txt        |   9 +-
 drivers/net/ethernet/hisilicon/hix5hd2_gmac.c      | 213 +++++++++++++++++++--
 2 files changed, 205 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
index 75d398b..3c02fac 100644
--- a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
+++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
@@ -1,7 +1,12 @@
 Hisilicon hix5hd2 gmac controller
 
 Required properties:
-- compatible: should be "hisilicon,hix5hd2-gmac".
+- compatible: should contain one of the following version strings:
+	* "hisilicon,hisi-gemac-v1"
+	* "hisilicon,hisi-gemac-v2"
+	and one of the following SoC string:
+	* "hisilicon,hix5hd2-gemac"
+	* "hisilicon,hi3798cv200-gemac"
 - reg: specifies base physical address(s) and size of the device registers.
   The first region is the MAC register base and size.
   The second region is external interface control register.
@@ -20,7 +25,7 @@ Required properties:
 
 Example:
 	gmac0: ethernet@f9840000 {
-		compatible = "hisilicon,hix5hd2-gmac";
+		compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
 		reg = <0xf9840000 0x1000>,<0xf984300c 0x4>;
 		interrupts = <0 71 4>;
 		#address-cells = <1>;
diff --git a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
index 275618b..679a5e5 100644
--- a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
+++ b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
@@ -11,6 +11,7 @@
 #include <linux/interrupt.h>
 #include <linux/etherdevice.h>
 #include <linux/platform_device.h>
+#include <linux/of_device.h>
 #include <linux/of_net.h>
 #include <linux/of_mdio.h>
 #include <linux/clk.h>
@@ -183,12 +184,19 @@
 #define DESC_DATA_LEN_OFF		16
 #define DESC_BUFF_LEN_OFF		0
 #define DESC_DATA_MASK			0x7ff
+#define DESC_SG				BIT(30)
+#define DESC_FRAGS_NUM_OFF		11
 
 /* DMA descriptor ring helpers */
 #define dma_ring_incr(n, s)		(((n) + 1) & ((s) - 1))
 #define dma_cnt(n)			((n) >> 5)
 #define dma_byte(n)			((n) << 5)
 
+#define HW_CAP_TSO			BIT(0)
+#define GEMAC_V1			0
+#define GEMAC_V2			(GEMAC_V1 | HW_CAP_TSO)
+#define HAS_CAP_TSO(hw_cap)		((hw_cap) & HW_CAP_TSO)
+
 struct hix5hd2_desc {
 	__le32 buff_addr;
 	__le32 cmd;
@@ -201,6 +209,27 @@ struct hix5hd2_desc_sw {
 	unsigned int	size;
 };
 
+struct hix5hd2_sg_desc_ring {
+	struct sg_desc *desc;
+	dma_addr_t phys_addr;
+};
+
+struct frags_info {
+	__le32 addr;
+	__le32 size;
+};
+
+/* hardware supported max skb frags num */
+#define SG_MAX_SKB_FRAGS	17
+struct sg_desc {
+	__le32 total_len;
+	__le32 resvd0;
+	__le32 linear_addr;
+	__le32 linear_len;
+	/* reserve one more frags for memory alignment */
+	struct frags_info frags[SG_MAX_SKB_FRAGS + 1];
+};
+
 #define QUEUE_NUMS	4
 struct hix5hd2_priv {
 	struct hix5hd2_desc_sw pool[QUEUE_NUMS];
@@ -208,6 +237,7 @@ struct hix5hd2_priv {
 #define rx_bq		pool[1]
 #define tx_bq		pool[2]
 #define tx_rq		pool[3]
+	struct hix5hd2_sg_desc_ring tx_ring;
 
 	void __iomem *base;
 	void __iomem *ctrl_base;
@@ -221,6 +251,7 @@ struct hix5hd2_priv {
 	struct device_node *phy_node;
 	phy_interface_t	phy_mode;
 
+	unsigned long hw_cap;
 	unsigned int speed;
 	unsigned int duplex;
 
@@ -511,6 +542,27 @@ next:
 	return num;
 }
 
+static void hix5hd2_clean_sg_desc(struct hix5hd2_priv *priv,
+				  struct sk_buff *skb, u32 pos)
+{
+	struct sg_desc *desc;
+	dma_addr_t addr;
+	u32 len;
+	int i;
+
+	desc = priv->tx_ring.desc + pos;
+
+	addr = le32_to_cpu(desc->linear_addr);
+	len = le32_to_cpu(desc->linear_len);
+	dma_unmap_single(priv->dev, addr, len, DMA_TO_DEVICE);
+
+	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+		addr = le32_to_cpu(desc->frags[i].addr);
+		len = le32_to_cpu(desc->frags[i].size);
+		dma_unmap_page(priv->dev, addr, len, DMA_TO_DEVICE);
+	}
+}
+
 static void hix5hd2_xmit_reclaim(struct net_device *dev)
 {
 	struct sk_buff *skb;
@@ -538,8 +590,15 @@ static void hix5hd2_xmit_reclaim(struct net_device *dev)
 		pkts_compl++;
 		bytes_compl += skb->len;
 		desc = priv->tx_rq.desc + pos;
-		addr = le32_to_cpu(desc->buff_addr);
-		dma_unmap_single(priv->dev, addr, skb->len, DMA_TO_DEVICE);
+
+		if (skb_shinfo(skb)->nr_frags) {
+			hix5hd2_clean_sg_desc(priv, skb, pos);
+		} else {
+			addr = le32_to_cpu(desc->buff_addr);
+			dma_unmap_single(priv->dev, addr, skb->len,
+					 DMA_TO_DEVICE);
+		}
+
 		priv->tx_skb[pos] = NULL;
 		dev_consume_skb_any(skb);
 		pos = dma_ring_incr(pos, TX_DESC_NUM);
@@ -600,12 +659,66 @@ static irqreturn_t hix5hd2_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static u32 hix5hd2_get_desc_cmd(struct sk_buff *skb, unsigned long hw_cap)
+{
+	u32 cmd = 0;
+
+	if (HAS_CAP_TSO(hw_cap)) {
+		if (skb_shinfo(skb)->nr_frags)
+			cmd |= DESC_SG;
+		cmd |= skb_shinfo(skb)->nr_frags << DESC_FRAGS_NUM_OFF;
+	} else {
+		cmd |= DESC_FL_FULL |
+			((skb->len & DESC_DATA_MASK) << DESC_BUFF_LEN_OFF);
+	}
+
+	cmd |= (skb->len & DESC_DATA_MASK) << DESC_DATA_LEN_OFF;
+	cmd |= DESC_VLD_BUSY;
+
+	return cmd;
+}
+
+static int hix5hd2_fill_sg_desc(struct hix5hd2_priv *priv,
+				struct sk_buff *skb, u32 pos)
+{
+	struct sg_desc *desc;
+	dma_addr_t addr;
+	int ret;
+	int i;
+
+	desc = priv->tx_ring.desc + pos;
+
+	desc->total_len = cpu_to_le32(skb->len);
+	addr = dma_map_single(priv->dev, skb->data, skb_headlen(skb),
+			      DMA_TO_DEVICE);
+	if (unlikely(dma_mapping_error(priv->dev, addr)))
+		return -EINVAL;
+	desc->linear_addr = cpu_to_le32(addr);
+	desc->linear_len = cpu_to_le32(skb_headlen(skb));
+
+	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+		skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+		int len = frag->size;
+
+		addr = skb_frag_dma_map(priv->dev, frag, 0, len, DMA_TO_DEVICE);
+		ret = dma_mapping_error(priv->dev, addr);
+		if (unlikely(ret))
+			return -EINVAL;
+		desc->frags[i].addr = cpu_to_le32(addr);
+		desc->frags[i].size = cpu_to_le32(len);
+	}
+
+	return 0;
+}
+
 static int hix5hd2_net_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct hix5hd2_priv *priv = netdev_priv(dev);
 	struct hix5hd2_desc *desc;
 	dma_addr_t addr;
 	u32 pos;
+	u32 cmd;
+	int ret;
 
 	/* software write pointer */
 	pos = dma_cnt(readl_relaxed(priv->base + TX_BQ_WR_ADDR));
@@ -616,18 +729,31 @@ static int hix5hd2_net_xmit(struct sk_buff *skb, struct net_device *dev)
 		return NETDEV_TX_BUSY;
 	}
 
-	addr = dma_map_single(priv->dev, skb->data, skb->len, DMA_TO_DEVICE);
-	if (dma_mapping_error(priv->dev, addr)) {
-		dev_kfree_skb_any(skb);
-		return NETDEV_TX_OK;
-	}
-
 	desc = priv->tx_bq.desc + pos;
+
+	cmd = hix5hd2_get_desc_cmd(skb, priv->hw_cap);
+	desc->cmd = cpu_to_le32(cmd);
+
+	if (skb_shinfo(skb)->nr_frags) {
+		ret = hix5hd2_fill_sg_desc(priv, skb, pos);
+		if (unlikely(ret)) {
+			dev_kfree_skb_any(skb);
+			dev->stats.tx_dropped++;
+			return NETDEV_TX_OK;
+		}
+		addr = priv->tx_ring.phys_addr + pos * sizeof(struct sg_desc);
+	} else {
+		addr = dma_map_single(priv->dev, skb->data, skb->len,
+				      DMA_TO_DEVICE);
+		if (unlikely(dma_mapping_error(priv->dev, addr))) {
+			dev_kfree_skb_any(skb);
+			dev->stats.tx_dropped++;
+			return NETDEV_TX_OK;
+		}
+	}
 	desc->buff_addr = cpu_to_le32(addr);
+
 	priv->tx_skb[pos] = skb;
-	desc->cmd = cpu_to_le32(DESC_VLD_BUSY | DESC_FL_FULL |
-				(skb->len & DESC_DATA_MASK) << DESC_DATA_LEN_OFF |
-				(skb->len & DESC_DATA_MASK) << DESC_BUFF_LEN_OFF);
 
 	/* ensure desc updated */
 	wmb();
@@ -862,10 +988,40 @@ error_free_pool:
 	return -ENOMEM;
 }
 
+static int hix5hd2_init_sg_desc_queue(struct hix5hd2_priv *priv)
+{
+	struct sg_desc *desc;
+	dma_addr_t phys_addr;
+
+	desc = (struct sg_desc *)dma_alloc_coherent(priv->dev,
+				TX_DESC_NUM * sizeof(struct sg_desc),
+				&phys_addr, GFP_KERNEL);
+	if (!desc)
+		return -ENOMEM;
+
+	priv->tx_ring.desc = desc;
+	priv->tx_ring.phys_addr = phys_addr;
+
+	return 0;
+}
+
+static void hix5hd2_destroy_sg_desc_queue(struct hix5hd2_priv *priv)
+{
+	if (priv->tx_ring.desc) {
+		dma_free_coherent(priv->dev,
+				  TX_DESC_NUM * sizeof(struct sg_desc),
+				  priv->tx_ring.desc, priv->tx_ring.phys_addr);
+		priv->tx_ring.desc = NULL;
+	}
+}
+
+static const struct of_device_id hix5hd2_of_match[];
+
 static int hix5hd2_dev_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct device_node *node = dev->of_node;
+	const struct of_device_id *of_id = NULL;
 	struct net_device *ndev;
 	struct hix5hd2_priv *priv;
 	struct resource *res;
@@ -883,6 +1039,13 @@ static int hix5hd2_dev_probe(struct platform_device *pdev)
 	priv->dev = dev;
 	priv->netdev = ndev;
 
+	of_id = of_match_device(hix5hd2_of_match, dev);
+	if (!of_id) {
+		ret = -EINVAL;
+		goto out_free_netdev;
+	}
+	priv->hw_cap = (unsigned long)of_id->data;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	priv->base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(priv->base)) {
@@ -972,11 +1135,24 @@ static int hix5hd2_dev_probe(struct platform_device *pdev)
 	ndev->ethtool_ops = &hix5hd2_ethtools_ops;
 	SET_NETDEV_DEV(ndev, dev);
 
+	if (HAS_CAP_TSO(priv->hw_cap))
+		ndev->hw_features |= NETIF_F_SG;
+
+	ndev->features |= ndev->hw_features | NETIF_F_HIGHDMA;
+	ndev->vlan_features |= ndev->features;
+
 	ret = hix5hd2_init_hw_desc_queue(priv);
 	if (ret)
 		goto out_phy_node;
 
 	netif_napi_add(ndev, &priv->napi, hix5hd2_poll, NAPI_POLL_WEIGHT);
+
+	if (HAS_CAP_TSO(priv->hw_cap)) {
+		ret = hix5hd2_init_sg_desc_queue(priv);
+		if (ret)
+			goto out_destroy_queue;
+	}
+
 	ret = register_netdev(priv->netdev);
 	if (ret) {
 		netdev_err(ndev, "register_netdev failed!");
@@ -988,6 +1164,8 @@ static int hix5hd2_dev_probe(struct platform_device *pdev)
 	return ret;
 
 out_destroy_queue:
+	if (HAS_CAP_TSO(priv->hw_cap))
+		hix5hd2_destroy_sg_desc_queue(priv);
 	netif_napi_del(&priv->napi);
 	hix5hd2_destroy_hw_desc_queue(priv);
 out_phy_node:
@@ -1012,6 +1190,8 @@ static int hix5hd2_dev_remove(struct platform_device *pdev)
 	mdiobus_unregister(priv->bus);
 	mdiobus_free(priv->bus);
 
+	if (HAS_CAP_TSO(priv->hw_cap))
+		hix5hd2_destroy_sg_desc_queue(priv);
 	hix5hd2_destroy_hw_desc_queue(priv);
 	of_node_put(priv->phy_node);
 	cancel_work_sync(&priv->tx_timeout_task);
@@ -1021,7 +1201,10 @@ static int hix5hd2_dev_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id hix5hd2_of_match[] = {
-	{.compatible = "hisilicon,hix5hd2-gmac",},
+	{ .compatible = "hisilicon,hisi-gemac-v1", .data = (void *)GEMAC_V1 },
+	{ .compatible = "hisilicon,hisi-gemac-v2", .data = (void *)GEMAC_V2 },
+	{ .compatible = "hisilicon,hix5hd2-gemac", .data = (void *)GEMAC_V1 },
+	{ .compatible = "hisilicon,hi3798cv200-gemac", .data = (void *)GEMAC_V2 },
 	{},
 };
 
@@ -1029,7 +1212,7 @@ MODULE_DEVICE_TABLE(of, hix5hd2_of_match);
 
 static struct platform_driver hix5hd2_dev_driver = {
 	.driver = {
-		.name = "hix5hd2-gmac",
+		.name = "hisi-gemac",
 		.of_match_table = hix5hd2_of_match,
 	},
 	.probe = hix5hd2_dev_probe,
@@ -1038,6 +1221,6 @@ static struct platform_driver hix5hd2_dev_driver = {
 
 module_platform_driver(hix5hd2_dev_driver);
 
-MODULE_DESCRIPTION("HISILICON HIX5HD2 Ethernet driver");
+MODULE_DESCRIPTION("HISILICON Gigabit Ethernet MAC driver");
 MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:hix5hd2-gmac");
+MODULE_ALIAS("platform:hisi-gemac");
-- 
2.8.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] net: hix5hd2_gmac: add tx scatter-gather feature

[Rob Herring]

On Thu, Aug 11, 2016 at 05:01:52PM +0800, Dongpo Li wrote:
> From: Li Dongpo <lidongpo@hisilicon.com>
> 
> The "hix5hd2" is SoC name, add the generic ethernet driver name.
> The "hisi-gemac-v1" is the basic version and "hisi-gemac-v2" adds
> the SG/TXCSUM/TSO/UFO features.
> This patch only adds the SG(scatter-gather) driver for transmitting,
> the drivers of other features will be submitted later.

The compatible string changes should probably be a separate patch.

> Signed-off-by: Dongpo Li <lidongpo@hisilicon.com>
> ---
>  .../bindings/net/hisilicon-hix5hd2-gmac.txt        |   9 +-
>  drivers/net/ethernet/hisilicon/hix5hd2_gmac.c      | 213 +++++++++++++++++++--
>  2 files changed, 205 insertions(+), 17 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
> index 75d398b..3c02fac 100644
> --- a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
> +++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
> @@ -1,7 +1,12 @@
>  Hisilicon hix5hd2 gmac controller
>  
>  Required properties:
> -- compatible: should be "hisilicon,hix5hd2-gmac".
> +- compatible: should contain one of the following version strings:
> +	* "hisilicon,hisi-gemac-v1"
> +	* "hisilicon,hisi-gemac-v2"
> +	and one of the following SoC string:
> +	* "hisilicon,hix5hd2-gemac"
> +	* "hisilicon,hi3798cv200-gemac"

Make it clear what the order should be.

2 SOC versions so far and 2 generic versions. I'm not really convinced 
that the generic string is needed.

>  - reg: specifies base physical address(s) and size of the device registers.
>    The first region is the MAC register base and size.
>    The second region is external interface control register.
> @@ -20,7 +25,7 @@ Required properties:
>  
>  Example:
>  	gmac0: ethernet@f9840000 {
> -		compatible = "hisilicon,hix5hd2-gmac";
> +		compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
>  		reg = <0xf9840000 0x1000>,<0xf984300c 0x4>;
>  		interrupts = <0 71 4>;
>  		#address-cells = <1>;

Re: [PATCH 1/4] net: hix5hd2_gmac: add tx scatter-gather feature

[Dongpo Li]

Hi Rob,
Many thanks for your review.

On 2016/8/13 2:43, Rob Herring wrote:
> On Thu, Aug 11, 2016 at 05:01:52PM +0800, Dongpo Li wrote:
>> From: Li Dongpo <lidongpo@hisilicon.com>
>>
>> The "hix5hd2" is SoC name, add the generic ethernet driver name.
>> The "hisi-gemac-v1" is the basic version and "hisi-gemac-v2" adds
>> the SG/TXCSUM/TSO/UFO features.
>> This patch only adds the SG(scatter-gather) driver for transmitting,
>> the drivers of other features will be submitted later.
> 
> The compatible string changes should probably be a separate patch.
> 
ok, I will split this patch into two patches, one for compatible string changes,
and one for driver feature implementation.

>> Signed-off-by: Dongpo Li <lidongpo@hisilicon.com>
>> ---
>>  .../bindings/net/hisilicon-hix5hd2-gmac.txt        |   9 +-
>>  drivers/net/ethernet/hisilicon/hix5hd2_gmac.c      | 213 +++++++++++++++++++--
>>  2 files changed, 205 insertions(+), 17 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
>> index 75d398b..3c02fac 100644
>> --- a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
>> +++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
>> @@ -1,7 +1,12 @@
>>  Hisilicon hix5hd2 gmac controller
>>  
>>  Required properties:
>> -- compatible: should be "hisilicon,hix5hd2-gmac".
>> +- compatible: should contain one of the following version strings:
>> +	* "hisilicon,hisi-gemac-v1"
>> +	* "hisilicon,hisi-gemac-v2"
>> +	and one of the following SoC string:
>> +	* "hisilicon,hix5hd2-gemac"
>> +	* "hisilicon,hi3798cv200-gemac"
> 
> Make it clear what the order should be.
> 
ok, I will put the SoC strings in alphabetical order.

> 2 SOC versions so far and 2 generic versions. I'm not really convinced 
> that the generic string is needed.
> 
Actually, there are more SoC versions not listed here. We aim to support hi3798cv200
this time, but may support more SoCs in future.

>>  - reg: specifies base physical address(s) and size of the device registers.
>>    The first region is the MAC register base and size.
>>    The second region is external interface control register.
>> @@ -20,7 +25,7 @@ Required properties:
>>  
>>  Example:
>>  	gmac0: ethernet@f9840000 {
>> -		compatible = "hisilicon,hix5hd2-gmac";
>> +		compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
>>  		reg = <0xf9840000 0x1000>,<0xf984300c 0x4>;
>>  		interrupts = <0 71 4>;
>>  		#address-cells = <1>;
> 
> .
> 


    Regards,
    Dongpo

.

Re: [PATCH 1/4] net: hix5hd2_gmac: add tx scatter-gather feature

[Rob Herring]

On Mon, Aug 15, 2016 at 1:50 AM, Dongpo Li <lidongpo@hisilicon.com> wrote:
> Hi Rob,
> Many thanks for your review.
>
> On 2016/8/13 2:43, Rob Herring wrote:
>> On Thu, Aug 11, 2016 at 05:01:52PM +0800, Dongpo Li wrote:
>>> From: Li Dongpo <lidongpo@hisilicon.com>
>>>
>>> The "hix5hd2" is SoC name, add the generic ethernet driver name.
>>> The "hisi-gemac-v1" is the basic version and "hisi-gemac-v2" adds
>>> the SG/TXCSUM/TSO/UFO features.
>>> This patch only adds the SG(scatter-gather) driver for transmitting,
>>> the drivers of other features will be submitted later.
>>
>> The compatible string changes should probably be a separate patch.
>>
> ok, I will split this patch into two patches, one for compatible string changes,
> and one for driver feature implementation.
>
>>> Signed-off-by: Dongpo Li <lidongpo@hisilicon.com>
>>> ---
>>>  .../bindings/net/hisilicon-hix5hd2-gmac.txt        |   9 +-
>>>  drivers/net/ethernet/hisilicon/hix5hd2_gmac.c      | 213 +++++++++++++++++++--
>>>  2 files changed, 205 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
>>> index 75d398b..3c02fac 100644
>>> --- a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
>>> +++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
>>> @@ -1,7 +1,12 @@
>>>  Hisilicon hix5hd2 gmac controller
>>>
>>>  Required properties:
>>> -- compatible: should be "hisilicon,hix5hd2-gmac".
>>> +- compatible: should contain one of the following version strings:
>>> +    * "hisilicon,hisi-gemac-v1"
>>> +    * "hisilicon,hisi-gemac-v2"
>>> +    and one of the following SoC string:
>>> +    * "hisilicon,hix5hd2-gemac"
>>> +    * "hisilicon,hi3798cv200-gemac"
>>
>> Make it clear what the order should be.
>>
> ok, I will put the SoC strings in alphabetical order.

No, I mean the most specific string comes first.

Rob

Re: [PATCH 1/4] net: hix5hd2_gmac: add tx scatter-gather feature

[Dongpo Li]

On 2016/8/16 0:18, Rob Herring wrote:
> On Mon, Aug 15, 2016 at 1:50 AM, Dongpo Li <lidongpo-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org> wrote:
>> Hi Rob,
>> Many thanks for your review.
>>
>> On 2016/8/13 2:43, Rob Herring wrote:
>>> On Thu, Aug 11, 2016 at 05:01:52PM +0800, Dongpo Li wrote:
>>>> From: Li Dongpo <lidongpo-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
>>>>
>>>> The "hix5hd2" is SoC name, add the generic ethernet driver name.
>>>> The "hisi-gemac-v1" is the basic version and "hisi-gemac-v2" adds
>>>> the SG/TXCSUM/TSO/UFO features.
>>>> This patch only adds the SG(scatter-gather) driver for transmitting,
>>>> the drivers of other features will be submitted later.
>>>
>>> The compatible string changes should probably be a separate patch.
>>>
>> ok, I will split this patch into two patches, one for compatible string changes,
>> and one for driver feature implementation.
>>
>>>> Signed-off-by: Dongpo Li <lidongpo-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
>>>> ---
>>>>  .../bindings/net/hisilicon-hix5hd2-gmac.txt        |   9 +-
>>>>  drivers/net/ethernet/hisilicon/hix5hd2_gmac.c      | 213 +++++++++++++++++++--
>>>>  2 files changed, 205 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
>>>> index 75d398b..3c02fac 100644
>>>> --- a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
>>>> +++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
>>>> @@ -1,7 +1,12 @@
>>>>  Hisilicon hix5hd2 gmac controller
>>>>
>>>>  Required properties:
>>>> -- compatible: should be "hisilicon,hix5hd2-gmac".
>>>> +- compatible: should contain one of the following version strings:
>>>> +    * "hisilicon,hisi-gemac-v1"
>>>> +    * "hisilicon,hisi-gemac-v2"
>>>> +    and one of the following SoC string:
>>>> +    * "hisilicon,hix5hd2-gemac"
>>>> +    * "hisilicon,hi3798cv200-gemac"
>>>
>>> Make it clear what the order should be.
>>>
>> ok, I will put the SoC strings in alphabetical order.
> 
> No, I mean the most specific string comes first.
> 
ok, I will fix it in next patch version. Thank you.

> Rob
> 
> .
> 


    Regards,
    Dongpo

.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] net: hix5hd2_gmac: add reset control and clock signals

[Dongpo Li]

From: Li Dongpo <lidongpo@hisilicon.com>

Add three reset control signals, "mac_core_rst", "mac_ifc_rst" and
"phy_rst".
The following diagram explained how the reset signals work.

                        SoC
|-----------------------------------------------------
|                               ------                |
|                               | cpu |               |
|                               ------                |
|                                  |                  |
|                              ------------ AMBA bus  |
|                         GMAC     |                  |
|                            ----------------------   |
| ------------- mac_core_rst | --------------      |  |
| |clock and   |-------------->|   mac core  |     |  |
| |reset       |             | --------------      |  |
| |generator   |----         |       |             |  |
| -------------     |        | ----------------    |  |
|          |        ---------->| mac interface |   |  |
|          |     mac_ifc_rst | ----------------    |  |
|          |                 |       |             |  |
|          |                 | ------------------  |  |
|          |phy_rst          | | RGMII interface | |  |
|          |                 | ------------------  |  |
|          |                 ----------------------   |
|----------|------------------------------------------|
           |                          |
           |                      ----------
           |--------------------- |PHY chip |
                                  ----------

The "mac_core_rst" represents "mac core reset signal", it resets
the mac core including packet processing unit, descriptor processing unit,
tx engine, rx engine, control unit.
The "mac_ifc_rst" represents "mac interface reset signal", it resets
the mac interface. The mac interface unit connects mac core and
data interface like MII/RMII/RGMII. After we set a new value of
interface mode, we must reset mac interface to reload the new mode value.
The "phy_rst" represents "phy reset signal", it does a hardware reset
on the PHY chip. This reset signal is optinal if the PHY can work well
without the hardware reset.

Add one more clock signal, the existing is MAC core clock,
and the new one is MAC interface clock.

Signed-off-by: Dongpo Li <lidongpo@hisilicon.com>
---
 .../bindings/net/hisilicon-hix5hd2-gmac.txt        |  16 ++-
 drivers/net/ethernet/hisilicon/hix5hd2_gmac.c      | 140 +++++++++++++++++++--
 2 files changed, 143 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
index 3c02fac..a0bf2ca 100644
--- a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
+++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
@@ -17,6 +17,16 @@ Required properties:
 - phy-handle: see ethernet.txt [1].
 - mac-address: see ethernet.txt [1].
 - clocks: clock phandle and specifier pair.
+- clock-names: contain the clock name "mac_core" and "mac_ifc".
+- resets: should contain the phandle to the MAC core reset signal(required),
+	the MAC interface reset signal(required)
+	and the PHY reset signal(optional).
+- reset-names: contain the reset signal name "mac_core"(required),
+	"mac_ifc"(required) and "phy"(optional).
+- hisilicon,phy-reset-delays-us: triplet of delays if PHY reset signal given.
+	The 1st cell is reset pre-delay in micro seconds.
+	The 2nd cell is reset pulse in micro seconds.
+	The 3rd cell is reset post-delay in micro seconds.
 
 - PHY subnode: inherits from phy binding [2]
 
@@ -33,7 +43,11 @@ Example:
 		phy-mode = "mii";
 		phy-handle = <&phy2>;
 		mac-address = [00 00 00 00 00 00];
-		clocks = <&clock HIX5HD2_MAC0_CLK>;
+		clocks = <&clock HIX5HD2_MAC0_CLK>, <&clock HIX5HD2_MAC_IFC0_CLK>;
+		clock-names = "mac_core", "mac_ifc";
+		resets = <&clock 0xcc 8>, <&clock 0xcc 10>, <&clock 0x120 4>;
+		reset-names = "mac_core", "mac_ifc", "phy";
+		hisilicon,phy-reset-delays-us = <10000 10000 30000>;
 
 		phy2: ethernet-phy@2 {
 			reg = <2>;
diff --git a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
index 679a5e5..11e70bb 100644
--- a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
+++ b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
@@ -14,6 +14,7 @@
 #include <linux/of_device.h>
 #include <linux/of_net.h>
 #include <linux/of_mdio.h>
+#include <linux/reset.h>
 #include <linux/clk.h>
 #include <linux/circ_buf.h>
 
@@ -197,6 +198,15 @@
 #define GEMAC_V2			(GEMAC_V1 | HW_CAP_TSO)
 #define HAS_CAP_TSO(hw_cap)		((hw_cap) & HW_CAP_TSO)
 
+#define PHY_RESET_DELAYS_PROPERTY	"hisilicon,phy-reset-delays-us"
+
+enum phy_reset_delays {
+	PRE_DELAY,
+	PULSE,
+	POST_DELAY,
+	DELAYS_NUM,
+};
+
 struct hix5hd2_desc {
 	__le32 buff_addr;
 	__le32 cmd;
@@ -255,12 +265,23 @@ struct hix5hd2_priv {
 	unsigned int speed;
 	unsigned int duplex;
 
-	struct clk *clk;
+	struct clk *mac_core_clk;
+	struct clk *mac_ifc_clk;
+	struct reset_control *mac_core_rst;
+	struct reset_control *mac_ifc_rst;
+	struct reset_control *phy_rst;
+	u32 phy_reset_delays[DELAYS_NUM];
 	struct mii_bus *bus;
 	struct napi_struct napi;
 	struct work_struct tx_timeout_task;
 };
 
+static inline void hix5hd2_mac_interface_reset(struct hix5hd2_priv *priv)
+{
+	reset_control_assert(priv->mac_ifc_rst);
+	reset_control_deassert(priv->mac_ifc_rst);
+}
+
 static void hix5hd2_config_port(struct net_device *dev, u32 speed, u32 duplex)
 {
 	struct hix5hd2_priv *priv = netdev_priv(dev);
@@ -293,6 +314,7 @@ static void hix5hd2_config_port(struct net_device *dev, u32 speed, u32 duplex)
 	if (duplex)
 		val |= GMAC_FULL_DUPLEX;
 	writel_relaxed(val, priv->ctrl_base);
+	hix5hd2_mac_interface_reset(priv);
 
 	writel_relaxed(BIT_MODE_CHANGE_EN, priv->base + MODE_CHANGE_EN);
 	if (speed == SPEED_1000)
@@ -807,16 +829,26 @@ static int hix5hd2_net_open(struct net_device *dev)
 	struct phy_device *phy;
 	int ret;
 
-	ret = clk_prepare_enable(priv->clk);
+	ret = clk_prepare_enable(priv->mac_core_clk);
+	if (ret < 0) {
+		netdev_err(dev, "failed to enable mac core clk %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(priv->mac_ifc_clk);
 	if (ret < 0) {
-		netdev_err(dev, "failed to enable clk %d\n", ret);
+		clk_disable_unprepare(priv->mac_core_clk);
+		netdev_err(dev, "failed to enable mac ifc clk %d\n", ret);
 		return ret;
 	}
 
 	phy = of_phy_connect(dev, priv->phy_node,
 			     &hix5hd2_adjust_link, 0, priv->phy_mode);
-	if (!phy)
+	if (!phy) {
+		clk_disable_unprepare(priv->mac_ifc_clk);
+		clk_disable_unprepare(priv->mac_core_clk);
 		return -ENODEV;
+	}
 
 	phy_start(phy);
 	hix5hd2_hw_init(priv);
@@ -847,7 +879,8 @@ static int hix5hd2_net_close(struct net_device *dev)
 		phy_disconnect(dev->phydev);
 	}
 
-	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->mac_ifc_clk);
+	clk_disable_unprepare(priv->mac_core_clk);
 
 	return 0;
 }
@@ -1015,6 +1048,45 @@ static void hix5hd2_destroy_sg_desc_queue(struct hix5hd2_priv *priv)
 	}
 }
 
+static inline void hix5hd2_mac_core_reset(struct hix5hd2_priv *priv)
+{
+	reset_control_assert(priv->mac_core_rst);
+	reset_control_deassert(priv->mac_core_rst);
+}
+
+static void hix5hd2_sleep_us(u32 time_us)
+{
+	u32 time_ms;
+
+	if (!time_us)
+		return;
+
+	time_ms = DIV_ROUND_UP(time_us, 1000);
+	if (time_ms < 20)
+		usleep_range(time_us, time_us + 500);
+	else
+		msleep(time_ms);
+}
+
+static void hix5hd2_phy_reset(struct hix5hd2_priv *priv)
+{
+	/* To make sure PHY hardware reset success,
+	 * we must keep PHY in deassert state first and
+	 * then complete the hardware reset operation
+	 */
+	reset_control_deassert(priv->phy_rst);
+	hix5hd2_sleep_us(priv->phy_reset_delays[PRE_DELAY]);
+
+	reset_control_assert(priv->phy_rst);
+	/* delay some time to ensure reset ok,
+	 * this depends on PHY hardware feature
+	 */
+	hix5hd2_sleep_us(priv->phy_reset_delays[PULSE]);
+	reset_control_deassert(priv->phy_rst);
+	/* delay some time to ensure later MDIO access */
+	hix5hd2_sleep_us(priv->phy_reset_delays[POST_DELAY]);
+}
+
 static const struct of_device_id hix5hd2_of_match[];
 
 static int hix5hd2_dev_probe(struct platform_device *pdev)
@@ -1060,23 +1132,62 @@ static int hix5hd2_dev_probe(struct platform_device *pdev)
 		goto out_free_netdev;
 	}
 
-	priv->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(priv->clk)) {
-		netdev_err(ndev, "failed to get clk\n");
+	priv->mac_core_clk = devm_clk_get(&pdev->dev, "mac_core");
+	if (IS_ERR(priv->mac_core_clk)) {
+		netdev_err(ndev, "failed to get mac core clk\n");
 		ret = -ENODEV;
 		goto out_free_netdev;
 	}
 
-	ret = clk_prepare_enable(priv->clk);
+	ret = clk_prepare_enable(priv->mac_core_clk);
 	if (ret < 0) {
-		netdev_err(ndev, "failed to enable clk %d\n", ret);
+		netdev_err(ndev, "failed to enable mac core clk %d\n", ret);
 		goto out_free_netdev;
 	}
 
+	priv->mac_ifc_clk = devm_clk_get(&pdev->dev, "mac_ifc");
+	if (IS_ERR(priv->mac_ifc_clk)) {
+		netdev_err(ndev, "failed to get mac ifc clk\n");
+		ret = -ENODEV;
+		goto out_disable_mac_core_clk;
+	}
+
+	ret = clk_prepare_enable(priv->mac_ifc_clk);
+	if (ret < 0) {
+		netdev_err(ndev, "failed to enable mac ifc clk %d\n", ret);
+		goto out_disable_mac_core_clk;
+	}
+
+	priv->mac_core_rst = devm_reset_control_get(dev, "mac_core");
+	if (IS_ERR(priv->mac_core_rst)) {
+		ret = PTR_ERR(priv->mac_core_rst);
+		goto out_disable_clk;
+	}
+	hix5hd2_mac_core_reset(priv);
+
+	priv->mac_ifc_rst = devm_reset_control_get(dev, "mac_ifc");
+	if (IS_ERR(priv->mac_ifc_rst)) {
+		ret = PTR_ERR(priv->mac_ifc_rst);
+		goto out_disable_clk;
+	}
+
+	priv->phy_rst = devm_reset_control_get(dev, "phy");
+	if (IS_ERR(priv->phy_rst)) {
+		priv->phy_rst = NULL;
+	} else {
+		ret = of_property_read_u32_array(node,
+						 PHY_RESET_DELAYS_PROPERTY,
+						 priv->phy_reset_delays,
+						 DELAYS_NUM);
+		if (ret)
+			goto out_disable_clk;
+		hix5hd2_phy_reset(priv);
+	}
+
 	bus = mdiobus_alloc();
 	if (bus == NULL) {
 		ret = -ENOMEM;
-		goto out_free_netdev;
+		goto out_disable_clk;
 	}
 
 	bus->priv = priv;
@@ -1159,7 +1270,8 @@ static int hix5hd2_dev_probe(struct platform_device *pdev)
 		goto out_destroy_queue;
 	}
 
-	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->mac_ifc_clk);
+	clk_disable_unprepare(priv->mac_core_clk);
 
 	return ret;
 
@@ -1174,6 +1286,10 @@ err_mdiobus:
 	mdiobus_unregister(bus);
 err_free_mdio:
 	mdiobus_free(bus);
+out_disable_clk:
+	clk_disable_unprepare(priv->mac_ifc_clk);
+out_disable_mac_core_clk:
+	clk_disable_unprepare(priv->mac_core_clk);
 out_free_netdev:
 	free_netdev(ndev);
 
-- 
2.8.2

Re: [PATCH 2/4] net: hix5hd2_gmac: add reset control and clock signals

[Rob Herring]

On Thu, Aug 11, 2016 at 05:01:53PM +0800, Dongpo Li wrote:
> From: Li Dongpo <lidongpo-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> 
> Add three reset control signals, "mac_core_rst", "mac_ifc_rst" and
> "phy_rst".
> The following diagram explained how the reset signals work.
> 
>                         SoC
> |-----------------------------------------------------
> |                               ------                |
> |                               | cpu |               |
> |                               ------                |
> |                                  |                  |
> |                              ------------ AMBA bus  |
> |                         GMAC     |                  |
> |                            ----------------------   |
> | ------------- mac_core_rst | --------------      |  |
> | |clock and   |-------------->|   mac core  |     |  |
> | |reset       |             | --------------      |  |
> | |generator   |----         |       |             |  |
> | -------------     |        | ----------------    |  |
> |          |        ---------->| mac interface |   |  |
> |          |     mac_ifc_rst | ----------------    |  |
> |          |                 |       |             |  |
> |          |                 | ------------------  |  |
> |          |phy_rst          | | RGMII interface | |  |
> |          |                 | ------------------  |  |
> |          |                 ----------------------   |
> |----------|------------------------------------------|
>            |                          |
>            |                      ----------
>            |--------------------- |PHY chip |
>                                   ----------
> 
> The "mac_core_rst" represents "mac core reset signal", it resets
> the mac core including packet processing unit, descriptor processing unit,
> tx engine, rx engine, control unit.
> The "mac_ifc_rst" represents "mac interface reset signal", it resets
> the mac interface. The mac interface unit connects mac core and
> data interface like MII/RMII/RGMII. After we set a new value of
> interface mode, we must reset mac interface to reload the new mode value.
> The "phy_rst" represents "phy reset signal", it does a hardware reset
> on the PHY chip. This reset signal is optinal if the PHY can work well
> without the hardware reset.
> 
> Add one more clock signal, the existing is MAC core clock,
> and the new one is MAC interface clock.
> 
> Signed-off-by: Dongpo Li <lidongpo-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> ---
>  .../bindings/net/hisilicon-hix5hd2-gmac.txt        |  16 ++-
>  drivers/net/ethernet/hisilicon/hix5hd2_gmac.c      | 140 +++++++++++++++++++--
>  2 files changed, 143 insertions(+), 13 deletions(-)


> 
> @@ -807,16 +829,26 @@ static int hix5hd2_net_open(struct net_device *dev)
>  	struct phy_device *phy;
>  	int ret;
>  
> -	ret = clk_prepare_enable(priv->clk);
> +	ret = clk_prepare_enable(priv->mac_core_clk);
> +	if (ret < 0) {
> +		netdev_err(dev, "failed to enable mac core clk %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(priv->mac_ifc_clk);
>  	if (ret < 0) {
> -		netdev_err(dev, "failed to enable clk %d\n", ret);
> +		clk_disable_unprepare(priv->mac_core_clk);
> +		netdev_err(dev, "failed to enable mac ifc clk %d\n", ret);

This change will break with existing DTs. The mac_ifc_clk should be 
optional.

Rob

>  		return ret;
>  	}
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] net: hix5hd2_gmac: add reset control and clock signals

[Dongpo Li]

Hi Rob,

On 2016/8/13 2:48, Rob Herring wrote:
> On Thu, Aug 11, 2016 at 05:01:53PM +0800, Dongpo Li wrote:
>> From: Li Dongpo <lidongpo@hisilicon.com>
>>
>> Add three reset control signals, "mac_core_rst", "mac_ifc_rst" and
>> "phy_rst".
>> The following diagram explained how the reset signals work.
>>
>>                         SoC
>> |-----------------------------------------------------
>> |                               ------                |
>> |                               | cpu |               |
>> |                               ------                |
>> |                                  |                  |
>> |                              ------------ AMBA bus  |
>> |                         GMAC     |                  |
>> |                            ----------------------   |
>> | ------------- mac_core_rst | --------------      |  |
>> | |clock and   |-------------->|   mac core  |     |  |
>> | |reset       |             | --------------      |  |
>> | |generator   |----         |       |             |  |
>> | -------------     |        | ----------------    |  |
>> |          |        ---------->| mac interface |   |  |
>> |          |     mac_ifc_rst | ----------------    |  |
>> |          |                 |       |             |  |
>> |          |                 | ------------------  |  |
>> |          |phy_rst          | | RGMII interface | |  |
>> |          |                 | ------------------  |  |
>> |          |                 ----------------------   |
>> |----------|------------------------------------------|
>>            |                          |
>>            |                      ----------
>>            |--------------------- |PHY chip |
>>                                   ----------
>>
>> The "mac_core_rst" represents "mac core reset signal", it resets
>> the mac core including packet processing unit, descriptor processing unit,
>> tx engine, rx engine, control unit.
>> The "mac_ifc_rst" represents "mac interface reset signal", it resets
>> the mac interface. The mac interface unit connects mac core and
>> data interface like MII/RMII/RGMII. After we set a new value of
>> interface mode, we must reset mac interface to reload the new mode value.
>> The "phy_rst" represents "phy reset signal", it does a hardware reset
>> on the PHY chip. This reset signal is optinal if the PHY can work well
>> without the hardware reset.
>>
>> Add one more clock signal, the existing is MAC core clock,
>> and the new one is MAC interface clock.
>>
>> Signed-off-by: Dongpo Li <lidongpo@hisilicon.com>
>> ---
>>  .../bindings/net/hisilicon-hix5hd2-gmac.txt        |  16 ++-
>>  drivers/net/ethernet/hisilicon/hix5hd2_gmac.c      | 140 +++++++++++++++++++--
>>  2 files changed, 143 insertions(+), 13 deletions(-)
> 
> 
>>
>> @@ -807,16 +829,26 @@ static int hix5hd2_net_open(struct net_device *dev)
>>  	struct phy_device *phy;
>>  	int ret;
>>  
>> -	ret = clk_prepare_enable(priv->clk);
>> +	ret = clk_prepare_enable(priv->mac_core_clk);
>> +	if (ret < 0) {
>> +		netdev_err(dev, "failed to enable mac core clk %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = clk_prepare_enable(priv->mac_ifc_clk);
>>  	if (ret < 0) {
>> -		netdev_err(dev, "failed to enable clk %d\n", ret);
>> +		clk_disable_unprepare(priv->mac_core_clk);
>> +		netdev_err(dev, "failed to enable mac ifc clk %d\n", ret);
> 
> This change will break with existing DTs. The mac_ifc_clk should be 
> optional.
> 
The mac_ifc_clk has existed in the existing hix5hd2. It's not implemented in the MAC driver
because the CLOCK driver implements a "complex" ethernet clock type.
So [PATCH 3/4] and [PATCH 4/4] are following this patch to change the CLOCK driver and existing DTs
at the same time.

> Rob
> 
>>  		return ret;
>>  	}
> 
> .
> 


    Regards,
    Dongpo

.

[PATCH 3/4] clk: hix5hd2: change ethernet clock type

[Dongpo Li]

Because the clock and reset signals share the same register,
we initialize reset controller when initializing clock controller.
So the ethernet driver will control the reset signal instead of
the clock driver.
All the ethernet clock is changed from complex clock to gate clock.
The original ethernet clock is really a "complex" clock because
it's obscure and hard to understand.

Signed-off-by: Dongpo Li <lidongpo@hisilicon.com>
---
 drivers/clk/hisilicon/clk-hix5hd2.c       | 72 +++++++------------------------
 include/dt-bindings/clock/hix5hd2-clock.h |  6 ++-
 2 files changed, 20 insertions(+), 58 deletions(-)

diff --git a/drivers/clk/hisilicon/clk-hix5hd2.c b/drivers/clk/hisilicon/clk-hix5hd2.c
index 14b05ef..f9689e3 100644
--- a/drivers/clk/hisilicon/clk-hix5hd2.c
+++ b/drivers/clk/hisilicon/clk-hix5hd2.c
@@ -12,6 +12,7 @@
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include "clk.h"
+#include "reset.h"
 
 static struct hisi_fixed_rate_clock hix5hd2_fixed_rate_clks[] __initdata = {
 	{ HIX5HD2_FIXED_1200M, "1200m", NULL, 0, 1200000000, },
@@ -93,8 +94,12 @@ static struct hisi_gate_clock hix5hd2_gate_clks[] __initdata = {
 	/* gsf */
 	{ HIX5HD2_FWD_BUS_CLK, "clk_fwd_bus", NULL, 0, 0xcc, 0, 0, },
 	{ HIX5HD2_FWD_SYS_CLK, "clk_fwd_sys", "clk_fwd_bus", 0, 0xcc, 5, 0, },
-	{ HIX5HD2_MAC0_PHY_CLK, "clk_fephy", "clk_fwd_sys",
-		 CLK_SET_RATE_PARENT, 0x120, 0, 0, },
+	{ HIX5HD2_MAC0_CLK, "clk_mac0", "clk_fwd_sys", 0, 0xcc, 1, 0, },
+	{ HIX5HD2_MAC_IFC0_CLK, "clk_mac_ifc0", "clk_fwd_sys", 0, 0xcc, 3, 0, },
+	{ HIX5HD2_MAC1_CLK, "clk_mac1", "clk_fwd_sys", 0, 0xcc, 2, 0, },
+	{ HIX5HD2_MAC_IFC1_CLK, "clk_mac_ifc1", "clk_fwd_sys", 0, 0xcc, 4, 0, },
+	{ HIX5HD2_MAC0_PHY_CLK, "clk_fephy", NULL,
+		 CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x120, 0, 0, },
 	/* wdg0 */
 	{ HIX5HD2_WDG0_CLK, "clk_wdg0", "24m",
 		CLK_SET_RATE_PARENT, 0x178, 0, 0, },
@@ -129,7 +134,6 @@ static struct hisi_gate_clock hix5hd2_gate_clks[] __initdata = {
 
 enum hix5hd2_clk_type {
 	TYPE_COMPLEX,
-	TYPE_ETHER,
 };
 
 struct hix5hd2_complex_clock {
@@ -157,10 +161,6 @@ struct hix5hd2_clk_complex {
 };
 
 static struct hix5hd2_complex_clock hix5hd2_complex_clks[] __initdata = {
-	{"clk_mac0", "clk_fephy", HIX5HD2_MAC0_CLK,
-		0xcc, 0xa, 0x500, 0x120, 0, 0x10, TYPE_ETHER},
-	{"clk_mac1", "clk_fwd_sys", HIX5HD2_MAC1_CLK,
-		0xcc, 0x14, 0xa00, 0x168, 0x2, 0, TYPE_ETHER},
 	{"clk_sata", NULL, HIX5HD2_SATA_CLK,
 		0xa8, 0x1f, 0x300, 0xac, 0x1, 0x0, TYPE_COMPLEX},
 	{"clk_usb", NULL, HIX5HD2_USB_CLK,
@@ -169,50 +169,6 @@ static struct hix5hd2_complex_clock hix5hd2_complex_clks[] __initdata = {
 
 #define to_complex_clk(_hw) container_of(_hw, struct hix5hd2_clk_complex, hw)
 
-static int clk_ether_prepare(struct clk_hw *hw)
-{
-	struct hix5hd2_clk_complex *clk = to_complex_clk(hw);
-	u32 val;
-
-	val = readl_relaxed(clk->ctrl_reg);
-	val |= clk->ctrl_clk_mask | clk->ctrl_rst_mask;
-	writel_relaxed(val, clk->ctrl_reg);
-	val &= ~(clk->ctrl_rst_mask);
-	writel_relaxed(val, clk->ctrl_reg);
-
-	val = readl_relaxed(clk->phy_reg);
-	val |= clk->phy_clk_mask;
-	val &= ~(clk->phy_rst_mask);
-	writel_relaxed(val, clk->phy_reg);
-	mdelay(10);
-
-	val &= ~(clk->phy_clk_mask);
-	val |= clk->phy_rst_mask;
-	writel_relaxed(val, clk->phy_reg);
-	mdelay(10);
-
-	val |= clk->phy_clk_mask;
-	val &= ~(clk->phy_rst_mask);
-	writel_relaxed(val, clk->phy_reg);
-	mdelay(30);
-	return 0;
-}
-
-static void clk_ether_unprepare(struct clk_hw *hw)
-{
-	struct hix5hd2_clk_complex *clk = to_complex_clk(hw);
-	u32 val;
-
-	val = readl_relaxed(clk->ctrl_reg);
-	val &= ~(clk->ctrl_clk_mask);
-	writel_relaxed(val, clk->ctrl_reg);
-}
-
-static struct clk_ops clk_ether_ops = {
-	.prepare = clk_ether_prepare,
-	.unprepare = clk_ether_unprepare,
-};
-
 static int clk_complex_enable(struct clk_hw *hw)
 {
 	struct hix5hd2_clk_complex *clk = to_complex_clk(hw);
@@ -269,10 +225,7 @@ hix5hd2_clk_register_complex(struct hix5hd2_complex_clock *clks, int nums,
 			return;
 
 		init.name = clks[i].name;
-		if (clks[i].type == TYPE_ETHER)
-			init.ops = &clk_ether_ops;
-		else
-			init.ops = &clk_complex_ops;
+		init.ops = &clk_complex_ops;
 
 		init.flags = CLK_IS_BASIC;
 		init.parent_names =
@@ -302,10 +255,17 @@ hix5hd2_clk_register_complex(struct hix5hd2_complex_clock *clks, int nums,
 static void __init hix5hd2_clk_init(struct device_node *np)
 {
 	struct hisi_clock_data *clk_data;
+	struct hisi_reset_controller *rstc;
+
+	rstc = hisi_reset_init(np);
+	if (!rstc)
+		return;
 
 	clk_data = hisi_clk_init(np, HIX5HD2_NR_CLKS);
-	if (!clk_data)
+	if (!clk_data) {
+		hisi_reset_exit(rstc);
 		return;
+	}
 
 	hisi_clk_register_fixed_rate(hix5hd2_fixed_rate_clks,
 				     ARRAY_SIZE(hix5hd2_fixed_rate_clks),
diff --git a/include/dt-bindings/clock/hix5hd2-clock.h b/include/dt-bindings/clock/hix5hd2-clock.h
index fd29c17..90f0731 100644
--- a/include/dt-bindings/clock/hix5hd2-clock.h
+++ b/include/dt-bindings/clock/hix5hd2-clock.h
@@ -74,10 +74,12 @@
 #define HIX5HD2_I2C4_RST		150
 #define HIX5HD2_I2C5_CLK		151
 #define HIX5HD2_I2C5_RST		152
+#define HIX5HD2_MAC0_CLK		153
+#define HIX5HD2_MAC1_CLK		154
+#define HIX5HD2_MAC_IFC0_CLK		155
+#define HIX5HD2_MAC_IFC1_CLK		156
 
 /* complex */
-#define HIX5HD2_MAC0_CLK		192
-#define HIX5HD2_MAC1_CLK		193
 #define HIX5HD2_SATA_CLK		194
 #define HIX5HD2_USB_CLK			195
 
-- 
2.8.2

Re: [PATCH 3/4] clk: hix5hd2: change ethernet clock type

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1970 bytes --]

Hi Dongpo,

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.8-rc1 next-20160811]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dongpo-Li/net-hix5hd2_gmac-add-tx-sg-feature-and-reset-clock-control-signals/20160811-170826
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-multi_v7_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/clk/hisilicon/clk-hix5hd2.c: In function 'hix5hd2_clk_init':
>> drivers/clk/hisilicon/clk-hix5hd2.c:260:25: error: passing argument 1 of 'hisi_reset_init' from incompatible pointer type [-Werror=incompatible-pointer-types]
     rstc = hisi_reset_init(np);
                            ^
   In file included from drivers/clk/hisilicon/clk-hix5hd2.c:15:0:
   drivers/clk/hisilicon/reset.h:25:31: note: expected 'struct platform_device *' but argument is of type 'struct device_node *'
    struct hisi_reset_controller *hisi_reset_init(struct platform_device *pdev);
                                  ^
   cc1: some warnings being treated as errors

vim +/hisi_reset_init +260 drivers/clk/hisilicon/clk-hix5hd2.c

   254	
   255	static void __init hix5hd2_clk_init(struct device_node *np)
   256	{
   257		struct hisi_clock_data *clk_data;
   258		struct hisi_reset_controller *rstc;
   259	
 > 260		rstc = hisi_reset_init(np);
   261		if (!rstc)
   262			return;
   263	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 38529 bytes --]

Re: [PATCH 3/4] clk: hix5hd2: change ethernet clock type

[Dongpo Li]

Hi all,

On 2016/8/11 20:09, kbuild test robot wrote:
> Hi Dongpo,
> 
> [auto build test ERROR on robh/for-next]
> [also build test ERROR on v4.8-rc1 next-20160811]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Dongpo-Li/net-hix5hd2_gmac-add-tx-sg-feature-and-reset-clock-control-signals/20160811-170826
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
> config: arm-multi_v7_defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 5.4.0-6) 5.4.0 20160609
> reproduce:
>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=arm 
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/clk/hisilicon/clk-hix5hd2.c: In function 'hix5hd2_clk_init':
>>> drivers/clk/hisilicon/clk-hix5hd2.c:260:25: error: passing argument 1 of 'hisi_reset_init' from incompatible pointer type [-Werror=incompatible-pointer-types]
>      rstc = hisi_reset_init(np);
>                             ^
>    In file included from drivers/clk/hisilicon/clk-hix5hd2.c:15:0:
>    drivers/clk/hisilicon/reset.h:25:31: note: expected 'struct platform_device *' but argument is of type 'struct device_node *'
>     struct hisi_reset_controller *hisi_reset_init(struct platform_device *pdev);
>                                   ^

Sorry for my mistake, the interface changes in newer patch. I will fix my problem in the next patch version.


    Regards,
    Dongpo

.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] ARM: dts: hix5hd2: add gmac clock and reset property

[Dongpo Li]

Add clock and reset property for gmac node.

Signed-off-by: Dongpo Li <lidongpo@hisilicon.com>
---
 arch/arm/boot/dts/hisi-x5hd2-dkb.dts |  2 ++
 arch/arm/boot/dts/hisi-x5hd2.dtsi    | 15 +++++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/hisi-x5hd2-dkb.dts b/arch/arm/boot/dts/hisi-x5hd2-dkb.dts
index d13af84..9da9283 100644
--- a/arch/arm/boot/dts/hisi-x5hd2-dkb.dts
+++ b/arch/arm/boot/dts/hisi-x5hd2-dkb.dts
@@ -57,6 +57,7 @@
 	#size-cells = <0>;
 	phy-handle = <&phy2>;
 	phy-mode = "mii";
+	hisilicon,phy-reset-delays-us = <1000 1000 20000>;
 	/* Placeholder, overwritten by bootloader */
 	mac-address = [00 00 00 00 00 00];
 	status = "okay";
@@ -71,6 +72,7 @@
 	#size-cells = <0>;
 	phy-handle = <&phy1>;
 	phy-mode = "rgmii";
+	hisilicon,phy-reset-delays-us = <10000 10000 30000>;
 	/* Placeholder, overwritten by bootloader */
 	mac-address = [00 00 00 00 00 00];
 	status = "okay";
diff --git a/arch/arm/boot/dts/hisi-x5hd2.dtsi b/arch/arm/boot/dts/hisi-x5hd2.dtsi
index fdcc23d..c391ce6 100644
--- a/arch/arm/boot/dts/hisi-x5hd2.dtsi
+++ b/arch/arm/boot/dts/hisi-x5hd2.dtsi
@@ -413,6 +413,7 @@
 				compatible = "hisilicon,hix5hd2-clock";
 				reg = <0 0x2000>;
 				#clock-cells = <1>;
+				#reset-cells = <2>;
 			};
 		};
 
@@ -436,18 +437,24 @@
 		};
 
 		gmac0: ethernet@1840000 {
-			compatible = "hisilicon,hix5hd2-gmac";
+			compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
 			reg = <0x1840000 0x1000>,<0x184300c 0x4>;
 			interrupts = <0 71 4>;
-			clocks = <&clock HIX5HD2_MAC0_CLK>;
+			clocks = <&clock HIX5HD2_MAC0_CLK>, <&clock HIX5HD2_MAC_IFC0_CLK>;
+			clock-names = "mac_core", "mac_ifc";
+			resets = <&clock 0xcc 8>, <&clock 0xcc 10>, <&clock 0x120 4>;
+			reset-names = "mac_core", "mac_ifc", "phy";
 			status = "disabled";
 		};
 
 		gmac1: ethernet@1841000 {
-			compatible = "hisilicon,hix5hd2-gmac";
+			compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
 			reg = <0x1841000 0x1000>,<0x1843010 0x4>;
 			interrupts = <0 72 4>;
-			clocks = <&clock HIX5HD2_MAC1_CLK>;
+			clocks = <&clock HIX5HD2_MAC1_CLK>, <&clock HIX5HD2_MAC_IFC1_CLK>;
+			clock-names = "mac_core", "mac_ifc";
+			resets = <&clock 0xcc 9>, <&clock 0xcc 11>, <&clock 0x168 2>;
+			reset-names = "mac_core", "mac_ifc", "phy";
 			status = "disabled";
 		};
 
-- 
2.8.2