Re: [PATCH 0/5] mfd: add amd side-band functionality

["Chatradhi, Naveen Krishna" <naveenkrishna.chatradhi@amd.com>]

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@amd.com>
>>
>> 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

monitoring, management using AMD defined protocols. One of them 
sbrmi_mailbox_xfer()is defined in

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

4. drivers/mfd/sbrmi-core.c will contain the common functions which can 
be used by i2c and i3c based drivers, such as

core protocol definitions, creation of misc device and an ioctl for the 
user interface.

This patchset is an attempt toadd support for these core protocols in such a way that Open-sourced and 
widely used https://github.com/amd/esmi_oob_library will continue to 
provide user-space programmable API. regards, Naveenk

>>   drivers/mfd/sbrmi-core.h      |  37 +++
>>   drivers/mfd/sbrmi-i2c.c       | 165 ++++++++++++
>>   include/linux/mfd/amd-sb.h    |  55 ++++
>>   include/uapi/linux/amd-apml.h |  74 +++++
>>   9 files changed, 871 insertions(+), 246 deletions(-)
>>   create mode 100644 drivers/mfd/sbrmi-core.c
>>   create mode 100644 drivers/mfd/sbrmi-core.h
>>   create mode 100644 drivers/mfd/sbrmi-i2c.c
>>   create mode 100644 include/linux/mfd/amd-sb.h
>>   create mode 100644 include/uapi/linux/amd-apml.h
>>
>> --
>> 2.25.1
>>
> --
> Lee Jones [李琼斯]