public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Jens Emil Schulz Ostergaard <jensemil.schulzostergaard@microchip.com>
To: Heiner Kallweit <hkallweit1@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Russell King <linux@armlinux.org.uk>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Horatiu Vultur <horatiu.vultur@microchip.com>,
	<o.rempel@pengutronix.de>,
	Steen Hegelund <Steen.Hegelund@microchip.com>,
	Daniel Machon <daniel.machon@microchip.com>
Cc: <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next v2] net: phy: micrel: Add support for lan9645x internal phy
Date: Wed, 25 Feb 2026 13:35:18 +0100	[thread overview]
Message-ID: <5876361bab237e2e1c73395d7e65c93279865f63.camel@microchip.com> (raw)
In-Reply-To: <00f03a3f-fc12-4875-b4b5-7470e7f0452d@gmail.com>

On Fri, 2026-02-20 at 21:50 +0100, Heiner Kallweit wrote:
> 
> On 20.02.2026 21:38, Jens Emil Schulz Ostergaard wrote:
> > Hi Heiner,
> > 
> > On Sat, 2026-01-31 at 11:00 +0100, Heiner Kallweit wrote:
> > > 
> > > 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()?
> > > 
> > > 
> > 
> > Sorry I missed your mail. genphy_soft_reset() will do a software (hard) reset via
> > bit 15 (BMCR_RESET). In particular it will reset the registers. We want to do
> > a software soft reset, which resets the PHY without changing register values.
> > 
> No, BMCR_RESET usually doesn't reset configuration registers. That's why the
> function is called genphy_*soft*_reset. In case your PHY behaves different,
> which configuration registers does it change?
> 

Ok I tought that was the usual behavior. The register spec says BMCR_RESET will reset
the PHY and all its registers to their default state.


> > 
> > > > > +    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
> > > > 
> > > 
> > > 
> > 
> 


  parent reply	other threads:[~2026-02-25 12:36 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-30  9:12 [PATCH net-next v2] net: phy: micrel: Add support for lan9645x internal phy Jens Emil Schulz Østergaard
2026-01-31  6:29 ` Jens Emil Schulz Ostergaard
2026-01-31 10:00   ` Heiner Kallweit
2026-02-20 20:38     ` Jens Emil Schulz Ostergaard
2026-02-20 20:50       ` Heiner Kallweit
2026-02-20 21:10         ` Russell King (Oracle)
2026-02-20 21:22           ` Andrew Lunn
2026-02-20 21:30           ` Heiner Kallweit
2026-02-20 21:49             ` Russell King (Oracle)
2026-02-25 12:35         ` Jens Emil Schulz Ostergaard [this message]
2026-02-25 13:39           ` Andrew Lunn
2026-02-25 20:57             ` Jens Emil Schulz Ostergaard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5876361bab237e2e1c73395d7e65c93279865f63.camel@microchip.com \
    --to=jensemil.schulzostergaard@microchip.com \
    --cc=Steen.Hegelund@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=daniel.machon@microchip.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=horatiu.vultur@microchip.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox