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 17673C433F5 for ; Tue, 1 Feb 2022 19:26:47 +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-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=gqqypID8zrJcUx7LfGw7FsLmB6oz00qe+iHrvYzuEgQ=; b=2TkqSS6BzQ/j2N PMhAcHG75DO+vhDeQrgaOd2sfcOQqipweA+p6GpA+mfaCuuGXHtSaZqTFbVhzsWeehMaFUvqHR95d 409sO4Ge2LB8HqjDNJIB9/Iz+UwczA6rIknHmJOxTeaIkdYiQm+0Rt5uxGZWyWx+/ncH/MbjkkfCV 5l9njWQ6X3Z6X3XDeaxTakTh8fJEVvkK5WocRvG4PXb6vYfFbEqNDOYzQvUThsiA1ERicCmA+l6e8 fEjWRleWNUA2GHFLFZFvSVgW0BBFKj3zLN6yQXkblrR13hY5ENBWeKxBD7Uc5VL91PRMco68XRuOp t/D21BIYNn4CssJ+Ht4g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nEylt-00DXR2-T1; Tue, 01 Feb 2022 19:24:58 +0000 Received: from mail-pf1-x429.google.com ([2607:f8b0:4864:20::429]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nEylq-00DXPx-G6; Tue, 01 Feb 2022 19:24:56 +0000 Received: by mail-pf1-x429.google.com with SMTP id n32so16742602pfv.11; Tue, 01 Feb 2022 11:24:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=WtH7J3WjmpcbZAmRlDiPtipYhiaSIsPLmtCwtZTQaEg=; b=fISu+HkAuf2TIGFzCTkNccdPeeMZDiwM8MEL7h1vd3cz9tNXhx297rf37ArL2vRaNu Dn0EDiHRycMoL8bX/iTXeoza8FQrwaDRyOgOx9d1a+9V0D5y1/MuNoMAYqfI6dDDvHnE SZki9pHC8doyCKPuI4SeHAgsLpfDmW3ytPwmwdd8mduOnySDIYdolsD+mVMVD/djYoas oD6i8ujIc6dU5ZpIu4ZvLp7M3j0I9eKBtAOqvVqmt16rYzmcoLL7nXheEsA7X+dvWoOy 4BrbNSFaxsNqBUgpmA70I81v8YEv1l+ywXjj4FrZcf6pZUI4wyOGaynFWokpm1e/zeSX 9nVQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=WtH7J3WjmpcbZAmRlDiPtipYhiaSIsPLmtCwtZTQaEg=; b=B4qP6lnRMhvZWHoSSH6a8El3hQFmK0UQtRg+AMSbVYnLpHHXx3lzd3TEtArkdR3+or ezWcZdCiltIAS1oe+6aWxWEJSG1Rs9DC98Nz694G69e+zv/MUs44wd6WjIrczMJcXW60 0BPRo/9n9q62NWbWo/i6HYl8vCkg/dQrEgGgfMdetzSpaE8CEE08PPcbju7KheOXD/4j d4uLDbHx1gdzU5ME1+xGAP+Ey9CL8NrTUssPho9uVFaZFXwYmtQr0+xZxikdFbDX6+D/ bbwVSFKgR4J+t1TIjjGke3eSV5aa6zzL+d7Ax+vLFaMaXVQbJsuH3mwV3vAOEjIm3dE+ Yq+A== X-Gm-Message-State: AOAM532BTFEKRbAqev5a0mTUbUNLFas1Nm2lLyyjEFjkrEqt1oqUZax6 QnUNkG9YI+GQDUK5pzLikOs= X-Google-Smtp-Source: ABdhPJztbYfkU+KttE8wtn650VxT8mj072Kn+nowKQeuvZyet6ZgN7WG6P6mqBVmuYIMdgqBVnIfCw== X-Received: by 2002:a05:6a00:2cf:: with SMTP id b15mr12530341pft.0.1643743492539; Tue, 01 Feb 2022 11:24:52 -0800 (PST) Received: from [10.230.2.88] ([192.19.161.250]) by smtp.gmail.com with ESMTPSA id mi18sm3531329pjb.35.2022.02.01.11.24.50 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 01 Feb 2022 11:24:51 -0800 (PST) Message-ID: <412a456d-7294-a124-8a01-f052915348b4@gmail.com> Date: Tue, 1 Feb 2022 11:24:49 -0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH] serial: 8250_bcm2835aux: Add ACPI support Content-Language: en-US To: Adrien Thierry , linux-serial@vger.kernel.org, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Cc: Jeremy Linton , Greg Kroah-Hartman , Jiri Slaby , Nicolas Saenz Julienne , Ray Jui , Scott Branden , bcm-kernel-feedback-list@broadcom.com References: <20220201185001.926338-1-athierry@redhat.com> From: Florian Fainelli In-Reply-To: <20220201185001.926338-1-athierry@redhat.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220201_112454_582244_73D2CDF5 X-CRM114-Status: GOOD ( 31.57 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2/1/2022 10:50 AM, Adrien Thierry wrote: > Add ACPI support to 8250_bcm2835aux driver. This makes it possible to > use the miniuart on the Raspberry Pi with the tianocore/edk2 UEFI > firmware. > > Signed-off-by: Adrien Thierry > --- > drivers/tty/serial/8250/8250_bcm2835aux.c | 103 +++++++++++++++++----- > 1 file changed, 83 insertions(+), 20 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c b/drivers/tty/serial/8250/8250_bcm2835aux.c > index fd95860cd..b904b321e 100644 > --- a/drivers/tty/serial/8250/8250_bcm2835aux.c > +++ b/drivers/tty/serial/8250/8250_bcm2835aux.c > @@ -12,6 +12,7 @@ > * simultaneously to rs485. > */ > > +#include > #include > #include > #include > @@ -44,6 +45,10 @@ struct bcm2835aux_data { > u32 cntl; > }; > > +struct bcm2835_aux_serial_acpi_driver_data { > + resource_size_t offset; > +}; > + > static void bcm2835aux_rs485_start_tx(struct uart_8250_port *up) > { > if (!(up->port.rs485.flags & SER_RS485_RX_DURING_TX)) { > @@ -82,8 +87,12 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev) > { > struct uart_8250_port up = { }; > struct bcm2835aux_data *data; > + struct bcm2835_aux_serial_acpi_driver_data *acpi_data; > struct resource *res; > int ret; > + resource_size_t mapbase; > + resource_size_t mapsize; > + unsigned int uartclk; > > /* allocate the custom structure */ > data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); > @@ -108,10 +117,12 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, data); > > - /* get the clock - this also enables the HW */ > - data->clk = devm_clk_get(&pdev->dev, NULL); > - if (IS_ERR(data->clk)) > - return dev_err_probe(&pdev->dev, PTR_ERR(data->clk), "could not get clk\n"); > + if (dev_of_node(&pdev->dev)) { > + /* get the clock - this also enables the HW */ > + data->clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(data->clk)) > + return dev_err_probe(&pdev->dev, PTR_ERR(data->clk), "could not get clk\n"); > + } This does not seem necessary, if the clk is NULL when probed via ACPI, all of the clk_* APIs will deal with that gracefully. If you need not to treat -ENOENT as a hard error here, consider switching to devm_clk_get_optional(). Given that you look at the 'clock-frequency' property, you can still have some generic code, something like: if (IS_ERR(data->clk)) { ret = device_property_read_u32(&pdev->dev, "clock-frequency", &uartclk); if (ret) return dev_err_probe(&pdev->dev, ret, "could not get clk\n"); } > > /* get the interrupt */ > ret = platform_get_irq(pdev, 0); > @@ -125,20 +136,59 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev) > dev_err(&pdev->dev, "memory resource not found"); > return -EINVAL; > } > - up.port.mapbase = res->start; > - up.port.mapsize = resource_size(res); > - > - /* Check for a fixed line number */ > - ret = of_alias_get_id(pdev->dev.of_node, "serial"); > - if (ret >= 0) > - up.port.line = ret; > - > - /* enable the clock as a last step */ > - ret = clk_prepare_enable(data->clk); > - if (ret) { > - dev_err(&pdev->dev, "unable to enable uart clock - %d\n", > - ret); > - return ret; All of that path can be common, and you can just define an offset to apply to the resource at the top after you fetched the memory resource. The offset will be non-0 for ACPI and 0 for non-ACPI. That is, no need for the intermediate variables and conditional paths whether this is ACPI apply this offset, or not. > + > + mapbase = res->start; > + mapsize = resource_size(res); > + > + if (has_acpi_companion(&pdev->dev)) { > + const struct acpi_device_id *match; > + > + match = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev); > + if (!match) > + return -ENODEV; > + > + acpi_data = (struct bcm2835_aux_serial_acpi_driver_data *)match->driver_data; > + > + /* Some UEFI implementations (e.g. tianocore/edk2 for the Raspberry Pi) > + * describe the miniuart with a base address that encompasses the auxiliary > + * registers shared between the miniuart and spi. > + * > + * This is due to historical reasons, see discussion here : > + * https://edk2.groups.io/g/devel/topic/87501357#84349 > + * > + * We need to add the offset between the miniuart and auxiliary > + * registers to get the real miniuart base address. And ACPI on the Pi4 is so widely deployed that fixing the miniuart resources is not an option at all? This really really continues to contribute to my impression that ACPI on the Pi4 is a fad more than a real thing, sorry. -- Florian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel