Hello, I don't have more comments, it looks good now. Maybe just think about my suggestion with common dtsi for all 3 models. And just one question - how does it look with flashing process on those devices?
Cheers, Piotr 2015-09-29 18:13 GMT+02:00 Kang Xiaoning <kan...@gmail.com>: > > > On Tue, Sep 29, 2015 at 4:31 AM, Piotr Dymacz <pep...@gmail.com> wrote: >> >> Hello, >> >> Just some small, cosmetic things... see inline, below. >> >> BTW. >> I see that these models are very similar (most of the dts files are >> just copy&paste). >> Maybe it would be better to make for them one, common dtsi file and >> separated dts files per model (take Lenovo Y1/Y1S as example). >> >> Cheers, >> Piotr >> >> 2015-09-28 16:42 GMT+02:00 Comman Kang <kan...@163.com>: >> > HiWiFi HC5661/5761/5861 models are manufactured by >> > http://www.hiwifi.com. These models have similar hardware specs(MT7620A + >> > 128M DDR2 + 16M flash). This patch adds support for them. >> > >> > The original author is Justin Liu (rss...@gmail.com). I ported the patch >> > to trunk and submitted it here with his approval. >> > >> > v2 fix >> > 1: Renamed files to remove manufacturer’s name. >> > 2: styling work >> > >> > Signed-off-by: Xiaoning Kang <kan...@163.com> >> > >> > >> > diff --git a/target/linux/ramips/dts/HC5661.dts >> > b/target/linux/ramips/dts/HC5661.dts >> > new file mode 100644 >> > index 0000000..b5b9d1a >> > --- /dev/null >> > +++ b/target/linux/ramips/dts/HC5661.dts >> > @@ -0,0 +1,172 @@ >> > +/dts-v1/; >> > + >> > +/include/ "mt7620a.dtsi" >> > + >> > +/ { >> > + compatible = "HC5661", "ralink,mt7620a-soc"; >> > + model = "HiWiFi HC5661"; >> > + >> > + chosen { >> > + bootargs = "console=ttyS0,115200"; >> > + }; >> > + >> > + palmbus@10000000 { >> > + sysc@0 { >> > + ralink,gpiomux = "i2c", "jtag"; >> > + ralink,uartmux = "gpio"; >> > + ralink,wdtmux = <1>; >> > + }; >> > + >> > + gpio0: gpio@600 { >> > + status = "okay"; >> > + }; >> > + >> > + gpio2: gpio@660 { >> > + status = "okay"; >> > + }; >> > + >> > + gpio3: gpio@688 { >> > + status = "okay"; >> > + }; >> > + >> > + spi@b00 { >> > + status = "okay"; >> > + >> > + m25p80@0 { >> > + #address-cells = <1>; >> > + #size-cells = <1>; >> > + compatible = "w25q128"; >> > + reg = <0 0>; >> > + linux,modalias = "m25p80", "w25q128"; >> > + spi-max-frequency = <10000000>; >> > + >> > + partition@0 { >> > + label = "u-boot"; >> > + reg = <0x0 0x30000>; >> > + }; >> > + >> > + partition@30000 { >> > + label = "u-boot-env"; >> > + reg = <0x30000 0x10000>; >> > + read-only; >> > + }; >> > + >> > + factory: partition@40000 { >> > + label = "factory"; >> > + reg = <0x40000 0x10000>; >> > + }; >> > + >> > + partition@50000 { >> > + label = "firmware"; >> > + reg = <0x50000 0xf80000>; >> > + }; >> > + >> > + partition@fd0000 { >> > + label = "hwf_config"; >> > + reg = <0xfd0000 0x10000>; >> > + }; >> > + >> > + bdinfo: partition@fe0000 { >> > + label = "bdinfo"; >> > + reg = <0xfe0000 0x10000>; >> > + }; >> > + >> > + partition@ff0000 { >> > + label = "backup"; >> > + reg = <0xff0000 0x10000>; >> > + }; >> > + }; >> > + }; >> > + }; >> > + >> > + ehci@101c0000 { >> > + status = "okay"; >> > + }; >> > + >> > + ohci@101c1000 { >> > + status = "okay"; >> > + }; >> > + >> > + ethernet@10100000 { >> > + pinctrl-names = "default"; >> > + pinctrl-0 = <&ephy_pins>; >> > + mtd-mac-address = <&factory 0x4>; >> > + ralink,port-map = "wllll"; >> > + }; >> > + >> > + sdhci@10130000 { >> > + status = "okay"; >> > + }; >> > + >> > + wmac@10180000 { >> > + ralink,mtd-eeprom = <&factory 0>; >> > + }; >> > + >> > + pcie@10140000 { >> > + status = "okay"; >> > + }; >> > + >> > + pinctrl { >> > + state_default: pinctrl0 { >> > + gpio { >> > + ralink,group = "uartf", "wled", "nd_sd"; >> > + ralink,function = "gpio"; >> > + }; >> > + >> > + pa { >> > + ralink,group = "pa"; >> > + ralink,function = "pa"; >> > + }; >> > + }; >> > + }; >> > + >> > + gpio-leds { >> > + compatible = "gpio-leds"; >> > + >> > + system { >> > + label = "hc5661:blue:system"; >> > + gpios = <&gpio0 9 1>; >> > + }; >> > + >> > + internet { >> > + label = "hc5661:blue:internet"; >> > + gpios = <&gpio0 11 1>; >> > + }; >> > + >> > + wlan2g { >> > + label = "hc5661:blue:wlan2g"; >> > + gpios = <&gpio3 0 1>; >> > + }; >> > + >> > + wlan5g { >> > + label = "hc5661:blue:wlan5g"; >> > + gpios = <&gpio0 7 1>; >> > + }; >> > + }; >> > + >> > + gpio-keys-polled { >> > + compatible = "gpio-keys-polled"; >> > + #address-cells = <1>; >> > + #size-cells = <0>; >> > + poll-interval = <20>; >> > + >> > + reset { >> > + label = "reset"; >> > + gpios = <&gpio0 12 1>; >> > + linux,code = <0x198>; >> > + }; >> > + }; >> > + >> > + gpio_export { >> > + compatible = "gpio-export"; >> > + #size-cells = <0>; >> > + >> > + usbpower { >> > + gpio-export,name = "usbpower"; >> > + gpio-export,output = <1>; >> > + gpios = <&gpio0 13 0>; >> > + }; >> > + }; >> > + >> >> This empty line here is unnecessary. >> >> > +}; >> > + >> >> Most (or maybe all) dts files in ramips target don't have empty lines at >> end. >> >> > diff --git a/target/linux/ramips/dts/HC5761.dts >> > b/target/linux/ramips/dts/HC5761.dts >> > new file mode 100644 >> > index 0000000..d271261 >> > --- /dev/null >> > +++ b/target/linux/ramips/dts/HC5761.dts >> > @@ -0,0 +1,172 @@ >> > +/dts-v1/; >> > + >> > +/include/ "mt7620a.dtsi" >> > + >> > +/ { >> > + compatible = "HC5761", "ralink,mt7620a-soc"; >> > + model = "HiWiFi HC5761"; >> > + >> > + chosen { >> > + bootargs = "console=ttyS0,115200"; >> > + }; >> > + >> > + palmbus@10000000 { >> > + sysc@0 { >> > + ralink,gpiomux = "i2c", "jtag"; >> > + ralink,uartmux = "gpio"; >> > + ralink,wdtmux = <1>; >> > + }; >> > + >> > + gpio0: gpio@600 { >> > + status = "okay"; >> > + }; >> > + >> > + gpio2: gpio@660 { >> > + status = "okay"; >> > + }; >> > + >> > + gpio3: gpio@688 { >> > + status = "okay"; >> > + }; >> > + >> > + spi@b00 { >> > + status = "okay"; >> > + >> > + m25p80@0 { >> > + #address-cells = <1>; >> > + #size-cells = <1>; >> > + compatible = "w25q128"; >> > + reg = <0 0>; >> > + linux,modalias = "m25p80", "w25q128"; >> > + spi-max-frequency = <10000000>; >> > + >> > + partition@0 { >> > + label = "u-boot"; >> > + reg = <0x0 0x30000>; >> > + }; >> > + >> > + partition@30000 { >> > + label = "u-boot-env"; >> > + reg = <0x30000 0x10000>; >> > + read-only; >> > + }; >> > + >> > + factory: partition@40000 { >> > + label = "factory"; >> > + reg = <0x40000 0x10000>; >> > + }; >> > + >> > + partition@50000 { >> > + label = "firmware"; >> > + reg = <0x50000 0xf80000>; >> > + }; >> > + >> > + partition@fd0000 { >> > + label = "hwf_config"; >> > + reg = <0xfd0000 0x10000>; >> > + }; >> > + >> > + bdinfo: partition@fe0000 { >> > + label = "bdinfo"; >> > + reg = <0xfe0000 0x10000>; >> > + }; >> > + >> > + partition@ff0000 { >> > + label = "backup"; >> > + reg = <0xff0000 0x10000>; >> > + }; >> > + }; >> > + }; >> > + }; >> > + >> > + ehci@101c0000 { >> > + status = "okay"; >> > + }; >> > + >> > + ohci@101c1000 { >> > + status = "okay"; >> > + }; >> > + >> > + ethernet@10100000 { >> > + pinctrl-names = "default"; >> > + pinctrl-0 = <&ephy_pins>; >> > + mtd-mac-address = <&factory 0x4>; >> > + ralink,port-map = "wllll"; >> > + }; >> > + >> > + sdhci@10130000 { >> > + status = "okay"; >> > + }; >> > + >> > + wmac@10180000 { >> > + ralink,mtd-eeprom = <&factory 0>; >> > + }; >> > + >> > + pcie@10140000 { >> > + status = "okay"; >> > + }; >> > + >> > + pinctrl { >> > + state_default: pinctrl0 { >> > + gpio { >> > + ralink,group = "uartf", "wled", "nd_sd"; >> > + ralink,function = "gpio"; >> > + }; >> > + >> > + pa { >> > + ralink,group = "pa"; >> > + ralink,function = "pa"; >> > + }; >> > + }; >> > + }; >> > + >> > + gpio-leds { >> > + compatible = "gpio-leds"; >> > + >> > + system { >> > + label = "hc5761:blue:system"; >> > + gpios = <&gpio0 9 1>; >> > + }; >> > + >> > + internet { >> > + label = "hc5761:blue:internet"; >> > + gpios = <&gpio0 11 1>; >> > + }; >> > + >> > + wlan2g { >> > + label = "hc5761:blue:wlan2g"; >> > + gpios = <&gpio3 0 1>; >> > + }; >> > + >> > + wlan5g { >> > + label = "hc5761:blue:wlan5g"; >> > + gpios = <&gpio0 7 1>; >> > + }; >> > + }; >> > + >> > + gpio-keys-polled { >> > + compatible = "gpio-keys-polled"; >> > + #address-cells = <1>; >> > + #size-cells = <0>; >> > + poll-interval = <20>; >> > + >> > + reset { >> > + label = "reset"; >> > + gpios = <&gpio0 12 1>; >> > + linux,code = <0x198>; >> > + }; >> > + }; >> > + >> > + gpio_export { >> > + compatible = "gpio-export"; >> > + #size-cells = <0>; >> > + >> > + usbpower { >> > + gpio-export,name = "usbpower"; >> > + gpio-export,output = <1>; >> > + gpios = <&gpio0 13 0>; >> > + }; >> > + }; >> > + >> >> Same here: this empty line here is unnecessary. >> >> > +}; >> > + >> >> And here. >> >> > diff --git a/target/linux/ramips/dts/HC5861.dts >> > b/target/linux/ramips/dts/HC5861.dts >> > new file mode 100644 >> > index 0000000..c0d9b93 >> > --- /dev/null >> > +++ b/target/linux/ramips/dts/HC5861.dts >> > @@ -0,0 +1,214 @@ >> > +/dts-v1/; >> > + >> > +/include/ "mt7620a.dtsi" >> > + >> > +/ { >> > + compatible = "HC5861", "ralink,mt7620a-soc"; >> > + model = "HiWiFi HC5861"; >> > + >> > + chosen { >> > + bootargs = "console=ttyS0,115200"; >> > + }; >> > + >> > + palmbus@10000000 { >> > + sysc@0 { >> > + ralink,gpiomux = "i2c", "jtag"; >> > + ralink,uartmux = "gpio"; >> > + ralink,wdtmux = <1>; >> > + }; >> > + >> > + gpio0: gpio@600 { >> > + status = "okay"; >> > + }; >> > + >> > + gpio2: gpio@660 { >> > + status = "okay"; >> > + }; >> > + >> > + gpio3: gpio@688 { >> > + status = "okay"; >> > + }; >> > + >> > + spi@b00 { >> > + status = "okay"; >> > + >> > + m25p80@0 { >> > + #address-cells = <1>; >> > + #size-cells = <1>; >> > + compatible = "w25q128"; >> > + reg = <0 0>; >> > + linux,modalias = "m25p80", "w25q128"; >> > + spi-max-frequency = <10000000>; >> > + >> > + partition@0 { >> > + label = "u-boot"; >> > + reg = <0x0 0x30000>; >> > + }; >> > + >> > + partition@30000 { >> > + label = "u-boot-env"; >> > + reg = <0x30000 0x10000>; >> > + read-only; >> > + }; >> > + >> > + factory: partition@40000 { >> > + label = "factory"; >> > + reg = <0x40000 0x10000>; >> > + }; >> > + >> > + partition@50000 { >> > + label = "firmware"; >> > + reg = <0x50000 0xf80000>; >> > + }; >> > + >> > + partition@fd0000 { >> > + label = "hwf_config"; >> > + reg = <0xfd0000 0x10000>; >> > + }; >> > + >> > + bdinfo: partition@fe0000 { >> > + label = "bdinfo"; >> > + reg = <0xfe0000 0x10000>; >> > + }; >> > + >> > + partition@ff0000 { >> > + label = "backup"; >> > + reg = <0xff0000 0x10000>; >> > + }; >> > + }; >> > + }; >> > + }; >> > + >> > + ehci@101c0000 { >> > + status = "okay"; >> > + }; >> > + >> > + ohci@101c1000 { >> > + status = "okay"; >> > + }; >> > + >> > + ethernet@10100000 { >> > + status = "okay"; >> > + mtd-mac-address = <&factory 0x4>; >> > + pinctrl-names = "default"; >> > + pinctrl-0 = <&rgmii1_pins &rgmii2_pins &mdio_pins>; >> > + ralink,port-map = "wllll"; >> > + >> > + port@4 { >> > + status = "okay"; >> > + phy-handle = <&phy4>; >> > + phy-mode = "rgmii"; >> > + }; >> > + >> > + port@5 { >> > + status = "okay"; >> > + phy-handle = <&phy5>; >> > + phy-mode = "rgmii"; >> > + }; >> > + >> > + mdio-bus { >> > + status = "okay"; >> > + >> > + phy4: ethernet-phy@4 { >> > + reg = <4>; >> > + phy-mode = "rgmii"; >> > + }; >> > + >> > + phy5: ethernet-phy@5 { >> > + reg = <5>; >> > + phy-mode = "rgmii"; >> > + }; >> > + }; >> > + }; >> > + >> > + gsw@10110000 { >> > + ralink,port4 = "gmac"; >> > + }; >> > + >> > + sdhci@10130000 { >> > + status = "okay"; >> > + }; >> > + >> > + wmac@10180000 { >> > + ralink,mtd-eeprom = <&factory 0>; >> > + }; >> > + >> > + pcie@10140000 { >> > + status = "okay"; >> > + }; >> > + >> > + pinctrl { >> > + state_default: pinctrl0 { >> > + gpio { >> > + ralink,group = "uartf", "wled", "nd_sd"; >> > + ralink,function = "gpio"; >> > + }; >> > + >> > + pa { >> > + ralink,group = "pa"; >> > + ralink,function = "pa"; >> > + }; >> > + }; >> > + }; >> > + >> > + gpio-leds { >> > + compatible = "gpio-leds"; >> > + >> > + system { >> > + label = "hc5861:blue:system"; >> > + gpios = <&gpio0 9 1>; >> > + }; >> > + >> > + wlan2g { >> > + label = "hc5861:blue:wlan2g"; >> > + gpios = <&gpio0 11 1>; >> > + }; >> > + >> > + internet { >> > + label = "hc5861:blue:internet"; >> > + gpios = <&gpio3 0 1>; >> > + }; >> > + >> > + wlan5g { >> > + label = "hc5861:blue:wlan5g"; >> > + gpios = <&gpio0 7 1>; >> > + }; >> > + >> > + turbo { >> > + label = "hc5861:blue:turbo"; >> > + gpios = <&gpio0 10 1>; >> > + }; >> > + }; >> > + >> > + gpio-keys-polled { >> > + compatible = "gpio-keys-polled"; >> > + #address-cells = <1>; >> > + #size-cells = <0>; >> > + poll-interval = <20>; >> > + >> > + reset { >> > + label = "reset"; >> > + gpios = <&gpio0 12 1>; >> > + linux,code = <0x198>; >> > + }; >> > + }; >> > + >> > + gpio_export { >> > + compatible = "gpio-export"; >> > + #size-cells = <0>; >> > + >> > + usbpower { >> > + gpio-export,name = "usbpower"; >> > + gpio-export,output = <0>; >> >> Other two models have here output = 1. >> Why this one is different? > > > Forget to say, I have no idea about it, I'm not familiar with hardware > related stuff :( it just comes from original author. > > If someone could confirm it is a typo, I'll fix it. > >> >> >> > + gpios = <&gpio0 13 0>; >> > + }; >> > + >> > + sdpower { >> > + gpio-export,name = "sdpower"; >> > + gpio-export,output = <0>; >> > + gpios = <&gpio0 8 0>; >> > + }; >> > + }; >> > + >> >> This empty line here is unnecessary. >> >> > +}; >> > + >> >> And last one here. >> >> > _______________________________________________ >> > openwrt-devel mailing list >> > openwrt-devel@lists.openwrt.org >> > https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel > > _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel