Messages in this thread | | | Subject | Re: [PATCH net-next v3 39/47] net: fman: memac: Add serdes support | From | Sean Anderson <> | Date | Thu, 21 Jul 2022 11:38:06 -0400 |
| |
On 7/21/22 9:30 AM, Camelia Alexandra Groza wrote: >> -----Original Message----- >> From: Sean Anderson <sean.anderson@seco.com> >> Sent: Saturday, July 16, 2022 1:00 >> To: David S . Miller <davem@davemloft.net>; Jakub Kicinski >> <kuba@kernel.org>; Madalin Bucur <madalin.bucur@nxp.com>; >> netdev@vger.kernel.org >> Cc: Paolo Abeni <pabeni@redhat.com>; Eric Dumazet >> <edumazet@google.com>; linux-arm-kernel@lists.infradead.org; Russell >> King <linux@armlinux.org.uk>; linux-kernel@vger.kernel.org; Sean Anderson >> <sean.anderson@seco.com> >> Subject: [PATCH net-next v3 39/47] net: fman: memac: Add serdes support >> >> This adds support for using a serdes which has to be configured. This is >> primarly in preparation for the next commit, which will then change the >> serdes mode dynamically. >> >> Signed-off-by: Sean Anderson <sean.anderson@seco.com> >> --- >> >> (no changes since v1) >> >> .../net/ethernet/freescale/fman/fman_memac.c | 48 >> ++++++++++++++++++- >> 1 file changed, 46 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c >> b/drivers/net/ethernet/freescale/fman/fman_memac.c >> index 02b3a0a2d5d1..a62fe860b1d0 100644 >> --- a/drivers/net/ethernet/freescale/fman/fman_memac.c >> +++ b/drivers/net/ethernet/freescale/fman/fman_memac.c >> @@ -13,6 +13,7 @@ >> #include <linux/io.h> >> #include <linux/phy.h> >> #include <linux/phy_fixed.h> >> +#include <linux/phy/phy.h> >> #include <linux/of_mdio.h> >> >> /* PCS registers */ >> @@ -324,6 +325,7 @@ struct fman_mac { >> void *fm; >> struct fman_rev_info fm_rev_info; >> bool basex_if; >> + struct phy *serdes; >> struct phy_device *pcsphy; >> bool allmulti_enabled; >> }; >> @@ -1203,17 +1205,55 @@ int memac_initialization(struct mac_device >> *mac_dev, >> } >> } >> >> + memac->serdes = devm_of_phy_get(mac_dev->dev, mac_node, >> "serdes"); > > devm_of_phy_get returns -ENOSYS on PPC builds because CONFIG_GENERIC_PHY isn't > enabled by default. Please add a dependency. > >> + if (PTR_ERR(memac->serdes) == -ENODEV) {
I think it is better to add -ENOSYS to the condition here. That way, the phy subsystem stays optional.
--Sean
>> + memac->serdes = NULL; >> + } else if (IS_ERR(memac->serdes)) { >> + err = PTR_ERR(memac->serdes); >> + dev_err_probe(mac_dev->dev, err, "could not get >> serdes\n"); >> + goto _return_fm_mac_free; >> + } else { >> + err = phy_init(memac->serdes); >> + if (err) { >> + dev_err_probe(mac_dev->dev, err, >> + "could not initialize serdes\n"); >> + goto _return_fm_mac_free; >> + } >> + >> + err = phy_power_on(memac->serdes); >> + if (err) { >> + dev_err_probe(mac_dev->dev, err, >> + "could not power on serdes\n"); >> + goto _return_phy_exit; >> + } >> + >> + if (memac->phy_if == PHY_INTERFACE_MODE_SGMII || >> + memac->phy_if == PHY_INTERFACE_MODE_1000BASEX || >> + memac->phy_if == PHY_INTERFACE_MODE_2500BASEX || >> + memac->phy_if == PHY_INTERFACE_MODE_QSGMII || >> + memac->phy_if == PHY_INTERFACE_MODE_XGMII) { >> + err = phy_set_mode_ext(memac->serdes, >> PHY_MODE_ETHERNET, >> + memac->phy_if); >> + if (err) { >> + dev_err_probe(mac_dev->dev, err, >> + "could not set serdes mode >> to %s\n", >> + phy_modes(memac->phy_if)); >> + goto _return_phy_power_off; >> + } >> + } >> + } >> + >> if (!mac_dev->phy_node && of_phy_is_fixed_link(mac_node)) { >> struct phy_device *phy; >> >> err = of_phy_register_fixed_link(mac_node); >> if (err) >> - goto _return_fm_mac_free; >> + goto _return_phy_power_off; >> >> fixed_link = kzalloc(sizeof(*fixed_link), GFP_KERNEL); >> if (!fixed_link) { >> err = -ENOMEM; >> - goto _return_fm_mac_free; >> + goto _return_phy_power_off; >> } >> >> mac_dev->phy_node = of_node_get(mac_node); >> @@ -1242,6 +1282,10 @@ int memac_initialization(struct mac_device >> *mac_dev, >> >> goto _return; >> >> +_return_phy_power_off: >> + phy_power_off(memac->serdes); >> +_return_phy_exit: >> + phy_exit(memac->serdes); >> _return_fixed_link_free: >> kfree(fixed_link); > > _return_fixed_link_free should execute before _return_phy_power_off and _return_phy_exit > >> _return_fm_mac_free: >> -- >> 2.35.1.1320.gc452695387.dirty >
| |