| Message ID | 20240210012634.600301-3-cassel@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | PCI endpoint BAR hardware description cleanup | expand |
On Sat, Feb 10, 2024 at 02:26:26AM +0100, Niklas Cassel wrote: > The definition of a reserved BAR is that EPF drivers should not touch > them. > > The definition of only_64bit is that the EPF driver must configure this > BAR as 64-bit. (An EPF driver is not allowed to choose if this BAR should > be configured as 32-bit or 64-bit.) > > Thus, it does not make sense to put only_64bit of a BAR that EPF drivers > are not allow to touch. > > Drop the only_64bit property from hardware descriptions that are of type > reserved BAR. > > Signed-off-by: Niklas Cassel <cassel@kernel.org> One nitpick below. With that addressed, Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/pci/controller/dwc/pci-keystone.c | 2 +- > drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 +- > drivers/pci/endpoint/pci-epc-core.c | 7 ------- > include/linux/pci-epc.h | 4 ++++ > 4 files changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c > index b2b93b4fa82d..844de4418724 100644 > --- a/drivers/pci/controller/dwc/pci-keystone.c > +++ b/drivers/pci/controller/dwc/pci-keystone.c > @@ -924,7 +924,7 @@ static const struct pci_epc_features ks_pcie_am654_epc_features = { > .linkup_notifier = false, > .msi_capable = true, > .msix_capable = true, > - .bar[BAR_0] = { .type = BAR_RESERVED, .only_64bit = true, }, > + .bar[BAR_0] = { .type = BAR_RESERVED, }, > .bar[BAR_1] = { .type = BAR_RESERVED, }, > .bar[BAR_2] = { .type = BAR_FIXED, .fixed_size = SZ_1M, }, > .bar[BAR_3] = { .type = BAR_FIXED, .fixed_size = SZ_64K, }, > diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c > index 265f65fc673f..639bc2e12476 100644 > --- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c > +++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c > @@ -415,7 +415,7 @@ static const struct uniphier_pcie_ep_soc_data uniphier_pro5_data = { > .bar[BAR_1] = { .type = BAR_RESERVED, }, > .bar[BAR_2] = { .only_64bit = true, }, > .bar[BAR_3] = { .type = BAR_RESERVED, }, > - .bar[BAR_4] = { .type = BAR_RESERVED, .only_64bit = true, }, > + .bar[BAR_4] = { .type = BAR_RESERVED, }, > .bar[BAR_5] = { .type = BAR_RESERVED, }, > }, > }; > diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c > index 7fe8f4336765..da3fc0795b0b 100644 > --- a/drivers/pci/endpoint/pci-epc-core.c > +++ b/drivers/pci/endpoint/pci-epc-core.c > @@ -120,13 +120,6 @@ enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features > /* If the BAR is not reserved, return it. */ > if (epc_features->bar[i].type != BAR_RESERVED) > return i; > - > - /* > - * If the BAR is reserved, and marked as 64-bit only, then the > - * succeeding BAR is also reserved. > - */ > - if (epc_features->bar[i].only_64bit) > - i++; > } > > return NO_BAR; > diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h > index 4ccb4f4f3883..bb9c4dfcea93 100644 > --- a/include/linux/pci-epc.h > +++ b/include/linux/pci-epc.h > @@ -164,6 +164,10 @@ enum pci_epc_bar_type { > * should be configured as 32-bit or 64-bit, the EPF driver must > * configure this BAR as 64-bit. Additionally, the BAR succeeding > * this BAR must be set to type BAR_RESERVED. > + * > + * only_64bit should not be set on a BAR of type BAR_RESERVED. > + * (If BARx is a 64-bit BAR that an EPF driver is not allowed to > + * touch, then you must set both BARx and BARx+1 as BAR_RESERVED.) "then both BARx and BARx+1 must be set to type BAR_RESERVED." - Mani
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c index b2b93b4fa82d..844de4418724 100644 --- a/drivers/pci/controller/dwc/pci-keystone.c +++ b/drivers/pci/controller/dwc/pci-keystone.c @@ -924,7 +924,7 @@ static const struct pci_epc_features ks_pcie_am654_epc_features = { .linkup_notifier = false, .msi_capable = true, .msix_capable = true, - .bar[BAR_0] = { .type = BAR_RESERVED, .only_64bit = true, }, + .bar[BAR_0] = { .type = BAR_RESERVED, }, .bar[BAR_1] = { .type = BAR_RESERVED, }, .bar[BAR_2] = { .type = BAR_FIXED, .fixed_size = SZ_1M, }, .bar[BAR_3] = { .type = BAR_FIXED, .fixed_size = SZ_64K, }, diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c index 265f65fc673f..639bc2e12476 100644 --- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c +++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c @@ -415,7 +415,7 @@ static const struct uniphier_pcie_ep_soc_data uniphier_pro5_data = { .bar[BAR_1] = { .type = BAR_RESERVED, }, .bar[BAR_2] = { .only_64bit = true, }, .bar[BAR_3] = { .type = BAR_RESERVED, }, - .bar[BAR_4] = { .type = BAR_RESERVED, .only_64bit = true, }, + .bar[BAR_4] = { .type = BAR_RESERVED, }, .bar[BAR_5] = { .type = BAR_RESERVED, }, }, }; diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c index 7fe8f4336765..da3fc0795b0b 100644 --- a/drivers/pci/endpoint/pci-epc-core.c +++ b/drivers/pci/endpoint/pci-epc-core.c @@ -120,13 +120,6 @@ enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features /* If the BAR is not reserved, return it. */ if (epc_features->bar[i].type != BAR_RESERVED) return i; - - /* - * If the BAR is reserved, and marked as 64-bit only, then the - * succeeding BAR is also reserved. - */ - if (epc_features->bar[i].only_64bit) - i++; } return NO_BAR; diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h index 4ccb4f4f3883..bb9c4dfcea93 100644 --- a/include/linux/pci-epc.h +++ b/include/linux/pci-epc.h @@ -164,6 +164,10 @@ enum pci_epc_bar_type { * should be configured as 32-bit or 64-bit, the EPF driver must * configure this BAR as 64-bit. Additionally, the BAR succeeding * this BAR must be set to type BAR_RESERVED. + * + * only_64bit should not be set on a BAR of type BAR_RESERVED. + * (If BARx is a 64-bit BAR that an EPF driver is not allowed to + * touch, then you must set both BARx and BARx+1 as BAR_RESERVED.) */ struct pci_epc_bar_desc { enum pci_epc_bar_type type;
The definition of a reserved BAR is that EPF drivers should not touch them. The definition of only_64bit is that the EPF driver must configure this BAR as 64-bit. (An EPF driver is not allowed to choose if this BAR should be configured as 32-bit or 64-bit.) Thus, it does not make sense to put only_64bit of a BAR that EPF drivers are not allow to touch. Drop the only_64bit property from hardware descriptions that are of type reserved BAR. Signed-off-by: Niklas Cassel <cassel@kernel.org> --- drivers/pci/controller/dwc/pci-keystone.c | 2 +- drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 +- drivers/pci/endpoint/pci-epc-core.c | 7 ------- include/linux/pci-epc.h | 4 ++++ 4 files changed, 6 insertions(+), 9 deletions(-)