`boot.loader.raspberrypi.firmwareConfig` got deprecated without providing alternatives

dorkeline commented 3 months ago

Describe the bug

Commit https://github.com/NixOS/nixpkgs/commit/a6e61a1ea9c71cec5eb83b1dc800805f55594a83 deprecated several options for raspberrypi boards due to them never having worked well.

Specifically the boot.loader.raspberryPi.firmwareConfig was not given any alternative apart from a vague example using hardware.deviceTree.overlays on the wiki with no explanation how to port preexisting dtoverlay/dtparam entries. Searching for github issues around dtoverlay/peripherals seems to indicate people have kept using the deprecated options which suggests there is an issue in documentation on how to migrate.

This would also be very useful for people migrating non-nixos setups using /boot/config.txt to nixos by greatly lowering the entry barrier of porting peripheral setups.

Expected behavior

A description, or a reference to a chapter in the manual, in the docstring/deprecation warning explaining how to port existing code using config.txt entries instead of just telling users their config will break with 24.11.

Notify maintainers

@samueldr (as the deprecator of the options)

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

[root@localhost:~]# nix-shell -p nix-info --run "nix-info -m"
 - system: `"aarch64-linux"`
 - host os: `Linux 6.1.63, NixOS, 24.05 (Uakari), 24.05.20240612.cc54fb4`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.18.2`
 - nixpkgs: `/nix/store/aa0zsahvyqnvzkicsh29jirm9hwv95pg-source`

Add a :+1: reaction to issues you find important.

chuangzhu commented 3 months ago

I agree. An alternative should be provided right in the manual page instead of only on the wiki. We should at least provide a link to nixos-hardware/raspberry-pi because it's non-trivial for anyone who are not expertized in embedded Linux to write those configs on their own

dorkeline commented 3 months ago

After looking a bit i found https://github.com/raspberrypi/firmware/tree/master/boot/overlays's readme mentioning the ovmerge utility which converts config.txt style dtoverlay entries into a dts file which would sound great as a convenience wrapper.

My entries in particular look like this

dtparam=spi=on
dtparam=i2s=on
dtoverlay=spi1-3cs
dtoverlay=mcp251xfd,spi0-0,interrupt=25
dtoverlay=mcp251xfd,spi1-0,interrupt=24

Running ovmerge on a checkout of https://github.com/raspberrypi/linux/tree/rpi-6.1.y/arch/arm/boot/dts/overlays

// redo: ovmerge -c mcp251xfd-overlay.dts,spi0-0,interrupt=25 mcp251xfd-overlay.dts,spi1-0,interrupt=24 spi1-3cs-overlay.dts

/dts-v1/;
/plugin/;

#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/pinctrl/bcm2835.h>

/ {
    compatible = "brcm,bcm2835";
    fragment@0 {
        target = <&spidev0>;
        __overlay__ {
            status = "disabled";
        };
    };
    fragment@1 {
        target = <&gpio>;
        __overlay__ {
            mcp251xfd_pins: mcp251xfd_spi0_0_pins {
                brcm,pins = <25>;
                brcm,function = <BCM2835_FSEL_GPIO_IN>;
            };
        };
    };
    fragment@2 {
        target-path = "/clocks";
        __overlay__ {
            clk_mcp251xfd_osc: mcp251xfd-spi0-0-osc {
                #clock-cells = <0>;
                compatible = "fixed-clock";
                clock-frequency = <40000000>;
            };
        };
    };
    fragment@3 {
        target = <&spi0>;
        __overlay__ {
            status = "okay";
            #address-cells = <1>;
            #size-cells = <0>;
            mcp251xfd@0 {
                compatible = "microchip,mcp251xfd";
                reg = <0>;
                pinctrl-names = "default";
                pinctrl-0 = <&mcp251xfd_pins>;
                spi-max-frequency = <20000000>;
                interrupt-parent = <&gpio>;
                interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
                clocks = <&clk_mcp251xfd_osc>;
            };
        };
    };
    fragment@4 {
        target-path = "spi1/spidev@0";
        __overlay__ {
            status = "disabled";
        };
    };
    fragment@5 {
        target = <&gpio>;
        __overlay__ {
            mcp251xfd_pins_1: mcp251xfd_spi1_0_pins {
                brcm,pins = <24>;
                brcm,function = <BCM2835_FSEL_GPIO_IN>;
            };
        };
    };
    fragment@6 {
        target-path = "/clocks";
        __overlay__ {
            clk_mcp251xfd_osc_1: mcp251xfd-spi1-0-osc {
                #clock-cells = <0>;
                compatible = "fixed-clock";
                clock-frequency = <40000000>;
            };
        };
    };
    fragment@7 {
        target = <&spi1>;
        __overlay__ {
            status = "okay";
            #address-cells = <1>;
            #size-cells = <0>;
            mcp251xfd@0 {
                compatible = "microchip,mcp251xfd";
                reg = <0>;
                pinctrl-names = "default";
                pinctrl-0 = <&mcp251xfd_pins_1>;
                spi-max-frequency = <20000000>;
                interrupt-parent = <&gpio>;
                interrupts = <24 IRQ_TYPE_LEVEL_LOW>;
                clocks = <&clk_mcp251xfd_osc_1>;
            };
        };
    };
    fragment@8 {
        target = <&gpio>;
        __overlay__ {
            spi1_pins: spi1_pins {
                brcm,pins = <19 20 21>;
                brcm,function = <3>;
            };
            spi1_cs_pins: spi1_cs_pins {
                brcm,pins = <18 17 16>;
                brcm,function = <1>;
            };
        };
    };
    fragment@9 {
        target = <&spi1>;
        frag1: __overlay__ {
            #address-cells = <1>;
            #size-cells = <0>;
            pinctrl-names = "default";
            pinctrl-0 = <&spi1_pins &spi1_cs_pins>;
            cs-gpios = <&gpio 18 1>, <&gpio 17 1>, <&gpio 16 1>;
            status = "okay";
            spidev1_0: spidev@0 {
                compatible = "spidev";
                reg = <0>;
                #address-cells = <1>;
                #size-cells = <0>;
                spi-max-frequency = <125000000>;
                status = "okay";
            };
            spidev1_1: spidev@1 {
                compatible = "spidev";
                reg = <1>;
                #address-cells = <1>;
                #size-cells = <0>;
                spi-max-frequency = <125000000>;
                status = "okay";
            };
            spidev1_2: spidev@2 {
                compatible = "spidev";
                reg = <2>;
                #address-cells = <1>;
                #size-cells = <0>;
                spi-max-frequency = <125000000>;
                status = "okay";
            };
        };
    };
    fragment@10 {
        target = <&aux>;
        __overlay__ {
            status = "okay";
        };
    };
    __overrides__ {
        cs0_pin = <&spi1_cs_pins>, "brcm,pins:0", <&frag1>, "cs-gpios:4";
        cs1_pin = <&spi1_cs_pins>, "brcm,pins:4", <&frag1>, "cs-gpios:16";
        cs2_pin = <&spi1_cs_pins>, "brcm,pins:8", <&frag1>, "cs-gpios:28";
        cs0_spidev = <&spidev1_0>, "status";
        cs1_spidev = <&spidev1_1>, "status";
        cs2_spidev = <&spidev1_2>, "status";
    };
};

Which i then added to my nixos config like this

  hardware = {
    raspberry-pi."4".apply-overlays-dtmerge.enable = true;
    deviceTree = {
      enable = true;
      filter = "*rpi-4-*.dtb";
      overlays = [{ name = "can-rpi-4-hat"; dtsFile = ./can-hat.dts; }];
    };
  };

But it doesn't seem to work, theres no /dev/spi{,dev} or can interfaces, i get nothing in dmesg about any overlay being applied and grepping for spi or mcp251xfd also shows no logs. I also tried with the filter option being commented out.

Sadly device trees are a new concept to me so i dont know whether im doing something stupid that is a dead end

dorkeline commented 3 months ago

It does seem what i wrote does work, but i had to split off spi1-cs3 and replace bcrm,bcm2835 with bcrm,bcm2711 for nix to pick it up. Im unsure why that is as it seems that bcm2835 is the base?

I've now made a small module for myself that mimicks the dtoverlay parameter in config.txt by using ovmerge and sed which is probably horrible:

{self, lib, config, pkgs, ...}: let
  inherit (builtins) map;
  inherit (lib) mkOption getExe;
  inherit (lib.types) listOf str;
  inherit (self.packages.${pkgs.system}) ovmerge;

  cfg = config.boot.raspi;
  kernelSrc = pkgs.fetchFromGitHub {
    owner = "raspberrypi";
    repo = "linux";
    rev = "cd92a9591833ea06d1f12391f6b027fcecf436a9";
    hash = "sha256-+9KpjeYFUeH0YCf40GICfTr/Tz++eNbUPenDOeKy+Vc=";
  };
in {
  options.boot.raspi.dtoverlays = mkOption {
    type = listOf str;
    default = [];
  };

  config = {
    hardware = {
      raspberry-pi."4".apply-overlays-dtmerge.enable = true;
      deviceTree = {
        enable = true;
        filter = "*rpi-4*.dtb";
        overlays = map (name: {
          inherit name;
          dtsFile = pkgs.runCommand "dtoverlay-${name}" {} ''
            cd ${kernelSrc}/arch/arm/boot/dts/overlays
            ${getExe ovmerge} ${name} | sed "s/brcm,bcm2835/brcm,bcm2711/g" > $out
          '';
        }) cfg.dtoverlays;
      };
    };
  };
}

So my original config.txt example now becomes:

boot.raspi.dtoverlays = [
  "spi1-3cs-overlay.dts"
  "mcp251xfd-overlay.dts,spi0-0,interrupt=25"
  "mcp251xfd-overlay.dts,spi1-0,interrupt=24"
];

which i am personally quite happy with, but the hackiness of the sed and also ordering issues (if i e.g. define the spi1-3cs line in another module and it gets ordered after the others it breaks compilation) makes me reluctant of upstreaming an approach like this.

NixOS / nixpkgs