search

tinygo-org / tinygo

Go compiler for small places. Microcontrollers, WebAssembly (WASM/WASI), and command-line tools. Based on LLVM.
https://tinygo.org
Other
15.49k stars 913 forks source link

gen-device-svd.go generates invalid assembler files for svd files with multi-line descriptions #4608

Open cibomahto opened 5 days ago

cibomahto commented 5 days ago

The rp2350 SVD file has a long, multi-line description tag inside the root device element:

<?xml version="1.0" encoding="utf-8"?>
<!--
Copyright (c) 2024 Raspberry Pi Ltd.

SPDX-License-Identifier: BSD-3-Clause
-->
<device schemaVersion="1.1" xmlns:xs="http://www.w3.org/2001/XMLSchema-instance" xs:noNamespaceSchemaLocation="CMSIS-SVD.xsd">
    <vendor>Raspberry Pi</vendor>
    <name>RP2350</name>
    <series>RP</series>
    <version>0.1</version>
    <description>
        Dual Cortex-M33 or Hazard3 processors at 150MHz
        520kB on-chip SRAM, in 10 independent banks
        Extended low-power sleep states with optional SRAM retention: as low as 10uA DVDD
        8kB of one-time-programmable storage (OTP)
        Up to 16MB of external QSPI flash/PSRAM via dedicated QSPI bus
            Additional 16MB flash/PSRAM accessible via optional second chip-select
        On-chip switched-mode power supply to generate core voltage
            Low-quiescent-current LDO mode can be enabled for sleep states
        2x on-chip PLLs for internal or external clock generation
        GPIOs are 5V-tolerant (powered), and 3.3V-failsafe (unpowered)
        Security features:
            Optional boot signing, enforced by on-chip mask ROM, with key fingerprint in OTP
            Protected OTP storage for optional boot decryption key
            Global bus filtering based on Arm or RISC-V security/privilege levels
            Peripherals, GPIOs and DMA channels individually assignable to security domains
            Hardware mitigations for fault injection attacks
            Hardware SHA-256 accelerator
        Peripherals:
            2x UARTs
            2x SPI controllers
            2x I2C controllers
            24x PWM channels
            USB 1.1 controller and PHY, with host and device support
            12x PIO state machines
            1x HSTX peripheral
    </description>
    <width>32</width>
    <size>32</size>

Running make gen-device on this svd file produces a .s file with only the first line commented correctly, which cause a compilation error when including the file:

// Automatically generated file. DO NOT EDIT.
// Generated by gen-device-svd.go from rp2350.svd, see https://github.com/posborne/cmsis-svd/tree/master/data/RaspberryPi

// Dual Cortex-M33 or Hazard3 processors at 150MHz
        520kB on-chip SRAM, in 10 independent banks
        Extended low-power sleep states with optional SRAM retention: as low as 10uA DVDD
        8kB of one-time-programmable storage (OTP)
        Up to 16MB of external QSPI flash/PSRAM via dedicated QSPI bus
            Additional 16MB flash/PSRAM accessible via optional second chip-select
        On-chip switched-mode power supply to generate core voltage
            Low-quiescent-current LDO mode can be enabled for sleep states
        2x on-chip PLLs for internal or external clock generation
        GPIOs are 5V-tolerant (powered), and 3.3V-failsafe (unpowered)
        Security features:
            Optional boot signing, enforced by on-chip mask ROM, with key fingerprint in OTP
            Protected OTP storage for optional boot decryption key
            Global bus filtering based on Arm or RISC-V security/privilege levels
            Peripherals, GPIOs and DMA channels individually assignable to security domains
            Hardware mitigations for fault injection attacks
            Hardware SHA-256 accelerator
        Peripherals:
            2x UARTs
            2x SPI controllers
            2x I2C controllers
            24x PWM channels
            USB 1.1 controller and PHY, with host and device support
            12x PIO state machines
            1x HSTX peripheral
//
//     Copyright (c) 2024 Raspberry Pi Ltd. SPDX-License-Identifier: BSD-3-Clause

.syntax unified

It looks like the description field is read into a metadata struct with minimal formatting, and then written out to the file.

The description field is also written to the corresponding .go file, but is enclosed in a / / comment block.

I have a potential fix here: https://github.com/Blinkinlabs/tinygo/commit/52bd80dcbc237b7972d918ac4edc961b7d095b51 , which borrows the formatting path used for the licenseBlock, and adds // to the beginning of each line. If this seems reasonable I can submit a pull request. I haven't checked if it interferes with the processing of other svd files.

It might also be nice to add an automated test to see if the generated go and assembler files are syntactically valid.

Here is the file generated using my patch:

// Automatically generated file. DO NOT EDIT.
// Generated by gen-device-svd.go from rp2350.svd, see https://github.com/posborne/cmsis-svd/tree/master/data/RaspberryPi

//
//             Dual Cortex-M33 or Hazard3 processors at 150MHz
//             520kB on-chip SRAM, in 10 independent banks
//             Extended low-power sleep states with optional SRAM retention: as low as 10uA DVDD
//             8kB of one-time-programmable storage (OTP)
//             Up to 16MB of external QSPI flash/PSRAM via dedicated QSPI bus
//                 Additional 16MB flash/PSRAM accessible via optional second chip-select
//             On-chip switched-mode power supply to generate core voltage
//                 Low-quiescent-current LDO mode can be enabled for sleep states
//             2x on-chip PLLs for internal or external clock generation
//             GPIOs are 5V-tolerant (powered), and 3.3V-failsafe (unpowered)
//             Security features:
//                 Optional boot signing, enforced by on-chip mask ROM, with key fingerprint in OTP
//                 Protected OTP storage for optional boot decryption key
//                 Global bus filtering based on Arm or RISC-V security/privilege levels
//                 Peripherals, GPIOs and DMA channels individually assignable to security domains
//                 Hardware mitigations for fault injection attacks
//                 Hardware SHA-256 accelerator
//             Peripherals:
//                 2x UARTs
//                 2x SPI controllers
//                 2x I2C controllers
//                 24x PWM channels
//                 USB 1.1 controller and PHY, with host and device support
//                 12x PIO state machines
//                 1x HSTX peripheral
//
//
//     Copyright (c) 2024 Raspberry Pi Ltd. SPDX-License-Identifier: BSD-3-Clause

.syntax unified
aykevl commented 4 days ago

Yes, this would be a welcome addition! Your proposed fix sounds like a good idea.