Hi Marc,
On 4/15/2019 2:24 PM, Marc Gonzalez wrote:
Trimming CC list for very minor suggestions (feel free to ignore)
Thanks for the review. Please find my responses inline below.
On 15/04/2019 10:07, Vivek Gautam wrote:
Extract the SID and add the information to context fault log.
This is specially useful in a distributed smmu architecture
where multiple masters are connected to smmu. SID information
helps to quickly identify the faulting master device.
I find it slightly clearer to keep acronyms capitalized, such as SMMU.
Sure, will change it.
@@ -575,7 +575,10 @@ static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
struct arm_smmu_device *smmu = smmu_domain->smmu;
+ void __iomem *gr1_base = ARM_SMMU_GR1(smmu);
void __iomem *cb_base;
+ u32 cbfrsynra;
+ u16 sid;
cb_base = ARM_SMMU_CB(smmu, cfg->cbndx);
fsr = readl_relaxed(cb_base + ARM_SMMU_CB_FSR);
@@ -586,9 +589,16 @@ static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
fsynr = readl_relaxed(cb_base + ARM_SMMU_CB_FSYNR0);
iova = readq_relaxed(cb_base + ARM_SMMU_CB_FAR);
+ cbfrsynra = readl_relaxed(gr1_base +
+ ARM_SMMU_GR1_CBFRSYNRA(cfg->cbndx));
I find it slightly clearer to write
void __iomem *reg;
...
reg = ARM_SMMU_GR1(smmu) + ARM_SMMU_GR1_CBFRSYNRA(cfg->cbndx);
cbfrsynra = readl_relaxed(reg);
I would actually argue here, to go with the flow of how this function
declares the variables, for the simple case of symmetry.
Let me know what do you think.
Thanks & regards
Vivek
Regards.