Re: [PATCH v4 04/10] board: lx2160a: serdes: report unknown SD1 cases

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Mon, Sep 28, 2026 at 12:13:23PM +0200, Vincent Jardin wrote:
> Check and if unknown display:
>   lx2160a serdes: unknown hwconfig serdes1:split, ignored
> 
> Signed-off-by: Vincent Jardin <vjardin@free.fr>
> 
> ---
> 
> Changes in v4:
> - New patch
> 
>  board/nxp/lx2160a/serdes_split.c | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/board/nxp/lx2160a/serdes_split.c b/board/nxp/lx2160a/serdes_split.c
> index 11bec35fd22..a8fbb278b3a 100644
> --- a/board/nxp/lx2160a/serdes_split.c
> +++ b/board/nxp/lx2160a/serdes_split.c
> @@ -443,17 +443,23 @@ static const struct sd1_mode sd1_modes[] = {
>  	},
>  };
>  
> -static const struct sd1_mode *sd1_mode_find(const char *name)
> +static const struct sd1_mode *sd1_mode_findn(const char *name, size_t n)
>  {
>  	int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(sd1_modes); i++)
> -		if (!strcmp(sd1_modes[i].name, name))
> +		if (strlen(sd1_modes[i].name) == n &&
> +		    !strncmp(sd1_modes[i].name, name, n))
>  			return &sd1_modes[i];
>  
>  	return NULL;
>  }
>  
> +static const struct sd1_mode *sd1_mode_find(const char *name)
> +{
> +	return sd1_mode_findn(name, strlen(name));
> +}

Hmm, there's a single other caller of sd1_mode_find(), from sd1_enter().
Is it really worth it to implement a shim over sd1_mode_findn()?
I don't think so.

Can you please plan accordingly and modify sd1_mode_find() to take the
function prototype of sd1_mode_find() from the get go?

> +
>  static void sd1_print_pcc(unsigned long base)
>  {
>  	printf("PCCC=%08x PCCD=%08x PCCE=%08x",
> @@ -503,6 +509,26 @@ static int sd1_enter(const struct sd1_mode *mode,
>  	return 0;
>  }
>  
> +static void sd1_report_unknown(void)
> +{
> +	const char *arg, *comma, *eq;
> +	size_t len, n;
> +
> +	arg = hwconfig_arg("serdes1", &len);
> +	while (arg && len) {
> +		comma = memchr(arg, ',', len);
> +		n = comma ? comma - arg : len;
> +		eq = memchr(arg, '=', n);
> +		if (n && !sd1_mode_findn(arg, eq ? eq - arg : n))
> +			printf("lx2160a serdes: unknown hwconfig serdes1:%.*s, ignored\n",
> +			       (int)n, arg);
> +		if (!comma)
> +			break;
> +		len -= n + 1;
> +		arg = comma + 1;

What is the point of this manual hand-rolling of sub-options?

As far as I can read from doc/README.fsl-hwconfig, the serdes1 syntax is
A: 'serdes1:2x100g[;other-options:other-values]', not
B: 'serdes1:suboption=value,suboption2=other_value[;other-options:other-values]'.

To parse syntax A, you don't need any comma or equal detection.
The 'len' provided by hwconfig_arg() should end at the next semicolon
separator already, or at the end of the string. You should be able to
print that directly. Or if not, you haven't explained the need for all
this complexity.

> +	}
> +}
> +
>  /*
>   *   setenv hwconfig serdes1:<mode>
>   *
> @@ -517,6 +543,8 @@ void lx2160a_serdes_apply_hwconfig(void)
>  	const struct sd1_mode *mode = NULL;
>  	int i;
>  
> +	sd1_report_unknown();
> +
>  	for (i = 0; i < ARRAY_SIZE(sd1_modes); i++) {
>  		if (!hwconfig_sub("serdes1", sd1_modes[i].name))
>  			continue;
> -- 
> 2.43.0
>