Re: [RESEND PATCH 2/2] PCI: rockchip: Add quirk to disable RC's ASPM L0s

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 29, 2016 at 09:34:09AM -0600, Rob Herring wrote:
> On Sun, Nov 13, 2016 at 10:11 PM, Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> wrote:
> > Rockchip's RC outputs 100MHz reference clock but there are
> > two methods for PHY to generate it.
> >
> > (1)One of them is to use system PLL to generate 100MHz clock and
> > the PHY will relock it and filter signal noise then outputs the
> > reference clock.
> >
> > (2)Another way is to share Soc's 24MHZ crystal oscillator with
> > PHY and force PHY's DLL to generate 100MHz internally.
> >
> > When using case(2), the exit from L0s doesn't work fine occasionally
> > due to the broken design of RC receiver's logical circuit. So even if
> > we use extended-synch, it still fails for PHY to relock the bits from
> > FTS sometimes. This will hang the system.
> >
> > Maybe we could argue that why not use case(1) to avoid it? The reason
> > is that as we could see the reference clock is derived from system PLL
> > and the path from it to PHY isn't so clean which means there are some
> > noise introduced by power-domain and other buses can't be filterd out
> > by PHY and we could see noise from the frequency spectrum by oscilloscope.
> > This makes the TX compatibility test a little difficult to pass the spec.
> > So case(1) and case(2) are both used indeed now. If using case(2), we
> > should disable RC's L0s support, and that is why we need this property to
> > indicate this quirk.
> >
> > Also after checking quirk.c, I noticed there is already a quirk for
> > disabling L0s unconditionally, quirk_disable_aspm_l0s. But obviously we
> > shouldn't do that as mentioned above that case(1) could still works fine
> > with L0s.
> >
> > Reported-by: Jeffy Chen <jeffy.chen@xxxxxxxxxxxxxx>
> > Cc: Brian Norris <briannorris@xxxxxxxxxxxx>
> > Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>
> >
> > ---
> >
> >  Documentation/devicetree/bindings/pci/rockchip-pcie.txt | 2 ++
> >  drivers/pci/host/pcie-rockchip.c                        | 9 +++++++++
> >  2 files changed, 11 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> > index ba67b39..cfa44a7 100644
> > --- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> > @@ -42,6 +42,8 @@ Required properties:
> >  Optional Property:
> >  - ep-gpios: contain the entry for pre-reset gpio
> >  - num-lanes: number of lanes to use
> > +- quirk,aspm-no-l0s: RC won't support ASPM L0s. This property is needed if
> 
> quirk is not a vendor. Drop it.

I dropped this patch from pci/host-rockchip for now.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux