On 5/25/20 7:17 AM, Sai Prakash Ranjan wrote:
Hi,
On 2020-05-25 16:38, Jonathan Marek wrote:
On 5/25/20 6:54 AM, Sai Prakash Ranjan wrote:
On 2020-05-25 15:39, Jonathan Marek wrote:
Hi,
On 5/25/20 5:42 AM, Sai Prakash Ranjan wrote:
Hi Jonathan,
On 2020-05-24 08:08, Jonathan Marek wrote:
Add the apps_smmu node for sm8250. Note that adding the iommus
field for
UFS is required because initializing the iommu removes the bypass
mapping
that created by the bootloader.
This statement doesn't seem right, you can just say since the
bypass is disabled
by default now, we need to add this property to enable translation
and avoid global faults.
If I use this patch [1] then the UFS iommu property isn't needed. In
downstream, the identity (bypass?) stream mapping is inherited from
the bootloader, and UFS is used without any iommu property. Setting
ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n doesn't make it work on its own
(without the UFS iommu property), so there's more to it than just
"bypass is disabled by default now".
https://patchwork.kernel.org/patch/11310757/
"iommus" property is not about inheriting stream mapping from
bootloader,
it is used to enable SMMU address translation for the corresponding
master when specified. So when you have disabled bypass, i.e.,
ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y or via cmdline
"arm-smmu.disable_bypass=1"
and iommus property with SID and mask is not specified, then it will
result
in SMMU global faults.
Downstream has bypass
enabled(ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n),so you
won't see any global faults if you do not have iommus property.
Patch in your link is for display because of the usecase for splash
screen
on android and some other devices where the bootloader will configure
SMMU,
it has not yet merged and not likely to get merged in the current state.
So yes "there is *not* much more to it than bypass is disabled by
default now"
and you have to specify "iommus" for the master devices or you should
enable bypass,
i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or arm-smmu.disable_bypass=n
Try without the patch in the link and without iommus for UFS and
ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y and you will see.
-Sai
I know that the "iommus" property is not about inheriting stream
mapping. Probing the iommu removes the stream mapping created by the
bootloader, the iommus property is added so that new mappings are
created to replace what was removed.
You seem to be under the impression that the SM8150/SM8250 bootloader
does not configure SMMU. It does, for both UFS and SDHC, just like it
does for display/splash screen on some devices.
It could be that bootloader does configure SMMU for UFS and SDHC, but the
upstream SMMU driver doesnt allow to inherit stream mapping from the
bootloader
yet, so adding iommus property based on the assumption that it is
inherited seems
wrong.
I never said adding the iommus property is for inheriting stream
mapping. I mentioned inheriting to say UFS works without the iommus
property on downstream (it inherits a identity/bypass mapping).
With either value of ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT, it will not
work without the iommus property.
I'm pretty sure that if you have ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n and
without iommus, it should work.
It doesn't work, with either ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or
ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y.
-Sai
