* Re: [PATCH v2] MdePkg: Support FDT library.
2023-04-12 15:30 [PATCH v2] MdePkg: Support FDT library Benny Lin
@ 2023-04-12 16:22 ` Michael D Kinney
0 siblings, 0 replies; 2+ messages in thread
From: Michael D Kinney @ 2023-04-12 16:22 UTC (permalink / raw)
To: Lin, Benny, devel@edk2.groups.io
Cc: Gao, Liming, Liu, Zhiguang, Chiu, Chasel, Lu, James, Guo, Gua,
Pedro Falcato, Kinney, Michael D
Hi Benny,
Your V1 series had patches 0,1,2. Why was this merged into 1 patch?
Or is this a patch on top of the V1 series?
Please resend V2 as entire patch series with all changes merged in and
please respond to all individual emails with feedback and which items
you are addressing in V2 and which items need further discussion.
Mike
> -----Original Message-----
> From: Lin, Benny <benny.lin@intel.com>
> Sent: Wednesday, April 12, 2023 8:30 AM
> To: devel@edk2.groups.io
> Cc: Lin, Benny <benny.lin@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>;
> Liu, Zhiguang <zhiguang.liu@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Lu, James <james.lu@intel.com>; Guo, Gua
> <gua.guo@intel.com>; Pedro Falcato <pedro.falcato@gmail.com>
> Subject: [PATCH v2] MdePkg: Support FDT library.
>
> From: Benny Lin <benny.lin@intel.com>
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4392
> Add FDT support in EDK2 by submodule 3rd party libfdt
> (https://github.com/devicetree-org/pylibfdt/tree/main/libfdt)
> 1. Correct the typo.
> 2. Remove no use definitions from LibFdtSupport.h.
> 3. Refer to LibcLib implementation by Pedro.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: James Lu <james.lu@intel.com>
> Cc: Gua Guo <gua.guo@intel.com>
> Cc: Pedro Falcato <pedro.falcato@gmail.com>
> Signed-off-by: Benny Lin <benny.lin@intel.com>
> ---
> MdePkg/Include/Library/FdtLib.h | 2 +-
> MdePkg/Library/BaseFdtLib/FdtLib.c | 2 +-
> MdePkg/Library/BaseFdtLib/LibFdtSupport.h | 6 +-
> MdePkg/Library/BaseFdtLib/LibFdtWrapper.c | 215 +++++++++++---------
> 4 files changed, 125 insertions(+), 100 deletions(-)
>
> diff --git a/MdePkg/Include/Library/FdtLib.h b/MdePkg/Include/Library/FdtLib.h
> index bcb097b77e..d1e67c773f 100644
> --- a/MdePkg/Include/Library/FdtLib.h
> +++ b/MdePkg/Include/Library/FdtLib.h
> @@ -276,7 +276,7 @@ FdtAddSubnode (
> );
>
>
>
> /**
>
> - Add or modify a porperty in the given node.
>
> + Add or modify a property in the given node.
>
>
>
> @param[in] Fdt The pointer to FDT blob.
>
> @param[in] NodeOffset The offset to the node offset which want to add in.
>
> diff --git a/MdePkg/Library/BaseFdtLib/FdtLib.c b/MdePkg/Library/BaseFdtLib/FdtLib.c
> index ba9a284e58..877c832c50 100644
> --- a/MdePkg/Library/BaseFdtLib/FdtLib.c
> +++ b/MdePkg/Library/BaseFdtLib/FdtLib.c
> @@ -259,7 +259,7 @@ FdtAddSubnode (
> }
>
>
>
> /**
>
> - Add or modify a porperty in the given node.
>
> + Add or modify a property in the given node.
>
>
>
> @param[in] Fdt The pointer to FDT blob.
>
> @param[in] NodeOffset The offset to the node offset which want to add in.
>
> diff --git a/MdePkg/Library/BaseFdtLib/LibFdtSupport.h b/MdePkg/Library/BaseFdtLib/LibFdtSupport.h
> index 58b0bb403e..92d7bf0946 100644
> --- a/MdePkg/Library/BaseFdtLib/LibFdtSupport.h
> +++ b/MdePkg/Library/BaseFdtLib/LibFdtSupport.h
> @@ -29,12 +29,10 @@ typedef BOOLEAN bool;
> //
>
> // Definitions for global constants used by libfdt library routines
>
> //
>
> -#ifndef INT_MAX
>
> -#define INT_MAX 0x7FFFFFFF /* Maximum (signed) int value */
>
> -#endif
>
> +#define INT_MAX 0x7FFFFFFF /* Maximum (signed) int value */
>
> #define INT32_MAX 0x7FFFFFFF /* Maximum (signed) int32 value */
>
> #define UINT32_MAX 0xFFFFFFFF /* Maximum unsigned int32 value */
>
> -#define MAX_STRING_SIZE 0x1000
>
> +#define ULONG_MAX 0xFFFFFFFF /* Maximum unsigned long value */
>
>
>
> //
>
> // Function prototypes of libfdt Library routines
>
> diff --git a/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c b/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
> index 3f1cc69dc6..50b533a2b0 100644
> --- a/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
> +++ b/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c
> @@ -8,131 +8,158 @@
> **/
>
>
>
> #include "LibFdtSupport.h"
>
> -#include <Library/DebugLib.h>
>
>
>
> -/**
>
> - Returns the first occurrence of a Null-terminated ASCII character
>
> - in a Null-terminated ASCII string.
>
> +char *
>
> +strchr (
>
> + const char *Str,
>
> + int Char
>
> + )
>
> +{
>
> + char *S;
>
>
>
> - This function scans the contents of the ASCII string specified by s
>
> - and returns the first occurrence of c. If c is not found in s,
>
> - then NULL is returned. If the length of c is zero, then s is returned.
>
> + S = (char *)Str;
>
>
>
> - @param s The pointer to a Null-terminated ASCII string.
>
> - @param c The pointer to a Null-terminated ASCII character to search for.
>
> + for ( ; ; S++) {
>
> + if (*S == Char) {
>
> + return S;
>
> + }
>
>
>
> - @retval NULL If the c does not appear in s.
>
> - @retval others If there is a match return the first occurrence of c.
>
> - If the length of c is zero,return s.
>
> + if (*S == '\0') {
>
> + return NULL;
>
> + }
>
> + }
>
> +}
>
>
>
> -**/
>
> char *
>
> -strchr (
>
> - const char *s,
>
> - int c
>
> +strrchr (
>
> + const char *Str,
>
> + int Char
>
> )
>
> {
>
> - char pattern[2];
>
> + char *S, *last;
>
>
>
> - pattern[0] = (CHAR8)c;
>
> - pattern[1] = 0;
>
> - return AsciiStrStr (s, pattern);
>
> -}
>
> + S = (char *)Str;
>
> + last = NULL;
>
>
>
> -/**
>
> - Returns the last occurrence of a Null-terminated ASCII character
>
> - in a Null-terminated ASCII string.
>
> -
>
> - This function scans the contents of the ASCII string specified by s
>
> - and returns the last occurrence of c. If c is not found in s,
>
> - then NULL is returned. If the length of c is zero, then s is returned.
>
> + for ( ; ; S++) {
>
> + if (*S == Char) {
>
> + last = S;
>
> + }
>
>
>
> - @param s The pointer to a Null-terminated ASCII string.
>
> - @param c The pointer to a Null-terminated ASCII character to search for.
>
> + if (*S == '\0') {
>
> + return last;
>
> + }
>
> + }
>
> +}
>
>
>
> - @retval NULL If the c does not appear in s.
>
> - @retval others If there is a match return the last occurrence of c.
>
> - If the length of c is zero,return s.
>
> +STATIC
>
> +int
>
> +__isspace (
>
> + int ch
>
> + )
>
> +{
>
> + // basic ASCII ctype.h:isspace(). Not efficient
>
> + return ch == '\r' || ch == '\n' || ch == ' ' || ch == '\t' || ch == '\v' || ch == '\f';
>
> +}
>
>
>
> -**/
>
> -char *
>
> -strrchr (
>
> - const char *s,
>
> - int c
>
> +unsigned long
>
> +strtoul (
>
> + const char *Nptr,
>
> + char **EndPtr,
>
> + int Base
>
> )
>
> {
>
> - char pattern[2];
>
> - CONST CHAR8 *LastMatch;
>
> - CONST CHAR8 *StringTmp;
>
> - CONST CHAR8 *SearchString;
>
> -
>
> - pattern[0] = (CHAR8)c;
>
> - pattern[1] = 0;
>
> - SearchString = pattern;
>
> -
>
> - //
>
> - // Return NULL if both strings are less long than PcdMaximumAsciiStringLength
>
> - //
>
> - if ((AsciiStrSize (s) == 0) || (AsciiStrSize (SearchString) == 0)) {
>
> - return NULL;
>
> + BOOLEAN Negate;
>
> + BOOLEAN Overflow;
>
> + unsigned long Val;
>
> +
>
> + Negate = FALSE;
>
> + Overflow = FALSE;
>
> + Val = 0;
>
> +
>
> + // Reject bad numeric bases
>
> + if ((Base < 0) || (Base == 1) || (Base > 36)) {
>
> + return 0;
>
> }
>
>
>
> - if (*SearchString == '\0') {
>
> - return (CHAR8 *)s;
>
> + // Skip whitespace
>
> + while (__isspace (*Nptr)) {
>
> + Nptr++;
>
> }
>
>
>
> - LastMatch = NULL;
>
> - do {
>
> - StringTmp = AsciiStrStr (s, SearchString);
>
> + // Check for + or - prefixes
>
> + if (*Nptr == '-') {
>
> + Negate = TRUE;
>
> + Nptr++;
>
> + } else if (*Nptr == '+') {
>
> + Nptr++;
>
> + }
>
>
>
> - if (StringTmp == NULL) {
>
> - break;
>
> + // Consume the start, autodetecting base if needed
>
> + if ((Nptr[0] == '0') && ((Nptr[1] == 'x') || (Nptr[1] == 'X')) && ((Base == 0) || (Base == 16))) {
>
> + // Hex
>
> + Nptr += 2;
>
> + Base = 16;
>
> + } else if ((Nptr[0] == '0') && ((Nptr[1] == 'b') || (Nptr[1] == 'B')) && ((Base == 0) || (Base == 2))) {
>
> + // Binary (standard pending C23)
>
> + Nptr += 2;
>
> + Base = 2;
>
> + } else if ((Nptr[0] == '0') && ((Base == 0) || (Base == 8))) {
>
> + // Octal
>
> + Nptr++;
>
> + Base = 8;
>
> + } else {
>
> + if (Base == 0) {
>
> + // Assume decimal
>
> + Base = 10;
>
> }
>
> + }
>
>
>
> - LastMatch = StringTmp;
>
> - s = StringTmp + 1;
>
> - } while (s != NULL);
>
> + while (TRUE) {
>
> + int Digit;
>
> + char C;
>
> + unsigned long NewVal;
>
>
>
> - return (CHAR8 *)LastMatch;
>
> -}
>
> + C = *Nptr;
>
> + Digit = -1;
>
> +
>
> + if ((C >= '0') && (C <= '9')) {
>
> + Digit = C - '0';
>
> + } else if ((C >= 'a') && (C <= 'z')) {
>
> + Digit = C - 'a' + 10;
>
> + } else if ((C >= 'A') && (C <= 'Z')) {
>
> + Digit = C - 'A' + 10;
>
> + }
>
>
>
> -/**
>
> - Convert a Null-terminated Ascii decimal or hexadecimal string to a value of type UINTN.
>
> + if ((Digit == -1) || (Digit >= Base)) {
>
> + // Note that this case also handles the \0
>
> + if (EndPtr) {
>
> + *EndPtr = (char *)Nptr;
>
> + }
>
>
>
> - This function outputs a value of type UINTN by interpreting the contents of
>
> - the Ascii string specified by String as a decimal or hexadecimal number.
>
> + break;
>
> + }
>
>
>
> - @param nptr Pointer to a Null-terminated Ascii string.
>
> - @param endptr Pointer to character that stops scan.
>
> - @param base Pointer to decimal or hexadecimal.
>
> + NewVal = Val * Base + Digit;
>
>
>
> - @retval MAX_UINTN If nptr is NULL.
>
> - If endptr is NULL.
>
> - If base is not 10 or 16.
>
> - @retval others Value is translated from String.
>
> + if (NewVal < Val) {
>
> + // Overflow
>
> + Overflow = TRUE;
>
> + }
>
>
>
> -**/
>
> -unsigned long
>
> -strtoul (
>
> - const char *nptr,
>
> - char **endptr,
>
> - int base
>
> - )
>
> -{
>
> - RETURN_STATUS Status;
>
> - UINTN ReturnValue;
>
> + Val = NewVal;
>
>
>
> - if (base == 10) {
>
> - Status = AsciiStrDecimalToUintnS (nptr, endptr, &ReturnValue);
>
> - } else if (base == 16) {
>
> - Status = AsciiStrHexToUintnS (nptr, endptr, &ReturnValue);
>
> - } else {
>
> - Status = RETURN_INVALID_PARAMETER;
>
> + Nptr++;
>
> + }
>
> +
>
> + if (Negate) {
>
> + Val = -Val;
>
> }
>
>
>
> - if (RETURN_ERROR (Status)) {
>
> - return MAX_UINTN;
>
> + if (Overflow) {
>
> + Val = ULONG_MAX;
>
> }
>
>
>
> - return (unsigned long)ReturnValue;
>
> + // TODO: We're lacking errno here.
>
> + return Val;
>
> }
>
> --
> 2.39.1.windows.1
^ permalink raw reply [flat|nested] 2+ messages in thread