Re: [RFC v3 5/7] dmaengine: Add Synopsys eDMA IP PCIe glue-logic

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

 



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.

> >>>> +	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.

> 
> > 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.

-- 
With Best Regards,
Andy Shevchenko





[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