[Skiboot] [PATCH v2 2/2] FSP/EPOW: Add device tree entries to export EPOW event timeouts

[Vipin K Parashar]

On 05/12/2015 11:11 AM, Vipin K Parashar wrote:
> Responses below
>
> On 05/12/2015 09:27 AM, Stewart Smith wrote:
>> Vipin K Parashar <vipin at linux.vnet.ibm.com> writes:
>>> This patch adds device tree entries to export timeout values for
>>> various EPOW events. A separate node is created for each EPOW class
>>> under EPOW device tree node. Under each EPOW class, property files
>>> are created containing timeout values for various EPOW events for
>>> that class. Timeout values are reported in seconds.
>>>     Host kernel should parse these device tree entries under
>>> EPOW node to know various EPOW event timeout values on various
>>> platforms. Entries under EPOW device tree node look as below:
>>>
>>> epow
>>> ├── compatible
>>> ├── epow-classes
>>> ├── linux,phandle
>>> ├── name
>>> ├── phandle
>>> ├── power
>>> │   ├── linux,phandle
>>> │   ├── name
>>> │   ├── phandle
>>> │   ├── ups-low-timeout
>>> │   └── ups-timeout
>> are timeouts always going to be static?
>>
>> Should we have a way to tell linux that "you have 5 minutes from now" on
>> an event rather than just in DT?
>
> These are HW implemented timeout values and thus static.
> Linux would use these values to get time for each event once it
> gets OPAL EPOW notification. EPOW event info is obtained via
> EPOW Status API.
>

Also to add existing EPOW API doesn't provide any interface to return
timeout value. It just returns status of all EPOWs pending in OPAL so we 
need
to export EPOW event timeouts via DT.
     Any suggestions how returning varying timeout for most critical 
EPOW event
can be returned via existing EPOW API ?  or a new one shall be 
introduced to
incorporate this ?

>>
>>> └── temp
>>>      ├── crit-ambient-temp-timeout
>>>      ├── crit-internal-temp-timeout
>>>      ├── high-ambient-temp-timeout
>>>      ├── high-internal-temp-timeout
>>>      ├── linux,phandle
>>>      ├── name
>>>      └── phandle
>>>
>>> Signed-off-by: Vipin K Parashar <vipin at linux.vnet.ibm.com>
>>> ---
>>>   hw/fsp/fsp-epow.c | 62
>>> +++++++++++++++++++++++++++++++++++++++++++------------
>> Please add docs in doc/device-tree/
>
> Will do.
>
>>
>>>   1 file changed, 49 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/hw/fsp/fsp-epow.c b/hw/fsp/fsp-epow.c
>>> index 97b082a..1ecb6fc 100644
>>> --- a/hw/fsp/fsp-epow.c
>>> +++ b/hw/fsp/fsp-epow.c
>>> @@ -273,31 +273,67 @@ static bool fsp_epow_message(u32 cmd_sub_mod, 
>>> struct fsp_msg *msg)
>>>       return false;
>>>   }
>>>
>>> +/* Create EPOW device tree */
>>> +static void create_epow_dt_entries(void)
>>> +{
>>> +    struct dt_node *node_epow;
>>> +    struct dt_node *node_epow_power, *node_epow_temp;
>>> +
>>> +    /* Create EPOW node */
>>> +    node_epow = dt_new(opal_node, "epow");
>>> +    if (!node_epow) {
>>> +        prerror(PREFIX "Failed to create epow device tree node\n");
>>> +        return;
>>> +    }
>>> +
>>> +    /* Add EPOW node properties */
>>> +    dt_add_property_strings(node_epow, "compatible", "ibm,opal-epow");
>>> +    dt_add_property_strings(node_epow, "epow-classes", "power",
>>> +            "temp", "cooling");
>>> +
>>> +    /*
>>> +    * Create EPOW child nodes and associated property entries
>>> +    *
>>> +    * Power EPOW properties
>>> +    */
>>> +    node_epow_power = dt_new(node_epow, "power");
>>> +    if (node_epow_power) {
>>> +        dt_add_property_cells(node_epow_power,
>>> +        "ups-timeout", TIMEOUT_EPOW_POWER_UPS);
>>> +        dt_add_property_cells(node_epow_power,
>>> +        "ups-low-timeout", TIMEOUT_EPOW_POWER_UPS_LOW);
>>> +    } else
>>> +        prerror(PREFIX "Failed to create epow/power dt node\n");
>>> +
>>> +    /* Temperature EPOW propeties */
>>> +    node_epow_temp = dt_new(node_epow, "temp");
>>> +    if (node_epow_temp) {
>>> +        dt_add_property_cells(node_epow_temp,
>>> +        "high-ambient-temp-timeout", TIMEOUT_EPOW_TEMP_HIGH_AMB);
>>> +        dt_add_property_cells(node_epow_temp,
>>> +        "crit-ambient-temp-timeout", TIMEOUT_EPOW_TEMP_CRIT_AMB);
>>> +        dt_add_property_cells(node_epow_temp,
>>> +        "high-internal-temp-timeout", TIMEOUT_EPOW_TEMP_HIGH_INT);
>>> +        dt_add_property_cells(node_epow_temp,
>>> +        "crit-internal-temp-timeout", TIMEOUT_EPOW_TEMP_CRIT_INT);
>>> +    } else
>>> +        prerror(PREFIX "Failed to create epow/temp dt node\n");
>>> +}
>>> +
>>>   static struct fsp_client fsp_epow_client = {
>>>       .message = fsp_epow_message,
>>>   };
>>>
>>>   void fsp_epow_init(void)
>>>   {
>>> -    struct dt_node *node_epow;
>>> -
>>>       /* Register FSP EPOW notifications */
>>>       fsp_register_client(&fsp_epow_client, FSP_MCLASS_SERVICE);
>>>
>>>       /* Register host OPAL EPOW interface */
>>>       opal_register(OPAL_GET_EPOW_STATUS, opal_get_epow_status, 2);
>>>
>>> -    /* Create EPOW device tree node */
>>> -    node_epow = dt_new(opal_node, "epow");
>>> -    if (!node_epow) {
>>> -        prerror(PREFIX "Failed to create epow device tree node\n");
>>> -        return;
>>> -    }
>>> -
>>> -    /* Add EPOW node properties */
>>> -    dt_add_property_strings(node_epow, "compatible", "ibm,opal-epow");
>>> -    dt_add_property_strings(node_epow, "epow-classes", "power", 
>>> "temp",
>>> -                    "cooling");
>>> +    /* Create device tree entries for EPOW */
>>> +    create_epow_dt_entries();
>>>
>>>       prlog(PR_INFO, PREFIX "FSP EPOW support initialized\n");
>>>   }
>> Same comment as to other patcH: this looks like API change.
>
> Reponded as reply to previous patch.
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot