When the node for this phy selector is a child node of a syscon node then the
property 'reg' is used as an offset into the parent regmap. When the node
is standalone and gets its own regmap this offset is pre-applied. So we need
to track which method was used to get the regmap and not apply the offset
in the standalone case.

Fixes: 1fdfa7cccd35 ("phy: ti: gmii-sel: Allow parent to not be syscon node")
Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/phy/ti/phy-gmii-sel.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/ti/phy-gmii-sel.c b/drivers/phy/ti/phy-gmii-sel.c
index 555b323f45da1..bc847d3879f79 100644
--- a/drivers/phy/ti/phy-gmii-sel.c
+++ b/drivers/phy/ti/phy-gmii-sel.c
@@ -64,6 +64,7 @@ struct phy_gmii_sel_priv {
 	u32 num_ports;
 	u32 reg_offset;
 	u32 qsgmii_main_ports;
+	bool no_offset;
 };
 
 static int phy_gmii_sel_mode(struct phy *phy, enum phy_mode mode, int submode)
@@ -402,7 +403,8 @@ static int phy_gmii_sel_init_ports(struct phy_gmii_sel_priv *priv)
 		priv->num_ports = size / sizeof(u32);
 		if (!priv->num_ports)
 			return -EINVAL;
-		priv->reg_offset = __be32_to_cpu(*offset);
+		if (!priv->no_offset)
+			priv->reg_offset = __be32_to_cpu(*offset);
 	}
 
 	if_phys = devm_kcalloc(dev, priv->num_ports,
@@ -471,6 +473,7 @@ static int phy_gmii_sel_probe(struct platform_device *pdev)
 			dev_err(dev, "Failed to get syscon %d\n", ret);
 			return ret;
 		}
+		priv->no_offset = true;
 	}
 
 	ret = phy_gmii_sel_init_ports(priv);
-- 
2.39.2

On Wed, 25 Oct 2023 09:33:02 -0500, Andrew Davis wrote:
> When the node for this phy selector is a child node of a syscon node then the
> property 'reg' is used as an offset into the parent regmap. When the node
> is standalone and gets its own regmap this offset is pre-applied. So we need
> to track which method was used to get the regmap and not apply the offset
> in the standalone case.
> 
> 
> [...]

Applied, thanks!

[1/1] phy: ti: gmii-sel: Fix register offset when parent is not a syscon node
      commit: 0f40d5099cd6d828fd7de6227d3eabe86016724c

Best regards,
-- 
~Vinod

On 25/10/2023 17:33, Andrew Davis wrote:
> When the node for this phy selector is a child node of a syscon node then the
> property 'reg' is used as an offset into the parent regmap. When the node
> is standalone and gets its own regmap this offset is pre-applied. So we need
> to track which method was used to get the regmap and not apply the offset
> in the standalone case.
> 
> Fixes: 1fdfa7cccd35 ("phy: ti: gmii-sel: Allow parent to not be syscon node")
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/phy/ti/phy-gmii-sel.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/ti/phy-gmii-sel.c b/drivers/phy/ti/phy-gmii-sel.c
> index 555b323f45da1..bc847d3879f79 100644
> --- a/drivers/phy/ti/phy-gmii-sel.c
> +++ b/drivers/phy/ti/phy-gmii-sel.c
> @@ -64,6 +64,7 @@ struct phy_gmii_sel_priv {
>  	u32 num_ports;
>  	u32 reg_offset;
>  	u32 qsgmii_main_ports;
> +	bool no_offset;
>  };
>  
>  static int phy_gmii_sel_mode(struct phy *phy, enum phy_mode mode, int submode)
> @@ -402,7 +403,8 @@ static int phy_gmii_sel_init_ports(struct phy_gmii_sel_priv *priv)
>  		priv->num_ports = size / sizeof(u32);
>  		if (!priv->num_ports)
>  			return -EINVAL;
> -		priv->reg_offset = __be32_to_cpu(*offset);

nit: new line before new 'if' block please.

> +		if (!priv->no_offset)
> +			priv->reg_offset = __be32_to_cpu(*offset);
>  	}
>  
>  	if_phys = devm_kcalloc(dev, priv->num_ports,
> @@ -471,6 +473,7 @@ static int phy_gmii_sel_probe(struct platform_device *pdev)
>  			dev_err(dev, "Failed to get syscon %d\n", ret);
>  			return ret;
>  		}
> +		priv->no_offset = true;
>  	}
>  
>  	ret = phy_gmii_sel_init_ports(priv);


Reviewed-by: Roger Quadros <rogerq@kernel.org>