| Message ID | 20240919043707.206400-1-fujita.tomonori@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: phy: qt2025: Fix warning: unused import DeviceId | expand |
On Thu, Sep 19, 2024 at 6:39 AM FUJITA Tomonori <fujita.tomonori@gmail.com> wrote: > > Fix the following warning when the driver is compiled as built-in: > > >> warning: unused import: `DeviceId` > --> drivers/net/phy/qt2025.rs:18:5 > | > 18 | DeviceId, Driver, > | ^^^^^^^^ > | > = note: `#[warn(unused_imports)]` on by default > > device_table in module_phy_driver macro is defined only when the > driver is built as module. Use an absolute module path in the macro > instead of importing `DeviceId`. > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202409190717.i135rfVo-lkp@intel.com/ > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> It may be nice to change the macro to always use the expression so that this warning doesn't happen again. Anyway, that is a separate issue. Reviewed-by: Alice Ryhl <aliceryhl@google.com>
On Thu, 19 Sep 2024 04:37:07 +0000 FUJITA Tomonori <fujita.tomonori@gmail.com> wrote: > Fix the following warning when the driver is compiled as built-in: > >>> warning: unused import: `DeviceId` > --> drivers/net/phy/qt2025.rs:18:5 > | > 18 | DeviceId, Driver, > | ^^^^^^^^ > | > = note: `#[warn(unused_imports)]` on by default > > device_table in module_phy_driver macro is defined only when the > driver is built as module. Use an absolute module path in the macro > instead of importing `DeviceId`. Oops, the last sentence isn't correct. It should've been something like: Use phy::DeviceId in the macro instead of importing `DeviceId` since `phy` is always used.
Hi, On Thu, 19 Sep 2024 08:17:42 +0200 Alice Ryhl <aliceryhl@google.com> wrote: > On Thu, Sep 19, 2024 at 6:39 AM FUJITA Tomonori > <fujita.tomonori@gmail.com> wrote: >> >> Fix the following warning when the driver is compiled as built-in: >> >> >> warning: unused import: `DeviceId` >> --> drivers/net/phy/qt2025.rs:18:5 >> | >> 18 | DeviceId, Driver, >> | ^^^^^^^^ >> | >> = note: `#[warn(unused_imports)]` on by default >> >> device_table in module_phy_driver macro is defined only when the >> driver is built as module. Use an absolute module path in the macro >> instead of importing `DeviceId`. >> >> Reported-by: kernel test robot <lkp@intel.com> >> Closes: https://lore.kernel.org/oe-kbuild-all/202409190717.i135rfVo-lkp@intel.com/ >> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> > > It may be nice to change the macro to always use the expression so > that this warning doesn't happen again. Like the C code does, a valuable is defined only when the driver is built as module because the valuable is used to create the information for module loading. So the macro adds `#[cfg(MODULE)]` like the following: #[cfg(MODULE)] #[no_mangle] static __mod_mdio__phydev_device_table: [::kernel::bindings::mdio_device_id; 2] = [ ::kernel::bindings::mdio_device_id { phy_id: 0x00000001, phy_id_mask: 0xffffffff, }, ::kernel::bindings::mdio_device_id { phy_id: 0, phy_id_mask: 0, }, ]; We can remove `#[cfg(MODULE)]` however an unused valuable to added to the kernel image when the driver is compiled as built-in. Seems that with `#[no_mangle]`, the compiler doesn't give a warning about unused valuable though. Is there a nice way to handle such case? > Anyway, that is a separate issue. > > Reviewed-by: Alice Ryhl <aliceryhl@google.com> Thanks a lot!
On Thu, Sep 19, 2024 at 6:39 AM FUJITA Tomonori <fujita.tomonori@gmail.com> wrote: > > Fix the following warning when the driver is compiled as built-in: > > >> warning: unused import: `DeviceId` > --> drivers/net/phy/qt2025.rs:18:5 > | > 18 | DeviceId, Driver, > | ^^^^^^^^ > | > = note: `#[warn(unused_imports)]` on by default The >> shows up as a quote on lore. Should this entire block be indented? > device_table in module_phy_driver macro is defined only when the > driver is built as module. Use an absolute module path in the macro nit: "as module" -> "as a module" > instead of importing `DeviceId`. > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202409190717.i135rfVo-lkp@intel.com/ > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> > --- Easy enough fix, thanks for being on top of it. Reviewed-by: Trevor Gross <tmgross@umich.edu>
On Fri, Sep 20, 2024 at 3:54 PM FUJITA Tomonori <fujita.tomonori@gmail.com> wrote: > > Hi, > > On Thu, 19 Sep 2024 08:17:42 +0200 > Alice Ryhl <aliceryhl@google.com> wrote: > > > It may be nice to change the macro to always use the expression so > > that this warning doesn't happen again. > > Like the C code does, a valuable is defined only when the driver is > built as module because the valuable is used to create the information > for module loading. So the macro adds `#[cfg(MODULE)]` like the > following: > > #[cfg(MODULE)] > #[no_mangle] > static __mod_mdio__phydev_device_table: [::kernel::bindings::mdio_device_id; 2] = [ > ::kernel::bindings::mdio_device_id { > phy_id: 0x00000001, > phy_id_mask: 0xffffffff, > }, > ::kernel::bindings::mdio_device_id { > phy_id: 0, > phy_id_mask: 0, > }, > ]; > > We can remove `#[cfg(MODULE)]` however an unused valuable to added to > the kernel image when the driver is compiled as built-in. Seems that > with `#[no_mangle]`, the compiler doesn't give a warning about unused > valuable though. > > Is there a nice way to handle such case? Maybe just something like the following? #[cfg(not(MODULE))] const _: [::kernel::bindings::mdio_device_id; 2] = [ /* .. */ ]; - Trevor
On Fri, Sep 20, 2024 at 3:53 PM FUJITA Tomonori <fujita.tomonori@gmail.com> wrote: > > Hi, > > On Thu, 19 Sep 2024 08:17:42 +0200 > Alice Ryhl <aliceryhl@google.com> wrote: > > > On Thu, Sep 19, 2024 at 6:39 AM FUJITA Tomonori > > <fujita.tomonori@gmail.com> wrote: > >> > >> Fix the following warning when the driver is compiled as built-in: > >> > >> >> warning: unused import: `DeviceId` > >> --> drivers/net/phy/qt2025.rs:18:5 > >> | > >> 18 | DeviceId, Driver, > >> | ^^^^^^^^ > >> | > >> = note: `#[warn(unused_imports)]` on by default > >> > >> device_table in module_phy_driver macro is defined only when the > >> driver is built as module. Use an absolute module path in the macro > >> instead of importing `DeviceId`. > >> > >> Reported-by: kernel test robot <lkp@intel.com> > >> Closes: https://lore.kernel.org/oe-kbuild-all/202409190717.i135rfVo-lkp@intel.com/ > >> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> > > > > It may be nice to change the macro to always use the expression so > > that this warning doesn't happen again. > > Like the C code does, a valuable is defined only when the driver is > built as module because the valuable is used to create the information > for module loading. So the macro adds `#[cfg(MODULE)]` like the > following: > > #[cfg(MODULE)] > #[no_mangle] > static __mod_mdio__phydev_device_table: [::kernel::bindings::mdio_device_id; 2] = [ > ::kernel::bindings::mdio_device_id { > phy_id: 0x00000001, > phy_id_mask: 0xffffffff, > }, > ::kernel::bindings::mdio_device_id { > phy_id: 0, > phy_id_mask: 0, > }, > ]; > > We can remove `#[cfg(MODULE)]` however an unused valuable to added to > the kernel image when the driver is compiled as built-in. Seems that > with `#[no_mangle]`, the compiler doesn't give a warning about unused > valuable though. > > Is there a nice way to handle such case? Put it in a const. That way it doesn't end up in the image if unused. const _TABLE_INIT: [::kernel::bindings::mdio_device_id; 2] = [ ::kernel::bindings::mdio_device_id { phy_id: 0x00000001, phy_id_mask: 0xffffffff, }, ::kernel::bindings::mdio_device_id { phy_id: 0, phy_id_mask: 0, }, ]; #[cfg(MODULE)] #[no_mangle] static __mod_mdio__phydev_device_table: [::kernel::bindings::mdio_device_id; 2] = _TABLE_INIT; Alice
On Fri, 20 Sep 2024 18:00:56 +0200 Alice Ryhl <aliceryhl@google.com> wrote: > Put it in a const. That way it doesn't end up in the image if unused. > > const _TABLE_INIT: [::kernel::bindings::mdio_device_id; 2] = [ > ::kernel::bindings::mdio_device_id { > phy_id: 0x00000001, > phy_id_mask: 0xffffffff, > }, > ::kernel::bindings::mdio_device_id { > phy_id: 0, > phy_id_mask: 0, > }, > ]; > > #[cfg(MODULE)] > #[no_mangle] > static __mod_mdio__phydev_device_table: > [::kernel::bindings::mdio_device_id; 2] = _TABLE_INIT; Yeah, `const` works! Thanks a lot guys! I'll send this change for the next merge window.
On Fri, 20 Sep 2024 17:42:10 +0200 Trevor Gross <tmgross@umich.edu> wrote: > On Thu, Sep 19, 2024 at 6:39 AM FUJITA Tomonori > <fujita.tomonori@gmail.com> wrote: >> >> Fix the following warning when the driver is compiled as built-in: >> >> >> warning: unused import: `DeviceId` >> --> drivers/net/phy/qt2025.rs:18:5 >> | >> 18 | DeviceId, Driver, >> | ^^^^^^^^ >> | >> = note: `#[warn(unused_imports)]` on by default > > The >> shows up as a quote on lore. Should this entire block be indented? Ah, I just copy and paste the original mail without thinking. I'll drop ">>". >> device_table in module_phy_driver macro is defined only when the >> driver is built as module. Use an absolute module path in the macro > > nit: "as module" -> "as a module" Oops, I'll update the commit log and send v2. >> instead of importing `DeviceId`. >> >> Reported-by: kernel test robot <lkp@intel.com> >> Closes: https://lore.kernel.org/oe-kbuild-all/202409190717.i135rfVo-lkp@intel.com/ >> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> >> --- > > Easy enough fix, thanks for being on top of it. > > Reviewed-by: Trevor Gross <tmgross@umich.edu> Thanks!
diff --git a/drivers/net/phy/qt2025.rs b/drivers/net/phy/qt2025.rs index 28d8981f410b..1ab065798175 100644 --- a/drivers/net/phy/qt2025.rs +++ b/drivers/net/phy/qt2025.rs @@ -15,7 +15,7 @@ use kernel::net::phy::{ self, reg::{Mmd, C45}, - DeviceId, Driver, + Driver, }; use kernel::prelude::*; use kernel::sizes::{SZ_16K, SZ_8K}; @@ -23,7 +23,7 @@ kernel::module_phy_driver! { drivers: [PhyQT2025], device_table: [ - DeviceId::new_with_driver::<PhyQT2025>(), + phy::DeviceId::new_with_driver::<PhyQT2025>(), ], name: "qt2025_phy", author: "FUJITA Tomonori <fujita.tomonori@gmail.com>",