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

[Sander Vanheule]

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