[RFC PATCH 4/7] ath79: prepare for 1-port TP-Link EAP2x5 devices

[Sander Vanheule]

On Fri, 2020-07-17 at 14:49 +0200, mail at adrianschmutzler.de wrote:
> Hi,

Hi Adrian,

Thanks for the quick feedback.

> > 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..1b55bb855c
> > --- /dev/null
> > +++ b/target/linux/ath79/dts/qca9563_tplink_eap2x5_1port.dtsi
> > @@ -0,0 +1,174 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT /dts-v1/;
> 
> dts-v1 should only be defined once, please remove it in the DTSI and
> keep it in the DTS (so we ensure we have it once and always at
> beginning of the DTS).
> 

Fixed.

> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/input/input.h>
> > +
> > +#include "qca956x.dtsi"
> > +
> > +/ {
> > +	aliases {
> > +		label-device-mac = &eth0;
> 
> label-mac-device?
> 

Right, fixed.

> > +		led-boot = &led_status_green;
> > +		led-failsafe = &led_status_amber;
> > +		led-running = &led_status_green;
> > +		led-upgrade = &led_status_amber;
> > +	};
> > +
> > +	leds {
> > +		compatible = "gpio-leds";
> > +
> > +		led_status_green: status_green {
> > +			status = "disabled";
> > +			label = "tp-link:green:status";
> > +			default-state = "on";
> > +		};
> > +
> > +		led_status_amber: status_amber {
> > +			status = "disabled";
> > +			label = "tp-link:amber:status";
> > +		};
> > +
> > +		led_status_red: status_red {
> > +			status = "disabled";
> > +			label = "tp-link:red:status";
> > +		};
> > +	};
> 
> Since what you add in the DTS files is not much less than what you
> have here, I'd prefer to just put the entire led node into the DTS
> file.
> It will be much easier to read and you won't need that status lines
> at all.
> 

I put this in in the DTSI with the status-lines so I could also
reference the led labels (once) in the DTSI. I've moved the led-*
aliases down to the DTSes too, together with /leds, to keep them
together.

> > +
> > +&gpio {
> > +	status = "okay";
> > +};
> This block can be dropped, gpio is not disabled in qca956x.dtsi

Dropped. You mentioned this in my eap245-v3 pull request too, must've
missed it here.

> > +
> > +&spi {
> > +	status = "okay";
> > +	num-cs = <1>;
> > +
> > +	flash at 0 {
> > +		compatible = "jedec,spi-nor";
> > +		reg = <0>;
> > +		spi-max-frequency = <25000000>;
> 
> Can this go faster?

25MHz is the (default) value used in TP-Link's GPL sources for all
three devices in this patch series. Note that these device all have
different PCBs, so what may work on one board, might not work on
another.

The SoC datasheet [1] confuses me. §8.21.2 'SPI address control' states
that the maximum SPI clock speed is f_AHB/2, or 129MHz in this case.
§10.3 'SPI Timing' on the other hand, quotes a minimum period of 40ns
or 25MHz maximum frequency, far below the f_AHB/2 value.

The four device boot logs I've seen up to now detect the gd25q128 chip,
so I'll assume this is what they use on all their devices. Datasheet of
that flash chip [2] lists 80MHz as maximum frequency for QSPI
operations.

I tried 80MHz and 65MHz, but that didn't result in proper reads. 50MHz,
55MHz, and 60MHz do appear to work on my eap245-v1, so I really
wouldn't risk going above 50MHz.

[1] https://github.com/Deoptim/atheros
[2] 
https://www.semiconductorstore.com/pages/asp/DownloadDirect.asp?sid=1594989703348


Best,
Sander