On Fri, Jun 16, 2023 at 04:53:10PM +0300, Radu Pirea (NXP OSS) wrote:
.config_intr() handles only the link event interrupt and should disable/enable the PTP interrupt also.
It's safe to disable/enable the PTP irq even if the egress ts irq is disabled. This interrupt, the PTP one, acts as a global switch for all PTP irqs.
Fixes: 514def5dd339 ("phy: nxp-c45-tja11xx: add timestamping support") CC: stable@vger.kernel.org # 5.15+ Signed-off-by: Radu Pirea (NXP OSS) radu-nicolae.pirea@oss.nxp.com
Please don't mix fixes and development work in one patchset. Please post this to applying to net, not net-next.
static int nxp_c45_config_intr(struct phy_device *phydev) {
- if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
- /* The return value is ignored on purpose. It might be < 0.
* 0x807A register is not present on SJA1110 PHYs.*/- if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
phy_set_bits_mmd(phydev, MDIO_MMD_VEND1,VEND1_PORT_FUNC_IRQ_EN, PTP_IRQS);
phy_set_bits_mmd() will not return an error if the register does not exist. There is no such indication for MDIO. This is going to do a read/modify/write. That read might get 0xffff, or random junk. And then the write back will be successful. The only time phy_read()/phy_write return error is when there is a problem within the bus master, like its clock gets turned off and the transfer times out.
So it is good to document you are accessing a register which might not exist, but there is no need to ignore the return code.
Andrew