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

[Greg KH <gregkh@linuxfoundation.org>]

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