Re: [PATCH v3 19/25] PCI: of: Add inbound resource parsing to helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 30, 2019 at 05:18:25PM -0500, Rob Herring wrote:
> On Wed, Oct 30, 2019 at 9:56 AM Lorenzo Pieralisi
> <lorenzo.pieralisi@xxxxxxx> wrote:
> >
> > On Wed, Oct 30, 2019 at 12:49:59PM +0000, Robin Murphy wrote:
> > > On 30/10/2019 11:48, Lorenzo Pieralisi wrote:
> > > > [+Cc Robin]
> > > >
> > > > On Wed, Oct 30, 2019 at 12:14:54PM +0530, Srinath Mannam wrote:
> > > > > Hi Lorenzo,
> > > > >
> > > > > Thanks for the details.
> > > > > Based on Robin's comment in the old patch, I thought dma_ranges list
> > > > > will be in sorted order.
> > > > > https://lore.kernel.org/lkml/741a4210-251c-9c00-d4a7-bc7ebf8cd57b@xxxxxxx/
> > > > >
> > > > > Now, another patch is required to sort the list before reserving in
> > > > > iova_reserve_pci_windows() function.
> > > > >
> > > > > Regards,
> > > > > Srinath.
> > > >
> > > > Don't top-post please.
> > > >
> > > > https://en.wikipedia.org/wiki/Posting_style#Top-posting
> > > >
> > > > Yes, the dma_ranges list must be sorted somehow I reckon
> > > > iova_reserve_pci_windows() is where it should be done (since that's
> > > > where the requirement is) or it can be done in
> > > > devm_of_pci_get_host_bridge_resources().
> > > >
> > > > Thoughts ?
> > >
> > > Right, strictly it's only iova_reserve_pci_windows() that needs the list
> > > sorted, it just worked out that maintaining the list in sorted order by
> > > construction took a fair bit less code than explicitly sorting it. In terms
> > > of preserving that behaviour in a slightly more generalised fashion I
> > > suppose we could add something like:
> > >
> > > void pci_add_resource_offset_sorted(struct list_head *resources,
> > >                               struct resource *res,
> > >                               resource_size_t offset)
> > > {
> > >       struct resource_entry *entry;
> > >
> > >       resource_list_for_each_entry(entry, resources)
> > >               if (entry->res.start > res.start)
> > >                       break;
> > >
> > >       pci_add_resource_offset(&entry->node, res, offset);
> > > }
> > >
> > > but if you'd rather add a specific resource_list_sort() or even just
> > > open-code it in iommu-dma, I don't have any real preference. The "least code
> > > necessary" approach definitely made sense when individual drivers were
> > > expected to build their own lists, but once it gets generalised then having
> > > a sensible and robust API becomes a more important consideration.
> >
> > I think that open coding it in iommu-dma is fine, @RobH would you be
> > able to add this to the series please ? I think it should be added to
> > prevent any regressions, we can't rely on dma-ranges entries order in DT
> > files.
> 
> I don't think it's good to be modifying the list as a side effect of
> calling iova_reserve_pci_windows() and making a copy of it wouldn't be
> great either. So I'm just going to keep it sorted in
> devm_of_pci_get_host_bridge_resources().

Yes I thought about that - that's the most reasonable approach I think.

Thanks,
Lorenzo



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux