* [PATCH 14/15] net: cxgb4vf: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:24 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'Casey Leedom'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
index 40c22e7..43bb012 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
@@ -2782,11 +2782,9 @@ err_unmap_bar:
err_free_adapter:
kfree(adapter);
- pci_set_drvdata(pdev, NULL);
err_release_regions:
pci_release_regions(pdev);
- pci_set_drvdata(pdev, NULL);
pci_clear_master(pdev);
err_disable_device:
@@ -2851,7 +2849,6 @@ static void cxgb4vf_pci_remove(struct pci_dev *pdev)
}
iounmap(adapter->regs);
kfree(adapter);
- pci_set_drvdata(pdev, NULL);
}
/*
--
1.7.10.4
^ permalink raw reply related
* [PATCH 15/15] net: enic: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:25 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'Christian Benvenuti',
'Sujith Sankar', 'Govindarajulu Varadarajan',
'Neel Patel', 'Nishank Trivedi'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/cisco/enic/enic_main.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index 7b756cf9..ff78dfa 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -2309,7 +2309,6 @@ err_out_release_regions:
err_out_disable_device:
pci_disable_device(pdev);
err_out_free_netdev:
- pci_set_drvdata(pdev, NULL);
free_netdev(netdev);
return err;
@@ -2338,7 +2337,6 @@ static void enic_remove(struct pci_dev *pdev)
enic_iounmap(enic);
pci_release_regions(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
free_netdev(netdev);
}
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 13/15] net: cxgb2: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:24 UTC (permalink / raw)
To: 'David S. Miller'; +Cc: netdev, 'Jingoo Han'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/chelsio/cxgb/cxgb2.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb/cxgb2.c b/drivers/net/ethernet/chelsio/cxgb/cxgb2.c
index d7048db..1d02105 100644
--- a/drivers/net/ethernet/chelsio/cxgb/cxgb2.c
+++ b/drivers/net/ethernet/chelsio/cxgb/cxgb2.c
@@ -1168,7 +1168,6 @@ out_free_dev:
pci_release_regions(pdev);
out_disable_pdev:
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
return err;
}
@@ -1347,7 +1346,6 @@ static void remove_one(struct pci_dev *pdev)
pci_release_regions(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
t1_sw_reset(pdev);
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 12/15] net: cxgb3: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:23 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'Divy Le Ray'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
index b650951..45d7733 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
@@ -3374,7 +3374,6 @@ out_release_regions:
pci_release_regions(pdev);
out_disable_device:
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
out:
return err;
}
@@ -3415,7 +3414,6 @@ static void remove_one(struct pci_dev *pdev)
kfree(adapter);
pci_release_regions(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
}
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 11/15] net: cxgb4: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:23 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'Dimitris Michailidis'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 85d0cda..8b929ee 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -6075,7 +6075,6 @@ sriov:
pci_disable_device(pdev);
out_release_regions:
pci_release_regions(pdev);
- pci_set_drvdata(pdev, NULL);
return err;
}
@@ -6123,7 +6122,6 @@ static void remove_one(struct pci_dev *pdev)
pci_disable_pcie_error_reporting(pdev);
pci_disable_device(pdev);
pci_release_regions(pdev);
- pci_set_drvdata(pdev, NULL);
} else
pci_release_regions(pdev);
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 10/15] net: bna: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:22 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'Rasesh Mody'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/brocade/bna/bnad.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/brocade/bna/bnad.c b/drivers/net/ethernet/brocade/bna/bnad.c
index b78e69e..f276433 100644
--- a/drivers/net/ethernet/brocade/bna/bnad.c
+++ b/drivers/net/ethernet/brocade/bna/bnad.c
@@ -3212,7 +3212,6 @@ bnad_init(struct bnad *bnad,
bnad->bar0 = ioremap_nocache(bnad->mmio_start, bnad->mmio_len);
if (!bnad->bar0) {
dev_err(&pdev->dev, "ioremap for bar0 failed\n");
- pci_set_drvdata(pdev, NULL);
return -ENOMEM;
}
pr_info("bar0 mapped to %p, len %llu\n", bnad->bar0,
--
1.7.10.4
^ permalink raw reply related
* [PATCH 09/15] net: tg3: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:21 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'Nithin Nayak Sujir',
'Michael Chan'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/broadcom/tg3.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 498569e..819d87c 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -17701,7 +17701,6 @@ err_out_free_res:
err_out_disable_pdev:
if (pci_is_enabled(pdev))
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
return err;
}
@@ -17733,7 +17732,6 @@ static void tg3_remove_one(struct pci_dev *pdev)
free_netdev(dev);
pci_release_regions(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
}
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 08/15] net: bnx2x: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:21 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'Eilon Greenstein'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index e53ff1e..8fd3432 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -12297,7 +12297,6 @@ err_out_release:
err_out_disable:
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
err_out:
return rc;
@@ -12840,7 +12839,6 @@ init_one_exit:
pci_release_regions(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
return rc;
}
@@ -12923,7 +12921,6 @@ static void __bnx2x_remove(struct pci_dev *pdev,
pci_release_regions(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
}
static void bnx2x_remove_one(struct pci_dev *pdev)
--
1.7.10.4
^ permalink raw reply related
* [PATCH 07/15] net: bnx2: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:21 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'Michael Chan'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/broadcom/bnx2.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2.c b/drivers/net/ethernet/broadcom/bnx2.c
index 6111870..d9980ad 100644
--- a/drivers/net/ethernet/broadcom/bnx2.c
+++ b/drivers/net/ethernet/broadcom/bnx2.c
@@ -8413,7 +8413,6 @@ err_out_release:
err_out_disable:
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
err_out:
return rc;
@@ -8546,7 +8545,6 @@ error:
pci_iounmap(pdev, bp->regview);
pci_release_regions(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
err_free:
free_netdev(dev);
return rc;
@@ -8578,7 +8576,6 @@ bnx2_remove_one(struct pci_dev *pdev)
pci_release_regions(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
}
static int
--
1.7.10.4
^ permalink raw reply related
* [PATCH 06/15] net: alx: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:20 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'Jay Cliburn',
'Chris Snook'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/atheros/alx/main.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index fc95b23..5aa5e81 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -1367,7 +1367,6 @@ static void alx_remove(struct pci_dev *pdev)
pci_disable_pcie_error_reporting(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
free_netdev(alx->dev);
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 05/15] net: amd8111e: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:19 UTC (permalink / raw)
To: 'David S. Miller'; +Cc: netdev, 'Jingoo Han'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/amd/amd8111e.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/amd/amd8111e.c b/drivers/net/ethernet/amd/amd8111e.c
index 1b1429d..d042511 100644
--- a/drivers/net/ethernet/amd/amd8111e.c
+++ b/drivers/net/ethernet/amd/amd8111e.c
@@ -1711,7 +1711,6 @@ static void amd8111e_remove_one(struct pci_dev *pdev)
free_netdev(dev);
pci_release_regions(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
}
}
static void amd8111e_config_ipg(struct net_device* dev)
@@ -1967,7 +1966,6 @@ err_free_reg:
err_disable_pdev:
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
return err;
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 04/15] net: pcnet32: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:19 UTC (permalink / raw)
To: 'David S. Miller'; +Cc: netdev, 'Jingoo Han', 'Don Fry'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Acked-by: Don Fry <pcnet32@frontier.com>
---
drivers/net/ethernet/amd/pcnet32.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/amd/pcnet32.c b/drivers/net/ethernet/amd/pcnet32.c
index bd4e640..38492e0 100644
--- a/drivers/net/ethernet/amd/pcnet32.c
+++ b/drivers/net/ethernet/amd/pcnet32.c
@@ -2818,7 +2818,6 @@ static void pcnet32_remove_one(struct pci_dev *pdev)
lp->init_block, lp->init_dma_addr);
free_netdev(dev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
}
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 03/15] net: starfire: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:18 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'Ion Badulescu'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/adaptec/starfire.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c
index 8b04bfc..171d73c 100644
--- a/drivers/net/ethernet/adaptec/starfire.c
+++ b/drivers/net/ethernet/adaptec/starfire.c
@@ -835,7 +835,6 @@ static int starfire_init_one(struct pci_dev *pdev,
return 0;
err_out_cleardev:
- pci_set_drvdata(pdev, NULL);
iounmap(base);
err_out_free_res:
pci_release_regions (pdev);
@@ -2012,7 +2011,6 @@ static void starfire_remove_one(struct pci_dev *pdev)
iounmap(np->base);
pci_release_regions(pdev);
- pci_set_drvdata(pdev, NULL);
free_netdev(dev); /* Will also free np!! */
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 02/15] net: 8390: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:18 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'Paul Gortmaker'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/8390/ne2k-pci.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/8390/ne2k-pci.c b/drivers/net/ethernet/8390/ne2k-pci.c
index 9220108..fc14a85 100644
--- a/drivers/net/ethernet/8390/ne2k-pci.c
+++ b/drivers/net/ethernet/8390/ne2k-pci.c
@@ -389,9 +389,7 @@ err_out_free_netdev:
free_netdev (dev);
err_out_free_res:
release_region (ioaddr, NE_IO_EXTENT);
- pci_set_drvdata (pdev, NULL);
return -ENODEV;
-
}
/*
@@ -655,7 +653,6 @@ static void ne2k_pci_remove_one(struct pci_dev *pdev)
release_region(dev->base_addr, NE_IO_EXTENT);
free_netdev(dev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
}
#ifdef CONFIG_PM
--
1.7.10.4
^ permalink raw reply related
* [PATCH 01/15] net: typhoon: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:17 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'David Dillow'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Acked-by: David Dillow <dave@thedillows.org>
---
drivers/net/ethernet/3com/typhoon.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
index 144942f6..465cc71 100644
--- a/drivers/net/ethernet/3com/typhoon.c
+++ b/drivers/net/ethernet/3com/typhoon.c
@@ -2525,7 +2525,6 @@ typhoon_remove_one(struct pci_dev *pdev)
pci_release_regions(pdev);
pci_clear_mwi(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
free_netdev(dev);
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 00/15] net: ethernet: remove unnecessary pci_set_drvdata() part 1
From: Jingoo Han @ 2013-10-18 0:16 UTC (permalink / raw)
To: 'David S. Miller'; +Cc: netdev, 'Jingoo Han'
Since commit 0998d0631001288a5974afc0b2a5f568bcdecb4d
(device-core: Ensure drvdata = NULL when no driver is bound),
the driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
---
drivers/net/ethernet/3com/typhoon.c | 1 -
drivers/net/ethernet/8390/ne2k-pci.c | 3 ---
drivers/net/ethernet/adaptec/starfire.c | 2 --
drivers/net/ethernet/amd/amd8111e.c | 2 --
drivers/net/ethernet/amd/pcnet32.c | 1 -
drivers/net/ethernet/atheros/alx/main.c | 1 -
drivers/net/ethernet/broadcom/bnx2.c | 3 ---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 3 ---
drivers/net/ethernet/broadcom/tg3.c | 2 --
drivers/net/ethernet/brocade/bna/bnad.c | 1 -
drivers/net/ethernet/chelsio/cxgb/cxgb2.c | 2 --
drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 2 --
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 2 --
drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 3 ---
drivers/net/ethernet/cisco/enic/enic_main.c | 2 --
15 files changed, 30 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next v3 3/9] static_key: WARN on usage before jump_label_init was called
From: Hannes Frederic Sowa @ 2013-10-18 0:01 UTC (permalink / raw)
To: Steven Rostedt; +Cc: netdev, linux-kernel, Peter Zijlstra, Andi Kleen
In-Reply-To: <20131017171940.173bab09@gandalf.local.home>
On Thu, Oct 17, 2013 at 05:19:40PM -0400, Steven Rostedt wrote:
> On Thu, 17 Oct 2013 07:31:57 +0200
> Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
>
> > Based on a patch from Andi Kleen.
>
> I'm fine with the patch, but the change log needs a lot more work.
> Like, why is this needed? I know, but does anyone else?
Ok. :)
I'll move the description from the changelog here and write something
in a few days (hope to get more feedback on the other parts, especially
net_get_random_once).
Does that also mean you are in principle ok with the patch weakening
the check in arch/x86/jump_label.c?
Thanks for the review,
Hannes
^ permalink raw reply
* [PATCH v3 net-next] net: refactor sk_page_frag_refill()
From: Eric Dumazet @ 2013-10-17 23:27 UTC (permalink / raw)
To: Michael Dalton
Cc: Daniel Borkmann, davem, netdev, Francesco Fusco, ycheng,
Neal Cardwell, Eric Northup
In-Reply-To: <1381639591.3392.31.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
While working on virtio_net new allocation strategy to increase
payload/truesize ratio, we found that refactoring sk_page_frag_refill()
was needed.
This patch splits sk_page_frag_refill() into two parts, adding
skb_page_frag_refill() which can be used without a socket.
While we are at it, add a minimum frag size of 32 for
sk_page_frag_refill()
Michael will either use netdev_alloc_frag() from softirq context,
or skb_page_frag_refill() from process context in refill_work()
(GFP_KERNEL allocations)
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Michael Dalton <mwdalton@google.com>
---
v3: page_frag_refill() -> skb_page_frag_refill()
v2: fix a off-by one error
include/linux/skbuff.h | 2 ++
net/core/sock.c | 27 +++++++++++++++++++++++----
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 1cd32f9..ba74474 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2062,6 +2062,8 @@ static inline void skb_frag_set_page(struct sk_buff *skb, int f,
__skb_frag_set_page(&skb_shinfo(skb)->frags[f], page);
}
+bool skb_page_frag_refill(unsigned int sz, struct page_frag *pfrag, gfp_t prio);
+
/**
* skb_frag_dma_map - maps a paged fragment via the DMA API
* @dev: the device to map the fragment to
diff --git a/net/core/sock.c b/net/core/sock.c
index fd6afa2..440afdc 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1847,7 +1847,17 @@ EXPORT_SYMBOL(sock_alloc_send_skb);
/* On 32bit arches, an skb frag is limited to 2^15 */
#define SKB_FRAG_PAGE_ORDER get_order(32768)
-bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
+/**
+ * skb_page_frag_refill - check that a page_frag contains enough room
+ * @sz: minimum size of the fragment we want to get
+ * @pfrag: pointer to page_frag
+ * @prio: priority for memory allocation
+ *
+ * Note: While this allocator tries to use high order pages, there is
+ * no guarantee that allocations succeed. Therefore, @sz MUST be
+ * less or equal than PAGE_SIZE.
+ */
+bool skb_page_frag_refill(unsigned int sz, struct page_frag *pfrag, gfp_t prio)
{
int order;
@@ -1856,16 +1866,16 @@ bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
pfrag->offset = 0;
return true;
}
- if (pfrag->offset < pfrag->size)
+ if (pfrag->offset + sz <= pfrag->size)
return true;
put_page(pfrag->page);
}
/* We restrict high order allocations to users that can afford to wait */
- order = (sk->sk_allocation & __GFP_WAIT) ? SKB_FRAG_PAGE_ORDER : 0;
+ order = (prio & __GFP_WAIT) ? SKB_FRAG_PAGE_ORDER : 0;
do {
- gfp_t gfp = sk->sk_allocation;
+ gfp_t gfp = prio;
if (order)
gfp |= __GFP_COMP | __GFP_NOWARN;
@@ -1877,6 +1887,15 @@ bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
}
} while (--order >= 0);
+ return false;
+}
+EXPORT_SYMBOL(skb_page_frag_refill);
+
+bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
+{
+ if (likely(skb_page_frag_refill(32U, pfrag, sk->sk_allocation)))
+ return true;
+
sk_enter_memory_pressure(sk);
sk_stream_moderate_sndbuf(sk);
return false;
^ permalink raw reply related
* Re: [PATCH v2 net-next] net: refactor sk_page_frag_refill()
From: Eric Dumazet @ 2013-10-17 23:13 UTC (permalink / raw)
To: David Miller
Cc: mwdalton, dborkman, netdev, ffusco, ycheng, ncardwell,
digitaleric
In-Reply-To: <20131017.154625.1687353288700164892.davem@davemloft.net>
On Thu, 2013-10-17 at 15:46 -0400, David Miller wrote:
> Please rename this to something like "skb_page_frag_refill" so that it
> is clear that this is a networking interface.
Sure will do, thanks !
^ permalink raw reply
* Re: [RFC net-next] ipv6: Use destination address determined by IPVS
From: Julian Anastasov @ 2013-10-17 23:03 UTC (permalink / raw)
To: Simon Horman
Cc: YOSHIFUJI Hideaki / 吉藤英明, lvs-devel,
netdev, Mark Brooks, Phil Oester
In-Reply-To: <1381881751-6719-1-git-send-email-horms@verge.net.au>
Hello,
Here is a solution that should work not only for IPVS.
If the change looks correct I'll send it in a separate message.
[PATCH net] ipv6: always prefer rt6i_gateway if present
From: Julian Anastasov <ja@ssi.bg>
In v3.9 6fd6ce2056de2709 ("ipv6: Do not depend on rt->n in
ip6_finish_output2()." changed the behaviour of ip6_finish_output2()
such that the recently introduced rt6_nexthop() is used
instead of an assigned neighbor.
As rt6_nexthop() prefers rt6i_gateway only for gatewayed
routes this causes a problem for users like IPVS, xt_TEE and
RAW(hdrincl) if they want to use different address for routing
compared to the destination address.
Fix it by considering the rt6i_gateway address in all
cases, so that traffic routed to address on local subnet is
not wrongly diverted to the destination address.
Thanks to Simon Horman and Phil Oester for spotting the
problematic commit.
Reported-by: Phil Oester <kernel@linuxace.com>
Reported-by: Mark Brooks <mark@loadbalancer.org>
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
Please review for possible side effects when using
rt6i_gateway without RTF_GATEWAY!
include/net/ip6_route.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index f525e70..481404a 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -196,7 +196,7 @@ static inline int ip6_skb_dst_mtu(struct sk_buff *skb)
static inline struct in6_addr *rt6_nexthop(struct rt6_info *rt, struct in6_addr *dest)
{
- if (rt->rt6i_flags & RTF_GATEWAY)
+ if (rt->rt6i_flags & RTF_GATEWAY || !ipv6_addr_any(&rt->rt6i_gateway))
return &rt->rt6i_gateway;
return dest;
}
--
1.8.3.1
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply related
* [PATCH net] qlcnic: Validate Tx queue only for 82xx adapters.
From: Himanshu Madhani @ 2013-10-17 22:26 UTC (permalink / raw)
To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Himanshu Madhani
From: Himanshu Madhani <himanshu.madhani@qlogic.com>
o validate Tx queue only in case of adapters which supports
multi Tx queue.
This patch is to fix regression introduced in commit
aa4a1f7df7cbb98797c9f4edfde3c726e2b3841f
"qlcnic: Enable Tx queue changes using ethtool for 82xx Series adapter"
Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c | 2 +-
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 8 +-------
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
index ebe4c86..ff83a9f 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
@@ -665,7 +665,7 @@ static int qlcnic_set_channels(struct net_device *dev,
return err;
}
- if (channel->tx_count) {
+ if (qlcnic_82xx_check(adapter) && channel->tx_count) {
err = qlcnic_validate_max_tx_rings(adapter, channel->tx_count);
if (err)
return err;
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index f07f2b0..9e61eb8 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -3651,11 +3651,6 @@ int qlcnic_validate_max_tx_rings(struct qlcnic_adapter *adapter, u32 txq)
u8 max_hw = QLCNIC_MAX_TX_RINGS;
u32 max_allowed;
- if (!qlcnic_82xx_check(adapter)) {
- netdev_err(netdev, "No Multi TX-Q support\n");
- return -EINVAL;
- }
-
if (!qlcnic_use_msi_x && !qlcnic_use_msi) {
netdev_err(netdev, "No Multi TX-Q support in INT-x mode\n");
return -EINVAL;
@@ -3695,8 +3690,7 @@ int qlcnic_validate_max_rss(struct qlcnic_adapter *adapter,
u8 max_hw = adapter->ahw->max_rx_ques;
u32 max_allowed;
- if (qlcnic_82xx_check(adapter) && !qlcnic_use_msi_x &&
- !qlcnic_use_msi) {
+ if (!qlcnic_use_msi_x && !qlcnic_use_msi) {
netdev_err(netdev, "No RSS support in INT-x mode\n");
return -EINVAL;
}
--
1.8.1.4
^ permalink raw reply related
* Re: [PATCH] can: add Renesas R-Car CAN driver
From: Sergei Shtylyov @ 2013-10-17 22:16 UTC (permalink / raw)
To: Wolfgang Grandegger, netdev, mkl, linux-can; +Cc: linux-sh, vksavl
In-Reply-To: <524BB883.2040400@grandegger.com>
Hello.
On 10/02/2013 10:09 AM, Wolfgang Grandegger wrote:
Sorry for the belated reply -- was on vacations.
> thanks for your contribution. The patch looks already quite good. Before
> I find time for a detailed review could you please check error handling
> and bus-off recovery by reporting the output of "$ candump -td -e
> any,0:0,#FFFFFFFF" while sending messages to the device ...
> 1. ... without cable connected
Terminal 1:
root@10.0.0.101:/opt/can-utils# ./cangen -n 1 -g 1 can0
root@10.0.0.101:/opt/can-utils#
Terminal 2:
root@10.0.0.101:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
(000.000000) can0 200000AC [8] 00 08 00 19 00 00 00 00 ERRORFRAME
controller-problem{tx-error-warning}
protocol-violation{{}{acknowledge-slot}}
no-acknowledgement-on-tx
bus-error
(000.004496) can0 20000004 [8] 00 20 00 00 00 00 00 00 ERRORFRAME
controller-problem{tx-error-passive}
So we get and stay in error- passive state:
root@10.0.0.101:/opt/can-utils# ip -details link show can0
2: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN qlen
10 link/can
can state ERROR-PASSIVE (berr-counter tx 128 rx 0) restart-ms 0
bitrate 297619 sample-point 0.714
tq 480 prop-seg 2 phase-seg1 2 phase-seg2 2 sjw 1
rcar_can: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..1024 brp-inc 1
clock 49999999
root@10.0.0.101:/opt/can-utils#
dmesg:
rcar_can rcar_can.0 can0: bitrate error 0.7%
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: ACK Error
rcar_can rcar_can.0 can0: Error passive interrupt
> 2. ... with short-circuited CAN high and low and doing some time later
> a manual recovery with "ip link set can0 type can restart"
Now we have auto recovery only. Manual recovery was tested with the first
driver version and worked.
Terminal 1:
root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
root@10.0.0.104:/opt/can-utils#
Terminal 2:
root@10.0.0.104:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
(000.000000) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
controller-problem{}
protocol-violation{{tx-dominant-bit-error}{}}
bus-error
(000.021147) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
controller-problem{}
bus-off
restarted-after-bus-off
(011.738522) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
controller-problem{}
protocol-violation{{tx-dominant-bit-error}{}}
bus-error
(000.021163) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
controller-problem{}
bus-off
restarted-after-bus-off
(001.666625) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
controller-problem{}
protocol-violation{{tx-dominant-bit-error}{}}
bus-error
(000.021157) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
controller-problem{}
bus-off
restarted-after-bus-off
dmesg:
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: Bit Error (dominant)
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus-off entry interrupt
rcar_can rcar_can.0 can0: bus-off
rcar_can rcar_can.0 can0: Bus-off recovery interrupt
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: Bit Error (dominant)
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus-off entry interrupt
rcar_can rcar_can.0 can0: bus-off
rcar_can rcar_can.0 can0: Bus-off recovery interrupt
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: Bit Error (dominant)
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus-off entry interrupt
rcar_can rcar_can.0 can0: bus-off
rcar_can rcar_can.0 can0: Bus-off recovery interrupt
> I also wonder if the messages are always sent in order. You could use
> the program "canfdtest" [1] from the can-utils for validation.
This program is PITA. With the driver workaroung it works:
FULL Duplex test:
on DUT:
root@10.0.0.104:/opt/can-utils# ./canfdtest -v can0
interface = can0, family = 29, type = 3, proto = 1
............................
root@am335x-evm:~# canconfig can0 bitrate 1000000
can0 bitrate: 1000000, sample-point: 0.750
root@am335x-evm:~# canconfig can0 start
[50164.566650] d_can d_can.1: can0: setting CAN BT = 0x2701
can0 state: ERROR-ACTIVE
root@am335x-evm:~# ./canfdtest -v -g can0
interface = can0, family = 29, type = 3, proto = 1
...............................................................................C
Test messages sent and received: 3826567
Exiting...
> Support for CAN_CTRLMODE_BERR_REPORTING would also be nice, especially
> if the bus errors reporting can easily be switched off (via interrupt mask).
Done.
> There is also the RCAN-TL1 (Renesas CAN Time Trigger Level 1) module
> e.g. available on the SH7203 Group of processors. Is this CAN controller
> compatible to some extend with the R-Car CAN or a completely different
> implementation?
No, not compatible.
> [1] https://gitorious.org/linux-can/can-utils/source/canfdtest.c
> Wolfgang.
WBR, Sergei
^ permalink raw reply
* Re: [PATCH] can: add Renesas R-Car CAN driver
From: Sergei Shtylyov @ 2013-10-17 21:54 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: netdev, wg, linux-can, linux-sh
In-Reply-To: <52487991.6000209@pengutronix.de>
Hello.
On 09/29/2013 11:03 PM, Marc Kleine-Budde wrote:
Sorry for the belated reply -- was on vacations.
>> Add support for the CAN controller found in Renesas R-Car SoCs.
> Is there a public available datasheet for the CAN core?
I don't think so.
> What architecture are the Renesas R-Car SoCs? They're ARM, aren't they?
ARM/SH, to be precise.
> What's R-Car's status on device tree conversion?
Ongoing, but at quite an early stage yet.
> More comments inline
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
[...]
>> Index: linux-can-next/drivers/net/can/rcar_can.c
>> ===================================================================
>> --- /dev/null
>> +++ linux-can-next/drivers/net/can/rcar_can.c
>> @@ -0,0 +1,898 @@
[...]
>> +#define RCAR_CAN_MIER1 0x42C /* CANi Mailbox Interrupt Enable Register 1 */
>> +#define RCAR_CAN_MKR(n) ((n) < 2 ? 0x430 + 4 * (n) : 0x400 + 4 * ((n) - 2))
>> + /* CANi Mask Register */
>> +#define RCAR_CAN_MKIVLR0 0x438 /* CANi Mask Invalid Register 0 */
>> +#define RCAR_CAN_MIER0 0x43C /* CANi Mailbox Interrupt Enable Register 0 */
>> +
>> +#define RCAR_CAN_MCTL(n) (0x800 + (n)) /* CANi Message Control Register */
>> +#define RCAR_CAN_CTLR 0x840 /* CANi Control Register */
>> +#define RCAR_CAN_STR 0x842 /* CANi Status Register */
>> +#define RCAR_CAN_BCR 0x844 /* CANi Bit Configuration Register */
>> +#define RCAR_CAN_CLKR 0x847 /* CANi Clock Select Register */
>> +#define RCAR_CAN_EIER 0x84C /* CANi Error Interrupt Enable Register */
>> +#define RCAR_CAN_EIFR 0x84D /* CANi Err Interrupt Factor Judge Register */
>> +#define RCAR_CAN_RECR 0x84E /* CANi Receive Error Count Register */
>> +#define RCAR_CAN_TECR 0x84F /* CANi Transmit Error Count Register */
>> +#define RCAR_CAN_ECSR 0x850 /* CANi Error Code Store Register */
>> +#define RCAR_CAN_MSSR 0x852 /* CANi Mailbox Search Status Register */
>> +#define RCAR_CAN_MSMR 0x853 /* CANi Mailbox Search Mode Register */
>> +#define RCAR_CAN_TCR 0x858 /* CANi Test Control Register */
>> +#define RCAR_CAN_IER 0x860 /* CANi Interrupt Enable Register */
>> +#define RCAR_CAN_ISR 0x861 /* CANi Interrupt Status Register */
> You might consider using an enum for the register offsets.
Doesn't seem a good idea to me, given the parametrized macros above
(corresponding to sequentially numbered registers).
>> +
>> +/* Offsets of RCAR_CAN Mailbox Registers */
>> +#define MBX_HDR_OFFSET 0x0
>> +#define MBX_DLC_OFFSET 0x5
>> +#define MBX_DATA_OFFSET 0x6
> can you please add the RCAR_ prefix to all defines.
I think they would look clumsy but if you insist on *all* defines, I'd do
it. I'm getting rid of the #define's above based on Wolfgang's feedback.
[...]
>> +static inline u32 rcar_can_mbx_readl(struct rcar_can_priv *priv,
>> + u32 mbxno, u8 offset)
>> +{
>> + return rcar_can_readl(priv, RCAR_CAN_MBX_SIZE * mbxno + offset);
> Can you define hide the RCAR_CAN_MBX_SIZE * mbxno into a macro?
> Something like:
> RCAR_CAN_MBX(mbxno)
Used a structure there based on Wolfgang's suggestion.
[...]
>> +static void rcar_can_error(struct net_device *ndev)
>> +{
[...]
>> + eifr = rcar_can_readb(priv, RCAR_CAN_EIFR);
>> + if (eifr & EIFR_EWIF) {
>> + netdev_dbg(priv->ndev, "Error warning interrupt\n");
>> + priv->can.state = CAN_STATE_ERROR_WARNING;
>> + priv->can.can_stats.error_warning++;
>> + cf->can_id |= CAN_ERR_CRTL;
>> + if (rcar_can_readb(priv, RCAR_CAN_TECR) > 96)
>> + cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
>> + if (rcar_can_readb(priv, RCAR_CAN_RECR) > 96)
>> + cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
>> + /* Clear interrupt condition */
>> + rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EWIF);
> The cast is not needed.
Unfortunately, I get a compiler warning without it.
>> + }
>> + if (eifr & EIFR_EPIF) {
>> + netdev_dbg(priv->ndev, "Error passive interrupt\n");
>> + priv->can.state = CAN_STATE_ERROR_PASSIVE;
>> + priv->can.can_stats.error_passive++;
>> + cf->can_id |= CAN_ERR_CRTL;
>> + if (rcar_can_readb(priv, RCAR_CAN_TECR) > 127)
>> + cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
>> + if (rcar_can_readb(priv, RCAR_CAN_RECR) > 127)
>> + cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
>> + /* Clear interrupt condition */
>> + rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EPIF);
>> + }
>> + if (eifr & EIFR_BOEIF) {
>> + netdev_dbg(priv->ndev, "Bus-off entry interrupt\n");
>> + priv->can.state = CAN_STATE_BUS_OFF;
>> + cf->can_id |= CAN_ERR_BUSOFF;
>> + /* Clear interrupt condition */
>> + rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_BOEIF);
>> + /* Disable all interrupts in bus-off to avoid int hog */
>> + rcar_can_writeb(priv, RCAR_CAN_EIER, 0);
>> + rcar_can_writeb(priv, RCAR_CAN_IER, 0);
>> + can_bus_off(ndev);
> How does the rcan recover from bus off?
Good question... this needs additional work.
[...]
>> +static irqreturn_t rcar_can_interrupt(int irq, void *dev_id)
>> +{
>> + struct net_device *ndev = (struct net_device *)dev_id;
>> + struct rcar_can_priv *priv = netdev_priv(ndev);
>> + struct net_device_stats *stats = &ndev->stats;
>> + u8 isr;
>> +
>> + isr = rcar_can_readb(priv, RCAR_CAN_ISR);
>> + if (isr & ISR_ERSF)
>> + rcar_can_error(ndev);
>> +
>> + if (isr & ISR_TXMF) {
>> + u32 ie_mask = 0;
>> +
>> + /* Set Transmit Mailbox Search Mode */
>> + rcar_can_writeb(priv, RCAR_CAN_MSMR, MSMR_TXMB);
> Can you please outline how to mailbox search mode works? What happens if
> you activate tx mailbox search mode here and rx mailbox search mode below?
Work of the search is not well documented. The controller has 4 search
modes, only one can be enabled at the moment.
I just don't know what happens if we get TX interrupt while handling RX.
Either we painlessly switch to TX search, finish it and return to RX search,
and it continues where we are interrupted... or we lose some RX packets.
>> + while (1) {
> I presonally don't like while (1) loops in an interrupt handler, can you
> rearange the code, so that you have an explizid loop termination?
It was done in order to avoid assignment in the *while* expression.
Although checkpatch.pl didn't complain about it, it does complain about such
in the *if* expression, so I thought it consistent to avoid this in the
*while* too.
>> + u8 mctl, mbx;
>> +
>> + mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
>> + if (mbx & MSSR_SEST)
>> + break;
>> + mbx &= MSSR_MBNST;
>> + mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
>> + /* Bits SENTDATA and TRMREQ cannot be
>> + * set to 0 simultaneously
>> + */
>> + mctl &= ~MCTL_TRMREQ;
>> + rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
>> + mctl &= ~MCTL_SENTDATA;
>> + /* Clear interrupt */
>> + rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
> Can you combine both writes to RCAR_CAN_MCTL(mbx) or does the hardware
> require two separate writes?
Yes, it does. Haven't you seen the comment above the writes?
>> + ie_mask |= BIT(mbx - FIRST_TX_MB);
>> + stats->tx_bytes += can_get_echo_skb(ndev,
>> + mbx - FIRST_TX_MB);
> Can you guarantee that you call can_get_echo_skb in the same order the
> frames get send?
Another good question...
[...]
>> + /* Disable mailbox interrupt, mark mailbox as free */
>> + if (ie_mask) {
>> + u32 mier1;
>> +
>> + spin_lock(&priv->mier_lock);
>> + mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
>> + rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 & ~ie_mask);
>> + spin_unlock(&priv->mier_lock);
>> + if (unlikely(netif_queue_stopped(ndev)))
>> + netif_wake_queue(ndev);
> You can call netif_wake_queue() unconditionally, it does the check for
> queue stopped anyway.
Indeed.
>> + }
>> + }
>> + if (isr & ISR_RXM1F) {
>> + if (napi_schedule_prep(&priv->napi)) {
>> + /* Disable Rx interrupts */
>> + rcar_can_writeb(priv, RCAR_CAN_IER,
>> + rcar_can_readb(priv, RCAR_CAN_IER) &
>> + ~IER_RXM1IE);
>> + __napi_schedule(&priv->napi);
>> + }
>> + }
>> + return IRQ_HANDLED;
> Do not return IRQ_HANDLED unconditionally. You should return IRQ_HANDLED
> only if there really was a interrupt for your device.
Yes, people have already complained about this. I'll add a check of the
interrupt status.
>> +static int rcar_can_set_bittiming(struct net_device *dev)
>> +{
>> + struct rcar_can_priv *priv = netdev_priv(dev);
>> + struct can_bittiming *bt = &priv->can.bittiming;
>> + u32 bcr;
>> + u16 ctlr;
>> + u8 clkr;
>> +
>> + ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
>> + if (ctlr & CTLR_SLPM) {
>> + /* Write to BCR in CAN reset mode or CAN halt mode */
>> + return -EBUSY;
> The framework guarantees that set_bittiming is only called when the
> interface is down.
Exactly. With the current driver code we get rcar_can_set_bittiming()
called when device is not opened. It is in the sleep mode and we can not touch
CTLR register in the sleep mode. I call this function when opening device
explicitly.
[...]
>> +static void rcar_can_start(struct net_device *ndev)
>> +{
>> + struct rcar_can_priv *priv = netdev_priv(ndev);
>> + u16 ctlr, n;
>> +
>> + /* Set controller to known mode:
>> + * - normal mailbox mode (no FIFO);
> Does the controller support FIFO?
Yes.
[...]
>> +static netdev_tx_t rcar_can_start_xmit(struct sk_buff *skb,
>> + struct net_device *ndev)
>> +{
[...]
>> + spin_lock_irqsave(&priv->mier_lock, flags);
>> + mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
>> + if (unlikely(mier1 == ~0U)) {
>> + spin_unlock_irqrestore(&priv->mier_lock, flags);
>> + netif_stop_queue(ndev);
>> + return NETDEV_TX_BUSY;
>> + }
>> + rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 | (mier1 + 1));
>> + spin_unlock_irqrestore(&priv->mier_lock, flags);
>> + mbxno = ffz(mier1) + FIRST_TX_MB;
>
> This smells fishy, for several reasons:
> The driver should guarantee that the frames are send in the same order
> as this function is called. If you pick the first free mailbox I suspect
> the hardware send the first mailbox ready to send. I don't know the
> hardware, but several CAN controllers I've worked with, function in that
> way.
There are two TX priority modes: ID priority transmit mode and mailbox
number transmit mode. Currently, ID priority mode is selected -- I didn't
realize that "the driver should guarantee that the frames are send in the same
order as this function is called". Still driver can work in the mailbox number
priority mode.
ffz(mier1) selects first free mailbox with lowest number. It will have
higher priority.
> Then you should rethink your flow control. You should call
> netif_stop_queue if all your hardware buffers are full, then the above
> NETDEV_TX_BUSY should not happen.
You mean that I should check the state of mailboxes at end of this
function? OK.
[...]
>> + mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbxno));
> Do you have to read mctl here?
Hm, possibly not. Need to re-check...
[...]
>> +static void rcar_can_rx_pkt(struct rcar_can_priv *priv, int mbx)
>> +{
[...]
>> + data = rcar_can_mbx_readl(priv, mbx, MBX_HDR_OFFSET);
>> + if (data & RCAR_CAN_IDE)
>> + cf->can_id = (data & CAN_EFF_MASK) | CAN_EFF_FLAG;
>> + else
>> + cf->can_id = (data >> RCAR_CAN_SID_SHIFT) & CAN_SFF_MASK;
>> + if (data & RCAR_CAN_RTR)
>> + cf->can_id |= CAN_RTR_FLAG;
>> +
>> + dlc = rcar_can_mbx_readb(priv, mbx, MBX_DLC_OFFSET);
>> + cf->can_dlc = get_can_dlc(dlc);
> Please don't copy data on received RTR frames.
OK.
>> + for (dlc = 0; dlc < cf->can_dlc; dlc++)
>> + cf->data[dlc] = rcar_can_mbx_readb(priv, mbx,
>> + MBX_DATA_OFFSET + dlc);
[...]
>> +static int rcar_can_rx_poll(struct napi_struct *napi, int quota)
>> +{
>> + struct rcar_can_priv *priv = container_of(napi,
>> + struct rcar_can_priv, napi);
>> + u32 num_pkts = 0;
> please make it an int
OK, overlooked this.
>> +
>> + /* Find mailbox */
>> + while (1) {
> please put the quota check into the while()
OK.
>> + u8 mctl, mbx;
>> +
>> + mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
>> + if (mbx & MSSR_SEST || num_pkts >= quota)
>> + break;
>> + mbx &= MSSR_MBNST;
>> + mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
>> + /* Clear interrupt */
>> + rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx),
>> + mctl & ~MCTL_NEWDATA);
>> + rcar_can_rx_pkt(priv, mbx);
> Which instruction reenables the mailbox?
Good question. It's not clear from documentation. For sure we can toggle
RECREQ bit. My current understanding is: mailbox is not ready to receive while
MCTL.NEWDATA is set. If so, need to clear interrupt after receive. May need to
check MSGLOST flag -- need to find out how to report overwritten messages.
>> + /* All packets processed */
>> + if (num_pkts < quota) {
>> + u8 ier;
>> +
>> + napi_complete(napi);
>> + ier = rcar_can_readb(priv, RCAR_CAN_IER);
>> + rcar_can_writeb(priv, RCAR_CAN_IER, ier | IER_RXM1IE);
> I the hardware doesn't modify the IER register, you can work on a shadow
> copy and only write, but never need to read from the hardware.
OK.
[...]
>> +static int rcar_can_probe(struct platform_device *pdev)
>> +{
[...]
>> + pdata = pdev->dev.platform_data;
> please use dev_get_platdata()
OK. I saw the patches converting the drivers to using this helper.
>> + clk_enable(priv->clk);
> You are missing the clock's prepare step.
We are not supporting common clock framework yet, so this step should be a
NOP AFAIU.
> You should call
> clk_prepare_enable(). Please move clock_prepare_enable() to the open()
> (and the disable_unprepare() to the close() function) so tha the core is
> not powered as long as the interface is down. You might have to enable
> the clock in the rcar_can_get_berr_counter() as it may be called if the
> interface is still down.
OK, will do.
>> +
>> + ndev->netdev_ops = &rcar_can_netdev_ops;
>> + ndev->irq = irq;
>> + ndev->flags |= IFF_ECHO;
>> + priv->ndev = ndev;
>> + priv->reg_base = addr;
>> + priv->clock_select = pdata->clock_select;
>> + priv->can.clock.freq = clk_get_rate(priv->clk);
>> + priv->can.bittiming_const = &rcar_can_bittiming_const;
>> + priv->can.do_set_bittiming = rcar_can_set_bittiming;
> No need to set this callback, as you're calling rcar_can_set_bittiming
> explizidly.
OK.
>> + priv->can.do_set_mode = rcar_can_do_set_mode;
>> + priv->can.do_get_berr_counter = rcar_can_get_berr_counter;
>> + priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES |
>> + CAN_CTRLMODE_ONE_SHOT;
> You don't handle 3_SAMPLES. Have you tested your driver in ONE_SHOT
I've tested in ONE_SHOT but the controller was forced into that mode.
Didn't find application that sets this mode.
> mode? How does the controller react if a frame cannot be send? Do you
> clear the skb that has previously been can_put_echo_skb()?
Another thing to look at -- maybe related to bus-off question.
>> +static int rcar_can_remove(struct platform_device *pdev)
>> +{
>> + struct net_device *ndev = platform_get_drvdata(pdev);
>> + struct rcar_can_priv *priv = netdev_priv(ndev);
>> + u16 ctlr;
>> +
>> + unregister_candev(ndev);
>> + netif_napi_del(&priv->napi);
>> + /* Go to sleep mode */
>> + ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
>> + rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr | CTLR_SLPM);
> You should put the controller in to sleep mode in close()
Seems OK to do this.
[...]
>> Index: linux-can-next/include/linux/can/platform/rcar_can.h
>> ===================================================================
>> --- /dev/null
>> +++ linux-can-next/include/linux/can/platform/rcar_can.h
>> @@ -0,0 +1,15 @@
>> +#ifndef _CAN_PLATFORM_RCAR_CAN_H_
>> +#define _CAN_PLATFORM_RCAR_CAN_H_
>> +
>> +#include <linux/types.h>
>> +
>> +/* Clock Select Register settings */
>> +#define CLKR_CLKEXT 3 /* Externally input clock */
>> +#define CLKR_CLKP2 1 /* Peripheral clock (clkp2) */
>> +#define CLKR_CLKP1 0 /* Peripheral clock (clkp1) */
> Can this be handled by the clock framework
No.
> and or Device Tree?
Yes.
> Marc
WBR, Sergei
^ permalink raw reply
* Re: [PATCH net-next v3 3/9] static_key: WARN on usage before jump_label_init was called
From: Steven Rostedt @ 2013-10-17 21:19 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev, linux-kernel, Peter Zijlstra, Andi Kleen
In-Reply-To: <1381987923-1524-4-git-send-email-hannes@stressinduktion.org>
On Thu, 17 Oct 2013 07:31:57 +0200
Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> Based on a patch from Andi Kleen.
I'm fine with the patch, but the change log needs a lot more work.
Like, why is this needed? I know, but does anyone else?
-- Steve
>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Andi Kleen <andi@firstfloor.org>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
> include/linux/jump_label.h | 10 ++++++++++
> include/linux/jump_label_ratelimit.h | 2 ++
> init/main.c | 7 +++++++
> kernel/jump_label.c | 5 +++++
> 4 files changed, 24 insertions(+)
>
> diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
> index a507907..e96be72 100644
> --- a/include/linux/jump_label.h
> +++ b/include/linux/jump_label.h
> @@ -48,6 +48,13 @@
>
> #include <linux/types.h>
> #include <linux/compiler.h>
> +#include <linux/bug.h>
> +
> +extern bool static_key_initialized;
> +
> +#define STATIC_KEY_CHECK_USE() WARN(!static_key_initialized, \
> + "%s used before call to jump_label_init", \
> + __func__)
>
> #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)
>
> @@ -128,6 +135,7 @@ struct static_key {
>
> static __always_inline void jump_label_init(void)
> {
> + static_key_initialized = true;
> }
>
> static __always_inline bool static_key_false(struct static_key *key)
> @@ -146,11 +154,13 @@ static __always_inline bool static_key_true(struct static_key *key)
>
> static inline void static_key_slow_inc(struct static_key *key)
> {
> + STATIC_KEY_CHECK_USE();
> atomic_inc(&key->enabled);
> }
>
> static inline void static_key_slow_dec(struct static_key *key)
> {
> + STATIC_KEY_CHECK_USE();
> atomic_dec(&key->enabled);
> }
>
> diff --git a/include/linux/jump_label_ratelimit.h b/include/linux/jump_label_ratelimit.h
> index 1137883..089f70f 100644
> --- a/include/linux/jump_label_ratelimit.h
> +++ b/include/linux/jump_label_ratelimit.h
> @@ -23,12 +23,14 @@ struct static_key_deferred {
> };
> static inline void static_key_slow_dec_deferred(struct static_key_deferred *key)
> {
> + STATIC_KEY_CHECK_USE();
> static_key_slow_dec(&key->key);
> }
> static inline void
> jump_label_rate_limit(struct static_key_deferred *key,
> unsigned long rl)
> {
> + STATIC_KEY_CHECK_USE();
> }
> #endif /* HAVE_JUMP_LABEL */
> #endif /* _LINUX_JUMP_LABEL_RATELIMIT_H */
> diff --git a/init/main.c b/init/main.c
> index af310af..27bbec1a 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -136,6 +136,13 @@ static char *execute_command;
> static char *ramdisk_execute_command;
>
> /*
> + * Used to generate warnings if static_key manipulation functions are used
> + * before jump_label_init is called.
> + */
> +bool static_key_initialized __read_mostly = false;
> +EXPORT_SYMBOL_GPL(static_key_initialized);
> +
> +/*
> * If set, this is an indication to the drivers that reset the underlying
> * device before going ahead with the initialization otherwise driver might
> * rely on the BIOS and skip the reset operation.
> diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> index 297a924..9019f15 100644
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -58,6 +58,7 @@ static void jump_label_update(struct static_key *key, int enable);
>
> void static_key_slow_inc(struct static_key *key)
> {
> + STATIC_KEY_CHECK_USE();
> if (atomic_inc_not_zero(&key->enabled))
> return;
>
> @@ -103,12 +104,14 @@ static void jump_label_update_timeout(struct work_struct *work)
>
> void static_key_slow_dec(struct static_key *key)
> {
> + STATIC_KEY_CHECK_USE();
> __static_key_slow_dec(key, 0, NULL);
> }
> EXPORT_SYMBOL_GPL(static_key_slow_dec);
>
> void static_key_slow_dec_deferred(struct static_key_deferred *key)
> {
> + STATIC_KEY_CHECK_USE();
> __static_key_slow_dec(&key->key, key->timeout, &key->work);
> }
> EXPORT_SYMBOL_GPL(static_key_slow_dec_deferred);
> @@ -116,6 +119,7 @@ EXPORT_SYMBOL_GPL(static_key_slow_dec_deferred);
> void jump_label_rate_limit(struct static_key_deferred *key,
> unsigned long rl)
> {
> + STATIC_KEY_CHECK_USE();
> key->timeout = rl;
> INIT_DELAYED_WORK(&key->work, jump_label_update_timeout);
> }
> @@ -212,6 +216,7 @@ void __init jump_label_init(void)
> key->next = NULL;
> #endif
> }
> + static_key_initialized = true;
> jump_label_unlock();
> }
>
^ permalink raw reply
* Oficiální vyhlá?ení OZNÁMENÍ
From: undp_chevronpetroleum1 @ 2013-10-17 20:16 UTC (permalink / raw)
Číslo ?ar?e : ( UK- 209-634 , E- 931-51 )
Oficiální vyhlá?ení OZNÁMENÍ
To je Vám oznámit, ?e jste byl vybrán pro peně?ní cenu
1,600,000.00 GB kilo mezinárodních programů uskutečňovaných na 15.října
2013 ve Spojeném království Chevron ropy / OSN o rozvoj
Program ( UNDP) Chcete-li zahájit zpracování va?í cenu jste kontaktovat
va?e nároky důstojník prostřednictvím na?ich akreditovaných agentů Prize Přenos jak je uvedeno
ní?e :
Mr.D. Matt
Organizace spojených národů Komplexní
PE9 2YP , Londýn
Spojené království
E- mail: derickmatt@googlemail.com
Kontaktujte jej , a poskytne mu na tajnou PIN kódu x5pukg2013 a
Va?e referenční číslo pro UNDP : 62082013EA-UK/21 . Také se Vám doporučuje
aby mu na základě uvedených informací, co nejdříve :
NÁROK PO?ADAVEK :
1.Name v plné vý?i : , 2.Address : , 3.Phone/Fax : ,
S pozdravem ,
Mrs.Tulise hnědá
Program Coordinator
POZNÁMKA: Chci, abyste věděli, ?e máme tři ?ťastné výherce v České republice se stalo, ?e jeden z nich
bude i rady, které byste se obrátit na vý?e uvedené tvrzení důstojníka co nejdříve .
^ 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