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

[Lee Jones <lee@kernel.org>]

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@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

MFD knows nothing of these characteristics.

> 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.

> 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?

> 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

drivers/misc?

> 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 [李琼斯]

-- 
Lee Jones [李琼斯]