Re: [PATCH 3/3] PCI/ASPM: Remove unnecessary ASPM_STATE_L1SS check

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

 



On Tue, Apr 11, 2023 at 04:40:34PM +0530, Ajay Agarwal wrote:
> Currently the driver checks if ASPM_STATE_L1SS is supported
> before calling aspm_calc_l1ss_info(), only for this function to
> return if ASPM_STATE_L1_2_MASK is not supported. Simplify the
> logic by directly checking for L1.2 mask.
> 
> Signed-off-by: Ajay Agarwal <ajayagarwal@xxxxxxxxxx>
> ---
>  drivers/pci/pcie/aspm.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 7c9935f331f1..8c45835e8016 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -481,9 +481,6 @@ static void aspm_calc_l1ss_info(struct pcie_link_state *link,
>  	u32 pctl1, pctl2, cctl1, cctl2;
>  	u32 pl1_2_enables, cl1_2_enables;
>  
> -	if (!(link->aspm_support & ASPM_STATE_L1_2_MASK))
> -		return;
> -
>  	/* Choose the greater of the two Port Common_Mode_Restore_Times */
>  	val1 = (parent_l1ss_cap & PCI_L1SS_CAP_CM_RESTORE_TIME) >> 8;
>  	val2 = (child_l1ss_cap & PCI_L1SS_CAP_CM_RESTORE_TIME) >> 8;
> @@ -616,7 +613,7 @@ static void aspm_l1ss_init(struct pcie_link_state *link)
>  	if (parent_l1ss_ctl1 & child_l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_2)
>  		link->aspm_enabled |= ASPM_STATE_L1_2_PCIPM;
>  
> -	if (link->aspm_support & ASPM_STATE_L1SS)
> +	if (link->aspm_support & ASPM_STATE_L1_2_MASK)
>  		aspm_calc_l1ss_info(link, parent_l1ss_cap, child_l1ss_cap);

I think the reason it was this way is because several of the relevant
names use "l1ss":

  ASPM_STATE_L1SS
  aspm_calc_l1ss_info
  calc_l1ss_pwron

But everything in aspm_calc_l1ss_info() is L1.2-specific, so I think
it would make sense to use your patch, and at the same time, rename
aspm_calc_l1ss_info() and calc_l1ss_pwron() to aspm_calc_l12_info()
and calc_l12_pwron() to match.

Bjorn



[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