Emilio G. Cota <c...@braap.org> writes:
> 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
No worries, it was more a general query thrown in the direction of the
sh4 maintainers ;-)
--
Alex Bennée
