Re: [Intel-wired-lan] [PATCH iwl-net v1] ixgbe: guard fwlog code by CONFIG_DEBUG_FS

[Michal Swiatkowski <michal.swiatkowski@linux.intel.com>]

On Wed, Oct 15, 2025 at 10:53:45AM -0700, Guenter Roeck wrote:
> On 10/15/25 10:32, Jacob Keller wrote:
> > 
> > 
> > On 10/14/2025 10:24 PM, Michal Swiatkowski wrote:
> > > On Tue, Oct 14, 2025 at 04:41:43PM -0700, Jacob Keller wrote:
> > > > 
> > > > 
> > > > On 10/14/2025 7:11 AM, Michal Swiatkowski wrote:
> > > > > Building the ixgbe without CONFIG_DEBUG_FS leads to a build error. Fix
> > > > > that by guarding fwlog code.
> > > > > 
> > > > > Fixes: 641585bc978e ("ixgbe: fwlog support for e610")
> > > > > Reported-by: Guenter Roeck <linux@roeck-us.net>
> > > > > Closes: https://lore.kernel.org/lkml/f594c621-f9e1-49f2-af31-23fbcb176058@roeck-us.net/
> > > > > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > > > > ---
> > > > 
> > > > Hm. It probably is best to make this optional and not require debugfs
> > > > via kconfig.
> > > 
> > > Make sense
> > > 
> > > > 
> > > > >   drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 2 ++
> > > > >   drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 8 ++++++++
> > > > >   2 files changed, 10 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> > > > > index c2f8189a0738..c5d76222df18 100644
> > > > > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> > > > > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> > > > > @@ -3921,6 +3921,7 @@ static int ixgbe_read_pba_string_e610(struct ixgbe_hw *hw, u8 *pba_num,
> > > > >   	return err;
> > > > >   }
> > > > > +#ifdef CONFIG_DEBUG_FS
> > > > >   static int __fwlog_send_cmd(void *priv, struct libie_aq_desc *desc, void *buf,
> > > > >   			    u16 size)
> > > > >   {
> > > > > @@ -3952,6 +3953,7 @@ void ixgbe_fwlog_deinit(struct ixgbe_hw *hw)
> > > > >   	libie_fwlog_deinit(&hw->fwlog);
> > > > >   }
> > > > > +#endif /* CONFIG_DEBUG_FS */
> > > > 
> > > > What does the fwlog module from libie do? Seems likely that it won't
> > > > compile without CONFIG_DEBUG_FS either...
> > > 
> > > Right, it shouldn't, because there is a dependency on fs/debugfs.
> > > It is building on my env, but maybe I don't have it fully cleaned.

BTW, it is building because DEBUG_FS also have dummy functions if not
selected.

> > > I wonder, because in ice there wasn't a check (or select) for
> > > CONFIG_DEBUG_FS for fwlog code.
> > > 
> > > Looks like LIBIE_FWLOG should select DEBUG_FS, right?
> > > I will send v2 with that, if it is fine.
> > > 
> > > Thanks
> > > 
> > My only worry is that could lead to ice selecting LIBIE_FWLOG which then
> > selects DEBUG_FS which then means we implicitly require DEBUG_FS regardless.
> > 
> > I don't quite remember the semantics of select and whether that would
> > let you build a kernel without DEBUG_FS.. I think some systems would
> > like to be able to disable DEBUG_FS as a way of limiting scope of
> > available interfaces exposed by the kernel.
> >

Yeah, the idea with dummy functions is better, thanks for your input.

> 
> If fwlog depends on debugfs, why not spell out that dependency and
> provide dummy functions if it isn't enabled ? The Kconfig entries
> selecting it could then be changed to
> 	select LIBIE_FWLOG if DEBUG_FS
> 

Sounds good, thanks.

> Guenter
>