CSR json to Zephyr DTS

[lachlansmith]

I'm attempting to use HCI SPI with a Particle Argon and an Arty to implement a full-fledged Bluetooth stack, however, I need to confirm the spi & gpio_out nodes are correct before I debug the connection with a logic analyser. I have two main questions:

- How are register sizes determined?
- How are gpio_out pins referenced/indexed in the phandle?

I'm also curious why litex,spi has interrupts when SPIMaster doesn't generate any in the interrupt map.

&uart0 {
    reg = <0xf0001800 0x20>;               <-- 0x20 calculated?
    interrupts = <0x2 0>;
};

&timer0 {
    reg = <0xf0002800 0x40>;               <-- 0x40 calculated?
    interrupts = <0x1 0>;
};

&spi0 {
    reg = <0xf0002000 0x??>;               <-- Need to calculate
    interrupts = <0x5 0>;   ????????
    cs-gpios = <&gpio_out 3 (0 << 0 | 1 << 4)>;

    bt-hci@0 {
    compatible = "zephyr,bt-hci-spi";
    reg = <0>;
    reset-gpios = <&gpio_out 0 (1 << 0 | 1 << 4)>;
    irq-gpios = <&gpio_out 1 (0 << 0 | 1 << 5)>;
    spi-max-frequency = <2000000>;
    label = "BT_HCI";
    };
};

&gpio_out {
    reg = <0xf0005800 0x??>;               <-- Need to calculate
};

I'm wondering how sizes are calculated because this statement in the Wiki still leaves me questioning, _"The size of the CSR segment, and also the number of bits dedicated to accessing LiteX CSRs is dictated by csr_addresswidth" (csr_address_width = 14). I understand these size values are coming from the overlay handlers in litex/tools/litex_json2dts_zephyr.py. Yet I can't see how the sizes in csr.json with csr-address-width evaluate to the hardcoded values in the handlers.

def SoC(soc_cls, **kwargs):
    class _SoC(soc_cls):
        csr_map = {**soc_cls.csr_map, **{
            "ctrl":                 0, # addr: 0xf0000000
            "uart":                 3, # addr: 0xf0001800
            "spi":                  4, # addr: 0xf0002000
            "timer0":               5, # addr: 0xf0002800
            "sdram":                6, # addr: 0xf0003000
            "identifier_mem":       7, # addr: 0xf0003800
            "gpio_out":             11, # addr: 0xf0005800
            "gpio_in":              12, # addr: 0xf0006000
            "ddrphy":               23, # addr: 0xf000b800
        }}

        interrupt_map = {**soc_cls.interrupt_map, **{
            "timer0":               1,
            "uart":                 2,
            # "spi":                  5,  ???
        }}

        mem_map = {
            "rom":                  0x00000000,
            "sram":                 0x01000000,
            "main_ram":             0x40000000,
            "csr":                  0xf0000000,
        }

        def __init__(self, **kwargs):
            soc_cls.__init__(self,
                cpu_variant="standard",
                max_sdram_size=0x10000000, # Limit mapped SDRAM to 256MB for now
                **kwargs)

            soc_cls.mem_map.update(self.mem_map)

            self.platform.add_extension([
                ("hci_spi", 0,
                    Subsignal("clk",  Pins("P14")),
                    Subsignal("mosi", Pins("U16"), Misc("PULLUP True")),
                    Subsignal("cs_n", Pins("T11"), Misc("PULLUP True")),    <-- conflict w/ necessary cs-gpio, can remove and use gpio here instead?
                    Subsignal("miso", Pins("V15"), Misc("PULLUP True")),
                    Misc("SLEW=FAST"),
                    IOStandard("LVCMOS33"),
                ),
                ("hci", 0,
                    Subsignal("reset", Pins("T14")),        <-- <&gpio_out 0 ... ?
                    Subsignal("irq",  Pins("R12")),          <-- <&gpio_out 1 ... ?
                    IOStandard("LVCMOS33"),
                ),
            ])

            self.submodules.spi = SPIMaster(
                pads=self.platform.request("hci_spi", 0), 
                data_width=8,
                sys_clk_freq=self.clk_freq,
                spi_clk_freq=2e6
            )

            self.submodules.gpio_out = GPIOOut(
                pads=self.platform.request("hci", 0), 
            )

    return _SoC(**kwargs)

    soc_kwargs = {**soc_core_argdict(args), **{"csr_data_width": 8, "integrated_rom_size": 0xfa00}}
    soc = SoC(arty.BaseSoC, **soc_kwargs)

Built with

soc.py --csr-json csr.json

csr.json.zip

[mithro]

FYI - @mateusz-holenko

[mateusz-holenko]

@michalsieron will take a look at that

[lachlansmith]

Hi @michalsieron thanks for looking at this. Hope you're not delving too deep cause I'd like to actually implement this and learn from it. I've more or less just hit a road block with the theory of calculating csr size and determining the gpio indexes.

[michalsieron]

Sorry for late answer, but here is what I was able to find

SPI (what you might be most interested in)

SPI driver in Zephyr for LiteX is slightly outdated and has incorrect offset for loopback register.

If you open SPIMaster implementation in LiteX you can see that previous register - chip select - has two flags on offsets 0 and 16. This means, that with 8-bit CSRs, it will take 3 subregisters - 17 bits require 3 bytes, so 3 subregisters with 8-bit CSRs.

With 32-bit CSRs, this shouldn't matter, because 3 bytes fit in one 4-byte subregister anyway, so offsets are correct.

But this makes me wonder, whether there are other things to fix in this driver.

We will take a look at that.

Register sizes

I believe, that at the time drivers were being written, those sizes were just sum of bytes taken in address space by CSRs assigned to given node. So for example uart0 would have rxtx, txfull, rxempty, ev_status, ev_pending, ev_enable, txempty and rxfull registers. Each of them fits inside one byte, so together they would take 8 subregisters and because each subregister is aligned to 4 bytes we get 8 * 4 = 32 = 0x20. I would imagine it goes the same for other handlers as well.

With changes I made in https://github.com/zephyrproject-rtos/zephyr/pull/45198 and https://github.com/enjoy-digital/litex/pull/1287, you can easily see what is going on.

&uart0 {
    reg = <0xf0001800 0x4
        0xf0001804 0x4
        0xf0001808 0x4
        0xf000180c 0x4
        0xf0001810 0x4
        0xf0001814 0x4
        0xf0001818 0x4
        0xf000181c 0x4>;
    reg-names = "rxtx",
        "txfull",
        "rxempty",
        "ev_status",
        "ev_pending",
        "ev_enable",
        "txempty",
        "rxfull";
    interrupts = <0x2 0>;
};

You can see that even though some of those registers are just 1 bit flags, their sizes are 4 bytes anyway. That is because the script generates overlay from csr.json file, where size is provided in number of subregisters. As number of subregisters is useless for driver when using LiteX HAL from https://github.com/zephyrproject-rtos/zephyr/pull/45196/, I normalized sizes to bytes.

Handlers in litex_json2dts_zephyr script

You may have also noticed, there is no handler for spi, only for spiflash. It means, that even if you were to add SPIMaster to your SoC, the script wouldn't generate overlay for it (unless you name the SPI submodule spiflash or change the name in handler).

There are also no handles for gpio nodes.

If you wrote those overlays yourself, do remember that number of gpios is configured with ngpios parameter (link to docs). Right now, there is no information on number of gpio pins in csr.json file, so the script cannot generate an overlay for it.

[mithro]

@mateusz-holenko - Please review this.

[mateusz-holenko]

Added my comments to #1287

[lachlansmith]

Hi @michalsieron, thanks for the breakdown and going over spi in more detail. I am mostly interested in spi. With one of my peripherals requiring it and litex,uart0 not supporting interrupts, I'll also need it to get BT HCI working.

So using the csr.json I attached prior with spi0. The registers with sizes are spi_control:2, spi_status:1, spi_mosi:1, spi_miso:1, spi_cs:3 and spi_loopback:1. Since csr.json provides these register sizes in number of subregisters the total size of spi0 is 9 * 4 = 36 = 0x24?

This makes sense however still doesn't sit well with me. From zephyr > dts > riscv > riscv32-litex-vexriscv.dtsi spi0 is originally defined with a register size of 0x34, which is not a multiple of 4. This node also references interrupts at position 5, however, the spi_litespi.c driver doesn't use any interrupts. So I'm unsure how this node was written and if it is since outdated it very much distracted me from reaching the above conclusion.

spi0: spi@e0002000 {
        compatible = "litex,spi";
    interrupt-parent = <&intc0>;
    interrupts = <5 0>;
    reg = <0xe0002000 0x34>;
    reg-names = "control";
    label = "spi0";
    status = "disabled";
    #address-cells = <1>;
    #size-cells = <0>;
};

I wrote the spi and gpio_out overlay handles because I assume the intended workflow for developing with Zephyr & LiteX is to do so. Otherwise, eth and i2c are really the only two supported transports at the moment.

Thanks for flagging up ngpios. With gpio thats okay if a script can't generate it, however, can a pins index still be determined somehow? Is it simply the order the pins are passed into GPIOOut?

[lachlansmith]

Hi @michalsieron could help me understand why SPIMaster is generating spi_cs with size 3.

"spi_cs": {
    "addr": 4026540052,
    "size": 3,
    "type": "rw"
}

The Zephyr driver spi_litespi.h expects this register to have size 1, 0x18 - 0x14 = 0x04 = 1 subregister. Is it cause the register is rw?

#define SPI_BASE_ADDR DT_INST_REG_ADDR(0)
#define SPI_CONTROL_REG SPI_BASE_ADDR
#define SPI_STATUS_REG (SPI_BASE_ADDR + 0x08)
#define SPI_MOSI_DATA_REG (SPI_BASE_ADDR + 0x0c)
#define SPI_MISO_DATA_REG (SPI_BASE_ADDR + 0x10)
#define SPI_CS_REG (SPI_BASE_ADDR + 0x14)
#define SPI_LOOPBACK_REG (SPI_BASE_ADDR + 0x18)

Is this what you meant by your comment on incorrect loopback support, given that spi_cs is no longer size 1, and offsetting the loopback register.

Nevermind, re-read the above, been awhile since I've looked back into this