Hi Dmitry, Suggest you first get internal specifications via official routes (I have no authority to distribute it) and then propose patches which wouldn't break existing exynos4210 model.
Thanks, Dmitry Solodkiy, Emulator/Kernel PL, Mobile Group, Moscow R&D center, Samsung Electronics -----Original Message----- From: Dmitry Zhurikhin [mailto:z...@ispras.ru] Sent: Wednesday, April 04, 2012 9:38 PM To: Maksim Kozlov Cc: Kyungmin Park; Igor Mitsyanko; qemu-devel@nongnu.org; Evgeny Voevodin; Dmitry Solodkiy Subject: Re: [PATCH] Support system reset in Exynos4210 On 2012-04-04 20:16, Maksim Kozlov wrote: > 04.04.2012 16:35, Dmitry Zhurikhin пишет: >> On 2012-04-04 15:55, Maksim Kozlov wrote: >>> 04.04.2012 14:08, Dmitry Zhurikhin пишет: >>>> Reset the system when 1 is written to SWRESET register >>>> >>>> Signed-off-by: Dmitry Zhurikhin<z...@ispras.ru> >>>> --- >>>> hw/exynos4210_pmu.c | 11 +++++++++++ >>>> 1 files changed, 11 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/hw/exynos4210_pmu.c b/hw/exynos4210_pmu.c index >>>> c12d750..edf6e34 100644 >>>> --- a/hw/exynos4210_pmu.c >>>> +++ b/hw/exynos4210_pmu.c >>>> @@ -25,6 +25,7 @@ >>>> */ >>>> >>>> #include "sysbus.h" >>>> +#include "sysemu.h" >>>> >>>> #ifndef DEBUG_PMU >>>> #define DEBUG_PMU 0 >>>> @@ -422,6 +423,16 @@ static void exynos4210_pmu_write(void *opaque, >>>> target_phys_addr_t offset, >>>> if (reg_p->offset == offset) { >>>> PRINT_DEBUG_EXTEND("%s<0x%04x> <- 0x%04x\n", >>>> reg_p->name, >>>> (uint32_t)offset, (uint32_t)val); >>>> + switch (offset) { >>>> + case SWRESET: >>>> + if (val& 1) { >>>> + qemu_system_reset_request(); >>>> + } >>>> + break; >>>> + default: >>>> + /* Nothing */ >>>> + break; >>>> + } >>>> s->reg[i] = val; >>>> return; >>>> } >>> It's not quite well. At first, when you do reset, appropriate status >>> must be set in RST_STAT register. At second, not all registers in >>> PMU should be set in default value after reset, so you should change >>> PMU reset function for handling different resets (see spec) So, this >>> functionality should be wrote more carefully >> Well, this is the case when there is a need to modify booting >> procedure depending on the values of these registers. I haven't >> found any such code in the current kernel. As I now remember saving >> their values was indeed important when we were trying to use an >> U-Boot bootloader. But as long as we are sticking with QEMU bootloader it >> doesn't matter. > > 3.4-rc1 and 2.6.36 kernels use INFORM5 register which should keep his > value during sw reset. And U-Boot (which can be used instead of kernel > for some purposes) uses INFORM[456] registers which should be saved as > well. > > My opinion is we should not add code which describe incorrect behavior > of the device regardless of whether kernel uses some registers or no. OK, this is a righteous position. Again, I think in this case implementing what you propose won't be neither bad nor good. But since you've asked, I'll try to do it. > >> Anyway this information is unfortunately absent in Exynos4210 public >> documentation. > Hm... Really, I've just found out that public specification doesn't > contain information about PMU I can make the changes according to Exynos3210 specification since they are pretty close. But of course I'd prefer that you send me full specifications. Dmitry, what you say? >>> >>> And use #define for registers and fields of the registers. It's more >>> clearly, as for me. >> As you say. > :) >>> >>> Regards, >>> MK >> >> Regards, >> Dmitry >> >> > Regards, Dmitry