Re: [Qemu-devel] [PATCH] hw/arm/pxa2xx: Correctly handle external GPIO reset requests

[Peter Maydell]

On 25 October 2016 at 14:27, Guenter Roeck <address@hidden> wrote:
> Hi Peter,
>
>
> On 10/25/2016 04:49 AM, Peter Maydell wrote:
>>
>> On 25 October 2016 at 03:06, Guenter Roeck <address@hidden> wrote:
>>>
>>> The internal GPIO reset, enabled with GPR_EN, only applies to GPIO pin 1.
>>> If other GPIO pins are used for reset, this is unrelated to GPR_EN, the
>>> reset is an external reset pin, and it resets the entire system.
>>>
>>> This fixes GPIO reset failures seen with various PXA270 emulations
>>> (akita,
>>> borzoi, spitz, tosa, terrier) when running Linux.
>>>
>>> Signed-off-by: Guenter Roeck <address@hidden>
>>> ---
>>>  hw/arm/pxa2xx.c | 10 +++++++++-
>>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
>>> index cb55704..2a2a821 100644
>>> --- a/hw/arm/pxa2xx.c
>>> +++ b/hw/arm/pxa2xx.c
>>> @@ -2048,10 +2048,18 @@ static void pxa2xx_reset(void *opaque, int line,
>>> int level)
>>>  {
>>>      PXA2xxState *s = (PXA2xxState *) opaque;
>>>
>>> -    if (level && (s->pm_regs[PCFR >> 2] & 0x10)) {     /* GPR_EN */
>>> +    /*
>>> +     * GPIO pin 1 is the CPU internal GPIO reset, enabled with GPR_EN.
>>> +     * Any other pin is board specific and resets the entire system.
>>> +     */
>>> +    if (line == 1 && level && (s->pm_regs[PCFR >> 2] & 0x10)) {
>>> /* GPR_EN */
>>>          cpu_reset(CPU(s->cpu));
>>>          /* TODO: reset peripherals */
>>>      }
>>> +
>>> +    if (line != 1 && level) {
>>> +        qemu_system_reset_request();
>>> +    }
>>
>>
>> It doesn't look to me like we wire up more than the first
>> line (at least the qdev_connect_gpio_out() calls which
>> connect up to s->reset in pxa255_init() and pxa270_init()
>> only connect up one line). What am I missing that can
>> cause line to be something other than 1?
>>
>
> Here is what added logging into the function tells me:
>
> reboot: Restarting system
> GR>> pxa2xx_reset(): line=0, level=1
>
> Maybe it is line 0 that is wired up ?

Yes, that makes sense -- the 'line' variable will be 0
on input to this function, but it is wired up to the
pxa2xx GPIO's output 1 (not 0). Should that be a power-on
reset or a CPU reset ?

thanks
-- PMM