Re: [PATCH v14 3/4] PCI: Bring the PCIe speed to MBps logic to new pcie_link_speed_to_mbps()

[Krishna Chaitanya Chundru <quic_krichai@quicinc.com>]

On 6/14/2024 6:02 PM, Ilpo Järvinen wrote:
> On Sun, 9 Jun 2024, Krishna chaitanya chundru wrote:
> 
>> Bring the switch case in pcie_link_speed_mbps() to new function to
>> the header file so that it can be used in other places like
>> in controller driver.
>>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>> ---
>>   drivers/pci/pci.c | 19 +------------------
>>   drivers/pci/pci.h | 22 ++++++++++++++++++++++
>>   2 files changed, 23 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index d2c388761ba9..6e50fa89b913 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -6027,24 +6027,7 @@ int pcie_link_speed_mbps(struct pci_dev *pdev)
>>   	if (err)
>>   		return err;
>>   
>> -	switch (to_pcie_link_speed(lnksta)) {
>> -	case PCIE_SPEED_2_5GT:
>> -		return 2500;
>> -	case PCIE_SPEED_5_0GT:
>> -		return 5000;
>> -	case PCIE_SPEED_8_0GT:
>> -		return 8000;
>> -	case PCIE_SPEED_16_0GT:
>> -		return 16000;
>> -	case PCIE_SPEED_32_0GT:
>> -		return 32000;
>> -	case PCIE_SPEED_64_0GT:
>> -		return 64000;
>> -	default:
>> -		break;
>> -	}
>> -
>> -	return -EINVAL;
>> +	return pcie_link_speed_to_mbps(to_pcie_link_speed(lnksta));
> 
> pcie_link_speed_mbps() calls pcie_link_speed_to_mbps(), seems quite
> confusing to me. Perhaps renaming one to pcie_dev_speed_mbps() would help
> against the almost identical naming.
> 
> In general, I don't like moving that code into a header file, did you
> check how large footprint the new function is (when it's not inlined)?
> 
if we remove this patch we see difference of 8, I think it should be fine.
with change
aarch64-linux-gnu-size ../drivers/pci/pci.o
    text    data     bss     dec     hex filename
   41440    1334      64   42838    a756 ../kobj/drivers/pci/pci.o
without the change
text    data     bss     dec     hex filename
   41432    1334      64   42830    a74e ../kobj/drivers/pci/pci.o

- Krishna Chaitanya.
> Unrelated to this patch, it would be nice if LNKSTA register read would
> not be needed at all here but since cur_bus_speed is what it is currently,
> it's just wishful thinking.
> 
>>   }
>>   EXPORT_SYMBOL(pcie_link_speed_mbps);
>>   
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index 1b021579f26a..391a5cd388bd 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -333,6 +333,28 @@ void pci_bus_put(struct pci_bus *bus);
>>   	 (speed) == PCIE_SPEED_2_5GT  ?  2500*8/10 : \
>>   	 0)
>>   
>> +static inline int pcie_link_speed_to_mbps(enum pci_bus_speed speed)
>> +{
>> +	switch (speed) {
>> +	case PCIE_SPEED_2_5GT:
>> +		return 2500;
>> +	case PCIE_SPEED_5_0GT:
>> +		return 5000;
>> +	case PCIE_SPEED_8_0GT:
>> +		return 8000;
>> +	case PCIE_SPEED_16_0GT:
>> +		return 16000;
>> +	case PCIE_SPEED_32_0GT:
>> +		return 32000;
>> +	case PCIE_SPEED_64_0GT:
>> +		return 64000;
>> +	default:
>> +		break;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
> 
> 
>