Re: [PATCH 1/2] PCI: endpoint: Introduce 'get_bar' to map fixed address BARs in EPC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 26, 2024 at 12:53 AM Damien Le Moal <dlemoal@xxxxxxxxxx> wrote:
>
> On 7/26/24 06:52, Niklas Cassel wrote:
> > On Thu, Jul 25, 2024 at 10:06:52PM +0530, Manivannan Sadhasivam wrote:
> >>
> >> I vary with you here. IMO EPF drivers have no business in knowing the BAR
> >> location as they are independent of controller (mostly except drivers like MHI).
> >> So an EPF driver should call a single API that just allocates/configures the
> >> BAR. For fixed address BAR, EPC core should be able to figure it out using the
> >> EPC features.
> >>
> >> For naming, we have 3 proposals as of now:
> >>
> >> 1. pci_epf_setup_bar() - This looks good, but somewhat collides with the
> >> existing pci_epc_set_bar() API.
> >>
> >> 2. pci_epc_alloc_set_bar() - Looks ugly, but aligns with the existing APIs.
> >>
> >> 3. pci_epc_get_bar() - Also looks good, but the usage of 'get' gives the
> >> impression that the BAR is fetched from somewhere, which is true for fixed
> >> address BAR, but not for dynamic BAR.
> >
> > pci_epc_configure_bar() ?
> > we could name the 'struct pci_epf_bar *' param 'conf'
>
> +1
>
> But let's spell this out: pci_epc_configure_bar(), to be sure to avoid any
> possible confusion (config could mean configure or configuration...).
>
> --
> Damien Le Moal
> Western Digital Research
>

+1

One thing to keep in mind is that 'struct pci_epf_bar 'conf' would be
an 'inout' parameter, where 'conf' gets changed in case of a fixed
address BAR or fixed 64-bit etc. This means the EPF code needs to
check 'conf' after the call. Also, if the caller sets flags and the
controller only handles different flags, do we return an error, or
configure the BAR with the only possible flags and let the caller
check if those flags are ok for the endpoint function ?

This is a bit unclear for me for the moment.

One solution could be to pass two 'pci_epf_bar' structs, one is to be
filed by the function call, and the other an optional parameter
'desired_conf' (i.e. can be 'NULL') if NULL it would mean let the
controller decide, if not NULL 'desired_conf' describes the EPF
requirements (e.g., chosen BAR address or certain flags, size, etc.)
and 'pci_epc_configure_bar()' would fail if it cannot satisfy the
'desired_conf' requirements.

But still 'desired_conf' is a structure with several things that could
be independently desired, e.g., a given address or BAR size, flags,
but not all of them might be a necessity.

Let me know what you think.

Have a nice week-end.

Rick





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux