On 2020/12/07 10:26, Bjorn Helgaas wrote:
> On Sun, Dec 06, 2020 at 11:08:14PM +0000, Chaitanya Kulkarni wrote:
>> On 12/6/20 11:45, Puranjay Mohan wrote:
>>> Callers of pci_find_capability should save the return value in u8.
>>> change type of variables from int to u8 to match the specification.
>>
>> I did not understand this, pci_find_capability() does not return u8.
>>
>> what is it that we are achieving by changing the variable type ?
>>
>> This patch will probably also generate type mismatch warning with
>>
>> certain static analyzers.
>
> There's a patch pending via the PCI tree to change the return type to
> u8. We can do one of:
>
> - Ignore this. It only changes something on the stack, so no real
> space saving and there's no problem assigning the u8 return value
> to the "int".
>
> - The maintainer could ack it and I could merge it via the PCI tree
> so it happens in the correct order (after the interface change).
That works for me. But this driver changes generally go through Jens block tree.
Jens,
Is this OK with you if Bjorn takes the patch through the PCI tree ?
>
> - The PCI core interface change will be merged for v5.11, so we
> could hold this until v5.12.
>
> I don't really have a preference. The only place there would really
> be a benefit would be if we store the return value in a struct, where
> we could potentially save three bytes.
>
> Bjorn
>
>>> Signed-off-by: Puranjay Mohan <puranjay12@xxxxxxxxx>
>>> ---
>>> drivers/block/mtip32xx/mtip32xx.c | 2 +-
>>> drivers/block/skd_main.c | 2 +-
>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
>>> index 153e2cdecb4d..da57d37c6d20 100644
>>> --- a/drivers/block/mtip32xx/mtip32xx.c
>>> +++ b/drivers/block/mtip32xx/mtip32xx.c
>>> @@ -3936,7 +3936,7 @@ static DEFINE_HANDLER(7);
>>>
>>> static void mtip_disable_link_opts(struct driver_data *dd, struct pci_dev *pdev)
>>> {
>>> - int pos;
>>> + u8 pos;
>>> unsigned short pcie_dev_ctrl;
>>>
>>> pos = pci_find_capability(pdev, PCI_CAP_ID_EXP);
>>> diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
>>> index a962b4551bed..16d59569129b 100644
>>> --- a/drivers/block/skd_main.c
>>> +++ b/drivers/block/skd_main.c
>>> @@ -3136,7 +3136,7 @@ MODULE_DEVICE_TABLE(pci, skd_pci_tbl);
>>>
>>> static char *skd_pci_info(struct skd_device *skdev, char *str)
>>> {
>>> - int pcie_reg;
>>> + u8 pcie_reg;
>>>
>>> strcpy(str, "PCIe (");
>>> pcie_reg = pci_find_capability(skdev->pdev, PCI_CAP_ID_EXP);
>>
>>
>
--
Damien Le Moal
Western Digital Research
