linux-kernel - Re: [PATCH net-next] net: phy: micrel: Add support for non PTP SKUs for lan8814

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251020063945.dwqgn5yphdwnt4vk@DEN-DL-M31836.microchip.com>
Date: Mon, 20 Oct 2025 08:39:45 +0200
From: Horatiu Vultur <horatiu.vultur@...rochip.com>
To: Gerhard Engleder <gerhard@...leder-embedded.com>
CC: <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<andrew@...n.ch>, <hkallweit1@...il.com>, <linux@...linux.org.uk>,
	<davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <richardcochran@...il.com>
Subject: Re: [PATCH net-next] net: phy: micrel: Add support for non PTP SKUs
 for lan8814

The 10/17/2025 23:15, Gerhard Engleder wrote:

Hi Gerhard,

> 
> On 17.10.25 09:47, Horatiu Vultur wrote:
> > The lan8814 has 4 different SKUs and for 2 of these SKUs the PTP is
> > disabled. All these SKUs have the same value in the register 2 and 3
> > meaning we can't differentiate them based on device id therefore check
> 
> Did you miss to start a new sentence?

Yes, I think so. I will update this.

> 
> > the SKU register and based on this allow or not to create a PTP device.
> > 
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@...rochip.com>
> > ---
> >   drivers/net/phy/micrel.c | 38 ++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 38 insertions(+)
> > 
> > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> > index 79ce3eb6752b6..16855bf8c3916 100644
> > --- a/drivers/net/phy/micrel.c
> > +++ b/drivers/net/phy/micrel.c
> > @@ -101,6 +101,8 @@
> >   #define LAN8814_CABLE_DIAG_VCT_DATA_MASK    GENMASK(7, 0)
> >   #define LAN8814_PAIR_BIT_SHIFT                      12
> > 
> > +#define LAN8814_SKUS                         0xB
> > +
> >   #define LAN8814_WIRE_PAIR_MASK                      0xF
> > 
> >   /* Lan8814 general Interrupt control/status reg in GPHY specific block. */
> > @@ -367,6 +369,9 @@
> > 
> >   #define LAN8842_REV_8832                    0x8832
> > 
> > +#define LAN8814_REV_LAN8814                  0x8814
> > +#define LAN8814_REV_LAN8818                  0x8818
> > +
> >   struct kszphy_hw_stat {
> >       const char *string;
> >       u8 reg;
> > @@ -449,6 +454,7 @@ struct kszphy_priv {
> >       bool rmii_ref_clk_sel;
> >       bool rmii_ref_clk_sel_val;
> >       bool clk_enable;
> > +     bool is_ptp_available;
> >       u64 stats[ARRAY_SIZE(kszphy_hw_stats)];
> >       struct kszphy_phy_stats phy_stats;
> >   };
> > @@ -4130,6 +4136,17 @@ static int lan8804_config_intr(struct phy_device *phydev)
> >       return 0;
> >   }
> > 
> > +/* Check if the PHY has 1588 support. There are multiple skus of the PHY and
> > + * some of them support PTP while others don't support it. This function will
> > + * return true is the sku supports it, otherwise will return false.
> > + */
> 
> Hasn't net also switched to the common kernel multiline comment style
> starting with an empty line?

I am not sure because I can see some previous commits where people used
the same comment style:
e82c64be9b45 ("net: stmmac: avoid PHY speed change when configuring MTU")
100dfa74cad9 ("net: dev_queue_xmit() llist adoption")

> 
> > +static bool lan8814_has_ptp(struct phy_device *phydev)
> > +{
> > +     struct kszphy_priv *priv = phydev->priv;
> > +
> > +     return priv->is_ptp_available;
> > +}
> > +
> >   static irqreturn_t lan8814_handle_interrupt(struct phy_device *phydev)
> >   {
> >       int ret = IRQ_NONE;
> > @@ -4146,6 +4163,9 @@ static irqreturn_t lan8814_handle_interrupt(struct phy_device *phydev)
> >               ret = IRQ_HANDLED;
> >       }
> > 
> > +     if (!lan8814_has_ptp(phydev))
> > +             return ret;
> > +
> >       while (true) {
> >               irq_status = lanphy_read_page_reg(phydev, LAN8814_PAGE_PORT_REGS,
> >                                                 PTP_TSU_INT_STS);
> > @@ -4207,6 +4227,9 @@ static void lan8814_ptp_init(struct phy_device *phydev)
> >           !IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING))
> >               return;
> > 
> > +     if (!lan8814_has_ptp(phydev))
> > +             return;
> > +
> >       lanphy_write_page_reg(phydev, LAN8814_PAGE_PORT_REGS,
> >                             TSU_HARD_RESET, TSU_HARD_RESET_);
> > 
> > @@ -4336,6 +4359,9 @@ static int __lan8814_ptp_probe_once(struct phy_device *phydev, char *pin_name,
> > 
> >   static int lan8814_ptp_probe_once(struct phy_device *phydev)
> >   {
> > +     if (!lan8814_has_ptp(phydev))
> > +             return 0;
> > +
> >       return __lan8814_ptp_probe_once(phydev, "lan8814_ptp_pin",
> >                                       LAN8814_PTP_GPIO_NUM);
> >   }
> > @@ -4450,6 +4476,18 @@ static int lan8814_probe(struct phy_device *phydev)
> >       devm_phy_package_join(&phydev->mdio.dev, phydev,
> >                             addr, sizeof(struct lan8814_shared_priv));
> > 
> > +     /* There are lan8814 SKUs that don't support PTP. Make sure that for
> > +      * those skus no PTP device is created. Here we check if the SKU
> > +      * supports PTP.
> > +      */
> 
> Check comment style.
> 
> > +     err = lanphy_read_page_reg(phydev, LAN8814_PAGE_COMMON_REGS,
> > +                                LAN8814_SKUS);
> > +     if (err < 0)
> > +             return err;
> > +
> > +     priv->is_ptp_available = err == LAN8814_REV_LAN8814 ||
> > +                              err == LAN8814_REV_LAN8818;
> > +
> >       if (phy_package_init_once(phydev)) {
> >               err = lan8814_release_coma_mode(phydev);
> >               if (err)
> 

-- 
/Horatiu