Re: [PATCH net-next v2] net: phy: micrel: Add support for lan9645x internal phy

[Heiner Kallweit <hkallweit1@gmail.com>]

On 1/31/2026 7:29 AM, Jens Emil Schulz Ostergaard wrote:
> Hi,
> 
> On Fri, 2026-01-30 at 10:12 +0100, Jens Emil Schulz Østergaard wrote:
>> LAN9645X is a family of switch chips with 5 internal copper phys. The
>> internal PHY is based on parts of LAN8832. This is a low-power, single
>> port triple-speed (10BASE-T/100BASE-TX/1000BASE-T) ethernet physical
>> layer transceiver (PHY) that supports transmission and reception of data
>> on standard CAT-5, as well as CAT-5e and CAT-6 Unshielded Twisted
>> Pair (UTP) cables.
>>
>> Add support for the internal PHY of the lan9645x chip family.
>>
>> Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
>> Reviewed-by: Daniel Machon <daniel.machon@microchip.com>
>> Signed-off-by: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
>> ---
>> Changes in v2:
>> - Check write errors in lan9645x_config_intr.
>> - Added more comments for the erratas which are not yet documented in
>>   the datasheet.
>> - Change workaround in lan9645x_suspend from ANEG restart and sleep to
>>   asserting Software Soft Reset and poll for self-clear.
>> - Return IRQ_NONE when IRQ is not handled in lan9645x_handle_interrupt.
>> - Link to v1: https://lore.kernel.org/r/20260123-phy_micrel_add_support_for_lan9645x_internal_phy-v1-1-8484b1a5a7fd@microchip.com
>> ---
>>  drivers/net/phy/micrel.c   | 152 +++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/micrel_phy.h |   1 +
>>  2 files changed, 153 insertions(+)
>>
>> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
>> index 225d4adf28be..fe77ad899f10 100644
>> --- a/drivers/net/phy/micrel.c
>> +++ b/drivers/net/phy/micrel.c
>> @@ -6502,6 +6502,142 @@ static void lan8842_get_phy_stats(struct phy_device *phydev,
>>  	stats->tx_errors = priv->phy_stats.tx_errors;
>>  }
>>  
>> +#define LAN9645X_CONTROL_REGISTER		0x1f
>> +#define LAN9645X_CONTROL_REGISTER_SOFT_RESET	BIT(1)
>> +
>> +#define LAN9645X_DAC_ICAS_AMP_POWER_DOWN	0x47
>> +#define LAN9645X_BTRX_QBIAS_POWER_DOWN		0x46
>> +#define LAN9645X_TX_LOW_I_CH_CD_POWER_MGMT	0x45
>> +#define LAN9645X_TX_LOW_I_CH_B_POWER_MGMT	0x44
>> +#define LAN9645X_TX_LOW_I_CH_A_POWER_MGMT	0x43
>> +
>> +static const struct lanphy_reg_data force_dac_tx_errata[] = {
>> +	/* Force channel A/B/C/D TX on */
>> +	{ LAN8814_PAGE_POWER_REGS,
>> +	  LAN9645X_DAC_ICAS_AMP_POWER_DOWN,
>> +	  0 },
>> +	/* Force channel A/B/C/D QBias on */
>> +	{ LAN8814_PAGE_POWER_REGS,
>> +	  LAN9645X_BTRX_QBIAS_POWER_DOWN,
>> +	  0xaa },
>> +	/* Tx low I on channel C/D overwrite */
>> +	{ LAN8814_PAGE_POWER_REGS,
>> +	  LAN9645X_TX_LOW_I_CH_CD_POWER_MGMT,
>> +	  0xbfff },
>> +	/* Channel B low I overwrite */
>> +	{ LAN8814_PAGE_POWER_REGS,
>> +	  LAN9645X_TX_LOW_I_CH_B_POWER_MGMT,
>> +	  0xabbf },
>> +	/* Channel A low I overwrite */
>> +	{ LAN8814_PAGE_POWER_REGS,
>> +	  LAN9645X_TX_LOW_I_CH_A_POWER_MGMT,
>> +	  0xbd3f },
>> +};
>> +
>> +static int lan9645x_config_init(struct phy_device *phydev)
>> +{
>> +	int ret;
>> +
>> +	/* Apply erratas from previous generations.  */
>> +	ret = lan8842_erratas(phydev);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	/* Apply errata for an issue where bringing a port down, can cause a few
>> +	 * CRC errors for traffic flowing through adjacent ports.
>> +	 */
>> +	return lanphy_write_reg_data(phydev, force_dac_tx_errata,
>> +				     ARRAY_SIZE(force_dac_tx_errata));
>> +}
>> +
>> +static int lan9645x_suspend(struct phy_device *phydev)
>> +{
>> +	int ret, val;
>> +
>> +	/* Force link down before software power down (SPD), by doing software
>> +	 * soft reset. This resets the PHY, but keeps all register configuration
>> +	 * intact. The bit self clears.
>> +	 *
>> +	 * This is needed as a workaround for an issue where performing SPD on a
>> +	 * port can bring adjacent ports down, when there is traffic flowing
>> +	 * through the ports.
>> +	 */
>> +	ret = phy_modify(phydev, LAN9645X_CONTROL_REGISTER,
>> +			 LAN9645X_CONTROL_REGISTER_SOFT_RESET, 1);

Any specific reason why you use a vendor-specific register here instead of BMCR
via genphy_soft_reset()?


>> +	if (ret)
>> +		return ret;
>> +
> 
> AI review pointed out an error here. I will fix in the next version.
> 
>> +	ret = phy_read_poll_timeout(phydev, LAN9645X_CONTROL_REGISTER, val,
>> +				    !(val & LAN9645X_CONTROL_REGISTER_SOFT_RESET),
>> +				    3000, 100000, true);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return genphy_suspend(phydev);
>> +}
>> +
>> +static int lan9645x_config_intr(struct phy_device *phydev)
>> +{
>> +	int err;
>> +
>> +	/* enable / disable interrupts */
>> +	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
>> +		/* This is an internal PHY of lan9645x and is not possible to
>> +		 * change the polarity of irq sources in the OIC (CPU_INTR)
>> +		 * found in lan9645x. Therefore change the polarity of the
>> +		 * interrupt in the PHY from being active low instead of active
>> +		 * high.
>> +		 */
>> +		err = phy_write(phydev, LAN8804_CONTROL,
>> +				LAN8804_CONTROL_INTR_POLARITY);
>> +		if (err)
>> +			return err;
>> +
>> +		/* By default interrupt buffer is open-drain in which case the
>> +		 * interrupt can be active only low. Therefore change the
>> +		 * interrupt buffer to be push-pull to be able to change
>> +		 * interrupt polarity.
>> +		 */
>> +		err = phy_write(phydev, LAN8804_OUTPUT_CONTROL,
>> +				LAN8804_OUTPUT_CONTROL_INTR_BUFFER);
>> +		if (err)
>> +			return err;
>> +
>> +		err = lan8814_ack_interrupt(phydev);
>> +		if (err)
>> +			return err;
>> +
>> +		err = phy_write(phydev, LAN8814_INTC,
>> +				LAN8814_INT_LINK | LAN8814_INT_FLF);
>> +	} else {
>> +		err = phy_write(phydev, LAN8814_INTC, 0);
>> +		if (err)
>> +			return err;
>> +
>> +		err = lan8814_ack_interrupt(phydev);
>> +	}
>> +
>> +	return err;
>> +}
>> +
>> +static irqreturn_t lan9645x_handle_interrupt(struct phy_device *phydev)
>> +{
>> +	int status;
>> +
>> +	status = phy_read(phydev, LAN8814_INTS);
>> +	if (status < 0) {
>> +		phy_error(phydev);
>> +		return IRQ_NONE;
>> +	}
>> +
>> +	if (status & (LAN8814_INT_LINK | LAN8814_INT_FLF)) {
>> +		phy_trigger_machine(phydev);
>> +		return IRQ_HANDLED;
>> +	}
>> +
>> +	return IRQ_NONE;
>> +}
>> +
>>  static struct phy_driver ksphy_driver[] = {
>>  {
>>  	PHY_ID_MATCH_MODEL(PHY_ID_KS8737),
>> @@ -6740,6 +6876,21 @@ static struct phy_driver ksphy_driver[] = {
>>  	.set_tunable	= lan8842_set_tunable,
>>  	.cable_test_start	= lan8814_cable_test_start,
>>  	.cable_test_get_status	= ksz886x_cable_test_get_status,
>> +}, {
>> +	PHY_ID_MATCH_MODEL(PHY_ID_LAN9645X),
>> +	.name		= "Microchip LAN9645X Gigabit PHY",
>> +	.config_init	= lan9645x_config_init,
>> +	.driver_data	= &ksz9021_type,
>> +	.probe		= kszphy_probe,
>> +	.soft_reset	= genphy_soft_reset,
>> +	.suspend	= lan9645x_suspend,
>> +	.resume		= genphy_resume,
>> +	.config_intr	= lan9645x_config_intr,
>> +	.handle_interrupt = lan9645x_handle_interrupt,
>> +	.get_tunable	= lan8842_get_tunable,
>> +	.set_tunable	= lan8842_set_tunable,
>> +	.get_phy_stats	= lan8842_get_phy_stats,
>> +	.update_stats	= lan8842_update_stats,
>>  }, {
>>  	PHY_ID_MATCH_MODEL(PHY_ID_KSZ9131),
>>  	.name		= "Microchip KSZ9131 Gigabit PHY",
>> @@ -6838,6 +6989,7 @@ static const struct mdio_device_id __maybe_unused micrel_tbl[] = {
>>  	{ PHY_ID_MATCH_MODEL(PHY_ID_LAN8804) },
>>  	{ PHY_ID_MATCH_MODEL(PHY_ID_LAN8841) },
>>  	{ PHY_ID_MATCH_MODEL(PHY_ID_LAN8842) },
>> +	{ PHY_ID_MATCH_MODEL(PHY_ID_LAN9645X) },
>>  	{ }
>>  };
>>  
>> diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
>> index ca691641788b..9c6f9817383f 100644
>> --- a/include/linux/micrel_phy.h
>> +++ b/include/linux/micrel_phy.h
>> @@ -33,6 +33,7 @@
>>  #define PHY_ID_LAN8804		0x00221670
>>  #define PHY_ID_LAN8841		0x00221650
>>  #define PHY_ID_LAN8842		0x002216C0
>> +#define PHY_ID_LAN9645X		0x002216D0
>>  
>>  #define PHY_ID_KSZ886X		0x00221430
>>  #define PHY_ID_KSZ8863		0x00221435
>>
>> ---
>> base-commit: cbe8e6bef6a3b4b895b47ea56f5952f1936aacb6
>> change-id: 20260114-phy_micrel_add_support_for_lan9645x_internal_phy-6ef9a782cb8b
>>
>> Best regards,
> 
> Thanks,
> 
> Emil
>