Thread (52 messages) 52 messages, 9 authors, 2023-11-27

Re: [PATCH net-next 9/9] net: pse-pd: Add PD692x0 PSE controller driver

From: Andrew Lunn <andrew@lunn.ch>
Date: 2023-11-22 16:55:11
Also in: linux-devicetree, linux-doc, lkml

quoted
quoted
+static int pd692x0_sendrecv_msg(struct pd692x0_priv *priv,
+				struct pd692x0_msg *msg,
+				struct pd692x0_msg_content *buf)
+{
+	struct device *dev = &priv->client->dev;
+	int ret;
+
+	ret = pd692x0_send_msg(priv, msg);
+	if (ret)
+		return ret;
+
+	ret = pd692x0_recv_msg(priv, msg, buf);  
So this function takes at least 10 seconds?
No, on normal communication it takes a bit more than 30ms.
So i think the first step is to refactor this code to make it clear
what the normal path is, and what the exception path is, and the
timing of each.
quoted
quoted
+	msg.content.sub[2] = id;
+	ret = pd692x0_sendrecv_msg(priv, &msg, &buf);  
So this is also 10 seconds? 

Given its name, it looks like this is called via ethtool? Is the
ethtool core holding RTNL? It is generally considered bad to hold RTNL for
that long.
Yes it is holding RTNL lock. Should I consider another behavior in case of
communication loss to not holding RTNL lock so long?
How often does it happen? On the scale of its a theoretical
possibility, through to it happens every N calls? Also, does it happen
on cold boot and reboot?

If its never supposed to happen, i would keep holding RTNL, and add a
pr_warn() that the PSE has crashed and burned, waiting for it to
reboot. If this is likely to happen on the first communication with
the device, we might want to do a dummy transfer during probe to get
is synchronized before we start using it with the RTNL held.

   Andrew
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help