[v1,00/26] net: dsa: microchip: stats64, fdb, error

Message ID 20221128115958.4049431-1-o.rempel@pengutronix.de
Headers
Series net: dsa: microchip: stats64, fdb, error |

Message

Oleksij Rempel Nov. 28, 2022, 11:59 a.m. UTC
  This patch series is a result of maintaining work on ksz8 part of
microchip driver. It includes stats64 and fdb support. Error handling.
Loopback fix and so on...

Oleksij Rempel (26):
  net: dsa: microchip: add stats64 support for ksz8 series of switches
  net: dsa: microchip: ksz8: ksz8_fdb_dump: fix port validation and VID
    information
  net: dsa: microchip: ksz8: ksz8_fdb_dump: fix not complete fdb
    extraction
  net: dsa: microchip: ksz8: ksz8_fdb_dump: fix time stamp extraction
  net: dsa: microchip: ksz8: ksz8_fdb_dump: do not extract ghost entry
    from empty table
  net: dsa: microchip: ksz8863_smi: fix bulk access
  net: dsa: microchip: ksz8_r_dyn_mac_table(): remove timestamp support
  net: dsa: microchip: make ksz8_r_dyn_mac_table() static
  net: dsa: microchip: ksz8_r_dyn_mac_table(): remove fid support
  net: dsa: microchip: ksz8: refactor ksz8_fdb_dump()
  net: dsa: microchip: ksz8: ksz8_fdb_dump: dump static MAC table
  net: dsa: microchip: ksz8: move static mac table operations to a
    separate functions
  net: dsa: microchip: ksz8: add fdb_add/del support
  net: dsa: microchip: KSZ88x3 fix loopback support
  net: dsa: microchip: ksz8_r_dyn_mac_table(): move main part of the
    code out of if statement
  net: dsa: microchip: ksz8_r_dyn_mac_table(): use ret instead of rc
  net: dsa: microchip: ksz8_r_dyn_mac_table(): ksz: do not return EAGAIN
    on timeout
  net: dsa: microchip: ksz8_r_dyn_mac_table(): return read/write error
    if we got any
  net: dsa: microchip: ksz8_r_dyn_mac_table(): use entries variable to
    signal 0 entries
  net: dsa: microchip: make ksz8_r_sta_mac_table() static
  net: dsa: microchip: ksz8_r_sta_mac_table(): do not use error code for
    empty entries
  net: dsa: microchip: ksz8_r_sta_mac_table(): make use of error values
    provided by read/write functions
  net: dsa: microchip: make ksz8_w_sta_mac_table() static
  net: dsa: microchip: ksz8_w_sta_mac_table(): make use of error values
    provided by read/write functions
  net: dsa: microchip: remove ksz_port:on variable
  net: dsa: microchip: ksz8: do not force flow control by default

 drivers/net/dsa/microchip/ksz8.h        |  14 +-
 drivers/net/dsa/microchip/ksz8795.c     | 440 +++++++++++++++---------
 drivers/net/dsa/microchip/ksz8795_reg.h |   2 +
 drivers/net/dsa/microchip/ksz8863_smi.c |  10 +-
 drivers/net/dsa/microchip/ksz_common.c  | 100 +++++-
 drivers/net/dsa/microchip/ksz_common.h  |   2 +-
 6 files changed, 377 insertions(+), 191 deletions(-)
  

Comments

Andrew Lunn Nov. 28, 2022, 1:51 p.m. UTC | #1
On Mon, Nov 28, 2022 at 12:59:32PM +0100, Oleksij Rempel wrote:
> This patch series is a result of maintaining work on ksz8 part of
> microchip driver. It includes stats64 and fdb support. Error handling.
> Loopback fix and so on...
> 
> Oleksij Rempel (26):

The netdev FAQ says:

* don’t post large series (> 15 patches), break them up

This seems like something which could easily be broken up.

     Andrew
  
Jakub Kicinski Nov. 28, 2022, 9:05 p.m. UTC | #2
On Mon, 28 Nov 2022 14:51:47 +0100 Andrew Lunn wrote:
> * don’t post large series (> 15 patches), break them up
> 
> This seems like something which could easily be broken up.

And name the target tree in the subject tag. And make sure it applies
cleanly to that tree (last patch is to blame AFAICT). And don't repost
within 24 hours.
  
Jacob Keller Nov. 28, 2022, 11:09 p.m. UTC | #3
On 11/28/2022 3:59 AM, Oleksij Rempel wrote:
> This patch series is a result of maintaining work on ksz8 part of
> microchip driver. It includes stats64 and fdb support. Error handling.
> Loopback fix and so on...
> 
> Oleksij Rempel (26):
>    net: dsa: microchip: add stats64 support for ksz8 series of switches
>    net: dsa: microchip: ksz8: ksz8_fdb_dump: fix port validation and VID
>      information
>    net: dsa: microchip: ksz8: ksz8_fdb_dump: fix not complete fdb
>      extraction
>    net: dsa: microchip: ksz8: ksz8_fdb_dump: fix time stamp extraction
>    net: dsa: microchip: ksz8: ksz8_fdb_dump: do not extract ghost entry
>      from empty table
>    net: dsa: microchip: ksz8863_smi: fix bulk access
>    net: dsa: microchip: ksz8_r_dyn_mac_table(): remove timestamp support
>    net: dsa: microchip: make ksz8_r_dyn_mac_table() static
>    net: dsa: microchip: ksz8_r_dyn_mac_table(): remove fid support
>    net: dsa: microchip: ksz8: refactor ksz8_fdb_dump()
>    net: dsa: microchip: ksz8: ksz8_fdb_dump: dump static MAC table
>    net: dsa: microchip: ksz8: move static mac table operations to a
>      separate functions
>    net: dsa: microchip: ksz8: add fdb_add/del support
>    net: dsa: microchip: KSZ88x3 fix loopback support
>    net: dsa: microchip: ksz8_r_dyn_mac_table(): move main part of the
>      code out of if statement
>    net: dsa: microchip: ksz8_r_dyn_mac_table(): use ret instead of rc
>    net: dsa: microchip: ksz8_r_dyn_mac_table(): ksz: do not return EAGAIN
>      on timeout
>    net: dsa: microchip: ksz8_r_dyn_mac_table(): return read/write error
>      if we got any
>    net: dsa: microchip: ksz8_r_dyn_mac_table(): use entries variable to
>      signal 0 entries
>    net: dsa: microchip: make ksz8_r_sta_mac_table() static
>    net: dsa: microchip: ksz8_r_sta_mac_table(): do not use error code for
>      empty entries
>    net: dsa: microchip: ksz8_r_sta_mac_table(): make use of error values
>      provided by read/write functions
>    net: dsa: microchip: make ksz8_w_sta_mac_table() static
>    net: dsa: microchip: ksz8_w_sta_mac_table(): make use of error values
>      provided by read/write functions
>    net: dsa: microchip: remove ksz_port:on variable
>    net: dsa: microchip: ksz8: do not force flow control by default
> 


My understanding is that we typically limit series to 15 patches. Do you 
have some justification for why this goes over 15 and can't reasonably 
be split into two series?

At a glance it seems like a bunch of smaller cleanups.
  
Oleksij Rempel Nov. 29, 2022, 5:35 a.m. UTC | #4
On Mon, Nov 28, 2022 at 03:09:19PM -0800, Jacob Keller wrote:
> 
> 
> On 11/28/2022 3:59 AM, Oleksij Rempel wrote:
> > This patch series is a result of maintaining work on ksz8 part of
> > microchip driver. It includes stats64 and fdb support. Error handling.
> > Loopback fix and so on...
> > 
> > Oleksij Rempel (26):
> >    net: dsa: microchip: add stats64 support for ksz8 series of switches
> >    net: dsa: microchip: ksz8: ksz8_fdb_dump: fix port validation and VID
> >      information
> >    net: dsa: microchip: ksz8: ksz8_fdb_dump: fix not complete fdb
> >      extraction
> >    net: dsa: microchip: ksz8: ksz8_fdb_dump: fix time stamp extraction
> >    net: dsa: microchip: ksz8: ksz8_fdb_dump: do not extract ghost entry
> >      from empty table
> >    net: dsa: microchip: ksz8863_smi: fix bulk access
> >    net: dsa: microchip: ksz8_r_dyn_mac_table(): remove timestamp support
> >    net: dsa: microchip: make ksz8_r_dyn_mac_table() static
> >    net: dsa: microchip: ksz8_r_dyn_mac_table(): remove fid support
> >    net: dsa: microchip: ksz8: refactor ksz8_fdb_dump()
> >    net: dsa: microchip: ksz8: ksz8_fdb_dump: dump static MAC table
> >    net: dsa: microchip: ksz8: move static mac table operations to a
> >      separate functions
> >    net: dsa: microchip: ksz8: add fdb_add/del support
> >    net: dsa: microchip: KSZ88x3 fix loopback support
> >    net: dsa: microchip: ksz8_r_dyn_mac_table(): move main part of the
> >      code out of if statement
> >    net: dsa: microchip: ksz8_r_dyn_mac_table(): use ret instead of rc
> >    net: dsa: microchip: ksz8_r_dyn_mac_table(): ksz: do not return EAGAIN
> >      on timeout
> >    net: dsa: microchip: ksz8_r_dyn_mac_table(): return read/write error
> >      if we got any
> >    net: dsa: microchip: ksz8_r_dyn_mac_table(): use entries variable to
> >      signal 0 entries
> >    net: dsa: microchip: make ksz8_r_sta_mac_table() static
> >    net: dsa: microchip: ksz8_r_sta_mac_table(): do not use error code for
> >      empty entries
> >    net: dsa: microchip: ksz8_r_sta_mac_table(): make use of error values
> >      provided by read/write functions
> >    net: dsa: microchip: make ksz8_w_sta_mac_table() static
> >    net: dsa: microchip: ksz8_w_sta_mac_table(): make use of error values
> >      provided by read/write functions
> >    net: dsa: microchip: remove ksz_port:on variable
> >    net: dsa: microchip: ksz8: do not force flow control by default
> > 
> 
> 
> My understanding is that we typically limit series to 15 patches. Do you
> have some justification for why this goes over 15 and can't reasonably be
> split into two series?
> 
> At a glance it seems like a bunch of smaller cleanups.

The previous patch set got request to do more clean ups:
https://lore.kernel.org/all/20221124101458.3353902-1-o.rempel@pengutronix.de/

I need to show, there are already more patches in the queue.

Regards,
Oleksij
  
Andrew Lunn Nov. 29, 2022, 2:15 p.m. UTC | #5
On Tue, Nov 29, 2022 at 06:35:39AM +0100, Oleksij Rempel wrote:
> On Mon, Nov 28, 2022 at 03:09:19PM -0800, Jacob Keller wrote:
> > 
> > 
> > On 11/28/2022 3:59 AM, Oleksij Rempel wrote:
> > > This patch series is a result of maintaining work on ksz8 part of
> > > microchip driver. It includes stats64 and fdb support. Error handling.
> > > Loopback fix and so on...
> > > 
> > > Oleksij Rempel (26):
> > >    net: dsa: microchip: add stats64 support for ksz8 series of switches
> > >    net: dsa: microchip: ksz8: ksz8_fdb_dump: fix port validation and VID
> > >      information
> > >    net: dsa: microchip: ksz8: ksz8_fdb_dump: fix not complete fdb
> > >      extraction
> > >    net: dsa: microchip: ksz8: ksz8_fdb_dump: fix time stamp extraction
> > >    net: dsa: microchip: ksz8: ksz8_fdb_dump: do not extract ghost entry
> > >      from empty table
> > >    net: dsa: microchip: ksz8863_smi: fix bulk access
> > >    net: dsa: microchip: ksz8_r_dyn_mac_table(): remove timestamp support
> > >    net: dsa: microchip: make ksz8_r_dyn_mac_table() static
> > >    net: dsa: microchip: ksz8_r_dyn_mac_table(): remove fid support
> > >    net: dsa: microchip: ksz8: refactor ksz8_fdb_dump()
> > >    net: dsa: microchip: ksz8: ksz8_fdb_dump: dump static MAC table
> > >    net: dsa: microchip: ksz8: move static mac table operations to a
> > >      separate functions
> > >    net: dsa: microchip: ksz8: add fdb_add/del support
> > >    net: dsa: microchip: KSZ88x3 fix loopback support
> > >    net: dsa: microchip: ksz8_r_dyn_mac_table(): move main part of the
> > >      code out of if statement
> > >    net: dsa: microchip: ksz8_r_dyn_mac_table(): use ret instead of rc
> > >    net: dsa: microchip: ksz8_r_dyn_mac_table(): ksz: do not return EAGAIN
> > >      on timeout
> > >    net: dsa: microchip: ksz8_r_dyn_mac_table(): return read/write error
> > >      if we got any
> > >    net: dsa: microchip: ksz8_r_dyn_mac_table(): use entries variable to
> > >      signal 0 entries
> > >    net: dsa: microchip: make ksz8_r_sta_mac_table() static
> > >    net: dsa: microchip: ksz8_r_sta_mac_table(): do not use error code for
> > >      empty entries
> > >    net: dsa: microchip: ksz8_r_sta_mac_table(): make use of error values
> > >      provided by read/write functions
> > >    net: dsa: microchip: make ksz8_w_sta_mac_table() static
> > >    net: dsa: microchip: ksz8_w_sta_mac_table(): make use of error values
> > >      provided by read/write functions
> > >    net: dsa: microchip: remove ksz_port:on variable
> > >    net: dsa: microchip: ksz8: do not force flow control by default
> > > 
> > 
> > 
> > My understanding is that we typically limit series to 15 patches. Do you
> > have some justification for why this goes over 15 and can't reasonably be
> > split into two series?
> > 
> > At a glance it seems like a bunch of smaller cleanups.
> 
> The previous patch set got request to do more clean ups:
> https://lore.kernel.org/all/20221124101458.3353902-1-o.rempel@pengutronix.de/
> 
> I need to show, there are already more patches in the queue.

There is some psychology involved here. I see 26 patches and decide i
need to allocate 30 minutes to this sometime, and put the review off
until later, without even looking at them. If i get 5 patches, i
probably just do it, knowing i will be finished pretty quickly. My
guess is, 5 patches a day for 5 days will be merged faster than 26
patches in one go.

     Andrew
  
Oleksij Rempel Nov. 30, 2022, 5:34 a.m. UTC | #6
On Tue, Nov 29, 2022 at 03:15:03PM +0100, Andrew Lunn wrote:
> On Tue, Nov 29, 2022 at 06:35:39AM +0100, Oleksij Rempel wrote:
> > On Mon, Nov 28, 2022 at 03:09:19PM -0800, Jacob Keller wrote:
> > > 
> > > My understanding is that we typically limit series to 15 patches. Do you
> > > have some justification for why this goes over 15 and can't reasonably be
> > > split into two series?
> > > 
> > > At a glance it seems like a bunch of smaller cleanups.
> > 
> > The previous patch set got request to do more clean ups:
> > https://lore.kernel.org/all/20221124101458.3353902-1-o.rempel@pengutronix.de/
> > 
> > I need to show, there are already more patches in the queue.
> 
> There is some psychology involved here. I see 26 patches and decide i
> need to allocate 30 minutes to this sometime, and put the review off
> until later, without even looking at them. If i get 5 patches, i
> probably just do it, knowing i will be finished pretty quickly. My
> guess is, 5 patches a day for 5 days will be merged faster than 26
> patches in one go.

Good point. Thx!

I'll split this patch set.

Regards,
Oleksij