Received-SPF: pass (google.com: domain of linux-kernel+bounces-179764-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Wed, 15 May 2024 11:32:17 +0100
From: Lee Jones <lee@kernel.org>
To: Guenter Roeck <linux@roeck-us.net>
Cc: "Chatradhi, Naveen Krishna" <naveenkrishna.chatradhi@amd.com>,
	Naveen Krishna Chatradhi <nchatrad@amd.com>,
	linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org,
	Akshay Gupta <Akshay.Gupta@amd.com>, arnd@arndb.de,
	gregkh@linuxfoundation.org
Subject: Re: [PATCH 2/2] sbrmi: Add support for APML protocols
Message-ID: <20240515103217.GA6035@google.com>
References: <20240502220554.2768611-1-nchatrad@amd.com>
 <20240502220554.2768611-2-nchatrad@amd.com>
 <dd4ef0b6-8272-40b6-9a50-edbeec14d5e4@roeck-us.net>
 <8cf0d519-ae6c-4ed0-899e-899f67479052@amd.com>
 <488e8eed-e0f0-4055-b43c-5422b6302632@roeck-us.net>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <488e8eed-e0f0-4055-b43c-5422b6302632@roeck-us.net>

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@amd.com>
> > > > 
> > > > 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 [李琼斯]