On Wed, Mar 13, 2024 at 11:57:57AM +0100, Niklas Cassel wrote:
> Simplify the loop in pci_epf_test_set_bar().
> If we allocated memory for the BAR, we need to call set_bar() for that
> BAR, if we did not allocated memory for that BAR, we need to skip.
> It is as simple as that. This also matches the logic in
> pci_epf_test_unbind().
>
> A 64-bit BAR will still only be one allocation, with the BAR succeeding
> the 64-bit BAR being null.
>
> While at it, remove the misleading comment.
> A EPC .set_bar() callback should never change the epf_bar->flags.
> (E.g. to set a 64-bit BAR if we requested a 32-bit BAR.)
>
> A .set_bar() callback should do what we request it to do.
> If it can't satisfy the request, it should return an error.
>
That's a valid assumption. But...
> If platform has a specific requirement, e.g. that a certain BAR has to
> be a 64-bit BAR, then it should specify that by setting the .only_64bit
> flag for that specific BAR in epc_features->bar[], such that
> pci_epf_alloc_space() will return a epf_bar with the 64-bit flag set.
> (Such that .set_bar() will receive a request to set a 64-bit BAR.)
>
Looks like pcie-cadence-ep is setting the PCI_BASE_ADDRESS_MEM_TYPE_64 flag if
the requested size is >2GB (I don't know why 2GB instead of 4GB in the first
place).
- Mani
> Signed-off-by: Niklas Cassel <cassel@xxxxxxxxxx>
> ---
> drivers/pci/endpoint/functions/pci-epf-test.c | 21 ++++---------------
> 1 file changed, 4 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 20c79610712d..91bbfcb1b3ed 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -709,31 +709,18 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
>
> static int pci_epf_test_set_bar(struct pci_epf *epf)
> {
> - int bar, add;
> - int ret;
> - struct pci_epf_bar *epf_bar;
> + int bar, ret;
> struct pci_epc *epc = epf->epc;
> struct device *dev = &epf->dev;
> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> - const struct pci_epc_features *epc_features;
> -
> - epc_features = epf_test->epc_features;
>
> - for (bar = 0; bar < PCI_STD_NUM_BARS; bar += add) {
> - epf_bar = &epf->bar[bar];
> - /*
> - * pci_epc_set_bar() sets PCI_BASE_ADDRESS_MEM_TYPE_64
> - * if the specific implementation required a 64-bit BAR,
> - * even if we only requested a 32-bit BAR.
> - */
> - add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1;
> -
> - if (epc_features->bar[bar].type == BAR_RESERVED)
> + for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> + if (!epf_test->reg[bar])
> continue;
>
> ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no,
> - epf_bar);
> + &epf->bar[bar]);
> if (ret) {
> pci_epf_free_space(epf, epf_test->reg[bar], bar,
> PRIMARY_INTERFACE);
> --
> 2.44.0
>
--
மணிவண்ணன் சதாசிவம்
