From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jon Hunter Subject: Re: [PATCH v8 1/3] iommu/arm-smmu: add NVIDIA implementation for dual ARM MMU-500 usage Date: Tue, 30 Jun 2020 17:32:41 +0100 Message-ID: <4037efc7-fbed-e8cf-dac7-212c65014e4e@nvidia.com> References: <20200630001051.12350-1-vdumpa@nvidia.com> <20200630001051.12350-2-vdumpa@nvidia.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Sender: "iommu" To: Krishna Reddy Cc: Sachin Nikam , "nicoleotsuka-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" , Mikko Perttunen , Bryan Huntsman , "will-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org" , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Pritesh Raithatha , Timo Alho , "iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org" , Nicolin Chen , "linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Yu-Huan Hsu , Thierry Reding , "robin.murphy-5wv7dgnIgG8@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , Bitan Biswas List-Id: linux-tegra@vger.kernel.org On 30/06/2020 17:23, Krishna Reddy wrote: >>> +struct arm_smmu_device *nvidia_smmu_impl_init(struct arm_smmu_device >>> +*smmu) { >>> + unsigned int i; > .... >>> + for (i = 1; i < MAX_SMMU_INSTANCES; i++) { >>> + struct resource *res; >>> + >>> + res = platform_get_resource(pdev, IORESOURCE_MEM, i); >>> + if (!res) >>> + break; > >> Currently this driver is only supported for Tegra194 which I understand has 3 SMMUs. Therefore, I don't feel that we should fail silently here, I think it is better to return an error if all 3 cannot be initialised. > > Initialization of all the three SMMU instances is not necessary here. That is not what I am saying. > The driver can work with all the possible number of instances 1, 2 and 3 based on the DT config though it doesn't make much sense to use it with 1 instance. > There is no silent failure here from driver point of view. If there is misconfig in DT, SMMU faults would catch issues. I disagree and you should return a proper error here. >>> + nvidia_smmu->bases[i] = devm_ioremap_resource(smmu->dev, res); >>> + if (IS_ERR(nvidia_smmu->bases[i])) >>> + return ERR_CAST(nvidia_smmu->bases[i]); > >> You want to use PTR_ERR() here. > > PTR_ERR() returns long integer. > This function returns a pointer. ERR_CAST is the right one to use here. Ah yes, indeed. OK that's fine. Jon -- nvpublic