çå: çå: çå: çå: [PATCH v6] mfd: Add support for RTS5250S power saving

[åé]

> 
> On Fri, Jan 19, 2018 at 07:38:22AM +0000, åé wrote:
> > > On Wed, Dec 27, 2017 at 05:37:50PM -0600, Bjorn Helgaas wrote:
> > > > On Tue, Dec 19, 2017 at 08:15:24AM +0000, åé wrote:
> > > > > > On Fri, Dec 15, 2017 at 09:42:45AM +0000, åé wrote:
> > > > > > > > [+cc Hans, Dave, linux-pci]
> > > > > > > >
> > > > > > > > On Thu, Sep 07, 2017 at 04:26:39PM +0800,
> > > > > > > > rui_feng@xxxxxxxxxxxxxx
> > > > > > wrote:
> > > > > > > > > From: Rui Feng <rui_feng@xxxxxxxxxxxxxx>
> > > > > > > >
> > > > > > > > I wish this had been posted to linux-pci before being merged.
> > > > > > > >
> > > > > > > > I'm concerned because some of this appears to overlap and
> > > > > > > > conflict with PCI core management of ASPM.
> > > > > > > >
> > > > > > > > I assume these devices advertise ASPM support in their
> > > > > > > > Link Capabilites registers, right?  If so, why isn't the
> > > > > > > > existing PCI core ASPM support sufficient?
> > > > > > > >
> > > > > > > When L1SS is configured, the device(hardware) can't enter
> > > > > > > L1SS status automatically, it need driver(software) to do
> > > > > > > some work to achieve the
> > > > > > function.
> > > > > >
> > > > > > So this is a hardware defect in the device?  As far as I know,
> > > > > > ASPM and L1SS are specified such that they should work without
> > > > > > special
> > > driver support.
> > > > > >
> > > > > Yes, you can say that.
> > > > >
> > > > > > > > > Enable power saving for RTS5250S as following steps:
> > > > > > > > > 1.Set 0xFE58 to enable clock power management.
> > > > > > > >
> > > > > > > > Is this clock power management something specific to
> > > > > > > > RTS5250S, or is it standard PCIe architected stuff?
> > > > > > > >
> > > > > > > 0xFE58 is specific register to RTS5250S not standard PCIe
> > > > > > > architected
> > > stuff.
> > > > > >
> > > > > > OK.  I asked because devices often mirror architected PCIe
> > > > > > config things in device-specific MMIO space, and if I squint
> > > > > > just right, I can sort of match up the register bits you used
> > > > > > with things in the PCIe
> > > spec.
> > > > > >
> > > > > > > > > 2.Check cfg space whether support L1SS or not.
> > > > > > > >
> > > > > > > > This sounds like standard PCIe ASPM L1 Substates, right?
> > > > > > > >
> > > > > > > Yes.
> > > > > > >
> > > > > > > > > 3.If support L1SS, set 0xFF03 to free clkreq.
> > > > > > > > > 4.When entering idle status, enable aspm
> > > > > > > > >   and set parameters for L1SS and LTR.
> > > > > > > > > 5.Wnen entering run status, disable aspm
> > > > > > > > >   and set parameters for L1SS and LTR.
> > > > > > > >
> > > > > > > > In general, drivers should not configure ASPM, L1SS, and
> > > > > > > > LTR themselves; the PCI core should do that.
> > > > > > > >
> > > > > > > > If a driver needs to tweak ASPM at run-time, it should use
> > > > > > > > interfaces exported by the PCI core to do so.
> > > > > > > >
> > > > > > > Which interface I can use to set ASPM? I use
> "pci_write_config_byte"
> > > now.
> > > > > >
> > > > > > What do you need to do?  include/linux/pci-aspm.h exports
> > > > > > pci_disable_link_state(), which is mainly used to avoid ASPM
> > > > > > states that have hardware errata.
> > > > > >
> > > > > I want to enable ASPM(L0 -> L1) and disable ASPM(L1 -> L0),
> > > > > which interface can I use?
> > > >
> > > > You can use pci_disable_link_state() to disable usage of L1.
> > > >
> > > > Currently there is no corresponding pci_enable_link_state().  What
> > > > if we added something like the following (untested)?  Would that
> > > > work for you?
> > >
> > > Hi Rui,
> > >
> > > Any thoughts on the patch below?
> >
> > I'm busy with other work, the patch seems ok, I will test it later.
> 
> Any idea when you might get back to this?
> 
> If you can't do it, I can try doing it myself, but of course, I don't know the details
> about the device errata and I wouldn't be able to test it.
> 
Of course, you can do it first.

> Bjorn
> 
> ------Please consider the environment before printing this e-mail.