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

[Rob Herring <robh@kernel.org>]

On Wed, Oct 30, 2019 at 9:56 AM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> 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@arm.com/
> > > >
> > > > 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().

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel