Repository navigation
hw/arm: add the RK3566 MicroDuck beta machine and device models - #47
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83fdb414ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The RK3566 UARTs have FIFOs up to 256 bytes and derive the character time from a functional clock. Add a "fifo-size" property so a device can select the FIFO depth, compute the FCR trigger levels from it, and migrate the configured depth. Add a "uartclk" clock input. When the clock has a source, measure the transmit progress and the receive timeout in clock ticks, batch backend writes at FIFO and interrupt boundaries, and stop receiving while the clock is gated. Devices that do not connect the clock keep the previous byte-at-a-time behaviour. Signed-off-by: Chao Liu <chao.liu@processmission.com>
The RK3566 UART is a DesignWare 8250 with a 64-byte FIFO. Expose the "fifo-size" property and the "uartclk" clock of the embedded serial device, report the FIFO depth through CPR and the current levels through TFL and RFL. Signed-off-by: Chao Liu <chao.liu@processmission.com>
A peripheral request covers one transfer burst. Queued AXI beats have not yet changed the peripheral FIFO level, so sampling the request while they are still queued can grant a second burst against the same space. Complete the queued beats before sampling the level again. Signed-off-by: Chao Liu <chao.liu@processmission.com>
The ISP and VENC register windows are accessed with byte, 32-bit and 64-bit MMIO. Widen the valid access sizes and allow unaligned accesses so guest register writes reach the model. Signed-off-by: Chao Liu <chao.liu@processmission.com>
RK3566 Linux uses the SCMI clock protocol for CPU, GPU, NPU and DDR DVFS. Add a "soc" property; when it is set to "rk3566", expose the RK3566 clock table with initial rates, implement CLOCK_DESCRIBE_RATES and report the enable state in CLOCK_ATTRIBUTES. RK3588 keeps the previous responses. Signed-off-by: Chao Liu <chao.liu@processmission.com>
The RK3566 RKNPU has a single core, a 32-bit DMA address space and one command per amount field, while RK3588 has a 40-bit address space and two commands per amount. Add "dma-address-bits" and "command-count-scale" properties that default to the RK3588 values. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Replace the synchronous byte-at-a-time model with a transfer engine that schedules wire clocks from the SPI clock and BAUDR, keeps 16-bit FIFOs, reports TX/RX levels and over/under-run status, and drives the PL330 DMA handshake and chip-select outputs. The wire clock comes from a "spiclk" input, or from the "clock-hz" property for boards that do not model the CRU clock tree. The post-load hook recomputes the interrupt state from the migrated registers. Skip raising the outputs under qtest, whose GIC input intercept aborts on the SPI IRQ lines. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Report the GHWPARAMS0/1/3/7 and TX/RX FIFO size registers, implement the endpoint set-stall and clear-stall commands, reject invalid DEPXFERCFG values, flush the device FIFO on a device command, and only start transfers while the host is connected. Expose the host connection as the "host-connected" property and update DSTS link state from DCTL.RunStop. Update the dwc3 UDC qtest to expect the hardware DSTS reset value. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Add the RK3566 Kconfig symbol and the hw/arm/rk3566 build directory. The machine is enabled by default for TCG aarch64 builds and selects the shared peripherals it uses. The device models and the machine are added by the following patches. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Model the RK3566 clock and reset unit with RK3328-style PLL registers, the high-word update registers, clock gates and resets used by the UART, I2C, SPI, I2S, PWM and media blocks. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Model the RK3566 general-purpose timer with load and count registers, prescaler, interrupt status and the clock event output used by the vendor kernel. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Model the RK3566 OTP controller so the vendor kernel can read the chip identification and analog tuning values from the fuse array. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Add the MicroDuck register I2C devices: the RK809 PMIC, the SYR827 buck regulator and the PCA9535 GPIO expander, each with its own register file and the RK809 interrupt handling. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Model the RK3566 temperature sensor ADC with the vendor calibration table, conversion completion and comparator interrupts, and a "temperature-mc" property for the temperature supplied by the physical model. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Model the four RK3566 PWM channels with period, duty, enable and counter registers. The counter value is derived from the virtual clock so the guest fan and backlight drivers see running channels. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Model the RK3566 system controller block with the PVTM oscillator measurement and the RNG register interface used by the vendor kernel. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Add the LSM6DSV16X head IMU as an I2C device with the accelerometer and gyroscope register map, the FIFO, and a chardev input that feeds measured samples from the physical model. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Model the RK3566 SARADC with channel selection, the reference and input voltages, and the conversion-complete interrupt used by the battery and joystick drivers. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Model the RK3566 I2S/TDM and PDM controllers with clock dividers, FIFOs, interrupt status and PL330 DMA handshakes. The machine connects the I2S1 and PDM paths to the RK809 codec and the face microphones. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Add the RK3566 single-core RKNPU as a variant of the shared Rockchip RKNPU core with a 32-bit DMA address space and one command per amount field. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Model the ST25R100 NFC controller with the SPI register interface, the oscillator and the ISO14443 no-response timer used by the MicroDuck NFC stack. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Model the VL53L8CX time-of-flight sensor with the SPI and DCI interface, the firmware upload validation, the DCI configuration and the depth frames supplied by the physical model through the "tof-data" chardev. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Model the RGA2 image converter with command DMA, IOMMU page translation, scaling and format conversion for the camera pipeline. The image conversion uses libswscale and libavutil, so the device is only built when those optional libraries are available. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Model the GC2093 image sensor, the CSI DPHY and the RKISP 2.1 capture path. Frames arrive on the "camera" chardev, are converted with libswscale and written to the IOMMU-mapped capture buffers. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Model the RK3566 VEPU540 H.264 encoder with per-frame QP and buffer status. Frames are encoded with x264 and the model reports the decoded frame statistics through libavcodec. Signed-off-by: Chao Liu <chao.liu@processmission.com>
83fdb41 to
e0c29af
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0c29af377
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| scaler = sws_getContext(s->width, s->height, AV_PIX_FMT_UYVY422, | ||
| width, height, format, SWS_BILINEAR, | ||
| NULL, NULL, NULL); |
There was a problem hiding this comment.
When the guest enables mainpath cropping, isp_capture_path() uses the crop dimensions to calculate the output size, but this scaler still consumes the entire backend frame, and the input pointer never incorporates the crop offsets. For example, a 320×180 crop of a 640×360 input produces a downscaled view of the full image instead of the selected region. This silently supplies incorrect image data to guest camera consumers. Apply the configured crop rectangle before scaling, or report an unsupported-operation error rather than completing the capture successfully.
Useful? React with 👍 / 👎.
Add the MicroDuck beta machine with four Cortex-A55 CPUs, a GICv3 interrupt controller, 1 GiB of RAM, the RK3566 peripheral set and the MicroDuck device tree fixups. The machine loads the official Linux image and device tree supplied with -kernel and -dtb. The media devices are only built when the optional libraries are available; report the missing support at startup instead of failing on an unknown device type. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Replace the hand-written MMIO switch with a RegisterInfo bank. DATA, STATUS, CONTROL and DELAY are described by RegisterAccessInfo entries and served through register_read_memory() and register_write_memory(). STATUS is read-only and derived from timer_pending() in post-read. The CONTROL write clears the interrupt and starts the conversion timer in the post-write hook; the hook is reset-safe because the reset value does not arm the timer, and the reset hook stops the conversion, clears the registers and deasserts the IRQ. DELAY keeps its 6-bit write mask and the conversion callback latches DATA and the completion bit exactly as before. The backing register array replaces the three separate migration fields while the conversion timer keeps its VMSTATE_TIMER_PTR entry. Addresses that are not part of the bank keep the previous behaviour: a read returns zero and a write has no effect. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Describe the control/status register bank with RegisterAccessInfo entries and serve the modelled registers through register_read_memory() and register_write_memory(). The temperature code is derived in post-read and follows the same enable/disable rule as before. INT_STATUS keeps its write-one-to-clear semantics, and the enable, threshold and interrupt enable writes recompute the comparator state and IRQ through a shared post-write hook. Registers that the model stores but does not decode, such as the register at 0x00, the word at 0x10 and the remaining words of the 0x100 window, keep their plain storage semantics through a wrapper MemoryRegionOps fallback, so guest writes and reads still round-trip instead of turning into guest errors. The device still has no migration state, matching the previous model. Signed-off-by: Chao Liu <chao.liu@processmission.com>
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Convert the RK3566 PWM bank to the RegisterInfo framework. The whole 0x100 window is now covered by a static register table: each of the four channels exposes CTRL, PERIOD, DUTY and ENABLE, and the unused words keep plain storage so the sparse window keeps its previous read/write semantics. The running counter is still derived in post_read from the virtual clock and the per-channel epoch, and the epoch is still captured on the rising edge of a channel enable in pre_write. Reset behaviour is unchanged: the device has no reset handler, so the registers keep their values. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Convert the RK3566 OTP bank to the RegisterInfo framework. The 16 KiB window is a hybrid bank: a small RegisterInfo table models the command, fuse data and status registers, while a wrapper MemoryRegionOps falls back to the existing register array for every other address. The fallback keeps the original write semantics: addresses below 0x120 still use the high-word write mask, addresses at or above 0x120 are plain storage, and reads return the backing array. The 0x20 command and the 0x108 fuse-data triggers stay in post_write, their self-clear behaviour is preserved, and 0x304 is modelled as a write-one-to-clear register in the table. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Convert the RK3566 CSI DPHY bank in the camera model to the RegisterInfo framework. Only the lane-enable word at offset 0 has behaviour: it is a single RegisterInfo entry whose post_write drives the link-enabled GPIO. The rest of the 4 KiB window keeps plain storage semantics through a wrapper MemoryRegionOps fallback. The lane-enable reset value (1) is kept in the table, and the reset handler reloads it through register_reset() so the GPIO is driven low on reset as before. The ISP and GC2093 models in this file are untouched, including the existing mainpath crop fix. Signed-off-by: Chao Liu <chao.liu@processmission.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78832ebaf3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| s->watch_tag = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP, | ||
| serial_watch_cb, s); |
There was a problem hiding this comment.
When a clock-connected UART has no chardev attached (for example, an unused MicroDuck serial port), qemu_chr_fe_write() returns 0 and qemu_chr_fe_add_watch() also returns 0. This path leaves tx_busy set without a watch. Once a single pending character has accumulated frame_ticks, the scheduling block repeatedly rearms the timer for now + 1, while TEMT never becomes set. Guest writes to an unconnected port can therefore cause a persistent timer storm and hang software polling for transmitter completion. Handle watch-registration failure by completing or dropping the pending transmission, as the existing non-clocked transmit path does.
Useful? React with 👍 / 👎.
Describe the control/status register bank with RegisterAccessInfo entries and serve it through register_read_memory() and register_write_memory(). SYS_CTRL keeps the self-clearing soft reset in a pre-write hook and derives its busy bit in post-read, CMD_CTRL starts the command DMA timer from post-write, INTERRUPT keeps the write-one-to-clear behaviour of the status bits, and VERSION is modelled as a read-only reset value. The 32 command block words at 0x100 stay plain register entries, so the DMA shadow still round-trips guest accesses. The migration state keeps the backing register array, the busy flag, the completion counters and the command timer with the same layout as before. Addresses outside the described registers are reported as unimplemented instead of silently stored, which is the accepted limit of the pure RegisterInfo bank. Add a qtest for the register bank: it checks the reset values, the plain command block storage, the soft reset, the derived busy status, the write-one-to-clear interrupt bits and the completion of a copy command through the command timer. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Describe each of the three system controller register layouts (PMU, RNG and PVTM) with its own RegisterAccessInfo table and select the table in realize from the device properties. A wrapper MemoryRegionOps forwards the modelled registers to register_read_memory() and register_write_memory(), and keeps the original masked-storage behaviour for every other address of the sparse 0x2000 window. The PMU mirrors, the RNG control register that fills the data window through qemu_guest_getrandom() and the PVTM start/period pair stay in post-write hooks. The PVTM status and result registers are read-only and their value is derived in post-read from the timer deadline, as before. The device still has no migration state. Add a qtest that instantiates the machine with the three variants: it checks the QOM children, the PMU mirror writes, the masked-storage fallback, the RNG write path and the PVTM measurement after the virtual clock advances. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Move the I2S/TDM and PDM control, status and FIFO-level registers to RegisterAccessInfo tables selected per device variant and initialized with register_init_block32(). The register block keeps a device-specific MemoryRegionOps wrapper. TXDR, RXDR and RXFIFO_DATA remain explicit FIFO data windows, and accesses outside the 0x100-byte register file keep the original read-zero and log-unimplemented behaviour. The INTCR, CLR, SYSCONFIG and INT_CLR write triggers run from pre_write hooks, the FIFO level reads run from post_read hooks, and the configure and update side effects run from a shared post_write hook that is suppressed while the device is reset. Reset defaults now come from the register table. Add the rk3566-audio-test qtest covering reset values, FIFO and mask behaviour, the I2S interrupt line and the half-frame timer. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Move the VEPU540 control, status and interrupt registers to a RegisterAccessInfo table initialized with register_init_block32(). The register block keeps a device-specific MemoryRegionOps wrapper. The encoder-owned status range still ignores guest writes, and the L2_ADDRESS, L2_WRITE and L2_READ index window reaches l2[320] through pre_write and post_read hooks. START keeps its timer arming side effect in post_write, CLEAR and INT_CLEAR keep their write triggers in pre_write, and INT_STATUS keeps the write-zero-clears-all behaviour. The device stays unmigratable and the x264 encode still runs from the timer callback. Add the rk3566-venc-test qtest covering reset values, interrupt semantics, the L2 window and the encode timer callback. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Add qtests for the timer, SARADC, TSADC, PWM, OTP, CSI D-PHY and the UART clock gating path. The tests cover reset values, register write masks, write-one-to-clear status bits, interrupt lines, timer and PWM counting, the storage fallback of unlisted addresses and the second PWM instance. The UART test gates the UART2 functional clock through the CRU and checks that the gated transmitter still drains queued bytes and reports THRE|TEMT, which is what firmware polling the line status expects. Add the test path pattern to the RK3566 MAINTAINERS entry. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Move the RK3566 ISP control and status registers to a RegisterInfo table. The 0x0-0x1600 register bank uses register_init_block32() with hooks for the shadow latch, interrupt clear/set and capture enable semantics; the sparse remainder of the 64 KiB window keeps plain storage through a wrapper fallback. The capture DMA timer, shadow latch helper and IRQ updates are unchanged, and the device stays unmigratable. Add a qtest for the reset values, latch triggers, interrupt registers, sparse fallback and software reset; the test skips when the media models are not built. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Convert the Rockchip SPI controller to the RegisterInfo framework. The control and status registers in the 0x00-0x48 window are described by a static RegisterAccessInfo table registered with register_init_block32, while the TXDR/RXDR FIFO apertures at 0x400 and 0x800 stay on an explicit wrapper MemoryRegionOps. The wrapper forwards the non-data addresses to register_read_memory()/register_write_memory() and keeps the FIFO and transfer engine handling for the data addresses. The register write masks, the SSIENR enable/disable side effects, the ICR write-one-to-clear behaviour and the FIFO level, SR and interrupt read derivations are preserved in pre_write/post_write/post_read hooks. Every post_write still calls rockchip_spi_update(), so the transfer engine is rescheduled exactly as before. Addresses outside the table keep returning zero on reads and keep logging LOG_UNIMP on writes. The state fields share storage with the register array through an anonymous union, so the VMState field names, arrays and version_id 4 stay unchanged. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Add rockchip-spi-test, which boots microduck-beta and checks the reset values, the register write masks, the FIFO level and interrupt derivations and the unimplemented access logging of the SPI1 and SPI3 windows. Extend rk3566-machine-test with an access width test for the ISP IOMMU window: 1, 2, 4 and 8 byte reads and writes plus unaligned accesses, matching the .valid 1..8/unaligned and 4 byte .impl description of the device. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Convert the RK3566 CRU and PMUCRU window to a hybrid RegisterInfo bank. A static RegisterAccessInfo table describes the PLL parameter words and the CLKSEL, CLKGATE and SOFTRST control ranges; a wrapper MemoryRegionOps keeps the previous masked-storage behaviour for every address outside the table, including the PLL CON3 words. The three write classes stay in the pre-write hook: PLL CON2 keeps the 24-bit high-word mask, fractional selector words are stored in full and the other words keep the high-half write mask taken from the written value. PLL CON1 derives the LOCK bit in a post-read hook, and the clock tree is recomputed from the post-write hook. The hook stays quiet while the reset handler reloads the reset values, so the tree is recomputed once per reset. The register array remains in VMState and post-load still recomputes the clock outputs. The PMUCRU variant selects its own table through the existing pmu property. That table covers the PLL block, the mode word and the two fractional selector words; the rest of the PMU window keeps the wrapper fallback. Add a qtest register test covering the reset values, masked and fractional writes, the derived PLL LOCK bit, clock gating and the PMUCRU variant. Signed-off-by: Chao Liu <chao.liu@processmission.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4d02c4fda
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| SysBusDevice parent_obj; | ||
| uint32_t regs[OTP_REG_SIZE / 4]; | ||
| uint8_t fuses[1024]; | ||
| RegisterInfo regs_info[OTP_REG_MAX]; |
There was a problem hiding this comment.
When the OTP device is instantiated, register_init_block32() indexes its RegisterInfo backing array by register address divided by four, not by descriptor position (see register_init_block() in hw/core/register.c). OTP_REG_MAX is only 3, but the descriptors at 0x20, 0x108 and 0x304 cause writes to indices 8, 66 and 193. All three writes therefore land beyond this array and the device allocation, corrupting host heap memory during MicroDuck machine initialization. Allocate at least 0x304 / 4 + 1 entries, or size the array for the full register window.
Useful? React with 👍 / 👎.
List the shared rockchip_spi device and the RK3566 test that covers its register bank under the Rockchip RK3566 entry. Signed-off-by: Chao Liu <chao.liu@processmission.com>
Restore the plain storage fallback for addresses outside the described RGA2 register bank. The register table only covers the control/status registers and the 32 command words, while the model used to store every word of the 0x1000 window. A wrapper MemoryRegionOps now forwards the described registers to register_read_memory() and register_write_memory() and keeps the pre-conversion read/write storage for every other address, masked to the access width. The reset, migration state, command DMA and timer behaviour are unchanged: the fallback shares the same register array that the RegisterInfo table and the command shadow use. Extend the qtest with unlisted offsets 0x14, 0x2c, 0xfc, 0x180, 0x400 and 0xffc, checking that 0xdeadbeef round-trips, and keep the existing subtests passing. Signed-off-by: Chao Liu <chao.liu@processmission.com>
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
register_init_block32() stores each table entry at regs_info[addr / 4]. The OTP table has entries at 0x20, 0x108 and 0x304, but the array was sized by the number of table entries, so realize wrote past its end for the higher offsets. Size the array by the last register word instead. Signed-off-by: Chao Liu <chao.liu@processmission.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30819a7a60
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case DWC3_DEPCMD_DEPCLEARSTALL: | ||
| s->ep_stalled[ep] = false; | ||
| break; |
There was a problem hiding this comment.
If the guest starts an IN transfer while the endpoint is stalled, DEPSTRTXFER records the pending TRB, but dwc3_udc_process_ep() returns without processing it. DEPCLEARSTALL then clears the flag without retrying that transfer. Since the only call to dwc3_udc_process_ep() is on DEPSTRTXFER, the TRB remains pending indefinitely and guest software waiting for its completion can hang even after clearing the halt. Retry pending work when clearing the stall.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The RK3566 models were all built from hw/arm/rk3566 even though they model devices from several QEMU hardware classes. Move each model next to the devices it belongs with, add the matching Kconfig symbols, and let the RK3566 SoC select them. Drop the hw/arm/rk3566 directory: the SoC model lives in hw/arm/rk3566.c and the board in hw/arm/microduck.c, mirroring rk3588.c and rock-5b.c. The camera model is split into the GC2093 sensor, the ISP and the CSI DPHY, so each part lands in its own class. The media models stay conditional on FFmpeg and x264. Qtests look the SoC devices up below the soc object. Add the Process Mission copyright header to the model sources created by this series. Signed-off-by: Chao Liu <chao.liu@processmission.com>
8ed19e8 to
ad94a26
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad94a266fb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sysbus_connect_irq(SYS_BUS_DEVICE(uart), 0, | ||
| qdev_get_gpio_in(DEVICE(&s->gic), uart_irqs[i])); |
There was a problem hiding this comment.
When a guest uses UART DMA on MicroDuck, the second sysbus_connect_irq() replaces the uart_irq connection installed immediately above it; GPIO output connections do not fan out. Consequently, uart_irq() never updates the PL330 peripheral-request inputs from LSR.THRE/DR. After PL330 reset these inputs remain ready regardless of FIFO state, allowing DMA to read an empty receive FIFO or overrun the transmit FIFO and corrupt serial data. Keep the callback as the sole connection, since it already forwards the interrupt to the GIC.
Useful? React with 👍 / 👎.
Add the RK3566 MicroDuck beta machine, its device models, and the QEMU device changes needed to boot the official MicroDuck beta Linux image. The series also addresses the automated review findings, the early-boot console stall, and moves the RK3566 MMIO control/status banks to the RegisterInfo framework.
What is added
microduck-betamachine: four Cortex-A55 CPUs, GICv3, 1 GiB RAM, eMMC, UART1..3, I2C/SPI buses, virtio-mmio management network, and the MicroDuck device tree fixups. Boots the official LinuxImageand DTB passed with-kerneland-dtb.Device models are organized by QEMU hardware class: the I2C slave models live in hw/i2c, the I2S/PDM controller in hw/audio, the RGA2 accelerator in hw/display, the CRU, system controller, PWM, ISP, VENC, RKNPU and NFC models in hw/misc, the OTP controller in hw/nvram, the SARADC in hw/adc, the timer in hw/timer, and the TSADC, GC2093, LSM6DSV16X and VL53L8CX sensors in hw/sensor. The RK3566 SoC model lives in hw/arm/rk3566.c and the MicroDuck board in hw/arm/microduck.c, mirroring rk3588.c and rock-5b.c. Each model source carries the Process Mission copyright header.
Shared device support
hw/char/serial: configurable FIFO size plus a UART clock that paces transmission and receive timeouts in clock ticks.hw/char/dw8250: FIFO size and level registers and the UART clock alias.hw/ssi/rockchip_spi: FIFO, interrupt, chip-select and PL330 DMA handshake model driven by the SPI clock; register bank converted to RegisterInfo.hw/usb/dwc3-udc: hardware parameter registers, endpoint stalls, device command FIFO flush and host connection state.hw/misc/rk3588_scmi: RK3566 clock table and CLOCK_DESCRIBE_RATES.hw/misc/rk3588_rknpu: 32-bit DMA address space and command-count scale.hw/misc/rockchip_iommu: byte, unaligned and 64-bit register accesses.hw/dma/pl330: complete a queued burst before sampling a peripheral request.Review findings
hw/arm/rk3566/vl53l8cx.c:depth_frame()re-read the live DCI zone count after ranging started, so a guest could make the 16/64-zone payload path index the 192-byte input buffer out of bounds. The validated zone count is now latched when ranging starts and re-checked before use.hw/arm/rk3566/rk3566-media-camera.c: the mainpath crop size was used for the output size but the scaler still consumed the full frame from its start. The crop offset and size are now applied to the scaler source window, and an invalid window fails the capture with a guest error.Boot fix
The MicroDuck image stopped printing right after
rcu: srcu_initbecause CPU0 was spinning in the early console polling LSR. The guest gatesclk_uart2duringof_clk_init; the uartclk-driven transmitter then stopped advancing pending bytes and never set LSR.TEMT. The serial model now flushes the queued bytes and reports the transmitter idle while the clock is gated. The image reachesmicroduck-beta login:.RegisterInfo migration
All RK3566 MMIO control/status banks now use the QEMU RegisterInfo API, with explicit MemoryRegionOps only for FIFO/command/index apertures and wrapper fallbacks for sparse windows. The framework decision record and the per-bank records are in the task audit directory; each record passes the qemu-peripheral-modeling framework-decision checker.
Converted banks: rk3566-timer, rk3566-saradc, rk3566-tsadc, rk3566-pwm, rk3566-otp, rk3566-csi-dphy, rk3566-rga2, rk3566-system (pmu/rng/pvtm variants), rk3566-i2s-tdm, rk3566-pdm, rk3566-rkvenc, rk3566-isp, rk3566-cru/pmucru, rockchip-spi.
CumulativeRegisterInfodoes not exist in QEMU upstream, any processmission/qemu ref, any oh-my-qemu ref, or GitHub code search. The conversion uses the existing RegisterInfo/RegisterInfoArray API; the missing layer remains an open question with the user.Independent review
An independent pass over the series found one P1 regression in the RGA2 conversion: the RegisterInfo table only covered the implemented words, so the remaining window lost its previous plain-storage behavior. The device uses a wrapper MemoryRegionOps again, dispatching implemented registers to RegisterInfo and unlisted addresses to the register backing store, with a qtest that writes and reads back the unlisted addresses.
The same pass requested MAINTAINERS coverage for the shared rockchip_spi model and its test, which is added.
The Codex P1/P2 fixes are correct by inspection. The P2 crop path has a functional probe that verifies byte-exact cropped output and rejection of invalid windows; a committed qtest for the crop path and the VL53L8CX ranging path remains deferred because both need the camera chardev or the ST firmware upload protocol. Local suite runs hit two load-related flakes (rk3588-rknpu and rock-5b, the latter killed by signal 9 under memory pressure); each test passes when rerun on an idle machine.
Testing
./scripts/checkpatch.pl --no-strict origin/devel..HEAD: 0 errors.aarch64-softmmuandarm-softmmu, with and without the optional FFmpeg/x264 media models.rk3566-*device tests and the ROCK 5B / RK3588 regressions.microduck-beta login:with the final tree.CI
The ubuntu-24.04 hosted runner links the action node runtime from
/opt/runner-cache. The workflow cleanup step kept that directory instead of deleting all of/opt, so the previous checkout failures no longer occur.Series
52 commits: RK3566 board build plumbing and device models, shared device support, documentation and MAINTAINERS, the two review fixes, the gated-transmitter boot fix, 14 RegisterInfo conversion commits with their qtests, the RGA2 storage fallback fix, the OTP register array bounds fix, the CI runner fix.