Re: [PATCH 1/4] PCI/ASPM: Add API to supply LTR L1.2 threshold

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Oct 30, 2017 at 4:03 AM, Vidya Sagar <vidyas@xxxxxxxxxx> wrote:
> adds API for host controller drivers to specify LTR L1.2
> threshold value if it is different from the default value.
> weak implementation of the API is added to supply default
> value
>
> Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx>
> ---
>  drivers/pci/pcie/aspm.c  | 11 ++++++++---
>  include/linux/pci-aspm.h |  6 ++++++
>  2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 1dfa10cc566b..c6e8604796e5 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -436,6 +436,11 @@ static struct pci_dev *pci_function_0(struct pci_bus *linkbus)
>         return NULL;
>  }
>
> +u32 __weak pcie_aspm_get_ltr_l_1_2_threshold(void)
> +{
> +       return LTR_L1_2_THRESHOLD_BITS;
> +}
> +
>  /* Calculate L1.2 PM substate timing parameters */
>  static void aspm_calc_l1ss_info(struct pcie_link_state *link,
>                                 struct aspm_register_info *upreg,
> @@ -458,10 +463,10 @@ static void aspm_calc_l1ss_info(struct pcie_link_state *link,
>         else
>                 link->l1ss.ctl1 |= val2 << 8;
>         /*
> -        * We currently use LTR L1.2 threshold to be fixed constant picked from
> -        * Intel's coreboot.
> +        * Get LTR L1.2 threshold value specific to a platform if present
> +        * Otherwise, get a constant value picked from Intel's coreboot.
>          */
> -       link->l1ss.ctl1 |= LTR_L1_2_THRESHOLD_BITS;
> +       link->l1ss.ctl1 |= pcie_aspm_get_ltr_l_1_2_threshold();
>
>         /* Choose the greater of the two T_pwr_on */
>         val1 = (upreg->l1ss_cap >> 19) & 0x1F;
> diff --git a/include/linux/pci-aspm.h b/include/linux/pci-aspm.h
> index 207c561fb40e..7ffde0f57688 100644
> --- a/include/linux/pci-aspm.h
> +++ b/include/linux/pci-aspm.h
> @@ -30,6 +30,7 @@ void pcie_aspm_powersave_config_link(struct pci_dev *pdev);
>  void pci_disable_link_state(struct pci_dev *pdev, int state);
>  void pci_disable_link_state_locked(struct pci_dev *pdev, int state);
>  void pcie_no_aspm(void);
> +u32 pcie_aspm_get_ltr_l_1_2_threshold(void);
>  #else
>  static inline void pcie_aspm_init_link_state(struct pci_dev *pdev)
>  {
> @@ -49,6 +50,11 @@ static inline void pci_disable_link_state(struct pci_dev *pdev, int state)
>  static inline void pcie_no_aspm(void)
>  {
>  }
> +
> +static inline u32 pcie_aspm_get_ltr_l_1_2_threshold(void)
> +{
> +       return 0;
> +}

Do we really need this (bogus) function definition in
!defined(CONFIG_PCIEASPM) case? I believe the other bogus definitions
in that section are needed because functions outside of
CONFIG_PCIEASPM call it.

>  #endif
>
>  #ifdef CONFIG_PCIEASPM_DEBUG /* this depends on CONFIG_PCIEASPM */
> --
> 2.7.4
>



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux