Policy on Tools Posting

Policy on Tools Posting

[Wilfred Smith]

1. Are there guidelines/procedures specific to submitting command line tools and utilities? I have heard that there may be a repository and/or path dedicated to CLI tools.

2. The specific tools I’m concerned with at the moment are a D-Bus enabled version of fruid-util, ipmb-util and sensor-util as they currently exist in the facebook/openbmc repository. Nothing to write home about. I’ve removed anything Facebook-specific.

3. I presume the correct course of action is to submit Gerrit patches for layers under meta-facebook unless instructed otherwise.

Thanks in advance,

Wilfred

Re: Policy on Tools Posting

[Andrew Jeffery]

Hi Wilfred,

On Tue, 13 Aug 2019, at 10:28, Wilfred Smith wrote:
> 
> 1. Are there guidelines/procedures specific to submitting command line 
> tools and utilities? I have heard that there may be a repository and/or 
> path dedicated to CLI tools.

There's not much in the way of commandline tools that are dedicated to
OpenBMC right now. The obvious one is `obmcutil`, but that lives in the
phosphor-state-manager repository which isn't really a spot to add other
assorted tools.

At a real stretch we could put things in openbmc-tools[1], though that's
not really intended for production code, more-so helpers for development
and debugging.

[1] https://github.com/openbmc/openbmc-tools

> 
> 2. The specific tools I’m concerned with at the moment are a D-Bus 
> enabled version of fruid-util

Maybe this could go in phosphor-inventory-manager?

>, ipmb-util 

Not really sure where we might put ipmb-util

> and sensor-util

Maybe phosphor-hwmon? Not really sure, just putting out some
straw-person suggestions.

Are there descriptions and example usage documented anywhere? That
would probably help us sort out what the possibilities are.

> as they 
> currently exist in the facebook/openbmc repository.
> Nothing to write 
> home about. I’ve removed anything Facebook-specific.

I guess at this point it just depends on whether you think they would or
could be widely adopted.

> 
> 3. I presume the correct course of action is to submit Gerrit patches 
> for layers under meta-facebook unless instructed otherwise.

You're talking about the recipes here rather than the actual tools? Or
are you proposing putting the source for the tools in meta-facebook?
In general we've tried to avoid that.

Cheers,

Andrew

Re: Policy on Tools Posting

[Wilfred Smith]

> 
> Are there descriptions and example usage documented anywhere? That
> would probably help us sort out what the possibilities are.



The mailing list strips attachments and images, so here’s text from the document I’ve been building for them. I’m still working on sensor-util. 

Once you’ve mulled it over, please let me know where I should put these.

FWIW, I think there will be a few more of these in the near future. They’re designed to be a single-executable (like Busybox) such that NV storage is minimized. There are build options for BZip’ed online help prose and bash completion.

Thanks,

Wilfred

---------------------------------------------
Overview: fruid-util

[img]

fruid-util is a Facebook-originated utility for OpenBMC. It runs on the server’s BMC and allows display of the FRU information for the components of the containing server. The FRU information is typically stored in a non-volatile memory (i.e. EEPROM, NOR/NAND flash, SPI) part within the component.

This utility exclusively reads information that has been made available through the BMC’s D-Bus service named “xyz.openbmc_project.FruDevice”. The FruDevice service may expose multiple objects, each representing a server component. Each of those server component objects exposes some number of properties as key-value pairs. For example, a “motherboard” component” may have its CPU type as a FRU property.

The previous version of this utility allowed the display of the properties for a component when its name was specified. Unfortunately, those names do not always map directly to the name of a server component object. For example, the object name for the motherboard may be “Tioga_Pass_Single_Side”. A mapping file may be specified, allowing the legacy keywords to be mapped to one or more object names based on a regular expression. 

The utility also allows probing for the presence of component object names and property key names with string comparisons accomplished by normal shell features.

Usage

fruid-util [—version] | [—mapFile pathToMapFile] [—perl] [ [—component ] componentexpr] [—property propertyexpr]
 

Exit codes

[img]


Map File Format

The mapping file is a UTF-8 text file containing one or more mapping lines, each terminated by a C standard newline sequence (e.g. CRLF).

Each mapping line consists of a keyword, followed by optional whitespace, an equals sign (‘=‘),optional whitespace and a regular expression that matches the desired component object names. 

If any line begins with a ‘#’ in column 1, it will be ignored and treated as a comment line

Example Output

To the extent reasonable and possible, output matches that of the original fruid-util utility. No attempt is made to generate FRU properties that are unavailable through the FRUDevice service.

fruid-util 

FRU Information					    : Tioga_Pass_Single_Side
———————————————					    : —————————
Address				   		    : 84
Board_Info_AM1					    : 02-000243
Board_Info_AM2					    : LBG-1G
Board_Language_Code				    : 0
Board_Manufacturer				    : Wistro
Board_Manufacture_Date          			: Fri Aug 25 19:17:00 2017
Board_Part_Number               			: B81.00X10.0109
Board_Product_Name						: Tioga Pass Single Side
Board_Serial_Number						: WTH1734038ZSA
Bus										: 6
Chassis_Info_AM1							: M7MX546700026
Chassis_Info_AM2							: _71C9N5700774
Chassis_Part_Number						: B81.00X01.0104
Chassis_Serial_Number						: WTF17370D2MA1
Chassis_Type								: 23
Common_Format_Version						: 1
Product_Asset_Tag						: 3718452
Product_Info_AM1							: 01-002572
Product_Info_AM2							: 1505546857
Product_Language_Code						: 0
Product_Manufacturer						: Wiwynn
Product_Part_Number						: B81.00X01.0104
Product_Product_Name						: Type6 Tioga Pass Single Side
Product_Serial_Number						: WTF17370D2MA1
Product_Version							: PVT

Function Call
Other utilities built on this platform may access this functionality by way of a C function call. The results of the query are written to stdout and are identical to the command line results.

exitCode <- fruidUtil( std::string& componentExpr, std::string& propertyExpr, std::string& mapPath, bool bPerlRegEx = false ); 

Overview: ipmb-util

Ipmb-util allows an IPMB (IPMI bridged) command to be sent to a target with the response written to stdout. 
[img]
Usage

Ipmb-util bus address netfn cmdID [b1 … bn] [—retries = n ] [ —timeout = ms ]
Ipmb-util bus address —file cmdPath [—retries = n ] [ —timeout = ms ]

where:
	bus and address specify the destination for the message
	netfn specifies the net function in hex (e.g. 18 for NetFn 06/LUN 00)
	cmdID specifies the command to be sent (e.g. 01 for GET DEVICE ID)
	retries specifies the number of times the command will be retried, default is 5
	timeout is the maximum delay for a response in milliseconds (default is 3 seconds)
	b1…bn are byte arguments for the command, specified as 2 hex digits per byte with whitespace between bytes

Command File Format
[img]
If a cmdPath is specified, each line of the command file contains 
	netfn cmdID [b1…bn]

where:
	netfn specifies the net function in hex (e.g. 18 for NetFn 06/LUN 00)
	cmdID specifies the command to be sent (e.g. 01 for GET DEVICE ID)
	b1…bn are byte arguments for the command, specified as 2 hex digits per byte with whitespace between bytes

Exit Codes

[img]


Function Call

Other utilities built on this platform may access this functionality by way of a C function call. The results of the query are written to stdout and are identical to the command line results.

exitCode <- ipmbUtil( Byte bus, Byte address, Byte cmdID, std::vector<Byte> abyArgs, unsigned int retryCount = 5, unsigned long timeoutInMs = 3000 );

exitCode <- ipmbUtil( Byte bus, Byte address,std::string& aCmdPath, unsigned int retryCount = 5, unsigned long timeoutInMs = 3000 );

Example Output


[img]

OK 03 02 01 05 10

Re: Policy on Tools Posting

[Andrew Geissler]

On Mon, Aug 12, 2019 at 7:58 PM Wilfred Smith <wilfredsmith@fb.com> wrote:
>
>
> 1. Are there guidelines/procedures specific to submitting command line tools and utilities? I have heard that there may be a repository and/or path dedicated to CLI tools.

The community has definitely tended to limit wrapper tools within
OpenBMC. We had a discussion a while back that we're open to some but
the API's to them really need to be thought out and reviewed because
command line tools become customer API's (i.e. people start writing
scripts on top of these tools that then become key to the
manufacturing process or some other critical area).

Anything that goes into OpenBMC needs to support OpenBMC interfaces.
For example, I'm not familiar with fruid-util's D-bus service
xyz.openbmc_project.FruDevice. A "busctl tree
xyz.openbmc_project.Inventory.Manager | cat" shows the inventory items
on OpenBMC.

One issue we have within OpenBMC is there may be different
implementations of the D-bus API's for a given area. For example,
Inventory has different implementations so I'm not sure which repo
would best fit your tool. That type of issue leads me to wonder if we
should put the tools with the interface definitions in
openbmc/phosphor-dbus-interfaces? Or maybe a separate phosphor-tools
repo would be more logical for these.

Either way, I think command line tools should each get their own
mini-design doc
(https://github.com/openbmc/docs/blob/master/designs/design-template.md)
with requirements and interfaces clearly defined for review by the
community. If we can find a generic tool that multiple people find
useful, we can then find a place to put it. Otherwise, you could host
your tools outside of openbmc/ github and just pull them into recipes
from within your meta-facebook layer.

> Thanks in advance,
>
> Wilfred

Re: Policy on Tools Posting

[Wilfred Smith]

To what extent Is this an actionable “ruling” vs “thoughts on the matter.” 

While large vendors can easily assign someone to create utilities like these, won’t less well staffed vendors be served by having a collection of utilities to choose from? If nothing else, I would think just having the examples available for their adaptation adds significant value. Should everyone need to write their own utility to send a message to ME?

Isn’t FruDevice a well-adopted interface? IpmbBridge? 

I suppose I’m unclear on the project audience, so please guide me. Are we providing hobbyist parts that a billion-dollar enterprise with person-hours to burn can use to create a production system or a near-complete solution that requires a few days of “roughing in” and customization by a three-person team in East Canberra? Is OpenBMC more minimalist “Alpine Linux” or fully loaded, ready-to-go “Ubuntu”?

What’s the dividing line between “Core OpenBMC” and “stuff you need to find or build yourself” where the OpenBMC community is aware, but hands-off?

Pardon me if there’s a document somewhere that states this, in which case a link would be appreciated.

Is the safest bet for the moment to follow your mini-design doc path and host in Facebook’s public Github as described in your last paragraph?

Again, pardon my ignorance as a latecomer here.

Wilfred

> On Aug 15, 2019, at 5:55 AM, Andrew Geissler <geissonator@gmail.com> wrote:
> 
> On Mon, Aug 12, 2019 at 7:58 PM Wilfred Smith <wilfredsmith@fb.com> wrote:
>> 
>> 
>> 1. Are there guidelines/procedures specific to submitting command line tools and utilities? I have heard that there may be a repository and/or path dedicated to CLI tools.
> 
> The community has definitely tended to limit wrapper tools within
> OpenBMC. We had a discussion a while back that we're open to some but
> the API's to them really need to be thought out and reviewed because
> command line tools become customer API's (i.e. people start writing
> scripts on top of these tools that then become key to the
> manufacturing process or some other critical area).
> 
> Anything that goes into OpenBMC needs to support OpenBMC interfaces.
> For example, I'm not familiar with fruid-util's D-bus service
> xyz.openbmc_project.FruDevice. A "busctl tree
> xyz.openbmc_project.Inventory.Manager | cat" shows the inventory items
> on OpenBMC.
> 
> One issue we have within OpenBMC is there may be different
> implementations of the D-bus API's for a given area. For example,
> Inventory has different implementations so I'm not sure which repo
> would best fit your tool. That type of issue leads me to wonder if we
> should put the tools with the interface definitions in
> openbmc/phosphor-dbus-interfaces? Or maybe a separate phosphor-tools
> repo would be more logical for these.
> 
> Either way, I think command line tools should each get their own
> mini-design doc
> (https://github.com/openbmc/docs/blob/master/designs/design-template.md)
> with requirements and interfaces clearly defined for review by the
> community. If we can find a generic tool that multiple people find
> useful, we can then find a place to put it. Otherwise, you could host
> your tools outside of openbmc/ github and just pull them into recipes
> from within your meta-facebook layer.
> 
>> Thanks in advance,
>> 
>> Wilfred

Re: Policy on Tools Posting

[Vijay Khemka]

On 8/15/19, 5:59 AM, "openbmc on behalf of Andrew Geissler" <openbmc-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of geissonator@gmail.com> wrote:

    On Mon, Aug 12, 2019 at 7:58 PM Wilfred Smith <wilfredsmith@fb.com> wrote:
    >
    >
    > 1. Are there guidelines/procedures specific to submitting command line tools and utilities? I have heard that there may be a repository and/or path dedicated to CLI tools.
    
    The community has definitely tended to limit wrapper tools within
    OpenBMC. We had a discussion a while back that we're open to some but
    the API's to them really need to be thought out and reviewed because
    command line tools become customer API's (i.e. people start writing
    scripts on top of these tools that then become key to the
    manufacturing process or some other critical area).
    
    Anything that goes into OpenBMC needs to support OpenBMC interfaces.
    For example, I'm not familiar with fruid-util's D-bus service
    xyz.openbmc_project.FruDevice. A "busctl tree
    xyz.openbmc_project.Inventory.Manager | cat" shows the inventory items
    on OpenBMC.
    
    One issue we have within OpenBMC is there may be different
    implementations of the D-bus API's for a given area. For example,
    Inventory has different implementations so I'm not sure which repo
    would best fit your tool. That type of issue leads me to wonder if we
    should put the tools with the interface definitions in
    openbmc/phosphor-dbus-interfaces? Or maybe a separate phosphor-tools
    repo would be more logical for these.
Andrew, I like the idea of having phosphor-tools which can be a placeholder 
for any commandline tools and can grow as per requirement. Currently
it can start with 3 proposed tools. 

    Either way, I think command line tools should each get their own
    mini-design doc
    (https://github.com/openbmc/docs/blob/master/designs/design-template.md)
Wilfred, Can you please create a document as per this design template and submit for review.

    with requirements and interfaces clearly defined for review by the
    community. If we can find a generic tool that multiple people find
    useful, we can then find a place to put it. Otherwise, you could host
    your tools outside of openbmc/ github and just pull them into recipes
    from within your meta-facebook layer.
    
    > Thanks in advance,
    >
    > Wilfred
    

Re: Policy on Tools Posting

[Andrew Jeffery]

> Is OpenBMC more 
> minimalist “Alpine Linux” or fully loaded, ready-to-go “Ubuntu”?

OpenBMC is a meta distribution, in the same way that Yocto is ("The Yocto
Project. It's not an embedded Linux distribution, it creates a custom one for
you").

OpenBMC is intended to be (heavily?) adapted to the requirements of your
environment. It exists to provide a set of tools that you can pick and choose
from in developing your environment (platform)-specific BMC distribution.
The hope is that at least some of what the project provides is useful to you,
and that we can work together on making better tools for everyone in the
community.

> 
> What’s the dividing line between “Core OpenBMC” and “stuff you need to 
> find or build yourself” where the OpenBMC community is aware, but 
> hands-off?

There isn't really a line, and that's somewhat the point. I think the reality
is the project is a lot more fluid in that regard than you're expecting it to be.

We're helping each-other out by coming to an understanding of everyone's
requirements and trying to develop interfaces and tools on the common
ground. This way we can leverage our collective experience and
development efforts.

Andrew

Re: Policy on Tools Posting

[Vijay Khemka]

On 8/16/19, 12:58 PM, "openbmc on behalf of Andrew Geissler" <openbmc-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of geissonator@gmail.com> wrote:

    On Thu, Aug 15, 2019 at 6:03 PM Joseph Reynolds <jrey@linux.ibm.com> wrote:
    >
    >
    > On 8/15/19 4:57 PM, Wilfred Smith wrote:
    > > My manager (Sai) is asking whether there is precedence for having utilities posted outside the OpenBMC repository. Do we want 100 OpenBMC tools repositories, each managed differently or 1 harmonized repository?
    > >
    > > Separately, is there any effort to create a “common core” for OpenBMC such that an effort akin to POSIX or the Single UNIX Specification isn’t needed ten years from now? Without standard API (or at least abstracted tools) for things like where FRU information is located or sending IPMB commands, isn’t the market for innovative software stifled (Android software market vs iOS, or even Linux vs Windows)?
    >
    > My view is to focus on enhancing the Redfish functions so that users of
    > OpenBMC systems can do everything they need to without having use Secure
    > Shell (ssh) or any of the command line utilities ssh can access (such as
    > systemctl, busctl, or obmctool).  See some publicly-readable IBM
    > discussion on this topic here: https://github.com/ibm-openbmc/dev/issues/612
    
    Once a system has shipped to a customer, ensuring 99% of what they need
    to use that server is available via Redfish definitely makes sense to me.
    
    > Meanwhile, when us developers need to access the BMC via ssh, we are
    > happy to use the existing command line utilities and don't see the need
    > for any more.
    
    Hmm, I don't think I agree with this. I think obmcutil has provided a
    lot of useful
    abstraction for developers and users of OpenBMC (like our lab team).
    No one really wants to remember some of the long cumbersome commands
    done by obmcutil, nor do they want to deal with situations where we change
    the underlying D-bus API in a certain version of firmware. obmcutil provides a
    nice abstraction to that and I could see tools that provide abstractions for
    looking at error logs or listing system inventory could also be useful. I don't
    think these tools should ever be things people purchasing OpenBMC based
    products count on using (but they could still be there if they wanted).

I agree with Andrew here. There are many instances where simple commandline 
utility helps in debugging and quickly testing rather than searching through all long 
commands.

I would prefer to have these ipmb, fru and sensor utils to be hosted in the repo 
Where obmcutil are. I couldn't find repo for obmcutil.
    
    >
    > I would prefer to see our energy focused on enhancements to Redfish and
    > on utilities which use the Redfish APIs (akin to redfishtool).
    >
    > - Joseph
    >
    > >
    > > Wilfred
    > >
    > >> On Aug 15, 2019, at 12:41 PM, openbmc-request@lists.ozlabs.org wrote:
    > >>
    > >> Send openbmc mailing list submissions to
    > >>      openbmc@lists.ozlabs.org
    > >>
    > >> To subscribe or unsubscribe via the World Wide Web, visit
    > >>      https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.ozlabs.org_listinfo_openbmc&d=DwICAg&c=5VD0RTtNlTh3ycd41b3MUw&r=-ektT-tD9zf2rfUisE63RqiDagGyhGey2hbEGa-47kc&m=CsXIqDLC_9ZYrVSwNllcHo7GjqAG9mj2S6NymPQTblk&s=_RrYmmMf-6XU6r5LsXsWLj8G0K_qaWyo6K2yDU5JGu8&e=
    > >> or, via email, send a message with subject or body 'help' to
    > >>      openbmc-request@lists.ozlabs.org
    > >>
    > >> You can reach the person managing the list at
    > >>      openbmc-owner@lists.ozlabs.org
    > >>
    > >> When replying, please edit your Subject line so it is more specific
    > >> than "Re: Contents of openbmc digest..."
    > >>
    > >>
    > >> Today's Topics:
    > >>
    > >>    1. Re: Policy on Tools Posting (Vijay Khemka)
    > >>    2. [PATCH dev-5.2] fsi: scom: Don't abort operations for minor
    > >>       errors (Eddie James)
    > >>    3. Re: [PATCH] net/ncsi: Ensure 32-bit boundary for data cksum
    > >>       (David Miller)
    > >>    4. Re: [PATCH v4 2/2] hwmon: pmbus: Add Inspur Power System
    > >>       power supply driver (Guenter Roeck)
    > >>
    > >>
    > >> ----------------------------------------------------------------------
    > >>
    > >> Message: 1
    > >> Date: Thu, 15 Aug 2019 19:07:26 +0000
    > >> From: Vijay Khemka <vijaykhemka@fb.com>
    > >> To: Andrew Geissler <geissonator@gmail.com>, Wilfred Smith
    > >>      <wilfredsmith@fb.com>
    > >> Cc: "openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>
    > >> Subject: Re: Policy on Tools Posting
    > >> Message-ID: <68929B76-8826-4DAD-A29E-DF7A119D00C5@fb.com>
    > >> Content-Type: text/plain; charset="utf-8"
    > >>
    > >>
    > >>
    > >> ?On 8/15/19, 5:59 AM, "openbmc on behalf of Andrew Geissler" <openbmc-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of geissonator@gmail.com> wrote:
    > >>
    > >>     On Mon, Aug 12, 2019 at 7:58 PM Wilfred Smith <wilfredsmith@fb.com> wrote:
    > >>>
    > >>> 1. Are there guidelines/procedures specific to submitting command line tools and utilities? I have heard that there may be a repository and/or path dedicated to CLI tools.
    > >>     The community has definitely tended to limit wrapper tools within
    > >>     OpenBMC. We had a discussion a while back that we're open to some but
    > >>     the API's to them really need to be thought out and reviewed because
    > >>     command line tools become customer API's (i.e. people start writing
    > >>     scripts on top of these tools that then become key to the
    > >>     manufacturing process or some other critical area).
    > >>
    > >>     Anything that goes into OpenBMC needs to support OpenBMC interfaces.
    > >>     For example, I'm not familiar with fruid-util's D-bus service
    > >>     xyz.openbmc_project.FruDevice. A "busctl tree
    > >>     xyz.openbmc_project.Inventory.Manager | cat" shows the inventory items
    > >>     on OpenBMC.
    > >>
    > >>     One issue we have within OpenBMC is there may be different
    > >>     implementations of the D-bus API's for a given area. For example,
    > >>     Inventory has different implementations so I'm not sure which repo
    > >>     would best fit your tool. That type of issue leads me to wonder if we
    > >>     should put the tools with the interface definitions in
    > >>     openbmc/phosphor-dbus-interfaces? Or maybe a separate phosphor-tools
    > >>     repo would be more logical for these.
    > >> Andrew, I like the idea of having phosphor-tools which can be a placeholder
    > >> for any commandline tools and can grow as per requirement. Currently
    > >> it can start with 3 proposed tools.
    > >>
    > >>     Either way, I think command line tools should each get their own
    > >>     mini-design doc
    > >>     (https://github.com/openbmc/docs/blob/master/designs/design-template.md)
    > >> Wilfred, Can you please create a document as per this design template and submit for review.
    > >>
    > >>     with requirements and interfaces clearly defined for review by the
    > >>     community. If we can find a generic tool that multiple people find
    > >>     useful, we can then find a place to put it. Otherwise, you could host
    > >>     your tools outside of openbmc/ github and just pull them into recipes
    > >>     from within your meta-facebook layer.
    > >>
    > >>> Thanks in advance,
    > >>>
    > >>> Wilfred
    > >
    > ...snip...
    >
    

Re: Policy on Tools Posting

[Andrew Jeffery]

Hi Wilfred,

On Fri, 16 Aug 2019, at 07:28, Wilfred Smith wrote:
> My manager (Sai) is asking whether there is precedence for having 
> utilities posted outside the OpenBMC repository.

As in, outside of the github organisation? There's nothing to stop you from
doing so (obviously we pull in a lot of software not directly written by the
project), however the thing to consider is whether the tools are generally
useful to the openbmc community. If they are then it's probably best that
we host them under the github org.

> Do we want 100 OpenBMC 
> tools repositories, each managed differently or 1 harmonized repository?

The answer to this is largely down to who maintains what and what their
preferences are.

> 
> Separately, is there any effort to create a “common core” for OpenBMC 
> such that an effort akin to POSIX or the Single UNIX Specification 
> isn’t needed ten years from now? Without standard API (or at least 
> abstracted tools) for things like where FRU information is located or 
> sending IPMB commands, isn’t the market for innovative software stifled 
> (Android software market vs iOS, or even Linux vs Windows)? 

Developing a standard set of interfaces is the aim of phosphor-dbus-interfaces.
We're exploring what's necessary in a bit of an evolutionary way rather than
top-down specification. I think that makes the result more flexible and open
to input based on different use-cases.

> 
> Wilfred

Finally, below is the content of a list digest. I have a few requests for your
future posts:

* Please reply in-line to (i.e. below) the points to which you are responding,
on a point-by-point basis. This makes the conversation much easier to follow.

* Try to trim content that's not relevant to the current thread of conversation.
This keeps your emails concise (I haven't trimmed the digest below because
it serves to support my point here).

* It looks like you've responded to a digest email from the list that happens to
include the thread that you yourself started. Please respond on the thread
itself, as responding to the digest splinters the conversation (mail clients that
support threading won't properly track the responses here).

Cheers

Andrew

> 
> > On Aug 15, 2019, at 12:41 PM, openbmc-request@lists.ozlabs.org wrote:
> > 
> > Send openbmc mailing list submissions to
> > 	openbmc@lists.ozlabs.org
> > 
> > To subscribe or unsubscribe via the World Wide Web, visit
> > 	https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.ozlabs.org_listinfo_openbmc&d=DwICAg&c=5VD0RTtNlTh3ycd41b3MUw&r=-ektT-tD9zf2rfUisE63RqiDagGyhGey2hbEGa-47kc&m=CsXIqDLC_9ZYrVSwNllcHo7GjqAG9mj2S6NymPQTblk&s=_RrYmmMf-6XU6r5LsXsWLj8G0K_qaWyo6K2yDU5JGu8&e= 
> > or, via email, send a message with subject or body 'help' to
> > 	openbmc-request@lists.ozlabs.org
> > 
> > You can reach the person managing the list at
> > 	openbmc-owner@lists.ozlabs.org
> > 
> > When replying, please edit your Subject line so it is more specific
> > than "Re: Contents of openbmc digest..."
> > 
> > 
> > Today's Topics:
> > 
> >   1. Re: Policy on Tools Posting (Vijay Khemka)
> >   2. [PATCH dev-5.2] fsi: scom: Don't abort operations for minor
> >      errors (Eddie James)
> >   3. Re: [PATCH] net/ncsi: Ensure 32-bit boundary for data cksum
> >      (David Miller)
> >   4. Re: [PATCH v4 2/2] hwmon: pmbus: Add Inspur Power System
> >      power supply driver (Guenter Roeck)
> > 
> > 
> > ----------------------------------------------------------------------
> > 
> > Message: 1
> > Date: Thu, 15 Aug 2019 19:07:26 +0000
> > From: Vijay Khemka <vijaykhemka@fb.com>
> > To: Andrew Geissler <geissonator@gmail.com>, Wilfred Smith
> > 	<wilfredsmith@fb.com>
> > Cc: "openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>
> > Subject: Re: Policy on Tools Posting
> > Message-ID: <68929B76-8826-4DAD-A29E-DF7A119D00C5@fb.com>
> > Content-Type: text/plain; charset="utf-8"
> > 
> > 
> > 
> > ?On 8/15/19, 5:59 AM, "openbmc on behalf of Andrew Geissler" <openbmc-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of geissonator@gmail.com> wrote:
> > 
> >    On Mon, Aug 12, 2019 at 7:58 PM Wilfred Smith <wilfredsmith@fb.com> wrote:
> >> 
> >> 
> >> 1. Are there guidelines/procedures specific to submitting command line tools and utilities? I have heard that there may be a repository and/or path dedicated to CLI tools.
> > 
> >    The community has definitely tended to limit wrapper tools within
> >    OpenBMC. We had a discussion a while back that we're open to some but
> >    the API's to them really need to be thought out and reviewed because
> >    command line tools become customer API's (i.e. people start writing
> >    scripts on top of these tools that then become key to the
> >    manufacturing process or some other critical area).
> > 
> >    Anything that goes into OpenBMC needs to support OpenBMC interfaces.
> >    For example, I'm not familiar with fruid-util's D-bus service
> >    xyz.openbmc_project.FruDevice. A "busctl tree
> >    xyz.openbmc_project.Inventory.Manager | cat" shows the inventory items
> >    on OpenBMC.
> > 
> >    One issue we have within OpenBMC is there may be different
> >    implementations of the D-bus API's for a given area. For example,
> >    Inventory has different implementations so I'm not sure which repo
> >    would best fit your tool. That type of issue leads me to wonder if we
> >    should put the tools with the interface definitions in
> >    openbmc/phosphor-dbus-interfaces? Or maybe a separate phosphor-tools
> >    repo would be more logical for these.
> > Andrew, I like the idea of having phosphor-tools which can be a placeholder 
> > for any commandline tools and can grow as per requirement. Currently
> > it can start with 3 proposed tools. 
> > 
> >    Either way, I think command line tools should each get their own
> >    mini-design doc
> >    (https://github.com/openbmc/docs/blob/master/designs/design-template.md)
> > Wilfred, Can you please create a document as per this design template and submit for review.
> > 
> >    with requirements and interfaces clearly defined for review by the
> >    community. If we can find a generic tool that multiple people find
> >    useful, we can then find a place to put it. Otherwise, you could host
> >    your tools outside of openbmc/ github and just pull them into recipes
> >    from within your meta-facebook layer.
> > 
> >> Thanks in advance,
> >> 
> >> Wilfred
> > 
> > 
> > 
> > ------------------------------
> > 
> > Message: 2
> > Date: Thu, 15 Aug 2019 14:11:03 -0500
> > From: Eddie James <eajames@linux.ibm.com>
> > To: openbmc@lists.ozlabs.org
> > Cc: joel@jms.id.au, Eddie James <eajames@linux.ibm.com>
> > Subject: [PATCH dev-5.2] fsi: scom: Don't abort operations for minor
> > 	errors
> > Message-ID: <1565896263-26277-1-git-send-email-eajames@linux.ibm.com>
> > 
> > The scom driver currently fails out of operations if certain system
> > errors are flagged in the status register; system checkstop, special
> > attention, or recoverable error. These errors won't impact the ability
> > of the scom engine to perform operations, so the driver should continue
> > under these conditions.
> > Also, don't do a PIB reset for these conditions, since it won't help.
> > 
> > Signed-off-by: Eddie James <eajames@linux.ibm.com>
> > ---
> > drivers/fsi/fsi-scom.c | 8 +-------
> > 1 file changed, 1 insertion(+), 7 deletions(-)
> > 
> > diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c
> > index 343153d..004dc03 100644
> > --- a/drivers/fsi/fsi-scom.c
> > +++ b/drivers/fsi/fsi-scom.c
> > @@ -38,8 +38,7 @@
> > #define SCOM_STATUS_PIB_RESP_MASK	0x00007000
> > #define SCOM_STATUS_PIB_RESP_SHIFT	12
> > 
> > -#define SCOM_STATUS_ANY_ERR		(SCOM_STATUS_ERR_SUMMARY | \
> > -					 SCOM_STATUS_PROTECTION | \
> > +#define SCOM_STATUS_ANY_ERR		(SCOM_STATUS_PROTECTION | \
> > 					 SCOM_STATUS_PARITY |	  \
> > 					 SCOM_STATUS_PIB_ABORT | \
> > 					 SCOM_STATUS_PIB_RESP_MASK)
> > @@ -251,11 +250,6 @@ static int handle_fsi2pib_status(struct scom_device *scom, uint32_t status)
> > 	/* Return -EBUSY on PIB abort to force a retry */
> > 	if (status & SCOM_STATUS_PIB_ABORT)
> > 		return -EBUSY;
> > -	if (status & SCOM_STATUS_ERR_SUMMARY) {
> > -		fsi_device_write(scom->fsi_dev, SCOM_FSI2PIB_RESET_REG, &dummy,
> > -				 sizeof(uint32_t));
> > -		return -EIO;
> > -	}
> > 	return 0;
> > }
> > 
> > -- 
> > 1.8.3.1
> > 
> > 
> > 
> > ------------------------------
> > 
> > Message: 3
> > Date: Thu, 15 Aug 2019 12:32:35 -0700 (PDT)
> > From: David Miller <davem@davemloft.net>
> > To: terry.s.duncan@linux.intel.com
> > Cc: sam@mendozajonas.com, netdev@vger.kernel.org,
> > 	linux-kernel@vger.kernel.org, openbmc@lists.ozlabs.org,
> > 	wak@google.com, joel@jms.id.au
> > Subject: Re: [PATCH] net/ncsi: Ensure 32-bit boundary for data cksum
> > Message-ID: <20190815.123235.516041583959546572.davem@davemloft.net>
> > Content-Type: Text/Plain; charset=us-ascii
> > 
> > From: "Terry S. Duncan" <terry.s.duncan@linux.intel.com>
> > Date: Tue, 13 Aug 2019 18:18:40 -0700
> > 
> >> The NCSI spec indicates that if the data does not end on a 32 bit
> >> boundary, one to three padding bytes equal to 0x00 shall be present to
> >> align the checksum field to a 32-bit boundary.
> >> 
> >> Signed-off-by: Terry S. Duncan <terry.s.duncan@linux.intel.com>
> >> ---
> >> net/ncsi/internal.h |  1 +
> >> net/ncsi/ncsi-cmd.c |  2 +-
> >> net/ncsi/ncsi-rsp.c | 12 ++++++++----
> >> 3 files changed, 10 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> >> index 0b3f0673e1a2..468a19fdfd88 100644
> >> --- a/net/ncsi/internal.h
> >> +++ b/net/ncsi/internal.h
> >> @@ -185,6 +185,7 @@ struct ncsi_package;
> >> #define NCSI_TO_CHANNEL(p, c)	(((p) << NCSI_PACKAGE_SHIFT) | (c))
> >> #define NCSI_MAX_PACKAGE	8
> >> #define NCSI_MAX_CHANNEL	32
> >> +#define NCSI_ROUND32(x)		(((x) + 3) & ~3) /* Round to 32 bit boundary */
> > 
> > I think we have enough of a proliferation of alignment macros, let's not add more.
> > 
> > Either define this to "ALIGN(x, 4)" or expand that into each of the locations:
> > 
> >> 	pchecksum = (__be32 *)((void *)h + sizeof(struct ncsi_pkt_hdr) +
> >> -		    nca->payload);
> >> +		    NCSI_ROUND32(nca->payload));
> > 
> > 		     ALIGN(nca->payload, 4)
> > 
> >> -	pchecksum = (__be32 *)((void *)(h + 1) + payload - 4);
> >> +	pchecksum = (__be32 *)((void *)(h + 1) + NCSI_ROUND32(payload) - 4);
> > 
> > 						 ALIGN(payload, 4)
> > 
> > 
> > etc.
> > 
> > 
> > ------------------------------
> > 
> > Message: 4
> > Date: Thu, 15 Aug 2019 12:41:02 -0700
> > From: Guenter Roeck <linux@roeck-us.net>
> > To: Vijay Khemka <vijaykhemka@fb.com>
> > Cc: John Wang <wangzqbj@inspur.com>, "jdelvare@suse.com"
> > 	<jdelvare@suse.com>, "corbet@lwn.net" <corbet@lwn.net>,
> > 	"linux-hwmon@vger.kernel.org" <linux-hwmon@vger.kernel.org>,
> > 	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
> > 	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
> > 	"openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>,
> > 	"duanzhijia01@inspur.com" <duanzhijia01@inspur.com>,
> > 	"mine260309@gmail.com" <mine260309@gmail.com>, "joel@jms.id.au"
> > 	<joel@jms.id.au>
> > Subject: Re: [PATCH v4 2/2] hwmon: pmbus: Add Inspur Power System
> > 	power supply driver
> > Message-ID: <20190815194102.GA11916@roeck-us.net>
> > Content-Type: text/plain; charset=utf-8
> > 
> > On Thu, Aug 15, 2019 at 06:43:52PM +0000, Vijay Khemka wrote:
> >> 
> >> 
> >> ?On 8/13/19, 1:36 AM, "openbmc on behalf of John Wang" <openbmc-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of wangzqbj@inspur.com> wrote:
> >> 
> >>    Add the driver to monitor Inspur Power System power supplies
> >>    with hwmon over pmbus.
> >> 
> >>    This driver adds sysfs attributes for additional power supply data,
> >>    including vendor, model, part_number, serial number,
> >>    firmware revision, hardware revision, and psu mode(active/standby).
> >> 
> >>    Signed-off-by: John Wang <wangzqbj@inspur.com>
> >>    ---
> >>    v4:
> >>        - Remove the additional tabs in the Makefile
> >>        - Rebased on 5.3-rc4, not 5.2
> >>    v3:
> >>        - Sort kconfig/makefile entries alphabetically
> >>        - Remove unnecessary initialization
> >>        - Use ATTRIBUTE_GROUPS instead of expanding directly
> >>        - Use memscan to avoid reimplementation
> >>    v2:
> >>        - Fix typos in commit message
> >>        - Invert Christmas tree
> >>        - Configure device with sysfs attrs, not debugfs entries
> >>        - Fix errno in fw_version_read, ENODATA to EPROTO
> >>        - Change the print format of fw-version
> >>        - Use sysfs_streq instead of strcmp("xxx" "\n", "xxx")
> >>        - Document sysfs attributes
> >>    ---
> >>     Documentation/hwmon/inspur-ipsps1.rst |  79 +++++++++
> >>     drivers/hwmon/pmbus/Kconfig           |   9 +
> >>     drivers/hwmon/pmbus/Makefile          |   1 +
> >>     drivers/hwmon/pmbus/inspur-ipsps.c    | 226 ++++++++++++++++++++++++++
> >>     4 files changed, 315 insertions(+)
> >>     create mode 100644 Documentation/hwmon/inspur-ipsps1.rst
> >>     create mode 100644 drivers/hwmon/pmbus/inspur-ipsps.c
> >> 
> >>    diff --git a/Documentation/hwmon/inspur-ipsps1.rst b/Documentation/hwmon/inspur-ipsps1.rst
> >>    new file mode 100644
> >>    index 000000000000..aa19f0ccc8b0
> >>    --- /dev/null
> >>    +++ b/Documentation/hwmon/inspur-ipsps1.rst
> >>    @@ -0,0 +1,79 @@
> >>    +Kernel driver inspur-ipsps1
> >>    +=======================
> >>    +
> >>    +Supported chips:
> >>    +
> >>    +  * Inspur Power System power supply unit
> >>    +
> >>    +Author: John Wang <wangzqbj@inspur.com>
> >>    +
> >>    +Description
> >>    +-----------
> >>    +
> >>    +This driver supports Inspur Power System power supplies. This driver
> >>    +is a client to the core PMBus driver.
> >>    +
> >>    +Usage Notes
> >>    +-----------
> >>    +
> >>    +This driver does not auto-detect devices. You will have to instantiate the
> >>    +devices explicitly. Please see Documentation/i2c/instantiating-devices for
> >>    +details.
> >>    +
> >>    +Sysfs entries
> >>    +-------------
> >>    +
> >>    +The following attributes are supported:
> >>    +
> >>    +======================= ======================================================
> >>    +curr1_input             Measured input current
> >>    +curr1_label             "iin"
> >>    +curr1_max               Maximum current
> >>    +curr1_max_alarm         Current high alarm
> >>    +curr2_input		Measured output current in mA.
> >>    +curr2_label		"iout1"
> >>    +curr2_crit              Critical maximum current
> >>    +curr2_crit_alarm        Current critical high alarm
> >>    +curr2_max               Maximum current
> >>    +curr2_max_alarm         Current high alarm
> >>    +
> >> Please align above details.
> >>    +fan1_alarm		Fan 1 warning.
> >>    +fan1_fault		Fan 1 fault.
> >>    +fan1_input		Fan 1 speed in RPM.
> >>    +
> >>    +in1_alarm		Input voltage under-voltage alarm.
> >>    +in1_input		Measured input voltage in mV.
> >>    +in1_label		"vin"
> >>    +in2_input		Measured output voltage in mV.
> >>    +in2_label		"vout1"
> >>    +in2_lcrit               Critical minimum output voltage
> >>    +in2_lcrit_alarm         Output voltage critical low alarm
> >>    +in2_max                 Maximum output voltage
> >>    +in2_max_alarm           Output voltage high alarm
> >>    +in2_min                 Minimum output voltage
> >>    +in2_min_alarm           Output voltage low alarm
> >>    +
> >>    +power1_alarm		Input fault or alarm.
> >>    +power1_input		Measured input power in uW.
> >>    +power1_label		"pin"
> >>    +power1_max              Input power limit
> >>    +power2_max_alarm	Output power high alarm
> >>    +power2_max              Output power limit
> >>    +power2_input		Measured output power in uW.
> >>    +power2_label		"pout"
> >>    +
> >> Same alignment issue in description.
> >>    +temp[1-3]_input		Measured temperature
> >>    +temp[1-2]_max		Maximum temperature
> >>    +temp[1-3]_max_alarm	Temperature high alarm
> >>    +
> >>    +vendor                  Manufacturer name
> >>    +model                   Product model
> >>    +part_number             Product part number
> >>    +serial_number           Product serial number
> >>    +fw_version              Firmware version
> >>    +hw_version              Hardware version
> >>    +mode                    Work mode. Can be set to active or
> >>    +                        standby, when set to standby, PSU will
> >>    +                        automatically switch between standby
> >>    +                        and redundancy mode.
> >>    +======================= ======================================================
> >>    diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> >>    index b6588483fae1..d62d69bb7e49 100644
> >>    --- a/drivers/hwmon/pmbus/Kconfig
> >>    +++ b/drivers/hwmon/pmbus/Kconfig
> >>    @@ -46,6 +46,15 @@ config SENSORS_IBM_CFFPS
> >>     	  This driver can also be built as a module. If so, the module will
> >>     	  be called ibm-cffps.
> >> 
> >>    +config SENSORS_INSPUR_IPSPS
> >>    +	tristate "INSPUR Power System Power Supply"
> >>    +	help
> >>    +	  If you say yes here you get hardware monitoring support for the INSPUR
> >>    +	  Power System power supply.
> >>    +
> >>    +	  This driver can also be built as a module. If so, the module will
> >>    +	  be called inspur-ipsps.
> >>    +
> >>     config SENSORS_IR35221
> >>     	tristate "Infineon IR35221"
> >>     	help
> >>    diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> >>    index c950ea9a5d00..03bacfcfd660 100644
> >>    --- a/drivers/hwmon/pmbus/Makefile
> >>    +++ b/drivers/hwmon/pmbus/Makefile
> >>    @@ -7,6 +7,7 @@ obj-$(CONFIG_PMBUS)		+= pmbus_core.o
> >>     obj-$(CONFIG_SENSORS_PMBUS)	+= pmbus.o
> >>     obj-$(CONFIG_SENSORS_ADM1275)	+= adm1275.o
> >>     obj-$(CONFIG_SENSORS_IBM_CFFPS)	+= ibm-cffps.o
> >>    +obj-$(CONFIG_SENSORS_INSPUR_IPSPS) += inspur-ipsps.o
> >>     obj-$(CONFIG_SENSORS_IR35221)	+= ir35221.o
> >>     obj-$(CONFIG_SENSORS_IR38064)	+= ir38064.o
> >>     obj-$(CONFIG_SENSORS_IRPS5401)	+= irps5401.o
> >>    diff --git a/drivers/hwmon/pmbus/inspur-ipsps.c b/drivers/hwmon/pmbus/inspur-ipsps.c
> >>    new file mode 100644
> >>    index 000000000000..fa981b881a60
> >>    --- /dev/null
> >>    +++ b/drivers/hwmon/pmbus/inspur-ipsps.c
> >>    @@ -0,0 +1,226 @@
> >>    +// SPDX-License-Identifier: GPL-2.0-or-later
> >>    +/*
> >>    + * Copyright 2019 Inspur Corp.
> >>    + */
> >>    +
> >>    +#include <linux/debugfs.h>
> >>    +#include <linux/device.h>
> >>    +#include <linux/fs.h>
> >>    +#include <linux/i2c.h>
> >>    +#include <linux/module.h>
> >>    +#include <linux/pmbus.h>
> >>    +#include <linux/hwmon-sysfs.h>
> >>    +
> >>    +#include "pmbus.h"
> >>    +
> >>    +#define IPSPS_REG_VENDOR_ID	0x99
> >>    +#define IPSPS_REG_MODEL		0x9A
> >>    +#define IPSPS_REG_FW_VERSION	0x9B
> >>    +#define IPSPS_REG_PN		0x9C
> >>    +#define IPSPS_REG_SN		0x9E
> >>    +#define IPSPS_REG_HW_VERSION	0xB0
> >>    +#define IPSPS_REG_MODE		0xFC
> >>    +
> >>    +#define MODE_ACTIVE		0x55
> >>    +#define MODE_STANDBY		0x0E
> >>    +#define MODE_REDUNDANCY		0x00
> >>    +
> >>    +#define MODE_ACTIVE_STRING		"active"
> >>    +#define MODE_STANDBY_STRING		"standby"
> >>    +#define MODE_REDUNDANCY_STRING		"redundancy"
> >>    +
> >>    +enum ipsps_index {
> >>    +	vendor,
> >>    +	model,
> >>    +	fw_version,
> >>    +	part_number,
> >>    +	serial_number,
> >>    +	hw_version,
> >>    +	mode,
> >>    +	num_regs,
> >>    +};
> >>    +
> >>    +static const u8 ipsps_regs[num_regs] = {
> >>    +	[vendor] = IPSPS_REG_VENDOR_ID,
> >>    +	[model] = IPSPS_REG_MODEL,
> >>    +	[fw_version] = IPSPS_REG_FW_VERSION,
> >>    +	[part_number] = IPSPS_REG_PN,
> >>    +	[serial_number] = IPSPS_REG_SN,
> >>    +	[hw_version] = IPSPS_REG_HW_VERSION,
> >>    +	[mode] = IPSPS_REG_MODE,
> >>    +};
> >>    +
> >>    +static ssize_t ipsps_string_show(struct device *dev,
> >>    +				 struct device_attribute *devattr,
> >>    +				 char *buf)
> >>    +{
> >>    +	u8 reg;
> >>    +	int rc;
> >>    +	char *p;
> >>    +	char data[I2C_SMBUS_BLOCK_MAX + 1];
> >>    +	struct i2c_client *client = to_i2c_client(dev->parent);
> >>    +	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> >>    +
> >>    +	reg = ipsps_regs[attr->index];
> >>    +	rc = i2c_smbus_read_block_data(client, reg, data);
> >>    +	if (rc < 0)
> >>    +		return rc;
> >>    +
> >>    +	/* filled with printable characters, ending with # */
> >>    +	p = memscan(data, '#', rc);
> >>    +	*p = '\0';
> >>    +
> >>    +	return snprintf(buf, PAGE_SIZE, "%s\n", data);
> >>    +}
> >>    +
> >>    +static ssize_t ipsps_fw_version_show(struct device *dev,
> >>    +				     struct device_attribute *devattr,
> >>    +				     char *buf)
> >>    +{
> >>    +	u8 reg;
> >>    +	int rc;
> >>    +	u8 data[I2C_SMBUS_BLOCK_MAX] = { 0 };
> >>    +	struct i2c_client *client = to_i2c_client(dev->parent);
> >>    +	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> >>    +
> >>    +	reg = ipsps_regs[attr->index];
> >>    +	rc = i2c_smbus_read_block_data(client, reg, data);
> >>    +	if (rc < 0)
> >>    +		return rc;
> >>    +
> >>    +	if (rc != 6)
> >>    +		return -EPROTO;
> >>    +
> >>    +	return snprintf(buf, PAGE_SIZE, "%u.%02u%u-%u.%02u\n",
> >>    +			data[1], data[2]/* < 100 */, data[3]/*< 10*/,
> >>    +			data[4], data[5]/* < 100 */);
> >>    +}
> >>    +
> >>    +static ssize_t ipsps_mode_show(struct device *dev,
> >>    +			       struct device_attribute *devattr, char *buf)
> >>    +{
> >>    +	u8 reg;
> >>    +	int rc;
> >>    +	struct i2c_client *client = to_i2c_client(dev->parent);
> >>    +	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> >>    +
> >>    +	reg = ipsps_regs[attr->index];
> >>    +	rc = i2c_smbus_read_byte_data(client, reg);
> >>    +	if (rc < 0)
> >>    +		return rc;
> >>    +
> >>    +	switch (rc) {
> >>    +	case MODE_ACTIVE:
> >>    +		return snprintf(buf, PAGE_SIZE, "[%s] %s %s\n",
> >>    +				MODE_ACTIVE_STRING,
> >>    +				MODE_STANDBY_STRING, MODE_REDUNDANCY_STRING);
> >>    +	case MODE_STANDBY:
> >>    +		return snprintf(buf, PAGE_SIZE, "%s [%s] %s\n",
> >>    +				MODE_ACTIVE_STRING,
> >>    +				MODE_STANDBY_STRING, MODE_REDUNDANCY_STRING);
> >>    +	case MODE_REDUNDANCY:
> >>    +		return snprintf(buf, PAGE_SIZE, "%s %s [%s]\n",
> >>    +				MODE_ACTIVE_STRING,
> >>    +				MODE_STANDBY_STRING, MODE_REDUNDANCY_STRING);
> >>    +	default:
> >>    +		return snprintf(buf, PAGE_SIZE, "unspecified\n");
> >>    +	}
> >>    +}
> >>    +
> >>    +static ssize_t ipsps_mode_store(struct device *dev,
> >>    +				struct device_attribute *devattr,
> >>    +				const char *buf, size_t count)
> >>    +{
> >>    +	u8 reg;
> >>    +	int rc;
> >>    +	struct i2c_client *client = to_i2c_client(dev->parent);
> >>    +	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> >>    +
> >>    +	reg = ipsps_regs[attr->index];
> >>    +	if (sysfs_streq(MODE_STANDBY_STRING, buf)) {
> >>    +		rc = i2c_smbus_write_byte_data(client, reg,
> >>    +					       MODE_STANDBY);
> >>    +		if (rc < 0)
> >>    +			return rc;
> >>    +		return count;
> >>    +	} else if (sysfs_streq(MODE_ACTIVE_STRING, buf)) {
> >>    +		rc = i2c_smbus_write_byte_data(client, reg,
> >>    +					       MODE_ACTIVE);
> >>    +		if (rc < 0)
> >>    +			return rc;
> >>    +		return count;
> >>    +	}
> >>    +
> >>    +	return -EINVAL;
> >>    +}
> >>    +
> >>    +static SENSOR_DEVICE_ATTR_RO(vendor, ipsps_string, vendor);
> >>    +static SENSOR_DEVICE_ATTR_RO(model, ipsps_string, model);
> >>    +static SENSOR_DEVICE_ATTR_RO(part_number, ipsps_string, part_number);
> >>    +static SENSOR_DEVICE_ATTR_RO(serial_number, ipsps_string, serial_number);
> >>    +static SENSOR_DEVICE_ATTR_RO(hw_version, ipsps_string, hw_version);
> >>    +static SENSOR_DEVICE_ATTR_RO(fw_version, ipsps_fw_version, fw_version);
> >>    +static SENSOR_DEVICE_ATTR_RW(mode, ipsps_mode, mode);
> >>    +
> >>    +static struct attribute *ipsps_attrs[] = {
> >>    +	&sensor_dev_attr_vendor.dev_attr.attr,
> >>    +	&sensor_dev_attr_model.dev_attr.attr,
> >>    +	&sensor_dev_attr_part_number.dev_attr.attr,
> >>    +	&sensor_dev_attr_serial_number.dev_attr.attr,
> >>    +	&sensor_dev_attr_hw_version.dev_attr.attr,
> >>    +	&sensor_dev_attr_fw_version.dev_attr.attr,
> >>    +	&sensor_dev_attr_mode.dev_attr.attr,
> >>    +	NULL,
> >>    +};
> >>    +
> >>    +ATTRIBUTE_GROUPS(ipsps);
> >>    +
> >>    +static struct pmbus_driver_info ipsps_info = {
> >>    +	.pages = 1,
> >>    +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT |
> >>    +		PMBUS_HAVE_IIN | PMBUS_HAVE_POUT | PMBUS_HAVE_PIN |
> >>    +		PMBUS_HAVE_FAN12 | PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 |
> >>    +		PMBUS_HAVE_TEMP3 | PMBUS_HAVE_STATUS_VOUT |
> >>    +		PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT |
> >>    +		PMBUS_HAVE_STATUS_TEMP | PMBUS_HAVE_STATUS_FAN12,
> >> This can be dynamic read by chip identify function
> > 
> > PMBUS_SKIP_STATUS_CHECK weakens auto-detetcion to some degree,
> > and auto-detection takes time since it needs to poll all registers
> > to determine if they exist. I don't mind if you insist, but I don't
> > immediately see the benefits.
> > 
> >>    +	.groups = ipsps_groups,
> >>    +};
> >>    +
> >>    +static struct pmbus_platform_data ipsps_pdata = {
> >>    +	.flags = PMBUS_SKIP_STATUS_CHECK,
> >>    +};
> >>    +
> >>    +static int ipsps_probe(struct i2c_client *client,
> >>    +		       const struct i2c_device_id *id)
> >>    +{
> >>    +	client->dev.platform_data = &ipsps_pdata;
> >> Allocate memory for this platform data inside tis function rather than having global variable.
> > 
> > Does that have any value other than consuming more memory
> > if there are multiple instances of the driver ?
> > 
> >>    +	return pmbus_do_probe(client, id, &ipsps_info);
> >>    +}
> >>    +
> >>    +static const struct i2c_device_id ipsps_id[] = {
> >>    +	{ "inspur_ipsps1", 0 },
> >>    +	{}
> >>    +};
> >>    +MODULE_DEVICE_TABLE(i2c, ipsps_id);
> >>    +
> >>    +static const struct of_device_id ipsps_of_match[] = {
> >>    +	{ .compatible = "inspur,ipsps1" },
> >>    +	{}
> >>    +};
> >>    +MODULE_DEVICE_TABLE(of, ipsps_of_match);
> >>    +
> >>    +static struct i2c_driver ipsps_driver = {
> >>    +	.driver = {
> >>    +		.name = "inspur-ipsps",
> >>    +		.of_match_table = ipsps_of_match,
> >>    +	},
> >>    +	.probe = ipsps_probe,
> >>    +	.remove = pmbus_do_remove,
> >>    +	.id_table = ipsps_id,
> >>    +};
> >>    +
> >>    +module_i2c_driver(ipsps_driver);
> >>    +
> >>    +MODULE_AUTHOR("John Wang");
> >>    +MODULE_DESCRIPTION("PMBus driver for Inspur Power System power supplies");
> >>    +MODULE_LICENSE("GPL");
> >>    -- 
> >>    2.17.1
> >> 
> >> 
> >> 
> > 
> > 
> > End of openbmc Digest, Vol 48, Issue 81
> > ***************************************
> 
>

Re: Policy on Tools Posting

[Andrew Geissler]

On Thu, Aug 15, 2019 at 4:57 PM Wilfred Smith <wilfredsmith@fb.com> wrote:
>
> My manager (Sai) is asking whether there is precedence for having utilities posted outside the OpenBMC repository. Do we want 100 OpenBMC tools repositories, each managed differently or 1 harmonized repository?

We do bring in some tools, like for our openpower systems we bring in a pdbg
tool[1]. That tool isn't really specific to OpenBMC though, just a
useful tool that
works within OpenBMC.

The spirit behind creating https://github.com/openbmc/openbmc-tools was to
not have a bunch of the same tools being written by a bunch of different
people. So far it's only been tools run outside of OpenBMC though. I know
AndrewJ mentioned he may be open to using that repo for what's proposed here.
It seems like a better fit then making some other tools repo. To me that's just
confusing when you go to a github project and see multiple repositories with
"tools" in the name.

[1]: https://github.com/openbmc/openbmc/blob/master/meta-openpower/recipes-bsp/pdbg/pdbg_2.2.bb

> Separately, is there any effort to create a “common core” for OpenBMC such that an effort akin to POSIX or the Single UNIX Specification isn’t needed ten years from now? Without standard API (or at least abstracted tools) for things like where FRU information is located or sending IPMB commands, isn’t the market for innovative software stifled (Android software market vs iOS, or even Linux vs Windows)?

Our focus so far has been on standardizing our external interfaces,
i.e. Redfish/IPMI.
There has not been much focus on the internals (i.e. ssh) because it's a lot
of work to version and standardize something and it's not something people
buying OpenBMC machines are very interested in. I think wrapper tools are our
best bet but there's been a lot of back and forth on that as well.

> Wilfred
>

Re: Policy on Tools Posting

[Andrew Geissler]

On Thu, Aug 15, 2019 at 6:03 PM Joseph Reynolds <jrey@linux.ibm.com> wrote:
>
>
> On 8/15/19 4:57 PM, Wilfred Smith wrote:
> > My manager (Sai) is asking whether there is precedence for having utilities posted outside the OpenBMC repository. Do we want 100 OpenBMC tools repositories, each managed differently or 1 harmonized repository?
> >
> > Separately, is there any effort to create a “common core” for OpenBMC such that an effort akin to POSIX or the Single UNIX Specification isn’t needed ten years from now? Without standard API (or at least abstracted tools) for things like where FRU information is located or sending IPMB commands, isn’t the market for innovative software stifled (Android software market vs iOS, or even Linux vs Windows)?
>
> My view is to focus on enhancing the Redfish functions so that users of
> OpenBMC systems can do everything they need to without having use Secure
> Shell (ssh) or any of the command line utilities ssh can access (such as
> systemctl, busctl, or obmctool).  See some publicly-readable IBM
> discussion on this topic here: https://github.com/ibm-openbmc/dev/issues/612

Once a system has shipped to a customer, ensuring 99% of what they need
to use that server is available via Redfish definitely makes sense to me.

> Meanwhile, when us developers need to access the BMC via ssh, we are
> happy to use the existing command line utilities and don't see the need
> for any more.

Hmm, I don't think I agree with this. I think obmcutil has provided a
lot of useful
abstraction for developers and users of OpenBMC (like our lab team).
No one really wants to remember some of the long cumbersome commands
done by obmcutil, nor do they want to deal with situations where we change
the underlying D-bus API in a certain version of firmware. obmcutil provides a
nice abstraction to that and I could see tools that provide abstractions for
looking at error logs or listing system inventory could also be useful. I don't
think these tools should ever be things people purchasing OpenBMC based
products count on using (but they could still be there if they wanted).

>
> I would prefer to see our energy focused on enhancements to Redfish and
> on utilities which use the Redfish APIs (akin to redfishtool).
>
> - Joseph
>
> >
> > Wilfred
> >
> >> On Aug 15, 2019, at 12:41 PM, openbmc-request@lists.ozlabs.org wrote:
> >>
> >> Send openbmc mailing list submissions to
> >>      openbmc@lists.ozlabs.org
> >>
> >> To subscribe or unsubscribe via the World Wide Web, visit
> >>      https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.ozlabs.org_listinfo_openbmc&d=DwICAg&c=5VD0RTtNlTh3ycd41b3MUw&r=-ektT-tD9zf2rfUisE63RqiDagGyhGey2hbEGa-47kc&m=CsXIqDLC_9ZYrVSwNllcHo7GjqAG9mj2S6NymPQTblk&s=_RrYmmMf-6XU6r5LsXsWLj8G0K_qaWyo6K2yDU5JGu8&e=
> >> or, via email, send a message with subject or body 'help' to
> >>      openbmc-request@lists.ozlabs.org
> >>
> >> You can reach the person managing the list at
> >>      openbmc-owner@lists.ozlabs.org
> >>
> >> When replying, please edit your Subject line so it is more specific
> >> than "Re: Contents of openbmc digest..."
> >>
> >>
> >> Today's Topics:
> >>
> >>    1. Re: Policy on Tools Posting (Vijay Khemka)
> >>    2. [PATCH dev-5.2] fsi: scom: Don't abort operations for minor
> >>       errors (Eddie James)
> >>    3. Re: [PATCH] net/ncsi: Ensure 32-bit boundary for data cksum
> >>       (David Miller)
> >>    4. Re: [PATCH v4 2/2] hwmon: pmbus: Add Inspur Power System
> >>       power supply driver (Guenter Roeck)
> >>
> >>
> >> ----------------------------------------------------------------------
> >>
> >> Message: 1
> >> Date: Thu, 15 Aug 2019 19:07:26 +0000
> >> From: Vijay Khemka <vijaykhemka@fb.com>
> >> To: Andrew Geissler <geissonator@gmail.com>, Wilfred Smith
> >>      <wilfredsmith@fb.com>
> >> Cc: "openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>
> >> Subject: Re: Policy on Tools Posting
> >> Message-ID: <68929B76-8826-4DAD-A29E-DF7A119D00C5@fb.com>
> >> Content-Type: text/plain; charset="utf-8"
> >>
> >>
> >>
> >> ?On 8/15/19, 5:59 AM, "openbmc on behalf of Andrew Geissler" <openbmc-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of geissonator@gmail.com> wrote:
> >>
> >>     On Mon, Aug 12, 2019 at 7:58 PM Wilfred Smith <wilfredsmith@fb.com> wrote:
> >>>
> >>> 1. Are there guidelines/procedures specific to submitting command line tools and utilities? I have heard that there may be a repository and/or path dedicated to CLI tools.
> >>     The community has definitely tended to limit wrapper tools within
> >>     OpenBMC. We had a discussion a while back that we're open to some but
> >>     the API's to them really need to be thought out and reviewed because
> >>     command line tools become customer API's (i.e. people start writing
> >>     scripts on top of these tools that then become key to the
> >>     manufacturing process or some other critical area).
> >>
> >>     Anything that goes into OpenBMC needs to support OpenBMC interfaces.
> >>     For example, I'm not familiar with fruid-util's D-bus service
> >>     xyz.openbmc_project.FruDevice. A "busctl tree
> >>     xyz.openbmc_project.Inventory.Manager | cat" shows the inventory items
> >>     on OpenBMC.
> >>
> >>     One issue we have within OpenBMC is there may be different
> >>     implementations of the D-bus API's for a given area. For example,
> >>     Inventory has different implementations so I'm not sure which repo
> >>     would best fit your tool. That type of issue leads me to wonder if we
> >>     should put the tools with the interface definitions in
> >>     openbmc/phosphor-dbus-interfaces? Or maybe a separate phosphor-tools
> >>     repo would be more logical for these.
> >> Andrew, I like the idea of having phosphor-tools which can be a placeholder
> >> for any commandline tools and can grow as per requirement. Currently
> >> it can start with 3 proposed tools.
> >>
> >>     Either way, I think command line tools should each get their own
> >>     mini-design doc
> >>     (https://github.com/openbmc/docs/blob/master/designs/design-template.md)
> >> Wilfred, Can you please create a document as per this design template and submit for review.
> >>
> >>     with requirements and interfaces clearly defined for review by the
> >>     community. If we can find a generic tool that multiple people find
> >>     useful, we can then find a place to put it. Otherwise, you could host
> >>     your tools outside of openbmc/ github and just pull them into recipes
> >>     from within your meta-facebook layer.
> >>
> >>> Thanks in advance,
> >>>
> >>> Wilfred
> >
> ...snip...
>

Re: Policy on Tools Posting

[Joseph Reynolds]

On 8/15/19 4:57 PM, Wilfred Smith wrote:
> My manager (Sai) is asking whether there is precedence for having utilities posted outside the OpenBMC repository. Do we want 100 OpenBMC tools repositories, each managed differently or 1 harmonized repository?
>
> Separately, is there any effort to create a “common core” for OpenBMC such that an effort akin to POSIX or the Single UNIX Specification isn’t needed ten years from now? Without standard API (or at least abstracted tools) for things like where FRU information is located or sending IPMB commands, isn’t the market for innovative software stifled (Android software market vs iOS, or even Linux vs Windows)?

My view is to focus on enhancing the Redfish functions so that users of 
OpenBMC systems can do everything they need to without having use Secure 
Shell (ssh) or any of the command line utilities ssh can access (such as 
systemctl, busctl, or obmctool).  See some publicly-readable IBM 
discussion on this topic here: https://github.com/ibm-openbmc/dev/issues/612

Meanwhile, when us developers need to access the BMC via ssh, we are 
happy to use the existing command line utilities and don't see the need 
for any more.

I would prefer to see our energy focused on enhancements to Redfish and 
on utilities which use the Redfish APIs (akin to redfishtool).

- Joseph

>
> Wilfred
>
>> On Aug 15, 2019, at 12:41 PM, openbmc-request@lists.ozlabs.org wrote:
>>
>> Send openbmc mailing list submissions to
>> 	openbmc@lists.ozlabs.org
>>
>> To subscribe or unsubscribe via the World Wide Web, visit
>> 	https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.ozlabs.org_listinfo_openbmc&d=DwICAg&c=5VD0RTtNlTh3ycd41b3MUw&r=-ektT-tD9zf2rfUisE63RqiDagGyhGey2hbEGa-47kc&m=CsXIqDLC_9ZYrVSwNllcHo7GjqAG9mj2S6NymPQTblk&s=_RrYmmMf-6XU6r5LsXsWLj8G0K_qaWyo6K2yDU5JGu8&e=
>> or, via email, send a message with subject or body 'help' to
>> 	openbmc-request@lists.ozlabs.org
>>
>> You can reach the person managing the list at
>> 	openbmc-owner@lists.ozlabs.org
>>
>> When replying, please edit your Subject line so it is more specific
>> than "Re: Contents of openbmc digest..."
>>
>>
>> Today's Topics:
>>
>>    1. Re: Policy on Tools Posting (Vijay Khemka)
>>    2. [PATCH dev-5.2] fsi: scom: Don't abort operations for minor
>>       errors (Eddie James)
>>    3. Re: [PATCH] net/ncsi: Ensure 32-bit boundary for data cksum
>>       (David Miller)
>>    4. Re: [PATCH v4 2/2] hwmon: pmbus: Add Inspur Power System
>>       power supply driver (Guenter Roeck)
>>
>>
>> ----------------------------------------------------------------------
>>
>> Message: 1
>> Date: Thu, 15 Aug 2019 19:07:26 +0000
>> From: Vijay Khemka <vijaykhemka@fb.com>
>> To: Andrew Geissler <geissonator@gmail.com>, Wilfred Smith
>> 	<wilfredsmith@fb.com>
>> Cc: "openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>
>> Subject: Re: Policy on Tools Posting
>> Message-ID: <68929B76-8826-4DAD-A29E-DF7A119D00C5@fb.com>
>> Content-Type: text/plain; charset="utf-8"
>>
>>
>>
>> ?On 8/15/19, 5:59 AM, "openbmc on behalf of Andrew Geissler" <openbmc-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of geissonator@gmail.com> wrote:
>>
>>     On Mon, Aug 12, 2019 at 7:58 PM Wilfred Smith <wilfredsmith@fb.com> wrote:
>>>
>>> 1. Are there guidelines/procedures specific to submitting command line tools and utilities? I have heard that there may be a repository and/or path dedicated to CLI tools.
>>     The community has definitely tended to limit wrapper tools within
>>     OpenBMC. We had a discussion a while back that we're open to some but
>>     the API's to them really need to be thought out and reviewed because
>>     command line tools become customer API's (i.e. people start writing
>>     scripts on top of these tools that then become key to the
>>     manufacturing process or some other critical area).
>>
>>     Anything that goes into OpenBMC needs to support OpenBMC interfaces.
>>     For example, I'm not familiar with fruid-util's D-bus service
>>     xyz.openbmc_project.FruDevice. A "busctl tree
>>     xyz.openbmc_project.Inventory.Manager | cat" shows the inventory items
>>     on OpenBMC.
>>
>>     One issue we have within OpenBMC is there may be different
>>     implementations of the D-bus API's for a given area. For example,
>>     Inventory has different implementations so I'm not sure which repo
>>     would best fit your tool. That type of issue leads me to wonder if we
>>     should put the tools with the interface definitions in
>>     openbmc/phosphor-dbus-interfaces? Or maybe a separate phosphor-tools
>>     repo would be more logical for these.
>> Andrew, I like the idea of having phosphor-tools which can be a placeholder
>> for any commandline tools and can grow as per requirement. Currently
>> it can start with 3 proposed tools.
>>
>>     Either way, I think command line tools should each get their own
>>     mini-design doc
>>     (https://github.com/openbmc/docs/blob/master/designs/design-template.md)
>> Wilfred, Can you please create a document as per this design template and submit for review.
>>
>>     with requirements and interfaces clearly defined for review by the
>>     community. If we can find a generic tool that multiple people find
>>     useful, we can then find a place to put it. Otherwise, you could host
>>     your tools outside of openbmc/ github and just pull them into recipes
>>     from within your meta-facebook layer.
>>
>>> Thanks in advance,
>>>
>>> Wilfred
>
...snip...

Re: Policy on Tools Posting

[Wilfred Smith]

My manager (Sai) is asking whether there is precedence for having utilities posted outside the OpenBMC repository. Do we want 100 OpenBMC tools repositories, each managed differently or 1 harmonized repository?

Separately, is there any effort to create a “common core” for OpenBMC such that an effort akin to POSIX or the Single UNIX Specification isn’t needed ten years from now? Without standard API (or at least abstracted tools) for things like where FRU information is located or sending IPMB commands, isn’t the market for innovative software stifled (Android software market vs iOS, or even Linux vs Windows)? 

Wilfred

> On Aug 15, 2019, at 12:41 PM, openbmc-request@lists.ozlabs.org wrote:
> 
> Send openbmc mailing list submissions to
> 	openbmc@lists.ozlabs.org
> 
> To subscribe or unsubscribe via the World Wide Web, visit
> 	https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.ozlabs.org_listinfo_openbmc&d=DwICAg&c=5VD0RTtNlTh3ycd41b3MUw&r=-ektT-tD9zf2rfUisE63RqiDagGyhGey2hbEGa-47kc&m=CsXIqDLC_9ZYrVSwNllcHo7GjqAG9mj2S6NymPQTblk&s=_RrYmmMf-6XU6r5LsXsWLj8G0K_qaWyo6K2yDU5JGu8&e= 
> or, via email, send a message with subject or body 'help' to
> 	openbmc-request@lists.ozlabs.org
> 
> You can reach the person managing the list at
> 	openbmc-owner@lists.ozlabs.org
> 
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of openbmc digest..."
> 
> 
> Today's Topics:
> 
>   1. Re: Policy on Tools Posting (Vijay Khemka)
>   2. [PATCH dev-5.2] fsi: scom: Don't abort operations for minor
>      errors (Eddie James)
>   3. Re: [PATCH] net/ncsi: Ensure 32-bit boundary for data cksum
>      (David Miller)
>   4. Re: [PATCH v4 2/2] hwmon: pmbus: Add Inspur Power System
>      power supply driver (Guenter Roeck)
> 
> 
> ----------------------------------------------------------------------
> 
> Message: 1
> Date: Thu, 15 Aug 2019 19:07:26 +0000
> From: Vijay Khemka <vijaykhemka@fb.com>
> To: Andrew Geissler <geissonator@gmail.com>, Wilfred Smith
> 	<wilfredsmith@fb.com>
> Cc: "openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>
> Subject: Re: Policy on Tools Posting
> Message-ID: <68929B76-8826-4DAD-A29E-DF7A119D00C5@fb.com>
> Content-Type: text/plain; charset="utf-8"
> 
> 
> 
> ?On 8/15/19, 5:59 AM, "openbmc on behalf of Andrew Geissler" <openbmc-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of geissonator@gmail.com> wrote:
> 
>    On Mon, Aug 12, 2019 at 7:58 PM Wilfred Smith <wilfredsmith@fb.com> wrote:
>> 
>> 
>> 1. Are there guidelines/procedures specific to submitting command line tools and utilities? I have heard that there may be a repository and/or path dedicated to CLI tools.
> 
>    The community has definitely tended to limit wrapper tools within
>    OpenBMC. We had a discussion a while back that we're open to some but
>    the API's to them really need to be thought out and reviewed because
>    command line tools become customer API's (i.e. people start writing
>    scripts on top of these tools that then become key to the
>    manufacturing process or some other critical area).
> 
>    Anything that goes into OpenBMC needs to support OpenBMC interfaces.
>    For example, I'm not familiar with fruid-util's D-bus service
>    xyz.openbmc_project.FruDevice. A "busctl tree
>    xyz.openbmc_project.Inventory.Manager | cat" shows the inventory items
>    on OpenBMC.
> 
>    One issue we have within OpenBMC is there may be different
>    implementations of the D-bus API's for a given area. For example,
>    Inventory has different implementations so I'm not sure which repo
>    would best fit your tool. That type of issue leads me to wonder if we
>    should put the tools with the interface definitions in
>    openbmc/phosphor-dbus-interfaces? Or maybe a separate phosphor-tools
>    repo would be more logical for these.
> Andrew, I like the idea of having phosphor-tools which can be a placeholder 
> for any commandline tools and can grow as per requirement. Currently
> it can start with 3 proposed tools. 
> 
>    Either way, I think command line tools should each get their own
>    mini-design doc
>    (https://github.com/openbmc/docs/blob/master/designs/design-template.md)
> Wilfred, Can you please create a document as per this design template and submit for review.
> 
>    with requirements and interfaces clearly defined for review by the
>    community. If we can find a generic tool that multiple people find
>    useful, we can then find a place to put it. Otherwise, you could host
>    your tools outside of openbmc/ github and just pull them into recipes
>    from within your meta-facebook layer.
> 
>> Thanks in advance,
>> 
>> Wilfred
> 
> 
> 
> ------------------------------
> 
> Message: 2
> Date: Thu, 15 Aug 2019 14:11:03 -0500
> From: Eddie James <eajames@linux.ibm.com>
> To: openbmc@lists.ozlabs.org
> Cc: joel@jms.id.au, Eddie James <eajames@linux.ibm.com>
> Subject: [PATCH dev-5.2] fsi: scom: Don't abort operations for minor
> 	errors
> Message-ID: <1565896263-26277-1-git-send-email-eajames@linux.ibm.com>
> 
> The scom driver currently fails out of operations if certain system
> errors are flagged in the status register; system checkstop, special
> attention, or recoverable error. These errors won't impact the ability
> of the scom engine to perform operations, so the driver should continue
> under these conditions.
> Also, don't do a PIB reset for these conditions, since it won't help.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
> drivers/fsi/fsi-scom.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c
> index 343153d..004dc03 100644
> --- a/drivers/fsi/fsi-scom.c
> +++ b/drivers/fsi/fsi-scom.c
> @@ -38,8 +38,7 @@
> #define SCOM_STATUS_PIB_RESP_MASK	0x00007000
> #define SCOM_STATUS_PIB_RESP_SHIFT	12
> 
> -#define SCOM_STATUS_ANY_ERR		(SCOM_STATUS_ERR_SUMMARY | \
> -					 SCOM_STATUS_PROTECTION | \
> +#define SCOM_STATUS_ANY_ERR		(SCOM_STATUS_PROTECTION | \
> 					 SCOM_STATUS_PARITY |	  \
> 					 SCOM_STATUS_PIB_ABORT | \
> 					 SCOM_STATUS_PIB_RESP_MASK)
> @@ -251,11 +250,6 @@ static int handle_fsi2pib_status(struct scom_device *scom, uint32_t status)
> 	/* Return -EBUSY on PIB abort to force a retry */
> 	if (status & SCOM_STATUS_PIB_ABORT)
> 		return -EBUSY;
> -	if (status & SCOM_STATUS_ERR_SUMMARY) {
> -		fsi_device_write(scom->fsi_dev, SCOM_FSI2PIB_RESET_REG, &dummy,
> -				 sizeof(uint32_t));
> -		return -EIO;
> -	}
> 	return 0;
> }
> 
> -- 
> 1.8.3.1
> 
> 
> 
> ------------------------------
> 
> Message: 3
> Date: Thu, 15 Aug 2019 12:32:35 -0700 (PDT)
> From: David Miller <davem@davemloft.net>
> To: terry.s.duncan@linux.intel.com
> Cc: sam@mendozajonas.com, netdev@vger.kernel.org,
> 	linux-kernel@vger.kernel.org, openbmc@lists.ozlabs.org,
> 	wak@google.com, joel@jms.id.au
> Subject: Re: [PATCH] net/ncsi: Ensure 32-bit boundary for data cksum
> Message-ID: <20190815.123235.516041583959546572.davem@davemloft.net>
> Content-Type: Text/Plain; charset=us-ascii
> 
> From: "Terry S. Duncan" <terry.s.duncan@linux.intel.com>
> Date: Tue, 13 Aug 2019 18:18:40 -0700
> 
>> The NCSI spec indicates that if the data does not end on a 32 bit
>> boundary, one to three padding bytes equal to 0x00 shall be present to
>> align the checksum field to a 32-bit boundary.
>> 
>> Signed-off-by: Terry S. Duncan <terry.s.duncan@linux.intel.com>
>> ---
>> net/ncsi/internal.h |  1 +
>> net/ncsi/ncsi-cmd.c |  2 +-
>> net/ncsi/ncsi-rsp.c | 12 ++++++++----
>> 3 files changed, 10 insertions(+), 5 deletions(-)
>> 
>> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
>> index 0b3f0673e1a2..468a19fdfd88 100644
>> --- a/net/ncsi/internal.h
>> +++ b/net/ncsi/internal.h
>> @@ -185,6 +185,7 @@ struct ncsi_package;
>> #define NCSI_TO_CHANNEL(p, c)	(((p) << NCSI_PACKAGE_SHIFT) | (c))
>> #define NCSI_MAX_PACKAGE	8
>> #define NCSI_MAX_CHANNEL	32
>> +#define NCSI_ROUND32(x)		(((x) + 3) & ~3) /* Round to 32 bit boundary */
> 
> I think we have enough of a proliferation of alignment macros, let's not add more.
> 
> Either define this to "ALIGN(x, 4)" or expand that into each of the locations:
> 
>> 	pchecksum = (__be32 *)((void *)h + sizeof(struct ncsi_pkt_hdr) +
>> -		    nca->payload);
>> +		    NCSI_ROUND32(nca->payload));
> 
> 		     ALIGN(nca->payload, 4)
> 
>> -	pchecksum = (__be32 *)((void *)(h + 1) + payload - 4);
>> +	pchecksum = (__be32 *)((void *)(h + 1) + NCSI_ROUND32(payload) - 4);
> 
> 						 ALIGN(payload, 4)
> 
> 
> etc.
> 
> 
> ------------------------------
> 
> Message: 4
> Date: Thu, 15 Aug 2019 12:41:02 -0700
> From: Guenter Roeck <linux@roeck-us.net>
> To: Vijay Khemka <vijaykhemka@fb.com>
> Cc: John Wang <wangzqbj@inspur.com>, "jdelvare@suse.com"
> 	<jdelvare@suse.com>, "corbet@lwn.net" <corbet@lwn.net>,
> 	"linux-hwmon@vger.kernel.org" <linux-hwmon@vger.kernel.org>,
> 	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
> 	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
> 	"openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>,
> 	"duanzhijia01@inspur.com" <duanzhijia01@inspur.com>,
> 	"mine260309@gmail.com" <mine260309@gmail.com>, "joel@jms.id.au"
> 	<joel@jms.id.au>
> Subject: Re: [PATCH v4 2/2] hwmon: pmbus: Add Inspur Power System
> 	power supply driver
> Message-ID: <20190815194102.GA11916@roeck-us.net>
> Content-Type: text/plain; charset=utf-8
> 
> On Thu, Aug 15, 2019 at 06:43:52PM +0000, Vijay Khemka wrote:
>> 
>> 
>> ?On 8/13/19, 1:36 AM, "openbmc on behalf of John Wang" <openbmc-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of wangzqbj@inspur.com> wrote:
>> 
>>    Add the driver to monitor Inspur Power System power supplies
>>    with hwmon over pmbus.
>> 
>>    This driver adds sysfs attributes for additional power supply data,
>>    including vendor, model, part_number, serial number,
>>    firmware revision, hardware revision, and psu mode(active/standby).
>> 
>>    Signed-off-by: John Wang <wangzqbj@inspur.com>
>>    ---
>>    v4:
>>        - Remove the additional tabs in the Makefile
>>        - Rebased on 5.3-rc4, not 5.2
>>    v3:
>>        - Sort kconfig/makefile entries alphabetically
>>        - Remove unnecessary initialization
>>        - Use ATTRIBUTE_GROUPS instead of expanding directly
>>        - Use memscan to avoid reimplementation
>>    v2:
>>        - Fix typos in commit message
>>        - Invert Christmas tree
>>        - Configure device with sysfs attrs, not debugfs entries
>>        - Fix errno in fw_version_read, ENODATA to EPROTO
>>        - Change the print format of fw-version
>>        - Use sysfs_streq instead of strcmp("xxx" "\n", "xxx")
>>        - Document sysfs attributes
>>    ---
>>     Documentation/hwmon/inspur-ipsps1.rst |  79 +++++++++
>>     drivers/hwmon/pmbus/Kconfig           |   9 +
>>     drivers/hwmon/pmbus/Makefile          |   1 +
>>     drivers/hwmon/pmbus/inspur-ipsps.c    | 226 ++++++++++++++++++++++++++
>>     4 files changed, 315 insertions(+)
>>     create mode 100644 Documentation/hwmon/inspur-ipsps1.rst
>>     create mode 100644 drivers/hwmon/pmbus/inspur-ipsps.c
>> 
>>    diff --git a/Documentation/hwmon/inspur-ipsps1.rst b/Documentation/hwmon/inspur-ipsps1.rst
>>    new file mode 100644
>>    index 000000000000..aa19f0ccc8b0
>>    --- /dev/null
>>    +++ b/Documentation/hwmon/inspur-ipsps1.rst
>>    @@ -0,0 +1,79 @@
>>    +Kernel driver inspur-ipsps1
>>    +=======================
>>    +
>>    +Supported chips:
>>    +
>>    +  * Inspur Power System power supply unit
>>    +
>>    +Author: John Wang <wangzqbj@inspur.com>
>>    +
>>    +Description
>>    +-----------
>>    +
>>    +This driver supports Inspur Power System power supplies. This driver
>>    +is a client to the core PMBus driver.
>>    +
>>    +Usage Notes
>>    +-----------
>>    +
>>    +This driver does not auto-detect devices. You will have to instantiate the
>>    +devices explicitly. Please see Documentation/i2c/instantiating-devices for
>>    +details.
>>    +
>>    +Sysfs entries
>>    +-------------
>>    +
>>    +The following attributes are supported:
>>    +
>>    +======================= ======================================================
>>    +curr1_input             Measured input current
>>    +curr1_label             "iin"
>>    +curr1_max               Maximum current
>>    +curr1_max_alarm         Current high alarm
>>    +curr2_input		Measured output current in mA.
>>    +curr2_label		"iout1"
>>    +curr2_crit              Critical maximum current
>>    +curr2_crit_alarm        Current critical high alarm
>>    +curr2_max               Maximum current
>>    +curr2_max_alarm         Current high alarm
>>    +
>> Please align above details.
>>    +fan1_alarm		Fan 1 warning.
>>    +fan1_fault		Fan 1 fault.
>>    +fan1_input		Fan 1 speed in RPM.
>>    +
>>    +in1_alarm		Input voltage under-voltage alarm.
>>    +in1_input		Measured input voltage in mV.
>>    +in1_label		"vin"
>>    +in2_input		Measured output voltage in mV.
>>    +in2_label		"vout1"
>>    +in2_lcrit               Critical minimum output voltage
>>    +in2_lcrit_alarm         Output voltage critical low alarm
>>    +in2_max                 Maximum output voltage
>>    +in2_max_alarm           Output voltage high alarm
>>    +in2_min                 Minimum output voltage
>>    +in2_min_alarm           Output voltage low alarm
>>    +
>>    +power1_alarm		Input fault or alarm.
>>    +power1_input		Measured input power in uW.
>>    +power1_label		"pin"
>>    +power1_max              Input power limit
>>    +power2_max_alarm	Output power high alarm
>>    +power2_max              Output power limit
>>    +power2_input		Measured output power in uW.
>>    +power2_label		"pout"
>>    +
>> Same alignment issue in description.
>>    +temp[1-3]_input		Measured temperature
>>    +temp[1-2]_max		Maximum temperature
>>    +temp[1-3]_max_alarm	Temperature high alarm
>>    +
>>    +vendor                  Manufacturer name
>>    +model                   Product model
>>    +part_number             Product part number
>>    +serial_number           Product serial number
>>    +fw_version              Firmware version
>>    +hw_version              Hardware version
>>    +mode                    Work mode. Can be set to active or
>>    +                        standby, when set to standby, PSU will
>>    +                        automatically switch between standby
>>    +                        and redundancy mode.
>>    +======================= ======================================================
>>    diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
>>    index b6588483fae1..d62d69bb7e49 100644
>>    --- a/drivers/hwmon/pmbus/Kconfig
>>    +++ b/drivers/hwmon/pmbus/Kconfig
>>    @@ -46,6 +46,15 @@ config SENSORS_IBM_CFFPS
>>     	  This driver can also be built as a module. If so, the module will
>>     	  be called ibm-cffps.
>> 
>>    +config SENSORS_INSPUR_IPSPS
>>    +	tristate "INSPUR Power System Power Supply"
>>    +	help
>>    +	  If you say yes here you get hardware monitoring support for the INSPUR
>>    +	  Power System power supply.
>>    +
>>    +	  This driver can also be built as a module. If so, the module will
>>    +	  be called inspur-ipsps.
>>    +
>>     config SENSORS_IR35221
>>     	tristate "Infineon IR35221"
>>     	help
>>    diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
>>    index c950ea9a5d00..03bacfcfd660 100644
>>    --- a/drivers/hwmon/pmbus/Makefile
>>    +++ b/drivers/hwmon/pmbus/Makefile
>>    @@ -7,6 +7,7 @@ obj-$(CONFIG_PMBUS)		+= pmbus_core.o
>>     obj-$(CONFIG_SENSORS_PMBUS)	+= pmbus.o
>>     obj-$(CONFIG_SENSORS_ADM1275)	+= adm1275.o
>>     obj-$(CONFIG_SENSORS_IBM_CFFPS)	+= ibm-cffps.o
>>    +obj-$(CONFIG_SENSORS_INSPUR_IPSPS) += inspur-ipsps.o
>>     obj-$(CONFIG_SENSORS_IR35221)	+= ir35221.o
>>     obj-$(CONFIG_SENSORS_IR38064)	+= ir38064.o
>>     obj-$(CONFIG_SENSORS_IRPS5401)	+= irps5401.o
>>    diff --git a/drivers/hwmon/pmbus/inspur-ipsps.c b/drivers/hwmon/pmbus/inspur-ipsps.c
>>    new file mode 100644
>>    index 000000000000..fa981b881a60
>>    --- /dev/null
>>    +++ b/drivers/hwmon/pmbus/inspur-ipsps.c
>>    @@ -0,0 +1,226 @@
>>    +// SPDX-License-Identifier: GPL-2.0-or-later
>>    +/*
>>    + * Copyright 2019 Inspur Corp.
>>    + */
>>    +
>>    +#include <linux/debugfs.h>
>>    +#include <linux/device.h>
>>    +#include <linux/fs.h>
>>    +#include <linux/i2c.h>
>>    +#include <linux/module.h>
>>    +#include <linux/pmbus.h>
>>    +#include <linux/hwmon-sysfs.h>
>>    +
>>    +#include "pmbus.h"
>>    +
>>    +#define IPSPS_REG_VENDOR_ID	0x99
>>    +#define IPSPS_REG_MODEL		0x9A
>>    +#define IPSPS_REG_FW_VERSION	0x9B
>>    +#define IPSPS_REG_PN		0x9C
>>    +#define IPSPS_REG_SN		0x9E
>>    +#define IPSPS_REG_HW_VERSION	0xB0
>>    +#define IPSPS_REG_MODE		0xFC
>>    +
>>    +#define MODE_ACTIVE		0x55
>>    +#define MODE_STANDBY		0x0E
>>    +#define MODE_REDUNDANCY		0x00
>>    +
>>    +#define MODE_ACTIVE_STRING		"active"
>>    +#define MODE_STANDBY_STRING		"standby"
>>    +#define MODE_REDUNDANCY_STRING		"redundancy"
>>    +
>>    +enum ipsps_index {
>>    +	vendor,
>>    +	model,
>>    +	fw_version,
>>    +	part_number,
>>    +	serial_number,
>>    +	hw_version,
>>    +	mode,
>>    +	num_regs,
>>    +};
>>    +
>>    +static const u8 ipsps_regs[num_regs] = {
>>    +	[vendor] = IPSPS_REG_VENDOR_ID,
>>    +	[model] = IPSPS_REG_MODEL,
>>    +	[fw_version] = IPSPS_REG_FW_VERSION,
>>    +	[part_number] = IPSPS_REG_PN,
>>    +	[serial_number] = IPSPS_REG_SN,
>>    +	[hw_version] = IPSPS_REG_HW_VERSION,
>>    +	[mode] = IPSPS_REG_MODE,
>>    +};
>>    +
>>    +static ssize_t ipsps_string_show(struct device *dev,
>>    +				 struct device_attribute *devattr,
>>    +				 char *buf)
>>    +{
>>    +	u8 reg;
>>    +	int rc;
>>    +	char *p;
>>    +	char data[I2C_SMBUS_BLOCK_MAX + 1];
>>    +	struct i2c_client *client = to_i2c_client(dev->parent);
>>    +	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
>>    +
>>    +	reg = ipsps_regs[attr->index];
>>    +	rc = i2c_smbus_read_block_data(client, reg, data);
>>    +	if (rc < 0)
>>    +		return rc;
>>    +
>>    +	/* filled with printable characters, ending with # */
>>    +	p = memscan(data, '#', rc);
>>    +	*p = '\0';
>>    +
>>    +	return snprintf(buf, PAGE_SIZE, "%s\n", data);
>>    +}
>>    +
>>    +static ssize_t ipsps_fw_version_show(struct device *dev,
>>    +				     struct device_attribute *devattr,
>>    +				     char *buf)
>>    +{
>>    +	u8 reg;
>>    +	int rc;
>>    +	u8 data[I2C_SMBUS_BLOCK_MAX] = { 0 };
>>    +	struct i2c_client *client = to_i2c_client(dev->parent);
>>    +	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
>>    +
>>    +	reg = ipsps_regs[attr->index];
>>    +	rc = i2c_smbus_read_block_data(client, reg, data);
>>    +	if (rc < 0)
>>    +		return rc;
>>    +
>>    +	if (rc != 6)
>>    +		return -EPROTO;
>>    +
>>    +	return snprintf(buf, PAGE_SIZE, "%u.%02u%u-%u.%02u\n",
>>    +			data[1], data[2]/* < 100 */, data[3]/*< 10*/,
>>    +			data[4], data[5]/* < 100 */);
>>    +}
>>    +
>>    +static ssize_t ipsps_mode_show(struct device *dev,
>>    +			       struct device_attribute *devattr, char *buf)
>>    +{
>>    +	u8 reg;
>>    +	int rc;
>>    +	struct i2c_client *client = to_i2c_client(dev->parent);
>>    +	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
>>    +
>>    +	reg = ipsps_regs[attr->index];
>>    +	rc = i2c_smbus_read_byte_data(client, reg);
>>    +	if (rc < 0)
>>    +		return rc;
>>    +
>>    +	switch (rc) {
>>    +	case MODE_ACTIVE:
>>    +		return snprintf(buf, PAGE_SIZE, "[%s] %s %s\n",
>>    +				MODE_ACTIVE_STRING,
>>    +				MODE_STANDBY_STRING, MODE_REDUNDANCY_STRING);
>>    +	case MODE_STANDBY:
>>    +		return snprintf(buf, PAGE_SIZE, "%s [%s] %s\n",
>>    +				MODE_ACTIVE_STRING,
>>    +				MODE_STANDBY_STRING, MODE_REDUNDANCY_STRING);
>>    +	case MODE_REDUNDANCY:
>>    +		return snprintf(buf, PAGE_SIZE, "%s %s [%s]\n",
>>    +				MODE_ACTIVE_STRING,
>>    +				MODE_STANDBY_STRING, MODE_REDUNDANCY_STRING);
>>    +	default:
>>    +		return snprintf(buf, PAGE_SIZE, "unspecified\n");
>>    +	}
>>    +}
>>    +
>>    +static ssize_t ipsps_mode_store(struct device *dev,
>>    +				struct device_attribute *devattr,
>>    +				const char *buf, size_t count)
>>    +{
>>    +	u8 reg;
>>    +	int rc;
>>    +	struct i2c_client *client = to_i2c_client(dev->parent);
>>    +	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
>>    +
>>    +	reg = ipsps_regs[attr->index];
>>    +	if (sysfs_streq(MODE_STANDBY_STRING, buf)) {
>>    +		rc = i2c_smbus_write_byte_data(client, reg,
>>    +					       MODE_STANDBY);
>>    +		if (rc < 0)
>>    +			return rc;
>>    +		return count;
>>    +	} else if (sysfs_streq(MODE_ACTIVE_STRING, buf)) {
>>    +		rc = i2c_smbus_write_byte_data(client, reg,
>>    +					       MODE_ACTIVE);
>>    +		if (rc < 0)
>>    +			return rc;
>>    +		return count;
>>    +	}
>>    +
>>    +	return -EINVAL;
>>    +}
>>    +
>>    +static SENSOR_DEVICE_ATTR_RO(vendor, ipsps_string, vendor);
>>    +static SENSOR_DEVICE_ATTR_RO(model, ipsps_string, model);
>>    +static SENSOR_DEVICE_ATTR_RO(part_number, ipsps_string, part_number);
>>    +static SENSOR_DEVICE_ATTR_RO(serial_number, ipsps_string, serial_number);
>>    +static SENSOR_DEVICE_ATTR_RO(hw_version, ipsps_string, hw_version);
>>    +static SENSOR_DEVICE_ATTR_RO(fw_version, ipsps_fw_version, fw_version);
>>    +static SENSOR_DEVICE_ATTR_RW(mode, ipsps_mode, mode);
>>    +
>>    +static struct attribute *ipsps_attrs[] = {
>>    +	&sensor_dev_attr_vendor.dev_attr.attr,
>>    +	&sensor_dev_attr_model.dev_attr.attr,
>>    +	&sensor_dev_attr_part_number.dev_attr.attr,
>>    +	&sensor_dev_attr_serial_number.dev_attr.attr,
>>    +	&sensor_dev_attr_hw_version.dev_attr.attr,
>>    +	&sensor_dev_attr_fw_version.dev_attr.attr,
>>    +	&sensor_dev_attr_mode.dev_attr.attr,
>>    +	NULL,
>>    +};
>>    +
>>    +ATTRIBUTE_GROUPS(ipsps);
>>    +
>>    +static struct pmbus_driver_info ipsps_info = {
>>    +	.pages = 1,
>>    +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT |
>>    +		PMBUS_HAVE_IIN | PMBUS_HAVE_POUT | PMBUS_HAVE_PIN |
>>    +		PMBUS_HAVE_FAN12 | PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 |
>>    +		PMBUS_HAVE_TEMP3 | PMBUS_HAVE_STATUS_VOUT |
>>    +		PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT |
>>    +		PMBUS_HAVE_STATUS_TEMP | PMBUS_HAVE_STATUS_FAN12,
>> This can be dynamic read by chip identify function
> 
> PMBUS_SKIP_STATUS_CHECK weakens auto-detetcion to some degree,
> and auto-detection takes time since it needs to poll all registers
> to determine if they exist. I don't mind if you insist, but I don't
> immediately see the benefits.
> 
>>    +	.groups = ipsps_groups,
>>    +};
>>    +
>>    +static struct pmbus_platform_data ipsps_pdata = {
>>    +	.flags = PMBUS_SKIP_STATUS_CHECK,
>>    +};
>>    +
>>    +static int ipsps_probe(struct i2c_client *client,
>>    +		       const struct i2c_device_id *id)
>>    +{
>>    +	client->dev.platform_data = &ipsps_pdata;
>> Allocate memory for this platform data inside tis function rather than having global variable.
> 
> Does that have any value other than consuming more memory
> if there are multiple instances of the driver ?
> 
>>    +	return pmbus_do_probe(client, id, &ipsps_info);
>>    +}
>>    +
>>    +static const struct i2c_device_id ipsps_id[] = {
>>    +	{ "inspur_ipsps1", 0 },
>>    +	{}
>>    +};
>>    +MODULE_DEVICE_TABLE(i2c, ipsps_id);
>>    +
>>    +static const struct of_device_id ipsps_of_match[] = {
>>    +	{ .compatible = "inspur,ipsps1" },
>>    +	{}
>>    +};
>>    +MODULE_DEVICE_TABLE(of, ipsps_of_match);
>>    +
>>    +static struct i2c_driver ipsps_driver = {
>>    +	.driver = {
>>    +		.name = "inspur-ipsps",
>>    +		.of_match_table = ipsps_of_match,
>>    +	},
>>    +	.probe = ipsps_probe,
>>    +	.remove = pmbus_do_remove,
>>    +	.id_table = ipsps_id,
>>    +};
>>    +
>>    +module_i2c_driver(ipsps_driver);
>>    +
>>    +MODULE_AUTHOR("John Wang");
>>    +MODULE_DESCRIPTION("PMBus driver for Inspur Power System power supplies");
>>    +MODULE_LICENSE("GPL");
>>    -- 
>>    2.17.1
>> 
>> 
>> 
> 
> 
> End of openbmc Digest, Vol 48, Issue 81
> ***************************************