netdev - RE: [PATCH net] gianfar: Don't force RGMII mode after reset, use defaults

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <VI1PR04MB4880A48175A5FE0F08AB7B2196760@VI1PR04MB4880.eurprd04.prod.outlook.com>
Date:   Wed, 13 Nov 2019 14:00:05 +0000
From:   Claudiu Manoil <claudiu.manoil@....com>
To:     "HEMANT RAMDASI (hramdasi)" <hramdasi@...co.com>,
        "Daniel Walker (danielwa)" <danielwa@...co.com>
CC:     "David S . Miller" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "Sathish Jarugumalli -X (sjarugum - ARICENT TECHNOLOGIES HOLDINGS
        LIMITED at Cisco)" <sjarugum@...co.com>
Subject: RE: [PATCH net] gianfar: Don't force RGMII mode after reset, use
 defaults

>-----Original Message-----
>From: HEMANT RAMDASI (hramdasi) <hramdasi@...co.com>
[...]
>> This bit must be set when in half-duplex mode (MACCFG2[Full_Duplex] is cleared).
>
> Should the bit be clear when in full duplex or it does not matter?
>

From my tests, in full duplex mode small frames won't get padded if this bit is disabled,
and will be counted as undersize frames and dropped. So this bit needs to be set
in full duplex mode to get packets smaller than 64B past the MAC (w/o software padding).
The statement above likely means that for half-duplex mode packets cannot egress
the MAC regardless of their size if the PAD_CRC bit is not set.  At least this is consistent
with my experiments.

>> 0 Frames presented to the MAC have a valid length and contain a CRC.
>> 1 The MAC pads all transmitted short frames and appends a CRC to every frame
>> regardless of padding requirement."
>>
>> So the driver sets this bit to have small frames padded. It always worked
>> this way, and I retested on P2020RDB and LS1021RDB and works.
>> Are you saying that padding does not work on your board with the current
>> upstream code?
>
>It works but the settings does not match with what's mentioned in p2020 rm
>and the bit 29 becomes redundant.
>

So, the PAD_CRC bit is not redundant, and for half-duplex mode it looks like this bit is
even mandatory to have Tx traffic at all.

This patch is however not about the PAD_CRC bit. It's about default interface mode
setting in the MACCFG2 register.  And I just noticed to my surprise that with the default
reset value for interface mode (00b) the SGMII link won't get up on my P2020RDB-PC
board, while the RGMII links don't have this problem.  On the newer LS1021ATWR board
there's no such issue (both sgmii and rgmii links work with default IF_Mode of 00).
So looks like IF_Mode was being initialized to "byte mode" (10b, aka RGMII 1G mode)
with a reason, so that older boards have functional links in all cases (i.e. sgmii).

Better to drop this patch for now to avoid such regressions for older boards.  This is what
happens when legacy driver code like this gets changed.  I'll have to ask h/w ppl for
clarification on this.

Thanks,
Claudiu