Date:   Mon, 20 Feb 2023 13:48:16 +0200
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     Ferenc Fejes <fejes@inf.elte.hu>
Cc:     netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Jamal Hadi Salim <jhs@mojatatu.com>,
        Cong Wang <xiyou.wangcong@gmail.com>,
        Jiri Pirko <jiri@resnulli.us>,
        Vinicius Costa Gomes <vinicius.gomes@intel.com>,
        Kurt Kanzenbach <kurt@linutronix.de>,
        Gerhard Engleder <gerhard@engleder-embedded.com>,
        Amritha Nambiar <amritha.nambiar@intel.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        UNGLinuxDriver@microchip.com, Andrew Lunn <andrew@lunn.ch>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Xiaoliang Yang <xiaoliang.yang_1@nxp.com>,
        Roger Quadros <rogerq@kernel.org>,
        Pranavi Somisetty <pranavi.somisetty@amd.com>,
        Harini Katakam <harini.katakam@amd.com>,
        linux-kernel@vger.kernel.org,
        Giuseppe Cavallaro <peppe.cavallaro@st.com>,
        Alexandre Torgue <alexandre.torgue@foss.st.com>,
        Jose Abreu <joabreu@synopsys.com>,
        Wong Vee Khee <vee.khee.wong@linux.intel.com>,
        Michael Sit Wei Hong <michael.wei.hong.sit@intel.com>,
        Jacob Keller <jacob.e.keller@intel.com>
Subject: Re: [PATCH v2 net-next 00/12] Add tc-mqprio and tc-taprio support
 for preemptible traffic classes
Message-ID: <20230220114816.vfpabqxmkq4zul24@skbuf>
References: <20230219135309.594188-1-vladimir.oltean@nxp.com>
 <d016f61224b293a77969c35d09d65d5cfea7d137.camel@inf.elte.hu>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <d016f61224b293a77969c35d09d65d5cfea7d137.camel@inf.elte.hu>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?MAtj4F6lZh/C/PniXz3I59ktTD1cNShugo//pwi+JmfT3K1W9gsgrwSjGw8f?=
 =?us-ascii?Q?00HqZUPyK8oyav35INj60uKa2qhjDjf3xnRModuKxmOGBL/YvVZ4gipMi5pN?=
 =?us-ascii?Q?LUjssTPEm4E3GsYr+yX8faMsZr4tYfvi9qv6UwRTNox46eJk101DewjrPOVL?=
 =?us-ascii?Q?KmLQjL3bpYK11mJikfchFzJuefnhLlMscmYSWZ5rgGZggBwbKd9DzY0TKhvL?=
 =?us-ascii?Q?1K5qP+Wfqc/Y9ZwYyfHGt9qqsrQgJkn3GcfhkpvcbHliZVL7yYFbo9SzY7Jh?=
 =?us-ascii?Q?e6FyYCcATa6mfaVQa1PINtszzih+oFdx2H65xtkRyhI5pwB7Whg3Xhg2xzOU?=
 =?us-ascii?Q?9LFaLr39meJRwfxTotvW2ftJ+HMlEE4o7yMRcPPm9RHN9dfDLYFJbD5WFQ2m?=
 =?us-ascii?Q?1I2U/YYmxdUmKDOUKHmzl1I3f0BPhSyDCJJzDmj0FsLxHIgqxBq9aX3EYdo7?=
 =?us-ascii?Q?pntKyDPXny3pZFbWLMFo+cM/jFAPT3ZVmIOL4+URp3YtGyDplInpe9mqeHYb?=
 =?us-ascii?Q?OgVz/UZ4UUgdhT+4cYbtk50RnkFFOVCzWZUp2HyUJVVRU/DsJidK7WsrDY27?=
 =?us-ascii?Q?UGKfnAolvNawWIqojcSbb6bwruzp4pfm6fC7a414U8oRa8+h2fQWLGDmVt7y?=
 =?us-ascii?Q?jjynAKDKzz2bayfBdf+AXVFN7zn439CpOT3aVXwwgAQPNKg10GhlU919pGUk?=
 =?us-ascii?Q?vRa8uQfl5+0E53yQP6FjIoo5IJMIN/E8U2EKALKfsqe42WqXPdIDsXUSEsIe?=
 =?us-ascii?Q?yljXjzXljbHTO4mAFVhXUOaHnH3j+0c/Ef0FCimMv7OXyQSL2d9SEonQ0Jcr?=
 =?us-ascii?Q?FMRouPrbEShkqtB5SuAskxTixy12Ttc3AcyOhE2FEZuPVeD3drrAo0fXAlhO?=
 =?us-ascii?Q?axxD+6qKZOFz58iCsDpsDxyEpPMUC0p7A9ZcohS+pUp3x4m9PH+Q9nJbe9YC?=
 =?us-ascii?Q?LY8+F1bcuVhxTQVQEF7a0u0Paf14uaRjLwjqFWMf0PjCzEKuCIVW7a/Rqs0v?=
 =?us-ascii?Q?s35nuv2oTs4nT9nvOLHrjfucbSt28Y8w/YOmdWpfZZTIrqBzbdH4UExvK9bH?=
 =?us-ascii?Q?ajLEV1kNIwbDGFQz0R7ScdLc+VQUZkw0ILFItgU0L8qgZ9JZAqR0F+arvZYP?=
 =?us-ascii?Q?J0EC8JYMDWegthvOBUim3KdZCdF+GcU9re3JZmwmlJbBlt3XKwTYc1+/qPve?=
 =?us-ascii?Q?lxaFBRnJhVODGwctL25gekLqbKsDXAzEh2vZrp1sW+FBXxwVvkK7SEIXUsaS?=
 =?us-ascii?Q?VwyexhV6QY6MwULwYDJNnjj8jIu6roT5CluDpswLfmD5im6jalcOimtwIO0T?=
 =?us-ascii?Q?oZ5GdcLyof7nSkBsmmVFQMc7cveNNOXuLRfKhBIElsvLjiCyDoYBhhYeUytP?=
 =?us-ascii?Q?D5v+Glev5xfzYfkfAYnMUaM0ZtJ5HQjQ0R4iI6rbiqN9ghwbXxXAG2yMqheI?=
 =?us-ascii?Q?ODyS+dbuGbvgWhqMhEwlCiLQLlQrqN6KqGDxy5U1RYkuGZm4cg2kgibSYtS0?=
 =?us-ascii?Q?UYo0xM6GL9rD00lnXCDu71/QlSppNga74b29BPJgq61xRq7dRfNwSmdxNdV5?=
 =?us-ascii?Q?eDh5fPs9e9Qdochu989V6JnlUtyrEHrT6YUCn6rkjkqR7oraaJI2sPhmS6Yi?=
 =?us-ascii?Q?Cg=3D=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 330c4832-2747-437a-166e-08db13385d74
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB5136.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Feb 2023 11:48:21.2073
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Fol+p8EdGEghnUuf0F2cgcyL5M9gk3E9wlGVh5kSukJ2S8dudw8eu7zjffDkX1DpKSo8m1Feg9GqUySsE5SIQw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR04MB8478
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Feb 20, 2023 at 12:15:57PM +0100, Ferenc Fejes wrote:
> LGTM.
> 
> Reviewed-by: Ferenc Fejes <fejes@inf.elte.hu>

Thanks a lot for the review!

Unfortunately I need to send a v3, because the C language apparently
doesn't like "default" switch cases with no code, and I need to make
this change (which surprises me, since the code did compile fine with
my gcc-arm-11.2-2022.02-x86_64-aarch64-none-linux-gnu toolchain):

diff --git a/drivers/net/ethernet/mscc/ocelot_mm.c b/drivers/net/ethernet/mscc/ocelot_mm.c
index 21d5656dfc70..f7766927bdd2 100644
--- a/drivers/net/ethernet/mscc/ocelot_mm.c
+++ b/drivers/net/ethernet/mscc/ocelot_mm.c
@@ -57,20 +57,16 @@ void ocelot_port_update_preemptible_tcs(struct ocelot *ocelot, int port)
 
 	lockdep_assert_held(&mm->lock);
 	
-	/* On NXP LS1028A, when using QSGMII, the port hangs if transmitting
-	 * preemptible frames at any other link speed than gigabit
+	/* Only commit preemptible TCs when MAC Merge is active.
+	 * On NXP LS1028A, when using QSGMII, the port hangs if transmitting
+	 * preemptible frames at any other link speed than gigabit, so avoid
+	 * preemption at lower speeds in this PHY mode.
 	 */
-	if (ocelot_port->phy_mode != PHY_INTERFACE_MODE_QSGMII ||
-	    ocelot_port->speed == SPEED_1000) {
-		/* Only commit preemptible TCs when MAC Merge is active */
-		switch (mm->verify_status) {
-		case ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED:
-		case ETHTOOL_MM_VERIFY_STATUS_DISABLED:
-			val = mm->preemptible_tcs;
-			break;
-		default:
-		}
-	}
+	if ((ocelot_port->phy_mode != PHY_INTERFACE_MODE_QSGMII ||
+	     ocelot_port->speed == SPEED_1000) &&
+	    (mm->verify_status == ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED ||
+	     mm->verify_status == ETHTOOL_MM_VERIFY_STATUS_DISABLED))
+		val = mm->preemptible_tcs;
 	
 	ocelot_rmw_rix(ocelot, QSYS_PREEMPTION_CFG_P_QUEUES(val),
 		       QSYS_PREEMPTION_CFG_P_QUEUES_M,

Besides, I'm also taking the opportunity to make one more change, and
really do a thorough job with the netlink extack: I will be passing it
down to the device driver in v3, via struct tc_mqprio_qopt_offload and
struct tc_taprio_qopt_offload.

I'll replicate your review tag for all patches from v2 that will be
present in an unchanged form in v3, ok?