| Message ID | 20240702185557.3699991-2-leitao@debian.org |
|---|---|
| State | Accepted |
| Commit | 9b5c33b1a3b7593c0e1d44d92c0093cc44e635fe |
| Headers | show |
| Series | crypto: caam: Unembed net_dev | expand |
On 7/2/2024 9:56 PM, Breno Leitao wrote: > If caam module is built without OF support, the compiler returns the > following warning: > > drivers/crypto/caam/ctrl.c:83:34: warning: 'imx8m_machine_match' defined but not used [-Wunused-const-variable=] > > imx8m_machine_match is only referenced by of_match_node(), which is set > to NULL if CONFIG_OF is not set, as of commit 5762c20593b6b ("dt: Add > empty of_match_node() macro"): > > #define of_match_node(_matches, _node) NULL > > Do not create imx8m_machine_match if CONFIG_OF is not set. > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202407011309.cpTuOGdg-lkp@intel.com/ > Suggested-by: Jakub Kicinski <kuba@kernel.org> > Signed-off-by: Breno Leitao <leitao@debian.org> > --- > drivers/crypto/caam/ctrl.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c > index bd418dea586d..d4b39184dbdb 100644 > --- a/drivers/crypto/caam/ctrl.c > +++ b/drivers/crypto/caam/ctrl.c > @@ -80,6 +80,7 @@ static void build_deinstantiation_desc(u32 *desc, int handle) > append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT); > } > > +#ifdef CONFIG_OF > static const struct of_device_id imx8m_machine_match[] = { > { .compatible = "fsl,imx8mm", }, > { .compatible = "fsl,imx8mn", }, > @@ -88,6 +89,7 @@ static const struct of_device_id imx8m_machine_match[] = { > { .compatible = "fsl,imx8ulp", }, > { } > }; > +#endif > Shouldn't using __maybe_unused instead of the ifdeffery be preferred in this case? I know my comment comes late (patch has been merged to net-next), sorry for this. Still I'd like to clarify this for future. Thanks, Horia
Hello Horia, On Fri, Jul 05, 2024 at 10:11:40AM +0000, Horia Geanta wrote: > On 7/2/2024 9:56 PM, Breno Leitao wrote: > > diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c > > index bd418dea586d..d4b39184dbdb 100644 > > --- a/drivers/crypto/caam/ctrl.c > > +++ b/drivers/crypto/caam/ctrl.c > > @@ -80,6 +80,7 @@ static void build_deinstantiation_desc(u32 *desc, int handle) > > append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT); > > } > > > > +#ifdef CONFIG_OF > > static const struct of_device_id imx8m_machine_match[] = { > > { .compatible = "fsl,imx8mm", }, > > { .compatible = "fsl,imx8mn", }, > > @@ -88,6 +89,7 @@ static const struct of_device_id imx8m_machine_match[] = { > > { .compatible = "fsl,imx8ulp", }, > > { } > > }; > > +#endif > Shouldn't using __maybe_unused instead of the ifdeffery be preferred > in this case? That is an option as well. Not sure if it makes any difference, tho. If you prefer __maybe_unused, I am more than happy to send a follow-up patch to convert the #ifdef to __maybe_unused. Up to you. Thanks!
On 7/8/2024 2:29 PM, Breno Leitao wrote: > Hello Horia, > > On Fri, Jul 05, 2024 at 10:11:40AM +0000, Horia Geanta wrote: >> On 7/2/2024 9:56 PM, Breno Leitao wrote: > >>> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c >>> index bd418dea586d..d4b39184dbdb 100644 >>> --- a/drivers/crypto/caam/ctrl.c >>> +++ b/drivers/crypto/caam/ctrl.c >>> @@ -80,6 +80,7 @@ static void build_deinstantiation_desc(u32 *desc, int handle) >>> append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT); >>> } >>> >>> +#ifdef CONFIG_OF >>> static const struct of_device_id imx8m_machine_match[] = { >>> { .compatible = "fsl,imx8mm", }, >>> { .compatible = "fsl,imx8mn", }, >>> @@ -88,6 +89,7 @@ static const struct of_device_id imx8m_machine_match[] = { >>> { .compatible = "fsl,imx8ulp", }, >>> { } >>> }; >>> +#endif > >> Shouldn't using __maybe_unused instead of the ifdeffery be preferred >> in this case? > > That is an option as well. Not sure if it makes any difference, tho. > In general, I prefer avoiding preprocessor conditionals. This seems to be suggested also here: https://www.kernel.org/doc/html/latest/process/coding-style.html#conditional-compilation > If you prefer __maybe_unused, I am more than happy to send a follow-up > patch to convert the #ifdef to __maybe_unused. Up to you. > Nah, please don't bother. Thanks, Horia
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index bd418dea586d..d4b39184dbdb 100644 --- a/drivers/crypto/caam/ctrl.c +++ b/drivers/crypto/caam/ctrl.c @@ -80,6 +80,7 @@ static void build_deinstantiation_desc(u32 *desc, int handle) append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT); } +#ifdef CONFIG_OF static const struct of_device_id imx8m_machine_match[] = { { .compatible = "fsl,imx8mm", }, { .compatible = "fsl,imx8mn", }, @@ -88,6 +89,7 @@ static const struct of_device_id imx8m_machine_match[] = { { .compatible = "fsl,imx8ulp", }, { } }; +#endif /* * run_descriptor_deco0 - runs a descriptor on DECO0, under direct control of
If caam module is built without OF support, the compiler returns the following warning: drivers/crypto/caam/ctrl.c:83:34: warning: 'imx8m_machine_match' defined but not used [-Wunused-const-variable=] imx8m_machine_match is only referenced by of_match_node(), which is set to NULL if CONFIG_OF is not set, as of commit 5762c20593b6b ("dt: Add empty of_match_node() macro"): #define of_match_node(_matches, _node) NULL Do not create imx8m_machine_match if CONFIG_OF is not set. Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202407011309.cpTuOGdg-lkp@intel.com/ Suggested-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Breno Leitao <leitao@debian.org> --- drivers/crypto/caam/ctrl.c | 2 ++ 1 file changed, 2 insertions(+)