Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
From: ext Russell King - ARM Linux
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Sat, 16 May 2009 11:55:14 +0200
> On Sat, May 16, 2009 at 10:20:36AM +0100, Russell King - ARM Linux wrote:
> > This looks all very convoluted. Why not do something like:
> >
> > static unsigned long iommu_base[] = {
> > OMAP3_MMU1_BASE,
> > OMAP3_MMU2_BASE,
> > };
> >
> > static int iommu_irq[] = {
> > OMAP3_MMU1_IRQ,
> > OMAP3_MMU2_IRQ,
> > };
>
> BTW, both of these can be __initdata.
Attached the updated one.
>From 2bfdb4130433708dc04e1c30aa5099d48de408e0 Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU
Date: Wed, 28 Jan 2009 21:32:04 +0200
Subject: [PATCH 3/6] omap iommu: omap3 iommu device registration
Signed-off-by: Hiroshi DOYU
---
arch/arm/mach-omap2/omap3-iommu.c | 103 +
1 files changed, 103 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
diff --git a/arch/arm/mach-omap2/omap3-iommu.c
b/arch/arm/mach-omap2/omap3-iommu.c
new file mode 100644
index 000..91ee38a
--- /dev/null
+++ b/arch/arm/mach-omap2/omap3-iommu.c
@@ -0,0 +1,103 @@
+/*
+ * omap iommu: omap3 device registration
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU
+ *
+ * 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.
+ */
+
+#include
+
+#include
+
+#define OMAP3_MMU1_BASE0x480bd400
+#define OMAP3_MMU2_BASE0x5d00
+#define OMAP3_MMU1_IRQ 24
+#define OMAP3_MMU2_IRQ 28
+
+
+static unsigned long iommu_base[] __initdata = {
+ OMAP3_MMU1_BASE,
+ OMAP3_MMU2_BASE,
+};
+
+static int iommu_irq[] __initdata = {
+ OMAP3_MMU1_IRQ,
+ OMAP3_MMU2_IRQ,
+};
+
+static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
+ {
+ .name = "isp",
+ .nr_tlb_entries = 8,
+ .clk_name = "cam_ick",
+ },
+ {
+ .name = "iva2",
+ .nr_tlb_entries = 32,
+ .clk_name = "iva2_ck",
+ },
+};
+#define NR_IOMMU_DEVICES ARRAY_SIZE(omap3_iommu_pdata)
+
+static struct platform_device *omap3_iommu_pdev[NR_IOMMU_DEVICES];
+
+static int __init omap3_iommu_init(void)
+{
+ int i, err;
+
+ for (i = 0; i < NR_IOMMU_DEVICES; i++) {
+ struct platform_device *pdev;
+ struct resource res[2];
+
+ pdev = platform_device_alloc("omap-iommu", i);
+ if (!pdev) {
+ err = -ENOMEM;
+ goto err_out;
+ }
+
+ memset(res, 0, sizeof(res));
+ res[0].start = iommu_base[i];
+ res[0].end = iommu_base[i] + MMU_REG_SIZE - 1;
+ res[0].flags = IORESOURCE_MEM;
+ res[1].start = res[1].end = iommu_irq[i];
+ res[1].flags = IORESOURCE_IRQ;
+
+ err = platform_device_add_resources(pdev, res,
+ ARRAY_SIZE(res));
+ if (err)
+ goto err_out;
+ err = platform_device_add_data(pdev, &omap3_iommu_pdata[i],
+ sizeof(omap3_iommu_pdata[0]));
+ if (err)
+ goto err_out;
+ err = platform_device_add(pdev);
+ if (err)
+ goto err_out;
+ omap3_iommu_pdev[i] = pdev;
+ }
+ return 0;
+
+err_out:
+ while (i--)
+ platform_device_put(omap3_iommu_pdev[i]);
+ return err;
+}
+module_init(omap3_iommu_init);
+
+static void __exit omap3_iommu_exit(void)
+{
+ int i;
+
+ for (i = 0; i < NR_IOMMU_DEVICES; i++)
+ platform_device_unregister(omap3_iommu_pdev[i]);
+}
+module_exit(omap3_iommu_exit);
+
+MODULE_AUTHOR("Hiroshi DOYU");
+MODULE_DESCRIPTION("omap iommu: omap3 device registration");
+MODULE_LICENSE("GPL v2");
--
1.6.0.4
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
From: ext Russell King - ARM Linux
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Sat, 16 May 2009 11:55:14 +0200
> On Sat, May 16, 2009 at 10:20:36AM +0100, Russell King - ARM Linux wrote:
> > This looks all very convoluted. Why not do something like:
> >
> > static unsigned long iommu_base[] = {
> > OMAP3_MMU1_BASE,
> > OMAP3_MMU2_BASE,
> > };
> >
> > static int iommu_irq[] = {
> > OMAP3_MMU1_IRQ,
> > OMAP3_MMU2_IRQ,
> > };
>
> BTW, both of these can be __initdata.
Attached the updated one.
>From 2bfdb4130433708dc04e1c30aa5099d48de408e0 Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU
Date: Wed, 28 Jan 2009 21:32:04 +0200
Subject: [PATCH 3/6] omap iommu: omap3 iommu device registration
Signed-off-by: Hiroshi DOYU
---
arch/arm/mach-omap2/omap3-iommu.c | 103 +
1 files changed, 103 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
diff --git a/arch/arm/mach-omap2/omap3-iommu.c
b/arch/arm/mach-omap2/omap3-iommu.c
new file mode 100644
index 000..91ee38a
--- /dev/null
+++ b/arch/arm/mach-omap2/omap3-iommu.c
@@ -0,0 +1,103 @@
+/*
+ * omap iommu: omap3 device registration
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU
+ *
+ * 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.
+ */
+
+#include
+
+#include
+
+#define OMAP3_MMU1_BASE0x480bd400
+#define OMAP3_MMU2_BASE0x5d00
+#define OMAP3_MMU1_IRQ 24
+#define OMAP3_MMU2_IRQ 28
+
+
+static unsigned long iommu_base[] __initdata = {
+ OMAP3_MMU1_BASE,
+ OMAP3_MMU2_BASE,
+};
+
+static int iommu_irq[] __initdata = {
+ OMAP3_MMU1_IRQ,
+ OMAP3_MMU2_IRQ,
+};
+
+static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
+ {
+ .name = "isp",
+ .nr_tlb_entries = 8,
+ .clk_name = "cam_ick",
+ },
+ {
+ .name = "iva2",
+ .nr_tlb_entries = 32,
+ .clk_name = "iva2_ck",
+ },
+};
+#define NR_IOMMU_DEVICES ARRAY_SIZE(omap3_iommu_pdata)
+
+static struct platform_device *omap3_iommu_pdev[NR_IOMMU_DEVICES];
+
+static int __init omap3_iommu_init(void)
+{
+ int i, err;
+
+ for (i = 0; i < NR_IOMMU_DEVICES; i++) {
+ struct platform_device *pdev;
+ struct resource res[2];
+
+ pdev = platform_device_alloc("omap-iommu", i);
+ if (!pdev) {
+ err = -ENOMEM;
+ goto err_out;
+ }
+
+ memset(res, 0, sizeof(res));
+ res[0].start = iommu_base[i];
+ res[0].end = iommu_base[i] + MMU_REG_SIZE - 1;
+ res[0].flags = IORESOURCE_MEM;
+ res[1].start = res[1].end = iommu_irq[i];
+ res[1].flags = IORESOURCE_IRQ;
+
+ err = platform_device_add_resources(pdev, res,
+ ARRAY_SIZE(res));
+ if (err)
+ goto err_out;
+ err = platform_device_add_data(pdev, &omap3_iommu_pdata[i],
+ sizeof(omap3_iommu_pdata[0]));
+ if (err)
+ goto err_out;
+ err = platform_device_add(pdev);
+ if (err)
+ goto err_out;
+ omap3_iommu_pdev[i] = pdev;
+ }
+ return 0;
+
+err_out:
+ while (i--)
+ platform_device_put(omap3_iommu_pdev[i]);
+ return err;
+}
+module_init(omap3_iommu_init);
+
+static void __exit omap3_iommu_exit(void)
+{
+ int i;
+
+ for (i = 0; i < NR_IOMMU_DEVICES; i++)
+ platform_device_unregister(omap3_iommu_pdev[i]);
+}
+module_exit(omap3_iommu_exit);
+
+MODULE_AUTHOR("Hiroshi DOYU");
+MODULE_DESCRIPTION("omap iommu: omap3 device registration");
+MODULE_LICENSE("GPL v2");
--
1.6.0.4
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
From: ext Russell King - ARM Linux
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Sat, 16 May 2009 11:55:14 +0200
> On Sat, May 16, 2009 at 10:20:36AM +0100, Russell King - ARM Linux wrote:
> > This looks all very convoluted. Why not do something like:
> >
> > static unsigned long iommu_base[] = {
> > OMAP3_MMU1_BASE,
> > OMAP3_MMU2_BASE,
> > };
> >
> > static int iommu_irq[] = {
> > OMAP3_MMU1_IRQ,
> > OMAP3_MMU2_IRQ,
> > };
>
> BTW, both of these can be __initdata.
OK, I'll send the update ones. Thank you for reviewing them.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Hi Russell,
From: ext Russell King - ARM Linux
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Sat, 16 May 2009 11:20:36 +0200
> On Tue, May 05, 2009 at 03:47:00PM +0300, Hiroshi DOYU wrote:
> > +static struct resource omap3_iommu_res[] = {
> > + { /* Camera ISP MMU */
> > + .start = OMAP3_MMU1_BASE,
> > + .end= OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> > + .flags = IORESOURCE_MEM,
> > + },
> > + {
> > + .start = OMAP3_MMU1_IRQ,
> > + .flags = IORESOURCE_IRQ,
> > + },
> > + { /* IVA2.2 MMU */
> > + .start = OMAP3_MMU2_BASE,
> > + .end= OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> > + .flags = IORESOURCE_MEM,
> > + },
> > + {
> > + .start = OMAP3_MMU2_IRQ,
> > + .flags = IORESOURCE_IRQ,
> > + },
> > +};
> > +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
>
> This looks all very convoluted. Why not do something like:
>
> static unsigned long iommu_base[] = {
> OMAP3_MMU1_BASE,
> OMAP3_MMU2_BASE,
> };
>
> static int iommu_irq[] = {
> OMAP3_MMU1_IRQ,
> OMAP3_MMU2_IRQ,
> };
>
> > +static struct platform_device *omap3_iommu_pdev[NR_IOMMU_DEVICES];
> > +
> > +static int __init omap3_iommu_init(void)
> > +{
> > + int i, err;
> > +
> > + for (i = 0; i < NR_IOMMU_DEVICES; i++) {
> > + struct platform_device *pdev;
> > +
> > + pdev = platform_device_alloc("omap-iommu", i + 1);
>
> Why number them 1 and up when zero is perfectly fine?
I just wanted to use the same expression in TRM. Actually no need to
start from 1.
>
> > + if (!pdev) {
> > + err = -ENOMEM;
> > + goto err_out;
> > + }
> > + err = platform_device_add_resources(pdev,
> > + &omap3_iommu_res[2 * i], NR_IOMMU_RES);
>
> struct resource res[2];
>
> res[0].start = iommu_base[i];
> res[0].end = iommu_base[i] + MMU_REG_SIZE - 1;
> res[0].flags = IORESOURCE_MEM;
> res[1].start = res[1].end = iommu_irq[i];
> res[1].flags = IORESOURCE_IRQ;
>
> err = platform_device_add_resources(pdev, &res,
> ARRAY_SIZE(res));
>
> There's no need to keep 'res' around since it's copied by
> add_resources.
OK.
>
> > + if (err)
> > + goto err_out;
> > + err = platform_device_add_data(pdev, &omap3_iommu_pdata[i],
> > + sizeof(omap3_iommu_pdata[0]));
> > + if (err)
> > + goto err_out;
> > + err = platform_device_add(pdev);
> > + if (err)
> > + goto err_out;
> > + omap3_iommu_pdev[i] = pdev;
> > + }
> > + return 0;
> > +
> > +err_out:
> > + while (i--)
> > + platform_device_put(omap3_iommu_pdev[i]);
> > + return err;
> > +}
> > +module_init(omap3_iommu_init);
> > +
> > +static void __exit omap3_iommu_exit(void)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < NR_IOMMU_DEVICES; i++)
> > + platform_device_unregister(omap3_iommu_pdev[i]);
> > +}
> > +module_exit(omap3_iommu_exit);
> > +
> > +MODULE_AUTHOR("Hiroshi DOYU");
> > +MODULE_DESCRIPTION("omap iommu: omap3 device registration");
> > +MODULE_LICENSE("GPL v2");
> >
> >
> > ---
> > List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
> > FAQ:http://www.arm.linux.org.uk/mailinglists/faq.php
> > Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
On Sat, May 16, 2009 at 12:54 PM, Russell King - ARM Linux
wrote:
> On Sat, May 16, 2009 at 12:38:23PM +0300, Felipe Contreras wrote:
>> On Sat, May 16, 2009 at 12:20 PM, Russell King - ARM Linux
>> wrote:
>> > On Tue, May 05, 2009 at 03:47:00PM +0300, Hiroshi DOYU wrote:
>> >> +static struct resource omap3_iommu_res[] = {
>> >> + { /* Camera ISP MMU */
>> >> + .start = OMAP3_MMU1_BASE,
>> >> + .end = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
>> >> + .flags = IORESOURCE_MEM,
>> >> + },
>> >> + {
>> >> + .start = OMAP3_MMU1_IRQ,
>> >> + .flags = IORESOURCE_IRQ,
>> >> + },
>> >> + { /* IVA2.2 MMU */
>> >> + .start = OMAP3_MMU2_BASE,
>> >> + .end = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
>> >> + .flags = IORESOURCE_MEM,
>> >> + },
>> >> + {
>> >> + .start = OMAP3_MMU2_IRQ,
>> >> + .flags = IORESOURCE_IRQ,
>> >> + },
>> >> +};
>> >> +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
>> >
>> > This looks all very convoluted. Why not do something like:
>> >
>> > static unsigned long iommu_base[] = {
>> > OMAP3_MMU1_BASE,
>> > OMAP3_MMU2_BASE,
>> > };
>> >
>> > static int iommu_irq[] = {
>> > OMAP3_MMU1_IRQ,
>> > OMAP3_MMU2_IRQ,
>> > };
>>
>> All your comments are pretty similar to my reorganize patch:
>> http://marc.info/?l=linux-omap&m=124172711303076&w=2
>
> ... which hasn't been posted to linux-arm-kernel, and so I won't see it.
You are not subscribed to linux-omap? I wasn't subscribed to
linux-arm-kernel at that point, I'll resend to there.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
On Sat, May 16, 2009 at 10:20:36AM +0100, Russell King - ARM Linux wrote:
> This looks all very convoluted. Why not do something like:
>
> static unsigned long iommu_base[] = {
> OMAP3_MMU1_BASE,
> OMAP3_MMU2_BASE,
> };
>
> static int iommu_irq[] = {
> OMAP3_MMU1_IRQ,
> OMAP3_MMU2_IRQ,
> };
BTW, both of these can be __initdata.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
On Sat, May 16, 2009 at 12:38:23PM +0300, Felipe Contreras wrote:
> On Sat, May 16, 2009 at 12:20 PM, Russell King - ARM Linux
> wrote:
> > On Tue, May 05, 2009 at 03:47:00PM +0300, Hiroshi DOYU wrote:
> >> +static struct resource omap3_iommu_res[] = {
> >> + { /* Camera ISP MMU */
> >> + .start = OMAP3_MMU1_BASE,
> >> + .end = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> >> + .flags = IORESOURCE_MEM,
> >> + },
> >> + {
> >> + .start = OMAP3_MMU1_IRQ,
> >> + .flags = IORESOURCE_IRQ,
> >> + },
> >> + { /* IVA2.2 MMU */
> >> + .start = OMAP3_MMU2_BASE,
> >> + .end = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> >> + .flags = IORESOURCE_MEM,
> >> + },
> >> + {
> >> + .start = OMAP3_MMU2_IRQ,
> >> + .flags = IORESOURCE_IRQ,
> >> + },
> >> +};
> >> +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
> >
> > This looks all very convoluted. Why not do something like:
> >
> > static unsigned long iommu_base[] = {
> > OMAP3_MMU1_BASE,
> > OMAP3_MMU2_BASE,
> > };
> >
> > static int iommu_irq[] = {
> > OMAP3_MMU1_IRQ,
> > OMAP3_MMU2_IRQ,
> > };
>
> All your comments are pretty similar to my reorganize patch:
> http://marc.info/?l=linux-omap&m=124172711303076&w=2
... which hasn't been posted to linux-arm-kernel, and so I won't see it.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
On Sat, May 16, 2009 at 12:20 PM, Russell King - ARM Linux
wrote:
> On Tue, May 05, 2009 at 03:47:00PM +0300, Hiroshi DOYU wrote:
>> +static struct resource omap3_iommu_res[] = {
>> + { /* Camera ISP MMU */
>> + .start = OMAP3_MMU1_BASE,
>> + .end = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
>> + .flags = IORESOURCE_MEM,
>> + },
>> + {
>> + .start = OMAP3_MMU1_IRQ,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + { /* IVA2.2 MMU */
>> + .start = OMAP3_MMU2_BASE,
>> + .end = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
>> + .flags = IORESOURCE_MEM,
>> + },
>> + {
>> + .start = OMAP3_MMU2_IRQ,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> +};
>> +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
>
> This looks all very convoluted. Why not do something like:
>
> static unsigned long iommu_base[] = {
> OMAP3_MMU1_BASE,
> OMAP3_MMU2_BASE,
> };
>
> static int iommu_irq[] = {
> OMAP3_MMU1_IRQ,
> OMAP3_MMU2_IRQ,
> };
All your comments are pretty similar to my reorganize patch:
http://marc.info/?l=linux-omap&m=124172711303076&w=2
--
Felipe Contreras
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
On Tue, May 05, 2009 at 03:47:00PM +0300, Hiroshi DOYU wrote:
> +static struct resource omap3_iommu_res[] = {
> + { /* Camera ISP MMU */
> + .start = OMAP3_MMU1_BASE,
> + .end= OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> + .flags = IORESOURCE_MEM,
> + },
> + {
> + .start = OMAP3_MMU1_IRQ,
> + .flags = IORESOURCE_IRQ,
> + },
> + { /* IVA2.2 MMU */
> + .start = OMAP3_MMU2_BASE,
> + .end= OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> + .flags = IORESOURCE_MEM,
> + },
> + {
> + .start = OMAP3_MMU2_IRQ,
> + .flags = IORESOURCE_IRQ,
> + },
> +};
> +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
This looks all very convoluted. Why not do something like:
static unsigned long iommu_base[] = {
OMAP3_MMU1_BASE,
OMAP3_MMU2_BASE,
};
static int iommu_irq[] = {
OMAP3_MMU1_IRQ,
OMAP3_MMU2_IRQ,
};
> +static struct platform_device *omap3_iommu_pdev[NR_IOMMU_DEVICES];
> +
> +static int __init omap3_iommu_init(void)
> +{
> + int i, err;
> +
> + for (i = 0; i < NR_IOMMU_DEVICES; i++) {
> + struct platform_device *pdev;
> +
> + pdev = platform_device_alloc("omap-iommu", i + 1);
Why number them 1 and up when zero is perfectly fine?
> + if (!pdev) {
> + err = -ENOMEM;
> + goto err_out;
> + }
> + err = platform_device_add_resources(pdev,
> + &omap3_iommu_res[2 * i], NR_IOMMU_RES);
struct resource res[2];
res[0].start = iommu_base[i];
res[0].end = iommu_base[i] + MMU_REG_SIZE - 1;
res[0].flags = IORESOURCE_MEM;
res[1].start = res[1].end = iommu_irq[i];
res[1].flags = IORESOURCE_IRQ;
err = platform_device_add_resources(pdev, &res,
ARRAY_SIZE(res));
There's no need to keep 'res' around since it's copied by add_resources.
> + if (err)
> + goto err_out;
> + err = platform_device_add_data(pdev, &omap3_iommu_pdata[i],
> +sizeof(omap3_iommu_pdata[0]));
> + if (err)
> + goto err_out;
> + err = platform_device_add(pdev);
> + if (err)
> + goto err_out;
> + omap3_iommu_pdev[i] = pdev;
> + }
> + return 0;
> +
> +err_out:
> + while (i--)
> + platform_device_put(omap3_iommu_pdev[i]);
> + return err;
> +}
> +module_init(omap3_iommu_init);
> +
> +static void __exit omap3_iommu_exit(void)
> +{
> + int i;
> +
> + for (i = 0; i < NR_IOMMU_DEVICES; i++)
> + platform_device_unregister(omap3_iommu_pdev[i]);
> +}
> +module_exit(omap3_iommu_exit);
> +
> +MODULE_AUTHOR("Hiroshi DOYU");
> +MODULE_DESCRIPTION("omap iommu: omap3 device registration");
> +MODULE_LICENSE("GPL v2");
>
>
> ---
> List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
> FAQ:http://www.arm.linux.org.uk/mailinglists/faq.php
> Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
On Fri, May 8, 2009 at 8:21 PM, Aguirre Rodriguez, Sergio Alberto
wrote:
> Hi,
>
> Just one comment below:
>
>> > Does the following make sense?
>> >
>> > +#define NR_IOMMU_RES 2
>> >
>> >
>> >
>> > + err = platform_device_add_resources(pdev,
>> > + omap3_iommu_res + i * NR_IOMMU_RES,
>> NR_IOMMU_RES);
>>
>> Yeap, also:
>>
>> > + err = platform_device_add_resources(pdev,
>> omap3_iommu_res + i * 2, 2);
>
> IMHO, I don't think it's a good idea to add magical numbers to any code in
> the kernel. Why is it better to NOT use a define?
I agree, but even a define is not good enough. For example you can
update the array without updating the define.
A better solution is to do what I did on my follow up patch series:
+ struct resource res[] = {
+ { .flags = IORESOURCE_MEM },
+ { .flags = IORESOURCE_IRQ },
+ };
+ err = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
No need for a define, and as soon as the array is updated so will the
second argument sent to platform_device_add_resources.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/6] omap iommu: omap3 iommu device registration
Hi, Just one comment below: > > Does the following make sense? > > > > +#define NR_IOMMU_RES 2 > > > > > > > > + err = platform_device_add_resources(pdev, > > + omap3_iommu_res + i * NR_IOMMU_RES, > NR_IOMMU_RES); > > Yeap, also: > > > + err = platform_device_add_resources(pdev, > omap3_iommu_res + i * 2, 2); IMHO, I don't think it's a good idea to add magical numbers to any code in the kernel. Why is it better to NOT use a define? Regards, Sergio -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
On Fri, May 8, 2009 at 7:10 AM, Hiroshi DOYU wrote:
> Hi Felipe,
>
> From: ext Felipe Contreras
> Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
> Date: Thu, 7 May 2009 22:05:00 +0200
>
>> On Tue, May 5, 2009 at 3:47 PM, Hiroshi DOYU wrote:
>> > Signed-off-by: Hiroshi DOYU
>> > ---
>> >
>> > arch/arm/mach-omap2/omap3-iommu.c | 105
>> > +
>> > 1 files changed, 105 insertions(+), 0 deletions(-)
>> > create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
>> >
>> > diff --git a/arch/arm/mach-omap2/omap3-iommu.c
>> > b/arch/arm/mach-omap2/omap3-iommu.c
>> > new file mode 100644
>> > index 000..367f36a
>> > --- /dev/null
>> > +++ b/arch/arm/mach-omap2/omap3-iommu.c
>> > @@ -0,0 +1,105 @@
>> > +/*
>> > + * omap iommu: omap3 device registration
>> > + *
>> > + * Copyright (C) 2008-2009 Nokia Corporation
>> > + *
>> > + * Written by Hiroshi DOYU
>> > + *
>> > + * 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.
>> > + */
>> > +
>> > +#include
>> > +
>> > +#include
>> > +
>> > +#define OMAP3_MMU1_BASE 0x480bd400
>> > +#define OMAP3_MMU2_BASE 0x5d00
>> > +#define OMAP3_MMU1_IRQ 24
>> > +#define OMAP3_MMU2_IRQ 28
>> > +
>> > +static struct resource omap3_iommu_res[] = {
>> > + { /* Camera ISP MMU */
>> > + .start = OMAP3_MMU1_BASE,
>> > + .end = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
>> > + .flags = IORESOURCE_MEM,
>> > + },
>> > + {
>> > + .start = OMAP3_MMU1_IRQ,
>> > + .flags = IORESOURCE_IRQ,
>> > + },
>> > + { /* IVA2.2 MMU */
>> > + .start = OMAP3_MMU2_BASE,
>> > + .end = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
>> > + .flags = IORESOURCE_MEM,
>> > + },
>> > + {
>> > + .start = OMAP3_MMU2_IRQ,
>> > + .flags = IORESOURCE_IRQ,
>> > + },
>> > +};
>> > +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
>>
>>
>>
>> > + err = platform_device_add_resources(pdev,
>> > + &omap3_iommu_res[2 * i], NR_IOMMU_RES);
>>
>> This is wrong. In this particular case NR_IOMMU_RES is 2, but not
>> because there's 2 resources: MEM and IRQ, but because there's 2 iommu
>> devices: isp and iva2.
>
> Does the following make sense?
>
> +#define NR_IOMMU_RES 2
>
>
>
> + err = platform_device_add_resources(pdev,
> + omap3_iommu_res + i * NR_IOMMU_RES,
> NR_IOMMU_RES);
Yeap, also:
> + err = platform_device_add_resources(pdev, omap3_iommu_res +
> i * 2, 2);
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Hi Felipe,
From: ext Felipe Contreras
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Thu, 7 May 2009 22:05:00 +0200
> On Tue, May 5, 2009 at 3:47 PM, Hiroshi DOYU wrote:
> > Signed-off-by: Hiroshi DOYU
> > ---
> >
> > arch/arm/mach-omap2/omap3-iommu.c | 105
> > +
> > 1 files changed, 105 insertions(+), 0 deletions(-)
> > create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
> >
> > diff --git a/arch/arm/mach-omap2/omap3-iommu.c
> > b/arch/arm/mach-omap2/omap3-iommu.c
> > new file mode 100644
> > index 000..367f36a
> > --- /dev/null
> > +++ b/arch/arm/mach-omap2/omap3-iommu.c
> > @@ -0,0 +1,105 @@
> > +/*
> > + * omap iommu: omap3 device registration
> > + *
> > + * Copyright (C) 2008-2009 Nokia Corporation
> > + *
> > + * Written by Hiroshi DOYU
> > + *
> > + * 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.
> > + */
> > +
> > +#include
> > +
> > +#include
> > +
> > +#define OMAP3_MMU1_BASE 0x480bd400
> > +#define OMAP3_MMU2_BASE 0x5d00
> > +#define OMAP3_MMU1_IRQ 24
> > +#define OMAP3_MMU2_IRQ 28
> > +
> > +static struct resource omap3_iommu_res[] = {
> > + { /* Camera ISP MMU */
> > + .start = OMAP3_MMU1_BASE,
> > + .end = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> > + .flags = IORESOURCE_MEM,
> > + },
> > + {
> > + .start = OMAP3_MMU1_IRQ,
> > + .flags = IORESOURCE_IRQ,
> > + },
> > + { /* IVA2.2 MMU */
> > + .start = OMAP3_MMU2_BASE,
> > + .end = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> > + .flags = IORESOURCE_MEM,
> > + },
> > + {
> > + .start = OMAP3_MMU2_IRQ,
> > + .flags = IORESOURCE_IRQ,
> > + },
> > +};
> > +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
>
>
>
> > + err = platform_device_add_resources(pdev,
> > + &omap3_iommu_res[2 * i], NR_IOMMU_RES);
>
> This is wrong. In this particular case NR_IOMMU_RES is 2, but not
> because there's 2 resources: MEM and IRQ, but because there's 2 iommu
> devices: isp and iva2.
Does the following make sense?
+#define NR_IOMMU_RES 2
+ err = platform_device_add_resources(pdev,
+ omap3_iommu_res + i * NR_IOMMU_RES,
NR_IOMMU_RES);
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
On Tue, May 5, 2009 at 3:47 PM, Hiroshi DOYU wrote:
> Signed-off-by: Hiroshi DOYU
> ---
>
> arch/arm/mach-omap2/omap3-iommu.c | 105
> +
> 1 files changed, 105 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
>
> diff --git a/arch/arm/mach-omap2/omap3-iommu.c
> b/arch/arm/mach-omap2/omap3-iommu.c
> new file mode 100644
> index 000..367f36a
> --- /dev/null
> +++ b/arch/arm/mach-omap2/omap3-iommu.c
> @@ -0,0 +1,105 @@
> +/*
> + * omap iommu: omap3 device registration
> + *
> + * Copyright (C) 2008-2009 Nokia Corporation
> + *
> + * Written by Hiroshi DOYU
> + *
> + * 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.
> + */
> +
> +#include
> +
> +#include
> +
> +#define OMAP3_MMU1_BASE 0x480bd400
> +#define OMAP3_MMU2_BASE 0x5d00
> +#define OMAP3_MMU1_IRQ 24
> +#define OMAP3_MMU2_IRQ 28
> +
> +static struct resource omap3_iommu_res[] = {
> + { /* Camera ISP MMU */
> + .start = OMAP3_MMU1_BASE,
> + .end = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> + .flags = IORESOURCE_MEM,
> + },
> + {
> + .start = OMAP3_MMU1_IRQ,
> + .flags = IORESOURCE_IRQ,
> + },
> + { /* IVA2.2 MMU */
> + .start = OMAP3_MMU2_BASE,
> + .end = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> + .flags = IORESOURCE_MEM,
> + },
> + {
> + .start = OMAP3_MMU2_IRQ,
> + .flags = IORESOURCE_IRQ,
> + },
> +};
> +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
> + err = platform_device_add_resources(pdev,
> + &omap3_iommu_res[2 * i], NR_IOMMU_RES);
This is wrong. In this particular case NR_IOMMU_RES is 2, but not
because there's 2 resources: MEM and IRQ, but because there's 2 iommu
devices: isp and iva2.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
On Wed, May 6, 2009 at 9:00 AM, Hiroshi DOYU wrote:
> From: ext Felipe Contreras
> Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
> Date: Tue, 5 May 2009 21:32:34 +0200
>
>> On Tue, May 5, 2009 at 3:47 PM, Hiroshi DOYU wrote:
>> > Signed-off-by: Hiroshi DOYU
>> > ---
>> >
>> > arch/arm/mach-omap2/omap3-iommu.c | 105
>> > +
>> > 1 files changed, 105 insertions(+), 0 deletions(-)
>> > create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
>> >
>> > diff --git a/arch/arm/mach-omap2/omap3-iommu.c
>> > b/arch/arm/mach-omap2/omap3-iommu.c
>> > new file mode 100644
>> > index 000..367f36a
>> > --- /dev/null
>> > +++ b/arch/arm/mach-omap2/omap3-iommu.c
>> > @@ -0,0 +1,105 @@
>> > +/*
>> > + * omap iommu: omap3 device registration
>> > + *
>> > + * Copyright (C) 2008-2009 Nokia Corporation
>> > + *
>> > + * Written by Hiroshi DOYU
>> > + *
>> > + * 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.
>> > + */
>> > +
>> > +#include
>> > +
>> > +#include
>> > +
>> > +#define OMAP3_MMU1_BASE 0x480bd400
>> > +#define OMAP3_MMU2_BASE 0x5d00
>> > +#define OMAP3_MMU1_IRQ 24
>> > +#define OMAP3_MMU2_IRQ 28
>> > +
>> > +static struct resource omap3_iommu_res[] = {
>> > + { /* Camera ISP MMU */
>> > + .start = OMAP3_MMU1_BASE,
>> > + .end = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
>> > + .flags = IORESOURCE_MEM,
>> > + },
>> > + {
>> > + .start = OMAP3_MMU1_IRQ,
>> > + .flags = IORESOURCE_IRQ,
>> > + },
>> > + { /* IVA2.2 MMU */
>> > + .start = OMAP3_MMU2_BASE,
>> > + .end = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
>> > + .flags = IORESOURCE_MEM,
>> > + },
>> > + {
>> > + .start = OMAP3_MMU2_IRQ,
>> > + .flags = IORESOURCE_IRQ,
>> > + },
>> > +};
>>
>> This will break TI's bridgedriver, right?
>
> Working around is easy, but let's go forward, correcting TI's bridgedriver;-)
>
> http://marc.info/?l=linux-omap&m=124148814309478&w=2
*sigh*
I can't recall how many times I've asked for this, I'm sending a patch
series (RFC) that allows exactly what I want.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
From: ext Felipe Contreras
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Tue, 5 May 2009 21:32:34 +0200
> On Tue, May 5, 2009 at 3:47 PM, Hiroshi DOYU wrote:
> > Signed-off-by: Hiroshi DOYU
> > ---
> >
> > arch/arm/mach-omap2/omap3-iommu.c | 105
> > +
> > 1 files changed, 105 insertions(+), 0 deletions(-)
> > create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
> >
> > diff --git a/arch/arm/mach-omap2/omap3-iommu.c
> > b/arch/arm/mach-omap2/omap3-iommu.c
> > new file mode 100644
> > index 000..367f36a
> > --- /dev/null
> > +++ b/arch/arm/mach-omap2/omap3-iommu.c
> > @@ -0,0 +1,105 @@
> > +/*
> > + * omap iommu: omap3 device registration
> > + *
> > + * Copyright (C) 2008-2009 Nokia Corporation
> > + *
> > + * Written by Hiroshi DOYU
> > + *
> > + * 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.
> > + */
> > +
> > +#include
> > +
> > +#include
> > +
> > +#define OMAP3_MMU1_BASE 0x480bd400
> > +#define OMAP3_MMU2_BASE 0x5d00
> > +#define OMAP3_MMU1_IRQ 24
> > +#define OMAP3_MMU2_IRQ 28
> > +
> > +static struct resource omap3_iommu_res[] = {
> > + { /* Camera ISP MMU */
> > + .start = OMAP3_MMU1_BASE,
> > + .end = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> > + .flags = IORESOURCE_MEM,
> > + },
> > + {
> > + .start = OMAP3_MMU1_IRQ,
> > + .flags = IORESOURCE_IRQ,
> > + },
> > + { /* IVA2.2 MMU */
> > + .start = OMAP3_MMU2_BASE,
> > + .end = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> > + .flags = IORESOURCE_MEM,
> > + },
> > + {
> > + .start = OMAP3_MMU2_IRQ,
> > + .flags = IORESOURCE_IRQ,
> > + },
> > +};
>
> This will break TI's bridgedriver, right?
Working around is easy, but let's go forward, correcting TI's bridgedriver;-)
http://marc.info/?l=linux-omap&m=124148814309478&w=2
>
> Can camera ISP and IVA request the registration of their respective
> IOMMU's from their codebase?
>
> --
> Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
On Tue, May 5, 2009 at 3:47 PM, Hiroshi DOYU wrote:
> Signed-off-by: Hiroshi DOYU
> ---
>
> arch/arm/mach-omap2/omap3-iommu.c | 105
> +
> 1 files changed, 105 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
>
> diff --git a/arch/arm/mach-omap2/omap3-iommu.c
> b/arch/arm/mach-omap2/omap3-iommu.c
> new file mode 100644
> index 000..367f36a
> --- /dev/null
> +++ b/arch/arm/mach-omap2/omap3-iommu.c
> @@ -0,0 +1,105 @@
> +/*
> + * omap iommu: omap3 device registration
> + *
> + * Copyright (C) 2008-2009 Nokia Corporation
> + *
> + * Written by Hiroshi DOYU
> + *
> + * 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.
> + */
> +
> +#include
> +
> +#include
> +
> +#define OMAP3_MMU1_BASE 0x480bd400
> +#define OMAP3_MMU2_BASE 0x5d00
> +#define OMAP3_MMU1_IRQ 24
> +#define OMAP3_MMU2_IRQ 28
> +
> +static struct resource omap3_iommu_res[] = {
> + { /* Camera ISP MMU */
> + .start = OMAP3_MMU1_BASE,
> + .end = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> + .flags = IORESOURCE_MEM,
> + },
> + {
> + .start = OMAP3_MMU1_IRQ,
> + .flags = IORESOURCE_IRQ,
> + },
> + { /* IVA2.2 MMU */
> + .start = OMAP3_MMU2_BASE,
> + .end = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> + .flags = IORESOURCE_MEM,
> + },
> + {
> + .start = OMAP3_MMU2_IRQ,
> + .flags = IORESOURCE_IRQ,
> + },
> +};
This will break TI's bridgedriver, right?
Can camera ISP and IVA request the registration of their respective
IOMMU's from their codebase?
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] omap iommu: omap3 iommu device registration
Signed-off-by: Hiroshi DOYU
---
arch/arm/mach-omap2/omap3-iommu.c | 105 +
1 files changed, 105 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
diff --git a/arch/arm/mach-omap2/omap3-iommu.c
b/arch/arm/mach-omap2/omap3-iommu.c
new file mode 100644
index 000..367f36a
--- /dev/null
+++ b/arch/arm/mach-omap2/omap3-iommu.c
@@ -0,0 +1,105 @@
+/*
+ * omap iommu: omap3 device registration
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU
+ *
+ * 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.
+ */
+
+#include
+
+#include
+
+#define OMAP3_MMU1_BASE0x480bd400
+#define OMAP3_MMU2_BASE0x5d00
+#define OMAP3_MMU1_IRQ 24
+#define OMAP3_MMU2_IRQ 28
+
+static struct resource omap3_iommu_res[] = {
+ { /* Camera ISP MMU */
+ .start = OMAP3_MMU1_BASE,
+ .end= OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
+ .flags = IORESOURCE_MEM,
+ },
+ {
+ .start = OMAP3_MMU1_IRQ,
+ .flags = IORESOURCE_IRQ,
+ },
+ { /* IVA2.2 MMU */
+ .start = OMAP3_MMU2_BASE,
+ .end= OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
+ .flags = IORESOURCE_MEM,
+ },
+ {
+ .start = OMAP3_MMU2_IRQ,
+ .flags = IORESOURCE_IRQ,
+ },
+};
+#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
+
+static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
+ {
+ .name = "isp",
+ .nr_tlb_entries = 8,
+ .clk_name = "cam_ick",
+ },
+ {
+ .name = "iva2",
+ .nr_tlb_entries = 32,
+ .clk_name = "iva2_ck",
+ },
+};
+#define NR_IOMMU_DEVICES ARRAY_SIZE(omap3_iommu_pdata)
+
+static struct platform_device *omap3_iommu_pdev[NR_IOMMU_DEVICES];
+
+static int __init omap3_iommu_init(void)
+{
+ int i, err;
+
+ for (i = 0; i < NR_IOMMU_DEVICES; i++) {
+ struct platform_device *pdev;
+
+ pdev = platform_device_alloc("omap-iommu", i + 1);
+ if (!pdev) {
+ err = -ENOMEM;
+ goto err_out;
+ }
+ err = platform_device_add_resources(pdev,
+ &omap3_iommu_res[2 * i], NR_IOMMU_RES);
+ if (err)
+ goto err_out;
+ err = platform_device_add_data(pdev, &omap3_iommu_pdata[i],
+ sizeof(omap3_iommu_pdata[0]));
+ if (err)
+ goto err_out;
+ err = platform_device_add(pdev);
+ if (err)
+ goto err_out;
+ omap3_iommu_pdev[i] = pdev;
+ }
+ return 0;
+
+err_out:
+ while (i--)
+ platform_device_put(omap3_iommu_pdev[i]);
+ return err;
+}
+module_init(omap3_iommu_init);
+
+static void __exit omap3_iommu_exit(void)
+{
+ int i;
+
+ for (i = 0; i < NR_IOMMU_DEVICES; i++)
+ platform_device_unregister(omap3_iommu_pdev[i]);
+}
+module_exit(omap3_iommu_exit);
+
+MODULE_AUTHOR("Hiroshi DOYU");
+MODULE_DESCRIPTION("omap iommu: omap3 device registration");
+MODULE_LICENSE("GPL v2");
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
From: ext Russell King - ARM Linux
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Wed, 28 Jan 2009 11:41:57 +0100
> On Tue, Jan 27, 2009 at 11:29:35PM +0200, Hiroshi DOYU wrote:
> > I attached the update one.
>
> Thanks. I want to hold off on taking these just a little bit longer.
OK. agreed.
> > +static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
> > + {
> > + .name = "isp",
> > + .nr_tlb_entries = 8,
> > + .clk_name = "cam_ick",
> > + },
> > + {
> > + .name = "iva2",
> > + .nr_tlb_entries = 32,
> > + .clk_name = "iva2_ck",
>
> With the omap clk implementation in a state of flux, hopefully moving
> towards something which better reflects the intentions of the clk API,
> passing clk names around becomes entirely wasteful and unnecessary.
>
> That's not to say that what you currently have won't work with the patches
> as they currently stand - it will work as is. I'd just rather avoid
> having to have a separate patch to convert this code as well.
OK. I'll update later, then.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
On Tue, Jan 27, 2009 at 11:29:35PM +0200, Hiroshi DOYU wrote:
> I attached the update one.
Thanks. I want to hold off on taking these just a little bit longer.
> +static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
> + {
> + .name = "isp",
> + .nr_tlb_entries = 8,
> + .clk_name = "cam_ick",
> + },
> + {
> + .name = "iva2",
> + .nr_tlb_entries = 32,
> + .clk_name = "iva2_ck",
With the omap clk implementation in a state of flux, hopefully moving
towards something which better reflects the intentions of the clk API,
passing clk names around becomes entirely wasteful and unnecessary.
That's not to say that what you currently have won't work with the patches
as they currently stand - it will work as is. I'd just rather avoid
having to have a separate patch to convert this code as well.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Hi Russell,
I attached the update one.
From: ext Russell King - ARM Linux
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Sat, 17 Jan 2009 17:21:39 +0100
> On Fri, Jan 16, 2009 at 10:37:20AM +0200, Hiroshi DOYU wrote:
> > +#include
>
> Is linux/io.h needed, or will a more specific include be better?
>
> > +#include
> > +
> > +#include
> > +
> > +#define DEVNAME "omap-iommu"
>
> I'm not sure this DEVNAME definition really helps anything.
>
> > +static void omap3_iommu_release(struct device *dev)
> > +{
> > +}
>
> Err, no. Never ever ever provide a NULL release function. Providing
> such a function is a screaming message that what you're doing is buggy.
>
> And if you get a warning through not providing such a function, it's
> telling you that what your overall approach with the driver API is
> buggy (and you haven't understood the implications of refcounted
> object management.)
>
> > +
> > +static struct platform_device omap3_iommu_pdev[] = {
> > + {
> > + .name = DEVNAME,
> > + .id = 1,
> > + .num_resources = ARRAY_SIZE(iommu1_res),
> > + .resource = iommu1_res,
> > + .dev= {
> > + .release = omap3_iommu_release,
> > + .platform_data = &omap3_iommu_pdata[0],
> > + },
> > + },
> > + {
> > + .name = DEVNAME,
> > + .id = 2,
> > + .num_resources = ARRAY_SIZE(iommu2_res),
> > + .resource = iommu2_res,
> > + .dev= {
> > + .release = omap3_iommu_release,
> > + .platform_data = &omap3_iommu_pdata[1],
> > + },
> > + },
> > +};
> > +
> > +static int __init omap3_iommu_init(void)
> > +{
> > + int i;
> > + for (i = 0; i < ARRAY_SIZE(omap3_iommu_pdev); i++)
> > + platform_device_register(&omap3_iommu_pdev[i]);
> > + return 0;
> > +}
> > +module_init(omap3_iommu_init);
> > +
> > +static void __exit omap3_iommu_exit(void)
> > +{
> > + int i;
> > + for (i = 0; i < ARRAY_SIZE(omap3_iommu_pdev); i++)
> > + platform_device_unregister(&omap3_iommu_pdev[i]);
>
> So... this can never be bug free - you can _never_ unregister statically
> allocated devices. Not even if you provide an empty release function.
>
> If you want to register and unregister device structures, it must be
> done using the correct APIs, and in the case of platform devices, that's
> the platform_device_alloc(), platform_device_add() and
> platform_device_unregister() APIs.
>From 5b61d40f516d7df3a597df5bd798cb06df01fb28 Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU
Date: Tue, 27 Jan 2009 22:46:36 +0200
Subject: [PATCH 3/6] omap iommu: omap3 iommu device registration
Signed-off-by: Hiroshi DOYU
---
arch/arm/mach-omap2/omap3-iommu.c | 103 +
1 files changed, 103 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
diff --git a/arch/arm/mach-omap2/omap3-iommu.c
b/arch/arm/mach-omap2/omap3-iommu.c
new file mode 100644
index 000..4f51c18
--- /dev/null
+++ b/arch/arm/mach-omap2/omap3-iommu.c
@@ -0,0 +1,103 @@
+/*
+ * omap iommu: omap3 device registration
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU
+ *
+ * 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.
+ */
+
+#include
+#include
+#include
+
+#define OMAP3_MMU1_BASE0x480bd400
+#define OMAP3_MMU2_BASE0x5d00
+#define OMAP3_MMU1_IRQ 24
+#define OMAP3_MMU2_IRQ 28
+
+static struct resource omap3_iommu_res[] = {
+ { /* Camera ISP MMU */
+ .start = OMAP3_MMU1_BASE,
+ .end= OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
+ .flags = IORESOURCE_MEM,
+ },
+ {
+ .start = OMAP3_MMU1_IRQ,
+ .flags = IORESOURCE_IRQ,
+ },
+ { /* IVA2.2 MMU */
+ .start = OMAP3_MMU2_BASE,
+ .end= OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
+ .flags = IORESOURCE_MEM,
+ },
+ {
+ .start = OMAP3_MMU2_IRQ,
+ .flags = IORESOURCE_IRQ,
+ },
+};
+#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
+
+static const struct iommu_platform_data omap3_iommu_pdata[] __i
Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
On Fri, Jan 16, 2009 at 10:37:20AM +0200, Hiroshi DOYU wrote:
> +#include
Is linux/io.h needed, or will a more specific include be better?
> +#include
> +
> +#include
> +
> +#define DEVNAME "omap-iommu"
I'm not sure this DEVNAME definition really helps anything.
> +static void omap3_iommu_release(struct device *dev)
> +{
> +}
Err, no. Never ever ever provide a NULL release function. Providing
such a function is a screaming message that what you're doing is buggy.
And if you get a warning through not providing such a function, it's
telling you that what your overall approach with the driver API is
buggy (and you haven't understood the implications of refcounted
object management.)
> +
> +static struct platform_device omap3_iommu_pdev[] = {
> + {
> + .name = DEVNAME,
> + .id = 1,
> + .num_resources = ARRAY_SIZE(iommu1_res),
> + .resource = iommu1_res,
> + .dev= {
> + .release = omap3_iommu_release,
> + .platform_data = &omap3_iommu_pdata[0],
> + },
> + },
> + {
> + .name = DEVNAME,
> + .id = 2,
> + .num_resources = ARRAY_SIZE(iommu2_res),
> + .resource = iommu2_res,
> + .dev= {
> + .release = omap3_iommu_release,
> + .platform_data = &omap3_iommu_pdata[1],
> + },
> + },
> +};
> +
> +static int __init omap3_iommu_init(void)
> +{
> + int i;
> + for (i = 0; i < ARRAY_SIZE(omap3_iommu_pdev); i++)
> + platform_device_register(&omap3_iommu_pdev[i]);
> + return 0;
> +}
> +module_init(omap3_iommu_init);
> +
> +static void __exit omap3_iommu_exit(void)
> +{
> + int i;
> + for (i = 0; i < ARRAY_SIZE(omap3_iommu_pdev); i++)
> + platform_device_unregister(&omap3_iommu_pdev[i]);
So... this can never be bug free - you can _never_ unregister statically
allocated devices. Not even if you provide an empty release function.
If you want to register and unregister device structures, it must be
done using the correct APIs, and in the case of platform devices, that's
the platform_device_alloc(), platform_device_add() and
platform_device_unregister() APIs.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] omap iommu: omap3 iommu device registration
Signed-off-by: Hiroshi DOYU
---
arch/arm/mach-omap2/omap3-iommu.c | 111 +
1 files changed, 111 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
diff --git a/arch/arm/mach-omap2/omap3-iommu.c
b/arch/arm/mach-omap2/omap3-iommu.c
new file mode 100644
index 000..52d0e56
--- /dev/null
+++ b/arch/arm/mach-omap2/omap3-iommu.c
@@ -0,0 +1,111 @@
+/*
+ * omap iommu: omap3 device registration
+ *
+ * Copyright (C) 2008 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU
+ *
+ * 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.
+ */
+
+#include
+#include
+
+#include
+
+#define DEVNAME "omap-iommu"
+
+/* Camera ISP MMU */
+#define OMAP3_MMU1_BASE0x480bd400
+#define OMAP3_MMU1_IRQ 24
+
+/* IVA2.2 MMU */
+#define OMAP3_MMU2_BASE0x5d00
+#define OMAP3_MMU2_IRQ 28
+
+static struct resource iommu1_res[] = { /* Camera ISP MMU */
+ {
+ .start = OMAP3_MMU1_BASE,
+ .end= OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
+ .flags = IORESOURCE_MEM,
+ },
+ {
+ .start = OMAP3_MMU1_IRQ,
+ .flags = IORESOURCE_IRQ,
+ },
+};
+
+static struct resource iommu2_res[] = { /* IVA2.2 MMU */
+ {
+ .start = OMAP3_MMU2_BASE,
+ .end= OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
+ .flags = IORESOURCE_MEM,
+ },
+ {
+ .start = OMAP3_MMU2_IRQ,
+ .flags = IORESOURCE_IRQ,
+ },
+};
+
+static struct iommu_platform_data omap3_iommu_pdata[] = {
+ {
+ .name = "isp",
+ .nr_tlb_entries = 8,
+ .clk_name = "cam_ick",
+ },
+ {
+ .name = "iva2",
+ .nr_tlb_entries = 32,
+ .clk_name = "iva2_ck",
+ },
+};
+
+static void omap3_iommu_release(struct device *dev)
+{
+}
+
+static struct platform_device omap3_iommu_pdev[] = {
+ {
+ .name = DEVNAME,
+ .id = 1,
+ .num_resources = ARRAY_SIZE(iommu1_res),
+ .resource = iommu1_res,
+ .dev= {
+ .release = omap3_iommu_release,
+ .platform_data = &omap3_iommu_pdata[0],
+ },
+ },
+ {
+ .name = DEVNAME,
+ .id = 2,
+ .num_resources = ARRAY_SIZE(iommu2_res),
+ .resource = iommu2_res,
+ .dev= {
+ .release = omap3_iommu_release,
+ .platform_data = &omap3_iommu_pdata[1],
+ },
+ },
+};
+
+static int __init omap3_iommu_init(void)
+{
+ int i;
+ for (i = 0; i < ARRAY_SIZE(omap3_iommu_pdev); i++)
+ platform_device_register(&omap3_iommu_pdev[i]);
+ return 0;
+}
+module_init(omap3_iommu_init);
+
+static void __exit omap3_iommu_exit(void)
+{
+ int i;
+ for (i = 0; i < ARRAY_SIZE(omap3_iommu_pdev); i++)
+ platform_device_unregister(&omap3_iommu_pdev[i]);
+}
+module_exit(omap3_iommu_exit);
+
+MODULE_AUTHOR("Hiroshi DOYU");
+MODULE_DESCRIPTION("omap iommu: omap3 device registration");
+MODULE_LICENSE("GPL v2");
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
