On Wed, Oct 31, 2018 at 13:54:14 +0000, Alex Bennée wrote:
>
> Emilio G. Cota <c...@braap.org> writes:
>
> > Cc: Aurelien Jarno <aurel...@aurel32.net>
> > Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
> > Signed-off-by: Emilio G. Cota <c...@braap.org>
> > ---
> > target/sh4/op_helper.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
> > index 4f825bae5a..57cc363ccc 100644
> > --- a/target/sh4/op_helper.c
> > +++ b/target/sh4/op_helper.c
> > @@ -105,7 +105,7 @@ void helper_sleep(CPUSH4State *env)
> > {
> > CPUState *cs = CPU(sh_env_get_cpu(env));
> >
> > - cs->halted = 1;
> > + cpu_halted_set(cs, 1);
>
> Looks good:
>
> Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
Thanks!
> > env->in_sleep = 1;
>
> I wonder if you could drop env->in_sleep from CPUSH4State?
> The test in superh_cpu_do_interrupt:
>
> if (do_irq && !env->in_sleep) {
> return; /* masked */
> }
> }
> env->in_sleep = 0;
>
> maybe be simplified is we cpu_set_halted(cs, 0) when servicing a
> delivered irq?
I don't know this target well, but on a quick look I'd worry about
that change interfering with other (generic) setters of
cpu->halted, e.g. cpu_common_reset, as well as the CPU loop
where cpu->halted is read.
In any case I'd leave additional cleanups out of this series,
which is already so long that git rebase is choking a bit
every time I add an R-b tag :D
Thanks,
Emilio
