-
Notifications
You must be signed in to change notification settings - Fork 34
Acacia I2C, Serial and Timer + new Python tooling #751
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
base: main
Are you sure you want to change the base?
Changes from 6 commits
959f432
a55c47c
013449b
6d06dfb
de0f706
55b8d7b
f8215fb
1d5b5c5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| # Copyright 2026, UNSW | ||
| # SPDX-License-Identifier: BSD-2-Clause | ||
|
|
||
| from .i2c import sDDFI2C | ||
| from .timer import sDDFTimer | ||
| from .serial import sDDFSerial | ||
| from .sddf import sDDFDriverClass, sDDFDriverConfig, sDDFDriverManifest | ||
| from .board import BOARDS, Board | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,189 @@ | ||
| # Copyright 2025, UNSW | ||
| # SPDX-License-Identifier: BSD-2-Clause | ||
| from dataclasses import dataclass | ||
| from typing import List, Optional, Tuple | ||
| from acacia import System, ProtectionDomain, aarch64, riscv64, x86_64, Arch | ||
| from importlib.metadata import version | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class DriverDouble: | ||
| compatible: str | ||
| node_path: str | ||
|
|
||
|
|
||
| @dataclass | ||
| class Board: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we remove the board.py indirection? This seems to duplicate information already present in the DTB which is never ideal since a human has to do this and is likely to make mistakes (such as the zcu paddr being wrong previously). This is probably a consideration independent of this PR.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Per other comment I replied to, basically yes we can, and we probably should, but that's something that is beyond the scope of just adding Acacia |
||
| name: str | ||
| arch: Arch | ||
| paddr_top: int | ||
| # Driver mappings -> (compatible, preferred_node) tuples | ||
| serial: Optional[DriverDouble] = DriverDouble(None, None) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Also, if it is |
||
| ethernet: Optional[DriverDouble] = DriverDouble(None, None) | ||
| timer: Optional[DriverDouble] = DriverDouble(None, None) | ||
| i2c: Optional[DriverDouble] = DriverDouble(None, None) | ||
| blk: Optional[DriverDouble] = DriverDouble(None, None) | ||
| partition: int = 0 | ||
| baud_rate: Optional[int] = None | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does this work when you have multiple ethernet devices (of the same or different kinds) in the same board? I think these should be lists of DriverDouble
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It didn't before these changes, from In general I think we mostly assume we have "single" instances of the driver subsystems/components. There are plenty of cases where it makes sense for systems to have multiple different block drivers, or i2c, or serial, and even multiple connections per client.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think for now we should keep assuming that. Maybe what we should do instead is that when you construct a system, by default what should happen is we iterate over the DTS file and instantiate a driver for every compatible string in the DTS we know about. We can have a user API that has a stable naming/ordering scheme and then you ask for the Network subsystem "manager" to connect your client to e.g. ethernet device 0 or 1, or ethernet device at this PCI bus location, or identified by this PCI vendor+device, or at this DTS location. Most of our examples can then get away with asking for device 0 (because, maybe say, we use the DTS chosen node in the DTS we provide to force it to be the right one). Then maybe users of our build system can construct systems in which you have connections, and if a particular block driver has no connections then we purge it from the system. (For clock drivers, maybe not, though you would assume there would be a connection between them and device drivers depending on them, so it could probably follow the same idea). This makes sense in the LionsOS context too, really. |
||
|
|
||
| # Keep this list in alphabetical order by board name | ||
| # TODO: convert to Dictionary | ||
| BOARDS: List[Board] = [ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I personally would prefer splitting these out one per supported board, then just importing the one you want.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I know the old sdfgen tooling did it this way)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then examples need to import every board anyway? |
||
| Board( | ||
| name="cheshire", | ||
| arch=riscv64, | ||
| paddr_top=0x90000000, | ||
| serial=DriverDouble("ns16550a", "soc/serial@3002000"), | ||
| i2c=DriverDouble("eth,i2c", "soc/i2c@3003000"), | ||
| ), | ||
| Board( | ||
| name="hifive_p550", | ||
| arch=riscv64, | ||
| paddr_top=0xA0000000, | ||
| serial=DriverDouble("snps,dw-apb-uart", "soc/serial@0x50900000"), | ||
| ), | ||
| Board( | ||
| name="imx8mm_evk", | ||
| arch=aarch64, | ||
| paddr_top=0x70000000, | ||
| serial=DriverDouble( | ||
| "fsl,imx8mm-uart", "soc@0/bus@30800000/spba-bus@30800000/serial@30890000" | ||
| ), | ||
| timer=DriverDouble("fsl,imx8mm-gpt", "soc@0/bus@30000000/timer@302d0000"), | ||
| ethernet=DriverDouble("", "soc@0/bus@30800000/ethernet@30be0000"), | ||
| ), | ||
| Board( | ||
| name="imx8mp_evk", | ||
| arch=aarch64, | ||
| paddr_top=0x70000000, | ||
| serial=DriverDouble( | ||
| "fsl,imx8mp-uart", "soc@0/bus@30800000/spba-bus@30800000/serial@30890000" | ||
| ), | ||
| timer=DriverDouble("fsl,imx8mp-gpt", "soc@0/bus@30000000/timer@302d0000"), | ||
| ethernet=DriverDouble("", "soc@0/bus@30800000/ethernet@30bf0000"), | ||
| ), | ||
| Board( | ||
| name="imx8mp_iotgate", | ||
| arch=aarch64, | ||
| paddr_top=0x70000000, | ||
| serial=DriverDouble("fsl,imx8mp-uart", "soc@0/bus@30800000/serial@30890000"), | ||
| timer=DriverDouble("fsl,imx8mp-gpt", "soc@0/bus@30000000/timer@302d0000"), | ||
| ethernet=DriverDouble("", "soc@0/bus@30800000/ethernet@30bf0000"), | ||
| ), | ||
| Board( | ||
| name="imx8mq_evk", | ||
| arch=aarch64, | ||
| paddr_top=0x70000000, | ||
| serial=DriverDouble("fsl,imx8mq-uart", "soc@0/bus@30800000/serial@30860000"), | ||
| timer=DriverDouble("fsl,imx8mq-gpt", "soc@0/bus@30000000/timer@302d0000"), | ||
| ethernet=DriverDouble("", "soc@0/bus@30800000/ethernet@30be0000"), | ||
| ), | ||
| Board( | ||
| name="kria_k26", | ||
| arch=aarch64, | ||
| paddr_top=0x70000000, | ||
| timer=DriverDouble("cdns,ttc", "axi/timer@ff140000"), | ||
| serial=DriverDouble("xlnx,zynqmp-uart", "axi/serial@ff010000"), | ||
| ), | ||
| Board( | ||
| name="maaxboard", | ||
| arch=aarch64, | ||
| paddr_top=0x70000000, | ||
| serial=DriverDouble("fsl,imx8mq-uart", "soc@0/bus@30800000/serial@30860000"), | ||
| timer=DriverDouble("fsl,imx8mq-gpt", "soc@0/bus@30000000/timer@302d0000"), | ||
| ethernet=DriverDouble("", "soc@0/bus@30800000/ethernet@30be0000"), | ||
| blk=DriverDouble("", "soc@0/bus@30800000/mmc@30b40000"), | ||
| partition=2, | ||
| ), | ||
| Board( | ||
| name="odroidc2", | ||
| arch=aarch64, | ||
| paddr_top=0x60000000, | ||
| serial=DriverDouble("amlogic,meson-gx-uart", "soc/bus@c8100000/serial@4c0"), | ||
| timer=DriverDouble("amlogic,meson-gxbb-wdt", "soc/bus@c1100000/watchdog@98d0"), | ||
| ethernet=DriverDouble("", "soc/ethernet@c9410000"), | ||
| baud_rate=115200, | ||
| ), | ||
| Board( | ||
| name="odroidc4", | ||
| arch=aarch64, | ||
| paddr_top=0x60000000, | ||
| i2c=DriverDouble("amlogic,meson-axg-i2c", "soc/bus@ffd00000/i2c@1d000"), | ||
| serial=DriverDouble("amlogic,meson-gx-uart", "soc/bus@ff800000/serial@3000"), | ||
| timer=DriverDouble("amlogic,meson-gxbb-wdt", "soc/bus@ffd00000/watchdog@f0d0"), | ||
| ethernet=DriverDouble("amlogic,meson-gx-uart", "soc/ethernet@ff3f0000"), | ||
| baud_rate=115200, | ||
| ), | ||
| Board( | ||
| name="qemu_virt_aarch64", | ||
| arch=aarch64, | ||
| paddr_top=0x6_0000_000, | ||
| serial=DriverDouble("arm,pl011", "pl011@9000000"), | ||
| timer=DriverDouble("arm,armv8-timer", "timer"), | ||
| blk=DriverDouble("", "virtio_mmio@a000200"), | ||
| ethernet=DriverDouble("", "virtio_mmio@a000000"), | ||
| i2c=None, | ||
| ), | ||
| Board( | ||
| name="qemu_virt_riscv64", | ||
| arch=riscv64, | ||
| paddr_top=0xA_0000_000, | ||
| serial=DriverDouble("ns16550a", "soc/serial@10000000"), | ||
| timer=DriverDouble("google,goldfish-rtc", "soc/rtc@101000"), | ||
| ethernet=DriverDouble("", "soc/virtio_mmio@10001000"), | ||
| blk=DriverDouble("", "soc/virtio_mmio@10002000"), | ||
| partition=0, | ||
| i2c=None, | ||
| ), | ||
| Board( | ||
| name="rock3b", | ||
| arch=aarch64, | ||
| paddr_top=0xEC000000, | ||
| serial=DriverDouble("snps,dw-apb-uart", "serial@fe660000"), | ||
| timer=DriverDouble("rockchip,rk3568-timer", "rktimer@fe5f0000"), | ||
| ethernet=DriverDouble("", "ethernet@fe2a0000"), | ||
|
omeh-a marked this conversation as resolved.
|
||
| baud_rate=1500000, | ||
| ), | ||
| Board( | ||
| name="rpi4b_1gb", | ||
| arch=aarch64, | ||
| paddr_top=0x2_000_000, | ||
| serial=DriverDouble("brcm,bcm2835-aux-uart", "soc/serial@7e215040"), | ||
| timer=DriverDouble("brcm,bcm2835-system-timer", "soc/timer@7e003000"), | ||
| ethernet=DriverDouble("", "scb/ethernet@7d580000"), | ||
| ), | ||
| Board( | ||
| name="serengeti", | ||
| arch=riscv64, | ||
| paddr_top=0x90000000, | ||
| serial=DriverDouble("ns16550a", "soc/serial@3002000"), | ||
| i2c=DriverDouble("eth,i2c", "soc/i2c@3003000"), | ||
| timer=DriverDouble("pulp,apb_timer", "soc/timer@300B000"), | ||
| ), | ||
| Board( | ||
| name="star64", | ||
| arch=riscv64, | ||
| paddr_top=0x100000000, | ||
| serial=DriverDouble("starfive,jh7110-uart", "soc/serial@10000000"), | ||
| timer=DriverDouble("starfive,jh7110-timer", "soc/timer@13050000"), | ||
| ethernet=DriverDouble("", "soc/ethernet@16030000"), | ||
| ), | ||
| Board( | ||
| name="zcu102", | ||
| arch=aarch64, | ||
| paddr_top=0x80000000, | ||
| timer=DriverDouble("cdns,ttc", "axi/timer@ff140000"), | ||
| serial=DriverDouble("xlnx,zynqmp-uart", "axi/serial@ff000000"), | ||
|
omeh-a marked this conversation as resolved.
|
||
| ), | ||
| Board( | ||
| name="x86_64_generic", | ||
| arch=x86_64, | ||
| paddr_top=0x7FFDF000, | ||
| ), | ||
| Board( | ||
| name="x86_64_generic_vtx", | ||
| arch=x86_64, | ||
| paddr_top=0x7FFDF000, | ||
| ), | ||
| ] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| # Copyright 2026, UNSW | ||
| # SPDX-License-Identifier: BSD-2-Clause | ||
|
|
||
| from dataclasses import dataclass | ||
| from typing import List, Dict, Type, Union, Optional | ||
| from collections import defaultdict | ||
|
|
||
|
|
||
| @dataclass | ||
| class DTSRegion: | ||
| name: str | ||
| perms: str = None | ||
| size: int = None | ||
| dt_idx: int = None | ||
|
|
||
|
|
||
| @dataclass | ||
| class DTSIRQ: | ||
| dt_index: int | ||
|
|
||
|
|
||
| @dataclass | ||
| class sDDFDriverConfig: | ||
| """ | ||
| Encapsulation of device tree fields describing an instance | ||
| of a driver. | ||
|
|
||
| WARNING: the order of regions and irqs affects the order they are | ||
| mapped into the driver in config structs! We REALLY shouldn't have | ||
| this be the case. This is a hangover from `config.json` and sdfgen. | ||
|
|
||
| TODO: make this better in future | ||
| """ | ||
|
|
||
| compatible: Union[List[str], str] | ||
| regions: List[DTSRegion] | ||
| irqs: List[DTSIRQ] | ||
|
|
||
| def __post_init__(self): | ||
| if type(self.compatible) is str: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isinstance, not type |
||
| self.compatible = [self.compatible] | ||
| assert type(self.regions) is list | ||
| assert type(self.irqs) is list | ||
|
|
||
|
|
||
| class __sDDFDriverManifest: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If anything I feel like this PR should just transliterate the exact same functionality from the zig sdfgen classes to Python, so that the difference is extremely minimal limited to a few type changes and import differences. Then we can discuss changing the API exposed in
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These changes were necessary due to acacia working differently than sdf-gen. There's no real way around this without basically just putting the same functionality elsewhere in the python that is added, and indeed this is doing something similarly to what sdf_gen did by scanning the sddf for config.jsons and keeping a register of them
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not all of the changes were necessary. Perhaps moving away from config.json is better. Is it definitely? IDK. But it's a separate change that is logically distinct from a Zig -> Python rewrite.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't just do a zig to python rewrite when we need to move all the changes that were formerly in a different tool, in a different language, to here. Again, this is just doing something similar to what sdf_gen did. See sddf.zig ... this is a replacement for these We can obviously change this in the future. But this bookkeeping isn't new. (I can add searching for config.jsons if we want, just simpler calling direct for now..)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if this was considered and then decided against but can you avoid all the add_driver_config repetition and use the json by doing something like this: Make the sDDFDriverClass class use the DT path to find the DT node like it does atm, then can it retrieve the compatible strings and provide this to the sDDFDriverManifest. Then the sDDFDriverManifest selects the json config from the compatible strings and parses the config into the form equivalent to what all the add_driver_config function calls are doing atm? I guess you would also provide the driver class name so the manifest knows which directory to search.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The main thing is that we will need to rejig this all soon anyway. Fundamentally the sdf_gen model isn't really any good for handling dependencies or per-board deviations. The goal here for me was to basically do the same thing minimally for now |
||
| """ | ||
| Wrapper class encapsulating sDDF driver manifest. This is a | ||
| mapping of driver subsystem type -> list of driver names -> | ||
| DTS fields. I.e. this encodes: | ||
| * Which drivers are compatible with what devices, according to the | ||
| device tree, | ||
| * What drivers are available in each driver class, | ||
| * Which driver subsystem types in sdfgen map to which drivers. | ||
|
|
||
| You should NOT make a new instance of this class! Use the | ||
| `sDDFDriverManifest()` function to get the global instance. | ||
| """ | ||
|
|
||
| def __init__(self): | ||
| self.map: Dict[Type[sDDFDeviceClass], Dict[str, sDDFDriverConfig]] = ( | ||
| defaultdict(dict) | ||
| ) | ||
|
|
||
| def add_driver_config( | ||
| self, | ||
| subsystem_type: Type[sDDFDriverConfig], | ||
| driver_name: str, | ||
| config: sDDFDriverConfig, | ||
| ): | ||
| # Refuse namespace collisions | ||
| if driver_name in self.map[subsystem_type]: | ||
| raise ValueError( | ||
| f"Driver named {driver_name} already exists for " f"{subsystem_type}!" | ||
| ) | ||
| self.map[subsystem_type][driver_name] = config | ||
|
|
||
| def __getitem__(self, item): | ||
| # Allow array syntax for indexing into dict of driver names per class type | ||
| return self.map[item] | ||
|
|
||
| def get_configs_matching_compatible( | ||
| self, subsystem_type: Type[sDDFDriverConfig], compat: str | ||
| ) -> List[sDDFDriverConfig]: | ||
| return [c for c in self.map[subsystem_type].values() if compat in c.compatible] | ||
|
|
||
|
|
||
| module_manifest = __sDDFDriverManifest() | ||
|
|
||
|
|
||
| def sDDFDriverManifest(): | ||
| return module_manifest | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should be under
acacia_sddf. It's not really clear what acacia is for a user. I think the timer/serial classes should either live in the appropriateserial/ortimer/directory and helpers undertools/metaortools/acacia.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a blunt reason for this: it means that we only need one Python import and it means that each driver class doesn't need to abuse importlib to find the others. I really think it's fine to have end users learn what Acacia is, given that they have to install it to use the sDDF. I did it as you suggest initially and it made things horrible. I'm really not eager to change this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think having to set up PYTHONPATH (which you can do entirely in the Makefile) is that bad. If you want just one import you can still do that without separating the components from the rest of the components, you just have some path-mangling.
The other thing is to consider external users: LionsOS, for instance, could just do "import sddf.timer" if it's located in the timer folder, whereas with this way you need to either do "import sddf.acacia_sddf.timer" or have top level exports (not necessary if your project structure follows python module structure, no need for init.py etc). They'd have to put sDDF in their PythonPath but that's fine and reasonable.
tldr; no, I disagree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users of anything will be using acacia anyway though? How can a user of acacia who must be using it in their metaprogram not already know what acacia is? Making the entire sDDF tree a python module just complicates things. The only functional diff from what you're describing is whether the module is called
sddforacacia_sddf. We can renameacacia_sddfto justsddfif it matters ... I personally think it's inconsequential, but I also really think it's worse for code organisation to scatter the python through the source tree.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sidenote: I don't think we will need to
import sddf.acacia_sddf.blaheither? We can do this exactly as it is done in the metaprogram and not make a top level sDDF module