Re: [PATCH 09/30] drm/i915: deprecate pci_get_bus_and_slot()

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

 



On Wed, 22 Nov 2017, Sinan Kaya <okaya@xxxxxxxxxxxxxx> wrote:
> On 11/22/2017 2:52 AM, Joonas Lahtinen wrote:
>> Dropping the extra mailing lists and Dave, this is a rather trivial thing.
>> 
>> On Wed, 2017-11-22 at 00:30 -0500, Sinan Kaya wrote:
>>> pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as
>>> where a PCI device is present. This restricts the device drivers to be
>>> reused for other domain numbers.
>>>
>>> Use pci_get_domain_bus_and_slot() with a domain number of 0 where we can't
>>> extract the domain number. Other places, use the actual domain number from
>>> the device.
>>>
>>> Signed-off-by: Sinan Kaya <okaya@xxxxxxxxxxxxxx>
>> 
>> <SNIP>
>> 
>>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>>> @@ -419,7 +419,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
>>>  
>>>  static int i915_get_bridge_dev(struct drm_i915_private *dev_priv)
>>>  {
>>> -	dev_priv->bridge_dev = pci_get_bus_and_slot(0, PCI_DEVFN(0, 0));
>>> +	uint16_t devfn = PCI_DEVFN(0, 0)
>> 
>> The would have to be "unsigned int" according to the function
>> signature.
>
> Even though the return value is unsigned int, PCI_DEVFN cannot be bigger
> than 16 bits. 
>
> http://elixir.free-electrons.com/linux/v4.14.1/source/include/uapi/linux/pci.h#L31
>
> /*
>  * The PCI interface treats multi-function devices as independent
>  * devices.  The slot/function address of each device is encoded
>  * in a single byte as follows:
>  *
>  *	7:3 = slot
>  *	2:0 = function
>  */
>
> It is common practice to use u16 for keeping devfn information in the
> kernel.
>
>> 
>>> +
>>> +	dev_priv->bridge_dev = pci_get_domain_bus_and_slot(0, 0, devfn);
>> 
>> But the most straightforward change is to simply convert to:
>> 
>> 	dev_priv->bridge_dev = pci_get_domain_bus_and_slot(0, 0,
>> 							   PCI_DEVFN(0, 0));
>> 
>> Can you please resend like that.
>
> I did this at the beginning and but, I hit a checkpatch problem with
> more than 80 characters. That's why, I moved the devfn value assignment
> to a different line.

Please ignore checkpatch when it makes the code worse. 80 is not a
strict limit.

BR,
Jani.


>
>> 
>> Looks like this is a part of abigger series, so others may prefer
>> latter form too, to avoid the variable.
>> 
>> Regards, Joonas
>> 

-- 
Jani Nikula, Intel Open Source Technology Center



[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