[OpenWrt-Devel] [PATCH v2 2/3] ramips: Get ethernet ports to be disabled from the device tree.
Vittorio G (VittGam)
openwrt at vittgam.net
Sat Dec 12 02:07:59 EST 2015
On 12/12/2015 07:20:43 CET, John Crispin wrote:
> On 12/12/2015 07:19, John Crispin wrote:
>> why a semicolon ? i believe a "," needs to be added instead please also
>> remove the superfluous "," while you are at it to make the comment
>> orthographically correct. it seems to be have been copied from SDK code.
> you already removed the extra "," sorry i missed that
Ok. Would "Enable all ports, Back Pressure and Flow Control" be okay?
> > - /* Copy disabled port configuration from bootloader setup */
> > - port_disable = esw_get_port_disable(esw);
> > + /* Copy disabled port configuration from device tree setup */
> > + if (esw->port_disable)
> > + port_disable = esw->port_disable;
> this changes the behavior of the code and will most likely break several
Why? In the previous code we're going to enable all ports anyway by clearing
all of the port disabled bits, so esw_get_port_disable would have returned
zero; and now port_disable is 0 by default if it isn't set in the device tree.
The only case where esw_get_port_disable would have returned a value different
from zero is when port 5 is not implemented in the SoC, but the port power
down logic that follows is not going to handle port 5 anyway. Furthermore,
user space reporting of the disabled status (eg. swconfig dev switch0 show)
is read again from the chip at every report request, so port 5 will show as
disabled correctly for SoCs where it is not implemented like RT5350.
Anyway, if you think that an "else port_disable = esw_get_port_disable(esw);"
would be better anyway, I can add it. (But I think it'd be superfluous...
Unless there's some SoC that behaves differently than this.)
> port_map is named as such as it is a port_map and not a port_disable. we
> already concluded yesterday that this is a bad code style.
It's the mapping of the disabled ports... But okay, I'll change every variable
to be "tmp" instead this time.
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
More information about the openwrt-devel