Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 14 Jun 2023 11:19:28 +0200
From:   Horatiu Vultur <horatiu.vultur@microchip.com>
To:     Richard Cochran <richardcochran@gmail.com>
CC:     <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <andrew@lunn.ch>, <hkallweit1@gmail.com>, <linux@armlinux.org.uk>,
        <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>
Subject: Re: [PATCH net-next v2 2/2] net: micrel: Schedule work to read
 seconds for lan8841
Message-ID: <20230614091928.5oi5r7sw7tac27lt@soft-dev3-1>
References: <20230613094526.69532-1-horatiu.vultur@microchip.com>
 <20230613094526.69532-3-horatiu.vultur@microchip.com>
 <ZIlG4otXfQ7uhMsc@hoboy.vegasvil.org>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
In-Reply-To: <ZIlG4otXfQ7uhMsc@hoboy.vegasvil.org>
Precedence: bulk

The 06/13/2023 21:49, Richard Cochran wrote:

Hi Richard,

> 
> On Tue, Jun 13, 2023 at 11:45:26AM +0200, Horatiu Vultur wrote:
> > @@ -3840,6 +3847,12 @@ static void lan8841_ptp_enable_processing(struct kszphy_ptp_priv *ptp_priv,
> >                              LAN8841_PTP_INSERT_TS_32BIT,
> >                              LAN8841_PTP_INSERT_TS_EN |
> >                              LAN8841_PTP_INSERT_TS_32BIT);
> > +
> > +             /* Schedule the work to read the seconds, which will be used in
> > +              * the received timestamp
> > +              */
> > +             schedule_delayed_work(&ptp_priv->seconds_work,
> > +                                   nsecs_to_jiffies(LAN8841_GET_SEC_LTC_DELAY));
> 
> Why not do this in the PTP kworker thread?

I presume you mean the work of reading the second part to be done in the
PTP kworker thread and not scheduling the seconds_work.
Because then it make sense to me and I think is a great idea.
> 
> The thread's scheduling can be easily tuned with chrt to give it
> appropriate priority, but work can't.

Nice, I didn't know about this.

> 
> Also, If you have seconds thread, then you don't have to defer the
> received frames.

Exactly, the PTP kworker thread will cache the seconds value while
lan8841_rxtstamp will read this value, so no need to defer these frames.

In this way I can get rid of seconds_work.

> 
> Thanks,
> Richard
> 

-- 
/Horatiu