[PATCH 4/3] iommu/io-pgtable: Rationalise quirk handling

Will Deacon will.deacon at arm.com
Fri Feb 12 07:55:18 PST 2016


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



More information about the linux-arm-kernel mailing list