[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
<SA1PR12MB8947279143D868E39D30F616817CA@SA1PR12MB8947.namprd12.prod.outlook.com>
Date: Fri, 20 Jun 2025 11:03:02 +0000
From: "Datta, Shubhrajyoti" <shubhrajyoti.datta@....com>
To: "Ghannam, Yazen" <Yazen.Ghannam@....com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>, "git (AMD-Xilinx)"
<git@....com>, Krzysztof Kozlowski <krzk@...nel.org>, Rob Herring
<robh@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Borislav Petkov
<bp@...en8.de>, Tony Luck <tony.luck@...el.com>, James Morse
<james.morse@....com>, Mauro Carvalho Chehab <mchehab@...nel.org>, Robert
Richter <rric@...nel.org>, "Gupta, Nipun" <Nipun.Gupta@....com>, "Agarwal,
Nikhil" <nikhil.agarwal@....com>
Subject: RE: [PATCH v7 2/5] cdx: Export Symbols for MCDI RPC and
Initialization
[AMD Official Use Only - AMD Internal Distribution Only]
> -----Original Message-----
> From: Datta, Shubhrajyoti
> Sent: Monday, June 16, 2025 5:51 PM
> To: Ghannam, Yazen <Yazen.Ghannam@....com>
> Cc: linux-kernel@...r.kernel.org; devicetree@...r.kernel.org; linux-
> edac@...r.kernel.org; git (AMD-Xilinx) <git@....com>; Krzysztof Kozlowski
> <krzk@...nel.org>; Rob Herring <robh@...nel.org>; Conor Dooley
> <conor+dt@...nel.org>; Borislav Petkov <bp@...en8.de>; Tony Luck
> <tony.luck@...el.com>; James Morse <james.morse@....com>; Mauro
> Carvalho Chehab <mchehab@...nel.org>; Robert Richter <rric@...nel.org>;
> Gupta, Nipun <Nipun.Gupta@....com>; Agarwal, Nikhil
> <nikhil.agarwal@....com>
> Subject: RE: [PATCH v7 2/5] cdx: Export Symbols for MCDI RPC and Initialization
>
>
>
> > -----Original Message-----
> > From: Ghannam, Yazen <Yazen.Ghannam@....com>
> > Sent: Saturday, June 14, 2025 1:41 AM
> > To: Datta, Shubhrajyoti <shubhrajyoti.datta@....com>
> > Cc: linux-kernel@...r.kernel.org; devicetree@...r.kernel.org; linux-
> > edac@...r.kernel.org; git (AMD-Xilinx) <git@....com>; Krzysztof
> > Kozlowski <krzk@...nel.org>; Rob Herring <robh@...nel.org>; Conor
> > Dooley <conor+dt@...nel.org>; Borislav Petkov <bp@...en8.de>; Tony
> > Luck <tony.luck@...el.com>; James Morse <james.morse@....com>; Mauro
> > Carvalho Chehab <mchehab@...nel.org>; Robert Richter
> > <rric@...nel.org>; Gupta, Nipun <Nipun.Gupta@....com>; Agarwal, Nikhil
> > <nikhil.agarwal@....com>
> > Subject: Re: [PATCH v7 2/5] cdx: Export Symbols for MCDI RPC and
> > Initialization
> >
> > On Thu, May 29, 2025 at 12:30:14PM +0530, Shubhrajyoti Datta wrote:
> > > The cdx_mcdi_init, cdx_mcdi_process_cmd, and cdx_mcdi_rpc functions
> > > are needed by VersalNET EDAC modules that interact with the MCDI
> > > (Management Controller Direct Interface) framework. These functions
> > > facilitate communication between different hardware components by
> > > enabling command execution and status management.
> > >
> > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@....com>
> > > ---
> > >
> > > Changes in v7:
> > > - Add the kernel doc description
> > > - Add the prototype from first patch to here
> > >
> > > Changes in v6:
> > > - Update commit description
> > >
> > > Changes in v2:
> > > - Export the symbols for module compilation
> > >
> > > drivers/cdx/controller/mcdi.c | 29 +++++++++++++++++++++++++++++
> > > include/linux/cdx/mcdi.h | 6 ++++++
> >
> > You've added the function prototypes to this new global header.
> >
> > But you didn't remove them from the local header.
> > drivers/cdx/controller/mcdi.h
> >
> > Also, you haven't included the new global header in the cdx/controller code.
> >
> > Even though this does compile, it doesn't seem proper.
> >
> > I expect you would want to do the following:
> >
> > 1) Add the common code to the global header.
> > 2) Remove the common code from the local header.
> > 3) Include the global header everywhere the common code is needed.
> >
> > Keeping the diff below for reference.
>
I plan to have the current (in the cdx) mcdi.h to mcdid.h and move the common parts to
Include/linux/cdx/mcdi.h
And for the bitfield.h will move to include/linux/cdx/bitfield.h as most of it is needed
by others
I plan to rename the existing mcdi.h under cdx/controller/ to mcdid.h.
The common prototypes and definitions will be moved to the new global header at
include/linux/cdx/mcdi.h.
I am using most of (close to 90/95 percent) the bitfield.h, I'll move it to include/linux/cdx/bitfield.h to
make it accessible for other subsystems.
Let me know if that sounds good.
>
> >
> > Thanks,
> > Yazen
> >
> > > 2 files changed, 35 insertions(+)
> > >
> > > diff --git a/drivers/cdx/controller/mcdi.c
> > > b/drivers/cdx/controller/mcdi.c index e760f8d347cc..f3cca4c884ff
> > > 100644
> > > --- a/drivers/cdx/controller/mcdi.c
> > > +++ b/drivers/cdx/controller/mcdi.c
> > > @@ -99,6 +99,19 @@ static unsigned long cdx_mcdi_rpc_timeout(struct
> > cdx_mcdi *cdx, unsigned int cmd
> > > return cdx->mcdi_ops->mcdi_rpc_timeout(cdx, cmd); }
> > >
> > > +/**
> > > + * cdx_mcdi_init - Initialize MCDI (Management Controller Driver
> > > +Interface) state
> > > + * @cdx: NIC through which to issue the command
> > > + *
> > > + * This function allocates and initializes internal MCDI structures
> > > +and resources
> > > + * for the CDX device, including the workqueue, locking primitives,
> > > +and command
> > > + * tracking mechanisms. It sets the initial operating mode and
> > > +prepares the device
> > > + * for MCDI operations.
> > > + *
> > > + * Return:
> > > + * * 0 - on success
> > > + * * -ENOMEM - if memory allocation or workqueue creation fails
> > > +*/
> > > int cdx_mcdi_init(struct cdx_mcdi *cdx) {
> > > struct cdx_mcdi_iface *mcdi;
> > > @@ -128,6 +141,7 @@ int cdx_mcdi_init(struct cdx_mcdi *cdx)
> > > fail:
> > > return rc;
> > > }
> > > +EXPORT_SYMBOL_GPL(cdx_mcdi_init);
> > >
> > > void cdx_mcdi_finish(struct cdx_mcdi *cdx) { @@ -553,6 +567,19 @@
> > > static void cdx_mcdi_start_or_queue(struct cdx_mcdi_iface *mcdi,
> > > cdx_mcdi_cmd_start_or_queue(mcdi, cmd); }
> > >
> > > +/**
> > > + * cdx_mcdi_process_cmd - Process an incoming MCDI response
> > > + * @cdx: NIC through which to issue the command
> > > + * @outbuf: Pointer to the response buffer received from the
> > > +management
> > controller
> > > + * @len: Length of the response buffer in bytes
> > > + *
> > > + * This function handles a response from the management controller.
> > > +It locates the
> > > + * corresponding command using the sequence number embedded in the
> > > +header,
> > > + * completes the command if it is still pending, and initiates any
> > > +necessary
> > cleanup.
> > > + *
> > > + * The function assumes that the response buffer is well-formed and
> > > +at least one
> > > + * dword in size.
> > > + */
> > > void cdx_mcdi_process_cmd(struct cdx_mcdi *cdx, struct cdx_dword
> > > *outbuf, int len) {
> > > struct cdx_mcdi_iface *mcdi;
> > > @@ -590,6 +617,7 @@ void cdx_mcdi_process_cmd(struct cdx_mcdi *cdx,
> > > struct cdx_dword *outbuf, int le
> > >
> > > cdx_mcdi_process_cleanup_list(mcdi->cdx, &cleanup_list); }
> > > +EXPORT_SYMBOL_GPL(cdx_mcdi_process_cmd);
> > >
> > > static void cdx_mcdi_cmd_work(struct work_struct *context) { @@
> > > -757,6 +785,7 @@ int cdx_mcdi_rpc(struct cdx_mcdi *cdx, unsigned int cmd,
> > > return cdx_mcdi_rpc_sync(cdx, cmd, inbuf, inlen, outbuf, outlen,
> > > outlen_actual, false);
> > > }
> > > +EXPORT_SYMBOL_GPL(cdx_mcdi_rpc);
> > >
> > > /**
> > > * cdx_mcdi_rpc_async - Schedule an MCDI command to run
> > > asynchronously diff --git a/include/linux/cdx/mcdi.h
> > > b/include/linux/cdx/mcdi.h index 46e3f63b062a..1344119e9a2c 100644
> > > --- a/include/linux/cdx/mcdi.h
> > > +++ b/include/linux/cdx/mcdi.h
> > > @@ -169,6 +169,12 @@ struct cdx_mcdi_data {
> > > u32 fn_flags;
> > > };
> > >
> > > +int cdx_mcdi_init(struct cdx_mcdi *cdx); void
> > > +cdx_mcdi_process_cmd(struct cdx_mcdi *cdx, struct cdx_dword
> > > +*outbuf, int len); int cdx_mcdi_rpc(struct cdx_mcdi *cdx, unsigned int cmd,
> > > + const struct cdx_dword *inbuf, size_t inlen,
> > > + struct cdx_dword *outbuf, size_t outlen, size_t *outlen_actual);
> > > +
> > > /*
> > > * We expect that 16- and 32-bit fields in MCDI requests and responses
> > > * are appropriately aligned, but 64-bit fields are only
> > > --
> > > 2.34.1
> > >
Powered by blists - more mailing lists