Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Date:   Fri, 6 Oct 2023 13:08:33 +0300
From:   Serge Semin <fancer.lancer@gmail.com>
To:     Rohan G Thomas <rohan.g.thomas@intel.com>,
        Jakub Kicinski <kuba@kernel.org>
Cc:     alexandre.torgue@foss.st.com, andriy.shevchenko@linux.intel.com,
        davem@davemloft.net, devicetree@vger.kernel.org,
        edumazet@google.com, joabreu@synopsys.com,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-stm32@st-md-mailman.stormreply.com,
        mcoquelin.stm32@gmail.com, netdev@vger.kernel.org,
        pabeni@redhat.com
Subject: Re: [PATCH net-next 1/1] net: stmmac: xgmac: EST interrupts handling
Message-ID: <cjgx6e3agc6gpvs75nhkf6wlztk73epmct6tcuooyqvk2nx2o2@vr5buyk637t3>
References: <20231005070538.0826bf9d@kernel.org>
 <20231006072319.22441-1-rohan.g.thomas@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231006072319.22441-1-rohan.g.thomas@intel.com>
Precedence: bulk

Hi Rohan, Jakub

On Fri, Oct 06, 2023 at 03:23:19PM +0800, Rohan G Thomas wrote:
> On Thu, 5 Oct 2023 07:05:38 -0700 Jakub Kicinski wrote:
> > On Thu, 5 Oct 2023 20:14:41 +0800 Rohan G Thomas wrote:
> > > > So the question now is whether we want Rohan to do this conversion
> > > > _first_, in DW QoS 5, and then add xgmac part. Or the patch should
> > > > go in as is and you'll follow up with the conversion?

Jakub, this was my intention if Rohan wouldn't have agreed to perform
the cleanup. Though I couldn't promise to do that in an instant.
I would have needed a month or two to find a free time-spot for that.

> > >
> > > If agreed, this commit can go in. I can submit another patch with the
> > > refactoring suggested by Serge.
> > 
> > Did you miss the emphasis I put on the word "first" in my reply?
> > Cleanup first, nobody will be keeping track whether your fulfilled your
> > promises or not :|
> 
> Hi Jakub,
> 
> Agreed. I'll do the cleanup first.

Rohan, thanks in advance. Although I don't see why it's required to be
done in the prescribed order only as long as the update comes in a
_single_ patchset. Adding EST IRS-status support to XGMAC core module
first, then factoring out both XGMAC and QoS (note both 4.x and 5.x
seems to support that) EST code would be also acceptable. Seeing you
have already done the first part, it would have taken less work in
general.

Jakub, what do you say if Rohan will just re-submit v2 with the
addition cleanup patch and let him to decided whether the cleanup
should be done first or after his XGMAC-EST IRQ update?

> > > Again, thanks Serge for the prompt response. Regarding the below point in your
> > > earlier response,

> > > > > 2. PTP time offset setup performed by means of the
> > > > > MTL_EST_CONTROL.PTOV field. DW QoS Eth v5.x HW manual claims it's "The
> > > > > value of PTP Clock period multiplied by 6 in nanoseconds." So either Jose got
> > > > > mistaken by using _9_ for DW XGMAC v3.x or the DW XGMAC indeed is
> > > > > different in that aspect.
> > > 
> > > This is a little confusing...
> > > I referred databooks for DW QoS Eth v5.30a and DW XGMAC v3.10a. In both this is
> > > mentioned as "The value of PTP Clock period multiplied by 9 in nanoseconds".

Interesting thing. My DW QoS Eth _v5.10a_ HW manual explicitly states
that it's multiplied by _6_ in nanoseconds (just rechecked). So either
there is a difference between the minor DW QoS Eth IP-core releases or
the older HW-manuals have had a typo in the MTL_EST_CONTROL.PTOV field
description. Synopsys normally describes such changes (whether it was
a mistake or a functional change) in the IP-core release notes. The
release notes document is supplied as a separate pdf file. Alas I
don't have one.( Even if I had it it would have been useless since the
change was introduced in the newer QoS IP-cores. Rohan, do you happen
to have the release notes for DW QoS Eth IP-core v5.30 at hands?
Something like DWC_ether_qos_rc_relnotes.pdf?

Also please double check that your DW QoS Eth v5.30a for sure states
that MTL_EST_CONTROL.PTOV contains value multiplied by _6_. So we
wouldn't be wasting time trying to workaround a more complex problem
than we already have.

-Serge(y)

> 
> Best Regards,
> Rohan