| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <VI1PR04MB5807D896858665A3F3C6418FF2909@VI1PR04MB5807.eurprd04.prod.outlook.com>
Date: Fri, 22 Jul 2022 12:43:41 +0000
From: Camelia Alexandra Groza <camelia.groza@....com>
To: Sean Anderson <sean.anderson@...o.com>,
"David S . Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Madalin Bucur <madalin.bucur@....com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: Paolo Abeni <pabeni@...hat.com>,
Eric Dumazet <edumazet@...gle.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
Russell King <linux@...linux.org.uk>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net-next v3 39/47] net: fman: memac: Add serdes support
> -----Original Message-----
> From: Sean Anderson <sean.anderson@...o.com>
> Sent: Thursday, July 21, 2022 18:38
> To: Camelia Alexandra Groza <camelia.groza@....com>; David S . Miller
> <davem@...emloft.net>; Jakub Kicinski <kuba@...nel.org>; Madalin Bucur
> <madalin.bucur@....com>; netdev@...r.kernel.org
> Cc: Paolo Abeni <pabeni@...hat.com>; Eric Dumazet
> <edumazet@...gle.com>; linux-arm-kernel@...ts.infradead.org; Russell
> King <linux@...linux.org.uk>; linux-kernel@...r.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@...o.com>
> >> Sent: Saturday, July 16, 2022 1:00
> >> To: David S . Miller <davem@...emloft.net>; Jakub Kicinski
> >> <kuba@...nel.org>; Madalin Bucur <madalin.bucur@....com>;
> >> netdev@...r.kernel.org
> >> Cc: Paolo Abeni <pabeni@...hat.com>; Eric Dumazet
> >> <edumazet@...gle.com>; linux-arm-kernel@...ts.infradead.org; Russell
> >> King <linux@...linux.org.uk>; linux-kernel@...r.kernel.org; Sean
> Anderson
> >> <sean.anderson@...o.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@...o.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
> >
Powered by blists - more mailing lists