| Message ID | ZEFqFg9RO+Vsj8Kv@debian (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v4] drivers/net/phy: add driver for Microchip LAN867x 10BASE-T1S PHY | expand |
On Thu, Apr 20, 2023 at 06:36:38PM +0200, Ramón Nordin Rodriguez wrote: > 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 <ramon.nordin.rodriguez@ferroamp.se> > --- Hi Ramón So the history is in the wrong place. It should be here, under the --- I've no idea what 'git am' will make of this patch, if it will apply. You should try saving the email and applying the patch yourself and see if it works. And my Reviewed-by: has not been added. Andrew
On Thu, Apr 20, 2023 at 06:49:13PM +0200, Andrew Lunn wrote: > On Thu, Apr 20, 2023 at 06:36:38PM +0200, Ramón Nordin Rodriguez wrote: > > 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 <ramon.nordin.rodriguez@ferroamp.se> > > --- > > Hi Ramón > > So the history is in the wrong place. It should be here, under the --- > I've no idea what 'git am' will make of this patch, if it will > apply. You should try saving the email and applying the patch yourself > and see if it works. > > And my Reviewed-by: has not been added. > > Andrew Dang it, I thought I had done every misstake already :(
Hi Ramon, Sorry for the delay. Please find my comments below. Best Regards, Parthiban V On 20/04/23 10:06 pm, Ramón Nordin Rodriguez wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > 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 <ramon.nordin.rodriguez@ferroamp.se> > --- > 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 > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index 54874555c921..15a0bd95f12c 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -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 > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > index b5138066ba04..64f649f2f62f 100644 > --- a/drivers/net/phy/Makefile > +++ b/drivers/net/phy/Makefile > @@ -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 > diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c > new file mode 100644 > index 000000000000..b7fd5141a117 > --- /dev/null > +++ b/drivers/net/phy/microchip_t1s.c > @@ -0,0 +1,138 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Driver for Microchip 10BASE-T1S LAN867X PHY > + * > + * Support: Microchip Phys: > + * lan8670, lan8671, lan8672 > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/phy.h> > + > +#define PHY_ID_LAN867X 0x0007C160 Do you think it makes sense to probe the driver based on the Revision number as well? because we have two more revisions in the pipeline and each revision has different settings for the initial configuration. If you agree with this, as per the current datasheet this PHY revision is 2 means Rev.B1. If you like, I would suggest to use the PHY_ID define as below, #define PHY_ID_LAN867X_REVB1 0x0007C162 > + > +#define LAN867X_REG_IRQ_1_CTL 0x001C > +#define LAN867X_REG_IRQ_2_CTL 0x001D > + > +/* The arrays below are pulled from the following table from AN1699 > + * Access MMD Address Value Mask > + * RMW 0x1F 0x00D0 0x0002 0x0E03 > + * RMW 0x1F 0x00D1 0x0000 0x0300 > + * RMW 0x1F 0x0084 0x3380 0xFFC0 > + * RMW 0x1F 0x0085 0x0006 0x000F > + * RMW 0x1F 0x008A 0xC000 0xF800 > + * RMW 0x1F 0x0087 0x801C 0x801C > + * RMW 0x1F 0x0088 0x033F 0x1FFF > + * W 0x1F 0x008B 0x0404 ------ > + * RMW 0x1F 0x0080 0x0600 0x0600 > + * RMW 0x1F 0x00F1 0x2400 0x7F00 > + * RMW 0x1F 0x0096 0x2000 0x2000 > + * W 0x1F 0x0099 0x7F80 ------ > + */ > + > +static const int lan867x_fixup_registers[12] = { > + 0x00D0, 0x00D1, 0x0084, 0x0085, > + 0x008A, 0x0087, 0x0088, 0x008B, > + 0x0080, 0x00F1, 0x0096, 0x0099, > +}; > + > +static const int lan867x_fixup_values[12] = { > + 0x0002, 0x0000, 0x3380, 0x0006, > + 0xC000, 0x801C, 0x033F, 0x0404, > + 0x0600, 0x2400, 0x2000, 0x7F80, > +}; > + > +static const int lan867x_fixup_masks[12] = { > + 0x0E03, 0x0300, 0xFFC0, 0x000F, > + 0xF800, 0x801C, 0x1FFF, 0xFFFF, > + 0x0600, 0x7F00, 0x2000, 0xFFFF, > +}; > + > +static int lan867x_config_init(struct phy_device *phydev) > +{ > + /* HW quirk: Microchip states in the application note (AN1699) for the phy > + * that a set of read-modify-write (rmw) operations has to be performed > + * on a set of seemingly magic registers. > + * The result of these operations is just described as 'optimal performance' > + * Microchip gives no explanation as to what these mmd regs do, > + * in fact they are marked as reserved in the datasheet. > + * It is unclear if phy_modify_mmd would be safe to use or if a write > + * really has to happen to each register. > + * In order to exacly conform to what is stated in the AN phy_write_mmd is > + * used, which might then write the same value back as read + modified. > + */ > + > + int reg_value; > + int err; > + int reg; > + > + /* Read-Modified Write Pseudocode (from AN1699) > + * current_val = read_register(mmd, addr) // Read current register value > + * new_val = current_val AND (NOT mask) // Clear bit fields to be written > + * new_val = new_val OR value // Set bits > + * write_register(mmd, addr, new_val) // Write back updated register value > + */ > + for (int i = 0; i < ARRAY_SIZE(lan867x_fixup_registers); i++) { > + reg = lan867x_fixup_registers[i]; > + reg_value = phy_read_mmd(phydev, MDIO_MMD_VEND2, reg); > + reg_value &= ~lan867x_fixup_masks[i]; > + reg_value |= lan867x_fixup_values[i]; > + err = phy_write_mmd(phydev, MDIO_MMD_VEND2, reg, reg_value); > + if (err != 0) > + return err; > + } > + > + /* None of the interrupts in the lan867x phy seem relevant. > + * Other phys inspect the link status and call phy_trigger_machine > + * in the interrupt handler. > + * This phy does not support link status, and thus has no interrupt > + * for it either. > + * So we'll just disable all interrupts on the chip. > + */ I could see the below in the datasheet section 4.7. When the device is in a reset state, the IRQ_N interrupt pin is high-impedance and will be pulled high through an external pull-up resistor. Once all device reset sources are deasserted, the device will begin its internal initialization. The device will assert the Reset Complete (RESETC) bit in the Status 2 (STS2) register to indicate that it has completed its internal initialization and is ready for configuration. As the Reset Complete status is non-maskable, the IRQ_N pin will always be asserted and driven low following a device reset Do you think it makes sense to clear/acknowledge the "reset_done" interrupt by reading the Status 2 register in the interrupt routine? > + err = phy_write_mmd(phydev, MDIO_MMD_VEND2, LAN867X_REG_IRQ_1_CTL, 0xFFFF); > + if (err != 0) > + return err; > + return phy_write_mmd(phydev, MDIO_MMD_VEND2, LAN867X_REG_IRQ_2_CTL, 0xFFFF); > +} > + > +static int lan867x_read_status(struct phy_device *phydev) > +{ > + /* The phy has some limitations, namely: > + * - always reports link up > + * - only supports 10MBit half duplex > + * - does not support auto negotiate > + */ > + phydev->link = 1; > + phydev->duplex = DUPLEX_HALF; > + phydev->speed = SPEED_10; > + phydev->autoneg = AUTONEG_DISABLE; > + > + return 0; > +} > + > +static struct phy_driver lan867x_driver[] = { > + { > + PHY_ID_MATCH_MODEL(PHY_ID_LAN867X), If you agree with the above PHY_ID comment then this can be below, PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVB1), > + .name = "LAN867X", > + .features = PHY_BASIC_T1S_P2MP_FEATURES, > + .config_init = lan867x_config_init, > + .read_status = lan867x_read_status, > + .get_plca_cfg = genphy_c45_plca_get_cfg, > + .set_plca_cfg = genphy_c45_plca_set_cfg, > + .get_plca_status = genphy_c45_plca_get_status, > + } > +}; > + > +module_phy_driver(lan867x_driver); > + > +static struct mdio_device_id __maybe_unused tbl[] = { > + { PHY_ID_MATCH_MODEL(PHY_ID_LAN867X) }, If you agree with the above PHY_ID comment then this can be below, PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVB1) > + { } > +}; > + > +MODULE_DEVICE_TABLE(mdio, tbl); > + > +MODULE_DESCRIPTION("Microchip 10BASE-T1S lan867x Phy driver"); > +MODULE_AUTHOR("Ramón Nordin Rodriguez"); > +MODULE_LICENSE("GPL"); > -- > 2.39.2 > > 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) > > v4: > - Moved history out of commit message > > 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
On Fri, Apr 21, 2023 at 06:44:02AM +0000, Parthiban.Veerasooran@microchip.com wrote: > Hi Ramon, > > Sorry for the delay. Please find my comments below. > > Best Regards, > Parthiban V > > On 20/04/23 10:06 pm, Ramón Nordin Rodriguez wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > 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 <ramon.nordin.rodriguez@ferroamp.se> > > --- > > 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 > > > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > > index 54874555c921..15a0bd95f12c 100644 > > --- a/drivers/net/phy/Kconfig > > +++ b/drivers/net/phy/Kconfig > > @@ -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 > > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > > index b5138066ba04..64f649f2f62f 100644 > > --- a/drivers/net/phy/Makefile > > +++ b/drivers/net/phy/Makefile > > @@ -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 > > diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c > > new file mode 100644 > > index 000000000000..b7fd5141a117 > > --- /dev/null > > +++ b/drivers/net/phy/microchip_t1s.c > > @@ -0,0 +1,138 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Driver for Microchip 10BASE-T1S LAN867X PHY > > + * > > + * Support: Microchip Phys: > > + * lan8670, lan8671, lan8672 > > + */ > > + > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/phy.h> > > + > > +#define PHY_ID_LAN867X 0x0007C160 > Do you think it makes sense to probe the driver based on the Revision > number as well? because we have two more revisions in the pipeline and > each revision has different settings for the initial configuration. If > you agree with this, as per the current datasheet this PHY revision is 2 > means Rev.B1. If you like, I would suggest to use the PHY_ID define as > below, > #define PHY_ID_LAN867X_REVB1 0x0007C162 Personally I'd save that for a later patch that also implements the initial config for the different revisions. Can't speak for what's common practice in netdev though. > > + > > +#define LAN867X_REG_IRQ_1_CTL 0x001C > > +#define LAN867X_REG_IRQ_2_CTL 0x001D > > + > > +/* The arrays below are pulled from the following table from AN1699 > > + * Access MMD Address Value Mask > > + * RMW 0x1F 0x00D0 0x0002 0x0E03 > > + * RMW 0x1F 0x00D1 0x0000 0x0300 > > + * RMW 0x1F 0x0084 0x3380 0xFFC0 > > + * RMW 0x1F 0x0085 0x0006 0x000F > > + * RMW 0x1F 0x008A 0xC000 0xF800 > > + * RMW 0x1F 0x0087 0x801C 0x801C > > + * RMW 0x1F 0x0088 0x033F 0x1FFF > > + * W 0x1F 0x008B 0x0404 ------ > > + * RMW 0x1F 0x0080 0x0600 0x0600 > > + * RMW 0x1F 0x00F1 0x2400 0x7F00 > > + * RMW 0x1F 0x0096 0x2000 0x2000 > > + * W 0x1F 0x0099 0x7F80 ------ > > + */ > > + > > +static const int lan867x_fixup_registers[12] = { > > + 0x00D0, 0x00D1, 0x0084, 0x0085, > > + 0x008A, 0x0087, 0x0088, 0x008B, > > + 0x0080, 0x00F1, 0x0096, 0x0099, > > +}; > > + > > +static const int lan867x_fixup_values[12] = { > > + 0x0002, 0x0000, 0x3380, 0x0006, > > + 0xC000, 0x801C, 0x033F, 0x0404, > > + 0x0600, 0x2400, 0x2000, 0x7F80, > > +}; > > + > > +static const int lan867x_fixup_masks[12] = { > > + 0x0E03, 0x0300, 0xFFC0, 0x000F, > > + 0xF800, 0x801C, 0x1FFF, 0xFFFF, > > + 0x0600, 0x7F00, 0x2000, 0xFFFF, > > +}; > > + > > +static int lan867x_config_init(struct phy_device *phydev) > > +{ > > + /* HW quirk: Microchip states in the application note (AN1699) for the phy > > + * that a set of read-modify-write (rmw) operations has to be performed > > + * on a set of seemingly magic registers. > > + * The result of these operations is just described as 'optimal performance' > > + * Microchip gives no explanation as to what these mmd regs do, > > + * in fact they are marked as reserved in the datasheet. > > + * It is unclear if phy_modify_mmd would be safe to use or if a write > > + * really has to happen to each register. > > + * In order to exacly conform to what is stated in the AN phy_write_mmd is > > + * used, which might then write the same value back as read + modified. > > + */ > > + > > + int reg_value; > > + int err; > > + int reg; > > + > > + /* Read-Modified Write Pseudocode (from AN1699) > > + * current_val = read_register(mmd, addr) // Read current register value > > + * new_val = current_val AND (NOT mask) // Clear bit fields to be written > > + * new_val = new_val OR value // Set bits > > + * write_register(mmd, addr, new_val) // Write back updated register value > > + */ > > + for (int i = 0; i < ARRAY_SIZE(lan867x_fixup_registers); i++) { > > + reg = lan867x_fixup_registers[i]; > > + reg_value = phy_read_mmd(phydev, MDIO_MMD_VEND2, reg); > > + reg_value &= ~lan867x_fixup_masks[i]; > > + reg_value |= lan867x_fixup_values[i]; > > + err = phy_write_mmd(phydev, MDIO_MMD_VEND2, reg, reg_value); > > + if (err != 0) > > + return err; > > + } > > + > > + /* None of the interrupts in the lan867x phy seem relevant. > > + * Other phys inspect the link status and call phy_trigger_machine > > + * in the interrupt handler. > > + * This phy does not support link status, and thus has no interrupt > > + * for it either. > > + * So we'll just disable all interrupts on the chip. > > + */ > I could see the below in the datasheet section 4.7. > > When the device is in a reset state, the IRQ_N interrupt pin is > high-impedance and will be pulled high through an > external pull-up resistor. Once all device reset sources are deasserted, > the device will begin its internal initialization. > The device will assert the Reset Complete (RESETC) bit in the Status 2 > (STS2) register to indicate that it has > completed its internal initialization and is ready for configuration. As > the Reset Complete status is non-maskable, the > IRQ_N pin will always be asserted and driven low following a device reset > > Do you think it makes sense to clear/acknowledge the "reset_done" > interrupt by reading the Status 2 register in the interrupt routine? > I don't think it's necessary when not populating the handle_interrupt member in the phy_driver struct. If there were any other interrupts we could act upon I'd say differently. IMHO the interruts on the chip are interesting, but it's elusive to me how to handle any of them gracefully in kernelspace. Say the 'unexpected beacon' interrupt for example, meaning two or more nodes have id 0 (i.e. PLCA coordinator), any of them can detect the collision, but which one yields. The desired result here would be different in different products/applications. Then this is probably just useful for debugging and it's easy to spot with an oscilloscope or signal analyser, so probably overkill to support debugging of the network from the driver. > > + err = phy_write_mmd(phydev, MDIO_MMD_VEND2, LAN867X_REG_IRQ_1_CTL, 0xFFFF); > > + if (err != 0) > > + return err; > > + return phy_write_mmd(phydev, MDIO_MMD_VEND2, LAN867X_REG_IRQ_2_CTL, 0xFFFF); > > +} > > + > > +static int lan867x_read_status(struct phy_device *phydev) > > +{ > > + /* The phy has some limitations, namely: > > + * - always reports link up > > + * - only supports 10MBit half duplex > > + * - does not support auto negotiate > > + */ > > + phydev->link = 1; > > + phydev->duplex = DUPLEX_HALF; > > + phydev->speed = SPEED_10; > > + phydev->autoneg = AUTONEG_DISABLE; > > + > > + return 0; > > +} > > + > > +static struct phy_driver lan867x_driver[] = { > > + { > > + PHY_ID_MATCH_MODEL(PHY_ID_LAN867X), > If you agree with the above PHY_ID comment then this can be below, > PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVB1), As stated earlier I think this can be added in a later patch with more functionality tied together. > > + .name = "LAN867X", > > + .features = PHY_BASIC_T1S_P2MP_FEATURES, > > + .config_init = lan867x_config_init, > > + .read_status = lan867x_read_status, > > + .get_plca_cfg = genphy_c45_plca_get_cfg, > > + .set_plca_cfg = genphy_c45_plca_set_cfg, > > + .get_plca_status = genphy_c45_plca_get_status, > > + } > > +}; > > + > > +module_phy_driver(lan867x_driver); > > + > > +static struct mdio_device_id __maybe_unused tbl[] = { > > + { PHY_ID_MATCH_MODEL(PHY_ID_LAN867X) }, > If you agree with the above PHY_ID comment then this can be below, > PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVB1) See the comment above Let me know if you disagree with my reasoning R > > + { } > > +}; > > + > > +MODULE_DEVICE_TABLE(mdio, tbl); > > + > > +MODULE_DESCRIPTION("Microchip 10BASE-T1S lan867x Phy driver"); > > +MODULE_AUTHOR("Ramón Nordin Rodriguez"); > > +MODULE_LICENSE("GPL"); > > -- > > 2.39.2 > > > > 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) > > > > v4: > > - Moved history out of commit message > > > > 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 >
Hi Ramon, Thanks for the clarification. Best Regards, Parthiban V On 21/04/23 12:59 pm, Ramón Nordin Rodriguez wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Fri, Apr 21, 2023 at 06:44:02AM +0000, Parthiban.Veerasooran@microchip.com wrote: >> Hi Ramon, >> >> Sorry for the delay. Please find my comments below. >> >> Best Regards, >> Parthiban V >> >> On 20/04/23 10:06 pm, Ramón Nordin Rodriguez wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> 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 <ramon.nordin.rodriguez@ferroamp.se> >>> --- >>> 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 >>> >>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig >>> index 54874555c921..15a0bd95f12c 100644 >>> --- a/drivers/net/phy/Kconfig >>> +++ b/drivers/net/phy/Kconfig >>> @@ -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 >>> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile >>> index b5138066ba04..64f649f2f62f 100644 >>> --- a/drivers/net/phy/Makefile >>> +++ b/drivers/net/phy/Makefile >>> @@ -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 >>> diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c >>> new file mode 100644 >>> index 000000000000..b7fd5141a117 >>> --- /dev/null >>> +++ b/drivers/net/phy/microchip_t1s.c >>> @@ -0,0 +1,138 @@ >>> +// SPDX-License-Identifier: GPL-2.0+ >>> +/* >>> + * Driver for Microchip 10BASE-T1S LAN867X PHY >>> + * >>> + * Support: Microchip Phys: >>> + * lan8670, lan8671, lan8672 >>> + */ >>> + >>> +#include <linux/kernel.h> >>> +#include <linux/module.h> >>> +#include <linux/phy.h> >>> + >>> +#define PHY_ID_LAN867X 0x0007C160 >> Do you think it makes sense to probe the driver based on the Revision >> number as well? because we have two more revisions in the pipeline and >> each revision has different settings for the initial configuration. If >> you agree with this, as per the current datasheet this PHY revision is 2 >> means Rev.B1. If you like, I would suggest to use the PHY_ID define as >> below, >> #define PHY_ID_LAN867X_REVB1 0x0007C162 > > Personally I'd save that for a later patch that also implements the > initial config for the different revisions. > Can't speak for what's common practice in netdev though. > >>> + >>> +#define LAN867X_REG_IRQ_1_CTL 0x001C >>> +#define LAN867X_REG_IRQ_2_CTL 0x001D >>> + >>> +/* The arrays below are pulled from the following table from AN1699 >>> + * Access MMD Address Value Mask >>> + * RMW 0x1F 0x00D0 0x0002 0x0E03 >>> + * RMW 0x1F 0x00D1 0x0000 0x0300 >>> + * RMW 0x1F 0x0084 0x3380 0xFFC0 >>> + * RMW 0x1F 0x0085 0x0006 0x000F >>> + * RMW 0x1F 0x008A 0xC000 0xF800 >>> + * RMW 0x1F 0x0087 0x801C 0x801C >>> + * RMW 0x1F 0x0088 0x033F 0x1FFF >>> + * W 0x1F 0x008B 0x0404 ------ >>> + * RMW 0x1F 0x0080 0x0600 0x0600 >>> + * RMW 0x1F 0x00F1 0x2400 0x7F00 >>> + * RMW 0x1F 0x0096 0x2000 0x2000 >>> + * W 0x1F 0x0099 0x7F80 ------ >>> + */ >>> + >>> +static const int lan867x_fixup_registers[12] = { >>> + 0x00D0, 0x00D1, 0x0084, 0x0085, >>> + 0x008A, 0x0087, 0x0088, 0x008B, >>> + 0x0080, 0x00F1, 0x0096, 0x0099, >>> +}; >>> + >>> +static const int lan867x_fixup_values[12] = { >>> + 0x0002, 0x0000, 0x3380, 0x0006, >>> + 0xC000, 0x801C, 0x033F, 0x0404, >>> + 0x0600, 0x2400, 0x2000, 0x7F80, >>> +}; >>> + >>> +static const int lan867x_fixup_masks[12] = { >>> + 0x0E03, 0x0300, 0xFFC0, 0x000F, >>> + 0xF800, 0x801C, 0x1FFF, 0xFFFF, >>> + 0x0600, 0x7F00, 0x2000, 0xFFFF, >>> +}; >>> + >>> +static int lan867x_config_init(struct phy_device *phydev) >>> +{ >>> + /* HW quirk: Microchip states in the application note (AN1699) for the phy >>> + * that a set of read-modify-write (rmw) operations has to be performed >>> + * on a set of seemingly magic registers. >>> + * The result of these operations is just described as 'optimal performance' >>> + * Microchip gives no explanation as to what these mmd regs do, >>> + * in fact they are marked as reserved in the datasheet. >>> + * It is unclear if phy_modify_mmd would be safe to use or if a write >>> + * really has to happen to each register. >>> + * In order to exacly conform to what is stated in the AN phy_write_mmd is >>> + * used, which might then write the same value back as read + modified. >>> + */ >>> + >>> + int reg_value; >>> + int err; >>> + int reg; >>> + >>> + /* Read-Modified Write Pseudocode (from AN1699) >>> + * current_val = read_register(mmd, addr) // Read current register value >>> + * new_val = current_val AND (NOT mask) // Clear bit fields to be written >>> + * new_val = new_val OR value // Set bits >>> + * write_register(mmd, addr, new_val) // Write back updated register value >>> + */ >>> + for (int i = 0; i < ARRAY_SIZE(lan867x_fixup_registers); i++) { >>> + reg = lan867x_fixup_registers[i]; >>> + reg_value = phy_read_mmd(phydev, MDIO_MMD_VEND2, reg); >>> + reg_value &= ~lan867x_fixup_masks[i]; >>> + reg_value |= lan867x_fixup_values[i]; >>> + err = phy_write_mmd(phydev, MDIO_MMD_VEND2, reg, reg_value); >>> + if (err != 0) >>> + return err; >>> + } >>> + >>> + /* None of the interrupts in the lan867x phy seem relevant. >>> + * Other phys inspect the link status and call phy_trigger_machine >>> + * in the interrupt handler. >>> + * This phy does not support link status, and thus has no interrupt >>> + * for it either. >>> + * So we'll just disable all interrupts on the chip. >>> + */ >> I could see the below in the datasheet section 4.7. >> >> When the device is in a reset state, the IRQ_N interrupt pin is >> high-impedance and will be pulled high through an >> external pull-up resistor. Once all device reset sources are deasserted, >> the device will begin its internal initialization. >> The device will assert the Reset Complete (RESETC) bit in the Status 2 >> (STS2) register to indicate that it has >> completed its internal initialization and is ready for configuration. As >> the Reset Complete status is non-maskable, the >> IRQ_N pin will always be asserted and driven low following a device reset >> >> Do you think it makes sense to clear/acknowledge the "reset_done" >> interrupt by reading the Status 2 register in the interrupt routine? >> > > I don't think it's necessary when not populating the handle_interrupt > member in the phy_driver struct. > If there were any other interrupts we could act upon I'd say > differently. > > IMHO the interruts on the chip are interesting, but it's elusive to me > how to handle any of them gracefully in kernelspace. Say the 'unexpected beacon' > interrupt for example, meaning two or more nodes have id 0 (i.e. PLCA > coordinator), any of them can detect the collision, but which one > yields. The desired result here would be different in different > products/applications. > Then this is probably just useful for debugging and it's easy to spot > with an oscilloscope or signal analyser, so probably overkill to support > debugging of the network from the driver. > >>> + err = phy_write_mmd(phydev, MDIO_MMD_VEND2, LAN867X_REG_IRQ_1_CTL, 0xFFFF); >>> + if (err != 0) >>> + return err; >>> + return phy_write_mmd(phydev, MDIO_MMD_VEND2, LAN867X_REG_IRQ_2_CTL, 0xFFFF); >>> +} >>> + >>> +static int lan867x_read_status(struct phy_device *phydev) >>> +{ >>> + /* The phy has some limitations, namely: >>> + * - always reports link up >>> + * - only supports 10MBit half duplex >>> + * - does not support auto negotiate >>> + */ >>> + phydev->link = 1; >>> + phydev->duplex = DUPLEX_HALF; >>> + phydev->speed = SPEED_10; >>> + phydev->autoneg = AUTONEG_DISABLE; >>> + >>> + return 0; >>> +} >>> + >>> +static struct phy_driver lan867x_driver[] = { >>> + { >>> + PHY_ID_MATCH_MODEL(PHY_ID_LAN867X), >> If you agree with the above PHY_ID comment then this can be below, >> PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVB1), > > As stated earlier I think this can be added in a later patch with more > functionality tied together. > >>> + .name = "LAN867X", >>> + .features = PHY_BASIC_T1S_P2MP_FEATURES, >>> + .config_init = lan867x_config_init, >>> + .read_status = lan867x_read_status, >>> + .get_plca_cfg = genphy_c45_plca_get_cfg, >>> + .set_plca_cfg = genphy_c45_plca_set_cfg, >>> + .get_plca_status = genphy_c45_plca_get_status, >>> + } >>> +}; >>> + >>> +module_phy_driver(lan867x_driver); >>> + >>> +static struct mdio_device_id __maybe_unused tbl[] = { >>> + { PHY_ID_MATCH_MODEL(PHY_ID_LAN867X) }, >> If you agree with the above PHY_ID comment then this can be below, >> PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVB1) > > See the comment above > > Let me know if you disagree with my reasoning > R > >>> + { } >>> +}; >>> + >>> +MODULE_DEVICE_TABLE(mdio, tbl); >>> + >>> +MODULE_DESCRIPTION("Microchip 10BASE-T1S lan867x Phy driver"); >>> +MODULE_AUTHOR("Ramón Nordin Rodriguez"); >>> +MODULE_LICENSE("GPL"); >>> -- >>> 2.39.2 >>> >>> 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) >>> >>> v4: >>> - Moved history out of commit message >>> >>> 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 >>
> Do you think it makes sense to probe the driver based on the Revision > number as well? because we have two more revisions in the pipeline and > each revision has different settings for the initial configuration. If > you agree with this, as per the current datasheet this PHY revision is 2 > means Rev.B1. If you like, I would suggest to use the PHY_ID define as > below, Do the current settings not work at all on future silicon? The words 'optimal performance' have been used when describing what this magic does. Which suggest it should work, but not optimally? Unless this is going to cause the magic smoke to escape, i say leave it as it is until you add explicit support for those revisions. > > + /* None of the interrupts in the lan867x phy seem relevant. > > + * Other phys inspect the link status and call phy_trigger_machine > > + * in the interrupt handler. > > + * This phy does not support link status, and thus has no interrupt > > + * for it either. > > + * So we'll just disable all interrupts on the chip. > > + */ > I could see the below in the datasheet section 4.7. > > When the device is in a reset state, the IRQ_N interrupt pin is > high-impedance and will be pulled high through an > external pull-up resistor. Once all device reset sources are deasserted, > the device will begin its internal initialization. > The device will assert the Reset Complete (RESETC) bit in the Status 2 > (STS2) register to indicate that it has > completed its internal initialization and is ready for configuration. As > the Reset Complete status is non-maskable, the > IRQ_N pin will always be asserted and driven low following a device reset > > Do you think it makes sense to clear/acknowledge the "reset_done" > interrupt by reading the Status 2 register in the interrupt routine? When interrupt support is added, you should do this. It is normal to clear all pending interrupt before enabling interrupts, since you don't want to handle stale interrupts. The only potential issue here is when somebody has a board which mixes multiple different PHYs using a shared interrupt. This PHY is going to block all other PHYs, which maybe do have working interrupt support. But there is no regression here, it has never worked. So it is not something which the first version of the driver needs to handle. Andrew
On Thu, Apr 20, 2023 at 06:36:38PM +0200, Ramón Nordin Rodriguez wrote: > 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 <ramon.nordin.rodriguez@ferroamp.se> ... > +static int lan867x_config_init(struct phy_device *phydev) > +{ > + /* HW quirk: Microchip states in the application note (AN1699) for the phy > + * that a set of read-modify-write (rmw) operations has to be performed > + * on a set of seemingly magic registers. > + * The result of these operations is just described as 'optimal performance' > + * Microchip gives no explanation as to what these mmd regs do, > + * in fact they are marked as reserved in the datasheet. > + * It is unclear if phy_modify_mmd would be safe to use or if a write > + * really has to happen to each register. > + * In order to exacly conform to what is stated in the AN phy_write_mmd is nit: s/exacly/exactly/ > + * used, which might then write the same value back as read + modified. > + */ ...
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 54874555c921..15a0bd95f12c 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -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 diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index b5138066ba04..64f649f2f62f 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -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 diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c new file mode 100644 index 000000000000..b7fd5141a117 --- /dev/null +++ b/drivers/net/phy/microchip_t1s.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Driver for Microchip 10BASE-T1S LAN867X PHY + * + * Support: Microchip Phys: + * lan8670, lan8671, lan8672 + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/phy.h> + +#define PHY_ID_LAN867X 0x0007C160 + +#define LAN867X_REG_IRQ_1_CTL 0x001C +#define LAN867X_REG_IRQ_2_CTL 0x001D + +/* The arrays below are pulled from the following table from AN1699 + * Access MMD Address Value Mask + * RMW 0x1F 0x00D0 0x0002 0x0E03 + * RMW 0x1F 0x00D1 0x0000 0x0300 + * RMW 0x1F 0x0084 0x3380 0xFFC0 + * RMW 0x1F 0x0085 0x0006 0x000F + * RMW 0x1F 0x008A 0xC000 0xF800 + * RMW 0x1F 0x0087 0x801C 0x801C + * RMW 0x1F 0x0088 0x033F 0x1FFF + * W 0x1F 0x008B 0x0404 ------ + * RMW 0x1F 0x0080 0x0600 0x0600 + * RMW 0x1F 0x00F1 0x2400 0x7F00 + * RMW 0x1F 0x0096 0x2000 0x2000 + * W 0x1F 0x0099 0x7F80 ------ + */ + +static const int lan867x_fixup_registers[12] = { + 0x00D0, 0x00D1, 0x0084, 0x0085, + 0x008A, 0x0087, 0x0088, 0x008B, + 0x0080, 0x00F1, 0x0096, 0x0099, +}; + +static const int lan867x_fixup_values[12] = { + 0x0002, 0x0000, 0x3380, 0x0006, + 0xC000, 0x801C, 0x033F, 0x0404, + 0x0600, 0x2400, 0x2000, 0x7F80, +}; + +static const int lan867x_fixup_masks[12] = { + 0x0E03, 0x0300, 0xFFC0, 0x000F, + 0xF800, 0x801C, 0x1FFF, 0xFFFF, + 0x0600, 0x7F00, 0x2000, 0xFFFF, +}; + +static int lan867x_config_init(struct phy_device *phydev) +{ + /* HW quirk: Microchip states in the application note (AN1699) for the phy + * that a set of read-modify-write (rmw) operations has to be performed + * on a set of seemingly magic registers. + * The result of these operations is just described as 'optimal performance' + * Microchip gives no explanation as to what these mmd regs do, + * in fact they are marked as reserved in the datasheet. + * It is unclear if phy_modify_mmd would be safe to use or if a write + * really has to happen to each register. + * In order to exacly conform to what is stated in the AN phy_write_mmd is + * used, which might then write the same value back as read + modified. + */ + + int reg_value; + int err; + int reg; + + /* Read-Modified Write Pseudocode (from AN1699) + * current_val = read_register(mmd, addr) // Read current register value + * new_val = current_val AND (NOT mask) // Clear bit fields to be written + * new_val = new_val OR value // Set bits + * write_register(mmd, addr, new_val) // Write back updated register value + */ + for (int i = 0; i < ARRAY_SIZE(lan867x_fixup_registers); i++) { + reg = lan867x_fixup_registers[i]; + reg_value = phy_read_mmd(phydev, MDIO_MMD_VEND2, reg); + reg_value &= ~lan867x_fixup_masks[i]; + reg_value |= lan867x_fixup_values[i]; + err = phy_write_mmd(phydev, MDIO_MMD_VEND2, reg, reg_value); + if (err != 0) + return err; + } + + /* None of the interrupts in the lan867x phy seem relevant. + * Other phys inspect the link status and call phy_trigger_machine + * in the interrupt handler. + * This phy does not support link status, and thus has no interrupt + * for it either. + * So we'll just disable all interrupts on the chip. + */ + err = phy_write_mmd(phydev, MDIO_MMD_VEND2, LAN867X_REG_IRQ_1_CTL, 0xFFFF); + if (err != 0) + return err; + return phy_write_mmd(phydev, MDIO_MMD_VEND2, LAN867X_REG_IRQ_2_CTL, 0xFFFF); +} + +static int lan867x_read_status(struct phy_device *phydev) +{ + /* The phy has some limitations, namely: + * - always reports link up + * - only supports 10MBit half duplex + * - does not support auto negotiate + */ + phydev->link = 1; + phydev->duplex = DUPLEX_HALF; + phydev->speed = SPEED_10; + phydev->autoneg = AUTONEG_DISABLE; + + return 0; +} + +static struct phy_driver lan867x_driver[] = { + { + PHY_ID_MATCH_MODEL(PHY_ID_LAN867X), + .name = "LAN867X", + .features = PHY_BASIC_T1S_P2MP_FEATURES, + .config_init = lan867x_config_init, + .read_status = lan867x_read_status, + .get_plca_cfg = genphy_c45_plca_get_cfg, + .set_plca_cfg = genphy_c45_plca_set_cfg, + .get_plca_status = genphy_c45_plca_get_status, + } +}; + +module_phy_driver(lan867x_driver); + +static struct mdio_device_id __maybe_unused tbl[] = { + { PHY_ID_MATCH_MODEL(PHY_ID_LAN867X) }, + { } +}; + +MODULE_DEVICE_TABLE(mdio, tbl); + +MODULE_DESCRIPTION("Microchip 10BASE-T1S lan867x Phy driver"); +MODULE_AUTHOR("Ramón Nordin Rodriguez"); +MODULE_LICENSE("GPL");
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 <ramon.nordin.rodriguez@ferroamp.se> --- 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