* [PATCH 1/2] [ARM] Introduce 'struct machine_class' for SoC level abstraction @ 2010-06-20 14:00 Eric Miao 2010-06-20 14:00 ` [PATCH 2/2] [ARM] pxa: make use of 'struct machine_class' Eric Miao 2010-06-21 7:27 ` [PATCH 1/2] [ARM] Introduce 'struct machine_class' for SoC level abstraction Jeremy Kerr 0 siblings, 2 replies; 10+ messages in thread From: Eric Miao @ 2010-06-20 14:00 UTC (permalink / raw) To: linux-arm-kernel From: Eric Miao <eric.y.miao@gmail.com> For machine class (mostly equivalent to SoC) level abstraction. The modification to head.S is preliminary, working but possibly can be made more elegant. Signed-off-by: Eric Miao <eric.miao@canonical.com> --- arch/arm/include/asm/mach/arch.h | 13 +++++++++++++ arch/arm/kernel/asm-offsets.c | 4 ++++ arch/arm/kernel/head-common.S | 3 +++ arch/arm/kernel/head.S | 12 +++++++++--- arch/arm/kernel/setup.c | 14 ++++++++++---- arch/arm/mm/mmu.c | 5 ++++- 6 files changed, 43 insertions(+), 8 deletions(-) diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h index c59842d..214ca29 100644 --- a/arch/arm/include/asm/mach/arch.h +++ b/arch/arm/include/asm/mach/arch.h @@ -14,6 +14,18 @@ struct tag; struct meminfo; struct sys_timer; +struct machine_class { + unsigned int phys_io; /* start of physical io */ + unsigned int io_pg_offst; /* byte offset for io + * page tabe entry */ + const char *name; /* machine class name */ + unsigned long boot_params; /* tagged list */ + + void (*map_io)(void);/* IO mapping function */ + void (*init_irq)(void); + struct sys_timer *timer; +}; + struct machine_desc { /* * Note! The first four elements are used @@ -25,6 +37,7 @@ struct machine_desc { * page tabe entry */ const char *name; /* architecture name */ + struct machine_class *class; /* machine class */ unsigned long boot_params; /* tagged list */ unsigned int video_start; /* start of video RAM */ diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c index 8835115..5c9038a 100644 --- a/arch/arm/kernel/asm-offsets.c +++ b/arch/arm/kernel/asm-offsets.c @@ -101,6 +101,10 @@ int main(void) DEFINE(MACHINFO_NAME, offsetof(struct machine_desc, name)); DEFINE(MACHINFO_PHYSIO, offsetof(struct machine_desc, phys_io)); DEFINE(MACHINFO_PGOFFIO, offsetof(struct machine_desc, io_pg_offst)); + DEFINE(MACHINFO_CLASS, offsetof(struct machine_desc, class)); + BLANK(); + DEFINE(MACHCLASS_PHYSIO, offsetof(struct machine_class, phys_io)); + DEFINE(MACHCLASS_PGOFFIO, offsetof(struct machine_class, io_pg_offst)); BLANK(); DEFINE(PROC_INFO_SZ, sizeof(struct proc_info_list)); DEFINE(PROCINFO_INITFUNC, offsetof(struct proc_info_list, __cpu_flush)); diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S index b9505aa..660756b 100644 --- a/arch/arm/kernel/head-common.S +++ b/arch/arm/kernel/head-common.S @@ -207,6 +207,7 @@ ENDPROC(lookup_processor_type) * Returns: * r3, r4, r6 corrupted * r5 = mach_info pointer in physical address space + * r7 = machine_class pointer in physical address space */ __lookup_machine_type: adr r3, 4b @@ -214,6 +215,8 @@ __lookup_machine_type: sub r3, r3, r4 @ get offset between virt&phys add r5, r5, r3 @ convert virt addresses to add r6, r6, r3 @ physical address space + ldr r7, [r5, #MACHINFO_CLASS] @ get machine class + add r7, r7, r3 1: ldr r3, [r5, #MACHINFO_TYPE] @ get machine type teq r3, r1 @ matches loader number? beq 2f @ found diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S index eb62bf9..06f8834 100644 --- a/arch/arm/kernel/head.S +++ b/arch/arm/kernel/head.S @@ -82,8 +82,9 @@ ENTRY(stext) bl __lookup_processor_type @ r5=procinfo r9=cpuid movs r10, r5 @ invalid processor (r5=0)? beq __error_p @ yes, error 'p' - bl __lookup_machine_type @ r5=machinfo + bl __lookup_machine_type @ r5=machinfo,r7=machine_class movs r8, r5 @ invalid machine (r5=0)? + mov r11, r7 beq __error_a @ yes, error 'a' bl __vet_atags bl __create_page_tables @@ -211,6 +212,7 @@ ENDPROC(__turn_mmu_on) * r8 = machinfo * r9 = cpuid * r10 = procinfo + * r11 = machine_class * * Returns: * r0, r3, r6, r7 corrupted @@ -295,13 +297,17 @@ __create_page_tables: * This allows debug messages to be output * via a serial console before paging_init. */ - ldr r3, [r8, #MACHINFO_PGOFFIO] + cmp r11, #0 + ldrne r3, [r11, #MACHCLASS_PGOFFIO] + ldreq r3, [r8, #MACHINFO_PGOFFIO] add r0, r4, r3 rsb r3, r3, #0x4000 @ PTRS_PER_PGD*sizeof(long) cmp r3, #0x0800 @ limit to 512MB movhi r3, #0x0800 add r6, r0, r3 - ldr r3, [r8, #MACHINFO_PHYSIO] + cmp r11, #0 + ldrne r3, [r11, #MACHCLASS_PHYSIO] + ldreq r3, [r8, #MACHINFO_PHYSIO] orr r3, r3, r7 1: str r3, [r0], #4 add r3, r3, #1 << 20 diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index 122d999..e5627da 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -667,12 +667,14 @@ void __init setup_arch(char **cmdline_p) { struct tag *tags = (struct tag *)&init_tags; struct machine_desc *mdesc; + struct machine_class *class; char *from = default_command_line; unwind_init(); setup_processor(); mdesc = setup_machine(machine_arch_type); + class = mdesc->class; machine_name = mdesc->name; if (mdesc->soft_reboot) @@ -680,8 +682,12 @@ void __init setup_arch(char **cmdline_p) if (__atags_pointer) tags = phys_to_virt(__atags_pointer); - else if (mdesc->boot_params) - tags = phys_to_virt(mdesc->boot_params); + else { + unsigned long boot_params = class ? class->boot_params : + mdesc->boot_params; + if (boot_params) + tags = phys_to_virt(boot_params); + } /* * If we have the old style parameters, convert them to @@ -729,8 +735,8 @@ void __init setup_arch(char **cmdline_p) /* * Set up various architecture-specific pointers */ - init_arch_irq = mdesc->init_irq; - system_timer = mdesc->timer; + init_arch_irq = class ? class->init_irq : mdesc->init_irq; + system_timer = class ? class->timer : mdesc->timer; init_machine = mdesc->init_machine; #ifdef CONFIG_VT diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c index 2858941..2af11dc 100644 --- a/arch/arm/mm/mmu.c +++ b/arch/arm/mm/mmu.c @@ -933,6 +933,7 @@ void __init reserve_node_zero(pg_data_t *pgdat) */ static void __init devicemaps_init(struct machine_desc *mdesc) { + struct machine_class *class = mdesc->class; struct map_desc map; unsigned long addr; void *vectors; @@ -995,7 +996,9 @@ static void __init devicemaps_init(struct machine_desc *mdesc) /* * Ask the machine support to map in the statically mapped devices. */ - if (mdesc->map_io) + if (class && class->map_io) + class->map_io(); + else if (mdesc->map_io) mdesc->map_io(); /* -- 1.7.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] [ARM] pxa: make use of 'struct machine_class' 2010-06-20 14:00 [PATCH 1/2] [ARM] Introduce 'struct machine_class' for SoC level abstraction Eric Miao @ 2010-06-20 14:00 ` Eric Miao 2010-06-20 16:08 ` Antonio Ospite 2010-06-21 7:27 ` [PATCH 1/2] [ARM] Introduce 'struct machine_class' for SoC level abstraction Jeremy Kerr 1 sibling, 1 reply; 10+ messages in thread From: Eric Miao @ 2010-06-20 14:00 UTC (permalink / raw) To: linux-arm-kernel From: Eric Miao <eric.y.miao@gmail.com> Signed-off-by: Eric Miao <eric.miao@canonical.com> --- arch/arm/mach-pxa/ezx.c | 60 ++++------------------------------------- arch/arm/mach-pxa/generic.h | 15 ++++++++++ arch/arm/mach-pxa/hx4700.c | 10 +------ arch/arm/mach-pxa/magician.c | 9 +----- arch/arm/mach-pxa/pxa25x.c | 11 +++++++ arch/arm/mach-pxa/pxa27x.c | 11 +++++++ 6 files changed, 46 insertions(+), 70 deletions(-) diff --git a/arch/arm/mach-pxa/ezx.c b/arch/arm/mach-pxa/ezx.c index 626c82b..2f4c8f1 100644 --- a/arch/arm/mach-pxa/ezx.c +++ b/arch/arm/mach-pxa/ezx.c @@ -795,15 +795,7 @@ static void __init a780_init(void) platform_add_devices(ARRAY_AND_SIZE(a780_devices)); } -MACHINE_START(EZX_A780, "Motorola EZX A780") - .phys_io = 0x40000000, - .io_pg_offst = (io_p2v(0x40000000) >> 18) & 0xfffc, - .boot_params = 0xa0000100, - .map_io = pxa_map_io, - .init_irq = pxa27x_init_irq, - .timer = &pxa_timer, - .init_machine = a780_init, -MACHINE_END +PXA27X_MACHINE(EZX_A780, a780_init, "Motorola EZX A780"); #endif #ifdef CONFIG_MACH_EZX_E680 @@ -861,15 +853,7 @@ static void __init e680_init(void) platform_add_devices(ARRAY_AND_SIZE(e680_devices)); } -MACHINE_START(EZX_E680, "Motorola EZX E680") - .phys_io = 0x40000000, - .io_pg_offst = (io_p2v(0x40000000) >> 18) & 0xfffc, - .boot_params = 0xa0000100, - .map_io = pxa_map_io, - .init_irq = pxa27x_init_irq, - .timer = &pxa_timer, - .init_machine = e680_init, -MACHINE_END +PXA27X_MACHINE(EZX_E680, e680_init, "Motorola EZX E680"); #endif #ifdef CONFIG_MACH_EZX_A1200 @@ -927,15 +911,7 @@ static void __init a1200_init(void) platform_add_devices(ARRAY_AND_SIZE(a1200_devices)); } -MACHINE_START(EZX_A1200, "Motorola EZX A1200") - .phys_io = 0x40000000, - .io_pg_offst = (io_p2v(0x40000000) >> 18) & 0xfffc, - .boot_params = 0xa0000100, - .map_io = pxa_map_io, - .init_irq = pxa27x_init_irq, - .timer = &pxa_timer, - .init_machine = a1200_init, -MACHINE_END +PXA27X_MACHINE(EZX_A1200, a1200_init, "Motorola EZX A1200"); #endif #ifdef CONFIG_MACH_EZX_A910 @@ -1119,15 +1095,7 @@ static void __init a910_init(void) platform_add_devices(ARRAY_AND_SIZE(a910_devices)); } -MACHINE_START(EZX_A910, "Motorola EZX A910") - .phys_io = 0x40000000, - .io_pg_offst = (io_p2v(0x40000000) >> 18) & 0xfffc, - .boot_params = 0xa0000100, - .map_io = pxa_map_io, - .init_irq = pxa27x_init_irq, - .timer = &pxa_timer, - .init_machine = a910_init, -MACHINE_END +PXA27X_MACHINE(EZX_A910, a910_init, "Motorola EZX A910"); #endif #ifdef CONFIG_MACH_EZX_E6 @@ -1185,15 +1153,7 @@ static void __init e6_init(void) platform_add_devices(ARRAY_AND_SIZE(e6_devices)); } -MACHINE_START(EZX_E6, "Motorola EZX E6") - .phys_io = 0x40000000, - .io_pg_offst = (io_p2v(0x40000000) >> 18) & 0xfffc, - .boot_params = 0xa0000100, - .map_io = pxa_map_io, - .init_irq = pxa27x_init_irq, - .timer = &pxa_timer, - .init_machine = e6_init, -MACHINE_END +PXA27X_MACHINE(EZX_E6, e6_init, "Motorola EZX E6"); #endif #ifdef CONFIG_MACH_EZX_E2 @@ -1225,13 +1185,5 @@ static void __init e2_init(void) platform_add_devices(ARRAY_AND_SIZE(e2_devices)); } -MACHINE_START(EZX_E2, "Motorola EZX E2") - .phys_io = 0x40000000, - .io_pg_offst = (io_p2v(0x40000000) >> 18) & 0xfffc, - .boot_params = 0xa0000100, - .map_io = pxa_map_io, - .init_irq = pxa27x_init_irq, - .timer = &pxa_timer, - .init_machine = e2_init, -MACHINE_END +PXA27X_MACHINE(EZX_E2, e2_init, "Motorola EZX E2"); #endif diff --git a/arch/arm/mach-pxa/generic.h b/arch/arm/mach-pxa/generic.h index 890fb90..6b6d767 100644 --- a/arch/arm/mach-pxa/generic.h +++ b/arch/arm/mach-pxa/generic.h @@ -72,3 +72,18 @@ void __init pxa_set_ffuart_info(void *info); void __init pxa_set_btuart_info(void *info); void __init pxa_set_stuart_info(void *info); void __init pxa_set_hwuart_info(void *info); + +extern struct machine_class machine_class_pxa25x; +extern struct machine_class machine_class_pxa27x; + +#define PXA25X_MACHINE(_type,_init,_name) \ +MACHINE_START(_type,_name) \ + .class = &machine_class_pxa25x, \ + .init_machine = _init, \ +} + +#define PXA27X_MACHINE(_type,_init,_name) \ +MACHINE_START(_type,_name) \ + .class = &machine_class_pxa27x, \ + .init_machine = _init, \ +} diff --git a/arch/arm/mach-pxa/hx4700.c b/arch/arm/mach-pxa/hx4700.c index 848c861..a1b1587 100644 --- a/arch/arm/mach-pxa/hx4700.c +++ b/arch/arm/mach-pxa/hx4700.c @@ -869,12 +869,4 @@ static void __init hx4700_init(void) mdelay(10); } -MACHINE_START(H4700, "HP iPAQ HX4700") - .phys_io = 0x40000000, - .io_pg_offst = (io_p2v(0x40000000) >> 18) & 0xfffc, - .boot_params = 0xa0000100, - .map_io = pxa_map_io, - .init_irq = pxa27x_init_irq, - .init_machine = hx4700_init, - .timer = &pxa_timer, -MACHINE_END +PXA27X_MACHINE_START(H4700, hx4700_init, "HP iPAQ HX4700"); diff --git a/arch/arm/mach-pxa/magician.c b/arch/arm/mach-pxa/magician.c index e81dd0c..6a86f22 100644 --- a/arch/arm/mach-pxa/magician.c +++ b/arch/arm/mach-pxa/magician.c @@ -764,11 +764,6 @@ static void __init magician_init(void) MACHINE_START(MAGICIAN, "HTC Magician") - .phys_io = 0x40000000, - .io_pg_offst = (io_p2v(0x40000000) >> 18) & 0xfffc, - .boot_params = 0xa0000100, - .map_io = pxa_map_io, - .init_irq = pxa27x_init_irq, - .init_machine = magician_init, - .timer = &pxa_timer, + .class = &machine_class_pxa27x, + .init_machine = magician_init, MACHINE_END diff --git a/arch/arm/mach-pxa/pxa25x.c b/arch/arm/mach-pxa/pxa25x.c index 0b9ad30..8993df6 100644 --- a/arch/arm/mach-pxa/pxa25x.c +++ b/arch/arm/mach-pxa/pxa25x.c @@ -23,6 +23,7 @@ #include <linux/suspend.h> #include <linux/sysdev.h> +#include <asm/mach/arch.h> #include <mach/hardware.h> #include <mach/irqs.h> #include <mach/gpio.h> @@ -341,6 +342,16 @@ static struct sys_device pxa25x_sysdev[] = { }, }; +struct machine_class machine_class_pxa25x __initdata = { + .name = "PXA25x", + .phys_io = 0x40000000, + .io_pg_offst = (io_p2v(0x40000000) >> 18) & 0xfffc, + .boot_params = 0xa0000100, + .timer = &pxa_timer, + .map_io = pxa_map_io, + .init_irq = pxa25x_init_irq, +}; + static int __init pxa25x_init(void) { int i, ret = 0; diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c index 0af3617..d64b194 100644 --- a/arch/arm/mach-pxa/pxa27x.c +++ b/arch/arm/mach-pxa/pxa27x.c @@ -18,6 +18,7 @@ #include <linux/platform_device.h> #include <linux/sysdev.h> +#include <asm/mach/arch.h> #include <mach/hardware.h> #include <asm/irq.h> #include <mach/irqs.h> @@ -403,6 +404,16 @@ static struct sys_device pxa27x_sysdev[] = { }, }; +struct machine_class machine_class_pxa27x __initdata = { + .name = "PXA27x", + .phys_io = 0x40000000, + .io_pg_offst = (io_p2v(0x40000000) >> 18) & 0xfffc, + .boot_params = 0xa0000100, + .timer = &pxa_timer, + .map_io = pxa_map_io, + .init_irq = pxa27x_init_irq, +}; + static int __init pxa27x_init(void) { int i, ret = 0; -- 1.7.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] [ARM] pxa: make use of 'struct machine_class' 2010-06-20 14:00 ` [PATCH 2/2] [ARM] pxa: make use of 'struct machine_class' Eric Miao @ 2010-06-20 16:08 ` Antonio Ospite 2010-06-21 1:06 ` Eric Miao 0 siblings, 1 reply; 10+ messages in thread From: Antonio Ospite @ 2010-06-20 16:08 UTC (permalink / raw) To: linux-arm-kernel On Sun, 20 Jun 2010 22:00:50 +0800 Eric Miao <eric.miao@canonical.com> wrote: > From: Eric Miao <eric.y.miao@gmail.com> > > Signed-off-by: Eric Miao <eric.miao@canonical.com> > --- > arch/arm/mach-pxa/ezx.c | 60 ++++------------------------------------- The ezx part looks OK. A comment about magician below. [...] > diff --git a/arch/arm/mach-pxa/magician.c b/arch/arm/mach-pxa/magician.c > index e81dd0c..6a86f22 100644 > --- a/arch/arm/mach-pxa/magician.c > +++ b/arch/arm/mach-pxa/magician.c > @@ -764,11 +764,6 @@ static void __init magician_init(void) > > > MACHINE_START(MAGICIAN, "HTC Magician") > - .phys_io = 0x40000000, > - .io_pg_offst = (io_p2v(0x40000000) >> 18) & 0xfffc, > - .boot_params = 0xa0000100, > - .map_io = pxa_map_io, > - .init_irq = pxa27x_init_irq, > - .init_machine = magician_init, > - .timer = &pxa_timer, > + .class = &machine_class_pxa27x, > + .init_machine = magician_init, > MACHINE_END Magician can use PXA27X_MACHINE() as well, can't it? [...] Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 198 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100620/9cfddfda/attachment.sig> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] [ARM] pxa: make use of 'struct machine_class' 2010-06-20 16:08 ` Antonio Ospite @ 2010-06-21 1:06 ` Eric Miao 0 siblings, 0 replies; 10+ messages in thread From: Eric Miao @ 2010-06-21 1:06 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jun 21, 2010 at 12:08 AM, Antonio Ospite <ospite@studenti.unina.it> wrote: > On Sun, 20 Jun 2010 22:00:50 +0800 > Eric Miao <eric.miao@canonical.com> wrote: > >> From: Eric Miao <eric.y.miao@gmail.com> >> >> Signed-off-by: Eric Miao <eric.miao@canonical.com> >> --- >> ?arch/arm/mach-pxa/ezx.c ? ? ?| ? 60 ++++------------------------------------- > > The ezx part looks OK. A comment about magician below. > > [...] >> diff --git a/arch/arm/mach-pxa/magician.c b/arch/arm/mach-pxa/magician.c >> index e81dd0c..6a86f22 100644 >> --- a/arch/arm/mach-pxa/magician.c >> +++ b/arch/arm/mach-pxa/magician.c >> @@ -764,11 +764,6 @@ static void __init magician_init(void) >> >> >> ?MACHINE_START(MAGICIAN, "HTC Magician") >> - ? ? .phys_io = 0x40000000, >> - ? ? .io_pg_offst = (io_p2v(0x40000000) >> 18) & 0xfffc, >> - ? ? .boot_params = 0xa0000100, >> - ? ? .map_io = pxa_map_io, >> - ? ? .init_irq = pxa27x_init_irq, >> - ? ? .init_machine = magician_init, >> - ? ? .timer = &pxa_timer, >> + ? ? .class ? ? ? ? ?= &machine_class_pxa27x, >> + ? ? .init_machine ? = magician_init, >> ?MACHINE_END > > Magician can use PXA27X_MACHINE() as well, can't it? Yes, it can. Missed that :-) ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] [ARM] Introduce 'struct machine_class' for SoC level abstraction 2010-06-20 14:00 [PATCH 1/2] [ARM] Introduce 'struct machine_class' for SoC level abstraction Eric Miao 2010-06-20 14:00 ` [PATCH 2/2] [ARM] pxa: make use of 'struct machine_class' Eric Miao @ 2010-06-21 7:27 ` Jeremy Kerr 2010-06-21 8:23 ` Eric Miao 1 sibling, 1 reply; 10+ messages in thread From: Jeremy Kerr @ 2010-06-21 7:27 UTC (permalink / raw) To: linux-arm-kernel Hi Eric, > For machine class (mostly equivalent to SoC) level abstraction. Could you detail your intended usage of machine_class and how this will fit in with the mdesc? Will the mdesc fields override the machine_class fields, or is this intended to be a replacement? Cheers, Jeremy ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] [ARM] Introduce 'struct machine_class' for SoC level abstraction 2010-06-21 7:27 ` [PATCH 1/2] [ARM] Introduce 'struct machine_class' for SoC level abstraction Jeremy Kerr @ 2010-06-21 8:23 ` Eric Miao 2010-06-21 9:02 ` Russell King - ARM Linux 0 siblings, 1 reply; 10+ messages in thread From: Eric Miao @ 2010-06-21 8:23 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jun 21, 2010 at 3:27 PM, Jeremy Kerr <jeremy.kerr@canonical.com> wrote: > Hi Eric, > >> For machine class (mostly equivalent to SoC) level abstraction. > > Could you detail your intended usage of machine_class and how this will fit in > with the mdesc? Will the mdesc fields override the machine_class fields, or is > this intended to be a replacement? > Good question, I've missed that part in the commit message. So my idea is to have machine_class fields to replace machine_desc completely in the end, but the situation is we need a smooth transition: 1. so for newly added machines with machine_class pointer, the fields in machine_class will dominate (ignoring the fields in machine_desc) 2. for legacy machine_desc (apparently without machine_class pointer), the fields within will be used, and these machines are not affected So my intention of machine_class is really a class of machines, instead of the class concept in C++. There might be useful situations for derivative, E.g. some PXA machines have their specific map_io, mainstone: static struct map_desc mainstone_io_desc[] __initdata = { { /* CPLD */ .virtual = MST_FPGA_VIRT, .pfn = __phys_to_pfn(MST_FPGA_PHYS), .length = 0x00100000, .type = MT_DEVICE } }; static void __init mainstone_map_io(void) { pxa_map_io(); iotable_init(mainstone_io_desc, ARRAY_SIZE(mainstone_io_desc)); } So that from derivative POV, it can be expressed as: if (mdesc->class && mdesc->class->map_io) mdesc->class->map_io(); if (mdesc->map_io()) mdesc->map_io(); But this seems to make things a bit over complicated, so for such cases I'd prefer: 1. have a separate machine_class for mainstone 2. or move the board specific map_io() separatedly ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] [ARM] Introduce 'struct machine_class' for SoC level abstraction 2010-06-21 8:23 ` Eric Miao @ 2010-06-21 9:02 ` Russell King - ARM Linux 2010-06-21 10:19 ` Eric Miao 0 siblings, 1 reply; 10+ messages in thread From: Russell King - ARM Linux @ 2010-06-21 9:02 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jun 21, 2010 at 04:23:22PM +0800, Eric Miao wrote: > So that from derivative POV, it can be expressed as: > > if (mdesc->class && mdesc->class->map_io) > mdesc->class->map_io(); > if (mdesc->map_io()) > mdesc->map_io(); > > But this seems to make things a bit over complicated, so for such cases > I'd prefer: > > 1. have a separate machine_class for mainstone > 2. or move the board specific map_io() separatedly How about making the presence of mdesc->map_io override the class version? So if mainstone provides its own map_io() function, it is still responsible (as is today) for calling the PXA specific map_io(). The same behaviour seems sensible to apply to the other class functions as well - allow platforms to override the class version, and leave the replacement platform function responsible for calling the class if that's what it needs to do. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] [ARM] Introduce 'struct machine_class' for SoC level abstraction 2010-06-21 9:02 ` Russell King - ARM Linux @ 2010-06-21 10:19 ` Eric Miao 2010-06-21 15:38 ` Nicolas Pitre 0 siblings, 1 reply; 10+ messages in thread From: Eric Miao @ 2010-06-21 10:19 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jun 21, 2010 at 5:02 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Mon, Jun 21, 2010 at 04:23:22PM +0800, Eric Miao wrote: >> So that from derivative POV, it can be expressed as: >> >> ? ? ? if (mdesc->class && mdesc->class->map_io) >> ? ? ? ? ? ? ? mdesc->class->map_io(); >> ? ? ? if (mdesc->map_io()) >> ? ? ? ? ? ? ? mdesc->map_io(); >> >> But this seems to make things a bit over complicated, so for such cases >> I'd prefer: >> >> 1. have a separate machine_class for mainstone >> 2. or move the board specific map_io() separatedly > > How about making the presence of mdesc->map_io override the class > version? > > So if mainstone provides its own map_io() function, it is still > responsible (as is today) for calling the PXA specific map_io(). > The same behaviour seems sensible to apply to the other class > functions as well - allow platforms to override the class > version, and leave the replacement platform function responsible > for calling the class if that's what it needs to do. > Yep, that's a good idea, and actually was as my first version. Yet my concern is this doesn't easily fit for the class data, i.e., allow the machine specific data to override the class data like below: if (valid_value(mdesc->phys_io)) use(mdesc->phys_io); else use(mdesc->class->phys_io); but since phys_io could be valid for value zero (as if not explicitly initialized), so we may end up defining like below to allow use of the default class values: MACHINE_START(....) .phys_io = INVALID_ADDRESS, .io_pg_offst = INVALID_ADDRESS, .... MACHINE_END This, however, creates some incompatiblity in semantics from the functions. That's why I chose a much simpler scheme: use the class version if class is there, otherwise use the machine specific one. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] [ARM] Introduce 'struct machine_class' for SoC level abstraction 2010-06-21 10:19 ` Eric Miao @ 2010-06-21 15:38 ` Nicolas Pitre 2010-06-21 15:44 ` Eric Miao 0 siblings, 1 reply; 10+ messages in thread From: Nicolas Pitre @ 2010-06-21 15:38 UTC (permalink / raw) To: linux-arm-kernel On Mon, 21 Jun 2010, Eric Miao wrote: > On Mon, Jun 21, 2010 at 5:02 PM, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: > > The same behaviour seems sensible to apply to the other class > > functions as well - allow platforms to override the class > > version, and leave the replacement platform function responsible > > for calling the class if that's what it needs to do. > > > > Yep, that's a good idea, and actually was as my first version. Yet my > concern is this doesn't easily fit for the class data, i.e., allow the > machine specific data to override the class data like below: > > if (valid_value(mdesc->phys_io)) > use(mdesc->phys_io); > else > use(mdesc->class->phys_io); > > but since phys_io could be valid for value zero (as if not explicitly > initialized), so we may end up defining like below to allow use of the > default class values: > > MACHINE_START(....) > .phys_io = INVALID_ADDRESS, > .io_pg_offst = INVALID_ADDRESS, > .... > MACHINE_END No, please don't do that. This is horribly ugly. And in fact I would actually be highly surprised if 0 was a sensible value to use for either of those fields anyway. No SOCs I know about has its IO at physical address0, and we're even less likely to remap them at virtual address 0 either. So having 0 meaning uninitialized makes perfect sense in practice. Nicolas ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] [ARM] Introduce 'struct machine_class' for SoC level abstraction 2010-06-21 15:38 ` Nicolas Pitre @ 2010-06-21 15:44 ` Eric Miao 0 siblings, 0 replies; 10+ messages in thread From: Eric Miao @ 2010-06-21 15:44 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jun 21, 2010 at 11:38 PM, Nicolas Pitre <nico@fluxnic.net> wrote: > On Mon, 21 Jun 2010, Eric Miao wrote: > >> On Mon, Jun 21, 2010 at 5:02 PM, Russell King - ARM Linux >> <linux@arm.linux.org.uk> wrote: >> > The same behaviour seems sensible to apply to the other class >> > functions as well - allow platforms to override the class >> > version, and leave the replacement platform function responsible >> > for calling the class if that's what it needs to do. >> > >> >> Yep, that's a good idea, and actually was as my first version. Yet my >> concern is this doesn't easily fit for the class data, i.e., allow the >> machine specific data to override the class data like below: >> >> ? ? ? if (valid_value(mdesc->phys_io)) >> ? ? ? ? ? ? ? use(mdesc->phys_io); >> ? ? ? else >> ? ? ? ? ? ? ? use(mdesc->class->phys_io); >> >> but since phys_io could be valid for value zero (as if not explicitly >> initialized), so we may end up defining like below to allow use of the >> default class values: >> >> MACHINE_START(....) >> ? ? ? .phys_io ? ? ? ?= INVALID_ADDRESS, >> ? ? ? .io_pg_offst ? ?= INVALID_ADDRESS, >> ? ? ? .... >> MACHINE_END > > No, please don't do that. ?This is horribly ugly. > > And in fact I would actually be highly surprised if 0 was a sensible > value to use for either of those fields anyway. ?No SOCs I know about > has its IO at physical address0, and we're even less likely to remap > them at virtual address 0 either. ?So having 0 meaning uninitialized > makes perfect sense in practice. > Hrm... that makes sense. I'll add some comment to the 'struct machine_class' and get this patch updated later. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-06-21 15:44 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-06-20 14:00 [PATCH 1/2] [ARM] Introduce 'struct machine_class' for SoC level abstraction Eric Miao 2010-06-20 14:00 ` [PATCH 2/2] [ARM] pxa: make use of 'struct machine_class' Eric Miao 2010-06-20 16:08 ` Antonio Ospite 2010-06-21 1:06 ` Eric Miao 2010-06-21 7:27 ` [PATCH 1/2] [ARM] Introduce 'struct machine_class' for SoC level abstraction Jeremy Kerr 2010-06-21 8:23 ` Eric Miao 2010-06-21 9:02 ` Russell King - ARM Linux 2010-06-21 10:19 ` Eric Miao 2010-06-21 15:38 ` Nicolas Pitre 2010-06-21 15:44 ` Eric Miao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).