| Message ID | 1516030541-6626-8-git-send-email-talgi@mellanox.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Report PCI device link status | expand |
On Mon, Jan 15, 2018 at 05:35:41PM +0200, Tal Gilboa wrote: > Newly introduced pcie_get_link_limits() function calculates > maximum available BW through the PCI chain. We can use this > value for mlx5e_get_pci_bw() instead of calculating ourselves. > > Signed-off-by: Tal Gilboa <talgi@mellanox.com> > Reviewed-by: Tariq Toukan <tariqt@mellanox.com> > --- > drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 17 +++-------------- > 1 file changed, 3 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c > index d2b057a..d1d7427 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c > @@ -3971,27 +3971,16 @@ static int mlx5e_get_pci_bw(struct mlx5_core_dev *mdev, u32 *pci_bw) > enum pcie_link_width width; > enum pci_bus_speed speed; > int err = 0; > + int bw; > > - err = pcie_get_minimum_link(mdev->pdev, &speed, &width); > + err = pcie_get_link_limits(mdev->pdev, &speed, &width, &bw); Why don't you refactor this a little so it calls pcie_print_link_status() like mlx5 and mlx4? I think this is the only call of pcie_get_link_limits() outside drivers/pci, and it seems like a little overkill here since you really don't need "speed" here. But I don't understand what's going on with mlx5e_get_max_linkspeed(). It doesn't look like that is actually looking at the PCIe link speed, so maybe there's something special going on there. > if (err) > return err; > > if (speed == PCI_SPEED_UNKNOWN || width == PCIE_LNK_WIDTH_UNKNOWN) > return -EINVAL; > > - switch (speed) { > - case PCIE_SPEED_2_5GT: > - *pci_bw = 2500 * width; > - break; > - case PCIE_SPEED_5_0GT: > - *pci_bw = 5000 * width; > - break; > - case PCIE_SPEED_8_0GT: > - *pci_bw = 8000 * width; > - break; > - default: > - return -EINVAL; > - } > + *pci_bw = bw; > > return 0; > } > -- > 1.8.3.1 >
On 2/22/2018 10:09 PM, Bjorn Helgaas wrote: > On Mon, Jan 15, 2018 at 05:35:41PM +0200, Tal Gilboa wrote: >> Newly introduced pcie_get_link_limits() function calculates >> maximum available BW through the PCI chain. We can use this >> value for mlx5e_get_pci_bw() instead of calculating ourselves. >> >> Signed-off-by: Tal Gilboa <talgi@mellanox.com> >> Reviewed-by: Tariq Toukan <tariqt@mellanox.com> >> --- >> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 17 +++-------------- >> 1 file changed, 3 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c >> index d2b057a..d1d7427 100644 >> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c >> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c >> @@ -3971,27 +3971,16 @@ static int mlx5e_get_pci_bw(struct mlx5_core_dev *mdev, u32 *pci_bw) >> enum pcie_link_width width; >> enum pci_bus_speed speed; >> int err = 0; >> + int bw; >> >> - err = pcie_get_minimum_link(mdev->pdev, &speed, &width); >> + err = pcie_get_link_limits(mdev->pdev, &speed, &width, &bw); > > Why don't you refactor this a little so it calls > pcie_print_link_status() like mlx5 and mlx4? > > I think this is the only call of pcie_get_link_limits() outside > drivers/pci, and it seems like a little overkill here since you really > don't need "speed" here. I wouldn't like to call pcie_print_link_status() if I don't really want to print the status. In this case mlx5e driver needs the PCI bandwidth limitation in order to make a decision. I think it is good that we expose a method to do precisely that. > > But I don't understand what's going on with mlx5e_get_max_linkspeed(). > It doesn't look like that is actually looking at the PCIe link speed, > so maybe there's something special going on there. mlx5e_get_max_linkspeed() queries for the port wire speed. mlx5e driver might change some settings (CQE zipping, LRO) in case PCI bandwidth can't support wire speed. > >> if (err) >> return err; >> >> if (speed == PCI_SPEED_UNKNOWN || width == PCIE_LNK_WIDTH_UNKNOWN) >> return -EINVAL; >> >> - switch (speed) { >> - case PCIE_SPEED_2_5GT: >> - *pci_bw = 2500 * width; >> - break; >> - case PCIE_SPEED_5_0GT: >> - *pci_bw = 5000 * width; >> - break; >> - case PCIE_SPEED_8_0GT: >> - *pci_bw = 8000 * width; >> - break; >> - default: >> - return -EINVAL; >> - } >> + *pci_bw = bw; >> >> return 0; >> } >> -- >> 1.8.3.1 >>
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c index d2b057a..d1d7427 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c @@ -3971,27 +3971,16 @@ static int mlx5e_get_pci_bw(struct mlx5_core_dev *mdev, u32 *pci_bw) enum pcie_link_width width; enum pci_bus_speed speed; int err = 0; + int bw; - err = pcie_get_minimum_link(mdev->pdev, &speed, &width); + err = pcie_get_link_limits(mdev->pdev, &speed, &width, &bw); if (err) return err; if (speed == PCI_SPEED_UNKNOWN || width == PCIE_LNK_WIDTH_UNKNOWN) return -EINVAL; - switch (speed) { - case PCIE_SPEED_2_5GT: - *pci_bw = 2500 * width; - break; - case PCIE_SPEED_5_0GT: - *pci_bw = 5000 * width; - break; - case PCIE_SPEED_8_0GT: - *pci_bw = 8000 * width; - break; - default: - return -EINVAL; - } + *pci_bw = bw; return 0; }