From mboxrd@z Thu Jan 1 00:00:00 1970 From: Will Deacon Subject: Re: [PATCH 2/9] iommu/arm-smmu: Calculate SMMU_CB_BASE from smmu register values Date: Fri, 27 Sep 2013 11:51:53 +0100 Message-ID: <20130927105153.GG9057@mudshark.cambridge.arm.com> References: <1380234982-1677-1-git-send-email-andreas.herrmann@calxeda.com> <1380234982-1677-3-git-send-email-andreas.herrmann@calxeda.com> <20130927095157.GA9057@mudshark.cambridge.arm.com> <20130927102359.GL3315@alberich> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20130927102359.GL3315@alberich> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Andreas Herrmann Cc: "iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org" , Rob Herring , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: iommu@lists.linux-foundation.org On Fri, Sep 27, 2013 at 11:23:59AM +0100, Andreas Herrmann wrote: > On Fri, Sep 27, 2013 at 05:51:57AM -0400, Will Deacon wrote: > > Hi Andreas, > > > > On Thu, Sep 26, 2013 at 11:36:14PM +0100, Andreas Herrmann wrote: > > > Currently it is derived from smmu resource size. In case of a > > > mismatchin between the two calculations trust DT more than register > > > values and overwrite cb_base. > > > > I thought the driver already favoured the DT? > > > > > @@ -1702,12 +1704,23 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu) > > > > > > /* Check that we ioremapped enough */ > > > size = 1 << (((id >> ID1_NUMPAGENDXB_SHIFT) & ID1_NUMPAGENDXB_MASK) + 1); > > > - size *= (smmu->pagesize << 1); > > > + size *= smmu->pagesize; > > > + smmu->cb_base = smmu->base + size; > > > + size *= 2; > > > + > > > if (smmu->size < size) > > > dev_warn(smmu->dev, > > > "device is 0x%lx bytes but only mapped 0x%lx!\n", > > > size, smmu->size); > > > > > > + t = (unsigned long) smmu->base + (smmu->size >> 1); > > > + if ((unsigned long)smmu->cb_base != t) { > > > + dev_warn(smmu->dev, "address space mismatch, " > > > + "overwriting cb_base (old: 0x%lx, new: 0x%lx)\n", > > > + (unsigned long) smmu->cb_base, t); > > > + smmu->cb_base = (void *) t; > > > + } > > > + > > > > I expect I'm just being slow here (only one coffee in), but I can't see what > > this gets us over the current use of resource_size (which goes and uses the > > DT). > > On balance it adds a warning if there is an inconsistency between the > resource size and the relevant registers describing the SMMU address > space. Well, we should already print the "device is 0x%lx bytes but only mapped 0x%lx!" message, which I think is enough to go and figure out what happened. Will