Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933547AbdCUQz3 (ORCPT ); Tue, 21 Mar 2017 12:55:29 -0400 Received: from smtprelay4.synopsys.com ([198.182.47.9]:52327 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932727AbdCUQxs (ORCPT ); Tue, 21 Mar 2017 12:53:48 -0400 Subject: Re: [PATCH 3/3] net: stmmac: Use AVB mode by default To: Thierry Reding , Joao Pinto References: <20170321151211.31841-1-thierry.reding@gmail.com> <20170321151211.31841-3-thierry.reding@gmail.com> <20170321164249.GB31875@ulmo.ba.sec> CC: "David S . Miller" , Giuseppe Cavallaro , Alexandre Torgue , "LABBE Corentin" , , From: Joao Pinto Message-ID: Date: Tue, 21 Mar 2017 16:52:42 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.107.19.117] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3074 Lines: 78 Às 4:50 PM de 3/21/2017, Joao Pinto escreveu: > Às 4:42 PM de 3/21/2017, Thierry Reding escreveu: >> On Tue, Mar 21, 2017 at 03:23:00PM +0000, Joao Pinto wrote: >>> Às 3:12 PM de 3/21/2017, Thierry Reding escreveu: >>>> From: Thierry Reding >>>> >>>> Prior to the recent multi-queue changes the driver would configure the >>>> queues to use the AVB mode, but the mode then got switched to DCB. The >>>> hardware still works fine in DCB mode, but my testing capabilities are >>>> limited, so it's safer to revert to the prior setting anyway. >>>> >>>> Signed-off-by: Thierry Reding >>>> --- >>>> include/linux/stmmac.h | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h >>>> index be47b859e954..8349a5c1537b 100644 >>>> --- a/include/linux/stmmac.h >>>> +++ b/include/linux/stmmac.h >>>> @@ -56,8 +56,8 @@ >>>> #define MTL_RX_ALGORITHM_WSP 0x5 >>>> >>>> /* RX/TX Queue Mode */ >>>> -#define MTL_QUEUE_DCB 0x0 >>>> -#define MTL_QUEUE_AVB 0x1 >>>> +#define MTL_QUEUE_AVB 0x0 >>>> +#define MTL_QUEUE_DCB 0x1 >>>> >>>> /* The MDC clock could be set higher than the IEEE 802.3 >>>> * specified frequency limit 0f 2.5 MHz, by programming a clock divider >>>> >>> >>> Thierry, I don't understand this patch. It will have 0 impact. >>> >>> In stmmac_platform configuration, 0 impact: >>> >>> if (of_property_read_bool(q_node, "snps,dcb-algorithm")) >>> plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB; >>> else if (of_property_read_bool(q_node, "snps,avb-algorithm")) >>> plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_AVB; >>> else >>> ** plat->rx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB; >>> >>> In dwmac4_core, 0 impact: >>> >>> value &= GMAC_RX_QUEUE_CLEAR(queue); >>> if (mode == MTL_QUEUE_AVB) >>> value |= GMAC_RX_AV_QUEUE_ENABLE(queue); >>> else if (mode == MTL_QUEUE_DCB) >>> value |= GMAC_RX_DCB_QUEUE_ENABLE(queue); >>> >>> I think you should set the default mode in (**). >> >> That was my initial attempt, but then I realized that for old DTBs, >> stmmac_mtl_setup() will already exit prematurely because of the missing >> snps,mtl-{rx,tx}-config properties. It's pretty much for the same reason >> as the separate assignment of the default {rx,tx}_queues_to_use. In this >> case it's somewhat more obfuscated, though. Changing AVB to be mode 0 >> means that plat->rx_queues_cfg[].mode_to_use will contain AVB as default >> because plat is devm_kzalloc()'ed. >> >> Effectively this change makes all queues use AVB by default unless they >> are configured using the new device tree bindings. > > Yes I keep forgeting that :), but you are assuming that > plat->rx_queues_cfg[queue].mode_to_use is 0 by default, which might not be the > case, but I agree with you that this is the simpler approach. Let's see what > others have to say. Forget what I said, yes devm_kzalloc() in plat guarantees this. I need a cup of coffee :). Acked-By: Joao Pinto >> >> Thierry >> >