| Message ID | 20220610153829.446516-5-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 | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 11 of 11 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| 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 | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 37 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
> The variable is just assigned the value of a macro, so it can be > removed. As I commented previously, the switches in this family with 10 ports do have a different value for RTL8365MB_LEARN_LIMIT_MAX. Once we add support for one of those models, we will somewhat revert this patch. I believe learn_limit_max would fit better inside the new static chip_info structure. Regards, Luiz > Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk> > --- > drivers/net/dsa/realtek/rtl8365mb.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c > index 3599fa5d9f14..676b88798976 100644 > --- a/drivers/net/dsa/realtek/rtl8365mb.c > +++ b/drivers/net/dsa/realtek/rtl8365mb.c > @@ -563,7 +563,6 @@ struct rtl8365mb_port { > * @irq: registered IRQ or zero > * @chip_id: chip identifier > * @chip_ver: chip silicon revision > - * @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 > * @ports: per-port data > @@ -577,7 +576,6 @@ struct rtl8365mb { > int irq; > u32 chip_id; > u32 chip_ver; > - u32 learn_limit_max; > struct rtl8365mb_cpu cpu; > struct mutex mib_lock; > struct rtl8365mb_port ports[RTL8365MB_MAX_NUM_PORTS]; > @@ -1088,15 +1086,13 @@ static void rtl8365mb_port_stp_state_set(struct dsa_switch *ds, int port, > static int rtl8365mb_port_set_learning(struct realtek_priv *priv, int port, > bool enable) > { > - struct rtl8365mb *mb = priv->chip_data; > - > /* Enable/disable learning by limiting the number of L2 addresses the > * port can learn. Realtek documentation states that a limit of zero > * disables learning. When enabling learning, set it to the chip's > * maximum. > */ > return regmap_write(priv->map, RTL8365MB_LUT_PORT_LEARN_LIMIT_REG(port), > - enable ? mb->learn_limit_max : 0); > + enable ? RTL8365MB_LEARN_LIMIT_MAX : 0); > } > > static int rtl8365mb_port_set_isolation(struct realtek_priv *priv, int port, > @@ -2003,7 +1999,6 @@ static int rtl8365mb_detect(struct realtek_priv *priv) > mb->priv = priv; > mb->chip_id = chip_id; > mb->chip_ver = chip_ver; > - 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); > > -- > 2.36.1 >
On Sat, Jun 11, 2022 at 10:40:33PM -0300, Luiz Angelo Daros de Luca wrote: > > The variable is just assigned the value of a macro, so it can be > > removed. > > As I commented previously, the switches in this family with 10 ports > do have a different value for RTL8365MB_LEARN_LIMIT_MAX. > Once we add support for one of those models, we will somewhat revert this patch. I wouldn't call that a revert, just normal development. > > I believe learn_limit_max would fit better inside the new static > chip_info structure. Other pedants may ask me what the point of such a patch is when the hardware is not even supported. That was my main reason for not incorporating your suggestion. The other reason is that, having actually experimented with the learn limit myself, I could in fact make my RTL8365MB-VC learn more than this presupposed maximum the vendor driver uses. I think it also depends on whether IVL/SVL is in use. So there might be more to it than you think.
diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c index 3599fa5d9f14..676b88798976 100644 --- a/drivers/net/dsa/realtek/rtl8365mb.c +++ b/drivers/net/dsa/realtek/rtl8365mb.c @@ -563,7 +563,6 @@ struct rtl8365mb_port { * @irq: registered IRQ or zero * @chip_id: chip identifier * @chip_ver: chip silicon revision - * @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 * @ports: per-port data @@ -577,7 +576,6 @@ struct rtl8365mb { int irq; u32 chip_id; u32 chip_ver; - u32 learn_limit_max; struct rtl8365mb_cpu cpu; struct mutex mib_lock; struct rtl8365mb_port ports[RTL8365MB_MAX_NUM_PORTS]; @@ -1088,15 +1086,13 @@ static void rtl8365mb_port_stp_state_set(struct dsa_switch *ds, int port, static int rtl8365mb_port_set_learning(struct realtek_priv *priv, int port, bool enable) { - struct rtl8365mb *mb = priv->chip_data; - /* Enable/disable learning by limiting the number of L2 addresses the * port can learn. Realtek documentation states that a limit of zero * disables learning. When enabling learning, set it to the chip's * maximum. */ return regmap_write(priv->map, RTL8365MB_LUT_PORT_LEARN_LIMIT_REG(port), - enable ? mb->learn_limit_max : 0); + enable ? RTL8365MB_LEARN_LIMIT_MAX : 0); } static int rtl8365mb_port_set_isolation(struct realtek_priv *priv, int port, @@ -2003,7 +1999,6 @@ static int rtl8365mb_detect(struct realtek_priv *priv) mb->priv = priv; mb->chip_id = chip_id; mb->chip_ver = chip_ver; - 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);