On 09.02.2021 17:40, Michael Walle wrote:
> Unfortunately, the IP101A and IP101G share the same PHY identifier.
> While most of the functions are somewhat backwards compatible, there is
> for example the APS_EN bit on the IP101A but on the IP101G this bit
> reserved. Also, the IP101G has many more functionalities.
>
> Deduce the model by accessing the page select register which - according
> to the datasheet - is not available on the IP101A. If this register is
> writable, assume we have an IP101G.
>
> Signed-off-by: Michael Walle <mich...@walle.cc>
> ---
> drivers/net/phy/icplus.c | 43 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/icplus.c b/drivers/net/phy/icplus.c
> index 036bac628b11..189a9a34ed5f 100644
> --- a/drivers/net/phy/icplus.c
> +++ b/drivers/net/phy/icplus.c
> @@ -44,6 +44,8 @@ MODULE_LICENSE("GPL");
> #define IP101A_G_IRQ_DUPLEX_CHANGE BIT(1)
> #define IP101A_G_IRQ_LINK_CHANGE BIT(0)
>
> +#define IP101G_PAGE_CONTROL 0x14
> +#define IP101G_PAGE_CONTROL_MASK GENMASK(4, 0)
> #define IP101G_DIGITAL_IO_SPEC_CTRL 0x1d
> #define IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32 BIT(2)
>
> @@ -61,8 +63,14 @@ enum ip101gr_sel_intr32 {
> IP101GR_SEL_INTR32_RXER,
> };
>
> +enum ip101_model {
> + IP101A,
> + IP101G,
> +};
> +
> struct ip101a_g_phy_priv {
> enum ip101gr_sel_intr32 sel_intr32;
> + enum ip101_model model;
> };
>
> static int ip175c_config_init(struct phy_device *phydev)
> @@ -175,6 +183,39 @@ static int ip175c_config_aneg(struct phy_device *phydev)
> return 0;
> }
>
> +/* The IP101A and the IP101G share the same PHY identifier.The IP101G seems
> to
> + * be a successor of the IP101A and implements more functions. Amongst other
> + * things a page select register, which is not available on the IP101. Use
> this
> + * to distinguish these two.
> + */
> +static int ip101a_g_detect_model(struct phy_device *phydev)
> +{
> + struct ip101a_g_phy_priv *priv = phydev->priv;
> + int oldval, ret;
> +
> + oldval = phy_read(phydev, IP101G_PAGE_CONTROL);
> + if (oldval < 0)
> + return oldval;
> +
> + ret = phy_write(phydev, IP101G_PAGE_CONTROL, 0xffff);
> + if (ret)
> + return ret;
> +
> + ret = phy_read(phydev, IP101G_PAGE_CONTROL);
> + if (ret < 0)
> + return ret;
> +
> + if (ret == IP101G_PAGE_CONTROL_MASK)
> + priv->model = IP101G;
> + else
> + priv->model = IP101A;
> +
> + phydev_dbg(phydev, "Detected %s\n",
> + priv->model == IP101G ? "IP101G" : "IP101A");
> +
> + return phy_write(phydev, IP101G_PAGE_CONTROL, oldval);
> +}
> +
> static int ip101a_g_probe(struct phy_device *phydev)
> {
> struct device *dev = &phydev->mdio.dev;
> @@ -203,7 +244,7 @@ static int ip101a_g_probe(struct phy_device *phydev)
>
> phydev->priv = priv;
>
> - return 0;
> + return ip101a_g_detect_model(phydev);
> }
>
> static int ip101a_g_config_init(struct phy_device *phydev)
>
You could also implement the match_phy_device callback. Then you can
have separate PHY drivers for IP101A/IP101G. Would be cleaner I think.
See the Realtek PHY driver for an example.
