Hi Adrian, On Mon, 2020-07-20 at 00:25 +0200, [email protected] wrote: > Hi, > > > Signed-off-by: Julien Dusser <[email protected]> > > Signed-off-by: Sander Vanheule <[email protected]> > > technically, if Julien is first SoB, you should also put him in the > author (From:) field.
The DTSI is derived from Julien's DTS for the EAP245 v1 [1]. By now I've made so many modifications, that I don't think Julien should be the person responsible for this file any more. But I would still like to credit his work. I'll swap the SoB order to reflect this. [1] https://github.com/j-d-r/openwrt/blob/ae7dc9bf871cd9f27ccc1f4bff335ab5e79bcae9/target/linux/ath79/dts/qca9563_tplink_eap245-v1.dts > > --- > > .../dts/qca9563_tplink_eap2x5_1port.dtsi | 139 > > ++++++++++++++++++ > > target/linux/ath79/image/generic-tp-link.mk | 10 ++ > > 2 files changed, 149 insertions(+) > > create mode 100644 > > target/linux/ath79/dts/qca9563_tplink_eap2x5_1port.dtsi > > > > diff --git > > a/target/linux/ath79/dts/qca9563_tplink_eap2x5_1port.dtsi > > b/target/linux/ath79/dts/qca9563_tplink_eap2x5_1port.dtsi > > new file mode 100644 > > index 0000000000..24f0b4f0ce > > --- /dev/null > > +++ b/target/linux/ath79/dts/qca9563_tplink_eap2x5_1port.dtsi > > @@ -0,0 +1,139 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT #include > > Not really important, but I'd prefer an empty line after the license. > (Broken in my mail anyway, but I looked at it in patchwork.) Fixed. > > +&pinmux { > > + mdio_pins: pinux_mdio_pins { > > Typo for the node name (pinux). Despite, including pinmux for the > node name actually doesn't make much sense, as it's part of &pinmux > anyway. > If you want it, it would make sense to add it to the DT label, as > this one is actually used in global context. > > I'd be fine with "mdio_pins: mdio_pins" as well, though, as "pins" > already tells whats going on. I went with your suggestion and changed it to "mdio_pins: mdio_pins". > > + /* GPIO 8 as MDC(0x21), GPIO 10 as MDIO(0x20) */ > > + pinctrl-single,bits = <0x8 0x00000021 0x000000ff>, > > + <0x8 0x00200000 0x00ff0000>; > > Err, is there a specific reason why you don't use: > > pinctrl-single,bits = <0x8 0x00200021 0x00ff00ff>; > > Or is the second offset supposed to 0xc or something? The lines are correct and entirely as intended. But I just didn't think about merging the two lines... *facepalm* Merged, and swapped the order in the comment to 10/8 to have the same order as in the specified bits. > > diff --git a/target/linux/ath79/image/generic-tp-link.mk > > b/target/linux/ath79/image/generic-tp-link.mk > > index 8a26e4bebe..d2cc8d09bd 100644 > > --- a/target/linux/ath79/image/generic-tp-link.mk > > +++ b/target/linux/ath79/image/generic-tp-link.mk > > @@ -362,6 +362,16 @@ define Device/tplink_cpe610-v2 endef > > TARGET_DEVICES += tplink_cpe610-v2 > > > > +define Device/tplink_eap2x5_1port > > + $(Device/tplink-safeloader) > > + SOC := qca9563 > > + IMAGE_SIZE := 15104k > > + LOADER_TYPE := elf > > + KERNEL := kernel-bin | append-dtb | lzma | loader-kernel > > + KERNEL_INITRAMFS := $$(KERNEL) > > + IMAGE/factory.bin := append-rootfs | tplink-safeloader factory | > > +pad-extra 128 endef > > + > > define Device/tplink_eap245-v3 > > This packaging of patches into different projects really makes it > hard to review. > > But anyway, limiting the device definition to 1port doesn't make much > sense conceptually (in contrast to the DTS, were it is useful). > > I'd go for tplink_eap2x5 here and already use it for the eap245-v3 in > this patch. > > Only IMAGE_SIZE would need to be moved to the devices then, and > that's better anyway. > Sorry for making this harder than it should be. I didn't want to duplicate the patches already on GitHub, and is one of the reasons I submitted this to the list as an RFC, without the complete patch set. If it's okay for you (and ynezz) I will add these patches to the EAP245 v3 pull request for the review (and change the topic and cover letter accordingly, merge the duplicated makefile code). Best, Sander _______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/mailman/listinfo/openwrt-devel
