On Thu, Nov 14, 2024 at 05:52:39PM -0500, Frank Li wrote:
> Introduce the helper function pci_epf_align_addr() to adjust addresses
> according to PCI BAR alignment requirements, converting addresses into base
> and offset values.
>
> Signed-off-by: Frank Li <Frank.Li@xxxxxxx>
> ---
> change from v6 to v7
> - new patch
> ---
> drivers/pci/endpoint/pci-epf-core.c | 39 +++++++++++++++++++++++++++++++++++++
> include/linux/pci-epf.h | 13 +++++++++++++
> 2 files changed, 52 insertions(+)
>
> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> index 8fa2797d4169a..a3f172cc786e9 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -464,6 +464,45 @@ struct pci_epf *pci_epf_create(const char *name)
> }
> EXPORT_SYMBOL_GPL(pci_epf_create);
>
> +/**
> + * pci_epf_align_addr() - Get base address and offset that match bar's
> + * alignment requirement
> + * @epf: the EPF device
> + * @addr: the address of the memory
> + * @bar: the BAR number corresponding to map addr
> + * @base: return base address, which match BAR's alignment requirement, nothing
> + * return if NULL
> + * @off: return offset, nothing return if NULL
> + *
> + * Helper function to convert input 'addr' to base and offset, which match
> + * BAR's alignment requirement.
> + */
> +int pci_epf_align_addr(struct pci_epf *epf, enum pci_barno bar, u64 addr, u64 *base, size_t *off)
Nit: perhaps rename this function to:
pci_epf_align_ib_addr()
or
pci_epf_align_inbound_addr()
to more clearly not confuse this with:
if (epc->ops->align_addr)
.align_addr()
(Ideally those functions should have been named align_ob_addr(),
or align_outbound_addr())
> +{
> + const struct pci_epc_features *epc_features;
> + u64 align;
> +
> + epc_features = pci_epc_get_features(epf->epc, epf->func_no, epf->vfunc_no);
> + if (!epc_features) {
> + dev_err(&epf->dev, "epc_features not implemented\n");
> + return -EOPNOTSUPP;
> + }
> +
> + align = epc_features->align;
> + align = align ? align : 128;
> + if (epc_features->bar[bar].type == BAR_FIXED)
> + align = max(epc_features->bar[bar].fixed_size, align);
> +
> + if (base)
> + *base = round_down(addr, align);
> +
> + if (off)
> + *off = addr & (align - 1);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_epf_align_addr);
> +
> static void pci_epf_dev_release(struct device *dev)
> {
> struct pci_epf *epf = to_pci_epf(dev);
> diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
> index 5374e6515ffa0..20f4f31ba9b36 100644
> --- a/include/linux/pci-epf.h
> +++ b/include/linux/pci-epf.h
> @@ -238,6 +238,19 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
> enum pci_epc_interface_type type);
> void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar,
> enum pci_epc_interface_type type);
> +
> +int pci_epf_align_addr(struct pci_epf *epf, enum pci_barno bar, u64 addr, u64 *base, size_t *off);
> +static inline int pci_epf_align_addr_lo_hi(struct pci_epf *epf, enum pci_barno bar,
> + u32 low, u32 high, u64 *base, size_t *off)
> +{
> + u64 addr = high;
> +
> + addr <<= 32;
> + addr |= low;
> +
> + return pci_epf_align_addr(epf, bar, addr, base, off);
> +}
I'm not sure if this function deserves to live :)
Can't the caller just do this before calling pci_epf_align_addr() ?
Kind regards,
Niklas
