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:44:23 +0100 Message-ID: References: <20200630001051.12350-1-vdumpa@nvidia.com> <20200630001051.12350-2-vdumpa@nvidia.com> <4037efc7-fbed-e8cf-dac7-212c65014e4e@nvidia.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4037efc7-fbed-e8cf-dac7-212c65014e4e-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> Content-Language: en-US Sender: linux-tegra-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Krishna Reddy Cc: "joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org" , "will-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org" , "robin.murphy-5wv7dgnIgG8@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , "iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org" , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Thierry Reding , Yu-Huan Hsu , Sachin Nikam , Pritesh Raithatha , Timo Alho , Bitan Biswas , Mikko Perttunen , Nicolin Chen , Bryan Huntsman , "nicoleotsuka-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" List-Id: linux-tegra@vger.kernel.org On 30/06/2020 17:32, Jon Hunter wrote: > 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. OK, well I see what you are saying, but if we intended to support all 3 for Tegra194, then we should ensure all 3 are initialised correctly. My concern here is testing, because when things break in upstream I am usually the one that tracks it down. Not having clear warning/error messages when something is not initialised as expected makes it harder. It would be better to query the number of SMMUs populated in device-tree and then ensure that all are initialised correctly. Jon -- nvpublic