.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: [email protected] # 5.15+
Signed-off-by: Radu Pirea (NXP OSS) <[email protected]>
---
Where is V1?
https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
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)
--
2.34.1
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: [email protected] # 5.15+
> Signed-off-by: Radu Pirea (NXP OSS) <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
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: [email protected] # 5.15+
> Signed-off-by: Radu Pirea (NXP OSS) <[email protected]>
> ---
>
> Where is V1?
> https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
>
> 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?
--
pw-bot: cr
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: [email protected] # 5.15+
>> Signed-off-by: Radu Pirea (NXP OSS) <[email protected]>
>> ---
>>
>> Where is V1?
>> https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
>>
>> Where is V2?
>> https://patchwork.kernel.org/project/netdevbpf/patch/20230616135323
>
> This link looks cut off.
https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
>
>> + /* 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
--
Radu P.