linux-kernel - Re: [RFC PATCH 2/2] net: selpcimac: Add driver for SEL PCIe network adapter

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:
 <PH0PR22MB3809723DDAE5431A0723B911E54B2@PH0PR22MB3809.namprd22.prod.outlook.com>
Date: Tue, 29 Oct 2024 22:51:46 +0000
From: Robert Joslyn <Robert_Joslyn@...inc.com>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "lee@...nel.org"
	<lee@...nel.org>
Subject: Re: [RFC PATCH 2/2] net: selpcimac: Add driver for SEL PCIe network
 adapter

> On Mon, 28 Oct 2024 15:35:08 -0700
> Robert Joslyn <robert_joslyn@...inc.com> wrote:
>
> > Add support for SEL FPGA based network adapters. The network device is
> > implemented as an FPGA IP core and enumerated by the selpvmf driver.
> > This is used on multiple devices, including:
> >  - SEL-3350 mainboard
> >  - SEL-3390E4 card
> >  - SEL-3390T card
>
> Make sure that you get the right people as recipients for this
> patchset. You would need at least the net maintainers, make sure to use
> the scripts/get_maintainers.pl tool to know who to send the patch to.

Thanks for pointing out that script, I'll use it next time.

> I haven't reviewed the code itself as this is a biiiiig patch, I
> suggest you try to split it into more digestable patches, focusing on
> individual aspects of the driver.

I'll work on splitting the patch up.

> One thing is the PHY support as you mention in the cover-letter, in the
> current state this driver re-implements PHY drivers from what I
> understand. You definitely need to use the kernel infra for PHY
> handling.
>
> As it seems this driver also re-implements SFP entirely, I suggest you
> look into phylink [1]. This will help you supporting the PHYs and SFPs.
> You can take a look at the mvneta.c and mvpp2 drivers for examples.
>
> Make sure you handle the mdio bus control using the dedicated framework
> (see mii_bus et al.).

Yes, the driver does it's own internal PHY handling for the 4 different PHYs we use. SFPs are also all done within this driver.

> I'd be happy to give you more reviews, but this would be a more
> manageable task with smaller patches :)
>
>Best regards,
>
>Maxime

Any review you can provide would be appreciated, but probably more useful after I rework this to use phylib and phylink. Then hopefully it's easier to review with smaller patches and using normal kernel frameworks rather than our custom ones.

Thanks for the help!

Robert