* [PATCH net-next 07/14] r8152: combine PHY reset with set_speed
From: Hayes Wang @ 2014-02-18 13:49 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1392731351-25502-1-git-send-email-hayeswang@realtek.com>
PHY reset is necessary after some hw settings. However, it would
cause the linking down, and so does the set_speed function. Combine
the PHY reset with set_speed function. That could reduce the frequency
of linking down and accessing the PHY register.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 57 ++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 45 insertions(+), 12 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index c7bae39..b3155da 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -436,6 +436,7 @@ enum rtl8152_flags {
RTL8152_SET_RX_MODE,
WORK_ENABLE,
RTL8152_LINK_CHG,
+ PHY_RESET,
};
/* Define these values to match your device */
@@ -1796,6 +1797,29 @@ static void r8152_power_cut_en(struct r8152 *tp, bool enable)
}
+static void rtl_phy_reset(struct r8152 *tp)
+{
+ u16 data;
+ int i;
+
+ clear_bit(PHY_RESET, &tp->flags);
+
+ data = r8152_mdio_read(tp, MII_BMCR);
+
+ /* don't reset again before the previous one complete */
+ if (data & BMCR_RESET)
+ return;
+
+ data |= BMCR_RESET;
+ r8152_mdio_write(tp, MII_BMCR, data);
+
+ for (i = 0; i < 50; i++) {
+ msleep(20);
+ if ((r8152_mdio_read(tp, MII_BMCR) & BMCR_RESET) == 0)
+ break;
+ }
+}
+
static void rtl_clear_bp(struct r8152 *tp)
{
ocp_write_dword(tp, MCU_TYPE_PLA, PLA_BP_0, 0);
@@ -1854,6 +1878,7 @@ static void r8152b_hw_phy_cfg(struct r8152 *tp)
}
r8152b_disable_aldps(tp);
+ set_bit(PHY_RESET, &tp->flags);
}
static void r8152b_exit_oob(struct r8152 *tp)
@@ -2042,6 +2067,8 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
data = sram_read(tp, SRAM_10M_AMP2);
data |= AMP_DN;
sram_write(tp, SRAM_10M_AMP2, data);
+
+ set_bit(PHY_RESET, &tp->flags);
}
static void r8153_u1u2en(struct r8152 *tp, bool enable)
@@ -2295,12 +2322,26 @@ static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u16 speed, u8 duplex)
bmcr = BMCR_ANENABLE | BMCR_ANRESTART;
}
+ if (test_bit(PHY_RESET, &tp->flags))
+ bmcr |= BMCR_RESET;
+
if (tp->mii.supports_gmii)
r8152_mdio_write(tp, MII_CTRL1000, gbcr);
r8152_mdio_write(tp, MII_ADVERTISE, anar);
r8152_mdio_write(tp, MII_BMCR, bmcr);
+ if (test_bit(PHY_RESET, &tp->flags)) {
+ int i;
+
+ clear_bit(PHY_RESET, &tp->flags);
+ for (i = 0; i < 50; i++) {
+ msleep(20);
+ if ((r8152_mdio_read(tp, MII_BMCR) & BMCR_RESET) == 0)
+ break;
+ }
+ }
+
out:
return ret;
@@ -2364,6 +2405,10 @@ static void rtl_work_func_t(struct work_struct *work)
if (test_bit(RTL8152_SET_RX_MODE, &tp->flags))
_rtl8152_set_rx_mode(tp->netdev);
+
+ if (test_bit(PHY_RESET, &tp->flags))
+ rtl_phy_reset(tp);
+
out1:
return;
}
@@ -2459,7 +2504,6 @@ static void r8152b_enable_fc(struct r8152 *tp)
static void r8152b_init(struct r8152 *tp)
{
u32 ocp_data;
- int i;
rtl_clear_bp(tp);
@@ -2491,14 +2535,6 @@ static void r8152b_init(struct r8152 *tp)
r8152b_enable_aldps(tp);
r8152b_enable_fc(tp);
- r8152_mdio_write(tp, MII_BMCR, BMCR_RESET | BMCR_ANENABLE |
- BMCR_ANRESTART);
- for (i = 0; i < 100; i++) {
- udelay(100);
- if (!(r8152_mdio_read(tp, MII_BMCR) & BMCR_RESET))
- break;
- }
-
/* enable rx aggregation */
ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_USB_CTRL);
ocp_data &= ~RX_AGG_DISABLE;
@@ -2569,9 +2605,6 @@ static void r8153_init(struct r8152 *tp)
r8153_enable_eee(tp);
r8153_enable_aldps(tp);
r8152b_enable_fc(tp);
-
- r8152_mdio_write(tp, MII_BMCR, BMCR_RESET | BMCR_ANENABLE |
- BMCR_ANRESTART);
}
static int rtl8152_suspend(struct usb_interface *intf, pm_message_t message)
--
1.8.4.2
^ permalink raw reply related
* [PATCH net-next 08/14] r8152: move some functions from probe to open
From: Hayes Wang @ 2014-02-18 13:49 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1392731351-25502-1-git-send-email-hayeswang@realtek.com>
Add up method for rtl_ops and asign relative functions. Move
clear_bp() and hw_phy_cfg() from init method to up method of rtl_ops.
Call rtl_ops.up() for ndo_open() and rtl_ops.down for ndo_stop().
Replace allocating the memory in probe() with in ndo_open().
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 45 +++++++++++++++++++++++++--------------------
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index b3155da..828572a 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -515,6 +515,7 @@ struct r8152 {
void (*init)(struct r8152 *);
int (*enable)(struct r8152 *);
void (*disable)(struct r8152 *);
+ void (*up)(struct r8152 *);
void (*down)(struct r8152 *);
void (*unload)(struct r8152 *);
} rtl_ops;
@@ -1878,6 +1879,10 @@ static void r8152b_hw_phy_cfg(struct r8152 *tp)
}
r8152b_disable_aldps(tp);
+
+ rtl_clear_bp(tp);
+
+ r8152b_enable_aldps(tp);
set_bit(PHY_RESET, &tp->flags);
}
@@ -1891,6 +1896,7 @@ static void r8152b_exit_oob(struct r8152 *tp)
ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, ocp_data);
rxdy_gated_en(tp, true);
+ r8152b_hw_phy_cfg(tp);
ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_NORAML);
ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CR, 0x00);
@@ -2033,6 +2039,8 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
r8152_mdio_write(tp, MII_BMCR, data);
}
+ r8153_clear_bp(tp);
+
if (tp->version == RTL_VER_03) {
data = ocp_reg_read(tp, OCP_EEE_CFG);
data &= ~CTAP_SHORT_EN;
@@ -2418,6 +2426,12 @@ static int rtl8152_open(struct net_device *netdev)
struct r8152 *tp = netdev_priv(netdev);
int res = 0;
+ res = alloc_all_mem(tp);
+ if (res)
+ goto out;
+
+ tp->rtl_ops.up(tp);
+
rtl8152_set_speed(tp, AUTONEG_ENABLE,
tp->mii.supports_gmii ? SPEED_1000 : SPEED_100,
DUPLEX_FULL);
@@ -2431,9 +2445,11 @@ static int rtl8152_open(struct net_device *netdev)
netif_device_detach(tp->netdev);
netif_warn(tp, ifup, netdev, "intr_urb submit failed: %d\n",
res);
+ free_all_mem(tp);
}
+out:
return res;
}
@@ -2447,9 +2463,11 @@ static int rtl8152_close(struct net_device *netdev)
cancel_delayed_work_sync(&tp->schedule);
netif_stop_queue(netdev);
tasklet_disable(&tp->tl);
- tp->rtl_ops.disable(tp);
+ tp->rtl_ops.down(tp);
tasklet_enable(&tp->tl);
+ free_all_mem(tp);
+
return res;
}
@@ -2505,21 +2523,14 @@ static void r8152b_init(struct r8152 *tp)
{
u32 ocp_data;
- rtl_clear_bp(tp);
-
if (tp->version == RTL_VER_01) {
ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_LED_FEATURE);
ocp_data &= ~LED_MODE_MASK;
ocp_write_word(tp, MCU_TYPE_PLA, PLA_LED_FEATURE, ocp_data);
}
- r8152b_hw_phy_cfg(tp);
-
r8152_power_cut_en(tp, false);
-
- r8152b_exit_oob(tp);
-
ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_PHY_PWR);
ocp_data |= TX_10M_IDLE_EN | PFM_PWM_SWITCH;
ocp_write_word(tp, MCU_TYPE_PLA, PLA_PHY_PWR, ocp_data);
@@ -2568,8 +2579,6 @@ static void r8153_init(struct r8152 *tp)
ocp_data &= ~TIMER11_EN;
ocp_write_word(tp, MCU_TYPE_USB, USB_WDT11_CTRL, ocp_data);
- r8153_clear_bp(tp);
-
ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_LED_FEATURE);
ocp_data &= ~LED_MODE_MASK;
ocp_write_word(tp, MCU_TYPE_PLA, PLA_LED_FEATURE, ocp_data);
@@ -2590,8 +2599,6 @@ static void r8153_init(struct r8152 *tp)
r8153_power_cut_en(tp, false);
r8153_u1u2en(tp, true);
- r8153_first_init(tp);
-
ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL, ALDPS_SPDWN_RATIO);
ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL2, EEE_SPDWN_RATIO);
ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3,
@@ -2618,10 +2625,10 @@ static int rtl8152_suspend(struct usb_interface *intf, pm_message_t message)
usb_kill_urb(tp->intr_urb);
cancel_delayed_work_sync(&tp->schedule);
tasklet_disable(&tp->tl);
+ tp->rtl_ops.down(tp);
+ tasklet_enable(&tp->tl);
}
- tp->rtl_ops.down(tp);
-
return 0;
}
@@ -2632,6 +2639,7 @@ static int rtl8152_resume(struct usb_interface *intf)
tp->rtl_ops.init(tp);
netif_device_attach(tp->netdev);
if (netif_running(tp->netdev)) {
+ tp->rtl_ops.up(tp);
rtl8152_set_speed(tp, AUTONEG_ENABLE,
tp->mii.supports_gmii ? SPEED_1000 : SPEED_100,
DUPLEX_FULL);
@@ -2639,7 +2647,6 @@ static int rtl8152_resume(struct usb_interface *intf)
netif_carrier_off(tp->netdev);
set_bit(WORK_ENABLE, &tp->flags);
usb_submit_urb(tp->intr_urb, GFP_KERNEL);
- tasklet_enable(&tp->tl);
}
return 0;
@@ -2780,6 +2787,7 @@ static int rtl_ops_init(struct r8152 *tp, const struct usb_device_id *id)
ops->init = r8152b_init;
ops->enable = rtl8152_enable;
ops->disable = rtl8152_disable;
+ ops->up = r8152b_exit_oob;
ops->down = rtl8152_down;
ops->unload = rtl8152_unload;
ret = 0;
@@ -2788,6 +2796,7 @@ static int rtl_ops_init(struct r8152 *tp, const struct usb_device_id *id)
ops->init = r8153_init;
ops->enable = rtl8153_enable;
ops->disable = rtl8152_disable;
+ ops->up = r8153_first_init;
ops->down = rtl8153_down;
ops->unload = rtl8153_unload;
ret = 0;
@@ -2803,6 +2812,7 @@ static int rtl_ops_init(struct r8152 *tp, const struct usb_device_id *id)
ops->init = r8153_init;
ops->enable = rtl8153_enable;
ops->disable = rtl8152_disable;
+ ops->up = r8153_first_init;
ops->down = rtl8153_down;
ops->unload = rtl8153_unload;
ret = 0;
@@ -2870,10 +2880,6 @@ static int rtl8152_probe(struct usb_interface *intf,
tp->rtl_ops.init(tp);
set_ethernet_addr(tp);
- ret = alloc_all_mem(tp);
- if (ret)
- goto out;
-
usb_set_intfdata(intf, tp);
ret = register_netdev(netdev);
@@ -2903,7 +2909,6 @@ static void rtl8152_disconnect(struct usb_interface *intf)
tasklet_kill(&tp->tl);
unregister_netdev(tp->netdev);
tp->rtl_ops.unload(tp);
- free_all_mem(tp);
free_netdev(tp->netdev);
}
}
--
1.8.4.2
^ permalink raw reply related
* [PATCH net-next 09/14] r8152: support WOL
From: Hayes Wang @ 2014-02-18 13:49 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1392731351-25502-1-git-send-email-hayeswang@realtek.com>
Support WOL for RTL8152 and RTL8153.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 118 ++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 105 insertions(+), 13 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 828572a..5d520be 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -23,7 +23,7 @@
#include <linux/ipv6.h>
/* Version Information */
-#define DRIVER_VERSION "v1.04.0 (2014/01/15)"
+#define DRIVER_VERSION "v1.05.0 (2014/02/18)"
#define DRIVER_AUTHOR "Realtek linux nic maintainers <nic_swsd@realtek.com>"
#define DRIVER_DESC "Realtek RTL8152/RTL8153 Based USB Ethernet Adapters"
#define MODULENAME "r8152"
@@ -62,6 +62,8 @@
#define PLA_RSTTELLY 0xe800
#define PLA_CR 0xe813
#define PLA_CRWECR 0xe81c
+#define PLA_CONFIG12 0xe81e /* CONFIG1, CONFIG2 */
+#define PLA_CONFIG34 0xe820 /* CONFIG3, CONFIG4 */
#define PLA_CONFIG5 0xe822
#define PLA_PHY_PWR 0xe84c
#define PLA_OOB_CTRL 0xe84f
@@ -216,7 +218,14 @@
/* PAL_BDC_CR */
#define ALDPS_PROXY_MODE 0x0001
+/* PLA_CONFIG34 */
+#define LINK_ON_WAKE_EN 0x0010
+#define LINK_OFF_WAKE_EN 0x0008
+
/* PLA_CONFIG5 */
+#define BWF_EN 0x0040
+#define MWF_EN 0x0020
+#define UWF_EN 0x0010
#define LAN_WAKE_EN 0x0002
/* PLA_LED_FEATURE */
@@ -521,6 +530,7 @@ struct r8152 {
} rtl_ops;
int intr_interval;
+ u32 saved_wolopts;
u32 msg_enable;
u32 tx_qlen;
u16 ocp_base;
@@ -1798,6 +1808,74 @@ static void r8152_power_cut_en(struct r8152 *tp, bool enable)
}
+#define WAKE_ANY (WAKE_PHY | WAKE_MAGIC | WAKE_UCAST | WAKE_BCAST | WAKE_MCAST)
+
+static u32 __rtl_get_wol(struct r8152 *tp)
+{
+ u32 ocp_data;
+ u32 wolopts = 0;
+
+ ocp_data = ocp_read_byte(tp, MCU_TYPE_PLA, PLA_CONFIG5);
+ if (!(ocp_data & LAN_WAKE_EN))
+ return 0;
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_CONFIG34);
+ if (ocp_data & LINK_ON_WAKE_EN)
+ wolopts |= WAKE_PHY;
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_CONFIG5);
+ if (ocp_data & UWF_EN)
+ wolopts |= WAKE_UCAST;
+ if (ocp_data & BWF_EN)
+ wolopts |= WAKE_BCAST;
+ if (ocp_data & MWF_EN)
+ wolopts |= WAKE_MCAST;
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_CFG_WOL);
+ if (ocp_data & MAGIC_EN)
+ wolopts |= WAKE_MAGIC;
+
+ return wolopts;
+}
+
+static void __rtl_set_wol(struct r8152 *tp, u32 wolopts)
+{
+ u32 ocp_data;
+
+ ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_CONFIG);
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_CONFIG34);
+ ocp_data &= ~LINK_ON_WAKE_EN;
+ if (wolopts & WAKE_PHY)
+ ocp_data |= LINK_ON_WAKE_EN;
+ ocp_write_word(tp, MCU_TYPE_PLA, PLA_CONFIG34, ocp_data);
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_CONFIG5);
+ ocp_data &= ~(UWF_EN | BWF_EN | MWF_EN | LAN_WAKE_EN);
+ if (wolopts & WAKE_UCAST)
+ ocp_data |= UWF_EN;
+ if (wolopts & WAKE_BCAST)
+ ocp_data |= BWF_EN;
+ if (wolopts & WAKE_MCAST)
+ ocp_data |= MWF_EN;
+ if (wolopts & WAKE_ANY)
+ ocp_data |= LAN_WAKE_EN;
+ ocp_write_word(tp, MCU_TYPE_PLA, PLA_CONFIG5, ocp_data);
+
+ ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_NORAML);
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_CFG_WOL);
+ ocp_data &= ~MAGIC_EN;
+ if (wolopts & WAKE_MAGIC)
+ ocp_data |= MAGIC_EN;
+ ocp_write_word(tp, MCU_TYPE_PLA, PLA_CFG_WOL, ocp_data);
+
+ if (wolopts & WAKE_ANY)
+ device_set_wakeup_enable(&tp->udev->dev, true);
+ else
+ device_set_wakeup_enable(&tp->udev->dev, false);
+}
+
static void rtl_phy_reset(struct r8152 *tp)
{
u16 data;
@@ -2002,10 +2080,6 @@ static void r8152b_enter_oob(struct r8152 *tp)
ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, RTL8152_RMS);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_CFG_WOL);
- ocp_data |= MAGIC_EN;
- ocp_write_word(tp, MCU_TYPE_PLA, PLA_CFG_WOL, ocp_data);
-
ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_CPCR);
ocp_data |= CPCR_RX_VLAN;
ocp_write_word(tp, MCU_TYPE_PLA, PLA_CPCR, ocp_data);
@@ -2018,8 +2092,6 @@ static void r8152b_enter_oob(struct r8152 *tp)
ocp_data |= NOW_IS_OOB | DIS_MCU_CLROOB;
ocp_write_byte(tp, MCU_TYPE_PLA, PLA_OOB_CTRL, ocp_data);
- ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CONFIG5, LAN_WAKE_EN);
-
rxdy_gated_en(tp, false);
ocp_data = ocp_read_dword(tp, MCU_TYPE_PLA, PLA_RCR);
@@ -2217,10 +2289,6 @@ static void r8153_enter_oob(struct r8152 *tp)
ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, RTL8152_RMS);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_CFG_WOL);
- ocp_data |= MAGIC_EN;
- ocp_write_word(tp, MCU_TYPE_PLA, PLA_CFG_WOL, ocp_data);
-
ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_TEREDO_CFG);
ocp_data &= ~TEREDO_WAKE_MASK;
ocp_write_word(tp, MCU_TYPE_PLA, PLA_TEREDO_CFG, ocp_data);
@@ -2237,8 +2305,6 @@ static void r8153_enter_oob(struct r8152 *tp)
ocp_data |= NOW_IS_OOB | DIS_MCU_CLROOB;
ocp_write_byte(tp, MCU_TYPE_PLA, PLA_OOB_CTRL, ocp_data);
- ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CONFIG5, LAN_WAKE_EN);
-
rxdy_gated_en(tp, false);
ocp_data = ocp_read_dword(tp, MCU_TYPE_PLA, PLA_RCR);
@@ -2652,6 +2718,24 @@ static int rtl8152_resume(struct usb_interface *intf)
return 0;
}
+static void rtl8152_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+{
+ struct r8152 *tp = netdev_priv(dev);
+
+ wol->supported = WAKE_ANY;
+ wol->wolopts = __rtl_get_wol(tp);
+}
+
+static int rtl8152_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+{
+ struct r8152 *tp = netdev_priv(dev);
+
+ __rtl_set_wol(tp, wol->wolopts);
+ tp->saved_wolopts = wol->wolopts & WAKE_ANY;
+
+ return 0;
+}
+
static void rtl8152_get_drvinfo(struct net_device *netdev,
struct ethtool_drvinfo *info)
{
@@ -2685,6 +2769,8 @@ static struct ethtool_ops ops = {
.get_settings = rtl8152_get_settings,
.set_settings = rtl8152_set_settings,
.get_link = ethtool_op_get_link,
+ .get_wol = rtl8152_get_wol,
+ .set_wol = rtl8152_set_wol,
};
static int rtl8152_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
@@ -2888,6 +2974,12 @@ static int rtl8152_probe(struct usb_interface *intf,
goto out1;
}
+ tp->saved_wolopts = __rtl_get_wol(tp);
+ if (tp->saved_wolopts)
+ device_set_wakeup_enable(&udev->dev, true);
+ else
+ device_set_wakeup_enable(&udev->dev, false);
+
netif_info(tp, probe, netdev, "%s\n", DRIVER_VERSION);
return 0;
--
1.8.4.2
^ permalink raw reply related
* [PATCH net-next 11/14] r8152: disable teredo for RTL8152
From: Hayes Wang @ 2014-02-18 13:49 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1392731351-25502-1-git-send-email-hayeswang@realtek.com>
Disable teredo for RTL8152 by default.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f303549..3ff11ed 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2021,6 +2021,7 @@ static void r8152b_exit_oob(struct r8152 *tp)
ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, ocp_data);
rxdy_gated_en(tp, true);
+ r8153_teredo_off(tp);
r8152b_hw_phy_cfg(tp);
ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_NORAML);
--
1.8.4.2
^ permalink raw reply related
* [PATCH net-next 12/14] r8152: replace netif_rx with netif_receive_skb
From: Hayes Wang @ 2014-02-18 13:49 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1392731351-25502-1-git-send-email-hayeswang@realtek.com>
Replace netif_rx with netif_receive_skb to avoid disabling irq frequently
for increasing the efficiency.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 3ff11ed..ff02d5d 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1464,7 +1464,7 @@ static void rx_bottom(struct r8152 *tp)
memcpy(skb->data, rx_data, pkt_len);
skb_put(skb, pkt_len);
skb->protocol = eth_type_trans(skb, netdev);
- netif_rx(skb);
+ netif_receive_skb(skb);
stats->rx_packets++;
stats->rx_bytes += pkt_len;
--
1.8.4.2
^ permalink raw reply related
* [PATCH net-next 13/14] r8152: set disable_hub_initiated_lpm
From: Hayes Wang @ 2014-02-18 13:49 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1392731351-25502-1-git-send-email-hayeswang@realtek.com>
Set disable_hub_initiated_lpm = 1.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ff02d5d..db98842 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3159,6 +3159,7 @@ static struct usb_driver rtl8152_driver = {
.resume = rtl8152_resume,
.reset_resume = rtl8152_resume,
.supports_autosuspend = 1,
+ .disable_hub_initiated_lpm = 1,
};
module_usb_driver(rtl8152_driver);
--
1.8.4.2
^ permalink raw reply related
* [PATCH net-next 14/14] r8152: support get_msglevel and set_msglevel
From: Hayes Wang @ 2014-02-18 13:49 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1392731351-25502-1-git-send-email-hayeswang@realtek.com>
Support get_msglevel and set_msglevel.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index db98842..0654bd3 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2852,6 +2852,20 @@ out_set_wol:
return ret;
}
+static u32 rtl8152_get_msglevel(struct net_device *dev)
+{
+ struct r8152 *tp = netdev_priv(dev);
+
+ return tp->msg_enable;
+}
+
+static void rtl8152_set_msglevel(struct net_device *dev, u32 value)
+{
+ struct r8152 *tp = netdev_priv(dev);
+
+ tp->msg_enable = value;
+}
+
static void rtl8152_get_drvinfo(struct net_device *netdev,
struct ethtool_drvinfo *info)
{
@@ -2895,6 +2909,8 @@ static struct ethtool_ops ops = {
.get_settings = rtl8152_get_settings,
.set_settings = rtl8152_set_settings,
.get_link = ethtool_op_get_link,
+ .get_msglevel = rtl8152_get_msglevel,
+ .set_msglevel = rtl8152_set_msglevel,
.get_wol = rtl8152_get_wol,
.set_wol = rtl8152_set_wol,
};
--
1.8.4.2
^ permalink raw reply related
* [PATCH net-next 10/14] r8152: support runtime suspend
From: Hayes Wang @ 2014-02-18 13:49 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1392731351-25502-1-git-send-email-hayeswang@realtek.com>
Support runtime suspend for RTL8152 and RTL8153.
Move tx_bottom() from tasklet to delayed_work. That avoids to
transmit tx packets after calling autosuspend.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 181 ++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 158 insertions(+), 23 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 5d520be..f303549 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -445,6 +445,7 @@ enum rtl8152_flags {
RTL8152_SET_RX_MODE,
WORK_ENABLE,
RTL8152_LINK_CHG,
+ SELECTIVE_SUSPEND,
PHY_RESET,
};
@@ -877,11 +878,21 @@ static u16 sram_read(struct r8152 *tp, u16 addr)
static int read_mii_word(struct net_device *netdev, int phy_id, int reg)
{
struct r8152 *tp = netdev_priv(netdev);
+ int ret;
if (phy_id != R8152_PHY_ID)
return -EINVAL;
- return r8152_mdio_read(tp, reg);
+ ret = usb_autopm_get_interface(tp->intf);
+ if (ret < 0)
+ goto out;
+
+ ret = r8152_mdio_read(tp, reg);
+
+ usb_autopm_put_interface(tp->intf);
+
+out:
+ return ret;
}
static
@@ -892,7 +903,12 @@ void write_mii_word(struct net_device *netdev, int phy_id, int reg, int val)
if (phy_id != R8152_PHY_ID)
return;
+ if (usb_autopm_get_interface(tp->intf) < 0)
+ return;
+
r8152_mdio_write(tp, reg, val);
+
+ usb_autopm_put_interface(tp->intf);
}
static
@@ -978,6 +994,8 @@ static void read_bulk_callback(struct urb *urb)
if (!netif_carrier_ok(netdev))
return;
+ usb_mark_last_busy(tp->udev);
+
switch (status) {
case 0:
if (urb->actual_length < ETH_ZLEN)
@@ -1045,6 +1063,8 @@ static void write_bulk_callback(struct urb *urb)
list_add_tail(&agg->list, &tp->tx_free);
spin_unlock_irqrestore(&tp->tx_lock, flags);
+ usb_autopm_put_interface_async(tp->intf);
+
if (!netif_carrier_ok(tp->netdev))
return;
@@ -1055,7 +1075,7 @@ static void write_bulk_callback(struct urb *urb)
return;
if (!skb_queue_empty(&tp->tx_queue))
- tasklet_schedule(&tp->tl);
+ schedule_delayed_work(&tp->schedule, 0);
}
static void intr_callback(struct urb *urb)
@@ -1313,7 +1333,7 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
{
struct sk_buff_head skb_head, *tx_queue = &tp->tx_queue;
unsigned long flags;
- int remain;
+ int remain, ret;
u8 *tx_data;
__skb_queue_head_init(&skb_head);
@@ -1361,19 +1381,28 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
spin_unlock_irqrestore(&tx_queue->lock, flags);
}
- netif_tx_lock(tp->netdev);
+ netif_tx_lock_bh(tp->netdev);
if (netif_queue_stopped(tp->netdev) &&
skb_queue_len(&tp->tx_queue) < tp->tx_qlen)
netif_wake_queue(tp->netdev);
- netif_tx_unlock(tp->netdev);
+ netif_tx_unlock_bh(tp->netdev);
+
+ ret = usb_autopm_get_interface(tp->intf);
+ if (ret < 0)
+ goto out_tx_fill;
usb_fill_bulk_urb(agg->urb, tp->udev, usb_sndbulkpipe(tp->udev, 2),
agg->head, (int)(tx_data - (u8 *)agg->head),
(usb_complete_t)write_bulk_callback, agg);
- return usb_submit_urb(agg->urb, GFP_ATOMIC);
+ ret = usb_submit_urb(agg->urb, GFP_KERNEL);
+ if (ret < 0)
+ usb_autopm_put_interface(tp->intf);
+
+out_tx_fill:
+ return ret;
}
static void rx_bottom(struct r8152 *tp)
@@ -1511,7 +1540,6 @@ static void bottom_half(unsigned long data)
return;
rx_bottom(tp);
- tx_bottom(tp);
}
static
@@ -1621,7 +1649,7 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
netif_stop_queue(netdev);
if (!list_empty(&tp->tx_free))
- tasklet_schedule(&tp->tl);
+ schedule_delayed_work(&tp->schedule, 0);
return NETDEV_TX_OK;
}
@@ -1876,6 +1904,25 @@ static void __rtl_set_wol(struct r8152 *tp, u32 wolopts)
device_set_wakeup_enable(&tp->udev->dev, false);
}
+static void rtl_runtime_suspend_enable(struct r8152 *tp, bool enable)
+{
+ if (enable) {
+ u32 ocp_data;
+
+ __rtl_set_wol(tp, WAKE_ANY);
+
+ ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_CONFIG);
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_CONFIG34);
+ ocp_data |= LINK_OFF_WAKE_EN;
+ ocp_write_word(tp, MCU_TYPE_PLA, PLA_CONFIG34, ocp_data);
+
+ ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_NORAML);
+ } else {
+ __rtl_set_wol(tp, tp->saved_wolopts);
+ }
+}
+
static void rtl_phy_reset(struct r8152 *tp)
{
u16 data;
@@ -2467,6 +2514,9 @@ static void rtl_work_func_t(struct work_struct *work)
{
struct r8152 *tp = container_of(work, struct r8152, schedule.work);
+ if (usb_autopm_get_interface(tp->intf) < 0)
+ return;
+
if (!test_bit(WORK_ENABLE, &tp->flags))
goto out1;
@@ -2479,12 +2529,14 @@ static void rtl_work_func_t(struct work_struct *work)
if (test_bit(RTL8152_SET_RX_MODE, &tp->flags))
_rtl8152_set_rx_mode(tp->netdev);
+ if (tp->speed & LINK_STATUS)
+ tx_bottom(tp);
if (test_bit(PHY_RESET, &tp->flags))
rtl_phy_reset(tp);
out1:
- return;
+ usb_autopm_put_interface(tp->intf);
}
static int rtl8152_open(struct net_device *netdev)
@@ -2496,6 +2548,21 @@ static int rtl8152_open(struct net_device *netdev)
if (res)
goto out;
+ res = usb_autopm_get_interface(tp->intf);
+ if (res < 0) {
+ free_all_mem(tp);
+ goto out;
+ }
+
+ /* The WORK_ENABLE may be set when autoresume occurs */
+ if (test_bit(WORK_ENABLE, &tp->flags)) {
+ clear_bit(WORK_ENABLE, &tp->flags);
+ usb_kill_urb(tp->intr_urb);
+ cancel_delayed_work_sync(&tp->schedule);
+ if (tp->speed & LINK_STATUS)
+ tp->rtl_ops.disable(tp);
+ }
+
tp->rtl_ops.up(tp);
rtl8152_set_speed(tp, AUTONEG_ENABLE,
@@ -2514,6 +2581,7 @@ static int rtl8152_open(struct net_device *netdev)
free_all_mem(tp);
}
+ usb_autopm_put_interface(tp->intf);
out:
return res;
@@ -2528,9 +2596,26 @@ static int rtl8152_close(struct net_device *netdev)
usb_kill_urb(tp->intr_urb);
cancel_delayed_work_sync(&tp->schedule);
netif_stop_queue(netdev);
- tasklet_disable(&tp->tl);
- tp->rtl_ops.down(tp);
- tasklet_enable(&tp->tl);
+
+ res = usb_autopm_get_interface(tp->intf);
+ if (res < 0) {
+ rtl_drop_queued_tx(tp);
+ } else {
+ /*
+ * The autosuspend may have been enabled and wouldn't
+ * be disable when autoresume occurs, because the
+ * netif_running() would be false.
+ */
+ if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
+ rtl_runtime_suspend_enable(tp, false);
+ clear_bit(SELECTIVE_SUSPEND, &tp->flags);
+ }
+
+ tasklet_disable(&tp->tl);
+ tp->rtl_ops.down(tp);
+ tasklet_enable(&tp->tl);
+ usb_autopm_put_interface(tp->intf);
+ }
free_all_mem(tp);
@@ -2684,15 +2769,22 @@ static int rtl8152_suspend(struct usb_interface *intf, pm_message_t message)
{
struct r8152 *tp = usb_get_intfdata(intf);
- netif_device_detach(tp->netdev);
+ if (PMSG_IS_AUTO(message))
+ set_bit(SELECTIVE_SUSPEND, &tp->flags);
+ else
+ netif_device_detach(tp->netdev);
if (netif_running(tp->netdev)) {
clear_bit(WORK_ENABLE, &tp->flags);
usb_kill_urb(tp->intr_urb);
cancel_delayed_work_sync(&tp->schedule);
- tasklet_disable(&tp->tl);
- tp->rtl_ops.down(tp);
- tasklet_enable(&tp->tl);
+ if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
+ rtl_runtime_suspend_enable(tp, true);
+ } else {
+ tasklet_disable(&tp->tl);
+ tp->rtl_ops.down(tp);
+ tasklet_enable(&tp->tl);
+ }
}
return 0;
@@ -2702,13 +2794,23 @@ static int rtl8152_resume(struct usb_interface *intf)
{
struct r8152 *tp = usb_get_intfdata(intf);
- tp->rtl_ops.init(tp);
- netif_device_attach(tp->netdev);
+ if (!test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
+ tp->rtl_ops.init(tp);
+ netif_device_attach(tp->netdev);
+ }
+
if (netif_running(tp->netdev)) {
- tp->rtl_ops.up(tp);
- rtl8152_set_speed(tp, AUTONEG_ENABLE,
+ if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
+ rtl_runtime_suspend_enable(tp, false);
+ clear_bit(SELECTIVE_SUSPEND, &tp->flags);
+ if (tp->speed & LINK_STATUS)
+ tp->rtl_ops.disable(tp);
+ } else {
+ tp->rtl_ops.up(tp);
+ rtl8152_set_speed(tp, AUTONEG_ENABLE,
tp->mii.supports_gmii ? SPEED_1000 : SPEED_100,
DUPLEX_FULL);
+ }
tp->speed = 0;
netif_carrier_off(tp->netdev);
set_bit(WORK_ENABLE, &tp->flags);
@@ -2722,18 +2824,31 @@ static void rtl8152_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
{
struct r8152 *tp = netdev_priv(dev);
+ if (usb_autopm_get_interface(tp->intf) < 0)
+ return;
+
wol->supported = WAKE_ANY;
wol->wolopts = __rtl_get_wol(tp);
+
+ usb_autopm_put_interface(tp->intf);
}
static int rtl8152_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
{
struct r8152 *tp = netdev_priv(dev);
+ int ret;
+
+ ret = usb_autopm_get_interface(tp->intf);
+ if (ret < 0)
+ goto out_set_wol;
__rtl_set_wol(tp, wol->wolopts);
tp->saved_wolopts = wol->wolopts & WAKE_ANY;
- return 0;
+ usb_autopm_put_interface(tp->intf);
+
+out_set_wol:
+ return ret;
}
static void rtl8152_get_drvinfo(struct net_device *netdev,
@@ -2760,8 +2875,18 @@ int rtl8152_get_settings(struct net_device *netdev, struct ethtool_cmd *cmd)
static int rtl8152_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
{
struct r8152 *tp = netdev_priv(dev);
+ int ret;
+
+ ret = usb_autopm_get_interface(tp->intf);
+ if (ret < 0)
+ goto out;
- return rtl8152_set_speed(tp, cmd->autoneg, cmd->speed, cmd->duplex);
+ ret = rtl8152_set_speed(tp, cmd->autoneg, cmd->speed, cmd->duplex);
+
+ usb_autopm_put_interface(tp->intf);
+
+out:
+ return ret;
}
static struct ethtool_ops ops = {
@@ -2777,7 +2902,11 @@ static int rtl8152_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
{
struct r8152 *tp = netdev_priv(netdev);
struct mii_ioctl_data *data = if_mii(rq);
- int res = 0;
+ int res;
+
+ res = usb_autopm_get_interface(tp->intf);
+ if (res < 0)
+ goto out;
switch (cmd) {
case SIOCGMIIPHY:
@@ -2800,6 +2929,9 @@ static int rtl8152_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
res = -EOPNOTSUPP;
}
+ usb_autopm_put_interface(tp->intf);
+
+out:
return res;
}
@@ -2962,6 +3094,8 @@ static int rtl8152_probe(struct usb_interface *intf,
tp->mii.phy_id = R8152_PHY_ID;
tp->mii.supports_gmii = 0;
+ intf->needs_remote_wakeup = 1;
+
r8152b_get_version(tp);
tp->rtl_ops.init(tp);
set_ethernet_addr(tp);
@@ -3023,6 +3157,7 @@ static struct usb_driver rtl8152_driver = {
.suspend = rtl8152_suspend,
.resume = rtl8152_resume,
.reset_resume = rtl8152_resume,
+ .supports_autosuspend = 1,
};
module_usb_driver(rtl8152_driver);
--
1.8.4.2
^ permalink raw reply related
* Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text
From: Andrew Lunn @ 2014-02-18 13:58 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Jason Cooper, David S. Miller, netdev, Gregory Clement,
Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia,
linux-arm-kernel
In-Reply-To: <20140218141303.77c77272@skate>
> At this point, I don't expect to see this IP in any other SOCs than
> Marvell Armada EBU, so I'll resend with a "depends on PLAT_ORION", as I
> agree it is much safer in terms of build problems.
Hi Thomas
PLAT_ORION is a bit of an odd thing now.
For me, PLAT_ORION means arch/arm/plat-orion.
But as far as i know, 370/XP does not actually use anything from
arch/arm/plat-orion. When kirkwood moves into mach-mvebu, it also will
not use any code from it, and i suspect dove is the same.
So maybe in a few cycles, when only mach-orion5x is left, we can merge
arch/arm/plat-orion into arch/arm/mach-orion5x and PLAT_ORION goes
away?
Or do we want to define that PLAT_ORION means any system which can
make use of mvebu drivers?
Andrew
^ permalink raw reply
* Re: [PATCH v2] net: ethernet: update dependency and help text of mvneta
From: Jason Cooper @ 2014-02-18 14:06 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: David S. Miller, netdev, Gregory Clement, Andrew Lunn,
Sebastian Hesselbarth, Ezequiel Garcia, linux-arm-kernel
In-Reply-To: <1392729491-24913-1-git-send-email-thomas.petazzoni@free-electrons.com>
On Tue, Feb 18, 2014 at 02:18:11PM +0100, Thomas Petazzoni wrote:
> With the introduction of the support for Armada 375 and Armada 38x,
> the hidden Kconfig option MACH_ARMADA_370_XP is being renamed to
> MACH_MVEBU_V7. Therefore, the dependency that was used for the mvneta
> driver can no longer work. This commit replaces this dependency by a
> dependency on PLAT_ORION, which is used similarly for the mv643xx_eth
> driver.
>
> In addition to this, it takes this opportunity to adjust the
> description and help text to indicate that the driver can is also used
> for Armada 38x. Note that Armada 375 cannot use this driver as it has
> a completely different networking unit, which will require a separate
> driver.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> Changes since v1:
> - Instead of removing the Kconfig dependency, replace it by a
> dependency on PLAT_ORION, in order to avoid building mvneta on too
> many architectures and getting weird build issues. Suggested by
> Jason Cooper.
> - Fix capitalization in the Kconfig help text.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> drivers/net/ethernet/marvell/Kconfig | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Acked-by: Jason Cooper <jason@lakedaemon.net>
thx,
Jason.
^ permalink raw reply
* Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text
From: Thomas Petazzoni @ 2014-02-18 14:10 UTC (permalink / raw)
To: Andrew Lunn
Cc: Jason Cooper, David S. Miller, netdev, Gregory Clement,
Sebastian Hesselbarth, Ezequiel Garcia, linux-arm-kernel
In-Reply-To: <20140218135809.GD17984@lunn.ch>
Dear Andrew Lunn,
On Tue, 18 Feb 2014 14:58:09 +0100, Andrew Lunn wrote:
> PLAT_ORION is a bit of an odd thing now.
>
> For me, PLAT_ORION means arch/arm/plat-orion.
No, it is PLAT_ORION_LEGACY which means arch/arm/plat-orion.
> But as far as i know, 370/XP does not actually use anything from
> arch/arm/plat-orion. When kirkwood moves into mach-mvebu, it also will
> not use any code from it, and i suspect dove is the same.
Interestingly, we have -I$(srctree)/arch/arm/plat-orion/include in
mach-mvebu/Makefile. Might be something to revisit later on.
> So maybe in a few cycles, when only mach-orion5x is left, we can merge
> arch/arm/plat-orion into arch/arm/mach-orion5x and PLAT_ORION goes
> away?
We also have mach-mv78xx0 to worry about, unless we decide to remove
support for it entirely. And even if plat-orion is merged into
mach-orion5x, we will keep PLAT_ORION as a way to indicate that the
platform needs to use a certain number of drivers (see below).
> Or do we want to define that PLAT_ORION means any system which can
> make use of mvebu drivers?
This is what it means today. The MV_XOR driver is under PLAT_ORION, the
gpio driver is under PLAT_ORION, the Device Bus driver is under
PLAT_ORION, the I2C driver is under PLAT_ORION, the pinctrl driver as
well, and so on and so on.
At the very end of the clean up, when even Orion5x support will be
merged in mach-mvebu/, then we can certainly replace these dependencies
by a "depends on ARCH_MVEBU". But for the time being, PLAT_ORION seems
like the right common denominator for all these platforms.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v2] of_mdio: fix phy interrupt passing
From: Sergei Shtylyov @ 2014-02-18 14:48 UTC (permalink / raw)
To: Ben Dooks, grant.likely
Cc: linux-kernel, devicetree, linux-kernel, netdev, linux-sh
In-Reply-To: <1392725818-558-1-git-send-email-ben.dooks@codethink.co.uk>
Hello.
On 18-02-2014 16:16, Ben Dooks wrote:
> The of_mdiobus_register_phy() is not setting phy->irq thus causing
> some drivers to incorrectly assume that the PHY does not have an
> IRQ associated with it. Not only do some drivers report no IRQ
> they do not install an interrupt handler for the PHY.
> Simplify the code setting irq and set the phy->irq at the same
> time so that we cover the following issues, which should cover
> all the cases the code will find:
> - Set phy->irq if node has irq property and mdio->irq is NULL
> - Set phy->irq if node has no irq and mdio->irq is not NULL
> - Leave phy->irq as PHY_POLL default if none of the above
> This fixes the issue:
> net eth0: attached PHY 1 (IRQ -1) to driver Micrel KSZ8041RNLI
> to the correct:
> net eth0: attached PHY 1 (IRQ 416) to driver Micrel KSZ8041RNLI
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> Since v1:
> - Updated phy->irq setting code
> - Deal with issue if mdio->irq array NULL
> Notes:
> It was discussed if this should be two patches, but we end up
> making enough changes in #2 the same area as #1 means that we
> basically rewrite #1 in #2. So keep it as 1 patch.
> ---
> drivers/of/of_mdio.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 875b7b6..46d95fc 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
[...]
> @@ -54,12 +54,14 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
> if (!phy || IS_ERR(phy))
> return 1;
>
> - if (mdio->irq) {
> - prev_irq = mdio->irq[addr];
> - mdio->irq[addr] =
> - irq_of_parse_and_map(child, 0);
> - if (!mdio->irq[addr])
> - mdio->irq[addr] = prev_irq;
> + rc = irq_of_parse_and_map(child, 0);
> + if (rc > 0) {
> + phy->irq = rc;
> + if (mdio->irq)
> + mdio->irq[addr] = rc;
> + } else {
> + if (mdio->irq)
You could write that as *else if* and save a line of code...
> + phy->irq = mdio->irq[addr];
> }
WBR, Sergei
^ permalink raw reply
* Re: [PATCH RFC] sctp: Update HEARTBEAT timer immediately after user changed HB.interval
From: Vlad Yasevich @ 2014-02-18 14:50 UTC (permalink / raw)
To: Xufeng Zhang, nhorman, davem; +Cc: linux-sctp, netdev, linux-kernel
In-Reply-To: <1392703010-6128-1-git-send-email-xufeng.zhang@windriver.com>
On 02/18/2014 12:56 AM, Xufeng Zhang wrote:
> For an established association, if user has updated the HB.interval
> parameter by setsockopt(), this new heartbeat interval will not
> take effect until:
> - the expiry of the heartbeat timer and new hearbeat is sent.
> - DATA chunk has been sent and the transport resets the timer.
> This could not meet the requirement of the user who need to
> get HEARTBEAT sent at the specified time.
>
> Thus, we need to update the heartbeat timer immediately after
> user has changed HB.interval.
>
> Signed-off-by: Xufeng Zhang <xufeng.zhang@windriver.com>
> ---
> net/sctp/socket.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 9e91d6e..699ae1e 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -2344,6 +2344,11 @@ static int sctp_apply_peer_addr_params(struct sctp_paddrparams *params,
> if (trans) {
> trans->hbinterval =
> msecs_to_jiffies(params->spp_hbinterval);
> +
> + /* Update the heartbeat timer immediately. */
> + if (!mod_timer(&trans->hb_timer,
> + sctp_transport_timeout(trans)))
> + sctp_transport_hold(trans);
This is causes a rather inconsistent behavior in that it doesn't
account of the amount of time left on the hb timer. Thus it doesn't
always do what commit log implies. If the remaining time is shorter
then the new timeout value, it will take longer till the next HB
the application expects.
If the application has very strict timing requirement on the HBs, it
should trigger the HB manually.
We could rework the code to allow the combination of
SPP_HB_DEMAND | SPP_HB_ENABLE to work as follows:
1) update the timer to the new value
2) Send an immediate HB
a) Update the hb timeout.
2a should probably be done regardless since it can cause 2 HB to be
outstanding at the same time.
-vlad
> } else if (asoc) {
> asoc->hbinterval =
> msecs_to_jiffies(params->spp_hbinterval);
>
^ permalink raw reply
* RE: [PATCH RFC] sctp: Update HEARTBEAT timer immediately after user changed HB.interval
From: David Laight @ 2014-02-18 15:39 UTC (permalink / raw)
To: 'Vlad Yasevich', Xufeng Zhang, nhorman@tuxdriver.com,
davem@davemloft.net
Cc: linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <5303731C.8020308@gmail.com>
> > +
> > + /* Update the heartbeat timer immediately. */
> > + if (!mod_timer(&trans->hb_timer,
> > + sctp_transport_timeout(trans)))
> > + sctp_transport_hold(trans);
>
> This is causes a rather inconsistent behavior in that it doesn't
> account of the amount of time left on the hb timer. Thus it doesn't
> always do what commit log implies. If the remaining time is shorter
> then the new timeout value, it will take longer till the next HB
> the application expects.
Being able to stop heartbeats being sent by repeatedly configuring
the interval is certainly not a good idea!
> If the application has very strict timing requirement on the HBs, it
> should trigger the HB manually.
>
> We could rework the code to allow the combination of
> SPP_HB_DEMAND | SPP_HB_ENABLE to work as follows:
> 1) update the timer to the new value
> 2) Send an immediate HB
> a) Update the hb timeout.
>
> 2a should probably be done regardless since it can cause 2 HB to be
> outstanding at the same time.
Sending a heartbeat 'on demand' might be problematic if one
has also just been sent (from the timer).
I'd have thought that you wouldn't want to send a heartbeat while
still expecting a response from the previous one (this might require
splitting the time interval in two).
David
^ permalink raw reply
* Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text
From: Ezequiel Garcia @ 2014-02-18 15:45 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Andrew Lunn, Jason Cooper, David S. Miller, netdev,
Gregory Clement, Sebastian Hesselbarth, linux-arm-kernel
In-Reply-To: <20140218151027.710ab2f3@skate>
On Tue, Feb 18, 2014 at 03:10:27PM +0100, Thomas Petazzoni wrote:
[..]
>
> At the very end of the clean up, when even Orion5x support will be
> merged in mach-mvebu/, then we can certainly replace these dependencies
> by a "depends on ARCH_MVEBU". But for the time being, PLAT_ORION seems
> like the right common denominator for all these platforms.
>
Last time we talked about this with Sebastian and Andrew it was decided
that the right choice is:
MACH_KIRKWOOD || MACH_DOVE || MACH_ARMADA_370_XP
Which would become MACH_MVEBU.
Of course, this is near the nitpick boundary, so AFAIC you can leave
PLAT_ORION as in v2, if you feel like.
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text
From: Thomas Petazzoni @ 2014-02-18 15:51 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: Andrew Lunn, Jason Cooper, David S. Miller, netdev,
Gregory Clement, Sebastian Hesselbarth, linux-arm-kernel
In-Reply-To: <20140218154512.GA10691@localhost>
Dear Ezequiel Garcia,
On Tue, 18 Feb 2014 12:45:12 -0300, Ezequiel Garcia wrote:
> > At the very end of the clean up, when even Orion5x support will be
> > merged in mach-mvebu/, then we can certainly replace these dependencies
> > by a "depends on ARCH_MVEBU". But for the time being, PLAT_ORION seems
> > like the right common denominator for all these platforms.
> >
>
> Last time we talked about this with Sebastian and Andrew it was decided
> that the right choice is:
>
> MACH_KIRKWOOD || MACH_DOVE || MACH_ARMADA_370_XP
And why not Orion5x and MV78xx0 ?
>
> Which would become MACH_MVEBU.
>
> Of course, this is near the nitpick boundary, so AFAIC you can leave
> PLAT_ORION as in v2, if you feel like.
As I suggested previously, PLAT_ORION is what *today* controls the
visibility of all Marvell EBU drivers. Please grep for PLAT_ORION in
your kernel tree. So why on earth would we invent something completely
different for mvneta?
When tomorrow ARCH_MVEBU will be used for all mvebu platforms, then
indeed PLAT_ORION can be changed to ARCH_MVEBU.
Your suggestion above to use MACH_ARMADA_370_XP is *precisely* what
this patch is removing, because MACH_ARMADA_370_XP is being removed
from arch/arm/mach-mvebu/.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply
* Re: [patch] af_packet: remove a stray tab in packet_set_ring()
From: Daniel Borkmann @ 2014-02-18 15:52 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Willem de Bruijn, Hannes Frederic Sowa,
Eric Dumazet, Phil Sutter, Atzm Watanabe, Richard Cochran, netdev,
kernel-janitors
In-Reply-To: <20140218122050.GB9604@elgon.mountain>
On 02/18/2014 01:20 PM, Dan Carpenter wrote:
> At first glance it looks like there is a missing curly brace but
> actually the code works the same either way. I have adjusted the
> indenting but left the code the same.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Agreed.
Actually, this whole construct could have been simplified ...
switch (po->tp_version) {
case TPACKET_V3:
/* Transmit path is not supported. We checked
* it above but just being paranoid
*/
if (!tx_ring)
init_prb_bdqc(po, rb, pg_vec, req_u, tx_ring);
break;
default:
break;
}
... into just one if condition. Looks a bit unnecessary as
there's just one case. Feel free to change that as well.
Otherwise ...
Acked-by: Daniel Borkmann <dborkman@redhat.com>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 6a2bb37506c5..7fd37de503d1 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -3786,7 +3786,7 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
> */
> if (!tx_ring)
> init_prb_bdqc(po, rb, pg_vec, req_u, tx_ring);
> - break;
> + break;
> default:
> break;
> }
>
^ permalink raw reply
* Re: [PATCH] sh_eth: call of_mdiobus_register() to register phys
From: Ben Dooks @ 2014-02-18 16:00 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: netdev, horms+renesas, linux-sh, magnus, linux-kernel
In-Reply-To: <53038C6A.1000600@cogentembedded.com>
On 18/02/14 16:38, Sergei Shtylyov wrote:
> Hello.
>
> On 02/17/2014 06:46 PM, Ben Dooks wrote:
>
>>>> If the sh_eth device is registered using OF, then the driver
>
>>> Which is not supported yet as my DT patch hasn't been merged.
>>> This patch seems somewhat premature.
>
>> I've got your OF patches in my local tree to test with, this
>> is what I found during that testing.
>
> The issue is that I didn't post my v3 patch to netdev due to
> net-next.git repo being closed at this moment and DaveM not wanting to
> see any patch targeted to it during this time. I've now posted v4 of my
> Ether DT patch to netdev.
Ok, I will look for these tomorrow.
>>>> should call of_mdiobus_register() to register any PHYs connected
>>>> to the system.
>
>>> That's not necessary (but good to have).
>
>> Well, it is necessary if you then want any PHYS bound to
>> the device to have their OF information to hand,
>
> Ether DT support worked for me without this fragment, at least.
Yes, it just that the PHY is not being linked to the relevant
OF node. The PHY gets bound, it will not be able to find the
DT info passed.
>>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>>>> ---
>>>> drivers/net/ethernet/renesas/sh_eth.c | 13 +++++++++++++
>>>> 1 file changed, 13 insertions(+)
>
>>>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c
>>>> b/drivers/net/ethernet/renesas/sh_eth.c
>>>> index 06970ac..165f0c4 100644
>>>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>>>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>>> [...]
>>>> @@ -2629,6 +2630,18 @@ static int sh_mdio_init(struct net_device
>>>> *ndev, int id,
>>>> snprintf(mdp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
>>>> mdp->pdev->name, id);
>>>>
>>>> + if (ndev->dev.parent->of_node) {
>>>> + dev_set_drvdata(&ndev->dev, mdp->mii_bus);
>>>> + ret = of_mdiobus_register(mdp->mii_bus,
>>>> + ndev->dev.parent->of_node);
>>>> + if (ret != 0) {
>>>> + dev_err(&ndev->dev, "of_mdiobus_register() failed\n");
>>>> + goto out_free_bus;
>>>> + }
>
>> I should probably only set the drvdata if the
>> of_mdiobus_register() succeeds.
>
> Yes. Probably should use *goto* as well since in that case the
> success path would be the same as the existing one.
I will look into that, not the biggest fan of gotos for
success cases.
>>>> + return 0;
>>>> + }
>>>> +
>>>> /* PHY IRQ */
>>>> mdp->mii_bus->irq = devm_kzalloc(&ndev->dev,
>>>> sizeof(int) * PHY_MAX_ADDR,
>
>>> Hm, I can only hope this works with PHY IRQ in DT mode.
>>> Would you mind if I include your patch into my Ether DT patch?
>
>> You are welcome to include it in your series, but I would like
>> to keep the credit for finding this.
>
> OK.
>
>> Also, FYI, for some reason the probe is not finding the correct
>> IRQ for this. I will have a look later when I get the board back
>> as to why this is:
>
>> net eth0: attached PHY 1 (IRQ -1) to driver Micrel KSZ8041RNLI
>
> Thanks for quickly fixing this. I've yet to study of_mdio.c code...
>
> WBR, Sergei
No problem,
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
^ permalink raw reply
* Re: [PATCH RFC] sctp: Update HEARTBEAT timer immediately after user changed HB.interval
From: Vlad Yasevich @ 2014-02-18 16:14 UTC (permalink / raw)
To: David Laight, Xufeng Zhang, nhorman@tuxdriver.com,
davem@davemloft.net
Cc: linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D0F6C2AEE@AcuExch.aculab.com>
On 02/18/2014 10:39 AM, David Laight wrote:
>>> +
>>> + /* Update the heartbeat timer immediately. */
>>> + if (!mod_timer(&trans->hb_timer,
>>> + sctp_transport_timeout(trans)))
>>> + sctp_transport_hold(trans);
>>
>> This is causes a rather inconsistent behavior in that it doesn't
>> account of the amount of time left on the hb timer. Thus it doesn't
>> always do what commit log implies. If the remaining time is shorter
>> then the new timeout value, it will take longer till the next HB
>> the application expects.
>
> Being able to stop heartbeats being sent by repeatedly configuring
> the interval is certainly not a good idea!
>
>> If the application has very strict timing requirement on the HBs, it
>> should trigger the HB manually.
>>
>> We could rework the code to allow the combination of
>> SPP_HB_DEMAND | SPP_HB_ENABLE to work as follows:
>> 1) update the timer to the new value
>> 2) Send an immediate HB
>> a) Update the hb timeout.
>>
>> 2a should probably be done regardless since it can cause 2 HB to be
>> outstanding at the same time.
>
> Sending a heartbeat 'on demand' might be problematic if one
> has also just been sent (from the timer).
Yes, you are right. This will trigger an rto doubling since we'll
be in a state where we have an outstanding HB without an ack.
>
> I'd have thought that you wouldn't want to send a heartbeat while
> still expecting a response from the previous one (this might require
> splitting the time interval in two).
This one is interesting. Technically, a user can request an immediate
HB any time, and if the user requests it, we should honor the request.
Also, the user may not know that there is an outstanding HB. I'll need
to think about this a bit.
Thanks for bringing this up.
-vlad
>
> David
>
>
>
^ permalink raw reply
* Re: [PATCH v2] of_mdio: fix phy interrupt passing
From: Grant Likely @ 2014-02-18 16:15 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-kernel, devicetree, linux-kernel, netdev, linux-sh,
sergei.shtylyov, Ben Dooks
In-Reply-To: <1392725818-558-1-git-send-email-ben.dooks@codethink.co.uk>
On Tue, 18 Feb 2014 12:16:58 +0000, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> The of_mdiobus_register_phy() is not setting phy->irq thus causing
> some drivers to incorrectly assume that the PHY does not have an
> IRQ associated with it. Not only do some drivers report no IRQ
> they do not install an interrupt handler for the PHY.
>
> Simplify the code setting irq and set the phy->irq at the same
> time so that we cover the following issues, which should cover
> all the cases the code will find:
>
> - Set phy->irq if node has irq property and mdio->irq is NULL
> - Set phy->irq if node has no irq and mdio->irq is not NULL
> - Leave phy->irq as PHY_POLL default if none of the above
>
> This fixes the issue:
> net eth0: attached PHY 1 (IRQ -1) to driver Micrel KSZ8041RNLI
>
> to the correct:
> net eth0: attached PHY 1 (IRQ 416) to driver Micrel KSZ8041RNLI
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Looks okay to me
Reviewed-by: Grant Likely <grant.likely@linaro.org>
>
> ---
> Since v1:
> - Updated phy->irq setting code
> - Deal with issue if mdio->irq array NULL
>
> Notes:
>
> It was discussed if this should be two patches, but we end up
> making enough changes in #2 the same area as #1 means that we
> basically rewrite #1 in #2. So keep it as 1 patch.
> ---
> drivers/of/of_mdio.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 875b7b6..46d95fc 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -44,7 +44,7 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
> {
> struct phy_device *phy;
> bool is_c45;
> - int rc, prev_irq;
> + int rc;
> u32 max_speed = 0;
>
> is_c45 = of_device_is_compatible(child,
> @@ -54,12 +54,14 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
> if (!phy || IS_ERR(phy))
> return 1;
>
> - if (mdio->irq) {
> - prev_irq = mdio->irq[addr];
> - mdio->irq[addr] =
> - irq_of_parse_and_map(child, 0);
> - if (!mdio->irq[addr])
> - mdio->irq[addr] = prev_irq;
> + rc = irq_of_parse_and_map(child, 0);
> + if (rc > 0) {
> + phy->irq = rc;
> + if (mdio->irq)
> + mdio->irq[addr] = rc;
> + } else {
> + if (mdio->irq)
> + phy->irq = mdio->irq[addr];
> }
>
> /* Associate the OF node with the device structure so it
> --
> 1.8.5.3
>
^ permalink raw reply
* Re: [PATCH] sh_eth: call of_mdiobus_register() to register phys
From: Sergei Shtylyov @ 2014-02-18 16:38 UTC (permalink / raw)
To: Ben Dooks; +Cc: netdev, horms+renesas, linux-sh, magnus, linux-kernel
In-Reply-To: <53022EE4.50607@codethink.co.uk>
Hello.
On 02/17/2014 06:46 PM, Ben Dooks wrote:
>>> If the sh_eth device is registered using OF, then the driver
>> Which is not supported yet as my DT patch hasn't been merged.
>> This patch seems somewhat premature.
> I've got your OF patches in my local tree to test with, this
> is what I found during that testing.
The issue is that I didn't post my v3 patch to netdev due to net-next.git
repo being closed at this moment and DaveM not wanting to see any patch
targeted to it during this time. I've now posted v4 of my Ether DT patch to
netdev.
>>> should call of_mdiobus_register() to register any PHYs connected
>>> to the system.
>> That's not necessary (but good to have).
> Well, it is necessary if you then want any PHYS bound to
> the device to have their OF information to hand,
Ether DT support worked for me without this fragment, at least.
>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>>> ---
>>> drivers/net/ethernet/renesas/sh_eth.c | 13 +++++++++++++
>>> 1 file changed, 13 insertions(+)
>>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c
>>> b/drivers/net/ethernet/renesas/sh_eth.c
>>> index 06970ac..165f0c4 100644
>>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>> [...]
>>> @@ -2629,6 +2630,18 @@ static int sh_mdio_init(struct net_device
>>> *ndev, int id,
>>> snprintf(mdp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
>>> mdp->pdev->name, id);
>>>
>>> + if (ndev->dev.parent->of_node) {
>>> + dev_set_drvdata(&ndev->dev, mdp->mii_bus);
>>> + ret = of_mdiobus_register(mdp->mii_bus,
>>> + ndev->dev.parent->of_node);
>>> + if (ret != 0) {
>>> + dev_err(&ndev->dev, "of_mdiobus_register() failed\n");
>>> + goto out_free_bus;
>>> + }
> I should probably only set the drvdata if the
> of_mdiobus_register() succeeds.
Yes. Probably should use *goto* as well since in that case the success
path would be the same as the existing one.
>>> + return 0;
>>> + }
>>> +
>>> /* PHY IRQ */
>>> mdp->mii_bus->irq = devm_kzalloc(&ndev->dev,
>>> sizeof(int) * PHY_MAX_ADDR,
>> Hm, I can only hope this works with PHY IRQ in DT mode.
>> Would you mind if I include your patch into my Ether DT patch?
> You are welcome to include it in your series, but I would like
> to keep the credit for finding this.
OK.
> Also, FYI, for some reason the probe is not finding the correct
> IRQ for this. I will have a look later when I get the board back
> as to why this is:
> net eth0: attached PHY 1 (IRQ -1) to driver Micrel KSZ8041RNLI
Thanks for quickly fixing this. I've yet to study of_mdio.c code...
WBR, Sergei
^ permalink raw reply
* Re: [PATCH] net: add init-regs for of_phy support
From: Sergei Shtylyov @ 2014-02-18 17:00 UTC (permalink / raw)
To: Mark Rutland, Ben Dooks
Cc: Florian Fainelli, netdev, devicetree@vger.kernel.org,
Linux-sh list, David Miller
In-Reply-To: <20140218115443.GC6051@e106331-lin.cambridge.arm.com>
Hello.
On 02/18/2014 02:54 PM, Mark Rutland wrote:
>> [snip]
>>>>> - fixing up some design mistake?
>>>>> - accounting for a specific board design?
>>>> Kind of both. This was invented to defy the necessity of having platform
>>>> fixup in the DT case (where there should be no board file to place it into).
>>>> I have already described that platform fixup necessary on the Renesas
>>>> Lager/Koelsch boards where the LED0 signat is connected to ETH_LINK signal
>>>> on the SoC and the PHY reset sets the LED control bits to default 0 which
>>>> means that LED0 will be LINK/ACTIVITY signal and thus blink on activity and
>>>> cause ETH_LINK to bounce off/on after each packet.
>>>>> In any case a PHY fixup would do the job for you.
>>>> Not in any case. In case of DT we have no place for it, so should invent
>>>> something involving DT.
>>> How is DT different than any machine probing mechanism here? The way
>>> to involve DT is to do the following:
>>> if (of_machine_is_compatible("renesas,foo-board-with-broken-micrel-phy"))
>>> phy_register_fixup(&foo_board_with_broken_micrel_phy);
>> Oh yes, but now I have to do that for Linux, for $BSD, and for
>> anything else I want to run on the device. I thought dt was meant
>> to allow us to describe the hardware.
> It does allow you to describe the hardware. Arbitrary register writes
> aren't a description of the hardware, they're a sequence of instructions
> that tells the OS nothing about the hardware and limit the ability of an
> OS to do something different that might be better.
> It's already the case that the OS has to have some knowledge of the
> hardware that's implicit in a binding. We don't expect to have to
> include bytecode to tell the OS how to poke a particular UART when it
> can figure that out from a compatible string.
>> If this is the case, let's just call this linuxtree and let everyone
>> else get on with their own thing again.
> This doesn't follow at all. Any OS needs to have some understanding of
> the hardware it will try to poke. Describing a specific sequence of
> writes in a DT is no more operating system independent than identifying
> the hardware and expecting the OS to have a driver for it. The
> requirements aren't any more suited to an individual OS in either case.
>> See also comment below.
>>> If your machine compatible string does not allow you to uniquely
>>> identify your machine, this is a DT problem, as this should really be
>>> the case. If you do not want to add this code to wherever this is
>>> relevant in arch/arm/mach-shmobile/board-*.c, neither is
>>> drivers/net/phy/phy_device.c this the place to add it.
>> So where should it be added? If we keep piling stuff into board files
>> in arch/arm.... then we're just back to the pre-dt case and going to
>> keep getting shouted at.
> The general trend has been to allocate new compatible strings for
> components and let individual drivers handle this.
> As far as I can see your case doesn't involve any components external to
> the PHY, so should probably live in a PHY driver. The PHY can have a
It does involve LEDs which should function in the way described by their
labels, and it does involve SoC for which ETH_LINK signal should remain stable
and not bouncing after each packet.
> specific compatible string with the generic string as a fallback (if it
> works to some degree without special poking).
It can but that doesn't solve this issue in any way. The issue is board
specific, not only PHY specific.
> I don't see that we need anything board-specific.
Did you read the text substantially above this in this mail for more
complete description of the issue? We're trying to emulate the PHY *platform*
fixup here which didn't belong with the PHY driver.
[...]
> Cheers,
> Mark.
WBR, Sergei
^ permalink raw reply
* Re: [PATCH] of_mdio: fix phy interrupt passing
From: Grant Likely @ 2014-02-18 17:02 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-kernel, devicetree, linux-kernel, netdev, linux-sh,
sergei.shtylyov
In-Reply-To: <53032A97.8050201@codethink.co.uk>
On Tue, 18 Feb 2014 09:40:39 +0000, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> On 18/02/14 09:30, Grant Likely wrote:
> > On Mon, 17 Feb 2014 16:29:40 +0000, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> >> The of_mdiobus_register_phy() is not setting phy->irq this causing
> >> some drivers to incorrectly assume that the PHY does not have an
> >> IRQ associated with it or install an interrupt handler for the
> >> PHY.
> >>
> >> Simplify the code setting irq and set the phy->irq at the same
> >> time so that the case if mdio->irq is not NULL is easier to read.
> >>
> >> This fixes the issue:
> >> net eth0: attached PHY 1 (IRQ -1) to driver Micrel KSZ8041RNLI
> >>
> >> to the correct:
> >> net eth0: attached PHY 1 (IRQ 416) to driver Micrel KSZ8041RNLI
> >>
> >> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> >> ---
> >> drivers/of/of_mdio.c | 12 ++++++------
> >> 1 file changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> >> index 875b7b6..7b3e7b0 100644
> >> --- a/drivers/of/of_mdio.c
> >> +++ b/drivers/of/of_mdio.c
> >> @@ -44,7 +44,7 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
> >> {
> >> struct phy_device *phy;
> >> bool is_c45;
> >> - int rc, prev_irq;
> >> + int rc;
> >> u32 max_speed = 0;
> >>
> >> is_c45 = of_device_is_compatible(child,
> >> @@ -55,11 +55,11 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
> >> return 1;
> >>
> >> if (mdio->irq) {
> >> - prev_irq = mdio->irq[addr];
> >> - mdio->irq[addr] =
> >> - irq_of_parse_and_map(child, 0);
> >> - if (!mdio->irq[addr])
> >> - mdio->irq[addr] = prev_irq;
> >
> > I cannot for the life for me remeber why the code was structured that
> > way. Your change is better.
> >
> >> + rc = irq_of_parse_and_map(child, 0);
> >> + if (rc > 0) {
> >> + mdio->irq[addr] = rc;
> >> + phy->irq = rc;
> >> + }
> >> }
> >
> > The outer if is merely protecting against no irq array being allocated
> > for the bus. Would not the following be better:
> >
> > rc = irq_of_parse_and_map(child, 0);
> > if (rc > 0) {
> > phy->irq = rc;
> > if (mdio->irq)
> > mdio->irq[addr] = rc;
> > }
>
> Thanks, that makes sense, although we've both failed to work
> out if mdio->irq is set, and rc <= 0 case, so:
>
> rc = irq_of_parse_and_map(child, 0);
> if (rc > 0) {
> phy->irq = rc;
> if (mdio->irq)
> mdio->irq[addr] = rc;
> } else {
> if (mdio->irq)
> phy->irq = mdio->irq[addr];
> }
Is this actually a valid thing to do? I think the only time mdio->irq[]
is non-zero is when it is set to PHY_POLL. Is it valid to set phy->irq
to PHY_POLL? I didn't think it was.
g.
>
> I think that covers all cases. This does rely on mdio->irq
> being initialised to PHY_POLL if allocated.
>
> Once this is in, it may be easier then to not allocate
> mdio->irq for the OF case by default unless the driver
> registering the PHY knows it has the interrupt number(s)
> for the PHY already.
>
> --
> Ben Dooks http://www.codethink.co.uk/
> Senior Engineer Codethink - Providing Genius
^ permalink raw reply
* Re: [PATCH net-next v5 1/9] xen-netback: Introduce TX grant map definitions
From: Ian Campbell @ 2014-02-18 17:06 UTC (permalink / raw)
To: Zoltan Kiss; +Cc: wei.liu2, xen-devel, netdev, linux-kernel, jonathan.davies
In-Reply-To: <1390253069-25507-2-git-send-email-zoltan.kiss@citrix.com>
On Mon, 2014-01-20 at 21:24 +0000, Zoltan Kiss wrote:
> This patch contains the new definitions necessary for grant mapping.
Is this just adding a bunch of (currently) unused functions? That's a
slightly odd way to structure a series. They don't seem to be "generic
helpers" or anything so it would be more normal to introduce these as
they get used -- it's a bit hard to review them out of context.
> v2:
This sort of intraversion changelog should go after the S-o-b and a
"---" marker. This way they are not included in the final commit
message.
[...]
> Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
v2: Blah blah
v3: Etc etc
> @@ -226,6 +248,12 @@ bool xenvif_rx_ring_slots_available(struct xenvif *vif, int needed);
>
> void xenvif_stop_queue(struct xenvif *vif);
>
> +/* Callback from stack when TX packet can be released */
> +void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success);
> +
> +/* Unmap a pending page, usually has to be called before xenvif_idx_release */
"usually" or always? How does one determine when it is or isn't
appropriate to call it later?
> +void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx);
> +
> extern bool separate_tx_rx_irq;
>
> #endif /* __XEN_NETBACK__COMMON_H__ */
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> index 7669d49..f0f0c3d 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -38,6 +38,7 @@
>
> #include <xen/events.h>
> #include <asm/xen/hypercall.h>
> +#include <xen/balloon.h>
What is this for?
> #define XENVIF_QUEUE_LENGTH 32
> #define XENVIF_NAPI_WEIGHT 64
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index bb241d0..195602f 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -773,6 +773,20 @@ static struct page *xenvif_alloc_page(struct xenvif *vif,
> return page;
> }
>
> +static inline void xenvif_tx_create_gop(struct xenvif *vif,
> + u16 pending_idx,
> + struct xen_netif_tx_request *txp,
> + struct gnttab_map_grant_ref *gop)
> +{
> + vif->pages_to_map[gop-vif->tx_map_ops] = vif->mmap_pages[pending_idx];
> + gnttab_set_map_op(gop, idx_to_kaddr(vif, pending_idx),
> + GNTMAP_host_map | GNTMAP_readonly,
> + txp->gref, vif->domid);
> +
> + memcpy(&vif->pending_tx_info[pending_idx].req, txp,
> + sizeof(*txp));
Can this not go in xenvif_tx_build_gops? Or conversely should the
non-mapping code there be factored out?
Given the presence of both kinds of gop the name of this function needs
to be more specific I think.
> +}
> +
> static struct gnttab_copy *xenvif_get_requests(struct xenvif *vif,
> struct sk_buff *skb,
> struct xen_netif_tx_request *txp,
> @@ -1612,6 +1626,107 @@ static int xenvif_tx_submit(struct xenvif *vif)
> return work_done;
> }
>
> +void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success)
> +{
> + unsigned long flags;
> + pending_ring_idx_t index;
> + u16 pending_idx = ubuf->desc;
> + struct pending_tx_info *temp =
> + container_of(ubuf, struct pending_tx_info, callback_struct);
> + struct xenvif *vif = container_of(temp - pending_idx,
This is subtracting a u16 from a pointer?
> + struct xenvif,
> + pending_tx_info[0]);
> +
> + spin_lock_irqsave(&vif->dealloc_lock, flags);
> + do {
> + pending_idx = ubuf->desc;
> + ubuf = (struct ubuf_info *) ubuf->ctx;
> + index = pending_index(vif->dealloc_prod);
> + vif->dealloc_ring[index] = pending_idx;
> + /* Sync with xenvif_tx_dealloc_action:
> + * insert idx then incr producer.
> + */
> + smp_wmb();
Is this really needed given that there is a lock held?
Or what is dealloc_lock protecting against?
> + vif->dealloc_prod++;
What happens if the dealloc ring becomes full, will this wrap and cause
havoc?
> + } while (ubuf);
> + wake_up(&vif->dealloc_wq);
> + spin_unlock_irqrestore(&vif->dealloc_lock, flags);
> +}
> +
> +static inline void xenvif_tx_dealloc_action(struct xenvif *vif)
> +{
> + struct gnttab_unmap_grant_ref *gop;
> + pending_ring_idx_t dc, dp;
> + u16 pending_idx, pending_idx_release[MAX_PENDING_REQS];
> + unsigned int i = 0;
> +
> + dc = vif->dealloc_cons;
> + gop = vif->tx_unmap_ops;
> +
> + /* Free up any grants we have finished using */
> + do {
> + dp = vif->dealloc_prod;
> +
> + /* Ensure we see all indices enqueued by all
> + * xenvif_zerocopy_callback().
> + */
> + smp_rmb();
> +
> + while (dc != dp) {
> + pending_idx =
> + vif->dealloc_ring[pending_index(dc++)];
> +
> + /* Already unmapped? */
> + if (vif->grant_tx_handle[pending_idx] ==
> + NETBACK_INVALID_HANDLE) {
> + netdev_err(vif->dev,
> + "Trying to unmap invalid handle! "
> + "pending_idx: %x\n", pending_idx);
> + BUG();
> + }
> +
> + pending_idx_release[gop-vif->tx_unmap_ops] =
> + pending_idx;
> + vif->pages_to_unmap[gop-vif->tx_unmap_ops] =
> + vif->mmap_pages[pending_idx];
> + gnttab_set_unmap_op(gop,
> + idx_to_kaddr(vif, pending_idx),
> + GNTMAP_host_map,
> + vif->grant_tx_handle[pending_idx]);
> + vif->grant_tx_handle[pending_idx] =
> + NETBACK_INVALID_HANDLE;
> + ++gop;
Can we run out of space in the gop array?
> + }
> +
> + } while (dp != vif->dealloc_prod);
> +
> + vif->dealloc_cons = dc;
No barrier here?
> + if (gop - vif->tx_unmap_ops > 0) {
> + int ret;
> + ret = gnttab_unmap_refs(vif->tx_unmap_ops,
> + vif->pages_to_unmap,
> + gop - vif->tx_unmap_ops);
> + if (ret) {
> + netdev_err(vif->dev, "Unmap fail: nr_ops %x ret %d\n",
> + gop - vif->tx_unmap_ops, ret);
> + for (i = 0; i < gop - vif->tx_unmap_ops; ++i) {
This seems liable to be a lot of spew on failure. Perhaps only log the
ones where gop[i].status != success.
Have you considered whether or not the frontend can force this error to
occur?
> + netdev_err(vif->dev,
> + " host_addr: %llx handle: %x status: %d\n",
> + gop[i].host_addr,
> + gop[i].handle,
> + gop[i].status);
> + }
> + BUG();
> + }
> + }
> +
> + for (i = 0; i < gop - vif->tx_unmap_ops; ++i)
> + xenvif_idx_release(vif, pending_idx_release[i],
> + XEN_NETIF_RSP_OKAY);
> +}
> +
> +
> /* Called after netfront has transmitted */
> int xenvif_tx_action(struct xenvif *vif, int budget)
> {
> @@ -1678,6 +1793,25 @@ static void xenvif_idx_release(struct xenvif *vif, u16 pending_idx,
> vif->mmap_pages[pending_idx] = NULL;
> }
>
> +void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx)
This is a single shot version of the batched xenvif_tx_dealloc_action
version? Why not just enqueue the idx to be unmapped later?
> +{
> + int ret;
> + struct gnttab_unmap_grant_ref tx_unmap_op;
> +
> + if (vif->grant_tx_handle[pending_idx] == NETBACK_INVALID_HANDLE) {
> + netdev_err(vif->dev,
> + "Trying to unmap invalid handle! pending_idx: %x\n",
> + pending_idx);
> + BUG();
> + }
> + gnttab_set_unmap_op(&tx_unmap_op,
> + idx_to_kaddr(vif, pending_idx),
> + GNTMAP_host_map,
> + vif->grant_tx_handle[pending_idx]);
> + ret = gnttab_unmap_refs(&tx_unmap_op, &vif->mmap_pages[pending_idx], 1);
> + BUG_ON(ret);
> + vif->grant_tx_handle[pending_idx] = NETBACK_INVALID_HANDLE;
> +}
>
> static void make_tx_response(struct xenvif *vif,
> struct xen_netif_tx_request *txp,
> @@ -1740,6 +1874,11 @@ static inline int tx_work_todo(struct xenvif *vif)
> return 0;
> }
>
> +static inline bool tx_dealloc_work_todo(struct xenvif *vif)
> +{
> + return vif->dealloc_cons != vif->dealloc_prod
> +}
> +
> void xenvif_unmap_frontend_rings(struct xenvif *vif)
> {
> if (vif->tx.sring)
> @@ -1826,6 +1965,28 @@ int xenvif_kthread(void *data)
> return 0;
> }
>
> +int xenvif_dealloc_kthread(void *data)
Is this going to be a thread per vif?
> +{
> + struct xenvif *vif = data;
> +
> + while (!kthread_should_stop()) {
> + wait_event_interruptible(vif->dealloc_wq,
> + tx_dealloc_work_todo(vif) ||
> + kthread_should_stop());
> + if (kthread_should_stop())
> + break;
> +
> + xenvif_tx_dealloc_action(vif);
> + cond_resched();
> + }
> +
> + /* Unmap anything remaining*/
> + if (tx_dealloc_work_todo(vif))
> + xenvif_tx_dealloc_action(vif);
> +
> + return 0;
> +}
> +
> static int __init netback_init(void)
> {
> int rc = 0;
^ permalink raw reply
* Re: [PATCH] net: add init-regs for of_phy support
From: Mark Rutland @ 2014-02-18 17:13 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Ben Dooks, Florian Fainelli, netdev, devicetree@vger.kernel.org,
Linux-sh list, David Miller
In-Reply-To: <53039193.4010500@cogentembedded.com>
On Tue, Feb 18, 2014 at 05:00:03PM +0000, Sergei Shtylyov wrote:
> Hello.
>
> On 02/18/2014 02:54 PM, Mark Rutland wrote:
>
> >> [snip]
>
> >>>>> - fixing up some design mistake?
> >>>>> - accounting for a specific board design?
>
> >>>> Kind of both. This was invented to defy the necessity of having platform
> >>>> fixup in the DT case (where there should be no board file to place it into).
> >>>> I have already described that platform fixup necessary on the Renesas
> >>>> Lager/Koelsch boards where the LED0 signat is connected to ETH_LINK signal
> >>>> on the SoC and the PHY reset sets the LED control bits to default 0 which
> >>>> means that LED0 will be LINK/ACTIVITY signal and thus blink on activity and
> >>>> cause ETH_LINK to bounce off/on after each packet.
>
> >>>>> In any case a PHY fixup would do the job for you.
>
> >>>> Not in any case. In case of DT we have no place for it, so should invent
> >>>> something involving DT.
>
> >>> How is DT different than any machine probing mechanism here? The way
> >>> to involve DT is to do the following:
>
> >>> if (of_machine_is_compatible("renesas,foo-board-with-broken-micrel-phy"))
> >>> phy_register_fixup(&foo_board_with_broken_micrel_phy);
>
> >> Oh yes, but now I have to do that for Linux, for $BSD, and for
> >> anything else I want to run on the device. I thought dt was meant
> >> to allow us to describe the hardware.
>
> > It does allow you to describe the hardware. Arbitrary register writes
> > aren't a description of the hardware, they're a sequence of instructions
> > that tells the OS nothing about the hardware and limit the ability of an
> > OS to do something different that might be better.
>
> > It's already the case that the OS has to have some knowledge of the
> > hardware that's implicit in a binding. We don't expect to have to
> > include bytecode to tell the OS how to poke a particular UART when it
> > can figure that out from a compatible string.
>
> >> If this is the case, let's just call this linuxtree and let everyone
> >> else get on with their own thing again.
>
> > This doesn't follow at all. Any OS needs to have some understanding of
> > the hardware it will try to poke. Describing a specific sequence of
> > writes in a DT is no more operating system independent than identifying
> > the hardware and expecting the OS to have a driver for it. The
> > requirements aren't any more suited to an individual OS in either case.
>
> >> See also comment below.
>
> >>> If your machine compatible string does not allow you to uniquely
> >>> identify your machine, this is a DT problem, as this should really be
> >>> the case. If you do not want to add this code to wherever this is
> >>> relevant in arch/arm/mach-shmobile/board-*.c, neither is
> >>> drivers/net/phy/phy_device.c this the place to add it.
>
> >> So where should it be added? If we keep piling stuff into board files
> >> in arch/arm.... then we're just back to the pre-dt case and going to
> >> keep getting shouted at.
>
> > The general trend has been to allocate new compatible strings for
> > components and let individual drivers handle this.
>
> > As far as I can see your case doesn't involve any components external to
> > the PHY, so should probably live in a PHY driver. The PHY can have a
>
> It does involve LEDs which should function in the way described by their
> labels, and it does involve SoC for which ETH_LINK signal should remain stable
> and not bouncing after each packet.
Ah, I see I misunderstood.
>
> > specific compatible string with the generic string as a fallback (if it
> > works to some degree without special poking).
>
> It can but that doesn't solve this issue in any way. The issue is board
> specific, not only PHY specific.
Sure. So additional properties are required.
>
> > I don't see that we need anything board-specific.
>
> Did you read the text substantially above this in this mail for more
> complete description of the issue? We're trying to emulate the PHY *platform*
> fixup here which didn't belong with the PHY driver.
Apologies, I was indeed mistaken.
Cheers,
Mark.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox