diff mbox series

[edk2,v2,3/5] ArmVirtPkg/FdtClientDxe: take DT node 'status' properties into account

Message ID 20181121115828.3026-4-ard.biesheuvel@linaro.org
State Accepted
Commit 32f5975770c309723ebb17642f89bbf9670955fd
Headers show
Series ArmPlatformPkg, ArmVirtPkg: discover NOR flash banks from DTB | expand

Commit Message

Ard Biesheuvel Nov. 21, 2018, 11:58 a.m. UTC 
DT has a [pseudo-]standardized 'status' property that can be set on
any node, and which signifies that a node should be treated as
absent unless it is set to 'ok' or 'okay'. So take this into account
when iterating over nodes.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.c | 38 +++++++++++++++++---
 1 file changed, 33 insertions(+), 5 deletions(-)

-- 
2.17.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek Nov. 21, 2018, 5:12 p.m. UTC | #1 
On 11/21/18 12:58, Ard Biesheuvel wrote:
> DT has a [pseudo-]standardized 'status' property that can be set on

> any node, and which signifies that a node should be treated as

> absent unless it is set to 'ok' or 'okay'.


(I now really want "oktopus" to be [pseudo-]standardized too! ;) /jk)

> So take this into account

> when iterating over nodes.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c | 38 +++++++++++++++++---

>  1 file changed, 33 insertions(+), 5 deletions(-)

> 

> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c

> index fb6e0aeb9215..5bfde381ecd0 100644

> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c

> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c

> @@ -78,6 +78,33 @@ SetNodeProperty (

>    return EFI_SUCCESS;

>  }

>  

> +STATIC

> +BOOLEAN

> +IsNodeEnabled (

> +  INT32                       Node

> +  )

> +{

> +  CONST CHAR8   *NodeStatus;

> +  INT32         Len;

> +

> +  //

> +  // A missing status property implies 'ok' so ignore any errors that

> +  // may occur here. If the status property is present, check whether

> +  // it is set to 'ok' or 'okay', anything else is treated as 'disabled'.

> +  //

> +  NodeStatus = fdt_getprop (mDeviceTreeBase, Node, "status", &Len);

> +  if (NodeStatus == NULL) {

> +    return TRUE;

> +  }

> +  if (Len >= 5 && AsciiStrCmp (NodeStatus, "okay") == 0) {

> +    return TRUE;

> +  }

> +  if (Len >= 3 && AsciiStrCmp (NodeStatus, "ok") == 0) {

> +    return TRUE;

> +  }

> +  return FALSE;

> +}

> +

>  STATIC

>  EFI_STATUS

>  EFIAPI

> @@ -101,6 +128,10 @@ FindNextCompatibleNode (

>        break;

>      }

>  

> +    if (!IsNodeEnabled (Next)) {

> +      continue;

> +    }

> +

>      Type = fdt_getprop (mDeviceTreeBase, Next, "compatible", &Len);

>      if (Type == NULL) {

>        continue;

> @@ -210,7 +241,6 @@ FindNextMemoryNodeReg (

>  {

>    INT32          Prev, Next;

>    CONST CHAR8    *DeviceType;

> -  CONST CHAR8    *NodeStatus;

>    INT32          Len;

>    EFI_STATUS     Status;

>  

> @@ -223,10 +253,8 @@ FindNextMemoryNodeReg (

>        break;

>      }

>  

> -    NodeStatus = fdt_getprop (mDeviceTreeBase, Next, "status", &Len);

> -    if (NodeStatus != NULL && AsciiStrCmp (NodeStatus, "okay") != 0) {

> -      DEBUG ((DEBUG_WARN, "%a: ignoring memory node with status \"%a\"\n",

> -        __FUNCTION__, NodeStatus));

> +    if (!IsNodeEnabled (Next)) {

> +      DEBUG ((DEBUG_WARN, "%a: ignoring disabled memory node\n", __FUNCTION__));

>        continue;

>      }

>  

> 


Reviewed-by: Laszlo Ersek <lersek@redhat.com>


Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Nov. 22, 2018, 1:12 p.m. UTC | #2 
On Wed, 21 Nov 2018 at 18:12, Laszlo Ersek <lersek@redhat.com> wrote:
>

> On 11/21/18 12:58, Ard Biesheuvel wrote:

> > DT has a [pseudo-]standardized 'status' property that can be set on

> > any node, and which signifies that a node should be treated as

> > absent unless it is set to 'ok' or 'okay'.

>

> (I now really want "oktopus" to be [pseudo-]standardized too! ;) /jk)

>


Be careful what you wish for :-)

> > So take this into account

> > when iterating over nodes.

> >

> > Contributed-under: TianoCore Contribution Agreement 1.1

> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> > ---

> >  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c | 38 +++++++++++++++++---

> >  1 file changed, 33 insertions(+), 5 deletions(-)

> >

> > diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c

> > index fb6e0aeb9215..5bfde381ecd0 100644

> > --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c

> > +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c

> > @@ -78,6 +78,33 @@ SetNodeProperty (

> >    return EFI_SUCCESS;

> >  }

> >

> > +STATIC

> > +BOOLEAN

> > +IsNodeEnabled (

> > +  INT32                       Node

> > +  )

> > +{

> > +  CONST CHAR8   *NodeStatus;

> > +  INT32         Len;

> > +

> > +  //

> > +  // A missing status property implies 'ok' so ignore any errors that

> > +  // may occur here. If the status property is present, check whether

> > +  // it is set to 'ok' or 'okay', anything else is treated as 'disabled'.

> > +  //

> > +  NodeStatus = fdt_getprop (mDeviceTreeBase, Node, "status", &Len);

> > +  if (NodeStatus == NULL) {

> > +    return TRUE;

> > +  }

> > +  if (Len >= 5 && AsciiStrCmp (NodeStatus, "okay") == 0) {

> > +    return TRUE;

> > +  }

> > +  if (Len >= 3 && AsciiStrCmp (NodeStatus, "ok") == 0) {

> > +    return TRUE;

> > +  }

> > +  return FALSE;

> > +}

> > +

> >  STATIC

> >  EFI_STATUS

> >  EFIAPI

> > @@ -101,6 +128,10 @@ FindNextCompatibleNode (

> >        break;

> >      }

> >

> > +    if (!IsNodeEnabled (Next)) {

> > +      continue;

> > +    }

> > +

> >      Type = fdt_getprop (mDeviceTreeBase, Next, "compatible", &Len);

> >      if (Type == NULL) {

> >        continue;

> > @@ -210,7 +241,6 @@ FindNextMemoryNodeReg (

> >  {

> >    INT32          Prev, Next;

> >    CONST CHAR8    *DeviceType;

> > -  CONST CHAR8    *NodeStatus;

> >    INT32          Len;

> >    EFI_STATUS     Status;

> >

> > @@ -223,10 +253,8 @@ FindNextMemoryNodeReg (

> >        break;

> >      }

> >

> > -    NodeStatus = fdt_getprop (mDeviceTreeBase, Next, "status", &Len);

> > -    if (NodeStatus != NULL && AsciiStrCmp (NodeStatus, "okay") != 0) {

> > -      DEBUG ((DEBUG_WARN, "%a: ignoring memory node with status \"%a\"\n",

> > -        __FUNCTION__, NodeStatus));

> > +    if (!IsNodeEnabled (Next)) {

> > +      DEBUG ((DEBUG_WARN, "%a: ignoring disabled memory node\n", __FUNCTION__));

> >        continue;

> >      }

> >

> >

>

> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

>


Thanks.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Philippe Mathieu-Daudé Nov. 22, 2018, 3:38 p.m. UTC | #3 
On 21/11/18 12:58, Ard Biesheuvel wrote:
> DT has a [pseudo-]standardized 'status' property that can be set on
> any node, and which signifies that a node should be treated as
> absent unless it is set to 'ok' or 'okay'. So take this into account
> when iterating over nodes.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c | 38 +++++++++++++++++---
>  1 file changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> index fb6e0aeb9215..5bfde381ecd0 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> @@ -78,6 +78,33 @@ SetNodeProperty (
>    return EFI_SUCCESS;
>  }
>  
> +STATIC
> +BOOLEAN
> +IsNodeEnabled (
> +  INT32                       Node
> +  )
> +{
> +  CONST CHAR8   *NodeStatus;
> +  INT32         Len;
> +
> +  //
> +  // A missing status property implies 'ok' so ignore any errors that
> +  // may occur here. If the status property is present, check whether
> +  // it is set to 'ok' or 'okay', anything else is treated as 'disabled'.
> +  //
> +  NodeStatus = fdt_getprop (mDeviceTreeBase, Node, "status", &Len);
> +  if (NodeStatus == NULL) {
> +    return TRUE;
> +  }
> +  if (Len >= 5 && AsciiStrCmp (NodeStatus, "okay") == 0) {
> +    return TRUE;
> +  }
> +  if (Len >= 3 && AsciiStrCmp (NodeStatus, "ok") == 0) {
> +    return TRUE;
> +  }
> +  return FALSE;
> +}
> +
>  STATIC
>  EFI_STATUS
>  EFIAPI
> @@ -101,6 +128,10 @@ FindNextCompatibleNode (
>        break;
>      }
>  
> +    if (!IsNodeEnabled (Next)) {
> +      continue;
> +    }
> +
>      Type = fdt_getprop (mDeviceTreeBase, Next, "compatible", &Len);
>      if (Type == NULL) {
>        continue;
> @@ -210,7 +241,6 @@ FindNextMemoryNodeReg (
>  {
>    INT32          Prev, Next;
>    CONST CHAR8    *DeviceType;
> -  CONST CHAR8    *NodeStatus;
>    INT32          Len;
>    EFI_STATUS     Status;
>  
> @@ -223,10 +253,8 @@ FindNextMemoryNodeReg (
>        break;
>      }
>  
> -    NodeStatus = fdt_getprop (mDeviceTreeBase, Next, "status", &Len);
> -    if (NodeStatus != NULL && AsciiStrCmp (NodeStatus, "okay") != 0) {
> -      DEBUG ((DEBUG_WARN, "%a: ignoring memory node with status \"%a\"\n",
> -        __FUNCTION__, NodeStatus));
> +    if (!IsNodeEnabled (Next)) {
> +      DEBUG ((DEBUG_WARN, "%a: ignoring disabled memory node\n", __FUNCTION__));
>        continue;
>      }
>  
>
diff mbox series

Patch

diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
index fb6e0aeb9215..5bfde381ecd0 100644
--- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
+++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
@@ -78,6 +78,33 @@  SetNodeProperty (
   return EFI_SUCCESS;
 }
 
+STATIC
+BOOLEAN
+IsNodeEnabled (
+  INT32                       Node
+  )
+{
+  CONST CHAR8   *NodeStatus;
+  INT32         Len;
+
+  //
+  // A missing status property implies 'ok' so ignore any errors that
+  // may occur here. If the status property is present, check whether
+  // it is set to 'ok' or 'okay', anything else is treated as 'disabled'.
+  //
+  NodeStatus = fdt_getprop (mDeviceTreeBase, Node, "status", &Len);
+  if (NodeStatus == NULL) {
+    return TRUE;
+  }
+  if (Len >= 5 && AsciiStrCmp (NodeStatus, "okay") == 0) {
+    return TRUE;
+  }
+  if (Len >= 3 && AsciiStrCmp (NodeStatus, "ok") == 0) {
+    return TRUE;
+  }
+  return FALSE;
+}
+
 STATIC
 EFI_STATUS
 EFIAPI
@@ -101,6 +128,10 @@  FindNextCompatibleNode (
       break;
     }
 
+    if (!IsNodeEnabled (Next)) {
+      continue;
+    }
+
     Type = fdt_getprop (mDeviceTreeBase, Next, "compatible", &Len);
     if (Type == NULL) {
       continue;
@@ -210,7 +241,6 @@  FindNextMemoryNodeReg (
 {
   INT32          Prev, Next;
   CONST CHAR8    *DeviceType;
-  CONST CHAR8    *NodeStatus;
   INT32          Len;
   EFI_STATUS     Status;
 
@@ -223,10 +253,8 @@  FindNextMemoryNodeReg (
       break;
     }
 
-    NodeStatus = fdt_getprop (mDeviceTreeBase, Next, "status", &Len);
-    if (NodeStatus != NULL && AsciiStrCmp (NodeStatus, "okay") != 0) {
-      DEBUG ((DEBUG_WARN, "%a: ignoring memory node with status \"%a\"\n",
-        __FUNCTION__, NodeStatus));
+    if (!IsNodeEnabled (Next)) {
+      DEBUG ((DEBUG_WARN, "%a: ignoring disabled memory node\n", __FUNCTION__));
       continue;
     }