All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Thomaiyar, Richard Marian" <richard.marian.thomaiyar@linux.intel.com>
To: Deepak Kodihalli <dkodihal@linux.vnet.ibm.com>,
	Patrick Williams <patrick@stwcx.xyz>
Cc: Brad Bishop <bradleyb@fuzziesquirrel.com>,
	James Feist <james.feist@linux.intel.com>,
	"Bhat, Sumanth" <sumanth.bhat@intel.com>,
	"openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>
Subject: Re: D-Bus interface to provide data to entity manager
Date: Fri, 29 May 2020 12:47:13 +0530	[thread overview]
Message-ID: <7e184454-b406-fc81-33e5-e03882743a95@linux.intel.com> (raw)
In-Reply-To: <fac63254-305a-fea2-36ba-a23597688da8@linux.vnet.ibm.com>


On 5/29/2020 10:39 AM, Deepak Kodihalli wrote:
> On 28/05/20 11:35 pm, Patrick Williams wrote:
>> On Thu, May 28, 2020 at 10:12:19PM +0530, Thomaiyar, Richard Marian 
>> wrote:
>>>
>>> On 5/28/2020 5:54 PM, Deepak Kodihalli wrote:
>>>> On 28/05/20 5:33 pm, Patrick Williams wrote:
>>
>>> Why do we need to have 2 different interfaces to represent the same
>>> information for FRU-to-inventory transformational (say ProductName). 
>>> This
>>> will make inventory manager to be updated for every FRU producer?. 
>>> Many of
>>> the properties are common, and we can form a common interface for 
>>> that, and
>>> rest can be maintained in it's specific interface. I understand that 
>>> current
>>> FRU to Entity-manager interface seems to be private, but we must make a
>>> common interface to represent like Product Name, PartNumer, Serial 
>>> Number
>>> etc. (instead of maintaining it in different interface saying IPMI / 
>>> PLDM
>>> Source, with different types). How about?
>
> Richard, I have concerns with this approach. Like I mentioned in one 
> my earlier emails, and Patrick also alludes to below, if you try to 
> make this common (event if it's for a subset of the properties) then 
> you basically arrive at the existing phosphor Inventory interfaces (eg 
> Inventory.Decorator.Asset).
>
> My question in my earlier mail was, if you do such a thing, then why 
> do you even need inventory producers? FruDevice and PLDM could have 
> hosted inventory on their own. If they still rely on the inventory 
> producers (EM and PIM) with this "common interface" approach, then 
> it's basically re-implementation/duplications of the 
> (Inventory.Decorator.Asset like) interface by two processes.

[Richard]: Basically FRU information (either IPMI/PLDM) is needed for 
the inventory producers to expose configuration, which FRU will not 
have. Say, based on FRU Product name, either we will expose 4 temp 
sensor / 2 (Now along with this one, we need to inform the product name 
through Inventory.Decorator.Asset). Now from Redfish point of it, 
Inventory.Decorator is what it uses. This is what i was asking with 2 
options in earlier mail (whether to change or stick with it (recommended)).

>
> The idea is for apps like FruDevice and PLDM, which are aware of a 
> specific FRU format, to host data in *that* format, to be consumed 
> *solely* by inventory producers (like EM and PIM).
>
[Richard]: Yes, but it doesn't need to expose those in that format? Say 
Manufacturer Name, it doesn't mater whether it is coming from PLDM FRU / 
IPMI FRU. Say we have a special handling for a particular manufacture / 
product, then irrespective of inventory producers both can handle the same.

If we have 2 different interface, then inventory producer may need to be 
aware about both and probe it accordingly. FRU producers code must be 
written in such a way that for these common properties it does the 
necessary conversion (Say make manufacturer as string, irrespective of 
any format it read through). Note: Specific stuff, we need to create a 
separate interface (as phosphor-dbus-interface at present doesn't 
support dynamic property addition/deletion). (Tomorrow, if we have any 
other proprietary way of producing FRU data, we can still work with this 
approach, with less or no change for other layers).

> Also note that (as James pointed out in his email), the IPMI FRU 
> format distinguishes Board/Chassis/Product areas. PLDM FRU format does 
> not. So there are differences. If a new FRU format is introduced, then 
> yes we would expect a new interface to show up under 
> Inventory/Source/<FruFormat>
[Richard]: Fru producers should do this conversion. Say Chassis Type 
(Irrespective of what area it comes from it is same). PLDM FRU mostly 
represents the product as a whole, but technically we can point it to 
all the needed using the Fru Record set to the Entity type mapping in 
the PDR record. Accordingly it needs to be exposed.
>
>
>> Yes, I am in favor of common interfaces for this where ever possible.
>>
>> Is there someone that knows the existing FruDevice implementation well
>> enough that can be included in this work to propose common interfaces
>> where it is appropriate?
>>
>>> Inventory/Source/General/Fru  (Maintain common things here Product 
>>> Name.
>>> This can be used by Inventory manager to advertise it (instead of 
>>> searching
>>> it in multiple interfaces/properties))
>>
>> Minor tweak here of 'Source/Common'?  When we have an existing Inventory
>> interface for this information should we mimic what is already in
>> Inventory?  At some point are we trying to be too common that we're
>> effectively reimplementing Inventory instances under a different name?
>>
[Richard]: Yes, currently, FRU to inventory and inventory to upper layer 
is what used. If we want to change it, we need to go with differnt 
option of using FRU to upper layer, but many of existing code will 
require change.

Regards,

Richard

  reply	other threads:[~2020-05-29  7:17 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-19  3:40 Processing PLDM FRU information with entity manager Deepak Kodihalli
2020-05-20 16:28 ` Brad Bishop
2020-05-20 16:56   ` James Feist
2020-05-20 17:35     ` Brad Bishop
2020-05-20 17:46       ` James Feist
2020-05-26 12:56 ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Deepak Kodihalli
2020-05-27 13:50   ` D-Bus interface to provide data to entity manager Thomaiyar, Richard Marian
2020-05-27 13:58     ` Deepak Kodihalli
2020-05-27 15:25       ` Thomaiyar, Richard Marian
2020-05-27 15:46         ` Deepak Kodihalli
2020-05-27 17:19           ` Thomaiyar, Richard Marian
2020-05-28 12:09             ` Patrick Williams
2020-05-28 12:03   ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Patrick Williams
2020-05-28 12:24     ` D-Bus interface to provide data to entity manager Deepak Kodihalli
2020-05-28 16:42       ` Thomaiyar, Richard Marian
2020-05-28 18:05         ` Patrick Williams
2020-05-28 18:31           ` James Feist
2020-05-29  5:11             ` Deepak Kodihalli
2020-05-29  5:09           ` Deepak Kodihalli
2020-05-29  7:17             ` Thomaiyar, Richard Marian [this message]
2020-05-29  7:31               ` Deepak Kodihalli
2020-05-29  9:03                 ` Thomaiyar, Richard Marian
2020-05-29 10:19                   ` Deepak Kodihalli
2020-05-29 10:30                     ` Deepak Kodihalli
2020-05-29 10:33                       ` Thomaiyar, Richard Marian
2020-05-28 15:43     ` D-Bus interface to provide data to entity manager (was: Processing PLDM FRU information with entity manager) Brad Bishop
2020-05-28 18:21       ` D-Bus interface to provide data to entity manager James Feist

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=7e184454-b406-fc81-33e5-e03882743a95@linux.intel.com \
    --to=richard.marian.thomaiyar@linux.intel.com \
    --cc=bradleyb@fuzziesquirrel.com \
    --cc=dkodihal@linux.vnet.ibm.com \
    --cc=james.feist@linux.intel.com \
    --cc=openbmc@lists.ozlabs.org \
    --cc=patrick@stwcx.xyz \
    --cc=sumanth.bhat@intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.