Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add counters to stm32xx-spi-server-core ringbuf #1670

Merged
merged 3 commits into from
Mar 21, 2024
Merged

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Mar 20, 2024

The stm32xx-spi-server-core ringbuf fills up pretty quickly when the system is busy, so it's hard to see the total number of SPI operations that may have occurred in the past. This branch adds counters to this ring buffer.

Note that this PR depends on an upstream idol change, oxidecomputer/idolatry#49, to allow us to derive the Count trait for the SpiOperation generated type. So, we should merge that first, and get this pointed back at oxidecomputer/idolatry@main.

For example (on my gimletlet):

$ humility ringbuf spi_server_core
humility: attached via ST-Link V3
humility: ring buffer drv_stm32h7_spi_server_core::__RINGBUF in net:
   TOTAL VARIANT
    3374 WaitISR
    3184 Tx
    3184 Rx
     590 Start(exchange)
     206 Start(write)
 NDX LINE      GEN    COUNT PAYLOAD
   6  467      117        2 Tx(0x0)
   7  531      117        2 WaitISR(0x40012)
   8  501      117        3 Rx(0x0)
   9  531      117        1 WaitISR(0x20012)
  10  531      117        1 WaitISR(0x10012)
  11  501      117        1 Rx(0x40)
  12  359      117        1 Start(exchange, (0x2, 0x4))
  13  467      117        1 Tx(0x2)
  14  467      117        1 Tx(0xcc)
  15  467      117        2 Tx(0x0)
  16  531      117        2 WaitISR(0x40012)
  17  501      117        2 Rx(0x0)
  18  501      117        1 Rx(0x56)
  19  531      117        1 WaitISR(0x20012)
  20  531      117        1 WaitISR(0x10012)
  21  501      117        1 Rx(0x9)
  22  359      117        1 Start(exchange, (0x2, 0x4))
  23  467      117        1 Tx(0x5)
  24  467      117        1 Tx(0xb0)
  25  467      117        2 Tx(0x0)
  26  531      117        2 WaitISR(0x40012)
  27  501      117        2 Rx(0x0)
  28  501      117        1 Rx(0x8)
  29  531      117        2 WaitISR(0x10012)
  30  501      117        1 Rx(0x78)
  31  359      117        1 Start(exchange, (0x2, 0x4))
  32  467      117        1 Tx(0x5)
  33  467      117        1 Tx(0x8c)
  34  467      117        2 Tx(0x0)
  35  531      117        2 WaitISR(0x40012)
  36  501      117        2 Rx(0x0)
  37  501      117        1 Rx(0x20)
  38  531      117        2 WaitISR(0x10012)
  39  501      117        1 Rx(0x31)
  40  359      117        1 Start(write, (0x4, 0x0))
  41  467      117        1 Tx(0x83)
  42  467      117        1 Tx(0xc)
  43  467      117        1 Tx(0x20)
  44  467      117        1 Tx(0x1c)
  45  531      117        2 WaitISR(0x40012)
  46  501      117        2 Rx(0x0)
  47  531      117        1 WaitISR(0x22017)
  48  501      117        1 Rx(0x20)
  49  531      117        1 WaitISR(0x10012)
  50  531      117        1 WaitISR(0x301f)
  51  501      117        1 Rx(0x31)
  52  359      117        1 Start(exchange, (0x2, 0x4))
  53  467      117        1 Tx(0x2)
  54  467      117        1 Tx(0xf0)
  55  467      117        2 Tx(0x0)
  56  531      117        2 WaitISR(0x40012)
  57  501      117        3 Rx(0x0)
  58  531      117        2 WaitISR(0x10012)
  59  501      117        1 Rx(0x40)
  60  359      117        1 Start(exchange, (0x2, 0x4))
  61  467      117        1 Tx(0x2)
  62  467      117        1 Tx(0xcc)
  63  467      117        2 Tx(0x0)
   0  531      118        2 WaitISR(0x40012)
   1  501      118        2 Rx(0x0)
   2  531      118        1 WaitISR(0x20012)
   3  501      118        1 Rx(0x0)
   4  531      118        2 WaitISR(0x10012)
   5  501      118        1 Rx(0x0)
humility: ring buffer drv_stm32h7_spi_server_core::__RINGBUF in sprot:

@hawkw hawkw requested review from cbiffle and mkeeter March 20, 2024 21:23
The `stm32xx-spi-server-core` ringbuf fills up pretty quickly when the
system is busy, so it's hard to see the total number of SPI operations
that may have occurred in the past. This branch adds counters to this
ring buffer.

Note that this PR depends on an upstream `idol` change,
oxidecomputer/idolatry#49, to allow us to derive the `Count` trait for
the `SpiOperation` generated type. So, we should merge that first, and
get this pointed back at oxidecomputer/idolatry@main.

For example (on my gimletlet):

```console
$ humility ringbuf spi_server_core
humility: attached via ST-Link V3
humility: ring buffer drv_stm32h7_spi_server_core::__RINGBUF in net:
   TOTAL VARIANT
    3374 WaitISR
    3184 Tx
    3184 Rx
     590 Start(exchange)
     206 Start(write)
 NDX LINE      GEN    COUNT PAYLOAD
   6  467      117        2 Tx(0x0)
   7  531      117        2 WaitISR(0x40012)
   8  501      117        3 Rx(0x0)
   9  531      117        1 WaitISR(0x20012)
  10  531      117        1 WaitISR(0x10012)
  11  501      117        1 Rx(0x40)
  12  359      117        1 Start(exchange, (0x2, 0x4))
  13  467      117        1 Tx(0x2)
  14  467      117        1 Tx(0xcc)
  15  467      117        2 Tx(0x0)
  16  531      117        2 WaitISR(0x40012)
  17  501      117        2 Rx(0x0)
  18  501      117        1 Rx(0x56)
  19  531      117        1 WaitISR(0x20012)
  20  531      117        1 WaitISR(0x10012)
  21  501      117        1 Rx(0x9)
  22  359      117        1 Start(exchange, (0x2, 0x4))
  23  467      117        1 Tx(0x5)
  24  467      117        1 Tx(0xb0)
  25  467      117        2 Tx(0x0)
  26  531      117        2 WaitISR(0x40012)
  27  501      117        2 Rx(0x0)
  28  501      117        1 Rx(0x8)
  29  531      117        2 WaitISR(0x10012)
  30  501      117        1 Rx(0x78)
  31  359      117        1 Start(exchange, (0x2, 0x4))
  32  467      117        1 Tx(0x5)
  33  467      117        1 Tx(0x8c)
  34  467      117        2 Tx(0x0)
  35  531      117        2 WaitISR(0x40012)
  36  501      117        2 Rx(0x0)
  37  501      117        1 Rx(0x20)
  38  531      117        2 WaitISR(0x10012)
  39  501      117        1 Rx(0x31)
  40  359      117        1 Start(write, (0x4, 0x0))
  41  467      117        1 Tx(0x83)
  42  467      117        1 Tx(0xc)
  43  467      117        1 Tx(0x20)
  44  467      117        1 Tx(0x1c)
  45  531      117        2 WaitISR(0x40012)
  46  501      117        2 Rx(0x0)
  47  531      117        1 WaitISR(0x22017)
  48  501      117        1 Rx(0x20)
  49  531      117        1 WaitISR(0x10012)
  50  531      117        1 WaitISR(0x301f)
  51  501      117        1 Rx(0x31)
  52  359      117        1 Start(exchange, (0x2, 0x4))
  53  467      117        1 Tx(0x2)
  54  467      117        1 Tx(0xf0)
  55  467      117        2 Tx(0x0)
  56  531      117        2 WaitISR(0x40012)
  57  501      117        3 Rx(0x0)
  58  531      117        2 WaitISR(0x10012)
  59  501      117        1 Rx(0x40)
  60  359      117        1 Start(exchange, (0x2, 0x4))
  61  467      117        1 Tx(0x2)
  62  467      117        1 Tx(0xcc)
  63  467      117        2 Tx(0x0)
   0  531      118        2 WaitISR(0x40012)
   1  501      118        2 Rx(0x0)
   2  531      118        1 WaitISR(0x20012)
   3  501      118        1 Rx(0x0)
   4  531      118        2 WaitISR(0x10012)
   5  501      118        1 Rx(0x0)
humility: ring buffer drv_stm32h7_spi_server_core::__RINGBUF in sprot:
```
Copy link
Collaborator

@cbiffle cbiffle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be kind of inclined to make this just counters, but that's because the ringbuf has come up in my profiles as a bottleneck for the SPI driver repeatedly. It's possible people are using its contents and converting it to just counters would be disruptive. Not sure.

@hawkw
Copy link
Member Author

hawkw commented Mar 21, 2024

I'd be kind of inclined to make this just counters, but that's because the ringbuf has come up in my profiles as a bottleneck for the SPI driver repeatedly. It's possible people are using its contents and converting it to just counters would be disruptive. Not sure.

Hmm, yeah...adding counters to a ringbuf that already exists does seem like a less controversial change than changing a ringbuf to counters. But, we could do that as well, if no one feels like having the ringbuffer is important...

@hawkw hawkw enabled auto-merge (squash) March 21, 2024 19:27
@hawkw hawkw merged commit 567dd06 into master Mar 21, 2024
103 checks passed
lzrd pushed a commit that referenced this pull request Mar 26, 2024
The `stm32xx-spi-server-core` ringbuf fills up pretty quickly when the
system is busy, so it's hard to see the total number of SPI operations
that may have occurred in the past. This branch adds counters to this
ring buffer.

Note that this PR depends on an upstream `idol` change,
oxidecomputer/idolatry#49, to allow us to derive the `Count` trait for
the `SpiOperation` generated type. So, we should merge that first, and
get this pointed back at oxidecomputer/idolatry@main.

For example (on my gimletlet):

```console
$ humility ringbuf spi_server_core
humility: attached via ST-Link V3
humility: ring buffer drv_stm32h7_spi_server_core::__RINGBUF in net:
   TOTAL VARIANT
    3374 WaitISR
    3184 Tx
    3184 Rx
     590 Start(exchange)
     206 Start(write)
 NDX LINE      GEN    COUNT PAYLOAD
   6  467      117        2 Tx(0x0)
   7  531      117        2 WaitISR(0x40012)
   8  501      117        3 Rx(0x0)
   9  531      117        1 WaitISR(0x20012)
  10  531      117        1 WaitISR(0x10012)
  11  501      117        1 Rx(0x40)
  12  359      117        1 Start(exchange, (0x2, 0x4))
  13  467      117        1 Tx(0x2)
  14  467      117        1 Tx(0xcc)
  15  467      117        2 Tx(0x0)
  16  531      117        2 WaitISR(0x40012)
  17  501      117        2 Rx(0x0)
  18  501      117        1 Rx(0x56)
  19  531      117        1 WaitISR(0x20012)
  20  531      117        1 WaitISR(0x10012)
  21  501      117        1 Rx(0x9)
  22  359      117        1 Start(exchange, (0x2, 0x4))
  23  467      117        1 Tx(0x5)
  24  467      117        1 Tx(0xb0)
  25  467      117        2 Tx(0x0)
  26  531      117        2 WaitISR(0x40012)
  27  501      117        2 Rx(0x0)
  28  501      117        1 Rx(0x8)
  29  531      117        2 WaitISR(0x10012)
  30  501      117        1 Rx(0x78)
  31  359      117        1 Start(exchange, (0x2, 0x4))
  32  467      117        1 Tx(0x5)
  33  467      117        1 Tx(0x8c)
  34  467      117        2 Tx(0x0)
  35  531      117        2 WaitISR(0x40012)
  36  501      117        2 Rx(0x0)
  37  501      117        1 Rx(0x20)
  38  531      117        2 WaitISR(0x10012)
  39  501      117        1 Rx(0x31)
  40  359      117        1 Start(write, (0x4, 0x0))
  41  467      117        1 Tx(0x83)
  42  467      117        1 Tx(0xc)
  43  467      117        1 Tx(0x20)
  44  467      117        1 Tx(0x1c)
  45  531      117        2 WaitISR(0x40012)
  46  501      117        2 Rx(0x0)
  47  531      117        1 WaitISR(0x22017)
  48  501      117        1 Rx(0x20)
  49  531      117        1 WaitISR(0x10012)
  50  531      117        1 WaitISR(0x301f)
  51  501      117        1 Rx(0x31)
  52  359      117        1 Start(exchange, (0x2, 0x4))
  53  467      117        1 Tx(0x2)
  54  467      117        1 Tx(0xf0)
  55  467      117        2 Tx(0x0)
  56  531      117        2 WaitISR(0x40012)
  57  501      117        3 Rx(0x0)
  58  531      117        2 WaitISR(0x10012)
  59  501      117        1 Rx(0x40)
  60  359      117        1 Start(exchange, (0x2, 0x4))
  61  467      117        1 Tx(0x2)
  62  467      117        1 Tx(0xcc)
  63  467      117        2 Tx(0x0)
   0  531      118        2 WaitISR(0x40012)
   1  501      118        2 Rx(0x0)
   2  531      118        1 WaitISR(0x20012)
   3  501      118        1 Rx(0x0)
   4  531      118        2 WaitISR(0x10012)
   5  501      118        1 Rx(0x0)
humility: ring buffer drv_stm32h7_spi_server_core::__RINGBUF in sprot:
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants