| Message ID | 20241104090750.12942-6-divya.koppera@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add ptp library for Microchip phys | expand |
> static int lan937x_dsp_workaround(struct phy_device *phydev, u16 ereg, u8 bank) > @@ -1472,6 +1478,12 @@ static int lan887x_probe(struct phy_device *phydev) > > phydev->priv = priv; > > + priv->clock = mchp_ptp_probe(phydev, MDIO_MMD_VEND1, > + MCHP_PTP_LTC_BASE_ADDR, > + MCHP_PTP_PORT_BASE_ADDR); In general, PHY interrupts are optional, since phylib will poll the PHY once per second for changes in link etc. Does mchp_ptp_probe() do the right thing if the PHY does not have an interrupt? Andrew
Hi Divya,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Divya-Koppera/net-phy-microchip_ptp-Add-header-file-for-Microchip-ptp-library/20241104-171132
base: net-next/main
patch link: https://lore.kernel.org/r/20241104090750.12942-6-divya.koppera%40microchip.com
patch subject: [PATCH net-next 5/5] net: phy: microchip_t1 : Add initialization of ptp for lan887x
config: x86_64-randconfig-121-20241105 (https://download.01.org/0day-ci/archive/20241105/202411051039.Yz1kJCOl-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241105/202411051039.Yz1kJCOl-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411051039.Yz1kJCOl-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
drivers/net/phy/microchip_t1.c: note: in included file:
>> drivers/net/phy/microchip_ptp.h:201:16: sparse: sparse: Using plain integer as NULL pointer
vim +201 drivers/net/phy/microchip_ptp.h
ca38715fe9dd46 Divya Koppera 2024-11-04 196
ca38715fe9dd46 Divya Koppera 2024-11-04 197 static inline struct mchp_ptp_clock *mchp_ptp_probe(struct phy_device *phydev,
ca38715fe9dd46 Divya Koppera 2024-11-04 198 u8 mmd, u16 clk_base,
ca38715fe9dd46 Divya Koppera 2024-11-04 199 u16 port_base)
ca38715fe9dd46 Divya Koppera 2024-11-04 200 {
ca38715fe9dd46 Divya Koppera 2024-11-04 @201 return 0;
ca38715fe9dd46 Divya Koppera 2024-11-04 202 }
ca38715fe9dd46 Divya Koppera 2024-11-04 203
Hi Andrew, Thanks for the comments. > -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Monday, November 4, 2024 7:34 PM > To: Divya Koppera - I30481 <Divya.Koppera@microchip.com> > Cc: Arun Ramadoss - I17769 <Arun.Ramadoss@microchip.com>; > UNGLinuxDriver <UNGLinuxDriver@microchip.com>; hkallweit1@gmail.com; > linux@armlinux.org.uk; davem@davemloft.net; edumazet@google.com; > kuba@kernel.org; pabeni@redhat.com; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; richardcochran@gmail.com > Subject: Re: [PATCH net-next 5/5] net: phy: microchip_t1 : Add initialization of > ptp for lan887x > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > > static int lan937x_dsp_workaround(struct phy_device *phydev, u16 > > ereg, u8 bank) @@ -1472,6 +1478,12 @@ static int lan887x_probe(struct > > phy_device *phydev) > > > > phydev->priv = priv; > > > > + priv->clock = mchp_ptp_probe(phydev, MDIO_MMD_VEND1, > > + MCHP_PTP_LTC_BASE_ADDR, > > + MCHP_PTP_PORT_BASE_ADDR); > > In general, PHY interrupts are optional, since phylib will poll the PHY once per > second for changes in link etc. Does mchp_ptp_probe() do the right thing if > the PHY does not have an interrupt? > Interrupts must be enabled by integrating platform(MAC/Switch). Currently mchp_ptp_probe() is not checking for interrupts flag and also irq might not be initialized by the time probe is called. To handle this, mchp_ptp_probe need to be called once from config_init instead of probe where a valid irq is available. Based on IRQ number, we can skip ptp enabling from config_init and set default_timestamp=false. Let me know if this approach is acceptable in interrupts disabled case. > Andrew Thanks, Divya
diff --git a/drivers/net/phy/microchip_t1.c b/drivers/net/phy/microchip_t1.c index 71d6050b2833..0a8b88d577c3 100644 --- a/drivers/net/phy/microchip_t1.c +++ b/drivers/net/phy/microchip_t1.c @@ -10,11 +10,15 @@ #include <linux/ethtool.h> #include <linux/ethtool_netlink.h> #include <linux/bitfield.h> +#include "microchip_ptp.h" #define PHY_ID_LAN87XX 0x0007c150 #define PHY_ID_LAN937X 0x0007c180 #define PHY_ID_LAN887X 0x0007c1f0 +#define MCHP_PTP_LTC_BASE_ADDR 0xe000 +#define MCHP_PTP_PORT_BASE_ADDR (MCHP_PTP_LTC_BASE_ADDR + 0x800) + /* External Register Control Register */ #define LAN87XX_EXT_REG_CTL (0x14) #define LAN87XX_EXT_REG_CTL_RD_CTL (0x1000) @@ -229,6 +233,7 @@ #define LAN887X_INT_STS 0xf000 #define LAN887X_INT_MSK 0xf001 +#define LAN887X_INT_MSK_P1588_MOD_INT_MSK BIT(3) #define LAN887X_INT_MSK_T1_PHY_INT_MSK BIT(2) #define LAN887X_INT_MSK_LINK_UP_MSK BIT(1) #define LAN887X_INT_MSK_LINK_DOWN_MSK BIT(0) @@ -319,6 +324,7 @@ struct lan887x_regwr_map { struct lan887x_priv { u64 stats[ARRAY_SIZE(lan887x_hw_stats)]; + struct mchp_ptp_clock *clock; }; static int lan937x_dsp_workaround(struct phy_device *phydev, u16 ereg, u8 bank) @@ -1472,6 +1478,12 @@ static int lan887x_probe(struct phy_device *phydev) phydev->priv = priv; + priv->clock = mchp_ptp_probe(phydev, MDIO_MMD_VEND1, + MCHP_PTP_LTC_BASE_ADDR, + MCHP_PTP_PORT_BASE_ADDR); + if (IS_ERR(priv->clock)) + return PTR_ERR(priv->clock); + return lan887x_phy_setup(phydev); } @@ -1518,6 +1530,7 @@ static void lan887x_get_strings(struct phy_device *phydev, u8 *data) static int lan887x_config_intr(struct phy_device *phydev) { + struct lan887x_priv *priv = phydev->priv; int rc; if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { @@ -1538,11 +1551,18 @@ static int lan887x_config_intr(struct phy_device *phydev) rc = phy_read_mmd(phydev, MDIO_MMD_VEND1, LAN887X_INT_STS); } - return rc < 0 ? rc : 0; + if (rc < 0) + return rc; + + return mchp_config_ptp_intr(priv->clock, LAN887X_INT_MSK, + LAN887X_INT_MSK_P1588_MOD_INT_MSK, + (phydev->interrupts == PHY_INTERRUPT_ENABLED)); } static irqreturn_t lan887x_handle_interrupt(struct phy_device *phydev) { + struct lan887x_priv *priv = phydev->priv; + int rc = IRQ_NONE; int irq_status; irq_status = phy_read_mmd(phydev, MDIO_MMD_VEND1, LAN887X_INT_STS); @@ -1553,10 +1573,13 @@ static irqreturn_t lan887x_handle_interrupt(struct phy_device *phydev) if (irq_status & LAN887X_MX_CHIP_TOP_LINK_MSK) { phy_trigger_machine(phydev); - return IRQ_HANDLED; + rc = IRQ_HANDLED; } - return IRQ_NONE; + if (irq_status & LAN887X_INT_MSK_P1588_MOD_INT_MSK) + rc = mchp_ptp_handle_interrupt(priv->clock); + + return rc; } static int lan887x_cd_reset(struct phy_device *phydev,
Add initialization of ptp for lan887x. Signed-off-by: Divya Koppera <divya.koppera@microchip.com> --- drivers/net/phy/microchip_t1.c | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-)