diff mbox series

[v4,08/10] hw/core/resettable: add support for warm reset

Message ID 20190821163341.16309-9-damien.hedde@greensocs.com
State New
Headers show
Series Multi-phase reset mechanism | expand

Commit Message

Damien Hedde Aug. 21, 2019, 4:33 p.m. UTC
Add the RESET_TYPE_WARM reset type.
Expand the actual implementation to support several types.

I used values which can be or'ed together. This way we can what reset
has been triggered.

Documentation is added in a following patch.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 hw/core/resettable.c    | 25 +++++++++++++++++++------
 include/hw/resettable.h | 22 +++++++++++++++++-----
 2 files changed, 36 insertions(+), 11 deletions(-)

Comments

Philippe Mathieu-Daudé May 10, 2020, 8:17 p.m. UTC | #1
Hi Damien,

On 8/21/19 6:33 PM, Damien Hedde wrote:
> Add the RESET_TYPE_WARM reset type.
> Expand the actual implementation to support several types.
> 
> I used values which can be or'ed together. This way we can what reset
> has been triggered.
> 
> Documentation is added in a following patch.
> 
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> ---
>   hw/core/resettable.c    | 25 +++++++++++++++++++------
>   include/hw/resettable.h | 22 +++++++++++++++++-----
>   2 files changed, 36 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/core/resettable.c b/hw/core/resettable.c
> index b534c2c7a4..80674292b3 100644
> --- a/hw/core/resettable.c
> +++ b/hw/core/resettable.c
> @@ -34,12 +34,17 @@ static void resettable_init_reset(Object *obj, ResetType type)
>       ResetState *s = rc->get_state(obj);
>       bool action_needed = false;
>   
> -    /* Only take action if we really enter reset for the 1st time. */
> +    /* ensure type is empty if no reset is in progress */
> +    if (s->count == 0) {
> +        s->type = 0;
> +    }
> +
>       /*
> -     * TODO: if adding more ResetType support, some additional checks
> -     * are probably needed here.
> +     * Only take action if:
> +     * + we are not already in cold reset,
> +     * + and we enter a new type of reset.
>        */
> -    if (s->count == 0) {
> +    if ((s->type & RESET_TYPE_COLD) == 0 && (s->type & type) == 0) {
>           action_needed = true;
>       }
>       s->count += 1;
> @@ -62,6 +67,7 @@ static void resettable_init_reset(Object *obj, ResetType type)
>   
>       /* exec init phase */
>       if (action_needed) {
> +        s->type |= type;
>           s->hold_phase_needed = true;
>           if (rc->phases.init) {
>               rc->phases.init(obj, type);
> @@ -133,8 +139,7 @@ static void resettable_exit_reset(Object *obj)
>   
>   void resettable_reset(Object *obj, ResetType type)
>   {
> -    /* TODO: change that when adding support for other reset types */
> -    assert(type == RESET_TYPE_COLD);
> +    assert(type == RESET_TYPE_COLD || type == RESET_TYPE_WARM);
>       trace_resettable_reset(obj, type);
>       resettable_init_reset(obj, type);
>       resettable_hold_reset(obj);
> @@ -154,6 +159,14 @@ bool resettable_is_resetting(Object *obj)
>       return (s->count > 0);
>   }
>   
> +ResetType resettable_get_type(Object *obj)
> +{
> +    ResettableClass *rc = RESETTABLE_GET_CLASS(obj);
> +    ResetState *s = rc->get_state(obj);
> +
> +    return s->type;
> +}
> +
>   void resettable_class_set_parent_phases(ResettableClass *rc,
>                                           ResettableInitPhase init,
>                                           ResettableHoldPhase hold,
> diff --git a/include/hw/resettable.h b/include/hw/resettable.h
> index 5808c3c187..1e77cbd75b 100644
> --- a/include/hw/resettable.h
> +++ b/include/hw/resettable.h
> @@ -12,15 +12,14 @@ typedef struct ResetState ResetState;
>   
>   /**
>    * ResetType:
> - * Types of reset.
> + * Types of reset, values can be OR'ed together.
>    *
>    * + Cold: reset resulting from a power cycle of the object.
> - *
> - * TODO: Support has to be added to handle more types. In particular,
> - * ResetState structure needs to be expanded.
> + * + Warm: reset without power cycling.
>    */
>   typedef enum ResetType {
> -    RESET_TYPE_COLD,
> +    RESET_TYPE_COLD = 0x1,
> +    RESET_TYPE_WARM = 0x2,

I'm a bit lost with the various iterations, what is the plan with warm 
reset, is this blocked due to discussion, API, something else?

>   } ResetType;
>   
>   /*
> @@ -107,11 +106,13 @@ typedef struct ResettablePhases ResettablePhases;
>    *
>    * @count: Number of reset level the object is into. It is incremented when
>    * the reset operation starts and decremented when it finishes.
> + * @type: Type of the in-progress reset. Valid only when count is non-zero.
>    * @hold_phase_needed: flag which indicates that we need to invoke the 'hold'
>    * phase handler for this object.
>    */
>   struct ResetState {
>       uint32_t count;
> +    ResetType type;
>       bool hold_phase_needed;
>   };
>   
> @@ -123,6 +124,17 @@ struct ResetState {
>    */
>   bool resettable_is_resetting(Object *obj);
>   
> +/**
> + * resettable_get_type:
> + * Return the current type of reset @obj is under.
> + *
> + * @obj must implement Resettable interface. Result is only valid if
> + * @resettable_is_resetting is true.
> + *
> + * Note: this return an OR'ed value of type if several reset were triggered
> + */
> +ResetType resettable_get_type(Object *obj);
> +
>   /**
>    * resettable_reset:
>    * Trigger a reset on a object @obj of type @type. @obj must implement
>
Damien Hedde May 11, 2020, 9:37 a.m. UTC | #2
Hi Philippe,

On 5/10/20 10:17 PM, Philippe Mathieu-Daudé wrote:
> Hi Damien,
> 
> On 8/21/19 6:33 PM, Damien Hedde wrote:
>> Add the RESET_TYPE_WARM reset type.
>> Expand the actual implementation to support several types.
>>
>>     /**
>>    * ResetType:
>> - * Types of reset.
>> + * Types of reset, values can be OR'ed together.
>>    *
>>    * + Cold: reset resulting from a power cycle of the object.
>> - *
>> - * TODO: Support has to be added to handle more types. In particular,
>> - * ResetState structure needs to be expanded.
>> + * + Warm: reset without power cycling.
>>    */
>>   typedef enum ResetType {
>> -    RESET_TYPE_COLD,
>> +    RESET_TYPE_COLD = 0x1,
>> +    RESET_TYPE_WARM = 0x2,
> 
> I'm a bit lost with the various iterations, what is the plan with warm
> reset, is this blocked due to discussion, API, something else?
> 

I removed it in the last versions of the series because it was adding
complexity. There were unsolved issues and discussions, in particular
regarding the interactions and propagation along trees between the
different types.

Damien
diff mbox series

Patch

diff --git a/hw/core/resettable.c b/hw/core/resettable.c
index b534c2c7a4..80674292b3 100644
--- a/hw/core/resettable.c
+++ b/hw/core/resettable.c
@@ -34,12 +34,17 @@  static void resettable_init_reset(Object *obj, ResetType type)
     ResetState *s = rc->get_state(obj);
     bool action_needed = false;
 
-    /* Only take action if we really enter reset for the 1st time. */
+    /* ensure type is empty if no reset is in progress */
+    if (s->count == 0) {
+        s->type = 0;
+    }
+
     /*
-     * TODO: if adding more ResetType support, some additional checks
-     * are probably needed here.
+     * Only take action if:
+     * + we are not already in cold reset,
+     * + and we enter a new type of reset.
      */
-    if (s->count == 0) {
+    if ((s->type & RESET_TYPE_COLD) == 0 && (s->type & type) == 0) {
         action_needed = true;
     }
     s->count += 1;
@@ -62,6 +67,7 @@  static void resettable_init_reset(Object *obj, ResetType type)
 
     /* exec init phase */
     if (action_needed) {
+        s->type |= type;
         s->hold_phase_needed = true;
         if (rc->phases.init) {
             rc->phases.init(obj, type);
@@ -133,8 +139,7 @@  static void resettable_exit_reset(Object *obj)
 
 void resettable_reset(Object *obj, ResetType type)
 {
-    /* TODO: change that when adding support for other reset types */
-    assert(type == RESET_TYPE_COLD);
+    assert(type == RESET_TYPE_COLD || type == RESET_TYPE_WARM);
     trace_resettable_reset(obj, type);
     resettable_init_reset(obj, type);
     resettable_hold_reset(obj);
@@ -154,6 +159,14 @@  bool resettable_is_resetting(Object *obj)
     return (s->count > 0);
 }
 
+ResetType resettable_get_type(Object *obj)
+{
+    ResettableClass *rc = RESETTABLE_GET_CLASS(obj);
+    ResetState *s = rc->get_state(obj);
+
+    return s->type;
+}
+
 void resettable_class_set_parent_phases(ResettableClass *rc,
                                         ResettableInitPhase init,
                                         ResettableHoldPhase hold,
diff --git a/include/hw/resettable.h b/include/hw/resettable.h
index 5808c3c187..1e77cbd75b 100644
--- a/include/hw/resettable.h
+++ b/include/hw/resettable.h
@@ -12,15 +12,14 @@  typedef struct ResetState ResetState;
 
 /**
  * ResetType:
- * Types of reset.
+ * Types of reset, values can be OR'ed together.
  *
  * + Cold: reset resulting from a power cycle of the object.
- *
- * TODO: Support has to be added to handle more types. In particular,
- * ResetState structure needs to be expanded.
+ * + Warm: reset without power cycling.
  */
 typedef enum ResetType {
-    RESET_TYPE_COLD,
+    RESET_TYPE_COLD = 0x1,
+    RESET_TYPE_WARM = 0x2,
 } ResetType;
 
 /*
@@ -107,11 +106,13 @@  typedef struct ResettablePhases ResettablePhases;
  *
  * @count: Number of reset level the object is into. It is incremented when
  * the reset operation starts and decremented when it finishes.
+ * @type: Type of the in-progress reset. Valid only when count is non-zero.
  * @hold_phase_needed: flag which indicates that we need to invoke the 'hold'
  * phase handler for this object.
  */
 struct ResetState {
     uint32_t count;
+    ResetType type;
     bool hold_phase_needed;
 };
 
@@ -123,6 +124,17 @@  struct ResetState {
  */
 bool resettable_is_resetting(Object *obj);
 
+/**
+ * resettable_get_type:
+ * Return the current type of reset @obj is under.
+ *
+ * @obj must implement Resettable interface. Result is only valid if
+ * @resettable_is_resetting is true.
+ *
+ * Note: this return an OR'ed value of type if several reset were triggered
+ */
+ResetType resettable_get_type(Object *obj);
+
 /**
  * resettable_reset:
  * Trigger a reset on a object @obj of type @type. @obj must implement