openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Bruce Mitchell <bruce.mitchell@linux.vnet.ibm.com>
To: Guenter Roeck <groeck@google.com>,
	"openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>,
	Ed Tanous <edtanous@google.com>
Cc: linux-hwmon@vger.kernel.org, linux-iio@vger.kernel.org,
	Eddie James <eajames@linux.ibm.com>,
	Guenter Roeck <groeck@chromium.org>,
	Guenter Roeck <linux@roeck-us.net>
Subject: Re: Seeking your opinion on ways to report both Altitude and Pressure sensors for the DPS310 as well as Temperature from dbus-sensors.
Date: Wed, 2 Jun 2021 12:01:18 -0700	[thread overview]
Message-ID: <c32a844b-1114-b423-7e7d-63821628f919@linux.vnet.ibm.com> (raw)
In-Reply-To: <CABXOdTcdkXzmi9qWWZapY19Z7-DsnXsz-yBWbaBqwhLrJpTDVw@mail.gmail.com>

On 6/2/2021 11:52, Guenter Roeck wrote:
> Hi,
> 
> On Wed, Jun 2, 2021 at 11:18 AM Bruce Mitchell
> <bruce.mitchell@linux.vnet.ibm.com> wrote:
>>
>> Forwarded to an expanded the To and Cc Lists.
>>
>> On 6/2/2021 09:21, Ed Tanous wrote:
>>> On Wed, Jun 2, 2021 at 9:14 AM Bruce Mitchell
>>> <bruce.mitchell@linux.vnet.ibm.com> wrote:
>>>>
>>>> On 6/2/2021 09:03, Ed Tanous wrote:
>>>>> On Wed, Jun 2, 2021 at 8:58 AM Bruce Mitchell
>>>>> <bruce.mitchell@linux.vnet.ibm.com> wrote:
>>>>>>
>>>>>> On 6/2/2021 08:39, Ed Tanous wrote:
>>>>>>> On Tue, Jun 1, 2021 at 8:43 AM Bruce Mitchell
>>>>>>> <bruce.mitchell@linux.vnet.ibm.com> wrote:
>>>>>>>>
>>>>>>>> Hello Ed,
>>>>>>>>
>>>>>>>> It has been suggest I seeking your opinion on ways to report both
>>>>>>>> Altitude and Pressure sensors for the DPS310 as well as Temperature from
>>>>>>>> dbus-sensors before going to far down the road.  Thus that is what I am
>>>>>>>> attempting to do in the email, others on the mailing list input is
>>>>>>>> desirable as well.
>>>>>>>
>>>>>>> Thanks for discussing this before getting too far along.  I haven't
>>>>>>> worked on any systems with physical pressure sensors, but I'm excited
>>>>>>> to see new things get added.
>>>>>>>
>>>>>>>>
>>>>>>>> As I see it, Altitude and Pressure are different in that
>>>>>>>>          1) Altitude is computed base off of essentially a policy
>>>>>>>
>>>>>>> I have no idea what this means.....   In what way is altitude a
>>>>>>> "policy"?  Can you elaborate a little?
>>>>>>>
>>>>>>
>>>>>> I view a mechanism is something like update a FLASH part with
>>>>>> an image provided.
>>>>>>
>>>>>> I view a policy is what decides if the the update of the FLASH part
>>>>>> with the specific image is allowed.
>>>>>>
>>>>>> I the case if Pressure and Temperature I view them as mechanism,
>>>>>> merely a simple reading and possibly some well defined computations
>>>>>> that are universal.
>>>>>>
>>>>>> With Altitude computed from Pressure there are several ways to
>>>>>> compute the Altitude and they are not universal.  So I see it as
>>>>>> a policy of which Pressure to Altitude model is chosen and why.
>>>>>
>>>>> Sounds like I interpreted your intention correctly. (I think).
>>>>
>>>> I believe you did.
>>>>
>>>>>
>>>>>>
>>>>>>>>          2) Pressures is a read measurement which is a mechanism
>>>>>>>>          3) Temperature is a read measurement which is also a mechanism
>>>>>>>
>>>>>>> I'm really struggling with the above to understand what you're getting
>>>>>>> after, so if I go down the wrong path, please forgive me.
>>>>>>>
>>>>>>> I think what you're saying is that altitude is calculated based on
>>>>>>> pressure + some transfer function to determine an altitude?  And that
>>>>>>> transfer function might be fungible depending on the platform?
>>>>>>>
>>>>>>> If I got the above right (big if) I would probably expect a new
>>>>>>> pressure sensor type to be added that reports a pressure sensor, then
>>>>>>> we'd put the transform code in something that looks a lot like CFM
>>>>>>> sensor (which oddly enough has a hardcoded 0 for altitude in its
>>>>>>> algorithm for systems without pressure sensors).  Considering how
>>>>>>> related a pressure sensor is to altitude, I could see putting them in
>>>>>>> the same application if you wanted;  It might simplify the code some.
>>>>>>>
>>>>>>>
>>>>>>> I think overall a better picture of what you're wanting to accomplish
>>>>>>> would be a good place to start, then we can iterate from there on what
>>>>>>> pieces we need that are new.
>>>>>>
>>>>>> I have Temperature, Pressure, possibly Humidity sensors all which are
>>>>>> variables to different models to compute Altitude from.  I do not have a
>>>>>> true Altitude sensor.
>>>>>
>>>>> This sounds exactly like the CFM sensor, and Exit air temp sensor;
>>>>> Most systems don't have exit air temp sensors, but they have input
>>>>> power and individual fan speeds, which can be put into models to
>>>>> determine CFM and ultimately exit air temperature.  I would expect
>>>>> Altitude to do something very similar in code (although with a
>>>>> completely different algorithm).
>>>>>
>>>>
>>>> So the DPS310 has 2 sensors in it a Pressure and a Temperature sensor.
>>>> Do I create a Pressure reading and a Temperature reading for the DPS310
>>>> and then add Altitude to it as well?
>>>>
>>>> Or do I create 3 separate things,  one for each Pressure, Temperature,
>>>> and Altitude?
>>>
>>> Assuming in this case "things" are intended to mean "entity manager
>>> exposes records" you would create one config record for the DPS310
>>> itself (which would in turn create 2 sensors).  This is one "record"
>>> because physically it's one part, and can't be separated, similar to a
>>> TMP421.  After that, I would create another config record for the
>>> "Here's the math to combine these into an altitude".  It might just be
>>> a type and a name, depending on how many inputs go into the transfer
>>> function to convert pressure+temperature into an altitude.
>>>
>>> If the math to combine into an altitude isn't system specific, I could
>>> be convinced that the math should go into a single record within the
>>> DPS310 exposes and have that live in the daemon itself, but I don't
>>> have enough detail on how these are usually deployed to know that.
>>>
>>
>> I prefer the the 2 record solution, it keeps the DPS310 self-contained.
>> And keeps the Altitude self-contained, to the models can evolve and
>> change; also if every a true altitude sensor were created it would help
>> keep better abstraction from the DPS310.
>>
>>>>
>>>> Also I believe I should be looking to the CFM sensor and Exit air
>>>> temperature sensor as reference examples.
>>>>
>>>>>>
>>>>>> I am being asked to provide Altitude.
>>>>>>
>>>>>> Personally I believe the desired feature is how much cooling a parcel of
>>>>>> air per unit of time.  Thus I would think air Temperature, Humidity, and
>>>>>> Density (probably compute-able from Pressure, but I have not checked on
>>>>>> the specifics) would be the important factors.
>>>>
>>
> 
> Again, I am obviously missing the entire discussion leading to this point, so
> my response may just be noise. If so, my apologies. Anyway, here is my $0.02:
> 
> Altitude can not be measured on its own with a sensor. It can, for example. be
> estimated from air pressure (maybe taking other information or sensors into
> account), or it can be provided (calculated) using GPS. Therefore, "altitude"
> measurements don't belong into the kernel but need to be calculated in user
> space from information which may or may not be provided by the kernel.
> How to do that is outside the scope of kernel development.
> 
> A kernel driver for DPS310 should report pressure and temperature data,
> not anything else that may be derived from it.
> 
> Thanks,
> Guenter
> 

You are correct Guenter, we are not talking kernel space here.
But the user land "driver" dbus-sensors with entity-manager as well this 
is all part of OpenBMC which is built on OpenEmbedded and Yocto.

I believe Ed and I are in agreement with "A kernel driver for DPS310 
should report pressure and temperature data, not anything else that may 
be derived from it."

Thank you for you validation of our conclusion; much appreciated.
(It is far easier when build something for the first time to get done 
correctly.)


-- 
Bruce



  parent reply	other threads:[~2021-06-02 19:01 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-01 15:42 Seeking your opinion on ways to report both Altitude and Pressure sensors for the DPS310 as well as Temperature from dbus-sensors Bruce Mitchell
2021-06-02 15:39 ` Ed Tanous
2021-06-02 15:58   ` Bruce Mitchell
2021-06-02 16:03     ` Ed Tanous
2021-06-02 16:13       ` Bruce Mitchell
2021-06-02 16:21         ` Ed Tanous
2021-06-02 16:34           ` Bruce Mitchell
     [not found]             ` <f1add89a-853d-2ddd-b07e-6345eb54e72b@linux.vnet.ibm.com>
     [not found]               ` <CABXOdTcdkXzmi9qWWZapY19Z7-DsnXsz-yBWbaBqwhLrJpTDVw@mail.gmail.com>
2021-06-02 19:01                 ` Bruce Mitchell [this message]
2021-06-11 16:57             ` Bruce Mitchell

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=c32a844b-1114-b423-7e7d-63821628f919@linux.vnet.ibm.com \
    --to=bruce.mitchell@linux.vnet.ibm.com \
    --cc=eajames@linux.ibm.com \
    --cc=edtanous@google.com \
    --cc=groeck@chromium.org \
    --cc=groeck@google.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=openbmc@lists.ozlabs.org \
    /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).