From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gloria.sntech.de (gloria.sntech.de [185.11.138.130]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A135C321457; Wed, 27 Aug 2025 07:58:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.11.138.130 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756281516; cv=none; b=Uos4gaGrV8J+ZsC34IYO7F94yjSlj8e0caAIH8D7y75igXAQ1SWmUWgKc/MG9iJhvGbgF+Te+ac8YeG+SsjVDVU56pQ4UA2Fk+4Zva41gcwGt1+H52wJNdAAr6523sVRAPrAV8/sqiX5WQBxoLLW9fjPn8K4PNosT6L9jgHBj6Y= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756281516; c=relaxed/simple; bh=uJQ6mSMA7ygeA5oFmyppBb4E1ANB4bt0CH+tDHmNVhk=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=FDH0WLI315E929ZsvP8JabPhl8XJ2nPlCPITpzh4/ez082DyA269Tm5z/ccoY3pzyV3gRky0wn9wWiMWOgbWJlJLz74zP669KHGSeohd+N56NIzabaGuP+dAR0CBlK0Saz43tuL2z61Zu2EZibvfDGrX538gdAtvBMtTIwdnbeI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=sntech.de; spf=pass smtp.mailfrom=sntech.de; dkim=pass (2048-bit key) header.d=sntech.de header.i=@sntech.de header.b=wx3V3gZx; arc=none smtp.client-ip=185.11.138.130 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=sntech.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=sntech.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=sntech.de header.i=@sntech.de header.b="wx3V3gZx" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sntech.de; s=gloria202408; h=Content-Type:Content-Transfer-Encoding:MIME-Version: References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From:Reply-To; bh=uJQ6mSMA7ygeA5oFmyppBb4E1ANB4bt0CH+tDHmNVhk=; b=wx3V3gZxVatzbagZOgxagDXMFS DbqD+upnRWY4VW0FHtPO/Ndv+DXLpODD94PrK/9DLjH8vLBGEQ3OLMT/aUxW6ZqLmjqOwg1fxqwox vjq06D2iO4V/KKRRgEVqhUMfuZMhlWKgZwrvAL+8D9VC6XyhW3OxzYtz5kMWvK3+YkiKP17ZKoAN1 PsU3gG2oT3sat8dFVC5XO7K7upNYFqUzfRPGrWQ5yhelFaccH7Bvp49FQAWHq3CHwUYVgTZkUjkGG +Plafk3ayemfPfDEvLfUCRGu5WAwmPASOTC1Yn6OUQirf5YAacGQsQfmRs3mo0NXYTjXfS98Lf962 +1H2LB/g==; Received: from [213.244.170.152] (helo=phil.localnet) by gloria.sntech.de with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1urB2j-0003FB-Nv; Wed, 27 Aug 2025 09:58:05 +0200 From: Heiko Stuebner To: Yury Norov , Rasmus Villemoes , Jaehoon Chung , Ulf Hansson , Shreeya Patel , Mauro Carvalho Chehab , Sandy Huang , Andy Yan , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Vinod Koul , Kishon Vijay Abraham I , Nicolas Frattaroli , Liam Girdwood , Mark Brown , Jaroslav Kysela , Takashi Iwai , Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Maxime Coquelin , Alexandre Torgue , Shawn Lin , Lorenzo Pieralisi , Krzysztof =?UTF-8?B?V2lsY3p5xYRza2k=?= , Manivannan Sadhasivam , Rob Herring , Bjorn Helgaas , Chanwoo Choi , MyungJoo Ham , Kyungmin Park , Qin Jian , Michael Turquette , Stephen Boyd , Nathan Chancellor , Nick Desaulniers , Bill Wendling , Justin Stitt , Nicolas Frattaroli Cc: kernel@collabora.com, linux-kernel@vger.kernel.org, linux-mmc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-phy@lists.infradead.org, linux-sound@vger.kernel.org, netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-pci@vger.kernel.org, linux-pm@vger.kernel.org, linux-clk@vger.kernel.org, llvm@lists.linux.dev, Nicolas Frattaroli Subject: Re: [PATCH v3 17/20] PCI: dw-rockchip: Switch to FIELD_PREP_WM16 macro Date: Wed, 27 Aug 2025 09:58:04 +0200 Message-ID: <5730130.X9hSmTKtgW@phil> In-Reply-To: <20250825-byeword-update-v3-17-947b841cdb29@collabora.com> References: <20250825-byeword-update-v3-0-947b841cdb29@collabora.com> <20250825-byeword-update-v3-17-947b841cdb29@collabora.com> Precedence: bulk X-Mailing-List: netdev@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" Am Montag, 25. August 2025, 10:28:37 Mitteleurop=C3=A4ische Sommerzeit schr= ieb Nicolas Frattaroli: > The era of hand-rolled HIWORD_UPDATE macros is over. >=20 > Like many other Rockchip drivers, pcie-dw-rockchip brings with it its > very own flavour of HIWORD_UPDATE. It's occasionally used without a > constant mask, which complicates matters. HIWORD_UPDATE_BIT is a > confusingly named addition, as it doesn't update the bit, it actually > sets all bits in the value to 1. HIWORD_DISABLE_BIT is similarly > confusing; it disables several bits at once by using the value as a mask > and the inverse of value as the value, and the "disabling only these" > effect comes from the hardware actually using the mask. The more obvious > approach would've been HIWORD_UPDATE(val, 0) in my opinion. >=20 > This is part of the motivation why this patch uses hw_bitfield.h's > FIELD_PREP_WM16 instead, where possible. FIELD_PREP_WM16 requires a > constant bit mask, which isn't possible where the irq number is used to > generate a bit mask. For that purpose, we replace it with a more robust > macro than what was there but that should also bring close to zero > runtime overhead: we actually mask the IRQ number to make sure we're not > writing garbage. >=20 > For the remaining bits, there also are some caveats. For starters, the > PCIE_CLIENT_ENABLE_LTSSM and PCIE_CLIENT_DISABLE_LTSSM were named in a > manner that isn't quite truthful to what they do. Their modification > actually spans not just the LTSSM bit but also another bit, flipping > only the LTSSM one, but keeping the other (which according to the TRM > has a reset value of 0) always enabled. This other bit is reserved as of > the IP version RK3588 uses at least, and I have my doubts as to whether > it was meant to be set, and whether it was meant to be set in that code > path. Either way, it's confusing. >=20 > Replace it with just writing either 1 or 0 to the LTSSM bit, using the > new FIELD_PREP_WM16 macro from hw_bitfield.h, which grants us the > benefit of better compile-time error checking. >=20 > The change of no longer setting the reserved bit doesn't appear to > change the behaviour on RK3568 in RC mode, where it's not marked as > reserved. >=20 > PCIE_CLIENT_RC_MODE/PCIE_CLIENT_EP_MODE was another field that wasn't > super clear on what the bit field modification actually is. As far as I > can tell, switching to RC mode doesn't actually write the correct value > to the field if any of its bits have been set previously, as it only > updates one bit of a 4 bit field. >=20 > Replace it by actually writing the full values to the field, using the > new FIELD_PREP_WM16 macro, which grants us the benefit of better > compile-time error checking. >=20 > This patch was tested on RK3588 (PCIe3 x4 controller), RK3576 (PCIe2 x1 > controller) and RK3568 (PCIe x2 controller), all in RC mode. >=20 > Acked-by: Bjorn Helgaas > Signed-off-by: Nicolas Frattaroli Reviewed-by: Heiko Stuebner