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

[Maksim Kozlov]

04.04.2012 21:37, Dmitry Zhurikhin пишет:
> 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?
PMUs in exynos4210 and in exynos3210 is not pretty close. They have 
significant differences.

> > > > 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