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@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxx>
> > > > > > ---
> > > > > >  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