| Message ID | 20220321152515.287119-1-andy.chiu@sifive.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v4,1/4] net: axienet: setup mdio unconditionally | expand |
On Mon, 2022-03-21 at 23:25 +0800, Andy Chiu wrote: > The call to axienet_mdio_setup should not depend on whether "phy-node" > pressents on the DT. Besides, since `lp->phy_node` is used if PHY is in > SGMII or 100Base-X modes, move it into the if statement. And the next patch > will remove `lp->phy_node` from driver's private structure and do an > of_node_put on it right away after use since it is not used elsewhere. > > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > Reviewed-by: Greentime Hu <greentime.hu@sifive.com> > --- > drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > index 6fd5157f0a6d..5d41b8de840a 100644 > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > @@ -2064,15 +2064,14 @@ static int axienet_probe(struct platform_device > *pdev) > if (ret) > goto cleanup_clk; > > - lp->phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0); > - if (lp->phy_node) { > - ret = axienet_mdio_setup(lp); > - if (ret) > - dev_warn(&pdev->dev, > - "error registering MDIO bus: %d\n", ret); > - } > + ret = axienet_mdio_setup(lp); > + if (ret) > + dev_warn(&pdev->dev, > + "error registering MDIO bus: %d\n", ret); > + > if (lp->phy_mode == PHY_INTERFACE_MODE_SGMII || > lp->phy_mode == PHY_INTERFACE_MODE_1000BASEX) { > + lp->phy_node = of_parse_phandle(pdev->dev.of_node, "phy- > handle", 0); > if (!lp->phy_node) { > dev_err(&pdev->dev, "phy-handle required for > 1000BaseX/SGMII\n"); > ret = -EINVAL; Reviewed-by: Robert Hancock <robert.hancock@calian.com>
Hi Andy
A patch series should always have a patch 0/X which explains the
overall purpose of the series. That then helps people understand how
the individual patches fit together.
Please also read the netdev FAQ. You are missing an indication of
which tree these patches are for.
Andrew
Thanks for pointing that out. The patchset I submitted is based on the net-next tree. But I think it should be on the net tree after reading the FAQ since it is a bug fix on DT handling. I will rework the patches and follow the format in a v5 patch. Andy
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 6fd5157f0a6d..5d41b8de840a 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -2064,15 +2064,14 @@ static int axienet_probe(struct platform_device *pdev) if (ret) goto cleanup_clk; - lp->phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0); - if (lp->phy_node) { - ret = axienet_mdio_setup(lp); - if (ret) - dev_warn(&pdev->dev, - "error registering MDIO bus: %d\n", ret); - } + ret = axienet_mdio_setup(lp); + if (ret) + dev_warn(&pdev->dev, + "error registering MDIO bus: %d\n", ret); + if (lp->phy_mode == PHY_INTERFACE_MODE_SGMII || lp->phy_mode == PHY_INTERFACE_MODE_1000BASEX) { + lp->phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0); if (!lp->phy_node) { dev_err(&pdev->dev, "phy-handle required for 1000BaseX/SGMII\n"); ret = -EINVAL;