netdev - Re: [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer

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: <YfQ8De5OMLDLKF6g@asheplyakov-rocket>
Date:   Fri, 28 Jan 2022 22:55:09 +0400
From:   Alexey Sheplyakov <asheplyakov@...ealt.ru>
To:     Serge Semin <Sergey.Semin@...kalelectronics.ru>
Cc:     netdev@...r.kernel.org,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Jose Abreu <joabreu@...opsys.com>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@...kalelectronics.ru>,
        Serge Semin <fancer.lancer@...il.com>,
        Evgeny Sinelnikov <sin@...ealt.ru>
Subject: Re: [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer

Hi, Serge,

On Fri, Jan 28, 2022 at 06:06:42PM +0300, Serge Semin wrote:
> Hello Alexey and network folks
> 
> First of all thanks for sharing this patchset with the community. The
> changes indeed provide a limited support for the DW GMAC embedded into
> the Baikal-T1/M1 SoCs. But the problem is that they don't cover all
> the IP-blocks/Platform-setup peculiarities

In general quite a number of Linux drivers (GPUs, WiFi chips, foreign
filesystems, you name it) provide a limited support for the corresponding
hardware (filesystem, protocol, etc) and don't cover all peculiarities.
Yet having such a limited support in the mainline kernel is much more
useful than no support at all (or having to use out-of-tree drivers,
obosolete vendor kernels, binary blobs, etc).

Therefore "does not cover all peculiarities" does not sound like a valid
reason for rejecting this driver. That said it's definitely up to stmmac
maintainers to decide if the code meets the quality standards, does not
cause excessive maintanence burden, etc.

> (believe me there are more
> than just 2*Tx-clock and embedded GPIO features), moreover giving a
> false impression of a full and stable Baikal-T1/M1 GMAC interface
> support.

Such an impression is not intended. Perhaps the commit message should
be improved. What about this:

8<---

net: stmmac: initial support of Baikal-T1/M SoCs GMAC

The gigabit Ethernet controller available in Baikal-T1 and Baikal-M
SoCs is a Synopsys DesignWare MAC IP core, already supported by
the stmmac driver.

This patch implements some SoC specific operations (DMA reset and
speed fixup) necessary (but in general not enough) for Baikal-T1/M
variants.

Note that this driver does NOT cover all the IP-blocks/Platform-setup
peculiarities. It's known to work just fine on some Baikal-T1 boards
(including BFK3.1 reference board) and some Baikal-M based boards
(TF307 revision D, LGP-16), however it might or might not work with
other boards.

8<---

> There are good reasons why we haven't submitted the GMAC/xGBE
> drivers so far. I've been working on the STMMAC code refactoring for
> more than six months now so the driver would be better structured and
> would support all of the required features including the DW XGMAC
> interface embedded into the SoCs. So please don't rush with this
> patchset including into the kernel. We are going to submit a more
> comprehensive and thoroughly structured series of patchsets including
> a bunch of STMMAC driver Fixes very soon. After that everyone will be
> happy ;)

Don't get me wrong, but I've heard the very same thing back in 2020.
It's 2022 now. So I decided it's time to mainline this primitive driver
(which is definitely far from perfect) so people can use the mainline
kernel on (some of) their Baikal-M/T1 based boards.

And this simple driver can be easily removed/replaced if/when a more
advanced version is ready.

> Also, Alexey, next time you submit something Baikal-related could you
> please Cc someone from our team?

Sure. Hopefully I'll get some useful feedback (that is, other than
"don't bother, use the kernel from SDK", or "we are working on it,
please wait").

> (I am sure you know Alexey' email or
> have seen my patches in the mailing lists.) Dmitry Dunaev hasn't been
> working for Baikal Electronics for more than four years now so his
> email address is disabled (you must have already noticed that by
> getting a bounce back email). Moreover you can't add someone'
> signed-off tag without getting a permission from one.

Yep. Hence the question: what is the proper way to mention that the code
I post is based on work of other people (if those people ignore my emails,
or or their addresses are not valid any more, etc)?

Best regards,
	Alexey