Re: [PATCH net-next v6 6/9] net: dpaa: Convert to phylink
On Fri, Sep 30, 2022 at 04:09:30PM -0400, Sean Anderson wrote:
> +static void memac_validate(struct phylink_config *config,
> +unsigned long *supported,
> +struct phylink_link_state *state)
> +{
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> + struct fman_mac *memac = fman_config_to_mac(config)->fman_mac;
> +
> + phylink_generic_validate(config, supported, state);
> +
> + if (phy_interface_mode_is_rgmii(state->interface) &&
> + memac->rgmii_no_half_duplex) {
> + phylink_caps_to_linkmodes(mask, MAC_10HD | MAC_100HD);
> + linkmode_andnot(supported, supported, mask);
> + linkmode_andnot(state->advertising, state->advertising, mask);
> + }
> +}
Having been through the rest of this with a fine tooth comb, nothing
else stands out with the exception of the above, which I think could
be done better with this patch:
http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue=e65a47c4053255bd51715d5550e21c869971258c
Since the above would become:
static void memac_validate(struct phylink_config *config,
unsigned long *supported,
struct phylink_link_state *state)
{
struct mac_device *mac_dev = fman_config_to_mac(config);
struct fman_mac *memac = mac_dev->fman_mac;
unsigned long caps;
caps = mac_dev->phylink_config.capabilities;
if (phy_interface_mode_is_rgmii(state->interface) &&
memac->rgmii_no_half_duplex)
caps &= ~(MAC_10HD | MAC_100HD);
phylink_validate_mask_caps(supported, state, caps);
}
If you want to pick up my patch that adds this helper into your series,
please do.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next v6 6/9] net: dpaa: Convert to phylink
On Fri, Sep 30, 2022 at 04:09:30PM -0400, Sean Anderson wrote:
> @@ -1064,43 +1061,50 @@ static struct phylink_pcs *memac_pcs_create(struct
> device_node *mac_node,
> return pcs;
> }
>
> +static bool memac_supports(struct mac_device *mac_dev, phy_interface_t iface)
> +{
> + /* If there's no serdes device, assume that it's been configured for
> + * whatever the default interface mode is.
> + */
> + if (!mac_dev->fman_mac->serdes)
> + return mac_dev->phy_if == iface;
> + /* Otherwise, ask the serdes */
> + return !phy_validate(mac_dev->fman_mac->serdes, PHY_MODE_ETHERNET,
> + iface, NULL);
> +}
> +
> int memac_initialization(struct mac_device *mac_dev,
>struct device_node *mac_node,
>struct fman_mac_params *params)
> {
> int err;
> + struct device_node *fixed;
> struct phylink_pcs *pcs;
> - struct fixed_phy_status *fixed_link;
> struct fman_mac *memac;
> + unsigned longcapabilities;
> + unsigned long *supported;
>
> + mac_dev->phylink_ops= _mac_ops;
> mac_dev->set_promisc= memac_set_promiscuous;
> mac_dev->change_addr= memac_modify_mac_address;
> mac_dev->add_hash_mac_addr = memac_add_hash_mac_address;
> mac_dev->remove_hash_mac_addr = memac_del_hash_mac_address;
> - mac_dev->set_tx_pause = memac_set_tx_pause_frames;
> - mac_dev->set_rx_pause = memac_accept_rx_pause_frames;
> mac_dev->set_exception = memac_set_exception;
> mac_dev->set_allmulti = memac_set_allmulti;
> mac_dev->set_tstamp = memac_set_tstamp;
> mac_dev->set_multi = fman_set_multi;
> - mac_dev->adjust_link= adjust_link_memac;
> mac_dev->enable = memac_enable;
> mac_dev->disable= memac_disable;
>
> - if (params->max_speed == SPEED_1)
> - mac_dev->phy_if = PHY_INTERFACE_MODE_XGMII;
> -
> mac_dev->fman_mac = memac_config(mac_dev, params);
> - if (!mac_dev->fman_mac) {
> - err = -EINVAL;
> - goto _return;
> - }
> + if (!mac_dev->fman_mac)
> + return -EINVAL;
>
> memac = mac_dev->fman_mac;
> memac->memac_drv_param->max_frame_length = fman_get_max_frm();
> memac->memac_drv_param->reset_on_init = true;
>
> - err = of_property_match_string(mac_node, "pcs-names", "xfi");
> + err = of_property_match_string(mac_node, "pcs-handle-names", "xfi");
While reading through the patch, I stumbled upon this - in the previous
patch, you introduce this code with "pcs-names" and then in this patch
you change the name of the property. I don't think this was mentioned in
the commit message (searching it for "pcs" didn't reveal anything) so
I'm wondering whether this name update should've been merged into the
previous patch instead of this one?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next v6 6/9] net: dpaa: Convert to phylink
On 10/4/22 12:13 PM, Russell King (Oracle) wrote:
> On Fri, Sep 30, 2022 at 04:09:30PM -0400, Sean Anderson wrote:
>> @@ -1064,43 +1061,50 @@ static struct phylink_pcs *memac_pcs_create(struct
>> device_node *mac_node,
>> return pcs;
>> }
>>
>> +static bool memac_supports(struct mac_device *mac_dev, phy_interface_t
>> iface)
>> +{
>> +/* If there's no serdes device, assume that it's been configured for
>> + * whatever the default interface mode is.
>> + */
>> +if (!mac_dev->fman_mac->serdes)
>> +return mac_dev->phy_if == iface;
>> +/* Otherwise, ask the serdes */
>> +return !phy_validate(mac_dev->fman_mac->serdes, PHY_MODE_ETHERNET,
>> + iface, NULL);
>> +}
>> +
>> int memac_initialization(struct mac_device *mac_dev,
>> struct device_node *mac_node,
>> struct fman_mac_params *params)
>> {
>> int err;
>> +struct device_node *fixed;
>> struct phylink_pcs *pcs;
>> -struct fixed_phy_status *fixed_link;
>> struct fman_mac *memac;
>> +unsigned longcapabilities;
>> +unsigned long *supported;
>>
>> +mac_dev->phylink_ops= _mac_ops;
>> mac_dev->set_promisc= memac_set_promiscuous;
>> mac_dev->change_addr= memac_modify_mac_address;
>> mac_dev->add_hash_mac_addr = memac_add_hash_mac_address;
>> mac_dev->remove_hash_mac_addr = memac_del_hash_mac_address;
>> -mac_dev->set_tx_pause = memac_set_tx_pause_frames;
>> -mac_dev->set_rx_pause = memac_accept_rx_pause_frames;
>> mac_dev->set_exception = memac_set_exception;
>> mac_dev->set_allmulti = memac_set_allmulti;
>> mac_dev->set_tstamp = memac_set_tstamp;
>> mac_dev->set_multi = fman_set_multi;
>> -mac_dev->adjust_link= adjust_link_memac;
>> mac_dev->enable = memac_enable;
>> mac_dev->disable= memac_disable;
>>
>> -if (params->max_speed == SPEED_1)
>> -mac_dev->phy_if = PHY_INTERFACE_MODE_XGMII;
>> -
>> mac_dev->fman_mac = memac_config(mac_dev, params);
>> -if (!mac_dev->fman_mac) {
>> -err = -EINVAL;
>> -goto _return;
>> -}
>> +if (!mac_dev->fman_mac)
>> +return -EINVAL;
>>
>> memac = mac_dev->fman_mac;
>> memac->memac_drv_param->max_frame_length = fman_get_max_frm();
>> memac->memac_drv_param->reset_on_init = true;
>>
>> -err = of_property_match_string(mac_node, "pcs-names", "xfi");
>> +err = of_property_match_string(mac_node, "pcs-handle-names", "xfi");
>
> While reading through the patch, I stumbled upon this - in the previous
> patch, you introduce this code with "pcs-names" and then in this patch
> you change the name of the property. I don't think this was mentioned in
> the commit message (searching it for "pcs" didn't reveal anything) so
> I'm wondering whether this name update should've been merged into the
> previous patch instead of this one?
Yes, you're right. It looks like I applied this update to the wrong
patch. Will fix for v7.
--Sean
[PATCH net-next v6 6/9] net: dpaa: Convert to phylink
This converts DPAA to phylink. All macs are converted. This should work
with no device tree modifications (including those made in this series),
except for QSGMII (as noted previously).
The mEMAC configuration is one of the tricker areas. I have tried to
capture all the restrictions across the various models. Most of the time,
we assume that if the serdes supports a mode or the phy-interface-mode
specifies it, then we support it. The only place we can't do this is
(RG)MII, since there's no serdes. In that case, we rely on a (new)
devicetree property. There are also several cases where half-duplex is
broken. Unfortunately, only a single compatible is used for the MAC, so we
have to use the board compatible instead.
The 10GEC conversion is very straightforward, since it only supports XAUI.
There is generally nothing to configure.
The dTSEC conversion is broadly similar to mEMAC, but is simpler because we
don't support configuring the SerDes (though this can be easily added) and
we don't have multiple PCSs. From what I can tell, there's nothing
different in the driver or documentation between SGMII and 1000BASE-X
except for the advertising. Similarly, I couldn't find anything about
2500BASE-X. In both cases, I treat them like SGMII. These modes aren't used
by any in-tree boards. Similarly, despite being mentioned in the driver, I
couldn't find any documented SoCs which supported QSGMII. I have left it
unimplemented for now.
10GEC and dTSEC have not been tested at all. I would greatly appreciate if
someone could try them out.
Signed-off-by: Sean Anderson
---
This has been tested on an LS1046ARDB.
With managed=phy, I was unable to get the interfaces to come up at all,
hence the default to in-band.
Changes in v6:
- Fix uninitialized variable in dtsec_mac_config
Changes in v3:
- Remove _return label from memac_initialization in favor of returning
directly
- Fix grabbing the default PCS not checking for -ENODATA from
of_property_match_string
- Set DTSEC_ECNTRL_R100M in dtsec_link_up instead of dtsec_mac_config
- Remove rmii/mii properties
Changes in v2:
- Remove unused variable slow_10g_if
- Restrict valid link modes based on the phy interface. This is easier
to set up, and mostly captures what I intended to do the first time.
We now have a custom validate which restricts half-duplex for some SoCs
for RGMII, but generally just uses the default phylink validate.
- Configure the SerDes in enable/disable
- Properly implement all ethtool ops and ioctls. These were mostly
stubbed out just enough to compile last time.
- Convert 10GEC and dTSEC as well
drivers/net/ethernet/freescale/dpaa/Kconfig | 4 +-
.../net/ethernet/freescale/dpaa/dpaa_eth.c| 89 +--
.../ethernet/freescale/dpaa/dpaa_ethtool.c| 90 +--
drivers/net/ethernet/freescale/fman/Kconfig | 1 -
.../net/ethernet/freescale/fman/fman_dtsec.c | 460 +++---
.../net/ethernet/freescale/fman/fman_mac.h| 10 -
.../net/ethernet/freescale/fman/fman_memac.c | 578 +-
.../net/ethernet/freescale/fman/fman_tgec.c | 131 ++--
drivers/net/ethernet/freescale/fman/mac.c | 168 +
drivers/net/ethernet/freescale/fman/mac.h | 23 +-
10 files changed, 630 insertions(+), 924 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa/Kconfig
b/drivers/net/ethernet/freescale/dpaa/Kconfig
index 0e1439fd00bd..2b560661c82a 100644
--- a/drivers/net/ethernet/freescale/dpaa/Kconfig
+++ b/drivers/net/ethernet/freescale/dpaa/Kconfig
@@ -2,8 +2,8 @@
menuconfig FSL_DPAA_ETH
tristate "DPAA Ethernet"
depends on FSL_DPAA && FSL_FMAN
- select PHYLIB
- select FIXED_PHY
+ select PHYLINK
+ select PCS_LYNX
help
Data Path Acceleration Architecture Ethernet driver,
supporting the Freescale QorIQ chips.
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 31cfa121333d..021ba999d86d 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -264,8 +264,19 @@ static int dpaa_netdev_init(struct net_device *net_dev,
net_dev->needed_headroom = priv->tx_headroom;
net_dev->watchdog_timeo = msecs_to_jiffies(tx_timeout);
- mac_dev->net_dev = net_dev;
+ /* The rest of the config is filled in by the mac device already */
+ mac_dev->phylink_config.dev = _dev->dev;
+ mac_dev->phylink_config.type = PHYLINK_NETDEV;
mac_dev->update_speed = dpaa_eth_cgr_set_speed;
+ mac_dev->phylink = phylink_create(_dev->phylink_config,
+ dev_fwnode(mac_dev->dev),
+ mac_dev->phy_if,
+ mac_dev->phylink_ops);
+ if (IS_ERR(mac_dev->phylink)) {
+ err = PTR_ERR(mac_dev->phylink);
+ dev_err_probe(dev, err, "Could not create phylink\n");
+ return
