From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 561B1C433DB for ; Tue, 9 Mar 2021 18:27:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 096CB6522E for ; Tue, 9 Mar 2021 18:27:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229805AbhCIS0k convert rfc822-to-8bit (ORCPT ); Tue, 9 Mar 2021 13:26:40 -0500 Received: from mail-yb1-f177.google.com ([209.85.219.177]:42338 "EHLO mail-yb1-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229691AbhCIS0Y (ORCPT ); Tue, 9 Mar 2021 13:26:24 -0500 Received: by mail-yb1-f177.google.com with SMTP id n195so14953115ybg.9 for ; Tue, 09 Mar 2021 10:26:23 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=aS99F6wsyZ+OH8LrhLwfK0hXrDzy8Tatm3jXreStxmw=; b=nbPfINEpCY2e55BzpqKfUtdNyHgYQLvtwxi1H7kDE969+oWlCwwKlTIkUA5sdXALP+ yK7PLB2CpOvLYH3w1s+oxWYNh8sIduEjb6drxpqokfyJSPjTStyCMZ7pKkvuAiC66En/ q/PDhZMqBps/eZ7+WGS5V7VhrYBo8DPONLljydsrS2VFmjCGUJq0/2pYa4GVHeUt5eAm LUq2WiMn7UohqoxZBclM9WTgzU2/NlfQMVGoBY/Q2CJ0Rj7SMTAhMUaAcBagEAsUh/J8 JbZ1SN3DBl7g9zVITzSXU6xJpuut1rsZdgEp6Ug7i3DTzhQDxRJXaK8G68G0KHmFGJAA vzSQ== X-Gm-Message-State: AOAM5306VfhMrizdxMT13qg78xn2u/6wvR9CFJWRAdYRtg3y8heIkvDK SBogjNKgj8p0D2ZGIaV4AdSvU0Dt9a2deiPBAqU= X-Google-Smtp-Source: ABdhPJzVt3/Wz6ZSl5r0b8C4WYxF0vf5cgEn4jzrSbwW/9U70GP5yMwVXjRB847VF8ChZ6yDPck12brK9OdgnoEv7nc= X-Received: by 2002:a25:2d1f:: with SMTP id t31mr44516561ybt.239.1615314383358; Tue, 09 Mar 2021 10:26:23 -0800 (PST) MIME-Version: 1.0 References: <20210308163445.103636-1-mailhol.vincent@wanadoo.fr> <20210308163445.103636-2-mailhol.vincent@wanadoo.fr> <2b43e72b-c561-d144-c01e-c4ea361cc932@pengutronix.de> <20210309125708.ei75tr5vp2sanfh6@pengutronix.de> <20210309153547.q7zspf46k6terxqv@pengutronix.de> In-Reply-To: From: Vincent MAILHOL Date: Wed, 10 Mar 2021 03:26:12 +0900 Message-ID: Subject: Re: [PATCH v12 1/1] can: usb: etas_es58X: add support for ETAS ES58X CAN USB interfaces To: Marc Kleine-Budde Cc: linux-can , Arunachalam Santhanam Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT Precedence: bulk List-ID: X-Mailing-List: linux-can@vger.kernel.org Le mer. 10 mars 2021 à 02:54, Vincent MAILHOL a écrit : > > On Wed. 10 Mar 2021 at 00:35, Marc Kleine-Budde wrote: > > > > On 09.03.2021 22:10:08, Vincent MAILHOL wrote: > > > Sounds good to me. I will prepare a patch to explain the issue > > > and try to introduce the dql_set_min_limit() function. > > > > > > Meanwhile, I would be thankful if you could continue the review :) > > > > Thanks for the mail, looks good. > > > > One note for the patch, though: > > > > > diff --git a/include/linux/dynamic_queue_limits.h b/include/linux/dynamic_queue_limits.h > > > index 407c2f281b64..32437f168a35 100644 > > > --- a/include/linux/dynamic_queue_limits.h > > > +++ b/include/linux/dynamic_queue_limits.h > > > @@ -103,6 +103,9 @@ void dql_reset(struct dql *dql); > > > /* Initialize dql state */ > > > void dql_init(struct dql *dql, unsigned int hold_time); > > > > > > +/* Set the dql minimum limit */ > > #ifdef CONFIG_DQL > > > +void dql_set_min_limit(struct dql *dql, unsigned int min_limit); > > #else > > static inline void dql_set_min_limit(struct dql *dql, unsigned int min_limit) > > { > > } > > #endif > > > + > > > #endif /* _KERNEL_ */ > > > > > > #endif /* _LINUX_DQL_H */ > > > diff --git a/lib/dynamic_queue_limits.c b/lib/dynamic_queue_limits.c > > > index fde0aa244148..8b6ad1e0a2e3 100644 > > > --- a/lib/dynamic_queue_limits.c > > > +++ b/lib/dynamic_queue_limits.c > > > > This file is only compiled if CONFIG_DQL is set, see lib/Makefile: > > > > | obj-$(CONFIG_DQL) += dynamic_queue_limits.o > > Got it. > > > > @@ -136,3 +136,11 @@ void dql_init(struct dql *dql, unsigned int hold_time) > > > dql_reset(dql); > > > } > > > EXPORT_SYMBOL(dql_init); > > > + > > > +void dql_set_min_limit(struct dql *dql, unsigned int min_limit) > > > +{ > > > +#ifdef CONFIG_BQL > > > > remove this ifdef > > > > > + dql->min_limit = min_limit; > > > +#endif > > > +} > > > +EXPORT_SYMBOL(dql_set_min_limit); > > Actually, after doing a few more tests, this is a bit more complicated > than anticipated. > The dql member of struct netdev_queue is also guarded by a #ifdef CONFIG_BQL: > https://elixir.bootlin.com/linux/latest/source/include/linux/netdevice.h#L629 > > This means that under the current idea, we would also need to guard > the call to dql_set_min_limit(): > #ifdef CONFIG_BQL > dql_set_min_limit(&netdev_get_tx_queue(netdev, 0)->dql, > es58x_dev->param->dql_limit_min); > #ifdef CONFIG_BQL > > This kills the initial intent of not using the #ifdef CONFIG_BQL to > set the value. > > This leads to the need to do: > void netdev_queue_set_dql_min_limit(struct netdev_queue *q, unsigned int min_limit) Of course, I meant: static inline void netdev_queue_set_dql_min_limit(struct netdev_queue *q, unsigned int min_limit) > { > #ifdef CONFIG_BQL > q->dql.min_limit = min_limit; > #endif > } > which would probably be inside /include/linux/netdevice.h. > > Does it make sense?