This patch series implements the nRF51 DETECT signal
in the GPIO peripheral. A qtest is added exercising the signal.

To implement the test, named out-GPIO IRQ interception had to be added
to the qtest framework. I also took the opportunity to improve IRQ
interception a bit by adding 'FAIL' responses when interception fails.
Otherwise, it is frustrating to troubleshoot why calls to
qtest_irq_intercept_out and friends appears to do nothing.

Chris Laplante (6):
  hw/gpio/nrf51: implement DETECT signal
  qtest: implement named interception of out-GPIO
  qtest: bail from irq_intercept_in if name is specified
  qtest: factor out qtest_install_gpio_out_intercepts
  qtest: irq_intercept_[out/in]: return FAIL if no intercepts are
    installed
  qtest: microbit-test: add tests for nRF51 DETECT

 hw/arm/nrf51_soc.c           |  1 +
 hw/gpio/nrf51_gpio.c         | 14 ++++++++-
 include/hw/gpio/nrf51_gpio.h |  1 +
 softmmu/qtest.c              | 56 ++++++++++++++++++++++++++----------
 tests/qtest/libqtest.c       |  6 ++++
 tests/qtest/libqtest.h       | 11 +++++++
 tests/qtest/microbit-test.c  | 42 +++++++++++++++++++++++++++
 7 files changed, 115 insertions(+), 16 deletions(-)

--
2.39.2

On Sat, 15 Jul 2023 at 00:27, Chris Laplante <chris@laplante.io> wrote:
>
> This patch series implements the nRF51 DETECT signal
> in the GPIO peripheral. A qtest is added exercising the signal.
>
> To implement the test, named out-GPIO IRQ interception had to be added
> to the qtest framework. I also took the opportunity to improve IRQ
> interception a bit by adding 'FAIL' responses when interception fails.
> Otherwise, it is frustrating to troubleshoot why calls to
> qtest_irq_intercept_out and friends appears to do nothing.

Thanks for this patchset and especially for the work
improving the qtest infrastructure. I've given my
comments on the different patches, and in some cases
reviewed-by tags. (Where I've given one of those, you should
add it to your commit message for the relevant patch under
your Signed-off-by: line, so that when you send the version
2 of the patchset we know that those parts are already
reviewed and don't need re-examining. If I said "make
some change; otherwise Reviewed-by" that means "make
that minor change, and then you can add the tag, etc".)

Do you have the parts of this feature that use the DETECT
signal in the POWER device, or have you not written those
yet ? If you have them, you could send those too in v2.

-- PMM

Hi Peter,

> Thanks for this patchset and especially for the work
> improving the qtest infrastructure. I've given my
> comments on the different patches, and in some cases
> reviewed-by tags. (Where I've given one of those, you should
> add it to your commit message for the relevant patch under
> your Signed-off-by: line, so that when you send the version
> 2 of the patchset we know that those parts are already
> reviewed and don't need re-examining. If I said "make
> some change; otherwise Reviewed-by" that means "make
> that minor change, and then you can add the tag, etc".)

Thanks very much for the feedback and help!

> Do you have the parts of this feature that use the DETECT
> signal in the POWER device, or have you not written those
> yet ? If you have them, you could send those too in v2.

That part is halfway done, so I will work on finishing it before submitting v2. Two questions regarding that (to potentially save us a v3): 

1. The nRF51 POWER device overlaps with the memory maps of the CLOCK and MPU devices. So I have created a CPM (CLOCK, POWER, MPU) device in hw/misc. Does that sound reasonable naming-wise?
2. I also have some implementations for pieces of CLOCK, namely the HFCLKSTART/HFCLKSTOP events and HFCLKSTARTED event. Should I include that in this patch series, or would you prefer it in a separate series? It is unrelated to DETECT and POWER.

Thanks,
Chris

On Tue, 25 Jul 2023 at 04:25, Chris Laplante <chris@laplante.io> wrote:
>
> Hi Peter,
>
> > Thanks for this patchset and especially for the work
> > improving the qtest infrastructure. I've given my
> > comments on the different patches, and in some cases
> > reviewed-by tags. (Where I've given one of those, you should
> > add it to your commit message for the relevant patch under
> > your Signed-off-by: line, so that when you send the version
> > 2 of the patchset we know that those parts are already
> > reviewed and don't need re-examining. If I said "make
> > some change; otherwise Reviewed-by" that means "make
> > that minor change, and then you can add the tag, etc".)
>
> Thanks very much for the feedback and help!
>
> > Do you have the parts of this feature that use the DETECT
> > signal in the POWER device, or have you not written those
> > yet ? If you have them, you could send those too in v2.
>
> That part is halfway done, so I will work on finishing it before submitting v2. Two questions regarding that (to potentially save us a v3):
>
> 1. The nRF51 POWER device overlaps with the memory maps of the CLOCK and MPU devices. So I have created a CPM (CLOCK, POWER, MPU) device in hw/misc. Does that sound reasonable naming-wise?

Yes, I think from QEMU's point of view the massive register
overlap makes them a single device. The name sounds OK
(give it the same kind of nrf51 prefix the rng device has).

> 2. I also have some implementations for pieces of CLOCK, namely the HFCLKSTART/HFCLKSTOP events and HFCLKSTARTED event. Should I include that in this patch series, or would you prefer it in a separate series? It is unrelated to DETECT and POWER.

If you think they're ready to go in, and it doesn't
make the series more than about 12-15 patches long,
you can put them on the end of the series. If the
patchset is starting to get a bit big then it might
be easier to get the POWER/DETECT parts reviewed
first.

thanks
-- PMM

Hi Peter,

> > 2. I also have some implementations for pieces of CLOCK, namely the HFCLKSTART/HFCLKSTOP events and HFCLKSTARTED event. Should I include that in this patch series, or would you prefer it in a separate series? It is unrelated to DETECT and POWER.
> 
> 
> If you think they're ready to go in, and it doesn't
> make the series more than about 12-15 patches long,
> you can put them on the end of the series. If the
> patchset is starting to get a bit big then it might
> be easier to get the POWER/DETECT parts reviewed
> first.

Unrelated question regarding the CLOCK module. Should I model the startup times for the various crystal oscillators? Or should I just assume they start instantly for simplicity? External xtal startup time is 750-800 us. Internal RC startup time is 4-5 us. I've already modeled the delay for the external xtal, but just wondering if its worth the extra code.

Thanks,
Chris

On Thu, 27 Jul 2023 at 23:51, Chris Laplante <chris@laplante.io> wrote:
>
> Hi Peter,
>
> > > 2. I also have some implementations for pieces of CLOCK, namely the HFCLKSTART/HFCLKSTOP events and HFCLKSTARTED event. Should I include that in this patch series, or would you prefer it in a separate series? It is unrelated to DETECT and POWER.
> >
> >
> > If you think they're ready to go in, and it doesn't
> > make the series more than about 12-15 patches long,
> > you can put them on the end of the series. If the
> > patchset is starting to get a bit big then it might
> > be easier to get the POWER/DETECT parts reviewed
> > first.
>
> Unrelated question regarding the CLOCK module. Should I model the startup times for the various crystal oscillators? Or should I just assume they start instantly for simplicity? External xtal startup time is 750-800 us. Internal RC startup time is 4-5 us. I've already modeled the delay for the external xtal, but just wondering if its worth the extra code.

We typically just have that sort of thing start instantly,
unless there's some specific guest workload that falls
over if you don't model the startup delay. Usually
modelling the delay is unnecessary complexity.

thanks
-- PMM

> > 2. I also have some implementations for pieces of CLOCK, namely the HFCLKSTART/HFCLKSTOP events and HFCLKSTARTED event. Should I include that in this patch series, or would you prefer it in a separate series? It is unrelated to DETECT and POWER.
> 
> 
> If you think they're ready to go in, and it doesn't
> make the series more than about 12-15 patches long,
> you can put them on the end of the series. If the
> patchset is starting to get a bit big then it might
> be easier to get the POWER/DETECT parts reviewed
> first.

I'm just going to send the POWER/DETECT bits first. There is quite a lot to emulate in CLOCK, POWER, and MPU, and I'd like to do a good job on it.

Thanks.
Chris