| Message ID | 20210905172328.26281-1-zajec5@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 63f8428b4077de3664eb0b252393c839b0b293ec |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: b53: Fix IMP port setup on BCM5301x | 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/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| 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, 185 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Sun, 5 Sep 2021 19:23:28 +0200 you wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > Broadcom's b53 switches have one IMP (Inband Management Port) that needs > to be programmed using its own designed register. IMP port may be > different than CPU port - especially on devices with multiple CPU ports. > > For that reason it's required to explicitly note IMP port index and > check for it when choosing a register to use. > > [...] Here is the summary with links: - net: dsa: b53: Fix IMP port setup on BCM5301x https://git.kernel.org/netdev/net/c/63f8428b4077 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
On 9/5/2021 11:10 AM, patchwork-bot+netdevbpf@kernel.org wrote: > Hello: > > This patch was applied to netdev/net.git (refs/heads/master): > > On Sun, 5 Sep 2021 19:23:28 +0200 you wrote: >> From: Rafał Miłecki <rafal@milecki.pl> >> >> Broadcom's b53 switches have one IMP (Inband Management Port) that needs >> to be programmed using its own designed register. IMP port may be >> different than CPU port - especially on devices with multiple CPU ports. >> >> For that reason it's required to explicitly note IMP port index and >> check for it when choosing a register to use. >> >> [...] > > Here is the summary with links: > - net: dsa: b53: Fix IMP port setup on BCM5301x > https://git.kernel.org/netdev/net/c/63f8428b4077 > > You are awesome, thank you! > -- > Deet-doot-dot, I am a bot. > https://korg.docs.kernel.org/patchwork/pwbot.html David, can you please wait more than 1h 47 minutes before applying a patch to give a review? This is absolutely not the way this should have been fixed because it adds to the driver's port information burden rather than not. This is not the first time this has happened, and this is really really starting to annoy the crap out of me. While I am appreciative of your responsiveness in applying patches, I am definitively not when it comes to not allowing a proper review to happen. So please, I am begging you, wait at least 12h, ideally 24h before applying a patch. You have patchwork, you have responsive maintainers, so nothing will get dropped on the floor. Thank you PS: for some reason Rafal's email address got turned into: "Rafał Miłecki <zajec5@gmail.com>"@ci.codeaurora.org. You might want to look into that as well.
On 9/5/2021 10:23 AM, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > Broadcom's b53 switches have one IMP (Inband Management Port) that needs > to be programmed using its own designed register. IMP port may be > different than CPU port - especially on devices with multiple CPU ports. There are two choices: port 5 or port 8, > > For that reason it's required to explicitly note IMP port index and > check for it when choosing a register to use. > > This commit fixes BCM5301x support. Those switches use CPU port 5 while > their IMP port is 8. Before this patch b53 was trying to program port 5 > with B53_PORT_OVERRIDE_CTRL instead of B53_GMII_PORT_OVERRIDE_CTRL(5). > > It may be possible to also replace "cpu_port" usages with > dsa_is_cpu_port() but that is out of the scope of thix BCM5301x fix. Actually this would have been well within the scope of this patch. > > Fixes: 967dd82ffc52 ("net: dsa: b53: Add support for Broadcom RoboSwitch") > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> I really don't like the duplication of the "imp_port" and "cpu_port" members, first because this caused us problems before, and second because for all switch entries except the BCM5301X, cpu_port == imp_port, so this a duplication, and a waste of storage space to encode information. In fact, there is no such thing as CPU port technically you chose either IMP0 or IMP1. IMP0 is port 8 and IMP1 is port 5. > --- > drivers/net/dsa/b53/b53_common.c | 28 +++++++++++++++++++++++++--- > drivers/net/dsa/b53/b53_priv.h | 1 + > 2 files changed, 26 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c > index 5646eb8afe38..604f54112665 100644 > --- a/drivers/net/dsa/b53/b53_common.c > +++ b/drivers/net/dsa/b53/b53_common.c > @@ -1144,7 +1144,7 @@ static void b53_force_link(struct b53_device *dev, int port, int link) > u8 reg, val, off; > > /* Override the port settings */ > - if (port == dev->cpu_port) { > + if (port == dev->imp_port) { This should be port == 8 > off = B53_PORT_OVERRIDE_CTRL; > val = PORT_OVERRIDE_EN; > } else { > @@ -1168,7 +1168,7 @@ static void b53_force_port_config(struct b53_device *dev, int port, > u8 reg, val, off; > > /* Override the port settings */ > - if (port == dev->cpu_port) { > + if (port == dev->imp_port) { Likewise > off = B53_PORT_OVERRIDE_CTRL; > val = PORT_OVERRIDE_EN; > } else { > @@ -1236,7 +1236,7 @@ static void b53_adjust_link(struct dsa_switch *ds, int port, > b53_force_link(dev, port, phydev->link); > > if (is531x5(dev) && phy_interface_is_rgmii(phydev)) { > - if (port == 8) > + if (port == dev->imp_port) That use of port 8 was correct. > off = B53_RGMII_CTRL_IMP; > else > off = B53_RGMII_CTRL_P(port); > @@ -2280,6 +2280,7 @@ struct b53_chip_data { > const char *dev_name; > u16 vlans; > u16 enabled_ports; > + u8 imp_port; > u8 cpu_port; > u8 vta_regs[3]; > u8 arl_bins; > @@ -2304,6 +2305,7 @@ static const struct b53_chip_data b53_switch_chips[] = { > .enabled_ports = 0x1f, > .arl_bins = 2, > .arl_buckets = 1024, > + .imp_port = 5, Could have used B53_CPU_PORT_25 here. > .cpu_port = B53_CPU_PORT_25, > .duplex_reg = B53_DUPLEX_STAT_FE, > }, > @@ -2314,6 +2316,7 @@ static const struct b53_chip_data b53_switch_chips[] = { > .enabled_ports = 0x1f, > .arl_bins = 2, > .arl_buckets = 1024, > + .imp_port = 5, > .cpu_port = B53_CPU_PORT_25, and here. > .duplex_reg = B53_DUPLEX_STAT_FE, > }, > @@ -2324,6 +2327,7 @@ static const struct b53_chip_data b53_switch_chips[] = { > .enabled_ports = 0x1f, > .arl_bins = 4, > .arl_buckets = 1024, > + .imp_port = 8, > .cpu_port = B53_CPU_PORT, and B53_CPU_PORT here and for each entry below. I will put this patch into my local test rack and see what breaks, and we can address this more cleanly with net-next. Another case where if we had more time to do a proper review we could come up with a small fix, and not create additional technical debt to fix in the next release cycle. Hope's spring is eternal, oh and I just came back from France, so I guess I am full of complaints, too :) -- Florian
On 05.09.2021 23:04, Florian Fainelli wrote: > On 9/5/2021 11:10 AM, patchwork-bot+netdevbpf@kernel.org wrote: >> Hello: >> >> This patch was applied to netdev/net.git (refs/heads/master): >> >> On Sun, 5 Sep 2021 19:23:28 +0200 you wrote: >>> From: Rafał Miłecki <rafal@milecki.pl> >>> >>> Broadcom's b53 switches have one IMP (Inband Management Port) that needs >>> to be programmed using its own designed register. IMP port may be >>> different than CPU port - especially on devices with multiple CPU ports. >>> >>> For that reason it's required to explicitly note IMP port index and >>> check for it when choosing a register to use. >>> >>> [...] >> >> Here is the summary with links: >> - net: dsa: b53: Fix IMP port setup on BCM5301x >> https://git.kernel.org/netdev/net/c/63f8428b4077 >> >> You are awesome, thank you! >> -- >> Deet-doot-dot, I am a bot. >> https://korg.docs.kernel.org/patchwork/pwbot.html > > David, can you please wait more than 1h 47 minutes before applying a patch to give a review? This is absolutely not the way this should have been fixed because it adds to the driver's port information burden rather than not. > > This is not the first time this has happened, and this is really really starting to annoy the crap out of me. While I am appreciative of your responsiveness in applying patches, I am definitively not when it comes to not allowing a proper review to happen. So please, I am begging you, wait at least 12h, ideally 24h before applying a patch. You have patchwork, you have responsive maintainers, so nothing will get dropped on the floor. I was also surprised a bit with that quick apply. I prefer to have my code reviewed properly. I'm OK with a revert and working on a better fix (or change for net-next) if that is a valid option. I can also work on fixing that fix as I surely don't mean to leave code as is when maintainer isn't happy about it.
> I was also surprised a bit with that quick apply. I prefer to have my > code reviewed properly. A few people are posting with RFC in the subject, to slow down the merge and give people a chance to actually comment. This is not how it should be, but that does appear to at least work. Andrew
> not allowing a proper review to happen. So please, I am begging you, wait at > least 12h, ideally 24h before applying a patch. 24 hours is too short. We all have lives outside of the kernel. I found the older policy of 3 days worked well. Enough time for those who had interest to do a review, but short enough to not really slow down development. And 3 days is still probably faster than any other subsystem. Andrew
On 05.09.2021 23:16, Florian Fainelli wrote: > On 9/5/2021 10:23 AM, Rafał Miłecki wrote: >> From: Rafał Miłecki <rafal@milecki.pl> >> >> Broadcom's b53 switches have one IMP (Inband Management Port) that needs >> to be programmed using its own designed register. IMP port may be >> different than CPU port - especially on devices with multiple CPU ports. > > There are two choices: port 5 or port 8, Yes. Depending on model I assign 5 or 8 in the b53_chip_data. What did I miss? >> For that reason it's required to explicitly note IMP port index and >> check for it when choosing a register to use. >> >> This commit fixes BCM5301x support. Those switches use CPU port 5 while >> their IMP port is 8. Before this patch b53 was trying to program port 5 >> with B53_PORT_OVERRIDE_CTRL instead of B53_GMII_PORT_OVERRIDE_CTRL(5). >> >> It may be possible to also replace "cpu_port" usages with >> dsa_is_cpu_port() but that is out of the scope of thix BCM5301x fix. > > Actually this would have been well within the scope of this patch. I guess it's a matter of taste, I prefer to remove "cpu_port" usage piece by piece. I think it makes it easier to catch mistakes during review and regression finding easier. For example I'm not exactly sure how to get rid of "cpu_port" in the: if (port != dev->cpu_port) { b53_force_port_config(dev, dev->cpu_port, 2000, DUPLEX_FULL, true, true); b53_force_link(dev, dev->cpu_port, 1); } >> Fixes: 967dd82ffc52 ("net: dsa: b53: Add support for Broadcom RoboSwitch") >> Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > > I really don't like the duplication of the "imp_port" and "cpu_port" members, first because this caused us problems before, and second because for all switch entries except the BCM5301X, cpu_port == imp_port, so this a duplication, and a waste of storage space to encode information. Well, this isn't exactly a duplication as values differ for BCM5301X. I guess you prefer to handle BCM5301X with extra conditions in code while I thought it to be cleaner to store that chip data in a struct. Let's discuss code changes and see how it could be handled differently. > In fact, there is no such thing as CPU port technically you chose either IMP0 or IMP1. IMP0 is port 8 and IMP1 is port 5. > >> --- >> drivers/net/dsa/b53/b53_common.c | 28 +++++++++++++++++++++++++--- >> drivers/net/dsa/b53/b53_priv.h | 1 + >> 2 files changed, 26 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c >> index 5646eb8afe38..604f54112665 100644 >> --- a/drivers/net/dsa/b53/b53_common.c >> +++ b/drivers/net/dsa/b53/b53_common.c >> @@ -1144,7 +1144,7 @@ static void b53_force_link(struct b53_device *dev, int port, int link) >> u8 reg, val, off; >> /* Override the port settings */ >> - if (port == dev->cpu_port) { >> + if (port == dev->imp_port) { > > This should be port == 8 What about devices that have IMP port 5? I think that change would break b53_force_link() for them. >> off = B53_PORT_OVERRIDE_CTRL; >> val = PORT_OVERRIDE_EN; >> } else { >> @@ -1168,7 +1168,7 @@ static void b53_force_port_config(struct b53_device *dev, int port, >> u8 reg, val, off; >> /* Override the port settings */ >> - if (port == dev->cpu_port) { >> + if (port == dev->imp_port) { > > Likewise Same question. >> off = B53_PORT_OVERRIDE_CTRL; >> val = PORT_OVERRIDE_EN; >> } else { >> @@ -1236,7 +1236,7 @@ static void b53_adjust_link(struct dsa_switch *ds, int port, >> b53_force_link(dev, port, phydev->link); >> if (is531x5(dev) && phy_interface_is_rgmii(phydev)) { >> - if (port == 8) >> + if (port == dev->imp_port) > > That use of port 8 was correct. I tried to avoid some magic. >> off = B53_RGMII_CTRL_IMP; >> else >> off = B53_RGMII_CTRL_P(port); >> @@ -2280,6 +2280,7 @@ struct b53_chip_data { >> const char *dev_name; >> u16 vlans; >> u16 enabled_ports; >> + u8 imp_port; >> u8 cpu_port; >> u8 vta_regs[3]; >> u8 arl_bins; >> @@ -2304,6 +2305,7 @@ static const struct b53_chip_data b53_switch_chips[] = { >> .enabled_ports = 0x1f, >> .arl_bins = 2, >> .arl_buckets = 1024, >> + .imp_port = 5, > > Could have used B53_CPU_PORT_25 here. I didn't use B53_CPU_PORT* defines as they don't apply anymore. That _25 suffix made sense when support for first devices with CPU/IMP port 5 was added. They were actually BCM*25 chipsets. I think we should probably have something like B53_IMP0 and B53_IMP1. What do you think about proposed names? >> .cpu_port = B53_CPU_PORT_25, >> .duplex_reg = B53_DUPLEX_STAT_FE, >> }, >> @@ -2314,6 +2316,7 @@ static const struct b53_chip_data b53_switch_chips[] = { >> .enabled_ports = 0x1f, >> .arl_bins = 2, >> .arl_buckets = 1024, >> + .imp_port = 5, >> .cpu_port = B53_CPU_PORT_25, > > and here. > >> .duplex_reg = B53_DUPLEX_STAT_FE, >> }, >> @@ -2324,6 +2327,7 @@ static const struct b53_chip_data b53_switch_chips[] = { >> .enabled_ports = 0x1f, >> .arl_bins = 4, >> .arl_buckets = 1024, >> + .imp_port = 8, >> .cpu_port = B53_CPU_PORT, > > and B53_CPU_PORT here and for each entry below. > > I will put this patch into my local test rack and see what breaks, and we can address this more cleanly with net-next. Another case where if we had more time to do a proper review we could come up with a small fix, and not create additional technical debt to fix in the next release cycle. Hope's spring is eternal, oh and I just came back from France, so I guess I am full of complaints, too :) You should have visit Disneyland! ;)
On Mon, 6 Sep 2021 02:48:34 +0200 Andrew Lunn wrote: > > not allowing a proper review to happen. So please, I am begging you, wait at > > least 12h, ideally 24h before applying a patch. The fixed wait time before applying would likely require more nuance. For example something like 0h for build fixed; 12h if reviewed by all area experts; 24h+ for the rest? Not counting weekends? > 24 hours is too short. We all have lives outside of the kernel. I > found the older policy of 3 days worked well. Enough time for those > who had interest to do a review, but short enough to not really slow > down development. And 3 days is still probably faster than any other > subsystem. It is deeply unsatisfying tho to be waiting for reviews 3 days, ping people and then have to apply the patch anyway based on one's own judgment. I personally dislike the uncertainty of silently waiting. I have floated the idea before, perhaps it's not taken seriously given speed of patchwork development, but would it be okay to have a strict time bound and then require people to mark patches in patchwork as "I'm planning to review this"? Right now we make some attempts to delegate to "Needs ACK" state but with mixed result (see the two patches hanging in that state now). Perhaps the "Plan to review" marking in pw is also putting the cart before the horse (quite likely, knowing my project management prowess.) Either way if we're expending brain cycles on process changes it would be cool to think more broadly than just "how long to set a timer for".
On 07.09.2021 03:48, Jakub Kicinski wrote: > On Mon, 6 Sep 2021 02:48:34 +0200 Andrew Lunn wrote: >>> not allowing a proper review to happen. So please, I am begging you, wait at >>> least 12h, ideally 24h before applying a patch. > > The fixed wait time before applying would likely require more nuance. > For example something like 0h for build fixed; 12h if reviewed by all > area experts; 24h+ for the rest? Not counting weekends? As a maintainer I'd probably prefer 48 hours with 24 h being a minimum. >> 24 hours is too short. We all have lives outside of the kernel. I >> found the older policy of 3 days worked well. Enough time for those >> who had interest to do a review, but short enough to not really slow >> down development. And 3 days is still probably faster than any other >> subsystem. > > It is deeply unsatisfying tho to be waiting for reviews 3 days, ping > people and then have to apply the patch anyway based on one's own > judgment. I personally dislike the uncertainty of silently waiting. > I have floated the idea before, perhaps it's not taken seriously given > speed of patchwork development, but would it be okay to have a strict > time bound and then require people to mark patches in patchwork as > "I'm planning to review this"? > > Right now we make some attempts to delegate to "Needs ACK" state but > with mixed result (see the two patches hanging in that state now). > > Perhaps the "Plan to review" marking in pw is also putting the cart > before the horse (quite likely, knowing my project management prowess.) > Either way if we're expending brain cycles on process changes it would > be cool to think more broadly than just "how long to set a timer for". "Needs ACK" sounds good for a state where net maintainers need code maintainer ack/review. Do you already have some special meaning for the "Under Review"? That sounds (compared to the "New") like someone actually planning to (n)ack a patch.
On Mon, Sep 06, 2021 at 06:48:38PM -0700, Jakub Kicinski wrote: > On Mon, 6 Sep 2021 02:48:34 +0200 Andrew Lunn wrote: > > > not allowing a proper review to happen. So please, I am begging you, wait at > > > least 12h, ideally 24h before applying a patch. > > The fixed wait time before applying would likely require more nuance. > For example something like 0h for build fixed; 12h if reviewed by all > area experts; 24h+ for the rest? Not counting weekends? > > > 24 hours is too short. We all have lives outside of the kernel. I > > found the older policy of 3 days worked well. Enough time for those > > who had interest to do a review, but short enough to not really slow > > down development. And 3 days is still probably faster than any other > > subsystem. > > It is deeply unsatisfying tho to be waiting for reviews 3 days, ping > people and then have to apply the patch anyway based on one's own > judgment. I would skip the ping bit and just apply it. Unless you really think it needs a deep review. > Right now we make some attempts to delegate to "Needs ACK" state but > with mixed result (see the two patches hanging in that state now). > > Perhaps the "Plan to review" marking in pw is also putting the cart > before the horse. For me personally, the reason i like three days is that sometimes i'm away in the wilderness, visiting friends, away on business, etc. I'm not checking emails. So having to take some sort of action to say i'm not going to take any action until later, just defeaters the point. > Either way if we're expending brain cycles on process changes it would > be cool to think more broadly than just "how long to set a timer for". One observation is that netdev drivers are very siloed. A vendor driver rarely gets reviewed by another vendor. I think reviews are mostly made by vendor neutral people, and they look for specific things. I'm interested in phy, ethtool, and anything which looks like it could be adding a new KAPI of some sort. There are people who trigger on the keyword XDP, or BPF, devlink etc. Some areas of netdev do tend to get more reviews than others. Again, my areas of interest, phys, DSA, ethtool. The core stack gets reviewed by Eric, routing by David, and i'm sure there are others in parts i don't take an interest and i've not noticed. If a patch is from somebody who is not the maintainer of a siloed driver, and the maintainer is active, then a review is more likely to happen. So some sort of model could be made of this, to predict if a patchset is likely to get reviewed, if time is allowed for the reviewer to actually do the review. I don't know if a mental model is sufficient, for the two people who are merging patches, or some tools could be produced to help a little. Maybe just simple things like, is the poster of the patch series the maintain of the driver it applies to. Maybe some keyword matching? Maybe Sasha Levin can run a machine learning system over the last few years of the netdev archive to train a model which will product if a patchset will get reviewed? That would be applicable outside of netdev, it could be a useful feature in general. Maybe it is a research project Julia Lawall at Inria could give to a PhD student? Andrew
On Tue, 7 Sep 2021 07:35:09 +0200 Rafał Miłecki wrote: > Do you already have some special meaning for the "Under Review"? That > sounds (compared to the "New") like someone actually planning to (n)ack > a patch. Under Review means patch has passed the basic triage and will be applied to netdev trees unless instructed otherwise. Practically speaking it's not expected to go via any sub-tree and the patchwork build bot results look sane. At least that's what it used to mean, with the advances in automatic delegation it seems that Dave is using the state less these days, I still follow the old protocol. Sub-maintainers in netdev are historically asked not to change patchwork state. The review delegation does not really work great for netdev, since there can only be one delegate and we use it to split user space tools vs netdev vs bpf. A more flexible scheme where maintainer remains as a delegate but multiple reviewers can be attached would be great, that's what I was alluding to earlier.
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index 5646eb8afe38..604f54112665 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -1144,7 +1144,7 @@ static void b53_force_link(struct b53_device *dev, int port, int link) u8 reg, val, off; /* Override the port settings */ - if (port == dev->cpu_port) { + if (port == dev->imp_port) { off = B53_PORT_OVERRIDE_CTRL; val = PORT_OVERRIDE_EN; } else { @@ -1168,7 +1168,7 @@ static void b53_force_port_config(struct b53_device *dev, int port, u8 reg, val, off; /* Override the port settings */ - if (port == dev->cpu_port) { + if (port == dev->imp_port) { off = B53_PORT_OVERRIDE_CTRL; val = PORT_OVERRIDE_EN; } else { @@ -1236,7 +1236,7 @@ static void b53_adjust_link(struct dsa_switch *ds, int port, b53_force_link(dev, port, phydev->link); if (is531x5(dev) && phy_interface_is_rgmii(phydev)) { - if (port == 8) + if (port == dev->imp_port) off = B53_RGMII_CTRL_IMP; else off = B53_RGMII_CTRL_P(port); @@ -2280,6 +2280,7 @@ struct b53_chip_data { const char *dev_name; u16 vlans; u16 enabled_ports; + u8 imp_port; u8 cpu_port; u8 vta_regs[3]; u8 arl_bins; @@ -2304,6 +2305,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x1f, .arl_bins = 2, .arl_buckets = 1024, + .imp_port = 5, .cpu_port = B53_CPU_PORT_25, .duplex_reg = B53_DUPLEX_STAT_FE, }, @@ -2314,6 +2316,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x1f, .arl_bins = 2, .arl_buckets = 1024, + .imp_port = 5, .cpu_port = B53_CPU_PORT_25, .duplex_reg = B53_DUPLEX_STAT_FE, }, @@ -2324,6 +2327,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x1f, .arl_bins = 4, .arl_buckets = 1024, + .imp_port = 8, .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2337,6 +2341,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x1f, .arl_bins = 4, .arl_buckets = 1024, + .imp_port = 8, .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2350,6 +2355,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x1f, .arl_bins = 4, .arl_buckets = 1024, + .imp_port = 8, .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS_9798, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2363,6 +2369,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x7f, .arl_bins = 4, .arl_buckets = 1024, + .imp_port = 8, .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS_9798, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2377,6 +2384,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .vta_regs = B53_VTA_REGS, + .imp_port = 8, .cpu_port = B53_CPU_PORT, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2389,6 +2397,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0xff, .arl_bins = 4, .arl_buckets = 1024, + .imp_port = 8, .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2402,6 +2411,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x1ff, .arl_bins = 4, .arl_buckets = 1024, + .imp_port = 8, .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2415,6 +2425,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0, /* pdata must provide them */ .arl_bins = 4, .arl_buckets = 1024, + .imp_port = 8, .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS_63XX, .duplex_reg = B53_DUPLEX_STAT_63XX, @@ -2428,6 +2439,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x1f, .arl_bins = 4, .arl_buckets = 1024, + .imp_port = 8, .cpu_port = B53_CPU_PORT_25, /* TODO: auto detect */ .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2441,6 +2453,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x1bf, .arl_bins = 4, .arl_buckets = 1024, + .imp_port = 8, .cpu_port = B53_CPU_PORT_25, /* TODO: auto detect */ .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2454,6 +2467,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x1bf, .arl_bins = 4, .arl_buckets = 1024, + .imp_port = 8, .cpu_port = B53_CPU_PORT_25, /* TODO: auto detect */ .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2467,6 +2481,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x1f, .arl_bins = 4, .arl_buckets = 1024, + .imp_port = 8, .cpu_port = B53_CPU_PORT_25, /* TODO: auto detect */ .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2480,6 +2495,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x1f, .arl_bins = 4, .arl_buckets = 1024, + .imp_port = 8, .cpu_port = B53_CPU_PORT_25, /* TODO: auto detect */ .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2493,6 +2509,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x1ff, .arl_bins = 4, .arl_buckets = 1024, + .imp_port = 8, .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2506,6 +2523,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x103, .arl_bins = 4, .arl_buckets = 1024, + .imp_port = 8, .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2520,6 +2538,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x1bf, .arl_bins = 4, .arl_buckets = 256, + .imp_port = 8, .cpu_port = 8, /* TODO: ports 4, 5, 8 */ .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2533,6 +2552,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x1ff, .arl_bins = 4, .arl_buckets = 1024, + .imp_port = 8, .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2546,6 +2566,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .enabled_ports = 0x1ff, .arl_bins = 4, .arl_buckets = 256, + .imp_port = 8, .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, @@ -2571,6 +2592,7 @@ static int b53_switch_init(struct b53_device *dev) dev->vta_regs[1] = chip->vta_regs[1]; dev->vta_regs[2] = chip->vta_regs[2]; dev->jumbo_pm_reg = chip->jumbo_pm_reg; + dev->imp_port = chip->imp_port; dev->cpu_port = chip->cpu_port; dev->num_vlans = chip->vlans; dev->num_arl_bins = chip->arl_bins; diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h index 9bf8319342b0..5d068acf7cf8 100644 --- a/drivers/net/dsa/b53/b53_priv.h +++ b/drivers/net/dsa/b53/b53_priv.h @@ -123,6 +123,7 @@ struct b53_device { /* used ports mask */ u16 enabled_ports; + unsigned int imp_port; unsigned int cpu_port; /* connect specific data */