> +#ifdef CONFIG_PCI_P2PDMA
> +int64_t opal_pci_set_p2p(uint64_t phb_init, uint64_t phb_target,
> + uint64_t desc, uint16_t pe_number);
> +#endif
There should be no need for the ifdef here.
> + /*
> + * Configuring the initiator's PHB requires to adjust its
> + * TVE#1 setting. Since the same device can be an initiator
> + * several times for different target devices, we need to keep
> + * a reference count to know when we can restore the default
> + * bypass setting on its TVE#1 when disabling. Opal is not
> + * tracking PE states, so we add a reference count on the PE
> + * in linux.
> + *
> + * For the target, the configuration is per PHB, so we keep a
> + * target reference count on the PHB.
> + */
This could be shortened a bit by using up the whole 80 lines available
in source files.
> + mutex_lock(&p2p_mutex);
> +
> + if (desc & OPAL_PCI_P2P_ENABLE) {
> + /* always go to opal to validate the configuration */
> + rc = opal_pci_set_p2p(phb_init->opal_id, phb_target->opal_id,
> + desc, pe_init->pe_number);
> +
> + if (rc != OPAL_SUCCESS) {
> + rc = -EIO;
> + goto out;
> + }
> +
> + pe_init->p2p_initiator_count++;
> + phb_target->p2p_target_count++;
> + } else {
> + if (!pe_init->p2p_initiator_count ||
> + !phb_target->p2p_target_count) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + if (--pe_init->p2p_initiator_count == 0)
> + pnv_pci_ioda2_set_bypass(pe_init, true);
> +
> + if (--phb_target->p2p_target_count == 0) {
> + rc = opal_pci_set_p2p(phb_init->opal_id,
> + phb_target->opal_id, desc,
> + pe_init->pe_number);
> + if (rc != OPAL_SUCCESS) {
> + rc = -EIO;
> + goto out;
> + }
> + }
> + }
> + rc = 0;
> +out:
> + mutex_unlock(&p2p_mutex);
> + return rc;
> +}
The enable and disable path shares almost no code, why not split it into
two functions?
> +static bool pnv_pci_controller_owns_addr(struct pci_controller *hose,
> + phys_addr_t addr, size_t size)
> +{
> + struct resource *r;
> + int i;
> +
> + /*
> + * it seems safe to assume the full range is under the same
> + * PHB, so we can ignore the size
Capitalize the first character in a multi-line comment, and use up the
whole 80 chars.
> + */
> + for (i = 0; i < 3; i++) {
Replace the magic 3 with ARRAY_SIZE(hose->mem_resources) ?
> + r = &hose->mem_resources[i];
Move the r declaration here and initialize it on the same line.
> + if (r->flags && (addr >= r->start) && (addr < r->end))
No need for the inner braces.
> + return true;
> + }
> + return false;
> +}
> +
> +/*
> + * find the phb owning a mmio address if not owned locally
> + */
> +static struct pnv_phb *pnv_pci_find_owning_phb(struct pci_dev *pdev,
> + phys_addr_t addr, size_t size)
> +{
> + struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> + struct pnv_phb *phb;
> +
> + /* fast path */
> + if (pnv_pci_controller_owns_addr(hose, addr, size))
> + return NULL;
Maybe open code the pci_bus_to_host(pdev->bus) call here, as we just
overwrite host in the list iteration?
> +
> + list_for_each_entry(hose, &hose_list, list_node) {
> + phb = hose->private_data;
Move the ohb declaration here and initialize it on the same line.
> + if (phb->type != PNV_PHB_NPU_NVLINK &&
> + phb->type != PNV_PHB_NPU_OCAPI) {
> + if (pnv_pci_controller_owns_addr(hose, addr, size))
> + return phb;
> + }
> + }
> + return NULL;
> +}
> +
> +static int pnv_pci_dma_map_resource(struct pci_dev *pdev,
> + phys_addr_t phys_addr, size_t size,
> + enum dma_data_direction dir)
> +{
> + struct pnv_phb *target_phb;
> + int rc;
> + u64 desc;
> +
> + target_phb = pnv_pci_find_owning_phb(pdev, phys_addr, size);
> + if (target_phb) {
Return early here for the !target_phb case?
> + desc = OPAL_PCI_P2P_ENABLE;
> + if (dir == DMA_TO_DEVICE)
> + desc |= OPAL_PCI_P2P_STORE;
> + else if (dir == DMA_FROM_DEVICE)
> + desc |= OPAL_PCI_P2P_LOAD;
> + else if (dir == DMA_BIDIRECTIONAL)
> + desc |= OPAL_PCI_P2P_LOAD | OPAL_PCI_P2P_STORE;
Seems like this could be split into a little helper.
> + rc = pnv_pci_ioda_set_p2p(pdev, target_phb, desc);
> + if (rc) {
> + dev_err(&pdev->dev, "Failed to setup PCI peer-to-peer for address %llx: %d\n",
> + phys_addr, rc);
> + return rc;
> + }
No need for this printk, the callers should already deal with mapping
failures.
