[SRU][J:linux-bluefield][PATCH v1 0/1] UBUNTU: SAUCE: mlxbf-gige: support fixed phy for Bobcat

[Bartlomiej Zolnierkiewicz]

Hi Asmaa,

The email title doesn't seem entirely correct ("...v1 0/1..").

On Mon, Feb 26, 2024 at 2:35 AM Asmaa Mnebhi <asmaa at nvidia.com> wrote:
>
> BugLink: https://bugs.launchpad.net/bugs/2054845
>
> There is no external PHY connected to the OOB MAC on the Bobcat
> board and no access to the MDIO bus. The OOB MAC is directly connected
> to the Marvell switch. So support a "fake PHY" and simulate an MDIO bus.
>
> If "fixed-link" property is supported in the ACPI table, register the
> fixed PHY driver.
>
> Signed-off-by: Asmaa Mnebhi <asmaa at nvidia.com>
> Reviewed-by: David Thompson <davthompson at nvidia.com>
> ---
>  .../ethernet/mellanox/mlxbf_gige/mlxbf_gige.h |  2 -
>  .../mellanox/mlxbf_gige/mlxbf_gige_main.c     | 58 ++++++++++++++++---
>  2 files changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h
> index a453b9cd9033..a85823a64d94 100644
> --- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h
> +++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h
> @@ -175,8 +175,6 @@ enum mlxbf_gige_res {
>  int mlxbf_gige_mdio_probe(struct platform_device *pdev,
>                           struct mlxbf_gige *priv);
>  void mlxbf_gige_mdio_remove(struct mlxbf_gige *priv);
> -irqreturn_t mlxbf_gige_mdio_handle_phy_interrupt(int irq, void *dev_id);
> -void mlxbf_gige_mdio_enable_phy_int(struct mlxbf_gige *priv);

The above change seems correct but please mention it in the patch description.

>  void mlxbf_gige_set_mac_rx_filter(struct mlxbf_gige *priv,
>                                   unsigned int index, u64 dmac);
> diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> index c0da9c05b12a..1f0e24e74e19 100644
> --- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> +++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> @@ -13,6 +13,7 @@
>  #include <linux/iopoll.h>
>  #include <linux/module.h>
>  #include <linux/phy.h>
> +#include <linux/phy_fixed.h>
>  #include <linux/platform_device.h>
>  #include <linux/skbuff.h>
>
> @@ -364,6 +365,7 @@ static struct mlxbf_gige_link_cfg mlxbf_gige_link_cfgs[] = {
>
>  static int mlxbf_gige_probe(struct platform_device *pdev)
>  {
> +       struct fixed_phy_status fphy_status = {};
>         struct phy_device *phydev;
>         struct net_device *netdev;
>         struct mlxbf_gige *priv;
> @@ -428,16 +430,49 @@ static int mlxbf_gige_probe(struct platform_device *pdev)
>         priv->rx_irq = platform_get_irq(pdev, MLXBF_GIGE_RECEIVE_PKT_INTR_IDX);
>         priv->llu_plu_irq = platform_get_irq(pdev, MLXBF_GIGE_LLU_PLU_INTR_IDX);
>
> -       phy_irq = acpi_dev_gpio_irq_get_by(ACPI_COMPANION(&pdev->dev), "phy-gpios", 0);
> -       if (phy_irq < 0) {
> -               dev_err(&pdev->dev, "Error getting PHY irq. Use polling instead");
> +       if (device_property_read_bool(&pdev->dev, "fixed-link")) {
> +               fphy_status.link = 1;
> +               err = device_property_read_u32(&pdev->dev, "full-duplex", &fphy_status.duplex);
> +               if (err) {
> +                       dev_err(&pdev->dev, "Failed to get duplex\n");
> +                       return -EINVAL;
> +               }
> +               err = device_property_read_u32(&pdev->dev, "speed", &fphy_status.speed);
> +               if (err) {
> +                       dev_err(&pdev->dev, "Failed to get speed\n");
> +                       return -EINVAL;
> +               }
> +               err = device_property_read_u32(&pdev->dev, "pause", &fphy_status.pause);
> +               if (err) {
> +                       dev_err(&pdev->dev, "Failed to get pause\n");
> +                       return -EINVAL;
> +               }
> +               err = device_property_read_u32(&pdev->dev, "asym-pause", &fphy_status.asym_pause);
> +               if (err) {
> +                       dev_err(&pdev->dev, "Failed to get asym-pause\n");
> +                       return -EINVAL;
> +               }
> +
> +               phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL);
> +               if (IS_ERR(phydev)) {
> +                       dev_err(&pdev->dev, "Failed to register fixed PHY device\n");
> +                       err = PTR_ERR(phydev);
> +                       goto out;

Shouldn't the driver also do the cleanup ("goto out;") on ACPI table
read errors?

--
Best regards,
Bartlomiej

> +               }
> +
>                 phy_irq = PHY_POLL;
> -       }
> +       } else {
> +               phy_irq = acpi_dev_gpio_irq_get_by(ACPI_COMPANION(&pdev->dev), "phy-gpios", 0);
> +               if (phy_irq < 0) {
> +                       dev_err(&pdev->dev, "Error getting PHY irq. Use polling instead");
> +                       phy_irq = PHY_POLL;
> +               }
>
> -       phydev = phy_find_first(priv->mdiobus);
> -       if (!phydev) {
> -               err = -ENODEV;
> -               goto out;
> +               phydev = phy_find_first(priv->mdiobus);
> +               if (!phydev) {
> +                       err = -ENODEV;
> +                       goto out;
> +               }
>         }
>
>         addr = phydev->mdio.addr;
> @@ -474,9 +509,14 @@ static int mlxbf_gige_probe(struct platform_device *pdev)
>  static int mlxbf_gige_remove(struct platform_device *pdev)
>  {
>         struct mlxbf_gige *priv = platform_get_drvdata(pdev);
> +       struct phy_device *phydev = priv->netdev->phydev;
>
>         unregister_netdev(priv->netdev);
> -       phy_disconnect(priv->netdev->phydev);
> +       phy_disconnect(phydev);
> +
> +       if (phy_is_pseudo_fixed_link(phydev))
> +               fixed_phy_unregister(phydev);
> +
>         mlxbf_gige_mdio_remove(priv);
>         platform_set_drvdata(pdev, NULL);
>