From nobody Tue Dec 16 14:34:59 2025 Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B70DD180A80 for ; Thu, 30 May 2024 22:46:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717109183; cv=none; b=Zih/koXrsTKnJ6XuzzwpklajZtDkkC87n6rvd56tzQroU//0enLMns6vCWN8wSABUpj3Apu4fMTFX/vel9Ys9y5EGBLVOqpLRKH8LsZ4L6vtD1Ts3u79fU9hqFZcSAdzpyfPPIduBk7++IHz/2lg81vLKkth79waiuXzBrZpdfw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717109183; c=relaxed/simple; bh=WlTgC2HNp92PM3IzRTxNjM3/DoK9im+ZJtxtFVePiKc=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=ZOoF7xPxqFFH+EdbREcObIcWzENRwEcr2WH8WvmN1/cpcBgqQDLe33nDNZ30tgxUT5B/1nODYlMw9Qmzd9Wxo2+OJGS4REC5aE3bCY8yIqLNcF1INsd33BLo9N6Sk2J7RPtA2kNOD+/V/cfLZQDY6+Q/c20JscxkrqeuQuqP3po= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b=JCdoY4tn; arc=none smtp.client-ip=209.85.214.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="JCdoY4tn" Received: by mail-pl1-f178.google.com with SMTP id d9443c01a7336-1f3469382f2so10990755ad.0 for ; Thu, 30 May 2024 15:46:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1717109181; x=1717713981; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=hVpFUdiW0gzcPxqtfC6QLNluJtCBHWicwGU5cZiObSw=; b=JCdoY4tnGqJZjLn3taXF+EYCT8VvA1MiSxfiGbBmpH2XFxMJVvJtRUmHcN6RcDI7oE 0RkQa7FVInoDi3HjJNLmyPR0U/filKQ3VjMT2xgzqpyPTMUZk5Yxz0DTvsesyQ9VM5Bu nuE0oJX0L4MMkFTCmfju55IG+LDEbTAaTEDW4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717109181; x=1717713981; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=hVpFUdiW0gzcPxqtfC6QLNluJtCBHWicwGU5cZiObSw=; b=GTyGdWHFYmgSGZjCHeqxeZo5kT4cyXJwXxkoMs+2dY0TjR8Jv/GfEY8XXuiiwsj78+ J8LffhOWHJ8DkFKbQYpfIWSkq+s628QV/f80vN980IMQV4KMQCJmj4/cnmYHGnwgIpgU MuRsbpjOu0fGm0QdFajlixHCaUbp21FiIGewaOy6QFlyewxdXddwucywxTzcO67fMWup gtz0yPra8Xc/E8S3S0XCyZw9SkxxcZs45YEpOUdu3UYI9/PRJFm+6bm1F4Yp9HBjLABt /SeD3xfL/WNqGrcJlquEi5VR13guDpL2+4GJeOKZEewZvpQ+QmEOqt5NYrrEA5cKwkCZ DEkw== X-Forwarded-Encrypted: i=1; AJvYcCWrFnM06iRggeC5B0R2Cfvp3MbgQbcMk6gt8EuuKcUAOTVK/8mBueEdfWe4z4dEgKmUb/TnWhHsSN0OpAF/qX4c6az97x/1X7U+CrXk X-Gm-Message-State: AOJu0YwG8TSVPH4vYn6fBYF06AxJcF5ePiP+lw+936jvkkm/fetnL6H/ llFGRExtxsf2o26LrlBMr+fYzNys0vWxyIY4+5w7n2ysFGRqKmN0BruCJxfS/A== X-Google-Smtp-Source: AGHT+IGf9zImQ/nEpQVPKVJ40w4LoZIcowQSUQ0pEx+3OA9SDdDSLgvm0fteajfMt17dU+D0vVNWDg== X-Received: by 2002:a17:902:e5cf:b0:1f4:71f1:d5f with SMTP id d9443c01a7336-1f63707e8a4mr2856975ad.47.1717109180980; Thu, 30 May 2024 15:46:20 -0700 (PDT) Received: from dianders.sjc.corp.google.com ([2620:15c:9d:2:564b:72b6:4827:cf6a]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1f632410b20sm2955795ad.273.2024.05.30.15.46.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 May 2024 15:46:19 -0700 (PDT) From: Douglas Anderson To: Greg Kroah-Hartman , Jiri Slaby Cc: linux-arm-msm@vger.kernel.org, John Ogness , =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= , Tony Lindgren , Stephen Boyd , linux-serial@vger.kernel.org, Yicong Yang , Johan Hovold , linux-kernel@vger.kernel.org, Bjorn Andersson , Andy Shevchenko , Konrad Dybcio , Douglas Anderson Subject: [PATCH v2 1/7] soc: qcom: geni-se: Add GP_LENGTH/IRQ_EN_SET/IRQ_EN_CLEAR registers Date: Thu, 30 May 2024 15:45:53 -0700 Message-ID: <20240530154553.v2.1.Ife7ced506aef1be3158712aa3ff34a006b973559@changeid> X-Mailer: git-send-email 2.45.1.288.g0e0cd299f1-goog In-Reply-To: <20240530224603.730042-1-dianders@chromium.org> References: <20240530224603.730042-1-dianders@chromium.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" For UART devices the M_GP_LENGTH is the TX word count. For other devices this is the transaction word count. For UART devices the S_GP_LENGTH is the RX word count. The IRQ_EN set/clear registers allow you to set or clear bits in the IRQ_EN register without needing a read-modify-write. Signed-off-by: Douglas Anderson Acked-by: Bjorn Andersson --- Since these new definitions are used in the future UART patches the hope is that they could be acked by Qualcomm folks and then go through the same tree as the UART patches that need them. Changes in v2: - New include/linux/soc/qcom/geni-se.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni= -se.h index 0f038a1a0330..8d07c442029b 100644 --- a/include/linux/soc/qcom/geni-se.h +++ b/include/linux/soc/qcom/geni-se.h @@ -88,11 +88,15 @@ struct geni_se { #define SE_GENI_M_IRQ_STATUS 0x610 #define SE_GENI_M_IRQ_EN 0x614 #define SE_GENI_M_IRQ_CLEAR 0x618 +#define SE_GENI_M_IRQ_EN_SET 0x61c +#define SE_GENI_M_IRQ_EN_CLEAR 0x620 #define SE_GENI_S_CMD0 0x630 #define SE_GENI_S_CMD_CTRL_REG 0x634 #define SE_GENI_S_IRQ_STATUS 0x640 #define SE_GENI_S_IRQ_EN 0x644 #define SE_GENI_S_IRQ_CLEAR 0x648 +#define SE_GENI_S_IRQ_EN_SET 0x64c +#define SE_GENI_S_IRQ_EN_CLEAR 0x650 #define SE_GENI_TX_FIFOn 0x700 #define SE_GENI_RX_FIFOn 0x780 #define SE_GENI_TX_FIFO_STATUS 0x800 @@ -101,6 +105,8 @@ struct geni_se { #define SE_GENI_RX_WATERMARK_REG 0x810 #define SE_GENI_RX_RFR_WATERMARK_REG 0x814 #define SE_GENI_IOS 0x908 +#define SE_GENI_M_GP_LENGTH 0x910 +#define SE_GENI_S_GP_LENGTH 0x914 #define SE_DMA_TX_IRQ_STAT 0xc40 #define SE_DMA_TX_IRQ_CLR 0xc44 #define SE_DMA_TX_FSM_RST 0xc58 --=20 2.45.1.288.g0e0cd299f1-goog From nobody Tue Dec 16 14:34:59 2025 Received: from mail-pl1-f176.google.com (mail-pl1-f176.google.com [209.85.214.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2BBDE183A61 for ; Thu, 30 May 2024 22:46:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.176 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717109185; cv=none; b=g5VxkLv6GO/VL49HSVhSzzydebGfeLDD5+JW2fHyJ927dTAaZaKxtBP8DJuSF9ZkJ0rI78/WQMdoSPDj7C+KqodWRvRUAF5CFkRXlS3iG/bDhlusyMJ5f8VQaz4LNrW84FM6eF3euxPMSbVH4ySpUf7Bk80x5Rb+jnVbSeD9PRQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717109185; c=relaxed/simple; bh=JvGABUZ3Lt4FbHnmcWkDhqUbvee4GNMb1KzXw/oRPI4=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=FwCmp/0ryRWhix9UUgcjiA8lPl8/cZ4a0VYkAXLinVH6io55axdK5OfJgpZEKByZrOYcJ/haVWSNArhcTKxOiXq9/xJB7IZr2fRspuFLg8hRCDF85Sch58OxU0kFfgUc18c6SifMH+1GAiMAaxJkV22KYL83TqrwW8KIvK0Gww8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b=agkvG0CQ; arc=none smtp.client-ip=209.85.214.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="agkvG0CQ" Received: by mail-pl1-f176.google.com with SMTP id d9443c01a7336-1f4a52b9413so11998755ad.2 for ; Thu, 30 May 2024 15:46:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1717109183; x=1717713983; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=f20bdelRQYxHx7q0JQ7q316V8FZx4HSKVjsqLajTHlA=; b=agkvG0CQvH/xM6sHMoGrbd/ki/U26GOTJ3AKsCHfUdymFYIFDFDcDbZg6/fY84IbKY peT4pniORTdFMpl2weLehZYLI6uZzFlA3u8Zkk7Es82EIQWde4fZ94EzmgHbrtmPI/ir ofKn5ftqb4Chg6pIRaP0pnvedL0xdWlbTFfvQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717109183; x=1717713983; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=f20bdelRQYxHx7q0JQ7q316V8FZx4HSKVjsqLajTHlA=; b=Z9CSOlM+TpfWcL/l8xpHpRgwZXjf7aHac2LlHdcO4FWKMSTLQLix3I8m6QT4trxGR7 f9Hh+/YWvzZiXCRq9EKg4WTVyqB5tokhhTGMGiT9B81oYfxazZ5uo4zJ+iah93ZNogN8 8iv6m/mWTxKLpjM4j7eYAtmXf/3zunoTc7MXwTsZDcCZ2B/jH8AxmGYr7S56ZdmxMwl9 8XWVx4zVQ6g41M+FV9TGIgUE84g4MjtTrKrbyK41Z2bnJnMvPiiHVSUrUJht+elpmjuo fFzORlIT1aReV1Mlj5iSm9fBF7Kn0Kp0aHCSeJrT2UiiAdDu+iAS+koj2xQWeOWyf0es fzXw== X-Forwarded-Encrypted: i=1; AJvYcCUGuVTBa93Jcd/rRo2Nh91Hy2T7oaIvj5cG+Z7nrTmPigA5WK2aM7E/PMWGV8c/SDpFzchiCw+Kj7OS7KTUl1wTObHQ7gpH0E8rTHTQ X-Gm-Message-State: AOJu0YznYywB5iscIjLiDTa4frrUvozgI59nrJX95Rjy8JjNjVsvPbBv fnqe9/wpdO6kyRMWZStH/CSnXcfLEzvs8OwPYt0JsJWzcdIpFwRBQHhzcgVh8Q== X-Google-Smtp-Source: AGHT+IGfax+l05lNl2UskJFXZtLTKIlUrnNHoGYAkrAQP1LpkIfI7gvGX9NbqfAhSBtevCUZYN2+lg== X-Received: by 2002:a17:903:22c1:b0:1f6:1778:7f1b with SMTP id d9443c01a7336-1f636fd9d3dmr2188115ad.1.1717109183463; Thu, 30 May 2024 15:46:23 -0700 (PDT) Received: from dianders.sjc.corp.google.com ([2620:15c:9d:2:564b:72b6:4827:cf6a]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1f632410b20sm2955795ad.273.2024.05.30.15.46.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 May 2024 15:46:22 -0700 (PDT) From: Douglas Anderson To: Greg Kroah-Hartman , Jiri Slaby Cc: linux-arm-msm@vger.kernel.org, John Ogness , =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= , Tony Lindgren , Stephen Boyd , linux-serial@vger.kernel.org, Yicong Yang , Johan Hovold , linux-kernel@vger.kernel.org, Bjorn Andersson , Andy Shevchenko , Konrad Dybcio , Douglas Anderson , Thomas Gleixner , Vijaya Krishna Nivarthi Subject: [PATCH v2 2/7] serial: qcom-geni: Fix the timeout in qcom_geni_serial_poll_bit() Date: Thu, 30 May 2024 15:45:54 -0700 Message-ID: <20240530154553.v2.2.I3e1968bbeee67e28fd4e15509950805b6665484a@changeid> X-Mailer: git-send-email 2.45.1.288.g0e0cd299f1-goog In-Reply-To: <20240530224603.730042-1-dianders@chromium.org> References: <20240530224603.730042-1-dianders@chromium.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" The qcom_geni_serial_poll_bit() is supposed to be able to be used to poll a bit that's will become set when a TX transfer finishes. Because of this it tries to set its timeout based on how long the UART will take to shift out all of the queued bytes. There are two problems here: 1. There appears to be a hidden extra word on the firmware side which is the word that the firmware has already taken out of the FIFO and is currently shifting out. We need to account for this. 2. The timeout calculation was assuming that it would only need 8 bits on the wire to shift out 1 byte. This isn't true. Typically 10 bits are used (8 data bits, 1 start and 1 stop bit), but as much as 13 bits could be used (14 if we allowed 9 bits per byte, which we don't). The too-short timeout was seen causing problems in a future patch which more properly waited for bytes to transfer out of the UART before cancelling. Fixes: c4f528795d1a ("tty: serial: msm_geni_serial: Add serial driver suppo= rt for GENI based QUP") Signed-off-by: Douglas Anderson --- Changes in v2: - New drivers/tty/serial/qcom_geni_serial.c | 32 ++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qco= m_geni_serial.c index 2bd25afe0d92..32e025705f99 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -271,7 +271,8 @@ static bool qcom_geni_serial_poll_bit(struct uart_port = *uport, u32 reg; struct qcom_geni_serial_port *port; unsigned int baud; - unsigned int fifo_bits; + unsigned int max_queued_bytes; + unsigned int max_queued_bits; unsigned long timeout_us =3D 20000; struct qcom_geni_private_data *private_data =3D uport->private_data; =20 @@ -280,12 +281,37 @@ static bool qcom_geni_serial_poll_bit(struct uart_por= t *uport, baud =3D port->baud; if (!baud) baud =3D 115200; - fifo_bits =3D port->tx_fifo_depth * port->tx_fifo_width; + + /* + * Add 1 to tx_fifo_depth to account for the hidden register + * on the firmware side that can hold a word. + */ + max_queued_bytes =3D + DIV_ROUND_UP((port->tx_fifo_depth + 1) * port->tx_fifo_width, + BITS_PER_BYTE); + + /* + * The maximum number of bits per byte on the wire is 13 from: + * - 1 start bit + * - 8 data bits + * - 1 parity bit + * - 3 stop bits + * + * While we could try count the actual bits per byte based on + * the port configuration, this is a rough timeout anyway so + * using the max is fine. + */ + max_queued_bits =3D max_queued_bytes * 13; + /* * Total polling iterations based on FIFO worth of bytes to be * sent at current baud. Add a little fluff to the wait. + * + * NOTE: this assumes that flow control isn't used, but with + * flow control we could wait indefinitely and that wouldn't + * be OK. */ - timeout_us =3D ((fifo_bits * USEC_PER_SEC) / baud) + 500; + timeout_us =3D ((max_queued_bits * USEC_PER_SEC) / baud) + 500; } =20 /* --=20 2.45.1.288.g0e0cd299f1-goog From nobody Tue Dec 16 14:34:59 2025 Received: from mail-pl1-f179.google.com (mail-pl1-f179.google.com [209.85.214.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 82239180A80 for ; Thu, 30 May 2024 22:46:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.179 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717109187; cv=none; b=NdAUkKymr4gH7LIt/yr0boTzfNHl/I0HFoklrFzFMgaJrgPzS/vUnTKUc19ROtfHfpqNSWyR7/PabU4hm7ZtAudbcLNamOLnEuQrnnCJNxxMtbJe6BCByR76bK3G6iJadBBSypYVD5LSiO59xrV5lEFB46MqLYkX/2QCWHwGPKQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717109187; c=relaxed/simple; bh=OHNRIjCt/AbOcKb2xouTeXmLM+pYeA2+sCM+PeZUBHc=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=aJW+z0zJ1tfF5BCEhSCToXLt2XoujEFjmHoJJMWfaERPapdWAa6OLhZjeJktxPdf9AOmJrFsKhSfhaYyWHcoe2F1iuBTTvOa/jMYkJp+MySKdLzOrt4wbOtmHTEcw2cqL8OC5iYGluqzhvJqMpIzZujC4oSEkbqgp66czia44qw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b=kb3Lrb30; arc=none smtp.client-ip=209.85.214.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="kb3Lrb30" Received: by mail-pl1-f179.google.com with SMTP id d9443c01a7336-1f4a5344ec7so10870395ad.1 for ; Thu, 30 May 2024 15:46:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1717109186; x=1717713986; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=UNGi3BaacIMJpkAV5tddhgVoLNe/JGSac2Iso/bclHo=; b=kb3Lrb30Bzxzp4hqWqNBdk2HVDHGurJUy1K2epaQtYVdH3NSQLgbkcZ4rUmblVRjdp wQChU1otXpzX/uRYQ4yYrshdLOI2d6/Il9Gkdbge3kmVAHYtqwwWNcWaR3xTUSzmnSa8 +u7X/etJRzqyqVl1iWIJbm1ShV2NtofjvS0pM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717109186; x=1717713986; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=UNGi3BaacIMJpkAV5tddhgVoLNe/JGSac2Iso/bclHo=; b=rXmEv45sMNoJqVDIefqZx74iaNkSfbGx0fM9KmVCUf04Bozaw/sNdHYkQTJgqCd0wO sNLCaw/mCzN7aMk3geqcWJlddBFZhdJqGHV4NfF+jTnjR5UPx76rw3DohsJBcWszFnB6 qUOlss2NaT5GURak2addH6JZvpr+j5KPY4Rnfrs3Kj51a7BrNsD0V1eHBIKGAz37LXo2 6V+G7nWtt09EgOKhA3DAyhhDXEBaDzFMnAqIwk5CsO0ft1BpgQSu/rN8nQr4gIGkmG5Q JR4z0DUo28c4NCcwmtkSsL0MBDeSfhQTFVfDfmDxwEbF/CAJT6O/dFPJRgT4uZqouJ4D L+PA== X-Forwarded-Encrypted: i=1; AJvYcCWfSGDNni3ruIDRUytgbVx6JQhTQ+GsavdI7xsyDz5YmaDdiI0IDejtePSD+jXMHRYuT1wtVKlG0nW2ugywuvhp2cjpcV6qNwEV7a3q X-Gm-Message-State: AOJu0YxxI4aQIi53g7gBS4jjb9TW7Rq4WuoHeB8Y9doaH0Ggxi5lTq5l 6PDPcXbITx6In6VMdFcNsHJEOhBSmc8+l86eAmDalG77VD5fhXF+fXPE+43PUg== X-Google-Smtp-Source: AGHT+IGPRBGVd/Uds8lu/CwCOmUT5FA6Rwai1ahagYPAZWNotAwpSu3qyKimjG2+vC0ln3VA0ZBt+w== X-Received: by 2002:a17:902:c401:b0:1f0:8cbf:c1b5 with SMTP id d9443c01a7336-1f6359c9c78mr3578145ad.16.1717109185878; Thu, 30 May 2024 15:46:25 -0700 (PDT) Received: from dianders.sjc.corp.google.com ([2620:15c:9d:2:564b:72b6:4827:cf6a]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1f632410b20sm2955795ad.273.2024.05.30.15.46.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 May 2024 15:46:24 -0700 (PDT) From: Douglas Anderson To: Greg Kroah-Hartman , Jiri Slaby Cc: linux-arm-msm@vger.kernel.org, John Ogness , =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= , Tony Lindgren , Stephen Boyd , linux-serial@vger.kernel.org, Yicong Yang , Johan Hovold , linux-kernel@vger.kernel.org, Bjorn Andersson , Andy Shevchenko , Konrad Dybcio , Douglas Anderson , Rob Herring Subject: [PATCH v2 3/7] serial: qcom-geni: Fix arg types for qcom_geni_serial_poll_bit() Date: Thu, 30 May 2024 15:45:55 -0700 Message-ID: <20240530154553.v2.3.I24a0de52dd7336908df180fa6b698e001f3aff82@changeid> X-Mailer: git-send-email 2.45.1.288.g0e0cd299f1-goog In-Reply-To: <20240530224603.730042-1-dianders@chromium.org> References: <20240530224603.730042-1-dianders@chromium.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" The "offset" passed in should be unsigned since it's always a positive offset from our memory mapped IO. The "field" should be u32 since we're anding it with a 32-bit value read from the device. Suggested-by: Stephen Boyd Signed-off-by: Douglas Anderson --- Changes in v2: - New drivers/tty/serial/qcom_geni_serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qco= m_geni_serial.c index 32e025705f99..71258eefa654 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -266,7 +266,7 @@ static bool qcom_geni_serial_secondary_active(struct ua= rt_port *uport) } =20 static bool qcom_geni_serial_poll_bit(struct uart_port *uport, - int offset, int field, bool set) + unsigned int offset, u32 field, bool set) { u32 reg; struct qcom_geni_serial_port *port; --=20 2.45.1.288.g0e0cd299f1-goog From nobody Tue Dec 16 14:34:59 2025 Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DB4B01862B8 for ; Thu, 30 May 2024 22:46:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.173 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717109190; cv=none; b=NYTtq0tcBkx1Pyj9YCrjmzIjBIomuC/Hi4KyNootTyhPbInKc0AuhHJ3K6rqtfQWMpIZc/0VbYoAZRsIsQYWZ4N/4xg2eggXaLV36dooGPc0urZwb/YraaRNNObpNm0RB53OWhKMzI3hWPyc5s4I4O9jHwg5Z/T3myJd554kg4U= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717109190; c=relaxed/simple; bh=cQNJ3XMzHwi5FaJGdzzeq5t/gSbidgfxwp2JxpD4I2Y=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=GxC+iT6qVk1mCmb1O3TJ9TtuSaMQEartCZ17TJP08IHy3KzpnhvZfe4zy1z2Xpq7CMghDB9ZslluyXhTXQueOadZmdaKZpVIslbDc2QDgZ2KALIateUXKxeHWs60ZAWcmgSUdVgzRV9+oBIhn5mtKjQEaR7rIC9BmfcnBNfEH98= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b=iFIMusMJ; arc=none smtp.client-ip=209.85.214.173 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="iFIMusMJ" Received: by mail-pl1-f173.google.com with SMTP id d9443c01a7336-1f47f07aceaso3876175ad.0 for ; Thu, 30 May 2024 15:46:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1717109188; x=1717713988; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=vGiOTL7ihlI5Yhs010iFkTmajDvoSp2W9SJUSNub5/M=; b=iFIMusMJDubVZ/kcABU8xhG0Ad/x5rACFtKQ+tQNrW/26dZpeEaREYmht8dyfH9lqB 0KNmgDAB37VbA3//YyU6RRbR65eLMbpg8JUhIiDNlCstF4u6OZzbQbgpZBEI1PBhFOFH EzDgx4PAFmxHB7W98te9SXmXLB3sgX2hz+aLk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717109188; x=1717713988; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=vGiOTL7ihlI5Yhs010iFkTmajDvoSp2W9SJUSNub5/M=; b=YnulS+yfPRA7e8ilM7W3Q6PqWt1441G6Ls2w8oQqLvsiAAR+kMj+TM5fNIezqRuY2c f5pnyoZhNnM6ZcqzR+8BJrYEUHeGe9+E+12UspxlUcIs8lIknbUWTCWBeDLJwhECUly2 iX1fyIneRY6h3jcUmxU9BArlJ8dKR0AYXN4vepe5Q7SG1AR3qHKW1O8efbOZzAzBkqqP ELIzUfjCMHHrJ74qEtIDb6o8cooGHMAnwlR49vLFvtsiuD8x/izey2tFq4+EW+T1jJjE 1quLkMjfPw/WpjPtI6x0rWmryL7Y2h2ESRPTH6f9BL7HTzFJZilyRUadIWdj1CW0wPLv Y1Fg== X-Forwarded-Encrypted: i=1; AJvYcCW9JYULgyzw0raAv4xib6vVsNBO/th2AIgfwcBDiSod++mj2dA6f/BsRh+MXFwdJhK/+le7cXfzgldVbOMdL+KWWLi7qO5V3v1IAmkW X-Gm-Message-State: AOJu0YxD7sKo0HMNMwkj27ib5AgkQp1yL296uTOvpaVROkS4ytRptOjK GgzlAzMORugIPm0WwUM8tRjUOl/KykXuKfDXzJv2Fr99kM0TzSj+rDHVMW8yPg== X-Google-Smtp-Source: AGHT+IGuNQy9vE7scNu6gP4OcjlhzBXrYXmV0BA270Yt9wNFVO3IPAIRHxMOBYrMyEWGHmskZxQBhg== X-Received: by 2002:a17:903:32c2:b0:1f3:b55:e247 with SMTP id d9443c01a7336-1f6370a783bmr1589435ad.55.1717109188230; Thu, 30 May 2024 15:46:28 -0700 (PDT) Received: from dianders.sjc.corp.google.com ([2620:15c:9d:2:564b:72b6:4827:cf6a]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1f632410b20sm2955795ad.273.2024.05.30.15.46.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 May 2024 15:46:27 -0700 (PDT) From: Douglas Anderson To: Greg Kroah-Hartman , Jiri Slaby Cc: linux-arm-msm@vger.kernel.org, John Ogness , =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= , Tony Lindgren , Stephen Boyd , linux-serial@vger.kernel.org, Yicong Yang , Johan Hovold , linux-kernel@vger.kernel.org, Bjorn Andersson , Andy Shevchenko , Konrad Dybcio , Douglas Anderson , Thomas Gleixner , Vijaya Krishna Nivarthi Subject: [PATCH v2 4/7] serial: qcom-geni: Introduce qcom_geni_serial_poll_bitfield() Date: Thu, 30 May 2024 15:45:56 -0700 Message-ID: <20240530154553.v2.4.Ic6411eab8d9d37acc451705f583fb535cd6dadb2@changeid> X-Mailer: git-send-email 2.45.1.288.g0e0cd299f1-goog In-Reply-To: <20240530224603.730042-1-dianders@chromium.org> References: <20240530224603.730042-1-dianders@chromium.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" With a small modification the qcom_geni_serial_poll_bit() function could be used to poll more than just a single bit. Let's generalize it. We'll make the qcom_geni_serial_poll_bit() into just a wrapper of the general function. Signed-off-by: Douglas Anderson --- The new function isn't used yet (except by the wrapper) but will be used in a future change. Changes in v2: - New drivers/tty/serial/qcom_geni_serial.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qco= m_geni_serial.c index 71258eefa654..539a6ac85511 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -265,8 +265,8 @@ static bool qcom_geni_serial_secondary_active(struct ua= rt_port *uport) return readl(uport->membase + SE_GENI_STATUS) & S_GENI_CMD_ACTIVE; } =20 -static bool qcom_geni_serial_poll_bit(struct uart_port *uport, - unsigned int offset, u32 field, bool set) +static bool qcom_geni_serial_poll_bitfield(struct uart_port *uport, + unsigned int offset, u32 field, u32 val) { u32 reg; struct qcom_geni_serial_port *port; @@ -321,7 +321,7 @@ static bool qcom_geni_serial_poll_bit(struct uart_port = *uport, timeout_us =3D DIV_ROUND_UP(timeout_us, 10) * 10; while (timeout_us) { reg =3D readl(uport->membase + offset); - if ((bool)(reg & field) =3D=3D set) + if ((reg & field) =3D=3D val) return true; udelay(10); timeout_us -=3D 10; @@ -329,6 +329,12 @@ static bool qcom_geni_serial_poll_bit(struct uart_port= *uport, return false; } =20 +static bool qcom_geni_serial_poll_bit(struct uart_port *uport, + unsigned int offset, u32 field, bool set) +{ + return qcom_geni_serial_poll_bitfield(uport, offset, field, set ? field := 0); +} + static void qcom_geni_serial_setup_tx(struct uart_port *uport, u32 xmit_si= ze) { u32 m_cmd; --=20 2.45.1.288.g0e0cd299f1-goog From nobody Tue Dec 16 14:34:59 2025 Received: from mail-pl1-f175.google.com (mail-pl1-f175.google.com [209.85.214.175]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8D94D1836F0 for ; Thu, 30 May 2024 22:46:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.175 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717109192; cv=none; b=p3lya6VdUhPPL+xQAWdZ2p16YRKZApV47i173FMFSAvz4r20cB8S2fEM2Gqcj6j52sy4BAZ/ilDUrcDWfXyQOgjWYW2A0sc78JHsqE9+EFunf01pysvXcjbpStH8K+ugJchs5MQ6qEwHKnh2DKdzvqOBJ644/yrl3ttTVGVvwwU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717109192; c=relaxed/simple; bh=78gYEUyzyaFfiXbdVxUAmTlMq0HG762eI0EiZRG8OIA=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=VAi99D1Qylx97eMmqd7agFin3UmyqUB/s547N+g+fzjuMALFTsdxxVm+TevfJklfcOSspVI6udLKUqep1E1OsWUHfYuK0+zrU/rh8GUIhDr9QhpCyO4GjnCexxEu7lhtOCyVUejS1YLGykZpIuNE6FTI0COVIdrXylaVp1oeIYs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b=FcoSSNVy; arc=none smtp.client-ip=209.85.214.175 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="FcoSSNVy" Received: by mail-pl1-f175.google.com with SMTP id d9443c01a7336-1f48e9414e9so12801025ad.0 for ; Thu, 30 May 2024 15:46:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1717109190; x=1717713990; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=vOWbb9qbkvOEUGdLBBAENqDT3ts5Zzk0WB4O6/YZK90=; b=FcoSSNVyr/CNq5JAJp0AcMlLHPUxBJsp538WTxDjHyo/z9WnYDhbhua40P5xFyE86Z fRCv58heDyuuMrjnyAa4uo6Hf3vAfL3nsEnbJdEHGgVrebjGd/FOTPsy68idyvgIG56F Jc1Q9ObCFAkC7dy2AcXk48YZIEtEBytYQOtVU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717109190; x=1717713990; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=vOWbb9qbkvOEUGdLBBAENqDT3ts5Zzk0WB4O6/YZK90=; b=o+A6WHXZLP9nurY8Pc4p3I33GdwSW6PEYQOo92pSJNsiYju2Xqj4z5y4AUvz8iDwyv VpHs87l3Djs+VieHGScTvwBeZXe8EN9h7vzrb1RhzOEt0ShiQBh7KdhwlBpX2Mcxy23y CY+0KPNLAmdVs6ZkQEDsMf2LEUg+wMFvnKGAFPvMik4hs4OFfmmj3KY4nD6ewtkedHnZ c72yUjQKEWwzh5iRnVa9BBv6vj4wPB7GGTkF+M7sBPnjHjP5ZDbK8Tu6PXQC2XZ9nkw+ V39wV9nhSQkCCO/hZ2tLOTwfDmhwhYNXWruIbJhM0W1nq1vmAMxyaRGTJ6ZV8KOWmv0i cFMA== X-Forwarded-Encrypted: i=1; AJvYcCXt38qIG0igQtC/qGyPBqshohVORc+5zVDIanTkjUuIpxTNNo5ID+KPPCKgp7q1m8SV9pZolNy8i2FiRvh4v5ukJbwqW+JriueFHq8M X-Gm-Message-State: AOJu0YzncAA3aKu7IVfw8hl+zPV0pNFKMtPuD7s9YLUgQ1D5W5Fgci/g HZz5w0yqEK6thWTDAKhvp3+PozIp9aSrvbwIX9lSiykv+8cQefvVVkn9EeJBsQ== X-Google-Smtp-Source: AGHT+IFKS0W6dOoGfNolcljOTmURsU8XJDFLlm8UaKsYnhx17kA5MKtjML0iYvJa38VzjuZeAIT9vA== X-Received: by 2002:a17:902:e544:b0:1f4:5dea:f3a1 with SMTP id d9443c01a7336-1f635a99418mr3524095ad.34.1717109190113; Thu, 30 May 2024 15:46:30 -0700 (PDT) Received: from dianders.sjc.corp.google.com ([2620:15c:9d:2:564b:72b6:4827:cf6a]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1f632410b20sm2955795ad.273.2024.05.30.15.46.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 May 2024 15:46:29 -0700 (PDT) From: Douglas Anderson To: Greg Kroah-Hartman , Jiri Slaby Cc: linux-arm-msm@vger.kernel.org, John Ogness , =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= , Tony Lindgren , Stephen Boyd , linux-serial@vger.kernel.org, Yicong Yang , Johan Hovold , linux-kernel@vger.kernel.org, Bjorn Andersson , Andy Shevchenko , Konrad Dybcio , Douglas Anderson , Thomas Gleixner Subject: [PATCH v2 5/7] serial: qcom-geni: Just set the watermark level once Date: Thu, 30 May 2024 15:45:57 -0700 Message-ID: <20240530154553.v2.5.Ie02dcdf46089457026de3bd665c08f0210711966@changeid> X-Mailer: git-send-email 2.45.1.288.g0e0cd299f1-goog In-Reply-To: <20240530224603.730042-1-dianders@chromium.org> References: <20240530224603.730042-1-dianders@chromium.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" There's no reason to set the TX watermark level to 0 when we disable TX since we're disabling the interrupt anyway. Just set the watermark level once at init time and leave it alone. Signed-off-by: Douglas Anderson --- Changes in v2: - New drivers/tty/serial/qcom_geni_serial.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qco= m_geni_serial.c index 539a6ac85511..d7814f9e5c26 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -418,7 +418,6 @@ static int qcom_geni_serial_get_char(struct uart_port *= uport) static void qcom_geni_serial_poll_put_char(struct uart_port *uport, unsigned char c) { - writel(DEF_TX_WM, uport->membase + SE_GENI_TX_WATERMARK_REG); qcom_geni_serial_setup_tx(uport, 1); WARN_ON(!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS, M_TX_FIFO_WATERMARK_EN, true)); @@ -462,7 +461,6 @@ __qcom_geni_serial_console_write(struct uart_port *upor= t, const char *s, bytes_to_send++; } =20 - writel(DEF_TX_WM, uport->membase + SE_GENI_TX_WATERMARK_REG); qcom_geni_serial_setup_tx(uport, bytes_to_send); for (i =3D 0; i < count; ) { size_t chars_to_write =3D 0; @@ -690,7 +688,6 @@ static void qcom_geni_serial_start_tx_fifo(struct uart_= port *uport) irq_en =3D readl(uport->membase + SE_GENI_M_IRQ_EN); irq_en |=3D M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN; =20 - writel(DEF_TX_WM, uport->membase + SE_GENI_TX_WATERMARK_REG); writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN); } =20 @@ -701,7 +698,6 @@ static void qcom_geni_serial_stop_tx_fifo(struct uart_p= ort *uport) =20 irq_en =3D readl(uport->membase + SE_GENI_M_IRQ_EN); irq_en &=3D ~(M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN); - writel(0, uport->membase + SE_GENI_TX_WATERMARK_REG); writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN); /* Possible stop tx is called multiple times. */ if (!qcom_geni_serial_main_active(uport)) @@ -1153,6 +1149,7 @@ static int qcom_geni_serial_port_setup(struct uart_po= rt *uport) false, true, true); geni_se_init(&port->se, UART_RX_WM, port->rx_fifo_depth - 2); geni_se_select_mode(&port->se, port->dev_data->mode); + writel(DEF_TX_WM, uport->membase + SE_GENI_TX_WATERMARK_REG); qcom_geni_serial_start_rx(uport); port->setup =3D true; =20 --=20 2.45.1.288.g0e0cd299f1-goog From nobody Tue Dec 16 14:34:59 2025 Received: from mail-pl1-f175.google.com (mail-pl1-f175.google.com [209.85.214.175]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8C16F19067C for ; Thu, 30 May 2024 22:46:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.175 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717109195; cv=none; b=WPQVfJTHfySh5h4n+L5nQJcY5q0o2KMydXcK97toPAsC2ooQKLbQ7TS1klvgNjXkJz31HzaC/SYaY3/WoAa/i/TL49BOtE/XpOaWxzcMlshP1P5Vtr5kLFcBXWfKcXT/VmkkRDmHWXCrQ6Gm0iUfHQGWRBCC/AG3P+8TOI0A1wc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717109195; c=relaxed/simple; bh=b6j3qep3/dVPBYOEA9OobtieM0cWDCA0y3UiGOZqxLI=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=UdxLz9BZ7wb6DJKjivksn98Lm2gWLaVKASkLrdpTVyAmeXgIqpYVcdr+tQqL1gu+RpCDSnc5drPK8wbMJuhQiLI1LhvohFWy/TnsnOO/X2yAfjigog4vu8aHWNRzOBH667yoyTznY1Bkhesl8AS+qlQRdF7WkNYiqvAXAmcvWII= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b=EsaDtyaO; arc=none smtp.client-ip=209.85.214.175 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="EsaDtyaO" Received: by mail-pl1-f175.google.com with SMTP id d9443c01a7336-1f3469382f2so10991515ad.0 for ; Thu, 30 May 2024 15:46:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1717109193; x=1717713993; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=mRt2EVlSGWfvgcEGvpTae3Wd9PPOHk66+wob5fd4cjk=; b=EsaDtyaOr6OA6ew2C9XkG8QpxMOvKG1hqu1ObiqSk5KT9f+vDWoMH5CkMrN+4RFvuq RJrIgn0veAQS44WdcUuSF0xXz2O0Jpp5jJnZIyVM2b+lYk54mf9YU2T44glRxXsUm04P oLXjiL0QZp22kaxCGrc8Pa6MV2khzRoikgIyg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717109193; x=1717713993; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=mRt2EVlSGWfvgcEGvpTae3Wd9PPOHk66+wob5fd4cjk=; b=MkfS21BF+L4zJ4AJLvZ+TmO8u77NIh5IffDRsL1SQ1wvI3NavCfTZ28hzm81t5tTnA ora1hqNuidTNaevQFwNooqbd4djAA7C4MpdDIhnuoxTaBZYY5H3au5X6rlvwfhXujTvC JePAGzRv+GXMpPcQeQiQiby7B3bFeKKlT65jFE3Gj5jHuOK+dnWkcsL5oyvn69JIrHNH MnngFivpIEJ2kUenzjbaIOj2IkCDzub7kv4Z8Ui4FJVjCgWV3wGyZZPXFiZUMKBoabuH uS/P0WocFCt07kqmzM08+C20Li8EKg6Sw3EUwqyMs/buE92JW5taLBOZZkML1A0zzxuJ etPQ== X-Forwarded-Encrypted: i=1; AJvYcCWJwzjF8SrGCrMKcHKrhqPGW66QJVmY1OObaWceoskObz6ezuHPehRtpaU7InxXcZF0Gmz7wzjlBtiQgeOEZ3+WuSVYD+xroUZjb+oZ X-Gm-Message-State: AOJu0YzOO/H2KLPc5Y0Ft0jgAMLBxH1QXunxVY4qQYggwuewHmhlFajn JemkNfmGyhUHxkjdKQ3lTqZcSdx+98oEOfTC7PpvDbt2niq4fTDMxgi4JJKzig== X-Google-Smtp-Source: AGHT+IFLS38Awc/uP8zdYxlKSCJW6ZygB1Oo+TXQ+lWWKB6xxCk7S6Eujk3HAmkCnnByluAGVdPUxQ== X-Received: by 2002:a17:902:f68b:b0:1e8:5cf9:37fc with SMTP id d9443c01a7336-1f63701f236mr3384745ad.35.1717109192829; Thu, 30 May 2024 15:46:32 -0700 (PDT) Received: from dianders.sjc.corp.google.com ([2620:15c:9d:2:564b:72b6:4827:cf6a]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1f632410b20sm2955795ad.273.2024.05.30.15.46.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 May 2024 15:46:31 -0700 (PDT) From: Douglas Anderson To: Greg Kroah-Hartman , Jiri Slaby Cc: linux-arm-msm@vger.kernel.org, John Ogness , =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= , Tony Lindgren , Stephen Boyd , linux-serial@vger.kernel.org, Yicong Yang , Johan Hovold , linux-kernel@vger.kernel.org, Bjorn Andersson , Andy Shevchenko , Konrad Dybcio , Douglas Anderson , Vijaya Krishna Nivarthi Subject: [PATCH v2 6/7] serial: qcom-geni: Fix suspend while active UART xfer Date: Thu, 30 May 2024 15:45:58 -0700 Message-ID: <20240530154553.v2.6.I0f81a5baa37d368f291c96ee4830abca337e3c87@changeid> X-Mailer: git-send-email 2.45.1.288.g0e0cd299f1-goog In-Reply-To: <20240530224603.730042-1-dianders@chromium.org> References: <20240530224603.730042-1-dianders@chromium.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" On devices using Qualcomm's GENI UART it is possible to get the UART stuck such that it no longer outputs data. Specifically, I could reproduce this problem by logging in via an agetty on the debug serial port (which was _not_ used for kernel console) and running: cat /var/log/messages ...and then (via an SSH session) forcing a few suspend/resume cycles. Digging into this showed a number of problems that are all related. The root of the problems was with qcom_geni_serial_stop_tx_fifo() which is called as part of the suspend process. Specific problems with that function: - When we cancel an in-progress "tx" command it doesn't appear to fully drain the FIFO. That meant qcom_geni_serial_tx_empty() continued to report that the FIFO wasn't empty. The qcom_geni_serial_start_tx_fifo() function didn't re-enable interrupts in this case so we'd never start transferring again. - We cancelled the current "tx" command but we forgot to zero out "tx_remaining". This confused logic elsewhere in the driver - From experimentation, it appears that cancelling the "tx" command could drop some of the queued up bytes. While maybe not the end of the world, it doesn't seem like we should be dropping bytes when stopping the FIFO, which is defined more of a "pause". One idea to fix the above would be to add FIFO draining to qcom_geni_serial_stop_tx_fifo(). However, digging into the documentation in serial_core.h for stop_tx() makes this seem like the wrong choice. Specifically stop_tx() is called with local interrupts disabled. Waiting for a FIFO (which might be 64 bytes big) to drain at 115.2 kbps doesn't seem like a wise move. Ideally qcom_geni_serial_stop_tx_fifo() would be able to pause the transmitter, but nothing in the documentation for the GENI UART makes me believe that is possible. Given the lack of better choices, we'll change qcom_geni_serial_stop_tx_fifo() to simply disable the TX_FIFO_WATERMARK interrupt and call it a day. This seems OK as per the serial core docs since stop_tx() is supposed to stop transferring bytes "as soon as possible" and there doesn't seem to be any possible way to stop transferring sooner. As part of this, get rid of some of the extra conditions on qcom_geni_serial_start_tx_fifo() which simply weren't needed and are now getting in the way. It's always fine to turn the interrupts on if we want to receive and it'll be up to the IRQ handler to turn them back off if somehow they're not needed. This works fine. Unfortunately, doing just the above change causes new/different problems with suspend/resume. Now if you suspend while an active transfer is happening you can find that after resume time you're no longer receiving UART interrupts at all. It appears to be important to drain the FIFO and send a "cancel" command if the UART is active to avoid this. Since we've already decided that qcom_geni_serial_stop_tx_fifo() shouldn't be doing this, let's add the draining / cancelling logic to the shutdown() call where it should be OK to delay a bit. This is called as part of the suspend process via uart_suspend_port(). Finally, with all of the above, the test case where we're spamming the UART with data and going through suspend/resume cycles doesn't kill the UART and doesn't drop bytes. NOTE: though I haven't gone back and validated on ancient code, it appears from code inspection that many of these problems have existed since the start of the driver. In the very least, I could reproduce the problems on vanilla v5.15. The problems don't seem to reproduce when using the serial port for kernel console output and also don't seem to reproduce if nothing is being printed to the console at suspend time, so this is presumably why they were not noticed until now. Fixes: c4f528795d1a ("tty: serial: msm_geni_serial: Add serial driver suppo= rt for GENI based QUP") Signed-off-by: Douglas Anderson --- There are still a number of problems with GENI UART after this but I've kept this change separate to make it easier to understand. Specifically on mainline just hitting "Ctrl-C" after dumping /var/log/messages to the serial port hangs things after the kfifo changes. Those issues will be addressed in future patches. Changes in v2: - Totally rework / rename patch to handle suspend while active xfer drivers/tty/serial/qcom_geni_serial.c | 97 +++++++++++++++++++++------ 1 file changed, 75 insertions(+), 22 deletions(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qco= m_geni_serial.c index d7814f9e5c26..10aeb0313f9b 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -131,6 +131,7 @@ struct qcom_geni_serial_port { bool brk; =20 unsigned int tx_remaining; + unsigned int tx_total; int wakeup_irq; bool rx_tx_swap; bool cts_rts_swap; @@ -337,11 +338,14 @@ static bool qcom_geni_serial_poll_bit(struct uart_por= t *uport, =20 static void qcom_geni_serial_setup_tx(struct uart_port *uport, u32 xmit_si= ze) { + struct qcom_geni_serial_port *port =3D to_dev_port(uport); u32 m_cmd; =20 writel(xmit_size, uport->membase + SE_UART_TX_TRANS_LEN); m_cmd =3D UART_START_TX << M_OPCODE_SHFT; writel(m_cmd, uport->membase + SE_GENI_M_CMD0); + + port->tx_total =3D xmit_size; } =20 static void qcom_geni_serial_poll_tx_done(struct uart_port *uport) @@ -361,6 +365,64 @@ static void qcom_geni_serial_poll_tx_done(struct uart_= port *uport) writel(irq_clear, uport->membase + SE_GENI_M_IRQ_CLEAR); } =20 +static void qcom_geni_serial_drain_tx_fifo(struct uart_port *uport) +{ + struct qcom_geni_serial_port *port =3D to_dev_port(uport); + + /* + * If the main sequencer is inactive it means that the TX command has + * been completed and all bytes have been sent. Nothing to do in that + * case. + */ + if (!qcom_geni_serial_main_active(uport)) + return; + + /* + * Wait until the FIFO has been drained. We've already taken bytes out + * of the higher level queue in qcom_geni_serial_send_chunk_fifo() so + * if we don't drain the FIFO but send the "cancel" below they seem to + * get lost. + */ + qcom_geni_serial_poll_bitfield(uport, SE_GENI_M_GP_LENGTH, 0xffffffff, + port->tx_total - port->tx_remaining); + + /* + * If clearing the FIFO made us inactive then we're done--no need for + * a cancel. + */ + if (!qcom_geni_serial_main_active(uport)) + return; + + /* + * Cancel the current command. After this the main sequencer will + * stop reporting that it's active and we'll have to start a new + * transfer command. + * + * If we skip doing this cancel and then continue with a system + * suspend while there's an active command in the main sequencer + * then after resume time we won't get any more interrupts on the + * main sequencer until we send the cancel. + */ + geni_se_cancel_m_cmd(&port->se); + if (!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS, + M_CMD_CANCEL_EN, true)) { + /* The cancel failed; try an abort as a fallback. */ + geni_se_abort_m_cmd(&port->se); + qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS, + M_CMD_ABORT_EN, true); + writel(M_CMD_ABORT_EN, uport->membase + SE_GENI_M_IRQ_CLEAR); + } + writel(M_CMD_CANCEL_EN, uport->membase + SE_GENI_M_IRQ_CLEAR); + + /* + * We've cancelled the current command. "tx_remaining" stores how + * many bytes are left to finish in the current command so we know + * when to start a new command. Since the command was cancelled we + * need to zero "tx_remaining". + */ + port->tx_remaining =3D 0; +} + static void qcom_geni_serial_abort_rx(struct uart_port *uport) { u32 irq_clear =3D S_CMD_DONE_EN | S_CMD_ABORT_EN; @@ -681,37 +743,18 @@ static void qcom_geni_serial_start_tx_fifo(struct uar= t_port *uport) { u32 irq_en; =20 - if (qcom_geni_serial_main_active(uport) || - !qcom_geni_serial_tx_empty(uport)) - return; - irq_en =3D readl(uport->membase + SE_GENI_M_IRQ_EN); irq_en |=3D M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN; - writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN); } =20 static void qcom_geni_serial_stop_tx_fifo(struct uart_port *uport) { u32 irq_en; - struct qcom_geni_serial_port *port =3D to_dev_port(uport); =20 irq_en =3D readl(uport->membase + SE_GENI_M_IRQ_EN); irq_en &=3D ~(M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN); writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN); - /* Possible stop tx is called multiple times. */ - if (!qcom_geni_serial_main_active(uport)) - return; - - geni_se_cancel_m_cmd(&port->se); - if (!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS, - M_CMD_CANCEL_EN, true)) { - geni_se_abort_m_cmd(&port->se); - qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS, - M_CMD_ABORT_EN, true); - writel(M_CMD_ABORT_EN, uport->membase + SE_GENI_M_IRQ_CLEAR); - } - writel(M_CMD_CANCEL_EN, uport->membase + SE_GENI_M_IRQ_CLEAR); } =20 static void qcom_geni_serial_handle_rx_fifo(struct uart_port *uport, bool = drop) @@ -1093,7 +1136,15 @@ static int setup_fifos(struct qcom_geni_serial_port = *port) } =20 =20 -static void qcom_geni_serial_shutdown(struct uart_port *uport) +static void qcom_geni_serial_shutdown_dma(struct uart_port *uport) +{ + disable_irq(uport->irq); + + qcom_geni_serial_stop_tx(uport); + qcom_geni_serial_stop_rx(uport); +} + +static void qcom_geni_serial_shutdown_fifo(struct uart_port *uport) { disable_irq(uport->irq); =20 @@ -1102,6 +1153,8 @@ static void qcom_geni_serial_shutdown(struct uart_por= t *uport) =20 qcom_geni_serial_stop_tx(uport); qcom_geni_serial_stop_rx(uport); + + qcom_geni_serial_drain_tx_fifo(uport); } =20 static int qcom_geni_serial_port_setup(struct uart_port *uport) @@ -1560,7 +1613,7 @@ static const struct uart_ops qcom_geni_console_pops = =3D { .startup =3D qcom_geni_serial_startup, .request_port =3D qcom_geni_serial_request_port, .config_port =3D qcom_geni_serial_config_port, - .shutdown =3D qcom_geni_serial_shutdown, + .shutdown =3D qcom_geni_serial_shutdown_fifo, .type =3D qcom_geni_serial_get_type, .set_mctrl =3D qcom_geni_serial_set_mctrl, .get_mctrl =3D qcom_geni_serial_get_mctrl, @@ -1582,7 +1635,7 @@ static const struct uart_ops qcom_geni_uart_pops =3D { .startup =3D qcom_geni_serial_startup, .request_port =3D qcom_geni_serial_request_port, .config_port =3D qcom_geni_serial_config_port, - .shutdown =3D qcom_geni_serial_shutdown, + .shutdown =3D qcom_geni_serial_shutdown_dma, .type =3D qcom_geni_serial_get_type, .set_mctrl =3D qcom_geni_serial_set_mctrl, .get_mctrl =3D qcom_geni_serial_get_mctrl, --=20 2.45.1.288.g0e0cd299f1-goog From nobody Tue Dec 16 14:34:59 2025 Received: from mail-pl1-f181.google.com (mail-pl1-f181.google.com [209.85.214.181]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0DFE0194C63 for ; Thu, 30 May 2024 22:46:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.181 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717109198; cv=none; b=ipcqbH/w78wt6HMdFfBJl7qGSJL2nQtPsV2Oh2IoyIXLZowLdFOm6DKDzQbKpMQ5nXGREdxyGMzFmuwQb4foRY7VlVpI5XM2BPUu+As4jCGfkBBIxebWhiiQLJEpLGaeKXu45li0C5tqONjZ++xIEF0zTbpcpf5CZ/Mq/m9SCoo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717109198; c=relaxed/simple; bh=dYQG2uZdy/1rVNXujDXBm4SlFNLD74RGPIkgsTY0dMI=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=aZ2X2B/IXFHfMgDLbWuykzMR5MZlGB1lWHq4q3oVnN2rMsVDyfyXv+lkL33CHuJe8cxVMysMLJMtYdzPYjls1Lvim1UTHt4uAj5qoz0Cyrd0k7pmYVm9AE2tQvtWUzlM0D99iE8F5btB0bHB7UWtbRwmf+zEFb17pDP4yE6C4Vg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b=iREbt5V0; arc=none smtp.client-ip=209.85.214.181 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="iREbt5V0" Received: by mail-pl1-f181.google.com with SMTP id d9443c01a7336-1f4a0050b9aso3379705ad.2 for ; Thu, 30 May 2024 15:46:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1717109195; x=1717713995; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=HsgVqar/ZibnobIU2tZqJgfYFupn4BGm0JZevOJYAUY=; b=iREbt5V0MPhTw0SIHk8bpoJdINZGAgaF9ADdNMB4/9MXvVpJO1sqX3RvUyfP/rSZ6z 2tRzCIp9bBdudJtssjhtK6DeujqBzeNc/DIdFc3llZ2yY0phBJ4PdZ/sv8wl/CcNGcJh oJbh1LHIeLAqvCJvKcndt6LaKvF6Hardkis5s= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717109195; x=1717713995; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=HsgVqar/ZibnobIU2tZqJgfYFupn4BGm0JZevOJYAUY=; b=avsSO9bbU9jyRmyscxOmf5ymypsPjaYc8YZ/5oKRI6utMsOBn8uSRkeXhIERfoH7UU g90dEDQ3OjkTzgOLQyBRVZudScm1wsVmAFf0264Nb5Bx7Ag5b8rOemY6X74piQUfdsCe 9lrqp5yfuEj82aC1eSRrVEBIuuSHDrwvbW162MzEohLfKKbO3+Ab8Bq3eW2aerCRYaaO hbcp4BxuUs1FJGe6XnK4PzfvVBOcjkJ+MTzAYILwaPckKOpNLnh7PRELRuush0jjlpAn BVgIKYnHAZgSFWb6aGojf0N2fzqnSWYj9dxLVC3iWKoFsVphhs4xBw+HBYNhYnGDEIR4 OH+Q== X-Forwarded-Encrypted: i=1; AJvYcCXpXWDX6cF85LyL64xwUYv9TDwNJpXtqTtqIfg/n/wWAKr57hREEMjZMrRcHMVhjqqBw9l2pBNOcDcnPdwHDTtc2bvyT0UijahoVao4 X-Gm-Message-State: AOJu0Yw44Qk7vuYoBmIUKEB7DOahxwhJdAyRo39jLfmTKIJq6TC5cTPy 8MP1fB61SzwahboK4hpqKazTzKDq/sVMxm+2KrUpJP0ntZ3JxLUTtVATxylCSw== X-Google-Smtp-Source: AGHT+IF/BnecJTcUljBsfx7blUPfSOOrIEqvvao0pHu4k4PsQWZAQS7ovwUVFXdlILFJGbMA9vd5WA== X-Received: by 2002:a17:903:2285:b0:1e2:9aa7:fd21 with SMTP id d9443c01a7336-1f6370a77admr1664575ad.54.1717109195265; Thu, 30 May 2024 15:46:35 -0700 (PDT) Received: from dianders.sjc.corp.google.com ([2620:15c:9d:2:564b:72b6:4827:cf6a]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1f632410b20sm2955795ad.273.2024.05.30.15.46.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 May 2024 15:46:34 -0700 (PDT) From: Douglas Anderson To: Greg Kroah-Hartman , Jiri Slaby Cc: linux-arm-msm@vger.kernel.org, John Ogness , =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= , Tony Lindgren , Stephen Boyd , linux-serial@vger.kernel.org, Yicong Yang , Johan Hovold , linux-kernel@vger.kernel.org, Bjorn Andersson , Andy Shevchenko , Konrad Dybcio , Douglas Anderson , Vijaya Krishna Nivarthi Subject: [PATCH v2 7/7] serial: qcom-geni: Rework TX in FIFO mode to fix hangs/lockups Date: Thu, 30 May 2024 15:45:59 -0700 Message-ID: <20240530154553.v2.7.I1af05e555c42a9c98435bb7aee0ee60e3dcd015e@changeid> X-Mailer: git-send-email 2.45.1.288.g0e0cd299f1-goog In-Reply-To: <20240530224603.730042-1-dianders@chromium.org> References: <20240530224603.730042-1-dianders@chromium.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" The fact that the Qualcomm GENI hardware interface is based around "packets" is really awkward to fit into Linux's UART design. Specifically, in order to send bytes you need to start up a new "command" saying how many bytes you want to send and then you need to send all those bytes. Once you've committed to sending that number of bytes it's very awkward to change your mind and send fewer, especially if you want to do so without dropping bytes on the ground. There may be a few cases where you might want to send fewer bytes than you originally expected: 1. You might want to interrupt the transfer with something higher priority, like the kernel console or kdb. 2. You might want to enter system suspend. 3. The user might have killed the program that had queued bytes for sending over the UART. Despite this awkwardness the Linux driver has still tried to send bytes using large transfers. Whenever the driver started a new transfer it would look at the number of bytes in the OS's queue and start a transfer for that many. The idea of using larger transfers is that it should be more efficient. When we're in the middle of a large transfer we can get interrupted when the hardware FIFO is close to empty and add more bytes in. Whenever we get to the end of a transfer we have to wait until the transfer is totally done before we can add more bytes and, depending on interrupt latency, that can cause the UART to idle a bit. Unfortunately there were lots of corner cases that the Linux driver didn't handle. One problem with the current driver is that if the user killed the program that queued bytes for sending over the UART then bad things would happen. Before commit 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo") we'd just send stale data out the UART. After that commit we'll hard lockup. Another problem with the current driver can be seen if you queue a bunch of data to the UART and enter kdb. Specifically on a device _without_ kernel console on the UART, with an agetty on the uart, and with kgdb on the UART, I did `cat /var/log/messages` and then dropped into kdb. After resuming from kdb console output stopped. Let's give up on trying to use large transfers in FIFO mode on GENI UART since there doesn't appear to be any way to solve these problems cleanly. Visually inspecting the console output even after these patches doesn't show any big pauses so this should be fine. In order to make this all work: - Switch the watermark interrupt to just being used to prime the TX pump. Once transfers are running we'll use "done" to queue the next batch. As part of this, change the watermark to fire whenever the queue is empty. - Never queue more than what can fit in the FIFO. This means we don't need to keep track of a command we're partway through. - For the console code and kgdb code where we can safely block while the queue empties we can just do that rather than trying to queue a command when one was already in progress. This didn't actually work so well which is presumably why there were some weird/awkward hacks in qcom_geni_serial_console_write(). - Leave the CMD_DONE interrupt enabled all the time since there's never any reason we don't want to see it. - Start using the "SE_GENI_M_IRQ_EN_SET" and "SE_GENI_M_IRQ_EN_CLEAR" registers to avoid read-modify-write of the "SE_GENI_M_IRQ_EN" register. We could do this in more of the driver if needed but for now just update code we're touching. Fixes: 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo") Fixes: a1fee899e5be ("tty: serial: qcom_geni_serial: Fix softlock") Signed-off-by: Douglas Anderson --- I'm listing two "fixes" commits here. The first is the kfifo change since it is very easy to see a hardlockup after that change. Almost certainly anyone with the kfifo patch wants this patch. I've also listed a much earlier patch as one being fixed since that was the one that made us send larger transfers. I've tested this commit on an sc7180-trogdor board both with and without kernel console going to the UART. I've tested across some suspend/resume cycles and with kgdb. I've also confirmed that bluetooth, which uses the DMA paths in this driver, continues to work. That all being said, a lot of things change here so I'd love any testing folks want to do. Changes in v2: - New drivers/tty/serial/qcom_geni_serial.c | 192 +++++++++++++------------- 1 file changed, 94 insertions(+), 98 deletions(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qco= m_geni_serial.c index 10aeb0313f9b..853f5288dde5 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -78,7 +78,7 @@ #define GENI_UART_CONS_PORTS 1 #define GENI_UART_PORTS 3 #define DEF_FIFO_DEPTH_WORDS 16 -#define DEF_TX_WM 2 +#define DEF_TX_WM 1 #define DEF_FIFO_WIDTH_BITS 32 #define UART_RX_WM 2 =20 @@ -129,8 +129,8 @@ struct qcom_geni_serial_port { void *rx_buf; u32 loopback; bool brk; + bool tx_fifo_stopped; =20 - unsigned int tx_remaining; unsigned int tx_total; int wakeup_irq; bool rx_tx_swap; @@ -363,6 +363,14 @@ static void qcom_geni_serial_poll_tx_done(struct uart_= port *uport) M_CMD_ABORT_EN, true); } writel(irq_clear, uport->membase + SE_GENI_M_IRQ_CLEAR); + + /* + * Re-enable the TX watermark interrupt when we clear the "done" + * in case we were waiting on the "done" bit before starting a new + * command. The interrupt routine will re-disable this if it's not + * appropriate. + */ + writel(M_TX_FIFO_WATERMARK_EN, uport->membase + SE_GENI_M_IRQ_EN_SET); } =20 static void qcom_geni_serial_drain_tx_fifo(struct uart_port *uport) @@ -384,7 +392,7 @@ static void qcom_geni_serial_drain_tx_fifo(struct uart_= port *uport) * get lost. */ qcom_geni_serial_poll_bitfield(uport, SE_GENI_M_GP_LENGTH, 0xffffffff, - port->tx_total - port->tx_remaining); + port->tx_total); =20 /* * If clearing the FIFO made us inactive then we're done--no need for @@ -413,14 +421,6 @@ static void qcom_geni_serial_drain_tx_fifo(struct uart= _port *uport) writel(M_CMD_ABORT_EN, uport->membase + SE_GENI_M_IRQ_CLEAR); } writel(M_CMD_CANCEL_EN, uport->membase + SE_GENI_M_IRQ_CLEAR); - - /* - * We've cancelled the current command. "tx_remaining" stores how - * many bytes are left to finish in the current command so we know - * when to start a new command. Since the command was cancelled we - * need to zero "tx_remaining". - */ - port->tx_remaining =3D 0; } =20 static void qcom_geni_serial_abort_rx(struct uart_port *uport) @@ -480,11 +480,12 @@ static int qcom_geni_serial_get_char(struct uart_port= *uport) static void qcom_geni_serial_poll_put_char(struct uart_port *uport, unsigned char c) { + qcom_geni_serial_drain_tx_fifo(uport); + qcom_geni_serial_setup_tx(uport, 1); WARN_ON(!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS, M_TX_FIFO_WATERMARK_EN, true)); writel(c, uport->membase + SE_GENI_TX_FIFOn); - writel(M_TX_FIFO_WATERMARK_EN, uport->membase + SE_GENI_M_IRQ_CLEAR); qcom_geni_serial_poll_tx_done(uport); } #endif @@ -514,6 +515,8 @@ __qcom_geni_serial_console_write(struct uart_port *upor= t, const char *s, int i; u32 bytes_to_send =3D count; =20 + qcom_geni_serial_drain_tx_fifo(uport); + for (i =3D 0; i < count; i++) { /* * uart_console_write() adds a carriage return for each newline. @@ -564,7 +567,6 @@ static void qcom_geni_serial_console_write(struct conso= le *co, const char *s, bool locked =3D true; unsigned long flags; u32 geni_status; - u32 irq_en; =20 WARN_ON(co->index < 0 || co->index >=3D GENI_UART_CONS_PORTS); =20 @@ -580,38 +582,10 @@ static void qcom_geni_serial_console_write(struct con= sole *co, const char *s, =20 geni_status =3D readl(uport->membase + SE_GENI_STATUS); =20 - if (!locked) { - /* - * We can only get here if an oops is in progress then we were - * unable to get the lock. This means we can't safely access - * our state variables like tx_remaining. About the best we - * can do is wait for the FIFO to be empty before we start our - * transfer, so we'll do that. - */ - qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS, - M_TX_FIFO_NOT_EMPTY_EN, false); - } else if ((geni_status & M_GENI_CMD_ACTIVE) && !port->tx_remaining) { - /* - * It seems we can't interrupt existing transfers if all data - * has been sent, in which case we need to look for done first. - */ - qcom_geni_serial_poll_tx_done(uport); - - if (!kfifo_is_empty(&uport->state->port.xmit_fifo)) { - irq_en =3D readl(uport->membase + SE_GENI_M_IRQ_EN); - writel(irq_en | M_TX_FIFO_WATERMARK_EN, - uport->membase + SE_GENI_M_IRQ_EN); - } - } - __qcom_geni_serial_console_write(uport, s, count); =20 - - if (locked) { - if (port->tx_remaining) - qcom_geni_serial_setup_tx(uport, port->tx_remaining); + if (locked) uart_port_unlock_irqrestore(uport, flags); - } } =20 static void handle_rx_console(struct uart_port *uport, u32 bytes, bool dro= p) @@ -688,9 +662,9 @@ static void qcom_geni_serial_stop_tx_dma(struct uart_po= rt *uport) =20 if (port->tx_dma_addr) { geni_se_tx_dma_unprep(&port->se, port->tx_dma_addr, - port->tx_remaining); + port->tx_total); port->tx_dma_addr =3D 0; - port->tx_remaining =3D 0; + port->tx_total =3D 0; } =20 geni_se_cancel_m_cmd(&port->se); @@ -735,26 +709,27 @@ static void qcom_geni_serial_start_tx_dma(struct uart= _port *uport) qcom_geni_serial_stop_tx_dma(uport); return; } - - port->tx_remaining =3D xmit_size; } =20 static void qcom_geni_serial_start_tx_fifo(struct uart_port *uport) { - u32 irq_en; + struct qcom_geni_serial_port *port =3D to_dev_port(uport); =20 - irq_en =3D readl(uport->membase + SE_GENI_M_IRQ_EN); - irq_en |=3D M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN; - writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN); + port->tx_fifo_stopped =3D false; + + /* Prime the pump to get data flowing. */ + writel(M_TX_FIFO_WATERMARK_EN, uport->membase + SE_GENI_M_IRQ_EN_SET); } =20 static void qcom_geni_serial_stop_tx_fifo(struct uart_port *uport) { - u32 irq_en; + struct qcom_geni_serial_port *port =3D to_dev_port(uport); =20 - irq_en =3D readl(uport->membase + SE_GENI_M_IRQ_EN); - irq_en &=3D ~(M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN); - writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN); + /* + * We can't do anything to safely pause the bytes that have already + * been queued up so just set a flag saying we shouldn't queue any more. + */ + port->tx_fifo_stopped =3D true; } =20 static void qcom_geni_serial_handle_rx_fifo(struct uart_port *uport, bool = drop) @@ -922,10 +897,20 @@ static void qcom_geni_serial_stop_tx(struct uart_port= *uport) uport->ops->stop_tx(uport); } =20 +static void qcom_geni_serial_enable_cmd_done(struct uart_port *uport) +{ + struct qcom_geni_serial_port *port =3D to_dev_port(uport); + + /* If we're not in FIFO mode we don't use CMD_DONE. */ + if (port->dev_data->mode !=3D GENI_SE_FIFO) + return; + + writel(M_CMD_DONE_EN, uport->membase + SE_GENI_M_IRQ_EN_SET); +} + static void qcom_geni_serial_send_chunk_fifo(struct uart_port *uport, unsigned int chunk) { - struct qcom_geni_serial_port *port =3D to_dev_port(uport); unsigned int tx_bytes, remaining =3D chunk; u8 buf[BYTES_PER_FIFO_WORD]; =20 @@ -938,52 +923,74 @@ static void qcom_geni_serial_send_chunk_fifo(struct u= art_port *uport, iowrite32_rep(uport->membase + SE_GENI_TX_FIFOn, buf, 1); =20 remaining -=3D tx_bytes; - port->tx_remaining -=3D tx_bytes; } } =20 -static void qcom_geni_serial_handle_tx_fifo(struct uart_port *uport, - bool done, bool active) +static void qcom_geni_serial_handle_tx_fifo(struct uart_port *uport) { struct qcom_geni_serial_port *port =3D to_dev_port(uport); struct tty_port *tport =3D &uport->state->port; size_t avail; size_t pending; u32 status; - u32 irq_en; unsigned int chunk; + bool active; =20 - status =3D readl(uport->membase + SE_GENI_TX_FIFO_STATUS); - - /* Complete the current tx command before taking newly added data */ - if (active) - pending =3D port->tx_remaining; - else - pending =3D kfifo_len(&tport->xmit_fifo); + /* + * The TX watermark interrupt is only used to "prime the pump" for + * transfers. Once transfers have been kicked off we always use the + * "done" interrupt to queue the next batch. Once were here we can + * always disable the TX watermark interrupt. + * + * NOTE: we use the TX watermark in this way because we don't ever + * kick off TX transfers larger than we can stuff into the FIFO. This + * is because bytes from the OS's circular queue can disappear and + * there's no known safe/non-blocking way to cancel the larger + * transfer when bytes disappear. See qcom_geni_serial_drain_tx_fifo() + * for an example of a safe (but blocking) way to drain, but that's + * not appropriate in an IRQ handler. We also can't just kick off one + * large transfer and queue bytes whenever because we're using 4 bytes + * per FIFO word and thus we can only queue non-multiple-of-4 bytes as + * in the last word of a transfer. + */ + writel(M_TX_FIFO_WATERMARK_EN, uport->membase + SE_GENI_M_IRQ_EN_CLEAR); =20 - /* All data has been transmitted and acknowledged as received */ - if (!pending && !status && done) { - qcom_geni_serial_stop_tx_fifo(uport); + /* + * If we've got an active TX command running then we expect to still + * see the "done" bit in the future and we can't kick off another + * transfer till then. Bail. NOTE: it's important that we read "active" + * after we've cleared the "done" interrupt (which the caller already + * did for us) so that we know that if we show as non-active we're + * guaranteed to later get "done". + * + * If nothing is pending we _also_ want to bail. Later start_tx() + * will start transfers again by temporarily turning on the TX + * watermark. + */ + active =3D readl(uport->membase + SE_GENI_STATUS) & M_GENI_CMD_ACTIVE; + pending =3D port->tx_fifo_stopped ? 0 : kfifo_len(&tport->xmit_fifo); + if (active || !pending) goto out_write_wakeup; - } =20 + /* Calculate how much space is available in the FIFO right now. */ + status =3D readl(uport->membase + SE_GENI_TX_FIFO_STATUS); avail =3D port->tx_fifo_depth - (status & TX_FIFO_WC); avail *=3D BYTES_PER_FIFO_WORD; =20 - chunk =3D min(avail, pending); - if (!chunk) + /* + * It's a bit odd if we get here and have bytes pending and we're + * handling a "done" or "TX watermark" interrupt but we don't + * have space in the FIFO. Stick in a warning and bail. + */ + if (!avail) { + dev_warn(uport->dev, "FIFO unexpectedly out of space\n"); goto out_write_wakeup; - - if (!port->tx_remaining) { - qcom_geni_serial_setup_tx(uport, pending); - port->tx_remaining =3D pending; - - irq_en =3D readl(uport->membase + SE_GENI_M_IRQ_EN); - if (!(irq_en & M_TX_FIFO_WATERMARK_EN)) - writel(irq_en | M_TX_FIFO_WATERMARK_EN, - uport->membase + SE_GENI_M_IRQ_EN); } =20 + + /* We're ready to throw some bytes into the FIFO. */ + chunk =3D min(avail, pending); + qcom_geni_serial_setup_tx(uport, chunk); qcom_geni_serial_send_chunk_fifo(uport, chunk); =20 /* @@ -991,17 +998,9 @@ static void qcom_geni_serial_handle_tx_fifo(struct uar= t_port *uport, * cleared it in qcom_geni_serial_isr it will have already reasserted * so we must clear it again here after our writes. */ - writel(M_TX_FIFO_WATERMARK_EN, - uport->membase + SE_GENI_M_IRQ_CLEAR); + writel(M_TX_FIFO_WATERMARK_EN, uport->membase + SE_GENI_M_IRQ_CLEAR); =20 out_write_wakeup: - if (!port->tx_remaining) { - irq_en =3D readl(uport->membase + SE_GENI_M_IRQ_EN); - if (irq_en & M_TX_FIFO_WATERMARK_EN) - writel(irq_en & ~M_TX_FIFO_WATERMARK_EN, - uport->membase + SE_GENI_M_IRQ_EN); - } - if (kfifo_len(&tport->xmit_fifo) < WAKEUP_CHARS) uart_write_wakeup(uport); } @@ -1011,10 +1010,10 @@ static void qcom_geni_serial_handle_tx_dma(struct u= art_port *uport) struct qcom_geni_serial_port *port =3D to_dev_port(uport); struct tty_port *tport =3D &uport->state->port; =20 - uart_xmit_advance(uport, port->tx_remaining); - geni_se_tx_dma_unprep(&port->se, port->tx_dma_addr, port->tx_remaining); + uart_xmit_advance(uport, port->tx_total); + geni_se_tx_dma_unprep(&port->se, port->tx_dma_addr, port->tx_total); port->tx_dma_addr =3D 0; - port->tx_remaining =3D 0; + port->tx_total =3D 0; =20 if (!kfifo_is_empty(&tport->xmit_fifo)) qcom_geni_serial_start_tx_dma(uport); @@ -1028,7 +1027,6 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void= *dev) u32 m_irq_en; u32 m_irq_status; u32 s_irq_status; - u32 geni_status; u32 dma; u32 dma_tx_status; u32 dma_rx_status; @@ -1046,7 +1044,6 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void= *dev) s_irq_status =3D readl(uport->membase + SE_GENI_S_IRQ_STATUS); dma_tx_status =3D readl(uport->membase + SE_DMA_TX_IRQ_STAT); dma_rx_status =3D readl(uport->membase + SE_DMA_RX_IRQ_STAT); - geni_status =3D readl(uport->membase + SE_GENI_STATUS); dma =3D readl(uport->membase + SE_GENI_DMA_MODE_EN); m_irq_en =3D readl(uport->membase + SE_GENI_M_IRQ_EN); writel(m_irq_status, uport->membase + SE_GENI_M_IRQ_CLEAR); @@ -1093,9 +1090,7 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void= *dev) } else { if (m_irq_status & m_irq_en & (M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN)) - qcom_geni_serial_handle_tx_fifo(uport, - m_irq_status & M_CMD_DONE_EN, - geni_status & M_GENI_CMD_ACTIVE); + qcom_geni_serial_handle_tx_fifo(uport); =20 if (s_irq_status & (S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN)) qcom_geni_serial_handle_rx_fifo(uport, drop_rx); @@ -1203,6 +1198,7 @@ static int qcom_geni_serial_port_setup(struct uart_po= rt *uport) geni_se_init(&port->se, UART_RX_WM, port->rx_fifo_depth - 2); geni_se_select_mode(&port->se, port->dev_data->mode); writel(DEF_TX_WM, uport->membase + SE_GENI_TX_WATERMARK_REG); + qcom_geni_serial_enable_cmd_done(uport); qcom_geni_serial_start_rx(uport); port->setup =3D true; =20 --=20 2.45.1.288.g0e0cd299f1-goog