Re: [PATCH 2/2] iommu: arm-smmu-qcom: add sdm670 compatible

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 22/09/2022 00:14, Konrad Dybcio wrote:


On 21.09.2022 21:05, Krzysztof Kozlowski wrote:
On 21/09/2022 20:48, Konrad Dybcio wrote:


On 21.09.2022 20:47, Konrad Dybcio wrote:


On 21.09.2022 09:31, Krzysztof Kozlowski wrote:
On 21/09/2022 00:39, Richard Acayan wrote:
The Snapdragon 670 needs the IOMMU for GENI I2C. Add a compatible string to
support it.

Signed-off-by: Richard Acayan <mailingradian@xxxxxxxxx>
---
  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index b2708de25ea3..bf9653b9eb89 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -431,6 +431,7 @@ static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
  	{ .compatible = "qcom,sc8180x-smmu-500" },
  	{ .compatible = "qcom,sc8280xp-smmu-500" },
  	{ .compatible = "qcom,sdm630-smmu-v2" },
+	{ .compatible = "qcom,sdm670-smmu-500" },

Why do we keep adding compatibles to the driver for apparently
compatible devices?

Because Linux has not funny run on bare Qualcomm hardware ever since at least msm8x60 times and
s/funny/fully

unfortunate typo, this is not funny, quite the contrary..

Konrad
we are not interacting with real hardware, only with Qualcomm's flawed virtual implementation
of it, that's abstracted to us through various generations of their saddening software stack. This
is also the case for many more standard components, even as far as the GIC on recent boards..

Unfortunately I don't get this explanation... you mean some other
firmware requires Linux drivers to use specific compatibles instead of
one fallback?
No, perhaps I misunderstood you.


All of these do not have driver data, so they are essentially compatible
for Linux driver. Growing this list in the driver seems pointless. What
is the benefit of growing driver with same entries, except more patches?
Compatible lists in smmu-impl files allow matching driver quirks for SMMUs themselves
and consumer devices (such as MDSS). The situation is more complicated, because some
qcom SMMUs also require more quirks than others (think 8974 vs 8994 vs 8996/pro&660&8998
vs 845+ vs adreno smmu in various flavours), so all qcom SMMUs need to use
`qcom_smmu_impl` and some others need even more quirks on top of that (that generally
hurt performance or functionality, so we don't want them when they're unnecessary).
If all generations of qcom SMMU implementation that bear the same name behaved anywhere
near consistent, there would be no need for keeping this around, instead requiring only
"qcom,broken-smmu" or something".

Excuse me for bumping this thread, I successfully forgot this message in drafts folder.

Granted the driver, your pending smmu patches and the current list of quirks (which includes qcom,msm8996-smmu-v2 and qcom,sdm845-smmu-500, and Konrad's unmerged quirks for -v2), I'd second the suggestion of adding a single qcom,msm-smmu-500 (or qcom,arm-smmu-500) entry as a generic fallback. Downstream device trees support this name by using the -v500 suffix (yes, it's a light argument, but still). This way we can apply SoC-specific quirks, while still retaining the generic entry here.

What's definitely has to be done is the merge of the platform data from arm-smmu-qcom-debug.c into arm-smmu-qcom.c. And having identical entries there just drives me crazy. I'll work on a patch later today.

For -v2 we already have the generic compat, however granted the lack of proper quirks in place, I'd abstain from reworking this part for now.

BTW: I'm trying to follow the commit a242f4297cfe ("iommu/arm-smmu-qcom: Skip the TTBR1 quirk for db820c."). It adds msm8996-specific quirk, but we lack the msm8996 compat entry in the array. Does this work at all?


Konrad

Best regards,
Krzysztof


--
With best wishes
Dmitry




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux