[PATCH 0/3] staging: octeon: add pr_fmt macro to ethernet drivers

[PATCH 0/3] staging: octeon: add pr_fmt macro to ethernet drivers

[AyushMukkanwar]

Add pr_fmt macro to three ethernet driver files in the octeon
staging driver to prefix log messages with the module name,
making debugging easier.

AyushMukkanwar (3):
  staging: octeon: ethernet-mem: add pr_fmt macro
  staging: octeon: ethernet-spi: add pr_fmt macro
  staging: octeon: ethernet: add pr_fmt macro

 drivers/staging/octeon/ethernet-mem.c | 1 +
 drivers/staging/octeon/ethernet-spi.c | 1 +
 drivers/staging/octeon/ethernet.c     | 1 +
 3 files changed, 3 insertions(+)

-- 
2.53.0

[PATCH 1/3] staging: octeon: ethernet-mem: add pr_fmt macro

[AyushMukkanwar]

Add pr_fmt macro to prefix log messages with the module
name for easier debugging.

Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
---
 drivers/staging/octeon/ethernet-mem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/octeon/ethernet-mem.c b/drivers/staging/octeon/ethernet-mem.c
index 532594957ebc..aea9c69667ce 100644
--- a/drivers/staging/octeon/ethernet-mem.c
+++ b/drivers/staging/octeon/ethernet-mem.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2003-2010 Cavium Networks
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
 #include <linux/slab.h>
-- 
2.53.0

[PATCH 2/3] staging: octeon: ethernet-spi: add pr_fmt macro

[AyushMukkanwar]

Add pr_fmt macro to prefix log messages with the module
name for easier debugging.

Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
---
 drivers/staging/octeon/ethernet-spi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/octeon/ethernet-spi.c b/drivers/staging/octeon/ethernet-spi.c
index 699c98c5ec13..f6cd34e933da 100644
--- a/drivers/staging/octeon/ethernet-spi.c
+++ b/drivers/staging/octeon/ethernet-spi.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2003-2007 Cavium Networks
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
 #include <linux/interrupt.h>
-- 
2.53.0

[PATCH 3/3] staging: octeon: ethernet: add pr_fmt macro

[AyushMukkanwar]

Add pr_fmt macro to prefix log messages with the module
name for easier debugging.

Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
---
 drivers/staging/octeon/ethernet.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
index eadb74fc14c8..5bb8c303f88b 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2003-2007 Cavium Networks
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 #include <linux/platform_device.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-- 
2.53.0

Re: [PATCH 3/3] staging: octeon: ethernet: add pr_fmt macro

[Greg KH]

On Tue, Mar 24, 2026 at 07:00:29PM +0530, AyushMukkanwar wrote:
> Add pr_fmt macro to prefix log messages with the module
> name for easier debugging.
> 
> Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
> ---
>  drivers/staging/octeon/ethernet.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
> index eadb74fc14c8..5bb8c303f88b 100644
> --- a/drivers/staging/octeon/ethernet.c
> +++ b/drivers/staging/octeon/ethernet.c
> @@ -5,6 +5,7 @@
>   * Copyright (c) 2003-2007 Cavium Networks
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  #include <linux/platform_device.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -- 
> 2.53.0
> 

How about working to remove the existing pr_*() calls with the proper
dev_*() and netdev_*() calls instead, so that pr_fmt() is not needed at
all?  That is the more "correct" solution here.

thanks,

greg k-h

Re: [PATCH 3/3] staging: octeon: ethernet: add pr_fmt macro

[Ayush Mukkanwar]

On Tue, Mar 24, 2026 at 7:58 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Mar 24, 2026 at 07:00:29PM +0530, AyushMukkanwar wrote:
> > Add pr_fmt macro to prefix log messages with the module
> > name for easier debugging.
> >
> > Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
> > ---
> >  drivers/staging/octeon/ethernet.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
> > index eadb74fc14c8..5bb8c303f88b 100644
> > --- a/drivers/staging/octeon/ethernet.c
> > +++ b/drivers/staging/octeon/ethernet.c
> > @@ -5,6 +5,7 @@
> >   * Copyright (c) 2003-2007 Cavium Networks
> >   */
> >
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> >  #include <linux/platform_device.h>
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> > --
> > 2.53.0
> >
>
> How about working to remove the existing pr_*() calls with the proper
> dev_*() and netdev_*() calls instead, so that pr_fmt() is not needed at
> all?  That is the more "correct" solution here.
>
> thanks,
>
> greg k-h

Hi Greg,

After investigating, the pr_*() calls in ethernet-mem.c and
ethernet-spi.c are inside functions that only receive hardware pool
indices or register structs with no net_device or device pointer
available, so dev_*() and netdev_*() replacements are not possible
there.

For ethernet.c, device pointers are available at most call sites and I
can replace those with dev_err() and netdev_err()/netdev_info()
appropriately. The two calls where no device pointer is available
would keep pr_err() as is.

Should I proceed with replacing what is possible in ethernet.c and
leave the pr_*() calls in ethernet-mem.c and ethernet-spi.c as they
are, or is there a preferred approach?

Thanks,
Ayush

Re: [PATCH 3/3] staging: octeon: ethernet: add pr_fmt macro

[Greg KH]

On Wed, Mar 25, 2026 at 02:33:00PM +0530, Ayush Mukkanwar wrote:
> On Tue, Mar 24, 2026 at 7:58 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Mar 24, 2026 at 07:00:29PM +0530, AyushMukkanwar wrote:
> > > Add pr_fmt macro to prefix log messages with the module
> > > name for easier debugging.
> > >
> > > Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
> > > ---
> > >  drivers/staging/octeon/ethernet.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
> > > index eadb74fc14c8..5bb8c303f88b 100644
> > > --- a/drivers/staging/octeon/ethernet.c
> > > +++ b/drivers/staging/octeon/ethernet.c
> > > @@ -5,6 +5,7 @@
> > >   * Copyright (c) 2003-2007 Cavium Networks
> > >   */
> > >
> > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > >  #include <linux/platform_device.h>
> > >  #include <linux/kernel.h>
> > >  #include <linux/module.h>
> > > --
> > > 2.53.0
> > >
> >
> > How about working to remove the existing pr_*() calls with the proper
> > dev_*() and netdev_*() calls instead, so that pr_fmt() is not needed at
> > all?  That is the more "correct" solution here.
> >
> > thanks,
> >
> > greg k-h
> 
> Hi Greg,
> 
> After investigating, the pr_*() calls in ethernet-mem.c and
> ethernet-spi.c are inside functions that only receive hardware pool
> indices or register structs with no net_device or device pointer
> available, so dev_*() and netdev_*() replacements are not possible
> there.
> 
> For ethernet.c, device pointers are available at most call sites and I
> can replace those with dev_err() and netdev_err()/netdev_info()
> appropriately. The two calls where no device pointer is available
> would keep pr_err() as is.

That's a good start, but for the others, work back up the call chain to
properly pass in a device pointer so that these warning/error messages
can get printed out properly.  Drivers should not have any "generic"
messages like that, as it does not show what device actually created the
message.

thanks,

greg k-h

Re: [PATCH 3/3] staging: octeon: ethernet: add pr_fmt macro

[Ayush Mukkanwar]

On Wed, Mar 25, 2026 at 2:41 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Mar 25, 2026 at 02:33:00PM +0530, Ayush Mukkanwar wrote:
> > On Tue, Mar 24, 2026 at 7:58 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Tue, Mar 24, 2026 at 07:00:29PM +0530, AyushMukkanwar wrote:
> > > > Add pr_fmt macro to prefix log messages with the module
> > > > name for easier debugging.
> > > >
> > > > Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
> > > > ---
> > > >  drivers/staging/octeon/ethernet.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
> > > > index eadb74fc14c8..5bb8c303f88b 100644
> > > > --- a/drivers/staging/octeon/ethernet.c
> > > > +++ b/drivers/staging/octeon/ethernet.c
> > > > @@ -5,6 +5,7 @@
> > > >   * Copyright (c) 2003-2007 Cavium Networks
> > > >   */
> > > >
> > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > >  #include <linux/platform_device.h>
> > > >  #include <linux/kernel.h>
> > > >  #include <linux/module.h>
> > > > --
> > > > 2.53.0
> > > >
> > >
> > > How about working to remove the existing pr_*() calls with the proper
> > > dev_*() and netdev_*() calls instead, so that pr_fmt() is not needed at
> > > all?  That is the more "correct" solution here.
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > Hi Greg,
> >
> > After investigating, the pr_*() calls in ethernet-mem.c and
> > ethernet-spi.c are inside functions that only receive hardware pool
> > indices or register structs with no net_device or device pointer
> > available, so dev_*() and netdev_*() replacements are not possible
> > there.
> >
> > For ethernet.c, device pointers are available at most call sites and I
> > can replace those with dev_err() and netdev_err()/netdev_info()
> > appropriately. The two calls where no device pointer is available
> > would keep pr_err() as is.
>
> That's a good start, but for the others, work back up the call chain to
> properly pass in a device pointer so that these warning/error messages
> can get printed out properly.  Drivers should not have any "generic"
> messages like that, as it does not show what device actually created the
> message.
>
> thanks,
>
> greg k-h

Hi Greg,

After tracing the call chains, the pr_warn() calls are inside
ethernet-mem.c. cvm_oct_mem_fill_fpa() which eventually reaches them
is called from two places:

1. ethernet.c via cvm_oct_configure_common_hw(), which is called from
cvm_oct_probe() - pdev is available here.
2. cvm_oct_rx_refill_pool() in ethernet-rx.h, which is called from
cvm_oct_poll() in ethernet-rx.c - no device pointer is available there
as it is called by the NAPI subsystem and oct_rx_group has no device
pointer either.

Since both call sites share cvm_oct_mem_fill_fpa(), adding a struct
device * parameter would break the second call site. The FPA pool is
shared hardware owned by the platform device, so storing &pdev->dev in
a static global during probe and using that in the mem functions in
ethernet-mem.c seems like the right approach. This avoids changing any
function parameters. The same global could also be used for the
pr_err() calls in ethernet-spi.c, where the interrupt handler has no
device pointer available either. Would that be acceptable, or is there
a preferred way to handle this?

Thanks,
Ayush

Re: [PATCH 3/3] staging: octeon: ethernet: add pr_fmt macro

[Greg KH]

On Thu, Mar 26, 2026 at 12:42:27AM +0530, Ayush Mukkanwar wrote:
> On Wed, Mar 25, 2026 at 2:41 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Mar 25, 2026 at 02:33:00PM +0530, Ayush Mukkanwar wrote:
> > > On Tue, Mar 24, 2026 at 7:58 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Tue, Mar 24, 2026 at 07:00:29PM +0530, AyushMukkanwar wrote:
> > > > > Add pr_fmt macro to prefix log messages with the module
> > > > > name for easier debugging.
> > > > >
> > > > > Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
> > > > > ---
> > > > >  drivers/staging/octeon/ethernet.c | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
> > > > > index eadb74fc14c8..5bb8c303f88b 100644
> > > > > --- a/drivers/staging/octeon/ethernet.c
> > > > > +++ b/drivers/staging/octeon/ethernet.c
> > > > > @@ -5,6 +5,7 @@
> > > > >   * Copyright (c) 2003-2007 Cavium Networks
> > > > >   */
> > > > >
> > > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > > >  #include <linux/platform_device.h>
> > > > >  #include <linux/kernel.h>
> > > > >  #include <linux/module.h>
> > > > > --
> > > > > 2.53.0
> > > > >
> > > >
> > > > How about working to remove the existing pr_*() calls with the proper
> > > > dev_*() and netdev_*() calls instead, so that pr_fmt() is not needed at
> > > > all?  That is the more "correct" solution here.
> > > >
> > > > thanks,
> > > >
> > > > greg k-h
> > >
> > > Hi Greg,
> > >
> > > After investigating, the pr_*() calls in ethernet-mem.c and
> > > ethernet-spi.c are inside functions that only receive hardware pool
> > > indices or register structs with no net_device or device pointer
> > > available, so dev_*() and netdev_*() replacements are not possible
> > > there.
> > >
> > > For ethernet.c, device pointers are available at most call sites and I
> > > can replace those with dev_err() and netdev_err()/netdev_info()
> > > appropriately. The two calls where no device pointer is available
> > > would keep pr_err() as is.
> >
> > That's a good start, but for the others, work back up the call chain to
> > properly pass in a device pointer so that these warning/error messages
> > can get printed out properly.  Drivers should not have any "generic"
> > messages like that, as it does not show what device actually created the
> > message.
> >
> > thanks,
> >
> > greg k-h
> 
> Hi Greg,
> 
> After tracing the call chains, the pr_warn() calls are inside
> ethernet-mem.c. cvm_oct_mem_fill_fpa() which eventually reaches them
> is called from two places:
> 
> 1. ethernet.c via cvm_oct_configure_common_hw(), which is called from
> cvm_oct_probe() - pdev is available here.
> 2. cvm_oct_rx_refill_pool() in ethernet-rx.h, which is called from
> cvm_oct_poll() in ethernet-rx.c - no device pointer is available there
> as it is called by the NAPI subsystem and oct_rx_group has no device
> pointer either.
> 
> Since both call sites share cvm_oct_mem_fill_fpa(), adding a struct
> device * parameter would break the second call site. The FPA pool is
> shared hardware owned by the platform device, so storing &pdev->dev in
> a static global during probe and using that in the mem functions in
> ethernet-mem.c seems like the right approach. This avoids changing any
> function parameters. The same global could also be used for the
> pr_err() calls in ethernet-spi.c, where the interrupt handler has no
> device pointer available either. Would that be acceptable, or is there
> a preferred way to handle this?

Drivers should NEVER have "shared hardware pools", they are always
device specific, so passing in the proper device everywhere is the
correct thing to be doing here.

thanks,

greg k-h

Re: [PATCH 3/3] staging: octeon: ethernet: add pr_fmt macro

[Ayush Mukkanwar]

On Mon, Mar 30, 2026 at 9:26 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Mar 26, 2026 at 12:42:27AM +0530, Ayush Mukkanwar wrote:
> > On Wed, Mar 25, 2026 at 2:41 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Wed, Mar 25, 2026 at 02:33:00PM +0530, Ayush Mukkanwar wrote:
> > > > On Tue, Mar 24, 2026 at 7:58 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > >
> > > > > On Tue, Mar 24, 2026 at 07:00:29PM +0530, AyushMukkanwar wrote:
> > > > > > Add pr_fmt macro to prefix log messages with the module
> > > > > > name for easier debugging.
> > > > > >
> > > > > > Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
> > > > > > ---
> > > > > >  drivers/staging/octeon/ethernet.c | 1 +
> > > > > >  1 file changed, 1 insertion(+)
> > > > > >
> > > > > > diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
> > > > > > index eadb74fc14c8..5bb8c303f88b 100644
> > > > > > --- a/drivers/staging/octeon/ethernet.c
> > > > > > +++ b/drivers/staging/octeon/ethernet.c
> > > > > > @@ -5,6 +5,7 @@
> > > > > >   * Copyright (c) 2003-2007 Cavium Networks
> > > > > >   */
> > > > > >
> > > > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > > > >  #include <linux/platform_device.h>
> > > > > >  #include <linux/kernel.h>
> > > > > >  #include <linux/module.h>
> > > > > > --
> > > > > > 2.53.0
> > > > > >
> > > > >
> > > > > How about working to remove the existing pr_*() calls with the proper
> > > > > dev_*() and netdev_*() calls instead, so that pr_fmt() is not needed at
> > > > > all?  That is the more "correct" solution here.
> > > > >
> > > > > thanks,
> > > > >
> > > > > greg k-h
> > > >
> > > > Hi Greg,
> > > >
> > > > After investigating, the pr_*() calls in ethernet-mem.c and
> > > > ethernet-spi.c are inside functions that only receive hardware pool
> > > > indices or register structs with no net_device or device pointer
> > > > available, so dev_*() and netdev_*() replacements are not possible
> > > > there.
> > > >
> > > > For ethernet.c, device pointers are available at most call sites and I
> > > > can replace those with dev_err() and netdev_err()/netdev_info()
> > > > appropriately. The two calls where no device pointer is available
> > > > would keep pr_err() as is.
> > >
> > > That's a good start, but for the others, work back up the call chain to
> > > properly pass in a device pointer so that these warning/error messages
> > > can get printed out properly.  Drivers should not have any "generic"
> > > messages like that, as it does not show what device actually created the
> > > message.
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > Hi Greg,
> >
> > After tracing the call chains, the pr_warn() calls are inside
> > ethernet-mem.c. cvm_oct_mem_fill_fpa() which eventually reaches them
> > is called from two places:
> >
> > 1. ethernet.c via cvm_oct_configure_common_hw(), which is called from
> > cvm_oct_probe() - pdev is available here.
> > 2. cvm_oct_rx_refill_pool() in ethernet-rx.h, which is called from
> > cvm_oct_poll() in ethernet-rx.c - no device pointer is available there
> > as it is called by the NAPI subsystem and oct_rx_group has no device
> > pointer either.
> >
> > Since both call sites share cvm_oct_mem_fill_fpa(), adding a struct
> > device * parameter would break the second call site. The FPA pool is
> > shared hardware owned by the platform device, so storing &pdev->dev in
> > a static global during probe and using that in the mem functions in
> > ethernet-mem.c seems like the right approach. This avoids changing any
> > function parameters. The same global could also be used for the
> > pr_err() calls in ethernet-spi.c, where the interrupt handler has no
> > device pointer available either. Would that be acceptable, or is there
> > a preferred way to handle this?
>
> Drivers should NEVER have "shared hardware pools", they are always
> device specific, so passing in the proper device everywhere is the
> correct thing to be doing here.
>
> thanks,
>
> greg k-h

Hi Greg,

I have made the changes for most of the call chains. However there is
one remaining blocker in ethernet.c.

cvm_oct_rx_refill_worker() in ethernet.c is a work queue callback with
a fixed signature that only receives struct work_struct *work. It has
no device pointer to pass to the cvm_oct_rx_refill_pool() call inside
it.
Currently cvm_oct_rx_refill_work is declared as a plain global
delayed_work in ethernet.c with no parent struct, so container_of
cannot be used to get a device pointer inside the worker.

One approach would be to embed the delayed_work inside a new struct
that also holds a struct device * pointer, set it during probe, and
use container_of inside the worker to get it. Is this the right
approach, or is there a preferred approach?

Thanks,
Ayush

Re: [PATCH 3/3] staging: octeon: ethernet: add pr_fmt macro

[Greg KH]

On Tue, Mar 31, 2026 at 12:46:10PM +0530, Ayush Mukkanwar wrote:
> On Mon, Mar 30, 2026 at 9:26 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Mar 26, 2026 at 12:42:27AM +0530, Ayush Mukkanwar wrote:
> > > On Wed, Mar 25, 2026 at 2:41 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Wed, Mar 25, 2026 at 02:33:00PM +0530, Ayush Mukkanwar wrote:
> > > > > On Tue, Mar 24, 2026 at 7:58 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > > >
> > > > > > On Tue, Mar 24, 2026 at 07:00:29PM +0530, AyushMukkanwar wrote:
> > > > > > > Add pr_fmt macro to prefix log messages with the module
> > > > > > > name for easier debugging.
> > > > > > >
> > > > > > > Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
> > > > > > > ---
> > > > > > >  drivers/staging/octeon/ethernet.c | 1 +
> > > > > > >  1 file changed, 1 insertion(+)
> > > > > > >
> > > > > > > diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
> > > > > > > index eadb74fc14c8..5bb8c303f88b 100644
> > > > > > > --- a/drivers/staging/octeon/ethernet.c
> > > > > > > +++ b/drivers/staging/octeon/ethernet.c
> > > > > > > @@ -5,6 +5,7 @@
> > > > > > >   * Copyright (c) 2003-2007 Cavium Networks
> > > > > > >   */
> > > > > > >
> > > > > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > > > > >  #include <linux/platform_device.h>
> > > > > > >  #include <linux/kernel.h>
> > > > > > >  #include <linux/module.h>
> > > > > > > --
> > > > > > > 2.53.0
> > > > > > >
> > > > > >
> > > > > > How about working to remove the existing pr_*() calls with the proper
> > > > > > dev_*() and netdev_*() calls instead, so that pr_fmt() is not needed at
> > > > > > all?  That is the more "correct" solution here.
> > > > > >
> > > > > > thanks,
> > > > > >
> > > > > > greg k-h
> > > > >
> > > > > Hi Greg,
> > > > >
> > > > > After investigating, the pr_*() calls in ethernet-mem.c and
> > > > > ethernet-spi.c are inside functions that only receive hardware pool
> > > > > indices or register structs with no net_device or device pointer
> > > > > available, so dev_*() and netdev_*() replacements are not possible
> > > > > there.
> > > > >
> > > > > For ethernet.c, device pointers are available at most call sites and I
> > > > > can replace those with dev_err() and netdev_err()/netdev_info()
> > > > > appropriately. The two calls where no device pointer is available
> > > > > would keep pr_err() as is.
> > > >
> > > > That's a good start, but for the others, work back up the call chain to
> > > > properly pass in a device pointer so that these warning/error messages
> > > > can get printed out properly.  Drivers should not have any "generic"
> > > > messages like that, as it does not show what device actually created the
> > > > message.
> > > >
> > > > thanks,
> > > >
> > > > greg k-h
> > >
> > > Hi Greg,
> > >
> > > After tracing the call chains, the pr_warn() calls are inside
> > > ethernet-mem.c. cvm_oct_mem_fill_fpa() which eventually reaches them
> > > is called from two places:
> > >
> > > 1. ethernet.c via cvm_oct_configure_common_hw(), which is called from
> > > cvm_oct_probe() - pdev is available here.
> > > 2. cvm_oct_rx_refill_pool() in ethernet-rx.h, which is called from
> > > cvm_oct_poll() in ethernet-rx.c - no device pointer is available there
> > > as it is called by the NAPI subsystem and oct_rx_group has no device
> > > pointer either.
> > >
> > > Since both call sites share cvm_oct_mem_fill_fpa(), adding a struct
> > > device * parameter would break the second call site. The FPA pool is
> > > shared hardware owned by the platform device, so storing &pdev->dev in
> > > a static global during probe and using that in the mem functions in
> > > ethernet-mem.c seems like the right approach. This avoids changing any
> > > function parameters. The same global could also be used for the
> > > pr_err() calls in ethernet-spi.c, where the interrupt handler has no
> > > device pointer available either. Would that be acceptable, or is there
> > > a preferred way to handle this?
> >
> > Drivers should NEVER have "shared hardware pools", they are always
> > device specific, so passing in the proper device everywhere is the
> > correct thing to be doing here.
> >
> > thanks,
> >
> > greg k-h
> 
> Hi Greg,
> 
> I have made the changes for most of the call chains. However there is
> one remaining blocker in ethernet.c.
> 
> cvm_oct_rx_refill_worker() in ethernet.c is a work queue callback with
> a fixed signature that only receives struct work_struct *work. It has
> no device pointer to pass to the cvm_oct_rx_refill_pool() call inside
> it.
> Currently cvm_oct_rx_refill_work is declared as a plain global
> delayed_work in ethernet.c with no parent struct, so container_of
> cannot be used to get a device pointer inside the worker.
> 
> One approach would be to embed the delayed_work inside a new struct
> that also holds a struct device * pointer, set it during probe, and
> use container_of inside the worker to get it. Is this the right
> approach, or is there a preferred approach?

There should not be any global data in a driver, it should all be on a
per-device basis, so if a workqueue needs to be added to the
device-specific structure, that's the correct solution to make.

thanks,

greg k-h

[PATCH v2 0/2] staging: octeon: replace pr_*() calls with dev_*() and netdev_*()

[AyushMukkanwar]

Replace pr_*() calls with dev_*() and netdev_*() across the octeon
ethernet drivers to include device information in log messages, as
suggested by Greg KH.

v2:
- Instead of adding pr_fmt() macros, replace pr_*() calls with
  proper dev_*() and netdev_*() calls
- Thread device pointer through call chains via function parameters
- Introduce struct octeon_ethernet_platform to pass device pointer
  through work queue callbacks via container_of
- Store device pointer in oct_rx_group for NAPI poll functions
- Use dev_id in interrupt handler to pass and retrieve device pointer

AyushMukkanwar (2):
  staging: octeon: ethernet-spi: replace pr_err with dev_err
  staging: octeon: ethernet: replace pr_* with dev_* and netdev_*

 drivers/staging/octeon/ethernet-mem.c | 26 ++++++------
 drivers/staging/octeon/ethernet-mem.h |  4 +-
 drivers/staging/octeon/ethernet-rx.c  |  6 ++-
 drivers/staging/octeon/ethernet-rx.h  |  6 +--
 drivers/staging/octeon/ethernet-spi.c | 59 +++++++++++++-------------
 drivers/staging/octeon/ethernet.c     | 61 ++++++++++++++++-----------
 6 files changed, 89 insertions(+), 73 deletions(-)

-- 
2.53.0

[PATCH v2 1/2] staging: octeon: ethernet-spi: replace pr_err with dev_err

[AyushMukkanwar]

Replace pr_err() calls with dev_err() to include device information
in log messages. The device pointer is passed through the interrupt
handler via dev_id, which is changed from &number_spi_ports to
dev->dev.parent in request_irq and free_irq.

Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
---
 drivers/staging/octeon/ethernet-spi.c | 59 ++++++++++++++-------------
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-spi.c b/drivers/staging/octeon/ethernet-spi.c
index 699c98c5ec13..8c02920c3cdc 100644
--- a/drivers/staging/octeon/ethernet-spi.c
+++ b/drivers/staging/octeon/ethernet-spi.c
@@ -17,67 +17,67 @@
 static int number_spi_ports;
 static int need_retrain[2] = { 0, 0 };
 
-static void cvm_oct_spxx_int_pr(union cvmx_spxx_int_reg spx_int_reg, int index)
+static void cvm_oct_spxx_int_pr(union cvmx_spxx_int_reg spx_int_reg, int index, struct device *dev)
 {
 	if (spx_int_reg.s.spf)
-		pr_err("SPI%d: SRX Spi4 interface down\n", index);
+		dev_err(dev, "SPI%d: SRX Spi4 interface down\n", index);
 	if (spx_int_reg.s.calerr)
-		pr_err("SPI%d: SRX Spi4 Calendar table parity error\n", index);
+		dev_err(dev, "SPI%d: SRX Spi4 Calendar table parity error\n", index);
 	if (spx_int_reg.s.syncerr)
-		pr_err("SPI%d: SRX Consecutive Spi4 DIP4 errors have exceeded SPX_ERR_CTL[ERRCNT]\n",
+		dev_err(dev, "SPI%d: SRX Consecutive Spi4 DIP4 errors have exceeded SPX_ERR_CTL[ERRCNT]\n",
 		       index);
 	if (spx_int_reg.s.diperr)
-		pr_err("SPI%d: SRX Spi4 DIP4 error\n", index);
+		dev_err(dev, "SPI%d: SRX Spi4 DIP4 error\n", index);
 	if (spx_int_reg.s.tpaovr)
-		pr_err("SPI%d: SRX Selected port has hit TPA overflow\n",
+		dev_err(dev, "SPI%d: SRX Selected port has hit TPA overflow\n",
 		       index);
 	if (spx_int_reg.s.rsverr)
-		pr_err("SPI%d: SRX Spi4 reserved control word detected\n",
+		dev_err(dev, "SPI%d: SRX Spi4 reserved control word detected\n",
 		       index);
 	if (spx_int_reg.s.drwnng)
-		pr_err("SPI%d: SRX Spi4 receive FIFO drowning/overflow\n",
+		dev_err(dev, "SPI%d: SRX Spi4 receive FIFO drowning/overflow\n",
 		       index);
 	if (spx_int_reg.s.clserr)
-		pr_err("SPI%d: SRX Spi4 packet closed on non-16B alignment without EOP\n",
+		dev_err(dev, "SPI%d: SRX Spi4 packet closed on non-16B alignment without EOP\n",
 		       index);
 	if (spx_int_reg.s.spiovr)
-		pr_err("SPI%d: SRX Spi4 async FIFO overflow\n", index);
+		dev_err(dev, "SPI%d: SRX Spi4 async FIFO overflow\n", index);
 	if (spx_int_reg.s.abnorm)
-		pr_err("SPI%d: SRX Abnormal packet termination (ERR bit)\n",
+		dev_err(dev, "SPI%d: SRX Abnormal packet termination (ERR bit)\n",
 		       index);
 	if (spx_int_reg.s.prtnxa)
-		pr_err("SPI%d: SRX Port out of range\n", index);
+		dev_err(dev, "SPI%d: SRX Port out of range\n", index);
 }
 
-static void cvm_oct_stxx_int_pr(union cvmx_stxx_int_reg stx_int_reg, int index)
+static void cvm_oct_stxx_int_pr(union cvmx_stxx_int_reg stx_int_reg, int index, struct device *dev)
 {
 	if (stx_int_reg.s.syncerr)
-		pr_err("SPI%d: STX Interface encountered a fatal error\n",
+		dev_err(dev, "SPI%d: STX Interface encountered a fatal error\n",
 		       index);
 	if (stx_int_reg.s.frmerr)
-		pr_err("SPI%d: STX FRMCNT has exceeded STX_DIP_CNT[MAXFRM]\n",
+		dev_err(dev, "SPI%d: STX FRMCNT has exceeded STX_DIP_CNT[MAXFRM]\n",
 		       index);
 	if (stx_int_reg.s.unxfrm)
-		pr_err("SPI%d: STX Unexpected framing sequence\n", index);
+		dev_err(dev, "SPI%d: STX Unexpected framing sequence\n", index);
 	if (stx_int_reg.s.nosync)
-		pr_err("SPI%d: STX ERRCNT has exceeded STX_DIP_CNT[MAXDIP]\n",
+		dev_err(dev, "SPI%d: STX ERRCNT has exceeded STX_DIP_CNT[MAXDIP]\n",
 		       index);
 	if (stx_int_reg.s.diperr)
-		pr_err("SPI%d: STX DIP2 error on the Spi4 Status channel\n",
+		dev_err(dev, "SPI%d: STX DIP2 error on the Spi4 Status channel\n",
 		       index);
 	if (stx_int_reg.s.datovr)
-		pr_err("SPI%d: STX Spi4 FIFO overflow error\n", index);
+		dev_err(dev, "SPI%d: STX Spi4 FIFO overflow error\n", index);
 	if (stx_int_reg.s.ovrbst)
-		pr_err("SPI%d: STX Transmit packet burst too big\n", index);
+		dev_err(dev, "SPI%d: STX Transmit packet burst too big\n", index);
 	if (stx_int_reg.s.calpar1)
-		pr_err("SPI%d: STX Calendar Table Parity Error Bank%d\n",
+		dev_err(dev, "SPI%d: STX Calendar Table Parity Error Bank%d\n",
 		       index, 1);
 	if (stx_int_reg.s.calpar0)
-		pr_err("SPI%d: STX Calendar Table Parity Error Bank%d\n",
+		dev_err(dev, "SPI%d: STX Calendar Table Parity Error Bank%d\n",
 		       index, 0);
 }
 
-static irqreturn_t cvm_oct_spi_spx_int(int index)
+static irqreturn_t cvm_oct_spi_spx_int(int index, struct device *dev)
 {
 	union cvmx_spxx_int_reg spx_int_reg;
 	union cvmx_stxx_int_reg stx_int_reg;
@@ -86,14 +86,14 @@ static irqreturn_t cvm_oct_spi_spx_int(int index)
 	cvmx_write_csr(CVMX_SPXX_INT_REG(index), spx_int_reg.u64);
 	if (!need_retrain[index]) {
 		spx_int_reg.u64 &= cvmx_read_csr(CVMX_SPXX_INT_MSK(index));
-		cvm_oct_spxx_int_pr(spx_int_reg, index);
+		cvm_oct_spxx_int_pr(spx_int_reg, index, dev);
 	}
 
 	stx_int_reg.u64 = cvmx_read_csr(CVMX_STXX_INT_REG(index));
 	cvmx_write_csr(CVMX_STXX_INT_REG(index), stx_int_reg.u64);
 	if (!need_retrain[index]) {
 		stx_int_reg.u64 &= cvmx_read_csr(CVMX_STXX_INT_MSK(index));
-		cvm_oct_stxx_int_pr(stx_int_reg, index);
+		cvm_oct_stxx_int_pr(stx_int_reg, index, dev);
 	}
 
 	cvmx_write_csr(CVMX_SPXX_INT_MSK(index), 0);
@@ -107,14 +107,15 @@ static irqreturn_t cvm_oct_spi_rml_interrupt(int cpl, void *dev_id)
 {
 	irqreturn_t return_status = IRQ_NONE;
 	union cvmx_npi_rsl_int_blocks rsl_int_blocks;
+	struct device *dev = dev_id;
 
 	/* Check and see if this interrupt was caused by the GMX block */
 	rsl_int_blocks.u64 = cvmx_read_csr(CVMX_NPI_RSL_INT_BLOCKS);
 	if (rsl_int_blocks.s.spx1) /* 19 - SPX1_INT_REG & STX1_INT_REG */
-		return_status = cvm_oct_spi_spx_int(1);
+		return_status = cvm_oct_spi_spx_int(1, dev);
 
 	if (rsl_int_blocks.s.spx0) /* 18 - SPX0_INT_REG & STX0_INT_REG */
-		return_status = cvm_oct_spi_spx_int(0);
+		return_status = cvm_oct_spi_spx_int(0, dev);
 
 	return return_status;
 }
@@ -196,7 +197,7 @@ int cvm_oct_spi_init(struct net_device *dev)
 
 	if (number_spi_ports == 0) {
 		r = request_irq(OCTEON_IRQ_RML, cvm_oct_spi_rml_interrupt,
-				IRQF_SHARED, "SPI", &number_spi_ports);
+				IRQF_SHARED, "SPI", dev->dev.parent);
 		if (r)
 			return r;
 	}
@@ -221,6 +222,6 @@ void cvm_oct_spi_uninit(struct net_device *dev)
 			cvmx_write_csr(CVMX_SPXX_INT_MSK(interface), 0);
 			cvmx_write_csr(CVMX_STXX_INT_MSK(interface), 0);
 		}
-		free_irq(OCTEON_IRQ_RML, &number_spi_ports);
+		free_irq(OCTEON_IRQ_RML, dev->dev.parent);
 	}
 }
-- 
2.53.0

Re: [PATCH v2 1/2] staging: octeon: ethernet-spi: replace pr_err with dev_err

[Greg KH]

On Tue, Mar 31, 2026 at 04:47:56PM +0530, AyushMukkanwar wrote:
> Replace pr_err() calls with dev_err() to include device information
> in log messages. The device pointer is passed through the interrupt
> handler via dev_id, which is changed from &number_spi_ports to
> dev->dev.parent in request_irq and free_irq.
> 
> Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
> ---
>  drivers/staging/octeon/ethernet-spi.c | 59 ++++++++++++++-------------
>  1 file changed, 30 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/staging/octeon/ethernet-spi.c b/drivers/staging/octeon/ethernet-spi.c
> index 699c98c5ec13..8c02920c3cdc 100644
> --- a/drivers/staging/octeon/ethernet-spi.c
> +++ b/drivers/staging/octeon/ethernet-spi.c
> @@ -17,67 +17,67 @@
>  static int number_spi_ports;
>  static int need_retrain[2] = { 0, 0 };
>  
> -static void cvm_oct_spxx_int_pr(union cvmx_spxx_int_reg spx_int_reg, int index)
> +static void cvm_oct_spxx_int_pr(union cvmx_spxx_int_reg spx_int_reg, int index, struct device *dev)

This is ok, but this, but usually the pointer is the first argument.
And shouldn't this be a netdev (see below...)

> @@ -107,14 +107,15 @@ static irqreturn_t cvm_oct_spi_rml_interrupt(int cpl, void *dev_id)
>  {
>  	irqreturn_t return_status = IRQ_NONE;
>  	union cvmx_npi_rsl_int_blocks rsl_int_blocks;
> +	struct device *dev = dev_id;

This isn't ok, the function prototype should really be a pointer, not a
void thing.

> @@ -196,7 +197,7 @@ int cvm_oct_spi_init(struct net_device *dev)
>  
>  	if (number_spi_ports == 0) {
>  		r = request_irq(OCTEON_IRQ_RML, cvm_oct_spi_rml_interrupt,
> -				IRQF_SHARED, "SPI", &number_spi_ports);
> +				IRQF_SHARED, "SPI", dev->dev.parent);

Wait, no, you can't do anything with the parent!

This is a netdev, keep it a netdev.  Don't pass it "up" the device
heirchary to the device pointer, use the real netdev pointer instead.

Also, it seems you didn't test build your changes, what happened?

thanks,

greg k-h

Re: [PATCH v2 1/2] staging: octeon: ethernet-spi: replace pr_err with dev_err

[Ayush Mukkanwar]

Hi Greg,

I'm preparing v4 of the octeon pr_*() conversion series, broken into
smaller patches as you requested.

While converting ethernet-spi.c, I hit a complication with the IRQ
handler. The SPI code registers a single shared IRQ across all SPI
ports:
if (number_spi_ports == 0)
    request_irq(OCTEON_IRQ_RML, handler, IRQF_SHARED, "SPI", dev_id);
number_spi_ports++;

The original code used &number_spi_ports (a static) as dev_id, so
request_irq and free_irq always matched. But to convert the handler's
pr_err() calls to netdev_err(), I need to pass a net_device * as
dev_id.

The problem is: the first SPI port registers the IRQ, but the last
port to uninit frees it with a different net_device *, causing a
dev_id mismatch.

Additionally, all the error messages in the handler are SPI4 bus-level
errors (DIP4, calendar parity, FIFO overflow, etc.), not specific to
any individual port. Would using dev_err() with the platform device be
acceptable here since the IRQ is registered once for the whole SPI
subsystem? Or would you prefer a different approach?

Thanks, Ayush

On Wed, Apr 1, 2026 at 3:39 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Mar 31, 2026 at 04:47:56PM +0530, AyushMukkanwar wrote:
> > Replace pr_err() calls with dev_err() to include device information
> > in log messages. The device pointer is passed through the interrupt
> > handler via dev_id, which is changed from &number_spi_ports to
> > dev->dev.parent in request_irq and free_irq.
> >
> > Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
> > ---
> >  drivers/staging/octeon/ethernet-spi.c | 59 ++++++++++++++-------------
> >  1 file changed, 30 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/staging/octeon/ethernet-spi.c b/drivers/staging/octeon/ethernet-spi.c
> > index 699c98c5ec13..8c02920c3cdc 100644
> > --- a/drivers/staging/octeon/ethernet-spi.c
> > +++ b/drivers/staging/octeon/ethernet-spi.c
> > @@ -17,67 +17,67 @@
> >  static int number_spi_ports;
> >  static int need_retrain[2] = { 0, 0 };
> >
> > -static void cvm_oct_spxx_int_pr(union cvmx_spxx_int_reg spx_int_reg, int index)
> > +static void cvm_oct_spxx_int_pr(union cvmx_spxx_int_reg spx_int_reg, int index, struct device *dev)
>
> This is ok, but this, but usually the pointer is the first argument.
> And shouldn't this be a netdev (see below...)
>
> > @@ -107,14 +107,15 @@ static irqreturn_t cvm_oct_spi_rml_interrupt(int cpl, void *dev_id)
> >  {
> >       irqreturn_t return_status = IRQ_NONE;
> >       union cvmx_npi_rsl_int_blocks rsl_int_blocks;
> > +     struct device *dev = dev_id;
>
> This isn't ok, the function prototype should really be a pointer, not a
> void thing.
>
> > @@ -196,7 +197,7 @@ int cvm_oct_spi_init(struct net_device *dev)
> >
> >       if (number_spi_ports == 0) {
> >               r = request_irq(OCTEON_IRQ_RML, cvm_oct_spi_rml_interrupt,
> > -                             IRQF_SHARED, "SPI", &number_spi_ports);
> > +                             IRQF_SHARED, "SPI", dev->dev.parent);
>
> Wait, no, you can't do anything with the parent!
>
> This is a netdev, keep it a netdev.  Don't pass it "up" the device
> heirchary to the device pointer, use the real netdev pointer instead.
>
> Also, it seems you didn't test build your changes, what happened?
>
> thanks,
>
> greg k-h

Re: [PATCH v2 1/2] staging: octeon: ethernet-spi: replace pr_err with dev_err

[Ayush Mukkanwar]

On Wed, Apr 1, 2026 at 3:39 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Mar 31, 2026 at 04:47:56PM +0530, AyushMukkanwar wrote:
> > Replace pr_err() calls with dev_err() to include device information
> > in log messages. The device pointer is passed through the interrupt
> > handler via dev_id, which is changed from &number_spi_ports to
> > dev->dev.parent in request_irq and free_irq.
> >
> > Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
> > ---
> >  drivers/staging/octeon/ethernet-spi.c | 59 ++++++++++++++-------------
> >  1 file changed, 30 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/staging/octeon/ethernet-spi.c b/drivers/staging/octeon/ethernet-spi.c
> > index 699c98c5ec13..8c02920c3cdc 100644
> > --- a/drivers/staging/octeon/ethernet-spi.c
> > +++ b/drivers/staging/octeon/ethernet-spi.c
> > @@ -17,67 +17,67 @@
> >  static int number_spi_ports;
> >  static int need_retrain[2] = { 0, 0 };
> >
> > -static void cvm_oct_spxx_int_pr(union cvmx_spxx_int_reg spx_int_reg, int index)
> > +static void cvm_oct_spxx_int_pr(union cvmx_spxx_int_reg spx_int_reg, int index, struct device *dev)
>
> This is ok, but this, but usually the pointer is the first argument.
> And shouldn't this be a netdev (see below...)
>
> > @@ -107,14 +107,15 @@ static irqreturn_t cvm_oct_spi_rml_interrupt(int cpl, void *dev_id)
> >  {
> >       irqreturn_t return_status = IRQ_NONE;
> >       union cvmx_npi_rsl_int_blocks rsl_int_blocks;
> > +     struct device *dev = dev_id;
>
> This isn't ok, the function prototype should really be a pointer, not a
> void thing.
>
> > @@ -196,7 +197,7 @@ int cvm_oct_spi_init(struct net_device *dev)
> >
> >       if (number_spi_ports == 0) {
> >               r = request_irq(OCTEON_IRQ_RML, cvm_oct_spi_rml_interrupt,
> > -                             IRQF_SHARED, "SPI", &number_spi_ports);
> > +                             IRQF_SHARED, "SPI", dev->dev.parent);
>
> Wait, no, you can't do anything with the parent!
>
> This is a netdev, keep it a netdev.  Don't pass it "up" the device
> heirchary to the device pointer, use the real netdev pointer instead.
>
> Also, it seems you didn't test build your changes, what happened?
>
> thanks,
>
> greg k-h

Hi Greg,

I'm preparing v4 of the octeon pr_*() conversion series, broken into
smaller patches as you requested.

While converting ethernet-spi.c, I hit a complication with the IRQ
handler. The SPI code registers a single shared IRQ across all SPI
ports:
if (number_spi_ports == 0)
    request_irq(OCTEON_IRQ_RML, handler, IRQF_SHARED, "SPI", dev_id);
number_spi_ports++;

The original code used &number_spi_ports (a static) as dev_id, so
request_irq and free_irq always matched. But to convert the handler's
pr_err() calls to netdev_err(), I need to pass a net_device * as
dev_id.

The problem is: the first SPI port registers the IRQ, but the last
port to uninit frees it with a different net_device *, causing a
dev_id mismatch.

Additionally, all the error messages in the handler are SPI4 bus-level
errors (DIP4, calendar parity, FIFO overflow, etc.), not specific to
any individual port. Would using dev_err() with the platform device be
acceptable here since the IRQ is registered once for the whole SPI
subsystem? Or would you prefer a different approach?

Thanks, Ayush

Re: [PATCH v2 1/2] staging: octeon: ethernet-spi: replace pr_err with dev_err

[Greg KH]

On Sat, Apr 04, 2026 at 02:50:56PM +0530, Ayush Mukkanwar wrote:
> On Wed, Apr 1, 2026 at 3:39 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Mar 31, 2026 at 04:47:56PM +0530, AyushMukkanwar wrote:
> > > Replace pr_err() calls with dev_err() to include device information
> > > in log messages. The device pointer is passed through the interrupt
> > > handler via dev_id, which is changed from &number_spi_ports to
> > > dev->dev.parent in request_irq and free_irq.
> > >
> > > Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
> > > ---
> > >  drivers/staging/octeon/ethernet-spi.c | 59 ++++++++++++++-------------
> > >  1 file changed, 30 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/drivers/staging/octeon/ethernet-spi.c b/drivers/staging/octeon/ethernet-spi.c
> > > index 699c98c5ec13..8c02920c3cdc 100644
> > > --- a/drivers/staging/octeon/ethernet-spi.c
> > > +++ b/drivers/staging/octeon/ethernet-spi.c
> > > @@ -17,67 +17,67 @@
> > >  static int number_spi_ports;
> > >  static int need_retrain[2] = { 0, 0 };
> > >
> > > -static void cvm_oct_spxx_int_pr(union cvmx_spxx_int_reg spx_int_reg, int index)
> > > +static void cvm_oct_spxx_int_pr(union cvmx_spxx_int_reg spx_int_reg, int index, struct device *dev)
> >
> > This is ok, but this, but usually the pointer is the first argument.
> > And shouldn't this be a netdev (see below...)
> >
> > > @@ -107,14 +107,15 @@ static irqreturn_t cvm_oct_spi_rml_interrupt(int cpl, void *dev_id)
> > >  {
> > >       irqreturn_t return_status = IRQ_NONE;
> > >       union cvmx_npi_rsl_int_blocks rsl_int_blocks;
> > > +     struct device *dev = dev_id;
> >
> > This isn't ok, the function prototype should really be a pointer, not a
> > void thing.
> >
> > > @@ -196,7 +197,7 @@ int cvm_oct_spi_init(struct net_device *dev)
> > >
> > >       if (number_spi_ports == 0) {
> > >               r = request_irq(OCTEON_IRQ_RML, cvm_oct_spi_rml_interrupt,
> > > -                             IRQF_SHARED, "SPI", &number_spi_ports);
> > > +                             IRQF_SHARED, "SPI", dev->dev.parent);
> >
> > Wait, no, you can't do anything with the parent!
> >
> > This is a netdev, keep it a netdev.  Don't pass it "up" the device
> > heirchary to the device pointer, use the real netdev pointer instead.
> >
> > Also, it seems you didn't test build your changes, what happened?
> >
> > thanks,
> >
> > greg k-h
> 
> Hi Greg,
> 
> I'm preparing v4 of the octeon pr_*() conversion series, broken into
> smaller patches as you requested.
> 
> While converting ethernet-spi.c, I hit a complication with the IRQ
> handler. The SPI code registers a single shared IRQ across all SPI
> ports:
> if (number_spi_ports == 0)
>     request_irq(OCTEON_IRQ_RML, handler, IRQF_SHARED, "SPI", dev_id);
> number_spi_ports++;
> 
> The original code used &number_spi_ports (a static) as dev_id, so
> request_irq and free_irq always matched. But to convert the handler's
> pr_err() calls to netdev_err(), I need to pass a net_device * as
> dev_id.
> 
> The problem is: the first SPI port registers the IRQ, but the last
> port to uninit frees it with a different net_device *, causing a
> dev_id mismatch.
> 
> Additionally, all the error messages in the handler are SPI4 bus-level
> errors (DIP4, calendar parity, FIFO overflow, etc.), not specific to
> any individual port. Would using dev_err() with the platform device be
> acceptable here since the IRQ is registered once for the whole SPI
> subsystem? Or would you prefer a different approach?

Use your best judgement here, it sounds like this driver needs lots of
cleanups!

good luck,

greg k-h

[PATCH v2 2/2] staging: octeon: ethernet: replace pr_* with dev_* and netdev_*

[AyushMukkanwar]

Replace pr_*() calls with dev_*() and netdev_*() to include device
information in log messages. A per-device struct octeon_ethernet_platform
is introduced to pass the device pointer through work queue callbacks
via container_of. The device pointer is also stored in oct_rx_group
to make it available in the NAPI poll function and propagated through
the memory pool functions by adding a struct device * parameter.

Signed-off-by: AyushMukkanwar <ayushmukkanwar@gmail.com>
---
 drivers/staging/octeon/ethernet-mem.c | 26 ++++++------
 drivers/staging/octeon/ethernet-mem.h |  4 +-
 drivers/staging/octeon/ethernet-rx.c  |  6 ++-
 drivers/staging/octeon/ethernet-rx.h  |  6 +--
 drivers/staging/octeon/ethernet.c     | 61 ++++++++++++++++-----------
 5 files changed, 59 insertions(+), 44 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-mem.c b/drivers/staging/octeon/ethernet-mem.c
index 532594957ebc..68e1d416c22c 100644
--- a/drivers/staging/octeon/ethernet-mem.c
+++ b/drivers/staging/octeon/ethernet-mem.c
@@ -44,7 +44,7 @@ static int cvm_oct_fill_hw_skbuff(int pool, int size, int elements)
  * @size:     Size of the buffer needed for the pool
  * @elements: Number of buffers to allocate
  */
-static void cvm_oct_free_hw_skbuff(int pool, int size, int elements)
+static void cvm_oct_free_hw_skbuff(int pool, int size, int elements, struct device *dev)
 {
 	char *memory;
 
@@ -59,10 +59,10 @@ static void cvm_oct_free_hw_skbuff(int pool, int size, int elements)
 	} while (memory);
 
 	if (elements < 0)
-		pr_warn("Freeing of pool %u had too many skbuffs (%d)\n",
+		dev_warn(dev, "Freeing of pool %u had too many skbuffs (%d)\n",
 			pool, elements);
 	else if (elements > 0)
-		pr_warn("Freeing of pool %u is missing %d skbuffs\n",
+		dev_warn(dev, "Freeing of pool %u is missing %d skbuffs\n",
 			pool, elements);
 }
 
@@ -74,7 +74,7 @@ static void cvm_oct_free_hw_skbuff(int pool, int size, int elements)
  *
  * Returns the actual number of buffers allocated.
  */
-static int cvm_oct_fill_hw_memory(int pool, int size, int elements)
+static int cvm_oct_fill_hw_memory(int pool, int size, int elements, struct device *dev)
 {
 	char *memory;
 	char *fpa;
@@ -93,7 +93,7 @@ static int cvm_oct_fill_hw_memory(int pool, int size, int elements)
 		 */
 		memory = kmalloc(size + 256, GFP_ATOMIC);
 		if (unlikely(!memory)) {
-			pr_warn("Unable to allocate %u bytes for FPA pool %d\n",
+			dev_warn(dev, "Unable to allocate %u bytes for FPA pool %d\n",
 				elements * size, pool);
 			break;
 		}
@@ -111,7 +111,7 @@ static int cvm_oct_fill_hw_memory(int pool, int size, int elements)
  * @size:     Size of each buffer in the pool
  * @elements: Number of buffers that should be in the pool
  */
-static void cvm_oct_free_hw_memory(int pool, int size, int elements)
+static void cvm_oct_free_hw_memory(int pool, int size, int elements, struct device *dev)
 {
 	char *memory;
 	char *fpa;
@@ -127,28 +127,28 @@ static void cvm_oct_free_hw_memory(int pool, int size, int elements)
 	} while (fpa);
 
 	if (elements < 0)
-		pr_warn("Freeing of pool %u had too many buffers (%d)\n",
+		dev_warn(dev, "Freeing of pool %u had too many buffers (%d)\n",
 			pool, elements);
 	else if (elements > 0)
-		pr_warn("Warning: Freeing of pool %u is missing %d buffers\n",
+		dev_warn(dev, "Warning: Freeing of pool %u is missing %d buffers\n",
 			pool, elements);
 }
 
-int cvm_oct_mem_fill_fpa(int pool, int size, int elements)
+int cvm_oct_mem_fill_fpa(int pool, int size, int elements, struct device *dev)
 {
 	int freed;
 
 	if (pool == CVMX_FPA_PACKET_POOL)
 		freed = cvm_oct_fill_hw_skbuff(pool, size, elements);
 	else
-		freed = cvm_oct_fill_hw_memory(pool, size, elements);
+		freed = cvm_oct_fill_hw_memory(pool, size, elements, dev);
 	return freed;
 }
 
-void cvm_oct_mem_empty_fpa(int pool, int size, int elements)
+void cvm_oct_mem_empty_fpa(int pool, int size, int elements, struct device *dev)
 {
 	if (pool == CVMX_FPA_PACKET_POOL)
-		cvm_oct_free_hw_skbuff(pool, size, elements);
+		cvm_oct_free_hw_skbuff(pool, size, elements, dev);
 	else
-		cvm_oct_free_hw_memory(pool, size, elements);
+		cvm_oct_free_hw_memory(pool, size, elements, dev);
 }
diff --git a/drivers/staging/octeon/ethernet-mem.h b/drivers/staging/octeon/ethernet-mem.h
index 692dcdb7154d..22a38846c751 100644
--- a/drivers/staging/octeon/ethernet-mem.h
+++ b/drivers/staging/octeon/ethernet-mem.h
@@ -5,5 +5,5 @@
  * Copyright (c) 2003-2007 Cavium Networks
  */
 
-int cvm_oct_mem_fill_fpa(int pool, int size, int elements);
-void cvm_oct_mem_empty_fpa(int pool, int size, int elements);
+int cvm_oct_mem_fill_fpa(int pool, int size, int elements, struct device *dev);
+void cvm_oct_mem_empty_fpa(int pool, int size, int elements, struct device *dev);
diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
index d0b43d50b83c..461f9077742e 100644
--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -35,6 +35,7 @@ static struct oct_rx_group {
 	int irq;
 	int group;
 	struct napi_struct napi;
+	struct device *dev;
 } oct_rx_group[16];
 
 /**
@@ -397,7 +398,7 @@ static int cvm_oct_poll(struct oct_rx_group *rx_group, int budget)
 		/* Restore the scratch area */
 		cvmx_scratch_write64(CVMX_SCR_SCRATCH, old_scratch);
 	}
-	cvm_oct_rx_refill_pool(0);
+	cvm_oct_rx_refill_pool(0, rx_group->dev);
 
 	return rx_count;
 }
@@ -448,7 +449,7 @@ void cvm_oct_poll_controller(struct net_device *dev)
 }
 #endif
 
-void cvm_oct_rx_initialize(void)
+void cvm_oct_rx_initialize(struct device *dev)
 {
 	int i;
 	struct net_device *dev_for_napi = NULL;
@@ -475,6 +476,7 @@ void cvm_oct_rx_initialize(void)
 
 		oct_rx_group[i].irq = OCTEON_IRQ_WORKQ0 + i;
 		oct_rx_group[i].group = i;
+		oct_rx_group[i].dev = dev;
 
 		/* Register an IRQ handler to receive POW interrupts */
 		ret = request_irq(oct_rx_group[i].irq, cvm_oct_do_interrupt, 0,
diff --git a/drivers/staging/octeon/ethernet-rx.h b/drivers/staging/octeon/ethernet-rx.h
index ff6482fa20d6..443c6208d09d 100644
--- a/drivers/staging/octeon/ethernet-rx.h
+++ b/drivers/staging/octeon/ethernet-rx.h
@@ -6,10 +6,10 @@
  */
 
 void cvm_oct_poll_controller(struct net_device *dev);
-void cvm_oct_rx_initialize(void);
+void cvm_oct_rx_initialize(struct device *dev);
 void cvm_oct_rx_shutdown(void);
 
-static inline void cvm_oct_rx_refill_pool(int fill_threshold)
+static inline void cvm_oct_rx_refill_pool(int fill_threshold, struct device *dev)
 {
 	int number_to_free;
 	int num_freed;
@@ -22,7 +22,7 @@ static inline void cvm_oct_rx_refill_pool(int fill_threshold)
 				      -number_to_free);
 		num_freed = cvm_oct_mem_fill_fpa(CVMX_FPA_PACKET_POOL,
 						 CVMX_FPA_PACKET_POOL_SIZE,
-						 number_to_free);
+						 number_to_free, dev);
 		if (num_freed != number_to_free) {
 			cvmx_fau_atomic_add32(FAU_NUM_PACKET_BUFFERS_TO_FREE,
 					      number_to_free - num_freed);
diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
index eadb74fc14c8..36fbeabe9eb2 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -104,11 +104,18 @@ struct net_device *cvm_oct_device[TOTAL_NUMBER_OF_PORTS];
 
 u64 cvm_oct_tx_poll_interval;
 
-static void cvm_oct_rx_refill_worker(struct work_struct *work);
-static DECLARE_DELAYED_WORK(cvm_oct_rx_refill_work, cvm_oct_rx_refill_worker);
+struct octeon_ethernet_platform {
+	struct device *dev;
+	struct delayed_work rx_refill_work;
+};
+
+static struct octeon_ethernet_platform *oct_plt;
 
 static void cvm_oct_rx_refill_worker(struct work_struct *work)
 {
+	struct octeon_ethernet_platform *plt = container_of(work, struct octeon_ethernet_platform,
+		rx_refill_work.work);
+
 	/*
 	 * FPA 0 may have been drained, try to refill it if we need
 	 * more than num_packet_buffers / 2, otherwise normal receive
@@ -116,10 +123,11 @@ static void cvm_oct_rx_refill_worker(struct work_struct *work)
 	 * could be received so cvm_oct_napi_poll would never be
 	 * invoked to do the refill.
 	 */
-	cvm_oct_rx_refill_pool(num_packet_buffers / 2);
+
+	cvm_oct_rx_refill_pool(num_packet_buffers / 2, plt->dev);
 
 	if (!atomic_read(&cvm_oct_poll_queue_stopping))
-		schedule_delayed_work(&cvm_oct_rx_refill_work, HZ);
+		schedule_delayed_work(&plt->rx_refill_work, HZ);
 }
 
 static void cvm_oct_periodic_worker(struct work_struct *work)
@@ -138,17 +146,17 @@ static void cvm_oct_periodic_worker(struct work_struct *work)
 		schedule_delayed_work(&priv->port_periodic_work, HZ);
 }
 
-static void cvm_oct_configure_common_hw(void)
+static void cvm_oct_configure_common_hw(struct device *dev)
 {
 	/* Setup the FPA */
 	cvmx_fpa_enable();
 	cvm_oct_mem_fill_fpa(CVMX_FPA_PACKET_POOL, CVMX_FPA_PACKET_POOL_SIZE,
-			     num_packet_buffers);
+			     num_packet_buffers, dev);
 	cvm_oct_mem_fill_fpa(CVMX_FPA_WQE_POOL, CVMX_FPA_WQE_POOL_SIZE,
-			     num_packet_buffers);
+			     num_packet_buffers, dev);
 	if (CVMX_FPA_OUTPUT_BUFFER_POOL != CVMX_FPA_PACKET_POOL)
 		cvm_oct_mem_fill_fpa(CVMX_FPA_OUTPUT_BUFFER_POOL,
-				     CVMX_FPA_OUTPUT_BUFFER_POOL_SIZE, 1024);
+				     CVMX_FPA_OUTPUT_BUFFER_POOL_SIZE, 1024, dev);
 
 #ifdef __LITTLE_ENDIAN
 	{
@@ -685,11 +693,17 @@ static int cvm_oct_probe(struct platform_device *pdev)
 
 	pip = pdev->dev.of_node;
 	if (!pip) {
-		pr_err("Error: No 'pip' in /aliases\n");
+		dev_err(&pdev->dev, "Error: No 'pip' in /aliases\n");
 		return -EINVAL;
 	}
 
-	cvm_oct_configure_common_hw();
+	oct_plt = devm_kzalloc(&pdev->dev, sizeof(*oct_plt), GFP_KERNEL);
+	if (!oct_plt)
+		return -ENOMEM;
+	oct_plt->dev = &pdev->dev;
+	INIT_DELAYED_WORK(&oct_plt->rx_refill_work, cvm_oct_rx_refill_worker);
+
+	cvm_oct_configure_common_hw(&pdev->dev);
 
 	cvmx_helper_initialize_packet_io_global();
 
@@ -783,16 +797,15 @@ static int cvm_oct_probe(struct platform_device *pdev)
 			dev->max_mtu = OCTEON_MAX_MTU - mtu_overhead;
 
 			if (register_netdev(dev) < 0) {
-				pr_err("Failed to register ethernet device for POW\n");
+				netdev_err(dev, "Failed to register ethernet device for POW\n");
 				free_netdev(dev);
 			} else {
 				cvm_oct_device[CVMX_PIP_NUM_INPUT_PORTS] = dev;
-				pr_info("%s: POW send group %d, receive group %d\n",
-					dev->name, pow_send_group,
-					pow_receive_group);
+				netdev_info(dev, "POW send group %d, receive group %d\n",
+				pow_send_group, pow_receive_group);
 			}
 		} else {
-			pr_err("Failed to allocate ethernet device for POW\n");
+			dev_err(&pdev->dev, "Failed to allocate ethernet device for POW\n");
 		}
 	}
 
@@ -812,8 +825,8 @@ static int cvm_oct_probe(struct platform_device *pdev)
 			struct net_device *dev =
 			    alloc_etherdev(sizeof(struct octeon_ethernet));
 			if (!dev) {
-				pr_err("Failed to allocate ethernet device for port %d\n",
-				       port);
+				dev_err(&pdev->dev, "Failed to allocate ethernet device for port %d\n",
+					port);
 				continue;
 			}
 
@@ -897,7 +910,7 @@ static int cvm_oct_probe(struct platform_device *pdev)
 			if (!dev->netdev_ops) {
 				free_netdev(dev);
 			} else if (register_netdev(dev) < 0) {
-				pr_err("Failed to register ethernet device for interface %d, port %d\n",
+				netdev_err(dev, "Failed to register ethernet device for interface %d, port %d\n",
 				       interface, priv->port);
 				free_netdev(dev);
 			} else {
@@ -912,14 +925,14 @@ static int cvm_oct_probe(struct platform_device *pdev)
 	}
 
 	cvm_oct_tx_initialize();
-	cvm_oct_rx_initialize();
+	cvm_oct_rx_initialize(&pdev->dev);
 
 	/*
 	 * 150 uS: about 10 1500-byte packets at 1GE.
 	 */
 	cvm_oct_tx_poll_interval = 150 * (octeon_get_clock_rate() / 1000000);
 
-	schedule_delayed_work(&cvm_oct_rx_refill_work, HZ);
+	schedule_delayed_work(&oct_plt->rx_refill_work, HZ);
 
 	return 0;
 }
@@ -931,7 +944,7 @@ static void cvm_oct_remove(struct platform_device *pdev)
 	cvmx_ipd_disable();
 
 	atomic_inc_return(&cvm_oct_poll_queue_stopping);
-	cancel_delayed_work_sync(&cvm_oct_rx_refill_work);
+	cancel_delayed_work_sync(&oct_plt->rx_refill_work);
 
 	cvm_oct_rx_shutdown();
 	cvm_oct_tx_shutdown();
@@ -959,12 +972,12 @@ static void cvm_oct_remove(struct platform_device *pdev)
 
 	/* Free the HW pools */
 	cvm_oct_mem_empty_fpa(CVMX_FPA_PACKET_POOL, CVMX_FPA_PACKET_POOL_SIZE,
-			      num_packet_buffers);
+			      num_packet_buffers, &pdev->dev);
 	cvm_oct_mem_empty_fpa(CVMX_FPA_WQE_POOL, CVMX_FPA_WQE_POOL_SIZE,
-			      num_packet_buffers);
+			      num_packet_buffers, &pdev->dev);
 	if (CVMX_FPA_OUTPUT_BUFFER_POOL != CVMX_FPA_PACKET_POOL)
 		cvm_oct_mem_empty_fpa(CVMX_FPA_OUTPUT_BUFFER_POOL,
-				      CVMX_FPA_OUTPUT_BUFFER_POOL_SIZE, 128);
+				      CVMX_FPA_OUTPUT_BUFFER_POOL_SIZE, 128, &pdev->dev);
 }
 
 static const struct of_device_id cvm_oct_match[] = {
-- 
2.53.0

Re: [PATCH v2 2/2] staging: octeon: ethernet: replace pr_* with dev_* and netdev_*

[kernel test robot]

Hi AyushMukkanwar,

kernel test robot noticed the following build warnings:

[auto build test WARNING on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/AyushMukkanwar/staging-octeon-ethernet-spi-replace-pr_err-with-dev_err/20260331-194415
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20260331111757.110703-3-ayushmukkanwar%40gmail.com
patch subject: [PATCH v2 2/2] staging: octeon: ethernet: replace pr_* with dev_* and netdev_*
config: parisc-randconfig-002-20260401 (https://download.01.org/0day-ci/archive/20260401/202604010854.Dg9b25Se-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260401/202604010854.Dg9b25Se-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202604010854.Dg9b25Se-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: drivers/staging/octeon/ethernet-mem.c:47 function parameter 'dev' not described in 'cvm_oct_free_hw_skbuff'
>> Warning: drivers/staging/octeon/ethernet-mem.c:77 function parameter 'dev' not described in 'cvm_oct_fill_hw_memory'
>> Warning: drivers/staging/octeon/ethernet-mem.c:114 function parameter 'dev' not described in 'cvm_oct_free_hw_memory'
>> Warning: drivers/staging/octeon/ethernet-mem.c:47 function parameter 'dev' not described in 'cvm_oct_free_hw_skbuff'
>> Warning: drivers/staging/octeon/ethernet-mem.c:77 function parameter 'dev' not described in 'cvm_oct_fill_hw_memory'
>> Warning: drivers/staging/octeon/ethernet-mem.c:114 function parameter 'dev' not described in 'cvm_oct_free_hw_memory'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 2/2] staging: octeon: ethernet: replace pr_* with dev_* and netdev_*

[Dan Carpenter]

On Tue, Mar 31, 2026 at 04:47:57PM +0530, AyushMukkanwar wrote:
> @@ -783,16 +797,15 @@ static int cvm_oct_probe(struct platform_device *pdev)
>  			dev->max_mtu = OCTEON_MAX_MTU - mtu_overhead;
>  
>  			if (register_netdev(dev) < 0) {
> -				pr_err("Failed to register ethernet device for POW\n");
> +				netdev_err(dev, "Failed to register ethernet device for POW\n");
>  				free_netdev(dev);
>  			} else {
>  				cvm_oct_device[CVMX_PIP_NUM_INPUT_PORTS] = dev;
> -				pr_info("%s: POW send group %d, receive group %d\n",
> -					dev->name, pow_send_group,
> -					pow_receive_group);
> +				netdev_info(dev, "POW send group %d, receive group %d\n",
> +				pow_send_group, pow_receive_group);
>  			}

Please run your patches through checkpatch.pl.

regards,
dan carpenter