* [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data
[not found] <cover.1546591275.git.xiaoting.liu@hxt-semitech.com>
@ 2019-01-04 8:55 ` Xiaoting Liu
2019-01-04 9:00 ` [PATCH 2/4] dt-bindings: power supply: Document dps650ab Xiaoting Liu
` (5 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Xiaoting Liu @ 2019-01-04 8:55 UTC (permalink / raw)
To: linux, jdelvare
Cc: openbmc, linux-hwmon, linux-kernel, Xiaoting Liu, Shunyong Yang
Current code compares device name with name in i2c_device_id to decide
whether PMBUS_SKIP_STATUS_CHECK should be set in pmbus_platform_data,
which makes adding new devices with PMBUS_SKIP_STATUS_CHECK should also
modify code in pmbus_probe().
This patch adds pmbus_device_info to save pages and flags. Its pointer
is put in driver_data of i2c_device_id, which makes adding new device
more straightforward.
Signed-off-by: Shunyong Yang <shunyong.yang@hxt-semitech.com>
Signed-off-by: Xiaoting Liu <xiaoting.liu@hxt-semitech.com>
---
drivers/hwmon/pmbus/pmbus.c | 54 +++++++++++++++++++++++++++------------------
include/linux/pmbus.h | 5 +++++
2 files changed, 37 insertions(+), 22 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
index 7688dab32f6e..aa4cf9636e99 100644
--- a/drivers/hwmon/pmbus/pmbus.c
+++ b/drivers/hwmon/pmbus/pmbus.c
@@ -172,13 +172,15 @@ static int pmbus_probe(struct i2c_client *client,
struct pmbus_driver_info *info;
struct pmbus_platform_data *pdata = NULL;
struct device *dev = &client->dev;
+ struct pmbus_device_info *device_info;
info = devm_kzalloc(dev, sizeof(struct pmbus_driver_info), GFP_KERNEL);
if (!info)
return -ENOMEM;
- if (!strcmp(id->name, "dps460") || !strcmp(id->name, "dps800") ||
- !strcmp(id->name, "sgd009")) {
+ device_info = (struct pmbus_device_info *)id->driver_data;
+
+ if (device_info->flags & PMBUS_SKIP_STATUS_CHECK) {
pdata = devm_kzalloc(dev, sizeof(struct pmbus_platform_data),
GFP_KERNEL);
if (!pdata)
@@ -187,36 +189,44 @@ static int pmbus_probe(struct i2c_client *client,
pdata->flags = PMBUS_SKIP_STATUS_CHECK;
}
- info->pages = id->driver_data;
+ info->pages = device_info->pages;
info->identify = pmbus_identify;
dev->platform_data = pdata;
return pmbus_do_probe(client, id, info);
}
+static const struct pmbus_device_info default_pmbus_info = {1, 0};
+static const struct pmbus_device_info dps460_pmbus_info = {
+ 1, PMBUS_SKIP_STATUS_CHECK};
+static const struct pmbus_device_info dps800_pmbus_info = {
+ 1, PMBUS_SKIP_STATUS_CHECK};
+static const struct pmbus_device_info sgd009_pmbus_info = {
+ 1, PMBUS_SKIP_STATUS_CHECK};
+static const struct pmbus_device_info pmbus_info = {0, 0};
/*
* Use driver_data to set the number of pages supported by the chip.
*/
static const struct i2c_device_id pmbus_id[] = {
- {"adp4000", 1},
- {"bmr453", 1},
- {"bmr454", 1},
- {"dps460", 1},
- {"dps800", 1},
- {"mdt040", 1},
- {"ncp4200", 1},
- {"ncp4208", 1},
- {"pdt003", 1},
- {"pdt006", 1},
- {"pdt012", 1},
- {"pmbus", 0},
- {"sgd009", 1},
- {"tps40400", 1},
- {"tps544b20", 1},
- {"tps544b25", 1},
- {"tps544c20", 1},
- {"tps544c25", 1},
- {"udt020", 1},
+ {"adp4000", (kernel_ulong_t)&default_pmbus_info},
+ {"bmr453", (kernel_ulong_t)&default_pmbus_info},
+ {"bmr454", (kernel_ulong_t)&default_pmbus_info},
+ {"dps460", (kernel_ulong_t)&dps460_pmbus_info},
+ {"dps800", (kernel_ulong_t)&dps800_pmbus_info},
+ {"mdt040", (kernel_ulong_t)&default_pmbus_info},
+ {"ncp4200", (kernel_ulong_t)&default_pmbus_info},
+ {"ncp4208", (kernel_ulong_t)&default_pmbus_info},
+ {"pdt003", (kernel_ulong_t)&default_pmbus_info},
+ {"pdt006", (kernel_ulong_t)&default_pmbus_info},
+ {"pdt012", (kernel_ulong_t)&default_pmbus_info},
+ {"pmbus", (kernel_ulong_t)&pmbus_info},
+ {"sgd009", (kernel_ulong_t)&sgd009_pmbus_info},
+ {"tps40400", (kernel_ulong_t)&default_pmbus_info},
+ {"tps544b20", (kernel_ulong_t)&default_pmbus_info},
+ {"tps544b25", (kernel_ulong_t)&default_pmbus_info},
+ {"tps544c20", (kernel_ulong_t)&default_pmbus_info},
+ {"tps544c25", (kernel_ulong_t)&default_pmbus_info},
+ {"udt020", (kernel_ulong_t)&default_pmbus_info},
{}
};
diff --git a/include/linux/pmbus.h b/include/linux/pmbus.h
index ee3c2aba2a8e..3c05edad7666 100644
--- a/include/linux/pmbus.h
+++ b/include/linux/pmbus.h
@@ -46,4 +46,9 @@ struct pmbus_platform_data {
struct regulator_init_data *reg_init_data;
};
+struct pmbus_device_info {
+ int pages;
+ u32 flags;
+};
+
#endif /* _PMBUS_H_ */
--
1.8.3.1
This email is intended only for the named addressee. It may contain information that is confidential/private, legally privileged, or copyright-protected, and you should handle it accordingly. If you are not the intended recipient, you do not have legal rights to retain, copy, or distribute this email or its contents, and should promptly delete the email and all electronic copies in your system; do not retain copies in any media. If you have received this email in error, please notify the sender promptly. Thank you.
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/4] dt-bindings: power supply: Document dps650ab
[not found] <cover.1546591275.git.xiaoting.liu@hxt-semitech.com>
2019-01-04 8:55 ` [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data Xiaoting Liu
@ 2019-01-04 9:00 ` Xiaoting Liu
2019-01-11 19:36 ` Rob Herring
2019-01-04 9:05 ` [PATCH 4/4] pmbus (dps650ab): add power supply driver Xiaoting Liu
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Xiaoting Liu @ 2019-01-04 9:00 UTC (permalink / raw)
To: jdelvare, linux, robh+dt, mark.rutland
Cc: openbmc, linux-hwmon, devicetree, shunyong.yang, dongsheng.wang,
Xiaoting Liu
Bindings for DPS650AB power, voltage, temperature, and fan monitering.
Signed-off-by: Xiaoting Liu <xiaoting.liu@hxt-semitech.com>
---
Documentation/devicetree/bindings/hwmon/dps650ab.txt | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/hwmon/dps650ab.txt b/Documentation/devicetree/bindings/hwmon/dps650ab.txt
new file mode 100644
index 000000000000..bbd601100e66
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/dps650ab.txt
@@ -0,0 +1,11 @@
+Bindings for DPS650AB power, voltage, temperature, and fan monitering
+
+Required properties:
+- compatible : "dps650ab"
+- reg : I2C address, one of 0x58, 0x59.
+
+Example:
+ dps650ab@58 {
+ compatible = "dps650ab";
+ reg = <0x58>;
+ };
--
1.8.3.1
This email is intended only for the named addressee. It may contain information that is confidential/private, legally privileged, or copyright-protected, and you should handle it accordingly. If you are not the intended recipient, you do not have legal rights to retain, copy, or distribute this email or its contents, and should promptly delete the email and all electronic copies in your system; do not retain copies in any media. If you have received this email in error, please notify the sender promptly. Thank you.
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/4] pmbus (dps650ab): add power supply driver
[not found] <cover.1546591275.git.xiaoting.liu@hxt-semitech.com>
2019-01-04 8:55 ` [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data Xiaoting Liu
2019-01-04 9:00 ` [PATCH 2/4] dt-bindings: power supply: Document dps650ab Xiaoting Liu
@ 2019-01-04 9:05 ` Xiaoting Liu
2019-01-04 14:41 ` Guenter Roeck
2019-01-04 14:54 ` [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data Guenter Roeck
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Xiaoting Liu @ 2019-01-04 9:05 UTC (permalink / raw)
To: linux, jdelvare
Cc: openbmc, linux-kernel, linux-hwmon, shunyong.yang,
dongsheng.wang, Xiaoting Liu
The Delta dps650ab provides main power and standby power to server.
dps650ab can be detected by MFR_ID and MFR_MODEL referring to
manufacturer's feedback. This patch adds driver to moniter power
supply status.
Signed-off-by: Xiaoting Liu <xiaoting.liu@hxt-semitech.com>
---
drivers/hwmon/pmbus/Kconfig | 10 +++++
drivers/hwmon/pmbus/Makefile | 1 +
drivers/hwmon/pmbus/dps650ab.c | 100 +++++++++++++++++++++++++++++++++++++++++
drivers/hwmon/pmbus/pmbus.c | 3 ++
4 files changed, 114 insertions(+)
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index 629cb45f8557..de4638396592 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -184,4 +184,14 @@ config SENSORS_ZL6100
This driver can also be built as a module. If so, the module will
be called zl6100.
+config SENSORS_DPS650AB
+ tristate "Delta DPS650AB"
+ default n
+ help
+ If you say yes here you get hardware monitoring support for the
+ Delta DPS650AB controller.
+
+ This driver can also be built as a module. If so, the module will
+ be called dps650ab.
+
endif # PMBUS
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index ea0e39518c21..414818230a26 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -21,3 +21,4 @@ obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o
obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o
obj-$(CONFIG_SENSORS_UCD9200) += ucd9200.o
obj-$(CONFIG_SENSORS_ZL6100) += zl6100.o
+obj-$(CONFIG_SENSORS_DPS650AB) += dps650ab.o
diff --git a/drivers/hwmon/pmbus/dps650ab.c b/drivers/hwmon/pmbus/dps650ab.c
new file mode 100644
index 000000000000..3c300e621f5a
--- /dev/null
+++ b/drivers/hwmon/pmbus/dps650ab.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Hardware monitoring driver for DELTA DPS650AB
+ *
+ * Copyright (c) 2018 Huaxintong Semiconductor Technology Co., Ltd.
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include "pmbus.h"
+
+#define DPS650AB_MFR_ID "DELTA"
+#define DPS650AB_MFR_MODEL "DPS-650AB-16 A"
+
+static int dps650ab_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct pmbus_driver_info *info;
+ u8 buf[I2C_SMBUS_BLOCK_MAX];
+ int ret;
+
+ memset(buf, 0, I2C_SMBUS_BLOCK_MAX);
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_READ_BYTE_DATA
+ | I2C_FUNC_SMBUS_READ_WORD_DATA
+ | I2C_FUNC_SMBUS_READ_BLOCK_DATA))
+ return -ENODEV;
+
+ ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
+ if (ret < 0) {
+ dev_err(&client->dev, "Failed to read PMBUS_MFR_ID\n");
+ return ret;
+ }
+
+ if (strncmp(buf, DPS650AB_MFR_ID, strlen(DPS650AB_MFR_ID))) {
+ dev_err(&client->dev, "DPS650AB_MFR_ID unrecognised\n");
+ return -ENODEV;
+ }
+
+ ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, buf);
+ if (ret < 0) {
+ dev_err(&client->dev, "Failed to read PMBUS_MFR_MODEL\n");
+ return ret;
+ }
+
+ if (strncmp(buf, DPS650AB_MFR_MODEL, strlen(DPS650AB_MFR_MODEL))) {
+ dev_err(&client->dev, "DPS650AB_MFR_MODEL unrecognised\n");
+ return -ENODEV;
+ }
+
+ info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ info->pages = 1;
+ info->format[PSC_VOLTAGE_IN] = linear;
+ info->format[PSC_VOLTAGE_OUT] = linear;
+ info->format[PSC_CURRENT_IN] = linear;
+ info->format[PSC_CURRENT_OUT] = linear;
+ info->format[PSC_POWER] = linear;
+ info->format[PSC_TEMPERATURE] = linear;
+
+ info->func[0] = PMBUS_HAVE_VIN
+ | PMBUS_HAVE_IIN | PMBUS_HAVE_PIN
+ | PMBUS_HAVE_STATUS_INPUT
+ | PMBUS_HAVE_POUT | PMBUS_HAVE_FAN12
+ | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
+ | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
+ | PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2
+ | PMBUS_HAVE_STATUS_TEMP;
+ info->func[1] = info->func[0];
+
+ return pmbus_do_probe(client, id, info);
+}
+
+static const struct i2c_device_id dps650ab_id[] = {
+ {"dps650ab", 1},
+ {}
+};
+
+MODULE_DEVICE_TABLE(i2c, dps650ab_id);
+
+static struct i2c_driver dps650ab_driver = {
+ .driver = {
+ .name = "dps650ab",
+ },
+ .probe = dps650ab_probe,
+ .remove = pmbus_do_remove,
+ .id_table = dps650ab_id,
+};
+
+module_i2c_driver(dps650ab_driver);
+
+MODULE_AUTHOR("Liuxiaoting <xiaoting.liu@hxt-semitech.com");
+MODULE_DESCRIPTION("PMBus driver for DPS650AB");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
index aa4cf9636e99..930e8a3e2366 100644
--- a/drivers/hwmon/pmbus/pmbus.c
+++ b/drivers/hwmon/pmbus/pmbus.c
@@ -204,6 +204,8 @@ static int pmbus_probe(struct i2c_client *client,
static const struct pmbus_device_info sgd009_pmbus_info = {
1, PMBUS_SKIP_STATUS_CHECK};
static const struct pmbus_device_info pmbus_info = {0, 0};
+static const struct pmbus_device_info dps650ab_pmbus_info = {
+ 1, PMBUS_SKIP_STATUS_CHECK};
/*
* Use driver_data to set the number of pages supported by the chip.
*/
@@ -227,6 +229,7 @@ static int pmbus_probe(struct i2c_client *client,
{"tps544c20", (kernel_ulong_t)&default_pmbus_info},
{"tps544c25", (kernel_ulong_t)&default_pmbus_info},
{"udt020", (kernel_ulong_t)&default_pmbus_info},
+ {"dps650ab", (kernel_ulong_t)&dps650ab_pmbus_info},
{}
};
--
1.8.3.1
This email is intended only for the named addressee. It may contain information that is confidential/private, legally privileged, or copyright-protected, and you should handle it accordingly. If you are not the intended recipient, you do not have legal rights to retain, copy, or distribute this email or its contents, and should promptly delete the email and all electronic copies in your system; do not retain copies in any media. If you have received this email in error, please notify the sender promptly. Thank you.
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] pmbus (dps650ab): add power supply driver
2019-01-04 9:05 ` [PATCH 4/4] pmbus (dps650ab): add power supply driver Xiaoting Liu
@ 2019-01-04 14:41 ` Guenter Roeck
0 siblings, 0 replies; 17+ messages in thread
From: Guenter Roeck @ 2019-01-04 14:41 UTC (permalink / raw)
To: Xiaoting Liu, jdelvare
Cc: openbmc, linux-kernel, linux-hwmon, shunyong.yang, dongsheng.wang
On 1/4/19 1:05 AM, Xiaoting Liu wrote:
> The Delta dps650ab provides main power and standby power to server.
> dps650ab can be detected by MFR_ID and MFR_MODEL referring to
> manufacturer's feedback. This patch adds driver to moniter power
> supply status.
>
> Signed-off-by: Xiaoting Liu <xiaoting.liu@hxt-semitech.com>
> ---
> drivers/hwmon/pmbus/Kconfig | 10 +++++
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/dps650ab.c | 100 +++++++++++++++++++++++++++++++++++++++++
> drivers/hwmon/pmbus/pmbus.c | 3 ++
> 4 files changed, 114 insertions(+)
>
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 629cb45f8557..de4638396592 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -184,4 +184,14 @@ config SENSORS_ZL6100
> This driver can also be built as a module. If so, the module will
> be called zl6100.
>
> +config SENSORS_DPS650AB
> + tristate "Delta DPS650AB"
> + default n
> + help
> + If you say yes here you get hardware monitoring support for the
> + Delta DPS650AB controller.
> +
> + This driver can also be built as a module. If so, the module will
> + be called dps650ab.
> +
> endif # PMBUS
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index ea0e39518c21..414818230a26 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -21,3 +21,4 @@ obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o
> obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o
> obj-$(CONFIG_SENSORS_UCD9200) += ucd9200.o
> obj-$(CONFIG_SENSORS_ZL6100) += zl6100.o
> +obj-$(CONFIG_SENSORS_DPS650AB) += dps650ab.o
> diff --git a/drivers/hwmon/pmbus/dps650ab.c b/drivers/hwmon/pmbus/dps650ab.c
> new file mode 100644
> index 000000000000..3c300e621f5a
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/dps650ab.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Hardware monitoring driver for DELTA DPS650AB
> + *
> + * Copyright (c) 2018 Huaxintong Semiconductor Technology Co., Ltd.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include "pmbus.h"
> +
> +#define DPS650AB_MFR_ID "DELTA"
> +#define DPS650AB_MFR_MODEL "DPS-650AB-16 A"
> +
> +static int dps650ab_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct pmbus_driver_info *info;
> + u8 buf[I2C_SMBUS_BLOCK_MAX];
> + int ret;
> +
> + memset(buf, 0, I2C_SMBUS_BLOCK_MAX);
> +
I don't think this is needed.
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_READ_BYTE_DATA
> + | I2C_FUNC_SMBUS_READ_WORD_DATA
> + | I2C_FUNC_SMBUS_READ_BLOCK_DATA))
> + return -ENODEV;
> +
> + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to read PMBUS_MFR_ID\n");
> + return ret;
> + }
> +
> + if (strncmp(buf, DPS650AB_MFR_ID, strlen(DPS650AB_MFR_ID))) {
It might make sense to verify the length of the returned string as well,
unless strings such as "DELTAX" are accepted on purpose. But even if
that is the case I would prefer an added check of "ret < strlen(DPS650AB_MFR_ID)"
over the memset() above....
> + dev_err(&client->dev, "DPS650AB_MFR_ID unrecognised\n");
> + return -ENODEV;
> + }
> +
> + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, buf);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to read PMBUS_MFR_MODEL\n");
> + return ret;
> + }
> +
> + if (strncmp(buf, DPS650AB_MFR_MODEL, strlen(DPS650AB_MFR_MODEL))) {
... to ensure that nothing bad slips through here. As currently written, the code
accepts a MFR_ID of "DELTA50AB-16 A" combined with a MFR_MODEL "DPS-6" as valid.
> + dev_err(&client->dev, "DPS650AB_MFR_MODEL unrecognised\n");
It would be a nice touch to actually list the unrecognized model. Otherwise
the message is quite useless. Same for MFR_ID.
> + return -ENODEV;
> + }
> +
> + info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + info->pages = 1;
> + info->format[PSC_VOLTAGE_IN] = linear;
> + info->format[PSC_VOLTAGE_OUT] = linear;
> + info->format[PSC_CURRENT_IN] = linear;
> + info->format[PSC_CURRENT_OUT] = linear;
> + info->format[PSC_POWER] = linear;
> + info->format[PSC_TEMPERATURE] = linear;
> +
> + info->func[0] = PMBUS_HAVE_VIN
> + | PMBUS_HAVE_IIN | PMBUS_HAVE_PIN
> + | PMBUS_HAVE_STATUS_INPUT
> + | PMBUS_HAVE_POUT | PMBUS_HAVE_FAN12
> + | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
> + | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> + | PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2
> + | PMBUS_HAVE_STATUS_TEMP;
> + info->func[1] = info->func[0];
> +
> + return pmbus_do_probe(client, id, info);
> +}
> +
> +static const struct i2c_device_id dps650ab_id[] = {
> + {"dps650ab", 1},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, dps650ab_id);
> +
> +static struct i2c_driver dps650ab_driver = {
> + .driver = {
> + .name = "dps650ab",
> + },
> + .probe = dps650ab_probe,
> + .remove = pmbus_do_remove,
> + .id_table = dps650ab_id,
> +};
> +
> +module_i2c_driver(dps650ab_driver);
> +
> +MODULE_AUTHOR("Liuxiaoting <xiaoting.liu@hxt-semitech.com");
> +MODULE_DESCRIPTION("PMBus driver for DPS650AB");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
> index aa4cf9636e99..930e8a3e2366 100644
> --- a/drivers/hwmon/pmbus/pmbus.c
> +++ b/drivers/hwmon/pmbus/pmbus.c
> @@ -204,6 +204,8 @@ static int pmbus_probe(struct i2c_client *client,
> static const struct pmbus_device_info sgd009_pmbus_info = {
> 1, PMBUS_SKIP_STATUS_CHECK};
> static const struct pmbus_device_info pmbus_info = {0, 0};
> +static const struct pmbus_device_info dps650ab_pmbus_info = {
> + 1, PMBUS_SKIP_STATUS_CHECK};
Something is conceptually wrong here. Either this change or a new driver,
but not both.
> /*
> * Use driver_data to set the number of pages supported by the chip.
> */
> @@ -227,6 +229,7 @@ static int pmbus_probe(struct i2c_client *client,
> {"tps544c20", (kernel_ulong_t)&default_pmbus_info},
> {"tps544c25", (kernel_ulong_t)&default_pmbus_info},
> {"udt020", (kernel_ulong_t)&default_pmbus_info},
> + {"dps650ab", (kernel_ulong_t)&dps650ab_pmbus_info},
Also, FWIW, this should be kept in alphabetic order.
> {}
> };
>
> --
> 1.8.3.1
>
>
>
>
> This email is intended only for the named addressee. It may contain information that is confidential/private, legally privileged, or copyright-protected, and you should handle it accordingly. If you are not the intended recipient, you do not have legal rights to retain, copy, or distribute this email or its contents, and should promptly delete the email and all electronic copies in your system; do not retain copies in any media. If you have received this email in error, please notify the sender promptly. Thank you.
>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data
2019-01-04 14:54 ` [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data Guenter Roeck
@ 2019-01-04 14:54 ` Guenter Roeck
0 siblings, 0 replies; 17+ messages in thread
From: Guenter Roeck @ 2019-01-04 14:54 UTC (permalink / raw)
To: Xiaoting Liu, jdelvare; +Cc: openbmc, linux-hwmon, linux-kernel, Shunyong Yang
On 1/4/19 12:55 AM, Xiaoting Liu wrote:
> Current code compares device name with name in i2c_device_id to decide
> whether PMBUS_SKIP_STATUS_CHECK should be set in pmbus_platform_data,
> which makes adding new devices with PMBUS_SKIP_STATUS_CHECK should also
> modify code in pmbus_probe().
>
> This patch adds pmbus_device_info to save pages and flags. Its pointer
> is put in driver_data of i2c_device_id, which makes adding new device
> more straightforward.
>
Good idea, but I don't see at this time where the patch is needed.
Maybe in patch 3/4 which is missing from the series ?
> Signed-off-by: Shunyong Yang <shunyong.yang@hxt-semitech.com>
> Signed-off-by: Xiaoting Liu <xiaoting.liu@hxt-semitech.com>
> ---
> drivers/hwmon/pmbus/pmbus.c | 54 +++++++++++++++++++++++++++------------------
> include/linux/pmbus.h | 5 +++++
> 2 files changed, 37 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
> index 7688dab32f6e..aa4cf9636e99 100644
> --- a/drivers/hwmon/pmbus/pmbus.c
> +++ b/drivers/hwmon/pmbus/pmbus.c
> @@ -172,13 +172,15 @@ static int pmbus_probe(struct i2c_client *client,
> struct pmbus_driver_info *info;
> struct pmbus_platform_data *pdata = NULL;
> struct device *dev = &client->dev;
> + struct pmbus_device_info *device_info;
>
> info = devm_kzalloc(dev, sizeof(struct pmbus_driver_info), GFP_KERNEL);
> if (!info)
> return -ENOMEM;
>
> - if (!strcmp(id->name, "dps460") || !strcmp(id->name, "dps800") ||
> - !strcmp(id->name, "sgd009")) {
> + device_info = (struct pmbus_device_info *)id->driver_data;
> +
> + if (device_info->flags & PMBUS_SKIP_STATUS_CHECK) {
> pdata = devm_kzalloc(dev, sizeof(struct pmbus_platform_data),
> GFP_KERNEL);
> if (!pdata)
> @@ -187,36 +189,44 @@ static int pmbus_probe(struct i2c_client *client,
> pdata->flags = PMBUS_SKIP_STATUS_CHECK;
> }
>
> - info->pages = id->driver_data;
> + info->pages = device_info->pages;
> info->identify = pmbus_identify;
> dev->platform_data = pdata;
>
> return pmbus_do_probe(client, id, info);
> }
>
> +static const struct pmbus_device_info default_pmbus_info = {1, 0};
> +static const struct pmbus_device_info dps460_pmbus_info = {
> + 1, PMBUS_SKIP_STATUS_CHECK};
> +static const struct pmbus_device_info dps800_pmbus_info = {
> + 1, PMBUS_SKIP_STATUS_CHECK};
> +static const struct pmbus_device_info sgd009_pmbus_info = {
> + 1, PMBUS_SKIP_STATUS_CHECK};
Three structures with exactly the same content does not add value.
Please merge into one with a common name that reflects its use.
> +static const struct pmbus_device_info pmbus_info = {0, 0};
default_pmbus_info and pmbus_info are badly named and ordered.
The name should reflect that one sets one page and that the other
leaves the number of pages unset.
I would suggest three structures and names, such as
pmbus_info_one
pmbus_info_one_skip
pmbus_info_zero
though I am open to better names.
> /*
> * Use driver_data to set the number of pages supported by the chip.
> */
> static const struct i2c_device_id pmbus_id[] = {
> - {"adp4000", 1},
> - {"bmr453", 1},
> - {"bmr454", 1},
> - {"dps460", 1},
> - {"dps800", 1},
> - {"mdt040", 1},
> - {"ncp4200", 1},
> - {"ncp4208", 1},
> - {"pdt003", 1},
> - {"pdt006", 1},
> - {"pdt012", 1},
> - {"pmbus", 0},
> - {"sgd009", 1},
> - {"tps40400", 1},
> - {"tps544b20", 1},
> - {"tps544b25", 1},
> - {"tps544c20", 1},
> - {"tps544c25", 1},
> - {"udt020", 1},
> + {"adp4000", (kernel_ulong_t)&default_pmbus_info},
> + {"bmr453", (kernel_ulong_t)&default_pmbus_info},
> + {"bmr454", (kernel_ulong_t)&default_pmbus_info},
> + {"dps460", (kernel_ulong_t)&dps460_pmbus_info},
> + {"dps800", (kernel_ulong_t)&dps800_pmbus_info},
> + {"mdt040", (kernel_ulong_t)&default_pmbus_info},
> + {"ncp4200", (kernel_ulong_t)&default_pmbus_info},
> + {"ncp4208", (kernel_ulong_t)&default_pmbus_info},
> + {"pdt003", (kernel_ulong_t)&default_pmbus_info},
> + {"pdt006", (kernel_ulong_t)&default_pmbus_info},
> + {"pdt012", (kernel_ulong_t)&default_pmbus_info},
> + {"pmbus", (kernel_ulong_t)&pmbus_info},
> + {"sgd009", (kernel_ulong_t)&sgd009_pmbus_info},
> + {"tps40400", (kernel_ulong_t)&default_pmbus_info},
> + {"tps544b20", (kernel_ulong_t)&default_pmbus_info},
> + {"tps544b25", (kernel_ulong_t)&default_pmbus_info},
> + {"tps544c20", (kernel_ulong_t)&default_pmbus_info},
> + {"tps544c25", (kernel_ulong_t)&default_pmbus_info},
> + {"udt020", (kernel_ulong_t)&default_pmbus_info},
> {}
> };
>
> diff --git a/include/linux/pmbus.h b/include/linux/pmbus.h
> index ee3c2aba2a8e..3c05edad7666 100644
> --- a/include/linux/pmbus.h
> +++ b/include/linux/pmbus.h
> @@ -46,4 +46,9 @@ struct pmbus_platform_data {
> struct regulator_init_data *reg_init_data;
> };
>
> +struct pmbus_device_info {
> + int pages;
> + u32 flags;
> +};
> +
This should not be needed here. The structure can be declared locally
in pmbus.c (its use in patch 4/4 is wrong).
> #endif /* _PMBUS_H_ */
> --
> 1.8.3.1
>
>
>
>
> This email is intended only for the named addressee. It may contain information that is confidential/private, legally privileged, or copyright-protected, and you should handle it accordingly. If you are not the intended recipient, you do not have legal rights to retain, copy, or distribute this email or its contents, and should promptly delete the email and all electronic copies in your system; do not retain copies in any media. If you have received this email in error, please notify the sender promptly. Thank you.
>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data
[not found] <cover.1546591275.git.xiaoting.liu@hxt-semitech.com>
` (2 preceding siblings ...)
2019-01-04 9:05 ` [PATCH 4/4] pmbus (dps650ab): add power supply driver Xiaoting Liu
@ 2019-01-04 14:54 ` Guenter Roeck
2019-01-04 14:54 ` Guenter Roeck
2019-01-05 17:06 ` [PATCH 2/4] dt-bindings: power supply: Document dps650ab Guenter Roeck
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Guenter Roeck @ 2019-01-04 14:54 UTC (permalink / raw)
To: linux-hwmon
On 1/4/19 12:55 AM, Xiaoting Liu wrote:
Current code compares device name with name in i2c_device_id to decide
whether PMBUS_SKIP_STATUS_CHECK should be set in pmbus_platform_data,
which makes adding new devices with PMBUS_SKIP_STATUS_CHECK should also
modify code in pmbus_probe().
This patch adds pmbus_device_info to save pages and flags. Its pointer
is put in driver_data of i2c_device_id, which makes adding new device
more straightforward.
Good idea, but I don't see at this time where the patch is needed.
Maybe in patch 3/4 which is missing from the series ?
Signed-off-by: Shunyong Yang <shunyong.y...@hxt-semitech.com>
Signed-off-by: Xiaoting Liu <xiaoting....@hxt-semitech.com>
---
drivers/hwmon/pmbus/pmbus.c | 54 +++++++++++++++++++++++++++------------------
include/linux/pmbus.h | 5 +++++
2 files changed, 37 insertions(+), 22 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
index 7688dab32f6e..aa4cf9636e99 100644
--- a/drivers/hwmon/pmbus/pmbus.c
+++ b/drivers/hwmon/pmbus/pmbus.c
@@ -172,13 +172,15 @@ static int pmbus_probe(struct i2c_client *client,
struct pmbus_driver_info *info;
struct pmbus_platform_data *pdata = NULL;
struct device *dev = &client->dev;
+ struct pmbus_device_info *device_info;
info = devm_kzalloc(dev, sizeof(struct pmbus_driver_info), GFP_KERNEL);
if (!info)
return -ENOMEM;
- if (!strcmp(id->name, "dps460") || !strcmp(id->name, "dps800") ||
- !strcmp(id->name, "sgd009")) {
+ device_info = (struct pmbus_device_info *)id->driver_data;
+
+ if (device_info->flags & PMBUS_SKIP_STATUS_CHECK) {
pdata = devm_kzalloc(dev, sizeof(struct pmbus_platform_data),
GFP_KERNEL);
if (!pdata)
@@ -187,36 +189,44 @@ static int pmbus_probe(struct i2c_client *client,
pdata->flags = PMBUS_SKIP_STATUS_CHECK;
}
- info->pages = id->driver_data;
+ info->pages = device_info->pages;
info->identify = pmbus_identify;
dev->platform_data = pdata;
return pmbus_do_probe(client, id, info);
}
+static const struct pmbus_device_info default_pmbus_info = {1, 0};
+static const struct pmbus_device_info dps460_pmbus_info = {
+ 1, PMBUS_SKIP_STATUS_CHECK};
+static const struct pmbus_device_info dps800_pmbus_info = {
+ 1, PMBUS_SKIP_STATUS_CHECK};
+static const struct pmbus_device_info sgd009_pmbus_info = {
+ 1, PMBUS_SKIP_STATUS_CHECK};
Three structures with exactly the same content does not add value.
Please merge into one with a common name that reflects its use.
+static const struct pmbus_device_info pmbus_info = {0, 0};
default_pmbus_info and pmbus_info are badly named and ordered.
The name should reflect that one sets one page and that the other
leaves the number of pages unset.
I would suggest three structures and names, such as
pmbus_info_one
pmbus_info_one_skip
pmbus_info_zero
though I am open to better names.
/*
* Use driver_data to set the number of pages supported by the chip.
*/
static const struct i2c_device_id pmbus_id[] = {
- {"adp4000", 1},
- {"bmr453", 1},
- {"bmr454", 1},
- {"dps460", 1},
- {"dps800", 1},
- {"mdt040", 1},
- {"ncp4200", 1},
- {"ncp4208", 1},
- {"pdt003", 1},
- {"pdt006", 1},
- {"pdt012", 1},
- {"pmbus", 0},
- {"sgd009", 1},
- {"tps40400", 1},
- {"tps544b20", 1},
- {"tps544b25", 1},
- {"tps544c20", 1},
- {"tps544c25", 1},
- {"udt020", 1},
+ {"adp4000", (kernel_ulong_t)&default_pmbus_info},
+ {"bmr453", (kernel_ulong_t)&default_pmbus_info},
+ {"bmr454", (kernel_ulong_t)&default_pmbus_info},
+ {"dps460", (kernel_ulong_t)&dps460_pmbus_info},
+ {"dps800", (kernel_ulong_t)&dps800_pmbus_info},
+ {"mdt040", (kernel_ulong_t)&default_pmbus_info},
+ {"ncp4200", (kernel_ulong_t)&default_pmbus_info},
+ {"ncp4208", (kernel_ulong_t)&default_pmbus_info},
+ {"pdt003", (kernel_ulong_t)&default_pmbus_info},
+ {"pdt006", (kernel_ulong_t)&default_pmbus_info},
+ {"pdt012", (kernel_ulong_t)&default_pmbus_info},
+ {"pmbus", (kernel_ulong_t)&pmbus_info},
+ {"sgd009", (kernel_ulong_t)&sgd009_pmbus_info},
+ {"tps40400", (kernel_ulong_t)&default_pmbus_info},
+ {"tps544b20", (kernel_ulong_t)&default_pmbus_info},
+ {"tps544b25", (kernel_ulong_t)&default_pmbus_info},
+ {"tps544c20", (kernel_ulong_t)&default_pmbus_info},
+ {"tps544c25", (kernel_ulong_t)&default_pmbus_info},
+ {"udt020", (kernel_ulong_t)&default_pmbus_info},
{}
};
diff --git a/include/linux/pmbus.h b/include/linux/pmbus.h
index ee3c2aba2a8e..3c05edad7666 100644
--- a/include/linux/pmbus.h
+++ b/include/linux/pmbus.h
@@ -46,4 +46,9 @@ struct pmbus_platform_data {
struct regulator_init_data *reg_init_data;
};
+struct pmbus_device_info {
+ int pages;
+ u32 flags;
+};
+
This should not be needed here. The structure can be declared locally
in pmbus.c (its use in patch 4/4 is wrong).
#endif /* _PMBUS_H_ */
--
1.8.3.1
This email is intended only for the named addressee. It may contain information
that is confidential/private, legally privileged, or copyright-protected, and
you should handle it accordingly. If you are not the intended recipient, you do
not have legal rights to retain, copy, or distribute this email or its
contents, and should promptly delete the email and all electronic copies in
your system; do not retain copies in any media. If you have received this email
in error, please notify the sender promptly. Thank you.
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] dt-bindings: power supply: Document dps650ab
2019-01-05 17:06 ` [PATCH 2/4] dt-bindings: power supply: Document dps650ab Guenter Roeck
@ 2019-01-05 17:05 ` Guenter Roeck
2019-01-07 6:30 ` Liu, Xiaoting
1 sibling, 0 replies; 17+ messages in thread
From: Guenter Roeck @ 2019-01-05 17:05 UTC (permalink / raw)
To: Xiaoting Liu
Cc: jdelvare, robh+dt, mark.rutland, openbmc, linux-hwmon,
devicetree, shunyong.yang, dongsheng.wang
On Fri, Jan 04, 2019 at 05:00:30PM +0800, Xiaoting Liu wrote:
> Bindings for DPS650AB power, voltage, temperature, and fan monitering.
>
Misleading subject. While the DPS650AB may ber a power supply,
it is a PMBus hwmon driver, and should be marked accordingly in the subject.
Guenter
> Signed-off-by: Xiaoting Liu <xiaoting.liu@hxt-semitech.com>
> ---
> Documentation/devicetree/bindings/hwmon/dps650ab.txt | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> --
> 1.8.3.1
>
>
>
>
> This email is intended only for the named addressee. It may contain information that is confidential/private, legally privileged, or copyright-protected, and you should handle it accordingly. If you are not the intended recipient, you do not have legal rights to retain, copy, or distribute this email or its contents, and should promptly delete the email and all electronic copies in your system; do not retain copies in any media. If you have received this email in error, please notify the sender promptly. Thank you.
>
> diff --git a/Documentation/devicetree/bindings/hwmon/dps650ab.txt b/Documentation/devicetree/bindings/hwmon/dps650ab.txt
> new file mode 100644
> index 000000000000..bbd601100e66
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/dps650ab.txt
> @@ -0,0 +1,11 @@
> +Bindings for DPS650AB power, voltage, temperature, and fan monitering
> +
> +Required properties:
> +- compatible : "dps650ab"
> +- reg : I2C address, one of 0x58, 0x59.
> +
> +Example:
> + dps650ab@58 {
> + compatible = "dps650ab";
> + reg = <0x58>;
> + };
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] dt-bindings: power supply: Document dps650ab
[not found] <cover.1546591275.git.xiaoting.liu@hxt-semitech.com>
` (3 preceding siblings ...)
2019-01-04 14:54 ` [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data Guenter Roeck
@ 2019-01-05 17:06 ` Guenter Roeck
2019-01-05 17:05 ` Guenter Roeck
2019-01-07 6:30 ` Liu, Xiaoting
2019-01-05 17:09 ` [PATCH 4/4] pmbus (dps650ab): add power supply driver Guenter Roeck
2019-01-07 2:53 ` [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data Yang, Shunyong
6 siblings, 2 replies; 17+ messages in thread
From: Guenter Roeck @ 2019-01-05 17:06 UTC (permalink / raw)
To: linux-hwmon
On Fri, Jan 04, 2019 at 05:00:30PM +0800, Xiaoting Liu wrote:
> Bindings for DPS650AB power, voltage, temperature, and fan monitering.
>
Misleading subject. While the DPS650AB may ber a power supply,
it is a PMBus hwmon driver, and should be marked accordingly in the subject.
Guenter
> Signed-off-by: Xiaoting Liu <xiaoting....@hxt-semitech.com>
> ---
> Documentation/devicetree/bindings/hwmon/dps650ab.txt | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> --
> 1.8.3.1
>
>
>
>
> This email is intended only for the named addressee. It may contain
> information that is confidential/private, legally privileged, or
> copyright-protected, and you should handle it accordingly. If you are not the
> intended recipient, you do not have legal rights to retain, copy, or
> distribute this email or its contents, and should promptly delete the email
> and all electronic copies in your system; do not retain copies in any media.
> If you have received this email in error, please notify the sender promptly.
> Thank you.
>
> diff --git a/Documentation/devicetree/bindings/hwmon/dps650ab.txt
> b/Documentation/devicetree/bindings/hwmon/dps650ab.txt
> new file mode 100644
> index 000000000000..bbd601100e66
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/dps650ab.txt
> @@ -0,0 +1,11 @@
> +Bindings for DPS650AB power, voltage, temperature, and fan monitering
> +
> +Required properties:
> +- compatible : "dps650ab"
> +- reg : I2C address, one of 0x58, 0x59.
> +
> +Example:
> + dps650ab@58 {
> + compatible = "dps650ab";
> + reg = <0x58>;
> + };
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] pmbus (dps650ab): add power supply driver
2019-01-05 17:09 ` [PATCH 4/4] pmbus (dps650ab): add power supply driver Guenter Roeck
@ 2019-01-05 17:08 ` Guenter Roeck
[not found] ` <a494c032f6344528a89bf9b9aa81a3d7@HXTBJIDCEMVIW02.hxtcorp.net>
1 sibling, 0 replies; 17+ messages in thread
From: Guenter Roeck @ 2019-01-05 17:08 UTC (permalink / raw)
To: Xiaoting Liu
Cc: jdelvare, openbmc, linux-kernel, linux-hwmon, shunyong.yang,
dongsheng.wang
On Fri, Jan 04, 2019 at 05:05:27PM +0800, Xiaoting Liu wrote:
> The Delta dps650ab provides main power and standby power to server.
> dps650ab can be detected by MFR_ID and MFR_MODEL referring to
> manufacturer's feedback. This patch adds driver to moniter power
> supply status.
>
Another comment: The subject should be something like
hwmon: (pmbus) Add driver for DPS650AB power supply
Additional comments inline below.
Thanks,
Guenter
> Signed-off-by: Xiaoting Liu <xiaoting.liu@hxt-semitech.com>
> ---
> drivers/hwmon/pmbus/Kconfig | 10 +++++
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/dps650ab.c | 100 +++++++++++++++++++++++++++++++++++++++++
> drivers/hwmon/pmbus/pmbus.c | 3 ++
> 4 files changed, 114 insertions(+)
>
> --
> 1.8.3.1
>
>
>
>
> This email is intended only for the named addressee. It may contain information that is confidential/private, legally privileged, or copyright-protected, and you should handle it accordingly. If you are not the intended recipient, you do not have legal rights to retain, copy, or distribute this email or its contents, and should promptly delete the email and all electronic copies in your system; do not retain copies in any media. If you have received this email in error, please notify the sender promptly. Thank you.
>
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 629cb45f8557..de4638396592 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -184,4 +184,14 @@ config SENSORS_ZL6100
> This driver can also be built as a module. If so, the module will
> be called zl6100.
>
> +config SENSORS_DPS650AB
> + tristate "Delta DPS650AB"
> + default n
> + help
> + If you say yes here you get hardware monitoring support for the
> + Delta DPS650AB controller.
> +
> + This driver can also be built as a module. If so, the module will
> + be called dps650ab.
> +
Ahplabetic order, please.
> endif # PMBUS
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index ea0e39518c21..414818230a26 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -21,3 +21,4 @@ obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o
> obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o
> obj-$(CONFIG_SENSORS_UCD9200) += ucd9200.o
> obj-$(CONFIG_SENSORS_ZL6100) += zl6100.o
> +obj-$(CONFIG_SENSORS_DPS650AB) += dps650ab.o
Alphabetic order, please.
> diff --git a/drivers/hwmon/pmbus/dps650ab.c b/drivers/hwmon/pmbus/dps650ab.c
> new file mode 100644
> index 000000000000..3c300e621f5a
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/dps650ab.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Hardware monitoring driver for DELTA DPS650AB
> + *
> + * Copyright (c) 2018 Huaxintong Semiconductor Technology Co., Ltd.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include "pmbus.h"
> +
> +#define DPS650AB_MFR_ID "DELTA"
> +#define DPS650AB_MFR_MODEL "DPS-650AB-16 A"
> +
> +static int dps650ab_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct pmbus_driver_info *info;
> + u8 buf[I2C_SMBUS_BLOCK_MAX];
> + int ret;
> +
> + memset(buf, 0, I2C_SMBUS_BLOCK_MAX);
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_READ_BYTE_DATA
> + | I2C_FUNC_SMBUS_READ_WORD_DATA
> + | I2C_FUNC_SMBUS_READ_BLOCK_DATA))
> + return -ENODEV;
> +
> + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to read PMBUS_MFR_ID\n");
> + return ret;
> + }
> +
> + if (strncmp(buf, DPS650AB_MFR_ID, strlen(DPS650AB_MFR_ID))) {
> + dev_err(&client->dev, "DPS650AB_MFR_ID unrecognised\n");
> + return -ENODEV;
> + }
> +
> + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, buf);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to read PMBUS_MFR_MODEL\n");
> + return ret;
> + }
> +
> + if (strncmp(buf, DPS650AB_MFR_MODEL, strlen(DPS650AB_MFR_MODEL))) {
> + dev_err(&client->dev, "DPS650AB_MFR_MODEL unrecognised\n");
> + return -ENODEV;
> + }
> +
> + info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + info->pages = 1;
> + info->format[PSC_VOLTAGE_IN] = linear;
> + info->format[PSC_VOLTAGE_OUT] = linear;
> + info->format[PSC_CURRENT_IN] = linear;
> + info->format[PSC_CURRENT_OUT] = linear;
> + info->format[PSC_POWER] = linear;
> + info->format[PSC_TEMPERATURE] = linear;
> +
> + info->func[0] = PMBUS_HAVE_VIN
> + | PMBUS_HAVE_IIN | PMBUS_HAVE_PIN
> + | PMBUS_HAVE_STATUS_INPUT
> + | PMBUS_HAVE_POUT | PMBUS_HAVE_FAN12
> + | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
> + | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> + | PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2
> + | PMBUS_HAVE_STATUS_TEMP;
> + info->func[1] = info->func[0];
> +
> + return pmbus_do_probe(client, id, info);
> +}
> +
> +static const struct i2c_device_id dps650ab_id[] = {
> + {"dps650ab", 1},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, dps650ab_id);
> +
> +static struct i2c_driver dps650ab_driver = {
> + .driver = {
> + .name = "dps650ab",
> + },
> + .probe = dps650ab_probe,
> + .remove = pmbus_do_remove,
> + .id_table = dps650ab_id,
> +};
> +
> +module_i2c_driver(dps650ab_driver);
> +
> +MODULE_AUTHOR("Liuxiaoting <xiaoting.liu@hxt-semitech.com");
> +MODULE_DESCRIPTION("PMBus driver for DPS650AB");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
> index aa4cf9636e99..930e8a3e2366 100644
> --- a/drivers/hwmon/pmbus/pmbus.c
> +++ b/drivers/hwmon/pmbus/pmbus.c
> @@ -204,6 +204,8 @@ static int pmbus_probe(struct i2c_client *client,
> static const struct pmbus_device_info sgd009_pmbus_info = {
> 1, PMBUS_SKIP_STATUS_CHECK};
> static const struct pmbus_device_info pmbus_info = {0, 0};
> +static const struct pmbus_device_info dps650ab_pmbus_info = {
> + 1, PMBUS_SKIP_STATUS_CHECK};
> /*
> * Use driver_data to set the number of pages supported by the chip.
> */
> @@ -227,6 +229,7 @@ static int pmbus_probe(struct i2c_client *client,
> {"tps544c20", (kernel_ulong_t)&default_pmbus_info},
> {"tps544c25", (kernel_ulong_t)&default_pmbus_info},
> {"udt020", (kernel_ulong_t)&default_pmbus_info},
> + {"dps650ab", (kernel_ulong_t)&dps650ab_pmbus_info},
> {}
> };
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] pmbus (dps650ab): add power supply driver
[not found] <cover.1546591275.git.xiaoting.liu@hxt-semitech.com>
` (4 preceding siblings ...)
2019-01-05 17:06 ` [PATCH 2/4] dt-bindings: power supply: Document dps650ab Guenter Roeck
@ 2019-01-05 17:09 ` Guenter Roeck
2019-01-05 17:08 ` Guenter Roeck
[not found] ` <a494c032f6344528a89bf9b9aa81a3d7@HXTBJIDCEMVIW02.hxtcorp.net>
2019-01-07 2:53 ` [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data Yang, Shunyong
6 siblings, 2 replies; 17+ messages in thread
From: Guenter Roeck @ 2019-01-05 17:09 UTC (permalink / raw)
To: linux-hwmon
On Fri, Jan 04, 2019 at 05:05:27PM +0800, Xiaoting Liu wrote:
> The Delta dps650ab provides main power and standby power to server.
> dps650ab can be detected by MFR_ID and MFR_MODEL referring to
> manufacturer's feedback. This patch adds driver to moniter power
> supply status.
>
Another comment: The subject should be something like
hwmon: (pmbus) Add driver for DPS650AB power supply
Additional comments inline below.
Thanks,
Guenter
> Signed-off-by: Xiaoting Liu <xiaoting....@hxt-semitech.com>
> ---
> drivers/hwmon/pmbus/Kconfig | 10 +++++
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/dps650ab.c | 100
> +++++++++++++++++++++++++++++++++++++++++
> drivers/hwmon/pmbus/pmbus.c | 3 ++
> 4 files changed, 114 insertions(+)
>
> --
> 1.8.3.1
>
>
>
>
> This email is intended only for the named addressee. It may contain
> information that is confidential/private, legally privileged, or
> copyright-protected, and you should handle it accordingly. If you are not the
> intended recipient, you do not have legal rights to retain, copy, or
> distribute this email or its contents, and should promptly delete the email
> and all electronic copies in your system; do not retain copies in any media.
> If you have received this email in error, please notify the sender promptly.
> Thank you.
>
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 629cb45f8557..de4638396592 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -184,4 +184,14 @@ config SENSORS_ZL6100
> This driver can also be built as a module. If so, the module will
> be called zl6100.
>
> +config SENSORS_DPS650AB
> + tristate "Delta DPS650AB"
> + default n
> + help
> + If you say yes here you get hardware monitoring support for the
> + Delta DPS650AB controller.
> +
> + This driver can also be built as a module. If so, the module will
> + be called dps650ab.
> +
Ahplabetic order, please.
> endif # PMBUS
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index ea0e39518c21..414818230a26 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -21,3 +21,4 @@ obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o
> obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o
> obj-$(CONFIG_SENSORS_UCD9200) += ucd9200.o
> obj-$(CONFIG_SENSORS_ZL6100) += zl6100.o
> +obj-$(CONFIG_SENSORS_DPS650AB) += dps650ab.o
Alphabetic order, please.
> diff --git a/drivers/hwmon/pmbus/dps650ab.c b/drivers/hwmon/pmbus/dps650ab.c
> new file mode 100644
> index 000000000000..3c300e621f5a
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/dps650ab.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Hardware monitoring driver for DELTA DPS650AB
> + *
> + * Copyright (c) 2018 Huaxintong Semiconductor Technology Co., Ltd.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include "pmbus.h"
> +
> +#define DPS650AB_MFR_ID "DELTA"
> +#define DPS650AB_MFR_MODEL "DPS-650AB-16 A"
> +
> +static int dps650ab_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct pmbus_driver_info *info;
> + u8 buf[I2C_SMBUS_BLOCK_MAX];
> + int ret;
> +
> + memset(buf, 0, I2C_SMBUS_BLOCK_MAX);
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_READ_BYTE_DATA
> + | I2C_FUNC_SMBUS_READ_WORD_DATA
> + | I2C_FUNC_SMBUS_READ_BLOCK_DATA))
> + return -ENODEV;
> +
> + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to read PMBUS_MFR_ID\n");
> + return ret;
> + }
> +
> + if (strncmp(buf, DPS650AB_MFR_ID, strlen(DPS650AB_MFR_ID))) {
> + dev_err(&client->dev, "DPS650AB_MFR_ID unrecognised\n");
> + return -ENODEV;
> + }
> +
> + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, buf);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to read PMBUS_MFR_MODEL\n");
> + return ret;
> + }
> +
> + if (strncmp(buf, DPS650AB_MFR_MODEL, strlen(DPS650AB_MFR_MODEL))) {
> + dev_err(&client->dev, "DPS650AB_MFR_MODEL unrecognised\n");
> + return -ENODEV;
> + }
> +
> + info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + info->pages = 1;
> + info->format[PSC_VOLTAGE_IN] = linear;
> + info->format[PSC_VOLTAGE_OUT] = linear;
> + info->format[PSC_CURRENT_IN] = linear;
> + info->format[PSC_CURRENT_OUT] = linear;
> + info->format[PSC_POWER] = linear;
> + info->format[PSC_TEMPERATURE] = linear;
> +
> + info->func[0] = PMBUS_HAVE_VIN
> + | PMBUS_HAVE_IIN | PMBUS_HAVE_PIN
> + | PMBUS_HAVE_STATUS_INPUT
> + | PMBUS_HAVE_POUT | PMBUS_HAVE_FAN12
> + | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
> + | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> + | PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2
> + | PMBUS_HAVE_STATUS_TEMP;
> + info->func[1] = info->func[0];
> +
> + return pmbus_do_probe(client, id, info);
> +}
> +
> +static const struct i2c_device_id dps650ab_id[] = {
> + {"dps650ab", 1},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, dps650ab_id);
> +
> +static struct i2c_driver dps650ab_driver = {
> + .driver = {
> + .name = "dps650ab",
> + },
> + .probe = dps650ab_probe,
> + .remove = pmbus_do_remove,
> + .id_table = dps650ab_id,
> +};
> +
> +module_i2c_driver(dps650ab_driver);
> +
> +MODULE_AUTHOR("Liuxiaoting <xiaoting....@hxt-semitech.com");
> +MODULE_DESCRIPTION("PMBus driver for DPS650AB");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
> index aa4cf9636e99..930e8a3e2366 100644
> --- a/drivers/hwmon/pmbus/pmbus.c
> +++ b/drivers/hwmon/pmbus/pmbus.c
> @@ -204,6 +204,8 @@ static int pmbus_probe(struct i2c_client *client,
> static const struct pmbus_device_info sgd009_pmbus_info = {
> 1, PMBUS_SKIP_STATUS_CHECK};
> static const struct pmbus_device_info pmbus_info = {0, 0};
> +static const struct pmbus_device_info dps650ab_pmbus_info = {
> + 1, PMBUS_SKIP_STATUS_CHECK};
> /*
> * Use driver_data to set the number of pages supported by the chip.
> */
> @@ -227,6 +229,7 @@ static int pmbus_probe(struct i2c_client *client,
> {"tps544c20", (kernel_ulong_t)&default_pmbus_info},
> {"tps544c25", (kernel_ulong_t)&default_pmbus_info},
> {"udt020", (kernel_ulong_t)&default_pmbus_info},
> + {"dps650ab", (kernel_ulong_t)&dps650ab_pmbus_info},
> {}
> };
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data
2019-01-07 2:53 ` [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data Yang, Shunyong
@ 2019-01-07 2:35 ` Yang, Shunyong
0 siblings, 0 replies; 17+ messages in thread
From: Yang, Shunyong @ 2019-01-07 2:35 UTC (permalink / raw)
To: Guenter Roeck, Liu, Xiaoting, jdelvare; +Cc: openbmc, linux-hwmon, linux-kernel
Hi, Guenter,
Thanks for reply.
On 2019/1/4 22:54, Guenter Roeck wrote:
> On 1/4/19 12:55 AM, Xiaoting Liu wrote:
>> Current code compares device name with name in i2c_device_id to decide
>> whether PMBUS_SKIP_STATUS_CHECK should be set in pmbus_platform_data,
>> which makes adding new devices with PMBUS_SKIP_STATUS_CHECK should also
>> modify code in pmbus_probe().
>>
>> This patch adds pmbus_device_info to save pages and flags. Its pointer
>> is put in driver_data of i2c_device_id, which makes adding new device
>> more straightforward.
>>
>
> Good idea, but I don't see at this time where the patch is needed.
> Maybe in patch 3/4 which is missing from the series ?
The reason we figure out this patch is that we want to add dps650ab to
pmbus_id[] in pmbus.c. As dps650ab has flag of PMBUS_SKIP_STATUS_CHECK
and we don't want to change pmbus_probe(), so make this patch as a
premise patch.
As you stated that adding both dps650ab in pmbus.c and adding a separate
dps650ab driver file is a conceptional mistake, I will work with
Xiaoting to decide which approach we should use.
>
>> Signed-off-by: Shunyong Yang <shunyong.yang@hxt-semitech.com>
>> Signed-off-by: Xiaoting Liu <xiaoting.liu@hxt-semitech.com>
>> ---
>> drivers/hwmon/pmbus/pmbus.c | 54 +++++++++++++++++++++++++++------------------
>> include/linux/pmbus.h | 5 +++++
>> 2 files changed, 37 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
>> index 7688dab32f6e..aa4cf9636e99 100644
>> --- a/drivers/hwmon/pmbus/pmbus.c
>> +++ b/drivers/hwmon/pmbus/pmbus.c
>> @@ -172,13 +172,15 @@ static int pmbus_probe(struct i2c_client *client,
>> struct pmbus_driver_info *info;
>> struct pmbus_platform_data *pdata = NULL;
>> struct device *dev = &client->dev;
>> + struct pmbus_device_info *device_info;
>>
>> info = devm_kzalloc(dev, sizeof(struct pmbus_driver_info), GFP_KERNEL);
>> if (!info)
>> return -ENOMEM;
>>
>> - if (!strcmp(id->name, "dps460") || !strcmp(id->name, "dps800") ||
>> - !strcmp(id->name, "sgd009")) {
>> + device_info = (struct pmbus_device_info *)id->driver_data;
>> +
>> + if (device_info->flags & PMBUS_SKIP_STATUS_CHECK) {
>> pdata = devm_kzalloc(dev, sizeof(struct pmbus_platform_data),
>> GFP_KERNEL);
>> if (!pdata)
>> @@ -187,36 +189,44 @@ static int pmbus_probe(struct i2c_client *client,
>> pdata->flags = PMBUS_SKIP_STATUS_CHECK;
>> }
>>
>> - info->pages = id->driver_data;
>> + info->pages = device_info->pages;
>> info->identify = pmbus_identify;
>> dev->platform_data = pdata;
>>
>> return pmbus_do_probe(client, id, info);
>> }
>>
>> +static const struct pmbus_device_info default_pmbus_info = {1, 0};
>> +static const struct pmbus_device_info dps460_pmbus_info = {
>> + 1, PMBUS_SKIP_STATUS_CHECK};
>> +static const struct pmbus_device_info dps800_pmbus_info = {
>> + 1, PMBUS_SKIP_STATUS_CHECK};
>> +static const struct pmbus_device_info sgd009_pmbus_info = {
>> + 1, PMBUS_SKIP_STATUS_CHECK};
>
> Three structures with exactly the same content does not add value.
> Please merge into one with a common name that reflects its use.
>
Thanks. we will following you suggestion to use common name as
pmbus_info_one_skip.
>> +static const struct pmbus_device_info pmbus_info = {0, 0};
>
> default_pmbus_info and pmbus_info are badly named and ordered.
> The name should reflect that one sets one page and that the other
> leaves the number of pages unset.
>
> I would suggest three structures and names, such as
>
> pmbus_info_one
> pmbus_info_one_skip
> pmbus_info_zero
>
> though I am open to better names.
Thanks. We will change.
>
>> /*
>> * Use driver_data to set the number of pages supported by the chip.
>> */
>> static const struct i2c_device_id pmbus_id[] = {
>> - {"adp4000", 1},
>> - {"bmr453", 1},
>> - {"bmr454", 1},
>> - {"dps460", 1},
>> - {"dps800", 1},
>> - {"mdt040", 1},
>> - {"ncp4200", 1},
>> - {"ncp4208", 1},
>> - {"pdt003", 1},
>> - {"pdt006", 1},
>> - {"pdt012", 1},
>> - {"pmbus", 0},
>> - {"sgd009", 1},
>> - {"tps40400", 1},
>> - {"tps544b20", 1},
>> - {"tps544b25", 1},
>> - {"tps544c20", 1},
>> - {"tps544c25", 1},
>> - {"udt020", 1},
>> + {"adp4000", (kernel_ulong_t)&default_pmbus_info},
>> + {"bmr453", (kernel_ulong_t)&default_pmbus_info},
>> + {"bmr454", (kernel_ulong_t)&default_pmbus_info},
>> + {"dps460", (kernel_ulong_t)&dps460_pmbus_info},
>> + {"dps800", (kernel_ulong_t)&dps800_pmbus_info},
>> + {"mdt040", (kernel_ulong_t)&default_pmbus_info},
>> + {"ncp4200", (kernel_ulong_t)&default_pmbus_info},
>> + {"ncp4208", (kernel_ulong_t)&default_pmbus_info},
>> + {"pdt003", (kernel_ulong_t)&default_pmbus_info},
>> + {"pdt006", (kernel_ulong_t)&default_pmbus_info},
>> + {"pdt012", (kernel_ulong_t)&default_pmbus_info},
>> + {"pmbus", (kernel_ulong_t)&pmbus_info},
>> + {"sgd009", (kernel_ulong_t)&sgd009_pmbus_info},
>> + {"tps40400", (kernel_ulong_t)&default_pmbus_info},
>> + {"tps544b20", (kernel_ulong_t)&default_pmbus_info},
>> + {"tps544b25", (kernel_ulong_t)&default_pmbus_info},
>> + {"tps544c20", (kernel_ulong_t)&default_pmbus_info},
>> + {"tps544c25", (kernel_ulong_t)&default_pmbus_info},
>> + {"udt020", (kernel_ulong_t)&default_pmbus_info},
>> {}
>> };
>>
>> diff --git a/include/linux/pmbus.h b/include/linux/pmbus.h
>> index ee3c2aba2a8e..3c05edad7666 100644
>> --- a/include/linux/pmbus.h
>> +++ b/include/linux/pmbus.h
>> @@ -46,4 +46,9 @@ struct pmbus_platform_data {
>> struct regulator_init_data *reg_init_data;
>> };
>>
>> +struct pmbus_device_info {
>> + int pages;
>> + u32 flags;
>> +};
>> +
>
> This should not be needed here. The structure can be declared locally
> in pmbus.c (its use in patch 4/4 is wrong).
>
OK. We will move the structure back to pmbus.c. And we will decide
whether we need 4/4.
Thanks.
Shunyong.
>> #endif /* _PMBUS_H_ */
>> --
>> 1.8.3.1
>>
>>
>>
>>
>> This email is intended only for the named addressee. It may contain information that is confidential/private, legally privileged, or copyright-protected, and you should handle it accordingly. If you are not the intended recipient, you do not have legal rights to retain, copy, or distribute this email or its contents, and should promptly delete the email and all electronic copies in your system; do not retain copies in any media. If you have received this email in error, please notify the sender promptly. Thank you.
>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data
[not found] <cover.1546591275.git.xiaoting.liu@hxt-semitech.com>
` (5 preceding siblings ...)
2019-01-05 17:09 ` [PATCH 4/4] pmbus (dps650ab): add power supply driver Guenter Roeck
@ 2019-01-07 2:53 ` Yang, Shunyong
2019-01-07 2:35 ` Yang, Shunyong
6 siblings, 1 reply; 17+ messages in thread
From: Yang, Shunyong @ 2019-01-07 2:53 UTC (permalink / raw)
To: linux-hwmon
Hi, Guenter,
Thanks for reply.
On 2019/1/4 22:54, Guenter Roeck wrote:
> On 1/4/19 12:55 AM, Xiaoting Liu wrote:
>> Current code compares device name with name in i2c_device_id to decide
>> whether PMBUS_SKIP_STATUS_CHECK should be set in pmbus_platform_data,
>> which makes adding new devices with PMBUS_SKIP_STATUS_CHECK should also
>> modify code in pmbus_probe().
>>
>> This patch adds pmbus_device_info to save pages and flags. Its pointer
>> is put in driver_data of i2c_device_id, which makes adding new device
>> more straightforward.
>>
>
> Good idea, but I don't see at this time where the patch is needed.
> Maybe in patch 3/4 which is missing from the series ?
The reason we figure out this patch is that we want to add dps650ab to
pmbus_id[] in pmbus.c. As dps650ab has flag of PMBUS_SKIP_STATUS_CHECK
and we don't want to change pmbus_probe(), so make this patch as a
premise patch.
As you stated that adding both dps650ab in pmbus.c and adding a separate
dps650ab driver file is a conceptional mistake, I will work with
Xiaoting to decide which approach we should use.
>
>> Signed-off-by: Shunyong Yang <shunyong.y...@hxt-semitech.com>
>> Signed-off-by: Xiaoting Liu <xiaoting....@hxt-semitech.com>
>> ---
>> drivers/hwmon/pmbus/pmbus.c | 54
>> +++++++++++++++++++++++++++------------------
>> include/linux/pmbus.h | 5 +++++
>> 2 files changed, 37 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
>> index 7688dab32f6e..aa4cf9636e99 100644
>> --- a/drivers/hwmon/pmbus/pmbus.c
>> +++ b/drivers/hwmon/pmbus/pmbus.c
>> @@ -172,13 +172,15 @@ static int pmbus_probe(struct i2c_client *client,
>> struct pmbus_driver_info *info;
>> struct pmbus_platform_data *pdata = NULL;
>> struct device *dev = &client->dev;
>> + struct pmbus_device_info *device_info;
>>
>> info = devm_kzalloc(dev, sizeof(struct pmbus_driver_info),
>> GFP_KERNEL);
>> if (!info)
>> return -ENOMEM;
>>
>> - if (!strcmp(id->name, "dps460") || !strcmp(id->name, "dps800") ||
>> - !strcmp(id->name, "sgd009")) {
>> + device_info = (struct pmbus_device_info *)id->driver_data;
>> +
>> + if (device_info->flags & PMBUS_SKIP_STATUS_CHECK) {
>> pdata = devm_kzalloc(dev, sizeof(struct
>> pmbus_platform_data),
>> GFP_KERNEL);
>> if (!pdata)
>> @@ -187,36 +189,44 @@ static int pmbus_probe(struct i2c_client *client,
>> pdata->flags = PMBUS_SKIP_STATUS_CHECK;
>> }
>>
>> - info->pages = id->driver_data;
>> + info->pages = device_info->pages;
>> info->identify = pmbus_identify;
>> dev->platform_data = pdata;
>>
>> return pmbus_do_probe(client, id, info);
>> }
>>
>> +static const struct pmbus_device_info default_pmbus_info = {1, 0};
>> +static const struct pmbus_device_info dps460_pmbus_info = {
>> + 1, PMBUS_SKIP_STATUS_CHECK};
>> +static const struct pmbus_device_info dps800_pmbus_info = {
>> + 1, PMBUS_SKIP_STATUS_CHECK};
>> +static const struct pmbus_device_info sgd009_pmbus_info = {
>> + 1, PMBUS_SKIP_STATUS_CHECK};
>
> Three structures with exactly the same content does not add value.
> Please merge into one with a common name that reflects its use.
>
Thanks. we will following you suggestion to use common name as
pmbus_info_one_skip.
>> +static const struct pmbus_device_info pmbus_info = {0, 0};
>
> default_pmbus_info and pmbus_info are badly named and ordered.
> The name should reflect that one sets one page and that the other
> leaves the number of pages unset.
>
> I would suggest three structures and names, such as
>
> pmbus_info_one
> pmbus_info_one_skip
> pmbus_info_zero
>
> though I am open to better names.
Thanks. We will change.
>
>> /*
>> * Use driver_data to set the number of pages supported by the chip.
>> */
>> static const struct i2c_device_id pmbus_id[] = {
>> - {"adp4000", 1},
>> - {"bmr453", 1},
>> - {"bmr454", 1},
>> - {"dps460", 1},
>> - {"dps800", 1},
>> - {"mdt040", 1},
>> - {"ncp4200", 1},
>> - {"ncp4208", 1},
>> - {"pdt003", 1},
>> - {"pdt006", 1},
>> - {"pdt012", 1},
>> - {"pmbus", 0},
>> - {"sgd009", 1},
>> - {"tps40400", 1},
>> - {"tps544b20", 1},
>> - {"tps544b25", 1},
>> - {"tps544c20", 1},
>> - {"tps544c25", 1},
>> - {"udt020", 1},
>> + {"adp4000", (kernel_ulong_t)&default_pmbus_info},
>> + {"bmr453", (kernel_ulong_t)&default_pmbus_info},
>> + {"bmr454", (kernel_ulong_t)&default_pmbus_info},
>> + {"dps460", (kernel_ulong_t)&dps460_pmbus_info},
>> + {"dps800", (kernel_ulong_t)&dps800_pmbus_info},
>> + {"mdt040", (kernel_ulong_t)&default_pmbus_info},
>> + {"ncp4200", (kernel_ulong_t)&default_pmbus_info},
>> + {"ncp4208", (kernel_ulong_t)&default_pmbus_info},
>> + {"pdt003", (kernel_ulong_t)&default_pmbus_info},
>> + {"pdt006", (kernel_ulong_t)&default_pmbus_info},
>> + {"pdt012", (kernel_ulong_t)&default_pmbus_info},
>> + {"pmbus", (kernel_ulong_t)&pmbus_info},
>> + {"sgd009", (kernel_ulong_t)&sgd009_pmbus_info},
>> + {"tps40400", (kernel_ulong_t)&default_pmbus_info},
>> + {"tps544b20", (kernel_ulong_t)&default_pmbus_info},
>> + {"tps544b25", (kernel_ulong_t)&default_pmbus_info},
>> + {"tps544c20", (kernel_ulong_t)&default_pmbus_info},
>> + {"tps544c25", (kernel_ulong_t)&default_pmbus_info},
>> + {"udt020", (kernel_ulong_t)&default_pmbus_info},
>> {}
>> };
>>
>> diff --git a/include/linux/pmbus.h b/include/linux/pmbus.h
>> index ee3c2aba2a8e..3c05edad7666 100644
>> --- a/include/linux/pmbus.h
>> +++ b/include/linux/pmbus.h
>> @@ -46,4 +46,9 @@ struct pmbus_platform_data {
>> struct regulator_init_data *reg_init_data;
>> };
>>
>> +struct pmbus_device_info {
>> + int pages;
>> + u32 flags;
>> +};
>> +
>
> This should not be needed here. The structure can be declared locally
> in pmbus.c (its use in patch 4/4 is wrong).
>
OK. We will move the structure back to pmbus.c. And we will decide
whether we need 4/4.
Thanks.
Shunyong.
>> #endif /* _PMBUS_H_ */
>> --
>> 1.8.3.1
>>
>>
>>
>>
>> This email is intended only for the named addressee. It may contain
>> information that is confidential/private, legally privileged, or
>> copyright-protected, and you should handle it accordingly. If you are not
>> the intended recipient, you do not have legal rights to retain, copy, or
>> distribute this email or its contents, and should promptly delete the email
>> and all electronic copies in your system; do not retain copies in any media.
>> If you have received this email in error, please notify the sender promptly.
>> Thank you.
>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] dt-bindings: power supply: Document dps650ab
2019-01-05 17:06 ` [PATCH 2/4] dt-bindings: power supply: Document dps650ab Guenter Roeck
2019-01-05 17:05 ` Guenter Roeck
@ 2019-01-07 6:30 ` Liu, Xiaoting
2019-01-07 6:35 ` Guenter Roeck
1 sibling, 1 reply; 17+ messages in thread
From: Liu, Xiaoting @ 2019-01-07 6:30 UTC (permalink / raw)
To: Guenter Roeck
Cc: jdelvare, robh+dt, mark.rutland, openbmc, linux-hwmon,
devicetree, Yang, Shunyong, Wang, Dongsheng
Hi Guenter Roeck,
Thanks for your reply.
On 2019/1/6 1:05, Guenter Roeck wrote:
> On Fri, Jan 04, 2019 at 05:00:30PM +0800, Xiaoting Liu wrote:
>> Bindings for DPS650AB power, voltage, temperature, and fan monitering.
>>
> Misleading subject. While the DPS650AB may ber a power supply,
> it is a PMBus hwmon driver, and should be marked accordingly in the subject.
>
> Guenter
We will change the subject as "dt-bindings: hwmon: Add dps650ab
documentation", is that okey?
Thanks,
Xiaoting.
>> Signed-off-by: Xiaoting Liu <xiaoting.liu@hxt-semitech.com>
>> ---
>> Documentation/devicetree/bindings/hwmon/dps650ab.txt | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> --
>> 1.8.3.1
>>
>>
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/dps650ab.txt b/Documentation/devicetree/bindings/hwmon/dps650ab.txt
>> new file mode 100644
>> index 000000000000..bbd601100e66
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/dps650ab.txt
>> @@ -0,0 +1,11 @@
>> +Bindings for DPS650AB power, voltage, temperature, and fan monitering
>> +
>> +Required properties:
>> +- compatible : "dps650ab"
>> +- reg : I2C address, one of 0x58, 0x59.
>> +
>> +Example:
>> + dps650ab@58 {
>> + compatible = "dps650ab";
>> + reg = <0x58>;
>> + };
This email is intended only for the named addressee. It may contain information that is confidential/private, legally privileged, or copyright-protected, and you should handle it accordingly. If you are not the intended recipient, you do not have legal rights to retain, copy, or distribute this email or its contents, and should promptly delete the email and all electronic copies in your system; do not retain copies in any media. If you have received this email in error, please notify the sender promptly. Thank you.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] pmbus (dps650ab): add power supply driver
[not found] ` <a494c032f6344528a89bf9b9aa81a3d7@HXTBJIDCEMVIW02.hxtcorp.net>
@ 2019-01-07 6:35 ` Guenter Roeck
0 siblings, 0 replies; 17+ messages in thread
From: Guenter Roeck @ 2019-01-07 6:35 UTC (permalink / raw)
To: Liu, Xiaoting
Cc: jdelvare, openbmc, linux-kernel, linux-hwmon, Yang, Shunyong,
Wang, Dongsheng
On 1/6/19 10:25 PM, Liu, Xiaoting wrote:
> Hi Guenter Roeck,
>
> Thanks for your apply, we will drop this patch and add the structpmbus_device_info in PMBus.c.
>
NP. Make sure though that the auto-detection finds all properties.
If it does, there is really no reason to have an extra driver.
Thanks,
Guenter
> Thanks,
> Xiaoting.
>
> On 2019/1/6 1:08, Guenter Roeck wrote:
>> On Fri, Jan 04, 2019 at 05:05:27PM +0800, Xiaoting Liu wrote:
>>> The Delta dps650ab provides main power and standby power to server.
>>> dps650ab can be detected by MFR_ID and MFR_MODEL referring to
>>> manufacturer's feedback. This patch adds driver to moniter power
>>> supply status.
>>>
>> Another comment: The subject should be something like
>> hwmon: (pmbus) Add driver for DPS650AB power supply
>>
>> Additional comments inline below.
>>
>> Thanks,
>> Guenter
>>> Signed-off-by: Xiaoting Liu<xiaoting.liu@hxt-semitech.com>
>>> ---
>>> drivers/hwmon/pmbus/Kconfig | 10 +++++
>>> drivers/hwmon/pmbus/Makefile | 1 +
>>> drivers/hwmon/pmbus/dps650ab.c | 100 +++++++++++++++++++++++++++++++++++++++++
>>> drivers/hwmon/pmbus/pmbus.c | 3 ++
>>> 4 files changed, 114 insertions(+)
>>>
>>> --
>>> 1.8.3.1
>>>
>>>
>>> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
>>> index 629cb45f8557..de4638396592 100644
>>> --- a/drivers/hwmon/pmbus/Kconfig
>>> +++ b/drivers/hwmon/pmbus/Kconfig
>>> @@ -184,4 +184,14 @@ config SENSORS_ZL6100
>>> This driver can also be built as a module. If so, the module will
>>> be called zl6100.
>>>
>>> +config SENSORS_DPS650AB
>>> + tristate "Delta DPS650AB"
>>> + default n
>>> + help
>>> + If you say yes here you get hardware monitoring support for the
>>> + Delta DPS650AB controller.
>>> +
>>> + This driver can also be built as a module. If so, the module will
>>> + be called dps650ab.
>>> +
>> Ahplabetic order, please.
>>
>>> endif # PMBUS
>>> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
>>> index ea0e39518c21..414818230a26 100644
>>> --- a/drivers/hwmon/pmbus/Makefile
>>> +++ b/drivers/hwmon/pmbus/Makefile
>>> @@ -21,3 +21,4 @@ obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o
>>> obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o
>>> obj-$(CONFIG_SENSORS_UCD9200) += ucd9200.o
>>> obj-$(CONFIG_SENSORS_ZL6100) += zl6100.o
>>> +obj-$(CONFIG_SENSORS_DPS650AB) += dps650ab.o
>> Alphabetic order, please.
>>
>>> diff --git a/drivers/hwmon/pmbus/dps650ab.c b/drivers/hwmon/pmbus/dps650ab.c
>>> new file mode 100644
>>> index 000000000000..3c300e621f5a
>>> --- /dev/null
>>> +++ b/drivers/hwmon/pmbus/dps650ab.c
>>> @@ -0,0 +1,100 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Hardware monitoring driver for DELTA DPS650AB
>>> + *
>>> + * Copyright (c) 2018 Huaxintong Semiconductor Technology Co., Ltd.
>>> + */
>>> +
>>> +#include <linux/err.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/init.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include "pmbus.h"
>>> +
>>> +#define DPS650AB_MFR_ID "DELTA"
>>> +#define DPS650AB_MFR_MODEL "DPS-650AB-16 A"
>>> +
>>> +static int dps650ab_probe(struct i2c_client *client,
>>> + const struct i2c_device_id *id)
>>> +{
>>> + struct pmbus_driver_info *info;
>>> + u8 buf[I2C_SMBUS_BLOCK_MAX];
>>> + int ret;
>>> +
>>> + memset(buf, 0, I2C_SMBUS_BLOCK_MAX);
>>> +
>>> + if (!i2c_check_functionality(client->adapter,
>>> + I2C_FUNC_SMBUS_READ_BYTE_DATA
>>> + | I2C_FUNC_SMBUS_READ_WORD_DATA
>>> + | I2C_FUNC_SMBUS_READ_BLOCK_DATA))
>>> + return -ENODEV;
>>> +
>>> + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
>>> + if (ret < 0) {
>>> + dev_err(&client->dev, "Failed to read PMBUS_MFR_ID\n");
>>> + return ret;
>>> + }
>>> +
>>> + if (strncmp(buf, DPS650AB_MFR_ID, strlen(DPS650AB_MFR_ID))) {
>>> + dev_err(&client->dev, "DPS650AB_MFR_ID unrecognised\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, buf);
>>> + if (ret < 0) {
>>> + dev_err(&client->dev, "Failed to read PMBUS_MFR_MODEL\n");
>>> + return ret;
>>> + }
>>> +
>>> + if (strncmp(buf, DPS650AB_MFR_MODEL, strlen(DPS650AB_MFR_MODEL))) {
>>> + dev_err(&client->dev, "DPS650AB_MFR_MODEL unrecognised\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
>>> + if (!info)
>>> + return -ENOMEM;
>>> +
>>> + info->pages = 1;
>>> + info->format[PSC_VOLTAGE_IN] = linear;
>>> + info->format[PSC_VOLTAGE_OUT] = linear;
>>> + info->format[PSC_CURRENT_IN] = linear;
>>> + info->format[PSC_CURRENT_OUT] = linear;
>>> + info->format[PSC_POWER] = linear;
>>> + info->format[PSC_TEMPERATURE] = linear;
>>> +
>>> + info->func[0] = PMBUS_HAVE_VIN
>>> + | PMBUS_HAVE_IIN | PMBUS_HAVE_PIN
>>> + | PMBUS_HAVE_STATUS_INPUT
>>> + | PMBUS_HAVE_POUT | PMBUS_HAVE_FAN12
>>> + | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
>>> + | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
>>> + | PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2
>>> + | PMBUS_HAVE_STATUS_TEMP;
>>> + info->func[1] = info->func[0];
>>> +
>>> + return pmbus_do_probe(client, id, info);
>>> +}
>>> +
>>> +static const struct i2c_device_id dps650ab_id[] = {
>>> + {"dps650ab", 1},
>>> + {}
>>> +};
>>> +
>>> +MODULE_DEVICE_TABLE(i2c, dps650ab_id);
>>> +
>>> +static struct i2c_driver dps650ab_driver = {
>>> + .driver = {
>>> + .name = "dps650ab",
>>> + },
>>> + .probe = dps650ab_probe,
>>> + .remove = pmbus_do_remove,
>>> + .id_table = dps650ab_id,
>>> +};
>>> +
>>> +module_i2c_driver(dps650ab_driver);
>>> +
>>> +MODULE_AUTHOR("Liuxiaoting <xiaoting.liu@hxt-semitech.com");
>>> +MODULE_DESCRIPTION("PMBus driver for DPS650AB");
>>> +MODULE_LICENSE("GPL v2");
>>> diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
>>> index aa4cf9636e99..930e8a3e2366 100644
>>> --- a/drivers/hwmon/pmbus/pmbus.c
>>> +++ b/drivers/hwmon/pmbus/pmbus.c
>>> @@ -204,6 +204,8 @@ static int pmbus_probe(struct i2c_client *client,
>>> static const struct pmbus_device_info sgd009_pmbus_info = {
>>> 1, PMBUS_SKIP_STATUS_CHECK};
>>> static const struct pmbus_device_info pmbus_info = {0, 0};
>>> +static const struct pmbus_device_info dps650ab_pmbus_info = {
>>> + 1, PMBUS_SKIP_STATUS_CHECK};
>>> /*
>>> * Use driver_data to set the number of pages supported by the chip.
>>> */
>>> @@ -227,6 +229,7 @@ static int pmbus_probe(struct i2c_client *client,
>>> {"tps544c20", (kernel_ulong_t)&default_pmbus_info},
>>> {"tps544c25", (kernel_ulong_t)&default_pmbus_info},
>>> {"udt020", (kernel_ulong_t)&default_pmbus_info},
>>> + {"dps650ab", (kernel_ulong_t)&dps650ab_pmbus_info},
>>> {}
>>> };
>
>
>
>
>
> This email is intended only for the named addressee. It may contain information that is confidential/private, legally privileged, or copyright-protected, and you should handle it accordingly. If you are not the intended recipient, you do not have legal rights to retain, copy, or distribute this email or its contents, and should promptly delete the email and all electronic copies in your system; do not retain copies in any media. If you have received this email in error, please notify the sender promptly. Thank you.
>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] dt-bindings: power supply: Document dps650ab
2019-01-07 6:30 ` Liu, Xiaoting
@ 2019-01-07 6:35 ` Guenter Roeck
0 siblings, 0 replies; 17+ messages in thread
From: Guenter Roeck @ 2019-01-07 6:35 UTC (permalink / raw)
To: Liu, Xiaoting
Cc: jdelvare, robh+dt, mark.rutland, openbmc, linux-hwmon,
devicetree, Yang, Shunyong, Wang, Dongsheng
On 1/6/19 10:30 PM, Liu, Xiaoting wrote:
> Hi Guenter Roeck,
>
> Thanks for your reply.
>
> On 2019/1/6 1:05, Guenter Roeck wrote:
>> On Fri, Jan 04, 2019 at 05:00:30PM +0800, Xiaoting Liu wrote:
>>> Bindings for DPS650AB power, voltage, temperature, and fan monitering.
>>>
>> Misleading subject. While the DPS650AB may ber a power supply,
>> it is a PMBus hwmon driver, and should be marked accordingly in the subject.
>>
>> Guenter
>
> We will change the subject as "dt-bindings: hwmon: Add dps650ab
> documentation", is that okey?
>
Sure.
Thanks,
Guenter
> Thanks,
>
> Xiaoting.
>
>>> Signed-off-by: Xiaoting Liu <xiaoting.liu@hxt-semitech.com>
>>> ---
>>> Documentation/devicetree/bindings/hwmon/dps650ab.txt | 11 +++++++++++
>>> 1 file changed, 11 insertions(+)
>>>
>>> --
>>> 1.8.3.1
>>>
>>>
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/dps650ab.txt b/Documentation/devicetree/bindings/hwmon/dps650ab.txt
>>> new file mode 100644
>>> index 000000000000..bbd601100e66
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/hwmon/dps650ab.txt
>>> @@ -0,0 +1,11 @@
>>> +Bindings for DPS650AB power, voltage, temperature, and fan monitering
>>> +
>>> +Required properties:
>>> +- compatible : "dps650ab"
>>> +- reg : I2C address, one of 0x58, 0x59.
>>> +
>>> +Example:
>>> + dps650ab@58 {
>>> + compatible = "dps650ab";
>>> + reg = <0x58>;
>>> + };
>
>
>
>
>
> This email is intended only for the named addressee. It may contain information that is confidential/private, legally privileged, or copyright-protected, and you should handle it accordingly. If you are not the intended recipient, you do not have legal rights to retain, copy, or distribute this email or its contents, and should promptly delete the email and all electronic copies in your system; do not retain copies in any media. If you have received this email in error, please notify the sender promptly. Thank you.
>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] dt-bindings: power supply: Document dps650ab
2019-01-04 9:00 ` [PATCH 2/4] dt-bindings: power supply: Document dps650ab Xiaoting Liu
@ 2019-01-11 19:36 ` Rob Herring
2019-01-14 2:41 ` Liu, Xiaoting
0 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2019-01-11 19:36 UTC (permalink / raw)
To: Xiaoting Liu
Cc: jdelvare, linux, mark.rutland, openbmc, linux-hwmon, devicetree,
shunyong.yang, dongsheng.wang
On Fri, Jan 04, 2019 at 05:00:30PM +0800, Xiaoting Liu wrote:
> Bindings for DPS650AB power, voltage, temperature, and fan monitering.
s/monitering/monitoring/
>
> Signed-off-by: Xiaoting Liu <xiaoting.liu@hxt-semitech.com>
> ---
> Documentation/devicetree/bindings/hwmon/dps650ab.txt | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/dps650ab.txt b/Documentation/devicetree/bindings/hwmon/dps650ab.txt
> new file mode 100644
> index 000000000000..bbd601100e66
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/dps650ab.txt
> @@ -0,0 +1,11 @@
> +Bindings for DPS650AB power, voltage, temperature, and fan monitering
> +
> +Required properties:
> +- compatible : "dps650ab"
Needs a vendor prefix.
> +- reg : I2C address, one of 0x58, 0x59.
> +
> +Example:
> + dps650ab@58 {
> + compatible = "dps650ab";
> + reg = <0x58>;
> + };
> --
> 1.8.3.1
>
>
>
>
> This email is intended only for the named addressee. It may contain information that is confidential/private, legally privileged, or copyright-protected, and you should handle it accordingly. If you are not the intended recipient, you do not have legal rights to retain, copy, or distribute this email or its contents, and should promptly delete the email and all electronic copies in your system; do not retain copies in any media. If you have received this email in error, please notify the sender promptly. Thank you.
We can't accept emails with these clauses.
Rob
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] dt-bindings: power supply: Document dps650ab
2019-01-11 19:36 ` Rob Herring
@ 2019-01-14 2:41 ` Liu, Xiaoting
0 siblings, 0 replies; 17+ messages in thread
From: Liu, Xiaoting @ 2019-01-14 2:41 UTC (permalink / raw)
To: Rob Herring
Cc: jdelvare, linux, mark.rutland, openbmc, linux-hwmon, devicetree,
Yang, Shunyong, Wang, Dongsheng
Hi Rob Herring,
Thanks for your reply.
On 2019/1/12 3:42, Rob Herring wrote:
> On Fri, Jan 04, 2019 at 05:00:30PM +0800, Xiaoting Liu wrote:
>> Bindings for DPS650AB power, voltage, temperature, and fan monitering.
> s/monitering/monitoring/
I delivered the latest v3 patch and updated the subject. you can refer
to the following link:
https://patchwork.ozlabs.org/patch/1022664/
>
>> Signed-off-by: Xiaoting Liu <xiaoting.liu@hxt-semitech.com>
>> ---
>> Documentation/devicetree/bindings/hwmon/dps650ab.txt | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/dps650ab.txt b/Documentation/devicetree/bindings/hwmon/dps650ab.txt
>> new file mode 100644
>> index 000000000000..bbd601100e66
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/dps650ab.txt
>> @@ -0,0 +1,11 @@
>> +Bindings for DPS650AB power, voltage, temperature, and fan monitering
>> +
>> +Required properties:
>> +- compatible : "dps650ab"
> Needs a vendor prefix.g
Add vendor delta on v3 patch.
>
>> +- reg : I2C address, one of 0x58, 0x59.
>> +
>> +Example:
>> + dps650ab@58 {
>> + compatible = "dps650ab";
>> + reg = <0x58>;
>> + };
>> --
>> 1.8.3.1
>>
>>
>>
>>
>> This email is intended only for the named addressee. It may contain information that is confidential/private, legally privileged, or copyright-protected, and you should handle it accordingly. If you are not the intended recipient, you do not have legal rights to retain, copy, or distribute this email or its contents, and should promptly delete the email and all electronic copies in your system; do not retain copies in any media. If you have received this email in error, please notify the sender promptly. Thank you.
> We can't accept emails with these clauses.
>
> Rob
Delete the message on v3 patch.
Thanks,
Xiaoting
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2019-01-14 2:41 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <cover.1546591275.git.xiaoting.liu@hxt-semitech.com>
2019-01-04 8:55 ` [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data Xiaoting Liu
2019-01-04 9:00 ` [PATCH 2/4] dt-bindings: power supply: Document dps650ab Xiaoting Liu
2019-01-11 19:36 ` Rob Herring
2019-01-14 2:41 ` Liu, Xiaoting
2019-01-04 9:05 ` [PATCH 4/4] pmbus (dps650ab): add power supply driver Xiaoting Liu
2019-01-04 14:41 ` Guenter Roeck
2019-01-04 14:54 ` [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data Guenter Roeck
2019-01-04 14:54 ` Guenter Roeck
2019-01-05 17:06 ` [PATCH 2/4] dt-bindings: power supply: Document dps650ab Guenter Roeck
2019-01-05 17:05 ` Guenter Roeck
2019-01-07 6:30 ` Liu, Xiaoting
2019-01-07 6:35 ` Guenter Roeck
2019-01-05 17:09 ` [PATCH 4/4] pmbus (dps650ab): add power supply driver Guenter Roeck
2019-01-05 17:08 ` Guenter Roeck
[not found] ` <a494c032f6344528a89bf9b9aa81a3d7@HXTBJIDCEMVIW02.hxtcorp.net>
2019-01-07 6:35 ` Guenter Roeck
2019-01-07 2:53 ` [PATCH 1/4] pmbus: associate PMBUS_SKIP_STATUS_CHECK with driver_data Yang, Shunyong
2019-01-07 2:35 ` Yang, Shunyong
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).