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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 83F71C5478C for ; Sat, 2 Mar 2024 20:59:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=srO4cT0OkGNcScthFm0zQbFhOai9ECpa2TY7sX8VvOw=; b=NPtR4HAkflTkrA JzkiL8m/L7JT8rBY52NEK93+LsWWBQiJadONt39AvNr5khTX3AZuB0gCuwZTclXH9xRW5NeD863kw Wpi1VkwyH0iTGadc59Uml8vxj60kGFKcgJBGqSUIiefKGGs5c1MhgqUrGkWYUzwe1orzWaKJWbr2o bSbTihJxjK2sRlfin3SFKwuXBjIBt7elkGNZJiCd1jqu97CDVqaC0H3Qg0J8zOWCeUXNlYLktUkOS koLV8/2wKqIp6Sql2c1SOwQ3w9oe9If9NRNyvbgegnIHkh2sD5HmZnXe7ZcxQLgcMOCF2vicCcmJQ X6AJs5T60SwAF11Upj6w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rgWRm-00000004PLf-0pIU; Sat, 02 Mar 2024 20:59:06 +0000 Received: from sin.source.kernel.org ([145.40.73.55]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rgWRj-00000004PKm-0iUs; Sat, 02 Mar 2024 20:59:04 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 9470BCE0224; Sat, 2 Mar 2024 20:58:57 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 485ABC433C7; Sat, 2 Mar 2024 20:58:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1709413135; bh=o+tr4CCbud8uzLtDh5MolTBCwG3kPTB0Bhx8jY/shKs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=UgMqkU+niNfrrsJKR325G2D+2tAy9twT34HdNKjDPPnzNigNMvu3vA2hb62A57nq0 PEMsKrj7r3EL2UOTajmaknqRQV/d2JCtFJ9pWYRVcF/57lRl/p8lzob7OmHi5mEfNC 4qrR2eTAghx7wO456Nvia8+/Eo8/UCKAhz4WW+Ok= Date: Sat, 2 Mar 2024 21:58:53 +0100 From: Greg Kroah-Hartman To: Andy Shevchenko Cc: Thomas Gleixner , linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, linux-rpi-kernel@lists.infradead.org, linux-mips@vger.kernel.org, linux-tegra@vger.kernel.org, Jiri Slaby , Joel Stanley , Andrew Jeffery , Florian Fainelli , Broadcom internal kernel review list , Ray Jui , Scott Branden , Al Cooper , Ilpo =?iso-8859-1?Q?J=E4rvinen?= , Paul Cercueil , Vladimir Zapolskiy , Thierry Reding , Jonathan Hunter , Kunihiko Hayashi , Masami Hiramatsu Subject: Re: [PATCH v2 03/14] serial: port: Introduce a common helper to read properties Message-ID: <2024030259-playback-starlit-a472@gregkh> References: <20240226142514.1485246-1-andriy.shevchenko@linux.intel.com> <20240226142514.1485246-4-andriy.shevchenko@linux.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240226142514.1485246-4-andriy.shevchenko@linux.intel.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240302_125903_597816_28FAC5C5 X-CRM114-Status: GOOD ( 34.12 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Feb 26, 2024 at 04:19:19PM +0200, Andy Shevchenko wrote: > Several serial drivers want to read the same or similar set of > the port properties. Make a common helper for them. > > Signed-off-by: Andy Shevchenko > --- > drivers/tty/serial/serial_port.c | 134 +++++++++++++++++++++++++++++++ > include/linux/serial_core.h | 1 + > 2 files changed, 135 insertions(+) > > diff --git a/drivers/tty/serial/serial_port.c b/drivers/tty/serial/serial_port.c > index 88975a4df306..ecc980e9dba6 100644 > --- a/drivers/tty/serial/serial_port.c > +++ b/drivers/tty/serial/serial_port.c > @@ -8,7 +8,10 @@ > > #include > #include > +#include > +#include > #include > +#include > #include > #include > > @@ -82,6 +85,137 @@ void uart_remove_one_port(struct uart_driver *drv, struct uart_port *port) > } > EXPORT_SYMBOL(uart_remove_one_port); > > +/** > + * uart_read_port_properties - read firmware properties of the given UART port I like, but: > + * @port: corresponding port > + * @use_defaults: apply defaults (when %true) or validate the values (when %false) Using random booleans in a function is horrid. Every time you see the function call, or want to call it, you need to go and look up what the boolean is and means. Make 2 public functions here, one that does it with use_defaults=true and one =false and then have them both call this one static function, that way the function names themselves are easy to read and understand and maintain over time. thanks, greg k-h > + * > + * The following device properties are supported: > + * - clock-frequency (optional) > + * - fifo-size (optional) > + * - no-loopback-test (optional) > + * - reg-shift (defaults may apply) > + * - reg-offset (value may be validated) > + * - reg-io-width (defaults may apply or value may be validated) > + * - interrupts (OF only) > + * - serial [alias ID] (OF only) > + * > + * If the port->dev is of struct platform_device type the interrupt line > + * will be retrieved via platform_get_irq() call against that device. > + * Otherwise it will be assigned by fwnode_irq_get() call. In both cases > + * the index 0 of the resource is used. > + * > + * The caller is responsible to initialize the following fields of the @port > + * ->dev (must be valid) > + * ->flags > + * ->mapbase > + * ->mapsize > + * ->regshift (if @use_defaults is false) > + * before calling this function. Alternatively the above mentioned fields > + * may be zeroed, in such case the only ones, that have associated properties > + * found, will be set to the respective values. > + * > + * If no error happened, the ->irq, ->mapbase, ->mapsize will be altered. > + * The ->iotype is always altered. > + * > + * When @use_defaults is true and the respective property is not found > + * the following values will be applied: > + * ->regshift = 0 > + * In this case IRQ must be provided, otherwise an error will be returned. > + * > + * When @use_defaults is false and the respective property is found > + * the following values will be validated: > + * - reg-io-width (->iotype) > + * - reg-offset (->mapsize against ->mapbase) > + * > + * Returns: 0 on success or negative errno on failure > + */ > +int uart_read_port_properties(struct uart_port *port, bool use_defaults) > +{ > + struct device *dev = port->dev; > + u32 value; > + int ret; > + > + /* Read optional UART functional clock frequency */ > + device_property_read_u32(dev, "clock-frequency", &port->uartclk); > + > + /* Read the registers alignment (default: 8-bit) */ > + ret = device_property_read_u32(dev, "reg-shift", &value); > + if (ret) > + port->regshift = use_defaults ? 0 : port->regshift; > + else > + port->regshift = value; > + > + /* Read the registers I/O access type (default: MMIO 8-bit) */ > + ret = device_property_read_u32(dev, "reg-io-width", &value); > + if (ret) { > + port->iotype = UPIO_MEM; > + } else { > + switch (value) { > + case 1: > + port->iotype = UPIO_MEM; > + break; > + case 2: > + port->iotype = UPIO_MEM16; > + break; > + case 4: > + port->iotype = device_is_big_endian(dev) ? UPIO_MEM32BE : UPIO_MEM32; > + break; > + default: > + if (!use_defaults) { > + dev_err(dev, "Unsupported reg-io-width (%u)\n", value); > + return -EINVAL; > + } > + port->iotype = UPIO_UNKNOWN; > + break; > + } > + } > + > + /* Read the address mapping base offset (default: no offset) */ > + ret = device_property_read_u32(dev, "reg-offset", &value); > + if (ret) > + value = 0; > + > + /* Check for shifted address mapping overflow */ > + if (!use_defaults && port->mapsize < value) { > + dev_err(dev, "reg-offset %u exceeds region size %pa\n", value, &port->mapsize); > + return -EINVAL; > + } > + > + port->mapbase += value; > + port->mapsize -= value; > + > + /* Read optional FIFO size */ > + device_property_read_u32(dev, "fifo-size", &port->fifosize); > + > + if (device_property_read_bool(dev, "no-loopback-test")) > + port->flags |= UPF_SKIP_TEST; > + > + /* Get index of serial line, if found in DT aliases */ > + ret = of_alias_get_id(dev_of_node(dev), "serial"); > + if (ret >= 0) > + port->line = ret; > + > + if (dev_is_platform(dev)) > + ret = platform_get_irq(to_platform_device(dev), 0); > + else > + ret = fwnode_irq_get(dev_fwnode(dev), 0); > + if (ret == -EPROBE_DEFER) > + return ret; > + if (ret > 0) > + port->irq = ret; > + else if (use_defaults) > + /* By default IRQ support is mandatory */ > + return ret; > + else > + port->irq = 0; > + > + port->flags |= UPF_SHARE_IRQ; > + > + return 0; > +} > +EXPORT_SYMBOL(uart_read_port_properties); EXPORT_SYMBOL_GPL()? I have to ask :) thanks, greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel