* [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver
@ 2020-09-15 18:22 Vladimir Oltean
2020-09-15 18:22 ` [PATCH net 1/7] net: mscc: ocelot: fix race condition with TX timestamping Vladimir Oltean
` (8 more replies)
0 siblings, 9 replies; 17+ messages in thread
From: Vladimir Oltean @ 2020-09-15 18:22 UTC (permalink / raw)
To: davem, netdev
Cc: yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver, claudiu.manoil,
alexandre.belloni, andrew, vivien.didelot, f.fainelli, kuba
From: Vladimir Oltean <vladimir.oltean@nxp.com>
This is a series of 7 assorted patches for "net", on the drivers for the
VSC7514 MIPS switch (Ocelot-1), the VSC9953 PowerPC (Seville), and a few
more that are common to all supported devices since they are in the
common library portion.
Vladimir Oltean (7):
net: mscc: ocelot: fix race condition with TX timestamping
net: mscc: ocelot: add locking for the port TX timestamp ID
net: dsa: seville: fix buffer size of the queue system
net: mscc: ocelot: check for errors on memory allocation of ports
net: mscc: ocelot: error checking when calling ocelot_init()
net: mscc: ocelot: refactor ports parsing code into a dedicated
function
net: mscc: ocelot: unregister net devices on unbind
drivers/net/dsa/ocelot/felix.c | 5 +-
drivers/net/dsa/ocelot/seville_vsc9953.c | 2 +-
drivers/net/ethernet/mscc/ocelot.c | 13 +-
drivers/net/ethernet/mscc/ocelot_net.c | 12 +-
drivers/net/ethernet/mscc/ocelot_vsc7514.c | 234 ++++++++++++---------
include/soc/mscc/ocelot.h | 1 +
net/dsa/tag_ocelot.c | 11 +-
7 files changed, 168 insertions(+), 110 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net 1/7] net: mscc: ocelot: fix race condition with TX timestamping
2020-09-15 18:22 [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver Vladimir Oltean
@ 2020-09-15 18:22 ` Vladimir Oltean
2020-09-15 18:22 ` [PATCH net 2/7] net: mscc: ocelot: add locking for the port TX timestamp ID Vladimir Oltean
` (7 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Vladimir Oltean @ 2020-09-15 18:22 UTC (permalink / raw)
To: davem, netdev
Cc: yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver, claudiu.manoil,
alexandre.belloni, andrew, vivien.didelot, f.fainelli, kuba
From: Vladimir Oltean <vladimir.oltean@nxp.com>
The TX-timestampable skb is added late to the ocelot_port->tx_skbs. It
is in a race with the TX timestamp IRQ, which checks that queue trying
to match the timestamp with the skb by the ts_id. The skb should be
added to the queue before the IRQ can fire.
Fixes: 4e3b0468e6d7 ("net: mscc: PTP Hardware Clock (PHC) support")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/ethernet/mscc/ocelot_net.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index 0668d23cdbfa..cacabc23215a 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -330,6 +330,7 @@ static int ocelot_port_xmit(struct sk_buff *skb, struct net_device *dev)
u8 grp = 0; /* Send everything on CPU group 0 */
unsigned int i, count, last;
int port = priv->chip_port;
+ bool do_tstamp;
val = ocelot_read(ocelot, QS_INJ_STATUS);
if (!(val & QS_INJ_STATUS_FIFO_RDY(BIT(grp))) ||
@@ -344,10 +345,14 @@ static int ocelot_port_xmit(struct sk_buff *skb, struct net_device *dev)
info.vid = skb_vlan_tag_get(skb);
/* Check if timestamping is needed */
+ do_tstamp = (ocelot_port_add_txtstamp_skb(ocelot_port, skb) == 0);
+
if (ocelot->ptp && shinfo->tx_flags & SKBTX_HW_TSTAMP) {
info.rew_op = ocelot_port->ptp_cmd;
- if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP)
+ if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP) {
info.rew_op |= (ocelot_port->ts_id % 4) << 3;
+ ocelot_port->ts_id++;
+ }
}
ocelot_gen_ifh(ifh, &info);
@@ -380,12 +385,9 @@ static int ocelot_port_xmit(struct sk_buff *skb, struct net_device *dev)
dev->stats.tx_packets++;
dev->stats.tx_bytes += skb->len;
- if (!ocelot_port_add_txtstamp_skb(ocelot_port, skb)) {
- ocelot_port->ts_id++;
- return NETDEV_TX_OK;
- }
+ if (!do_tstamp)
+ dev_kfree_skb_any(skb);
- dev_kfree_skb_any(skb);
return NETDEV_TX_OK;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net 2/7] net: mscc: ocelot: add locking for the port TX timestamp ID
2020-09-15 18:22 [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver Vladimir Oltean
2020-09-15 18:22 ` [PATCH net 1/7] net: mscc: ocelot: fix race condition with TX timestamping Vladimir Oltean
@ 2020-09-15 18:22 ` Vladimir Oltean
2020-09-16 11:12 ` Alexandre Belloni
2020-09-17 0:19 ` David Miller
2020-09-15 18:22 ` [PATCH net 3/7] net: dsa: seville: fix buffer size of the queue system Vladimir Oltean
` (6 subsequent siblings)
8 siblings, 2 replies; 17+ messages in thread
From: Vladimir Oltean @ 2020-09-15 18:22 UTC (permalink / raw)
To: davem, netdev
Cc: yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver, claudiu.manoil,
alexandre.belloni, andrew, vivien.didelot, f.fainelli, kuba
From: Vladimir Oltean <vladimir.oltean@nxp.com>
The ocelot_port->ts_id is used to:
- populate skb->cb[0] for matching the TX timestamp in the PTP IRQ with
an skb.
- populate the REW_OP from the injection header of the ongoing skb.
Only then is ocelot_port->ts_id incremented.
This is a problem because, at least theoretically, another timestampable
skb might use the same ocelot_port->ts_id before that is incremented. So
the logic of using and incrementing the timestamp id should be atomic
per port.
The solution is to use the global ocelot_port->ts_id only while
protected by the associated ocelot_port->ts_id_lock. That's where we
populate skb->cb[0]. Note that for ocelot,
ocelot_port_add_txtstamp_skb() is called for the actual skb, but for
felix, it is called for the skb's clone. That is something which will
also be changed.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/ethernet/mscc/ocelot.c | 13 ++++++++++++-
drivers/net/ethernet/mscc/ocelot_net.c | 6 ++----
include/soc/mscc/ocelot.h | 1 +
net/dsa/tag_ocelot.c | 11 +++++++----
4 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 5abb7d2b0a9e..8caf3afd464d 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -421,10 +421,15 @@ int ocelot_port_add_txtstamp_skb(struct ocelot_port *ocelot_port,
if (ocelot->ptp && shinfo->tx_flags & SKBTX_HW_TSTAMP &&
ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP) {
+ spin_lock(&ocelot_port->ts_id_lock);
+
shinfo->tx_flags |= SKBTX_IN_PROGRESS;
/* Store timestamp ID in cb[0] of sk_buff */
- skb->cb[0] = ocelot_port->ts_id % 4;
+ skb->cb[0] = ocelot_port->ts_id;
+ ocelot_port->ts_id = (ocelot_port->ts_id + 1) % 4;
skb_queue_tail(&ocelot_port->tx_skbs, skb);
+
+ spin_unlock(&ocelot_port->ts_id_lock);
return 0;
}
return -ENODATA;
@@ -1443,6 +1448,12 @@ int ocelot_init(struct ocelot *ocelot)
if (!ocelot->stats_queue)
return -ENOMEM;
+ for (port = 0; port < ocelot->num_phys_ports; port++) {
+ struct ocelot_port *ocelot_port = ocelot->ports[port];
+
+ spin_lock_init(&ocelot_port->ts_id_lock);
+ }
+
INIT_LIST_HEAD(&ocelot->multicast);
ocelot_mact_init(ocelot);
ocelot_vlan_init(ocelot);
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index cacabc23215a..8490e42e9e2d 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -349,10 +349,8 @@ static int ocelot_port_xmit(struct sk_buff *skb, struct net_device *dev)
if (ocelot->ptp && shinfo->tx_flags & SKBTX_HW_TSTAMP) {
info.rew_op = ocelot_port->ptp_cmd;
- if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP) {
- info.rew_op |= (ocelot_port->ts_id % 4) << 3;
- ocelot_port->ts_id++;
- }
+ if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP)
+ info.rew_op |= skb->cb[0] << 3;
}
ocelot_gen_ifh(ifh, &info);
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index da369b12005f..4521dd602ddc 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -566,6 +566,7 @@ struct ocelot_port {
u8 ptp_cmd;
struct sk_buff_head tx_skbs;
u8 ts_id;
+ spinlock_t ts_id_lock;
phy_interface_t phy_mode;
diff --git a/net/dsa/tag_ocelot.c b/net/dsa/tag_ocelot.c
index 42f327c06dca..b4fc05cafaa6 100644
--- a/net/dsa/tag_ocelot.c
+++ b/net/dsa/tag_ocelot.c
@@ -160,11 +160,14 @@ static struct sk_buff *ocelot_xmit(struct sk_buff *skb,
packing(injection, &qos_class, 19, 17, OCELOT_TAG_LEN, PACK, 0);
if (ocelot->ptp && (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
+ struct sk_buff *clone = DSA_SKB_CB(skb)->clone;
+
rew_op = ocelot_port->ptp_cmd;
- if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP) {
- rew_op |= (ocelot_port->ts_id % 4) << 3;
- ocelot_port->ts_id++;
- }
+ /* Retrieve timestamp ID populated inside skb->cb[0] of the
+ * clone by ocelot_port_add_txtstamp_skb
+ */
+ if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP)
+ rew_op |= clone->cb[0] << 3;
packing(injection, &rew_op, 125, 117, OCELOT_TAG_LEN, PACK, 0);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net 3/7] net: dsa: seville: fix buffer size of the queue system
2020-09-15 18:22 [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver Vladimir Oltean
2020-09-15 18:22 ` [PATCH net 1/7] net: mscc: ocelot: fix race condition with TX timestamping Vladimir Oltean
2020-09-15 18:22 ` [PATCH net 2/7] net: mscc: ocelot: add locking for the port TX timestamp ID Vladimir Oltean
@ 2020-09-15 18:22 ` Vladimir Oltean
2020-09-15 18:22 ` [PATCH net 4/7] net: mscc: ocelot: check for errors on memory allocation of ports Vladimir Oltean
` (5 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Vladimir Oltean @ 2020-09-15 18:22 UTC (permalink / raw)
To: davem, netdev
Cc: yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver, claudiu.manoil,
alexandre.belloni, andrew, vivien.didelot, f.fainelli, kuba
From: Vladimir Oltean <vladimir.oltean@nxp.com>
The VSC9953 Seville switch has 2 megabits of buffer split into 4360
words of 60 bytes each.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/dsa/ocelot/seville_vsc9953.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c
index 2d6a5f5758f8..83a1ab9393e9 100644
--- a/drivers/net/dsa/ocelot/seville_vsc9953.c
+++ b/drivers/net/dsa/ocelot/seville_vsc9953.c
@@ -1018,7 +1018,7 @@ static const struct felix_info seville_info_vsc9953 = {
.vcap_is2_keys = vsc9953_vcap_is2_keys,
.vcap_is2_actions = vsc9953_vcap_is2_actions,
.vcap = vsc9953_vcap_props,
- .shared_queue_sz = 128 * 1024,
+ .shared_queue_sz = 2048 * 1024,
.num_mact_rows = 2048,
.num_ports = 10,
.mdio_bus_alloc = vsc9953_mdio_bus_alloc,
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net 4/7] net: mscc: ocelot: check for errors on memory allocation of ports
2020-09-15 18:22 [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver Vladimir Oltean
` (2 preceding siblings ...)
2020-09-15 18:22 ` [PATCH net 3/7] net: dsa: seville: fix buffer size of the queue system Vladimir Oltean
@ 2020-09-15 18:22 ` Vladimir Oltean
2020-09-17 0:21 ` David Miller
2020-09-15 18:22 ` [PATCH net 5/7] net: mscc: ocelot: error checking when calling ocelot_init() Vladimir Oltean
` (4 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Vladimir Oltean @ 2020-09-15 18:22 UTC (permalink / raw)
To: davem, netdev
Cc: yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver, claudiu.manoil,
alexandre.belloni, andrew, vivien.didelot, f.fainelli, kuba
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Do not proceed probing if we couldn't allocate memory for the ports
array, just error out.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/ethernet/mscc/ocelot_vsc7514.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index 65408bc994c4..99872f1b7460 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -993,6 +993,8 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
ocelot->ports = devm_kcalloc(&pdev->dev, ocelot->num_phys_ports,
sizeof(struct ocelot_port *), GFP_KERNEL);
+ if (!ocelot->ports)
+ return -ENOMEM;
ocelot->vcap_is2_keys = vsc7514_vcap_is2_keys;
ocelot->vcap_is2_actions = vsc7514_vcap_is2_actions;
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net 5/7] net: mscc: ocelot: error checking when calling ocelot_init()
2020-09-15 18:22 [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver Vladimir Oltean
` (3 preceding siblings ...)
2020-09-15 18:22 ` [PATCH net 4/7] net: mscc: ocelot: check for errors on memory allocation of ports Vladimir Oltean
@ 2020-09-15 18:22 ` Vladimir Oltean
2020-09-17 0:24 ` David Miller
2020-09-15 18:22 ` [PATCH net 6/7] net: mscc: ocelot: refactor ports parsing code into a dedicated function Vladimir Oltean
` (3 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Vladimir Oltean @ 2020-09-15 18:22 UTC (permalink / raw)
To: davem, netdev
Cc: yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver, claudiu.manoil,
alexandre.belloni, andrew, vivien.didelot, f.fainelli, kuba
From: Vladimir Oltean <vladimir.oltean@nxp.com>
ocelot_init() allocates memory, resets the switch and polls for a status
register, things which can fail. Stop probing the driver in that case,
and propagate the error result.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/dsa/ocelot/felix.c | 5 ++++-
drivers/net/ethernet/mscc/ocelot_vsc7514.c | 5 ++++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index a1e1d3824110..f7b43f8d56ed 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -571,7 +571,10 @@ static int felix_setup(struct dsa_switch *ds)
if (err)
return err;
- ocelot_init(ocelot);
+ err = ocelot_init(ocelot);
+ if (err)
+ return err;
+
if (ocelot->ptp) {
err = ocelot_init_timestamp(ocelot, &ocelot_ptp_clock_info);
if (err) {
diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index 99872f1b7460..91a915d0693f 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -1000,7 +1000,10 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
ocelot->vcap_is2_actions = vsc7514_vcap_is2_actions;
ocelot->vcap = vsc7514_vcap_props;
- ocelot_init(ocelot);
+ err = ocelot_init(ocelot);
+ if (err)
+ return err;
+
if (ocelot->ptp) {
err = ocelot_init_timestamp(ocelot, &ocelot_ptp_clock_info);
if (err) {
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net 6/7] net: mscc: ocelot: refactor ports parsing code into a dedicated function
2020-09-15 18:22 [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver Vladimir Oltean
` (4 preceding siblings ...)
2020-09-15 18:22 ` [PATCH net 5/7] net: mscc: ocelot: error checking when calling ocelot_init() Vladimir Oltean
@ 2020-09-15 18:22 ` Vladimir Oltean
2020-09-15 18:22 ` [PATCH net 7/7] net: mscc: ocelot: unregister net devices on unbind Vladimir Oltean
` (2 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Vladimir Oltean @ 2020-09-15 18:22 UTC (permalink / raw)
To: davem, netdev
Cc: yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver, claudiu.manoil,
alexandre.belloni, andrew, vivien.didelot, f.fainelli, kuba
From: Vladimir Oltean <vladimir.oltean@nxp.com>
mscc_ocelot_probe() is already pretty large and hard to follow. So move
the code for parsing ports in a separate function.
This makes it easier for the next patch to just call
mscc_ocelot_release_ports from the error path of mscc_ocelot_init_ports.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/ethernet/mscc/ocelot_vsc7514.c | 223 +++++++++++----------
1 file changed, 118 insertions(+), 105 deletions(-)
diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index 91a915d0693f..851e79e11aed 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -896,123 +896,26 @@ static struct ptp_clock_info ocelot_ptp_clock_info = {
.enable = ocelot_ptp_enable,
};
-static int mscc_ocelot_probe(struct platform_device *pdev)
+static int mscc_ocelot_init_ports(struct platform_device *pdev)
{
- struct device_node *np = pdev->dev.of_node;
+ struct ocelot *ocelot = platform_get_drvdata(pdev);
+ struct device_node *np = ocelot->dev->of_node;
struct device_node *ports, *portnp;
- int err, irq_xtr, irq_ptp_rdy;
- struct ocelot *ocelot;
- struct regmap *hsio;
- unsigned int i;
-
- struct {
- enum ocelot_target id;
- char *name;
- u8 optional:1;
- } io_target[] = {
- { SYS, "sys" },
- { REW, "rew" },
- { QSYS, "qsys" },
- { ANA, "ana" },
- { QS, "qs" },
- { S2, "s2" },
- { PTP, "ptp", 1 },
- };
-
- if (!np && !pdev->dev.platform_data)
- return -ENODEV;
-
- ocelot = devm_kzalloc(&pdev->dev, sizeof(*ocelot), GFP_KERNEL);
- if (!ocelot)
- return -ENOMEM;
-
- platform_set_drvdata(pdev, ocelot);
- ocelot->dev = &pdev->dev;
-
- for (i = 0; i < ARRAY_SIZE(io_target); i++) {
- struct regmap *target;
- struct resource *res;
-
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
- io_target[i].name);
-
- target = ocelot_regmap_init(ocelot, res);
- if (IS_ERR(target)) {
- if (io_target[i].optional) {
- ocelot->targets[io_target[i].id] = NULL;
- continue;
- }
- return PTR_ERR(target);
- }
-
- ocelot->targets[io_target[i].id] = target;
- }
-
- hsio = syscon_regmap_lookup_by_compatible("mscc,ocelot-hsio");
- if (IS_ERR(hsio)) {
- dev_err(&pdev->dev, "missing hsio syscon\n");
- return PTR_ERR(hsio);
- }
-
- ocelot->targets[HSIO] = hsio;
-
- err = ocelot_chip_init(ocelot, &ocelot_ops);
- if (err)
- return err;
-
- irq_xtr = platform_get_irq_byname(pdev, "xtr");
- if (irq_xtr < 0)
- return -ENODEV;
-
- err = devm_request_threaded_irq(&pdev->dev, irq_xtr, NULL,
- ocelot_xtr_irq_handler, IRQF_ONESHOT,
- "frame extraction", ocelot);
- if (err)
- return err;
-
- irq_ptp_rdy = platform_get_irq_byname(pdev, "ptp_rdy");
- if (irq_ptp_rdy > 0 && ocelot->targets[PTP]) {
- err = devm_request_threaded_irq(&pdev->dev, irq_ptp_rdy, NULL,
- ocelot_ptp_rdy_irq_handler,
- IRQF_ONESHOT, "ptp ready",
- ocelot);
- if (err)
- return err;
-
- /* Both the PTP interrupt and the PTP bank are available */
- ocelot->ptp = 1;
- }
+ int err;
ports = of_get_child_by_name(np, "ethernet-ports");
if (!ports) {
- dev_err(&pdev->dev, "no ethernet-ports child node found\n");
+ dev_err(ocelot->dev, "no ethernet-ports child node found\n");
return -ENODEV;
}
ocelot->num_phys_ports = of_get_child_count(ports);
- ocelot->ports = devm_kcalloc(&pdev->dev, ocelot->num_phys_ports,
+ ocelot->ports = devm_kcalloc(ocelot->dev, ocelot->num_phys_ports,
sizeof(struct ocelot_port *), GFP_KERNEL);
if (!ocelot->ports)
return -ENOMEM;
- ocelot->vcap_is2_keys = vsc7514_vcap_is2_keys;
- ocelot->vcap_is2_actions = vsc7514_vcap_is2_actions;
- ocelot->vcap = vsc7514_vcap_props;
-
- err = ocelot_init(ocelot);
- if (err)
- return err;
-
- if (ocelot->ptp) {
- err = ocelot_init_timestamp(ocelot, &ocelot_ptp_clock_info);
- if (err) {
- dev_err(ocelot->dev,
- "Timestamp initialization failed\n");
- ocelot->ptp = 0;
- }
- }
-
/* No NPI port */
ocelot_configure_cpu(ocelot, -1, OCELOT_TAG_PREFIX_NONE,
OCELOT_TAG_PREFIX_NONE);
@@ -1103,14 +1006,124 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
priv->serdes = serdes;
}
+out_put_ports:
+ of_node_put(ports);
+ return err;
+}
+
+static int mscc_ocelot_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ int err, irq_xtr, irq_ptp_rdy;
+ struct ocelot *ocelot;
+ struct regmap *hsio;
+ unsigned int i;
+
+ struct {
+ enum ocelot_target id;
+ char *name;
+ u8 optional:1;
+ } io_target[] = {
+ { SYS, "sys" },
+ { REW, "rew" },
+ { QSYS, "qsys" },
+ { ANA, "ana" },
+ { QS, "qs" },
+ { S2, "s2" },
+ { PTP, "ptp", 1 },
+ };
+
+ if (!np && !pdev->dev.platform_data)
+ return -ENODEV;
+
+ ocelot = devm_kzalloc(&pdev->dev, sizeof(*ocelot), GFP_KERNEL);
+ if (!ocelot)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, ocelot);
+ ocelot->dev = &pdev->dev;
+
+ for (i = 0; i < ARRAY_SIZE(io_target); i++) {
+ struct regmap *target;
+ struct resource *res;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+ io_target[i].name);
+
+ target = ocelot_regmap_init(ocelot, res);
+ if (IS_ERR(target)) {
+ if (io_target[i].optional) {
+ ocelot->targets[io_target[i].id] = NULL;
+ continue;
+ }
+ return PTR_ERR(target);
+ }
+
+ ocelot->targets[io_target[i].id] = target;
+ }
+
+ hsio = syscon_regmap_lookup_by_compatible("mscc,ocelot-hsio");
+ if (IS_ERR(hsio)) {
+ dev_err(&pdev->dev, "missing hsio syscon\n");
+ return PTR_ERR(hsio);
+ }
+
+ ocelot->targets[HSIO] = hsio;
+
+ err = ocelot_chip_init(ocelot, &ocelot_ops);
+ if (err)
+ return err;
+
+ irq_xtr = platform_get_irq_byname(pdev, "xtr");
+ if (irq_xtr < 0)
+ return -ENODEV;
+
+ err = devm_request_threaded_irq(&pdev->dev, irq_xtr, NULL,
+ ocelot_xtr_irq_handler, IRQF_ONESHOT,
+ "frame extraction", ocelot);
+ if (err)
+ return err;
+
+ irq_ptp_rdy = platform_get_irq_byname(pdev, "ptp_rdy");
+ if (irq_ptp_rdy > 0 && ocelot->targets[PTP]) {
+ err = devm_request_threaded_irq(&pdev->dev, irq_ptp_rdy, NULL,
+ ocelot_ptp_rdy_irq_handler,
+ IRQF_ONESHOT, "ptp ready",
+ ocelot);
+ if (err)
+ return err;
+
+ /* Both the PTP interrupt and the PTP bank are available */
+ ocelot->ptp = 1;
+ }
+
+ ocelot->vcap_is2_keys = vsc7514_vcap_is2_keys;
+ ocelot->vcap_is2_actions = vsc7514_vcap_is2_actions;
+ ocelot->vcap = vsc7514_vcap_props;
+
+ err = ocelot_init(ocelot);
+ if (err)
+ return err;
+
+ err = mscc_ocelot_init_ports(pdev);
+ if (err)
+ return err;
+
+ if (ocelot->ptp) {
+ err = ocelot_init_timestamp(ocelot, &ocelot_ptp_clock_info);
+ if (err) {
+ dev_err(ocelot->dev,
+ "Timestamp initialization failed\n");
+ ocelot->ptp = 0;
+ }
+ }
+
register_netdevice_notifier(&ocelot_netdevice_nb);
register_switchdev_notifier(&ocelot_switchdev_nb);
register_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
dev_info(&pdev->dev, "Ocelot switch probed\n");
-out_put_ports:
- of_node_put(ports);
return err;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net 7/7] net: mscc: ocelot: unregister net devices on unbind
2020-09-15 18:22 [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver Vladimir Oltean
` (5 preceding siblings ...)
2020-09-15 18:22 ` [PATCH net 6/7] net: mscc: ocelot: refactor ports parsing code into a dedicated function Vladimir Oltean
@ 2020-09-15 18:22 ` Vladimir Oltean
2020-09-15 21:19 ` [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver Horatiu Vultur
2020-09-16 9:56 ` Alexandre Belloni
8 siblings, 0 replies; 17+ messages in thread
From: Vladimir Oltean @ 2020-09-15 18:22 UTC (permalink / raw)
To: davem, netdev
Cc: yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver, claudiu.manoil,
alexandre.belloni, andrew, vivien.didelot, f.fainelli, kuba
From: Vladimir Oltean <vladimir.oltean@nxp.com>
This driver was not unregistering its network interfaces on unbind.
Now it is.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/ethernet/mscc/ocelot_vsc7514.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index 851e79e11aed..b8f895483653 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -896,6 +896,26 @@ static struct ptp_clock_info ocelot_ptp_clock_info = {
.enable = ocelot_ptp_enable,
};
+static void mscc_ocelot_release_ports(struct ocelot *ocelot)
+{
+ int port;
+
+ for (port = 0; port < ocelot->num_phys_ports; port++) {
+ struct ocelot_port_private *priv;
+ struct ocelot_port *ocelot_port;
+
+ ocelot_port = ocelot->ports[port];
+ if (!ocelot_port)
+ continue;
+
+ priv = container_of(ocelot_port, struct ocelot_port_private,
+ port);
+
+ unregister_netdev(priv->dev);
+ free_netdev(priv->dev);
+ }
+}
+
static int mscc_ocelot_init_ports(struct platform_device *pdev)
{
struct ocelot *ocelot = platform_get_drvdata(pdev);
@@ -1008,6 +1028,7 @@ static int mscc_ocelot_init_ports(struct platform_device *pdev)
out_put_ports:
of_node_put(ports);
+ mscc_ocelot_release_ports(ocelot);
return err;
}
@@ -1132,6 +1153,7 @@ static int mscc_ocelot_remove(struct platform_device *pdev)
struct ocelot *ocelot = platform_get_drvdata(pdev);
ocelot_deinit_timestamp(ocelot);
+ mscc_ocelot_release_ports(ocelot);
ocelot_deinit(ocelot);
unregister_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
unregister_switchdev_notifier(&ocelot_switchdev_nb);
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver
2020-09-15 18:22 [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver Vladimir Oltean
` (6 preceding siblings ...)
2020-09-15 18:22 ` [PATCH net 7/7] net: mscc: ocelot: unregister net devices on unbind Vladimir Oltean
@ 2020-09-15 21:19 ` Horatiu Vultur
2020-09-16 9:56 ` Alexandre Belloni
8 siblings, 0 replies; 17+ messages in thread
From: Horatiu Vultur @ 2020-09-15 21:19 UTC (permalink / raw)
To: Vladimir Oltean
Cc: davem, netdev, yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver,
claudiu.manoil, alexandre.belloni, andrew, vivien.didelot,
f.fainelli, kuba
The 09/15/2020 21:22, Vladimir Oltean wrote:
>
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> This is a series of 7 assorted patches for "net", on the drivers for the
> VSC7514 MIPS switch (Ocelot-1), the VSC9953 PowerPC (Seville), and a few
> more that are common to all supported devices since they are in the
> common library portion.
I have looked over ocelot changes and they look fine to me.
Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com>
>
> Vladimir Oltean (7):
> net: mscc: ocelot: fix race condition with TX timestamping
> net: mscc: ocelot: add locking for the port TX timestamp ID
> net: dsa: seville: fix buffer size of the queue system
> net: mscc: ocelot: check for errors on memory allocation of ports
> net: mscc: ocelot: error checking when calling ocelot_init()
> net: mscc: ocelot: refactor ports parsing code into a dedicated
> function
> net: mscc: ocelot: unregister net devices on unbind
>
> drivers/net/dsa/ocelot/felix.c | 5 +-
> drivers/net/dsa/ocelot/seville_vsc9953.c | 2 +-
> drivers/net/ethernet/mscc/ocelot.c | 13 +-
> drivers/net/ethernet/mscc/ocelot_net.c | 12 +-
> drivers/net/ethernet/mscc/ocelot_vsc7514.c | 234 ++++++++++++---------
> include/soc/mscc/ocelot.h | 1 +
> net/dsa/tag_ocelot.c | 11 +-
> 7 files changed, 168 insertions(+), 110 deletions(-)
>
> --
> 2.25.1
>
--
/Horatiu
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver
2020-09-15 18:22 [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver Vladimir Oltean
` (7 preceding siblings ...)
2020-09-15 21:19 ` [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver Horatiu Vultur
@ 2020-09-16 9:56 ` Alexandre Belloni
8 siblings, 0 replies; 17+ messages in thread
From: Alexandre Belloni @ 2020-09-16 9:56 UTC (permalink / raw)
To: Vladimir Oltean
Cc: davem, netdev, yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver,
claudiu.manoil, andrew, vivien.didelot, f.fainelli, kuba
Hi,
On 15/09/2020 21:22:22+0300, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> This is a series of 7 assorted patches for "net", on the drivers for the
> VSC7514 MIPS switch (Ocelot-1), the VSC9953 PowerPC (Seville), and a few
> more that are common to all supported devices since they are in the
> common library portion.
>
> Vladimir Oltean (7):
> net: mscc: ocelot: fix race condition with TX timestamping
> net: mscc: ocelot: add locking for the port TX timestamp ID
> net: dsa: seville: fix buffer size of the queue system
> net: mscc: ocelot: check for errors on memory allocation of ports
> net: mscc: ocelot: error checking when calling ocelot_init()
> net: mscc: ocelot: refactor ports parsing code into a dedicated
> function
> net: mscc: ocelot: unregister net devices on unbind
>
> drivers/net/dsa/ocelot/felix.c | 5 +-
> drivers/net/dsa/ocelot/seville_vsc9953.c | 2 +-
> drivers/net/ethernet/mscc/ocelot.c | 13 +-
> drivers/net/ethernet/mscc/ocelot_net.c | 12 +-
> drivers/net/ethernet/mscc/ocelot_vsc7514.c | 234 ++++++++++++---------
> include/soc/mscc/ocelot.h | 1 +
> net/dsa/tag_ocelot.c | 11 +-
> 7 files changed, 168 insertions(+), 110 deletions(-)
>
This series is leading to multiple crashes on my vc7524 board. I'm
trying to pinpoint the problematic commits
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 2/7] net: mscc: ocelot: add locking for the port TX timestamp ID
2020-09-15 18:22 ` [PATCH net 2/7] net: mscc: ocelot: add locking for the port TX timestamp ID Vladimir Oltean
@ 2020-09-16 11:12 ` Alexandre Belloni
2020-09-16 12:25 ` Vladimir Oltean
2020-09-17 0:19 ` David Miller
1 sibling, 1 reply; 17+ messages in thread
From: Alexandre Belloni @ 2020-09-16 11:12 UTC (permalink / raw)
To: Vladimir Oltean
Cc: davem, netdev, yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver,
claudiu.manoil, andrew, vivien.didelot, f.fainelli, kuba
On 15/09/2020 21:22:24+0300, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> The ocelot_port->ts_id is used to:
> - populate skb->cb[0] for matching the TX timestamp in the PTP IRQ with
> an skb.
> - populate the REW_OP from the injection header of the ongoing skb.
> Only then is ocelot_port->ts_id incremented.
>
> This is a problem because, at least theoretically, another timestampable
> skb might use the same ocelot_port->ts_id before that is incremented. So
> the logic of using and incrementing the timestamp id should be atomic
> per port.
>
> The solution is to use the global ocelot_port->ts_id only while
> protected by the associated ocelot_port->ts_id_lock. That's where we
> populate skb->cb[0]. Note that for ocelot,
> ocelot_port_add_txtstamp_skb() is called for the actual skb, but for
> felix, it is called for the skb's clone. That is something which will
> also be changed.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/ethernet/mscc/ocelot.c | 13 ++++++++++++-
> drivers/net/ethernet/mscc/ocelot_net.c | 6 ++----
> include/soc/mscc/ocelot.h | 1 +
> net/dsa/tag_ocelot.c | 11 +++++++----
> 4 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
> index 5abb7d2b0a9e..8caf3afd464d 100644
> --- a/drivers/net/ethernet/mscc/ocelot.c
> +++ b/drivers/net/ethernet/mscc/ocelot.c
> @@ -421,10 +421,15 @@ int ocelot_port_add_txtstamp_skb(struct ocelot_port *ocelot_port,
>
> if (ocelot->ptp && shinfo->tx_flags & SKBTX_HW_TSTAMP &&
> ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP) {
> + spin_lock(&ocelot_port->ts_id_lock);
> +
> shinfo->tx_flags |= SKBTX_IN_PROGRESS;
> /* Store timestamp ID in cb[0] of sk_buff */
> - skb->cb[0] = ocelot_port->ts_id % 4;
> + skb->cb[0] = ocelot_port->ts_id;
> + ocelot_port->ts_id = (ocelot_port->ts_id + 1) % 4;
> skb_queue_tail(&ocelot_port->tx_skbs, skb);
> +
> + spin_unlock(&ocelot_port->ts_id_lock);
> return 0;
> }
> return -ENODATA;
> @@ -1443,6 +1448,12 @@ int ocelot_init(struct ocelot *ocelot)
> if (!ocelot->stats_queue)
> return -ENOMEM;
>
> + for (port = 0; port < ocelot->num_phys_ports; port++) {
> + struct ocelot_port *ocelot_port = ocelot->ports[port];
> +
At this point, ocelot_port is NULL because ocelot_init is called before
the port structures are allocated in mscc_ocelot_probe
> + spin_lock_init(&ocelot_port->ts_id_lock);
> + }
> +
> INIT_LIST_HEAD(&ocelot->multicast);
> ocelot_mact_init(ocelot);
> ocelot_vlan_init(ocelot);
> diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
> index cacabc23215a..8490e42e9e2d 100644
> --- a/drivers/net/ethernet/mscc/ocelot_net.c
> +++ b/drivers/net/ethernet/mscc/ocelot_net.c
> @@ -349,10 +349,8 @@ static int ocelot_port_xmit(struct sk_buff *skb, struct net_device *dev)
>
> if (ocelot->ptp && shinfo->tx_flags & SKBTX_HW_TSTAMP) {
> info.rew_op = ocelot_port->ptp_cmd;
> - if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP) {
> - info.rew_op |= (ocelot_port->ts_id % 4) << 3;
> - ocelot_port->ts_id++;
> - }
> + if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP)
> + info.rew_op |= skb->cb[0] << 3;
> }
>
> ocelot_gen_ifh(ifh, &info);
> diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
> index da369b12005f..4521dd602ddc 100644
> --- a/include/soc/mscc/ocelot.h
> +++ b/include/soc/mscc/ocelot.h
> @@ -566,6 +566,7 @@ struct ocelot_port {
> u8 ptp_cmd;
> struct sk_buff_head tx_skbs;
> u8 ts_id;
> + spinlock_t ts_id_lock;
>
> phy_interface_t phy_mode;
>
> diff --git a/net/dsa/tag_ocelot.c b/net/dsa/tag_ocelot.c
> index 42f327c06dca..b4fc05cafaa6 100644
> --- a/net/dsa/tag_ocelot.c
> +++ b/net/dsa/tag_ocelot.c
> @@ -160,11 +160,14 @@ static struct sk_buff *ocelot_xmit(struct sk_buff *skb,
> packing(injection, &qos_class, 19, 17, OCELOT_TAG_LEN, PACK, 0);
>
> if (ocelot->ptp && (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
> + struct sk_buff *clone = DSA_SKB_CB(skb)->clone;
> +
> rew_op = ocelot_port->ptp_cmd;
> - if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP) {
> - rew_op |= (ocelot_port->ts_id % 4) << 3;
> - ocelot_port->ts_id++;
> - }
> + /* Retrieve timestamp ID populated inside skb->cb[0] of the
> + * clone by ocelot_port_add_txtstamp_skb
> + */
> + if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP)
> + rew_op |= clone->cb[0] << 3;
>
> packing(injection, &rew_op, 125, 117, OCELOT_TAG_LEN, PACK, 0);
> }
> --
> 2.25.1
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 2/7] net: mscc: ocelot: add locking for the port TX timestamp ID
2020-09-16 11:12 ` Alexandre Belloni
@ 2020-09-16 12:25 ` Vladimir Oltean
0 siblings, 0 replies; 17+ messages in thread
From: Vladimir Oltean @ 2020-09-16 12:25 UTC (permalink / raw)
To: Alexandre Belloni
Cc: davem, netdev, yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver,
claudiu.manoil, andrew, vivien.didelot, f.fainelli, kuba
On Wed, Sep 16, 2020 at 01:12:04PM +0200, Alexandre Belloni wrote:
> > + for (port = 0; port < ocelot->num_phys_ports; port++) {
> > + struct ocelot_port *ocelot_port = ocelot->ports[port];
> > +
>
> At this point, ocelot_port is NULL because ocelot_init is called before
> the port structures are allocated in mscc_ocelot_probe
Yikes, you're right, thanks for testing!
-Vladimir
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 2/7] net: mscc: ocelot: add locking for the port TX timestamp ID
2020-09-15 18:22 ` [PATCH net 2/7] net: mscc: ocelot: add locking for the port TX timestamp ID Vladimir Oltean
2020-09-16 11:12 ` Alexandre Belloni
@ 2020-09-17 0:19 ` David Miller
2020-09-17 23:43 ` Vladimir Oltean
1 sibling, 1 reply; 17+ messages in thread
From: David Miller @ 2020-09-17 0:19 UTC (permalink / raw)
To: olteanv
Cc: netdev, yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver,
claudiu.manoil, alexandre.belloni, andrew, vivien.didelot,
f.fainelli, kuba
From: Vladimir Oltean <olteanv@gmail.com>
Date: Tue, 15 Sep 2020 21:22:24 +0300
> This is a problem because, at least theoretically, another timestampable
> skb might use the same ocelot_port->ts_id before that is incremented. So
> the logic of using and incrementing the timestamp id should be atomic
> per port.
Have you actually observed this race in practice?
All transmit calls are serialized by the netdev transmit spinlock.
Let's not add locking if it is not actually necessary.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 4/7] net: mscc: ocelot: check for errors on memory allocation of ports
2020-09-15 18:22 ` [PATCH net 4/7] net: mscc: ocelot: check for errors on memory allocation of ports Vladimir Oltean
@ 2020-09-17 0:21 ` David Miller
0 siblings, 0 replies; 17+ messages in thread
From: David Miller @ 2020-09-17 0:21 UTC (permalink / raw)
To: olteanv
Cc: netdev, yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver,
claudiu.manoil, alexandre.belloni, andrew, vivien.didelot,
f.fainelli, kuba
From: Vladimir Oltean <olteanv@gmail.com>
Date: Tue, 15 Sep 2020 21:22:26 +0300
> @@ -993,6 +993,8 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
>
> ocelot->ports = devm_kcalloc(&pdev->dev, ocelot->num_phys_ports,
> sizeof(struct ocelot_port *), GFP_KERNEL);
> + if (!ocelot->ports)
> + return -ENOMEM;
This leaks the reference to OF node 'ports'.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 5/7] net: mscc: ocelot: error checking when calling ocelot_init()
2020-09-15 18:22 ` [PATCH net 5/7] net: mscc: ocelot: error checking when calling ocelot_init() Vladimir Oltean
@ 2020-09-17 0:24 ` David Miller
0 siblings, 0 replies; 17+ messages in thread
From: David Miller @ 2020-09-17 0:24 UTC (permalink / raw)
To: olteanv
Cc: netdev, yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver,
claudiu.manoil, alexandre.belloni, andrew, vivien.didelot,
f.fainelli, kuba
From: Vladimir Oltean <olteanv@gmail.com>
Date: Tue, 15 Sep 2020 21:22:27 +0300
> diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
> index 99872f1b7460..91a915d0693f 100644
> --- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
> +++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
> @@ -1000,7 +1000,10 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> ocelot->vcap_is2_actions = vsc7514_vcap_is2_actions;
> ocelot->vcap = vsc7514_vcap_props;
>
> - ocelot_init(ocelot);
> + err = ocelot_init(ocelot);
> + if (err)
> + return err;
> +
This also leaks the OF device 'ports' object.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 2/7] net: mscc: ocelot: add locking for the port TX timestamp ID
2020-09-17 0:19 ` David Miller
@ 2020-09-17 23:43 ` Vladimir Oltean
2020-09-17 23:57 ` David Miller
0 siblings, 1 reply; 17+ messages in thread
From: Vladimir Oltean @ 2020-09-17 23:43 UTC (permalink / raw)
To: David Miller
Cc: netdev, yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver,
claudiu.manoil, alexandre.belloni, andrew, vivien.didelot,
f.fainelli, kuba
On Wed, Sep 16, 2020 at 05:19:26PM -0700, David Miller wrote:
> From: Vladimir Oltean <olteanv@gmail.com>
> Date: Tue, 15 Sep 2020 21:22:24 +0300
>
> > This is a problem because, at least theoretically, another timestampable
> > skb might use the same ocelot_port->ts_id before that is incremented. So
> > the logic of using and incrementing the timestamp id should be atomic
> > per port.
>
> Have you actually observed this race in practice?
>
> All transmit calls are serialized by the netdev transmit spinlock.
>
> Let's not add locking if it is not actually necessary.
It's a bit more complicated.
This code is also used from DSA, and DSA now declares NETIF_F_LLTX.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 2/7] net: mscc: ocelot: add locking for the port TX timestamp ID
2020-09-17 23:43 ` Vladimir Oltean
@ 2020-09-17 23:57 ` David Miller
0 siblings, 0 replies; 17+ messages in thread
From: David Miller @ 2020-09-17 23:57 UTC (permalink / raw)
To: olteanv
Cc: netdev, yangbo.lu, xiaoliang.yang_1, UNGLinuxDriver,
claudiu.manoil, alexandre.belloni, andrew, vivien.didelot,
f.fainelli, kuba
From: Vladimir Oltean <olteanv@gmail.com>
Date: Fri, 18 Sep 2020 02:43:40 +0300
> On Wed, Sep 16, 2020 at 05:19:26PM -0700, David Miller wrote:
>> From: Vladimir Oltean <olteanv@gmail.com>
>> Date: Tue, 15 Sep 2020 21:22:24 +0300
>>
>> > This is a problem because, at least theoretically, another timestampable
>> > skb might use the same ocelot_port->ts_id before that is incremented. So
>> > the logic of using and incrementing the timestamp id should be atomic
>> > per port.
>>
>> Have you actually observed this race in practice?
>>
>> All transmit calls are serialized by the netdev transmit spinlock.
>>
>> Let's not add locking if it is not actually necessary.
>
> It's a bit more complicated.
> This code is also used from DSA, and DSA now declares NETIF_F_LLTX.
Please document that in the commit log message, thank you.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-09-17 23:57 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-15 18:22 [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver Vladimir Oltean
2020-09-15 18:22 ` [PATCH net 1/7] net: mscc: ocelot: fix race condition with TX timestamping Vladimir Oltean
2020-09-15 18:22 ` [PATCH net 2/7] net: mscc: ocelot: add locking for the port TX timestamp ID Vladimir Oltean
2020-09-16 11:12 ` Alexandre Belloni
2020-09-16 12:25 ` Vladimir Oltean
2020-09-17 0:19 ` David Miller
2020-09-17 23:43 ` Vladimir Oltean
2020-09-17 23:57 ` David Miller
2020-09-15 18:22 ` [PATCH net 3/7] net: dsa: seville: fix buffer size of the queue system Vladimir Oltean
2020-09-15 18:22 ` [PATCH net 4/7] net: mscc: ocelot: check for errors on memory allocation of ports Vladimir Oltean
2020-09-17 0:21 ` David Miller
2020-09-15 18:22 ` [PATCH net 5/7] net: mscc: ocelot: error checking when calling ocelot_init() Vladimir Oltean
2020-09-17 0:24 ` David Miller
2020-09-15 18:22 ` [PATCH net 6/7] net: mscc: ocelot: refactor ports parsing code into a dedicated function Vladimir Oltean
2020-09-15 18:22 ` [PATCH net 7/7] net: mscc: ocelot: unregister net devices on unbind Vladimir Oltean
2020-09-15 21:19 ` [PATCH net 0/7] Bugfixes in Microsemi Ocelot switch driver Horatiu Vultur
2020-09-16 9:56 ` Alexandre Belloni
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.