[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0c8c5f34-c5cb-4a81-98fc-e3b957a2a8e9@intel.com>
Date: Wed, 15 Oct 2025 10:32:47 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
CC: <intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>, "Guenter
Roeck" <linux@...ck-us.net>
Subject: Re: [Intel-wired-lan] [PATCH iwl-net v1] ixgbe: guard fwlog code by
CONFIG_DEBUG_FS
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@...ck-us.net>
>>> Closes: https://lore.kernel.org/lkml/f594c621-f9e1-49f2-af31-23fbcb176058@roeck-us.net/
>>> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@...ux.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.
> 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.
Thanks,
Jake
Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)
Powered by blists - more mailing lists