Re: [PATCH v2 4/8] board: ti: j721e: Add resume detection for J7200

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 1896 bytes --]

On Tue, Nov 07, 2023 at 05:17:58PM +0100, Thomas Richard wrote:

> Add the capability to detect a resume.
> To detect the resume, SPL searches a magic value (0xBA) in a register
> of PMICA.
> This value is set by DM-Firmware during the suspend sequence.
> 
> Based on the work of Gregory CLEMENT <gregory.clement@bootlin.com>
> 
> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> ---
> 
> (no changes since v1)
> 
>  board/ti/j721e/evm.c | 54 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/board/ti/j721e/evm.c b/board/ti/j721e/evm.c
> index 38fe447d8f..b4b94c8c69 100644
> --- a/board/ti/j721e/evm.c
> +++ b/board/ti/j721e/evm.c
> @@ -22,6 +22,9 @@
>  #include <spl.h>
>  #include <dm.h>
>  #include <dm/uclass-internal.h>
> +#if (IS_ENABLED(CONFIG_SPL_BUILD) && IS_ENABLED(CONFIG_TARGET_J7200_R5_EVM))
> +#include <power/pmic.h>
> +#endif

We _really_ should not be guarding include files. If the code doesn't
compile with the header included, we need to figure out what's wrong
with the header or if, ugh, a more worst case of moving the #include to
by the now-guarded functions.

[snip]
> +#if (IS_ENABLED(CONFIG_SPL_BUILD) && IS_ENABLED(CONFIG_TARGET_J7200_R5_EVM))
> +
> +#define SCRATCH_PAD_REG_3 0xCB
> +
> +#define MAGIC_SUSPEND 0xBA
> +
> +static int resuming = -1;
> +
> +int board_is_resuming(void)

I wonder if we should (a) have a file for just r5 related code and then
(b) rely on this being discarded on link anyhow for non-SPL and we just
comment that this is only used in resume in SPL. Guarding functions with
#if (IS_ENABLED(CONFIG_A) && IS_ENABLED(CONFIG_B))
isn't what the macros were intended for (and yes, I know checkpatch.pl
complains otherwise, which is a point of contention).

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]