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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E8A01ECAAA1 for ; Fri, 9 Sep 2022 12:41:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229789AbiIIMlh (ORCPT ); Fri, 9 Sep 2022 08:41:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49784 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230270AbiIIMld (ORCPT ); Fri, 9 Sep 2022 08:41:33 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1EFD4D99CF; Fri, 9 Sep 2022 05:41:33 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id B949C61FC9; Fri, 9 Sep 2022 12:41:32 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2491EC433C1; Fri, 9 Sep 2022 12:41:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1662727292; bh=/ZRVUtdEfLA1Q7ddcRbz1aDFhErn44JrkcHfJH9/laI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=JKUR0m9UMvBhQxBe1X97QpC6YoeIjOoYfUfFEGgnNSI2+wxr8RFJ4I7VDZaZ3pRiX IzWJcWq5b22fbPNQfs+Moe37jxulkGnEWAaL9GG5ZJ8h0F12OzCD8u3vW81bjTGAVe NlcmB+4gfZPdHRSRjtRDKFTLBpONtOzYF5bmyw1rkjnqN4qcXY5YQOIDUXV1SrYexh Ng3+PCREQk9Wc0XsWhOaKuysTBtg92IzTSLlAeG6xoTXc6OiAEes30zx1EKxnJNiyS g22XScVNPl1dHk+AfyKhD6hzWg+PTN8EMgNsLMxBO2VApJtPk8nv1mPgJEOR1SamzD Mngf5Wq3wMByA== Received: from johan by xi.lan with local (Exim 4.94.2) (envelope-from ) id 1oWdKH-0001g1-Hm; Fri, 09 Sep 2022 14:41:41 +0200 Date: Fri, 9 Sep 2022 14:41:41 +0200 From: Johan Hovold To: Jiri Slaby Cc: Arnd Bergmann , Russell King , Greg Kroah-Hartman , Ilpo =?utf-8?B?SsOkcnZpbmVu?= , linux-serial , LKML , Tobias Klauser , Richard Genoud , Nicolas Ferre , Alexandre Belloni , Claudiu Beznea , Vladimir Zapolskiy , Liviu Dudau , Sudeep Holla , Lorenzo Pieralisi , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , Andreas =?utf-8?Q?F=C3=A4rber?= , Manivannan Sadhasivam , Florian Fainelli , bcm-kernel-feedback-list@broadcom.com, Pali =?utf-8?B?Um9ow6Fy?= , Kevin Cernekee , Palmer Dabbelt , Paul Walmsley , Orson Zhai , Baolin Wang , Chunyan Zhang , Patrice Chotard , linux-riscv@lists.infradead.org Subject: Re: [PATCH v3 0/4] tty: TX helpers Message-ID: References: <4e9b4471-a6f2-4b16-d830-67d253ae4e6a@linux.intel.com> <715b40ba-1bcc-4582-bed1-ef41126c7b94@www.fastmail.com> <2197faa3-0217-41e0-8ff0-b5396561c623@www.fastmail.com> <5feff23c-9458-616c-66ce-13cca5829162@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5feff23c-9458-616c-66ce-13cca5829162@kernel.org> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Sep 09, 2022 at 12:53:04PM +0200, Jiri Slaby wrote: > On 07. 09. 22, 16:56, Arnd Bergmann wrote: > > On Wed, Sep 7, 2022, at 3:52 PM, Russell King (Oracle) wrote: > >> On Wed, Sep 07, 2022 at 02:36:37PM +0200, Greg Kroah-Hartman wrote: > >> > >> Of course, it would have been nicer to see the definition of this > >> macro, because then we can understand what the "ch" argument is to > >> this macro, and how that relates to the macro argument that is > >> shown in the example as a writel(). > > > > I pulled out the 'ch' variable from the macro to avoid having > > the macro define local variables that are then passed to the > > inner expressions. > > Note that I had "port" and "ch" as a part of the macro parameters in > [v2], but it didn't help the situation much. > >> Maybe a more complete example would help clear up the confusion? > >> Arnd? > > > > Here is a patch on top of the series that would implement the > > uart_port_tx_helper_limited() and uart_port_tx_helper() > > macros that can be used directly from drivers in place of defining > > local functions, with the (alphabetically) first two drivers > > converted to that. > > If there are no objections, I will push the patches this directorin. I > like this more than [v2] or [v3] (the helper macros). Actually, I > mentioned this wait_event() style in [v1], but I perhaps simplified the > concept too much to completely eliminate the need of a wrapper function. > And that made it too complicated/too hard to understand. This sounds much better. You also had some users that needed some preamble which could now go in the same function (e.g. altera_jtaguart_tx_chars()). > Except I'd drop the "_helper" part from the name. Originally (in [v1]), > I had uart_port_tx() and uart_port_tx_limited() functions. In [v2+v3], I > added _helper to avoid confusion as we were generating a helpers using > the macros. Yes, technically, uart_port_tx() is still a helper, but I > think it's superfluous to have it in the name now. That would also be an in improvement. For the altera example you could end up with something like: static void altera_jtaguart_tx_chars(struct altera_jtaguart *pp) { struct uart_port *port = &pp->port; unsigned int space; space = (readl(port->membase + ALTERA_JTAGUART_CONTROL_REG) & ALTERA_JTAGUART_CONTROL_WSPACE_MSK) >> ALTERA_JTAGUART_CONTROL_WSPACE_OFF; uart_port_tx_chars(port, space, writel(ch, port->membase + ALTERA_JTAGUART_DATA_REG)); } which would be understandable. If there are too many arguments to be passed in, then perhaps you can explore Arnd's (and you own) suggestion to use inline helpers so that the arguments are named. Johan