Change in libgfxinit[master]: gma registers: Implement `Success` parameter for Wait*()

[Nico Huber (Code Review)]

Hello Arthur Heymans, Matt DeVillier, Thomas Heijligen,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/libgfxinit/+/35709

to review the following change.

Change subject: gma registers: Implement `Success` parameter for Wait*()
......................................................................

gma registers: Implement `Success` parameter for Wait*()

Change-Id: Ia0e30e3467400e9d53a32c9bfc97d2d5f00df4aa
Signed-off-by: Nico Huber 
---
M common/hw-gfx-gma-registers.adb
M common/hw-gfx-gma-registers.ads
2 files changed, 82 insertions(+), 24 deletions(-)

git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/09/35709/1

diff --git a/common/hw-gfx-gma-registers.adb b/common/hw-gfx-gma-registers.adb
index 380bced..3f0d7ae 100644
--- a/common/hw-gfx-gma-registers.adb
+++ b/common/hw-gfx-gma-registers.adb
@@ -286,18 +286,20 @@

----------------------------------------------------------------------------

-   -- TODO: Should have Success parameter
+   pragma Warnings (GNATprove, Off, "unused assignment to ""Ignored_Success""");
+
    -- Wait for the bits in @Register@ indicated by @Mask@ to be of @Value@
    procedure Wait
-     (Register : Registers_Index;
-      Mask     : Word32;
-      Value    : Word32;
-      TOut_MS  : Natural := Default_Timeout_MS;
-      Verbose  : Boolean := False)
+     (Register : in     Registers_Index;
+      Mask     : in     Word32;
+      Value    : in     Word32;
+      TOut_MS  : in     Natural := Default_Timeout_MS;
+      Verbose  : in     Boolean := False;
+      Success  :    out Boolean)
    is
       Current : Word32;
       Timeout : Time.T;
-      Timed_Out : Boolean;
+      Timed_Out : Boolean := False;
    begin
       pragma Debug (Debug.Put (GNAT.Source_Info.Enclosing_Entity & ":  "));
       pragma Debug (Debug.Put_Word32 (Value));
@@ -310,43 +312,80 @@

Timeout := Time.MS_From_Now (TOut_MS);
       loop
-         Timed_Out := Time.Timed_Out (Timeout);
          Read (Register, Current, Verbose);
          if (Current and Mask) = Value then
+            -- Ignore timeout if we succeeded anyway.
+            Timed_Out := False;
             exit;
          end if;
          pragma Debug (Timed_Out, Debug.Put (GNAT.Source_Info.Enclosing_Entity));
          pragma Debug (Timed_Out, Debug.Put_Line (": Timed Out!"));
          exit when Timed_Out;
+
+         Timed_Out := Time.Timed_Out (Timeout);
       end loop;
+
+      Success := not Timed_Out;
+   end Wait;
+
+   procedure Wait
+     (Register : Registers_Index;
+      Mask     : Word32;
+      Value    : Word32;
+      TOut_MS  : Natural := Default_Timeout_MS;
+      Verbose  : Boolean := False)
+   is
+      Ignored_Success : Boolean;
+   begin
+      Wait (Register, Mask, Value, TOut_MS, Verbose, Ignored_Success);
    end Wait;

----------------------------------------------------------------------------

-   -- TODO: Should have Success parameter
    -- Wait for all bits in @Register@ indicated by @Mask@ to be set
    procedure Wait_Set_Mask
      (Register : in     Registers_Index;
       Mask     : in     Word32;
       TOut_MS  : in     Natural := Default_Timeout_MS;
-      Verbose  : in     Boolean := False)
-   is
+      Verbose  : in     Boolean := False;
+      Success  :    out Boolean) is
    begin
-      Wait (Register, Mask, Mask, TOut_MS, Verbose);
+      Wait (Register, Mask, Mask, TOut_MS, Verbose, Success);
+   end Wait_Set_Mask;
+
+   procedure Wait_Set_Mask
+     (Register : Registers_Index;
+      Mask     : Word32;
+      TOut_MS  : Natural := Default_Timeout_MS;
+      Verbose  : Boolean := False)
+   is
+      Ignored_Success : Boolean;
+   begin
+      Wait (Register, Mask, Mask, TOut_MS, Verbose, Ignored_Success);
    end Wait_Set_Mask;

----------------------------------------------------------------------------

-   -- TODO: Should have Success parameter
    -- Wait for bits in @Register@ indicated by @Mask@ to be clear
    procedure Wait_Unset_Mask
+     (Register   : in     Registers_Index;
+      Mask       : in     Word32;
+      TOut_MS    : in     Natural := Default_Timeout_MS;
+      Verbose    : in     Boolean := False;
+      Success    :    out Boolean) is
+   begin
+      Wait (Register, Mask, 0, TOut_MS, Verbose, Success);
+   end;
+
+   procedure Wait_Unset_Mask
      (Register : Registers_Index;
       Mask     : Word32;
-      TOut_MS  : in     Natural := Default_Timeout_MS;
-      Verbose  : in     Boolean := False)
+      TOut_MS  : Natural := Default_Timeout_MS;
+      Verbose  : Boolean := False)
    is
+      Ignored_Success : Boolean;
    begin
-      Wait (Register, Mask, 0, TOut_MS, Verbose);
+      Wait (Register, Mask, 0, TOut_MS, Verbose, Ignored_Success);
    end Wait_Unset_Mask;

----------------------------------------------------------------------------
diff --git a/common/hw-gfx-gma-registers.ads b/common/hw-gfx-gma-registers.ads
index 5cffe76..e9c8ab4 100644
--- a/common/hw-gfx-gma-registers.ads
+++ b/common/hw-gfx-gma-registers.ads
@@ -1687,6 +1687,13 @@
    pragma Warnings (GNATprove, Off, "unused initial value of ""Verbose""",
                     Reason => "Only used on debugging path");
    procedure Wait
+     (Register : in     Registers_Index;
+      Mask     : in     Word32;
+      Value    : in     Word32;
+      TOut_MS  : in     Natural := Default_Timeout_MS;
+      Verbose  : in     Boolean := False;
+      Success  :    out Boolean);
+   procedure Wait
      (Register : Registers_Index;
       Mask     : Word32;
       Value    : Word32;
@@ -1694,16 +1701,28 @@
       Verbose  : Boolean := False);

procedure Wait_Set_Mask
-      (Register : Registers_Index;
-       Mask     : Word32;
-       TOut_MS  : Natural := Default_Timeout_MS;
-       Verbose  : Boolean := False);
+     (Register : in     Registers_Index;
+      Mask     : in     Word32;
+      TOut_MS  : in     Natural := Default_Timeout_MS;
+      Verbose  : in     Boolean := False;
+      Success  :    out Boolean);
+   procedure Wait_Set_Mask
+     (Register : Registers_Index;
+      Mask     : Word32;
+      TOut_MS  : Natural := Default_Timeout_MS;
+      Verbose  : Boolean := False);

procedure Wait_Unset_Mask
-      (Register : Registers_Index;
-       Mask     : Word32;
-       TOut_MS  : Natural := Default_Timeout_MS;
-       Verbose  : Boolean := False);
+     (Register : in     Registers_Index;
+      Mask     : in     Word32;
+      TOut_MS  : in     Natural := Default_Timeout_MS;
+      Verbose  : in     Boolean := False;
+      Success  :    out Boolean);
+   procedure Wait_Unset_Mask
+     (Register : Registers_Index;
+      Mask     : Word32;
+      TOut_MS  : Natural := Default_Timeout_MS;
+      Verbose  : Boolean := False);
    pragma Warnings (GNATprove, On, "unused initial value of ""Verbose""");

procedure Set_Mask

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/35709 )

Change subject: gma registers: Implement `Success` parameter for Wait*()
......................................................................

Patch Set 1: Verified+1

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/35709 )

Change subject: gma registers: Implement `Success` parameter for Wait*()
......................................................................

Patch Set 1: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/35709 )

Change subject: gma registers: Implement `Success` parameter for Wait*()
......................................................................

Patch Set 1: Code-Review+2

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/libgfxinit/+/35709 )

Change subject: gma registers: Implement `Success` parameter for Wait*()
......................................................................

gma registers: Implement `Success` parameter for Wait*()

Change-Id: Ia0e30e3467400e9d53a32c9bfc97d2d5f00df4aa
Signed-off-by: Nico Huber 
Reviewed-on: https://review.coreboot.org/c/libgfxinit/+/35709
Reviewed-by: Arthur Heymans 
Reviewed-by: Patrick Georgi 
---
M common/hw-gfx-gma-registers.adb
M common/hw-gfx-gma-registers.ads
2 files changed, 82 insertions(+), 24 deletions(-)

Approvals:
  Nico Huber: Verified
  Patrick Georgi: Looks good to me, approved
  Arthur Heymans: Looks good to me, approved

diff --git a/common/hw-gfx-gma-registers.adb b/common/hw-gfx-gma-registers.adb
index 380bced..3f0d7ae 100644
--- a/common/hw-gfx-gma-registers.adb
+++ b/common/hw-gfx-gma-registers.adb
@@ -286,18 +286,20 @@

----------------------------------------------------------------------------

-   -- TODO: Should have Success parameter
+   pragma Warnings (GNATprove, Off, "unused assignment to ""Ignored_Success""");
+
    -- Wait for the bits in @Register@ indicated by @Mask@ to be of @Value@
    procedure Wait
-     (Register : Registers_Index;
-      Mask     : Word32;
-      Value    : Word32;
-      TOut_MS  : Natural := Default_Timeout_MS;
-      Verbose  : Boolean := False)
+     (Register : in     Registers_Index;
+      Mask     : in     Word32;
+      Value    : in     Word32;
+      TOut_MS  : in     Natural := Default_Timeout_MS;
+      Verbose  : in     Boolean := False;
+      Success  :    out Boolean)
    is
       Current : Word32;
       Timeout : Time.T;
-      Timed_Out : Boolean;
+      Timed_Out : Boolean := False;
    begin
       pragma Debug (Debug.Put (GNAT.Source_Info.Enclosing_Entity & ":  "));
       pragma Debug (Debug.Put_Word32 (Value));
@@ -310,43 +312,80 @@

Timeout := Time.MS_From_Now (TOut_MS);
       loop
-         Timed_Out := Time.Timed_Out (Timeout);
          Read (Register, Current, Verbose);
          if (Current and Mask) = Value then
+            -- Ignore timeout if we succeeded anyway.
+            Timed_Out := False;
             exit;
          end if;
          pragma Debug (Timed_Out, Debug.Put (GNAT.Source_Info.Enclosing_Entity));
          pragma Debug (Timed_Out, Debug.Put_Line (": Timed Out!"));
          exit when Timed_Out;
+
+         Timed_Out := Time.Timed_Out (Timeout);
       end loop;
+
+      Success := not Timed_Out;
+   end Wait;
+
+   procedure Wait
+     (Register : Registers_Index;
+      Mask     : Word32;
+      Value    : Word32;
+      TOut_MS  : Natural := Default_Timeout_MS;
+      Verbose  : Boolean := False)
+   is
+      Ignored_Success : Boolean;
+   begin
+      Wait (Register, Mask, Value, TOut_MS, Verbose, Ignored_Success);
    end Wait;

----------------------------------------------------------------------------

-   -- TODO: Should have Success parameter
    -- Wait for all bits in @Register@ indicated by @Mask@ to be set
    procedure Wait_Set_Mask
      (Register : in     Registers_Index;
       Mask     : in     Word32;
       TOut_MS  : in     Natural := Default_Timeout_MS;
-      Verbose  : in     Boolean := False)
-   is
+      Verbose  : in     Boolean := False;
+      Success  :    out Boolean) is
    begin
-      Wait (Register, Mask, Mask, TOut_MS, Verbose);
+      Wait (Register, Mask, Mask, TOut_MS, Verbose, Success);
+   end Wait_Set_Mask;
+
+   procedure Wait_Set_Mask
+     (Register : Registers_Index;
+      Mask     : Word32;
+      TOut_MS  : Natural := Default_Timeout_MS;
+      Verbose  : Boolean := False)
+   is
+      Ignored_Success : Boolean;
+   begin
+      Wait (Register, Mask, Mask, TOut_MS, Verbose, Ignored_Success);
    end Wait_Set_Mask;

----------------------------------------------------------------------------

-   -- TODO: Should have Success parameter
    -- Wait for bits in @Register@ indicated by @Mask@ to be clear
    procedure Wait_Unset_Mask
+     (Register   : in     Registers_Index;
+      Mask       : in     Word32;
+      TOut_MS    : in     Natural := Default_Timeout_MS;
+      Verbose    : in     Boolean := False;
+      Success    :    out Boolean) is
+   begin
+      Wait (Register, Mask, 0, TOut_MS, Verbose, Success);
+   end;
+
+   procedure Wait_Unset_Mask
      (Register : Registers_Index;
       Mask     : Word32;
-      TOut_MS  : in     Natural := Default_Timeout_MS;
-      Verbose  : in     Boolean := False)
+      TOut_MS  : Natural := Default_Timeout_MS;
+      Verbose  : Boolean := False)
    is
+      Ignored_Success : Boolean;
    begin
-      Wait (Register, Mask, 0, TOut_MS, Verbose);
+      Wait (Register, Mask, 0, TOut_MS, Verbose, Ignored_Success);
    end Wait_Unset_Mask;

----------------------------------------------------------------------------
diff --git a/common/hw-gfx-gma-registers.ads b/common/hw-gfx-gma-registers.ads
index 5cffe76..e9c8ab4 100644
--- a/common/hw-gfx-gma-registers.ads
+++ b/common/hw-gfx-gma-registers.ads
@@ -1687,6 +1687,13 @@
    pragma Warnings (GNATprove, Off, "unused initial value of ""Verbose""",
                     Reason => "Only used on debugging path");
    procedure Wait
+     (Register : in     Registers_Index;
+      Mask     : in     Word32;
+      Value    : in     Word32;
+      TOut_MS  : in     Natural := Default_Timeout_MS;
+      Verbose  : in     Boolean := False;
+      Success  :    out Boolean);
+   procedure Wait
      (Register : Registers_Index;
       Mask     : Word32;
       Value    : Word32;
@@ -1694,16 +1701,28 @@
       Verbose  : Boolean := False);

procedure Wait_Set_Mask
-      (Register : Registers_Index;
-       Mask     : Word32;
-       TOut_MS  : Natural := Default_Timeout_MS;
-       Verbose  : Boolean := False);
+     (Register : in     Registers_Index;
+      Mask     : in     Word32;
+      TOut_MS  : in     Natural := Default_Timeout_MS;
+      Verbose  : in     Boolean := False;
+      Success  :    out Boolean);
+   procedure Wait_Set_Mask
+     (Register : Registers_Index;
+      Mask     : Word32;
+      TOut_MS  : Natural := Default_Timeout_MS;
+      Verbose  : Boolean := False);

procedure Wait_Unset_Mask
-      (Register : Registers_Index;
-       Mask     : Word32;
-       TOut_MS  : Natural := Default_Timeout_MS;
-       Verbose  : Boolean := False);
+     (Register : in     Registers_Index;
+      Mask     : in     Word32;
+      TOut_MS  : in     Natural := Default_Timeout_MS;
+      Verbose  : in     Boolean := False;
+      Success  :    out Boolean);
+   procedure Wait_Unset_Mask
+     (Register : Registers_Index;
+      Mask     : Word32;
+      TOut_MS  : Natural := Default_Timeout_MS;
+      Verbose  : Boolean := False);
    pragma Warnings (GNATprove, On, "unused initial value of ""Verbose""");

procedure Set_Mask