[AMD Official Use Only - AMD Internal Distribution Only] > -----Original Message----- > From: Gupta, Suraj > Sent: Thursday, March 13, 2025 9:01 AM > To: Andrew Lunn <andrew@xxxxxxx>; Russell King (Oracle) > <linux@xxxxxxxxxxxxxxx> > Cc: Pandey, Radhey Shyam <radhey.shyam.pandey@xxxxxxx>; > andrew+netdev@xxxxxxx; davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; > kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx; robh@xxxxxxxxxx; krzk+dt@xxxxxxxxxx; > conor+dt@xxxxxxxxxx; Simek, Michal <michal.simek@xxxxxxx>; > netdev@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; git (AMD-Xilinx) <git@xxxxxxx>; Katakam, > Harini <harini.katakam@xxxxxxx> > Subject: RE: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only > configuration. > > > > > -----Original Message----- > > From: Andrew Lunn <andrew@xxxxxxx> > > Sent: Thursday, March 13, 2025 3:41 AM > > To: Russell King (Oracle) <linux@xxxxxxxxxxxxxxx> > > Cc: Gupta, Suraj <Suraj.Gupta2@xxxxxxx>; Pandey, Radhey Shyam > > <radhey.shyam.pandey@xxxxxxx>; andrew+netdev@xxxxxxx; > > davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; > > pabeni@xxxxxxxxxx; robh@xxxxxxxxxx; krzk+dt@xxxxxxxxxx; > > conor+dt@xxxxxxxxxx; Simek, Michal <michal.simek@xxxxxxx>; > > netdev@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > > linux-kernel@xxxxxxxxxxxxxxx; linux-arm- kernel@xxxxxxxxxxxxxxxxxxx; > > git (AMD-Xilinx) <git@xxxxxxx>; Katakam, Harini > > <harini.katakam@xxxxxxx> > > Subject: Re: [PATCH net-next V2 2/2] net: axienet: Add support for > > 2500base-X only configuration. > > > > Caution: This message originated from an External Source. Use proper > > caution when opening attachments, clicking links, or responding. > > > > > > > This is not an approach that works with the Linux kernel, sorry. > > > > > > What we have today is a driver that works for people's hardware - > > > and we don't know what the capabilities of that hardware is. > > > > > > If there's hardware out there today which has XAE_ABILITY_2_5G set, > > > but defaults to <=1G mode, this will work with the current driver. > > > However, with your patch applied, it stops working because instead > > > of the driver indicating MAC_10FD | MAC_100FD | MAC_1000FD, it only > > > indicates MAC_2500FD. If this happens, it will regress users setups, > > > and that is something we try not to do. > > > > > > Saying "someone else needs to add the code for their FPGA logic" > > > misses the point - there may not be "someone else" to do that, which > > > means the only option is to revert your change if it were merged. > > > That in itself can cause its own user regressions because obviously > > > stuff that works with this patch stops working. > > > > > > This is why we're being cautious, and given your responses, it's not > > > making Andrew or myself feel that there's a reasonable approach > > > being taken here. > > > > > > >From everything you have said, I am getting the feeling that using > > > XAE_ABILITY_2_5G to decide which of (1) or (2) is supported is just > > > wrong. Given that we're talking about an implementation that has > > > been synthesized at 2.5G and can't operate slower, maybe there's > > > some way that could be created to specify that in DT? > > > > > > e.g. (and I'm sure the DT folk aren't going to like it)... > > > > > > xlnx,axi-ethernet-X.YY.Z-2.5G > > > > > > (where X.YY.Z is the version) for implementations that can _only_ do > > > 2.5G, and leave all other implementations only doing 1G and below. > > > > > > Or maybe some DT property. Or something else. > > > > Given that AMD has been talking about an FPGA, not silicon, i actually > > think it would be best to change the IP to explicitly enumerate how it > > has been synthesised. Make use of some register bits which currently > > read as 0. Current IP would then remain as 1000BaseX/SGMII, > > independent of how they have been synthesised. Newer versions of the > > IP will then set the bits if they have been synthesised as 2) or 3), > > and the driver can then enable that capability, without breaking > > current generation systems. Plus there needs to be big fat warning for > > anybody upgrading to the latest version of the IP for bug fixes to ensure they > correctly set the synthesis options because it now actually matters. > > > > Andrew > > Synthesis options I mentioned in comment might sound confusing, let me clear it up. > Actual synthesis options (as seen from configuration UI) IP provides are (1) and (2). > When a user selects (2), IP comes with default 2.5G but also contains 1G > capabilities which can be enabled and work with by adding switching FPGA logic > (that makes it (3)). > > So, in short if a user selects (1): It's <=1G only. > If it selects (2): It's 2.5G only but can be made (3) by FPGA logic changes. So > whatever existing systems for (3) would be working at default (2). > > This is the reason we didn't described (3) in V1 series as that is not provided by IP > but can be synthesized after FPGA changes. > Hope I'm able to answer your questions. > I understand your concerns that current solution might break if any existing system uses (3). Russel's suggestion to use DT compatible we can try to send as RFC and check if that is accepted by DT maintainers. Andrew's suggestion is complete IP solution and will involve IP changes to correct ability register behaviour based on synthesis time selection in a new IP version. But this will need internal discussions and IP rework and might take few months. Please let me know your thoughts on it.
