[RFC PATCH v2 3/6] ath79: prepare for 1-port TP-Link EAP2x5 devices

Sander Vanheule sander at svanheule.net
Mon Jul 20 12:27:52 EDT 2020


Hi Adrian,

On Mon, 2020-07-20 at 00:25 +0200, mail at adrianschmutzler.de wrote:
> Hi,
> 
> > Signed-off-by: Julien Dusser <julien.dusser at free.fr>
> > Signed-off-by: Sander Vanheule <sander at svanheule.net>
> 
> 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




More information about the openwrt-devel mailing list