On 01/21/2013 12:36 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren@xxxxxxxxxxxxx> wrote @ Fri, 18 Jan 2013 17:44:13 +0100:
>
>>> Even the checks if "offs" is in some of register blocks could be
>>> ifdef'ed out with DEBUG. "smmu->regbase" can be calculated in probe()
>>> as below. I don't think that we don't need to access "mc" DT entry to
>>> get this address. since "smmu"'s 1st reg block always starts at 0x10.
>>>
>>> /* same as "mc"'s 1st reg block */
>>> smmu->regbase = smmu->reg[0] & PAGE_MASK;
>>
>> I don't see regbase in the existing driver or your patch. Are you
>
> I attached the update one below just for "regbase".
>
>> proposing to simply make readl/writel add the offset onto a base address
>> that's calculated like that? That may not work in general; if the SMMU
>> register ranges cross a page boundary, and the various separate ranges
>> end up getting mapped to non-contiguous virtual addresses, using a
>> single base address won't work.
>
> That's not in general, but I think that this works because the 1st
> SMMU register block offset is fixed(0x10) against MC base. If this is
> not acceptable, then, I think that SMMU driver needs to access to MC
> node to get the its base. What do you think?
> drivers/iommu/tegra-smmu.c | 62 ++++++++++++++++++++++++--------------------
> 1 file changed, 34 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> +#ifdef DEBUG
> +static inline void smmu_check_reg_range(size_t offs)
Like I said before, when is DEBUG defined? Rarely I suspect. It'd be
best to simply enable smmu_check_reg_range() all the time. As such ...
> +{
> + int i;
> +
> + for (i = 0; i < smmu->nregs; i++) {
> + BUG_ON(offs < smmu->regs[i] - smmu->regbase);
> + if (offs <= smmu->rege[i] - smmu->regbase)
> + break;
> + }
> +}
> +#else
> +static inline void smmu_check_reg_range(size_t offs) { }
> +#endif
> +
> static inline u32 smmu_read(struct smmu_device *smmu, size_t offs)
> {
> - BUG_ON(offs < 0x10);
> - if (offs < 0x3c)
> - return readl(smmu->regs[0] + offs - 0x10);
> - BUG_ON(offs < 0x1f0);
> - if (offs < 0x200)
> - return readl(smmu->regs[1] + offs - 0x1f0);
> - BUG_ON(offs < 0x228);
> - if (offs < 0x284)
> - return readl(smmu->regs[2] + offs - 0x228);
> - BUG();
> + smmu_check_reg_range(offs);
... here, you'd be doing the loop every access anyway, so you may as
well not calculate regbase at all, move the body of
smmu_check_reg_range() into smmu_read()/smmu_write(), and do the access
inside the if statement inside the loop, with the per-range mapping.
> + return readl(smmu->regbase + offs);
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
