Received-SPF: pass (google.com: domain of linux-kernel+bounces-40443-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: rohan.g.thomas@intel.com
To: esben@geanix.com
Cc: alexandre.torgue@foss.st.com,
	conor+dt@kernel.org,
	davem@davemloft.net,
	devicetree@vger.kernel.org,
	edumazet@google.com,
	fancer.lancer@gmail.com,
	joabreu@synopsys.com,
	krzysztof.kozlowski+dt@linaro.org,
	kuba@kernel.org,
	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,
	peppe.cavallaro@st.com,
	robh@kernel.org,
	rohan.g.thomas@intel.com
Subject: RE: [PATCH net-next 1/2] dt-bindings: net: snps,dwmac: Time Based Scheduling
Date: Sat, 27 Jan 2024 01:36:34 +0800
Message-Id: <20240126173634.13162-1-rohan.g.thomas@intel.com>
In-Reply-To: <87msss4gtj.fsf@geanix.com>
References: <87msss4gtj.fsf@geanix.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

From: Rohan G Thomas <rohan.g.thomas@intel.com>

On Fri, 26 Jan 2024 09:52:40 +0100, Esben Haabendal wrote:
Hi Esben,

Thanks for your comments. Like to get some clarification on a few
things.

> >>
> >>Seems like OS configuration and policy.
> >
> > Tx queues need to be configured for TBS during hw setup itself as
> > special enhanced descriptors are used by the hw for TBS support
> > enabled queues. Switching between enhanced and normal descriptors on
> > run is not feasible. So this flag is for enabling "Enhanced
> > Descriptors for Time Based Scheduling". This I think is a hw specific
> > requirement.
> 
> Support for enhanced descriptors is definitely hardware specific.
> Enabling the use of enhanced descriptors is a configuration choice.
> 
> The tricky part here is that the whole devicetree bindings story for the
> stmmac driver is filled with such configuration choices. As such, it is
> only natural to add the property you are suggesting here. I completely
> agree. But you can also argue that it is "wrong", because it does not
> just describe the hardware, but also a configuration choice.

Isn't this requirement of using enhanced tx desc instead of normal tx
desc to support TBS is specific to Synopsys IP? Switching from
normal desc to enhanced desc at the time of tc-etf qdisc offload
cannot be done without traffic disruption, which I don't think is
acceptable. Since this behavior is IP specific, can we consider
this as an OS configuration choice?

Agreed that this feature(use of enhanced desc) can be enabled from
glue drivers. But I added this dt property, thinking this feature is
specific and common to DWMAC core and we can enable this feature for
stmmac platform driver without a glue driver. If this is not
acceptable, I can think of doing this from the glue driver.

> >>Doesn't eh DWMAC have capability registers for supported features? Or
> >>did they forget per queue capabilities?
> >
> > Yes, capability registers are available. For DWMAC5 IP, if TBSSEL bit
> > is set, then TBS is supported by all Tx queues.
> 
> Not true. Some NXP imx8 and imx9 chips support Synopsys MAC 5.10a IP,
> and does not support TBS for queue 0. And they have TBSSEL bit set, but
> no TBS_CH support.

AFAIU from Synopsys DWMAC5 Databook, all queues support TBS. But TBS
cannot coexist with TSO. So all glue drivers enabling TBS feature
avoid queue 0 to support TSO. Please correct me if I'm wrong.

> 
> > For DWXGMAC IP, if TBSSEL bit is set, then TBS is supported by TBS_CH
> > number of Tx queues starting from the highest Tx queue. But because of
> > the hw limitations mentioned above, TBS cannot be enabled for all
> > capable queues.
> >

BR,
Rohan