[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [PATCH 1/1] arm: virt: change GPIO trigger interrupt
From: |
Shannon Zhao |
Subject: |
Re: [Qemu-trivial] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse |
Date: |
Sat, 20 Feb 2016 18:53:37 +0800 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 |
Hi Wei,
On 2016/2/10 6:59, Wei Huang wrote:
>
> On 02/04/2016 12:51 AM, Shannon Zhao wrote:
>>
>>
>> On 2016/2/4 14:10, Wei Huang wrote:
>>>
>>> On 02/03/2016 07:44 PM, Shannon Zhao wrote:
>
> <snip>
>
>>> I reversed the order of edge pulling. The state is 1 according to printk
>>> inside gpio_keys driver. However the reboot still failed with two
>>> reboots (1 very early, 1 later).
>>>
>> Because to make the input work, it should call input_event twice I think.
>>
>> input_event(input, type, button->code, 1) means the button pressed
>> input_event(input, type, button->code, 0) means the button released
>>
>> But We only see guest entering gpio_keys_gpio_report_event once.
>>
>> My original purpose is like below:
>>
>> call qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1) to make guest
>> execute input_event(input, type, button->code, 1)
>> call qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0) to make guest
>> execute input_event(input, type, button->code, 0).
>>
>> But even though it calls qemu_set_irq twice, it only calls pl061_update
>> once in qemu.
>
> Hi Shannon,
>
> Assuming that we are talking about the special case you found (i.e. send
> reboot very early and then send another one after guest VM fully
> booted). Dug into the code further, here are my findings:
>
> === Why ACPI case failed? ===
> QEMU pl061.c checks the current request against the new request (see
> s->old_in_data ^ s->data in pl061.c file). If no change, nothing will
> happen.
>
> So two consecutive reboots will cause the following state change;
> apparently the second request won't trigger VM reboot because
> pl01_update() decides _not_ to inject IRQ into guest VM.
>
> (PL061State fields) data old_in_data istate
> VM boot 0 0 0
> 1st ACPI reboot request 8 0 0
> After VM PL061 driver ACK 8 8 0
> 2nd ACPI reboot request 8 no-change, no IRQ <==
>
> To solve this problem, we have to invert PL061State->data at least once
> to trigger IRQ inside VM. Two solutions:
>
> S1: "Pulse"
> static void virt_powerdown_req(Notifier *n, void *opaque)
> {
> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0);
> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1);
> }
>
> S2: "Invert"
> static int old_gpio_level = 0;
> static void virt_powerdown_req(Notifier *n, void *opaque)
> {
> /* use gpio Pin 3 for power button event */
> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), !old_gpio_level);
> old_gpio_level = !old_gpio_level;
> }
>
The S2 still doesn't work. After sending the early first reboot, whne
guest boots well, it doesn't react to the second reboot while it reacts
to the third one.
> Both S1 and S2 works for ACPI. But S1 has problem with DT, which is
> explained below.
>
> === Why DT fails with S1 ===
> DT approach requires both PL061 and gpio-keys to work together. Looking
> into guest kernel gpio-keys/input code, you will find that it only
> reacts to both LEVEL-HI and input changes. Here is the code snip from
> drivers/input/input.c file:
>
> /* auto-repeat bypasses state updates */
> if (value == 2) {
> disposition = INPUT_PASS_TO_HANDLERS;
> break;
> }
>
> if (!!test_bit(code, dev->key) != !!value) {
> __change_bit(code, dev->key);
> disposition = INPUT_PASS_TO_HANDLERS;
> }
>
> Unless we adds gpio-keys DT property to "autorepeat", the
> "!!test_bit(code, dev->key) != !!value" is always FALSE with S1. Thus
> systemd won't receive any input event; and no power-switch will be
> triggered.
>
> In comparison S2 will work because value is changed very time.
>
> === Summary ===
> 1. Cleaning PL061 state is required. That means "[PATCH V2 1/2] ARM:
> PL061: Clear PL061 device state after reset" is necessary.
> 2. S2 is better. To support S2, we needs to change GPIO IRQ polarity to
> AML_ACTIVE_BOTH in ACPI description.
>
Thanks. But we don't have the AML_ACTIVE_BOTH since there is only one
bit for "Interrupt Polarity" in ACPI table.
See ACPI SPEC 6.0 "Table 6-216 Extended Interrupt Descriptor Definition"
Bit [2] Interrupt Polarity, _LL
0 Active-High: This interrupt is sampled when the signal is high,
or true.
1 Active-Low: This interrupt is sampled when the signal is low, or
false.
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -326,7 +326,7 @@ static void acpi_dsdt_add_gpio(Aml *scope, const
> MemMapEntry *gpio_memmap,
> Aml *aei = aml_resource_template();
> /* Pin 3 for power button */
> const uint32_t pin_list[1] = {3};
> - aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH,
> + aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, 3,
What's the meaning of 3 here?
> AML_EXCLUSIVE, AML_PULL_UP, 0, pin_list, 1,
> "GPO0", NULL, 0));
> aml_append(dev, aml_name_decl("_AEI", aei));
Thanks,
--
Shannon
- Re: [Qemu-trivial] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse, Michael Tokarev, 2016/02/03
- Re: [Qemu-trivial] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse, Peter Maydell, 2016/02/03
- Re: [Qemu-trivial] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse, Wei Huang, 2016/02/03
- Re: [Qemu-trivial] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse, Shannon Zhao, 2016/02/03
- Re: [Qemu-trivial] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse, Wei Huang, 2016/02/04
- Re: [Qemu-trivial] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse, Shannon Zhao, 2016/02/04
- Re: [Qemu-trivial] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse, Wei Huang, 2016/02/09
- Re: [Qemu-trivial] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse,
Shannon Zhao <=
- Re: [Qemu-trivial] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse, Wei Huang, 2016/02/24
- Re: [Qemu-trivial] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse, Shannon Zhao, 2016/02/26
- Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse, Peter Maydell, 2016/02/26
- Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse, Shannon Zhao, 2016/02/26
- Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse, Peter Maydell, 2016/02/26
- Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse, Wei Huang, 2016/02/26
- Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse, Peter Maydell, 2016/02/26
- Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse, Shannon Zhao, 2016/02/26