From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) (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 F31261DE3AD for ; Mon, 6 Jan 2025 14:00:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.44 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736172036; cv=none; b=nD2op8yMGO524gN/9cBKRU/Kal+YjknjkNO6qfSJgq74shrJ1A5kDTuwpcIv0WeRykYM5RlUEc5IZr+lTXYTxJjhuYyvDzNs6vUciJnvBFSZ7Th9xic+NxoTGSFZmi+4tHs3MfQJ1mVU7oI4b58i/6Szq+AXXVOeq+j8vgE8agg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736172036; c=relaxed/simple; bh=1BNqprANFARLX7QiedDZOdix7i0iUCsfKc05vZpKdYA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=kB17hzSGE1caZrSCzKlM7dj5HwQ2L7wM8Sl2H/nLry9ujwB0Ft7mduWrfEJY03jN3ILPgBP3WXlzhqNaVWHnINCj1qZqcb0T/MqDK9Q85Dv8Ohha3rkRxhRQczzX/4Xna7rumeNMPkOUOGkcMaUg6HDwoDJvhgpcdpqXkoddUDw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=U2FyV+A1; arc=none smtp.client-ip=209.85.128.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="U2FyV+A1" Received: by mail-wm1-f44.google.com with SMTP id 5b1f17b1804b1-43690d4605dso62487145e9.0 for ; Mon, 06 Jan 2025 06:00:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1736172032; x=1736776832; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=G4YVUyvlmLs4ShZ5WoAXv9zQ/4rn/hNmmIlIQjqYU1c=; b=U2FyV+A1hghsyg7FXeajjfWcvfmuyOXCourdYHvvt5OnxyrQAZQWVUA6/fLabjckDW iWUKm4gJSQvNGhLpFVbK4evhbwHPkN7yjd9NO1kCJdeYpBC4dndX9kQ4VHxGRKbW+3ut TwVM+A71p7+OtjsFNVEPGPScvpda2fb5P4nu8KlH+Vcege+7IZMRQ6KBKODOtt2Ucfet j3oLruni1Zb4XvUIOQmhNv3/yXElvTg5YIZElW08IYGbd8zyOevRuvPH7dRc+TWeAgWo kttAPEKOq2ZMeF6n/fYCJdnS6LM8dUEAAtYP+0xRSaH8yK1hK85FDBdJSF/0BcO7hilQ dTLw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736172032; x=1736776832; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=G4YVUyvlmLs4ShZ5WoAXv9zQ/4rn/hNmmIlIQjqYU1c=; b=qOQ16wzA4ZVYuSrxq+EIPgipX0CAyzzX5vqSO4MgRzyEpfwdl1utr1Qa+gnXkEVLAa fLuctgnrUPg0FQvTOJCscxgJgGbkiYaK7omb49JOHrrC8neLsmzsnLV5v6oEOKVSlo+3 n2+jMQyAhAqBfonrD/kOWFJpfCbzM2NmxuixIFsjIoh4UY3uiO4Ibo0a8jo2SHF2NQMl 5aLOjaKxm2MFJdMCckfXm9WjN6i+VQlZU791gPWjrXSzjgjJyVd5Jv0YWleC/UIBjuFd qL5xk5z/CdyxoKh1EsvTZjJHoAqtejlPThJpU0VC86kEBlQpKTdbp6U6etFEwQxJv9F4 0u8g== X-Forwarded-Encrypted: i=1; AJvYcCUWGBS+JW1lAjkHyuRHgfKIWAK98OosBMi4GaJ5jItDanq9AtEC6JNu31Eiy0gSFIF+gXpOVoNx7bhbjVA=@vger.kernel.org X-Gm-Message-State: AOJu0YxuFYvP9OHXKudgsOVal3xBVTeddlU/T/JDZlnfjE2dPziICsWE yj+3tkSrwkL/ea6t9cqJNPE4CK2auZy0Wey+Kn6HryqCfOnlJA8Nv5fkCDebGNjIf2Cpsh27n21 l X-Gm-Gg: ASbGncuK2eX75hofLG2wYnv+RW1/kjcWbcgTKBUWZBjRNE+Xyr0q+OOz86jeZaKFRAa tbeLbrrPWOr3QSJlw6x9AwEtu+JX2noLgioRra/k8uHyU7ggKtSK3u6a1fO7vQ4PhWJ30NSGqWg +SpR9sQnL9mWdS0B7Z1Y3wSj0W9+Mx6Iyrpg6mmdvlxuAlFJQSiUo4IoficeWYc6nSq4m2UwsfD TMVDuMKExUsv04PKp4q+yIiRsw2H2g7XsjBWBRVNT3UduenhyovBXTyLg== X-Google-Smtp-Source: AGHT+IEqaDb8lNaJ2brtbrHX0OiZbuauW9GLm6TzQr7FyD4aBcCJ9xDlwVNH3hoY7B3x4d3Qs9SuiQ== X-Received: by 2002:a05:6000:1848:b0:385:f220:f798 with SMTP id ffacd0b85a97d-38a221f3001mr38289712f8f.6.1736172031927; Mon, 06 Jan 2025 06:00:31 -0800 (PST) Received: from pathway.suse.cz ([176.114.240.50]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-43661219a7csm565553555e9.24.2025.01.06.06.00.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Jan 2025 06:00:31 -0800 (PST) Date: Mon, 6 Jan 2025 15:00:28 +0100 From: Petr Mladek To: John Ogness Cc: Greg Kroah-Hartman , Jiri Slaby , Sergey Senozhatsky , Steven Rostedt , Thomas Gleixner , Esben Haabendal , linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org, Florian Fainelli , Ray Jui , Scott Branden , Broadcom internal kernel review list , Andy Shevchenko , Arnd Bergmann , Sunil V L , Matt Turner , Stefan Wahren , Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , Kevin Hilman , Markus Schneider-Pargmann , Udit Kumar , Griffin Kroah-Hartman , Niklas Schnelle , Serge Semin , linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH tty-next v4 4/6] serial: 8250: Provide flag for IER toggling for RS485 Message-ID: References: <20241227224523.28131-1-john.ogness@linutronix.de> <20241227224523.28131-5-john.ogness@linutronix.de> <84jzbaxg13.fsf@jogness.linutronix.de> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <84jzbaxg13.fsf@jogness.linutronix.de> On Sun 2025-01-05 01:32:00, John Ogness wrote: > On 2025-01-03, Petr Mladek wrote: > > My understanding is that the nested IER manipulation does not > > harm. > > This statement implies that it is OK for UART_IER_RLSI|UART_IER_RDI bits > to be set in UART_IER even though the console will write to UART_TX, > because the _nesting_ contexts would set those bits rather than > restoring the original value of 0x0. This is a misunderstanding. I am sorry I was not clear enough. To be more clear. By the nested context I meant if (em485) { if (em485->tx_stopped) up->rs485_start_tx(up); call by serial8250_console_write(). The original code did: + up->rs485_start_tx() + serial8250_em485_start_tx() + serial8250_stop_rx() , where serial8250_stop_rx() cleared the flags: static void serial8250_stop_rx(struct uart_port *port) { [...] up->ier &= ~(UART_IER_RLSI | UART_IER_RDI); serial_port_out(port, UART_IER, up->ier); [...] } But the flags were already cleared by: __serial8250_clear_IER(up); called by serial8250_console_write() _earlier_. Which did: static void __serial8250_clear_IER(struct uart_8250_port *up) { if (up->capabilities & UART_CAP_UUE) serial_out(up, UART_IER, UART_IER_UUE); else serial_out(up, UART_IER, 0); } It means that the nested serial8250_stop_rx() did not change anything. It was a NOP. The bits were already cleared. Similar, the counter part. The bits were later set by up->rs485_stop_tx(up) which did: + serial8250_em485_stop_tx void serial8250_em485_stop_tx(struct uart_8250_port *p, bool toggle_ier) { [...] p->ier |= UART_IER_RLSI | UART_IER_RDI; serial_port_out(&p->port, UART_IER, p->ier); [...] } But it was after the writing the message so that it did not affect the operation. > I ran some tests and leaving these bits set during Tx does not appear to > cause an issue, but it is difficult to say because the context > interrupted by a nesting context will only print at most 1 > character. Also, it is writing under spin_lock_irqsave() so that might > be masking any effects. Perhaps UART_IER is temporarly cleared because > of other bits that would cause problems during Tx? > > I would need to create a specific test to investigate this > further. Regardless, it still is a cosmetic ugliness that bits are not > being properly restored, even if it turns out these particular bits are > not problematic during Tx. I think that you do not need to investigate it further. We should keep IER cleared when writing the message. > > All in all, the patch looks good to me. > > > > Reviewed-by: Petr Mladek > > Thanks for the review. I interpret it to mean that I do not need to make > any changes to this patch for v5. Yup, I am fine with the patch as it is. Best Regards, Petr