linux-kernel - Re: [PATCH v3 1/7] arm/virt: place power button pin number on a define

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAFEAcA8VWc3eQZqfJP9k5LYF-9aLChHQ+uS9UBfGV6nvybDxqQ@mail.gmail.com>
Date: Tue, 30 Jul 2024 09:29:37 +0100
From: Peter Maydell <peter.maydell@...aro.org>
To: Igor Mammedov <imammedo@...hat.com>
Cc: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>, 
	Jonathan Cameron <Jonathan.Cameron@...wei.com>, Shiju Jose <shiju.jose@...wei.com>, 
	"Michael S. Tsirkin" <mst@...hat.com>, Ani Sinha <anisinha@...hat.com>, 
	Shannon Zhao <shannon.zhaosl@...il.com>, linux-kernel@...r.kernel.org, 
	qemu-arm@...gnu.org, qemu-devel@...gnu.org
Subject: Re: [PATCH v3 1/7] arm/virt: place power button pin number on a define

On Tue, 30 Jul 2024 at 08:26, Igor Mammedov <imammedo@...hat.com> wrote:
>
> On Mon, 22 Jul 2024 08:45:53 +0200
> Mauro Carvalho Chehab <mchehab+huawei@...nel.org> wrote:
>
> > Having magic numbers inside the code is not a good idea, as it
> > is error-prone. So, instead, create a macro with the number
> > definition.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>

> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index b0c68d66a345..c99c8b1713c6 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1004,7 +1004,7 @@ static void virt_powerdown_req(Notifier *n, void *opaque)
> >      if (s->acpi_dev) {
> >          acpi_send_event(s->acpi_dev, ACPI_POWER_DOWN_STATUS);
> >      } else {
> > -        /* use gpio Pin 3 for power button event */
> > +        /* use gpio Pin for power button event */
> >          qemu_set_irq(qdev_get_gpio_in(gpio_key_dev, 0), 1);
>
> /me confused, it was saying Pin 3 but is passing 0 as argument where as elsewhere
> you are passing 3. Is this a bug?

No. The gpio_key_dev is a gpio-key device which has one
input (which you assert to "press the key") and one output,
which goes high when the key is pressed and then falls
100ms later. The virt board wires up the output of the
gpio-key device to input 3 on the PL061 GPIO controller.
(This happens in create_gpio_keys().) So the code is correct
to assert input 0 on the gpio-key device and the comment
isn't wrong that this results in GPIO pin 3 being asserted:
the link is just indirect.

thanks
-- PMM