Policy on Tools Posting
Wilfred Smith
wilfredsmith at fb.com
Fri Aug 16 07:57:09 AEST 2019
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 at lists.ozlabs.org wrote:
>
> Send openbmc mailing list submissions to
> openbmc at 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 at lists.ozlabs.org
>
> You can reach the person managing the list at
> openbmc-owner at 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 at fb.com>
> To: Andrew Geissler <geissonator at gmail.com>, Wilfred Smith
> <wilfredsmith at fb.com>
> Cc: "openbmc at lists.ozlabs.org" <openbmc at lists.ozlabs.org>
> Subject: Re: Policy on Tools Posting
> Message-ID: <68929B76-8826-4DAD-A29E-DF7A119D00C5 at 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 at lists.ozlabs.org on behalf of geissonator at gmail.com> wrote:
>
> On Mon, Aug 12, 2019 at 7:58 PM Wilfred Smith <wilfredsmith at 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 at linux.ibm.com>
> To: openbmc at lists.ozlabs.org
> Cc: joel at jms.id.au, Eddie James <eajames at 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 at 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 at 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 at davemloft.net>
> To: terry.s.duncan at linux.intel.com
> Cc: sam at mendozajonas.com, netdev at vger.kernel.org,
> linux-kernel at vger.kernel.org, openbmc at lists.ozlabs.org,
> wak at google.com, joel at jms.id.au
> Subject: Re: [PATCH] net/ncsi: Ensure 32-bit boundary for data cksum
> Message-ID: <20190815.123235.516041583959546572.davem at davemloft.net>
> Content-Type: Text/Plain; charset=us-ascii
>
> From: "Terry S. Duncan" <terry.s.duncan at 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 at 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 at roeck-us.net>
> To: Vijay Khemka <vijaykhemka at fb.com>
> Cc: John Wang <wangzqbj at inspur.com>, "jdelvare at suse.com"
> <jdelvare at suse.com>, "corbet at lwn.net" <corbet at lwn.net>,
> "linux-hwmon at vger.kernel.org" <linux-hwmon at vger.kernel.org>,
> "linux-doc at vger.kernel.org" <linux-doc at vger.kernel.org>,
> "linux-kernel at vger.kernel.org" <linux-kernel at vger.kernel.org>,
> "openbmc at lists.ozlabs.org" <openbmc at lists.ozlabs.org>,
> "duanzhijia01 at inspur.com" <duanzhijia01 at inspur.com>,
> "mine260309 at gmail.com" <mine260309 at gmail.com>, "joel at jms.id.au"
> <joel at jms.id.au>
> Subject: Re: [PATCH v4 2/2] hwmon: pmbus: Add Inspur Power System
> power supply driver
> Message-ID: <20190815194102.GA11916 at 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 at lists.ozlabs.org on behalf of wangzqbj at 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 at 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 at 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
> ***************************************
More information about the openbmc
mailing list