| Message ID | 20201118220357.22292-9-m.grzeschik@pengutronix.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: microchip: make ksz8795 driver more dynamic | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | fail | Errors and warnings before: 7 this patch: 7 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 40 lines checked |
| netdev/build_allmodconfig_warn | fail | Errors and warnings before: 7 this patch: 7 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
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@pengutronix.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... Andrew
On 11/18/2020 2:03 PM, 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@pengutronix.de> With Andrew's comment addressed on clarifying the intent of port_cnt: Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> 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@pengutronix.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.
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, }, { .chip_id = 0x8794, @@ -1192,7 +1192,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 = 3, /* total physical port count */ + .port_cnt = 4, }, { .chip_id = 0x8765, @@ -1201,7 +1201,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, }, }; @@ -1237,7 +1237,7 @@ static int ksz8795_switch_init(struct ksz_device *dev) dev->mib_cnt = ARRAY_SIZE(mib_names); dev->mib_port_cnt = TOTAL_PORT_NUM; - dev->phy_port_cnt = dev->port_cnt; + dev->phy_port_cnt = dev->port_cnt - 1; dev->cpu_port = dev->mib_port_cnt - 1; dev->host_mask = BIT(dev->cpu_port); @@ -1259,7 +1259,7 @@ static int ksz8795_switch_init(struct ksz_device *dev) } /* set the real number of ports */ - dev->ds->num_ports = dev->port_cnt + 1; + dev->ds->num_ports = dev->port_cnt; return 0; }
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@pengutronix.de> --- drivers/net/dsa/microchip/ksz8795.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)