Message-ID: <20220207152213.folasobthqadj6f2@mobilestation>
Date:   Mon, 7 Feb 2022 18:22:13 +0300
From:   Serge Semin <Sergey.Semin@...kalelectronics.ru>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     Serge Semin <fancer.lancer@...il.com>,
        Alexey Sheplyakov <asheplyakov@...ealt.ru>,
        <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>,
        Evgeny Sinelnikov <sin@...ealt.ru>
Subject: Re: [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer

On Tue, Feb 01, 2022 at 11:08:38AM -0800, Jakub Kicinski wrote:
> On Tue, 1 Feb 2022 18:54:39 +0300 Serge Semin wrote:
> > On Fri, Jan 28, 2022 at 12:27:18PM -0800, Jakub Kicinski wrote:
> > > On Fri, 28 Jan 2022 22:55:09 +0400 Alexey Sheplyakov wrote:  
> > > > 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.  
> > > 
> > > Sounds sensible, Serge please take a look at the v2 and let us know if
> > > there are any bugs in there. Or any differences in DT bindings or user
> > > visible behaviors with what you're planning to do. If the driver is
> > > functional and useful it can evolve and gain support for features and
> > > platforms over time.  
> > 
> > I've already posted my comments in this thread regarding the main
> > problematic issues of the driver, but Alexey for some reason ignored
> > them (dropped from his reply). Do you want me to copy my comments to
> > v2 and to proceed with review there?
> 
> Right, on a closer look there are indeed comments you raised that were
> not addressed and not constrained to future compatibility. 
> 
> Alexey, please take another look at those and provide a changelog in
> your next posting so we can easily check what was addressed.
> 
> > Regarding the DT-bindings and the user-visible behavior. Right, I'll
> > add my comments in this matter. Thanks for suggesting. This was one of
> > the problems why I was against the driver integrating into the kernel.
> > One of our patchset brings a better organization to the current
> > DT-bindings of the Synopsys DW *MAC devices. In particular it splits
> > up the generic bindings for the vendor-specific MACs to use and the
> > bindings for the pure DW MAC compatible devices. In addition the
> > patchset will add the generic Tx/Rx clocks DT-bindings and the
> > DT-bindings for the AXI/MTL nodes. All of that and the rest of our
> > work will be posted a bit later as a set of the incremental patchsets
> > with small changes, one by one, for an easier review. We just need
> > some more time to finish the left of the work. The reason why the
> > already developed patches hasn't been delivered yet is that the rest
> > of the work may cause adding changes into the previous patches. In
> > order to decrease a number of the patches to review and present a
> > complete work for the community, we decided to post the patchsets
> > after the work is fully done.
> 

> TBH starting to post stuff is probably best choice you can make,
> for example the DT rework you mention sounds like a refactoring 
> you can perform without posting any Baikal support.

I wouldn't say that the DT-part is a refactoring. Since it's
DT-bindings I can't change it' interface much (as I'd like to for
instance in the snps,axi-config or mtp-{tx,rx}-config sub-nodes
bindings). At the very least I can't make them incompatible with
already defined DT-interface. So that was more like an optimization
with updating the Yaml-schemas to be more generic for all the DW MACs:
MAC100, GMAC, EQOS and xGMAC.

Regarding submitting the stuff now and delivering the updates
afterwards. Thanks for suggesting. I thought about that at first, but
then changed my mind. The thing is that I am still in progress of the
STMMAC-related development. Even though a few more tasks left to be
done, they will concerns the bindings change. In addition working on
review comments will distract me from the rest of the STMMAC tasks. So
I'd rather finish all what we've planned first and then start sending
the series one after another with well structured cover letters. Thus
the community will have a coherent set of sets and less patches for
review, while I'll have less gray hairs due to deadlines and
distractions.)

-Sergey

Powered by blists - more mailing lists