On Fri, Feb 12, 2016 at 03:23:04PM +0000, Robin Murphy wrote:
> On 12/02/16 12:32, Laurent Pinchart wrote:
> >On Friday 12 February 2016 12:08:58 Will Deacon wrote:
> >>On Thu, Feb 11, 2016 at 04:13:45PM +0000, Robin Murphy wrote:
> >>>@@ -173,10 +187,12 @@ static inline void io_pgtable_tlb_sync(struct
> >>>io_pgtable *iop)
> >>> *
> >>> * @alloc: Allocate a set of page tables described by cfg.
> >>> * @free: Free the page tables associated with iop.
> >>>+ * @supported_quirks: Bitmap of quirks supported by the format.
> >>> */
> >>> struct io_pgtable_init_fns {
> >>> struct io_pgtable *(*alloc)(struct io_pgtable_cfg *cfg, void
> >*cookie);
> >>> void (*free)(struct io_pgtable *iop);
> >>>+ unsigned int supported_quirks;
> >>> };
> >>
> >>My only gripe here is that this is supposed to be a struct of function
> >>pointers... get_supported_quirks() ?
> >
> >Can't we instead say it's a struct of function pointers and static data ? :-)
> >A function to retrieve the supported quirks will just contribute to kernel
> >bloat without adding any extra benefit.
>
> Bleh - personally I think it's marginally more obvious and readable to have
> the information looking like data, but I'd much sooner just have a
> hard-coded "if (cfq->quirks & ..." check in every format's alloc function
> than introduce a whole new callback. Does that sound like a reasonable
> compromise?
Yeah, do that. You never know, there could be quirks that are only supported
based on other things in the cfg, so it's more extensible that way too.
Will
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
