Add support for additional PSU sensor in pmbus driver.#214
Add support for additional PSU sensor in pmbus driver.#214VenkatCisco wants to merge 2 commits intosonic-net:masterfrom
Conversation
PSU650W has 4 temperature sensors and pmbus driver has support for 3 senosrs. Added support for 4th temp sensor (i.e. PSU Outlet temperature sensor) Signed-off-by: Venkat Garigipati <venkatg@cisco.com>
paulmenzel
left a comment
paulmenzel
left a comment
There was a problem hiding this comment.
This looks like a generic Linux kernel improvement, so needs to be sent upstream, so SONiC won’t need to maintain this patch for a long time. The subsystem is PMBUS HARDWARE MONITORING DRIVERS.
| From cfb4b5420339995a76f765e72b1c916029f264ae Mon Sep 17 00:00:00 2001 | ||
| From: Madhava Reddy Siddareddygari <msiddare@cisco.com> | ||
| Date: Mon, 24 May 2021 16:02:21 -0700 | ||
| Subject: [PATCH] Add supporting for additional PSU sensor in pmbus driver |
There was a problem hiding this comment.
Nit:
hwmon: (pmbus) Support fourth PSU temperature sensor
| Date: Mon, 24 May 2021 16:02:21 -0700 | ||
| Subject: [PATCH] Add supporting for additional PSU sensor in pmbus driver | ||
|
|
||
| PSU650W has 4 temperature sensors and pmbus driver has support for 3 senosrs. Added support for 4th temp sensor (i.e. PSU Outlet temperature sensor) |
| Date: Mon, 24 May 2021 16:02:21 -0700 | ||
| Subject: [PATCH] Add supporting for additional PSU sensor in pmbus driver | ||
|
|
||
| PSU650W has 4 temperature sensors and pmbus driver has support for 3 senosrs. Added support for 4th temp sensor (i.e. PSU Outlet temperature sensor) |
There was a problem hiding this comment.
PSU650W sounds pretty generic. Can you please list the full model name?
There was a problem hiding this comment.
addressed in the new PR updates. Please review.
PSU650W has 4 temperature sensors and pmbus driver has support for 3 sensors. Added support for 4th temp sensor. (i.e. PSU Outlet temperature sensor) 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: Venkat Garigipati <venkatg@cisco.com>
VenkatCisco
left a comment
VenkatCisco
left a comment
There was a problem hiding this comment.
Updated PR based on feedback. Please review the changes.
|
why is this pr still in draft mode? |
|
Hi @jleveque - can you please review, thanks. |
| PMBUS_MFR_DATE = 0x9D, | ||
| PMBUS_MFR_SERIAL = 0x9E, | ||
|
|
||
| + PMBUS_READ_TEMPERATURE_4 = 0xDF, |
There was a problem hiding this comment.
Reading through drivers/hwmon/pmbus/pmbus.h, is that a standard PMBus register, or should it be a “virtual register”?
* 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.
There was a problem hiding this comment.
Hi Paul,
I was reading up on pmbus commands.
Please check the following link page 103,
https://470q2hhkn9g15l4bc2btbal1-wpengine.netdna-ssl.com/wp-content/uploads/2021/05/PMBus-Specification-Rev-1-2-Part-II-20100906.pdf
According to the specifications the 0xDF is Manufacturer specific command. These are not Virtual registers.
There was a problem hiding this comment.
Thank you for looking this up. As it’s manufacturer specific, you cannot put it into the “general” core driver and define it as temperature sensor 4. Run git grep -i 0xdf drivers/hwmon/pmbus/ to see that. I guess, you need to create a dedicated driver for that PSU. There are several dedicated driver in drivers/hwmon/pmbus/ already. But I am not a PMBus developer, so am not able to help here further.
|
cisco will work on a new device driver to handle extra sensor temperature. |
### **Issue** Fix the issue sonic-net/sonic-buildimage#21290 No info log found in syslog on 202405 image for caclmgrd ### **Work item tracking** - Microsoft ADO **(number only)**: 30611546 ### Why did it happen RP sonic-net/sonic-buildimage#17171, introduced a new Class SysLogger, DaemonBase will choose SysLogger by default PR sonic-net/sonic-buildimage#19232, it added noticed level and make it to be default level which suppresses INFO logs. `caclmgr.set_min_log_priority_info()` it sets min log priority to info, this function is in Logger class, SysLogger doesn't have this function. But DaemonBase still inherits Logger which implements set_min_log_priority_info, that's why even caclmgrd called this function, it didn't throw exception. But it didn't make INFO level effect in SysLogger which is actually used in caclmgrd Even change to use Logger by setting `use_syslogger=False`, it still doesn't work. The root cause is that it added a new instance for logger, `self.logger_instance`, any instance inherited from DaemonBase class can't change the debug level, the level they changed is their own instance, not the self.logger_instance's level. ### **How to fix** The solution here for caclmgrd is to choose logger.Logger class instead of DaemonBase. ### **How to verify it** Test it on 202405
5 participants
PSU650W has 4 temperature sensors and pmbus driver has support for 3 senosrs. Added support for 4th temp sensor (i.e. PSU Outlet temperature sensor)
Signed-off-by: Venkat Garigipati venkatg@cisco.com