Re: [PATCH] iommu/arm-smmu-qcom: create qcom_smmu_impl for ACPI boot

From: Shawn Guo <shawn.guo@linaro.org>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>,
	Will Deacon <will@kernel.org>,
	lorenzo.pieralisi@arm.com, linux-arm-kernel@lists.infradead.org,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH] iommu/arm-smmu-qcom: create qcom_smmu_impl for ACPI boot
Date: Sun, 28 Mar 2021 16:44:12 +0800	[thread overview]
Message-ID: <20210328084411.GB24152@dragon> (raw)
In-Reply-To: <e7385b23-a91b-dd8d-5e9a-3505a18a2249@arm.com>

On Thu, Mar 25, 2021 at 05:07:52PM +0000, Robin Murphy wrote:
> On 2021-03-25 17:02, Bjorn Andersson wrote:
> > On Thu 25 Mar 09:59 CDT 2021, Will Deacon wrote:
> > 
> > > [+ Lorenzo]
> > > 
> > > On Mon, Mar 01, 2021 at 03:40:21PM +0800, Shawn Guo wrote:
> > > > Though qcom_adreno_smmu_impl is not used by ACPI boot right now,
> > > > qcom_smmu_impl is already required at least to boot up Lenovo Flex 5G
> > > > laptop.  Let's check asl_compiler_id in IORT header to ensure we are
> > > > running a QCOM SMMU and create qcom_smmu_impl for it.
> > > > 
> > > > !np is used to check ACPI boot, because fwnode of SMMU device is
> > > > a static allocation and thus has_acpi_companion() doesn't work here.
> > > > 
> > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > > ---
> > > >   drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 33 ++++++++++++++++++++++
> > > >   1 file changed, 33 insertions(+)
> > > 
> > > I don't know what a "asl_compiler_id" is, but it doesn't sound like it
> > > has an awful lot to do with the SMMU.
> > > 
> > 
> > I would prefer that we somehow relate this to the particular board,
> > rather than all Qualcomm-related ACPI tables. E.g. by relying on the
> > SMMU devices having a _HID of QCOM0409.
> > 
> > Shawn, any reason for this wouldn't be possible?
> 
> To do something broadly similar to identify HiSilicon's SMMU PMCG
> implementation, we use acpi_match_platform_list() - could we do the same for
> this?

Thanks for the suggestion, Robin!  Yes, acpi_platform_list is useful
for our problem as well.  But we will need multiple entries even for
a single Snapdragon SoC.  For example, both Lenovo Flex 5G and Microsoft
Surface Pro X are powered by Snapdragon SC8180X, but their IORT tables
have different fields that acpi_match_platform_list() is looking at.
If this is not a problem, yeah, acpi_platform_list is a better way out,
and I will start updating the patch.

Shawn

* Lenovo Flex 5G
[000h 0000   4]                    Signature : "IORT"    [IO Remapping Table]
[004h 0004   4]                 Table Length : 00001943
[008h 0008   1]                     Revision : 00
[009h 0009   1]                     Checksum : DA
[00Ah 0010   6]                       Oem ID : "LENOVO"
[010h 0016   8]                 Oem Table ID : "CB-01   "
[018h 0024   4]                 Oem Revision : 00008180
[01Ch 0028   4]              Asl Compiler ID : "QCOM"
[020h 0032   4]        Asl Compiler Revision : 00000001

* Microsoft Surface Pro X
[000h 0000   4]                    Signature : "IORT"    [IO Remapping Table]
[004h 0004   4]                 Table Length : 0000169C
[008h 0008   1]                     Revision : 00
[009h 0009   1]                     Checksum : 3A
[00Ah 0010   6]                       Oem ID : "QCOM  "
[010h 0016   8]                 Oem Table ID : "QCOMEDK2"
[018h 0024   4]                 Oem Revision : 00008180
[01Ch 0028   4]              Asl Compiler ID : "QCOM"
[020h 0032   4]        Asl Compiler Revision : 00000001

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel