Re: [PATCH 10/22] PCI: Add pci_resource_to_peer()

[Bjorn Helgaas <helgaas@xxxxxxxxxx>]

On Tue, Sep 15, 2015 at 12:10:55PM -0500, Will Davis wrote:
> Add helper to convert a struct resource to a peer DMA address.
> 
> Signed-off-by: Will Davis <wdavis@xxxxxxxxxx>
> ---
>  include/linux/pci.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index db0cb51..2a9deff 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1508,6 +1508,22 @@ static inline void pci_resource_to_user(const struct pci_dev *dev, int bar,
>  }
>  #endif /* HAVE_ARCH_PCI_RESOURCE_TO_USER */
>  
> +#ifdef CONFIG_HAS_DMA_P2P
> +static inline bool pci_resource_to_peer(struct pci_dev *dev,

I'm not really a fan of functions that return bool unless the
function name is obviously a predicate.  So in this case, I'd
return 0 or a negative errno.

> +					struct pci_dev *peer,
> +					struct resource *res,
> +					dma_peer_addr_t *peer_addr) {
> +	if (pci_peer_traffic_supported(dev, peer)) {
> +		struct pci_bus_region region;
> +
> +		pcibios_resource_to_bus(dev->bus, &region, res);
> +		*peer_addr = region.start;
> +		return true;
> +	}
> +
> +	return false;

It's trivial, but I'd structure it like this, which is sort of the
typical "return errors early" pattern, and reduces indentation of the
mainline path:

  if (!pci_peer_traffic_supported(dev, peer))
    return -EINVAL;

  pcibios_resource_to_bus(dev->bus, &region, res);
  *peer_addr = region.start;
  return 0;

> +}
> +#endif /* CONFIG_HAS_DMA_P2P */
>  
>  /*
>   *  The world is not perfect and supplies us with broken PCI devices.
> -- 
> 2.5.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html