Em Mon, 5 Aug 2024 16:04:39 +0200
Igor Mammedov <imamm...@redhat.com> escreveu:
> On Thu, 1 Aug 2024 15:15:44 +0200
> Mauro Carvalho Chehab <mchehab+hua...@kernel.org> wrote:
>
> > Em Tue, 30 Jul 2024 13:26:20 +0200
> > Igor Mammedov <imamm...@redhat.com> escreveu:
> >
> > > On Tue, 30 Jul 2024 09:29:37 +0100
> > > Peter Maydell <peter.mayd...@linaro.org> wrote:
> > >
> > > > On Tue, 30 Jul 2024 at 08:26, Igor Mammedov <imamm...@redhat.com>
> > > > wrote:
> > > > >
> > > > > On Mon, 22 Jul 2024 08:45:53 +0200
> > > > > Mauro Carvalho Chehab <mchehab+hua...@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+hua...@kernel.org>
> > > > > > Reviewed-by: Jonathan Cameron <jonathan.came...@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.
> > >
> > > it's likely obvious to ARM folks, but maybe comment should
> > > clarify above for unaware.
> >
> > Not sure if a comment here with the pin number is a good idea.
> > After all, this patch was originated because we were using
> > Pin 6 for GPIO error, while the comment was outdated (stating
> > that it was pin 8 instead) :-)
> >
> > After this series, there will be two GPIO pins used inside arm/virt,
> > both defined at arm/virt.h:
> >
> > /* GPIO pins */
> > #define GPIO_PIN_POWER_BUTTON 3
> > #define GPIO_PIN_GENERIC_ERROR 6
> >
> > Those macros are used when GPIOs are created:
> >
> > static void create_gpio_keys(char *fdt, DeviceState *pl061_dev,
> > uint32_t phandle)
> > {
> > gpio_key_dev = sysbus_create_simple("gpio-key", -1,
> > qdev_get_gpio_in(pl061_dev,
> >
> > GPIO_PIN_POWER_BUTTON));
> > gpio_error_dev = sysbus_create_simple("gpio-key", -1,
> > qdev_get_gpio_in(pl061_dev,
> >
> > GPIO_PIN_GENERIC_ERROR));
> > So, at least for me, it is clear that gpio_key_dev is using pin 3.
>
> if you switch to using already existing GED device,
> then this patch will go away since event will be delivered by GED
> instead of GPIO + _AEI.
This patch is actually independent from the rest. It is related to a power
down event, and not related at all with error inject.
The rationale for keeping it on this series was due to the original
patch 2 (as otherwise merge conflicts would rise). It can now be merged
in separate.
Btw, this is doing a cleanup requested by Michael and Peter:
https://lore.kernel.org/qemu-devel/CAFEAcA-PYnZ-32MRX+PgvzhnoAV80zBKMYg61j2f=ohagfw...@mail.gmail.com/
Thanks,
Mauro
