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

[Peter Maydell]

On Tue, 30 Jul 2024 at 08:26, Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Mon, 22 Jul 2024 08:45:53 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.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@kernel.org>
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.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