drivers/net/phy: add driver for Microchip LAN867x 10BASE-T1S PHY
Subsystems (3)
PATCH v3 1/1 drivers/net/phy: add driver for Microchip LAN867x 10BASE-T1S PHY
Changes:
v2:
- Removed mentioning of not supporting auto-negotiation from commit
message
- Renamed file drivers/net/phy/lan867x.c ->
drivers/net/phy/microchip_t1s.c
- Renamed Kconfig option to reflect implementation filename (from
LAN867X_PHY to MICROCHIP_T1S_PHY)
- Moved entry in drivers/net/phy/KConfig to correct sort order
- Moved entry in drivers/net/phy/Makefile to correct sort order
- Moved variable declarations to conform to reverse christmas tree order
(in func lan867x_config_init)
- Moved register write to disable chip interrupts to func lan867x_config_init, when omitting the irq disable all togheter I got null pointer dereference, see the call trace below:
Call Trace:
<TASK>
phy_interrupt+0xa8/0xf0 [libphy]
irq_thread_fn+0x1c/0x60
irq_thread+0xf7/0x1c0
? __pfx_irq_thread_dtor+0x10/0x10
? __pfx_irq_thread+0x10/0x10
kthread+0xe6/0x110
? __pfx_kthread+0x10/0x10
ret_from_fork+0x29/0x50
</TASK>- Removed func lan867x_config_interrupt and removed the member .config_intr from the phy_driver struct
v3:
- Indentation level in drivers/net/phy/Kconfig
- Moved const arrays into global scope and made them static in order to have them placed in the .rodata section
- Renamed array variables, since they are no longer as closely scoped as earlier
- Added comment about why phy_write_mmd is used over phy_modify_mmd (this should have been addressed in the V2 change since it was brought up in the V1 review)
- Return result of last call instead of saving it in a var and then returning the var (in lan867x_config_init)
Testing:
This has been tested with ethtool --set/get-plca-cfg and verified on an oscilloscope where it was observed that:
- The PLCA beacon was enabled/disabled when setting the node-id to 0/not 0
- The PLCA beacon is transmitted with the expected frequency when changing max nodes
- Two devices using the evaluation board EVB-LAN8670-USB could ping each other
This patch adds support for the Microchip LAN867x 10BASE-T1S family (LAN8670/1/2). The driver supports P2MP with PLCA.
--- drivers/net/phy/Kconfig | 5 ++ drivers/net/phy/Makefile | 1 + drivers/net/phy/microchip_t1s.c | 138 ++++++++++++++++++++++++++++++++ 3 files changed, 144 insertions(+) create mode 100644 drivers/net/phy/microchip_t1s.c
drivers/net/phy/Kconfig +5 -0
@@ -235,6 +235,11 @@ config MICREL_PHY help Supports the KSZ9021, VSC8201, KS8001 PHYs. +config MICROCHIP_T1S_PHY + tristate "Microchip 10BASE-T1S Ethernet PHY" + help + Currently supports the LAN8670, LAN8671, LAN8672 + config MICROCHIP_PHY tristate "Microchip PHYs" help
drivers/net/phy/Makefile +1 -0
@@ -74,6 +74,7 @@ obj-$(CONFIG_MICREL_KS8995MA) += spi_ks8995.o obj-$(CONFIG_MICREL_PHY) += micrel.o obj-$(CONFIG_MICROCHIP_PHY) += microchip.o obj-$(CONFIG_MICROCHIP_T1_PHY) += microchip_t1.o +obj-$(CONFIG_MICROCHIP_T1S_PHY) += microchip_t1s.o obj-$(CONFIG_MICROSEMI_PHY) += mscc/ obj-$(CONFIG_MOTORCOMM_PHY) += motorcomm.o obj-$(CONFIG_NATIONAL_PHY) += national.o
drivers/net/phy/microchip_t1s.c new file +138 -0
138 lines not on this page · raw message
Signature · 1 line
2.39.2
Andrew Lunn
maintainerQuoted 52 lines · Ramón Nordin Rodriguez
Changes:
v2:- Removed mentioning of not supporting auto-negotiation from commit
message
- Renamed filedrivers/net/phy/lan867x.c->
drivers/net/phy/microchip_t1s.c
- RenamedKconfigoption to reflect implementation filename (from
LAN867X_PHYtoMICROCHIP_T1S_PHY)
- Moved entry indrivers/net/phy/KConfigto correct sort order
- Moved entry indrivers/net/phy/Makefileto correct sort order
- Moved variable declarations to conform to reverse christmas tree order
(in funclan867x_config_init)
- Moved register write to disable chip interrupts to funclan867x_config_init, when omitting the irq disable all togheter I got null pointer dereference, see the call trace below:Call Trace: <TASK> phy_interrupt+0xa8/0xf0 [libphy] irq_thread_fn+0x1c/0x60 irq_thread+0xf7/0x1c0 ? __pfx_irq_thread_dtor+0x10/0x10 ? __pfx_irq_thread+0x10/0x10 kthread+0xe6/0x110 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x29/0x50 </TASK>- Removed func
lan867x_config_interruptand removed the member .config_intr from thephy_driverstructv3:- Indentation level in
drivers/net/phy/Kconfig
- Moved const arrays into global scope and made them static in order to have them placed in the .rodata section
- Renamed array variables, since they are no longer as closely scoped as earlier
- Added comment about whyphy_write_mmdis used overphy_modify_mmd(this should have been addressed in the V2 change since it was brought up in the V1 review)
- Return result of last call instead of saving it in a var and then returning the var (inlan867x_config_init)Testing:
This has been tested with ethtool --set/get-plca-cfg and verified on an oscilloscope where it was observed that:
- The PLCA beacon was enabled/disabled when setting the node-id to 0/not 0
- The PLCA beacon is transmitted with the expected frequency when changing max nodes
- Two devices using the evaluation board EVB-LAN8670-USB could ping each other
Quoted 4 lines
This patch adds support for the Microchip LAN867x 10BASE-T1S family (LAN8670/1/2). The driver supports P2MP with PLCA.
Signed-off-by Ramón Nordin Rodriguez <[email protected]>
All the above ends up in the commit message, as you see in git log. So this last paragraph should really be first. As you have it, the history of the patch will also be included in the commit. Most Linux subsystems don't want that, although DaveM has argued it maybe should be in the commit message. But i personally think we have good archives of emails, and search engines for those archives, so i don't see the need.
Anything you put after the --- will get discarded when the Maintainers perform the merge. So i suggest you move most of what you have in the commit message below the ---. You can also have two ---, which is how i tend to do it, so i can keep the history in my git repo, but it then gets removed when merged upstream.
Additionally, you should add any Reviewed-by:, Acked-by: to your patches. Only discard them if you make major changed which invalidates any reviews.
Andrew
Quoted 58 lines · Andrew Lunn
Quoted 52 lines · Ramón Nordin Rodriguez
Changes:
v2:- Removed mentioning of not supporting auto-negotiation from commit
message
- Renamed filedrivers/net/phy/lan867x.c->
drivers/net/phy/microchip_t1s.c
- RenamedKconfigoption to reflect implementation filename (from
LAN867X_PHYtoMICROCHIP_T1S_PHY)
- Moved entry indrivers/net/phy/KConfigto correct sort order
- Moved entry indrivers/net/phy/Makefileto correct sort order
- Moved variable declarations to conform to reverse christmas tree order
(in funclan867x_config_init)
- Moved register write to disable chip interrupts to funclan867x_config_init, when omitting the irq disable all togheter I got null pointer dereference, see the call trace below:Call Trace: <TASK> phy_interrupt+0xa8/0xf0 [libphy] irq_thread_fn+0x1c/0x60 irq_thread+0xf7/0x1c0 ? __pfx_irq_thread_dtor+0x10/0x10 ? __pfx_irq_thread+0x10/0x10 kthread+0xe6/0x110 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x29/0x50 </TASK>- Removed func
lan867x_config_interruptand removed the member .config_intr from thephy_driverstructv3:- Indentation level in
drivers/net/phy/Kconfig
- Moved const arrays into global scope and made them static in order to have them placed in the .rodata section
- Renamed array variables, since they are no longer as closely scoped as earlier
- Added comment about whyphy_write_mmdis used overphy_modify_mmd(this should have been addressed in the V2 change since it was brought up in the V1 review)
- Return result of last call instead of saving it in a var and then returning the var (inlan867x_config_init)Testing:
This has been tested with ethtool --set/get-plca-cfg and verified on an oscilloscope where it was observed that:
- The PLCA beacon was enabled/disabled when setting the node-id to 0/not 0
- The PLCA beacon is transmitted with the expected frequency when changing max nodes
- Two devices using the evaluation board EVB-LAN8670-USB could ping each otherQuoted 4 lines
This patch adds support for the Microchip LAN867x 10BASE-T1S family (LAN8670/1/2). The driver supports P2MP with PLCA.
Signed-off-by Ramón Nordin Rodriguez <[email protected]>
Quoted 8 lines
All the above ends up in the commit message, as you see in git log. So this last paragraph should really be first. As you have it, the history of the patch will also be included in the commit. Most Linux subsystems don't want that, although DaveM has argued it maybe should be in the commit message. But i personally think we have good archives of emails, and search engines for those archives, so i don't see the need.
The intention was not to include the history in the commit message, as you allude to this was a misstake on my part.
Quoted 5 lines
Anything you put after the --- will get discarded when the Maintainers perform the merge. So i suggest you move most of what you have in the commit message below the ---. You can also have two ---, which is how i tend to do it, so i can keep the history in my git repo, but it then gets removed when merged upstream.
I'll move the history to after the ---
Quoted 6 lines
Additionally, you should add any Reviewed-by:, Acked-by: to your patches. Only discard them if you make major changed which invalidates any reviews.
Andrew
I will post a V4 soon as the 24h window closes on V4, with
- history after the patch
- reviewers:
- Andrew Lunn <[email protected]>
- Vladimir Oltean <[email protected]>
Hoping this is the last of the newbie misstakes I've managed to pull off.
Ramón