[PATCH net v4] phy: mscc: Fix timestamping for vsc8584

[PATCH net v4] phy: mscc: Fix timestamping for vsc8584

[Horatiu Vultur]

There was a problem when we received frames and the frames were
timestamped. The driver is configured to store the nanosecond part of
the timestmap in the ptp reserved bits and it would take the second part
by reading the LTC. The problem is that when reading the LTC we are in
atomic context and to read the second part will go over mdio bus which
might sleep, so we get an error.
The fix consists in actually put all the frames in a queue and start the
aux work and in that work to read the LTC and then calculate the full
received time.

Fixes: 7d272e63e0979d ("net: phy: mscc: timestamping and PHC support")
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>

---
v3->v4:
- remove empty line

v2->v3:
- make sure to flush the rx_skbs_list when the driver is removed

v1->v2:
- use sk_buff_head instead of a list_head and spinlock_t
- stop allocating vsc8431_skb but put the timestamp in skb->cb
---
 drivers/net/phy/mscc/mscc.h      | 12 ++++++++
 drivers/net/phy/mscc/mscc_main.c | 12 ++++++++
 drivers/net/phy/mscc/mscc_ptp.c  | 49 ++++++++++++++++++++++++--------
 3 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
index 138355f1ab0bc..b8c6ba7c7834e 100644
--- a/drivers/net/phy/mscc/mscc.h
+++ b/drivers/net/phy/mscc/mscc.h
@@ -365,6 +365,13 @@ struct vsc85xx_hw_stat {
 	u16 mask;
 };
 
+struct vsc8531_skb_cb {
+	u32 ns;
+};
+
+#define VSC8531_SKB_CB(skb) \
+	((struct vsc8531_skb_cb *)((skb)->cb))
+
 struct vsc8531_private {
 	int rate_magic;
 	u16 supp_led_modes;
@@ -413,6 +420,11 @@ struct vsc8531_private {
 	 */
 	struct mutex ts_lock;
 	struct mutex phc_lock;
+
+	/* list of skbs that were received and need timestamp information but it
+	 * didn't received it yet
+	 */
+	struct sk_buff_head rx_skbs_list;
 };
 
 /* Shared structure between the PHYs of the same package.
diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 37e3e931a8e53..800da302ae632 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -2368,6 +2368,13 @@ static int vsc85xx_probe(struct phy_device *phydev)
 	return vsc85xx_dt_led_modes_get(phydev, default_mode);
 }
 
+static void vsc85xx_remove(struct phy_device *phydev)
+{
+	struct vsc8531_private *priv = phydev->priv;
+
+	skb_queue_purge(&priv->rx_skbs_list);
+}
+
 /* Microsemi VSC85xx PHYs */
 static struct phy_driver vsc85xx_driver[] = {
 {
@@ -2630,6 +2637,7 @@ static struct phy_driver vsc85xx_driver[] = {
 	.config_intr    = &vsc85xx_config_intr,
 	.suspend	= &genphy_suspend,
 	.resume		= &genphy_resume,
+	.remove		= &vsc85xx_remove,
 	.probe		= &vsc8574_probe,
 	.set_wol	= &vsc85xx_wol_set,
 	.get_wol	= &vsc85xx_wol_get,
@@ -2657,6 +2665,7 @@ static struct phy_driver vsc85xx_driver[] = {
 	.config_intr    = &vsc85xx_config_intr,
 	.suspend	= &genphy_suspend,
 	.resume		= &genphy_resume,
+	.remove		= &vsc85xx_remove,
 	.probe		= &vsc8574_probe,
 	.set_wol	= &vsc85xx_wol_set,
 	.get_wol	= &vsc85xx_wol_get,
@@ -2684,6 +2693,7 @@ static struct phy_driver vsc85xx_driver[] = {
 	.config_intr    = &vsc85xx_config_intr,
 	.suspend	= &genphy_suspend,
 	.resume		= &genphy_resume,
+	.remove		= &vsc85xx_remove,
 	.probe		= &vsc8584_probe,
 	.get_tunable	= &vsc85xx_get_tunable,
 	.set_tunable	= &vsc85xx_set_tunable,
@@ -2709,6 +2719,7 @@ static struct phy_driver vsc85xx_driver[] = {
 	.config_intr    = &vsc85xx_config_intr,
 	.suspend	= &genphy_suspend,
 	.resume		= &genphy_resume,
+	.remove		= &vsc85xx_remove,
 	.probe		= &vsc8584_probe,
 	.get_tunable	= &vsc85xx_get_tunable,
 	.set_tunable	= &vsc85xx_set_tunable,
@@ -2734,6 +2745,7 @@ static struct phy_driver vsc85xx_driver[] = {
 	.config_intr    = &vsc85xx_config_intr,
 	.suspend	= &genphy_suspend,
 	.resume		= &genphy_resume,
+	.remove		= &vsc85xx_remove,
 	.probe		= &vsc8584_probe,
 	.get_tunable	= &vsc85xx_get_tunable,
 	.set_tunable	= &vsc85xx_set_tunable,
diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c
index 275706de5847c..de6c7312e8f29 100644
--- a/drivers/net/phy/mscc/mscc_ptp.c
+++ b/drivers/net/phy/mscc/mscc_ptp.c
@@ -1194,9 +1194,7 @@ static bool vsc85xx_rxtstamp(struct mii_timestamper *mii_ts,
 {
 	struct vsc8531_private *vsc8531 =
 		container_of(mii_ts, struct vsc8531_private, mii_ts);
-	struct skb_shared_hwtstamps *shhwtstamps = NULL;
 	struct vsc85xx_ptphdr *ptphdr;
-	struct timespec64 ts;
 	unsigned long ns;
 
 	if (!vsc8531->ptp->configured)
@@ -1206,27 +1204,52 @@ static bool vsc85xx_rxtstamp(struct mii_timestamper *mii_ts,
 	    type == PTP_CLASS_NONE)
 		return false;
 
-	vsc85xx_gettime(&vsc8531->ptp->caps, &ts);
-
 	ptphdr = get_ptp_header_rx(skb, vsc8531->ptp->rx_filter);
 	if (!ptphdr)
 		return false;
 
-	shhwtstamps = skb_hwtstamps(skb);
-	memset(shhwtstamps, 0, sizeof(struct skb_shared_hwtstamps));
-
 	ns = ntohl(ptphdr->rsrvd2);
 
-	/* nsec is in reserved field */
-	if (ts.tv_nsec < ns)
-		ts.tv_sec--;
+	VSC8531_SKB_CB(skb)->ns = ns;
+	skb_queue_tail(&vsc8531->rx_skbs_list, skb);
 
-	shhwtstamps->hwtstamp = ktime_set(ts.tv_sec, ns);
-	netif_rx(skb);
+	ptp_schedule_worker(vsc8531->ptp->ptp_clock, 0);
 
 	return true;
 }
 
+static long vsc85xx_do_aux_work(struct ptp_clock_info *info)
+{
+	struct vsc85xx_ptp *ptp = container_of(info, struct vsc85xx_ptp, caps);
+	struct skb_shared_hwtstamps *shhwtstamps = NULL;
+	struct phy_device *phydev = ptp->phydev;
+	struct vsc8531_private *priv = phydev->priv;
+	struct sk_buff_head received;
+	struct sk_buff *rx_skb;
+	struct timespec64 ts;
+	unsigned long flags;
+
+	__skb_queue_head_init(&received);
+	spin_lock_irqsave(&priv->rx_skbs_list.lock, flags);
+	skb_queue_splice_tail_init(&priv->rx_skbs_list, &received);
+	spin_unlock_irqrestore(&priv->rx_skbs_list.lock, flags);
+
+	vsc85xx_gettime(info, &ts);
+	while ((rx_skb = __skb_dequeue(&received)) != NULL) {
+		shhwtstamps = skb_hwtstamps(rx_skb);
+		memset(shhwtstamps, 0, sizeof(struct skb_shared_hwtstamps));
+
+		if (ts.tv_nsec < VSC8531_SKB_CB(rx_skb)->ns)
+			ts.tv_sec--;
+
+		shhwtstamps->hwtstamp = ktime_set(ts.tv_sec,
+						  VSC8531_SKB_CB(rx_skb)->ns);
+		netif_rx(rx_skb);
+	}
+
+	return -1;
+}
+
 static const struct ptp_clock_info vsc85xx_clk_caps = {
 	.owner		= THIS_MODULE,
 	.name		= "VSC85xx timer",
@@ -1240,6 +1263,7 @@ static const struct ptp_clock_info vsc85xx_clk_caps = {
 	.adjfine	= &vsc85xx_adjfine,
 	.gettime64	= &vsc85xx_gettime,
 	.settime64	= &vsc85xx_settime,
+	.do_aux_work	= &vsc85xx_do_aux_work,
 };
 
 static struct vsc8531_private *vsc8584_base_priv(struct phy_device *phydev)
@@ -1567,6 +1591,7 @@ int vsc8584_ptp_probe(struct phy_device *phydev)
 
 	mutex_init(&vsc8531->phc_lock);
 	mutex_init(&vsc8531->ts_lock);
+	skb_queue_head_init(&vsc8531->rx_skbs_list);
 
 	/* Retrieve the shared load/save GPIO. Request it as non exclusive as
 	 * the same GPIO can be requested by all the PHYs of the same package.
-- 
2.34.1

Re: [PATCH net v4] phy: mscc: Fix timestamping for vsc8584

[Vadim Fedorenko]

On 18/08/2025 09:10, Horatiu Vultur wrote:
> There was a problem when we received frames and the frames were
> timestamped. The driver is configured to store the nanosecond part of
> the timestmap in the ptp reserved bits and it would take the second part
> by reading the LTC. The problem is that when reading the LTC we are in
> atomic context and to read the second part will go over mdio bus which
> might sleep, so we get an error.
> The fix consists in actually put all the frames in a queue and start the
> aux work and in that work to read the LTC and then calculate the full
> received time.
> 
> Fixes: 7d272e63e0979d ("net: phy: mscc: timestamping and PHC support")
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> 
> ---
> v3->v4:
> - remove empty line
> 
> v2->v3:
> - make sure to flush the rx_skbs_list when the driver is removed
> 
> v1->v2:
> - use sk_buff_head instead of a list_head and spinlock_t
> - stop allocating vsc8431_skb but put the timestamp in skb->cb

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

Re: [PATCH net v4] phy: mscc: Fix timestamping for vsc8584

[Vladimir Oltean]

On Mon, Aug 18, 2025 at 10:10:29AM +0200, Horatiu Vultur wrote:
> diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
> index 37e3e931a8e53..800da302ae632 100644
> --- a/drivers/net/phy/mscc/mscc_main.c
> +++ b/drivers/net/phy/mscc/mscc_main.c
> @@ -2368,6 +2368,13 @@ static int vsc85xx_probe(struct phy_device *phydev)
>  	return vsc85xx_dt_led_modes_get(phydev, default_mode);
>  }
>  
> +static void vsc85xx_remove(struct phy_device *phydev)
> +{
> +	struct vsc8531_private *priv = phydev->priv;
> +
> +	skb_queue_purge(&priv->rx_skbs_list);
> +}

Have you tested this patch with an unbind/bind cycle? Haven't you found
that a call to ptp_clock_unregister() is also missing?

Re: [PATCH net v4] phy: mscc: Fix timestamping for vsc8584

[Vadim Fedorenko]

On 18/08/2025 14:21, Vladimir Oltean wrote:
> On Mon, Aug 18, 2025 at 10:10:29AM +0200, Horatiu Vultur wrote:
>> diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
>> index 37e3e931a8e53..800da302ae632 100644
>> --- a/drivers/net/phy/mscc/mscc_main.c
>> +++ b/drivers/net/phy/mscc/mscc_main.c
>> @@ -2368,6 +2368,13 @@ static int vsc85xx_probe(struct phy_device *phydev)
>>   	return vsc85xx_dt_led_modes_get(phydev, default_mode);
>>   }
>>   
>> +static void vsc85xx_remove(struct phy_device *phydev)
>> +{
>> +	struct vsc8531_private *priv = phydev->priv;
>> +
>> +	skb_queue_purge(&priv->rx_skbs_list);
>> +}
> 
> Have you tested this patch with an unbind/bind cycle? Haven't you found
> that a call to ptp_clock_unregister() is also missing?

It was missing before this patch as well, probably needs another patch
to fix this issue

Re: [PATCH net v4] phy: mscc: Fix timestamping for vsc8584

[Horatiu Vultur]

The 08/18/2025 16:21, Vladimir Oltean wrote:

Hi Vladimir,

> 
> On Mon, Aug 18, 2025 at 10:10:29AM +0200, Horatiu Vultur wrote:
> > diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
> > index 37e3e931a8e53..800da302ae632 100644
> > --- a/drivers/net/phy/mscc/mscc_main.c
> > +++ b/drivers/net/phy/mscc/mscc_main.c
> > @@ -2368,6 +2368,13 @@ static int vsc85xx_probe(struct phy_device *phydev)
> >       return vsc85xx_dt_led_modes_get(phydev, default_mode);
> >  }
> >
> > +static void vsc85xx_remove(struct phy_device *phydev)
> > +{
> > +     struct vsc8531_private *priv = phydev->priv;
> > +
> > +     skb_queue_purge(&priv->rx_skbs_list);
> > +}
> 
> Have you tested this patch with an unbind/bind cycle? Haven't you found
> that a call to ptp_clock_unregister() is also missing?

I haven't tested unbind/bind cycle. As I said also to Paolo[1], I will need
to look in this issue with missing ptp_clock_unregister(). But I want to
do that in a separate patch after getting this accepted.

[1] https://lkml.org/lkml/2025/8/13/345

-- 
/Horatiu

Re: [PATCH net v4] phy: mscc: Fix timestamping for vsc8584

[Vladimir Oltean]

On Mon, Aug 18, 2025 at 02:53:22PM +0100, Vadim Fedorenko wrote:
> On 18/08/2025 14:21, Vladimir Oltean wrote:
> > On Mon, Aug 18, 2025 at 10:10:29AM +0200, Horatiu Vultur wrote:
> > > diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
> > > index 37e3e931a8e53..800da302ae632 100644
> > > --- a/drivers/net/phy/mscc/mscc_main.c
> > > +++ b/drivers/net/phy/mscc/mscc_main.c
> > > @@ -2368,6 +2368,13 @@ static int vsc85xx_probe(struct phy_device *phydev)
> > >   	return vsc85xx_dt_led_modes_get(phydev, default_mode);
> > >   }
> > > +static void vsc85xx_remove(struct phy_device *phydev)
> > > +{
> > > +	struct vsc8531_private *priv = phydev->priv;
> > > +
> > > +	skb_queue_purge(&priv->rx_skbs_list);
> > > +}
> > 
> > Have you tested this patch with an unbind/bind cycle? Haven't you found
> > that a call to ptp_clock_unregister() is also missing?
> 
> It was missing before this patch as well, probably needs another patch
> to fix this issue

Sure, separate patch, but the addition I highlighted is pretty obviously
untested.

Re: [PATCH net v4] phy: mscc: Fix timestamping for vsc8584

[Vladimir Oltean]

On Mon, Aug 18, 2025 at 03:56:58PM +0200, Horatiu Vultur wrote:
> The 08/18/2025 16:21, Vladimir Oltean wrote:
> 
> Hi Vladimir,
> 
> > 
> > On Mon, Aug 18, 2025 at 10:10:29AM +0200, Horatiu Vultur wrote:
> > > diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
> > > index 37e3e931a8e53..800da302ae632 100644
> > > --- a/drivers/net/phy/mscc/mscc_main.c
> > > +++ b/drivers/net/phy/mscc/mscc_main.c
> > > @@ -2368,6 +2368,13 @@ static int vsc85xx_probe(struct phy_device *phydev)
> > >       return vsc85xx_dt_led_modes_get(phydev, default_mode);
> > >  }
> > >
> > > +static void vsc85xx_remove(struct phy_device *phydev)
> > > +{
> > > +     struct vsc8531_private *priv = phydev->priv;
> > > +
> > > +     skb_queue_purge(&priv->rx_skbs_list);
> > > +}
> > 
> > Have you tested this patch with an unbind/bind cycle? Haven't you found
> > that a call to ptp_clock_unregister() is also missing?
> 
> I haven't tested unbind/bind cycle. As I said also to Paolo[1], I will need
> to look in this issue with missing ptp_clock_unregister(). But I want to
> do that in a separate patch after getting this accepted.
> 
> [1] https://lkml.org/lkml/2025/8/13/345
> 
> -- 
> /Horatiu

Ok, is there anything preventing you from looking into that issue as well?
The two problems are introduced by the same commit, and fixes will be
backported to all the same stable kernels. I don't exactly understand
why you'd add some code to the PHY's remove() method, but not enough in
order for it to work.

Re: [PATCH net v4] phy: mscc: Fix timestamping for vsc8584

[Horatiu Vultur]

The 08/18/2025 17:13, Vladimir Oltean wrote:
> 
> On Mon, Aug 18, 2025 at 03:56:58PM +0200, Horatiu Vultur wrote:
> > The 08/18/2025 16:21, Vladimir Oltean wrote:
> >
> > Hi Vladimir,
> >
> > >
> > > On Mon, Aug 18, 2025 at 10:10:29AM +0200, Horatiu Vultur wrote:
> > > > diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
> > > > index 37e3e931a8e53..800da302ae632 100644
> > > > --- a/drivers/net/phy/mscc/mscc_main.c
> > > > +++ b/drivers/net/phy/mscc/mscc_main.c
> > > > @@ -2368,6 +2368,13 @@ static int vsc85xx_probe(struct phy_device *phydev)
> > > >       return vsc85xx_dt_led_modes_get(phydev, default_mode);
> > > >  }
> > > >
> > > > +static void vsc85xx_remove(struct phy_device *phydev)
> > > > +{
> > > > +     struct vsc8531_private *priv = phydev->priv;
> > > > +
> > > > +     skb_queue_purge(&priv->rx_skbs_list);
> > > > +}
> > >
> > > Have you tested this patch with an unbind/bind cycle? Haven't you found
> > > that a call to ptp_clock_unregister() is also missing?
> >
> > I haven't tested unbind/bind cycle. As I said also to Paolo[1], I will need
> > to look in this issue with missing ptp_clock_unregister(). But I want to
> > do that in a separate patch after getting this accepted.
> >
> > [1] https://lkml.org/lkml/2025/8/13/345
> >
> > --
> > /Horatiu
> 
> Ok, is there anything preventing you from looking into that issue as well?

Nothing prevents me for looking at this issue. I just need to alocate
some time for this.

> The two problems are introduced by the same commit, and fixes will be
> backported to all the same stable kernels. I don't exactly understand
> why you'd add some code to the PHY's remove() method, but not enough in
> order for it to work.

Yes, I understand that but the fix for ptp_clock_unregister will fix a
different issue that this patch is trying to fix. That is the reason why
I prefer not to add that fix now, just to make things more clear.

-- 
/Horatiu

Re: [PATCH net v4] phy: mscc: Fix timestamping for vsc8584

[Vladimir Oltean]

On Mon, Aug 18, 2025 at 04:19:25PM +0200, Horatiu Vultur wrote:
> Nothing prevents me for looking at this issue. I just need to alocate
> some time for this.
> 
> > The two problems are introduced by the same commit, and fixes will be
> > backported to all the same stable kernels. I don't exactly understand
> > why you'd add some code to the PHY's remove() method, but not enough in
> > order for it to work.
> 
> Yes, I understand that but the fix for ptp_clock_unregister will fix a
> different issue that this patch is trying to fix. That is the reason why
> I prefer not to add that fix now, just to make things more clear.

Not sure "clear" for whom. One of the rules from Documentation/process/stable-kernel-rules.rst
is "It must be obviously correct and tested.", which to me makes it confusing
why you wouldn't fix that issue first (within the same patch set), and then
test this patch during unbind/bind to confirm that it achieves what it intends.

I think the current state of the art is that unbinding a PHY that the
MAC hasn't connected to will work, whereas unbinding a connected PHY,
where the state machine is running, will crash the kernel. To be
perfectly clear, the request is just for the case that is supposed to
work given current phylib implementation, aka with the MAC unconnected
(put administratively down or also unbound, depending on whether it
connects to the PHY at probe time or ndo_open() time).

I don't see where the reluctance comes from - is it that there are going
to be 2 patches instead of 1? My reluctance as a reviewer comes from the
fact that I'm analyzing the change in the larger context and not seeing
how the remove() method you introduced makes any practical difference.
Not sure what I'm supposed to say.

Re: [PATCH net v4] phy: mscc: Fix timestamping for vsc8584

[Horatiu Vultur]

The 08/18/2025 17:37, Vladimir Oltean wrote:
> 
> On Mon, Aug 18, 2025 at 04:19:25PM +0200, Horatiu Vultur wrote:
> > Nothing prevents me for looking at this issue. I just need to alocate
> > some time for this.
> >
> > > The two problems are introduced by the same commit, and fixes will be
> > > backported to all the same stable kernels. I don't exactly understand
> > > why you'd add some code to the PHY's remove() method, but not enough in
> > > order for it to work.
> >
> > Yes, I understand that but the fix for ptp_clock_unregister will fix a
> > different issue that this patch is trying to fix. That is the reason why
> > I prefer not to add that fix now, just to make things more clear.
> 
> Not sure "clear" for whom. One of the rules from Documentation/process/stable-kernel-rules.rst
> is "It must be obviously correct and tested.", which to me makes it confusing
> why you wouldn't fix that issue first (within the same patch set), and then
> test this patch during unbind/bind to confirm that it achieves what it intends.

I have tested the patch by inserting and removing the kernel module. And
I have check that remove function was called and see that it tries to
flush the queue.

> 
> I think the current state of the art is that unbinding a PHY that the
> MAC hasn't connected to will work, whereas unbinding a connected PHY,
> where the state machine is running, will crash the kernel. To be
> perfectly clear, the request is just for the case that is supposed to
> work given current phylib implementation, aka with the MAC unconnected
> (put administratively down or also unbound, depending on whether it
> connects to the PHY at probe time or ndo_open() time).
> 
> I don't see where the reluctance comes from - is it that there are going
> to be 2 patches instead of 1? My reluctance as a reviewer comes from the
> fact that I'm analyzing the change in the larger context and not seeing
> how the remove() method you introduced makes any practical difference.
> Not sure what I'm supposed to say.

I don't have anything against it, like I said before I thought those are
2 different issues. But if you think otherwise I can add a new patch in
this series, no problem.

Why do you say that the function remove() doesn't make any practical
difference?

-- 
/Horatiu

Re: [PATCH net v4] phy: mscc: Fix timestamping for vsc8584

[Vladimir Oltean]

On Tue, Aug 19, 2025 at 08:40:11AM +0200, Horatiu Vultur wrote:
> The 08/18/2025 17:37, Vladimir Oltean wrote:
> > 
> > On Mon, Aug 18, 2025 at 04:19:25PM +0200, Horatiu Vultur wrote:
> > > Nothing prevents me for looking at this issue. I just need to alocate
> > > some time for this.
> > >
> > > > The two problems are introduced by the same commit, and fixes will be
> > > > backported to all the same stable kernels. I don't exactly understand
> > > > why you'd add some code to the PHY's remove() method, but not enough in
> > > > order for it to work.
> > >
> > > Yes, I understand that but the fix for ptp_clock_unregister will fix a
> > > different issue that this patch is trying to fix. That is the reason why
> > > I prefer not to add that fix now, just to make things more clear.
> > 
> > Not sure "clear" for whom. One of the rules from Documentation/process/stable-kernel-rules.rst
> > is "It must be obviously correct and tested.", which to me makes it confusing
> > why you wouldn't fix that issue first (within the same patch set), and then
> > test this patch during unbind/bind to confirm that it achieves what it intends.
> 
> I have tested the patch by inserting and removing the kernel module. And
> I have check that remove function was called and see that it tries to
> flush the queue.

Ok, it's great that you tested it.

> > I think the current state of the art is that unbinding a PHY that the
> > MAC hasn't connected to will work, whereas unbinding a connected PHY,
> > where the state machine is running, will crash the kernel. To be
> > perfectly clear, the request is just for the case that is supposed to
> > work given current phylib implementation, aka with the MAC unconnected
> > (put administratively down or also unbound, depending on whether it
> > connects to the PHY at probe time or ndo_open() time).
> > 
> > I don't see where the reluctance comes from - is it that there are going
> > to be 2 patches instead of 1? My reluctance as a reviewer comes from the
> > fact that I'm analyzing the change in the larger context and not seeing
> > how the remove() method you introduced makes any practical difference.
> > Not sure what I'm supposed to say.
> 
> I don't have anything against it, like I said before I thought those are
> 2 different issues. But if you think otherwise I can add a new patch in
> this series, no problem.
> 
> Why do you say that the function remove() doesn't make any practical
> difference?

I had thought that the rx_skbs_list can still be queued to, through the
dangling ops that are left behind in /sys/class/ptp/ when the PHY driver
is removed. But it looks like this isn't the case, and the issues are
indeed unrelated.

Re: [PATCH net v4] phy: mscc: Fix timestamping for vsc8584

[Vladimir Oltean]

On Mon, Aug 18, 2025 at 10:10:29AM +0200, Horatiu Vultur wrote:
> There was a problem when we received frames and the frames were
> timestamped. The driver is configured to store the nanosecond part of
> the timestmap in the ptp reserved bits and it would take the second part
> by reading the LTC. The problem is that when reading the LTC we are in
> atomic context and to read the second part will go over mdio bus which
> might sleep, so we get an error.
> The fix consists in actually put all the frames in a queue and start the
> aux work and in that work to read the LTC and then calculate the full
> received time.
> 
> Fixes: 7d272e63e0979d ("net: phy: mscc: timestamping and PHC support")
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

I think the patch is in "Changes Requested" in patchwork because of me,
so I'll try this, see if it helps:

pw-bot: under-review

Re: [PATCH net v4] phy: mscc: Fix timestamping for vsc8584

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 18 Aug 2025 10:10:29 +0200 you wrote:
> There was a problem when we received frames and the frames were
> timestamped. The driver is configured to store the nanosecond part of
> the timestmap in the ptp reserved bits and it would take the second part
> by reading the LTC. The problem is that when reading the LTC we are in
> atomic context and to read the second part will go over mdio bus which
> might sleep, so we get an error.
> The fix consists in actually put all the frames in a queue and start the
> aux work and in that work to read the LTC and then calculate the full
> received time.
> 
> [...]

Here is the summary with links:
  - [net,v4] phy: mscc: Fix timestamping for vsc8584
    https://git.kernel.org/netdev/net/c/bc1a59cff9f7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html