| Message ID | 20210210164746.26336-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, 87 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 10.02.2021 17:47, 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. > > Split the combined IP101A/G driver into two separate drivers. > > Signed-off-by: Michael Walle <michael@walle.cc> > --- > Changes since v1: > - use match_phy_device() as suggested by Heiner > > Andrew, I've dropped your Reviewed-by because of this. > > drivers/net/phy/icplus.c | 69 ++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 67 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/phy/icplus.c b/drivers/net/phy/icplus.c > index 036bac628b11..1bc9baa9048f 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) > > @@ -301,6 +303,58 @@ static irqreturn_t ip101a_g_handle_interrupt(struct phy_device *phydev) > return IRQ_HANDLED; > } > > +static int ip101a_g_has_page_register(struct phy_device *phydev) > +{ > + int oldval, val, 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; > + > + val = phy_read(phydev, IP101G_PAGE_CONTROL); > + if (val < 0) > + return val; > + > + ret = phy_write(phydev, IP101G_PAGE_CONTROL, oldval); > + if (ret) > + return ret; > + > + return val == IP101G_PAGE_CONTROL_MASK; > +} > + > +static int ip101a_g_match_phy_device(struct phy_device *phydev, bool ip101a) > +{ > + int ret; > + > + if (phydev->phy_id != IP101A_PHY_ID) > + 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 there is a page select register, which is not > + * available on the IP101A. Use this to distinguish these two. > + */ > + ret = ip101a_g_has_page_register(phydev); > + if (ret < 0) > + return ret; > + > + return (ip101a) ? !ret : ret; Simpler would be: return ip101a == !ret; > +} > + > +static int ip101a_match_phy_device(struct phy_device *phydev) > +{ > + return ip101a_g_match_phy_device(phydev, true); > +} > + > +static int ip101g_match_phy_device(struct phy_device *phydev) > +{ > + return ip101a_g_match_phy_device(phydev, false); > +} > + > static struct phy_driver icplus_driver[] = { > { > PHY_ID_MATCH_MODEL(IP175C_PHY_ID), > @@ -320,8 +374,19 @@ static struct phy_driver icplus_driver[] = { > .suspend = genphy_suspend, > .resume = genphy_resume, > }, { > - PHY_ID_MATCH_EXACT(IP101A_PHY_ID), > - .name = "ICPlus IP101A/G", > + .name = "ICPlus IP101A", > + .match_phy_device = ip101a_match_phy_device, > + /* PHY_BASIC_FEATURES */ These comments have once been automatically added as part of a change in phy_driver structure. For new drivers they don't make too much sense. > + .probe = ip101a_g_probe, > + .config_intr = ip101a_g_config_intr, > + .handle_interrupt = ip101a_g_handle_interrupt, > + .config_init = ip101a_g_config_init, > + .soft_reset = genphy_soft_reset, > + .suspend = genphy_suspend, > + .resume = genphy_resume, > +}, { > + .name = "ICPlus IP101G", > + .match_phy_device = ip101g_match_phy_device, > /* PHY_BASIC_FEATURES */ > .probe = ip101a_g_probe, > .config_intr = ip101a_g_config_intr, >
diff --git a/drivers/net/phy/icplus.c b/drivers/net/phy/icplus.c index 036bac628b11..1bc9baa9048f 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) @@ -301,6 +303,58 @@ static irqreturn_t ip101a_g_handle_interrupt(struct phy_device *phydev) return IRQ_HANDLED; } +static int ip101a_g_has_page_register(struct phy_device *phydev) +{ + int oldval, val, 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; + + val = phy_read(phydev, IP101G_PAGE_CONTROL); + if (val < 0) + return val; + + ret = phy_write(phydev, IP101G_PAGE_CONTROL, oldval); + if (ret) + return ret; + + return val == IP101G_PAGE_CONTROL_MASK; +} + +static int ip101a_g_match_phy_device(struct phy_device *phydev, bool ip101a) +{ + int ret; + + if (phydev->phy_id != IP101A_PHY_ID) + 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 there is a page select register, which is not + * available on the IP101A. Use this to distinguish these two. + */ + ret = ip101a_g_has_page_register(phydev); + if (ret < 0) + return ret; + + return (ip101a) ? !ret : ret; +} + +static int ip101a_match_phy_device(struct phy_device *phydev) +{ + return ip101a_g_match_phy_device(phydev, true); +} + +static int ip101g_match_phy_device(struct phy_device *phydev) +{ + return ip101a_g_match_phy_device(phydev, false); +} + static struct phy_driver icplus_driver[] = { { PHY_ID_MATCH_MODEL(IP175C_PHY_ID), @@ -320,8 +374,19 @@ static struct phy_driver icplus_driver[] = { .suspend = genphy_suspend, .resume = genphy_resume, }, { - PHY_ID_MATCH_EXACT(IP101A_PHY_ID), - .name = "ICPlus IP101A/G", + .name = "ICPlus IP101A", + .match_phy_device = ip101a_match_phy_device, + /* PHY_BASIC_FEATURES */ + .probe = ip101a_g_probe, + .config_intr = ip101a_g_config_intr, + .handle_interrupt = ip101a_g_handle_interrupt, + .config_init = ip101a_g_config_init, + .soft_reset = genphy_soft_reset, + .suspend = genphy_suspend, + .resume = genphy_resume, +}, { + .name = "ICPlus IP101G", + .match_phy_device = ip101g_match_phy_device, /* PHY_BASIC_FEATURES */ .probe = ip101a_g_probe, .config_intr = ip101a_g_config_intr,
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. Split the combined IP101A/G driver into two separate drivers. Signed-off-by: Michael Walle <michael@walle.cc> --- Changes since v1: - use match_phy_device() as suggested by Heiner Andrew, I've dropped your Reviewed-by because of this. drivers/net/phy/icplus.c | 69 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 2 deletions(-)