On Wed, Aug 20, 2025 at 07:18:39PM +0800, Randolph Lin wrote:
> Introduce a callback for outbound ATU range checking to support
> range validations specific to cases that deviate from the generic
> check.
>
> Signed-off-by: Randolph Lin <randolph@xxxxxxxxxxxxx>
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 18 +++++++++++++-----
> drivers/pci/controller/dwc/pcie-designware.h | 3 +++
> 2 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 89aad5a08928..f410aefaeb5e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -535,12 +535,20 @@ int dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
> u32 retries, val;
> u64 limit_addr;
>
> - limit_addr = parent_bus_addr + atu->size - 1;
> + if (pci->ops && pci->ops->outbound_atu_check) {
> + val = pci->ops->outbound_atu_check(pci, atu, &limit_addr);
The return is not a "val" and not a "u32". It should be named "ret"
or similar. Should be "int" since the callback and
dw_pcie_prog_outbound_atu() return "int". But see below for possible
signature change.
Also not 100% convinced this is needed, see other patch where this is
implemented.
> + if (val)
> + return val;
> + } else {
> + limit_addr = parent_bus_addr + atu->size - 1;
>
> - if ((limit_addr & ~pci->region_limit) != (parent_bus_addr & ~pci->region_limit) ||
> - !IS_ALIGNED(parent_bus_addr, pci->region_align) ||
> - !IS_ALIGNED(atu->pci_addr, pci->region_align) || !atu->size) {
> - return -EINVAL;
> + if ((limit_addr & ~pci->region_limit) !=
> + (parent_bus_addr & ~pci->region_limit) ||
> + !IS_ALIGNED(parent_bus_addr, pci->region_align) ||
> + !IS_ALIGNED(atu->pci_addr, pci->region_align) ||
> + !atu->size) {
> + return -EINVAL;
> + }
> }
>
> dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_LOWER_BASE,
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 00f52d472dcd..40dd2c83b1c7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -469,6 +469,9 @@ struct dw_pcie_ep {
>
> struct dw_pcie_ops {
> u64 (*cpu_addr_fixup)(struct dw_pcie *pcie, u64 cpu_addr);
> + u32 (*outbound_atu_check)(struct dw_pcie *pcie,
> + const struct dw_pcie_ob_atu_cfg *atu,
> + u64 *limit_addr);
I have kind of an allergic reaction to things named "check" because
the name doesn't suggest anything about what the function does or what
it will return. For bool functions, I prefer a name that's a
predicate that can be either true or false, e.g., "valid".
This isn't a bool, but possibly *could* be, e.g.,
"outbound_atu_addr_valid()". Then the caller would be something like:
if (!pci->ops->outbound_atu_addr_valid(...))
return -EINVAL;
