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
 
 /*
  * run_descriptor_deco0 - runs a descriptor on DECO0, under direct control of
-- 
2.43.0

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