.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 ---
Where is V1? https://patchwork.kernel.org/project/netdevbpf/patch/20230410124856.287753-1...
Where is V2? https://patchwork.kernel.org/project/netdevbpf/patch/20230616135323
drivers/net/phy/nxp-c45-tja11xx.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/nxp-c45-tja11xx.c b/drivers/net/phy/nxp-c45-tja11xx.c index 029875a59ff8..f0d047019f33 100644 --- a/drivers/net/phy/nxp-c45-tja11xx.c +++ b/drivers/net/phy/nxp-c45-tja11xx.c @@ -63,6 +63,9 @@ #define VEND1_PORT_ABILITIES 0x8046 #define PTP_ABILITY BIT(3)
+#define VEND1_PORT_FUNC_IRQ_EN 0x807A +#define PTP_IRQS BIT(3) + #define VEND1_PORT_INFRA_CONTROL 0xAC00 #define PORT_INFRA_CONTROL_EN BIT(14)
@@ -890,12 +893,26 @@ static int nxp_c45_start_op(struct phy_device *phydev)
static int nxp_c45_config_intr(struct phy_device *phydev) { - if (phydev->interrupts == PHY_INTERRUPT_ENABLED) + int ret; + + /* 0x807A register is not present on SJA1110 PHYs. */ + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { + ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND1, + VEND1_PORT_FUNC_IRQ_EN, PTP_IRQS); + if (ret) + return ret; + return phy_set_bits_mmd(phydev, MDIO_MMD_VEND1, VEND1_PHY_IRQ_EN, PHY_IRQ_LINK_EVENT); - else + } else { + ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, + VEND1_PORT_FUNC_IRQ_EN, PTP_IRQS); + if (ret) + return ret; + return phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, VEND1_PHY_IRQ_EN, PHY_IRQ_LINK_EVENT); + } }
static irqreturn_t nxp_c45_handle_interrupt(struct phy_device *phydev)
On Mon, Jun 19, 2023 at 04:28:51PM +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
Reviewed-by: Andrew Lunn andrew@lunn.ch
Andrew
On Mon, 19 Jun 2023 16:28:51 +0300 Radu Pirea (NXP OSS) wrote:
Subject: [PATCH net v3 1/1] net: phy: nxp-c45-tja11xx: fix the PTP interrupt enablig/disabling
typo: enablig -> enabling
.config_intr() handles only the link event interrupt and should disable/enable the PTP interrupt also.
I don't understand this sentence, TBH, could you rephrase?
Fixes: 514def5dd339 ("phy: nxp-c45-tja11xx: add timestamping support")
For a fix we really need to commit message to say what the problem is, in terms which will be understood by the user. User visible behavior.
CC: stable@vger.kernel.org # 5.15+ Signed-off-by: Radu Pirea (NXP OSS) radu-nicolae.pirea@oss.nxp.com
Where is V1? https://patchwork.kernel.org/project/netdevbpf/patch/20230410124856.287753-1...
Where is V2? https://patchwork.kernel.org/project/netdevbpf/patch/20230616135323
This link looks cut off.
- /* 0x807A register is not present on SJA1110 PHYs. */
Meaning? It's safe because the operation will be ignored?
Hi Jakub,
On 21.06.2023 06:31, Jakub Kicinski wrote:
On Mon, 19 Jun 2023 16:28:51 +0300 Radu Pirea (NXP OSS) wrote:
Subject: [PATCH net v3 1/1] net: phy: nxp-c45-tja11xx: fix the PTP interrupt enablig/disabling
typo: enablig -> enabling
.config_intr() handles only the link event interrupt and should disable/enable the PTP interrupt also.
I don't understand this sentence, TBH, could you rephrase? >
Fixes: 514def5dd339 ("phy: nxp-c45-tja11xx: add timestamping support")
For a fix we really need to commit message to say what the problem is, in terms which will be understood by the user. User visible behavior.
If tools like ptp4l are killed, will leave egress timestamp interrupt enabled. And I would like to say that I was able to trigger any bug related to this, but I wasn't. So, I don't have any bad behaviour to describe :)
However, now I realize that disabling all the PTP IRQs is not a smart way to fix this virtual pseudo issue.
CC: stable@vger.kernel.org # 5.15+ Signed-off-by: Radu Pirea (NXP OSS) radu-nicolae.pirea@oss.nxp.com
Where is V1? https://patchwork.kernel.org/project/netdevbpf/patch/20230410124856.287753-1...
Where is V2? https://patchwork.kernel.org/project/netdevbpf/patch/20230616135323
This link looks cut off.
https://patchwork.kernel.org/project/netdevbpf/patch/20230616135323.98215-2-...
/* 0x807A register is not present on SJA1110 PHYs. */
Meaning? It's safe because the operation will be ignored?
Yes. The PHY will ignore the writes and will return 0 on reads.
-- pw-bot: cr
linux-stable-mirror@lists.linaro.org