* [PATCH net-next v1 2/5] hinic: add support to set and get irq coalesce
From: Luo bin @ 2020-06-20 9:42 UTC (permalink / raw)
To: davem; +Cc: linux-kernel, netdev, luoxianjun, yin.yinshi, cloud.wangxiaoyun
In-Reply-To: <20200620094258.13181-1-luobin9@huawei.com>
add support to set TX/RX irq coalesce params with ethtool -C and
get these params with ethtool -c.
Signed-off-by: Luo bin <luobin9@huawei.com>
---
drivers/net/ethernet/huawei/hinic/hinic_dev.h | 8 +
.../net/ethernet/huawei/hinic/hinic_ethtool.c | 294 ++++++++++++++++++
.../net/ethernet/huawei/hinic/hinic_hw_dev.c | 62 ++++
.../net/ethernet/huawei/hinic/hinic_hw_dev.h | 21 ++
.../net/ethernet/huawei/hinic/hinic_hw_mgmt.h | 3 +
.../net/ethernet/huawei/hinic/hinic_main.c | 56 ++++
drivers/net/ethernet/huawei/hinic/hinic_rx.c | 19 +-
drivers/net/ethernet/huawei/hinic/hinic_tx.c | 19 ++
8 files changed, 481 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_dev.h b/drivers/net/ethernet/huawei/hinic/hinic_dev.h
index 48b40be3e84d..75d6dee948f5 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_dev.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_dev.h
@@ -49,6 +49,12 @@ enum hinic_rss_hash_type {
HINIC_RSS_HASH_ENGINE_TYPE_MAX,
};
+struct hinic_intr_coal_info {
+ u8 pending_limt;
+ u8 coalesce_timer_cfg;
+ u8 resend_timer_cfg;
+};
+
struct hinic_dev {
struct net_device *netdev;
struct hinic_hwdev *hwdev;
@@ -82,6 +88,8 @@ struct hinic_dev {
struct hinic_rss_type rss_type;
u8 *rss_hkey_user;
s32 *rss_indir_user;
+ struct hinic_intr_coal_info *rx_intr_coalesce;
+ struct hinic_intr_coal_info *tx_intr_coalesce;
struct hinic_sriov_info sriov_info;
};
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
index c2afa0585dbf..750b4d786c50 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
@@ -49,6 +49,30 @@
#define ETHTOOL_ADD_ADVERTISED_LINK_MODE(ecmd, mode) \
((ecmd)->advertising |= ADVERTISED_##mode)
+#define COALESCE_PENDING_LIMIT_UNIT 8
+#define COALESCE_TIMER_CFG_UNIT 9
+#define COALESCE_ALL_QUEUE 0xFFFF
+#define COALESCE_MAX_PENDING_LIMIT (255 * COALESCE_PENDING_LIMIT_UNIT)
+#define COALESCE_MAX_TIMER_CFG (255 * COALESCE_TIMER_CFG_UNIT)
+#define OBJ_STR_MAX_LEN 32
+
+#define CHECK_COALESCE_ALIGN(coal, item, unit) \
+do { \
+ if ((coal)->item % (unit)) \
+ netif_warn(nic_dev, drv, netdev, \
+ "%s in %d units, change to %d\n", \
+ #item, (unit), (((coal)->item / (unit)) * (unit)));\
+} while (0)
+
+#define CHECK_COALESCE_CHANGED(coal, item, unit, ori_val, obj_str) \
+do { \
+ if (((coal)->item / (unit)) != (ori_val)) \
+ netif_info(nic_dev, drv, netdev, \
+ "Change %s from %d to %d %s\n", \
+ #item, (ori_val) * (unit), \
+ (((coal)->item / (unit)) * (unit)), (obj_str));\
+} while (0)
+
struct hw2ethtool_link_mode {
enum ethtool_link_mode_bit_indices link_mode_bit;
u32 speed;
@@ -614,6 +638,258 @@ static int hinic_set_ringparam(struct net_device *netdev,
return 0;
}
+static int __hinic_get_coalesce(struct net_device *netdev,
+ struct ethtool_coalesce *coal, u16 queue)
+{
+ struct hinic_dev *nic_dev = netdev_priv(netdev);
+ struct hinic_intr_coal_info *rx_intr_coal_info;
+ struct hinic_intr_coal_info *tx_intr_coal_info;
+
+ if (queue == COALESCE_ALL_QUEUE) {
+ /* get tx/rx irq0 as default parameters */
+ rx_intr_coal_info = &nic_dev->rx_intr_coalesce[0];
+ tx_intr_coal_info = &nic_dev->tx_intr_coalesce[0];
+ } else {
+ if (queue >= nic_dev->num_qps) {
+ netif_err(nic_dev, drv, netdev,
+ "Invalid queue_id: %d\n", queue);
+ return -EINVAL;
+ }
+ rx_intr_coal_info = &nic_dev->rx_intr_coalesce[queue];
+ tx_intr_coal_info = &nic_dev->tx_intr_coalesce[queue];
+ }
+
+ /* coalesce_timer is in unit of 9us */
+ coal->rx_coalesce_usecs = rx_intr_coal_info->coalesce_timer_cfg *
+ COALESCE_TIMER_CFG_UNIT;
+ /* coalesced_frames is in unit of 8 */
+ coal->rx_max_coalesced_frames = rx_intr_coal_info->pending_limt *
+ COALESCE_PENDING_LIMIT_UNIT;
+ coal->tx_coalesce_usecs = tx_intr_coal_info->coalesce_timer_cfg *
+ COALESCE_TIMER_CFG_UNIT;
+ coal->tx_max_coalesced_frames = tx_intr_coal_info->pending_limt *
+ COALESCE_PENDING_LIMIT_UNIT;
+
+ return 0;
+}
+
+static int is_coalesce_exceed_limit(struct net_device *netdev,
+ const struct ethtool_coalesce *coal)
+{
+ struct hinic_dev *nic_dev = netdev_priv(netdev);
+
+ if (coal->rx_coalesce_usecs > COALESCE_MAX_TIMER_CFG) {
+ netif_err(nic_dev, drv, netdev,
+ "Rx_coalesce_usecs out of range[%d-%d]\n", 0,
+ COALESCE_MAX_TIMER_CFG);
+ return -EOPNOTSUPP;
+ }
+
+ if (coal->rx_max_coalesced_frames > COALESCE_MAX_PENDING_LIMIT) {
+ netif_err(nic_dev, drv, netdev,
+ "Rx_max_coalesced_frames out of range[%d-%d]\n", 0,
+ COALESCE_MAX_PENDING_LIMIT);
+ return -EOPNOTSUPP;
+ }
+
+ if (coal->tx_coalesce_usecs > COALESCE_MAX_TIMER_CFG) {
+ netif_err(nic_dev, drv, netdev,
+ "Tx_coalesce_usecs out of range[%d-%d]\n", 0,
+ COALESCE_MAX_TIMER_CFG);
+ return -EOPNOTSUPP;
+ }
+
+ if (coal->tx_max_coalesced_frames > COALESCE_MAX_PENDING_LIMIT) {
+ netif_err(nic_dev, drv, netdev,
+ "Tx_max_coalesced_frames out of range[%d-%d]\n", 0,
+ COALESCE_MAX_PENDING_LIMIT);
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+static int set_queue_coalesce(struct hinic_dev *nic_dev, u16 q_id,
+ struct hinic_intr_coal_info *coal,
+ bool set_rx_coal)
+{
+ struct hinic_intr_coal_info *intr_coal = NULL;
+ struct hinic_msix_config interrupt_info = {0};
+ struct net_device *netdev = nic_dev->netdev;
+ u16 msix_idx;
+ int err;
+
+ intr_coal = set_rx_coal ? &nic_dev->rx_intr_coalesce[q_id] :
+ &nic_dev->tx_intr_coalesce[q_id];
+
+ intr_coal->coalesce_timer_cfg = coal->coalesce_timer_cfg;
+ intr_coal->pending_limt = coal->pending_limt;
+
+ /* netdev not running or qp not in using,
+ * don't need to set coalesce to hw
+ */
+ if (!(nic_dev->flags & HINIC_INTF_UP) ||
+ q_id >= nic_dev->num_qps)
+ return 0;
+
+ msix_idx = set_rx_coal ? nic_dev->rxqs[q_id].rq->msix_entry :
+ nic_dev->txqs[q_id].sq->msix_entry;
+ interrupt_info.msix_index = msix_idx;
+ interrupt_info.coalesce_timer_cnt = intr_coal->coalesce_timer_cfg;
+ interrupt_info.pending_cnt = intr_coal->pending_limt;
+ interrupt_info.resend_timer_cnt = intr_coal->resend_timer_cfg;
+
+ err = hinic_set_interrupt_cfg(nic_dev->hwdev, &interrupt_info);
+ if (err)
+ netif_warn(nic_dev, drv, netdev,
+ "Failed to set %s queue%d coalesce",
+ set_rx_coal ? "rx" : "tx", q_id);
+
+ return err;
+}
+
+static int __set_hw_coal_param(struct hinic_dev *nic_dev,
+ struct hinic_intr_coal_info *intr_coal,
+ u16 queue, bool set_rx_coal)
+{
+ int err;
+ u16 i;
+
+ if (queue == COALESCE_ALL_QUEUE) {
+ for (i = 0; i < nic_dev->max_qps; i++) {
+ err = set_queue_coalesce(nic_dev, i, intr_coal,
+ set_rx_coal);
+ if (err)
+ return err;
+ }
+ } else {
+ if (queue >= nic_dev->num_qps) {
+ netif_err(nic_dev, drv, nic_dev->netdev,
+ "Invalid queue_id: %d\n", queue);
+ return -EINVAL;
+ }
+ err = set_queue_coalesce(nic_dev, queue, intr_coal,
+ set_rx_coal);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+static int __hinic_set_coalesce(struct net_device *netdev,
+ struct ethtool_coalesce *coal, u16 queue)
+{
+ struct hinic_intr_coal_info *ori_rx_intr_coal = NULL;
+ struct hinic_intr_coal_info *ori_tx_intr_coal = NULL;
+ struct hinic_dev *nic_dev = netdev_priv(netdev);
+ struct hinic_intr_coal_info rx_intr_coal = {0};
+ struct hinic_intr_coal_info tx_intr_coal = {0};
+ char obj_str[OBJ_STR_MAX_LEN] = {0};
+ bool set_rx_coal = false;
+ bool set_tx_coal = false;
+ int err;
+
+ err = is_coalesce_exceed_limit(netdev, coal);
+ if (err)
+ return err;
+
+ CHECK_COALESCE_ALIGN(coal, rx_coalesce_usecs, COALESCE_TIMER_CFG_UNIT);
+ CHECK_COALESCE_ALIGN(coal, rx_max_coalesced_frames,
+ COALESCE_PENDING_LIMIT_UNIT);
+ CHECK_COALESCE_ALIGN(coal, tx_coalesce_usecs, COALESCE_TIMER_CFG_UNIT);
+ CHECK_COALESCE_ALIGN(coal, tx_max_coalesced_frames,
+ COALESCE_PENDING_LIMIT_UNIT);
+
+ if (coal->rx_coalesce_usecs || coal->rx_max_coalesced_frames) {
+ rx_intr_coal.coalesce_timer_cfg =
+ (u8)(coal->rx_coalesce_usecs / COALESCE_TIMER_CFG_UNIT);
+ rx_intr_coal.pending_limt = (u8)(coal->rx_max_coalesced_frames /
+ COALESCE_PENDING_LIMIT_UNIT);
+ set_rx_coal = true;
+ }
+
+ if (coal->tx_coalesce_usecs || coal->tx_max_coalesced_frames) {
+ tx_intr_coal.coalesce_timer_cfg =
+ (u8)(coal->tx_coalesce_usecs / COALESCE_TIMER_CFG_UNIT);
+ tx_intr_coal.pending_limt = (u8)(coal->tx_max_coalesced_frames /
+ COALESCE_PENDING_LIMIT_UNIT);
+ set_tx_coal = true;
+ }
+
+ if (queue == COALESCE_ALL_QUEUE) {
+ ori_rx_intr_coal = &nic_dev->rx_intr_coalesce[0];
+ ori_tx_intr_coal = &nic_dev->tx_intr_coalesce[0];
+ err = snprintf(obj_str, OBJ_STR_MAX_LEN, "for netdev");
+ } else {
+ ori_rx_intr_coal = &nic_dev->rx_intr_coalesce[queue];
+ ori_tx_intr_coal = &nic_dev->tx_intr_coalesce[queue];
+ err = snprintf(obj_str, OBJ_STR_MAX_LEN, "for queue %d", queue);
+ }
+ if (err <= 0 || err >= OBJ_STR_MAX_LEN) {
+ netif_err(nic_dev, drv, netdev, "Failed to snprintf string, function return(%d) and dest_len(%d)\n",
+ err, OBJ_STR_MAX_LEN);
+ return -EFAULT;
+ }
+
+ CHECK_COALESCE_CHANGED(coal, rx_coalesce_usecs, COALESCE_TIMER_CFG_UNIT,
+ ori_rx_intr_coal->coalesce_timer_cfg, obj_str);
+ CHECK_COALESCE_CHANGED(coal, rx_max_coalesced_frames,
+ COALESCE_PENDING_LIMIT_UNIT,
+ ori_rx_intr_coal->pending_limt, obj_str);
+ CHECK_COALESCE_CHANGED(coal, tx_coalesce_usecs, COALESCE_TIMER_CFG_UNIT,
+ ori_tx_intr_coal->coalesce_timer_cfg, obj_str);
+ CHECK_COALESCE_CHANGED(coal, tx_max_coalesced_frames,
+ COALESCE_PENDING_LIMIT_UNIT,
+ ori_tx_intr_coal->pending_limt, obj_str);
+
+ /* setting coalesce timer or pending limit to zero will disable
+ * coalesce
+ */
+ if (set_rx_coal && (!rx_intr_coal.coalesce_timer_cfg ||
+ !rx_intr_coal.pending_limt))
+ netif_warn(nic_dev, drv, netdev, "RX coalesce will be disabled\n");
+ if (set_tx_coal && (!tx_intr_coal.coalesce_timer_cfg ||
+ !tx_intr_coal.pending_limt))
+ netif_warn(nic_dev, drv, netdev, "TX coalesce will be disabled\n");
+
+ if (set_rx_coal) {
+ err = __set_hw_coal_param(nic_dev, &rx_intr_coal, queue, true);
+ if (err)
+ return err;
+ }
+ if (set_tx_coal) {
+ err = __set_hw_coal_param(nic_dev, &tx_intr_coal, queue, false);
+ if (err)
+ return err;
+ }
+ return 0;
+}
+
+static int hinic_get_coalesce(struct net_device *netdev,
+ struct ethtool_coalesce *coal)
+{
+ return __hinic_get_coalesce(netdev, coal, COALESCE_ALL_QUEUE);
+}
+
+static int hinic_set_coalesce(struct net_device *netdev,
+ struct ethtool_coalesce *coal)
+{
+ return __hinic_set_coalesce(netdev, coal, COALESCE_ALL_QUEUE);
+}
+
+static int hinic_get_per_queue_coalesce(struct net_device *netdev, u32 queue,
+ struct ethtool_coalesce *coal)
+{
+ return __hinic_get_coalesce(netdev, coal, queue);
+}
+
+static int hinic_set_per_queue_coalesce(struct net_device *netdev, u32 queue,
+ struct ethtool_coalesce *coal)
+{
+ return __hinic_set_coalesce(netdev, coal, queue);
+}
+
static void hinic_get_pauseparam(struct net_device *netdev,
struct ethtool_pauseparam *pause)
{
@@ -1303,12 +1579,21 @@ static void hinic_get_strings(struct net_device *netdev,
}
static const struct ethtool_ops hinic_ethtool_ops = {
+ .supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS |
+ ETHTOOL_COALESCE_RX_MAX_FRAMES |
+ ETHTOOL_COALESCE_TX_USECS |
+ ETHTOOL_COALESCE_TX_MAX_FRAMES,
+
.get_link_ksettings = hinic_get_link_ksettings,
.set_link_ksettings = hinic_set_link_ksettings,
.get_drvinfo = hinic_get_drvinfo,
.get_link = ethtool_op_get_link,
.get_ringparam = hinic_get_ringparam,
.set_ringparam = hinic_set_ringparam,
+ .get_coalesce = hinic_get_coalesce,
+ .set_coalesce = hinic_set_coalesce,
+ .get_per_queue_coalesce = hinic_get_per_queue_coalesce,
+ .set_per_queue_coalesce = hinic_set_per_queue_coalesce,
.get_pauseparam = hinic_get_pauseparam,
.set_pauseparam = hinic_set_pauseparam,
.get_channels = hinic_get_channels,
@@ -1325,11 +1610,20 @@ static const struct ethtool_ops hinic_ethtool_ops = {
};
static const struct ethtool_ops hinicvf_ethtool_ops = {
+ .supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS |
+ ETHTOOL_COALESCE_RX_MAX_FRAMES |
+ ETHTOOL_COALESCE_TX_USECS |
+ ETHTOOL_COALESCE_TX_MAX_FRAMES,
+
.get_link_ksettings = hinic_get_link_ksettings,
.get_drvinfo = hinic_get_drvinfo,
.get_link = ethtool_op_get_link,
.get_ringparam = hinic_get_ringparam,
.set_ringparam = hinic_set_ringparam,
+ .get_coalesce = hinic_get_coalesce,
+ .set_coalesce = hinic_set_coalesce,
+ .get_per_queue_coalesce = hinic_get_per_queue_coalesce,
+ .set_per_queue_coalesce = hinic_set_per_queue_coalesce,
.get_channels = hinic_get_channels,
.set_channels = hinic_set_channels,
.get_rxnfc = hinic_get_rxnfc,
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
index 747d50b841ba..4de50e4ba4df 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
@@ -705,6 +705,68 @@ static int hinic_l2nic_reset(struct hinic_hwdev *hwdev)
return 0;
}
+int hinic_get_interrupt_cfg(struct hinic_hwdev *hwdev,
+ struct hinic_msix_config *interrupt_info)
+{
+ u16 out_size = sizeof(*interrupt_info);
+ struct hinic_pfhwdev *pfhwdev;
+ int err;
+
+ if (!hwdev || !interrupt_info)
+ return -EINVAL;
+
+ pfhwdev = container_of(hwdev, struct hinic_pfhwdev, hwdev);
+
+ interrupt_info->func_id = HINIC_HWIF_FUNC_IDX(hwdev->hwif);
+
+ err = hinic_msg_to_mgmt(&pfhwdev->pf_to_mgmt, HINIC_MOD_COMM,
+ HINIC_COMM_CMD_MSI_CTRL_REG_RD_BY_UP,
+ interrupt_info, sizeof(*interrupt_info),
+ interrupt_info, &out_size, HINIC_MGMT_MSG_SYNC);
+ if (err || !out_size || interrupt_info->status) {
+ dev_err(&hwdev->hwif->pdev->dev, "Failed to get interrupt config, err: %d, status: 0x%x, out size: 0x%x\n",
+ err, interrupt_info->status, out_size);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+int hinic_set_interrupt_cfg(struct hinic_hwdev *hwdev,
+ struct hinic_msix_config *interrupt_info)
+{
+ u16 out_size = sizeof(*interrupt_info);
+ struct hinic_msix_config temp_info;
+ struct hinic_pfhwdev *pfhwdev;
+ int err;
+
+ if (!hwdev)
+ return -EINVAL;
+
+ pfhwdev = container_of(hwdev, struct hinic_pfhwdev, hwdev);
+
+ interrupt_info->func_id = HINIC_HWIF_FUNC_IDX(hwdev->hwif);
+
+ err = hinic_get_interrupt_cfg(hwdev, &temp_info);
+ if (err)
+ return -EINVAL;
+
+ interrupt_info->lli_credit_cnt = temp_info.lli_timer_cnt;
+ interrupt_info->lli_timer_cnt = temp_info.lli_timer_cnt;
+
+ err = hinic_msg_to_mgmt(&pfhwdev->pf_to_mgmt, HINIC_MOD_COMM,
+ HINIC_COMM_CMD_MSI_CTRL_REG_WR_BY_UP,
+ interrupt_info, sizeof(*interrupt_info),
+ interrupt_info, &out_size, HINIC_MGMT_MSG_SYNC);
+ if (err || !out_size || interrupt_info->status) {
+ dev_err(&hwdev->hwif->pdev->dev, "Failed to get interrupt config, err: %d, status: 0x%x, out size: 0x%x\n",
+ err, interrupt_info->status, out_size);
+ return -EIO;
+ }
+
+ return 0;
+}
+
/**
* hinic_init_hwdev - Initialize the NIC HW
* @pdev: the NIC pci device
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h
index cc776ca2d737..ed3cc154ce18 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h
@@ -285,6 +285,21 @@ struct hinic_cmd_l2nic_reset {
u16 reset_flag;
};
+struct hinic_msix_config {
+ u8 status;
+ u8 version;
+ u8 rsvd0[6];
+
+ u16 func_id;
+ u16 msix_index;
+ u8 pending_cnt;
+ u8 coalesce_timer_cnt;
+ u8 lli_timer_cnt;
+ u8 lli_credit_cnt;
+ u8 resend_timer_cnt;
+ u8 rsvd1[3];
+};
+
struct hinic_hwdev {
struct hinic_hwif *hwif;
struct msix_entry *msix_entries;
@@ -378,4 +393,10 @@ int hinic_hwdev_hw_ci_addr_set(struct hinic_hwdev *hwdev, struct hinic_sq *sq,
void hinic_hwdev_set_msix_state(struct hinic_hwdev *hwdev, u16 msix_index,
enum hinic_msix_state flag);
+int hinic_get_interrupt_cfg(struct hinic_hwdev *hwdev,
+ struct hinic_msix_config *interrupt_info);
+
+int hinic_set_interrupt_cfg(struct hinic_hwdev *hwdev,
+ struct hinic_msix_config *interrupt_info);
+
#endif
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_mgmt.h b/drivers/net/ethernet/huawei/hinic/hinic_hw_mgmt.h
index c2b142c08b0e..a3349ae30ff3 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_mgmt.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_mgmt.h
@@ -78,6 +78,9 @@ enum hinic_comm_cmd {
HINIC_COMM_CMD_CEQ_CTRL_REG_WR_BY_UP = 0x33,
+ HINIC_COMM_CMD_MSI_CTRL_REG_WR_BY_UP,
+ HINIC_COMM_CMD_MSI_CTRL_REG_RD_BY_UP,
+
HINIC_COMM_CMD_L2NIC_RESET = 0x4b,
HINIC_COMM_CMD_PAGESIZE_SET = 0x50,
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_main.c b/drivers/net/ethernet/huawei/hinic/hinic_main.c
index e69edb01fd9b..e328effd12d2 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_main.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_main.c
@@ -69,6 +69,10 @@ MODULE_PARM_DESC(rx_weight, "Number Rx packets for NAPI budget (default=64)");
#define HINIC_WAIT_SRIOV_CFG_TIMEOUT 15000
+#define HINIC_DEAULT_TXRX_MSIX_PENDING_LIMIT 2
+#define HINIC_DEAULT_TXRX_MSIX_COALESC_TIMER_CFG 32
+#define HINIC_DEAULT_TXRX_MSIX_RESEND_TIMER_CFG 7
+
static int change_mac_addr(struct net_device *netdev, const u8 *addr);
static int set_features(struct hinic_dev *nic_dev,
@@ -1008,6 +1012,48 @@ static int set_features(struct hinic_dev *nic_dev,
return hinic_dcb_set_pfc(nic_dev->hwdev, 0, 0);
}
+static int hinic_init_intr_coalesce(struct hinic_dev *nic_dev)
+{
+ u64 size;
+ u16 i;
+
+ size = sizeof(struct hinic_intr_coal_info) * nic_dev->max_qps;
+ nic_dev->rx_intr_coalesce = kzalloc(size, GFP_KERNEL);
+ if (!nic_dev->rx_intr_coalesce) {
+ dev_err(&nic_dev->hwdev->hwif->pdev->dev, "Failed to alloc rx intr coalesce\n");
+ return -ENOMEM;
+ }
+ nic_dev->tx_intr_coalesce = kzalloc(size, GFP_KERNEL);
+ if (!nic_dev->tx_intr_coalesce) {
+ kfree(nic_dev->rx_intr_coalesce);
+ dev_err(&nic_dev->hwdev->hwif->pdev->dev, "Failed to alloc tx intr coalesce\n");
+ return -ENOMEM;
+ }
+
+ for (i = 0; i < nic_dev->max_qps; i++) {
+ nic_dev->rx_intr_coalesce[i].pending_limt =
+ HINIC_DEAULT_TXRX_MSIX_PENDING_LIMIT;
+ nic_dev->rx_intr_coalesce[i].coalesce_timer_cfg =
+ HINIC_DEAULT_TXRX_MSIX_COALESC_TIMER_CFG;
+ nic_dev->rx_intr_coalesce[i].resend_timer_cfg =
+ HINIC_DEAULT_TXRX_MSIX_RESEND_TIMER_CFG;
+ nic_dev->tx_intr_coalesce[i].pending_limt =
+ HINIC_DEAULT_TXRX_MSIX_PENDING_LIMIT;
+ nic_dev->tx_intr_coalesce[i].coalesce_timer_cfg =
+ HINIC_DEAULT_TXRX_MSIX_COALESC_TIMER_CFG;
+ nic_dev->tx_intr_coalesce[i].resend_timer_cfg =
+ HINIC_DEAULT_TXRX_MSIX_RESEND_TIMER_CFG;
+ }
+
+ return 0;
+}
+
+static void hinic_free_intr_coalesce(struct hinic_dev *nic_dev)
+{
+ kfree(nic_dev->tx_intr_coalesce);
+ kfree(nic_dev->rx_intr_coalesce);
+}
+
/**
* nic_dev_init - Initialize the NIC device
* @pdev: the NIC pci device
@@ -1144,8 +1190,16 @@ static int nic_dev_init(struct pci_dev *pdev)
goto err_reg_netdev;
}
+ err = hinic_init_intr_coalesce(nic_dev);
+ if (err) {
+ netif_err(nic_dev, drv, netdev, "Failed to init_intr_coalesce\n");
+ goto err_init_intr;
+ }
+
return 0;
+err_init_intr:
+ unregister_netdev(netdev);
err_reg_netdev:
err_set_features:
hinic_hwdev_cb_unregister(nic_dev->hwdev,
@@ -1258,6 +1312,8 @@ static void hinic_remove(struct pci_dev *pdev)
hinic_pci_sriov_disable(pdev);
}
+ hinic_free_intr_coalesce(nic_dev);
+
unregister_netdev(netdev);
hinic_port_del_mac(nic_dev, netdev->dev_addr, 0);
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_rx.c b/drivers/net/ethernet/huawei/hinic/hinic_rx.c
index af20d0dd6de7..c9a65a1f0347 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_rx.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_rx.c
@@ -478,11 +478,15 @@ static irqreturn_t rx_irq(int irq, void *data)
static int rx_request_irq(struct hinic_rxq *rxq)
{
struct hinic_dev *nic_dev = netdev_priv(rxq->netdev);
+ struct hinic_msix_config interrupt_info = {0};
+ struct hinic_intr_coal_info *intr_coal = NULL;
struct hinic_hwdev *hwdev = nic_dev->hwdev;
struct hinic_rq *rq = rxq->rq;
struct hinic_qp *qp;
int err;
+ qp = container_of(rq, struct hinic_qp, rq);
+
rx_add_napi(rxq);
hinic_hwdev_msix_set(hwdev, rq->msix_entry,
@@ -490,13 +494,26 @@ static int rx_request_irq(struct hinic_rxq *rxq)
RX_IRQ_NO_LLI_TIMER, RX_IRQ_NO_CREDIT,
RX_IRQ_NO_RESEND_TIMER);
+ intr_coal = &nic_dev->rx_intr_coalesce[qp->q_id];
+ interrupt_info.msix_index = rq->msix_entry;
+ interrupt_info.coalesce_timer_cnt = intr_coal->coalesce_timer_cfg;
+ interrupt_info.pending_cnt = intr_coal->pending_limt;
+ interrupt_info.resend_timer_cnt = intr_coal->resend_timer_cfg;
+
+ err = hinic_set_interrupt_cfg(hwdev, &interrupt_info);
+ if (err) {
+ netif_err(nic_dev, drv, rxq->netdev,
+ "Failed to set RX interrupt coalescing attribute\n");
+ rx_del_napi(rxq);
+ return err;
+ }
+
err = request_irq(rq->irq, rx_irq, 0, rxq->irq_name, rxq);
if (err) {
rx_del_napi(rxq);
return err;
}
- qp = container_of(rq, struct hinic_qp, rq);
cpumask_set_cpu(qp->q_id % num_online_cpus(), &rq->affinity_mask);
return irq_set_affinity_hint(rq->irq, &rq->affinity_mask);
}
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_tx.c b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
index 4c66a0bc1b28..0f6d27f29de5 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_tx.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
@@ -718,12 +718,17 @@ static irqreturn_t tx_irq(int irq, void *data)
static int tx_request_irq(struct hinic_txq *txq)
{
struct hinic_dev *nic_dev = netdev_priv(txq->netdev);
+ struct hinic_msix_config interrupt_info = {0};
+ struct hinic_intr_coal_info *intr_coal = NULL;
struct hinic_hwdev *hwdev = nic_dev->hwdev;
struct hinic_hwif *hwif = hwdev->hwif;
struct pci_dev *pdev = hwif->pdev;
struct hinic_sq *sq = txq->sq;
+ struct hinic_qp *qp;
int err;
+ qp = container_of(sq, struct hinic_qp, sq);
+
tx_napi_add(txq, nic_dev->tx_weight);
hinic_hwdev_msix_set(nic_dev->hwdev, sq->msix_entry,
@@ -731,6 +736,20 @@ static int tx_request_irq(struct hinic_txq *txq)
TX_IRQ_NO_LLI_TIMER, TX_IRQ_NO_CREDIT,
TX_IRQ_NO_RESEND_TIMER);
+ intr_coal = &nic_dev->tx_intr_coalesce[qp->q_id];
+ interrupt_info.msix_index = sq->msix_entry;
+ interrupt_info.coalesce_timer_cnt = intr_coal->coalesce_timer_cfg;
+ interrupt_info.pending_cnt = intr_coal->pending_limt;
+ interrupt_info.resend_timer_cnt = intr_coal->resend_timer_cfg;
+
+ err = hinic_set_interrupt_cfg(hwdev, &interrupt_info);
+ if (err) {
+ netif_err(nic_dev, drv, txq->netdev,
+ "Failed to set TX interrupt coalescing attribute\n");
+ tx_napi_del(txq);
+ return err;
+ }
+
err = request_irq(sq->irq, tx_irq, 0, txq->irq_name, txq);
if (err) {
dev_err(&pdev->dev, "Failed to request Tx irq\n");
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v1 3/5] hinic: add self test support
From: Luo bin @ 2020-06-20 9:42 UTC (permalink / raw)
To: davem; +Cc: linux-kernel, netdev, luoxianjun, yin.yinshi, cloud.wangxiaoyun
In-Reply-To: <20200620094258.13181-1-luobin9@huawei.com>
add support to excute internal and external loopback test with
ethtool -t cmd.
Signed-off-by: Luo bin <luobin9@huawei.com>
---
drivers/net/ethernet/huawei/hinic/hinic_dev.h | 6 +
.../net/ethernet/huawei/hinic/hinic_ethtool.c | 178 ++++++++++++++++++
.../net/ethernet/huawei/hinic/hinic_hw_dev.h | 3 +
.../net/ethernet/huawei/hinic/hinic_port.c | 30 +++
.../net/ethernet/huawei/hinic/hinic_port.h | 16 ++
drivers/net/ethernet/huawei/hinic/hinic_rx.c | 39 ++++
drivers/net/ethernet/huawei/hinic/hinic_tx.c | 61 ++++++
drivers/net/ethernet/huawei/hinic/hinic_tx.h | 2 +
8 files changed, 335 insertions(+)
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_dev.h b/drivers/net/ethernet/huawei/hinic/hinic_dev.h
index 75d6dee948f5..9adb755f0820 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_dev.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_dev.h
@@ -20,11 +20,14 @@
#define HINIC_DRV_NAME "hinic"
+#define LP_PKT_CNT 64
+
enum hinic_flags {
HINIC_LINK_UP = BIT(0),
HINIC_INTF_UP = BIT(1),
HINIC_RSS_ENABLE = BIT(2),
HINIC_LINK_DOWN = BIT(3),
+ HINIC_LP_TEST = BIT(4),
};
struct hinic_rx_mode_work {
@@ -91,6 +94,9 @@ struct hinic_dev {
struct hinic_intr_coal_info *rx_intr_coalesce;
struct hinic_intr_coal_info *tx_intr_coalesce;
struct hinic_sriov_info sriov_info;
+ int lb_test_rx_idx;
+ int lb_pkt_len;
+ u8 *lb_test_rx_buf;
};
#endif
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
index 750b4d786c50..a461099dfbb9 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
@@ -150,6 +150,16 @@ static struct hw2ethtool_link_mode
},
};
+#define LP_DEFAULT_TIME 5 /* seconds */
+#define LP_PKT_LEN 1514
+
+#define PORT_DOWN_ERR_IDX 0
+enum diag_test_index {
+ INTERNAL_LP_TEST = 0,
+ EXTERNAL_LP_TEST = 1,
+ DIAG_TEST_MAX = 2,
+};
+
static void set_link_speed(struct ethtool_link_ksettings *link_ksettings,
enum hinic_speed speed)
{
@@ -1314,6 +1324,11 @@ static struct hinic_stats hinic_function_stats[] = {
HINIC_FUNC_STAT(rx_err_vport),
};
+static char hinic_test_strings[][ETH_GSTRING_LEN] = {
+ "Internal lb test (on/offline)",
+ "External lb test (external_lb)",
+};
+
#define HINIC_PORT_STAT(_stat_item) { \
.name = #_stat_item, \
.size = sizeof_field(struct hinic_phy_port_stats, _stat_item), \
@@ -1523,6 +1538,8 @@ static int hinic_get_sset_count(struct net_device *netdev, int sset)
int count, q_num;
switch (sset) {
+ case ETH_SS_TEST:
+ return ARRAY_LEN(hinic_test_strings);
case ETH_SS_STATS:
q_num = nic_dev->num_qps;
count = ARRAY_LEN(hinic_function_stats) +
@@ -1545,6 +1562,9 @@ static void hinic_get_strings(struct net_device *netdev,
u16 i, j;
switch (stringset) {
+ case ETH_SS_TEST:
+ memcpy(data, *hinic_test_strings, sizeof(hinic_test_strings));
+ return;
case ETH_SS_STATS:
for (i = 0; i < ARRAY_LEN(hinic_function_stats); i++) {
memcpy(p, hinic_function_stats[i].name,
@@ -1578,6 +1598,163 @@ static void hinic_get_strings(struct net_device *netdev,
}
}
+static int hinic_run_lp_test(struct hinic_dev *nic_dev, u32 test_time)
+{
+ u8 *lb_test_rx_buf = nic_dev->lb_test_rx_buf;
+ struct net_device *netdev = nic_dev->netdev;
+ struct sk_buff *skb_tmp = NULL;
+ struct sk_buff *skb = NULL;
+ u32 cnt = test_time * 5;
+ u8 *test_data = NULL;
+ u32 i;
+ u8 j;
+
+ skb_tmp = alloc_skb(LP_PKT_LEN, GFP_ATOMIC);
+ if (!skb_tmp)
+ return -ENOMEM;
+
+ test_data = __skb_put(skb_tmp, LP_PKT_LEN);
+
+ memset(test_data, 0xFF, 2 * ETH_ALEN);
+ test_data[ETH_ALEN] = 0xFE;
+ test_data[2 * ETH_ALEN] = 0x08;
+ test_data[2 * ETH_ALEN + 1] = 0x0;
+
+ for (i = ETH_HLEN; i < LP_PKT_LEN; i++)
+ test_data[i] = i & 0xFF;
+
+ skb_tmp->queue_mapping = 0;
+ skb_tmp->ip_summed = CHECKSUM_COMPLETE;
+ skb_tmp->dev = netdev;
+
+ for (i = 0; i < cnt; i++) {
+ nic_dev->lb_test_rx_idx = 0;
+ memset(lb_test_rx_buf, 0, LP_PKT_CNT * LP_PKT_LEN);
+
+ for (j = 0; j < LP_PKT_CNT; j++) {
+ skb = pskb_copy(skb_tmp, GFP_ATOMIC);
+ if (!skb) {
+ dev_kfree_skb_any(skb_tmp);
+ netif_err(nic_dev, drv, netdev,
+ "Copy skb failed for loopback test\n");
+ return -ENOMEM;
+ }
+
+ /* mark index for every pkt */
+ skb->data[LP_PKT_LEN - 1] = j;
+
+ if (hinic_lb_xmit_frame(skb, netdev)) {
+ dev_kfree_skb_any(skb);
+ dev_kfree_skb_any(skb_tmp);
+ netif_err(nic_dev, drv, netdev,
+ "Xmit pkt failed for loopback test\n");
+ return -EBUSY;
+ }
+ }
+
+ /* wait till all pkts received to RX buffer */
+ msleep(200);
+
+ for (j = 0; j < LP_PKT_CNT; j++) {
+ if (memcmp(lb_test_rx_buf + j * LP_PKT_LEN,
+ skb_tmp->data, LP_PKT_LEN - 1) ||
+ (*(lb_test_rx_buf + j * LP_PKT_LEN +
+ LP_PKT_LEN - 1) != j)) {
+ dev_kfree_skb_any(skb_tmp);
+ netif_err(nic_dev, drv, netdev,
+ "Compare pkt failed in loopback test(index=0x%02x, data[%d]=0x%02x)\n",
+ j + i * LP_PKT_CNT,
+ LP_PKT_LEN - 1,
+ *(lb_test_rx_buf + j * LP_PKT_LEN +
+ LP_PKT_LEN - 1));
+ return -EIO;
+ }
+ }
+ }
+
+ dev_kfree_skb_any(skb_tmp);
+ netif_info(nic_dev, drv, netdev, "Loopback test succeed.\n");
+ return 0;
+}
+
+static int do_lp_test(struct hinic_dev *nic_dev, u32 flags, u32 test_time,
+ enum diag_test_index *test_index)
+{
+ struct net_device *netdev = nic_dev->netdev;
+ u8 *lb_test_rx_buf = NULL;
+ int err = 0;
+
+ if (!(flags & ETH_TEST_FL_EXTERNAL_LB)) {
+ *test_index = INTERNAL_LP_TEST;
+ if (hinic_set_loopback_mode(nic_dev->hwdev,
+ HINIC_INTERNAL_LP_MODE, true)) {
+ netif_err(nic_dev, drv, netdev,
+ "Failed to set port loopback mode before loopback test\n");
+ return -EIO;
+ }
+ } else {
+ *test_index = EXTERNAL_LP_TEST;
+ }
+
+ lb_test_rx_buf = vmalloc(LP_PKT_CNT * LP_PKT_LEN);
+ if (!lb_test_rx_buf) {
+ err = -ENOMEM;
+ } else {
+ nic_dev->lb_test_rx_buf = lb_test_rx_buf;
+ nic_dev->lb_pkt_len = LP_PKT_LEN;
+ nic_dev->flags |= HINIC_LP_TEST;
+ err = hinic_run_lp_test(nic_dev, test_time);
+ nic_dev->flags &= ~HINIC_LP_TEST;
+ msleep(100);
+ vfree(lb_test_rx_buf);
+ nic_dev->lb_test_rx_buf = NULL;
+ }
+
+ if (!(flags & ETH_TEST_FL_EXTERNAL_LB)) {
+ if (hinic_set_loopback_mode(nic_dev->hwdev,
+ HINIC_INTERNAL_LP_MODE, false)) {
+ netif_err(nic_dev, drv, netdev,
+ "Failed to cancel port loopback mode after loopback test\n");
+ err = -EIO;
+ }
+ }
+
+ return err;
+}
+
+static void hinic_diag_test(struct net_device *netdev,
+ struct ethtool_test *eth_test, u64 *data)
+{
+ struct hinic_dev *nic_dev = netdev_priv(netdev);
+ enum hinic_port_link_state link_state;
+ enum diag_test_index test_index = 0;
+ int err = 0;
+
+ memset(data, 0, DIAG_TEST_MAX * sizeof(u64));
+
+ /* don't support loopback test when netdev is closed. */
+ if (!(nic_dev->flags & HINIC_INTF_UP)) {
+ netif_err(nic_dev, drv, netdev,
+ "Do not support loopback test when netdev is closed\n");
+ eth_test->flags |= ETH_TEST_FL_FAILED;
+ data[PORT_DOWN_ERR_IDX] = 1;
+ return;
+ }
+
+ netif_carrier_off(netdev);
+
+ err = do_lp_test(nic_dev, eth_test->flags, LP_DEFAULT_TIME,
+ &test_index);
+ if (err) {
+ eth_test->flags |= ETH_TEST_FL_FAILED;
+ data[test_index] = 1;
+ }
+
+ err = hinic_port_link_state(nic_dev, &link_state);
+ if (!err && link_state == HINIC_LINK_STATE_UP)
+ netif_carrier_on(netdev);
+}
+
static const struct ethtool_ops hinic_ethtool_ops = {
.supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS |
ETHTOOL_COALESCE_RX_MAX_FRAMES |
@@ -1607,6 +1784,7 @@ static const struct ethtool_ops hinic_ethtool_ops = {
.get_sset_count = hinic_get_sset_count,
.get_ethtool_stats = hinic_get_ethtool_stats,
.get_strings = hinic_get_strings,
+ .self_test = hinic_diag_test,
};
static const struct ethtool_ops hinicvf_ethtool_ops = {
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h
index ed3cc154ce18..c92c39a50b81 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h
@@ -97,6 +97,9 @@ enum hinic_port_cmd {
HINIC_PORT_CMD_FWCTXT_INIT = 69,
+ HINIC_PORT_CMD_GET_LOOPBACK_MODE = 72,
+ HINIC_PORT_CMD_SET_LOOPBACK_MODE,
+
HINIC_PORT_CMD_ENABLE_SPOOFCHK = 78,
HINIC_PORT_CMD_GET_MGMT_VERSION = 88,
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_port.c b/drivers/net/ethernet/huawei/hinic/hinic_port.c
index 8b007a268675..53ea2740ba9f 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_port.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_port.c
@@ -1241,3 +1241,33 @@ int hinic_dcb_set_pfc(struct hinic_hwdev *hwdev, u8 pfc_en, u8 pfc_bitmap)
return 0;
}
+
+int hinic_set_loopback_mode(struct hinic_hwdev *hwdev, u32 mode, u32 enable)
+{
+ struct hinic_port_loopback lb = {0};
+ u16 out_size = sizeof(lb);
+ int err;
+
+ lb.mode = mode;
+ lb.en = enable;
+
+ if (mode < LOOP_MODE_MIN || mode > LOOP_MODE_MAX) {
+ dev_err(&hwdev->hwif->pdev->dev,
+ "Invalid loopback mode %d to set\n", mode);
+ return -EINVAL;
+ }
+
+ err = hinic_port_msg_cmd(hwdev, HINIC_PORT_CMD_SET_LOOPBACK_MODE,
+ &lb, sizeof(lb), &lb, &out_size);
+ if (err || !out_size || lb.status) {
+ dev_err(&hwdev->hwif->pdev->dev,
+ "Failed to set loopback mode %d en %d, err: %d, status: 0x%x, out size: 0x%x\n",
+ mode, enable, err, lb.status, out_size);
+ return -EIO;
+ }
+
+ dev_info(&hwdev->hwif->pdev->dev,
+ "Set loopback mode %d en %d succeed\n", mode, enable);
+
+ return 0;
+}
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_port.h b/drivers/net/ethernet/huawei/hinic/hinic_port.h
index 7b17460d4e2c..b21956d7232e 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_port.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_port.h
@@ -652,6 +652,20 @@ struct hinic_set_pfc {
u8 rsvd1[4];
};
+/* get or set loopback mode, need to modify by base API */
+#define HINIC_INTERNAL_LP_MODE 5
+#define LOOP_MODE_MIN 1
+#define LOOP_MODE_MAX 6
+
+struct hinic_port_loopback {
+ u8 status;
+ u8 version;
+ u8 rsvd[6];
+
+ u32 mode;
+ u32 en;
+};
+
int hinic_port_add_mac(struct hinic_dev *nic_dev, const u8 *addr,
u16 vlan_id);
@@ -749,6 +763,8 @@ int hinic_set_hw_pause_info(struct hinic_hwdev *hwdev,
int hinic_dcb_set_pfc(struct hinic_hwdev *hwdev, u8 pfc_en, u8 pfc_bitmap);
+int hinic_set_loopback_mode(struct hinic_hwdev *hwdev, u32 mode, u32 enable);
+
int hinic_open(struct net_device *netdev);
int hinic_close(struct net_device *netdev);
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_rx.c b/drivers/net/ethernet/huawei/hinic/hinic_rx.c
index c9a65a1f0347..5bee951fe9d4 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_rx.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_rx.c
@@ -316,6 +316,39 @@ static int rx_recv_jumbo_pkt(struct hinic_rxq *rxq, struct sk_buff *head_skb,
return num_wqes;
}
+static void hinic_copy_lp_data(struct hinic_dev *nic_dev,
+ struct sk_buff *skb)
+{
+ struct net_device *netdev = nic_dev->netdev;
+ u8 *lb_buf = nic_dev->lb_test_rx_buf;
+ int lb_len = nic_dev->lb_pkt_len;
+ int pkt_offset, frag_len, i;
+ void *frag_data = NULL;
+
+ if (nic_dev->lb_test_rx_idx == LP_PKT_CNT) {
+ nic_dev->lb_test_rx_idx = 0;
+ netif_warn(nic_dev, drv, netdev, "Loopback test warning, receive too more test pkts\n");
+ }
+
+ if (skb->len != nic_dev->lb_pkt_len) {
+ netif_warn(nic_dev, drv, netdev, "Wrong packet length\n");
+ nic_dev->lb_test_rx_idx++;
+ return;
+ }
+
+ pkt_offset = nic_dev->lb_test_rx_idx * lb_len;
+ frag_len = (int)skb_headlen(skb);
+ memcpy(lb_buf + pkt_offset, skb->data, frag_len);
+ pkt_offset += frag_len;
+ for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+ frag_data = skb_frag_address(&skb_shinfo(skb)->frags[i]);
+ frag_len = (int)skb_frag_size(&skb_shinfo(skb)->frags[i]);
+ memcpy((lb_buf + pkt_offset), frag_data, frag_len);
+ pkt_offset += frag_len;
+ }
+ nic_dev->lb_test_rx_idx++;
+}
+
/**
* rxq_recv - Rx handler
* @rxq: rx queue
@@ -330,6 +363,7 @@ static int rxq_recv(struct hinic_rxq *rxq, int budget)
u64 pkt_len = 0, rx_bytes = 0;
struct hinic_rq *rq = rxq->rq;
struct hinic_rq_wqe *rq_wqe;
+ struct hinic_dev *nic_dev;
unsigned int free_wqebbs;
struct hinic_rq_cqe *cqe;
int num_wqes, pkts = 0;
@@ -342,6 +376,8 @@ static int rxq_recv(struct hinic_rxq *rxq, int budget)
u32 vlan_len;
u16 vid;
+ nic_dev = netdev_priv(netdev);
+
while (pkts < budget) {
num_wqes = 0;
@@ -384,6 +420,9 @@ static int rxq_recv(struct hinic_rxq *rxq, int budget)
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid);
}
+ if (unlikely(nic_dev->flags & HINIC_LP_TEST))
+ hinic_copy_lp_data(nic_dev, skb);
+
skb_record_rx_queue(skb, qp->q_id);
skb->protocol = eth_type_trans(skb, rxq->netdev);
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_tx.c b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
index 0f6d27f29de5..a97498ee6914 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_tx.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
@@ -459,6 +459,67 @@ static int hinic_tx_offload(struct sk_buff *skb, struct hinic_sq_task *task,
return 0;
}
+netdev_tx_t hinic_lb_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
+{
+ struct hinic_dev *nic_dev = netdev_priv(netdev);
+ u16 prod_idx, q_id = skb->queue_mapping;
+ struct netdev_queue *netdev_txq;
+ int nr_sges, err = NETDEV_TX_OK;
+ struct hinic_sq_wqe *sq_wqe;
+ unsigned int wqe_size;
+ struct hinic_txq *txq;
+ struct hinic_qp *qp;
+
+ txq = &nic_dev->txqs[q_id];
+ qp = container_of(txq->sq, struct hinic_qp, sq);
+ nr_sges = skb_shinfo(skb)->nr_frags + 1;
+
+ err = tx_map_skb(nic_dev, skb, txq->sges);
+ if (err)
+ goto skb_error;
+
+ wqe_size = HINIC_SQ_WQE_SIZE(nr_sges);
+
+ sq_wqe = hinic_sq_get_wqe(txq->sq, wqe_size, &prod_idx);
+ if (!sq_wqe) {
+ netif_stop_subqueue(netdev, qp->q_id);
+
+ sq_wqe = hinic_sq_get_wqe(txq->sq, wqe_size, &prod_idx);
+ if (sq_wqe) {
+ netif_wake_subqueue(nic_dev->netdev, qp->q_id);
+ goto process_sq_wqe;
+ }
+
+ tx_unmap_skb(nic_dev, skb, txq->sges);
+
+ u64_stats_update_begin(&txq->txq_stats.syncp);
+ txq->txq_stats.tx_busy++;
+ u64_stats_update_end(&txq->txq_stats.syncp);
+ err = NETDEV_TX_BUSY;
+ wqe_size = 0;
+ goto flush_skbs;
+ }
+
+process_sq_wqe:
+ hinic_sq_prepare_wqe(txq->sq, prod_idx, sq_wqe, txq->sges, nr_sges);
+ hinic_sq_write_wqe(txq->sq, prod_idx, sq_wqe, skb, wqe_size);
+
+flush_skbs:
+ netdev_txq = netdev_get_tx_queue(netdev, q_id);
+ if ((!netdev_xmit_more()) || (netif_xmit_stopped(netdev_txq)))
+ hinic_sq_write_db(txq->sq, prod_idx, wqe_size, 0);
+
+ return err;
+
+skb_error:
+ dev_kfree_skb_any(skb);
+ u64_stats_update_begin(&txq->txq_stats.syncp);
+ txq->txq_stats.tx_dropped++;
+ u64_stats_update_end(&txq->txq_stats.syncp);
+
+ return NETDEV_TX_OK;
+}
+
netdev_tx_t hinic_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
{
struct hinic_dev *nic_dev = netdev_priv(netdev);
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_tx.h b/drivers/net/ethernet/huawei/hinic/hinic_tx.h
index f158b7db7fb8..b3c8657774a7 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_tx.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_tx.h
@@ -44,6 +44,8 @@ void hinic_txq_clean_stats(struct hinic_txq *txq);
void hinic_txq_get_stats(struct hinic_txq *txq, struct hinic_txq_stats *stats);
+netdev_tx_t hinic_lb_xmit_frame(struct sk_buff *skb, struct net_device *netdev);
+
netdev_tx_t hinic_xmit_frame(struct sk_buff *skb, struct net_device *netdev);
int hinic_init_txq(struct hinic_txq *txq, struct hinic_sq *sq,
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v5 0/3] net: dsa: qca8k: Improve SGMII interface handling
From: Jonathan McDowell @ 2020-06-20 10:30 UTC (permalink / raw)
To: Russell King - ARM Linux admin, Andrew Lunn, Florian Fainelli,
Vladimir Oltean
Cc: Vivien Didelot, David Miller, Jakub Kicinski, netdev,
linux-kernel
In-Reply-To: <cover.1591816172.git.noodles@earth.li>
This 3 patch series migrates the qca8k switch driver over to PHYLINK,
and then adds the SGMII clean-ups (i.e. the missing initialisation) on
top of that as a second patch. The final patch is a simple spelling fix
in a comment.
As before, tested with a device where the CPU connection is RGMII (i.e.
the common current use case) + one where the CPU connection is SGMII. I
don't have any devices where the SGMII interface is brought out to
something other than the CPU.
v5:
- Move spelling fix to separate patch
- Use ds directly rather than ds->priv
v4:
- Enable pcs_poll so we keep phylink updated when doing in-band
negotiation
- Explicitly check for PHY_INTERFACE_MODE_1000BASEX when setting SGMII
port mode.
- Address Vladimir's review comments
v3:
- Move phylink changes to separate patch
- Address rmk review comments
v2:
- Switch to phylink
- Avoid need for device tree configuration options
Jonathan McDowell (3):
net: dsa: qca8k: Switch to PHYLINK instead of PHYLIB
net: dsa: qca8k: Improve SGMII interface handling
net: dsa: qca8k: Minor comment spelling fix
drivers/net/dsa/qca8k.c | 341 ++++++++++++++++++++++++++++------------
drivers/net/dsa/qca8k.h | 13 ++
2 files changed, 256 insertions(+), 98 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH net-next v5 2/3] net: dsa: qca8k: Improve SGMII interface handling
From: Jonathan McDowell @ 2020-06-20 10:31 UTC (permalink / raw)
To: Russell King - ARM Linux admin, Andrew Lunn, Florian Fainelli,
Vladimir Oltean
Cc: Vivien Didelot, David Miller, Jakub Kicinski, netdev,
linux-kernel
In-Reply-To: <cover.1592648711.git.noodles@earth.li>
This patch improves the handling of the SGMII interface on the QCA8K
devices. Previously the driver did no configuration of the port, even if
it was selected. We now configure it up in the appropriate
PHY/MAC/Base-X mode depending on what phylink tells us we are connected
to and ensure it is enabled.
Tested with a device where the CPU connection is RGMII (i.e. the common
current use case) + one where the CPU connection is SGMII. I don't have
any devices where the SGMII interface is brought out to something other
than the CPU.
Signed-off-by: Jonathan McDowell <noodles@earth.li>
---
drivers/net/dsa/qca8k.c | 33 ++++++++++++++++++++++++++++++++-
drivers/net/dsa/qca8k.h | 13 +++++++++++++
2 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 63b84789f16b..11d1c290d90f 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -673,6 +673,9 @@ qca8k_setup(struct dsa_switch *ds)
/* Flush the FDB table */
qca8k_fdb_flush(priv);
+ /* We don't have interrupts for link changes, so we need to poll */
+ ds->pcs_poll = true;
+
return 0;
}
@@ -681,7 +684,7 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
const struct phylink_link_state *state)
{
struct qca8k_priv *priv = ds->priv;
- u32 reg;
+ u32 reg, val;
switch (port) {
case 0: /* 1st CPU port */
@@ -740,6 +743,34 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
case PHY_INTERFACE_MODE_1000BASEX:
/* Enable SGMII on the port */
qca8k_write(priv, reg, QCA8K_PORT_PAD_SGMII_EN);
+
+ /* Enable/disable SerDes auto-negotiation as necessary */
+ val = qca8k_read(priv, QCA8K_REG_PWS);
+ if (phylink_autoneg_inband(mode))
+ val &= ~QCA8K_PWS_SERDES_AEN_DIS;
+ else
+ val |= QCA8K_PWS_SERDES_AEN_DIS;
+ qca8k_write(priv, QCA8K_REG_PWS, val);
+
+ /* Configure the SGMII parameters */
+ val = qca8k_read(priv, QCA8K_REG_SGMII_CTRL);
+
+ val |= QCA8K_SGMII_EN_PLL | QCA8K_SGMII_EN_RX |
+ QCA8K_SGMII_EN_TX | QCA8K_SGMII_EN_SD;
+
+ if (dsa_is_cpu_port(ds, port)) {
+ /* CPU port, we're talking to the CPU MAC, be a PHY */
+ val &= ~QCA8K_SGMII_MODE_CTRL_MASK;
+ val |= QCA8K_SGMII_MODE_CTRL_PHY;
+ } else if (state->interface == PHY_INTERFACE_MODE_SGMII) {
+ val &= ~QCA8K_SGMII_MODE_CTRL_MASK;
+ val |= QCA8K_SGMII_MODE_CTRL_MAC;
+ } else if (state->interface == PHY_INTERFACE_MODE_1000BASEX) {
+ val &= ~QCA8K_SGMII_MODE_CTRL_MASK;
+ val |= QCA8K_SGMII_MODE_CTRL_BASEX;
+ }
+
+ qca8k_write(priv, QCA8K_REG_SGMII_CTRL, val);
break;
default:
dev_err(ds->dev, "xMII mode %s not supported for port %d\n",
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index 42d6ea24eb14..10ef2bca2cde 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -36,6 +36,8 @@
#define QCA8K_MAX_DELAY 3
#define QCA8K_PORT_PAD_RGMII_RX_DELAY_EN BIT(24)
#define QCA8K_PORT_PAD_SGMII_EN BIT(7)
+#define QCA8K_REG_PWS 0x010
+#define QCA8K_PWS_SERDES_AEN_DIS BIT(7)
#define QCA8K_REG_MODULE_EN 0x030
#define QCA8K_MODULE_EN_MIB BIT(0)
#define QCA8K_REG_MIB 0x034
@@ -69,6 +71,7 @@
#define QCA8K_PORT_STATUS_LINK_UP BIT(8)
#define QCA8K_PORT_STATUS_LINK_AUTO BIT(9)
#define QCA8K_PORT_STATUS_LINK_PAUSE BIT(10)
+#define QCA8K_PORT_STATUS_FLOW_AUTO BIT(12)
#define QCA8K_REG_PORT_HDR_CTRL(_i) (0x9c + (_i * 4))
#define QCA8K_PORT_HDR_CTRL_RX_MASK GENMASK(3, 2)
#define QCA8K_PORT_HDR_CTRL_RX_S 2
@@ -77,6 +80,16 @@
#define QCA8K_PORT_HDR_CTRL_ALL 2
#define QCA8K_PORT_HDR_CTRL_MGMT 1
#define QCA8K_PORT_HDR_CTRL_NONE 0
+#define QCA8K_REG_SGMII_CTRL 0x0e0
+#define QCA8K_SGMII_EN_PLL BIT(1)
+#define QCA8K_SGMII_EN_RX BIT(2)
+#define QCA8K_SGMII_EN_TX BIT(3)
+#define QCA8K_SGMII_EN_SD BIT(4)
+#define QCA8K_SGMII_CLK125M_DELAY BIT(7)
+#define QCA8K_SGMII_MODE_CTRL_MASK (BIT(22) | BIT(23))
+#define QCA8K_SGMII_MODE_CTRL_BASEX (0 << 22)
+#define QCA8K_SGMII_MODE_CTRL_PHY (1 << 22)
+#define QCA8K_SGMII_MODE_CTRL_MAC (2 << 22)
/* EEE control registers */
#define QCA8K_REG_EEE_CTRL 0x100
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v5 1/3] net: dsa: qca8k: Switch to PHYLINK instead of PHYLIB
From: Jonathan McDowell @ 2020-06-20 10:30 UTC (permalink / raw)
To: Russell King - ARM Linux admin, Andrew Lunn, Florian Fainelli,
Vladimir Oltean
Cc: Vivien Didelot, David Miller, Jakub Kicinski, netdev,
linux-kernel
In-Reply-To: <cover.1592648711.git.noodles@earth.li>
Update the driver to use the new PHYLINK callbacks, removing the
legacy adjust_link callback.
Signed-off-by: Jonathan McDowell <noodles@earth.li>
---
drivers/net/dsa/qca8k.c | 306 +++++++++++++++++++++++++++-------------
1 file changed, 210 insertions(+), 96 deletions(-)
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index d2b5ab403e06..63b84789f16b 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -14,6 +14,7 @@
#include <linux/of_platform.h>
#include <linux/if_bridge.h>
#include <linux/mdio.h>
+#include <linux/phylink.h>
#include <linux/gpio/consumer.h>
#include <linux/etherdevice.h>
@@ -418,55 +419,6 @@ qca8k_mib_init(struct qca8k_priv *priv)
mutex_unlock(&priv->reg_mutex);
}
-static int
-qca8k_set_pad_ctrl(struct qca8k_priv *priv, int port, int mode)
-{
- u32 reg, val;
-
- switch (port) {
- case 0:
- reg = QCA8K_REG_PORT0_PAD_CTRL;
- break;
- case 6:
- reg = QCA8K_REG_PORT6_PAD_CTRL;
- break;
- default:
- pr_err("Can't set PAD_CTRL on port %d\n", port);
- return -EINVAL;
- }
-
- /* Configure a port to be directly connected to an external
- * PHY or MAC.
- */
- switch (mode) {
- case PHY_INTERFACE_MODE_RGMII:
- /* RGMII mode means no delay so don't enable the delay */
- val = QCA8K_PORT_PAD_RGMII_EN;
- qca8k_write(priv, reg, val);
- break;
- case PHY_INTERFACE_MODE_RGMII_ID:
- /* RGMII_ID needs internal delay. This is enabled through
- * PORT5_PAD_CTRL for all ports, rather than individual port
- * registers
- */
- qca8k_write(priv, reg,
- QCA8K_PORT_PAD_RGMII_EN |
- QCA8K_PORT_PAD_RGMII_TX_DELAY(QCA8K_MAX_DELAY) |
- QCA8K_PORT_PAD_RGMII_RX_DELAY(QCA8K_MAX_DELAY));
- qca8k_write(priv, QCA8K_REG_PORT5_PAD_CTRL,
- QCA8K_PORT_PAD_RGMII_RX_DELAY_EN);
- break;
- case PHY_INTERFACE_MODE_SGMII:
- qca8k_write(priv, reg, QCA8K_PORT_PAD_SGMII_EN);
- break;
- default:
- pr_err("xMII mode %d not supported\n", mode);
- return -EINVAL;
- }
-
- return 0;
-}
-
static void
qca8k_port_set_status(struct qca8k_priv *priv, int port, int enable)
{
@@ -639,9 +591,7 @@ static int
qca8k_setup(struct dsa_switch *ds)
{
struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
- phy_interface_t phy_mode = PHY_INTERFACE_MODE_NA;
int ret, i;
- u32 mask;
/* Make sure that port 0 is the cpu port */
if (!dsa_is_cpu_port(ds, 0)) {
@@ -661,24 +611,9 @@ qca8k_setup(struct dsa_switch *ds)
if (ret)
return ret;
- /* Initialize CPU port pad mode (xMII type, delays...) */
- ret = of_get_phy_mode(dsa_to_port(ds, QCA8K_CPU_PORT)->dn, &phy_mode);
- if (ret) {
- pr_err("Can't find phy-mode for master device\n");
- return ret;
- }
- ret = qca8k_set_pad_ctrl(priv, QCA8K_CPU_PORT, phy_mode);
- if (ret < 0)
- return ret;
-
- /* Enable CPU Port, force it to maximum bandwidth and full-duplex */
- mask = QCA8K_PORT_STATUS_SPEED_1000 | QCA8K_PORT_STATUS_TXFLOW |
- QCA8K_PORT_STATUS_RXFLOW | QCA8K_PORT_STATUS_DUPLEX;
- qca8k_write(priv, QCA8K_REG_PORT_STATUS(QCA8K_CPU_PORT), mask);
+ /* Enable CPU Port */
qca8k_reg_set(priv, QCA8K_REG_GLOBAL_FW_CTRL0,
QCA8K_GLOBAL_FW_CTRL0_CPU_PORT_EN);
- qca8k_port_set_status(priv, QCA8K_CPU_PORT, 1);
- priv->port_sts[QCA8K_CPU_PORT].enabled = 1;
/* Enable MIB counters */
qca8k_mib_init(priv);
@@ -693,10 +628,9 @@ qca8k_setup(struct dsa_switch *ds)
qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i),
QCA8K_PORT_LOOKUP_MEMBER, 0);
- /* Disable MAC by default on all user ports */
+ /* Disable MAC by default on all ports */
for (i = 1; i < QCA8K_NUM_PORTS; i++)
- if (dsa_is_user_port(ds, i))
- qca8k_port_set_status(priv, i, 0);
+ qca8k_port_set_status(priv, i, 0);
/* Forward all unknown frames to CPU port for Linux processing */
qca8k_write(priv, QCA8K_REG_GLOBAL_FW_CTRL1,
@@ -743,44 +677,222 @@ qca8k_setup(struct dsa_switch *ds)
}
static void
-qca8k_adjust_link(struct dsa_switch *ds, int port, struct phy_device *phy)
+qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
+ const struct phylink_link_state *state)
{
struct qca8k_priv *priv = ds->priv;
u32 reg;
- /* Force fixed-link setting for CPU port, skip others. */
- if (!phy_is_pseudo_fixed_link(phy))
+ switch (port) {
+ case 0: /* 1st CPU port */
+ if (state->interface != PHY_INTERFACE_MODE_RGMII &&
+ state->interface != PHY_INTERFACE_MODE_RGMII_ID &&
+ state->interface != PHY_INTERFACE_MODE_SGMII)
+ return;
+
+ reg = QCA8K_REG_PORT0_PAD_CTRL;
+ break;
+ case 1:
+ case 2:
+ case 3:
+ case 4:
+ case 5:
+ /* Internal PHY, nothing to do */
+ return;
+ case 6: /* 2nd CPU port / external PHY */
+ if (state->interface != PHY_INTERFACE_MODE_RGMII &&
+ state->interface != PHY_INTERFACE_MODE_RGMII_ID &&
+ state->interface != PHY_INTERFACE_MODE_SGMII &&
+ state->interface != PHY_INTERFACE_MODE_1000BASEX)
+ return;
+
+ reg = QCA8K_REG_PORT6_PAD_CTRL;
+ break;
+ default:
+ dev_err(ds->dev, "%s: unsupported port: %i\n", __func__, port);
+ return;
+ }
+
+ if (port != 6 && phylink_autoneg_inband(mode)) {
+ dev_err(ds->dev, "%s: in-band negotiation unsupported\n",
+ __func__);
+ return;
+ }
+
+ switch (state->interface) {
+ case PHY_INTERFACE_MODE_RGMII:
+ /* RGMII mode means no delay so don't enable the delay */
+ qca8k_write(priv, reg, QCA8K_PORT_PAD_RGMII_EN);
+ break;
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ /* RGMII_ID needs internal delay. This is enabled through
+ * PORT5_PAD_CTRL for all ports, rather than individual port
+ * registers
+ */
+ qca8k_write(priv, reg,
+ QCA8K_PORT_PAD_RGMII_EN |
+ QCA8K_PORT_PAD_RGMII_TX_DELAY(QCA8K_MAX_DELAY) |
+ QCA8K_PORT_PAD_RGMII_RX_DELAY(QCA8K_MAX_DELAY));
+ qca8k_write(priv, QCA8K_REG_PORT5_PAD_CTRL,
+ QCA8K_PORT_PAD_RGMII_RX_DELAY_EN);
+ break;
+ case PHY_INTERFACE_MODE_SGMII:
+ case PHY_INTERFACE_MODE_1000BASEX:
+ /* Enable SGMII on the port */
+ qca8k_write(priv, reg, QCA8K_PORT_PAD_SGMII_EN);
+ break;
+ default:
+ dev_err(ds->dev, "xMII mode %s not supported for port %d\n",
+ phy_modes(state->interface), port);
return;
+ }
+}
- /* Set port speed */
- switch (phy->speed) {
- case 10:
- reg = QCA8K_PORT_STATUS_SPEED_10;
+static void
+qca8k_phylink_validate(struct dsa_switch *ds, int port,
+ unsigned long *supported,
+ struct phylink_link_state *state)
+{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
+
+ switch (port) {
+ case 0: /* 1st CPU port */
+ if (state->interface != PHY_INTERFACE_MODE_NA &&
+ state->interface != PHY_INTERFACE_MODE_RGMII &&
+ state->interface != PHY_INTERFACE_MODE_RGMII_ID &&
+ state->interface != PHY_INTERFACE_MODE_SGMII)
+ goto unsupported;
break;
- case 100:
- reg = QCA8K_PORT_STATUS_SPEED_100;
+ case 1:
+ case 2:
+ case 3:
+ case 4:
+ case 5:
+ /* Internal PHY */
+ if (state->interface != PHY_INTERFACE_MODE_NA &&
+ state->interface != PHY_INTERFACE_MODE_GMII)
+ goto unsupported;
break;
- case 1000:
- reg = QCA8K_PORT_STATUS_SPEED_1000;
+ case 6: /* 2nd CPU port / external PHY */
+ if (state->interface != PHY_INTERFACE_MODE_NA &&
+ state->interface != PHY_INTERFACE_MODE_RGMII &&
+ state->interface != PHY_INTERFACE_MODE_RGMII_ID &&
+ state->interface != PHY_INTERFACE_MODE_SGMII &&
+ state->interface != PHY_INTERFACE_MODE_1000BASEX)
+ goto unsupported;
break;
default:
- dev_dbg(priv->dev, "port%d link speed %dMbps not supported.\n",
- port, phy->speed);
+unsupported:
+ linkmode_zero(supported);
return;
}
- /* Set duplex mode */
- if (phy->duplex == DUPLEX_FULL)
- reg |= QCA8K_PORT_STATUS_DUPLEX;
+ phylink_set_port_modes(mask);
+ phylink_set(mask, Autoneg);
+
+ phylink_set(mask, 1000baseT_Full);
+ phylink_set(mask, 10baseT_Half);
+ phylink_set(mask, 10baseT_Full);
+ phylink_set(mask, 100baseT_Half);
+ phylink_set(mask, 100baseT_Full);
+
+ if (state->interface == PHY_INTERFACE_MODE_1000BASEX)
+ phylink_set(mask, 1000baseX_Full);
+
+ phylink_set(mask, Pause);
+ phylink_set(mask, Asym_Pause);
+
+ linkmode_and(supported, supported, mask);
+ linkmode_and(state->advertising, state->advertising, mask);
+}
+
+static int
+qca8k_phylink_mac_link_state(struct dsa_switch *ds, int port,
+ struct phylink_link_state *state)
+{
+ struct qca8k_priv *priv = ds->priv;
+ u32 reg;
- /* Force flow control */
- if (dsa_is_cpu_port(ds, port))
- reg |= QCA8K_PORT_STATUS_RXFLOW | QCA8K_PORT_STATUS_TXFLOW;
+ reg = qca8k_read(priv, QCA8K_REG_PORT_STATUS(port));
+
+ state->link = !!(reg & QCA8K_PORT_STATUS_LINK_UP);
+ state->an_complete = state->link;
+ state->an_enabled = !!(reg & QCA8K_PORT_STATUS_LINK_AUTO);
+ state->duplex = (reg & QCA8K_PORT_STATUS_DUPLEX) ? DUPLEX_FULL :
+ DUPLEX_HALF;
+
+ switch (reg & QCA8K_PORT_STATUS_SPEED) {
+ case QCA8K_PORT_STATUS_SPEED_10:
+ state->speed = SPEED_10;
+ break;
+ case QCA8K_PORT_STATUS_SPEED_100:
+ state->speed = SPEED_100;
+ break;
+ case QCA8K_PORT_STATUS_SPEED_1000:
+ state->speed = SPEED_1000;
+ break;
+ default:
+ state->speed = SPEED_UNKNOWN;
+ break;
+ }
+
+ state->pause = MLO_PAUSE_NONE;
+ if (reg & QCA8K_PORT_STATUS_RXFLOW)
+ state->pause |= MLO_PAUSE_RX;
+ if (reg & QCA8K_PORT_STATUS_TXFLOW)
+ state->pause |= MLO_PAUSE_TX;
+
+ return 1;
+}
+
+static void
+qca8k_phylink_mac_link_down(struct dsa_switch *ds, int port, unsigned int mode,
+ phy_interface_t interface)
+{
+ struct qca8k_priv *priv = ds->priv;
- /* Force link down before changing MAC options */
qca8k_port_set_status(priv, port, 0);
+}
+
+static void
+qca8k_phylink_mac_link_up(struct dsa_switch *ds, int port, unsigned int mode,
+ phy_interface_t interface, struct phy_device *phydev,
+ int speed, int duplex, bool tx_pause, bool rx_pause)
+{
+ struct qca8k_priv *priv = ds->priv;
+ u32 reg;
+
+ if (phylink_autoneg_inband(mode)) {
+ reg = QCA8K_PORT_STATUS_LINK_AUTO;
+ } else {
+ switch (speed) {
+ case SPEED_10:
+ reg = QCA8K_PORT_STATUS_SPEED_10;
+ break;
+ case SPEED_100:
+ reg = QCA8K_PORT_STATUS_SPEED_100;
+ break;
+ case SPEED_1000:
+ reg = QCA8K_PORT_STATUS_SPEED_1000;
+ break;
+ default:
+ reg = QCA8K_PORT_STATUS_LINK_AUTO;
+ break;
+ }
+
+ if (duplex == DUPLEX_FULL)
+ reg |= QCA8K_PORT_STATUS_DUPLEX;
+
+ if (rx_pause || dsa_is_cpu_port(ds, port))
+ reg |= QCA8K_PORT_STATUS_RXFLOW;
+
+ if (tx_pause || dsa_is_cpu_port(ds, port))
+ reg |= QCA8K_PORT_STATUS_TXFLOW;
+ }
+
+ reg |= QCA8K_PORT_STATUS_TXMAC | QCA8K_PORT_STATUS_RXMAC;
+
qca8k_write(priv, QCA8K_REG_PORT_STATUS(port), reg);
- qca8k_port_set_status(priv, port, 1);
}
static void
@@ -937,13 +1049,11 @@ qca8k_port_enable(struct dsa_switch *ds, int port,
{
struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
- if (!dsa_is_user_port(ds, port))
- return 0;
-
qca8k_port_set_status(priv, port, 1);
priv->port_sts[port].enabled = 1;
- phy_support_asym_pause(phy);
+ if (dsa_is_user_port(ds, port))
+ phy_support_asym_pause(phy);
return 0;
}
@@ -1026,7 +1136,6 @@ qca8k_get_tag_protocol(struct dsa_switch *ds, int port,
static const struct dsa_switch_ops qca8k_switch_ops = {
.get_tag_protocol = qca8k_get_tag_protocol,
.setup = qca8k_setup,
- .adjust_link = qca8k_adjust_link,
.get_strings = qca8k_get_strings,
.get_ethtool_stats = qca8k_get_ethtool_stats,
.get_sset_count = qca8k_get_sset_count,
@@ -1040,6 +1149,11 @@ static const struct dsa_switch_ops qca8k_switch_ops = {
.port_fdb_add = qca8k_port_fdb_add,
.port_fdb_del = qca8k_port_fdb_del,
.port_fdb_dump = qca8k_port_fdb_dump,
+ .phylink_validate = qca8k_phylink_validate,
+ .phylink_mac_link_state = qca8k_phylink_mac_link_state,
+ .phylink_mac_config = qca8k_phylink_mac_config,
+ .phylink_mac_link_down = qca8k_phylink_mac_link_down,
+ .phylink_mac_link_up = qca8k_phylink_mac_link_up,
};
static int
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v5 3/3] net: dsa: qca8k: Minor comment spelling fix
From: Jonathan McDowell @ 2020-06-20 10:31 UTC (permalink / raw)
To: Russell King - ARM Linux admin, Andrew Lunn, Florian Fainelli,
Vladimir Oltean
Cc: Vivien Didelot, David Miller, Jakub Kicinski, netdev,
linux-kernel
In-Reply-To: <cover.1592648711.git.noodles@earth.li>
Signed-off-by: Jonathan McDowell <noodles@earth.li>
---
drivers/net/dsa/qca8k.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 11d1c290d90f..4acad5fa0c84 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -647,7 +647,7 @@ qca8k_setup(struct dsa_switch *ds)
QCA8K_PORT_LOOKUP_MEMBER, dsa_user_ports(ds));
}
- /* Invividual user ports get connected to CPU port only */
+ /* Individual user ports get connected to CPU port only */
if (dsa_is_user_port(ds, i)) {
int shift = 16 * (i % 2);
--
2.20.1
^ permalink raw reply related
* Re: net/mlx5e: bind() always returns EINVAL with XDP_ZEROCOPY
From: Kal Cutter Conley @ 2020-06-20 10:42 UTC (permalink / raw)
To: Jonathan Lemon
Cc: Saeed Mahameed, brouer@redhat.com, Maxim Mikityanskiy,
magnus.karlsson@intel.com, toke.hoiland-jorgensen@kau.se,
xdp-newbies@vger.kernel.org, Tariq Toukan, gospo@broadcom.com,
netdev@vger.kernel.org, bjorn.topel@intel.com
In-Reply-To: <20200618150347.ihtdvsfuurgfka7i@bsd-mbp.dhcp.thefacebook.com>
On Thu, Jun 18, 2020 at 5:23 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
>
> On Sun, Jun 14, 2020 at 10:55:30AM +0200, Kal Cutter Conley wrote:
> > Hi Saeed,
> > Thanks for explaining the reasoning behind the special mlx5 queue
> > numbering with XDP zerocopy.
> >
> > We have a process using AF_XDP that also shares the network interface
> > with other processes on the system. ethtool rx flow classification
> > rules are used to route the traffic to the appropriate XSK queue
> > N..(2N-1). The issue is these queues are only valid as long they are
> > active (as far as I can tell). This means if my AF_XDP process dies
> > other processes no longer receive ingress traffic routed over queues
> > N..(2N-1) even though my XDP program is still loaded and would happily
> > always return XDP_PASS. Other drivers do not have this usability issue
> > because they use queues that are always valid. Is there a simple
> > workaround for this issue? It seems to me queues N..(2N-1) should
> > simply map to 0..(N-1) when they are not active?
>
> If your XDP program returns XDP_PASS, the packet should be delivered to
> the xsk socket. If the application isn't running, where would it go?
>
> I do agree that the usability of this can be improved. What if the flow
> rules are inserted and removed along with queue creatioin/destruction?
I think I misunderstood your suggestion here. Do you mean the rules
should be inserted / removed on the hardware level but still show in
ethtool even if they are not active in the hardware? In this case the
rules always occupy a "location" but just never apply if the
respective queues are not "enabled". I think this would be the best
possible solution.
> --
> Jonathan
Kal
^ permalink raw reply
* Re: INFO: trying to register non-static key in ath9k_htc_rxep
From: syzbot @ 2020-06-20 10:57 UTC (permalink / raw)
To: andreyknvl, ath9k-devel, davem, kuba, kvalo, linux-kernel,
linux-usb, linux-wireless, netdev, syzkaller-bugs
In-Reply-To: <0000000000006bf03c05a86205bb@google.com>
syzbot has found a reproducer for the following crash on:
HEAD commit: f8f02d5c USB: OTG: rename product list of devices
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
console output: https://syzkaller.appspot.com/x/log.txt?x=15fd18a5100000
kernel config: https://syzkaller.appspot.com/x/.config?x=f1981539b6376b73
dashboard link: https://syzkaller.appspot.com/bug?extid=4d2d56175b934b9a7bf9
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14519481100000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=110318e9100000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+4d2d56175b934b9a7bf9@syzkaller.appspotmail.com
INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.8.0-rc1-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xf6/0x16e lib/dump_stack.c:118
assign_lock_key kernel/locking/lockdep.c:894 [inline]
register_lock_class+0x1228/0x16d0 kernel/locking/lockdep.c:1206
__lock_acquire+0x101/0x6270 kernel/locking/lockdep.c:4259
lock_acquire+0x18b/0x7c0 kernel/locking/lockdep.c:4959
__raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
_raw_spin_lock_irqsave+0x32/0x50 kernel/locking/spinlock.c:159
ath9k_htc_rxep+0x31/0x210 drivers/net/wireless/ath/ath9k/htc_drv_txrx.c:1128
ath9k_htc_rx_msg+0x2d9/0xb00 drivers/net/wireless/ath/ath9k/htc_hst.c:459
ath9k_hif_usb_rx_stream drivers/net/wireless/ath/ath9k/hif_usb.c:638 [inline]
ath9k_hif_usb_rx_cb+0xc76/0x1050 drivers/net/wireless/ath/ath9k/hif_usb.c:671
__usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650
usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716
dummy_timer+0x125e/0x32b4 drivers/usb/gadget/udc/dummy_hcd.c:1967
call_timer_fn+0x1ac/0x6e0 kernel/time/timer.c:1404
expire_timers kernel/time/timer.c:1449 [inline]
__run_timers kernel/time/timer.c:1773 [inline]
__run_timers kernel/time/timer.c:1740 [inline]
run_timer_softirq+0x5e5/0x14c0 kernel/time/timer.c:1786
__do_softirq+0x21e/0x996 kernel/softirq.c:292
asm_call_on_stack+0xf/0x20 arch/x86/entry/entry_64.S:711
</IRQ>
__run_on_irqstack arch/x86/include/asm/irq_stack.h:22 [inline]
run_on_irqstack_cond arch/x86/include/asm/irq_stack.h:48 [inline]
do_softirq_own_stack+0x109/0x140 arch/x86/kernel/irq_64.c:77
invoke_softirq kernel/softirq.c:387 [inline]
__irq_exit_rcu kernel/softirq.c:417 [inline]
irq_exit_rcu+0x16f/0x1a0 kernel/softirq.c:429
sysvec_apic_timer_interrupt+0xd3/0x1b0 arch/x86/kernel/apic/apic.c:1091
asm_sysvec_apic_timer_interrupt+0x12/0x20 arch/x86/include/asm/idtentry.h:596
RIP: 0010:native_irq_disable arch/x86/include/asm/irqflags.h:49 [inline]
RIP: 0010:arch_local_irq_disable arch/x86/include/asm/irqflags.h:89 [inline]
RIP: 0010:acpi_safe_halt drivers/acpi/processor_idle.c:112 [inline]
RIP: 0010:acpi_safe_halt+0x72/0x90 drivers/acpi/processor_idle.c:108
Code: 74 06 5b e9 c0 32 9f fb e8 bb 32 9f fb e8 c6 96 a4 fb e9 0c 00 00 00 e8 ac 32 9f fb 0f 00 2d 45 6e 84 00 e8 a0 32 9f fb fb f4 <fa> e8 b8 94 a4 fb 5b e9 92 32 9f fb 48 89 df e8 7a e1 c8 fb eb ab
RSP: 0018:ffff8881da22fc60 EFLAGS: 00000293
RAX: ffff8881da213200 RBX: 0000000000000000 RCX: 1ffffffff1014efa
RDX: 0000000000000000 RSI: ffffffff85a03aa0 RDI: ffff8881da213a38
RBP: ffff8881d8d2a864 R08: 0000000000000000 R09: 0000000000000001
R10: 0000000000000000 R11: 0000000000000000 R12: ffff8881d8d2a864
R13: 1ffff1103b445f96 R14: ffff8881d8d2a865 R15: 0000000000000001
acpi_idle_do_entry+0xa9/0xe0 drivers/acpi/processor_idle.c:525
acpi_idle_enter+0x42b/0xac0 drivers/acpi/processor_idle.c:651
cpuidle_enter_state+0xdb/0xc20 drivers/cpuidle/cpuidle.c:234
cpuidle_enter+0x4a/0xa0 drivers/cpuidle/cpuidle.c:345
call_cpuidle kernel/sched/idle.c:117 [inline]
cpuidle_idle_call kernel/sched/idle.c:207 [inline]
do_idle+0x3c2/0x500 kernel/sched/idle.c:269
cpu_startup_entry+0x14/0x20 kernel/sched/idle.c:365
start_secondary+0x294/0x370 arch/x86/kernel/smpboot.c:268
secondary_startup_64+0xb6/0xc0 arch/x86/kernel/head_64.S:243
BUG: unable to handle page fault for address: ffffffffffffffc8
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 7226067 P4D 7226067 PUD 7228067 PMD 0
Oops: 0000 [#1] SMP KASAN
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.8.0-rc1-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:ath9k_htc_rxep+0xb5/0x210 drivers/net/wireless/ath/ath9k/htc_drv_txrx.c:1130
Code: 8b 43 38 48 8d 58 c8 49 39 c4 0f 84 ee 00 00 00 e8 70 56 62 fe 48 89 d8 48 c1 e8 03 0f b6 04 28 84 c0 74 06 0f 8e 0a 01 00 00 <44> 0f b6 3b 31 ff 44 89 fe e8 ad 57 62 fe 45 84 ff 75 a8 e8 43 56
RSP: 0018:ffff8881db3098b0 EFLAGS: 00010046
RAX: 0000000000000000 RBX: ffffffffffffffc8 RCX: ffffffff81274370
RDX: 0000000000000000 RSI: ffffffff82dd16d0 RDI: ffff8881db309820
RBP: dffffc0000000000 R08: 0000000000000004 R09: ffffed103b661305
R10: 0000000000000003 R11: ffffed103b661304 R12: ffff8881cd69b538
R13: ffff8881cd69b100 R14: ffff8881cd69b548 R15: ffffed10392ce210
FS: 0000000000000000(0000) GS:ffff8881db300000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffc8 CR3: 00000001cf9f6000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<IRQ>
ath9k_htc_rx_msg+0x2d9/0xb00 drivers/net/wireless/ath/ath9k/htc_hst.c:459
ath9k_hif_usb_rx_stream drivers/net/wireless/ath/ath9k/hif_usb.c:638 [inline]
ath9k_hif_usb_rx_cb+0xc76/0x1050 drivers/net/wireless/ath/ath9k/hif_usb.c:671
__usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650
usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716
dummy_timer+0x125e/0x32b4 drivers/usb/gadget/udc/dummy_hcd.c:1967
call_timer_fn+0x1ac/0x6e0 kernel/time/timer.c:1404
expire_timers kernel/time/timer.c:1449 [inline]
__run_timers kernel/time/timer.c:1773 [inline]
__run_timers kernel/time/timer.c:1740 [inline]
run_timer_softirq+0x5e5/0x14c0 kernel/time/timer.c:1786
__do_softirq+0x21e/0x996 kernel/softirq.c:292
asm_call_on_stack+0xf/0x20 arch/x86/entry/entry_64.S:711
</IRQ>
__run_on_irqstack arch/x86/include/asm/irq_stack.h:22 [inline]
run_on_irqstack_cond arch/x86/include/asm/irq_stack.h:48 [inline]
do_softirq_own_stack+0x109/0x140 arch/x86/kernel/irq_64.c:77
invoke_softirq kernel/softirq.c:387 [inline]
__irq_exit_rcu kernel/softirq.c:417 [inline]
irq_exit_rcu+0x16f/0x1a0 kernel/softirq.c:429
sysvec_apic_timer_interrupt+0xd3/0x1b0 arch/x86/kernel/apic/apic.c:1091
asm_sysvec_apic_timer_interrupt+0x12/0x20 arch/x86/include/asm/idtentry.h:596
RIP: 0010:native_irq_disable arch/x86/include/asm/irqflags.h:49 [inline]
RIP: 0010:arch_local_irq_disable arch/x86/include/asm/irqflags.h:89 [inline]
RIP: 0010:acpi_safe_halt drivers/acpi/processor_idle.c:112 [inline]
RIP: 0010:acpi_safe_halt+0x72/0x90 drivers/acpi/processor_idle.c:108
Code: 74 06 5b e9 c0 32 9f fb e8 bb 32 9f fb e8 c6 96 a4 fb e9 0c 00 00 00 e8 ac 32 9f fb 0f 00 2d 45 6e 84 00 e8 a0 32 9f fb fb f4 <fa> e8 b8 94 a4 fb 5b e9 92 32 9f fb 48 89 df e8 7a e1 c8 fb eb ab
RSP: 0018:ffff8881da22fc60 EFLAGS: 00000293
RAX: ffff8881da213200 RBX: 0000000000000000 RCX: 1ffffffff1014efa
RDX: 0000000000000000 RSI: ffffffff85a03aa0 RDI: ffff8881da213a38
RBP: ffff8881d8d2a864 R08: 0000000000000000 R09: 0000000000000001
R10: 0000000000000000 R11: 0000000000000000 R12: ffff8881d8d2a864
R13: 1ffff1103b445f96 R14: ffff8881d8d2a865 R15: 0000000000000001
acpi_idle_do_entry+0xa9/0xe0 drivers/acpi/processor_idle.c:525
acpi_idle_enter+0x42b/0xac0 drivers/acpi/processor_idle.c:651
cpuidle_enter_state+0xdb/0xc20 drivers/cpuidle/cpuidle.c:234
cpuidle_enter+0x4a/0xa0 drivers/cpuidle/cpuidle.c:345
call_cpuidle kernel/sched/idle.c:117 [inline]
cpuidle_idle_call kernel/sched/idle.c:207 [inline]
do_idle+0x3c2/0x500 kernel/sched/idle.c:269
cpu_startup_entry+0x14/0x20 kernel/sched/idle.c:365
start_secondary+0x294/0x370 arch/x86/kernel/smpboot.c:268
secondary_startup_64+0xb6/0xc0 arch/x86/kernel/head_64.S:243
Modules linked in:
CR2: ffffffffffffffc8
---[ end trace 5a637b710bbf1999 ]---
RIP: 0010:ath9k_htc_rxep+0xb5/0x210 drivers/net/wireless/ath/ath9k/htc_drv_txrx.c:1130
Code: 8b 43 38 48 8d 58 c8 49 39 c4 0f 84 ee 00 00 00 e8 70 56 62 fe 48 89 d8 48 c1 e8 03 0f b6 04 28 84 c0 74 06 0f 8e 0a 01 00 00 <44> 0f b6 3b 31 ff 44 89 fe e8 ad 57 62 fe 45 84 ff 75 a8 e8 43 56
RSP: 0018:ffff8881db3098b0 EFLAGS: 00010046
RAX: 0000000000000000 RBX: ffffffffffffffc8 RCX: ffffffff81274370
RDX: 0000000000000000 RSI: ffffffff82dd16d0 RDI: ffff8881db309820
RBP: dffffc0000000000 R08: 0000000000000004 R09: ffffed103b661305
R10: 0000000000000003 R11: ffffed103b661304 R12: ffff8881cd69b538
R13: ffff8881cd69b100 R14: ffff8881cd69b548 R15: ffffed10392ce210
FS: 0000000000000000(0000) GS:ffff8881db300000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffc8 CR3: 00000001cf9f6000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
^ permalink raw reply
* mlx5e uplink hairpin forwarding
From: Tonghao Zhang @ 2020-06-20 12:02 UTC (permalink / raw)
To: Eli Cohen, ozsh, Saeed Mahameed; +Cc: Linux Kernel Network Developers
Hi Eli
I review your patches and try to test it.
$ tc filter add dev enp130s0f0 protocol ip prio 1 root flower dst_ip
11.12.13.14 skip_sw action mirred egress redirect dev enp130s0f1
or
$ tc filter add dev enp130s0f0 protocol ip prio 1 parent ffff: flower
dst_ip 11.12.13.14 skip_sw action mirred egress redirect dev
enp130s0f1
TC can't install the rules above. The error message:
mlx5_core: devices are both uplink, can't offload forwarding.
So how can I install hairpin rules between uplink and uplink forwarding ?
The test environment
kernel 5.8.0-rc1+, the last commit id:69119673bd50b176ded34032fadd41530fb5af21
NIC MCX512A-ACA_Ax
FW 16.27.2008
enp130s0f0、enp130s0f1 are uplink rep.
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=249ccc3c95bd1bca17406b5b3d0474fd67220931
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=613f53fe09a27f928a7d05132e1a74b5136e8f04
--
Best regards, Tonghao
^ permalink raw reply
* Re: [PATCH net-next 0/2] devlink: Add board_serial_number field to info_get cb.
From: Jiri Pirko @ 2020-06-20 12:09 UTC (permalink / raw)
To: Vasundhara Volam; +Cc: davem, netdev, michael.chan, kuba, jiri, jacob.e.keller
In-Reply-To: <1592640947-10421-1-git-send-email-vasundhara-v.volam@broadcom.com>
Sat, Jun 20, 2020 at 10:15:45AM CEST, vasundhara-v.volam@broadcom.com wrote:
>This patchset adds support for board_serial_number to devlink info_get
>cb and also use it in bnxt_en driver.
>
>Sample output:
>
>$ devlink dev info pci/0000:af:00.1
>pci/0000:af:00.1:
> driver bnxt_en
> serial_number 00-10-18-FF-FE-AD-1A-00
> board_serial_number 433551F+172300000
> versions:
> fixed:
> board.id 7339763 Rev 0.
We have board.id already here. I understand that the serial number does
not belong under the same nest, as it is not a "version".
However, could you at least maintain the format:
board.serial_number
?
> asic.id 16D7
> asic.rev 1
> running:
> fw 216.1.216.0
> fw.psid 0.0.0
> fw.mgmt 216.1.192.0
> fw.mgmt.api 1.10.1
> fw.ncsi 0.0.0.0
> fw.roce 216.1.16.0
>
>Vasundhara Volam (2):
> devlink: Add support for board_serial_number to info_get cb.
> bnxt_en: Add board_serial_number field to info_get cb
>
> Documentation/networking/devlink/devlink-info.rst | 12 +++++-------
> drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 7 +++++++
> include/net/devlink.h | 2 ++
> include/uapi/linux/devlink.h | 2 ++
> net/core/devlink.c | 8 ++++++++
> 5 files changed, 24 insertions(+), 7 deletions(-)
>
>--
>1.8.3.1
>
^ permalink raw reply
* Re: [net-next 1/6] net: marvell: prestera: Add driver for Prestera family ASIC devices
From: Vadym Kochan @ 2020-06-20 12:56 UTC (permalink / raw)
To: Ido Schimmel
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, Ido Schimmel,
Andrew Lunn, Oleksandr Mazur, Serhiy Boiko, Serhiy Pshyk,
Volodymyr Mytnyk, Taras Chornyi, Andrii Savka, netdev,
linux-kernel, Mickey Rachamim
In-Reply-To: <20200603092358.GA1841966@splinter>
Hi Ido,
On Wed, Jun 03, 2020 at 12:23:58PM +0300, Ido Schimmel wrote:
> On Mon, Jun 01, 2020 at 01:50:13PM +0300, Vadym Kochan wrote:
> > Hi Ido,
> >
> > On Sat, May 30, 2020 at 06:48:01PM +0300, Ido Schimmel wrote:
> > > On Thu, May 28, 2020 at 06:12:40PM +0300, Vadym Kochan wrote:
> > >
> >
> > [...]
> >
> > > Nit: "From" ?
> > >
> > > > + PRESTERA_DSA_CMD_FROM_CPU,
> > > > +};
> > > > +
> > > > +struct prestera_dsa_vlan {
> > > > + u16 vid;
> > > > + u8 vpt;
> > > > + u8 cfi_bit;
> > > > + bool is_tagged;
> > > > +};
> > > > +
> > > > +struct prestera_dsa {
> > > > + struct prestera_dsa_vlan vlan;
> > > > + u32 hw_dev_num;
> > > > + u32 port_num;
> > > > +};
> > > > +
> > > > +int prestera_dsa_parse(struct prestera_dsa *dsa, const u8 *dsa_buf);
> > > > +int prestera_dsa_build(const struct prestera_dsa *dsa, u8 *dsa_buf);
> > > > +
> > > > +#endif /* _PRESTERA_DSA_H_ */
> > > > diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> > > > new file mode 100644
> > > > index 000000000000..3aa3974f957a
> > > > --- /dev/null
> > > > +++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> > > > @@ -0,0 +1,610 @@
> > > > +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
> > > > +/* Copyright (c) 2019-2020 Marvell International Ltd. All rights reserved */
> > > > +
> > > > +#include <linux/etherdevice.h>
> > > > +#include <linux/ethtool.h>
> > > > +#include <linux/netdevice.h>
> > > > +#include <linux/list.h>
> > > > +
> > > > +#include "prestera.h"
> > > > +#include "prestera_hw.h"
> > > > +
> > > > +#define PRESTERA_SWITCH_INIT_TIMEOUT 30000000 /* 30sec */
> > >
> > > Out of curiosity, how long does it actually take you to initialize the
> > > hardware?
It might be minimum 10-15 secs.
> > >
> > > Also, I find it useful to note the units in the name, so:
> > >
> > > #define PRESTERA_SWITCH_INIT_TIMEOUT_US (30 * 1000 * 1000)
> > >
> > > BTW, it says 30 seconds in comment, but the call chain where it is used
> > > is:
> > >
> > > prestera_cmd_ret_wait(, PRESTERA_SWITCH_INIT_TIMEOUT)
> > > __prestera_cmd_ret(..., wait)
> > > prestera_fw_send_req(..., waitms)
> > > prestera_fw_cmd_send(..., waitms)
> > > prestera_fw_wait_reg32(..., waitms)
> > > readl_poll_timeout(..., waitms * 1000)
> > >
> > > So I think you should actually define it as:
> > >
> > > #define PRESTERA_SWITCH_INIT_TIMEOUT_MS (30 * 1000)
> > >
> > > And rename all these 'wait' arguments to 'waitms' so it's clearer which
> > > unit they expect.
> > >
> > [...]
> > > > +static int __prestera_cmd_ret(struct prestera_switch *sw,
> > > > + enum prestera_cmd_type_t type,
> > > > + struct prestera_msg_cmd *cmd, size_t clen,
> > > > + struct prestera_msg_ret *ret, size_t rlen,
> > > > + int wait)
> > > > +{
> > > > + struct prestera_device *dev = sw->dev;
> > > > + int err;
> > > > +
> > > > + cmd->type = type;
> > > > +
> > > > + err = dev->send_req(dev, (u8 *)cmd, clen, (u8 *)ret, rlen, wait);
> > > > + if (err)
> > > > + return err;
> > > > +
> > > > + if (ret->cmd.type != PRESTERA_CMD_TYPE_ACK)
> > > > + return -EBADE;
> > > > + if (ret->status != PRESTERA_CMD_ACK_OK)
> > >
> > > You don't have more states here other than OK / FAIL ? It might help you
> > > in debugging if you include them. You might find trace_devlink_hwerr()
> > > useful.
> > Thanks, I will consider this.
> >
> > >
> > > > + return -EINVAL;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int prestera_cmd_ret(struct prestera_switch *sw,
> > > > + enum prestera_cmd_type_t type,
> > > > + struct prestera_msg_cmd *cmd, size_t clen,
> > > > + struct prestera_msg_ret *ret, size_t rlen)
> > > > +{
> > > > + return __prestera_cmd_ret(sw, type, cmd, clen, ret, rlen, 0);
> > > > +}
> > > > +
> > > > +static int prestera_cmd_ret_wait(struct prestera_switch *sw,
> > > > + enum prestera_cmd_type_t type,
> > > > + struct prestera_msg_cmd *cmd, size_t clen,
> > > > + struct prestera_msg_ret *ret, size_t rlen,
> > > > + int wait)
> > > > +{
> > > > + return __prestera_cmd_ret(sw, type, cmd, clen, ret, rlen, wait);
> > > > +}
> > > > +
> > > > +static int prestera_cmd(struct prestera_switch *sw,
> > > > + enum prestera_cmd_type_t type,
> > > > + struct prestera_msg_cmd *cmd, size_t clen)
> > > > +{
> > > > + struct prestera_msg_common_resp resp;
> > > > +
> > > > + return prestera_cmd_ret(sw, type, cmd, clen, &resp.ret, sizeof(resp));
> > > > +}
> > > > +
> > > > +static int prestera_fw_parse_port_evt(u8 *msg, struct prestera_event *evt)
> > > > +{
> > > > + struct prestera_msg_event_port *hw_evt;
> > > > +
> > > > + hw_evt = (struct prestera_msg_event_port *)msg;
> > > > +
> > > > + evt->port_evt.port_id = hw_evt->port_id;
> > > > +
> > > > + if (evt->id == PRESTERA_PORT_EVENT_STATE_CHANGED)
> > > > + evt->port_evt.data.oper_state = hw_evt->param.oper_state;
> > > > + else
> > > > + return -EINVAL;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static struct prestera_fw_evt_parser {
> > > > + int (*func)(u8 *msg, struct prestera_event *evt);
> > > > +} fw_event_parsers[PRESTERA_EVENT_TYPE_MAX] = {
> > > > + [PRESTERA_EVENT_TYPE_PORT] = {.func = prestera_fw_parse_port_evt},
> > > > +};
> > > > +
> > > > +static struct prestera_fw_event_handler *
> > > > +__find_event_handler(const struct prestera_switch *sw,
> > > > + enum prestera_event_type type)
> > > > +{
> > > > + struct prestera_fw_event_handler *eh;
> > > > +
> > > > + list_for_each_entry_rcu(eh, &sw->event_handlers, list) {
> > >
> > > It does not look that this is always called under RCU which will result
> > > in various splats. For example in the following call path:
> > >
> > > prestera_device_register()
> > > prestera_switch_init()
> > > prestera_event_handlers_register()
> > > prestera_hw_event_handler_register()
> > > __find_event_handler()
> > >
> > > You want to make sure that you are testing with various debug options.
> > > For example:
> > So, right. Currently this prestera_hw_event_handler_register is called
> > synchronously and as I understand does not require additional locking
> > when use list rcu API. I will check with these options which you
> > suggested.
> >
> > >
> > > # Debug options
> > > ## General debug options
> > > config_enable CONFIG_PREEMPT
> > > config_enable CONFIG_DEBUG_PREEMPT
> > > config_enable CONFIG_DEBUG_INFO
> > > config_enable CONFIG_UNWINDER_ORC
> > > config_enable CONFIG_DYNAMIC_DEBUG
> > > config_enable CONFIG_DEBUG_NOTIFIERS
> > > ## Lock debugging
> > > config_enable CONFIG_LOCKDEP
> > > config_enable CONFIG_PROVE_LOCKING
> > > config_enable CONFIG_DEBUG_ATOMIC_SLEEP
> > > config_enable CONFIG_PROVE_RCU
> > > config_enable CONFIG_DEBUG_MUTEXES
> > > config_enable CONFIG_DEBUG_SPINLOCK
> > > config_enable CONFIG_LOCK_STAT
> > > ## Memory debugging
> > > config_enable CONFIG_DEBUG_VM
> > > config_enable CONFIG_FORTIFY_SOURCE
> > > config_enable CONFIG_KASAN
> > > config_enable CONFIG_KASAN_EXTRA
> > > config_enable CONFIG_KASAN_INLINE
> > > ## Reference counting debugging
> > > config_enable CONFIG_REFCOUNT_FULL
> > > ## Lockups debugging
> > > config_enable CONFIG_LOCKUP_DETECTOR
> > > config_enable CONFIG_SOFTLOCKUP_DETECTOR
> > > config_enable CONFIG_HARDLOCKUP_DETECTOR
> > > config_enable CONFIG_DETECT_HUNG_TASK
> > > config_enable CONFIG_WQ_WATCHDOG
> > > config_enable CONFIG_DETECT_HUNG_TASK
> > > config_set_val CONFIG_DEFAULT_HUNG_TASK_TIMEOUT 120
> > > ## Undefined behavior debugging
> > > config_enable CONFIG_UBSAN
> > > config_enable CONFIG_UBSAN_SANITIZE_ALL
> > > config_disable CONFIG_UBSAN_ALIGNMENT
> > > config_disable CONFIG_UBSAN_NULL
> > > ## Memory debugging
> > > config_enable CONFIG_SLUB_DEBUG
> > > config_enable CONFIG_SLUB_DEBUG_ON
> > > config_enable CONFIG_DEBUG_PAGEALLOC
> > > config_enable CONFIG_DEBUG_KMEMLEAK
> > > config_disable CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF
> > > config_set_val CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE 8192
> > > config_enable CONFIG_DEBUG_STACKOVERFLOW
> > > config_enable CONFIG_DEBUG_LIST
> > > config_enable CONFIG_DEBUG_PER_CPU_MAPS
> > > config_set_val CONFIG_DEBUG_OBJECTS_ENABLE_DEFAULT 1
> > > config_enable CONFIG_DEBUG_OBJECTS
> > > config_enable CONFIG_DEBUG_OBJECTS_FREE
> > > config_enable CONFIG_DEBUG_OBJECTS_TIMERS
> > > config_enable CONFIG_DEBUG_OBJECTS_WORK
> > > config_enable CONFIG_DEBUG_OBJECTS_PERCPU_COUNTER
> > > config_enable CONFIG_DMA_API_DEBUG
> > > ## Lock debugging
> > > config_enable CONFIG_DEBUG_LOCK_ALLOC
> > > config_enable CONFIG_PROVE_LOCKING
> > > config_enable CONFIG_LOCK_STAT
> > > config_enable CONFIG_DEBUG_OBJECTS_RCU_HEAD
> > > config_enable CONFIG_SPARSE_RCU_POINTER
> > >
> > > > + if (eh->type == type)
> > > > + return eh;
> > > > + }
> > > > +
> > > > + return NULL;
> > > > +}
> > > > +
> > > > +static int prestera_find_event_handler(const struct prestera_switch *sw,
> > > > + enum prestera_event_type type,
> > > > + struct prestera_fw_event_handler *eh)
> > > > +{
> > > > + struct prestera_fw_event_handler *tmp;
> > > > + int err = 0;
> > > > +
> > > > + rcu_read_lock();
> > > > + tmp = __find_event_handler(sw, type);
> > > > + if (tmp)
> > > > + *eh = *tmp;
> > > > + else
> > > > + err = -EEXIST;
> > > > + rcu_read_unlock();
> > > > +
> > > > + return err;
> > > > +}
> > > > +
> > > > +static int prestera_evt_recv(struct prestera_device *dev, u8 *buf, size_t size)
> > > > +{
> > > > + struct prestera_msg_event *msg = (struct prestera_msg_event *)buf;
> > > > + struct prestera_switch *sw = dev->priv;
> > > > + struct prestera_fw_event_handler eh;
> > > > + struct prestera_event evt;
> > > > + int err;
> > > > +
> > > > + if (msg->type >= PRESTERA_EVENT_TYPE_MAX)
> > > > + return -EINVAL;
> > > > +
> > > > + err = prestera_find_event_handler(sw, msg->type, &eh);
> > > > +
> > > > + if (err || !fw_event_parsers[msg->type].func)
> > > > + return 0;
> > > > +
> > > > + evt.id = msg->id;
> > > > +
> > > > + err = fw_event_parsers[msg->type].func(buf, &evt);
> > > > + if (!err)
> > > > + eh.func(sw, &evt, eh.arg);
> > > > +
> > > > + return err;
> > > > +}
> > > > +
> > > > +static void prestera_pkt_recv(struct prestera_device *dev)
> > > > +{
> > > > + struct prestera_switch *sw = dev->priv;
> > > > + struct prestera_fw_event_handler eh;
> > > > + struct prestera_event ev;
> > > > + int err;
> > > > +
> > > > + ev.id = PRESTERA_RXTX_EVENT_RCV_PKT;
> > > > +
> > > > + err = prestera_find_event_handler(sw, PRESTERA_EVENT_TYPE_RXTX, &eh);
> > > > + if (err)
> > > > + return;
> > > > +
> > > > + eh.func(sw, &ev, eh.arg);
> > > > +}
> > > > +
> > > > +int prestera_hw_port_info_get(const struct prestera_port *port,
> > > > + u16 *fp_id, u32 *hw_id, u32 *dev_id)
> > > > +{
> > > > + struct prestera_msg_port_info_resp resp;
> > > > + struct prestera_msg_port_info_req req = {
> > > > + .port = port->id
> > > > + };
> > > > + int err;
> > > > +
> > > > + err = prestera_cmd_ret(port->sw, PRESTERA_CMD_TYPE_PORT_INFO_GET,
> > > > + &req.cmd, sizeof(req), &resp.ret, sizeof(resp));
> > > > + if (err)
> > > > + return err;
> > > > +
> > > > + *hw_id = resp.hw_id;
> > > > + *dev_id = resp.dev_id;
> > > > + *fp_id = resp.fp_id;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +int prestera_hw_switch_mac_set(struct prestera_switch *sw, char *mac)
> > > > +{
> > > > + struct prestera_msg_switch_attr_req req = {
> > > > + .attr = PRESTERA_CMD_SWITCH_ATTR_MAC,
> > > > + };
> > > > +
> > > > + memcpy(req.param.mac, mac, sizeof(req.param.mac));
> > > > +
> > > > + return prestera_cmd(sw, PRESTERA_CMD_TYPE_SWITCH_ATTR_SET,
> > > > + &req.cmd, sizeof(req));
> > > > +}
> > > > +
> > > > +int prestera_hw_switch_init(struct prestera_switch *sw)
> > > > +{
> > > > + struct prestera_msg_switch_init_resp resp;
> > > > + struct prestera_msg_common_req req;
> > > > + int err;
> > > > +
> > > > + INIT_LIST_HEAD(&sw->event_handlers);
> > > > +
> > > > + err = prestera_cmd_ret_wait(sw, PRESTERA_CMD_TYPE_SWITCH_INIT,
> > > > + &req.cmd, sizeof(req),
> > > > + &resp.ret, sizeof(resp),
> > > > + PRESTERA_SWITCH_INIT_TIMEOUT);
> > > > + if (err)
> > > > + return err;
> > > > +
> > > > + sw->id = resp.switch_id;
> > > > + sw->port_count = resp.port_count;
> > > > + sw->mtu_min = PRESTERA_MIN_MTU;
> > > > + sw->mtu_max = resp.mtu_max;
> > > > + sw->dev->recv_msg = prestera_evt_recv;
> > > > + sw->dev->recv_pkt = prestera_pkt_recv;
> > > > +
> > > > + return 0;
> > > > +}
> > >
> > > Consider adding prestera_hw_switch_fini() that verifies that
> > > '&sw->event_handlers' is empty.
> > >
> > As I see it can just warn on if list is not empty, right ?
>
> Yes, something like:
>
> WARN_ON(!list_empty(&sw->event_handlers));
>
> >
> > > > +
> > > > +int prestera_hw_port_state_set(const struct prestera_port *port,
> > > > + bool admin_state)
> > > > +{
> > > > + struct prestera_msg_port_attr_req req = {
> > > > + .attr = PRESTERA_CMD_PORT_ATTR_ADMIN_STATE,
> > > > + .port = port->hw_id,
> > > > + .dev = port->dev_id,
> > > > + .param = {.admin_state = admin_state}
> > > > + };
> > > > +
> > > > + return prestera_cmd(port->sw, PRESTERA_CMD_TYPE_PORT_ATTR_SET,
> > > > + &req.cmd, sizeof(req));
> > > > +}
> > > > +
> >
> > [...]
> >
> > > > +
> > > > +struct prestera_port *prestera_port_find_by_hwid(struct prestera_switch *sw,
> > > > + u32 dev_id, u32 hw_id)
> > > > +{
> > > > + struct prestera_port *port;
> > > > +
> > > > + rcu_read_lock();
> > > > +
> > > > + list_for_each_entry_rcu(port, &sw->port_list, list) {
> > > > + if (port->dev_id == dev_id && port->hw_id == hw_id) {
> > > > + rcu_read_unlock();
> > > > + return port;
> > >
> > > This does not look correct. You call rcu_read_unlock(), but do not take
> > > a reference on the object, so nothing prevents it from being freed.
> > >
> > Currently there is no logic which can dynamically create/delete the
> > port, so how do you think may I just use synchronize_rcu() when freeing
> > the port instance on module unlolad ?
>
> I don't understand what RCU is meant to protect here. You call
> rcu_read_unlock() and then return the port. Calling synchronize_rcu()
> before freeing the port will not prevent the caller of
> prestera_port_find_by_hwid() from accessing freed memory.
I just removed these rcu things, and looks like they does not
needed currently.
>
> >
> > > > + }
> > > > + }
> > > > +
> > > > + rcu_read_unlock();
> > > > +
> > > > + return NULL;
> > > > +}
> > > > +
> > > > +static struct prestera_port *prestera_find_port(struct prestera_switch *sw,
> > > > + u32 port_id)
> > > > +{
> > > > + struct prestera_port *port;
> > > > +
> > > > + rcu_read_lock();
> > > > +
> > > > + list_for_each_entry_rcu(port, &sw->port_list, list) {
> > > > + if (port->id == port_id)
> > > > + break;
> > > > + }
> > > > +
> > > > + rcu_read_unlock();
> > > > +
> > > > + return port;
> > >
> > > Same here.
> > >
> > > > +}
> > > > +
> > > > +static int prestera_port_state_set(struct net_device *dev, bool is_up)
> > > > +{
> > > > + struct prestera_port *port = netdev_priv(dev);
> > > > + int err;
> > > > +
> > > > + if (!is_up)
> > > > + netif_stop_queue(dev);
> > > > +
> > > > + err = prestera_hw_port_state_set(port, is_up);
> > > > +
> > > > + if (is_up && !err)
> > > > + netif_start_queue(dev);
> > > > +
> > > > + return err;
> > > > +}
> > > > +
> >
> > [...]
> >
> > > > +static void prestera_port_destroy(struct prestera_port *port)
> > > > +{
> > > > + struct net_device *dev = port->dev;
> > > > +
> > > > + cancel_delayed_work_sync(&port->cached_hw_stats.caching_dw);
> > > > + unregister_netdev(dev);
> > > > +
> > > > + list_del_rcu(&port->list);
> > > > +
> > >
> > > I'm not sure what is the point of these blank lines. Best to remove
> > > them.
> > Will fix it.
> >
> > >
> > > > + free_netdev(dev);
> > > > +}
> > > > +
> > > > +static void prestera_destroy_ports(struct prestera_switch *sw)
> > > > +{
> > > > + struct prestera_port *port, *tmp;
> > > > + struct list_head remove_list;
> > > > +
> > > > + INIT_LIST_HEAD(&remove_list);
> > > > +
> > > > + list_splice_init(&sw->port_list, &remove_list);
> > > > +
> > > > + list_for_each_entry_safe(port, tmp, &remove_list, list)
> > > > + prestera_port_destroy(port);
> > > > +}
> > > > +
> > > > +static int prestera_create_ports(struct prestera_switch *sw)
> > > > +{
> > > > + u32 port;
> > > > + int err;
> > > > +
> > > > + for (port = 0; port < sw->port_count; port++) {
> > > > + err = prestera_port_create(sw, port);
> > > > + if (err)
> > > > + goto err_ports_init;
> > >
> > > err_port_create ?
> > >
> > > > + }
> > > > +
> > > > + return 0;
> > > > +
> > > > +err_ports_init:
> > > > + prestera_destroy_ports(sw);
> > >
> > > I'm not a fan of this construct. I find it best to always do proper
> > > rollback in the error path. Then you can always maintain init() being
> > > followed by fini() which is much easier to review.
> > As I understand you meant to move this destroy_ports() recovery to the
> > error path in xxx_switch_init() ?
>
> No, I mean do proper rollback in this error path by calling
> prestera_port_destroy() for each port you created thus far.
>
> Then move prestera_destroy_ports() after prestera_create_ports(),
> instead of before it.
But it will look same as prestera_destroy_ports(), do you think
this is not a problem to have a same logic doubled ?
>
> >
> > >
> > > > + return err;
> > > > +}
> > > > +
> > > > +static void prestera_port_handle_event(struct prestera_switch *sw,
> > > > + struct prestera_event *evt, void *arg)
> > > > +{
> > > > + struct delayed_work *caching_dw;
> > > > + struct prestera_port *port;
> > > > +
> > > > + port = prestera_find_port(sw, evt->port_evt.port_id);
> > > > + if (!port)
> > > > + return;
> > > > +
> > > > + caching_dw = &port->cached_hw_stats.caching_dw;
> > > > +
> > > > + if (evt->id == PRESTERA_PORT_EVENT_STATE_CHANGED) {
> > > > + if (evt->port_evt.data.oper_state) {
> > > > + netif_carrier_on(port->dev);
> > > > + if (!delayed_work_pending(caching_dw))
> > > > + queue_delayed_work(prestera_wq, caching_dw, 0);
> > > > + } else {
> > > > + netif_carrier_off(port->dev);
> > > > + if (delayed_work_pending(caching_dw))
> > > > + cancel_delayed_work(caching_dw);
> > > > + }
> > > > + }
> > > > +}
> > > > +
> > > > +static void prestera_event_handlers_unregister(struct prestera_switch *sw)
> > > > +{
> > > > + prestera_hw_event_handler_unregister(sw, PRESTERA_EVENT_TYPE_PORT,
> > > > + prestera_port_handle_event);
> > > > +}
> > >
> > > Please reverse the order so that register() is first.
> > >
> > > > +
> > > > +static int prestera_event_handlers_register(struct prestera_switch *sw)
> > > > +{
> > > > + return prestera_hw_event_handler_register(sw, PRESTERA_EVENT_TYPE_PORT,
> > > > + prestera_port_handle_event,
> > > > + NULL);
> > > > +}
> > > > +
> > > > +static int prestera_switch_set_base_mac_addr(struct prestera_switch *sw)
> > > > +{
> > > > + struct device_node *base_mac_np;
> > > > + struct device_node *np;
> > > > +
> > > > + np = of_find_compatible_node(NULL, NULL, "marvell,prestera");
> > > > + if (np) {
> > > > + base_mac_np = of_parse_phandle(np, "base-mac-provider", 0);
> > > > + if (base_mac_np) {
> > > > + const char *base_mac;
> > > > +
> > > > + base_mac = of_get_mac_address(base_mac_np);
> > > > + of_node_put(base_mac_np);
> > > > + if (!IS_ERR(base_mac))
> > > > + ether_addr_copy(sw->base_mac, base_mac);
> > > > + }
> > > > + }
> > > > +
> > > > + if (!is_valid_ether_addr(sw->base_mac)) {
> > > > + eth_random_addr(sw->base_mac);
> > > > + dev_info(sw->dev->dev, "using random base mac address\n");
> > > > + }
> > > > +
> > > > + return prestera_hw_switch_mac_set(sw, sw->base_mac);
> > > > +}
> > > > +
> > > > +static int prestera_switch_init(struct prestera_switch *sw)
> > > > +{
> > > > + int err;
> > > > +
> > > > + err = prestera_hw_switch_init(sw);
> > > > + if (err) {
> > > > + dev_err(prestera_dev(sw), "Failed to init Switch device\n");
> > > > + return err;
> > > > + }
> > > > +
> > > > + INIT_LIST_HEAD(&sw->port_list);
> > > > +
> > > > + err = prestera_switch_set_base_mac_addr(sw);
> > > > + if (err)
> > > > + return err;
> > > > +
> > > > + err = prestera_rxtx_switch_init(sw);
> > > > + if (err)
> > > > + return err;
> > > > +
> > > > + err = prestera_event_handlers_register(sw);
> > > > + if (err)
> > > > + return err;
> > > > +
> > > > + err = prestera_create_ports(sw);
> > > > + if (err)
> > > > + goto err_ports_create;
> > > > +
> > > > + return 0;
> > > > +
> > > > +err_ports_create:
> > > > + prestera_event_handlers_unregister(sw);
> > > > +
> > >
> > > You are missing prestera_rxtx_switch_fini() here... With init() always
> > > followed by fini() you can easily tell that the error path is not
> > > symmetric with fini().
> > Yeah, for some reason I mixed this fix with Switchdev patch, I will fix
> > this.
> >
> > >
> > > > + return err;
> > > > +}
> > > > +
> > > > +static void prestera_switch_fini(struct prestera_switch *sw)
> > > > +{
> > > > + prestera_destroy_ports(sw);
> > > > + prestera_event_handlers_unregister(sw);
> > > > + prestera_rxtx_switch_fini(sw);
> > > > +}
> > > > +
> > > > +int prestera_device_register(struct prestera_device *dev)
> > > > +{
> > > > + struct prestera_switch *sw;
> > > > + int err;
> > > > +
> > > > + sw = kzalloc(sizeof(*sw), GFP_KERNEL);
> > > > + if (!sw)
> > > > + return -ENOMEM;
> > > > +
> > > > + dev->priv = sw;
> > > > + sw->dev = dev;
> > > > +
> > > > + err = prestera_switch_init(sw);
> > > > + if (err) {
> > > > + kfree(sw);
> > > > + return err;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +EXPORT_SYMBOL(prestera_device_register);
> > > > +
> > > > +void prestera_device_unregister(struct prestera_device *dev)
> > > > +{
> > > > + struct prestera_switch *sw = dev->priv;
> > > > +
> > > > + prestera_switch_fini(sw);
> > > > + kfree(sw);
> > > > +}
> >
> > [...]
> >
> > > > +int prestera_rxtx_port_init(struct prestera_port *port)
> > > > +{
> > > > + int err;
> > > > +
> > > > + err = prestera_hw_rxtx_port_init(port);
> > > > + if (err)
> > > > + return err;
> > > > +
> > > > + port->dev->needed_headroom = PRESTERA_DSA_HLEN + ETH_FCS_LEN;
> > >
> > > Why do you need headroom for FCS?
> > >
> > I had issue when the SKB did not have additional bytes for the FCS, so I
> > thought to added this to the needed_headroom to be sure.
>
> But FCS is at the end of the frame. 'needed_headroom' is for headers you
> need to prepend to each frame.
>
> Which issues did you have? In mlxsw FCS is computed in hardware and we
> pass the length of the frame without the FCS when posting the frame to
> hardware.
Looks like it really not needed.
>
> >
> > > > + return 0;
> > > > +}
> > > > +
> > > > +netdev_tx_t prestera_rxtx_xmit(struct prestera_port *port, struct sk_buff *skb)
> > > > +{
> > > > + struct prestera_dsa dsa;
> > > > +
> > > > + dsa.hw_dev_num = port->dev_id;
> > > > + dsa.port_num = port->hw_id;
> > > > +
> > > > + if (skb_cow_head(skb, PRESTERA_DSA_HLEN) < 0)
> > > > + return NET_XMIT_DROP;
> > > > +
> > > > + skb_push(skb, PRESTERA_DSA_HLEN);
> > > > + memmove(skb->data, skb->data + PRESTERA_DSA_HLEN, 2 * ETH_ALEN);
> > > > +
> > > > + if (prestera_dsa_build(&dsa, skb->data + 2 * ETH_ALEN) != 0)
> > > > + return NET_XMIT_DROP;
> > > > +
> > > > + return prestera_sdma_xmit(&port->sw->rxtx->sdma, skb);
> > > > +}
> > > > diff --git a/drivers/net/ethernet/marvell/prestera/prestera_rxtx.h b/drivers/net/ethernet/marvell/prestera/prestera_rxtx.h
> > > > new file mode 100644
> > > > index 000000000000..bbbadfa5accf
> > > > --- /dev/null
> > > > +++ b/drivers/net/ethernet/marvell/prestera/prestera_rxtx.h
> > > > @@ -0,0 +1,21 @@
> > > > +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
> > > > + *
> > > > + * Copyright (c) 2019-2020 Marvell International Ltd. All rights reserved.
> > > > + *
> > > > + */
> > > > +
> > > > +#ifndef _PRESTERA_RXTX_H_
> > > > +#define _PRESTERA_RXTX_H_
> > > > +
> > > > +#include <linux/netdevice.h>
> > > > +
> > > > +#include "prestera.h"
> > > > +
> > > > +int prestera_rxtx_switch_init(struct prestera_switch *sw);
> > > > +void prestera_rxtx_switch_fini(struct prestera_switch *sw);
> > > > +
> > > > +int prestera_rxtx_port_init(struct prestera_port *port);
> > > > +
> > > > +netdev_tx_t prestera_rxtx_xmit(struct prestera_port *port, struct sk_buff *skb);
> > > > +
> > > > +#endif /* _PRESTERA_RXTX_H_ */
> > > > --
> > > > 2.17.1
> > > >
> >
> > Thank you for review.
^ permalink raw reply
* Re: [PATCH net-next 1/2] devlink: Add support for board_serial_number to info_get cb.
From: Jiri Pirko @ 2020-06-20 13:06 UTC (permalink / raw)
To: Vasundhara Volam; +Cc: davem, netdev, michael.chan, kuba, jiri, jacob.e.keller
In-Reply-To: <1592640947-10421-2-git-send-email-vasundhara-v.volam@broadcom.com>
Sat, Jun 20, 2020 at 10:15:46AM CEST, vasundhara-v.volam@broadcom.com wrote:
>Board serial number is a serial number, often available in PCI
>*Vital Product Data*.
>
>Also, update devlink-info.rst documentation file.
>
>Cc: Jiri Pirko <jiri@mellanox.com>
>Cc: Jakub Kicinski <kuba@kernel.org>
>Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
>Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next 2/2] bnxt_en: Add board_serial_number field to info_get cb
From: Jiri Pirko @ 2020-06-20 13:07 UTC (permalink / raw)
To: Vasundhara Volam; +Cc: davem, netdev, michael.chan, kuba, jiri, jacob.e.keller
In-Reply-To: <1592640947-10421-3-git-send-email-vasundhara-v.volam@broadcom.com>
Sat, Jun 20, 2020 at 10:15:47AM CEST, vasundhara-v.volam@broadcom.com wrote:
>Add board_serial_number field info to info_get cb via devlink,
>if driver can fetch the information from the device.
>
>Cc: Jiri Pirko <jiri@mellanox.com>
>Cc: Jakub Kicinski <kuba@kernel.org>
>Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
>Reviewed-by: Michael Chan <michael.chan@broadcom.com>
>---
> drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>index a812beb..16eca3b 100644
>--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>@@ -411,6 +411,13 @@ static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
> return rc;
> }
>
>+ if (strlen(bp->board_serialno)) {
>+ rc = devlink_info_board_serial_number_put(req,
No need for linebreak here.
>+ bp->board_serialno);
>+ if (rc)
>+ return rc;
>+ }
>+
> sprintf(buf, "%X", bp->chip_num);
> rc = devlink_info_version_fixed_put(req,
> DEVLINK_INFO_VERSION_GENERIC_ASIC_ID, buf);
>--
>1.8.3.1
>
^ permalink raw reply
* [Resend PATCH net] bridge: uapi: mrp: Fix MRP_PORT_ROLE
From: Horatiu Vultur @ 2020-06-20 13:14 UTC (permalink / raw)
To: nikolay, davem, UNGLinuxDriver, linux-kernel, netdev; +Cc: Horatiu Vultur
Currently the MRP_PORT_ROLE_NONE has the value 0x2 but this is in conflict
with the IEC 62439-2 standard. The standard defines the following port
roles: primary (0x0), secondary(0x1), interconnect(0x2).
Therefore remove the port role none.
Fixes: 4714d13791f831 ("bridge: uapi: mrp: Add mrp attributes.")
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
include/uapi/linux/mrp_bridge.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/uapi/linux/mrp_bridge.h b/include/uapi/linux/mrp_bridge.h
index 84f15f48a7cb1..bee3665402129 100644
--- a/include/uapi/linux/mrp_bridge.h
+++ b/include/uapi/linux/mrp_bridge.h
@@ -36,7 +36,6 @@ enum br_mrp_port_state_type {
enum br_mrp_port_role_type {
BR_MRP_PORT_ROLE_PRIMARY,
BR_MRP_PORT_ROLE_SECONDARY,
- BR_MRP_PORT_ROLE_NONE,
};
enum br_mrp_tlv_header_type {
--
2.26.2
^ permalink raw reply related
* Re: [net-next 0/6] net: marvell: prestera: Add Switchdev driver for Prestera family ASIC device 98DX326x (AC3x)
From: Vadym Kochan @ 2020-06-20 13:25 UTC (permalink / raw)
To: Jiri Pirko
Cc: Ido Schimmel, David S. Miller, Jakub Kicinski, Jiri Pirko,
Ido Schimmel, Andrew Lunn, Oleksandr Mazur, Serhiy Boiko,
Serhiy Pshyk, Volodymyr Mytnyk, Taras Chornyi, Andrii Savka,
netdev, linux-kernel, Mickey Rachamim
In-Reply-To: <20200601062417.GC2282@nanopsycho>
Hi Jiri, Ido,
On Mon, Jun 01, 2020 at 08:24:17AM +0200, Jiri Pirko wrote:
> Sat, May 30, 2020 at 05:54:29PM CEST, idosch@idosch.org wrote:
> >On Sat, May 30, 2020 at 05:52:31PM +0300, Vadym Kochan wrote:
>
> [...]
>
>
> >> > WARNING: do not add new typedefs
> >> > #1064: FILE: drivers/net/ethernet/marvell/prestera/prestera_hw.h:32:
> >> > +typedef void (*prestera_event_cb_t)
> >> I may be wrong, as I remember Jiri suggested it and looks like
> >> it makes sense. I really don't have strong opinion about this.
> >
> >OK, so I'll let Jiri comment when he is back at work.
>
> I was not aware of this warning, but for function callbacks, I think it
> is very handy to have them as typedef instead of repeating the prototype
> over and over. For that, I don't think this warning makes sense.
>
> [...]
As I said I have no strong opinion on this, but Jiri's suggestion makes
sense. It looks like typedef check was mostly about 'struct' and native
types definition.
Regards,
Vadym Kochan
^ permalink raw reply
* Re: Question on DSA switches, IGMP forwarding and switchdev
From: Andrew Lunn @ 2020-06-20 14:35 UTC (permalink / raw)
To: Daniel Mack
Cc: Jason Cobham, netdev@vger.kernel.org, Ido Schimmel, Jiri Pirko,
Ivan Vecera, Florian Fainelli
In-Reply-To: <eb6b5f84-2a5a-1938-0657-0eac9f2390df@zonque.org>
> So yes, we can read the code here, but I'm wondering which packet types
> would then get this flag set, and which won't. Because in case of
> IGMP/MLD, the packets are in fact forwarded, but the meaning of the flag
> in skb is to prevent the skb from being forwarded further, which seems
> wrong in all cases.
>
> I'm thinking maybe the flag should never be set?
It is a while since i did much with multicast, so please correct me
when i'm wrong...
IGMP can use different group addresses as far as i remember.
Join/leave uses the group address of interest. But query can use
224.0.0.1 or the group address.
The bridge should learn from joins, either spontaneous or as a reply
to a query. When it sees a join, it should add a multicast FDB to the
hardware for the group, so traffic is forwarded out that port.
So for real multicast traffic, we do want the flag set, the hardware
should be doing the forwarding. If we don't set the flag, we end up
with duplication when the SW bridge also forwards the multicast
traffic.
For IGMP/MLD itself, we probably need to see what the switch does. For
IGMP using the group address, does the multicast FDB rule match and
cause the hardware to forward the IGMP? If yes, then we need the flag
set, otherwise the IGMP gets duplicated. If no, then we don't want the
flag set, and leave the SW bridge to do the forwarding, or reply
itself if it is the querier.
For IGMP using 224.0.0.1, do we ever get a multicast FDB added for
that?
It sounds like you have a better test setup than i have. Can you play
with this?
6352 uses the EDSA tagger. But the same bits exist in the DSA tag,
which the 6390 uses, due to incompatibility reasons. So it would be
nice to extend both taggers.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH/RFC 1/5] dt-bindings: net: renesas,ravb: Document internal clock delay properties
From: Andrew Lunn @ 2020-06-20 14:43 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Geert Uytterhoeven, Sergei Shtylyov, David S . Miller,
Jakub Kicinski, Rob Herring, Philippe Schenker, Florian Fainelli,
Heiner Kallweit, Kazuya Mizuguchi, Wolfram Sang, netdev,
devicetree, linux-renesas-soc
In-Reply-To: <e6d0bfc5-9d75-2dc9-2bfa-671c32cb0b7c@rempel-privat.de>
On Sat, Jun 20, 2020 at 07:47:16AM +0200, Oleksij Rempel wrote:
> Hi Geert,
>
> Am 19.06.20 um 21:15 schrieb Geert Uytterhoeven:
> > Some EtherAVB variants support internal clock delay configuration, which
> > can add larger delays than the delays that are typically supported by
> > the PHY (using an "rgmii-*id" PHY mode, and/or "[rt]xc-skew-ps"
> > properties).
> >
> > Add properties for configuring the internal MAC delays.
> > These properties are mandatory, even when specified as zero, to
> > distinguish between old and new DTBs.
> >
> > Update the example accordingly.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > .../devicetree/bindings/net/renesas,ravb.txt | 29 ++++++++++---------
> > 1 file changed, 16 insertions(+), 13 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/renesas,ravb.txt b/Documentation/devicetree/bindings/net/renesas,ravb.txt
> > index 032b76f14f4fdb38..488ada78b6169b8e 100644
> > --- a/Documentation/devicetree/bindings/net/renesas,ravb.txt
> > +++ b/Documentation/devicetree/bindings/net/renesas,ravb.txt
> > @@ -64,6 +64,18 @@ Optional properties:
> > AVB_LINK signal.
> > - renesas,ether-link-active-low: boolean, specify when the AVB_LINK signal is
> > active-low instead of normal active-high.
> > +- renesas,rxc-delay-ps: Internal RX clock delay.
>
> may be it make sense to add a generic delay property for MACs and PHYs?
See Dan Murphys "RGMII Internal delay common property" patchset. That
patchset is addressing the PHY side. Maybe we can build on that to
address the MAC side?
Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 1/8] net: phy: add support for a common probe between shared PHYs
From: Andrew Lunn @ 2020-06-20 14:49 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, f.fainelli, hkallweit1, richardcochran, alexandre.belloni,
UNGLinuxDriver, netdev, linux-kernel, thomas.petazzoni,
allan.nielsen, foss
In-Reply-To: <20200619122300.2510533-2-antoine.tenart@bootlin.com>
On Fri, Jun 19, 2020 at 02:22:53PM +0200, Antoine Tenart wrote:
> Shared PHYs (PHYs in the same hardware package) may have shared
> registers and their drivers would usually need to share information.
> There is currently a way to have a shared (part of the) init, by using
> phy_package_init_once(). This patch extends the logic to share parts of
> the probe to allow sharing the initialization of locks or resources
> retrieval.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 2/8] net: phy: mscc: fix copyright and author information in MACsec
From: Andrew Lunn @ 2020-06-20 14:50 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, f.fainelli, hkallweit1, richardcochran, alexandre.belloni,
UNGLinuxDriver, netdev, linux-kernel, thomas.petazzoni,
allan.nielsen, foss
In-Reply-To: <20200619122300.2510533-3-antoine.tenart@bootlin.com>
On Fri, Jun 19, 2020 at 02:22:54PM +0200, Antoine Tenart wrote:
> All headers in the MSCC PHY driver have been copied and pasted from the
> original mscc.c file. However the information is not necessarily
> correct, as in the MACsec support. Fix this.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 3/8] net: phy: mscc: remove the TR CLK disable magic value
From: Andrew Lunn @ 2020-06-20 14:52 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, f.fainelli, hkallweit1, richardcochran, alexandre.belloni,
UNGLinuxDriver, netdev, linux-kernel, thomas.petazzoni,
allan.nielsen, foss
In-Reply-To: <20200619122300.2510533-4-antoine.tenart@bootlin.com>
On Fri, Jun 19, 2020 at 02:22:55PM +0200, Antoine Tenart wrote:
> From: Quentin Schulz <quentin.schulz@bootlin.com>
>
> This patch adds a define for the 0x8000 magic value used to perform
> enable/disable actions on the "token ring clock". The patch is only
> cosmetic.
I assume this is not 802.5 Token Ring?
>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* [PATCH net v3] net: phy: smsc: fix printing too many logs
From: Dejin Zheng @ 2020-06-20 14:55 UTC (permalink / raw)
To: andrew, f.fainelli, hkallweit1, linux, davem, kuba, netdev
Cc: linux-kernel, Dejin Zheng, Kevin Groeneveld
Commit 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout()
to simplify the code") will print a lot of logs as follows when Ethernet
cable is not connected:
[ 4.473105] SMSC LAN8710/LAN8720 2188000.ethernet-1:00: lan87xx_read_status failed: -110
When wait 640 ms for check ENERGYON bit, the timeout should not be
regarded as an actual error and an error message also should not be
printed. due to a hardware bug in LAN87XX device, it leads to unstable
detection of plugging in Ethernet cable when LAN87xx is in Energy Detect
Power-Down mode. the workaround for it involves, when the link is down,
and at each read_status() call:
- disable EDPD mode, forcing the PHY out of low-power mode
- waiting 640ms to see if we have any energy detected from the media
- re-enable entry to EDPD mode
This is presumably enough to allow the PHY to notice that a cable is
connected, and resume normal operations to negotiate with the partner.
The problem is that when no media is detected, the 640ms wait times
out and this commit was modified to prints an error message. it is an
inappropriate conversion by used phy_read_poll_timeout() to introduce
this bug. so fix this issue by use read_poll_timeout() to replace
phy_read_poll_timeout().
Fixes: 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout() to simplify the code")
Reported-by: Kevin Groeneveld <kgroeneveld@gmail.com>
Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com>
---
v2 -> v3:
- modify commit message to why need this patch. And many thanks
to Andrew and Russell for their comments.
v1 -> v2:
- add more commit message spell out what the change does.
drivers/net/phy/smsc.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 93da7d3d0954..74568ae16125 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -122,10 +122,13 @@ static int lan87xx_read_status(struct phy_device *phydev)
if (rc < 0)
return rc;
- /* Wait max 640 ms to detect energy */
- phy_read_poll_timeout(phydev, MII_LAN83C185_CTRL_STATUS, rc,
- rc & MII_LAN83C185_ENERGYON, 10000,
- 640000, true);
+ /* Wait max 640 ms to detect energy and the timeout is not
+ * an actual error.
+ */
+ read_poll_timeout(phy_read, rc,
+ rc & MII_LAN83C185_ENERGYON || rc < 0,
+ 10000, 640000, true, phydev,
+ MII_LAN83C185_CTRL_STATUS);
if (rc < 0)
return rc;
--
2.25.0
^ permalink raw reply related
* Re: [PATCH net-next v3 6/8] net: phy: mscc: timestamping and PHC support
From: Richard Cochran @ 2020-06-20 15:00 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, andrew, f.fainelli, hkallweit1, alexandre.belloni,
UNGLinuxDriver, netdev, linux-kernel, thomas.petazzoni,
allan.nielsen, foss
In-Reply-To: <20200619122300.2510533-7-antoine.tenart@bootlin.com>
On Fri, Jun 19, 2020 at 02:22:58PM +0200, Antoine Tenart wrote:
> +static void vsc85xx_dequeue_skb(struct vsc85xx_ptp *ptp)
> +{
> + struct skb_shared_hwtstamps shhwtstamps;
> + struct vsc85xx_ts_fifo fifo;
> + struct sk_buff *skb;
> + u8 skb_sig[16], *p;
> + int i, len;
> + u32 reg;
> +
> + memset(&fifo, 0, sizeof(fifo));
> + p = (u8 *)&fifo;
> +
> + reg = vsc85xx_ts_read_csr(ptp->phydev, PROCESSOR,
> + MSCC_PHY_PTP_EGR_TS_FIFO(0));
> + if (reg & PTP_EGR_TS_FIFO_EMPTY)
> + return;
> +
> + *p++ = reg & 0xff;
> + *p++ = (reg >> 8) & 0xff;
> +
> + /* Read the current FIFO item. Reading FIFO6 pops the next one. */
> + for (i = 1; i < 7; i++) {
> + reg = vsc85xx_ts_read_csr(ptp->phydev, PROCESSOR,
> + MSCC_PHY_PTP_EGR_TS_FIFO(i));
> + *p++ = reg & 0xff;
> + *p++ = (reg >> 8) & 0xff;
> + *p++ = (reg >> 16) & 0xff;
> + *p++ = (reg >> 24) & 0xff;
> + }
> +
> + len = skb_queue_len(&ptp->tx_queue);
> + if (len < 1)
> + return;
> +
> + while (len--) {
> + skb = __skb_dequeue(&ptp->tx_queue);
> + if (!skb)
> + return;
> +
> + /* Can't get the signature of the packet, won't ever
> + * be able to have one so let's dequeue the packet.
> + */
> + if (get_sig(skb, skb_sig) < 0)
> + continue;
This leaks the skb.
> + /* Check if we found the signature we were looking for. */
> + if (!memcmp(skb_sig, fifo.sig, sizeof(fifo.sig))) {
> + memset(&shhwtstamps, 0, sizeof(shhwtstamps));
> + shhwtstamps.hwtstamp = ktime_set(fifo.secs, fifo.ns);
> + skb_complete_tx_timestamp(skb, &shhwtstamps);
> +
> + return;
> + }
> +
> + /* Valid signature but does not match the one of the
> + * packet in the FIFO right now, reschedule it for later
> + * packets.
> + */
> + __skb_queue_tail(&ptp->tx_queue, skb);
> + }
> +}
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH net-next v3 5/8] net: phy: mscc: 1588 block initialization
From: Andrew Lunn @ 2020-06-20 15:10 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, f.fainelli, hkallweit1, richardcochran, alexandre.belloni,
UNGLinuxDriver, netdev, linux-kernel, thomas.petazzoni,
allan.nielsen, foss
In-Reply-To: <20200619122300.2510533-6-antoine.tenart@bootlin.com>
On Fri, Jun 19, 2020 at 02:22:57PM +0200, Antoine Tenart wrote:
> From: Quentin Schulz <quentin.schulz@bootlin.com>
>
> This patch adds the first parts of the 1588 support in the MSCC PHY,
> with registers definition and the 1588 block initialization.
>
> Those PHYs are distributed in hardware packages containing multiple
> times the PHY. The VSC8584 for example is composed of 4 PHYs. With
> hardware packages, parts of the logic is usually common and one of the
> PHY has to be used for some parts of the initialization. Following this
> logic, the 1588 blocks of those PHYs are shared between two PHYs and
> accessing the registers has to be done using the "base" PHY of the
> group. This is handled thanks to helpers in the PTP code (and locks).
> We also need the MDIO bus lock while performing a single read or write
> to the 1588 registers as the read/write are composed of multiple MDIO
> transactions (and we don't want other threads updating the page).
Locking sounds complex. I assume LOCKDEP was your friend in getting
this correct and deadlock free.
> + /* For multiple port PHYs; the MDIO address of the base PHY in the
> + * pair of two PHYs that share a 1588 engine. PHY0 and PHY2 are coupled.
> + * PHY1 and PHY3 as well. PHY0 and PHY1 are base PHYs for their
> + * respective pair.
There are some evil hardware engineers out there :-(
It would be good it Richard gave this code a once over.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 6/8] net: phy: mscc: timestamping and PHC support
From: Andrew Lunn @ 2020-06-20 15:16 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, f.fainelli, hkallweit1, richardcochran, alexandre.belloni,
UNGLinuxDriver, netdev, linux-kernel, thomas.petazzoni,
allan.nielsen, foss
In-Reply-To: <20200619122300.2510533-7-antoine.tenart@bootlin.com>
> + /* Retrieve the shared load/save GPIO. Request it as non exclusive as
> + * the same GPIO can be requested by all the PHYs of the same package.
> + * Ths GPIO must be used with the phc_lock taken (the lock is shared
This
Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 6/8] net: phy: mscc: timestamping and PHC support
From: Andrew Lunn @ 2020-06-20 15:21 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, f.fainelli, hkallweit1, richardcochran, alexandre.belloni,
UNGLinuxDriver, netdev, linux-kernel, thomas.petazzoni,
allan.nielsen, foss
In-Reply-To: <20200619122300.2510533-7-antoine.tenart@bootlin.com>
> + /* Retrieve the shared load/save GPIO. Request it as non exclusive as
> + * the same GPIO can be requested by all the PHYs of the same package.
> + * Ths GPIO must be used with the phc_lock taken (the lock is shared
> + * between all PHYs).
> + */
> + vsc8531->load_save = devm_gpiod_get_optional(&phydev->mdio.dev, "load-save",
> + GPIOD_FLAGS_BIT_NONEXCLUSIVE |
> + GPIOD_OUT_LOW);
> + if (IS_ERR(vsc8531->load_save)) {
> + phydev_err(phydev, "Can't get load-save GPIO (%ld)\n",
> + PTR_ERR(vsc8531->load_save));
> + return PTR_ERR(vsc8531->load_save);
> + }
> +
I can understand the GPIO being optional, it is only needed when PTP
is being used. But i don't see a test anywhere that when PTP is being
used the GPIO is provided. What actually happens if it is missing and
somebody tries to use the PTP? Maybe only register the PTP parts with
the core if the GPIO has been found in DT?
Andrew
^ 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