lkml.org 
[lkml]   [2016]   [Apr]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] PCI: designware: move remaining rc setup code to dw_pcie_setup_rc()
    On Tue, Apr 12, 2016 at 09:43:32AM +0000, Gabriele Paoloni wrote:
    > Hi Bjorn
    >
    > [...]
    >
    > > > >
    > > > > What's the hisi plan for resuming after suspend-to-RAM? How does
    > > the
    > > > > RC get reprogrammed after it loses all its state?
    > > >
    > > > PM is not part of the driver yet. This is planned for near
    > > > future release so haven't made such considerations yet
    > > > >
    > > > > What would break if hisi did call dw_pcie_setup_rc()? I know you
    > > said
    > > > > it would overwrite what the bootloader already did, which is true.
    > > >
    > > > I am try to figure this out now with our HW team.
    > > >
    > > > >
    > > > > But hisi does call dw_pcie_host_init(), so it reads pp->mem (which
    > > > > determines pp->mem_base) and pp->lanes from the DT. Other drivers
    > > > > then call dw_pcie_setup_rc() which programs the RC based on
    > > > > pp->mem_base and pp->lanes. So hisi assumes UEFI programmed the RC
    > > to
    > > > > match the DT, while the other drivers read the DT and program the
    > > RC
    > > > > to match. The latter seems more robust because it enforces the
    > > > > consistency rather than relying on it.
    > > >
    > > > Yes I agree with you, however we have preferred to move RC config to
    > > > BIOS to have a single driver to support multiple versions of the
    > > > same SoC.
    > >
    > > I think there are two reasonable approaches:
    > >
    > > 1) A single generic driver that doesn't have any knowledge about the
    > > chipset registers; it uses run-time firmware interfaces to manage
    > > the bridge. The ACPI pci_root.c driver is the best example so far
    > > and works very well. It supports basically all x86 and ia64
    > > chipsets and requires no kernel work for new ones.
    > >
    > > 2) Native drivers specific to each chipset. These may get
    > > configuration information from DT, but they do their own
    > > register-level programming of the device without run-time help from
    > > firmware.
    > >
    > > I think hisi is a native driver because it uses hip05/hip06 registers
    > > to check link state and perform config operations. And apparently you
    > > rely on the ATU, BAR, class, and link width programming currently done
    > > in dw_pcie_host_init(). But you want to rely on pre-boot firmware to
    > > set up the link. That doesn't make sense to me -- if the driver wants
    > > to twiddle the registers, it should know how to do it all. I don't
    > > see how you can reasonably manage this half-way approach.
    > >
    > > > The patch I proposed above does the same job as the original patch
    > > > proposed by Jisheng and also allows hisi driver to call the moved
    > > > code.
    > > >
    > > > Do you see anything wrong with it?
    > >
    > > Only that it makes the structure more complicated and we haven't
    > > identified a corresponding benefit yet.
    >
    > Finally I have checked that assigning .host_init function pointer
    > in our driver to call dw_pcie_setup_rc() will not affect the values
    > already set by BIOS.
    >
    > Also I agree with you that a hybrid approach is not ideal.
    >
    > So I will update the driver to call dw_pcie_setup_rc() from
    > .host_init and ask the BIOS team to update the firmware for next
    > releases (the driver will be backward compatible anyway).

    Am I right in assuming that the patch currently in my tree:

    https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-designware&id=1488aefa37a4033080942c860294d13c613ec829

    will work for you? I'm going to assume so unless I hear otherwise.

    Bjorn

    \
     
     \ /
      Last update: 2016-04-21 18:01    [W:3.018 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site