openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Thu Nguyen <thu@amperemail.onmicrosoft.com>
To: Ed Tanous <edtanous@google.com>
Cc: "openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>,
	Matt Spinler <mspinler@linux.ibm.com>,
	Thang Nguyen OS <thang@os.amperecomputing.com>
Subject: Re: Add socsesor daemon to handle Ampere SoC sensors in dbus-sensors.
Date: Thu, 22 Jul 2021 23:35:59 +0700	[thread overview]
Message-ID: <0e248fdd-4517-434e-3288-6037bee44af1@amperemail.onmicrosoft.com> (raw)
In-Reply-To: <CAH2-KxCypE01QyLJ+0n0muOs-4a3yB8+caFowpqVgZKm611nWA@mail.gmail.com>

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


On 22/07/2021 23:15, Ed Tanous wrote:
> On Thu, Jul 22, 2021 at 8:56 AM Thu Nguyen
> <thu@amperemail.onmicrosoft.com> wrote:
>> Dear Ed Tanous,
>>
>>
>> As we discussed before about the Ampere CPU daemon in OpenBmc general discord channel. I'm planing to submit the some patches which will add to AmpereSoC daemon source code to dbus-sensors for reviewing. This daemon will handle Ampere Altra SoC sensors. The functions of this daemon are the same as CPUSensor daemon which is handling Xeon CPU sensors.
>>
>> Below is what I will add.
>>
>>      1. Add some files to to Dbus-sensors source:
>>
>>          ./src/AmpereSoCMain.cpp : Match the sensors configurations in entity-manager configuration file with sensor paths in the device driver. And create the SoC sensor dbus objects.
> At this same time, we should rename "CPU sensor" to "Intel CPU" so we
> can easily differentiate.  Ideally your new daemon would also be
> called "AmpereCPU" to keep the naming conventions the same.

I will use AmpereCPU name. And will rename "CPU sensor" to "Intel CPU"

>
>>          ./src/AmpereSoC.cpp : Create the sensor handler and update sensor values to the sensors dbus objects.
>>
>>          ./include/AmpereSoC.hpp : Declare functions and constants.
>>
>>          The structure and the functions of the files are similar the files which create ADCSensor daemon or PSUSensors daemon...
>>
>>      2. Add AmpereSoC daemon which will be named as "xyz.openbmc_project.socsensor.service".
>>
>>          Below is dbus-sensors daemons in our system:
> This list seems disconnected from AmpereSOC.  Was there something you
> were looking for input on there?
>
These are the list of daemons in our system:
~# systemctl | grep sensor
phosphor-virtual-sensor.service                   active running   Virtual sensors
xyz.openbmc_project.adcsensor.service             active running   Adc Sensor
xyz.openbmc_project.fansensor.service             active running   Fan Sensor
xyz.openbmc_project.hwmontempsensor.service       active running   Hwmon Temp Sensor
xyz.openbmc_project.psusensor.service             active running   PSU Sensor
xyz.openbmc_project.socsensor.service             active running   SoC Sensor

>
>
>      3. Add dbus interface to public sensor dbus objects. The name of this interface is "xyz.openbmc_project.SoCSensor".
>
>          Below is some sensors in this dbus interface.
> Neat.

Below are some Ampere SoC sensors in SoCSensor dbus interface.
~# busctl tree xyz.openbmc_project.SoCSensor
`-/xyz
`-/xyz/openbmc_project
     |-/xyz/openbmc_project/inventory
     | `-/xyz/openbmc_project/inventory/system
     |   `-/xyz/openbmc_project/inventory/system/chassis
     |     `-/xyz/openbmc_project/inventory/system/chassis/motherboard
     |       |-/xyz/openbmc_project/inventory/system/chassis/motherboard/CPU_1
     |       `-/xyz/openbmc_project/inventory/system/chassis/motherboard/CPU_2
     `-/xyz/openbmc_project/sensors
     |-/xyz/openbmc_project/sensors/current
     | |-/xyz/openbmc_project/sensors/current/S0_Core_VRD_Curr
     | |-/xyz/openbmc_project/sensors/current/S0_DIMM_VR1_Curr
     | |-/xyz/openbmc_project/sensors/current/S0_DIMM_VR2_Curr
     |-/xyz/openbmc_project/sensors/power
     | |-/xyz/openbmc_project/sensors/power/S0_Core_VRD_Pwr
     | |-/xyz/openbmc_project/sensors/power/S0_DIMM_VR1_Pwr
     | |-/xyz/openbmc_project/sensors/power/S0_DIMM_VR2_Pwr
     |-/xyz/openbmc_project/sensors/temperature
     | |-/xyz/openbmc_project/sensors/temperature/S0_Core_VRD_Temp
     | |-/xyz/openbmc_project/sensors/temperature/S0_DIMM_CH4_Temp
     | |-/xyz/openbmc_project/sensors/temperature/S0_DIMM_VRD_Temp
     `-/xyz/openbmc_project/sensors/voltage
         |-/xyz/openbmc_project/sensors/voltage/S0_CPU_VCORE
         |-/xyz/openbmc_project/sensors/voltage/S0_DIMM_VR1_Volt
         |-/xyz/openbmc_project/sensors/voltage/S0_DIMM_VR2_Volt

>>
>>
>>      4. The causes of this adding are:
>>          + The Ampere SoCs and their interfaces are quite complicated to use the current daemons.
> Considering that ampere CPUs support their own disparate OOB
> interface, I suspect it's ideal that they would be separated out into
> their own daemon.

Currently, Our openbmc uses I2C and GPIO to communicate/monitor our CPU thru SCP to get sensors, errors and events.
The openbmc is depend on SCP.

>
>>          + We can't use CPUSensor daemon which is specific for Intel Xeon. We use I2C interface to access the SoC sensors which Intel Xeon using PCIE.
> This is fine, and what I would've expected.  CPU sensor is fairly
> specific to PECI today.
>
>>          + Our SoC hwmon drivers are different with the other hwmon drivers in linux kernel. We are using one MDF driver name ac01-smpro, it have three sub drivers ac01-hwmon which report the temperature, the consumed power, current and voltage of components in SOC.
>>          + In the near future, We append AmpereSoC daemon to monitor SoC RAS and SoC events as you suggested.
>>
>>      Please let me know if you have any comments about this plan.
>
> I'm looking forward to the patches.  I don't see any concerns, as
> everything above looks like you're following the existing interfaces.
>
>>
>> Thanks.
>>
>> Thu.
>>
>>
>>
>> CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is for the sole use of the intended recipient(s) and contains information that is confidential and proprietary to Ampere Computing or its subsidiaries. It is to be used solely for the purpose of furthering the parties' business relationship. Any unauthorized review, copying, or distribution of this email (or any attachments thereto) is strictly prohibited. If you are not the intended recipient, please contact the sender immediately and permanently delete the original and any copies of this email and any attachments thereto.

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

      reply	other threads:[~2021-07-22 16:37 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <e8730a3b-9cf3-7167-20f2-e7031a6c6f69@amperemail.onmicrosoft.com>
2021-07-22 16:15 ` Add socsesor daemon to handle Ampere SoC sensors in dbus-sensors Ed Tanous
2021-07-22 16:35   ` Thu Nguyen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0e248fdd-4517-434e-3288-6037bee44af1@amperemail.onmicrosoft.com \
    --to=thu@amperemail.onmicrosoft.com \
    --cc=edtanous@google.com \
    --cc=mspinler@linux.ibm.com \
    --cc=openbmc@lists.ozlabs.org \
    --cc=thang@os.amperecomputing.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).