| Message ID | 20210209164051.18156-6-michael@walle.cc (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: icplus: cleanups and new features | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 6 of 6 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 69 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
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 <michael@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.
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)
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 <michael@walle.cc> --- drivers/net/phy/icplus.c | 43 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-)