[OpenWrt-Devel] [BUG] lantiq: net: wrong operator

[Mathias Kresin]

08/02/2019 09:23, Petr Cvek:
> Hello,
> 
> There is a wrong code in 0025-NET-MIPS-lantiq-adds-xrx200-net.patch [1], the original code:
> 
> +	link->rx_flow = !!(xrx200sw_read_x(XRX200_MAC_CTRL_0_FCON, port) && 0x0010);
> +	link->tx_flow = !!(xrx200sw_read_x(XRX200_MAC_CTRL_0_FCON, port) && 0x0020);
> 
> wants to mask the register value and is using a logical AND and not a bitwise AND.
> 
> The fix should be:
> 
> +	link->rx_flow = !!(xrx200sw_read_x(XRX200_MAC_CTRL_0_FCON, port) & 0x0010);
> +	link->tx_flow = !!(xrx200sw_read_x(XRX200_MAC_CTRL_0_FCON, port) & 0x0020);
> 
> References:
> [1] https://github.com/openwrt/openwrt/blob/master/target/linux/lantiq/patches-4.14/0025-NET-MIPS-lantiq-adds-xrx200-net.patch#L937
> 
>

Hey Petr,

it looks indeed wrong. And looking more closer at the lines, I don't get 
why we need the double negation.

Out of curiosity, do you noticed the typo during some tests or by 
reviewing the code? I'm not able to locale any code which writes the 
XRX200_MAC_CTRL_0_FCON register and would allow to change the flow 
control settings.

Would you mind to send a patch to fix the typo? Maybe we can make this 
part of the code a bit more obvious by changing it to something like:

link->rx_flow = xrx200sw_read_x(XRX200_MAC_CTRL_0_FCON, port) & BIT(4);
link->tx_flow = xrx200sw_read_x(XRX200_MAC_CTRL_0_FCON, port) & BIT(5);

The autoneg source look suspicious as well. I rather would suspect it is 
something related to flow control rather than the link speed 
autonegotiation. But might be I got the code wrong. In the end it is all 
guessing, since I don't have a datasheet for the switch.

Mathias

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel