On 12/12/2015 08:07, Vittorio G (VittGam) wrote: > Hi, > > 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?
i believe that would be correct punctuation > >> > - /* 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 >> board. > > 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.) > tl;dr >> 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. > if you want this merged you will need to add a port_disabled variable > Cheers, > Vittorio G > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
