Re: [Qemu-devel] [PATCH] Support system reset in Exynos4210

[Dmitry Solodkiy]

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:address@hidden 
Sent: Wednesday, April 04, 2012 9:38 PM
To: Maksim Kozlov
Cc: Kyungmin Park; Igor Mitsyanko; address@hidden; 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<address@hidden>
>>>> ---
>>>>    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