* [RFC][PATCH] hwmon: (pmbus) Support 4th PSU temperature sensor
@ 2021-07-28 9:38 Paul Menzel
2021-07-28 9:40 ` Paul Menzel
2021-07-28 13:05 ` Guenter Roeck
0 siblings, 2 replies; 3+ messages in thread
From: Paul Menzel @ 2021-07-28 9:38 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare
Cc: Guohan Lu, Madhava Reddy Siddareddygari, Venkat Garigipati,
Billie Alsup, Paul Menzel, linux-hwmon, linux-kernel
From: Madhava Reddy Siddareddygari <msiddare@cisco.com>
PSU650W has four temperature sensors, while the pmbus driver currently
only support three temperature sensors.
So, support a fourth temp sensor, i. e. PSU outlet temperature sensor,
by copying what is done for temperature sensor 3, and use register 0xDF.
PSU650W is based on LITE-ON vendor.
LITE-ON MFG part numbers for the PSU are PS-2651-3SB5 Z and PS-2651-3SA5 Z.
Signed-off-by: Madhava Reddy Siddareddygari <msiddare@cisco.com>
Signed-off-by: Venkat Garigipati <venkatg@cisco.com>
Cc: Billie Alsup <balsup@cisco.com>
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
This as a RFC, as we know 0xDF is a manufacturer specific register, and
cannot be added to the PMBus core driver. It was submitted to SONiC [1]
by Cisco engineers.
It’d be great if the maintainers good suggest how to easily implement a
custom driver for that PSU?
[1]: https://github.com/Azure/sonic-linux-kernel/pull/214
drivers/hwmon/pmbus/pmbus.c | 4 +++-
drivers/hwmon/pmbus/pmbus.h | 3 +++
drivers/hwmon/pmbus/pmbus_core.c | 38 ++++++++++++++++++++++++++++++++
3 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
index d0d386990af5..df2a782a1105 100644
--- a/drivers/hwmon/pmbus/pmbus.c
+++ b/drivers/hwmon/pmbus/pmbus.c
@@ -60,8 +60,10 @@ static void pmbus_find_sensor_groups(struct i2c_client *client,
info->func[0] |= PMBUS_HAVE_TEMP2;
if (pmbus_check_word_register(client, 0, PMBUS_READ_TEMPERATURE_3))
info->func[0] |= PMBUS_HAVE_TEMP3;
+ if (pmbus_check_word_register(client, 0, PMBUS_READ_TEMPERATURE_4))
+ info->func[0] |= PMBUS_HAVE_TEMP4;
if (info->func[0] & (PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2
- | PMBUS_HAVE_TEMP3)
+ | PMBUS_HAVE_TEMP3 | PMBUS_HAVE_TEMP4)
&& pmbus_check_byte_register(client, 0,
PMBUS_STATUS_TEMPERATURE))
info->func[0] |= PMBUS_HAVE_STATUS_TEMP;
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index e0aa8aa46d8c..1522c8c7cade 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -135,6 +135,8 @@ enum pmbus_regs {
PMBUS_MFR_MAX_TEMP_2 = 0xC1,
PMBUS_MFR_MAX_TEMP_3 = 0xC2,
+ PMBUS_READ_TEMPERATURE_4 = 0xDF,
+
/*
* Virtual registers.
* Useful to support attributes which are not supported by standard PMBus
@@ -401,6 +403,7 @@ enum pmbus_sensor_classes {
#define PMBUS_HAVE_PWM12 BIT(20)
#define PMBUS_HAVE_PWM34 BIT(21)
#define PMBUS_HAVE_SAMPLES BIT(22)
+#define PMBUS_HAVE_TEMP4 BIT(23)
#define PMBUS_PHASE_VIRTUAL BIT(30) /* Phases on this page are virtual */
#define PMBUS_PAGE_VIRTUAL BIT(31) /* Page is virtual */
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 776ee2237be2..b084b5ba6d45 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -1810,6 +1810,32 @@ static const struct pmbus_limit_attr temp_limit_attrs3[] = {
},
};
+static const struct pmbus_limit_attr temp_limit_attrs4[] = {
+ {
+ .reg = PMBUS_UT_WARN_LIMIT,
+ .low = true,
+ .attr = "min",
+ .alarm = "min_alarm",
+ .sbit = PB_TEMP_UT_WARNING,
+ }, {
+ .reg = PMBUS_UT_FAULT_LIMIT,
+ .low = true,
+ .attr = "lcrit",
+ .alarm = "lcrit_alarm",
+ .sbit = PB_TEMP_UT_FAULT,
+ }, {
+ .reg = PMBUS_OT_WARN_LIMIT,
+ .attr = "max",
+ .alarm = "max_alarm",
+ .sbit = PB_TEMP_OT_WARNING,
+ }, {
+ .reg = PMBUS_OT_FAULT_LIMIT,
+ .attr = "crit",
+ .alarm = "crit_alarm",
+ .sbit = PB_TEMP_OT_FAULT,
+ }
+};
+
static const struct pmbus_sensor_attr temp_attributes[] = {
{
.reg = PMBUS_READ_TEMPERATURE_1,
@@ -1847,6 +1873,18 @@ static const struct pmbus_sensor_attr temp_attributes[] = {
.gbit = PB_STATUS_TEMPERATURE,
.limit = temp_limit_attrs3,
.nlimit = ARRAY_SIZE(temp_limit_attrs3),
+ }, {
+ .reg = PMBUS_READ_TEMPERATURE_4,
+ .class = PSC_TEMPERATURE,
+ .paged = true,
+ .update = true,
+ .compare = true,
+ .func = PMBUS_HAVE_TEMP4,
+ .sfunc = PMBUS_HAVE_STATUS_TEMP,
+ .sbase = PB_STATUS_TEMP_BASE,
+ .gbit = PB_STATUS_TEMPERATURE,
+ .limit = temp_limit_attrs4,
+ .nlimit = ARRAY_SIZE(temp_limit_attrs4),
}
};
--
2.32.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC][PATCH] hwmon: (pmbus) Support 4th PSU temperature sensor
2021-07-28 9:38 [RFC][PATCH] hwmon: (pmbus) Support 4th PSU temperature sensor Paul Menzel
@ 2021-07-28 9:40 ` Paul Menzel
2021-07-28 13:05 ` Guenter Roeck
1 sibling, 0 replies; 3+ messages in thread
From: Paul Menzel @ 2021-07-28 9:40 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare
Cc: Guohan Lu, Madhava Reddy Siddareddygari, Venkat Garigipati,
Billie Alsup, linux-hwmon, linux-kernel
Dear Linux folks,
Am 28.07.21 um 11:38 schrieb Paul Menzel:
> From: Madhava Reddy Siddareddygari <msiddare@cisco.com>
>
> PSU650W has four temperature sensors, while the pmbus driver currently
> only support three temperature sensors.
>
> So, support a fourth temp sensor, i. e. PSU outlet temperature sensor,
> by copying what is done for temperature sensor 3, and use register 0xDF.
>
> PSU650W is based on LITE-ON vendor.
> LITE-ON MFG part numbers for the PSU are PS-2651-3SB5 Z and PS-2651-3SA5 Z.
>
> Signed-off-by: Madhava Reddy Siddareddygari <msiddare@cisco.com>
> Signed-off-by: Venkat Garigipati <venkatg@cisco.com>
> Cc: Billie Alsup <balsup@cisco.com>
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
> ---
> This as a RFC, as we know 0xDF is a manufacturer specific register, and
> cannot be added to the PMBus core driver. It was submitted to SONiC [1]
> by Cisco engineers.
>
> It’d be great if the maintainers [could] suggest how to easily implement a
I meant *could* instead of *good*.
> custom driver for that PSU?
Kind regards,
Paul
> [1]: https://github.com/Azure/sonic-linux-kernel/pull/214
>
> drivers/hwmon/pmbus/pmbus.c | 4 +++-
> drivers/hwmon/pmbus/pmbus.h | 3 +++
> drivers/hwmon/pmbus/pmbus_core.c | 38 ++++++++++++++++++++++++++++++++
> 3 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
> index d0d386990af5..df2a782a1105 100644
> --- a/drivers/hwmon/pmbus/pmbus.c
> +++ b/drivers/hwmon/pmbus/pmbus.c
> @@ -60,8 +60,10 @@ static void pmbus_find_sensor_groups(struct i2c_client *client,
> info->func[0] |= PMBUS_HAVE_TEMP2;
> if (pmbus_check_word_register(client, 0, PMBUS_READ_TEMPERATURE_3))
> info->func[0] |= PMBUS_HAVE_TEMP3;
> + if (pmbus_check_word_register(client, 0, PMBUS_READ_TEMPERATURE_4))
> + info->func[0] |= PMBUS_HAVE_TEMP4;
> if (info->func[0] & (PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2
> - | PMBUS_HAVE_TEMP3)
> + | PMBUS_HAVE_TEMP3 | PMBUS_HAVE_TEMP4)
> && pmbus_check_byte_register(client, 0,
> PMBUS_STATUS_TEMPERATURE))
> info->func[0] |= PMBUS_HAVE_STATUS_TEMP;
> diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> index e0aa8aa46d8c..1522c8c7cade 100644
> --- a/drivers/hwmon/pmbus/pmbus.h
> +++ b/drivers/hwmon/pmbus/pmbus.h
> @@ -135,6 +135,8 @@ enum pmbus_regs {
> PMBUS_MFR_MAX_TEMP_2 = 0xC1,
> PMBUS_MFR_MAX_TEMP_3 = 0xC2,
>
> + PMBUS_READ_TEMPERATURE_4 = 0xDF,
> +
> /*
> * Virtual registers.
> * Useful to support attributes which are not supported by standard PMBus
> @@ -401,6 +403,7 @@ enum pmbus_sensor_classes {
> #define PMBUS_HAVE_PWM12 BIT(20)
> #define PMBUS_HAVE_PWM34 BIT(21)
> #define PMBUS_HAVE_SAMPLES BIT(22)
> +#define PMBUS_HAVE_TEMP4 BIT(23)
>
> #define PMBUS_PHASE_VIRTUAL BIT(30) /* Phases on this page are virtual */
> #define PMBUS_PAGE_VIRTUAL BIT(31) /* Page is virtual */
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 776ee2237be2..b084b5ba6d45 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -1810,6 +1810,32 @@ static const struct pmbus_limit_attr temp_limit_attrs3[] = {
> },
> };
>
> +static const struct pmbus_limit_attr temp_limit_attrs4[] = {
> + {
> + .reg = PMBUS_UT_WARN_LIMIT,
> + .low = true,
> + .attr = "min",
> + .alarm = "min_alarm",
> + .sbit = PB_TEMP_UT_WARNING,
> + }, {
> + .reg = PMBUS_UT_FAULT_LIMIT,
> + .low = true,
> + .attr = "lcrit",
> + .alarm = "lcrit_alarm",
> + .sbit = PB_TEMP_UT_FAULT,
> + }, {
> + .reg = PMBUS_OT_WARN_LIMIT,
> + .attr = "max",
> + .alarm = "max_alarm",
> + .sbit = PB_TEMP_OT_WARNING,
> + }, {
> + .reg = PMBUS_OT_FAULT_LIMIT,
> + .attr = "crit",
> + .alarm = "crit_alarm",
> + .sbit = PB_TEMP_OT_FAULT,
> + }
> +};
> +
> static const struct pmbus_sensor_attr temp_attributes[] = {
> {
> .reg = PMBUS_READ_TEMPERATURE_1,
> @@ -1847,6 +1873,18 @@ static const struct pmbus_sensor_attr temp_attributes[] = {
> .gbit = PB_STATUS_TEMPERATURE,
> .limit = temp_limit_attrs3,
> .nlimit = ARRAY_SIZE(temp_limit_attrs3),
> + }, {
> + .reg = PMBUS_READ_TEMPERATURE_4,
> + .class = PSC_TEMPERATURE,
> + .paged = true,
> + .update = true,
> + .compare = true,
> + .func = PMBUS_HAVE_TEMP4,
> + .sfunc = PMBUS_HAVE_STATUS_TEMP,
> + .sbase = PB_STATUS_TEMP_BASE,
> + .gbit = PB_STATUS_TEMPERATURE,
> + .limit = temp_limit_attrs4,
> + .nlimit = ARRAY_SIZE(temp_limit_attrs4),
> }
> };
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC][PATCH] hwmon: (pmbus) Support 4th PSU temperature sensor
2021-07-28 9:38 [RFC][PATCH] hwmon: (pmbus) Support 4th PSU temperature sensor Paul Menzel
2021-07-28 9:40 ` Paul Menzel
@ 2021-07-28 13:05 ` Guenter Roeck
1 sibling, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2021-07-28 13:05 UTC (permalink / raw)
To: Paul Menzel, Jean Delvare
Cc: Guohan Lu, Madhava Reddy Siddareddygari, Venkat Garigipati,
Billie Alsup, linux-hwmon, linux-kernel
On 7/28/21 2:38 AM, Paul Menzel wrote:
> From: Madhava Reddy Siddareddygari <msiddare@cisco.com>
>
> PSU650W has four temperature sensors, while the pmbus driver currently
> only support three temperature sensors.
>
> So, support a fourth temp sensor, i. e. PSU outlet temperature sensor,
> by copying what is done for temperature sensor 3, and use register 0xDF.
>
> PSU650W is based on LITE-ON vendor.
> LITE-ON MFG part numbers for the PSU are PS-2651-3SB5 Z and PS-2651-3SA5 Z.
>
> Signed-off-by: Madhava Reddy Siddareddygari <msiddare@cisco.com>
> Signed-off-by: Venkat Garigipati <venkatg@cisco.com>
> Cc: Billie Alsup <balsup@cisco.com>
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
> ---
> This as a RFC, as we know 0xDF is a manufacturer specific register, and
> cannot be added to the PMBus core driver. It was submitted to SONiC [1]
> by Cisco engineers.
>
> It’d be great if the maintainers good suggest how to easily implement a
> custom driver for that PSU?
>
Implement a chip driver with a set of read/write functions.
Claim it has two pages, with the second page only implementing a single
temperature sensor. In the read/write functions, handle page 0 normally,
and access the second sensor whenever page 1 is accessed.
> [1]: https://github.com/Azure/sonic-linux-kernel/pull/214
The comments there are a bit concerning.
"* Virtual registers.
* Useful to support attributes which are not supported by standard PMBus
* registers but exist as manufacturer specific registers on individual chips.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
* Must be mapped to real registers in device specific code.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"
Then,
"... is Manufacturer specific command. These are not Virtual registers."
Isn't that exactly what the "Virtual registers" description states ?
How can that be explained better ?
Either case, I see no need to define a virtual register for a 4th temperature
sensor. This can easily be implemented with the mechanism described above.
Guenter
>
> drivers/hwmon/pmbus/pmbus.c | 4 +++-
> drivers/hwmon/pmbus/pmbus.h | 3 +++
> drivers/hwmon/pmbus/pmbus_core.c | 38 ++++++++++++++++++++++++++++++++
> 3 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
> index d0d386990af5..df2a782a1105 100644
> --- a/drivers/hwmon/pmbus/pmbus.c
> +++ b/drivers/hwmon/pmbus/pmbus.c
> @@ -60,8 +60,10 @@ static void pmbus_find_sensor_groups(struct i2c_client *client,
> info->func[0] |= PMBUS_HAVE_TEMP2;
> if (pmbus_check_word_register(client, 0, PMBUS_READ_TEMPERATURE_3))
> info->func[0] |= PMBUS_HAVE_TEMP3;
> + if (pmbus_check_word_register(client, 0, PMBUS_READ_TEMPERATURE_4))
> + info->func[0] |= PMBUS_HAVE_TEMP4;
> if (info->func[0] & (PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2
> - | PMBUS_HAVE_TEMP3)
> + | PMBUS_HAVE_TEMP3 | PMBUS_HAVE_TEMP4)
> && pmbus_check_byte_register(client, 0,
> PMBUS_STATUS_TEMPERATURE))
> info->func[0] |= PMBUS_HAVE_STATUS_TEMP;
> diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> index e0aa8aa46d8c..1522c8c7cade 100644
> --- a/drivers/hwmon/pmbus/pmbus.h
> +++ b/drivers/hwmon/pmbus/pmbus.h
> @@ -135,6 +135,8 @@ enum pmbus_regs {
> PMBUS_MFR_MAX_TEMP_2 = 0xC1,
> PMBUS_MFR_MAX_TEMP_3 = 0xC2,
>
> + PMBUS_READ_TEMPERATURE_4 = 0xDF,
> +
> /*
> * Virtual registers.
> * Useful to support attributes which are not supported by standard PMBus
> @@ -401,6 +403,7 @@ enum pmbus_sensor_classes {
> #define PMBUS_HAVE_PWM12 BIT(20)
> #define PMBUS_HAVE_PWM34 BIT(21)
> #define PMBUS_HAVE_SAMPLES BIT(22)
> +#define PMBUS_HAVE_TEMP4 BIT(23)
>
> #define PMBUS_PHASE_VIRTUAL BIT(30) /* Phases on this page are virtual */
> #define PMBUS_PAGE_VIRTUAL BIT(31) /* Page is virtual */
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 776ee2237be2..b084b5ba6d45 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -1810,6 +1810,32 @@ static const struct pmbus_limit_attr temp_limit_attrs3[] = {
> },
> };
>
> +static const struct pmbus_limit_attr temp_limit_attrs4[] = {
> + {
> + .reg = PMBUS_UT_WARN_LIMIT,
> + .low = true,
> + .attr = "min",
> + .alarm = "min_alarm",
> + .sbit = PB_TEMP_UT_WARNING,
> + }, {
> + .reg = PMBUS_UT_FAULT_LIMIT,
> + .low = true,
> + .attr = "lcrit",
> + .alarm = "lcrit_alarm",
> + .sbit = PB_TEMP_UT_FAULT,
> + }, {
> + .reg = PMBUS_OT_WARN_LIMIT,
> + .attr = "max",
> + .alarm = "max_alarm",
> + .sbit = PB_TEMP_OT_WARNING,
> + }, {
> + .reg = PMBUS_OT_FAULT_LIMIT,
> + .attr = "crit",
> + .alarm = "crit_alarm",
> + .sbit = PB_TEMP_OT_FAULT,
> + }
> +};
> +
> static const struct pmbus_sensor_attr temp_attributes[] = {
> {
> .reg = PMBUS_READ_TEMPERATURE_1,
> @@ -1847,6 +1873,18 @@ static const struct pmbus_sensor_attr temp_attributes[] = {
> .gbit = PB_STATUS_TEMPERATURE,
> .limit = temp_limit_attrs3,
> .nlimit = ARRAY_SIZE(temp_limit_attrs3),
> + }, {
> + .reg = PMBUS_READ_TEMPERATURE_4,
> + .class = PSC_TEMPERATURE,
> + .paged = true,
> + .update = true,
> + .compare = true,
> + .func = PMBUS_HAVE_TEMP4,
> + .sfunc = PMBUS_HAVE_STATUS_TEMP,
> + .sbase = PB_STATUS_TEMP_BASE,
> + .gbit = PB_STATUS_TEMPERATURE,
> + .limit = temp_limit_attrs4,
> + .nlimit = ARRAY_SIZE(temp_limit_attrs4),
> }
> };
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-07-28 13:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-28 9:38 [RFC][PATCH] hwmon: (pmbus) Support 4th PSU temperature sensor Paul Menzel
2021-07-28 9:40 ` Paul Menzel
2021-07-28 13:05 ` Guenter Roeck
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).