Re: [PATCH v2] net: macb: add fixed-link node support

[Nicolas Ferre]

On 23/06/2017 at 16:54, Michael Grzeschik wrote:
> In case the MACB is directly connected to a
> non-mdio PHY/device, it should be possible to provide
> a fixed link configuration in the DT.
> 
> Signed-off-by: Michael Grzeschik 
> ---
>   v1 -> v2: - moved of_phy_connect case and phy_connect_direct into if else 
> cases
> - moved of_pphy_register_fixed_link into if(np) case
>   - added of_node_put(bp->phy_node); into macb_remove

Hi Michael,

by re-scanning my email backlog, I found this remark that may apply to
your patch (already in Linus' tree actually)...

> ---
>  drivers/net/ethernet/cadence/macb.c | 98 
> ++---
>  drivers/net/ethernet/cadence/macb.h |  1 +
>  2 files changed, 60 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.c 
> b/drivers/net/ethernet/cadence/macb.c
> index 91f7492623d3f..5781f1ede8c6e 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -425,32 +425,40 @@ static int macb_mii_probe(struct net_device *dev)
>   int phy_irq;
>   int ret;
>  
> - phydev = phy_find_first(bp->mii_bus);
> - if (!phydev) {
> - netdev_err(dev, "no PHY found\n");
> - return -ENXIO;
> - }
> + if (bp->phy_node) {
> + phydev = of_phy_connect(dev, bp->phy_node,
> + _handle_link_change, 0,
> + bp->phy_interface);
> + if (!phydev)
> + return -ENODEV;
> + } else {
> + phydev = phy_find_first(bp->mii_bus);
> + if (!phydev) {
> + netdev_err(dev, "no PHY found\n");
> + return -ENXIO;
> + }
>  
> - pdata = dev_get_platdata(>pdev->dev);
> - if (pdata) {
> - if (gpio_is_valid(pdata->phy_irq_pin)) {
> - ret = devm_gpio_request(>pdev->dev,
> - pdata->phy_irq_pin, "phy int");
> - if (!ret) {
> - phy_irq = gpio_to_irq(pdata->phy_irq_pin);
> - phydev->irq = (phy_irq < 0) ? PHY_POLL : 
> phy_irq;
> + pdata = dev_get_platdata(>pdev->dev);
> + if (pdata) {
> + if (gpio_is_valid(pdata->phy_irq_pin)) {
> + ret = devm_gpio_request(>pdev->dev,
> + pdata->phy_irq_pin, 
> "phy int");
> + if (!ret) {
> + phy_irq = 
> gpio_to_irq(pdata->phy_irq_pin);
> + phydev->irq = (phy_irq < 0) ? PHY_POLL 
> : phy_irq;
> + }
> + } else {
> + phydev->irq = PHY_POLL;
>   }
> - } else {
> - phydev->irq = PHY_POLL;
>   }
> - }
>  
> - /* attach the mac to the phy */
> - ret = phy_connect_direct(dev, phydev, _handle_link_change,
> -  bp->phy_interface);
> - if (ret) {
> - netdev_err(dev, "Could not attach to PHY\n");
> - return ret;
> + /* attach the mac to the phy */
> + ret = phy_connect_direct(dev, phydev, _handle_link_change,
> +  bp->phy_interface);
> + if (ret) {
> + netdev_err(dev, "Could not attach to PHY\n");
> + return ret;
> + }
>   }
>  
>   /* mask with MAC supported features */
> @@ -499,26 +507,37 @@ static int macb_mii_init(struct macb *bp)
>  
>   np = bp->pdev->dev.of_node;
>   if (np) {
> - /* try dt phy registration */
> - err = of_mdiobus_register(bp->mii_bus, np);
> + if (of_phy_is_fixed_link(np)) {
> + if (of_phy_register_fixed_link(np) < 0) {

remark done here by Andrew:
https://www.spinics.net/lists/netdev/msg421186.html

about missing of_phy_deregister_fixed_link().

Can you tell me if you would have time to have a look at it?


Thanks, best regards,
  Nicolas

> + dev_err(>pdev->dev,
> + "broken fixed-link specification\n");
> + goto err_out_unregister_bus;
> + }
> + bp->phy_node = of_node_get(np);
>  
> - /* fallback to standard phy registration if no phy were
> -  * found during dt phy registration
> -  */
> - if (!err && !phy_find_first(bp->mii_bus)) {
> - for (i = 0; i < PHY_MAX_ADDR; i++) {
> - struct phy_device *phydev;
> -
> - phydev = mdiobus_scan(bp->mii_bus, i);
> - 

Re: [PATCH v2] net: macb: add fixed-link node support

[David Miller]

From: Michael Grzeschik 
Date: Fri, 23 Jun 2017 16:54:10 +0200

> In case the MACB is directly connected to a
> non-mdio PHY/device, it should be possible to provide
> a fixed link configuration in the DT.
> 
> Signed-off-by: Michael Grzeschik 

Applied, thanks.

[PATCH v2] net: macb: add fixed-link node support

[Michael Grzeschik]

In case the MACB is directly connected to a
non-mdio PHY/device, it should be possible to provide
a fixed link configuration in the DT.

Signed-off-by: Michael Grzeschik 
---
  v1 -> v2: - moved of_phy_connect case and phy_connect_direct into if else 
cases
- moved of_pphy_register_fixed_link into if(np) case
- added of_node_put(bp->phy_node); into macb_remove
---
 drivers/net/ethernet/cadence/macb.c | 98 ++---
 drivers/net/ethernet/cadence/macb.h |  1 +
 2 files changed, 60 insertions(+), 39 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index 91f7492623d3f..5781f1ede8c6e 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -425,32 +425,40 @@ static int macb_mii_probe(struct net_device *dev)
int phy_irq;
int ret;
 
-   phydev = phy_find_first(bp->mii_bus);
-   if (!phydev) {
-   netdev_err(dev, "no PHY found\n");
-   return -ENXIO;
-   }
+   if (bp->phy_node) {
+   phydev = of_phy_connect(dev, bp->phy_node,
+   _handle_link_change, 0,
+   bp->phy_interface);
+   if (!phydev)
+   return -ENODEV;
+   } else {
+   phydev = phy_find_first(bp->mii_bus);
+   if (!phydev) {
+   netdev_err(dev, "no PHY found\n");
+   return -ENXIO;
+   }
 
-   pdata = dev_get_platdata(>pdev->dev);
-   if (pdata) {
-   if (gpio_is_valid(pdata->phy_irq_pin)) {
-   ret = devm_gpio_request(>pdev->dev,
-   pdata->phy_irq_pin, "phy int");
-   if (!ret) {
-   phy_irq = gpio_to_irq(pdata->phy_irq_pin);
-   phydev->irq = (phy_irq < 0) ? PHY_POLL : 
phy_irq;
+   pdata = dev_get_platdata(>pdev->dev);
+   if (pdata) {
+   if (gpio_is_valid(pdata->phy_irq_pin)) {
+   ret = devm_gpio_request(>pdev->dev,
+   pdata->phy_irq_pin, 
"phy int");
+   if (!ret) {
+   phy_irq = 
gpio_to_irq(pdata->phy_irq_pin);
+   phydev->irq = (phy_irq < 0) ? PHY_POLL 
: phy_irq;
+   }
+   } else {
+   phydev->irq = PHY_POLL;
}
-   } else {
-   phydev->irq = PHY_POLL;
}
-   }
 
-   /* attach the mac to the phy */
-   ret = phy_connect_direct(dev, phydev, _handle_link_change,
-bp->phy_interface);
-   if (ret) {
-   netdev_err(dev, "Could not attach to PHY\n");
-   return ret;
+   /* attach the mac to the phy */
+   ret = phy_connect_direct(dev, phydev, _handle_link_change,
+bp->phy_interface);
+   if (ret) {
+   netdev_err(dev, "Could not attach to PHY\n");
+   return ret;
+   }
}
 
/* mask with MAC supported features */
@@ -499,26 +507,37 @@ static int macb_mii_init(struct macb *bp)
 
np = bp->pdev->dev.of_node;
if (np) {
-   /* try dt phy registration */
-   err = of_mdiobus_register(bp->mii_bus, np);
+   if (of_phy_is_fixed_link(np)) {
+   if (of_phy_register_fixed_link(np) < 0) {
+   dev_err(>pdev->dev,
+   "broken fixed-link specification\n");
+   goto err_out_unregister_bus;
+   }
+   bp->phy_node = of_node_get(np);
 
-   /* fallback to standard phy registration if no phy were
-* found during dt phy registration
-*/
-   if (!err && !phy_find_first(bp->mii_bus)) {
-   for (i = 0; i < PHY_MAX_ADDR; i++) {
-   struct phy_device *phydev;
-
-   phydev = mdiobus_scan(bp->mii_bus, i);
-   if (IS_ERR(phydev) &&
-   PTR_ERR(phydev) != -ENODEV) {
-   err = PTR_ERR(phydev);
-   break;
+   err = mdiobus_register(bp->mii_bus);
+   } else {
+   /* try dt phy registration */
+   err = of_mdiobus_register(bp->mii_bus, np);
+
+   /* fallback to standard phy registration