From: Dominic Rath <dominic.rath@ibv-augsburg.net>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: "Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
"Kishon Vijay Abraham I" <kishon@ti.com>,
"Christian Gmeiner" <christian.gmeiner@gmail.com>,
linux-kernel@vger.kernel.org, "Tom Joseph" <tjoseph@cadence.com>,
"Rob Herring" <robh@kernel.org>,
"Krzysztof Wilczyński" <kw@linux.com>,
"Bjorn Helgaas" <bhelgaas@google.com>,
linux-pci@vger.kernel.org
Subject: Re: [PATCH] PCI: cadence: respond to received PTM Requests
Date: Thu, 5 May 2022 16:43:47 +0200 [thread overview]
Message-ID: <20220505144347.GA19346@JADEVM-DRA> (raw)
In-Reply-To: <20220429192825.GA82239@bhelgaas>
On Fri, Apr 29, 2022 at 02:28:25PM -0500, Bjorn Helgaas wrote:
> On Fri, Apr 29, 2022 at 11:56:27AM +0100, Lorenzo Pieralisi wrote:
> > On Tue, Feb 22, 2022 at 07:40:54PM +0530, Kishon Vijay Abraham I wrote:
> > > On 18/02/22 6:50 pm, Bjorn Helgaas wrote:
> > > > On Fri, Feb 18, 2022 at 04:26:48PM +0530, Kishon Vijay Abraham I wrote:
> > > >> On 01/02/22 3:35 am, Bjorn Helgaas wrote:
> > > >>> Update subject line to match previous conventions ("git log --oneline
> > > >>> drivers/pci/controller/cadence/pcie-cadence-host.c" to see).
> > > >>>
> > > >>> On Mon, Jan 31, 2022 at 01:08:27PM +0100, Christian Gmeiner wrote:
> > > >>>> This enables the Controller [RP] to automatically respond
> > > >>>> with Response/ResponseD messages.
> > > >
> > > >>>> +static void cdns_pcie_host_enable_ptm_response(struct cdns_pcie *pcie)
> > > >>>> +{
> > > >>>> + u32 val;
> > > >>>> +
> > > >>>> + val = cdns_pcie_readl(pcie, CDNS_PCIE_LM_PTM_CTRL);
> > > >>>> + cdns_pcie_writel(pcie, CDNS_PCIE_LM_PTM_CTRL, val | CDNS_PCIE_LM_TPM_CTRL_PTMRSEN);
> > > >>>
> > > >>> I assume this is some device-specific enable bit that is effectively
> > > >>> ANDed with PCI_PTM_CTRL_ENABLE in the Precision Time Measurement
> > > >>> Capability?
> > > >>
> > > >> That's correct. This bit enables Controller [RP] to respond to the
> > > >> received PTM Requests.
> > > >
> > > > Great! Christian, can you update the commit log to reflect that
> > > > both this bit *and* PCI_PTM_CTRL_ENABLE must be set for the RP to
> > > > respond to received PTM Requests?
> > > >
> > > > When CDNS_PCIE_LM_TPM_CTRL_PTMRSEN is cleared, do PCI_PTM_CAP_ROOT
> > > > and the PTM Responder Capable bit (for which we don't have a #define)
> > > > read as zero?
> > >
> > > I see both PTM Responder Capable bit and PTM Root Capable is
> > > by-default set to '1'.
> >
> > Without this patch applied and with no other SW setting
> > CDNS_PCIE_LM_TPM_CTRL_PTMRSEN, correct ?
Yes. The capability bits default to '1', even if
CDNS_PCIE_LM_TPM_CTRL_PTMRSEN is cleared. Strictly speaking the current
behavior is wrong, and the capability bits should be cleared instead.
> > > root@am64xx-evm:~# devmem2 0xD000A24
> > >
> > > /dev/mem opened.
> > > Memory mapped at address 0xffffa8980000.
> > > Read at address 0x0D000A24 (0xffffa8980a24): 0x00000406
> > >
> > > And this bit can be programmed through the local management APB
> > > interface if required.
> >
> > Which bit ? CDNS_PCIE_LM_TPM_CTRL_PTMRSEN ?
I believe "these bits" would have been more correct. Since these are
capability bits, the AM64* TRM lists them as read-only, but of course
they can be modified. To write them, you need to set bit [21], e.g.
you can write the PTM capability register at address 0xD200A24. At
the TRM address 0xD000A24 it is read-only.
> >
> > > But with this patch which enables PTM by default for RC, it
> > > wouldn't be required to clear those bits.
> >
> > Yes but that does not comply with the specifications as Bjorn
> > pointed out below.
> >
> > We can merge this patch but it would be good to investigate on this
> > point.
>
> I *think* this is OK. Correct me if I'm wrong:
>
> - We're talking about a Root Port.
>
> - The Root Port's PTM Capability reads as 0x00000406 (PTM Responder
> Capable and PTM Root Capable set).
>
> - Without this patch, setting PTM Enable does nothing, and the Root
> Port does not send PTM Responses.
>
> This is the non-conforming situation because the Port claims that
> it implements the PTM Responder role, but it can't actually be
> enabled.
>
> - With this patch that sets CDNS_PCIE_LM_TPM_CTRL_PTMRSEN, the PTM
> Enable bit still powers up as zero, so the Port does not send PTM
> Responses, but setting PTM Enable enables PTM Responses from the
> Root Port.
This is correct.
>
> So I think that after setting CDNS_PCIE_LM_TPM_CTRL_PTMRSEN, the PTM
> capability works as per spec.
>
> I think the proposed subject of "Enable Controller to respond to
> received PTM Requests" is somewhat misleading, though, because PTM
> responses still aren't enabled until we set PTM Enable. I suggest
> something like:
>
> PCI: cadence: Allow PTM Responder to be enabled
I'll ask Christian for permission to resend his patch with the subject
changed.
> > > > I think that would be the correct behavior per PCIe r6.0, sec
> > > > 7.9.15.2, and it would avoid the confusion of having the PTM
> > > > Capability register advertise functionality that cannot be enabled via
> > > > the PTM Control register.
next prev parent reply other threads:[~2022-05-05 14:49 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-31 12:08 [PATCH] PCI: cadence: respond to received PTM Requests Christian Gmeiner
2022-01-31 22:05 ` Bjorn Helgaas
2022-02-04 14:45 ` Christian Gmeiner
2022-02-18 10:56 ` Kishon Vijay Abraham I
2022-02-18 13:13 ` Christian Gmeiner
2022-02-18 13:20 ` Bjorn Helgaas
2022-02-18 13:42 ` Christian Gmeiner
2022-02-22 14:10 ` Kishon Vijay Abraham I
2022-04-29 10:56 ` Lorenzo Pieralisi
2022-04-29 19:28 ` Bjorn Helgaas
2022-05-05 14:43 ` Dominic Rath [this message]
2022-05-05 14:48 ` Christian Gmeiner
2022-05-11 12:37 ` Lorenzo Pieralisi
2022-05-12 6:00 ` Christian Gmeiner
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=20220505144347.GA19346@JADEVM-DRA \
--to=dominic.rath@ibv-augsburg.net \
--cc=bhelgaas@google.com \
--cc=christian.gmeiner@gmail.com \
--cc=helgaas@kernel.org \
--cc=kishon@ti.com \
--cc=kw@linux.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=robh@kernel.org \
--cc=tjoseph@cadence.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).