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 ?
>
> > 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 ?
>
> > 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.
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 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.
