Re: [PATCH 1/6] hw/gpio/nrf51: implement DETECT signal

[Peter Maydell]

On Sat, 15 Jul 2023 at 00:27, Chris Laplante <chris@laplante.io> wrote:
>
> Implement nRF51 DETECT signal in the GPIO peripheral.
>
> The reference manual makes mention of a per-pin DETECT signal, but these
> are not exposed to the user. See 
> https://devzone.nordicsemi.com/f/nordic-q-a/39858/gpio-per-pin-detect-signal-available
> for more information. Currently, I don't see a reason to model these.

I agree -- they seem to be internal to the GPIO module,
so we don't need to model them as qemu_irq lines.

> Signed-off-by: Chris Laplante <chris@laplante.io>
> ---
>  hw/arm/nrf51_soc.c           |  1 +
>  hw/gpio/nrf51_gpio.c         | 14 +++++++++++++-
>  include/hw/gpio/nrf51_gpio.h |  1 +
>  3 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/nrf51_soc.c b/hw/arm/nrf51_soc.c
> index 34da0d62f0..7ae54e18be 100644
> --- a/hw/arm/nrf51_soc.c
> +++ b/hw/arm/nrf51_soc.c
> @@ -150,6 +150,7 @@ static void nrf51_soc_realize(DeviceState *dev_soc, Error 
> **errp)
>
>      /* Pass all GPIOs to the SOC layer so they are available to the board */
>      qdev_pass_gpios(DEVICE(&s->gpio), dev_soc, NULL);
> +    qdev_pass_gpios(DEVICE(&s->gpio), dev_soc, "detect");

Is the DETECT line really exposed external to the SoC?
I had a look at the nRF51822 datasheet and it suggests not.
For purposes of supporting the wake-up-on-gpio functionality
we don't need to expose it to the board -- the SoC layer
can just wire it up to the POWER device. (In fact, exposing
it to the board makes it harder, because you can't connect
one qemu_irq to two places, so if we let the board connect
it somewhere then the SoC can't conveniently connect it
to the POWER device without doing extra work to split it.)

The logic for calculating DETECT looks good to me.

thanks
-- PMM