[PATCH] realtek: add support for D-Link DGS-1210-10P H/W:R1

[Adrian Schmutzler]

Hi,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces at lists.openwrt.org]
> On Behalf Of Paul Fertser
> Sent: Samstag, 18. September 2021 00:14
> To: openwrt-devel at lists.openwrt.org
> Cc: Paul Fertser <fercerpav at 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 at 0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +		spi-max-frequency = <50000000>;
> +
> +		partitions {
> +			compatible = "fixed-partitions";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			partition at 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 at 80000 {
> +				label = "u-boot-env";
> +				reg = <0x00080000 0x40000>;
> +				read-only;
> +			};
> +			partition at c0000 {
> +				label = "u-boot-env2";
> +				reg = <0x000c0000 0x40000>;
> +			};
> +			partition at 100000 {
> +				label = "firmware";
> +				compatible = "denx,uimage";
> +				reg = <0x00100000 0xe80000>;
> +			};
> +			partition at f80000 {
> +				label = "kernel2";
> +				reg = <0x00f80000 0x180000>;
> +			};
> +			partition at 1100000 {
> +				label = "rootfs2";
> +				reg = <0x01100000 0xd00000>;
> +			};
> +			partition at 1e00000 {
> +				label = "jffs2";
> +				reg = <0x01e00000 0x200000>;
> +			};
> +		};
> +	};
> +};
> +
> +&ethernet0 {
> +	mdio: mdio-bus {
> +		compatible = "realtek,rtl838x-mdio";
> +		regmap = <&ethernet0>;
> +		#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 at 28 {
> +			ethernet = <&ethernet0>;
> +			reg = <28>;
> +			phy-mode = "internal";

I'd put an empty line before the block.

> +			fixed-link {
> +				speed = <1000>;
> +				full-duplex;
> +			};
> +		};
> +	};
> +};
> +
> +&switch0 {
> +	ports {
> +		port at 24 {
> +			sfp = <&sfp0>;
> +		};
> +
> +		port at 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 at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openpgp-digital-signature.asc
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.openwrt.org/pipermail/openwrt-devel/attachments/20210924/d5314e51/attachment.sig>