Às 10:43 AM de 1/17/2017, Joao Pinto escreveu: > > Hi Lukasz, > > Às 9:44 PM de 1/16/2017, Lukasz Majewski escreveu: >> Hi Joao, >> >>> >>> Hi, >>> >>> Às 10:13 AM de 1/16/2017, Kishon Vijay Abraham I escreveu: >>>> + Joao, Jingoo >>>> >>>> Hi, >>>> >>>> On Monday 16 January 2017 03:01 PM, Lukasz Majewski wrote: >>>>> Hi Kishon, >>>>> >>>>>> Hi Łukasz, >>>>>> >>>>>> On Monday 16 January 2017 12:19 PM, Lukasz Majewski wrote: >>>>>>> Hi Kishon, >>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> On Sunday 15 January 2017 06:49 PM, Lukasz Majewski wrote: >>>>>>>>> Some devices (due to e.g. bad PCIe signal integrity) require to >>>>>>>>> run with forced GEN1 speed on PCIe bus. >>>>>>>>> >>>>>>>>> This patch changes the speed explicitly on dra7 based devices >>>>>>>>> when proper device tree attribute is defined for the PCIe >>>>>>>>> controller. >>>>>>>>> >>>>>>>>> Signed-off-by: Lukasz Majewski <lukma@xxxxxxx> >>>>>>>> >>>>>>>> Bjorn has already queued a patch to do the same thing >>>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_cgit_linux_kernel_git_helgaas_pci.git_log_-3Fh-3Dpci_host-2Ddra7xx&d=DwIDaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=zD82T5n4WcL7Ga-NSY2NI7KE75xQ99hN-mW2yX46wQk&s=E8zk1CbKxGH-f3fw_WpXxFU-A8BLkgA8NusCaxk1SvA&e= >>>>>>> >>>>>>> It seems like Bjorn only modifies CAP registers. >>>>>> >>>>>> The patch also modifies the LNKCTL2 register. >>>>>>> >>>>>>> He also needs to change register with 0x080C offset to actually >>>>>>> ( PCIECTRL_PL_WIDTH_SPEED_CTL ) >>>>>> >>>>>> This bit is used to initiate speed change (after the link is >>>>>> initialized in GEN1). Resetting the bit (like what you have done >>>>>> here) prevents speed change. >>>>> >>>>> This is strange, but e2e advised me to do things as I did in the >>>>> patch to _force_ GEN1 operation on PCIe2 port [1] (AM5728) >>>>> >>>>> Link: >>>>> [1] >>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__e2e.ti.com_support_arm_sitara-5Farm_f_791_t_566421&d=DwIDaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=zD82T5n4WcL7Ga-NSY2NI7KE75xQ99hN-mW2yX46wQk&s=uXLwglyRYqKpwp1JSxkOWmKpQ2wjfhgofpm8DCfquNw&e= >>>>> >>>>> Both patches modify 0x5180 007C register to set GEN1 capability >>>>> (PCI_EXP_LNKCAP_SLS_2_5GB) >>>>> >>>>> The problem is with second register (in your patch): >>>>> >>>>> From SPRUHZ6G TRM: >>>>> >>>>> PCIECTRL_EP_DBICS_LNK_CAS_2 (0x5180 00A0) >>>>> - TRGT_LINK_SPEED (Reset 0x1) - "Target Link Speed" - no more >>>>> description in TRM >>>>> >>>>> It is set to PCI_EXP_LNKCAP_SLS_2_5GB = 0x1, which is the same as >>>>> default /reset value. >>>> >>>> The default value is 0x2 (or else none of the cards would have >>>> enumerated in GEN2) >>>>> >>>>> >>>>> Could you clarify which way to _force_ PCIe GEN1 operation is >>>>> correct? Mine shows differences in lspci output (as posted in [1]). >>>> >>>> You'll see the difference even with the patch in Bjorn's tree ;-) >>>> >>>> I think these are 2 different approaches to keep the link at GEN1. >>>> Joao or Jingoo, do you have any suggestion here? >>> >>> I studied the Databook, >> >> Could you reveal which databook do you have in mind? Is that the TRM for >> AM5728? > > I checked the Designware PCIe Databook, since it is based on this IP. > >> >>> and both approaches seem to be right, >>> dependently of the Core configuration and setup. >>> >>> The standard manual speed change sequence is: >>> a) Write to PCIE_CAP_TARGET_LINK_SPEED (indicating desired speed) >> >> Do you mean TRGT_LINK_SPEED @ 0x5180 00A0 ? > > Correct. > >> >>> b) Clear "Directed Speed Change" >> >> CFG_DIRECTED_SPEED_CHANGE @ 0x5180 080C > > Correct. > >> >>> c) Set "Directed Speed Change" >>> >>> If "Directed Speed Change" is set (DEFAULT_GEN2_SPEED_CHANGE is the >>> default value), it will execute LTSSM to initiate speed change to >>> Gen2 or Gen3, after link is started in Gen1, and then the bit is >>> automatically cleared. >> >> Ok, so with default settings (after reset) we do have Gen1 speed link >> and when we enable LTSSM (with LTSSM_EN bit setting) we negotiate to >> Gen2/Gen3. > > Yes, that's the expected behavior. I submited this direct question to R&D and > will have your doubt answered soon. According to R&D if you set "Target Link Speed" to Gen1 before setting LTSSM_EN bit, the controller should stay in GEN1. > >> >>> >>> Lukasz is reseting this bit, in order to avoid the LTSSM to be >>> executed, which is correct. >> >> So with CFG_DIRECTED_SPEED_CHANGE = 0, when I start LTSSM (with >> LTSSM_EN) the state machine returns immediately and leaves link in the >> Gen1? >> >> The pci-dra7 driver always sets LTSSM_EN bit to start link negotiation. >> >>> There is another way to prevent this >>> automatic speed change, which is to set GEN1 speed before link up >>> which might be difficult in some setups, so Kishon's also right. >>> >>> In my opinion Lukasz approach would be the one that might be more >>> universal and more "secure". >> >> The robustness is the key here since there are some devices, which on >> particular HW must only work with Gen1 speed. When we start LTSSM state >> machine and hence start negotiation to Gen2, not always the result of >> LTSSM is correct and device is properly recognized. >> >>> >>> Joao >>> >>> >>>> >>>>> >>>>>> >>>>>> IMO the better way is to set the LNKCTL2 to GEN1 instead of >>>>>> hacking the IP register. >>>>> >>>>> From the original patch description: >>>>> >>>>> "Add support to force Root Complex to work in GEN1 mode if so >>>>> desired, but don't force GEN1 mode on any board just yet." >>>>> >>>>> Are there any (floating around) patches allowing forcing GEN1 >>>>> operation on any board (I would like to reuse/port them to my >>>>> current solution)? >>>> >>>> For setting to GEN1 mode, "max-link-speed" should be set to 1 in dt >>>> with the patch in Bjorn's tree. >>>> >>>> Thanks >>>> Kishon >>>> >>> >> >> Best regards, >> >> Lukasz Majewski >> >> -- >> >> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk >> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany >> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@xxxxxxx >> > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
