[PATCH] net: atlantic: Rename PCI driver struct to end in _driver

[PATCH] net: atlantic: Rename PCI driver struct to end in _driver

[Uwe Kleine-König]

This is not only a cosmetic change because the section mismatch checks
also depend on the name and for drivers the checks are stricter than for
ops.

However aq_pci_driver also passes the stricter checks just fine, so no
further changes needed.

Fixes: 97bde5c4f909 ("net: ethernet: aquantia: Support for NIC-specific code")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
index 08630ee94251..ed5231dece3f 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
@@ -463,7 +463,7 @@ static const struct dev_pm_ops aq_pm_ops = {
 };
 #endif
 
-static struct pci_driver aq_pci_ops = {
+static struct pci_driver aq_pci_driver = {
 	.name = AQ_CFG_DRV_NAME,
 	.id_table = aq_pci_tbl,
 	.probe = aq_pci_probe,
@@ -476,11 +476,11 @@ static struct pci_driver aq_pci_ops = {
 
 int aq_pci_func_register_driver(void)
 {
-	return pci_register_driver(&aq_pci_ops);
+	return pci_register_driver(&aq_pci_driver);
 }
 
 void aq_pci_func_unregister_driver(void)
 {
-	pci_unregister_driver(&aq_pci_ops);
+	pci_unregister_driver(&aq_pci_driver);
 }
 

base-commit: e04c78d86a9699d136910cfc0bdcf01087e3267e
-- 
2.49.0

Re: [PATCH] net: atlantic: Rename PCI driver struct to end in _driver

[Simon Horman]

On Fri, Jun 27, 2025 at 11:46:41AM +0200, Uwe Kleine-König wrote:
> This is not only a cosmetic change because the section mismatch checks
> also depend on the name and for drivers the checks are stricter than for
> ops.
> 
> However aq_pci_driver also passes the stricter checks just fine, so no
> further changes needed.
> 
> Fixes: 97bde5c4f909 ("net: ethernet: aquantia: Support for NIC-specific code")

From a Networking subsystem point of view
this feels more like an enhancement than a bug fix.
Can we drop the Fixes tag?

> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>

Re: [PATCH] net: atlantic: Rename PCI driver struct to end in _driver

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

Hello Simon,

On Fri, Jun 27, 2025 at 08:47:52PM +0100, Simon Horman wrote:
> On Fri, Jun 27, 2025 at 11:46:41AM +0200, Uwe Kleine-König wrote:
> > This is not only a cosmetic change because the section mismatch checks
> > also depend on the name and for drivers the checks are stricter than for
> > ops.
> > 
> > However aq_pci_driver also passes the stricter checks just fine, so no
> > further changes needed.
> > 
> > Fixes: 97bde5c4f909 ("net: ethernet: aquantia: Support for NIC-specific code")
> 
> From a Networking subsystem point of view
> this feels more like an enhancement than a bug fix.
> Can we drop the Fixes tag?

I think it's right to include it, but I won't argue if you apply the
patch without it.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] net: atlantic: Rename PCI driver struct to end in _driver

[Jakub Kicinski]

On Fri, 27 Jun 2025 11:46:41 +0200 Uwe Kleine-König wrote:
> This is not only a cosmetic change because the section mismatch checks
> also depend on the name and for drivers the checks are stricter than for
> ops.

Could you add more info about what check you're talking about or quote
the warning you're fixing? I'm not following..

Re: [PATCH] net: atlantic: Rename PCI driver struct to end in _driver

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1706 bytes --]

On Fri, Jun 27, 2025 at 04:36:52PM -0700, Jakub Kicinski wrote:
> On Fri, 27 Jun 2025 11:46:41 +0200 Uwe Kleine-König wrote:
> > This is not only a cosmetic change because the section mismatch checks
> > also depend on the name and for drivers the checks are stricter than for
> > ops.
> 
> Could you add more info about what check you're talking about or quote
> the warning you're fixing? I'm not following..

If you do 

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
index ed5231dece3f..2ee5900337bb 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
@@ -208,7 +208,7 @@ static void aq_pci_free_irq_vectors(struct aq_nic_s *self)
 	pci_free_irq_vectors(self->pdev);
 }
 
-static int aq_pci_probe(struct pci_dev *pdev,
+static int __init aq_pci_probe(struct pci_dev *pdev,
 			const struct pci_device_id *pci_id)
 {
 	struct net_device *ndev;

this is buggy; so it's justified that you get:

	WARNING: modpost: vmlinux: section mismatch in reference: aq_pci_driver+0x8 (section: .data) -> aq_pci_probe (section: .init.text)
	ERROR: modpost: Section mismatches detected.

. However if the driver struct is named "aq_pci_ops", the warning is
suppressed due to

        /* symbols in data sections that may refer to any init/exit sections */
        if (match(fromsec, PATTERNS(DATA_SECTIONS)) &&
            match(tosec, PATTERNS(ALL_INIT_SECTIONS, ALL_EXIT_SECTIONS)) &&
            match(fromsym, PATTERNS("*_ops", "*_probe", "*_console")))
                return 0;

in scripts/mod/modpost.c.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] net: atlantic: Rename PCI driver struct to end in _driver

[Simon Horman]

On Fri, Jun 27, 2025 at 11:18:45PM +0200, Uwe Kleine-König wrote:
> Hello Simon,
> 
> On Fri, Jun 27, 2025 at 08:47:52PM +0100, Simon Horman wrote:
> > On Fri, Jun 27, 2025 at 11:46:41AM +0200, Uwe Kleine-König wrote:
> > > This is not only a cosmetic change because the section mismatch checks
> > > also depend on the name and for drivers the checks are stricter than for
> > > ops.
> > > 
> > > However aq_pci_driver also passes the stricter checks just fine, so no
> > > further changes needed.
> > > 
> > > Fixes: 97bde5c4f909 ("net: ethernet: aquantia: Support for NIC-specific code")
> > 
> > From a Networking subsystem point of view
> > this feels more like an enhancement than a bug fix.
> > Can we drop the Fixes tag?
> 
> I think it's right to include it, but I won't argue if you apply the
> patch without it.

For Networking we generally use Fixes tags for bug fixes,
which in general address some adverse user-visible behaviour,
e.g. a panic.

Of course there is always room for interpretation. But based
on my understanding of this patch I would lean towards it
not being a bug fix. Again, in the sense that bug fix is usually
used for networking patches.

So I would suggest:

1. Target the patch (and others like it) at net-next

   Subject: [PATCH net-next] ...

2. Don't include a Fixes tag

3. Optionally refer to the patch that introduced the problem by
   using the following in the commit message.

   commit 97bde5c4f909 ("net: ethernet: aquantia: Support for NIC-specific
   code")

Thanks in advance for taking my opinion into consideration.

Re: [PATCH] net: atlantic: Rename PCI driver struct to end in _driver

[Jakub Kicinski]

On Sun, 29 Jun 2025 12:09:34 +0200 Uwe Kleine-König wrote:
> If you do 
> 
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
> index ed5231dece3f..2ee5900337bb 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
> @@ -208,7 +208,7 @@ static void aq_pci_free_irq_vectors(struct aq_nic_s *self)
>  	pci_free_irq_vectors(self->pdev);
>  }
>  
> -static int aq_pci_probe(struct pci_dev *pdev,
> +static int __init aq_pci_probe(struct pci_dev *pdev,
>  			const struct pci_device_id *pci_id)
>  {
>  	struct net_device *ndev;
> 
> this is buggy; so it's justified that you get:
> 
> 	WARNING: modpost: vmlinux: section mismatch in reference: aq_pci_driver+0x8 (section: .data) -> aq_pci_probe (section: .init.text)
> 	ERROR: modpost: Section mismatches detected.
> 
> . However if the driver struct is named "aq_pci_ops", the warning is
> suppressed due to
> 
>         /* symbols in data sections that may refer to any init/exit sections */
>         if (match(fromsec, PATTERNS(DATA_SECTIONS)) &&
>             match(tosec, PATTERNS(ALL_INIT_SECTIONS, ALL_EXIT_SECTIONS)) &&
>             match(fromsym, PATTERNS("*_ops", "*_probe", "*_console")))
>                 return 0;
> 
> in scripts/mod/modpost.c.

Now it makes perfect sense :) I see you already reposted with a better
message. IIF you have more such patches I'd use the word "suppressed" -
something along the lines "the section mismatch warnings are suppressed
if the function is assigned to a struct with a name ending in _ops".