On 6/18/2024 5:57 PM, Lee Jones wrote:
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
On Tue, 18 Jun 2024, Chatradhi, Naveen Krishna wrote:
On 6/14/2024 8:19 PM, Lee Jones wrote:
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
On Fri, 14 Jun 2024, Chatradhi, Naveen Krishna wrote:
On 6/13/2024 10:35 PM, Lee Jones wrote:
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
On Thu, 30 May 2024, Naveen Krishna Chatradhi wrote:
From: Akshay Gupta <akshay.gupta@xxxxxxx>
At present, sbrmi under hwmon subsystem is probed as an i2c
driver and reports power.
However, APML interface defines few other protocols to support
OOB system management functionality.
This patchset the following
1. Based on hwmon maintainers feedback, move the i2c client
probe and sbrmi core functionality to drivers/mfd/
2. Add an MFD cell, which probes the hwmon/sbrmi and continues to
report power using the symbol exported by the mfd/sbrmi-core.
3. Convert i2c to regmap which provides multiple benefits
over direct smbus APIs.
4. Register a misc device which provides
a. An ioctl interface through node /dev/sbrmiX
b. Open-sourced and widely used https://github.com/amd/esmi_oob_library
will continue to provide user-space programmable API for the following
- Mailbox xfer (already defined in sbrmi_mailbox_xfer())
- CPUID access
- MCAMSR access
Akshay Gupta (5):
hwmon/mfd sbrmi: Move core sbrmi from hwmon to MFD
mfd: sbrmi: Add mfd cell to I2C probe to be used by hwmon
mfd/hwmon sbrmi: Use regmap subsystem
mfd: sbrmi: Clear sbrmi status register bit SwAlertSts
mfd/hwmon: sbrmi: Add support for APML protocols
drivers/hwmon/Kconfig | 1 +
drivers/hwmon/sbrmi.c | 284 +++-----------------
drivers/mfd/Kconfig | 9 +-
drivers/mfd/Makefile | 2 +
drivers/mfd/sbrmi-core.c | 490 ++++++++++++++++++++++++++++++++++
It's not clear to me what any of these 500 lines do, but they do not
look like a good fit for MFD. Perhaps I'm missing something. Can you
provide some more information about the device and why you think MFD is
a suitable location for it?
Hi lee,
Data center processors from AMD provide a side-band (often called
out-of-band) path for manageability
called Advanced Platform Management Link (APML), which consists of i2c/i3c
client devices called
Side-band Remote Management Interface (SB-RMI) and Side-band Temperature
Sensor Interface (SB-TSI).
We have i2c client drivers upstreamed under drivers/hwmon sbrmi.c and
sbtsi_temp.c reporting power and
temperature via hwmon interfaces. However, sbrmi device can also provide
performance, telemetry and RAS
MFD knows nothing of these characteristics.
Yes, we will modify the implementation to define ops structure with
functionality that
can be called by platforms drivers in hwmon and other subsystems.
monitoring, management using AMD defined protocols. One of them
sbrmi_mailbox_xfer()is defined in
I large portion of this should be moved out to drivers/mailbox.
we have explored the mailbox subsystem, APML xfer is not a ful-fledge
mailbox interface as such,
it is a custom protocol, which accepts inputs and provides outputs over
i2c/i3c. It does not support
multichannel (tx/rx) or have IRQs or a memory mapped IO and it is one of the
protocols supported
by the RMI device.
drivers/hwmon/sbrmi.c.
Patchset would do the following
1. Move core functionality from hwmon/sbrmi.c to drivers/mfd/sbrmi-i2c.c as
an i2c_driver.
2. Convert the hwmon/sbrmi.c to a platform driver.
3. Use mfd_add_devices() API to add cells which will probe the platform
driver under hwmon/sbrmi.c
How many devices will mfd_add_devices() be registering?
This patch is adding one hwmon device.
We can add additional cell which probes a platform driver in drivers/misc
which handles
the user space interface part.
It sounds like MFD is (once more) being used as a dumping ground for
random devices which do not fit anywhere else. A Multi-Function Device
driver's role is to create shared resources (memory, IRQs, Clocks,
Regulators, etc) for and register more than one real device that uses
those shared resources, that's all. Even if you were to move the Misc
part out, using that as a second device to prove it MFD-worthy is not
going to fly. Take a look at what these devices usually consist of:
git grep -W "struct mfd_cell.*{" -- drivers/mfd
sbrmi is one physical i2c/i3c device with a set of registers and an IRQ
which provides multiple functions via different protocols.
|------------| -> apml_xfer
| sbrmi | -> cpuid_xfer
|-----------| -> msr_mca_xfer
L-----------> IRQ
We were thinking of adding each functionality as an mfd_cell + 1
mfd_cell for hwmon interface.
I understand it now, it doesn't fit well. Can you suggest any other
sub-system that can absorb this.
I can think of moving everything to a misc driver with a misc device
node and
register a hwmon device for reporting power. please suggest.
This submission results in a 650-line driver that registers a single
cell. One that is attributed only to the device it's being removed
from.
I see that Guenter already said:
"This is not hardware monitoring functionality and would have to
reside elsewhere, outside the hwmon subsystem."
Well it's not MFD functionality either. Sorry.
--
Lee Jones [李琼斯]
