RE: Some questions about "Add option for SEL commands for Journal-based SEL entries"

* RE: Some questions about "Add option for SEL commands for Journal-based SEL entries"
       [not found]         ` <dc07bda4-f926-1b8f-a157-1b0ecfc59e33@linux.intel.com>
@ 2019-03-29 11:00           ` Will Liang (梁永鉉)
  2019-03-29 18:14             ` Bills, Jason M
  0 siblings, 1 reply; 2+ messages in thread
From: Will Liang (梁永鉉) @ 2019-03-29 11:00 UTC (permalink / raw)
  To: Bills, Jason M
  Cc: Richard Tung (董彥屏),
	Buddy Huang (黃天鴻),
	George Hung (洪忠敬),
	OpenBMC Maillist

[-- Attachment #1: Type: text/plain, Size: 7174 bytes --]

Hi Jason,

Sorry to bother you again.

After solving sensor type and event type problem for discrete sensor(memory) , I encountered another issue.
From the code, it seems only support getting the sensor number of threshold-based sensors.
How do I get the sensor  number of discrete sensors?
Would you have any suggestion or plan to get sensor number of discrete sensors?

BRs,
Will

Hi Will,

You are welcome to push changes directly to Gerrit for review.  I did a
quick check of the code and it looks okay to push with one minor change
of THRESHLOD -> THRESHOLD.

Thanks,
-Jason

On 3/26/2019 8:28 PM, Will Liang (梁永鉉) wrote:
> Hi Jason,
>
> I have seen that the code has been resolved from the conflict and I can
> bitbake it successfully too. Thanks.
>
> As indicated in my previous Email,  I need the "memory" sensor type and
> "sensor-specific" event type.
>
> Could you please help me to review the following changes.
>
> Andif you don't have any concerns, I will push the code to Gerrit >
> sdrutils.hpp      | 34 +++++++++++++++++++++-------------
>
> sensorhandler.hpp |  8 ++++++++
>
> 2 files changed, 29 insertions(+), 13 deletions(-)
>
> diff --git a/sdrutils.hpp b/sdrutils.hpp
>
> index a91f3ea..bff2896 100644
>
> --- a/sdrutils.hpp
>
> +++ b/sdrutils.hpp
>
> @@ -20,9 +20,10 @@
>
> #include <boost/container/flat_map.hpp>
>
> #include <cstring>
>
> #include <phosphor-logging/log.hpp>
>
> -
>
> +#include <types.hpp>
>
> #pragma once
>
> +
>
> #ifdef JOURNAL_SEL
>
> struct CmpStrVersion
>
> {
>
> @@ -74,13 +75,14 @@ struct CmpStr
>
>       }
>
> };
>
> -const static boost::container::flat_map<const char*, ipmi_sensor_types,
> CmpStr>
>
> -    sensorTypes{{{"temperature", IPMI_SENSOR_TEMP},
>
> -                 {"voltage", IPMI_SENSOR_VOLTAGE},
>
> -                 {"current", IPMI_SENSOR_CURRENT},
>
> -                 {"fan_tach", IPMI_SENSOR_FAN},
>
> -                 {"fan_pwm", IPMI_SENSOR_FAN},
>
> -                 {"power", IPMI_SENSOR_OTHER}}};
>
> +const static boost::container::flat_map<const char*,
> std::pair<ipmi_sensor_types, ipmi_event_types>, CmpStr>
>
> +    sensorAndEventType{{{"temperature",
> std::make_pair(IPMI_SENSOR_TEMP, THRESHLOD)},
>
> +                 {"voltage", std::make_pair(IPMI_SENSOR_VOLTAGE,
> THRESHLOD)},
>
> +                 {"current", std::make_pair(IPMI_SENSOR_CURRENT,
> THRESHLOD)},
>
> +                 {"fan_tach", std::make_pair(IPMI_SENSOR_FAN, THRESHLOD)},
>
> +                 {"fan_pwm", std::make_pair(IPMI_SENSOR_FAN, THRESHLOD)},
>
> +                 {"power", std::make_pair(IPMI_SENSOR_OTHER, THRESHLOD)},
>
> +                 {"memory", std::make_pair(IPMI_SENSOR_MEMORY,
> SENSOR_SPECIFIC)}}};
>
>   inline static std::string getSensorTypeStringFromPath(const
> std::string& path)
>
> {
>
> @@ -105,12 +107,11 @@ inline static uint8_t getSensorTypeFromPath(const
> std::string& path)
>
> {
>
>       uint8_t sensorType = 0;
>
>       std::string type = getSensorTypeStringFromPath(path);
>
> -    auto findSensor = sensorTypes.find(type.c_str());
>
> -    if (findSensor != sensorTypes.end())
>
> +    auto findSensor = sensorAndEventType.find(type.c_str());
>
> +    if (findSensor != sensorAndEventType.end())
>
>       {
>
> -        sensorType = findSensor->second;
>
> +        sensorType = findSensor->second.first;
>
>       } // else default 0x0 RESERVED
>
> -
>
>       return sensorType;
>
> }
>
> @@ -128,7 +129,14 @@ inline static uint8_t getSensorNumberFromPath(const
> std::string& path)
>
> inline static uint8_t getSensorEventTypeFromPath(const std::string& path)
>
> {
>
>       // TODO: Add support for additional reading types as needed
>
> -    return 0x1; // reading type = threshold
>
> +    uint8_t eventType = 0x00;
>
> +    std::string type = getSensorTypeStringFromPath(path);
>
> +    auto findSensor = sensorAndEventType.find(type.c_str());
>
> +    if (findSensor != sensorAndEventType.end())
>
> +    {
>
> +        eventType = findSensor->second.second;
>
> +    }
>
> +    return eventType; // reading type = threshold
>
> }
>
>   inline static std::string getPathFromSensorNumber(uint8_t sensorNum)
>
> diff --git a/sensorhandler.hpp b/sensorhandler.hpp
>
> index d3b6378..e449076 100644
>
> --- a/sensorhandler.hpp
>
> +++ b/sensorhandler.hpp
>
> @@ -36,6 +36,14 @@ enum ipmi_sensor_types
>
>       IPMI_SENSOR_FAN = 0x04,
>
>       IPMI_SENSOR_OTHER = 0x0B,
>
>       IPMI_SENSOR_TPM = 0xCC,
>
> +    IPMI_SENSOR_MEMORY = 0x0C,
>
> +};
>
> +
>
> +enum ipmi_event_types
>
> +{
>
> +    UNSPECIFIED = 0x00,
>
> +    THRESHLOD = 0x01,
>
> +    SENSOR_SPECIFIC = 0x6f,
>
> };
>
>   #define MAX_DBUS_PATH 128
>
> BRs,
>
> Will
>
> Hi Will,
>
> On 3/13/2019 5:05 AM, Will Liang (梁永鉉) wrote:
>> Hi Jason,
>>
>> Thanks for your response!
>>
>> I tried to modify the code, but I encountered some problems after pull the code from Gerrit.
>> After I pull your codes and run "bitbake obmc-phosphor-image " command.
>> It will shows ERROR: Task (/home/will/openbmc/meta-phosphor/recipes-phosphor/ipmi/phosphor-ipmi-net_git.bb:do_compile) failed with exit code '1'
>> Do you have this error? Or what else do I need to do?
>
> Sorry, it's been a while since I built this patch, and there have been
> some big changes to ipmid since I pushed it.  I'm not able to test it
> right now, but it may just need to be rebased?
>>
>> I'm sorry, maybe this is a dumb question but I don’t understand how you defined this "JOURNAL_SEL" parameter from *.bbappend?
>> I tried it but it still can't run into the #ifdef JOURNAL_SEL part.
>
> There are no dumb questions with Yocto. :)  You need to add this line to
> a phosphor-ipmi-host_%.bbappend:
> EXTRA_OECONF = " --with-journal-sel"
>
> That should cause the define to be set by autoconfig during the build.
>
> Thanks,
> -Jason
>
>>
>> BRs,
>> Will
>>
>>> Hi Will,
>>>
>>> On 3/11/2019 12:39 AM, Will Liang (梁永鉉) wrote:
>>>> Hi Jason,
>>>>
>>>> I've reviewed the code you pushed to
>>> gerrit(https://gerrit.openbmc-project.xyz/#/c/openbmc/phosphor-host-ipmid/+
>>> /12951/).
>>>>
>>>
>>> I'm not sure if the patch will be approved by the maintainers.  So you are
>>> aware, while it's in review, I have pushed the same code to the intel-ipmi-oem
>>> repository.
>>>
>>>> In sdrutils.hpp, the getSensorEventTypeFromPath() is set TODO status.
>>>> If I need other event types and sensor types for my projects, do I
>>>> have to complete the code and send you the code? Or I just push it myself?
>>>>
>>>
>>> I'd suggest building your changes as a new patch that depends on the existing
>>> patch.  Then if it's approved, your patch can be merged.  If not, you can port
>>> the changes as needed.
>>>
>>> Thanks,
>>> -Jason
>>>
>>>> For example, our project requires a "memory" sensor type and a
>>>> "sensor-specific" event type, but I am confused about which way is
>>> acceptable.
>>>>
>>>> BRs,
>>>> Will.
>>>>
>

[-- Attachment #2: Type: text/html, Size: 13193 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread