Subject: Re: [PATCH net-next 0/6] net: dsa: always use phylink

From: "Russell King (Oracle)" <[email-masked]>
Date: Sat, 16 Jul 2022 12:43:00 +0100

On Sat, Jul 16, 2022 at 02:15:51PM +0300, Vladimir Oltean wrote:
> On Fri, Jul 15, 2022 at 04:03:59PM -0700, Jakub Kicinski wrote:
> > On Fri, 15 Jul 2022 21:59:24 +0100 Russell King (Oracle) wrote:
> > > The only thing that delayed them was your eventual comments about
> > > re-working how it was being done. Yet again, posting the RFC series
> > > created very little in the way of feedback. I'm getting to the point
> > > of thinking its a waste of time posting RFC patches - it's counter
> > > productive. RFC means "request for comments" but it seems that many
> > > interpret it as "I can ignore it".
> > 
> > I'm afraid you are correct. Dave used to occasionally apply RFC patches
> > which kept reviewers on their toes a little bit (it kept me for sure).
> > These days patchwork automatically marks patches as RFC based on
> > the subject, tossing them out of "Action required" queue. So they are
> > extremely easy to ignore.
> > 
> > Perhaps an alternative way of posting would be to write "RFC only,
> > please don't apply" at the end of the cover letter. Maybe folks will 
> > at least get thru reading the cover letter then :S
> 
> Again, expressing complaints to me for responding late is misdirected
> frustration. The fact that I chose to leave my comments only when
> Russell gave up on waiting for feedback from Andrew doesn't mean I
> ignored his RFC patches, it just means I didn't want to add noise and
> ask for minor changes when it wasn't clear that this is the overall
> final direction that the series would follow. I still have preferences
> about the way in which this patch set gets accepted, and now seems like
> the proper moment to express them.

In the first RFC series I sent on the 24 June, I explicitly asked the
following questions:

Obvious questions:
1. Should phylink_get_caps() be augmented in this way, or should it be
   a separate method?

2. DSA has traditionally used "interface mode for the maximum supported
   speed on this port" where the interface mode is programmable (via
   its internal port_max_speed_mode() method) but this is only present
   for a few of the sub-drivers. Is reporting the current interface
   mode correct where this method is not implemented?

Obvious questions:
1. Should we be allowing half-duplex for this?
2. If we do allow half-duplex, should we prefer fastest speed over
   duplex setting, or should we prefer fastest full-duplex speed
   over any half-duplex?
3. How do we sanely switch DSA from its current behaviour to always
   using phylink for these ports without breakage - this is the
   difficult one, because it's not obvious which drivers have been
   coded to either work around this quirk of the DSA implementation.
   For example, if we start forcing the link down before calling
   dsa_port_phylink_create(), and we then fail to set max-fixed-link,
   then the CPU/DSA port is going to fail, and we're going to have
   lots of regressions.

I even stated: "Please look at the patches and make suggestions on how
we can proceed to clean up this quirk of DSA." and made no mention of
wanting something explicitly from Andrew.

Yet, none of those questions were answered.

So no, Jakub's comments are *not* misdirected at all. Go back and read
my June 24th RFC series yourself:

https://lore.kernel.org/all/[email-masked]/

I've *tried* my best to be kind and collaborative, but I've been
ignored. Now I'm hacked off. This could have been avoided by responding
to my explicit questions sooner, rather than at the -rc6/-rc7 stage of
the show.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!