linux-kernel - Re: [PATCH v14 3/4] PCI: Bring the PCIe speed to MBps logic to new pcie_link_speed_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <15693666-edef-60c9-80e1-c74db00e07aa@quicinc.com>
Date: Wed, 19 Jun 2024 20:00:21 +0530
From: Krishna Chaitanya Chundru <quic_krichai@...cinc.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
CC: Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio
	<konrad.dybcio@...aro.org>,
        Rob Herring <robh@...nel.org>,
        "Krzysztof
 Kozlowski" <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley
	<conor+dt@...nel.org>,
        Manivannan Sadhasivam
	<manivannan.sadhasivam@...aro.org>,
        Lorenzo Pieralisi
	<lpieralisi@...nel.org>,
        Krzysztof Wilczyński
	<kw@...ux.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>, <johan+linaro@...nel.org>,
        <bmasney@...hat.com>, <djakov@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        <linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>, <linux-pci@...r.kernel.org>,
        <vireshk@...nel.org>, <quic_vbadigan@...cinc.com>,
        <quic_skananth@...cinc.com>, <quic_nitegupt@...cinc.com>,
        <quic_parass@...cinc.com>, <krzysztof.kozlowski@...aro.org>
Subject: Re: [PATCH v14 3/4] PCI: Bring the PCIe speed to MBps logic to new
 pcie_link_speed_to_mbps()



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@...cinc.com>
>> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
>> Acked-by: Bjorn Helgaas <bhelgaas@...gle.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.
> 
Ack I will modify in the next patch.
> 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)?
> 
I checked with and without inline and I don't see any difference in both
cases
aarch64-linux-gnu-size ../drivers/pci/pci.o
    text    data     bss     dec     hex filename
   41440    1334      64   42838    a756 ../kobj/drivers/pci/pci.o

   text    data     bss     dec     hex filename
   41440    1334      64   42838    a756 ../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 thinki
> 
>>   }
>>   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;
>> +}
> 
> 
>