On Wed, Oct 18, 2023 at 01:23:13PM -0400, Sean Anderson wrote:
On 10/18/23 09:51, Heinrich Schuchardt wrote:
On 10/17/23 00:27, Simon Glass wrote:
This code is normally compiled for sifive, but sandbox can also compile it. We should not use UNIT_TEST as a synonym for SANDBOX, since it is possible to disable UNIT_TEST for sandbox.
Drop the condition since it isn't needed.
This is not what the patch does. It introduces another condition.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Sean Anderson seanga2@gmail.com
Changes in v3:
- Just drop the condition instead
include/k210/pll.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/k210/pll.h b/include/k210/pll.h index fd16a89cb203..6dd60b2eb4fc 100644 --- a/include/k210/pll.h +++ b/include/k210/pll.h @@ -13,7 +13,7 @@ struct k210_pll_config { u8 od; };
-#ifdef CONFIG_UNIT_TEST +#ifdef CONFIG_SANDBOX
We should be able to compile with and without unit tests on the MAIX boards. Why should CONFIG_SANDBOX have any relevance here?
Why don't we make k210_pll_calc_config() non-static in all cases?
U-Boot is smaller when it is static. But the forward declaration can be made unconditionally, as long as it is unused.
I think part of the problem with this patch is confusing what it's doing. The issue here is that we can (and this is good!) and do compile drivers/clk/clk_k210.c for sandbox, so it gets coverity scans and so forth. However, if we build sandbox with UNIT_TEST=n then we get an undefined reference to 'nop'. Because sandbox doesn't define nop(). This header then provides nop for sandbox. But, we can do something clearer, now that this is spelled out.