[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <65e3d97e-4a24-4696-9bca-c3f7833144d2@samsung.com>
Date: Tue, 16 Sep 2025 08:36:29 +0200
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: "myunggeun.ji" <myunggeun.ji@...sung.com>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Alim Akhtar <alim.akhtar@...sung.com>, Joerg Roedel
<joro@...tes.org>, Will Deacon <will@...nel.org>, Robin Murphy
<robin.murphy@....com>, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org,
linux-kernel@...r.kernel.org, iommu@...ts.linux.dev
Cc: Jongho Park <jongho0910.park@...sung.com>, kiisung lee
<kiisung.lee@...sung.com>
Subject: Re: [PATCH 1/2] iommu/exynos: Implement register set and fault
handling on SysMMU v9
On 15.09.2025 07:13, myunggeun.ji wrote:
> SysMMU v9 has a bit different registers.
> - Major and Minor version BIT are changed to BIT[31:28] and BIT[27:24]
> - FLPT(First Level Page Table) offset is changed.
> - interrupt status register has different bits w.r.t. previous SysMMU
> versions
>
> Add correct register set and fault handling for SysMMU v9,
> according to all mentioned differences.
>
> Signed-off-by: myunggeun.ji <myunggeun.ji@...sung.com>
> ---
> drivers/iommu/exynos-iommu.c | 73 +++++++++++++++++++++++++++++++-----
> 1 file changed, 64 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index b6edd178fe25..00f4129a7bf2 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -152,7 +152,9 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
>
> #define MMU_MAJ_VER(val) ((val) >> 7)
> #define MMU_MIN_VER(val) ((val) & 0x7F)
> -#define MMU_RAW_VER(reg) (((reg) >> 21) & ((1 << 11) - 1)) /* 11 bits */
> +#define MMU_RAW_VER(reg) (((reg) >> 28 < 7) ? \
> + (((reg) >> 21) & ((1 << 11) - 1)) : \
> + (((reg) >> 24) & ((1 << 8) - 1)))
>
> #define MAKE_MMU_VER(maj, min) ((((maj) & 0xF) << 7) | ((min) & 0x7F))
>
> @@ -171,6 +173,17 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
> #define REG_V7_CAPA1 0x874
> #define REG_V7_CTRL_VM 0x8000
>
> +/* v9.x registers */
> +#define REG_V9_CTRL_VM 0x8000
> +#define REG_MMU_CONTEXT0_CFG_ATTRIBUTE_VM 0x8408
> +
> +#define MMU_MAJ_VER_V9(val) ((val) >> 4)
> +#define MMU_MIN_VER_V9(val) ((val) & 0xF)
> +#define MMU_RAW_VER_V9(reg) (((reg) >> 24) & ((1 << 8) - 1)) /* 8 bits */
> +
> +#define MAKE_MMU_VER_V9(maj, min) ((((maj) & 0xF) << 7) | ((min) & 0xF))
> +#define MAKE_MMU_VM_OFFSET(vid) ((vid) * 0x1000)
> +
> #define has_sysmmu(dev) (dev_iommu_priv_get(dev) != NULL)
>
> static struct device *dma_dev;
> @@ -228,6 +241,14 @@ static const char * const sysmmu_v7_fault_names[] = {
> "RESERVED"
> };
>
> +static const char * const sysmmu_v9_fault_names[] = {
> + "PTW",
> + "PAGE",
> + "ACCESS PROTECTION",
> + "CONTEXT_FAULT",
> + "RESERVED"
> +};
> +
> /*
> * This structure is attached to dev->iommu->priv of the master device
> * on device add, contains a list of SYSMMU controllers defined by device tree,
> @@ -363,6 +384,19 @@ static int exynos_sysmmu_v7_get_fault_info(struct sysmmu_drvdata *data,
> return 0;
> }
>
> +static int exynos_sysmmu_v9_get_fault_info(struct sysmmu_drvdata *data,
> + unsigned int itype,
> + struct sysmmu_fault *fault)
> +{
> + u32 info = readl(SYSMMU_REG(data, fault_info));
> +
> + fault->addr = readl(SYSMMU_REG(data, fault_va));
> + fault->name = sysmmu_v9_fault_names[itype % 5];
> + fault->type = (info & BIT(20)) ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ;
> +
> + return 0;
> +}
> +
> /* SysMMU v1..v3 */
> static const struct sysmmu_variant sysmmu_v1_variant = {
> .flush_all = 0x0c,
> @@ -420,6 +454,21 @@ static const struct sysmmu_variant sysmmu_v7_vm_variant = {
> .get_fault_info = exynos_sysmmu_v7_get_fault_info,
> };
>
> +/* SysMMU v9: VM capable register layout */
> +static const struct sysmmu_variant sysmmu_v9_vm_variant = {
> + .pt_base = 0x8404,
> + .flush_all = 0x8010,
> + .flush_entry = 0x8014,
> + .flush_start = 0x8020,
> + .flush_end = 0x8024,
> + .int_status = 0x8060,
> + .int_clear = 0x8064,
> + .fault_va = 0x8070,
> + .fault_info = 0x8074,
> +
> + .get_fault_info = exynos_sysmmu_v9_get_fault_info,
> +};
> +
> static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom)
> {
> return container_of(dom, struct exynos_iommu_domain, domain);
> @@ -522,19 +571,26 @@ static void __sysmmu_get_version(struct sysmmu_drvdata *data)
> ver = readl(data->sfrbase + REG_MMU_VERSION);
>
> /* controllers on some SoCs don't report proper version */
> +
> if (ver == 0x80000001u)
> data->version = MAKE_MMU_VER(1, 0);
> else
> data->version = MMU_RAW_VER(ver);
>
> - dev_dbg(data->sysmmu, "hardware version: %d.%d\n",
> - MMU_MAJ_VER(data->version), MMU_MIN_VER(data->version));
> + if (data->version != 0x91)
> + dev_err(data->sysmmu, "hardware version: %d.%d\n",
> + MMU_MAJ_VER(data->version), MMU_MIN_VER(data->version));
> + else if (data->version == 0x91)
> + dev_err(data->sysmmu, "hardware version: %d.%d\n",
> + MMU_MAJ_VER_V9(data->version), MMU_MIN_VER_V9(data->version));
Sorry, this starts looking over-engineered. Simply split data->version
into data->major_version and data->minor_version and fill them with
proper values. There is no point storing them together differently for
each hw version and then decoding in each use.
>
> - if (MMU_MAJ_VER(data->version) < 5) {
> + if (data->version == 0x91) {
> + data->variant = &sysmmu_v9_vm_variant;
> + } else if (MMU_MAJ_VER(data->version) < 5) {
> data->variant = &sysmmu_v1_variant;
> } else if (MMU_MAJ_VER(data->version) < 7) {
> data->variant = &sysmmu_v5_variant;
> - } else {
> + } else if (MMU_MAJ_VER(data->version) < 9) {
> if (__sysmmu_has_capa1(data))
> __sysmmu_get_vcr(data);
> if (data->has_vcr)
> @@ -763,10 +819,9 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
> if (IS_ERR(data->pclk))
> return PTR_ERR(data->pclk);
>
> - if (!data->clk && (!data->aclk || !data->pclk)) {
> - dev_err(dev, "Failed to get device clock(s)!\n");
> - return -ENOSYS;
> - }
> + /* There is no clock information after v9 */
> + if (!data->clk && (!data->aclk || !data->pclk))
> + dev_warn(dev, "Failed to get device clock(s)!\n");
If there is really no separate clock to control, then check that
explicitly instead of printing a misleading warning.
> data->clk_master = devm_clk_get_optional(dev, "master");
> if (IS_ERR(data->clk_master))
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Powered by blists - more mailing lists