[PATCH] fru: ops: Display FRU fields properly for 0xc1 fields

[Michal Simek]

On 1/25/23 13:06, Michal Simek wrote:
> From: Algapally Santosh Sagar <santoshsagar.algapally at amd.com>
> 
> FRU data is not displayed properly in case of 0xc1 fields.
> The 0xC1 can be used in two cases.
> 1. Char record type 8-bit ASCII + Latin 1 with length of 1.
> (For example board revision 'A')
> 
> 2. C1h (type/length byte encoded to indicate no more info fields).
> which can follow by 00h to fill all remaining unused space
> 
> Hence removed the check end-of-the field c1 to allow c1 fields.
> 
> "ASCII+LATIN1" is defined as the printable characters from the
> first set of 256 characters of Unicode 6.2 (U+0000h through U+00FFh,
> inclusive) expressed as an eight-bit value. (Unicode follows ISO/IEC
> 8859-1 in the layout of printable characters up to U+00FFh).
> 
> So, print only printable chars and limit range from 0x20 ' ' to 0x7e '-'
> which will be also indication if 0xc1 behaves as record with one char or
> end of record.
> 
> Signed-off-by: Algapally Santosh Sagar <santoshsagar.algapally at amd.com>
> Signed-off-by: Michal Simek <michal.simek at amd.com>
> ---
> 
>   board/xilinx/common/fru_ops.c | 43 +++++++++++++++++++++++------------
>   1 file changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/board/xilinx/common/fru_ops.c b/board/xilinx/common/fru_ops.c
> index c4f009affc5e..167252c240cd 100644
> --- a/board/xilinx/common/fru_ops.c
> +++ b/board/xilinx/common/fru_ops.c
> @@ -1,6 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0
>   /*
> - * (C) Copyright 2019 - 2020 Xilinx, Inc.
> + * (C) Copyright 2019 - 2022, Xilinx, Inc.
> + * (C) Copyright 2022 - 2023, Advanced Micro Devices, Inc.
>    */
>   
>   #include <common.h>
> @@ -61,9 +62,6 @@ static int fru_check_type_len(u8 type_len, u8 language, u8 *type)
>   {
>   	int len;
>   
> -	if (type_len == FRU_TYPELEN_EOF)
> -		return -EINVAL;
> -
>   	*type = (type_len & FRU_TYPELEN_CODE_MASK) >> FRU_TYPELEN_TYPE_SHIFT;
>   
>   	len = type_len & FRU_TYPELEN_LEN_MASK;
> @@ -172,9 +170,16 @@ static int fru_parse_board(unsigned long addr)
>   {
>   	u8 i, type;
>   	int len;
> -	u8 *data, *term, *limit;
> +	u8 *data, *term, *limit, *next_addr, *eof;
>   
>   	memcpy(&fru_data.brd.ver, (void *)addr, 6);
> +
> +	/*
> +	 * eof marks the last data byte (without checksum). That's why checksum
> +	 * is address length - 1 and last data byte is length - 2.
> +	 */
> +	eof = (u8 *)(fru_data.brd.len * 8 + addr - 2);
> +
>   	addr += 6;
>   	data = (u8 *)&fru_data.brd.manufacturer_type_len;
>   
> @@ -184,10 +189,21 @@ static int fru_parse_board(unsigned long addr)
>   	for (i = 0; ; i++, data += FRU_BOARD_MAX_LEN) {
>   		len = fru_check_type_len(*(u8 *)addr, fru_data.brd.lang_code,
>   					 &type);
> +		next_addr = (u8 *)addr + 1;
> +
> +		if ((u8 *)addr >= eof) {
> +			debug("Reach EOF record: addr %lx, eof %lx\n", addr,
> +			      (unsigned long)eof);
> +			break;
> +		}
> +
>   		/*
> -		 * Stop cature if it end of fields
> +		 * Stop capture if the type is ASCII and valid field length
> +		 * is 1 (0xc1) and next FRU data is less than 0x20 (space " ")
> +		 * or it is 0x7f (delete 'DEL').
>   		 */
> -		if (len == -EINVAL)
> +		if (type == FRU_TYPELEN_TYPE_ASCII8 && len == 1	&&
> +		    (*next_addr < 0x20 || *next_addr == 0x7F))
>   			break;
>   
>   		/* Stop when amount of chars is more then fields to record */
> @@ -332,9 +348,11 @@ static int fru_display_board(struct fru_board_data *brd, int verbose)
>   	for (u8 i = 0; i < (sizeof(boardinfo) / sizeof(*boardinfo)); i++) {
>   		len = fru_check_type_len(*data++, brd->lang_code,
>   					 &type);
> -		if (len == -EINVAL) {
> -			printf("**** EOF for Board Area ****\n");
> -			break;
> +
> +		/* Empty record has no len/type filled */
> +		if (!len) {
> +			debug("%s not found\n", boardinfo[i]);
> +			continue;
>   		}
>   
>   		if (type <= FRU_TYPELEN_TYPE_ASCII8 &&
> @@ -344,11 +362,6 @@ static int fru_display_board(struct fru_board_data *brd, int verbose)
>   		else
>   			debug("Type code: %s\n", typecode[type + 1]);
>   
> -		if (!len) {
> -			debug("%s not found\n", boardinfo[i]);
> -			continue;
> -		}
> -
>   		switch (type) {
>   		case FRU_TYPELEN_TYPE_BINARY:
>   			debug("Length: %d\n", len);

Applied.
M