| Message ID | 20220715215954.1449214-40-sean.anderson@seco.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dpaa: Convert to phylink | expand |
> -----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) { > + 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
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 >
> -----Original Message----- > From: Sean Anderson <sean.anderson@seco.com> > Sent: Thursday, July 21, 2022 18:38 > To: Camelia Alexandra Groza <camelia.groza@nxp.com>; 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 > Subject: Re: [PATCH net-next v3 39/47] net: fman: memac: Add serdes > support > > > > 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 Sure, sounds good. > >> + 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 > >
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"); + if (PTR_ERR(memac->serdes) == -ENODEV) { + 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_fm_mac_free:
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(-)