RE: [RFC v1 2/7] iommu/arm-smmu-v3: Add erratum framework functions

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

 





> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@xxxxxxx]
> Sent: Tuesday, May 16, 2017 2:08 PM
> To: Shameerali Kolothum Thodi; will.deacon@xxxxxxx;
> mark.rutland@xxxxxxx; lorenzo.pieralisi@xxxxxxx; hanjun.guo@xxxxxxxxxx
> Cc: Gabriele Paoloni; John Garry; iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> acpi@xxxxxxxxxxxxxxx; devel@xxxxxxxxxx; Linuxarm; Wangzhou (B);
> Guohanjun (Hanjun Guo)
> Subject: Re: [RFC v1 2/7] iommu/arm-smmu-v3: Add erratum framework
> functions
> 
> On 13/05/17 10:47, shameer wrote:
> > This will provide a way to replace the existing skip_prefetch_cmd
> > erratum using the new framework.
> 
> Yikes, between this and patch 1 we're already pushing 70 lines of new
> code, and it still doesn't actually do anything yet. Implementing the
> SMMUv3 equivalent of SMMUv2's acpi_smmu_get_data() would probably
> be
> about 10 lines; all you need to do is set some quirk flags based on a
> compatible value. These quirks aren't really any different in principle
> to the firmware COHACC overrides that we already process.
> 
> Sorry, I'm saying no to a massively overengineered "framework" for
> something so relatively simple.

Thanks Robin for going through patches. The "framework" was added thinking
it might be useful when multiple quirk implementations are added to the
SMMUv3 driver from different vendors. As you said it doesn't look like, 
adding any value at the moment. I will revise patch set based on SMMUv2s
way for ACPI and retain the broken-* for dtb. 

Thanks,
Shameer
 
> Robin.
> 
> > Signed-off-by: shameer <shameerali.kolothum.thodi@xxxxxxxxxx>
> > ---
> >  drivers/iommu/arm-smmu-v3.c | 58
> +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> >
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-
> v3.c
> > index a166590..f20d5d5 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -664,16 +664,72 @@ enum smmu_erratum_match_type {
> >  	se_match_dt,
> >  };
> >
> > +void erratum_skip_prefetch_cmd(struct arm_smmu_device *smmu, void
> *arg)
> > +{
> > +	smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
> > +}
> > +
> >  struct smmu_erratum_workaround {
> >  	enum smmu_erratum_match_type match_type;
> >  	const void *id;	/* Indicate the Erratum ID */
> >  	const char *desc_str;
> > +	void (*enable)(struct arm_smmu_device *, void *);
> >  };
> >
> >  static const struct smmu_erratum_workaround smmu_workarounds[] = {
> >
> >  };
> >
> > +typedef bool (*se_match_fn_t)(const struct
> smmu_erratum_workaround *,
> > +							  const void *);
> > +static
> > +bool smmu_check_dt_erratum(const struct smmu_erratum_workaround
> *wa,
> > +						   const void *arg)
> > +{
> > +	const struct device_node *np = arg;
> > +
> > +	return of_property_read_bool(np, wa->id);
> > +}
> > +
> > +static void smmu_enable_errata(struct arm_smmu_device *smmu,
> > +				enum smmu_erratum_match_type type,
> > +				se_match_fn_t match_fn,
> > +				void *arg)
> > +{
> > +	const struct smmu_erratum_workaround *wa =
> smmu_workarounds;
> > +
> > +	for (; wa->desc_str; wa++) {
> > +		if (wa->match_type != type)
> > +			continue;
> > +
> > +		if (match_fn(wa, arg)) {
> > +			if (wa->enable) {
> > +				wa->enable(smmu, arg);
> > +				dev_info(smmu->dev,
> > +					"Enabling workaround for %s\n",
> > +					 wa->desc_str);
> > +			}
> > +		}
> > +	}
> > +}
> > +
> > +
> > +static void smmu_check_workarounds(struct arm_smmu_device *smmu,
> > +				  enum smmu_erratum_match_type type,
> > +				  void *arg)
> > +{
> > +	se_match_fn_t match_fn = NULL;
> > +
> > +	switch (type) {
> > +	case se_match_dt:
> > +		match_fn = smmu_check_dt_erratum;
> > +		break;
> > +	}
> > +
> > +	smmu_enable_errata(smmu, type, match_fn, arg);
> > +
> > +}
> > +
> >  static struct arm_smmu_domain *to_smmu_domain(struct
> iommu_domain *dom)
> >  {
> >  	return container_of(dom, struct arm_smmu_domain, domain);
> > @@ -2641,6 +2697,8 @@ static int arm_smmu_device_dt_probe(struct
> platform_device *pdev,
> >
> >  	parse_driver_options(smmu);
> >
> > +	smmu_check_workarounds(smmu, se_match_dt, dev->of_node);
> > +
> >  	if (of_dma_is_coherent(dev->of_node))
> >  		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
> >
> >

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux