Hi, > -----Original Message----- > From: openwrt-devel [mailto:openwrt-devel-boun...@lists.openwrt.org] > On Behalf Of Paul Fertser > Sent: Samstag, 18. September 2021 00:14 > To: openwrt-devel@lists.openwrt.org > Cc: Paul Fertser <fercer...@gmail.com> > Subject: [PATCH] realtek: add support for D-Link DGS-1210-10P H/W:R1
Some mostly nitpick comments below.
[...]
> +
> +&spi0 {
> + status = "okay";
I like empty lines after status for readability.
> + flash@0 {
> + compatible = "jedec,spi-nor";
> + reg = <0>;
> + spi-max-frequency = <50000000>;
> +
> + partitions {
> + compatible = "fixed-partitions";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + partition@0 {
> + label = "u-boot";
> + reg = <0x00000000 0x80000>;
This is a bit hard to read, particularly since you switch the size to 6 digits
later. Would you just use 7 digits for all numbers here (offset and size)?
First could be abbreviated to 0x0 of course.
> + read-only;
> + };
I'd put empty lines between nodes.
> + partition@80000 {
> + label = "u-boot-env";
> + reg = <0x00080000 0x40000>;
> + read-only;
> + };
> + partition@c0000 {
> + label = "u-boot-env2";
> + reg = <0x000c0000 0x40000>;
> + };
> + partition@100000 {
> + label = "firmware";
> + compatible = "denx,uimage";
> + reg = <0x00100000 0xe80000>;
> + };
> + partition@f80000 {
> + label = "kernel2";
> + reg = <0x00f80000 0x180000>;
> + };
> + partition@1100000 {
> + label = "rootfs2";
> + reg = <0x01100000 0xd00000>;
> + };
> + partition@1e00000 {
> + label = "jffs2";
> + reg = <0x01e00000 0x200000>;
> + };
> + };
> + };
> +};
> +
> +ðernet0 {
> + mdio: mdio-bus {
> + compatible = "realtek,rtl838x-mdio";
> + regmap = <ðernet0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + INTERNAL_PHY(8)
> + INTERNAL_PHY(9)
> + INTERNAL_PHY(10)
> + INTERNAL_PHY(11)
> + INTERNAL_PHY(12)
> + INTERNAL_PHY(13)
> + INTERNAL_PHY(14)
> + INTERNAL_PHY(15)
> + INTERNAL_PHY(24)
> + INTERNAL_PHY(26)
> + };
> +};
> +
> +&switch0 {
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + SWITCH_PORT(8, 1, internal)
> + SWITCH_PORT(9, 2, internal)
> + SWITCH_PORT(10, 3, internal)
> + SWITCH_PORT(11, 4, internal)
> + SWITCH_PORT(12, 5, internal)
> + SWITCH_PORT(13, 6, internal)
> + SWITCH_PORT(14, 7, internal)
> + SWITCH_PORT(15, 8, internal)
> + SWITCH_SFP_PORT(24, 9, rgmii-id)
> + SWITCH_SFP_PORT(26, 10, rgmii-id)
> +
> + port@28 {
> + ethernet = <ðernet0>;
> + reg = <28>;
> + phy-mode = "internal";
I'd put an empty line before the block.
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> +};
> +
> +&switch0 {
> + ports {
> + port@24 {
> + sfp = <&sfp0>;
> + };
> +
> + port@26 {
> + sfp = <&sfp1>;
> + };
> + };
> +};
> diff --git a/target/linux/realtek/image/Makefile
> b/target/linux/realtek/image/Makefile
> index 5900750797e8..727f7bfa4164 100644
> --- a/target/linux/realtek/image/Makefile
> +++ b/target/linux/realtek/image/Makefile
> @@ -60,6 +60,14 @@ define Device/d-link_dgs-1210-10p endef
> TARGET_DEVICES += d-link_dgs-1210-10p
>
> +define Device/d-link_dgs-1210-10p-r1
> + $(Device/d-link_dgs-1210)
> + DEVICE_MODEL := DGS-1210-10P-R1
Is this R1 actually a revision? If yes, I'd prefer DEVICE_VARIANT "R1" here.
Best
Adrian
> + # for rtl83xx-poe
> + DEVICE_PACKAGES += libubox-lua libubus-lua libuci-lua lua-rs232 endef
> +TARGET_DEVICES += d-link_dgs-1210-10p-r1
> +
> define Device/d-link_dgs-1210-16
> $(Device/d-link_dgs-1210)
> DEVICE_MODEL := DGS-1210-16
> --
> 2.17.1
>
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
openpgp-digital-signature.asc
Description: PGP signature
_______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
