Re: [PATCH v10 3/3] iommu/exynos: Add iommu driver for Exynos Platforms
On Wed, Mar 7, 2012 at 2:28 PM, InKi Dae wrote:
>> +static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
>> +{
>> + /* SYSMMU is in blocked when interrupt occurred. */
>> + struct sysmmu_drvdata *data = dev_id;
>> + struct resource *irqres;
>> + struct platform_device *pdev;
>> + enum EXYNOS_SYSMMU_INTERRUPT_TYPE itype;
>
> how about to change EXYNOS_SYSMMU_INTERRUPT_TYPE to small letter? just
> for code clean.
>
I will. please refer to the my reply to Kyungmin Park.
>> + unsigned long addr = -1;
>> +
>> + int i, ret = -ENOSYS;
>> +
>> + read_lock(&data->lock);
>> +
>> + WARN_ON(!is_sysmmu_active(data));
>> +
>> + pdev = to_platform_device(data->sysmmu);
>> + for (i = 0; i < pdev->num_resources; i++) {
>> + irqres = platform_get_resource(pdev, IORESOURCE_IRQ, i);
>> + if (irqres && ((int)irqres->start == irq))
>> + break;
>> + }
>
> is there any reason that it should get irq resources in interrupt
> handler? if not so then how about to move it into probe()?
>
The platform device of ISP has a lot of IRQ sources.
The above for clause determines which IRQ source (System MMU)
generated the interrupt.
>> +
>> + if (i == pdev->num_resources) {
>> + itype = SYSMMU_FAULT_UNKNOWN;
>> + } else {
>> + i /= 2;
>> +
>> + itype = (enum EXYNOS_SYSMMU_INTERRUPT_TYPE)
>
> ditto.
>
Thank you.
Cho KyongHo.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 3/3] iommu/exynos: Add iommu driver for Exynos Platforms
> +static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
> +{
> + /* SYSMMU is in blocked when interrupt occurred. */
> + struct sysmmu_drvdata *data = dev_id;
> + struct resource *irqres;
> + struct platform_device *pdev;
> + enum EXYNOS_SYSMMU_INTERRUPT_TYPE itype;
how about to change EXYNOS_SYSMMU_INTERRUPT_TYPE to small letter? just
for code clean.
> + unsigned long addr = -1;
> +
> + int i, ret = -ENOSYS;
> +
> + read_lock(&data->lock);
> +
> + WARN_ON(!is_sysmmu_active(data));
> +
> + pdev = to_platform_device(data->sysmmu);
> + for (i = 0; i < pdev->num_resources; i++) {
> + irqres = platform_get_resource(pdev, IORESOURCE_IRQ, i);
> + if (irqres && ((int)irqres->start == irq))
> + break;
> + }
is there any reason that it should get irq resources in interrupt
handler? if not so then how about to move it into probe()?
> +
> + if (i == pdev->num_resources) {
> + itype = SYSMMU_FAULT_UNKNOWN;
> + } else {
> + i /= 2;
> +
> + itype = (enum EXYNOS_SYSMMU_INTERRUPT_TYPE)
ditto.
> + __ffs(__raw_readl(data->sfrbases[i] +
> REG_INT_STATUS));
> + if (WARN_ON(!((itype >= 0) && (itype <
> SYSMMU_FAULT_UNKNOWN
> + itype = SYSMMU_FAULT_UNKNOWN;
> + else
> + addr = __raw_readl(
> + data->sfrbases[i] + fault_reg_offset[itype]);
> + }
> +
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 3/3] iommu/exynos: Add iommu driver for Exynos Platforms
On Tue, Mar 6, 2012 at 10:13 PM, Kyungmin Park wrote:
> On Tue, Mar 6, 2012 at 5:31 PM, KyongHo Cho wrote:
>> HAALgBjAGgAbwBAAHMAYQBtAHMAdQBuAGcALgBjAG8AbQA=;Tue,
>> 06 Mar 2012 08:31:29
>> GMT;WwBQAEEAVABDAEgAIAB2ADEAMAAgADMALwAzAF0AIABpAG8AbQBtAHUALwBlAHgAeQBuAG8AcwA6ACAAQQBkAGQAIABpAG8AbQBtAHUAIABkAHIAaQB2AGUAcgAgAGYAbwByACAARQB4AHkAbgBvAHMAIABQAGwAYQB0AGYAbwByAG0AcwA=
>> x-cr-puzzleid: {CF0AAF04-8639-4D69-B4E1-BF71EA1B0A70}
> What's this?
I think it is inserted by Outlook and it was not found in the email editor
before sending this patch. I am unable to use sendmail due to security
reason in my company.
I will find a way not to insert the above message. Thank you.
>>
>> This is the System MMU driver and IOMMU API implementation for
>> Exynos SOC platforms. Exynos platforms has more than 10 System
>> MMUs dedicated for each multimedia accellerators.
>>
>> The System MMU driver is already in arc/arm/plat-s5p but it is
>> moved to drivers/iommu due to Ohad Ben-Cohen gathered IOMMU drivers
>> there
>>
>> Any device driver in Exynos platforms that needs to control its
>> System MMU must call platform_set_sysmmu() to inform System MMU
>> driver who will control it.
>> platform_set_sysmmu() is defined in
> Does it really required? if we can remove it. it can be compiled
> without arch dependency.
The function must be called while board initialization and all source files for
the purpose is placed in 'mach' directory. Thus device driver is not dependent
on arch except the definition of data structure conveyed in
device.platform_data.
Since our System MMUs are attached to each peripheral devices, it is required
to define relationships between System MMUs and peripheral devices.
platform_set_sysmmu() defines the relationship.
>> +
>> +#ifdef CONFIG_EXYNOS_IOMMU_DEBUG
>> +#define DEBUG
>> +#endif
> If it's used only at here. you can add it at Makefile.
>
> ifeq ($(CONFIG_EXYNOS_IOMMU_DEBUG),y)
> CFLAGS-exynos-iommu += -DDEBUG
> endif
>
> Of course you can select as your preference.
Thank you for advice.
>> +
>> +#define lv2ent_fault(pent) ((*(pent) & 3) == 0)
>> +#define lv2ent_small(pent) ((*(pent) & 2) == 2)
> #define lv2ent_small(pent) ((*(pent) & 3) == 2)?
The last bit is 'don't-care'.
It is intentionally ignored in the above comparison.
>> +
>> +#define REG_MMU_CTRL 0x000
>> +#define REG_MMU_CFG 0x004
>> +#define REG_MMU_STATUS 0x008
>> +#define REG_MMU_FLUSH 0x00C
>> +#define REG_MMU_FLUSH_ENTRY 0x010
>> +#define REG_PT_BASE_ADDR 0x014
>> +#define REG_INT_STATUS 0x018
>> +#define REG_INT_CLEAR 0x01C
>> +#define REG_MMU_VERSION 0x034
> Please place it in order.
I found that a mistake while defining the offsets of control registers.
I will fix it.
>> +
>> +enum EXYNOS_SYSMMU_INTERRUPT_TYPE {
>> + SYSMMU_PAGEFAULT,
>> + SYSMMU_AR_MULTIHIT,
>> + SYSMMU_AW_MULTIHIT,
>> + SYSMMU_BUSERROR,
>> + SYSMMU_AR_SECURITY,
>> + SYSMMU_AR_ACCESS,
>> + SYSMMU_AW_SECURITY,
>> + SYSMMU_AW_PROTECTION, /* 7 */
>> + SYSMMU_FAULT_UNKNOWN,
>> + SYSMMU_FAULTS_NUM
> missing comma, "SYSMMU_FAULTS_NUM,"
Yes it is. I don't think that it is needed to appending comma after
the last element
unless it is predictable that adding elements after the last element
in the future.
>> +};
>> +
>> +typedef int (*sysmmu_fault_handler_t)(enum EXYNOS_SYSMMU_INTERRUPT_TYPE
>> itype,
>> + unsigned long pgtable_base, unsigned long
>> fault_addr);
>> +
>> +static unsigned short fault_reg_offset[SYSMMU_FAULTS_NUM] = {
>> + REG_PAGE_FAULT_ADDR,
>> + REG_AR_FAULT_ADDR,
>> + REG_AW_FAULT_ADDR,
>> + REG_DEFAULT_SLAVE_ADDR,
>> + REG_AR_FAULT_ADDR,
>> + REG_AR_FAULT_ADDR,
>> + REG_AW_FAULT_ADDR,
>> + REG_AW_FAULT_ADDR
> ditto.
Answered above.
>> +};
>> +
>> +static char *sysmmu_fault_name[SYSMMU_FAULTS_NUM] = {
>> + "PAGE FAULT",
>> + "AR MULTI-HIT FAULT",
>> + "AW MULTI-HIT FAULT",
>> + "BUS ERROR",
>> + "AR SECURITY PROTECTION FAULT",
>> + "AR ACCESS PROTECTION FAULT",
>> + "AW SECURITY PROTECTION FAULT",
>> + "AW ACCESS PROTECTION FAULT",
>> + "UNKNOWN FAULT"
> ditto.
Answered above.
>> +void exynos_sysmmu_set_prefbuf(struct device *dev,
>> + unsigned long base0, unsigned long size0,
>> + unsigned long base1, unsigned long size1)
>> +{
>> + struct sysmmu_drvdata *data = dev_get_drvdata(dev->archdata.iommu);
>> + unsigned long flags;
>> + int i;
>> +
>> + BUG_ON((base0 + size0) <= base0);
>> + BUG_ON((size1 > 0) && ((base1 + size1) <= base1));
> It's bogus check. it's impossible. Do you think overflow case?
Yes I do.
It is prepared to detect invalid parameters from device drivers.
>> +
>> + read_lock_irqsave(&data->lock, flags);
>> + if (!is_sysmmu_active(data))
>> +
Re: [PATCH v10 3/3] iommu/exynos: Add iommu driver for Exynos Platforms
Hi,
some not critical comments.
Thank you,
Kyungmin Park
On Tue, Mar 6, 2012 at 5:31 PM, KyongHo Cho wrote:
> HAALgBjAGgAbwBAAHMAYQBtAHMAdQBuAGcALgBjAG8AbQA=;Tue,
> 06 Mar 2012 08:31:29
> GMT;WwBQAEEAVABDAEgAIAB2ADEAMAAgADMALwAzAF0AIABpAG8AbQBtAHUALwBlAHgAeQBuAG8AcwA6ACAAQQBkAGQAIABpAG8AbQBtAHUAIABkAHIAaQB2AGUAcgAgAGYAbwByACAARQB4AHkAbgBvAHMAIABQAGwAYQB0AGYAbwByAG0AcwA=
> x-cr-puzzleid: {CF0AAF04-8639-4D69-B4E1-BF71EA1B0A70}
What's this?
>
> This is the System MMU driver and IOMMU API implementation for
> Exynos SOC platforms. Exynos platforms has more than 10 System
> MMUs dedicated for each multimedia accellerators.
>
> The System MMU driver is already in arc/arm/plat-s5p but it is
> moved to drivers/iommu due to Ohad Ben-Cohen gathered IOMMU drivers
> there
>
> Any device driver in Exynos platforms that needs to control its
> System MMU must call platform_set_sysmmu() to inform System MMU
> driver who will control it.
> platform_set_sysmmu() is defined in
Does it really required? if we can remove it. it can be compiled
without arch dependency.
>
> Cc: Joerg Roedel
> Cc: Kukjin Kim
> Signed-off-by: KyongHo Cho
> ---
> drivers/iommu/Kconfig | 21 +
> drivers/iommu/Makefile | 1 +
> drivers/iommu/exynos-iommu.c | 1088
> ++
> 3 files changed, 1110 insertions(+), 0 deletions(-)
> create mode 100644 drivers/iommu/exynos-iommu.c
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 6bea696..25d3eed 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -142,4 +142,25 @@ config OMAP_IOMMU_DEBUG
>
> Say N unless you know you need this.
>
> +config EXYNOS_IOMMU
> + bool "Exynos IOMMU Support"
> + depends on EXYNOS_DEV_SYSMMU
> + select IOMMU_API
> + help
> + Support for the IOMMU(System MMU) of Samsung Exynos application
> + processor family. This enables H/W multimedia accellerators to see
> + non-linear physical memory chunks as a linear memory in their
> + address spaces
> +
> + If unsure, say N here.
> +
> +config EXYNOS_IOMMU_DEBUG
> + bool "Debugging log for Exynos IOMMU"
> + depends on EXYNOS_IOMMU
> + help
> + Select this to see the detailed log message that shows what
> + happens in the IOMMU driver
> +
> + Say N unless you need kernel log message for IOMMU debugging
> +
> endif # IOMMU_SUPPORT
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 0e36b49..5a8fd97 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_IRQ_REMAP) += intr_remapping.o
> obj-$(CONFIG_OMAP_IOMMU) += omap-iommu.o
> obj-$(CONFIG_OMAP_IOVMM) += omap-iovmm.o
> obj-$(CONFIG_OMAP_IOMMU_DEBUG) += omap-iommu-debug.o
> +obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> new file mode 100644
> index 000..4c1c48a
> --- /dev/null
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -0,0 +1,1088 @@
> +/* linux/drivers/iommu/exynos_iommu.c
> + *
> + * Copyright (c) 2011 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifdef CONFIG_EXYNOS_IOMMU_DEBUG
> +#define DEBUG
> +#endif
If it's used only at here. you can add it at Makefile.
ifeq ($(CONFIG_EXYNOS_IOMMU_DEBUG),y)
CFLAGS-exynos-iommu+= -DDEBUG
endif
Of course you can select as your preference.
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +#include
> +
> +#define S5P_MMU_CTRL 0x000
> +#define S5P_MMU_CFG 0x004
> +#define S5P_MMU_STATUS 0x008
> +#define S5P_MMU_FLUSH 0x00C
> +#define S5P_PT_BASE_ADDR 0x014
> +#define S5P_INT_STATUS 0x018
> +#define S5P_INT_CLEAR 0x01C
> +#define S5P_PAGE_FAULT_ADDR 0x024
> +#define S5P_AW_FAULT_ADDR 0x028
> +#define S5P_AR_FAULT_ADDR 0x02C
> +#define S5P_DEFAULT_SLAVE_ADDR 0x030
> +#define S5P_MMU_VERSION 0x034
> +#define S5P_PB0_SADDR 0x04C
> +#define S5P_PB0_EADDR 0x050
> +#define S5P_PB1_SADDR 0x054
> +#define S5P_PB1_EADDR 0x058
> +
> +#define SECT_ORDER 20
> +#define LPAGE_ORDER 16
> +#define SPAGE_ORDER 12
> +
> +#define SECT_SIZE (1 << SECT_ORDER)
> +#define LPAGE_SIZE (1 << LPAGE_ORDER)
> +#define SPAGE_SIZE (1 << SPAGE_ORDER)
> +
> +#define SECT_MASK (~(SECT_SIZE - 1))
> +#define LPAGE_MASK (~(LPAGE_SIZE - 1))
> +#define SPAGE_MASK (~(SPAG
[PATCH v10 3/3] iommu/exynos: Add iommu driver for Exynos Platforms
HAALgBjAGgAbwBAAHMAYQBtAHMAdQBuAGcALgBjAG8AbQA=;Tue,
06 Mar 2012 08:31:29
GMT;WwBQAEEAVABDAEgAIAB2ADEAMAAgADMALwAzAF0AIABpAG8AbQBtAHUALwBlAHgAeQBuAG8AcwA6ACAAQQBkAGQAIABpAG8AbQBtAHUAIABkAHIAaQB2AGUAcgAgAGYAbwByACAARQB4AHkAbgBvAHMAIABQAGwAYQB0AGYAbwByAG0AcwA=
x-cr-puzzleid: {CF0AAF04-8639-4D69-B4E1-BF71EA1B0A70}
This is the System MMU driver and IOMMU API implementation for
Exynos SOC platforms. Exynos platforms has more than 10 System
MMUs dedicated for each multimedia accellerators.
The System MMU driver is already in arc/arm/plat-s5p but it is
moved to drivers/iommu due to Ohad Ben-Cohen gathered IOMMU drivers
there
Any device driver in Exynos platforms that needs to control its
System MMU must call platform_set_sysmmu() to inform System MMU
driver who will control it.
platform_set_sysmmu() is defined in
Cc: Joerg Roedel
Cc: Kukjin Kim
Signed-off-by: KyongHo Cho
---
drivers/iommu/Kconfig| 21 +
drivers/iommu/Makefile |1 +
drivers/iommu/exynos-iommu.c | 1088 ++
3 files changed, 1110 insertions(+), 0 deletions(-)
create mode 100644 drivers/iommu/exynos-iommu.c
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 6bea696..25d3eed 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -142,4 +142,25 @@ config OMAP_IOMMU_DEBUG
Say N unless you know you need this.
+config EXYNOS_IOMMU
+ bool "Exynos IOMMU Support"
+ depends on EXYNOS_DEV_SYSMMU
+ select IOMMU_API
+ help
+ Support for the IOMMU(System MMU) of Samsung Exynos application
+ processor family. This enables H/W multimedia accellerators to see
+ non-linear physical memory chunks as a linear memory in their
+ address spaces
+
+ If unsure, say N here.
+
+config EXYNOS_IOMMU_DEBUG
+ bool "Debugging log for Exynos IOMMU"
+ depends on EXYNOS_IOMMU
+ help
+ Select this to see the detailed log message that shows what
+ happens in the IOMMU driver
+
+ Say N unless you need kernel log message for IOMMU debugging
+
endif # IOMMU_SUPPORT
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 0e36b49..5a8fd97 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_IRQ_REMAP) += intr_remapping.o
obj-$(CONFIG_OMAP_IOMMU) += omap-iommu.o
obj-$(CONFIG_OMAP_IOVMM) += omap-iovmm.o
obj-$(CONFIG_OMAP_IOMMU_DEBUG) += omap-iommu-debug.o
+obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
new file mode 100644
index 000..4c1c48a
--- /dev/null
+++ b/drivers/iommu/exynos-iommu.c
@@ -0,0 +1,1088 @@
+/* linux/drivers/iommu/exynos_iommu.c
+ *
+ * Copyright (c) 2011 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifdef CONFIG_EXYNOS_IOMMU_DEBUG
+#define DEBUG
+#endif
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#include
+
+#define S5P_MMU_CTRL 0x000
+#define S5P_MMU_CFG0x004
+#define S5P_MMU_STATUS 0x008
+#define S5P_MMU_FLUSH 0x00C
+#define S5P_PT_BASE_ADDR 0x014
+#define S5P_INT_STATUS 0x018
+#define S5P_INT_CLEAR 0x01C
+#define S5P_PAGE_FAULT_ADDR0x024
+#define S5P_AW_FAULT_ADDR 0x028
+#define S5P_AR_FAULT_ADDR 0x02C
+#define S5P_DEFAULT_SLAVE_ADDR 0x030
+#define S5P_MMU_VERSION0x034
+#define S5P_PB0_SADDR 0x04C
+#define S5P_PB0_EADDR 0x050
+#define S5P_PB1_SADDR 0x054
+#define S5P_PB1_EADDR 0x058
+
+#define SECT_ORDER 20
+#define LPAGE_ORDER 16
+#define SPAGE_ORDER 12
+
+#define SECT_SIZE (1 << SECT_ORDER)
+#define LPAGE_SIZE (1 << LPAGE_ORDER)
+#define SPAGE_SIZE (1 << SPAGE_ORDER)
+
+#define SECT_MASK (~(SECT_SIZE - 1))
+#define LPAGE_MASK (~(LPAGE_SIZE - 1))
+#define SPAGE_MASK (~(SPAGE_SIZE - 1))
+
+#define lv1ent_fault(sent) (((*(sent) & 3) == 0) || ((*(sent) & 3) == 3))
+#define lv1ent_page(sent) ((*(sent) & 3) == 1)
+#define lv1ent_section(sent) ((*(sent) & 3) == 2)
+
+#define lv2ent_fault(pent) ((*(pent) & 3) == 0)
+#define lv2ent_small(pent) ((*(pent) & 2) == 2)
+#define lv2ent_large(pent) ((*(pent) & 3) == 1)
+
+#define section_phys(sent) (*(sent) & SECT_MASK)
+#define section_offs(iova) ((iova) & 0xF)
+#define lpage_phys(pent) (*(pent) & LPAGE_MASK)
+#define lpage_offs(iova) ((iova) & 0x)
+#define spage_phys(pent) (*(pent) & SPAGE_MASK)
+#define spage_offs(iova) ((iova) & 0xFFF)
+
+#define lv1ent_offset(iova) ((iova) >> SECT_
