Re: [PATCH] PCI: endpoint: pci-epf-test: Allow overriding default BAR sizes

[Niklas Cassel <cassel@kernel.org>]

On Wed, Jan 28, 2026 at 05:35:15PM +0100, Niklas Cassel wrote:
> On Wed, Jan 28, 2026 at 01:47:45PM +0530, Manivannan Sadhasivam wrote:

(snip)

> But I am slightly worried of the complexity it brings...
> Because we already have an .epc_init callback in an EPF driver.
> If we also bring add a ".pre_start_link" callback in an EPF driver,
> how should an EPF driver developer know what to put in each callback?
> 
> I guess in that case we would want to remove the .epc_init callback?
> 
> (Because calling .set_bar() etc. should work to be called in a
> .pre_start_link for both those driver relying on external refclock and
> for those that don't.
> 
> If you do
> # echo 1 > controllers/a40000000.pcie-ep/start
> on a platform that requires an external reflock, before the external
> refclock is available, that is your own fault.
> 
> I guess we could detect that and give a nice error.)

pci_epf_test_alloc_space() is called by pci_epf_test_bind().

pci_epc_set_bar() is called by pci_epf_test_epc_init().

On platforms that do not require an external reference clock to do
register writes to their own config space, i.e. all drivers except
(pcie-qcom and pcie-tegra194), a single configfs link command will cause
both both of these callback to be called directly after each other:

# ln -s functions/pci_epf_test/func1 controllers/a40000000.pcie-ep/
[  578.073152] pci_epf_test_bind
[  578.075614] pci_epf_test_epc_init


So, unless we want to change the design of pci-epf-test to:
1) Move the allocation of BARs from .bind() to a new
   .pre_start_link() callback.
2) Move the call to pci_epc_set_bar() from epc_init() to a new
   .pre_start_link() callback.
3) Remove the now unused .epc_init() callback.

...and for consistency, we would also need to do these steps for all
the other existing EPF drivers...

I am not convinced that it is worth reworking the EPF callbacks for this
(test driver) feature, which will only used for PCI endpoint developers,
especially given the complexity it would require.

For platforms drivers that require external reference clock, is it possible
to call
echo 1 > start

before we get refclock?

If so the design would probably need to be changed even more drastically...



When we introduce .epc_init, one of the reasons why we decided to have
alloc_space() and set_bar() in two different callbacks, was if a host driver
toggled PERST, the EPF driver would not need to call free() on the backing
memory it has allocated, e.g. I can see that pci_epf_test_epc_deinit() calls
clear_bar(), but the only place that actually frees the memory backing the
BARs is pci_epf_test_unbind().


What if the user wants to do:

# echo 1024 > functions/pci_epf_test/func1/pci_epf_test.0/bar1_size
# echo 0 > controllers/a40000000.pcie-ep/start
# echo 2048 > functions/pci_epf_test/func1/pci_epf_test.0/bar1_size
# echo 1 > controllers/a40000000.pcie-ep/start

Do we want to make the allocation each time we echo 1 > start... ?

If someone stops the link.. is it really wrong to free the memory?

I'm not against this design, but with the complexity that we want
.epc_init() to only call set_bar(), and we probably want .pre_start_link()
to call alloc_space()...

I don't think that the small feature in this patch is enough
justification to modify how all EPF driver to their backing memory
allocations just for this...

Instead I will just send a new version such that if the user get -EINVAL
if trying to change bar*_size in configfs AFTER the EPF has been bound to
an EPC (i.e. it can only be done before binding to an EPC.), as that seems
like the most pragmatic compromise that will avoid (another) redesign of
the EPF callbacks.


Kind regards,
Niklas