From: Bjorn Helgaas <helgaas@kernel.org>
To: Anand Moon <linux.amoon@gmail.com>
Cc: "Shawn Lin" <shawn.lin@rock-chips.com>,
"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
"Manivannan Sadhasivam" <mani@kernel.org>,
"Rob Herring" <robh@kernel.org>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Heiko Stuebner" <heiko@sntech.de>,
"open list:PCIE DRIVER FOR ROCKCHIP" <linux-pci@vger.kernel.org>,
"open list:PCIE DRIVER FOR ROCKCHIP"
<linux-rockchip@lists.infradead.org>,
"moderated list:ARM/Rockchip SoC support"
<linux-arm-kernel@lists.infradead.org>,
"open list" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC v1 1/5] PCI: rockchip: Fix Link Control register offset and enable ASPM/CLKREQ
Date: Tue, 18 Nov 2025 11:50:10 -0600 [thread overview]
Message-ID: <20251118175010.GA2540980@bhelgaas> (raw)
In-Reply-To: <20251117181023.482138-2-linux.amoon@gmail.com>
On Mon, Nov 17, 2025 at 11:40:09PM +0530, Anand Moon wrote:
> As per the RK3399 TRM (Part 2, 17.6.6.1.31), the Link Control register
> (RC_CONFIG_LC) resides at an offset of 0xd0 within the Root Complex (RC)
> configuration space, not at the offset of the PCI Express Capability List
> (0xc0). Following changes correct the register offset to use
> PCIE_RC_CONFIG_LC (0xd0) to configure link control.
>
> Additionally, this commit explicitly enables ASPM (Active State Power
> Management) control and the CLKREQ# (Clock Request) mechanism as part of
> the Link Control register programming when enabling bandwidth
> notifications.
Don't do two things at once in the same patch. Fix the register
offset in one patch. Actually, as I mentioned at [1], there's a lot
of fixing to do there, and I'm not even going to consider other
changes until the #define mess is cleaned up.
What I'd really like to see is at least two patches here: one that
clearly makes no functional change -- don't try to fix anything, just
make it 100% obvious that all the offsets stay the same. Then make a
separate patch that *only* changes any of the offsets that are wrong.
I don't think there should even be an ASPM change. The PCI core
should be enabling L0s and L1 itself for DT systems like this. And
ASPM needs to be enabled only when both ends of the link support it,
and only in a specific order. The PCI core pays attention to that,
but this patch does not.
Bjorn
[1] https://lore.kernel.org/r/20251118005056.GA2541796@bhelgaas
next prev parent reply other threads:[~2025-11-18 17:50 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-17 18:10 [RFC v1 0/5] Fix some register offset as per RK3399 TRM part 2 Anand Moon
2025-11-17 18:10 ` [RFC v1 1/5] PCI: rockchip: Fix Link Control register offset and enable ASPM/CLKREQ Anand Moon
2025-11-18 17:50 ` Bjorn Helgaas [this message]
2025-11-19 14:19 ` Anand Moon
2025-11-20 3:44 ` Bjorn Helgaas
2025-11-20 13:58 ` Anand Moon
2025-11-26 10:33 ` Anand Moon
2025-11-26 14:39 ` Bjorn Helgaas
2025-11-27 7:34 ` Anand Moon
2025-11-17 18:10 ` [RFC v1 2/5] PCI: rockchip: Fix Device Control register offset for Max payload size Anand Moon
2025-11-18 0:50 ` Bjorn Helgaas
2025-11-18 11:16 ` Anand Moon
2025-11-17 18:10 ` [RFC v1 3/5] PCI: rockchip: Fix Slot Capability Register offset for slot power limit Anand Moon
2025-11-17 18:10 ` [RFC v1 4/5] PCI: rockchip: Fix Link Control and Status Register 2 for target link speed Anand Moon
2025-11-17 18:10 ` [RFC v1 5/5] PCI: rockchip: Fix Linkwidth Control Register offset for Retrain Link Anand Moon
2025-11-18 0:54 ` [RFC v1 0/5] Fix some register offset as per RK3399 TRM part 2 Shawn Lin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251118175010.GA2540980@bhelgaas \
--to=helgaas@kernel.org \
--cc=bhelgaas@google.com \
--cc=heiko@sntech.de \
--cc=kwilczynski@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux.amoon@gmail.com \
--cc=lpieralisi@kernel.org \
--cc=mani@kernel.org \
--cc=robh@kernel.org \
--cc=shawn.lin@rock-chips.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).