* [PATCH 1/3] hwmon: pmbus: Reduce clear fault page invocations
@ 2023-08-17 9:25 Naresh Solanki
2023-08-17 9:25 ` [PATCH 2/3] hwmon: pmbus: Drop unnecessary clear fault page Naresh Solanki
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Naresh Solanki @ 2023-08-17 9:25 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare
Cc: Patrick Rudolph, Naresh Solanki, linux-hwmon, linux-kernel
From: Patrick Rudolph <patrick.rudolph@9elements.com>
Observing I2C traffic revealed consecutive transmission of CLEAR_FAULT
commands. While this doesn't cause issues, it extends driver probe time.
Avoid invoking pmbus_clear_fault_page for virtual registers, as they're
managed by the driver, not the chip.
TEST: Verified using an I2C bus analyzer that only one CLEAR_FAULT
command is send instead 5 in a row.
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
drivers/hwmon/pmbus/pmbus_core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 69a4e62b6c8d..cbfabdd69056 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -561,7 +561,8 @@ static bool pmbus_check_register(struct i2c_client *client,
rv = pmbus_check_status_cml(client);
if (rv < 0 && (data->flags & PMBUS_READ_STATUS_AFTER_FAILED_CHECK))
data->read_status(client, -1);
- pmbus_clear_fault_page(client, -1);
+ if (reg < PMBUS_VIRT_BASE)
+ pmbus_clear_fault_page(client, -1);
return rv >= 0;
}
base-commit: fc8df28be7155ddcf8731b4c85fd3372811ba5ac
--
2.41.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/3] hwmon: pmbus: Drop unnecessary clear fault page
2023-08-17 9:25 [PATCH 1/3] hwmon: pmbus: Reduce clear fault page invocations Naresh Solanki
@ 2023-08-17 9:25 ` Naresh Solanki
2023-08-19 12:18 ` Guenter Roeck
2023-08-17 9:25 ` [PATCH 3/3] hwmon: pmbus: Fix -EIO seen on pli1209 Naresh Solanki
2023-08-19 12:17 ` [PATCH 1/3] hwmon: pmbus: Reduce clear fault page invocations Guenter Roeck
2 siblings, 1 reply; 6+ messages in thread
From: Naresh Solanki @ 2023-08-17 9:25 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare
Cc: Patrick Rudolph, Naresh Solanki, linux-hwmon, linux-kernel
From: Patrick Rudolph <patrick.rudolph@9elements.com>
The pmbus_check_byte_register function already calls clear
fault page, so there's no need to do it again in
pmbus_identify_common.
TEST: Verified using an I2C bus analyser to confirm that only
one clear fault page is being send instead of two in a row.
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
drivers/hwmon/pmbus/pmbus_core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index cbfabdd69056..1363d9f89181 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -2541,7 +2541,6 @@ static int pmbus_identify_common(struct i2c_client *client,
}
}
- pmbus_clear_fault_page(client, page);
return 0;
}
--
2.41.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] hwmon: pmbus: Drop unnecessary clear fault page
2023-08-17 9:25 ` [PATCH 2/3] hwmon: pmbus: Drop unnecessary clear fault page Naresh Solanki
@ 2023-08-19 12:18 ` Guenter Roeck
0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2023-08-19 12:18 UTC (permalink / raw)
To: Naresh Solanki; +Cc: Jean Delvare, Patrick Rudolph, linux-hwmon, linux-kernel
On Thu, Aug 17, 2023 at 11:25:25AM +0200, Naresh Solanki wrote:
> From: Patrick Rudolph <patrick.rudolph@9elements.com>
>
> The pmbus_check_byte_register function already calls clear
> fault page, so there's no need to do it again in
> pmbus_identify_common.
>
> TEST: Verified using an I2C bus analyser to confirm that only
> one clear fault page is being send instead of two in a row.
>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
Applied.
Thanks,
Guenter
> ---
> drivers/hwmon/pmbus/pmbus_core.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index cbfabdd69056..1363d9f89181 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -2541,7 +2541,6 @@ static int pmbus_identify_common(struct i2c_client *client,
> }
> }
>
> - pmbus_clear_fault_page(client, page);
> return 0;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] hwmon: pmbus: Fix -EIO seen on pli1209
2023-08-17 9:25 [PATCH 1/3] hwmon: pmbus: Reduce clear fault page invocations Naresh Solanki
2023-08-17 9:25 ` [PATCH 2/3] hwmon: pmbus: Drop unnecessary clear fault page Naresh Solanki
@ 2023-08-17 9:25 ` Naresh Solanki
2023-08-19 12:19 ` Guenter Roeck
2023-08-19 12:17 ` [PATCH 1/3] hwmon: pmbus: Reduce clear fault page invocations Guenter Roeck
2 siblings, 1 reply; 6+ messages in thread
From: Naresh Solanki @ 2023-08-17 9:25 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare
Cc: Patrick Rudolph, Naresh Solanki, linux-hwmon, linux-kernel
From: Patrick Rudolph <patrick.rudolph@9elements.com>
After doing performance optimizations the pli1209 driver failed to
probe with a probabilty of 2%. It wasn't able to read the PMBUS_OPERATION
register due to an -EIO error.
An investigation showed that the PLI1209 takes 230 usec to execute the
CLEAR_FAULTS command. During that time it's busy and NACKs all requests
on the SMBUS interface. It also NACKs reads on PMBUS_STATUS_BYTE
making it impossible to poll the BUSY flag.
Add a custom write_data function to just wait for not BUSY unconditionally
after sending a CLEAR_FAULTS command.
TEST: Verified using an I2C bus analyser that no more NACKs are seen after
sending a CLEAR_FAULTS command.
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
drivers/hwmon/pmbus/pli1209bc.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/hwmon/pmbus/pli1209bc.c b/drivers/hwmon/pmbus/pli1209bc.c
index 7d8bd3167b21..c95433790b11 100644
--- a/drivers/hwmon/pmbus/pli1209bc.c
+++ b/drivers/hwmon/pmbus/pli1209bc.c
@@ -5,6 +5,7 @@
* Copyright (c) 2022 9elements GmbH
*/
+#include <linux/delay.h>
#include <linux/i2c.h>
#include <linux/module.h>
#include <linux/pmbus.h>
@@ -53,6 +54,30 @@ static int pli1209bc_read_word_data(struct i2c_client *client, int page,
}
}
+static int pli1209bc_write_byte(struct i2c_client *client, int page, u8 reg)
+{
+ int ret;
+
+ switch (reg) {
+ case PMBUS_CLEAR_FAULTS:
+ ret = pmbus_write_byte(client, page, reg);
+ /*
+ * PLI1209 takes 230 usec to execute the CLEAR_FAULTS command.
+ * During that time it's busy and NACKs all requests on the
+ * SMBUS interface. It also NACKs reads on PMBUS_STATUS_BYTE
+ * making it impossible to poll the BUSY flag.
+ *
+ * Just wait for not BUSY unconditionally.
+ */
+ usleep_range(250, 300);
+ break;
+ default:
+ ret = -ENODATA;
+ break;
+ }
+ return ret;
+}
+
#if IS_ENABLED(CONFIG_SENSORS_PLI1209BC_REGULATOR)
static const struct regulator_desc pli1209bc_reg_desc = {
.name = "vout2",
@@ -102,6 +127,7 @@ static struct pmbus_driver_info pli1209bc_info = {
| PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP
| PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT,
.read_word_data = pli1209bc_read_word_data,
+ .write_byte = pli1209bc_write_byte,
#if IS_ENABLED(CONFIG_SENSORS_PLI1209BC_REGULATOR)
.num_regulators = 1,
.reg_desc = &pli1209bc_reg_desc,
--
2.41.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] hwmon: pmbus: Fix -EIO seen on pli1209
2023-08-17 9:25 ` [PATCH 3/3] hwmon: pmbus: Fix -EIO seen on pli1209 Naresh Solanki
@ 2023-08-19 12:19 ` Guenter Roeck
0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2023-08-19 12:19 UTC (permalink / raw)
To: Naresh Solanki; +Cc: Jean Delvare, Patrick Rudolph, linux-hwmon, linux-kernel
On Thu, Aug 17, 2023 at 11:25:26AM +0200, Naresh Solanki wrote:
> From: Patrick Rudolph <patrick.rudolph@9elements.com>
>
> After doing performance optimizations the pli1209 driver failed to
> probe with a probabilty of 2%. It wasn't able to read the PMBUS_OPERATION
> register due to an -EIO error.
>
> An investigation showed that the PLI1209 takes 230 usec to execute the
> CLEAR_FAULTS command. During that time it's busy and NACKs all requests
> on the SMBUS interface. It also NACKs reads on PMBUS_STATUS_BYTE
> making it impossible to poll the BUSY flag.
>
> Add a custom write_data function to just wait for not BUSY unconditionally
> after sending a CLEAR_FAULTS command.
>
> TEST: Verified using an I2C bus analyser that no more NACKs are seen after
> sending a CLEAR_FAULTS command.
>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
Applied.
Thanks,
Guenter
> ---
> drivers/hwmon/pmbus/pli1209bc.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/drivers/hwmon/pmbus/pli1209bc.c b/drivers/hwmon/pmbus/pli1209bc.c
> index 7d8bd3167b21..c95433790b11 100644
> --- a/drivers/hwmon/pmbus/pli1209bc.c
> +++ b/drivers/hwmon/pmbus/pli1209bc.c
> @@ -5,6 +5,7 @@
> * Copyright (c) 2022 9elements GmbH
> */
>
> +#include <linux/delay.h>
> #include <linux/i2c.h>
> #include <linux/module.h>
> #include <linux/pmbus.h>
> @@ -53,6 +54,30 @@ static int pli1209bc_read_word_data(struct i2c_client *client, int page,
> }
> }
>
> +static int pli1209bc_write_byte(struct i2c_client *client, int page, u8 reg)
> +{
> + int ret;
> +
> + switch (reg) {
> + case PMBUS_CLEAR_FAULTS:
> + ret = pmbus_write_byte(client, page, reg);
> + /*
> + * PLI1209 takes 230 usec to execute the CLEAR_FAULTS command.
> + * During that time it's busy and NACKs all requests on the
> + * SMBUS interface. It also NACKs reads on PMBUS_STATUS_BYTE
> + * making it impossible to poll the BUSY flag.
> + *
> + * Just wait for not BUSY unconditionally.
> + */
> + usleep_range(250, 300);
> + break;
> + default:
> + ret = -ENODATA;
> + break;
> + }
> + return ret;
> +}
> +
> #if IS_ENABLED(CONFIG_SENSORS_PLI1209BC_REGULATOR)
> static const struct regulator_desc pli1209bc_reg_desc = {
> .name = "vout2",
> @@ -102,6 +127,7 @@ static struct pmbus_driver_info pli1209bc_info = {
> | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP
> | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT,
> .read_word_data = pli1209bc_read_word_data,
> + .write_byte = pli1209bc_write_byte,
> #if IS_ENABLED(CONFIG_SENSORS_PLI1209BC_REGULATOR)
> .num_regulators = 1,
> .reg_desc = &pli1209bc_reg_desc,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] hwmon: pmbus: Reduce clear fault page invocations
2023-08-17 9:25 [PATCH 1/3] hwmon: pmbus: Reduce clear fault page invocations Naresh Solanki
2023-08-17 9:25 ` [PATCH 2/3] hwmon: pmbus: Drop unnecessary clear fault page Naresh Solanki
2023-08-17 9:25 ` [PATCH 3/3] hwmon: pmbus: Fix -EIO seen on pli1209 Naresh Solanki
@ 2023-08-19 12:17 ` Guenter Roeck
2 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2023-08-19 12:17 UTC (permalink / raw)
To: Naresh Solanki; +Cc: Jean Delvare, Patrick Rudolph, linux-hwmon, linux-kernel
On Thu, Aug 17, 2023 at 11:25:24AM +0200, Naresh Solanki wrote:
> From: Patrick Rudolph <patrick.rudolph@9elements.com>
>
> Observing I2C traffic revealed consecutive transmission of CLEAR_FAULT
> commands. While this doesn't cause issues, it extends driver probe time.
>
> Avoid invoking pmbus_clear_fault_page for virtual registers, as they're
> managed by the driver, not the chip.
>
> TEST: Verified using an I2C bus analyzer that only one CLEAR_FAULT
> command is send instead 5 in a row.
>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
Applied.
Thanks,
Guenter
> ---
> drivers/hwmon/pmbus/pmbus_core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>
> base-commit: fc8df28be7155ddcf8731b4c85fd3372811ba5ac
>
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 69a4e62b6c8d..cbfabdd69056 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -561,7 +561,8 @@ static bool pmbus_check_register(struct i2c_client *client,
> rv = pmbus_check_status_cml(client);
> if (rv < 0 && (data->flags & PMBUS_READ_STATUS_AFTER_FAILED_CHECK))
> data->read_status(client, -1);
> - pmbus_clear_fault_page(client, -1);
> + if (reg < PMBUS_VIRT_BASE)
> + pmbus_clear_fault_page(client, -1);
> return rv >= 0;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-19 12:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-17 9:25 [PATCH 1/3] hwmon: pmbus: Reduce clear fault page invocations Naresh Solanki
2023-08-17 9:25 ` [PATCH 2/3] hwmon: pmbus: Drop unnecessary clear fault page Naresh Solanki
2023-08-19 12:18 ` Guenter Roeck
2023-08-17 9:25 ` [PATCH 3/3] hwmon: pmbus: Fix -EIO seen on pli1209 Naresh Solanki
2023-08-19 12:19 ` Guenter Roeck
2023-08-19 12:17 ` [PATCH 1/3] hwmon: pmbus: Reduce clear fault page invocations Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®