* sunhme: some cleanups
@ 2022-02-03 16:20 Rolf Eike Beer
2022-02-03 16:21 ` [PATCH 1/3] sunhme: remove unused tx_dump_ring() Rolf Eike Beer
` (3 more replies)
0 siblings, 4 replies; 23+ messages in thread
From: Rolf Eike Beer @ 2022-02-03 16:20 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 812 bytes --]
Hi,
it still matches my sense of humor to see fries offered in a network driver,
so every now and then I point someone to the driver. This time I looked a bit
more around and noticed some things I have already done wrong before myself.
Time to clean them up so it's still funny while serving as a good example ;)
Things that are changed:
-drop unused tx_dump_ring()
-fix version number inconsistency
-forward error code from pci_enable_device()
Things that I may also do in the, but can't test because I don't have any
hardware:
-switch to devres, which fixes a memleak of the dma_alloc_coherent() memory if
register_netdev() fails [mostly done]
-use netdev_* print macros to print the device name in a consistent way
I can also send an immediate fix for the first one if you prefer.
Greetings,
Eike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH 1/3] sunhme: remove unused tx_dump_ring() 2022-02-03 16:20 sunhme: some cleanups Rolf Eike Beer @ 2022-02-03 16:21 ` Rolf Eike Beer 2022-07-27 3:42 ` Sean Anderson 2022-02-03 16:22 ` [PATCH 2/3] sunhme: fix the version number in struct ethtool_drvinfo Rolf Eike Beer ` (2 subsequent siblings) 3 siblings, 1 reply; 23+ messages in thread From: Rolf Eike Beer @ 2022-02-03 16:21 UTC (permalink / raw) To: netdev I can't find a reference to it in the entire git history. Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> --- drivers/net/ethernet/sun/sunhme.c | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c index ad9029ae6848..22abfe58f728 100644 --- a/drivers/net/ethernet/sun/sunhme.c +++ b/drivers/net/ethernet/sun/sunhme.c @@ -135,25 +135,9 @@ static __inline__ void tx_dump_log(void) this = (this + 1) & (TX_LOG_LEN - 1); } } -static __inline__ void tx_dump_ring(struct happy_meal *hp) -{ - struct hmeal_init_block *hb = hp->happy_block; - struct happy_meal_txd *tp = &hb->happy_meal_txd[0]; - int i; - - for (i = 0; i < TX_RING_SIZE; i+=4) { - printk("TXD[%d..%d]: [%08x:%08x] [%08x:%08x] [%08x:%08x] [%08x:%08x]\n", - i, i + 4, - le32_to_cpu(tp[i].tx_flags), le32_to_cpu(tp[i].tx_addr), - le32_to_cpu(tp[i + 1].tx_flags), le32_to_cpu(tp[i + 1].tx_addr), - le32_to_cpu(tp[i + 2].tx_flags), le32_to_cpu(tp[i + 2].tx_addr), - le32_to_cpu(tp[i + 3].tx_flags), le32_to_cpu(tp[i + 3].tx_addr)); - } -} #else #define tx_add_log(hp, a, s) do { } while(0) #define tx_dump_log() do { } while(0) -#define tx_dump_ring(hp) do { } while(0) #endif #ifdef HMEDEBUG -- 2.31.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] sunhme: remove unused tx_dump_ring() 2022-02-03 16:21 ` [PATCH 1/3] sunhme: remove unused tx_dump_ring() Rolf Eike Beer @ 2022-07-27 3:42 ` Sean Anderson 0 siblings, 0 replies; 23+ messages in thread From: Sean Anderson @ 2022-07-27 3:42 UTC (permalink / raw) To: Rolf Eike Beer, netdev On 2/3/22 11:21 AM, Rolf Eike Beer wrote: > I can't find a reference to it in the entire git history. > > Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> > --- This is probably just there to be enabled for debugging. In any case, Reviewed-by: Sean Anderson <seanga2@gmail.com> ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/3] sunhme: fix the version number in struct ethtool_drvinfo 2022-02-03 16:20 sunhme: some cleanups Rolf Eike Beer 2022-02-03 16:21 ` [PATCH 1/3] sunhme: remove unused tx_dump_ring() Rolf Eike Beer @ 2022-02-03 16:22 ` Rolf Eike Beer 2022-02-03 17:12 ` Andrew Lunn ` (2 more replies) 2022-02-03 16:23 ` [PATCH 3/3] sunhme: forward the error code from pci_enable_device() Rolf Eike Beer 2022-02-14 18:31 ` [PATCH 4/x] sunhme: switch to devres Rolf Eike Beer 3 siblings, 3 replies; 23+ messages in thread From: Rolf Eike Beer @ 2022-02-03 16:22 UTC (permalink / raw) To: netdev Fixes: 050bbb196392b9c178f82b1205a23dd2f915ee93 Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> --- drivers/net/ethernet/sun/sunhme.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c index 22abfe58f728..43834339bb43 100644 --- a/drivers/net/ethernet/sun/sunhme.c +++ b/drivers/net/ethernet/sun/sunhme.c @@ -2470,8 +2470,8 @@ static void hme_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info { struct happy_meal *hp = netdev_priv(dev); - strlcpy(info->driver, "sunhme", sizeof(info->driver)); - strlcpy(info->version, "2.02", sizeof(info->version)); + strlcpy(info->driver, DRV_NAME, sizeof(info->driver)); + strlcpy(info->version, DRV_VERSION, sizeof(info->version)); if (hp->happy_flags & HFLAG_PCI) { struct pci_dev *pdev = hp->happy_dev; strlcpy(info->bus_info, pci_name(pdev), sizeof(info->bus_info)); -- 2.31.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] sunhme: fix the version number in struct ethtool_drvinfo 2022-02-03 16:22 ` [PATCH 2/3] sunhme: fix the version number in struct ethtool_drvinfo Rolf Eike Beer @ 2022-02-03 17:12 ` Andrew Lunn 2022-02-05 11:27 ` Rolf Eike Beer 2022-02-03 21:53 ` Jakub Kicinski 2022-02-14 18:33 ` [PATCH 2/3 v2] " Rolf Eike Beer 2 siblings, 1 reply; 23+ messages in thread From: Andrew Lunn @ 2022-02-03 17:12 UTC (permalink / raw) To: Rolf Eike Beer; +Cc: netdev On Thu, Feb 03, 2022 at 05:22:23PM +0100, Rolf Eike Beer wrote: > Fixes: 050bbb196392b9c178f82b1205a23dd2f915ee93 > Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> > --- > drivers/net/ethernet/sun/sunhme.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c > index 22abfe58f728..43834339bb43 100644 > --- a/drivers/net/ethernet/sun/sunhme.c > +++ b/drivers/net/ethernet/sun/sunhme.c > @@ -2470,8 +2470,8 @@ static void hme_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info > { > struct happy_meal *hp = netdev_priv(dev); > > - strlcpy(info->driver, "sunhme", sizeof(info->driver)); > - strlcpy(info->version, "2.02", sizeof(info->version)); > + strlcpy(info->driver, DRV_NAME, sizeof(info->driver)); > + strlcpy(info->version, DRV_VERSION, sizeof(info->version)); I would suggest you drop setting info->version. The kernel will fill it with the current kernel version, which is much more meaningful. Andrew ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] sunhme: fix the version number in struct ethtool_drvinfo 2022-02-03 17:12 ` Andrew Lunn @ 2022-02-05 11:27 ` Rolf Eike Beer 2022-02-05 14:48 ` Andrew Lunn 0 siblings, 1 reply; 23+ messages in thread From: Rolf Eike Beer @ 2022-02-05 11:27 UTC (permalink / raw) To: Andrew Lunn; +Cc: netdev [-- Attachment #1: Type: text/plain, Size: 1244 bytes --] Am Donnerstag, 3. Februar 2022, 18:12:40 CET schrieb Andrew Lunn: > On Thu, Feb 03, 2022 at 05:22:23PM +0100, Rolf Eike Beer wrote: > > Fixes: 050bbb196392b9c178f82b1205a23dd2f915ee93 > > Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> > > --- > > > > drivers/net/ethernet/sun/sunhme.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/ethernet/sun/sunhme.c > > b/drivers/net/ethernet/sun/sunhme.c index 22abfe58f728..43834339bb43 > > 100644 > > --- a/drivers/net/ethernet/sun/sunhme.c > > +++ b/drivers/net/ethernet/sun/sunhme.c > > @@ -2470,8 +2470,8 @@ static void hme_get_drvinfo(struct net_device *dev, > > struct ethtool_drvinfo *info> > > { > > > > struct happy_meal *hp = netdev_priv(dev); > > > > - strlcpy(info->driver, "sunhme", sizeof(info->driver)); > > - strlcpy(info->version, "2.02", sizeof(info->version)); > > + strlcpy(info->driver, DRV_NAME, sizeof(info->driver)); > > + strlcpy(info->version, DRV_VERSION, sizeof(info->version)); > > I would suggest you drop setting info->version. The kernel will fill > it with the current kernel version, which is much more meaningful. Would it make sense to completely remove the version number from the driver then? Eike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] sunhme: fix the version number in struct ethtool_drvinfo 2022-02-05 11:27 ` Rolf Eike Beer @ 2022-02-05 14:48 ` Andrew Lunn 2022-02-05 15:57 ` Rolf Eike Beer 0 siblings, 1 reply; 23+ messages in thread From: Andrew Lunn @ 2022-02-05 14:48 UTC (permalink / raw) To: Rolf Eike Beer; +Cc: netdev > > > struct ethtool_drvinfo *info> > > > { > > > > > > struct happy_meal *hp = netdev_priv(dev); > > > > > > - strlcpy(info->driver, "sunhme", sizeof(info->driver)); > > > - strlcpy(info->version, "2.02", sizeof(info->version)); > > > + strlcpy(info->driver, DRV_NAME, sizeof(info->driver)); > > > + strlcpy(info->version, DRV_VERSION, sizeof(info->version)); > > > > I would suggest you drop setting info->version. The kernel will fill > > it with the current kernel version, which is much more meaningful. > > Would it make sense to completely remove the version number from the driver > then? If it is not used anywhere else, yes, you can remove it. Andrew ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] sunhme: fix the version number in struct ethtool_drvinfo 2022-02-05 14:48 ` Andrew Lunn @ 2022-02-05 15:57 ` Rolf Eike Beer 2022-02-05 16:32 ` Andrew Lunn 0 siblings, 1 reply; 23+ messages in thread From: Rolf Eike Beer @ 2022-02-05 15:57 UTC (permalink / raw) To: Andrew Lunn; +Cc: netdev [-- Attachment #1: Type: text/plain, Size: 884 bytes --] Am Samstag, 5. Februar 2022, 15:48:41 CET schrieb Andrew Lunn: > > > > struct ethtool_drvinfo *info> > > > > > > > > { > > > > > > > > struct happy_meal *hp = netdev_priv(dev); > > > > > > > > - strlcpy(info->driver, "sunhme", sizeof(info->driver)); > > > > - strlcpy(info->version, "2.02", sizeof(info->version)); > > > > + strlcpy(info->driver, DRV_NAME, sizeof(info->driver)); > > > > + strlcpy(info->version, DRV_VERSION, sizeof(info->version)); > > > > > > I would suggest you drop setting info->version. The kernel will fill > > > it with the current kernel version, which is much more meaningful. > > > > Would it make sense to completely remove the version number from the > > driver > > then? > > If it is not used anywhere else, yes, you can remove it. Of course it prints the number on module load… but otherwise it does nothing with it. [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] sunhme: fix the version number in struct ethtool_drvinfo 2022-02-05 15:57 ` Rolf Eike Beer @ 2022-02-05 16:32 ` Andrew Lunn 0 siblings, 0 replies; 23+ messages in thread From: Andrew Lunn @ 2022-02-05 16:32 UTC (permalink / raw) To: Rolf Eike Beer; +Cc: netdev > > If it is not used anywhere else, yes, you can remove it. > > Of course it prints the number on module load… but otherwise it does nothing > with it. You could remove that as well. The basic problem is, the version string does not identify the sources with enough accuracy. It says nothing about back ported fixes in stable kernels. It tells you nothing about vendor patches to the network core, etc. Andrew ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] sunhme: fix the version number in struct ethtool_drvinfo 2022-02-03 16:22 ` [PATCH 2/3] sunhme: fix the version number in struct ethtool_drvinfo Rolf Eike Beer 2022-02-03 17:12 ` Andrew Lunn @ 2022-02-03 21:53 ` Jakub Kicinski 2022-02-14 18:33 ` [PATCH 2/3 v2] " Rolf Eike Beer 2 siblings, 0 replies; 23+ messages in thread From: Jakub Kicinski @ 2022-02-03 21:53 UTC (permalink / raw) To: Rolf Eike Beer; +Cc: netdev On Thu, 03 Feb 2022 17:22:23 +0100 Rolf Eike Beer wrote: > Fixes: 050bbb196392b9c178f82b1205a23dd2f915ee93 This is not the correct format for a fixes tag, FWIW. ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/3 v2] sunhme: fix the version number in struct ethtool_drvinfo 2022-02-03 16:22 ` [PATCH 2/3] sunhme: fix the version number in struct ethtool_drvinfo Rolf Eike Beer 2022-02-03 17:12 ` Andrew Lunn 2022-02-03 21:53 ` Jakub Kicinski @ 2022-02-14 18:33 ` Rolf Eike Beer 2 siblings, 0 replies; 23+ messages in thread From: Rolf Eike Beer @ 2022-02-14 18:33 UTC (permalink / raw) To: netdev The driver prints a version number on startup, use the same one here. Fixes: 050bbb196392 ("[NET] sunhme: Convert to new SBUS driver framework.") Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> --- v2: correct "Fixes" line syntax drivers/net/ethernet/sun/sunhme.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c index 22abfe58f728..43834339bb43 100644 --- a/drivers/net/ethernet/sun/sunhme.c +++ b/drivers/net/ethernet/sun/sunhme.c @@ -2470,8 +2470,8 @@ static void hme_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info { struct happy_meal *hp = netdev_priv(dev); - strlcpy(info->driver, "sunhme", sizeof(info->driver)); - strlcpy(info->version, "2.02", sizeof(info->version)); + strlcpy(info->driver, DRV_NAME, sizeof(info->driver)); + strlcpy(info->version, DRV_VERSION, sizeof(info->version)); if (hp->happy_flags & HFLAG_PCI) { struct pci_dev *pdev = hp->happy_dev; strlcpy(info->bus_info, pci_name(pdev), sizeof(info->bus_info)); -- 2.34.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 3/3] sunhme: forward the error code from pci_enable_device() 2022-02-03 16:20 sunhme: some cleanups Rolf Eike Beer 2022-02-03 16:21 ` [PATCH 1/3] sunhme: remove unused tx_dump_ring() Rolf Eike Beer 2022-02-03 16:22 ` [PATCH 2/3] sunhme: fix the version number in struct ethtool_drvinfo Rolf Eike Beer @ 2022-02-03 16:23 ` Rolf Eike Beer 2022-07-27 3:48 ` Sean Anderson 2022-02-14 18:31 ` [PATCH 4/x] sunhme: switch to devres Rolf Eike Beer 3 siblings, 1 reply; 23+ messages in thread From: Rolf Eike Beer @ 2022-02-03 16:23 UTC (permalink / raw) To: netdev This already returns a proper error value, so pass it to the caller. Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> --- drivers/net/ethernet/sun/sunhme.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c index 43834339bb43..980a936ce8d1 100644 --- a/drivers/net/ethernet/sun/sunhme.c +++ b/drivers/net/ethernet/sun/sunhme.c @@ -2969,11 +2969,12 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, strcpy(prom_name, "SUNW,hme"); #endif - err = -ENODEV; + err = pci_enable_device(pdev); - if (pci_enable_device(pdev)) + if (err) goto err_out; pci_set_master(pdev); + err = -ENODEV; if (!strcmp(prom_name, "SUNW,qfe") || !strcmp(prom_name, "qfe")) { qp = quattro_pci_find(pdev); -- 2.31.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 3/3] sunhme: forward the error code from pci_enable_device() 2022-02-03 16:23 ` [PATCH 3/3] sunhme: forward the error code from pci_enable_device() Rolf Eike Beer @ 2022-07-27 3:48 ` Sean Anderson 0 siblings, 0 replies; 23+ messages in thread From: Sean Anderson @ 2022-07-27 3:48 UTC (permalink / raw) To: Rolf Eike Beer, netdev On 2/3/22 11:23 AM, Rolf Eike Beer wrote: > This already returns a proper error value, so pass it to the caller. > > Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> > --- > drivers/net/ethernet/sun/sunhme.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c > index 43834339bb43..980a936ce8d1 100644 > --- a/drivers/net/ethernet/sun/sunhme.c > +++ b/drivers/net/ethernet/sun/sunhme.c > @@ -2969,11 +2969,12 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > strcpy(prom_name, "SUNW,hme"); > #endif > > - err = -ENODEV; > + err = pci_enable_device(pdev); > > - if (pci_enable_device(pdev)) > + if (err) > goto err_out; > pci_set_master(pdev); > + err = -ENODEV; > if (!strcmp(prom_name, "SUNW,qfe") || !strcmp(prom_name, "qfe")) { > qp = quattro_pci_find(pdev); > This is incrementally better, but there are several more calls like this. A follow up converting those as well would be good. Reviewed-by: Sean Anderson <seanga2@gmail.com> ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 4/x] sunhme: switch to devres 2022-02-03 16:20 sunhme: some cleanups Rolf Eike Beer ` (2 preceding siblings ...) 2022-02-03 16:23 ` [PATCH 3/3] sunhme: forward the error code from pci_enable_device() Rolf Eike Beer @ 2022-02-14 18:31 ` Rolf Eike Beer 2022-07-27 3:49 ` Sean Anderson 2022-08-29 13:22 ` [PATCH 4/4 v2] " Rolf Eike Beer 3 siblings, 2 replies; 23+ messages in thread From: Rolf Eike Beer @ 2022-02-14 18:31 UTC (permalink / raw) To: netdev This not only removes a lot of code, it also fixes the memleak of the DMA memory when register_netdev() fails. Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> --- drivers/net/ethernet/sun/sunhme.c | 55 +++++++++---------------------- 1 file changed, 16 insertions(+), 39 deletions(-) diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c index 980a936ce8d1..ec78f43f75c9 100644 --- a/drivers/net/ethernet/sun/sunhme.c +++ b/drivers/net/ethernet/sun/sunhme.c @@ -2952,7 +2952,6 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, struct happy_meal *hp; struct net_device *dev; void __iomem *hpreg_base; - unsigned long hpreg_res; int i, qfe_slot = -1; char prom_name[64]; u8 addr[ETH_ALEN]; @@ -2969,7 +2968,7 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, strcpy(prom_name, "SUNW,hme"); #endif - err = pci_enable_device(pdev); + err = pcim_enable_device(pdev); if (err) goto err_out; @@ -2987,10 +2986,11 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, goto err_out; } - dev = alloc_etherdev(sizeof(struct happy_meal)); - err = -ENOMEM; - if (!dev) + dev = devm_alloc_etherdev(&pdev->dev, sizeof(struct happy_meal)); + if (!dev) { + err = -ENOMEM; goto err_out; + } SET_NETDEV_DEV(dev, &pdev->dev); if (hme_version_printed++ == 0) @@ -3009,21 +3009,23 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, qp->happy_meals[qfe_slot] = dev; } - hpreg_res = pci_resource_start(pdev, 0); - err = -ENODEV; if ((pci_resource_flags(pdev, 0) & IORESOURCE_IO) != 0) { printk(KERN_ERR "happymeal(PCI): Cannot find proper PCI device base address.\n"); goto err_out_clear_quattro; } - if (pci_request_regions(pdev, DRV_NAME)) { + + if (!devm_request_region(&pdev->dev, pci_resource_start(pdev, 0), + pci_resource_len(pdev, 0), + DRV_NAME)) { printk(KERN_ERR "happymeal(PCI): Cannot obtain PCI resources, " "aborting.\n"); goto err_out_clear_quattro; } - if ((hpreg_base = ioremap(hpreg_res, 0x8000)) == NULL) { + hpreg_base = pcim_iomap(pdev, 0, 0x8000); + if (!hpreg_base) { printk(KERN_ERR "happymeal(PCI): Unable to remap card memory.\n"); - goto err_out_free_res; + goto err_out_clear_quattro; } for (i = 0; i < 6; i++) { @@ -3089,11 +3091,10 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, hp->happy_bursts = DMA_BURSTBITS; #endif - hp->happy_block = dma_alloc_coherent(&pdev->dev, PAGE_SIZE, + hp->happy_block = dmam_alloc_coherent(&pdev->dev, PAGE_SIZE, &hp->hblock_dvma, GFP_KERNEL); - err = -ENODEV; if (!hp->happy_block) - goto err_out_iounmap; + goto err_out_clear_quattro; hp->linkcheck = 0; hp->timer_state = asleep; @@ -3127,11 +3128,11 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, happy_meal_set_initial_advertisement(hp); spin_unlock_irq(&hp->happy_lock); - err = register_netdev(hp->dev); + err = devm_register_netdev(&pdev->dev, dev); if (err) { printk(KERN_ERR "happymeal(PCI): Cannot register net device, " "aborting.\n"); - goto err_out_iounmap; + goto err_out_clear_quattro; } pci_set_drvdata(pdev, hp); @@ -3164,37 +3165,14 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, return 0; -err_out_iounmap: - iounmap(hp->gregs); - -err_out_free_res: - pci_release_regions(pdev); - err_out_clear_quattro: if (qp != NULL) qp->happy_meals[qfe_slot] = NULL; - free_netdev(dev); - err_out: return err; } -static void happy_meal_pci_remove(struct pci_dev *pdev) -{ - struct happy_meal *hp = pci_get_drvdata(pdev); - struct net_device *net_dev = hp->dev; - - unregister_netdev(net_dev); - - dma_free_coherent(hp->dma_dev, PAGE_SIZE, - hp->happy_block, hp->hblock_dvma); - iounmap(hp->gregs); - pci_release_regions(hp->happy_dev); - - free_netdev(net_dev); -} - static const struct pci_device_id happymeal_pci_ids[] = { { PCI_DEVICE(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_HAPPYMEAL) }, { } /* Terminating entry */ @@ -3206,7 +3184,6 @@ static struct pci_driver hme_pci_driver = { .name = "hme", .id_table = happymeal_pci_ids, .probe = happy_meal_pci_probe, - .remove = happy_meal_pci_remove, }; static int __init happy_meal_pci_init(void) -- 2.34.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 4/x] sunhme: switch to devres 2022-02-14 18:31 ` [PATCH 4/x] sunhme: switch to devres Rolf Eike Beer @ 2022-07-27 3:49 ` Sean Anderson 2022-07-27 3:58 ` Sean Anderson 2022-08-29 13:22 ` [PATCH 4/4 v2] " Rolf Eike Beer 1 sibling, 1 reply; 23+ messages in thread From: Sean Anderson @ 2022-07-27 3:49 UTC (permalink / raw) To: Rolf Eike Beer, netdev On 2/14/22 1:31 PM, Rolf Eike Beer wrote: > This not only removes a lot of code, it also fixes the memleak of the DMA > memory when register_netdev() fails. > > Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> > --- > drivers/net/ethernet/sun/sunhme.c | 55 +++++++++---------------------- > 1 file changed, 16 insertions(+), 39 deletions(-) > > diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c > index 980a936ce8d1..ec78f43f75c9 100644 > --- a/drivers/net/ethernet/sun/sunhme.c > +++ b/drivers/net/ethernet/sun/sunhme.c > @@ -2952,7 +2952,6 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > struct happy_meal *hp; > struct net_device *dev; > void __iomem *hpreg_base; > - unsigned long hpreg_res; > int i, qfe_slot = -1; > char prom_name[64]; > u8 addr[ETH_ALEN]; > @@ -2969,7 +2968,7 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > strcpy(prom_name, "SUNW,hme"); > #endif > > - err = pci_enable_device(pdev); > + err = pcim_enable_device(pdev); > > if (err) > goto err_out; > @@ -2987,10 +2986,11 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > goto err_out; > } > > - dev = alloc_etherdev(sizeof(struct happy_meal)); > - err = -ENOMEM; > - if (!dev) > + dev = devm_alloc_etherdev(&pdev->dev, sizeof(struct happy_meal)); > + if (!dev) { > + err = -ENOMEM; > goto err_out; > + } > SET_NETDEV_DEV(dev, &pdev->dev); > > if (hme_version_printed++ == 0) > @@ -3009,21 +3009,23 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > qp->happy_meals[qfe_slot] = dev; > } > > - hpreg_res = pci_resource_start(pdev, 0); > - err = -ENODEV; > if ((pci_resource_flags(pdev, 0) & IORESOURCE_IO) != 0) { > printk(KERN_ERR "happymeal(PCI): Cannot find proper PCI device base address.\n"); > goto err_out_clear_quattro; > } > - if (pci_request_regions(pdev, DRV_NAME)) { > + > + if (!devm_request_region(&pdev->dev, pci_resource_start(pdev, 0), > + pci_resource_len(pdev, 0), > + DRV_NAME)) { > printk(KERN_ERR "happymeal(PCI): Cannot obtain PCI resources, " > "aborting.\n"); > goto err_out_clear_quattro; > } > > - if ((hpreg_base = ioremap(hpreg_res, 0x8000)) == NULL) { > + hpreg_base = pcim_iomap(pdev, 0, 0x8000); > + if (!hpreg_base) { > printk(KERN_ERR "happymeal(PCI): Unable to remap card memory.\n"); > - goto err_out_free_res; > + goto err_out_clear_quattro; > } > > for (i = 0; i < 6; i++) { > @@ -3089,11 +3091,10 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > hp->happy_bursts = DMA_BURSTBITS; > #endif > > - hp->happy_block = dma_alloc_coherent(&pdev->dev, PAGE_SIZE, > + hp->happy_block = dmam_alloc_coherent(&pdev->dev, PAGE_SIZE, > &hp->hblock_dvma, GFP_KERNEL); > - err = -ENODEV; > if (!hp->happy_block) > - goto err_out_iounmap; > + goto err_out_clear_quattro; > > hp->linkcheck = 0; > hp->timer_state = asleep; > @@ -3127,11 +3128,11 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > happy_meal_set_initial_advertisement(hp); > spin_unlock_irq(&hp->happy_lock); > > - err = register_netdev(hp->dev); > + err = devm_register_netdev(&pdev->dev, dev); > if (err) { > printk(KERN_ERR "happymeal(PCI): Cannot register net device, " > "aborting.\n"); > - goto err_out_iounmap; > + goto err_out_clear_quattro; > } > > pci_set_drvdata(pdev, hp); > @@ -3164,37 +3165,14 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > > return 0; > > -err_out_iounmap: > - iounmap(hp->gregs); > - > -err_out_free_res: > - pci_release_regions(pdev); > - > err_out_clear_quattro: > if (qp != NULL) > qp->happy_meals[qfe_slot] = NULL; > > - free_netdev(dev); > - > err_out: > return err; > } > > -static void happy_meal_pci_remove(struct pci_dev *pdev) > -{ > - struct happy_meal *hp = pci_get_drvdata(pdev); > - struct net_device *net_dev = hp->dev; > - > - unregister_netdev(net_dev); > - > - dma_free_coherent(hp->dma_dev, PAGE_SIZE, > - hp->happy_block, hp->hblock_dvma); > - iounmap(hp->gregs); > - pci_release_regions(hp->happy_dev); > - > - free_netdev(net_dev); > -} > - > static const struct pci_device_id happymeal_pci_ids[] = { > { PCI_DEVICE(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_HAPPYMEAL) }, > { } /* Terminating entry */ > @@ -3206,7 +3184,6 @@ static struct pci_driver hme_pci_driver = { > .name = "hme", > .id_table = happymeal_pci_ids, > .probe = happy_meal_pci_probe, > - .remove = happy_meal_pci_remove, > }; > > static int __init happy_meal_pci_init(void) > This looks good, but doesn't apply cleanly. I rebased it as follows: From 5acfa13935277e312361c5630b49aea02399b8b8 Mon Sep 17 00:00:00 2001 From: Rolf Eike Beer <eike-kernel@sf-tec.de> Date: Mon, 14 Feb 2022 19:31:09 +0100 Subject: [PATCH] sunhme: switch to devres This not only removes a lot of code, it also fixes the memleak of the DMA memory when register_netdev() fails. Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> [ rebased onto net-next/master ] Signed-off-by: Sean Anderson <seanga2@gmail.com> Reviewed-by: Sean Anderson <seanga2@gmail.com> --- drivers/net/ethernet/sun/sunhme.c | 59 +++++++++---------------------- 1 file changed, 16 insertions(+), 43 deletions(-) diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c index eebe8c5f480c..e83774ffaa7a 100644 --- a/drivers/net/ethernet/sun/sunhme.c +++ b/drivers/net/ethernet/sun/sunhme.c @@ -2933,7 +2933,6 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, struct happy_meal *hp; struct net_device *dev; void __iomem *hpreg_base; - unsigned long hpreg_res; int i, qfe_slot = -1; char prom_name[64]; u8 addr[ETH_ALEN]; @@ -2950,7 +2949,7 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, strcpy(prom_name, "SUNW,hme"); #endif - err = pci_enable_device(pdev); + err = pcim_enable_device(pdev); if (err) goto err_out; @@ -2968,10 +2967,11 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, goto err_out; } - dev = alloc_etherdev(sizeof(struct happy_meal)); - err = -ENOMEM; - if (!dev) + dev = devm_alloc_etherdev(&pdev->dev, sizeof(struct happy_meal)); + if (!dev) { + err = -ENOMEM; goto err_out; + } SET_NETDEV_DEV(dev, &pdev->dev); if (hme_version_printed++ == 0) @@ -2990,21 +2990,23 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, qp->happy_meals[qfe_slot] = dev; } - hpreg_res = pci_resource_start(pdev, 0); - err = -ENODEV; if ((pci_resource_flags(pdev, 0) & IORESOURCE_IO) != 0) { printk(KERN_ERR "happymeal(PCI): Cannot find proper PCI device base address.\n"); goto err_out_clear_quattro; } - if (pci_request_regions(pdev, DRV_NAME)) { + + if (!devm_request_region(&pdev->dev, pci_resource_start(pdev, 0), + pci_resource_len(pdev, 0), + DRV_NAME)) { printk(KERN_ERR "happymeal(PCI): Cannot obtain PCI resources, " "aborting.\n"); goto err_out_clear_quattro; } - if ((hpreg_base = ioremap(hpreg_res, 0x8000)) == NULL) { + hpreg_base = pcim_iomap(pdev, 0, 0x8000); + if (!hpreg_base) { printk(KERN_ERR "happymeal(PCI): Unable to remap card memory.\n"); - goto err_out_free_res; + goto err_out_clear_quattro; } for (i = 0; i < 6; i++) { @@ -3070,11 +3072,10 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, hp->happy_bursts = DMA_BURSTBITS; #endif - hp->happy_block = dma_alloc_coherent(&pdev->dev, PAGE_SIZE, + hp->happy_block = dmam_alloc_coherent(&pdev->dev, PAGE_SIZE, &hp->hblock_dvma, GFP_KERNEL); - err = -ENODEV; if (!hp->happy_block) - goto err_out_iounmap; + goto err_out_clear_quattro; hp->linkcheck = 0; hp->timer_state = asleep; @@ -3108,11 +3109,11 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, happy_meal_set_initial_advertisement(hp); spin_unlock_irq(&hp->happy_lock); - err = register_netdev(hp->dev); + err = devm_register_netdev(&pdev->dev, dev); if (err) { printk(KERN_ERR "happymeal(PCI): Cannot register net device, " "aborting.\n"); - goto err_out_free_coherent; + goto err_out_clear_quattro; } pci_set_drvdata(pdev, hp); @@ -3145,41 +3146,14 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, return 0; -err_out_free_coherent: - dma_free_coherent(hp->dma_dev, PAGE_SIZE, - hp->happy_block, hp->hblock_dvma); - -err_out_iounmap: - iounmap(hp->gregs); - -err_out_free_res: - pci_release_regions(pdev); - err_out_clear_quattro: if (qp != NULL) qp->happy_meals[qfe_slot] = NULL; - free_netdev(dev); - err_out: return err; } -static void happy_meal_pci_remove(struct pci_dev *pdev) -{ - struct happy_meal *hp = pci_get_drvdata(pdev); - struct net_device *net_dev = hp->dev; - - unregister_netdev(net_dev); - - dma_free_coherent(hp->dma_dev, PAGE_SIZE, - hp->happy_block, hp->hblock_dvma); - iounmap(hp->gregs); - pci_release_regions(hp->happy_dev); - - free_netdev(net_dev); -} - static const struct pci_device_id happymeal_pci_ids[] = { { PCI_DEVICE(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_HAPPYMEAL) }, { } /* Terminating entry */ @@ -3191,7 +3165,6 @@ static struct pci_driver hme_pci_driver = { .name = "hme", .id_table = happymeal_pci_ids, .probe = happy_meal_pci_probe, - .remove = happy_meal_pci_remove, }; static int __init happy_meal_pci_init(void) -- 2.35.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 4/x] sunhme: switch to devres 2022-07-27 3:49 ` Sean Anderson @ 2022-07-27 3:58 ` Sean Anderson 2022-07-28 19:52 ` Rolf Eike Beer 0 siblings, 1 reply; 23+ messages in thread From: Sean Anderson @ 2022-07-27 3:58 UTC (permalink / raw) To: Rolf Eike Beer, netdev On 7/26/22 11:49 PM, Sean Anderson wrote: > On 2/14/22 1:31 PM, Rolf Eike Beer wrote: >> This not only removes a lot of code, it also fixes the memleak of the DMA >> memory when register_netdev() fails. >> >> Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> >> --- >> drivers/net/ethernet/sun/sunhme.c | 55 +++++++++---------------------- >> 1 file changed, 16 insertions(+), 39 deletions(-) >> >> diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c >> index 980a936ce8d1..ec78f43f75c9 100644 >> --- a/drivers/net/ethernet/sun/sunhme.c >> +++ b/drivers/net/ethernet/sun/sunhme.c >> @@ -2952,7 +2952,6 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, >> struct happy_meal *hp; >> struct net_device *dev; >> void __iomem *hpreg_base; >> - unsigned long hpreg_res; >> int i, qfe_slot = -1; >> char prom_name[64]; >> u8 addr[ETH_ALEN]; >> @@ -2969,7 +2968,7 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, >> strcpy(prom_name, "SUNW,hme"); >> #endif >> - err = pci_enable_device(pdev); >> + err = pcim_enable_device(pdev); >> if (err) >> goto err_out; >> @@ -2987,10 +2986,11 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, >> goto err_out; >> } >> - dev = alloc_etherdev(sizeof(struct happy_meal)); >> - err = -ENOMEM; >> - if (!dev) >> + dev = devm_alloc_etherdev(&pdev->dev, sizeof(struct happy_meal)); >> + if (!dev) { >> + err = -ENOMEM; >> goto err_out; >> + } >> SET_NETDEV_DEV(dev, &pdev->dev); >> if (hme_version_printed++ == 0) >> @@ -3009,21 +3009,23 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, >> qp->happy_meals[qfe_slot] = dev; >> } >> - hpreg_res = pci_resource_start(pdev, 0); >> - err = -ENODEV; >> if ((pci_resource_flags(pdev, 0) & IORESOURCE_IO) != 0) { >> printk(KERN_ERR "happymeal(PCI): Cannot find proper PCI device base address.\n"); >> goto err_out_clear_quattro; >> } >> - if (pci_request_regions(pdev, DRV_NAME)) { >> + >> + if (!devm_request_region(&pdev->dev, pci_resource_start(pdev, 0), >> + pci_resource_len(pdev, 0), >> + DRV_NAME)) { >> printk(KERN_ERR "happymeal(PCI): Cannot obtain PCI resources, " >> "aborting.\n"); >> goto err_out_clear_quattro; >> } >> - if ((hpreg_base = ioremap(hpreg_res, 0x8000)) == NULL) { >> + hpreg_base = pcim_iomap(pdev, 0, 0x8000); >> + if (!hpreg_base) { >> printk(KERN_ERR "happymeal(PCI): Unable to remap card memory.\n"); >> - goto err_out_free_res; >> + goto err_out_clear_quattro; >> } >> for (i = 0; i < 6; i++) { >> @@ -3089,11 +3091,10 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, >> hp->happy_bursts = DMA_BURSTBITS; >> #endif >> - hp->happy_block = dma_alloc_coherent(&pdev->dev, PAGE_SIZE, >> + hp->happy_block = dmam_alloc_coherent(&pdev->dev, PAGE_SIZE, >> &hp->hblock_dvma, GFP_KERNEL); >> - err = -ENODEV; >> if (!hp->happy_block) >> - goto err_out_iounmap; >> + goto err_out_clear_quattro; >> hp->linkcheck = 0; >> hp->timer_state = asleep; >> @@ -3127,11 +3128,11 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, >> happy_meal_set_initial_advertisement(hp); >> spin_unlock_irq(&hp->happy_lock); >> - err = register_netdev(hp->dev); >> + err = devm_register_netdev(&pdev->dev, dev); >> if (err) { >> printk(KERN_ERR "happymeal(PCI): Cannot register net device, " >> "aborting.\n"); >> - goto err_out_iounmap; >> + goto err_out_clear_quattro; >> } >> pci_set_drvdata(pdev, hp); >> @@ -3164,37 +3165,14 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, >> return 0; >> -err_out_iounmap: >> - iounmap(hp->gregs); >> - >> -err_out_free_res: >> - pci_release_regions(pdev); >> - >> err_out_clear_quattro: >> if (qp != NULL) >> qp->happy_meals[qfe_slot] = NULL; >> - free_netdev(dev); >> - >> err_out: >> return err; >> } >> -static void happy_meal_pci_remove(struct pci_dev *pdev) >> -{ >> - struct happy_meal *hp = pci_get_drvdata(pdev); >> - struct net_device *net_dev = hp->dev; >> - >> - unregister_netdev(net_dev); >> - >> - dma_free_coherent(hp->dma_dev, PAGE_SIZE, >> - hp->happy_block, hp->hblock_dvma); >> - iounmap(hp->gregs); >> - pci_release_regions(hp->happy_dev); >> - >> - free_netdev(net_dev); >> -} >> - >> static const struct pci_device_id happymeal_pci_ids[] = { >> { PCI_DEVICE(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_HAPPYMEAL) }, >> { } /* Terminating entry */ >> @@ -3206,7 +3184,6 @@ static struct pci_driver hme_pci_driver = { >> .name = "hme", >> .id_table = happymeal_pci_ids, >> .probe = happy_meal_pci_probe, >> - .remove = happy_meal_pci_remove, >> }; >> static int __init happy_meal_pci_init(void) >> > > This looks good, but doesn't apply cleanly. I rebased it as follows: > > From 5acfa13935277e312361c5630b49aea02399b8b8 Mon Sep 17 00:00:00 2001 > From: Rolf Eike Beer <eike-kernel@sf-tec.de> > Date: Mon, 14 Feb 2022 19:31:09 +0100 > Subject: [PATCH] sunhme: switch to devres > > This not only removes a lot of code, it also fixes the memleak of the DMA > memory when register_netdev() fails. > > Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> > [ rebased onto net-next/master ] > Signed-off-by: Sean Anderson <seanga2@gmail.com> > Reviewed-by: Sean Anderson <seanga2@gmail.com> > --- > drivers/net/ethernet/sun/sunhme.c | 59 +++++++++---------------------- > 1 file changed, 16 insertions(+), 43 deletions(-) > > diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c > index eebe8c5f480c..e83774ffaa7a 100644 > --- a/drivers/net/ethernet/sun/sunhme.c > +++ b/drivers/net/ethernet/sun/sunhme.c > @@ -2933,7 +2933,6 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > struct happy_meal *hp; > struct net_device *dev; > void __iomem *hpreg_base; > - unsigned long hpreg_res; > int i, qfe_slot = -1; > char prom_name[64]; > u8 addr[ETH_ALEN]; > @@ -2950,7 +2949,7 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > strcpy(prom_name, "SUNW,hme"); > #endif > > - err = pci_enable_device(pdev); > + err = pcim_enable_device(pdev); > > if (err) > goto err_out; > @@ -2968,10 +2967,11 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > goto err_out; > } > > - dev = alloc_etherdev(sizeof(struct happy_meal)); > - err = -ENOMEM; > - if (!dev) > + dev = devm_alloc_etherdev(&pdev->dev, sizeof(struct happy_meal)); > + if (!dev) { > + err = -ENOMEM; > goto err_out; > + } > SET_NETDEV_DEV(dev, &pdev->dev); > > if (hme_version_printed++ == 0) > @@ -2990,21 +2990,23 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > qp->happy_meals[qfe_slot] = dev; > } > > - hpreg_res = pci_resource_start(pdev, 0); > - err = -ENODEV; > if ((pci_resource_flags(pdev, 0) & IORESOURCE_IO) != 0) { > printk(KERN_ERR "happymeal(PCI): Cannot find proper PCI device base address.\n"); > goto err_out_clear_quattro; > } > - if (pci_request_regions(pdev, DRV_NAME)) { > + > + if (!devm_request_region(&pdev->dev, pci_resource_start(pdev, 0), > + pci_resource_len(pdev, 0), > + DRV_NAME)) { Actually, it looks like you are failing to set err from these *m calls, like what you fixed in patch 3. Can you address this for v2? > printk(KERN_ERR "happymeal(PCI): Cannot obtain PCI resources, " > "aborting.\n"); > goto err_out_clear_quattro; > } > > - if ((hpreg_base = ioremap(hpreg_res, 0x8000)) == NULL) { > + hpreg_base = pcim_iomap(pdev, 0, 0x8000); > + if (!hpreg_base) { > printk(KERN_ERR "happymeal(PCI): Unable to remap card memory.\n"); > - goto err_out_free_res; > + goto err_out_clear_quattro; > } > > for (i = 0; i < 6; i++) { > @@ -3070,11 +3072,10 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > hp->happy_bursts = DMA_BURSTBITS; > #endif > > - hp->happy_block = dma_alloc_coherent(&pdev->dev, PAGE_SIZE, > + hp->happy_block = dmam_alloc_coherent(&pdev->dev, PAGE_SIZE, > &hp->hblock_dvma, GFP_KERNEL); > - err = -ENODEV; > if (!hp->happy_block) > - goto err_out_iounmap; > + goto err_out_clear_quattro; > > hp->linkcheck = 0; > hp->timer_state = asleep; > @@ -3108,11 +3109,11 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > happy_meal_set_initial_advertisement(hp); > spin_unlock_irq(&hp->happy_lock); > > - err = register_netdev(hp->dev); > + err = devm_register_netdev(&pdev->dev, dev); > if (err) { > printk(KERN_ERR "happymeal(PCI): Cannot register net device, " > "aborting.\n"); > - goto err_out_free_coherent; > + goto err_out_clear_quattro; > } > > pci_set_drvdata(pdev, hp); > @@ -3145,41 +3146,14 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > > return 0; > > -err_out_free_coherent: > - dma_free_coherent(hp->dma_dev, PAGE_SIZE, > - hp->happy_block, hp->hblock_dvma); > - > -err_out_iounmap: > - iounmap(hp->gregs); > - > -err_out_free_res: > - pci_release_regions(pdev); > - > err_out_clear_quattro: > if (qp != NULL) > qp->happy_meals[qfe_slot] = NULL; > > - free_netdev(dev); > - > err_out: > return err; > } > > -static void happy_meal_pci_remove(struct pci_dev *pdev) > -{ > - struct happy_meal *hp = pci_get_drvdata(pdev); > - struct net_device *net_dev = hp->dev; > - > - unregister_netdev(net_dev); > - > - dma_free_coherent(hp->dma_dev, PAGE_SIZE, > - hp->happy_block, hp->hblock_dvma); > - iounmap(hp->gregs); > - pci_release_regions(hp->happy_dev); > - > - free_netdev(net_dev); > -} > - > static const struct pci_device_id happymeal_pci_ids[] = { > { PCI_DEVICE(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_HAPPYMEAL) }, > { } /* Terminating entry */ > @@ -3191,7 +3165,6 @@ static struct pci_driver hme_pci_driver = { > .name = "hme", > .id_table = happymeal_pci_ids, > .probe = happy_meal_pci_probe, > - .remove = happy_meal_pci_remove, > }; > > static int __init happy_meal_pci_init(void) ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/x] sunhme: switch to devres 2022-07-27 3:58 ` Sean Anderson @ 2022-07-28 19:52 ` Rolf Eike Beer 2022-07-29 0:33 ` Sean Anderson 0 siblings, 1 reply; 23+ messages in thread From: Rolf Eike Beer @ 2022-07-28 19:52 UTC (permalink / raw) To: Sean Anderson; +Cc: netdev Am 2022-07-27 05:58, schrieb Sean Anderson: > On 7/26/22 11:49 PM, Sean Anderson wrote: >> This looks good, but doesn't apply cleanly. I rebased it as follows: Looks like what my local rebase has also produced. The sentence about the leak from the commitmessage can be dropped then, as this leak has already been fixed. >> diff --git a/drivers/net/ethernet/sun/sunhme.c >> b/drivers/net/ethernet/sun/sunhme.c >> index eebe8c5f480c..e83774ffaa7a 100644 >> --- a/drivers/net/ethernet/sun/sunhme.c >> +++ b/drivers/net/ethernet/sun/sunhme.c >> @@ -2990,21 +2990,23 @@ static int happy_meal_pci_probe(struct pci_dev >> *pdev, >> qp->happy_meals[qfe_slot] = dev; >> } >> >> - hpreg_res = pci_resource_start(pdev, 0); >> - err = -ENODEV; >> if ((pci_resource_flags(pdev, 0) & IORESOURCE_IO) != 0) { >> printk(KERN_ERR "happymeal(PCI): Cannot find proper PCI >> device base address.\n"); >> goto err_out_clear_quattro; >> } >> - if (pci_request_regions(pdev, DRV_NAME)) { >> + >> + if (!devm_request_region(&pdev->dev, pci_resource_start(pdev, 0), >> + pci_resource_len(pdev, 0), >> + DRV_NAME)) { > > Actually, it looks like you are failing to set err from these *m > calls, like what > you fixed in patch 3. Can you address this for v2? It returns NULL on error, there is no error code I can set. Regards, Eike ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/x] sunhme: switch to devres 2022-07-28 19:52 ` Rolf Eike Beer @ 2022-07-29 0:33 ` Sean Anderson 2022-08-01 15:14 ` Rolf Eike Beer 0 siblings, 1 reply; 23+ messages in thread From: Sean Anderson @ 2022-07-29 0:33 UTC (permalink / raw) To: Rolf Eike Beer; +Cc: netdev On 7/28/22 3:52 PM, Rolf Eike Beer wrote: > Am 2022-07-27 05:58, schrieb Sean Anderson: >> On 7/26/22 11:49 PM, Sean Anderson wrote: > >>> This looks good, but doesn't apply cleanly. I rebased it as follows: > > Looks like what my local rebase has also produced. > > The sentence about the leak from the commitmessage can be dropped then, > as this leak has already been fixed. > >>> diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c >>> index eebe8c5f480c..e83774ffaa7a 100644 >>> --- a/drivers/net/ethernet/sun/sunhme.c >>> +++ b/drivers/net/ethernet/sun/sunhme.c >>> @@ -2990,21 +2990,23 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, >>> qp->happy_meals[qfe_slot] = dev; >>> } >>> >>> - hpreg_res = pci_resource_start(pdev, 0); >>> - err = -ENODEV; >>> if ((pci_resource_flags(pdev, 0) & IORESOURCE_IO) != 0) { >>> printk(KERN_ERR "happymeal(PCI): Cannot find proper PCI device base address.\n"); >>> goto err_out_clear_quattro; >>> } >>> - if (pci_request_regions(pdev, DRV_NAME)) { >>> + >>> + if (!devm_request_region(&pdev->dev, pci_resource_start(pdev, 0), >>> + pci_resource_len(pdev, 0), >>> + DRV_NAME)) { >> >> Actually, it looks like you are failing to set err from these *m >> calls, like what >> you fixed in patch 3. Can you address this for v2? > > It returns NULL on error, there is no error code I can set. So it does. A quick grep shows that most drivers return -EBUSY. --Sean ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/x] sunhme: switch to devres 2022-07-29 0:33 ` Sean Anderson @ 2022-08-01 15:14 ` Rolf Eike Beer 2022-08-24 15:45 ` Rolf Eike Beer 0 siblings, 1 reply; 23+ messages in thread From: Rolf Eike Beer @ 2022-08-01 15:14 UTC (permalink / raw) To: Sean Anderson; +Cc: netdev [-- Attachment #1: Type: text/plain, Size: 1838 bytes --] Am Freitag, 29. Juli 2022, 02:33:01 CEST schrieb Sean Anderson: > On 7/28/22 3:52 PM, Rolf Eike Beer wrote: > > Am 2022-07-27 05:58, schrieb Sean Anderson: > >> On 7/26/22 11:49 PM, Sean Anderson wrote: > >>> This looks good, but doesn't apply cleanly. I rebased it as follows: > > Looks like what my local rebase has also produced. > > > > The sentence about the leak from the commitmessage can be dropped then, > > as this leak has already been fixed. > > > >>> diff --git a/drivers/net/ethernet/sun/sunhme.c > >>> b/drivers/net/ethernet/sun/sunhme.c index eebe8c5f480c..e83774ffaa7a > >>> 100644 > >>> --- a/drivers/net/ethernet/sun/sunhme.c > >>> +++ b/drivers/net/ethernet/sun/sunhme.c > >>> @@ -2990,21 +2990,23 @@ static int happy_meal_pci_probe(struct pci_dev > >>> *pdev, qp->happy_meals[qfe_slot] = dev; > >>> } > >>> > >>> - hpreg_res = pci_resource_start(pdev, 0); > >>> - err = -ENODEV; > >>> if ((pci_resource_flags(pdev, 0) & IORESOURCE_IO) != 0) { > >>> printk(KERN_ERR "happymeal(PCI): Cannot find proper PCI device > >>> base address.\n"); goto err_out_clear_quattro; > >>> } > >>> - if (pci_request_regions(pdev, DRV_NAME)) { > >>> + > >>> + if (!devm_request_region(&pdev->dev, pci_resource_start(pdev, 0), > >>> + pci_resource_len(pdev, 0), > >>> + DRV_NAME)) { > >> > >> Actually, it looks like you are failing to set err from these *m > >> calls, like what > >> you fixed in patch 3. Can you address this for v2? > > > > It returns NULL on error, there is no error code I can set. > > So it does. A quick grep shows that most drivers return -EBUSY. Sure, I just meant that there is no error code I can pass on. I can change that to -EBUSY if you prefer that, currently it just returns -ENODEV as the old code has done before. Eike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/x] sunhme: switch to devres 2022-08-01 15:14 ` Rolf Eike Beer @ 2022-08-24 15:45 ` Rolf Eike Beer 2022-08-24 15:57 ` Sean Anderson 0 siblings, 1 reply; 23+ messages in thread From: Rolf Eike Beer @ 2022-08-24 15:45 UTC (permalink / raw) To: Sean Anderson; +Cc: netdev [-- Attachment #1: Type: text/plain, Size: 2070 bytes --] Am Montag, 1. August 2022, 17:14:39 CEST schrieb Rolf Eike Beer: > Am Freitag, 29. Juli 2022, 02:33:01 CEST schrieb Sean Anderson: > > On 7/28/22 3:52 PM, Rolf Eike Beer wrote: > > > Am 2022-07-27 05:58, schrieb Sean Anderson: > > >> On 7/26/22 11:49 PM, Sean Anderson wrote: > > >>> This looks good, but doesn't apply cleanly. I rebased it as follows: > > > Looks like what my local rebase has also produced. > > > > > > The sentence about the leak from the commitmessage can be dropped then, > > > as this leak has already been fixed. > > > > > >>> diff --git a/drivers/net/ethernet/sun/sunhme.c > > >>> b/drivers/net/ethernet/sun/sunhme.c index eebe8c5f480c..e83774ffaa7a > > >>> 100644 > > >>> --- a/drivers/net/ethernet/sun/sunhme.c > > >>> +++ b/drivers/net/ethernet/sun/sunhme.c > > >>> @@ -2990,21 +2990,23 @@ static int happy_meal_pci_probe(struct pci_dev > > >>> *pdev, qp->happy_meals[qfe_slot] = dev; > > >>> > > >>> } > > >>> > > >>> - hpreg_res = pci_resource_start(pdev, 0); > > >>> - err = -ENODEV; > > >>> > > >>> if ((pci_resource_flags(pdev, 0) & IORESOURCE_IO) != 0) { > > >>> > > >>> printk(KERN_ERR "happymeal(PCI): Cannot find proper PCI > > >>> device > > >>> > > >>> base address.\n"); goto err_out_clear_quattro; > > >>> > > >>> } > > >>> > > >>> - if (pci_request_regions(pdev, DRV_NAME)) { > > >>> + > > >>> + if (!devm_request_region(&pdev->dev, pci_resource_start(pdev, 0), > > >>> + pci_resource_len(pdev, 0), > > >>> + DRV_NAME)) { > > >> > > >> Actually, it looks like you are failing to set err from these *m > > >> calls, like what > > >> you fixed in patch 3. Can you address this for v2? > > > > > > It returns NULL on error, there is no error code I can set. > > > > So it does. A quick grep shows that most drivers return -EBUSY. > > Sure, I just meant that there is no error code I can pass on. I can change > that to -EBUSY if you prefer that, currently it just returns -ENODEV as the > old code has done before. Ping? Eike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/x] sunhme: switch to devres 2022-08-24 15:45 ` Rolf Eike Beer @ 2022-08-24 15:57 ` Sean Anderson 0 siblings, 0 replies; 23+ messages in thread From: Sean Anderson @ 2022-08-24 15:57 UTC (permalink / raw) To: Rolf Eike Beer; +Cc: netdev On 8/24/22 11:45 AM, Rolf Eike Beer wrote: > Am Montag, 1. August 2022, 17:14:39 CEST schrieb Rolf Eike Beer: >> Am Freitag, 29. Juli 2022, 02:33:01 CEST schrieb Sean Anderson: >>> On 7/28/22 3:52 PM, Rolf Eike Beer wrote: >>>> Am 2022-07-27 05:58, schrieb Sean Anderson: >>>>> On 7/26/22 11:49 PM, Sean Anderson wrote: >>>>>> This looks good, but doesn't apply cleanly. I rebased it as follows: >>>> Looks like what my local rebase has also produced. >>>> >>>> The sentence about the leak from the commitmessage can be dropped then, >>>> as this leak has already been fixed. >>>> >>>>>> diff --git a/drivers/net/ethernet/sun/sunhme.c >>>>>> b/drivers/net/ethernet/sun/sunhme.c index eebe8c5f480c..e83774ffaa7a >>>>>> 100644 >>>>>> --- a/drivers/net/ethernet/sun/sunhme.c >>>>>> +++ b/drivers/net/ethernet/sun/sunhme.c >>>>>> @@ -2990,21 +2990,23 @@ static int happy_meal_pci_probe(struct pci_dev >>>>>> *pdev, qp->happy_meals[qfe_slot] = dev; >>>>>> >>>>>> } >>>>>> >>>>>> - hpreg_res = pci_resource_start(pdev, 0); >>>>>> - err = -ENODEV; >>>>>> >>>>>> if ((pci_resource_flags(pdev, 0) & IORESOURCE_IO) != 0) { >>>>>> >>>>>> printk(KERN_ERR "happymeal(PCI): Cannot find proper PCI >>>>>> device >>>>>> >>>>>> base address.\n"); goto err_out_clear_quattro; >>>>>> >>>>>> } >>>>>> >>>>>> - if (pci_request_regions(pdev, DRV_NAME)) { >>>>>> + >>>>>> + if (!devm_request_region(&pdev->dev, pci_resource_start(pdev, 0), >>>>>> + pci_resource_len(pdev, 0), >>>>>> + DRV_NAME)) { >>>>> >>>>> Actually, it looks like you are failing to set err from these *m >>>>> calls, like what >>>>> you fixed in patch 3. Can you address this for v2? >>>> >>>> It returns NULL on error, there is no error code I can set. >>> >>> So it does. A quick grep shows that most drivers return -EBUSY. >> >> Sure, I just meant that there is no error code I can pass on. I can change >> that to -EBUSY if you prefer that, currently it just returns -ENODEV as the >> old code has done before. > > Ping? I think -EBUSY is a good return here. I have a WIP at [1] of some logging cleanups on top of your commits. --Sean [1] https://github.com/Forty-Bot/linux/commits/hme_base ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 4/4 v2] sunhme: switch to devres 2022-02-14 18:31 ` [PATCH 4/x] sunhme: switch to devres Rolf Eike Beer 2022-07-27 3:49 ` Sean Anderson @ 2022-08-29 13:22 ` Rolf Eike Beer 2022-08-30 0:16 ` Jakub Kicinski 1 sibling, 1 reply; 23+ messages in thread From: Rolf Eike Beer @ 2022-08-29 13:22 UTC (permalink / raw) To: netdev; +Cc: Sean Anderson Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> --- drivers/net/ethernet/sun/sunhme.c | 60 +++++++++---------------------- 1 file changed, 17 insertions(+), 43 deletions(-) v2: -return -EBUSY in case the PCI region can't be requested diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c index eebe8c5f480c..df6e630a5024 100644 --- a/drivers/net/ethernet/sun/sunhme.c +++ b/drivers/net/ethernet/sun/sunhme.c @@ -2933,7 +2933,6 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, struct happy_meal *hp; struct net_device *dev; void __iomem *hpreg_base; - unsigned long hpreg_res; int i, qfe_slot = -1; char prom_name[64]; u8 addr[ETH_ALEN]; @@ -2950,7 +2949,7 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, strcpy(prom_name, "SUNW,hme"); #endif - err = pci_enable_device(pdev); + err = pcim_enable_device(pdev); if (err) goto err_out; @@ -2968,10 +2967,11 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, goto err_out; } - dev = alloc_etherdev(sizeof(struct happy_meal)); - err = -ENOMEM; - if (!dev) + dev = devm_alloc_etherdev(&pdev->dev, sizeof(struct happy_meal)); + if (!dev) { + err = -ENOMEM; goto err_out; + } SET_NETDEV_DEV(dev, &pdev->dev); if (hme_version_printed++ == 0) @@ -2990,21 +2990,24 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, qp->happy_meals[qfe_slot] = dev; } - hpreg_res = pci_resource_start(pdev, 0); - err = -ENODEV; if ((pci_resource_flags(pdev, 0) & IORESOURCE_IO) != 0) { printk(KERN_ERR "happymeal(PCI): Cannot find proper PCI device base address.\n"); goto err_out_clear_quattro; } - if (pci_request_regions(pdev, DRV_NAME)) { + + if (!devm_request_region(&pdev->dev, pci_resource_start(pdev, 0), + pci_resource_len(pdev, 0), + DRV_NAME)) { printk(KERN_ERR "happymeal(PCI): Cannot obtain PCI resources, " "aborting.\n"); + err = -EBUSY; goto err_out_clear_quattro; } - if ((hpreg_base = ioremap(hpreg_res, 0x8000)) == NULL) { + hpreg_base = pcim_iomap(pdev, 0, 0x8000); + if (!hpreg_base) { printk(KERN_ERR "happymeal(PCI): Unable to remap card memory.\n"); - goto err_out_free_res; + goto err_out_clear_quattro; } for (i = 0; i < 6; i++) { @@ -3070,11 +3073,10 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, hp->happy_bursts = DMA_BURSTBITS; #endif - hp->happy_block = dma_alloc_coherent(&pdev->dev, PAGE_SIZE, + hp->happy_block = dmam_alloc_coherent(&pdev->dev, PAGE_SIZE, &hp->hblock_dvma, GFP_KERNEL); - err = -ENODEV; if (!hp->happy_block) - goto err_out_iounmap; + goto err_out_clear_quattro; hp->linkcheck = 0; hp->timer_state = asleep; @@ -3108,11 +3110,11 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, happy_meal_set_initial_advertisement(hp); spin_unlock_irq(&hp->happy_lock); - err = register_netdev(hp->dev); + err = devm_register_netdev(&pdev->dev, dev); if (err) { printk(KERN_ERR "happymeal(PCI): Cannot register net device, " "aborting.\n"); - goto err_out_free_coherent; + goto err_out_clear_quattro; } pci_set_drvdata(pdev, hp); @@ -3145,41 +3147,14 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, return 0; -err_out_free_coherent: - dma_free_coherent(hp->dma_dev, PAGE_SIZE, - hp->happy_block, hp->hblock_dvma); - -err_out_iounmap: - iounmap(hp->gregs); - -err_out_free_res: - pci_release_regions(pdev); - err_out_clear_quattro: if (qp != NULL) qp->happy_meals[qfe_slot] = NULL; - free_netdev(dev); - err_out: return err; } -static void happy_meal_pci_remove(struct pci_dev *pdev) -{ - struct happy_meal *hp = pci_get_drvdata(pdev); - struct net_device *net_dev = hp->dev; - - unregister_netdev(net_dev); - - dma_free_coherent(hp->dma_dev, PAGE_SIZE, - hp->happy_block, hp->hblock_dvma); - iounmap(hp->gregs); - pci_release_regions(hp->happy_dev); - - free_netdev(net_dev); -} - static const struct pci_device_id happymeal_pci_ids[] = { { PCI_DEVICE(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_HAPPYMEAL) }, { } /* Terminating entry */ @@ -3191,7 +3166,6 @@ static struct pci_driver hme_pci_driver = { .name = "hme", .id_table = happymeal_pci_ids, .probe = happy_meal_pci_probe, - .remove = happy_meal_pci_remove, }; static int __init happy_meal_pci_init(void) -- 2.35.3 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 4/4 v2] sunhme: switch to devres 2022-08-29 13:22 ` [PATCH 4/4 v2] " Rolf Eike Beer @ 2022-08-30 0:16 ` Jakub Kicinski 0 siblings, 0 replies; 23+ messages in thread From: Jakub Kicinski @ 2022-08-30 0:16 UTC (permalink / raw) To: Rolf Eike Beer; +Cc: netdev, Sean Anderson On Mon, 29 Aug 2022 15:22:06 +0200 Rolf Eike Beer wrote: > Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> Please add a commit message with a justification, and realign continuation lines. Are you using this HW? > v2: > -return -EBUSY in case the PCI region can't be requested This looks like a fix which needs to be sent separately first and then you can do the re-factoring on top. ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2022-08-30 0:16 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-03 16:20 sunhme: some cleanups Rolf Eike Beer 2022-02-03 16:21 ` [PATCH 1/3] sunhme: remove unused tx_dump_ring() Rolf Eike Beer 2022-07-27 3:42 ` Sean Anderson 2022-02-03 16:22 ` [PATCH 2/3] sunhme: fix the version number in struct ethtool_drvinfo Rolf Eike Beer 2022-02-03 17:12 ` Andrew Lunn 2022-02-05 11:27 ` Rolf Eike Beer 2022-02-05 14:48 ` Andrew Lunn 2022-02-05 15:57 ` Rolf Eike Beer 2022-02-05 16:32 ` Andrew Lunn 2022-02-03 21:53 ` Jakub Kicinski 2022-02-14 18:33 ` [PATCH 2/3 v2] " Rolf Eike Beer 2022-02-03 16:23 ` [PATCH 3/3] sunhme: forward the error code from pci_enable_device() Rolf Eike Beer 2022-07-27 3:48 ` Sean Anderson 2022-02-14 18:31 ` [PATCH 4/x] sunhme: switch to devres Rolf Eike Beer 2022-07-27 3:49 ` Sean Anderson 2022-07-27 3:58 ` Sean Anderson 2022-07-28 19:52 ` Rolf Eike Beer 2022-07-29 0:33 ` Sean Anderson 2022-08-01 15:14 ` Rolf Eike Beer 2022-08-24 15:45 ` Rolf Eike Beer 2022-08-24 15:57 ` Sean Anderson 2022-08-29 13:22 ` [PATCH 4/4 v2] " Rolf Eike Beer 2022-08-30 0:16 ` Jakub Kicinski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).