Re: [Qemu-devel] [PATCH v2 4/4] ARM: exynos4210_pmu: Add software reset support

[Maksim Kozlov <m.kozlov@samsung.com>]

20.07.2012 18:32, Peter Maydell пишет:
> On 12 July 2012 17:54, Maksim Kozlov<m.kozlov@samsung.com>  wrote:
>> Signed-off-by: Maksim Kozlov<m.kozlov@samsung.com>
>> ---
>>   hw/exynos4210_pmu.c |   40 +++++++++++++++++++++++++++++++++-------
>>   1 files changed, 33 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/exynos4210_pmu.c b/hw/exynos4210_pmu.c
>> index 7f09c79..96588d9 100644
>> --- a/hw/exynos4210_pmu.c
>> +++ b/hw/exynos4210_pmu.c
>> @@ -18,13 +18,8 @@
>>    *  with this program; if not, see<http://www.gnu.org/licenses/>.
>>    */
>>
>> -/*
>> - * This model implements PMU registers just as a bulk of memory. Currently,
>> - * the only reason this device exists is that secondary CPU boot loader
>> - * uses PMU INFORM5 register as a holding pen.
>> - */
>> -
>>   #include "sysbus.h"
>> +#include "sysemu.h"
>>
>>   #ifndef DEBUG_PMU
>>   #define DEBUG_PMU           0
>> @@ -230,6 +225,8 @@
>>
>>   #define EXYNOS4210_PMU_REGS_MEM_SIZE 0x3d0c
>>
>> +#define SWRESET_SYSTEM_MASK     0x00000001
>> +
>>   typedef struct Exynos4210PmuReg {
>>       const char  *name; /* for debug only */
>>       uint32_t     offset;
>> @@ -458,7 +455,17 @@ static void exynos4210_pmu_write(void *opaque, target_phys_addr_t offset,
>>       PRINT_DEBUG_EXTEND("%s [0x%04x]<- 0x%04x\n",
>>                exynos4210_pmu_regs[index].name, (uint32_t)offset, (uint32_t)val);
>>
>> -    s->reg[index] = val;
>> +    switch (offset) {
>> +    case SWRESET:
>> +        if (val&  SWRESET_SYSTEM_MASK) {
>> +            s->reg[index] = val;
>> +            qemu_system_reset_request();
>> +        }
>> +        break;
>> +    default:
>> +        s->reg[index] = val;
>> +        break;
>> +    }
>>   }
>>
>>   static const MemoryRegionOps exynos4210_pmu_ops = {
>> @@ -477,9 +484,28 @@ static void exynos4210_pmu_reset(DeviceState *dev)
>>       Exynos4210PmuState *s =
>>               container_of(dev, Exynos4210PmuState, busdev.qdev);
>>       unsigned i;
>> +    uint32_t index = exynos4210_pmu_get_register_index(s, SWRESET);
>> +    uint32_t swreset = s->reg[index];
>>
>>       /* Set default values for registers */
>>       for (i = 0; i<  PMU_NUM_OF_REGISTERS; i++) {
>> +        if (swreset) {
>> +            switch (exynos4210_pmu_regs[i].offset) {
>> +            case INFORM0:
>> +            case INFORM1:
>> +            case INFORM2:
>> +            case INFORM3:
>> +            case INFORM4:
>> +            case INFORM5:
>> +            case INFORM6:
>> +            case INFORM7:
>> +            case PS_HOLD_CONTROL:
>> +                /* keep these registers during SW reset */
>> +                continue;
>> +            default:
>> +                break;
>> +            }
>> +        }
>>           s->reg[i] = exynos4210_pmu_regs[i].reset_value;
>>       }
>>   }
>
> This patch seems to be trying to make a distinction that QEMU doesn't
> support, ie between system wide "software reset" and system wide
> "hard reset". I'm not convinced about the wisdom of trying to paper
> over this lack with a single-device workaround.

Ok. Thank for review

And I found out that this patch contains a mistake, therefore it should 
be rejected in any case

>
> -- PMM
>
>