On 19/01/2019 15:45, Andy Shevchenko wrote:
> On Tue, Jan 15, 2019 at 12:48:34PM +0000, Gustavo Pimentel wrote:
>> On 15/01/2019 05:43, Andy Shevchenko wrote:
>>> On Mon, Jan 14, 2019 at 11:38:02AM +0000, Gustavo Pimentel wrote:
>>>> On 11/01/2019 19:47, Andy Shevchenko wrote:
>>>>> On Fri, Jan 11, 2019 at 07:33:41PM +0100, Gustavo Pimentel wrote:
>
>>>>>> +static bool disable_msix;
>>>>>> +module_param(disable_msix, bool, 0644);
>>>>>> +MODULE_PARM_DESC(disable_msix, "Disable MSI-X interrupts");
>>>>>
>>>>> Why?!
>>>>> We are no allow new module parameters without very strong arguments.
>>>>
>>>> Since this is a reference driver and might be used to test customized HW
>>>> solutions, I added this parameter to allow the possibility to test the solution
>>>> forcing the MSI feature binding. This is required specially if who will test
>>>> this solution has a Root Complex with both features available (MSI and MSI-X),
>>>> because the Kernel will give always preference to MSI-X binding (assuming that
>>>> the EP has also both features available).
>>>
>>> Yes, you may do it for testing purposes, but it doesn't fit the kernel standards.
>>
>> Ok, but how should I proceed? May I leave it or substitute by another way to do
>> it? If so, how?
>> As I said, the intended is to be only used for this test case, on normal
>> operation the parameter it should be always false.
>
> Keep out-of-tree patch for your needs.
Ok.
>
>>>>>> + if (!pdata) {
>>>>>> + dev_err(dev, "%s missing data structure\n", pci_name(pdev));
>>>>>> + return -EFAULT;
>>>>>> + }
>>>>>
>>>>> Useless check.
>>>>
>>>> Why? It's just a precaution, isn't it a good practice always to think of the
>>>> worst case?
>>>
>>> You just can put an implicit requirement of pdata rather than doing this
>>
>> Ok, how can I do it? What I should add to the code to force that?
>
> Not adding, removing. That's what I put before.
I thought there was some API or code design to force that. Sorry my bad.
>
>>
>>> useless check. I don't believe it would make sense to have NULL pdata for the
>>> driver since it wouldn't be functional anyhow.
>>
>> Yes, you're right without pdata the driver can't do anything.
>
