On Tue, 14 May 2024, Guenter Roeck wrote: > On 5/14/24 12:15, Chatradhi, Naveen Krishna wrote: > > + @Misc and @MFD maintainers in CC > > > > Hi > > > > On 5/3/2024 3:56 AM, Guenter Roeck wrote: > > > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > > > > > > > On 5/2/24 15:05, Naveen Krishna Chatradhi wrote: > > > > From: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@xxxxxxx> > > > > > > > > The present sbrmi module only support reporting power. However, AMD data > > > > center processors support various system management functionality > > > > Out-of-band over Advanced Platform Management Link APML. > > > > > > > > Register a miscdevice, which creates a device /dev/sbrmiX with an IOCTL > > > > interface for the user space to invoke the following protocols. > > > > - Mailbox read/write (already defined in sbrmi_mailbox_xfer()) > > > > - CPUID read > > > > - MCAMSR read > > > > > > > > > > This is not hardware monitoring functionality and would have to reside > > > elsewhere, outside the hwmon subsystem. > > > > I thought as much, please provide your opinion on the following approach. > > > > Background: 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. > > > > As adding the core functionality of the APML interface in drivers/hwmon/sbrmi is not the correct approach. > > > > We would like do the following > > > > 1. Move the i2c client probe, misc device registration and rmi_mailbox_xfer() function to a drivers/misc. > > > > 2. Add an MFD device with a cell, which probes the hwmon/sbrmi and continues to report power using the symbols exported by the misc/sbrmi. > > > > afaik mfd API function must not be used outside drivers/mfd. The mfd maintainer > is (or at least used to be) pretty strict on that. The core code of a > multi-function device might better be implemented in drivers/mfd, with > drivers in drivers/misc (for the misc device) and drivers/hwmon/ (for > hwmon functionality). The misc and hwmon drivers could then communicate > with the core using regmap. Yes, please only use the MFD API from drivers/mfd. There are 'offenders' that slipped by me, but in general if you need to create an MFD then it should be located in the MFD subsystem. > drivers/mailbox/ supports mailbox style communication. I don't know if that > would apply to the mailbox functionality implemented here. -- Lee Jones [李琼斯]