Messages in this thread | | | From | Paweł Dembicki <> | Date | Wed, 14 Feb 2024 13:56:10 +0100 | Subject | Re: [PATCH net-next v4 02/15] net: dsa: vsc73xx: convert to PHYLINK |
| |
śr., 14 lut 2024 o 00:19 Florian Fainelli <f.fainelli@gmail.com> napisał(a): > > On 2/13/24 14:03, Pawel Dembicki wrote: > > This patch replaces the adjust_link api with the phylink apis that provide > > equivalent functionality. > > > > The remaining functionality from the adjust_link is now covered in the > > phylink_mac_link_* and phylink_mac_config. > > > > Removes: > > .adjust_link > > Adds: > > .phylink_mac_config > > .phylink_mac_link_up > > .phylink_mac_link_down > > The implementation of phylink_mac_link_down() strictly mimics what had > been done by adjust_link() in the phydev->link == 0 case, but it really > makes me wonder whether some bits do not logically belong to > phylink_mac_link_up(), like "Accept packets again" for instance. > > Are we certain there was not an assumption before that we would get > adjust_link() called first with phydev->link = 0, and then phydev->link > =1 and that this specific sequence would program things just the way we > want?
Yes, it was the simplest conversion possible, without any improvements.
Some part is implementation of datasheet (description of ARBEMPTY register):
/* Discard packets */ vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0, VSC73XX_ARBDISC, BIT(port), BIT(port));
/* Wait until queue is empty */ ret = read_poll_timeout(vsc73xx_read, err, err < 0 || (val & BIT(port)), 1000, 10000, false, vsc, VSC73XX_BLOCK_ARBITER, 0, VSC73XX_ARBEMPTY, &val); if (ret) dev_err(vsc->dev, "timeout waiting for block arbiter\n"); else if (err < 0) dev_err(vsc->dev, "error reading arbiter\n");
/* Put this port into reset */ vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG, VSC73XX_MAC_CFG_RESET);
I agree that VSC73XX_ARBDISC should be moved to phylink_mac_link_up. Other things could be optimised and it needs more care. (eg. This implementation doesn't disable phy when the interface goes down.) I plan to tweak it after the driver becomes usable. Please let me know if it should be fixed in this patch.
-- Best Regards, Pawel Dembicki
| |