Re: [PATCH v3 3/4] PCI: endpoint: Add API for DOE initialization and setup in EPC core

[Aksh Garg]

On 15/05/26 18:17, Manivannan Sadhasivam wrote:
> On Fri, May 15, 2026 at 10:21:52AM +0530, Aksh Garg wrote:
> > On 14/05/26 13:38, Manivannan Sadhasivam wrote:
> > > On Mon, Apr 27, 2026 at 10:47:24AM +0530, Aksh Garg wrote:
> > > > Add pci_epc_setup_doe() API in EPC core driver to initialize and setup
> > > > the DOE framework for an endpoint controller. The API discovers the DOE
> > > > capabilities (extended capability ID 0x2E), and registers each discovered
> > > > DOE mailbox for all the functions in the endpoint controller. This API
> > > > should be invoked by the controller driver during probe based on the
> > > > doe_capable feature.
> > > >
> > > > Add pci_epc_destroy_doe() API in EPC core driver for cleanup of DOE
> > > > resources, which should be invoked by the controller driver during
> > > > controller cleanup based on the doe_capable feature.
> > > >
> > > > Co-developed-by: Siddharth Vadapalli <s-vadapalli@xxxxxx>
> > > > Signed-off-by: Siddharth Vadapalli <s-vadapalli@xxxxxx>
> > > > Signed-off-by: Aksh Garg <a-garg7@xxxxxx>
> > > > ---
> > > >
> > > > Changes from v2 to v3:
> > > > - Rebased on 7.1-rc1.
> > > >
> > > > Changes since v1:
> > > > - New patch added to v2 (not present in v1)
> > > >
> > > > v2: https://lore.kernel.org/all/20260401073022.215805-4-a-garg7@xxxxxx/
> > > >
> > > > This patch is introduced based on the feedback provided by Manivannan
> > > > Sadhasivam at [1].
> > >
> > > Sweet! But I was expecting you to add atleast one EPC driver implementation to
> > > make use of these APIs.
> > >
> > > Also, why can't you call these APIs from the EPC core directly? Maybe during
> > > pci_epc_init_notify() once the register accesses become valid.
> >
> > Can we add the DOE initialization API to pci_epc_init_notify()? This
> > API seems to be called to notify the EPF drivers that the EPC device's
> > initialization has been completed, as the name and description suggests.
>
> That's correct. But there is no harm in calling something like
> pci_epc_init_capabilities() inside its definition. Only concern would be that
> pci_epc_init_notify() is mostly called from threaded IRQ handlers. So loading
> the handler would not be recommended. But since it is threaded anyway and we
> don't have a better place to call, it would be OK.
>
> We could've called this from pci_epc_{create/start}, but some controllers won't
> allow accessing CSRs without REFCLK. So only after pci_init_notify(), CSRs can
> be accessed.
>
> > As 'pci_epc_doe_setup' is a part of EPC initialization, I thought the
> > EPC drivers should call this API before calling the pci_epc_init_notify().
> >
> > However, I agree with your suggestion to call the DOE setup API directly
> > from the EPC core instead of sprinkling over the EPC drivers. I would
> > recommend renaming the pci_epc_init_notify() API (and hence the
> > pci_epc_deinit_notify() as well) to something like pci_epc_init_complete(),
> > and add the DOE setup API/logic just before the
> > logic of notifying the EPF devices.
>
> No need to rename this API. Just use as is:
>
> 	pci_epc_init_notify()
> 		-> pci_epc_init_capabilities()
> 			-> pci_epc_init_doe()
> 		-> epf->event_ops->epc_init()

Thank you for the suggestion, I will incorporate these changes in v4 series.

Regards,
Aksh Garg

> - Mani