* [PATCH 0/3] pmbus: ibm-cffps: Add support for version 2 of PSU
@ 2019-08-30 16:09 Eddie James
2019-08-30 16:09 ` [PATCH 1/3] dt-bindings: hwmon: Document ibm,cffps2 compatible string Eddie James
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Eddie James @ 2019-08-30 16:09 UTC (permalink / raw)
To: linux-hwmon
Cc: linux-kernel, linux-aspeed, devicetree, linux, andrew, joel,
mark.rutland, robh+dt, jdelvare, Eddie James
Version 2 of this PSU supports a second page of data and changes the
format of the FW version command. Use the devicetree binding (or the I2C
device ID) to determine which version the driver should use. Therefore add
the new compatible string to the devicetree documentation and change the
Swift system devicetree to use version 2.
Eddie James (3):
dt-bindings: hwmon: Document ibm,cffps2 compatible string
ARM: dts: aspeed: swift: Change power supplies to version 2
pmbus: ibm-cffps: Add support for version 2 of the PSU
.../devicetree/bindings/hwmon/ibm,cffps1.txt | 8 +-
arch/arm/boot/dts/aspeed-bmc-opp-swift.dts | 4 +-
drivers/hwmon/pmbus/ibm-cffps.c | 109 ++++++++++++++++-----
3 files changed, 94 insertions(+), 27 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/3] dt-bindings: hwmon: Document ibm,cffps2 compatible string 2019-08-30 16:09 [PATCH 0/3] pmbus: ibm-cffps: Add support for version 2 of PSU Eddie James @ 2019-08-30 16:09 ` Eddie James 2019-08-30 16:09 ` [PATCH 2/3] ARM: dts: aspeed: swift: Change power supplies to version 2 Eddie James 2019-08-30 16:09 ` [PATCH 3/3] pmbus: ibm-cffps: Add support for version 2 of the PSU Eddie James 2 siblings, 0 replies; 7+ messages in thread From: Eddie James @ 2019-08-30 16:09 UTC (permalink / raw) To: linux-hwmon Cc: linux-kernel, linux-aspeed, devicetree, linux, andrew, joel, mark.rutland, robh+dt, jdelvare, Eddie James Document the compatible string for version 2 of the IBM CFFPS PSU. Signed-off-by: Eddie James <eajames@linux.ibm.com> --- Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt b/Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt index f68a0a6..1036f65 100644 --- a/Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt +++ b/Documentation/devicetree/bindings/hwmon/ibm,cffps1.txt @@ -1,8 +1,10 @@ -Device-tree bindings for IBM Common Form Factor Power Supply Version 1 ----------------------------------------------------------------------- +Device-tree bindings for IBM Common Form Factor Power Supply Versions 1 and 2 +----------------------------------------------------------------------------- Required properties: - - compatible = "ibm,cffps1"; + - compatible : Must be one of the following: + "ibm,cffps1" + "ibm,cffps2" - reg = < I2C bus address >; : Address of the power supply on the I2C bus. -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] ARM: dts: aspeed: swift: Change power supplies to version 2 2019-08-30 16:09 [PATCH 0/3] pmbus: ibm-cffps: Add support for version 2 of PSU Eddie James 2019-08-30 16:09 ` [PATCH 1/3] dt-bindings: hwmon: Document ibm,cffps2 compatible string Eddie James @ 2019-08-30 16:09 ` Eddie James 2019-08-30 16:09 ` [PATCH 3/3] pmbus: ibm-cffps: Add support for version 2 of the PSU Eddie James 2 siblings, 0 replies; 7+ messages in thread From: Eddie James @ 2019-08-30 16:09 UTC (permalink / raw) To: linux-hwmon Cc: linux-kernel, linux-aspeed, devicetree, linux, andrew, joel, mark.rutland, robh+dt, jdelvare, Eddie James Swift power supplies are version 2 of the IBM CFFPS. Make it so in the devicetree. Signed-off-by: Eddie James <eajames@linux.ibm.com> --- arch/arm/boot/dts/aspeed-bmc-opp-swift.dts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-swift.dts b/arch/arm/boot/dts/aspeed-bmc-opp-swift.dts index f14f745..815b865 100644 --- a/arch/arm/boot/dts/aspeed-bmc-opp-swift.dts +++ b/arch/arm/boot/dts/aspeed-bmc-opp-swift.dts @@ -494,7 +494,7 @@ }; power-supply@68 { - compatible = "ibm,cffps1"; + compatible = "ibm,cffps2"; reg = <0x68>; }; @@ -504,7 +504,7 @@ }; power-supply@69 { - compatible = "ibm,cffps1"; + compatible = "ibm,cffps2"; reg = <0x69>; }; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] pmbus: ibm-cffps: Add support for version 2 of the PSU 2019-08-30 16:09 [PATCH 0/3] pmbus: ibm-cffps: Add support for version 2 of PSU Eddie James 2019-08-30 16:09 ` [PATCH 1/3] dt-bindings: hwmon: Document ibm,cffps2 compatible string Eddie James 2019-08-30 16:09 ` [PATCH 2/3] ARM: dts: aspeed: swift: Change power supplies to version 2 Eddie James @ 2019-08-30 16:09 ` Eddie James 2019-08-30 17:36 ` Guenter Roeck 2019-09-02 7:49 ` kbuild test robot 2 siblings, 2 replies; 7+ messages in thread From: Eddie James @ 2019-08-30 16:09 UTC (permalink / raw) To: linux-hwmon Cc: linux-kernel, linux-aspeed, devicetree, linux, andrew, joel, mark.rutland, robh+dt, jdelvare, Eddie James Version 2 of the PSU supports a second page of data and changes the format of the FW version. Use the devicetree binding to differentiate between the version the driver should use. Signed-off-by: Eddie James <eajames@linux.ibm.com> --- drivers/hwmon/pmbus/ibm-cffps.c | 109 ++++++++++++++++++++++++++++++++-------- 1 file changed, 87 insertions(+), 22 deletions(-) diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c index ee2ee9e..ca26fbd 100644 --- a/drivers/hwmon/pmbus/ibm-cffps.c +++ b/drivers/hwmon/pmbus/ibm-cffps.c @@ -12,16 +12,20 @@ #include <linux/leds.h> #include <linux/module.h> #include <linux/mutex.h> +#include <linux/of_device.h> #include <linux/pmbus.h> #include "pmbus.h" +#define CFFPS_VERSIONS 2 + #define CFFPS_FRU_CMD 0x9A #define CFFPS_PN_CMD 0x9B #define CFFPS_SN_CMD 0x9E #define CFFPS_CCIN_CMD 0xBD -#define CFFPS_FW_CMD_START 0xFA -#define CFFPS_FW_NUM_BYTES 4 +#define CFFPS_FW_CMD 0xFA +#define CFFPS1_FW_NUM_BYTES 4 +#define CFFPS2_FW_NUM_WORDS 3 #define CFFPS_SYS_CONFIG_CMD 0xDA #define CFFPS_INPUT_HISTORY_CMD 0xD6 @@ -61,6 +65,7 @@ struct ibm_cffps_input_history { }; struct ibm_cffps { + int version; struct i2c_client *client; struct ibm_cffps_input_history input_history; @@ -132,6 +137,8 @@ static ssize_t ibm_cffps_debugfs_op(struct file *file, char __user *buf, struct ibm_cffps *psu = to_psu(idxp, idx); char data[I2C_SMBUS_BLOCK_MAX] = { 0 }; + pmbus_set_page(psu->client, 0); + switch (idx) { case CFFPS_DEBUGFS_INPUT_HISTORY: return ibm_cffps_read_input_history(psu, buf, count, ppos); @@ -152,16 +159,36 @@ static ssize_t ibm_cffps_debugfs_op(struct file *file, char __user *buf, rc = snprintf(data, 5, "%04X", rc); goto done; case CFFPS_DEBUGFS_FW: - for (i = 0; i < CFFPS_FW_NUM_BYTES; ++i) { - rc = i2c_smbus_read_byte_data(psu->client, - CFFPS_FW_CMD_START + i); - if (rc < 0) - return rc; + switch (psu->version) { + case 1: + for (i = 0; i < CFFPS1_FW_NUM_BYTES; ++i) { + rc = i2c_smbus_read_byte_data(psu->client, + CFFPS_FW_CMD + + i); + if (rc < 0) + return rc; + + snprintf(&data[i * 2], 3, "%02X", rc); + } - snprintf(&data[i * 2], 3, "%02X", rc); - } + rc = i * 2; + break; + case 2: + for (i = 0; i < CFFPS2_FW_NUM_WORDS; ++i) { + rc = i2c_smbus_read_word_data(psu->client, + CFFPS_FW_CMD + + i); + if (rc < 0) + return rc; + + snprintf(&data[i * 4], 5, "%04X", rc); + } - rc = i * 2; + rc = i * 4; + break; + default: + return -EOPNOTSUPP; + } goto done; default: return -EINVAL; @@ -279,6 +306,8 @@ static void ibm_cffps_led_brightness_set(struct led_classdev *led_cdev, psu->led_state = CFFPS_LED_ON; } + pmbus_set_page(psu->client, 0); + rc = i2c_smbus_write_byte_data(psu->client, CFFPS_SYS_CONFIG_CMD, psu->led_state); if (rc < 0) @@ -299,6 +328,8 @@ static int ibm_cffps_led_blink_set(struct led_classdev *led_cdev, if (led_cdev->brightness == LED_OFF) return 0; + pmbus_set_page(psu->client, 0); + rc = i2c_smbus_write_byte_data(psu->client, CFFPS_SYS_CONFIG_CMD, CFFPS_LED_BLINK); if (rc < 0) @@ -328,15 +359,32 @@ static void ibm_cffps_create_led_class(struct ibm_cffps *psu) dev_warn(dev, "failed to register led class: %d\n", rc); } -static struct pmbus_driver_info ibm_cffps_info = { - .pages = 1, - .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | - 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, - .read_byte_data = ibm_cffps_read_byte_data, - .read_word_data = ibm_cffps_read_word_data, +static struct pmbus_driver_info ibm_cffps_info[CFFPS_VERSIONS] = { + [0] = { + .pages = 1, + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | + 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, + .read_byte_data = ibm_cffps_read_byte_data, + .read_word_data = ibm_cffps_read_word_data, + }, + [1] = { + .pages = 2, + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | + 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, + .func[1] = PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | + PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 | + PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT, + .read_byte_data = ibm_cffps_read_byte_data, + .read_word_data = ibm_cffps_read_word_data, + }, }; static struct pmbus_platform_data ibm_cffps_pdata = { @@ -346,13 +394,21 @@ static void ibm_cffps_create_led_class(struct ibm_cffps *psu) static int ibm_cffps_probe(struct i2c_client *client, const struct i2c_device_id *id) { - int i, rc; + int i, rc, vs; struct dentry *debugfs; struct dentry *ibm_cffps_dir; struct ibm_cffps *psu; + const void *md = of_device_get_match_data(&client->dev); + + if (md) + vs = (int)md; + else if (id) + vs = (int)id->driver_data; + else + vs = 1; client->dev.platform_data = &ibm_cffps_pdata; - rc = pmbus_do_probe(client, id, &ibm_cffps_info); + rc = pmbus_do_probe(client, id, &ibm_cffps_info[vs - 1]); if (rc) return rc; @@ -364,6 +420,7 @@ static int ibm_cffps_probe(struct i2c_client *client, if (!psu) return 0; + psu->version = vs; psu->client = client; mutex_init(&psu->input_history.update_lock); psu->input_history.last_update = jiffies - HZ; @@ -406,12 +463,20 @@ static int ibm_cffps_probe(struct i2c_client *client, static const struct i2c_device_id ibm_cffps_id[] = { { "ibm_cffps1", 1 }, + { "ibm_cffps2", 2 }, {} }; MODULE_DEVICE_TABLE(i2c, ibm_cffps_id); static const struct of_device_id ibm_cffps_of_match[] = { - { .compatible = "ibm,cffps1" }, + { + .compatible = "ibm,cffps1", + .data = (void *)1 + }, + { + .compatible = "ibm,cffps2", + .data = (void *)2 + }, {} }; MODULE_DEVICE_TABLE(of, ibm_cffps_of_match); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] pmbus: ibm-cffps: Add support for version 2 of the PSU 2019-08-30 16:09 ` [PATCH 3/3] pmbus: ibm-cffps: Add support for version 2 of the PSU Eddie James @ 2019-08-30 17:36 ` Guenter Roeck 2019-08-30 18:19 ` Eddie James 2019-09-02 7:49 ` kbuild test robot 1 sibling, 1 reply; 7+ messages in thread From: Guenter Roeck @ 2019-08-30 17:36 UTC (permalink / raw) To: Eddie James Cc: linux-hwmon, linux-kernel, linux-aspeed, devicetree, andrew, joel, mark.rutland, robh+dt, jdelvare On Fri, Aug 30, 2019 at 11:09:45AM -0500, Eddie James wrote: > Version 2 of the PSU supports a second page of data and changes the > format of the FW version. Use the devicetree binding to differentiate > between the version the driver should use. > > Signed-off-by: Eddie James <eajames@linux.ibm.com> > --- > drivers/hwmon/pmbus/ibm-cffps.c | 109 ++++++++++++++++++++++++++++++++-------- > 1 file changed, 87 insertions(+), 22 deletions(-) > > diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c > index ee2ee9e..ca26fbd 100644 > --- a/drivers/hwmon/pmbus/ibm-cffps.c > +++ b/drivers/hwmon/pmbus/ibm-cffps.c > @@ -12,16 +12,20 @@ > #include <linux/leds.h> > #include <linux/module.h> > #include <linux/mutex.h> > +#include <linux/of_device.h> > #include <linux/pmbus.h> > > #include "pmbus.h" > > +#define CFFPS_VERSIONS 2 > + Any chance you can use an enum for the versions ? Using version numbers 1/2 combined with array indices 0/1 is confusing, error prone, and seems unnecessary. Thanks, Guenter > #define CFFPS_FRU_CMD 0x9A > #define CFFPS_PN_CMD 0x9B > #define CFFPS_SN_CMD 0x9E > #define CFFPS_CCIN_CMD 0xBD > -#define CFFPS_FW_CMD_START 0xFA > -#define CFFPS_FW_NUM_BYTES 4 > +#define CFFPS_FW_CMD 0xFA > +#define CFFPS1_FW_NUM_BYTES 4 > +#define CFFPS2_FW_NUM_WORDS 3 > #define CFFPS_SYS_CONFIG_CMD 0xDA > > #define CFFPS_INPUT_HISTORY_CMD 0xD6 > @@ -61,6 +65,7 @@ struct ibm_cffps_input_history { > }; > > struct ibm_cffps { > + int version; > struct i2c_client *client; > > struct ibm_cffps_input_history input_history; > @@ -132,6 +137,8 @@ static ssize_t ibm_cffps_debugfs_op(struct file *file, char __user *buf, > struct ibm_cffps *psu = to_psu(idxp, idx); > char data[I2C_SMBUS_BLOCK_MAX] = { 0 }; > > + pmbus_set_page(psu->client, 0); > + > switch (idx) { > case CFFPS_DEBUGFS_INPUT_HISTORY: > return ibm_cffps_read_input_history(psu, buf, count, ppos); > @@ -152,16 +159,36 @@ static ssize_t ibm_cffps_debugfs_op(struct file *file, char __user *buf, > rc = snprintf(data, 5, "%04X", rc); > goto done; > case CFFPS_DEBUGFS_FW: > - for (i = 0; i < CFFPS_FW_NUM_BYTES; ++i) { > - rc = i2c_smbus_read_byte_data(psu->client, > - CFFPS_FW_CMD_START + i); > - if (rc < 0) > - return rc; > + switch (psu->version) { > + case 1: > + for (i = 0; i < CFFPS1_FW_NUM_BYTES; ++i) { > + rc = i2c_smbus_read_byte_data(psu->client, > + CFFPS_FW_CMD + > + i); > + if (rc < 0) > + return rc; > + > + snprintf(&data[i * 2], 3, "%02X", rc); > + } > > - snprintf(&data[i * 2], 3, "%02X", rc); > - } > + rc = i * 2; > + break; > + case 2: > + for (i = 0; i < CFFPS2_FW_NUM_WORDS; ++i) { > + rc = i2c_smbus_read_word_data(psu->client, > + CFFPS_FW_CMD + > + i); > + if (rc < 0) > + return rc; > + > + snprintf(&data[i * 4], 5, "%04X", rc); > + } > > - rc = i * 2; > + rc = i * 4; > + break; > + default: > + return -EOPNOTSUPP; > + } > goto done; > default: > return -EINVAL; > @@ -279,6 +306,8 @@ static void ibm_cffps_led_brightness_set(struct led_classdev *led_cdev, > psu->led_state = CFFPS_LED_ON; > } > > + pmbus_set_page(psu->client, 0); > + > rc = i2c_smbus_write_byte_data(psu->client, CFFPS_SYS_CONFIG_CMD, > psu->led_state); > if (rc < 0) > @@ -299,6 +328,8 @@ static int ibm_cffps_led_blink_set(struct led_classdev *led_cdev, > if (led_cdev->brightness == LED_OFF) > return 0; > > + pmbus_set_page(psu->client, 0); > + > rc = i2c_smbus_write_byte_data(psu->client, CFFPS_SYS_CONFIG_CMD, > CFFPS_LED_BLINK); > if (rc < 0) > @@ -328,15 +359,32 @@ static void ibm_cffps_create_led_class(struct ibm_cffps *psu) > dev_warn(dev, "failed to register led class: %d\n", rc); > } > > -static struct pmbus_driver_info ibm_cffps_info = { > - .pages = 1, > - .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | > - 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, > - .read_byte_data = ibm_cffps_read_byte_data, > - .read_word_data = ibm_cffps_read_word_data, > +static struct pmbus_driver_info ibm_cffps_info[CFFPS_VERSIONS] = { > + [0] = { > + .pages = 1, > + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | > + 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, > + .read_byte_data = ibm_cffps_read_byte_data, > + .read_word_data = ibm_cffps_read_word_data, > + }, > + [1] = { > + .pages = 2, > + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | > + 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, > + .func[1] = PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | > + PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 | > + PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT, > + .read_byte_data = ibm_cffps_read_byte_data, > + .read_word_data = ibm_cffps_read_word_data, > + }, > }; > > static struct pmbus_platform_data ibm_cffps_pdata = { > @@ -346,13 +394,21 @@ static void ibm_cffps_create_led_class(struct ibm_cffps *psu) > static int ibm_cffps_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > - int i, rc; > + int i, rc, vs; > struct dentry *debugfs; > struct dentry *ibm_cffps_dir; > struct ibm_cffps *psu; > + const void *md = of_device_get_match_data(&client->dev); > + > + if (md) > + vs = (int)md; > + else if (id) > + vs = (int)id->driver_data; > + else > + vs = 1; > > client->dev.platform_data = &ibm_cffps_pdata; > - rc = pmbus_do_probe(client, id, &ibm_cffps_info); > + rc = pmbus_do_probe(client, id, &ibm_cffps_info[vs - 1]); > if (rc) > return rc; > > @@ -364,6 +420,7 @@ static int ibm_cffps_probe(struct i2c_client *client, > if (!psu) > return 0; > > + psu->version = vs; > psu->client = client; > mutex_init(&psu->input_history.update_lock); > psu->input_history.last_update = jiffies - HZ; > @@ -406,12 +463,20 @@ static int ibm_cffps_probe(struct i2c_client *client, > > static const struct i2c_device_id ibm_cffps_id[] = { > { "ibm_cffps1", 1 }, > + { "ibm_cffps2", 2 }, > {} > }; > MODULE_DEVICE_TABLE(i2c, ibm_cffps_id); > > static const struct of_device_id ibm_cffps_of_match[] = { > - { .compatible = "ibm,cffps1" }, > + { > + .compatible = "ibm,cffps1", > + .data = (void *)1 > + }, > + { > + .compatible = "ibm,cffps2", > + .data = (void *)2 > + }, > {} > }; > MODULE_DEVICE_TABLE(of, ibm_cffps_of_match); > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] pmbus: ibm-cffps: Add support for version 2 of the PSU 2019-08-30 17:36 ` Guenter Roeck @ 2019-08-30 18:19 ` Eddie James 0 siblings, 0 replies; 7+ messages in thread From: Eddie James @ 2019-08-30 18:19 UTC (permalink / raw) To: Guenter Roeck Cc: linux-hwmon, linux-kernel, linux-aspeed, devicetree, andrew, joel, mark.rutland, robh+dt, jdelvare On 8/30/19 12:36 PM, Guenter Roeck wrote: > On Fri, Aug 30, 2019 at 11:09:45AM -0500, Eddie James wrote: >> Version 2 of the PSU supports a second page of data and changes the >> format of the FW version. Use the devicetree binding to differentiate >> between the version the driver should use. >> >> Signed-off-by: Eddie James <eajames@linux.ibm.com> >> --- >> drivers/hwmon/pmbus/ibm-cffps.c | 109 ++++++++++++++++++++++++++++++++-------- >> 1 file changed, 87 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c >> index ee2ee9e..ca26fbd 100644 >> --- a/drivers/hwmon/pmbus/ibm-cffps.c >> +++ b/drivers/hwmon/pmbus/ibm-cffps.c >> @@ -12,16 +12,20 @@ >> #include <linux/leds.h> >> #include <linux/module.h> >> #include <linux/mutex.h> >> +#include <linux/of_device.h> >> #include <linux/pmbus.h> >> >> #include "pmbus.h" >> >> +#define CFFPS_VERSIONS 2 >> + > Any chance you can use an enum for the versions ? Using version > numbers 1/2 combined with array indices 0/1 is confusing, error > prone, and seems unnecessary. Sure, good idea. Thanks, Eddie > > Thanks, > Guenter > >> #define CFFPS_FRU_CMD 0x9A >> #define CFFPS_PN_CMD 0x9B >> #define CFFPS_SN_CMD 0x9E >> #define CFFPS_CCIN_CMD 0xBD >> -#define CFFPS_FW_CMD_START 0xFA >> -#define CFFPS_FW_NUM_BYTES 4 >> +#define CFFPS_FW_CMD 0xFA >> +#define CFFPS1_FW_NUM_BYTES 4 >> +#define CFFPS2_FW_NUM_WORDS 3 >> #define CFFPS_SYS_CONFIG_CMD 0xDA >> >> #define CFFPS_INPUT_HISTORY_CMD 0xD6 >> @@ -61,6 +65,7 @@ struct ibm_cffps_input_history { >> }; >> >> struct ibm_cffps { >> + int version; >> struct i2c_client *client; >> >> struct ibm_cffps_input_history input_history; >> @@ -132,6 +137,8 @@ static ssize_t ibm_cffps_debugfs_op(struct file *file, char __user *buf, >> struct ibm_cffps *psu = to_psu(idxp, idx); >> char data[I2C_SMBUS_BLOCK_MAX] = { 0 }; >> >> + pmbus_set_page(psu->client, 0); >> + >> switch (idx) { >> case CFFPS_DEBUGFS_INPUT_HISTORY: >> return ibm_cffps_read_input_history(psu, buf, count, ppos); >> @@ -152,16 +159,36 @@ static ssize_t ibm_cffps_debugfs_op(struct file *file, char __user *buf, >> rc = snprintf(data, 5, "%04X", rc); >> goto done; >> case CFFPS_DEBUGFS_FW: >> - for (i = 0; i < CFFPS_FW_NUM_BYTES; ++i) { >> - rc = i2c_smbus_read_byte_data(psu->client, >> - CFFPS_FW_CMD_START + i); >> - if (rc < 0) >> - return rc; >> + switch (psu->version) { >> + case 1: >> + for (i = 0; i < CFFPS1_FW_NUM_BYTES; ++i) { >> + rc = i2c_smbus_read_byte_data(psu->client, >> + CFFPS_FW_CMD + >> + i); >> + if (rc < 0) >> + return rc; >> + >> + snprintf(&data[i * 2], 3, "%02X", rc); >> + } >> >> - snprintf(&data[i * 2], 3, "%02X", rc); >> - } >> + rc = i * 2; >> + break; >> + case 2: >> + for (i = 0; i < CFFPS2_FW_NUM_WORDS; ++i) { >> + rc = i2c_smbus_read_word_data(psu->client, >> + CFFPS_FW_CMD + >> + i); >> + if (rc < 0) >> + return rc; >> + >> + snprintf(&data[i * 4], 5, "%04X", rc); >> + } >> >> - rc = i * 2; >> + rc = i * 4; >> + break; >> + default: >> + return -EOPNOTSUPP; >> + } >> goto done; >> default: >> return -EINVAL; >> @@ -279,6 +306,8 @@ static void ibm_cffps_led_brightness_set(struct led_classdev *led_cdev, >> psu->led_state = CFFPS_LED_ON; >> } >> >> + pmbus_set_page(psu->client, 0); >> + >> rc = i2c_smbus_write_byte_data(psu->client, CFFPS_SYS_CONFIG_CMD, >> psu->led_state); >> if (rc < 0) >> @@ -299,6 +328,8 @@ static int ibm_cffps_led_blink_set(struct led_classdev *led_cdev, >> if (led_cdev->brightness == LED_OFF) >> return 0; >> >> + pmbus_set_page(psu->client, 0); >> + >> rc = i2c_smbus_write_byte_data(psu->client, CFFPS_SYS_CONFIG_CMD, >> CFFPS_LED_BLINK); >> if (rc < 0) >> @@ -328,15 +359,32 @@ static void ibm_cffps_create_led_class(struct ibm_cffps *psu) >> dev_warn(dev, "failed to register led class: %d\n", rc); >> } >> >> -static struct pmbus_driver_info ibm_cffps_info = { >> - .pages = 1, >> - .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | >> - 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, >> - .read_byte_data = ibm_cffps_read_byte_data, >> - .read_word_data = ibm_cffps_read_word_data, >> +static struct pmbus_driver_info ibm_cffps_info[CFFPS_VERSIONS] = { >> + [0] = { >> + .pages = 1, >> + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | >> + 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, >> + .read_byte_data = ibm_cffps_read_byte_data, >> + .read_word_data = ibm_cffps_read_word_data, >> + }, >> + [1] = { >> + .pages = 2, >> + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | >> + 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, >> + .func[1] = PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | >> + PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 | >> + PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT, >> + .read_byte_data = ibm_cffps_read_byte_data, >> + .read_word_data = ibm_cffps_read_word_data, >> + }, >> }; >> >> static struct pmbus_platform_data ibm_cffps_pdata = { >> @@ -346,13 +394,21 @@ static void ibm_cffps_create_led_class(struct ibm_cffps *psu) >> static int ibm_cffps_probe(struct i2c_client *client, >> const struct i2c_device_id *id) >> { >> - int i, rc; >> + int i, rc, vs; >> struct dentry *debugfs; >> struct dentry *ibm_cffps_dir; >> struct ibm_cffps *psu; >> + const void *md = of_device_get_match_data(&client->dev); >> + >> + if (md) >> + vs = (int)md; >> + else if (id) >> + vs = (int)id->driver_data; >> + else >> + vs = 1; >> >> client->dev.platform_data = &ibm_cffps_pdata; >> - rc = pmbus_do_probe(client, id, &ibm_cffps_info); >> + rc = pmbus_do_probe(client, id, &ibm_cffps_info[vs - 1]); >> if (rc) >> return rc; >> >> @@ -364,6 +420,7 @@ static int ibm_cffps_probe(struct i2c_client *client, >> if (!psu) >> return 0; >> >> + psu->version = vs; >> psu->client = client; >> mutex_init(&psu->input_history.update_lock); >> psu->input_history.last_update = jiffies - HZ; >> @@ -406,12 +463,20 @@ static int ibm_cffps_probe(struct i2c_client *client, >> >> static const struct i2c_device_id ibm_cffps_id[] = { >> { "ibm_cffps1", 1 }, >> + { "ibm_cffps2", 2 }, >> {} >> }; >> MODULE_DEVICE_TABLE(i2c, ibm_cffps_id); >> >> static const struct of_device_id ibm_cffps_of_match[] = { >> - { .compatible = "ibm,cffps1" }, >> + { >> + .compatible = "ibm,cffps1", >> + .data = (void *)1 >> + }, >> + { >> + .compatible = "ibm,cffps2", >> + .data = (void *)2 >> + }, >> {} >> }; >> MODULE_DEVICE_TABLE(of, ibm_cffps_of_match); >> -- >> 1.8.3.1 >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] pmbus: ibm-cffps: Add support for version 2 of the PSU 2019-08-30 16:09 ` [PATCH 3/3] pmbus: ibm-cffps: Add support for version 2 of the PSU Eddie James 2019-08-30 17:36 ` Guenter Roeck @ 2019-09-02 7:49 ` kbuild test robot 1 sibling, 0 replies; 7+ messages in thread From: kbuild test robot @ 2019-09-02 7:49 UTC (permalink / raw) To: Eddie James Cc: kbuild-all, linux-hwmon, linux-kernel, linux-aspeed, devicetree, linux, andrew, joel, mark.rutland, robh+dt, jdelvare, Eddie James [-- Attachment #1: Type: text/plain, Size: 3531 bytes --] Hi Eddie, I love your patch! Perhaps something to improve: [auto build test WARNING on linus/master] [cannot apply to v5.3-rc6 next-20190830] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Eddie-James/pmbus-ibm-cffps-Add-support-for-version-2-of-PSU/20190901-151755 config: x86_64-randconfig-s0-09021304 (attached as .config) compiler: gcc-7 (Debian 7.4.0-11) 7.4.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All warnings (new ones prefixed by >>): drivers/hwmon/pmbus/ibm-cffps.c: In function 'ibm_cffps_probe': >> drivers/hwmon/pmbus/ibm-cffps.c:404:8: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] vs = (int)md; ^ vim +404 drivers/hwmon/pmbus/ibm-cffps.c 393 394 static int ibm_cffps_probe(struct i2c_client *client, 395 const struct i2c_device_id *id) 396 { 397 int i, rc, vs; 398 struct dentry *debugfs; 399 struct dentry *ibm_cffps_dir; 400 struct ibm_cffps *psu; 401 const void *md = of_device_get_match_data(&client->dev); 402 403 if (md) > 404 vs = (int)md; 405 else if (id) 406 vs = (int)id->driver_data; 407 else 408 vs = 1; 409 410 client->dev.platform_data = &ibm_cffps_pdata; 411 rc = pmbus_do_probe(client, id, &ibm_cffps_info[vs - 1]); 412 if (rc) 413 return rc; 414 415 /* 416 * Don't fail the probe if there isn't enough memory for leds and 417 * debugfs. 418 */ 419 psu = devm_kzalloc(&client->dev, sizeof(*psu), GFP_KERNEL); 420 if (!psu) 421 return 0; 422 423 psu->version = vs; 424 psu->client = client; 425 mutex_init(&psu->input_history.update_lock); 426 psu->input_history.last_update = jiffies - HZ; 427 428 ibm_cffps_create_led_class(psu); 429 430 /* Don't fail the probe if we can't create debugfs */ 431 debugfs = pmbus_get_debugfs_dir(client); 432 if (!debugfs) 433 return 0; 434 435 ibm_cffps_dir = debugfs_create_dir(client->name, debugfs); 436 if (!ibm_cffps_dir) 437 return 0; 438 439 for (i = 0; i < CFFPS_DEBUGFS_NUM_ENTRIES; ++i) 440 psu->debugfs_entries[i] = i; 441 442 debugfs_create_file("input_history", 0444, ibm_cffps_dir, 443 &psu->debugfs_entries[CFFPS_DEBUGFS_INPUT_HISTORY], 444 &ibm_cffps_fops); 445 debugfs_create_file("fru", 0444, ibm_cffps_dir, 446 &psu->debugfs_entries[CFFPS_DEBUGFS_FRU], 447 &ibm_cffps_fops); 448 debugfs_create_file("part_number", 0444, ibm_cffps_dir, 449 &psu->debugfs_entries[CFFPS_DEBUGFS_PN], 450 &ibm_cffps_fops); 451 debugfs_create_file("serial_number", 0444, ibm_cffps_dir, 452 &psu->debugfs_entries[CFFPS_DEBUGFS_SN], 453 &ibm_cffps_fops); 454 debugfs_create_file("ccin", 0444, ibm_cffps_dir, 455 &psu->debugfs_entries[CFFPS_DEBUGFS_CCIN], 456 &ibm_cffps_fops); 457 debugfs_create_file("fw_version", 0444, ibm_cffps_dir, 458 &psu->debugfs_entries[CFFPS_DEBUGFS_FW], 459 &ibm_cffps_fops); 460 461 return 0; 462 } 463 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 30810 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-09-02 7:50 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-30 16:09 [PATCH 0/3] pmbus: ibm-cffps: Add support for version 2 of PSU Eddie James 2019-08-30 16:09 ` [PATCH 1/3] dt-bindings: hwmon: Document ibm,cffps2 compatible string Eddie James 2019-08-30 16:09 ` [PATCH 2/3] ARM: dts: aspeed: swift: Change power supplies to version 2 Eddie James 2019-08-30 16:09 ` [PATCH 3/3] pmbus: ibm-cffps: Add support for version 2 of the PSU Eddie James 2019-08-30 17:36 ` Guenter Roeck 2019-08-30 18:19 ` Eddie James 2019-09-02 7:49 ` kbuild test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).