Le 22/05/2023 à 13:10, Simon Horman a écrit :
On Mon, May 22, 2023 at 01:35:38PM +0300, Dan Carpenter wrote:
On Mon, May 22, 2023 at 12:12:48PM +0200, Simon Horman wrote:
On Sat, May 20, 2023 at 10:30:17AM +0200, Christophe JAILLET wrote:
If an error occures after calling nv_mgmt_acquire_sema(), it should be
undone with a corresponding nv_mgmt_release_sema() call.
nit: s/occures/occurs/
Add it in the error handling path of the probe as already done in the
remove function.
I was going to ask what happens if nv_mgmt_acquire_sema() fails.
Then I realised that it always returns 0.
Perhaps it would be worth changing it's return type to void at some point.
What? No? It returns true on success and false on failure.
drivers/net/ethernet/nvidia/forcedeth.c
5377 static int nv_mgmt_acquire_sema(struct net_device *dev)
5378 {
5379 struct fe_priv *np = netdev_priv(dev);
5380 u8 __iomem *base = get_hwbase(dev);
5381 int i;
5382 u32 tx_ctrl, mgmt_sema;
5383
5384 for (i = 0; i < 10; i++) {
5385 mgmt_sema = readl(base + NvRegTransmitterControl) & NVREG_XMITCTL_MGMT_SEMA_MASK;
5386 if (mgmt_sema == NVREG_XMITCTL_MGMT_SEMA_FREE)
5387 break;
5388 msleep(500);
5389 }
5390
5391 if (mgmt_sema != NVREG_XMITCTL_MGMT_SEMA_FREE)
5392 return 0;
5393
5394 for (i = 0; i < 2; i++) {
5395 tx_ctrl = readl(base + NvRegTransmitterControl);
5396 tx_ctrl |= NVREG_XMITCTL_HOST_SEMA_ACQ;
5397 writel(tx_ctrl, base + NvRegTransmitterControl);
5398
5399 /* verify that semaphore was acquired */
5400 tx_ctrl = readl(base + NvRegTransmitterControl);
5401 if (((tx_ctrl & NVREG_XMITCTL_HOST_SEMA_MASK) == NVREG_XMITCTL_HOST_SEMA_ACQ) &&
5402 ((tx_ctrl & NVREG_XMITCTL_MGMT_SEMA_MASK) == NVREG_XMITCTL_MGMT_SEMA_FREE)) {
5403 np->mgmt_sema = 1;
5404 return 1;
^^^^^^^^^
Success path.
5405 } else
5406 udelay(50);
5407 }
5408
5409 return 0;
5410 }
Thanks Dan,
my eyes deceived me.
In that case, my question is: what if nv_mgmt_acquire_sema() fails?
But I think the answer is that nv_mgmt_release_sema() will do
nothing because mgmt_sema is not set.
At least, it is my understanding.
Can you fix the typo s/occures/occurs/ when applying the patch, or do
you really need a v2 only for that?
CJ.
So I think we are good.
