| Message ID | 20220606134553.2919693-3-alvin@pqrs.dk (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: realtek: rtl8365mb: improve handling of PHY modes | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | fail | Errors and warnings before: 0 this patch: 1 |
| netdev/cc_maintainers | success | CCed 11 of 11 maintainers |
| netdev/build_clang | fail | Errors and warnings before: 0 this patch: 2 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | fail | Errors and warnings before: 0 this patch: 1 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 29 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
> There is no real need for this variable: the line change interrupt mask > is sufficiently masked out when getting linkup_ind and linkdown_ind in > the interrupt handler. Yes, it was currently useless as well as priv->num_ports (it is a constant). I wonder if we should really create irq threads for unused ports (!dsa_is_unused_port()). Some models have only 2+1 ports and we are always dealing with 10/11 ports. If dsa_is_unused_port() is too costly to be used everywhere, we could keep port_mask and iterate over it (for_each_set_bit) instead of from 0 til priv->num_ports-1. Regards, Luiz
On Tue, Jun 07, 2022 at 10:34:38AM -0300, Luiz Angelo Daros de Luca wrote: > > There is no real need for this variable: the line change interrupt mask > > is sufficiently masked out when getting linkup_ind and linkdown_ind in > > the interrupt handler. > > Yes, it was currently useless as well as priv->num_ports (it is a constant). > > I wonder if we should really create irq threads for unused ports > (!dsa_is_unused_port()). Some models have only 2+1 ports and we are > always dealing with 10/11 ports. > If dsa_is_unused_port() is too costly to be used everywhere, we could > keep port_mask and iterate over it (for_each_set_bit) instead of from > 0 til priv->num_ports-1. Seems like premature optimization, I prefer to keep it simple. Kind regards, Alvin
diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c index 0cc90e96aab7..c64219271a2b 100644 --- a/drivers/net/dsa/realtek/rtl8365mb.c +++ b/drivers/net/dsa/realtek/rtl8365mb.c @@ -564,7 +564,6 @@ struct rtl8365mb_port { * @irq: registered IRQ or zero * @chip_id: chip identifier * @chip_ver: chip silicon revision - * @port_mask: mask of all ports * @learn_limit_max: maximum number of L2 addresses the chip can learn * @cpu: CPU tagging and CPU port configuration for this chip * @mib_lock: prevent concurrent reads of MIB counters @@ -579,7 +578,6 @@ struct rtl8365mb { int irq; u32 chip_id; u32 chip_ver; - u32 port_mask; u32 learn_limit_max; struct rtl8365mb_cpu cpu; struct mutex mib_lock; @@ -1540,7 +1538,7 @@ static irqreturn_t rtl8365mb_irq(int irq, void *data) linkdown_ind = FIELD_GET(RTL8365MB_PORT_LINKDOWN_IND_MASK, val); - line_changes = (linkup_ind | linkdown_ind) & mb->port_mask; + line_changes = linkup_ind | linkdown_ind; } if (!line_changes) @@ -2029,7 +2027,6 @@ static int rtl8365mb_detect(struct realtek_priv *priv) mb->priv = priv; mb->chip_id = chip_id; mb->chip_ver = chip_ver; - mb->port_mask = GENMASK(priv->num_ports - 1, 0); mb->learn_limit_max = RTL8365MB_LEARN_LIMIT_MAX; mb->jam_table = rtl8365mb_init_jam_8365mb_vc; mb->jam_size = ARRAY_SIZE(rtl8365mb_init_jam_8365mb_vc);