| Message ID | 1318539963-3329-4-git-send-email-sjg@chromium.org |
|---|---|
| State | New, archived |
| Headers | show |
Le 13/10/2011 23:05, Simon Glass a écrit : > Some SOCs have do not start up with their 'main' CPU. The first U-Boot > code may then be executed with a CPU which does not have a CP15, or not a > useful one. > > Here we split the initialization of CP15 into a separate call, which can > be performed later if required. > > Once the main CPU is running, you should call cpu_init_cp15() to perform > this init as early as possible. > > Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not > need to change, this CP15 init is still skipped in that case. The only > impact for these boards is that the cpu_init_cp15() will be available > even if it is never used on these boards. I'm not too sure I understand how this is working: if you are moving cp15 init to later, it will still be done by the same core that would have done it earlier, won't it? IOW, I would like to better understand how this boot core/main core business works. How does the main core start execution? At which address? Does it go through its reset vector? Do the cores share the same location for reset vectors? Etc. Amicalement,
Hi Albert, On Fri, Oct 21, 2011 at 1:58 PM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Le 13/10/2011 23:05, Simon Glass a écrit : >> >> Some SOCs have do not start up with their 'main' CPU. The first U-Boot >> code may then be executed with a CPU which does not have a CP15, or not a >> useful one. >> >> Here we split the initialization of CP15 into a separate call, which can >> be performed later if required. >> >> Once the main CPU is running, you should call cpu_init_cp15() to perform >> this init as early as possible. >> >> Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not >> need to change, this CP15 init is still skipped in that case. The only >> impact for these boards is that the cpu_init_cp15() will be available >> even if it is never used on these boards. > > I'm not too sure I understand how this is working: if you are moving cp15 > init to later, it will still be done by the same core that would have done > it earlier, won't it? > > IOW, I would like to better understand how this boot core/main core business > works. How does the main core start execution? At which address? Does it go > through its reset vector? Do the cores share the same location for reset > vectors? Etc. For Tegra is it like this: The ARM7 CPU (called AVP for Audio Video Processor) starts up first. It runs the boot ROM and then U-Boot and gets as far as arch_cpu_init(). The AVP does not have a CP15 or a cache so cannot run the CP15 init code. The AVP then starts up the first Cortex-A9 (an ARMv7 architecture CPU). This CPU (the main core, if you like) starts from the same address as the first one (i.e. the start of U-Boot). It is as if this is the core that we really wanted to use, but it wasn't available initially. This main core runs through arch_cpu_init() and sails into board_init_f(). At this point no CP15 init has been done. Basically there is now an assumption in start.S that the current CPU has a CP15 and that it is useful to fiddle with it. By splitting out that code into a separate function we can: 1. Avoid the AVP fiddling with CP15 2. Have the Cotex-A9 (the intended target of the code) get its CP15 init when needed Note that all the cores have a unified view of memory, peripherals, etc. Regards, Simon > > Amicalement, > -- > Albert. >
Le 21/10/2011 23:18, Simon Glass a écrit : > Hi Albert, > > On Fri, Oct 21, 2011 at 1:58 PM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: >> Le 13/10/2011 23:05, Simon Glass a écrit : >>> >>> Some SOCs have do not start up with their 'main' CPU. The first U-Boot >>> code may then be executed with a CPU which does not have a CP15, or not a >>> useful one. >>> >>> Here we split the initialization of CP15 into a separate call, which can >>> be performed later if required. >>> >>> Once the main CPU is running, you should call cpu_init_cp15() to perform >>> this init as early as possible. >>> >>> Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not >>> need to change, this CP15 init is still skipped in that case. The only >>> impact for these boards is that the cpu_init_cp15() will be available >>> even if it is never used on these boards. >> >> I'm not too sure I understand how this is working: if you are moving cp15 >> init to later, it will still be done by the same core that would have done >> it earlier, won't it? >> >> IOW, I would like to better understand how this boot core/main core business >> works. How does the main core start execution? At which address? Does it go >> through its reset vector? Do the cores share the same location for reset >> vectors? Etc. > > For Tegra is it like this: > > The ARM7 CPU (called AVP for Audio Video Processor) starts up first. > It runs the boot ROM and then U-Boot and gets as far as > arch_cpu_init(). The AVP does not have a CP15 or a cache so cannot run > the CP15 init code. The AVP then starts up the first Cortex-A9 (an > ARMv7 architecture CPU). This CPU (the main core, if you like) starts > from the same address as the first one (i.e. the start of U-Boot). It > is as if this is the core that we really wanted to use, but it wasn't > available initially. This main core runs through arch_cpu_init() and > sails into board_init_f(). At this point no CP15 init has been done. Thanks. So what this amounts to is, both cores will run the same binary, and I assume the AVP will shut itself off once the A9 runs. But what I don't get is, if A9 goes through the same sequence of code as AVP, then it will execute cp15 where is is not, won't it? Anyway: your patch moves cp15 init far enough that the AVP won't execute it, but the A9 will. Only, what will happen when another multiple-core ARM SoC gets U-Boot support but the location you chose for cp15 init is inadequate for it? Shall we move cp15 init again, and where? I'd prefer the cp15 init to stay where it is but execute only for A9, for instance by checking some core identification register. > Basically there is now an assumption in start.S that the current CPU > has a CP15 and that it is useful to fiddle with it. By splitting out > that code into a separate function we can: > > 1. Avoid the AVP fiddling with CP15 > 2. Have the Cotex-A9 (the intended target of the code) get its CP15 > init when needed > > Note that all the cores have a unified view of memory, peripherals, etc. > > Regards, > Simon > >> >> Amicalement, >> -- >> Albert. >> > > Amicalement,
Hi Albert, On Fri, Oct 21, 2011 at 2:43 PM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Le 21/10/2011 23:18, Simon Glass a écrit : >> >> Hi Albert, >> >> On Fri, Oct 21, 2011 at 1:58 PM, Albert ARIBAUD >> <albert.u.boot@aribaud.net> wrote: >>> >>> Le 13/10/2011 23:05, Simon Glass a écrit : >>>> >>>> Some SOCs have do not start up with their 'main' CPU. The first U-Boot >>>> code may then be executed with a CPU which does not have a CP15, or not >>>> a >>>> useful one. >>>> >>>> Here we split the initialization of CP15 into a separate call, which can >>>> be performed later if required. >>>> >>>> Once the main CPU is running, you should call cpu_init_cp15() to perform >>>> this init as early as possible. >>>> >>>> Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not >>>> need to change, this CP15 init is still skipped in that case. The only >>>> impact for these boards is that the cpu_init_cp15() will be available >>>> even if it is never used on these boards. >>> >>> I'm not too sure I understand how this is working: if you are moving cp15 >>> init to later, it will still be done by the same core that would have >>> done >>> it earlier, won't it? >>> >>> IOW, I would like to better understand how this boot core/main core >>> business >>> works. How does the main core start execution? At which address? Does it >>> go >>> through its reset vector? Do the cores share the same location for reset >>> vectors? Etc. >> >> For Tegra is it like this: >> >> The ARM7 CPU (called AVP for Audio Video Processor) starts up first. >> It runs the boot ROM and then U-Boot and gets as far as >> arch_cpu_init(). The AVP does not have a CP15 or a cache so cannot run >> the CP15 init code. The AVP then starts up the first Cortex-A9 (an >> ARMv7 architecture CPU). This CPU (the main core, if you like) starts >> from the same address as the first one (i.e. the start of U-Boot). It >> is as if this is the core that we really wanted to use, but it wasn't >> available initially. This main core runs through arch_cpu_init() and >> sails into board_init_f(). At this point no CP15 init has been done. > > Thanks. So what this amounts to is, both cores will run the same binary, and > I assume the AVP will shut itself off once the A9 runs. But what I don't get Yes > is, if A9 goes through the same sequence of code as AVP, then it will > execute cp15 where is is not, won't it? On Tegra we have CONFIG_SKIP_LOWLEVEL_INIT set to avoid this. That much is already supported by U-Boot. > > Anyway: your patch moves cp15 init far enough that the AVP won't execute it, > but the A9 will. Well, on Tegra we call the cp15 init directly when it is safe to do so, later. > > Only, what will happen when another multiple-core ARM SoC gets U-Boot > support but the location you chose for cp15 init is inadequate for it? Shall > we move cp15 init again, and where? If CONFIG_SKIP_LOWLEVEL_INIT is not defined, then the low level init will operate exactly as now. My patch effectively just allows you to have CONFIG_SKIP_LOWLEVEL_INIT but later call part of that lowlevel init. In my view, the cp15 init should not be lumped in with the 'memory init' or whatever else it expected to be done in the lowlevel init. > > I'd prefer the cp15 init to stay where it is but execute only for A9, for > instance by checking some core identification register. Well I actually haven't moved it! It is just that previously it was impossible to call cp15_init from anywhere later. What you say can be done, it would involve some assembler though and would need to be another CONFIG option. Was trying to avoid adding new assembler. Regards, Simon > >> Basically there is now an assumption in start.S that the current CPU >> has a CP15 and that it is useful to fiddle with it. By splitting out >> that code into a separate function we can: >> >> 1. Avoid the AVP fiddling with CP15 >> 2. Have the Cotex-A9 (the intended target of the code) get its CP15 >> init when needed >> >> Note that all the cores have a unified view of memory, peripherals, etc. >> >> Regards, >> Simon >> >>> >>> Amicalement, >>> -- >>> Albert. >>> >> >> > > > Amicalement, > -- > Albert. >
Le 21/10/2011 23:54, Simon Glass a écrit : > Hi Albert, > > On Fri, Oct 21, 2011 at 2:43 PM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: >> Le 21/10/2011 23:18, Simon Glass a écrit : >>> >>> Hi Albert, >>> >>> On Fri, Oct 21, 2011 at 1:58 PM, Albert ARIBAUD >>> <albert.u.boot@aribaud.net> wrote: >>>> >>>> Le 13/10/2011 23:05, Simon Glass a écrit : >>>>> >>>>> Some SOCs have do not start up with their 'main' CPU. The first U-Boot >>>>> code may then be executed with a CPU which does not have a CP15, or not >>>>> a >>>>> useful one. >>>>> >>>>> Here we split the initialization of CP15 into a separate call, which can >>>>> be performed later if required. >>>>> >>>>> Once the main CPU is running, you should call cpu_init_cp15() to perform >>>>> this init as early as possible. >>>>> >>>>> Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not >>>>> need to change, this CP15 init is still skipped in that case. The only >>>>> impact for these boards is that the cpu_init_cp15() will be available >>>>> even if it is never used on these boards. >>>> >>>> I'm not too sure I understand how this is working: if you are moving cp15 >>>> init to later, it will still be done by the same core that would have >>>> done >>>> it earlier, won't it? >>>> >>>> IOW, I would like to better understand how this boot core/main core >>>> business >>>> works. How does the main core start execution? At which address? Does it >>>> go >>>> through its reset vector? Do the cores share the same location for reset >>>> vectors? Etc. >>> >>> For Tegra is it like this: >>> >>> The ARM7 CPU (called AVP for Audio Video Processor) starts up first. >>> It runs the boot ROM and then U-Boot and gets as far as >>> arch_cpu_init(). The AVP does not have a CP15 or a cache so cannot run >>> the CP15 init code. The AVP then starts up the first Cortex-A9 (an >>> ARMv7 architecture CPU). This CPU (the main core, if you like) starts >>> from the same address as the first one (i.e. the start of U-Boot). It >>> is as if this is the core that we really wanted to use, but it wasn't >>> available initially. This main core runs through arch_cpu_init() and >>> sails into board_init_f(). At this point no CP15 init has been done. >> >> Thanks. So what this amounts to is, both cores will run the same binary, and >> I assume the AVP will shut itself off once the A9 runs. But what I don't get > > Yes > >> is, if A9 goes through the same sequence of code as AVP, then it will >> execute cp15 where is is not, won't it? > > On Tegra we have CONFIG_SKIP_LOWLEVEL_INIT set to avoid this. That > much is already supported by U-Boot. I still don't get it: if you have lowlevel init skipped for one core, it will be for the other as well. >> Anyway: your patch moves cp15 init far enough that the AVP won't execute it, >> but the A9 will. > > Well, on Tegra we call the cp15 init directly when it is safe to do so, later. > >> >> Only, what will happen when another multiple-core ARM SoC gets U-Boot >> support but the location you chose for cp15 init is inadequate for it? Shall >> we move cp15 init again, and where? > > If CONFIG_SKIP_LOWLEVEL_INIT is not defined, then the low level init > will operate exactly as now. My patch effectively just allows you to > have CONFIG_SKIP_LOWLEVEL_INIT but later call part of that lowlevel > init. In my view, the cp15 init should not be lumped in with the > 'memory init' or whatever else it expected to be done in the lowlevel > init. I think I get the point: due to the fact that both cores run the same starup code path, and due to the fact that they run it one after the other and share a lot of devices, at mots one of them shoud perform low level inits (e.g. RAM init), and some low-level inits can only be performed by one of them (e.g., cp15). >> I'd prefer the cp15 init to stay where it is but execute only for A9, for >> instance by checking some core identification register. > > Well I actually haven't moved it! It is just that previously it was > impossible to call cp15_init from anywhere later. It is moved, in that it belongs to low level init... of A9. > What you say can be done, it would involve some assembler though and > would need to be another CONFIG option. Was trying to avoid adding new > assembler. Low level init is about assembler and, well, low level. :) But I don't see why there should be another CONFIG option. IIUC, you should be able to do with only CONFIG_SKIP_LOWLEVEL_INIT: within the low level init code under it, you would do the equivalent of a switch, with one branch for AVM (and DDR init etc) and one branch for A9 (with cp15 init etc). > Regards, > Simon Amicalement,
Hi Albert, On Fri, Oct 21, 2011 at 3:24 PM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Le 21/10/2011 23:54, Simon Glass a écrit : >> >> Hi Albert, >> >> On Fri, Oct 21, 2011 at 2:43 PM, Albert ARIBAUD >> <albert.u.boot@aribaud.net> wrote: >>> >>> Le 21/10/2011 23:18, Simon Glass a écrit : >>>> >>>> Hi Albert, >>>> >>>> On Fri, Oct 21, 2011 at 1:58 PM, Albert ARIBAUD >>>> <albert.u.boot@aribaud.net> wrote: >>>>> >>>>> Le 13/10/2011 23:05, Simon Glass a écrit : >>>>>> >>>>>> Some SOCs have do not start up with their 'main' CPU. The first U-Boot >>>>>> code may then be executed with a CPU which does not have a CP15, or >>>>>> not >>>>>> a >>>>>> useful one. >>>>>> >>>>>> Here we split the initialization of CP15 into a separate call, which >>>>>> can >>>>>> be performed later if required. >>>>>> >>>>>> Once the main CPU is running, you should call cpu_init_cp15() to >>>>>> perform >>>>>> this init as early as possible. >>>>>> >>>>>> Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should >>>>>> not >>>>>> need to change, this CP15 init is still skipped in that case. The only >>>>>> impact for these boards is that the cpu_init_cp15() will be available >>>>>> even if it is never used on these boards. >>>>> >>>>> I'm not too sure I understand how this is working: if you are moving >>>>> cp15 >>>>> init to later, it will still be done by the same core that would have >>>>> done >>>>> it earlier, won't it? >>>>> >>>>> IOW, I would like to better understand how this boot core/main core >>>>> business >>>>> works. How does the main core start execution? At which address? Does >>>>> it >>>>> go >>>>> through its reset vector? Do the cores share the same location for >>>>> reset >>>>> vectors? Etc. >>>> >>>> For Tegra is it like this: >>>> >>>> The ARM7 CPU (called AVP for Audio Video Processor) starts up first. >>>> It runs the boot ROM and then U-Boot and gets as far as >>>> arch_cpu_init(). The AVP does not have a CP15 or a cache so cannot run >>>> the CP15 init code. The AVP then starts up the first Cortex-A9 (an >>>> ARMv7 architecture CPU). This CPU (the main core, if you like) starts >>>> from the same address as the first one (i.e. the start of U-Boot). It >>>> is as if this is the core that we really wanted to use, but it wasn't >>>> available initially. This main core runs through arch_cpu_init() and >>>> sails into board_init_f(). At this point no CP15 init has been done. >>> >>> Thanks. So what this amounts to is, both cores will run the same binary, >>> and >>> I assume the AVP will shut itself off once the A9 runs. But what I don't >>> get >> >> Yes >> >>> is, if A9 goes through the same sequence of code as AVP, then it will >>> execute cp15 where is is not, won't it? >> >> On Tegra we have CONFIG_SKIP_LOWLEVEL_INIT set to avoid this. That >> much is already supported by U-Boot. > > I still don't get it: if you have lowlevel init skipped for one core, it > will be for the other as well. OK so let's run through the U-Boot code in order for each of the two CPUs: First AVP: _start skips lowlevel init (so no CP15) board_init_f arch_cpu_init (at this point it starts up the A9 and halts) A9: _start skips lowlevel init (so no CP15) board_init_f arch_cpu_init cpu_init_cp15 The call to cpu_init_cp15() is performed in arch_cpu_init() when we detect that we are the A9. > >>> Anyway: your patch moves cp15 init far enough that the AVP won't execute >>> it, >>> but the A9 will. >> >> Well, on Tegra we call the cp15 init directly when it is safe to do so, >> later. >> >>> >>> Only, what will happen when another multiple-core ARM SoC gets U-Boot >>> support but the location you chose for cp15 init is inadequate for it? >>> Shall >>> we move cp15 init again, and where? >> >> If CONFIG_SKIP_LOWLEVEL_INIT is not defined, then the low level init >> will operate exactly as now. My patch effectively just allows you to >> have CONFIG_SKIP_LOWLEVEL_INIT but later call part of that lowlevel >> init. In my view, the cp15 init should not be lumped in with the >> 'memory init' or whatever else it expected to be done in the lowlevel >> init. > > I think I get the point: due to the fact that both cores run the same starup > code path, and due to the fact that they run it one after the other and > share a lot of devices, at mots one of them shoud perform low level inits > (e.g. RAM init), and some low-level inits can only be performed by one of > them (e.g., cp15). Well on Tegra the RAM init is down by the boot ROM. There is no low level init bar the cp15 stuff. > >>> I'd prefer the cp15 init to stay where it is but execute only for A9, for >>> instance by checking some core identification register. >> >> Well I actually haven't moved it! It is just that previously it was >> impossible to call cp15_init from anywhere later. > > It is moved, in that it belongs to low level init... of A9. OK, I see - you mean moved in order if not in source code file. > >> What you say can be done, it would involve some assembler though and >> would need to be another CONFIG option. Was trying to avoid adding new >> assembler. > > Low level init is about assembler and, well, low level. :) Yes but it's yuck. Part of the clean-up is to remove most of the assembler - really very little is needed. > > But I don't see why there should be another CONFIG option. IIUC, you should > be able to do with only CONFIG_SKIP_LOWLEVEL_INIT: within the low level init > code under it, you would do the equivalent of a switch, with one branch for > AVM (and DDR init etc) and one branch for A9 (with cp15 init etc). Yes I can, but I need to be able to call cp15_init. And I can't do that because if CONFIG_SKIP_LOWLEVEL_INIT is defined, that code is compiled out! So I need to move that cp15_init code outside the CONFIG_SKIP_LOWLEVEL_INIT #ifdef. That is all I am trying to do, honest! Regards, Simon > > Amicalement, > -- > Albert. >
Le 22/10/2011 07:05, Simon Glass a écrit : >>> Well I actually haven't moved it! It is just that previously it was >>> impossible to call cp15_init from anywhere later. >> >> It is moved, in that it belongs to low level init... of A9. > > OK, I see - you mean moved in order if not in source code file. Yes. In the code, though, it belongs to low-level init. >>> What you say can be done, it would involve some assembler though and >>> would need to be another CONFIG option. Was trying to avoid adding new >>> assembler. >> >> Low level init is about assembler and, well, low level. :) > > Yes but it's yuck. Part of the clean-up is to remove most of the > assembler - really very little is needed. I don't think "yuck" is a valid reasoned argument against assembly language :) but I assume what you mainly mean is 'hard to understand and replaceable by easy to understand C code'. I agree to the first part, and to the second one for the general case, but not for the startup path where, precisely, we don't have a working C run-time and most of the understanding requires knowledge of hardware, not only general programming knowledge. Past that, C is ok -- but then beware: some C++ guy could chime in and contend that C is "yuck" by your own standards. :) >> But I don't see why there should be another CONFIG option. IIUC, you should >> be able to do with only CONFIG_SKIP_LOWLEVEL_INIT: within the low level init >> code under it, you would do the equivalent of a switch, with one branch for >> AVM (and DDR init etc) and one branch for A9 (with cp15 init etc). > > Yes I can, but I need to be able to call cp15_init. And I can't do > that because if CONFIG_SKIP_LOWLEVEL_INIT is defined, that code is > compiled out! So I need to move that cp15_init code outside the > CONFIG_SKIP_LOWLEVEL_INIT #ifdef. That is all I am trying to do, > honest! I understand, and I think the approach here is wrong, because you *do* want low-level init in the A9 case, and thus you should not have CONFIG_SKIP_LOWLEVEL_INIT set. But you also need the AVP low-level init to be empty, and the AVP and A9 must follow the same startup path. And all this it not necessarily required for all armv7 platforms, but some of them will want lowlevel init, and some not. Considering all this... For now, I would opt for a CONFIG_ARCH_GENERIC_LOWLEVEL_INIT option which start.S would use to compile out the low level init code definition (like CONFIG_SKIP_LOWLEVEL_INIT does) but not the the calls to it (UNline CONFIG_SKIP_LOWLEVEL_INIT does). Then you could provide a SoC-specific version of lowlevel_init. In the longer term, I would be more in favor of a weak definition system with hierarchical ARCH, ISA, SoC, board priority order (yes, I am kind of fond of it) for lowlevel init -- but that will require looking into asm weak symbol handling and may require splitting of the assembly code function by function. > Regards, > Simon Amicalement,
Hi Albert, On Sat, Oct 22, 2011 at 12:56 AM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Le 22/10/2011 07:05, Simon Glass a écrit : > >>>> Well I actually haven't moved it! It is just that previously it was >>>> impossible to call cp15_init from anywhere later. >>> >>> It is moved, in that it belongs to low level init... of A9. >> >> OK, I see - you mean moved in order if not in source code file. > > Yes. In the code, though, it belongs to low-level init. Arguably this could go in a library like arch/arm/cpu/armv7/lib/cache.S > >>>> What you say can be done, it would involve some assembler though and >>>> would need to be another CONFIG option. Was trying to avoid adding new >>>> assembler. >>> >>> Low level init is about assembler and, well, low level. :) >> >> Yes but it's yuck. Part of the clean-up is to remove most of the >> assembler - really very little is needed. > > I don't think "yuck" is a valid reasoned argument against assembly language > :) but I assume what you mainly mean is 'hard to understand and replaceable > by easy to understand C code'. I agree to the first part, and to the second > one for the general case, but not for the startup path where, precisely, we > don't have a working C run-time and most of the understanding requires > knowledge of hardware, not only general programming knowledge. Past that, C I'm not arguing for the cache init stuff to move to C, just trying to avoid any creeping growth of assembler when C can do it. > is ok -- but then beware: some C++ guy could chime in and contend that C is > "yuck" by your own standards. :) The C++ guy who finds himself in U-Boot is probably lost :-) > >>> But I don't see why there should be another CONFIG option. IIUC, you >>> should >>> be able to do with only CONFIG_SKIP_LOWLEVEL_INIT: within the low level >>> init >>> code under it, you would do the equivalent of a switch, with one branch >>> for >>> AVM (and DDR init etc) and one branch for A9 (with cp15 init etc). >> >> Yes I can, but I need to be able to call cp15_init. And I can't do >> that because if CONFIG_SKIP_LOWLEVEL_INIT is defined, that code is >> compiled out! So I need to move that cp15_init code outside the >> CONFIG_SKIP_LOWLEVEL_INIT #ifdef. That is all I am trying to do, >> honest! > > I understand, and I think the approach here is wrong, because you *do* want > low-level init in the A9 case, and thus you should not have > CONFIG_SKIP_LOWLEVEL_INIT set. But you also need the AVP low-level init to > be empty, and the AVP and A9 must follow the same startup path. And all this > it not necessarily required for all armv7 platforms, but some of them will > want lowlevel init, and some not. Considering all this... > > For now, I would opt for a CONFIG_ARCH_GENERIC_LOWLEVEL_INIT option which > start.S would use to compile out the low level init code definition (like > CONFIG_SKIP_LOWLEVEL_INIT does) but not the the calls to it (UNline > CONFIG_SKIP_LOWLEVEL_INIT does). Then you could provide a SoC-specific > version of lowlevel_init. If I understand you correctly, this is the opposite of what I want. It is basically what we have now. All ARMv7 chips are going to turn off MMU, caches, flush TLBs, and the like - this is an architecture feature, not an SOC one. We have conflated ARM arch init with SOC init as you say, but the solution is not to force the SOC to copy code from start.S just so that it can do ARM arch init later. And I'd argue for LOWLEVEL_INIT being about setting up the memory so we can run code, and in particular relocate it later. The cp15_init stuff should arguably be in arch_cpu_init() (or perhaps a new arch_init()) as the first thing called from board_init_f() in my view. So how about I create a patch to move the cp15_init() code into arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later? > > In the longer term, I would be more in favor of a weak definition system > with hierarchical ARCH, ISA, SoC, board priority order (yes, I am kind of > fond of it) for lowlevel init -- but that will require looking into asm weak > symbol handling and may require splitting of the assembly code function by > function. I think you are trying to avoid the #ifdef CONFIG_ARCH_CPU_INIT arch_cpu_init, #endif #ifdef ... some_other_thing #endif in the board_init_f() function table. Yes I agree it would tidy things up, although I wonder if Graeme's initcall thing isn't better again. It has the same confusion level coefficient (only the linker decides what code is included) with a bit more flexibility. Then each arch / board / cpu / soc / squirrel can decide on its own init and put it in its own file. C only please :-) Regards, Simon > >> Regards, >> Simon > > Amicalement, > -- > Albert. >
Hi Albert, On Sat, Oct 22, 2011 at 9:13 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Albert, > > On Sat, Oct 22, 2011 at 12:56 AM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: >> Le 22/10/2011 07:05, Simon Glass a écrit : >> >>>>> Well I actually haven't moved it! It is just that previously it was >>>>> impossible to call cp15_init from anywhere later. >>>> >>>> It is moved, in that it belongs to low level init... of A9. >>> >>> OK, I see - you mean moved in order if not in source code file. >> >> Yes. In the code, though, it belongs to low-level init. > > Arguably this could go in a library like arch/arm/cpu/armv7/lib/cache.S > >> >>>>> What you say can be done, it would involve some assembler though and >>>>> would need to be another CONFIG option. Was trying to avoid adding new >>>>> assembler. >>>> >>>> Low level init is about assembler and, well, low level. :) >>> >>> Yes but it's yuck. Part of the clean-up is to remove most of the >>> assembler - really very little is needed. >> >> I don't think "yuck" is a valid reasoned argument against assembly language >> :) but I assume what you mainly mean is 'hard to understand and replaceable >> by easy to understand C code'. I agree to the first part, and to the second >> one for the general case, but not for the startup path where, precisely, we >> don't have a working C run-time and most of the understanding requires >> knowledge of hardware, not only general programming knowledge. Past that, C > > I'm not arguing for the cache init stuff to move to C, just trying to > avoid any creeping growth of assembler when C can do it. > >> is ok -- but then beware: some C++ guy could chime in and contend that C is >> "yuck" by your own standards. :) > > The C++ guy who finds himself in U-Boot is probably lost :-) > >> >>>> But I don't see why there should be another CONFIG option. IIUC, you >>>> should >>>> be able to do with only CONFIG_SKIP_LOWLEVEL_INIT: within the low level >>>> init >>>> code under it, you would do the equivalent of a switch, with one branch >>>> for >>>> AVM (and DDR init etc) and one branch for A9 (with cp15 init etc). >>> >>> Yes I can, but I need to be able to call cp15_init. And I can't do >>> that because if CONFIG_SKIP_LOWLEVEL_INIT is defined, that code is >>> compiled out! So I need to move that cp15_init code outside the >>> CONFIG_SKIP_LOWLEVEL_INIT #ifdef. That is all I am trying to do, >>> honest! >> >> I understand, and I think the approach here is wrong, because you *do* want >> low-level init in the A9 case, and thus you should not have >> CONFIG_SKIP_LOWLEVEL_INIT set. But you also need the AVP low-level init to >> be empty, and the AVP and A9 must follow the same startup path. And all this >> it not necessarily required for all armv7 platforms, but some of them will >> want lowlevel init, and some not. Considering all this... >> >> For now, I would opt for a CONFIG_ARCH_GENERIC_LOWLEVEL_INIT option which >> start.S would use to compile out the low level init code definition (like >> CONFIG_SKIP_LOWLEVEL_INIT does) but not the the calls to it (UNline >> CONFIG_SKIP_LOWLEVEL_INIT does). Then you could provide a SoC-specific >> version of lowlevel_init. > > If I understand you correctly, this is the opposite of what I want. It > is basically what we have now. > > All ARMv7 chips are going to turn off MMU, caches, flush TLBs, and the > like - this is an architecture feature, not an SOC one. We have > conflated ARM arch init with SOC init as you say, but the solution is > not to force the SOC to copy code from start.S just so that it can do > ARM arch init later. > > And I'd argue for LOWLEVEL_INIT being about setting up the memory so > we can run code, and in particular relocate it later. The cp15_init > stuff should arguably be in arch_cpu_init() (or perhaps a new > arch_init()) as the first thing called from board_init_f() in my view. > > So how about I create a patch to move the cp15_init() code into > arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later? I had a look at this, and it is a bit of a pain since the SPL builds all bring in files by name from random directories around U-Boot, including arch/arm/cpu/armv7/start.S. If I move code out of that file into lib.S or cp15.S then I need to fix up various SPL builds. But it seems to me that it would be better for arch/arm/cpu/armv7/Makefile (another other Makefiles) to specify what object files are needed for an SPL build, rather than putting it in *_spl/Makefile. Therefore I have left this along, and would like proceed as previously! I will send an updated patch series when MAKEALL finishes. Regards, Simon > >> >> In the longer term, I would be more in favor of a weak definition system >> with hierarchical ARCH, ISA, SoC, board priority order (yes, I am kind of >> fond of it) for lowlevel init -- but that will require looking into asm weak >> symbol handling and may require splitting of the assembly code function by >> function. > > I think you are trying to avoid the > > #ifdef CONFIG_ARCH_CPU_INIT > arch_cpu_init, > #endif > #ifdef ... > some_other_thing > #endif > > in the board_init_f() function table. Yes I agree it would tidy things > up, although I wonder if Graeme's initcall thing isn't better again. > It has the same confusion level coefficient (only the linker decides > what code is included) with a bit more flexibility. Then each arch / > board / cpu / soc / squirrel can decide on its own init and put it in > its own file. C only please :-) > > Regards, > Simon > >> >>> Regards, >>> Simon >> >> Amicalement, >> -- >> Albert. >> >
Le 22/10/2011 18:13, Simon Glass a écrit : > Hi Albert, > > On Sat, Oct 22, 2011 at 12:56 AM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: >> Le 22/10/2011 07:05, Simon Glass a écrit : >> >>>>> Well I actually haven't moved it! It is just that previously it was >>>>> impossible to call cp15_init from anywhere later. >>>> >>>> It is moved, in that it belongs to low level init... of A9. >>> >>> OK, I see - you mean moved in order if not in source code file. >> >> Yes. In the code, though, it belongs to low-level init. > > Arguably this could go in a library like arch/arm/cpu/armv7/lib/cache.S > >> >>>>> What you say can be done, it would involve some assembler though and >>>>> would need to be another CONFIG option. Was trying to avoid adding new >>>>> assembler. >>>> >>>> Low level init is about assembler and, well, low level. :) >>> >>> Yes but it's yuck. Part of the clean-up is to remove most of the >>> assembler - really very little is needed. >> >> I don't think "yuck" is a valid reasoned argument against assembly language >> :) but I assume what you mainly mean is 'hard to understand and replaceable >> by easy to understand C code'. I agree to the first part, and to the second >> one for the general case, but not for the startup path where, precisely, we >> don't have a working C run-time and most of the understanding requires >> knowledge of hardware, not only general programming knowledge. Past that, C > > I'm not arguing for the cache init stuff to move to C, just trying to > avoid any creeping growth of assembler when C can do it. > >> is ok -- but then beware: some C++ guy could chime in and contend that C is >> "yuck" by your own standards. :) > > The C++ guy who finds himself in U-Boot is probably lost :-) > >> >>>> But I don't see why there should be another CONFIG option. IIUC, you >>>> should >>>> be able to do with only CONFIG_SKIP_LOWLEVEL_INIT: within the low level >>>> init >>>> code under it, you would do the equivalent of a switch, with one branch >>>> for >>>> AVM (and DDR init etc) and one branch for A9 (with cp15 init etc). >>> >>> Yes I can, but I need to be able to call cp15_init. And I can't do >>> that because if CONFIG_SKIP_LOWLEVEL_INIT is defined, that code is >>> compiled out! So I need to move that cp15_init code outside the >>> CONFIG_SKIP_LOWLEVEL_INIT #ifdef. That is all I am trying to do, >>> honest! >> >> I understand, and I think the approach here is wrong, because you *do* want >> low-level init in the A9 case, and thus you should not have >> CONFIG_SKIP_LOWLEVEL_INIT set. But you also need the AVP low-level init to >> be empty, and the AVP and A9 must follow the same startup path. And all this >> it not necessarily required for all armv7 platforms, but some of them will >> want lowlevel init, and some not. Considering all this... >> >> For now, I would opt for a CONFIG_ARCH_GENERIC_LOWLEVEL_INIT option which >> start.S would use to compile out the low level init code definition (like >> CONFIG_SKIP_LOWLEVEL_INIT does) but not the the calls to it (UNline >> CONFIG_SKIP_LOWLEVEL_INIT does). Then you could provide a SoC-specific >> version of lowlevel_init. > > If I understand you correctly, this is the opposite of what I want. It > is basically what we have now. > > All ARMv7 chips are going to turn off MMU, caches, flush TLBs, and the > like - this is an architecture feature, not an SOC one. We have > conflated ARM arch init with SOC init as you say, but the solution is > not to force the SOC to copy code from start.S just so that it can do > ARM arch init later. Yes, all ARMv7 CPUs will turn off caches etc, and thus, they all will want to do the cp15 init where it is now. Only the Tegra wants has issues with it so far. > And I'd argue for LOWLEVEL_INIT being about setting up the memory so > we can run code, and in particular relocate it later. The cp15_init > stuff should arguably be in arch_cpu_init() (or perhaps a new > arch_init()) as the first thing called from board_init_f() in my view. I disagree that cp15 could be set later: if we don't set it at low level init, we cannot be sure whether data caches are off or on, and if on, what they contain already. > So how about I create a patch to move the cp15_init() code into > arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later? I don't mind moving it to armv7/lib. I do mind the "later". >> In the longer term, I would be more in favor of a weak definition system >> with hierarchical ARCH, ISA, SoC, board priority order (yes, I am kind of >> fond of it) for lowlevel init -- but that will require looking into asm weak >> symbol handling and may require splitting of the assembly code function by >> function. > > I think you are trying to avoid the > > #ifdef CONFIG_ARCH_CPU_INIT > arch_cpu_init, > #endif > #ifdef ... > some_other_thing > #endif > > in the board_init_f() function table. Yes I agree it would tidy things > up, although I wonder if Graeme's initcall thing isn't better again. > It has the same confusion level coefficient (only the linker decides > what code is included) with a bit more flexibility. Then each arch / > board / cpu / soc / squirrel can decide on its own init and put it in > its own file. C only please :-) AFAIU, initcall does not have an override mechanism by which a later module can replace an initcall symbol already defined; two modules defining the same function name as initcall will just end up causing a linker error. > Regards, > Simon > >> >>> Regards, >>> Simon >> >> Amicalement, >> -- >> Albert. Amicalement,
Hi Albert, On Mon, Oct 24, 2011 at 1:04 PM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Le 22/10/2011 18:13, Simon Glass a écrit : >> >> Hi Albert, >> >> On Sat, Oct 22, 2011 at 12:56 AM, Albert ARIBAUD >> <albert.u.boot@aribaud.net> wrote: >>> >>> Le 22/10/2011 07:05, Simon Glass a écrit : >>> >>>>>> Well I actually haven't moved it! It is just that previously it was >>>>>> impossible to call cp15_init from anywhere later. >>>>> >>>>> It is moved, in that it belongs to low level init... of A9. >>>> >>>> OK, I see - you mean moved in order if not in source code file. >>> >>> Yes. In the code, though, it belongs to low-level init. >> >> Arguably this could go in a library like arch/arm/cpu/armv7/lib/cache.S >> >>> >>>>>> What you say can be done, it would involve some assembler though and >>>>>> would need to be another CONFIG option. Was trying to avoid adding new >>>>>> assembler. >>>>> >>>>> Low level init is about assembler and, well, low level. :) >>>> >>>> Yes but it's yuck. Part of the clean-up is to remove most of the >>>> assembler - really very little is needed. >>> >>> I don't think "yuck" is a valid reasoned argument against assembly >>> language >>> :) but I assume what you mainly mean is 'hard to understand and >>> replaceable >>> by easy to understand C code'. I agree to the first part, and to the >>> second >>> one for the general case, but not for the startup path where, precisely, >>> we >>> don't have a working C run-time and most of the understanding requires >>> knowledge of hardware, not only general programming knowledge. Past that, >>> C >> >> I'm not arguing for the cache init stuff to move to C, just trying to >> avoid any creeping growth of assembler when C can do it. >> >>> is ok -- but then beware: some C++ guy could chime in and contend that C >>> is >>> "yuck" by your own standards. :) >> >> The C++ guy who finds himself in U-Boot is probably lost :-) >> >>> >>>>> But I don't see why there should be another CONFIG option. IIUC, you >>>>> should >>>>> be able to do with only CONFIG_SKIP_LOWLEVEL_INIT: within the low level >>>>> init >>>>> code under it, you would do the equivalent of a switch, with one branch >>>>> for >>>>> AVM (and DDR init etc) and one branch for A9 (with cp15 init etc). >>>> >>>> Yes I can, but I need to be able to call cp15_init. And I can't do >>>> that because if CONFIG_SKIP_LOWLEVEL_INIT is defined, that code is >>>> compiled out! So I need to move that cp15_init code outside the >>>> CONFIG_SKIP_LOWLEVEL_INIT #ifdef. That is all I am trying to do, >>>> honest! >>> >>> I understand, and I think the approach here is wrong, because you *do* >>> want >>> low-level init in the A9 case, and thus you should not have >>> CONFIG_SKIP_LOWLEVEL_INIT set. But you also need the AVP low-level init >>> to >>> be empty, and the AVP and A9 must follow the same startup path. And all >>> this >>> it not necessarily required for all armv7 platforms, but some of them >>> will >>> want lowlevel init, and some not. Considering all this... >>> >>> For now, I would opt for a CONFIG_ARCH_GENERIC_LOWLEVEL_INIT option which >>> start.S would use to compile out the low level init code definition (like >>> CONFIG_SKIP_LOWLEVEL_INIT does) but not the the calls to it (UNline >>> CONFIG_SKIP_LOWLEVEL_INIT does). Then you could provide a SoC-specific >>> version of lowlevel_init. >> >> If I understand you correctly, this is the opposite of what I want. It >> is basically what we have now. >> >> All ARMv7 chips are going to turn off MMU, caches, flush TLBs, and the >> like - this is an architecture feature, not an SOC one. We have >> conflated ARM arch init with SOC init as you say, but the solution is >> not to force the SOC to copy code from start.S just so that it can do >> ARM arch init later. > > Yes, all ARMv7 CPUs will turn off caches etc, and thus, they all will want > to do the cp15 init where it is now. Only the Tegra wants has issues with it > so far. Yes > >> And I'd argue for LOWLEVEL_INIT being about setting up the memory so >> we can run code, and in particular relocate it later. The cp15_init >> stuff should arguably be in arch_cpu_init() (or perhaps a new >> arch_init()) as the first thing called from board_init_f() in my view. > > I disagree that cp15 could be set later: if we don't set it at low level > init, we cannot be sure whether data caches are off or on, and if on, what > they contain already. Well on Tegra the AVP cannot execute this code. There is no danger in this case since it doesn't have caches, etc. We execute the code as soon as practical on the A9. > >> So how about I create a patch to move the cp15_init() code into >> arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later? > > I don't mind moving it to armv7/lib. I do mind the "later". OK, but the AVP cannot execute this code, whereas the A9 needs to execute it on start-up. We define CONFIG_SKIP_LOWLEVEL_INIT on Tegra, and the mere fact that this option exists suggests that platforms are permitted to do it. We don't need to do any memory init, and the CP15 init can be called in arch_cpu_init() once we establish that we are an A9 and not an AVP. Does that sound ok? The only problem I can see with this is if the data cache is on. This could happen if you load U-Boot with (say) tftp and jump to it. Well 'don't do that!'. We also hope that the D-cache has at least been flushed or there is nothing we can do. One fix for this (prior to the refactor you are planning and mentioned in your earlier email) is perhaps to implement a run-time check for the existence of CP15. Is that better or worse? Regards, Simon > >>> In the longer term, I would be more in favor of a weak definition system >>> with hierarchical ARCH, ISA, SoC, board priority order (yes, I am kind of >>> fond of it) for lowlevel init -- but that will require looking into asm >>> weak >>> symbol handling and may require splitting of the assembly code function >>> by >>> function. >> >> I think you are trying to avoid the >> >> #ifdef CONFIG_ARCH_CPU_INIT >> arch_cpu_init, >> #endif >> #ifdef ... >> some_other_thing >> #endif >> >> in the board_init_f() function table. Yes I agree it would tidy things >> up, although I wonder if Graeme's initcall thing isn't better again. >> It has the same confusion level coefficient (only the linker decides >> what code is included) with a bit more flexibility. Then each arch / >> board / cpu / soc / squirrel can decide on its own init and put it in >> its own file. C only please :-) > > AFAIU, initcall does not have an override mechanism by which a later module > can replace an initcall symbol already defined; two modules defining the > same function name as initcall will just end up causing a linker error. > >> Regards, >> Simon >> >>> >>>> Regards, >>>> Simon >>> >>> Amicalement, >>> -- >>> Albert. > > Amicalement, > -- > Albert. >
Le 24/10/2011 22:14, Simon Glass a écrit : >>> So how about I create a patch to move the cp15_init() code into >>> arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later? >> >> I don't mind moving it to armv7/lib. I do mind the "later". > > OK, but the AVP cannot execute this code, whereas the A9 needs to > execute it on start-up. Yes, and that's the real problem: if you did not have that AVP running the same code path as the A9, you wouldn't even have though of moving cp15 init elsewhere. > We define CONFIG_SKIP_LOWLEVEL_INIT on Tegra, and the mere fact that > this option exists suggests that platforms are permitted to do it. We > don't need to do any memory init, and the CP15 init can be called in > arch_cpu_init() once we establish that we are an A9 and not an AVP. > Does that sound ok? Yes, CONFIG_SKIP_LOWLEVEL_INIT exists for a reason, but that reason is not "I'm running this code on two CPUs", it is "those inits have already been done". CONFIG_SKIP_LOWLEVEL_INITS is meant for U-Boot being loaded by SPL, so SPL has done the inits already. And I also agree that we should do the cp15 init as soon as we have checked that we're the A9, not the AVP. But this check has to be done during low level init, based on some immediate identification, not based on where in the code we are. > The only problem I can see with this is if the data cache is on. This > could happen if you load U-Boot with (say) tftp and jump to it. Well > 'don't do that!'. We also hope that the D-cache has at least been > flushed or there is nothing we can do. One fix for this (prior to the > refactor you are planning and mentioned in your earlier email) is > perhaps to implement a run-time check for the existence of CP15. Is > that better or worse? According to ARM, coprocessors 14 and 15 should always be supported from ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra specification available, I'd like to have a look at it. > Regards, > Simon Amicalement,
Hi Albert, On Mon, Oct 24, 2011 at 2:21 PM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Le 24/10/2011 22:14, Simon Glass a écrit : > >>>> So how about I create a patch to move the cp15_init() code into >>>> arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later? >>> >>> I don't mind moving it to armv7/lib. I do mind the "later". >> >> OK, but the AVP cannot execute this code, whereas the A9 needs to >> execute it on start-up. > > Yes, and that's the real problem: if you did not have that AVP running the > same code path as the A9, you wouldn't even have though of moving cp15 init > elsewhere. What is your point here? I am trying to get to a simple easy-to-understand execution path in a fairly complex system. So far the model is that the AVP starts up U-Boot until arch_cpu_init(), then halts and the A9 takes over and runs the same code again. It is very easy to understand and debug. It is easy to set breakpoints in startup code on either core. It is possible to even put printf() debugging on either core with the pre-console support. It has a lot of advantages over more complex schemes. It removes a CONFIG_TEGRA2 from start.S too! > >> We define CONFIG_SKIP_LOWLEVEL_INIT on Tegra, and the mere fact that >> this option exists suggests that platforms are permitted to do it. We >> don't need to do any memory init, and the CP15 init can be called in >> arch_cpu_init() once we establish that we are an A9 and not an AVP. >> Does that sound ok? > > Yes, CONFIG_SKIP_LOWLEVEL_INIT exists for a reason, but that reason is not > "I'm running this code on two CPUs", it is "those inits have already been > done". CONFIG_SKIP_LOWLEVEL_INITS is meant for U-Boot being loaded by SPL, > so SPL has done the inits already. I thought it pre-dated SPL, but ok. > > And I also agree that we should do the cp15 init as soon as we have checked > that we're the A9, not the AVP. > > But this check has to be done during low level init, based on some immediate > identification, not based on where in the code we are. There is no facility in there for this at present. Are you asking for a new call to find out at run-time whether the CP15 init should be called? If so, then I can fairly easily do that. >> The only problem I can see with this is if the data cache is on. This >> could happen if you load U-Boot with (say) tftp and jump to it. Well >> 'don't do that!'. We also hope that the D-cache has at least been >> flushed or there is nothing we can do. One fix for this (prior to the >> refactor you are planning and mentioned in your earlier email) is >> perhaps to implement a run-time check for the existence of CP15. Is >> that better or worse? > > According to ARM, coprocessors 14 and 15 should always be supported from > ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra > specification available, I'd like to have a look at it. Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU init on the AVP and see what happens. Regards, Simon > >> Regards, >> Simon > > Amicalement, > -- > Albert. >
Hi Simon, Le 25/10/2011 07:02, Simon Glass a écrit : > Hi Albert, > > On Mon, Oct 24, 2011 at 2:21 PM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: >> Le 24/10/2011 22:14, Simon Glass a écrit : >> >>>>> So how about I create a patch to move the cp15_init() code into >>>>> arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later? >>>> >>>> I don't mind moving it to armv7/lib. I do mind the "later". >>> >>> OK, but the AVP cannot execute this code, whereas the A9 needs to >>> execute it on start-up. >> >> Yes, and that's the real problem: if you did not have that AVP running the >> same code path as the A9, you wouldn't even have though of moving cp15 init >> elsewhere. > > What is your point here? I am trying to get to a simple > easy-to-understand execution path in a fairly complex system. So far > the model is that the AVP starts up U-Boot until arch_cpu_init(), then > halts and the A9 takes over and runs the same code again. It is very > easy to understand and debug. It is easy to set breakpoints in startup > code on either core. It is possible to even put printf() debugging on > either core with the pre-console support. It has a lot of advantages > over more complex schemes. It removes a CONFIG_TEGRA2 from start.S > too! My point is precisely to keep the execution path simple and reliable for as many ARM architectures as possible, and I consider that placing parts of the low level initialization at different times for different cores is not simple and possibly not reliable, as it makes memory state uncertain during part of the start.S path for the A9. I do understand that to you, this platform you're working on is easy to understand and debug, but then, you've got some experience with it, and it all makes sens to you, but it might not to other ARM developser who don't have access to it. Besides, what I suggest doing (keeping cp15 access where it is, but subject it to the condition that it's not the AVP trying to execute it) does not preclude doing early printf() etc. >>> We define CONFIG_SKIP_LOWLEVEL_INIT on Tegra, and the mere fact that >>> this option exists suggests that platforms are permitted to do it. We >>> don't need to do any memory init, and the CP15 init can be called in >>> arch_cpu_init() once we establish that we are an A9 and not an AVP. >>> Does that sound ok? >> >> Yes, CONFIG_SKIP_LOWLEVEL_INIT exists for a reason, but that reason is not >> "I'm running this code on two CPUs", it is "those inits have already been >> done". CONFIG_SKIP_LOWLEVEL_INITS is meant for U-Boot being loaded by SPL, >> so SPL has done the inits already. > > I thought it pre-dated SPL, but ok. It may have, and before or after SPL, it was/is immensely useful as a debugging aid. As such, it nearly disappeared at one point, but with SPL, it has received a non-debugging, functional, utility. >> And I also agree that we should do the cp15 init as soon as we have checked >> that we're the A9, not the AVP. >> >> But this check has to be done during low level init, based on some immediate >> identification, not based on where in the code we are. > > There is no facility in there for this at present. Are you asking for > a new call to find out at run-time whether the CP15 init should be > called? If so, then I can fairly easily do that. Is there really no register that the core can access which would tell it which core it actually is? No version/revision register either? >>> The only problem I can see with this is if the data cache is on. This >>> could happen if you load U-Boot with (say) tftp and jump to it. Well >>> 'don't do that!'. We also hope that the D-cache has at least been >>> flushed or there is nothing we can do. One fix for this (prior to the >>> refactor you are planning and mentioned in your earlier email) is >>> perhaps to implement a run-time check for the existence of CP15. Is >>> that better or worse? >> >> According to ARM, coprocessors 14 and 15 should always be supported from >> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra >> specification available, I'd like to have a look at it. > > Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU > init on the AVP and see what happens. Please do check this. It may be that you can actually write to it from AVP even though it has no or not all expected effect; and if that is confirmed, then you don't have to separate the boot paths any more, > Regards, > Simon Amicalement,
Hi Albert, On Mon, Oct 24, 2011 at 11:41 PM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Hi Simon, > > Le 25/10/2011 07:02, Simon Glass a écrit : >> >> Hi Albert, >> >> On Mon, Oct 24, 2011 at 2:21 PM, Albert ARIBAUD >> <albert.u.boot@aribaud.net> wrote: >>> >>> Le 24/10/2011 22:14, Simon Glass a écrit : >>> >>>>>> So how about I create a patch to move the cp15_init() code into >>>>>> arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later? >>>>> >>>>> I don't mind moving it to armv7/lib. I do mind the "later". >>>> >>>> OK, but the AVP cannot execute this code, whereas the A9 needs to >>>> execute it on start-up. >>> >>> Yes, and that's the real problem: if you did not have that AVP running >>> the >>> same code path as the A9, you wouldn't even have though of moving cp15 >>> init >>> elsewhere. >> >> What is your point here? I am trying to get to a simple >> easy-to-understand execution path in a fairly complex system. So far >> the model is that the AVP starts up U-Boot until arch_cpu_init(), then >> halts and the A9 takes over and runs the same code again. It is very >> easy to understand and debug. It is easy to set breakpoints in startup >> code on either core. It is possible to even put printf() debugging on >> either core with the pre-console support. It has a lot of advantages >> over more complex schemes. It removes a CONFIG_TEGRA2 from start.S >> too! > > My point is precisely to keep the execution path simple and reliable for as > many ARM architectures as possible, and I consider that placing parts of the > low level initialization at different times for different cores is not > simple and possibly not reliable, as it makes memory state uncertain during > part of the start.S path for the A9. Certainly if we can manage it so much the better. The memory is set up by the boot ROM and the A9 has just come out of reset. > > I do understand that to you, this platform you're working on is easy to > understand and debug, but then, you've got some experience with it, and it > all makes sens to you, but it might not to other ARM developser who don't > have access to it. > > Besides, what I suggest doing (keeping cp15 access where it is, but subject > it to the condition that it's not the AVP trying to execute it) does not > preclude doing early printf() etc. But it does involve some extremely early soc-specific code. With my current patch set there is none of this in start.S (the #ifdef TEGRA2 currently there is removed). > >>>> We define CONFIG_SKIP_LOWLEVEL_INIT on Tegra, and the mere fact that >>>> this option exists suggests that platforms are permitted to do it. We >>>> don't need to do any memory init, and the CP15 init can be called in >>>> arch_cpu_init() once we establish that we are an A9 and not an AVP. >>>> Does that sound ok? >>> >>> Yes, CONFIG_SKIP_LOWLEVEL_INIT exists for a reason, but that reason is >>> not >>> "I'm running this code on two CPUs", it is "those inits have already been >>> done". CONFIG_SKIP_LOWLEVEL_INITS is meant for U-Boot being loaded by >>> SPL, >>> so SPL has done the inits already. >> >> I thought it pre-dated SPL, but ok. > > It may have, and before or after SPL, it was/is immensely useful as a > debugging aid. As such, it nearly disappeared at one point, but with SPL, it > has received a non-debugging, functional, utility. > >>> And I also agree that we should do the cp15 init as soon as we have >>> checked >>> that we're the A9, not the AVP. >>> >>> But this check has to be done during low level init, based on some >>> immediate >>> identification, not based on where in the code we are. >> >> There is no facility in there for this at present. Are you asking for >> a new call to find out at run-time whether the CP15 init should be >> called? If so, then I can fairly easily do that. > > Is there really no register that the core can access which would tell it > which core it actually is? No version/revision register either? Yes of course - this is the C function ap20_cpu_is_cortexa9(), currently used from C code. But please think about what you actually want in start.S. Do you really want an #ifdef TEGRA2 with a check of a SOC register, followed by a branch around the cp15_init code? > >>>> The only problem I can see with this is if the data cache is on. This >>>> could happen if you load U-Boot with (say) tftp and jump to it. Well >>>> 'don't do that!'. We also hope that the D-cache has at least been >>>> flushed or there is nothing we can do. One fix for this (prior to the >>>> refactor you are planning and mentioned in your earlier email) is >>>> perhaps to implement a run-time check for the existence of CP15. Is >>>> that better or worse? >>> >>> According to ARM, coprocessors 14 and 15 should always be supported from >>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra >>> specification available, I'd like to have a look at it. >> >> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU >> init on the AVP and see what happens. > > Please do check this. It may be that you can actually write to it from AVP > even though it has no or not all expected effect; and if that is confirmed, > then you don't have to separate the boot paths any more, Yes I'm tempted to go with this even if the ARM ARM is silent on the matter. I will try it out today. Regards, Simon > Amicalement, > -- > Albert. >
Hi Albert, On Tue, Oct 25, 2011 at 6:44 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Albert, > > On Mon, Oct 24, 2011 at 11:41 PM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: >> Hi Simon, >> >> Le 25/10/2011 07:02, Simon Glass a écrit : >>> >>> Hi Albert, >>> >>> On Mon, Oct 24, 2011 at 2:21 PM, Albert ARIBAUD >>> <albert.u.boot@aribaud.net> wrote: >>>> >>>> Le 24/10/2011 22:14, Simon Glass a écrit : >>>> >>>>>>> So how about I create a patch to move the cp15_init() code into >>>>>>> arch/arm/cpu/armv7/lib/lowlevel.S or similar so I can call it later? >>>>>> >>>>>> I don't mind moving it to armv7/lib. I do mind the "later". >>>>> >>>>> OK, but the AVP cannot execute this code, whereas the A9 needs to >>>>> execute it on start-up. >>>> >>>> Yes, and that's the real problem: if you did not have that AVP running >>>> the >>>> same code path as the A9, you wouldn't even have though of moving cp15 >>>> init >>>> elsewhere. >>> >>> What is your point here? I am trying to get to a simple >>> easy-to-understand execution path in a fairly complex system. So far >>> the model is that the AVP starts up U-Boot until arch_cpu_init(), then >>> halts and the A9 takes over and runs the same code again. It is very >>> easy to understand and debug. It is easy to set breakpoints in startup >>> code on either core. It is possible to even put printf() debugging on >>> either core with the pre-console support. It has a lot of advantages >>> over more complex schemes. It removes a CONFIG_TEGRA2 from start.S >>> too! >> >> My point is precisely to keep the execution path simple and reliable for as >> many ARM architectures as possible, and I consider that placing parts of the >> low level initialization at different times for different cores is not >> simple and possibly not reliable, as it makes memory state uncertain during >> part of the start.S path for the A9. > > Certainly if we can manage it so much the better. > > The memory is set up by the boot ROM and the A9 has just come out of reset. > >> >> I do understand that to you, this platform you're working on is easy to >> understand and debug, but then, you've got some experience with it, and it >> all makes sens to you, but it might not to other ARM developser who don't >> have access to it. >> >> Besides, what I suggest doing (keeping cp15 access where it is, but subject >> it to the condition that it's not the AVP trying to execute it) does not >> preclude doing early printf() etc. > > But it does involve some extremely early soc-specific code. With my > current patch set there is none of this in start.S (the #ifdef TEGRA2 > currently there is removed). > >> >>>>> We define CONFIG_SKIP_LOWLEVEL_INIT on Tegra, and the mere fact that >>>>> this option exists suggests that platforms are permitted to do it. We >>>>> don't need to do any memory init, and the CP15 init can be called in >>>>> arch_cpu_init() once we establish that we are an A9 and not an AVP. >>>>> Does that sound ok? >>>> >>>> Yes, CONFIG_SKIP_LOWLEVEL_INIT exists for a reason, but that reason is >>>> not >>>> "I'm running this code on two CPUs", it is "those inits have already been >>>> done". CONFIG_SKIP_LOWLEVEL_INITS is meant for U-Boot being loaded by >>>> SPL, >>>> so SPL has done the inits already. >>> >>> I thought it pre-dated SPL, but ok. >> >> It may have, and before or after SPL, it was/is immensely useful as a >> debugging aid. As such, it nearly disappeared at one point, but with SPL, it >> has received a non-debugging, functional, utility. >> >>>> And I also agree that we should do the cp15 init as soon as we have >>>> checked >>>> that we're the A9, not the AVP. >>>> >>>> But this check has to be done during low level init, based on some >>>> immediate >>>> identification, not based on where in the code we are. >>> >>> There is no facility in there for this at present. Are you asking for >>> a new call to find out at run-time whether the CP15 init should be >>> called? If so, then I can fairly easily do that. >> >> Is there really no register that the core can access which would tell it >> which core it actually is? No version/revision register either? > > Yes of course - this is the C function ap20_cpu_is_cortexa9(), > currently used from C code. > > But please think about what you actually want in start.S. Do you > really want an #ifdef TEGRA2 with a check of a SOC register, followed > by a branch around the cp15_init code? ^^^^ This is the bit we need to sort out. > >> >>>>> The only problem I can see with this is if the data cache is on. This >>>>> could happen if you load U-Boot with (say) tftp and jump to it. Well >>>>> 'don't do that!'. We also hope that the D-cache has at least been >>>>> flushed or there is nothing we can do. One fix for this (prior to the >>>>> refactor you are planning and mentioned in your earlier email) is >>>>> perhaps to implement a run-time check for the existence of CP15. Is >>>>> that better or worse? >>>> >>>> According to ARM, coprocessors 14 and 15 should always be supported from >>>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra >>>> specification available, I'd like to have a look at it. >>> >>> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU >>> init on the AVP and see what happens. >> >> Please do check this. It may be that you can actually write to it from AVP >> even though it has no or not all expected effect; and if that is confirmed, >> then you don't have to separate the boot paths any more, > > Yes I'm tempted to go with this even if the ARM ARM is silent on the > matter. I will try it out today. Silly me of course it doesn't work - since if the CP15 doesn't have those features then you get undef instruction exception. Please see ^^^ above! Regards, Simon > > Regards, > Simon > >> Amicalement, >> -- >> Albert. >> >
Hi Simon, Le 25/10/2011 23:16, Simon Glass a écrit : >> But please think about what you actually want in start.S. Do you >> really want an #ifdef TEGRA2 with a check of a SOC register, followed >> by a branch around the cp15_init code? > > ^^^^ > This is the bit we need to sort out. Indeed, but I dont want a '#ifdef TEGRA2'. In the worst case I want a 'if (!core_is_avp())' by way of an identification register, and in the best case, I just want cp15 to be set unconditionally. >>>>> According to ARM, coprocessors 14 and 15 should always be supported from >>>>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra >>>>> specification available, I'd like to have a look at it. >>>> >>>> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU >>>> init on the AVP and see what happens. >>> >>> Please do check this. It may be that you can actually write to it from AVP >>> even though it has no or not all expected effect; and if that is confirmed, >>> then you don't have to separate the boot paths any more, >> >> Yes I'm tempted to go with this even if the ARM ARM is silent on the >> matter. I will try it out today. > > Silly me of course it doesn't work - since if the CP15 doesn't have > those features then you get undef instruction exception. Please see > ^^^ above! Are you stating a reasoning here or the results of an actual test? What causes an undefined instruction exception is the absence of a coprocessor to handle the instruction, not the presence of a coprocessor which incidentally cannot perform all of what"s being asked. > Regards, > Simon Amicalement,
Hi Albert, On Thu, Oct 27, 2011 at 1:05 PM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Hi Simon, > > Le 25/10/2011 23:16, Simon Glass a écrit : > >>> But please think about what you actually want in start.S. Do you >>> really want an #ifdef TEGRA2 with a check of a SOC register, followed >>> by a branch around the cp15_init code? >> >> ^^^^ >> This is the bit we need to sort out. > > Indeed, but I dont want a '#ifdef TEGRA2'. In the worst case I want a 'if > (!core_is_avp())' by way of an identification register, and in the best > case, I just want cp15 to be set unconditionally. You already have #ifdef TEGRA2 (look at the top of start.S). That is what I am trying to remove. How can I call core_is_avp() on an SOC that isn't Tegra? Can you please write a little code fragment that I should use in start.S, so that I can understand what is required here. > >>>>>> According to ARM, coprocessors 14 and 15 should always be supported >>>>>> from >>>>>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra >>>>>> specification available, I'd like to have a look at it. >>>>> >>>>> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU >>>>> init on the AVP and see what happens. >>>> >>>> Please do check this. It may be that you can actually write to it from >>>> AVP >>>> even though it has no or not all expected effect; and if that is >>>> confirmed, >>>> then you don't have to separate the boot paths any more, >>> >>> Yes I'm tempted to go with this even if the ARM ARM is silent on the >>> matter. I will try it out today. >> >> Silly me of course it doesn't work - since if the CP15 doesn't have >> those features then you get undef instruction exception. Please see >> ^^^ above! > > Are you stating a reasoning here or the results of an actual test? What > causes an undefined instruction exception is the absence of a coprocessor to > handle the instruction, not the presence of a coprocessor which incidentally > cannot perform all of what"s being asked. Yes I did test it - it hangs. Would you like me to spend some time to debug exactly why? I suspect it is an undef instruction, since executing cache instructions on an ARM7TDMI is dodgy, but I could check if it would help. But let's assume for now we can't do that! Regards, Simon > >> Regards, >> Simon > > Amicalement, > -- > Albert. >
Le 27/10/2011 22:17, Simon Glass a écrit : > Hi Albert, > > On Thu, Oct 27, 2011 at 1:05 PM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: >> Hi Simon, >> >> Le 25/10/2011 23:16, Simon Glass a écrit : >> >>>> But please think about what you actually want in start.S. Do you >>>> really want an #ifdef TEGRA2 with a check of a SOC register, followed >>>> by a branch around the cp15_init code? >>> >>> ^^^^ >>> This is the bit we need to sort out. >> >> Indeed, but I dont want a '#ifdef TEGRA2'. In the worst case I want a 'if >> (!core_is_avp())' by way of an identification register, and in the best >> case, I just want cp15 to be set unconditionally. > > You already have #ifdef TEGRA2 (look at the top of start.S). That is > what I am trying to remove. > > How can I call core_is_avp() on an SOC that isn't Tegra? Can you > please write a little code fragment that I should use in start.S, so > that I can understand what is required here. > >> >>>>>>> According to ARM, coprocessors 14 and 15 should always be supported >>>>>>> from >>>>>>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra >>>>>>> specification available, I'd like to have a look at it. >>>>>> >>>>>> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU >>>>>> init on the AVP and see what happens. >>>>> >>>>> Please do check this. It may be that you can actually write to it from >>>>> AVP >>>>> even though it has no or not all expected effect; and if that is >>>>> confirmed, >>>>> then you don't have to separate the boot paths any more, >>>> >>>> Yes I'm tempted to go with this even if the ARM ARM is silent on the >>>> matter. I will try it out today. >>> >>> Silly me of course it doesn't work - since if the CP15 doesn't have >>> those features then you get undef instruction exception. Please see >>> ^^^ above! >> >> Are you stating a reasoning here or the results of an actual test? What >> causes an undefined instruction exception is the absence of a coprocessor to >> handle the instruction, not the presence of a coprocessor which incidentally >> cannot perform all of what"s being asked. > > Yes I did test it - it hangs. Would you like me to spend some time to > debug exactly why? I suspect it is an undef instruction, since > executing cache instructions on an ARM7TDMI is dodgy, but I could > check if it would help. But let's assume for now we can't do that! What was the test exactly? Depending on which register of CP15 you try to access, things could be different. Can you test MRC p15,0,<Rt>,c0,c0,0 ? That's the MIDR and it should be always accessible, plus it should have a unique value for each type of core, so it could be used to not write cp15 for caches etc from the AVP. > Regards, > Simon Amicalement,
Hi Albert, On Thu, Oct 27, 2011 at 3:39 PM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Le 27/10/2011 22:17, Simon Glass a écrit : >> >> Hi Albert, >> >> On Thu, Oct 27, 2011 at 1:05 PM, Albert ARIBAUD >> <albert.u.boot@aribaud.net> wrote: >>> >>> Hi Simon, >>> >>> Le 25/10/2011 23:16, Simon Glass a écrit : >>> >>>>> But please think about what you actually want in start.S. Do you >>>>> really want an #ifdef TEGRA2 with a check of a SOC register, followed >>>>> by a branch around the cp15_init code? >>>> >>>> ^^^^ >>>> This is the bit we need to sort out. >>> >>> Indeed, but I dont want a '#ifdef TEGRA2'. In the worst case I want a 'if >>> (!core_is_avp())' by way of an identification register, and in the best >>> case, I just want cp15 to be set unconditionally. >> >> You already have #ifdef TEGRA2 (look at the top of start.S). That is >> what I am trying to remove. >> >> How can I call core_is_avp() on an SOC that isn't Tegra? Can you >> please write a little code fragment that I should use in start.S, so >> that I can understand what is required here. >> >>> >>>>>>>> According to ARM, coprocessors 14 and 15 should always be supported >>>>>>>> from >>>>>>>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra >>>>>>>> specification available, I'd like to have a look at it. >>>>>>> >>>>>>> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU >>>>>>> init on the AVP and see what happens. >>>>>> >>>>>> Please do check this. It may be that you can actually write to it from >>>>>> AVP >>>>>> even though it has no or not all expected effect; and if that is >>>>>> confirmed, >>>>>> then you don't have to separate the boot paths any more, >>>>> >>>>> Yes I'm tempted to go with this even if the ARM ARM is silent on the >>>>> matter. I will try it out today. >>>> >>>> Silly me of course it doesn't work - since if the CP15 doesn't have >>>> those features then you get undef instruction exception. Please see >>>> ^^^ above! >>> >>> Are you stating a reasoning here or the results of an actual test? What >>> causes an undefined instruction exception is the absence of a coprocessor >>> to >>> handle the instruction, not the presence of a coprocessor which >>> incidentally >>> cannot perform all of what"s being asked. >> >> Yes I did test it - it hangs. Would you like me to spend some time to >> debug exactly why? I suspect it is an undef instruction, since >> executing cache instructions on an ARM7TDMI is dodgy, but I could >> check if it would help. But let's assume for now we can't do that! > > What was the test exactly? Depending on which register of CP15 you try to > access, things could be different. Can you test MRC p15,0,<Rt>,c0,c0,0 ? > That's the MIDR and it should be always accessible, plus it should have a > unique value for each type of core, so it could be used to not write cp15 > for caches etc from the AVP. I'm sure that is fine. So do you want me to add a check that the ID shows architecture ARMv7, and then skip the cache code if not? It should be safe enough since it is a clear signal that something is not right. It would also handle the case where an Cortex-A7 (with a cache) is attached. Regards, Simon > >> Regards, >> Simon > > Amicalement, > -- > Albert. >
Hi Albert, On Thu, Oct 27, 2011 at 3:46 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Albert, > > On Thu, Oct 27, 2011 at 3:39 PM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: >> Le 27/10/2011 22:17, Simon Glass a écrit : >>> >>> Hi Albert, >>> >>> On Thu, Oct 27, 2011 at 1:05 PM, Albert ARIBAUD >>> <albert.u.boot@aribaud.net> wrote: >>>> >>>> Hi Simon, >>>> >>>> Le 25/10/2011 23:16, Simon Glass a écrit : >>>> >>>>>> But please think about what you actually want in start.S. Do you >>>>>> really want an #ifdef TEGRA2 with a check of a SOC register, followed >>>>>> by a branch around the cp15_init code? >>>>> >>>>> ^^^^ >>>>> This is the bit we need to sort out. >>>> >>>> Indeed, but I dont want a '#ifdef TEGRA2'. In the worst case I want a 'if >>>> (!core_is_avp())' by way of an identification register, and in the best >>>> case, I just want cp15 to be set unconditionally. >>> >>> You already have #ifdef TEGRA2 (look at the top of start.S). That is >>> what I am trying to remove. >>> >>> How can I call core_is_avp() on an SOC that isn't Tegra? Can you >>> please write a little code fragment that I should use in start.S, so >>> that I can understand what is required here. >>> >>>> >>>>>>>>> According to ARM, coprocessors 14 and 15 should always be supported >>>>>>>>> from >>>>>>>>> ARMv6 up. Are you sure the AVP does not have it? If there is a Tegra >>>>>>>>> specification available, I'd like to have a look at it. >>>>>>>> >>>>>>>> Sorry I meant caches/MMU, etc. I should try doing the CP15 cache/MMU >>>>>>>> init on the AVP and see what happens. >>>>>>> >>>>>>> Please do check this. It may be that you can actually write to it from >>>>>>> AVP >>>>>>> even though it has no or not all expected effect; and if that is >>>>>>> confirmed, >>>>>>> then you don't have to separate the boot paths any more, >>>>>> >>>>>> Yes I'm tempted to go with this even if the ARM ARM is silent on the >>>>>> matter. I will try it out today. >>>>> >>>>> Silly me of course it doesn't work - since if the CP15 doesn't have >>>>> those features then you get undef instruction exception. Please see >>>>> ^^^ above! >>>> >>>> Are you stating a reasoning here or the results of an actual test? What >>>> causes an undefined instruction exception is the absence of a coprocessor >>>> to >>>> handle the instruction, not the presence of a coprocessor which >>>> incidentally >>>> cannot perform all of what"s being asked. >>> >>> Yes I did test it - it hangs. Would you like me to spend some time to >>> debug exactly why? I suspect it is an undef instruction, since >>> executing cache instructions on an ARM7TDMI is dodgy, but I could >>> check if it would help. But let's assume for now we can't do that! >> >> What was the test exactly? Depending on which register of CP15 you try to >> access, things could be different. Can you test MRC p15,0,<Rt>,c0,c0,0 ? >> That's the MIDR and it should be always accessible, plus it should have a >> unique value for each type of core, so it could be used to not write cp15 >> for caches etc from the AVP. The test was mrc p15, 0, r0, c0, c0, 0 @ get ID register and r0, r0, #0xf0000 @ get architecture cmp r0, #0xf0000 @ check for > ARMv6 movne pc, lr @ else skip cache init Unfortunately I think it is a plain ARM7TDMI with no CP15. Regards, Simon > > I'm sure that is fine. So do you want me to add a check that the ID > shows architecture ARMv7, and then skip the cache code if not? It > should be safe enough since it is a clear signal that something is not > right. It would also handle the case where an Cortex-A7 (with a cache) > is attached. > > Regards, > Simon > >> >>> Regards, >>> Simon >> >> Amicalement, >> -- >> Albert. >> >
Hi Simon, Le 28/10/2011 00:46, Simon Glass a écrit : > Hi Albert, >>>>>> Yes I'm tempted to go with this even if the ARM ARM is silent on the >>>>>> matter. I will try it out today. >>>>> >>>>> Silly me of course it doesn't work - since if the CP15 doesn't have >>>>> those features then you get undef instruction exception. Please see >>>>> ^^^ above! >>>> >>>> Are you stating a reasoning here or the results of an actual test? What >>>> causes an undefined instruction exception is the absence of a coprocessor >>>> to >>>> handle the instruction, not the presence of a coprocessor which >>>> incidentally >>>> cannot perform all of what"s being asked. >>> >>> Yes I did test it - it hangs. Would you like me to spend some time to >>> debug exactly why? I suspect it is an undef instruction, since >>> executing cache instructions on an ARM7TDMI is dodgy, but I could >>> check if it would help. But let's assume for now we can't do that! >> >> What was the test exactly? Depending on which register of CP15 you try to >> access, things could be different. Can you test MRC p15,0,<Rt>,c0,c0,0 ? >> That's the MIDR and it should be always accessible, plus it should have a >> unique value for each type of core, so it could be used to not write cp15 >> for caches etc from the AVP. > > I'm sure that is fine. So do you want me to add a check that the ID > shows architecture ARMv7, and then skip the cache code if not? > It should be safe enough since it is a clear signal that something is not > right. It would also handle the case where an Cortex-A7 (with a cache) > is attached. That's the general idea of what I am asking for: test for actual conditions when needed rather than assume conditions at build time. However I don't want to test for ARMv7 architecture, because there is no reason for *all* ARMv7 cores to fail handling current cp15 init. What I want here is to test for NVidia Tegra2 AVP (using the Implementer, Variant, Primary part number and possibly even Revision fields of MIDR), since you have demonstrated it to not handle the current cp15 init. Plus, run-time testing on MIDR could later on allow us to reduce ARM init code path to a single, common start.S for generic/common ARM init code, and specific code would be 'switched' into, based on MIDR -- that could allow for multiple board support in a single binary. > Regards, > Simon Amicalement,
Le 28/10/2011 03:43, Simon Glass a écrit : > The test was > > mrc p15, 0, r0, c0, c0, 0 @ get ID register > and r0, r0, #0xf0000 @ get architecture > cmp r0, #0xf0000 @ check for> ARMv6 > movne pc, lr @ else skip cache init > > Unfortunately I think it is a plain ARM7TDMI with no CP15. What about other fields in r0 right after mrc? > Regards, > Simon Amicalement,
Hi Albert, On Thu, Oct 27, 2011 at 10:09 PM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Le 28/10/2011 03:43, Simon Glass a écrit : > >> The test was >> >> mrc p15, 0, r0, c0, c0, 0 @ get ID register >> and r0, r0, #0xf0000 @ get architecture >> cmp r0, #0xf0000 @ check for> ARMv6 >> movne pc, lr @ else skip cache init >> >> Unfortunately I think it is a plain ARM7TDMI with no CP15. > > What about other fields in r0 right after mrc? I don't really understand that sentence, sorry. The ARM7TDMI does not have a CP15 and aborts if I try to access it. Just in case there is something odd going on I checked with DSTREAM / RVdebug and it definitely doesn't have a CP15. [as Ford Prefect would say, I counted them twice] The simplest thing I have been able to think of that does not involve exceptions, differing instruction behaviour, doing the init later or putting in some Tegra-specific code is to check for the existence of the Q bit in the CPSR (actually APSR on ARMv7). This does seem to work and I have verified both in my old 1996 ARM ARM DDI 0100B and the ARMv7-A one (DDI 0406B) that from an architecture point of view this should work. The Q bit is RAZ on ARMv4T. I believe this will cope with the Cortex-A7 / A-15 combinations and possibly even Cortex-R4 / A-15 although I have not tested this. I suppose we can deal with this when it becomes an issue. So I have redone this one patch with that in mind, and adjusted the series slightly to fit with this. I will resend it when it completes MAKEALL. I hope that this resolves the matter, but if not(!), I would very much appreciate it if you could send through some actual pseudo code showing what you are looking for, to avoid any confusion. Thanks, Simon > >> Regards, >> Simon > > Amicalement, > -- > Albert. >
Hi Simon, Le 29/10/2011 02:36, Simon Glass a écrit : > Hi Albert, > > On Thu, Oct 27, 2011 at 10:09 PM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: >> Le 28/10/2011 03:43, Simon Glass a écrit : >> >>> The test was >>> >>> mrc p15, 0, r0, c0, c0, 0 @ get ID register >>> and r0, r0, #0xf0000 @ get architecture >>> cmp r0, #0xf0000 @ check for> ARMv6 >>> movne pc, lr @ else skip cache init >>> >>> Unfortunately I think it is a plain ARM7TDMI with no CP15. >> >> What about other fields in r0 right after mrc? > > I don't really understand that sentence, sorry. > > The ARM7TDMI does not have a CP15 and aborts if I try to access it. > Just in case there is something odd going on I checked with DSTREAM / > RVdebug and it definitely doesn't have a CP15. [as Ford Prefect would > say, I counted them twice] Ok, so debug tools do not show cp15. But tools can be tailored to what tool makes think is needed -- I could tell about some debugging tools that will not let me see all I want a core because the debug designers had finite resources and what I wanted was not a priority to them. OTOH, according to ARM, ARM7DTMI is an ARMv4T architecture, and indeed cp15 is mandatory only for ARMv6 and up, but ARM also states cp15 support was a de facto standard already for ARMv4. So I am left with the question: would the Tegra2 AVP be the only ARM implementation supported by U-Boot that does not have a cp15? That is possible, but I want direct testimony from Nividia. This is why I asked about the Tegra2 TRM, or whatever Nvidia calls it, in case it would explicitely state if AVP has cp15 support or not. Failing that, I'd be ok with experimenting but through the AVP, not through debugging tools -- encoding a cp15 MIDR read in the U-Boot startup code, step through it with the debugger and see if it causes an UND or not, and if not, what is the hex value of r0 -- maybe that is exactly what you did, but I am not 100% sure it is, hence my insistence. I am especially surprised that a recent core be synthesized without a means for run-time core identification, especially in a design with two ARM cores. > The simplest thing I have been able to think of that does not involve > exceptions, differing instruction behaviour, doing the init later or > putting in some Tegra-specific code is to check for the existence of > the Q bit in the CPSR (actually APSR on ARMv7). This does seem to work > and I have verified both in my old 1996 ARM ARM DDI 0100B and the > ARMv7-A one (DDI 0406B) that from an architecture point of view this > should work. The Q bit is RAZ on ARMv4T. This could hep if we really cannot access the Main ID Register on the AVP. > I believe this will cope with the Cortex-A7 / A-15 combinations and > possibly even Cortex-R4 / A-15 although I have not tested this. I > suppose we can deal with this when it becomes an issue. > > So I have redone this one patch with that in mind, and adjusted the > series slightly to fit with this. I will resend it when it completes > MAKEALL. > > I hope that this resolves the matter, but if not(!), I would very much > appreciate it if you could send through some actual pseudo code > showing what you are looking for, to avoid any confusion. Well, I just want to see if the MIDR is accessible and what its value is, so I want the AVP to execute mrc p15, 0, r0, c0, c0, 0 The ending 0 is what selects MIDR rather than other cp15 registers -- other values can cause UND (and I would gladly understand that AVP goes UND for reading cp15 CTR for instance). The simplest test would be to insert the exact instruction above in the reset sequence in start.S right after SVC32 switch, debug the reset execution path, see if the mrc above goes UND or else check r0's contents after mrc is done. > Thanks, > Simon Amicalement,
Hi Albert, On Sun, Oct 30, 2011 at 3:16 AM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Hi Simon, > > Le 29/10/2011 02:36, Simon Glass a écrit : >> >> Hi Albert, >> >> On Thu, Oct 27, 2011 at 10:09 PM, Albert ARIBAUD >> <albert.u.boot@aribaud.net> wrote: >>> >>> Le 28/10/2011 03:43, Simon Glass a écrit : >>> >>>> The test was >>>> >>>> mrc p15, 0, r0, c0, c0, 0 @ get ID register >>>> and r0, r0, #0xf0000 @ get architecture >>>> cmp r0, #0xf0000 @ check for> ARMv6 >>>> movne pc, lr @ else skip cache init >>>> >>>> Unfortunately I think it is a plain ARM7TDMI with no CP15. >>> >>> What about other fields in r0 right after mrc? >> >> I don't really understand that sentence, sorry. >> >> The ARM7TDMI does not have a CP15 and aborts if I try to access it. >> Just in case there is something odd going on I checked with DSTREAM / >> RVdebug and it definitely doesn't have a CP15. [as Ford Prefect would >> say, I counted them twice] > > Ok, so debug tools do not show cp15. But tools can be tailored to what tool > makes think is needed -- I could tell about some debugging tools that will > not let me see all I want a core because the debug designers had finite > resources and what I wanted was not a priority to them. Ye of little faith... > > OTOH, according to ARM, ARM7DTMI is an ARMv4T architecture, and indeed cp15 > is mandatory only for ARMv6 and up, but ARM also states cp15 support was a > de facto standard already for ARMv4. Yes it is de facto when you have a core with that support - typically an MMU and caches. But in that case it would have three digits, as in ARM720T, rather than ARM7TDMI. It is definitely an ARM7TDMI (actually -S) core in the datasheet, and there is no external CP15 block shown, etc. (Although I can see a cache!) > > So I am left with the question: would the Tegra2 AVP be the only ARM > implementation supported by U-Boot that does not have a cp15? That is > possible, but I want direct testimony from Nividia. No there are lots and lots of ARM7TDMIs out there, just not that many that run U-Boot - and Linux is hard without an MMU. But even in the U-Boot tree we have the s3c44b0 cpu which seems to be a plain ARM7TDMI. In terms of direct testimony, two engineers are on this thread and may wish to chime in. The datasheet is pretty clear to me... > > This is why I asked about the Tegra2 TRM, or whatever Nvidia calls it, in > case it would explicitely state if AVP has cp15 support or not. Failing See above, it clearly states ARM7TDMI-S. > that, I'd be ok with experimenting but through the AVP, not through > debugging tools -- encoding a cp15 MIDR read in the U-Boot startup code, > step through it with the debugger and see if it causes an UND or not, and if > not, what is the hex value of r0 -- maybe that is exactly what you did, but > I am not 100% sure it is, hence my insistence. OK I have done this, and yes the AVP definitely takes the undef exception when you execute: mrc p15, 0, r0, c0, c0, 0 > > I am especially surprised that a recent core be synthesized without a means > for run-time core identification, especially in a design with two ARM cores. The -S means synthesized. It does have its own method of identification as I mentioned. There is a Tegra-specific register that I can read. > >> The simplest thing I have been able to think of that does not involve >> exceptions, differing instruction behaviour, doing the init later or >> putting in some Tegra-specific code is to check for the existence of >> the Q bit in the CPSR (actually APSR on ARMv7). This does seem to work >> and I have verified both in my old 1996 ARM ARM DDI 0100B and the >> ARMv7-A one (DDI 0406B) that from an architecture point of view this >> should work. The Q bit is RAZ on ARMv4T. > > This could hep if we really cannot access the Main ID Register on the AVP. OK, well I will send a patch set up with this change. > >> I believe this will cope with the Cortex-A7 / A-15 combinations and >> possibly even Cortex-R4 / A-15 although I have not tested this. I >> suppose we can deal with this when it becomes an issue. >> >> So I have redone this one patch with that in mind, and adjusted the >> series slightly to fit with this. I will resend it when it completes >> MAKEALL. >> >> I hope that this resolves the matter, but if not(!), I would very much >> appreciate it if you could send through some actual pseudo code >> showing what you are looking for, to avoid any confusion. > > Well, I just want to see if the MIDR is accessible and what its value is, so > I want the AVP to execute > > mrc p15, 0, r0, c0, c0, 0 > > The ending 0 is what selects MIDR rather than other cp15 registers -- other > values can cause UND (and I would gladly understand that AVP goes UND for > reading cp15 CTR for instance). > > The simplest test would be to insert the exact instruction above in the > reset sequence in start.S right after SVC32 switch, debug the reset > execution path, see if the mrc above goes UND or else check r0's contents > after mrc is done. OK see above, I have done this. What I meant was for you to provide a code sequence that you want in start.S. Hopefully my patch will fit the bill. I am very happy for this to be rewritten/changed down the track. But for now, and assuming you don't want to call a function to find out whether or not CP15 exists, I have created something which seems to solve the problem. There are many other problems to solve. I feel that this one has had its fair share of attention! Regards, Simon > >> Thanks, >> Simon > > Amicalement, > -- > Albert. >
From one of our HW engineers: The AVP does not have a CP15. As this thread speculates, it is an ARM7TDMI which is a basic ARM V4 core with no ARM co-processors or MMU. Also, ARM app note 99 (DAI0099C_core_type_rev_id.pdf) says the same thing. HTH, Tom > -----Original Message----- > From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass > Sent: Monday, October 31, 2011 2:44 PM > To: Albert ARIBAUD > Cc: U-Boot Mailing List; Tom Warren; Stephen Warren > Subject: Re: [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit() > > Hi Albert, > > On Sun, Oct 30, 2011 at 3:16 AM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: > > Hi Simon, > > > > Le 29/10/2011 02:36, Simon Glass a écrit : > >> > >> Hi Albert, > >> > >> On Thu, Oct 27, 2011 at 10:09 PM, Albert ARIBAUD > >> <albert.u.boot@aribaud.net> wrote: > >>> > >>> Le 28/10/2011 03:43, Simon Glass a écrit : > >>> > >>>> The test was > >>>> > >>>> mrc p15, 0, r0, c0, c0, 0 @ get ID register > >>>> and r0, r0, #0xf0000 @ get architecture > >>>> cmp r0, #0xf0000 @ check for> ARMv6 > >>>> movne pc, lr @ else skip cache init > >>>> > >>>> Unfortunately I think it is a plain ARM7TDMI with no CP15. > >>> > >>> What about other fields in r0 right after mrc? > >> > >> I don't really understand that sentence, sorry. > >> > >> The ARM7TDMI does not have a CP15 and aborts if I try to access it. > >> Just in case there is something odd going on I checked with DSTREAM / > >> RVdebug and it definitely doesn't have a CP15. [as Ford Prefect would > >> say, I counted them twice] > > > > Ok, so debug tools do not show cp15. But tools can be tailored to what > tool > > makes think is needed -- I could tell about some debugging tools that will > > not let me see all I want a core because the debug designers had finite > > resources and what I wanted was not a priority to them. > > Ye of little faith... > > > > > OTOH, according to ARM, ARM7DTMI is an ARMv4T architecture, and indeed > cp15 > > is mandatory only for ARMv6 and up, but ARM also states cp15 support was a > > de facto standard already for ARMv4. > > Yes it is de facto when you have a core with that support - typically > an MMU and caches. But in that case it would have three digits, as in > ARM720T, rather than ARM7TDMI. It is definitely an ARM7TDMI (actually > -S) core in the datasheet, and there is no external CP15 block shown, > etc. (Although I can see a cache!) > > > > > So I am left with the question: would the Tegra2 AVP be the only ARM > > implementation supported by U-Boot that does not have a cp15? That is > > possible, but I want direct testimony from Nividia. > > No there are lots and lots of ARM7TDMIs out there, just not that many > that run U-Boot - and Linux is hard without an MMU. But even in the > U-Boot tree we have the s3c44b0 cpu which seems to be a plain > ARM7TDMI. > > In terms of direct testimony, two engineers are on this thread and may > wish to chime in. The datasheet is pretty clear to me... > > > > > This is why I asked about the Tegra2 TRM, or whatever Nvidia calls it, in > > case it would explicitely state if AVP has cp15 support or not. Failing > > See above, it clearly states ARM7TDMI-S. > > > that, I'd be ok with experimenting but through the AVP, not through > > debugging tools -- encoding a cp15 MIDR read in the U-Boot startup code, > > step through it with the debugger and see if it causes an UND or not, and > if > > not, what is the hex value of r0 -- maybe that is exactly what you did, > but > > I am not 100% sure it is, hence my insistence. > > OK I have done this, and yes the AVP definitely takes the undef > exception when you execute: > > mrc p15, 0, r0, c0, c0, 0 > > > > > I am especially surprised that a recent core be synthesized without a > means > > for run-time core identification, especially in a design with two ARM > cores. > > The -S means synthesized. It does have its own method of > identification as I mentioned. There is a Tegra-specific register that > I can read. > > > > >> The simplest thing I have been able to think of that does not involve > >> exceptions, differing instruction behaviour, doing the init later or > >> putting in some Tegra-specific code is to check for the existence of > >> the Q bit in the CPSR (actually APSR on ARMv7). This does seem to work > >> and I have verified both in my old 1996 ARM ARM DDI 0100B and the > >> ARMv7-A one (DDI 0406B) that from an architecture point of view this > >> should work. The Q bit is RAZ on ARMv4T. > > > > This could hep if we really cannot access the Main ID Register on the AVP. > > OK, well I will send a patch set up with this change. > > > > >> I believe this will cope with the Cortex-A7 / A-15 combinations and > >> possibly even Cortex-R4 / A-15 although I have not tested this. I > >> suppose we can deal with this when it becomes an issue. > >> > >> So I have redone this one patch with that in mind, and adjusted the > >> series slightly to fit with this. I will resend it when it completes > >> MAKEALL. > >> > >> I hope that this resolves the matter, but if not(!), I would very much > >> appreciate it if you could send through some actual pseudo code > >> showing what you are looking for, to avoid any confusion. > > > > Well, I just want to see if the MIDR is accessible and what its value is, > so > > I want the AVP to execute > > > > mrc p15, 0, r0, c0, c0, 0 > > > > The ending 0 is what selects MIDR rather than other cp15 registers -- > other > > values can cause UND (and I would gladly understand that AVP goes UND for > > reading cp15 CTR for instance). > > > > The simplest test would be to insert the exact instruction above in the > > reset sequence in start.S right after SVC32 switch, debug the reset > > execution path, see if the mrc above goes UND or else check r0's contents > > after mrc is done. > > OK see above, I have done this. What I meant was for you to provide a > code sequence that you want in start.S. Hopefully my patch will fit > the bill. > > I am very happy for this to be rewritten/changed down the track. But > for now, and assuming you don't want to call a function to find out > whether or not CP15 exists, I have created something which seems to > solve the problem. There are many other problems to solve. I feel that > this one has had its fair share of attention! > > Regards, > Simon > > > > >> Thanks, > >> Simon > > > > Amicalement, > > -- > > Albert. > > ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. -----------------------------------------------------------------------------------
Hi Albert. On Mon, Oct 31, 2011 at 3:54 PM, Tom Warren <TWarren@nvidia.com> wrote: > From one of our HW engineers: > > The AVP does not have a CP15. > As this thread speculates, it is an ARM7TDMI which is a basic ARM V4 core with no ARM co-processors or MMU. > > Also, ARM app note 99 (DAI0099C_core_type_rev_id.pdf) says the same thing. > > HTH, > > Tom > >> -----Original Message----- >> From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass >> Sent: Monday, October 31, 2011 2:44 PM >> To: Albert ARIBAUD >> Cc: U-Boot Mailing List; Tom Warren; Stephen Warren >> Subject: Re: [PATCH 3/9] arm: Move CP15 init out of cpu_init_crit() >> >> Hi Albert, >> >> On Sun, Oct 30, 2011 at 3:16 AM, Albert ARIBAUD >> <albert.u.boot@aribaud.net> wrote: >> > Hi Simon, >> > >> > Le 29/10/2011 02:36, Simon Glass a écrit : >> >> >> >> Hi Albert, >> >> >> >> On Thu, Oct 27, 2011 at 10:09 PM, Albert ARIBAUD >> >> <albert.u.boot@aribaud.net> wrote: >> >>> >> >>> Le 28/10/2011 03:43, Simon Glass a écrit : >> >>> >> >>>> The test was >> >>>> >> >>>> mrc p15, 0, r0, c0, c0, 0 @ get ID register >> >>>> and r0, r0, #0xf0000 @ get architecture >> >>>> cmp r0, #0xf0000 @ check for> ARMv6 >> >>>> movne pc, lr @ else skip cache init >> >>>> >> >>>> Unfortunately I think it is a plain ARM7TDMI with no CP15. >> >>> >> >>> What about other fields in r0 right after mrc? >> >> >> >> I don't really understand that sentence, sorry. >> >> >> >> The ARM7TDMI does not have a CP15 and aborts if I try to access it. >> >> Just in case there is something odd going on I checked with DSTREAM / >> >> RVdebug and it definitely doesn't have a CP15. [as Ford Prefect would >> >> say, I counted them twice] >> > >> > Ok, so debug tools do not show cp15. But tools can be tailored to what >> tool >> > makes think is needed -- I could tell about some debugging tools that will >> > not let me see all I want a core because the debug designers had finite >> > resources and what I wanted was not a priority to them. >> >> Ye of little faith... >> >> > >> > OTOH, according to ARM, ARM7DTMI is an ARMv4T architecture, and indeed >> cp15 >> > is mandatory only for ARMv6 and up, but ARM also states cp15 support was a >> > de facto standard already for ARMv4. >> >> Yes it is de facto when you have a core with that support - typically >> an MMU and caches. But in that case it would have three digits, as in >> ARM720T, rather than ARM7TDMI. It is definitely an ARM7TDMI (actually >> -S) core in the datasheet, and there is no external CP15 block shown, >> etc. (Although I can see a cache!) >> >> > >> > So I am left with the question: would the Tegra2 AVP be the only ARM >> > implementation supported by U-Boot that does not have a cp15? That is >> > possible, but I want direct testimony from Nividia. >> >> No there are lots and lots of ARM7TDMIs out there, just not that many >> that run U-Boot - and Linux is hard without an MMU. But even in the >> U-Boot tree we have the s3c44b0 cpu which seems to be a plain >> ARM7TDMI. >> >> In terms of direct testimony, two engineers are on this thread and may >> wish to chime in. The datasheet is pretty clear to me... >> >> > >> > This is why I asked about the Tegra2 TRM, or whatever Nvidia calls it, in >> > case it would explicitely state if AVP has cp15 support or not. Failing >> >> See above, it clearly states ARM7TDMI-S. >> >> > that, I'd be ok with experimenting but through the AVP, not through >> > debugging tools -- encoding a cp15 MIDR read in the U-Boot startup code, >> > step through it with the debugger and see if it causes an UND or not, and >> if >> > not, what is the hex value of r0 -- maybe that is exactly what you did, >> but >> > I am not 100% sure it is, hence my insistence. >> >> OK I have done this, and yes the AVP definitely takes the undef >> exception when you execute: >> >> mrc p15, 0, r0, c0, c0, 0 >> >> > >> > I am especially surprised that a recent core be synthesized without a >> means >> > for run-time core identification, especially in a design with two ARM >> cores. >> >> The -S means synthesized. It does have its own method of >> identification as I mentioned. There is a Tegra-specific register that >> I can read. >> >> > >> >> The simplest thing I have been able to think of that does not involve >> >> exceptions, differing instruction behaviour, doing the init later or >> >> putting in some Tegra-specific code is to check for the existence of >> >> the Q bit in the CPSR (actually APSR on ARMv7). This does seem to work >> >> and I have verified both in my old 1996 ARM ARM DDI 0100B and the >> >> ARMv7-A one (DDI 0406B) that from an architecture point of view this >> >> should work. The Q bit is RAZ on ARMv4T. >> > >> > This could hep if we really cannot access the Main ID Register on the AVP. >> >> OK, well I will send a patch set up with this change. >> >> > >> >> I believe this will cope with the Cortex-A7 / A-15 combinations and >> >> possibly even Cortex-R4 / A-15 although I have not tested this. I >> >> suppose we can deal with this when it becomes an issue. >> >> >> >> So I have redone this one patch with that in mind, and adjusted the >> >> series slightly to fit with this. I will resend it when it completes >> >> MAKEALL. >> >> >> >> I hope that this resolves the matter, but if not(!), I would very much >> >> appreciate it if you could send through some actual pseudo code >> >> showing what you are looking for, to avoid any confusion. >> > >> > Well, I just want to see if the MIDR is accessible and what its value is, >> so >> > I want the AVP to execute >> > >> > mrc p15, 0, r0, c0, c0, 0 >> > >> > The ending 0 is what selects MIDR rather than other cp15 registers -- >> other >> > values can cause UND (and I would gladly understand that AVP goes UND for >> > reading cp15 CTR for instance). >> > >> > The simplest test would be to insert the exact instruction above in the >> > reset sequence in start.S right after SVC32 switch, debug the reset >> > execution path, see if the mrc above goes UND or else check r0's contents >> > after mrc is done. >> >> OK see above, I have done this. What I meant was for you to provide a >> code sequence that you want in start.S. Hopefully my patch will fit >> the bill. >> >> I am very happy for this to be rewritten/changed down the track. But >> for now, and assuming you don't want to call a function to find out >> whether or not CP15 exists, I have created something which seems to >> solve the problem. There are many other problems to solve. I feel that >> this one has had its fair share of attention! Have you had a chance to look at the revised patch that I sent? Regards, Simon >> >> Regards, >> Simon >> >> > >> >> Thanks, >> >> Simon >> > >> > Amicalement, >> > -- >> > Albert. >> > > ----------------------------------------------------------------------------------- > This email message is for the sole use of the intended recipient(s) and may contain > confidential information. Any unauthorized review, use, disclosure or distribution > is prohibited. If you are not the intended recipient, please contact the sender by > reply email and destroy all copies of the original message. > ----------------------------------------------------------------------------------- >
Hi Simon et al., Thanks to you and others for their patience. Let me summarize the thread: - ARM7TDMI definitely lacks a cp15, as AN99 explicitely states; - ARM provides no other, reliable, way to identify an ARM7TDMI reliably; - Identification through a manufacturer-specific method just defeats the goal. This makes it impossible to support ARM7TDMI (as well as others listed in AM99) with a start.S code that contains cp15 instructions. OTOH, I've gone through a number of ARM TRMs for cores that have caches/MMU and they all state that these are disabled on reset. I will also assume that when U-Boot starts indirectly through SPL or any other mechanism, and if this mechanism uses caches/MMU, then it will disable them properly before passing control to U-Boot, just like U-Boot disables caches etc before passing control to e.g. Linux. I understand that this initialization is done so that the the cache/MMU is in a correct state, and this must still be ensured, but I now think that ensuring this is the burden of whatever passed control to the start label (this includes true resets, branching from SPL or any other IPL, and --ugh-- branching to start from inside U-Boot). Based on this, I revise my opinion that the caches and MMU cleaning code in start.S should be kept there, and actually I now consider that they should be removed entirely. However, removing cp15 inits would mean a *third* revision of your patchset, plus it could cause unexpected issues which I don't want to happen at this point of the release cycle. Therefore, I'll go back to your patch as posted on oct 13th. Apologies for the long discussion. Amicalement,
Hi Simon, Going back to this version of the patch as indicated, I notice something weird: Le 13/10/2011 23:05, Simon Glass a écrit : > Some SOCs have do not start up with their 'main' CPU. The first U-Boot > code may then be executed with a CPU which does not have a CP15, or not a > useful one. > > Here we split the initialization of CP15 into a separate call, which can > be performed later if required. > > Once the main CPU is running, you should call cpu_init_cp15() to perform > this init as early as possible. > > Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not > need to change, this CP15 init is still skipped in that case. The only > impact for these boards is that the cpu_init_cp15() will be available > even if it is never used on these boards. > > Signed-off-by: Simon Glass<sjg@chromium.org> > --- > arch/arm/cpu/armv7/start.S | 24 ++++++++++++++++++------ > arch/arm/include/asm/u-boot-arm.h | 3 +++ > 2 files changed, 21 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S > index db8e9d2..7fdd422 100644 > --- a/arch/arm/cpu/armv7/start.S > +++ b/arch/arm/cpu/armv7/start.S > @@ -168,6 +168,7 @@ next: > #endif > /* the mask ROM code should have PLL and others stable */ > #ifndef CONFIG_SKIP_LOWLEVEL_INIT > + bl cpu_reset_cp15 > bl cpu_init_crit > #endif > > @@ -305,17 +306,16 @@ jump_2_ram: > _board_init_r_ofs: > .word board_init_r - _start > > - > -#ifndef CONFIG_SKIP_LOWLEVEL_INIT > /************************************************************************* > * > - * CPU_init_critical registers > + * cpu_reset_cp15 > * > - * setup important registers > - * setup memory timing > + * Setup CP15 registers (cache, MMU, TLBs). The I-cache is turned on unless > + * CONFIG_SYS_ICACHE_OFF is defined. > * > *************************************************************************/ > -cpu_init_crit: > +.globl cpu_init_cp15 > +cpu_init_cp15: Seems like half this patch says 'cpu_reset_cp15' and then the second half says 'cpu_init_cp15' (I prefer 'cpu_init_cp15'). > /* > * Invalidate L1 I/D > */ > @@ -340,7 +340,19 @@ cpu_init_crit: > orr r0, r0, #0x00001000 @ set bit 12 (I) I-cache > #endif > mcr p15, 0, r0, c1, c0, 0 > + mov pc, lr @ back to my caller > > + > +#ifndef CONFIG_SKIP_LOWLEVEL_INIT > +/************************************************************************* > + * > + * CPU_init_critical registers > + * > + * setup important registers > + * setup memory timing > + * > + *************************************************************************/ > +cpu_init_crit: > /* > * Jump to board specific initialization... > * The Mask ROM will have already initialized > diff --git a/arch/arm/include/asm/u-boot-arm.h b/arch/arm/include/asm/u-boot-arm.h > index d3308f7..4ca75f9 100644 > --- a/arch/arm/include/asm/u-boot-arm.h > +++ b/arch/arm/include/asm/u-boot-arm.h > @@ -46,6 +46,9 @@ extern ulong IRQ_STACK_START_IN; /* 8 bytes in IRQ stack */ > int cpu_init(void); > int cleanup_before_linux(void); > > +/* Set up ARMv7 MMU, caches and TLBs */ > +void cpu_init_cp15(void); > + > /* cpu/.../arch/cpu.c */ > int arch_cpu_init(void); > int arch_misc_init(void); Amicalement,
Hi Albert, On Sat, Nov 5, 2011 at 1:09 AM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Hi Simon et al., > > Thanks to you and others for their patience. Let me summarize the thread: > > - ARM7TDMI definitely lacks a cp15, as AN99 explicitely states; > > - ARM provides no other, reliable, way to identify an ARM7TDMI reliably; > > - Identification through a manufacturer-specific method just defeats the > goal. > > This makes it impossible to support ARM7TDMI (as well as others listed in > AM99) with a start.S code that contains cp15 instructions. > > OTOH, I've gone through a number of ARM TRMs for cores that have caches/MMU > and they all state that these are disabled on reset. I will also assume that > when U-Boot starts indirectly through SPL or any other mechanism, and if > this mechanism uses caches/MMU, then it will disable them properly before > passing control to U-Boot, just like U-Boot disables caches etc before > passing control to e.g. Linux. > > I understand that this initialization is done so that the the cache/MMU is > in a correct state, and this must still be ensured, but I now think that > ensuring this is the burden of whatever passed control to the start label > (this includes true resets, branching from SPL or any other IPL, and --ugh-- > branching to start from inside U-Boot). > > Based on this, I revise my opinion that the caches and MMU cleaning code in > start.S should be kept there, and actually I now consider that they should > be removed entirely. > > However, removing cp15 inits would mean a *third* revision of your patchset, > plus it could cause unexpected issues which I don't want to happen at this > point of the release cycle. Yes - I did something similar at v2 where I moved it into a library. Best left alone for now. I would prefer to continue this discussion if needed later when we have got this series committed. > > Therefore, I'll go back to your patch as posted on oct 13th. > > Apologies for the long discussion. > > Amicalement, > -- > Albert. > OK, it will at least need a rebase - I will take another look. Also please note that my series is based on Stephen Warren's MMC series (1/4 went in via the MMC tree, the other three are ready for your to pick up) and also Stephen's GPIO patch which is still pending. Please pick up Stephen's patches first. Regards, Simon
Hi Albert, On Sat, Nov 5, 2011 at 1:17 AM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Hi Simon, > > Going back to this version of the patch as indicated, I notice something > weird: > > Le 13/10/2011 23:05, Simon Glass a écrit : >> >> Some SOCs have do not start up with their 'main' CPU. The first U-Boot >> code may then be executed with a CPU which does not have a CP15, or not a >> useful one. >> >> Here we split the initialization of CP15 into a separate call, which can >> be performed later if required. >> >> Once the main CPU is running, you should call cpu_init_cp15() to perform >> this init as early as possible. >> >> Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not >> need to change, this CP15 init is still skipped in that case. The only >> impact for these boards is that the cpu_init_cp15() will be available >> even if it is never used on these boards. >> >> Signed-off-by: Simon Glass<sjg@chromium.org> >> --- >> arch/arm/cpu/armv7/start.S | 24 ++++++++++++++++++------ >> arch/arm/include/asm/u-boot-arm.h | 3 +++ >> 2 files changed, 21 insertions(+), 6 deletions(-) >> >> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S >> index db8e9d2..7fdd422 100644 >> --- a/arch/arm/cpu/armv7/start.S >> +++ b/arch/arm/cpu/armv7/start.S >> @@ -168,6 +168,7 @@ next: >> #endif >> /* the mask ROM code should have PLL and others stable */ >> #ifndef CONFIG_SKIP_LOWLEVEL_INIT >> + bl cpu_reset_cp15 >> bl cpu_init_crit >> #endif >> >> @@ -305,17 +306,16 @@ jump_2_ram: >> _board_init_r_ofs: >> .word board_init_r - _start >> >> - >> -#ifndef CONFIG_SKIP_LOWLEVEL_INIT >> >> /************************************************************************* >> * >> - * CPU_init_critical registers >> + * cpu_reset_cp15 >> * >> - * setup important registers >> - * setup memory timing >> + * Setup CP15 registers (cache, MMU, TLBs). The I-cache is turned on >> unless >> + * CONFIG_SYS_ICACHE_OFF is defined. >> * >> >> *************************************************************************/ >> -cpu_init_crit: >> +.globl cpu_init_cp15 >> +cpu_init_cp15: > > Seems like half this patch says 'cpu_reset_cp15' and then the second half > says 'cpu_init_cp15' (I prefer 'cpu_init_cp15'). Given this and the activity in the past 3 weeks I have done a new serial v4 with a rebase and this fixed. The only change I made to the series was in this one patch to correct the first call to cpu_init_cp15 and update the comment. But I suggest you pick up this new series to avoid conflicts. Regards, Simon > >> /* >> * Invalidate L1 I/D >> */ >> @@ -340,7 +340,19 @@ cpu_init_crit: >> orr r0, r0, #0x00001000 @ set bit 12 (I) I-cache >> #endif >> mcr p15, 0, r0, c1, c0, 0 >> + mov pc, lr @ back to my caller >> >> + >> +#ifndef CONFIG_SKIP_LOWLEVEL_INIT >> >> +/************************************************************************* >> + * >> + * CPU_init_critical registers >> + * >> + * setup important registers >> + * setup memory timing >> + * >> + >> *************************************************************************/ >> +cpu_init_crit: >> /* >> * Jump to board specific initialization... >> * The Mask ROM will have already initialized >> diff --git a/arch/arm/include/asm/u-boot-arm.h >> b/arch/arm/include/asm/u-boot-arm.h >> index d3308f7..4ca75f9 100644 >> --- a/arch/arm/include/asm/u-boot-arm.h >> +++ b/arch/arm/include/asm/u-boot-arm.h >> @@ -46,6 +46,9 @@ extern ulong IRQ_STACK_START_IN; /* 8 bytes in IRQ >> stack */ >> int cpu_init(void); >> int cleanup_before_linux(void); >> >> +/* Set up ARMv7 MMU, caches and TLBs */ >> +void cpu_init_cp15(void); >> + >> /* cpu/.../arch/cpu.c */ >> int arch_cpu_init(void); >> int arch_misc_init(void); > > Amicalement, > -- > Albert. >
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index db8e9d2..7fdd422 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -168,6 +168,7 @@ next: #endif /* the mask ROM code should have PLL and others stable */ #ifndef CONFIG_SKIP_LOWLEVEL_INIT + bl cpu_reset_cp15 bl cpu_init_crit #endif @@ -305,17 +306,16 @@ jump_2_ram: _board_init_r_ofs: .word board_init_r - _start - -#ifndef CONFIG_SKIP_LOWLEVEL_INIT /************************************************************************* * - * CPU_init_critical registers + * cpu_reset_cp15 * - * setup important registers - * setup memory timing + * Setup CP15 registers (cache, MMU, TLBs). The I-cache is turned on unless + * CONFIG_SYS_ICACHE_OFF is defined. * *************************************************************************/ -cpu_init_crit: +.globl cpu_init_cp15 +cpu_init_cp15: /* * Invalidate L1 I/D */ @@ -340,7 +340,19 @@ cpu_init_crit: orr r0, r0, #0x00001000 @ set bit 12 (I) I-cache #endif mcr p15, 0, r0, c1, c0, 0 + mov pc, lr @ back to my caller + +#ifndef CONFIG_SKIP_LOWLEVEL_INIT +/************************************************************************* + * + * CPU_init_critical registers + * + * setup important registers + * setup memory timing + * + *************************************************************************/ +cpu_init_crit: /* * Jump to board specific initialization... * The Mask ROM will have already initialized diff --git a/arch/arm/include/asm/u-boot-arm.h b/arch/arm/include/asm/u-boot-arm.h index d3308f7..4ca75f9 100644 --- a/arch/arm/include/asm/u-boot-arm.h +++ b/arch/arm/include/asm/u-boot-arm.h @@ -46,6 +46,9 @@ extern ulong IRQ_STACK_START_IN; /* 8 bytes in IRQ stack */ int cpu_init(void); int cleanup_before_linux(void); +/* Set up ARMv7 MMU, caches and TLBs */ +void cpu_init_cp15(void); + /* cpu/.../arch/cpu.c */ int arch_cpu_init(void); int arch_misc_init(void);
Some SOCs have do not start up with their 'main' CPU. The first U-Boot code may then be executed with a CPU which does not have a CP15, or not a useful one. Here we split the initialization of CP15 into a separate call, which can be performed later if required. Once the main CPU is running, you should call cpu_init_cp15() to perform this init as early as possible. Existing ARMv7 boards which define CONFIG_SKIP_LOWLEVEL_INIT should not need to change, this CP15 init is still skipped in that case. The only impact for these boards is that the cpu_init_cp15() will be available even if it is never used on these boards. Signed-off-by: Simon Glass <sjg@chromium.org> --- arch/arm/cpu/armv7/start.S | 24 ++++++++++++++++++------ arch/arm/include/asm/u-boot-arm.h | 3 +++ 2 files changed, 21 insertions(+), 6 deletions(-)