On Mon, Dec 7, 2020 at 11:37 PM Keith Busch <kbusch@xxxxxxxxxx> wrote: > > On Mon, Dec 07, 2020 at 11:23:03PM +0530, Puranjay Mohan wrote: > > On Mon, Dec 7, 2020 at 8:23 PM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > > > > > > Can we take a step back? I think in general drivers should not bother > > > with pci_find_capability. Both mtip32xx and skd want to find out if > > > the devices are PCIe devices, skd then just prints the link speed for > > > which we have much better helpers, mtip32xx than messes with DEVCTL > > > which seems actively dangerous to me. > > > > The skd driver uses pci_find_capability(skdev->pdev, PCI_CAP_ID_EXP); > > to check if the device is PCIe, it can use pci_is_pcie() to do that. > > After that it uses pci_read_config_word(skdev->pdev, pcie_reg, > > &pcie_lstat); to read the Link Status Register, I think > > it should use pcie_capability_read_word(skdev->pdev, PCI_EXP_LNKSTA, > > &pcie_lstat); > > > > Please let me know if the above changes are correct so I may send a patch. > > You just want to print the current link speed, right? Does > skdev->pdev->bus->cur_bus_speed provide what you need? After discussing with Bjorn, we concluded that we don't need to print the speed as it is already printed by the PCI core for every device. PCI core calls __pcie_print_link_status() for every device, and hence the skd_pci_info() is redundant and can be removed? -- Thanks and Regards Yours Truly, Puranjay Mohan
