diff mbox

am335x: cpsw: phy ignores max-speed setting

Message ID CAA93jw5=LDirktyC+rvpLi-kywUSosj6QV8-na5p3-f=PxKcWQ@mail.gmail.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Dave Taht Nov. 6, 2014, 4:51 p.m. UTC 
ooh! ooh! I have a BQL enablement patch for the cpsw that I have no
means of testing against multiple phys. Could
you give the attached very small patch a shot along the way?

The results I get on the beaglebone vs netperf-wrapper are pretty
spectacular - huge increase in throughput, big reduction in
latency.

http://snapon.lab.bufferbloat.net/~d/beagle_bql/bql_makes_a_difference.png
http://snapon.lab.bufferbloat.net/~d/beagle_bql/beaglebonewithbql.png

On Thu, Nov 6, 2014 at 8:25 AM, Yegor Yefremov
<yegorslists@googlemail.com> wrote:
> I' m trying to override max-speed setting for both CPSW connected
> PHYs. This is my DTS section for configuring CPSW:
>
> &mac {
>         pinctrl-names = "default", "sleep";
>         pinctrl-0 = <&cpsw_default>;
>         pinctrl-1 = <&cpsw_sleep>;
>         dual_emac = <1>;
>
>         status = "okay";
> };
>
> &davinci_mdio {
>         pinctrl-names = "default", "sleep";
>         pinctrl-0 = <&davinci_mdio_default>;
>         pinctrl-1 = <&davinci_mdio_sleep>;
>
>         status = "okay";
> };
>
> &cpsw_emac0 {
>         phy_id = <&davinci_mdio>, <0>;
>         phy-mode = "rgmii-id";
>         dual_emac_res_vlan = <1>;
>         max-speed = <100>;
> };
>
> &cpsw_emac1 {
>         phy_id = <&davinci_mdio>, <1>;
>         phy-mode = "rgmii-id";
>         dual_emac_res_vlan = <2>;
>         max-speed = <100>;
> };
>
> But in drivers/net/phy/phy_device.c->of_set_phy_supported() routine I
> don't get through node check, i.e. node == NULL. Any idea why?
>
> static void of_set_phy_supported(struct phy_device *phydev)
> {
>         struct device_node *node = phydev->dev.of_node;
>         u32 max_speed;
>
>         if (!IS_ENABLED(CONFIG_OF_MDIO))
>                 return;
>
>         if (!node)
>                 return;
>
>         if (!of_property_read_u32(node, "max-speed", &max_speed)) {
>                 /* The default values for phydev->supported are
> provided by the PHY
>                  * driver "features" member, we want to reset to sane
> defaults fist
>                  * before supporting higher speeds.
>                  */
>                 phydev->supported &= PHY_DEFAULT_FEATURES;
>
>                 switch (max_speed) {
>                 default:
>                         return;
>
>                 case SPEED_1000:
>                         phydev->supported |= PHY_1000BT_FEATURES;
>                 case SPEED_100:
>                         phydev->supported |= PHY_100BT_FEATURES;
>                 case SPEED_10:
>                         phydev->supported |= PHY_10BT_FEATURES;
>                 }
>         }
> }
>
> Yegor
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Eric Dumazet Nov. 6, 2014, 5:28 p.m. UTC | #1 
On Thu, 2014-11-06 at 08:51 -0800, Dave Taht wrote:
> ooh! ooh! I have a BQL enablement patch for the cpsw that I have no
> means of testing against multiple phys. Could
> you give the attached very small patch a shot along the way?
> 
> The results I get on the beaglebone vs netperf-wrapper are pretty
> spectacular - huge increase in throughput, big reduction in
> latency.

Please send this patch inline, so that we can comment, and start a new
thread.

@@ -1375,9 +1380,11 @@ static netdev_tx_t cpsw_ndo_start_xmit(struct sk_buff *skb,
                skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
 
        skb_tx_timestamp(skb);
-
+       len = max(skb->len, CPSW_MIN_PACKET_SIZE);
+       netdev_sent_queue(ndev,len);
        ret = cpsw_tx_packet_submit(ndev, priv, skb);
        if (unlikely(ret != 0)) {
+               netdev_completed_queue(ndev,1,len);
<<you can not do that, its racy with other netdev_completed_queue() calls from TX completion >>
                cpsw_err(priv, tx_err, "desc submit failed\n");
                goto fail;
        }


You need to call netdev_sent_queue(ndev, len); at the correct place,
because we can not 'undo' it.



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

Patch

From 7eccb26dc8f6d09660b22fcbd868572d050df26f Mon Sep 17 00:00:00 2001
From: Dave Taht <dave.taht@bufferbloat.net>
Date: Thu, 6 Nov 2014 08:45:30 -0800
Subject: [PATCH] Add BQL support to the TI cpsw driver

Tested on the beaglebone black.

I get a huge improvement in both throughput and latency.

Latency goes from 60ms worst case with pfifo_fast, and 12ms worst case with
sch_fq to 2.5ms with BQL enabled.

Throughput improved also.
---
 drivers/net/ethernet/ti/cpsw.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index d879448..5934fbc 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -118,7 +118,7 @@  do {								\
 #define CPDMA_TXCP		0x40
 #define CPDMA_RXCP		0x60
 
-#define CPSW_POLL_WEIGHT	64
+#define CPSW_POLL_WEIGHT	16
 #define CPSW_MIN_PACKET_SIZE	60
 #define CPSW_MAX_PACKET_SIZE	(1500 + 14 + 4 + 4)
 
@@ -693,6 +693,7 @@  static void cpsw_tx_handler(void *token, int len, int status)
 	cpts_tx_timestamp(priv->cpts, skb);
 	ndev->stats.tx_packets++;
 	ndev->stats.tx_bytes += len;
+	netdev_completed_queue(ndev,1,len);
 	dev_kfree_skb_any(skb);
 }
 
@@ -1307,6 +1308,8 @@  static int cpsw_ndo_open(struct net_device *ndev)
 		cpsw_set_coalesce(ndev, &coal);
 	}
 
+	netdev_reset_queue(ndev);
+	dev_info(priv->dev, "BQL enabled\n");
 	napi_enable(&priv->napi);
 	cpdma_ctlr_start(priv->dma);
 	cpsw_intr_enable(priv);
@@ -1341,6 +1344,7 @@  static int cpsw_ndo_stop(struct net_device *ndev)
 	netif_stop_queue(priv->ndev);
 	napi_disable(&priv->napi);
 	netif_carrier_off(priv->ndev);
+	netdev_reset_queue(priv->ndev);
 
 	if (cpsw_common_res_usage_state(priv) <= 1) {
 		cpts_unregister(priv->cpts);
@@ -1361,6 +1365,7 @@  static netdev_tx_t cpsw_ndo_start_xmit(struct sk_buff *skb,
 {
 	struct cpsw_priv *priv = netdev_priv(ndev);
 	int ret;
+	int len;
 
 	ndev->trans_start = jiffies;
 
@@ -1375,9 +1380,11 @@  static netdev_tx_t cpsw_ndo_start_xmit(struct sk_buff *skb,
 		skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
 
 	skb_tx_timestamp(skb);
-
+	len = max(skb->len, CPSW_MIN_PACKET_SIZE);
+	netdev_sent_queue(ndev,len);
 	ret = cpsw_tx_packet_submit(ndev, priv, skb);
 	if (unlikely(ret != 0)) {
+		netdev_completed_queue(ndev,1,len);
 		cpsw_err(priv, tx_err, "desc submit failed\n");
 		goto fail;
 	}
-- 
1.9.1