netdev - RE: [PATCH 08/11] net: dsa: microchip: ksz8795: align port

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] [day] [month] [year] [list]

Date:   Thu, 19 Nov 2020 18:06:03 +0000
From:   <Tristram.Ha@...rochip.com>
To:     <andrew@...n.ch>, <m.grzeschik@...gutronix.de>
CC:     <netdev@...r.kernel.org>, <f.fainelli@...il.com>,
        <davem@...emloft.net>, <kernel@...gutronix.de>,
        <matthias.schiffer@...tq-group.com>, <Woojung.Huh@...rochip.com>,
        <UNGLinuxDriver@...rochip.com>
Subject: RE: [PATCH 08/11] net: dsa: microchip: ksz8795: align port_cnt usage
 with other microchip drivers

> On Wed, Nov 18, 2020 at 11:03:54PM +0100, Michael Grzeschik wrote:
> > The ksz8795 driver is using port_cnt differently to the other microchip
> > DSA drivers. It sets it to the external physical port count, than the
> > whole port count (including the cpu port). This patch is aligning the
> > variables purpose with the other microchip drivers.
> >
> > Signed-off-by: Michael Grzeschik <m.grzeschik@...gutronix.de>
> > ---
> >  drivers/net/dsa/microchip/ksz8795.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/dsa/microchip/ksz8795.c
> b/drivers/net/dsa/microchip/ksz8795.c
> > index 17dc720df2340b0..10c9b301833dd59 100644
> > --- a/drivers/net/dsa/microchip/ksz8795.c
> > +++ b/drivers/net/dsa/microchip/ksz8795.c
> > @@ -1183,7 +1183,7 @@ static const struct ksz_chip_data
> ksz8795_switch_chips[] = {
> >               .num_alus = 0,
> >               .num_statics = 8,
> >               .cpu_ports = 0x10,      /* can be configured as cpu port */
> > -             .port_cnt = 4,          /* total physical port count */
> > +             .port_cnt = 5,
> 
> Rather than remove the comment, please could you update the
> comment. port_cnt is too generic to know its exact meaning without a
> helpful comment. And this might be why this driver is different...

At one time there are 3 distinctions of the ports used in the drivers for KSZ switches.
Physical ports require valid link to operate.  They are the usual ports users interact with.
The total port count is usually physical port count + 1.  The last port is the host port.
They all have the usual port controls like receive, transmit, QoS, and other functions.
That last port may not have MIB counters.  That is why another variable is used to
manage handling of MIB counters in a loop.

The KSZ9477/KSZ9897 family of switches is a new design where any port can be a host port.
It also has extra RGMII/SGMII port that makes the term "physical port" ambiguous.

KSZ8795 has 5 ports.  The last is always the host port.
KSZ8794 has 3 physical ports, but the last port is still 5.  Port 4 is disabled.

There is another KSZ8895 switch which also has 5 ports.  It has a variation KSZ8864 which
disables the first port.

Now the DSA layer treats each port individually and there is less use of a loop of ports
Inside the switch driver it is good to consolidate those port variables.