[PATCH net v4 0/7] net: dsa: mt7530: fix multiple CPU ports, BPDU and LLDP handling

arinc9.unal@gmail.com posted 7 patches 1 year, 3 months ago
There is a newer version of this series
MAINTAINERS              |  5 +--
drivers/net/dsa/mt7530.c | 75 ++++++++++++++++++++++++++++++++++++-------
drivers/net/dsa/mt7530.h | 26 +++++++++------
include/net/dsa.h        |  8 +++++
net/dsa/dsa.c            | 24 +++++++++++++-
5 files changed, 115 insertions(+), 23 deletions(-)
[PATCH net v4 0/7] net: dsa: mt7530: fix multiple CPU ports, BPDU and LLDP handling
Posted by arinc9.unal@gmail.com 1 year, 3 months ago
Hi.

This patch series fixes hopefully all issues regarding multiple CPU ports
and the handling of LLDP frames and BPDUs.

I am adding me as a maintainer, I've got some code improvements on the way.
I will keep an eye on this driver and the patches submitted for it in the
future.

Arınç

v4: Make the patch logs and my comments in the code easier to understand.
v3: Fix the from header on the patches. Write a cover letter.
v2: Add patches to fix the handling of LLDP frames and BPDUs.

Arınç ÜNAL (7):
  net: dsa: mt7530: fix trapping frames with multiple CPU ports on MT7531
  net: dsa: mt7530: fix trapping frames with multiple CPU ports on MT7530
  net: dsa: mt7530: fix trapping frames on non-MT7621 SoC MT7530 switch
  net: dsa: mt7530: fix handling of BPDUs on MT7530 switch
  net: dsa: mt7530: fix handling of LLDP frames
  net: dsa: introduce preferred_default_local_cpu_port and use on MT7530
  MAINTAINERS: add me as maintainer of MEDIATEK SWITCH DRIVER

 MAINTAINERS              |  5 +--
 drivers/net/dsa/mt7530.c | 75 ++++++++++++++++++++++++++++++++++++-------
 drivers/net/dsa/mt7530.h | 26 +++++++++------
 include/net/dsa.h        |  8 +++++
 net/dsa/dsa.c            | 24 +++++++++++++-
 5 files changed, 115 insertions(+), 23 deletions(-)


Re: [PATCH net v4 0/7] net: dsa: mt7530: fix multiple CPU ports, BPDU and LLDP handling
Posted by Russell King (Oracle) 1 year, 3 months ago
Hi,

Please slow down your rate of patch submission - I haven't had a chance
to review the other patches yet (and I suspect no one else has.) Always
allow a bit of time for discussion.

Just because you receive one comment doesn't mean you need to rush to
get a new series out. Give it at least a few days because there may be
further discussion of the points raised.

Sending new versions quickly after previous comments significantly
increases reviewer workload.

Thanks.

On Mon, Jun 12, 2023 at 10:59:38AM +0300, arinc9.unal@gmail.com wrote:
> Hi.
> 
> This patch series fixes hopefully all issues regarding multiple CPU ports
> and the handling of LLDP frames and BPDUs.
> 
> I am adding me as a maintainer, I've got some code improvements on the way.
> I will keep an eye on this driver and the patches submitted for it in the
> future.
> 
> Arınç
> 
> v4: Make the patch logs and my comments in the code easier to understand.
> v3: Fix the from header on the patches. Write a cover letter.
> v2: Add patches to fix the handling of LLDP frames and BPDUs.
> 
> Arınç ÜNAL (7):
>   net: dsa: mt7530: fix trapping frames with multiple CPU ports on MT7531
>   net: dsa: mt7530: fix trapping frames with multiple CPU ports on MT7530
>   net: dsa: mt7530: fix trapping frames on non-MT7621 SoC MT7530 switch
>   net: dsa: mt7530: fix handling of BPDUs on MT7530 switch
>   net: dsa: mt7530: fix handling of LLDP frames
>   net: dsa: introduce preferred_default_local_cpu_port and use on MT7530
>   MAINTAINERS: add me as maintainer of MEDIATEK SWITCH DRIVER
> 
>  MAINTAINERS              |  5 +--
>  drivers/net/dsa/mt7530.c | 75 ++++++++++++++++++++++++++++++++++++-------
>  drivers/net/dsa/mt7530.h | 26 +++++++++------
>  include/net/dsa.h        |  8 +++++
>  net/dsa/dsa.c            | 24 +++++++++++++-
>  5 files changed, 115 insertions(+), 23 deletions(-)
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net v4 0/7] net: dsa: mt7530: fix multiple CPU ports, BPDU and LLDP handling
Posted by Russell King (Oracle) 1 year, 3 months ago
On Mon, Jun 12, 2023 at 12:37:29PM +0100, Russell King (Oracle) wrote:
> Hi,
> 
> Please slow down your rate of patch submission - I haven't had a chance
> to review the other patches yet (and I suspect no one else has.) Always
> allow a bit of time for discussion.
> 
> Just because you receive one comment doesn't mean you need to rush to
> get a new series out. Give it at least a few days because there may be
> further discussion of the points raised.
> 
> Sending new versions quickly after previous comments significantly
> increases reviewer workload.

And a very illustratory point is that I responded with a follow up to
your reply on v2, hadn't noticed that you'd sent v4, and the comments
I subsequently made on v2 apply to v4... and I haven't even looked at
v3 yet.

This is precisely why you need to stop "I've received an email, I've
made changes. Quick! Post the next version!" No, don't do that. Wait
a while for further feedback before posting the next version,
particularly if you've replied to reviewer comments - give the
reviewer some time to respond before posting the next version.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net v4 0/7] net: dsa: mt7530: fix multiple CPU ports, BPDU and LLDP handling
Posted by Andrew Lunn 1 year, 3 months ago
On Mon, Jun 12, 2023 at 09:52:29PM +0100, Russell King (Oracle) wrote:
> On Mon, Jun 12, 2023 at 12:37:29PM +0100, Russell King (Oracle) wrote:
> > Hi,
> > 
> > Please slow down your rate of patch submission - I haven't had a chance
> > to review the other patches yet (and I suspect no one else has.) Always
> > allow a bit of time for discussion.
> > 
> > Just because you receive one comment doesn't mean you need to rush to
> > get a new series out. Give it at least a few days because there may be
> > further discussion of the points raised.
> > 
> > Sending new versions quickly after previous comments significantly
> > increases reviewer workload.
> 
> And a very illustratory point is that I responded with a follow up to
> your reply on v2, hadn't noticed that you'd sent v4, and the comments
> I subsequently made on v2 apply to v4... and I haven't even looked at
> v3 yet.

Hi Arınç

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#netdev-faq

says:

  don't repost your patches within one 24h period

  2.6.6. Resending after review¶

  Allow at least 24 hours to pass between postings. This will ensure
  reviewers from all geographical locations have a chance to chime
  in. Do not wait too long (weeks) between postings either as it will
  make it harder for reviewers to recall all the context.
 
  Make sure you address all the feedback in your new posting. Do not
  post a new version of the code if the discussion about the previous
  version is still ongoing, unless directly instructed by a reviewer.

During a weekend, i would say 24 hours is way too short, and 3 days is
more like it, given that for a lot of people being a Maintainer is a
day job, 9-5 week days.

You should also try to gauge how fast Maintainers are reacting. 24
hours is often too fast. You know Russell is interested in these
patches, so don't send a new version until you actually get feedback
from him, and the discussion has come to a conclusion.

     Andrew
Re: [PATCH net v4 0/7] net: dsa: mt7530: fix multiple CPU ports, BPDU and LLDP handling
Posted by Arınç ÜNAL 1 year, 3 months ago
On 13.06.2023 00:30, Andrew Lunn wrote:
> On Mon, Jun 12, 2023 at 09:52:29PM +0100, Russell King (Oracle) wrote:
>> On Mon, Jun 12, 2023 at 12:37:29PM +0100, Russell King (Oracle) wrote:
>>> Hi,
>>>
>>> Please slow down your rate of patch submission - I haven't had a chance
>>> to review the other patches yet (and I suspect no one else has.) Always
>>> allow a bit of time for discussion.
>>>
>>> Just because you receive one comment doesn't mean you need to rush to
>>> get a new series out. Give it at least a few days because there may be
>>> further discussion of the points raised.
>>>
>>> Sending new versions quickly after previous comments significantly
>>> increases reviewer workload.
>>
>> And a very illustratory point is that I responded with a follow up to
>> your reply on v2, hadn't noticed that you'd sent v4, and the comments
>> I subsequently made on v2 apply to v4... and I haven't even looked at
>> v3 yet.
> 
> Hi Arınç
> 
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#netdev-faq
> 
> says:
> 
>    don't repost your patches within one 24h period
> 
>    2.6.6. Resending after review¶
> 
>    Allow at least 24 hours to pass between postings. This will ensure
>    reviewers from all geographical locations have a chance to chime
>    in. Do not wait too long (weeks) between postings either as it will
>    make it harder for reviewers to recall all the context.
>   
>    Make sure you address all the feedback in your new posting. Do not
>    post a new version of the code if the discussion about the previous
>    version is still ongoing, unless directly instructed by a reviewer.
> 
> During a weekend, i would say 24 hours is way too short, and 3 days is
> more like it, given that for a lot of people being a Maintainer is a
> day job, 9-5 week days.
> 
> You should also try to gauge how fast Maintainers are reacting. 24
> hours is often too fast. You know Russell is interested in these
> patches, so don't send a new version until you actually get feedback
> from him, and the discussion has come to a conclusion.

Understood, thank you both for the kind warning.

Arınç