From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [RFC,v3,5/7] dmaengine: Add Synopsys eDMA IP PCIe glue-logic From: Andy Shevchenko Message-Id: <20190115054358.GB9170@smile.fi.intel.com> Date: Tue, 15 Jan 2019 07:43:58 +0200 To: Gustavo Pimentel Cc: "linux-pci@vger.kernel.org" , "dmaengine@vger.kernel.org" , Vinod Koul , Dan Williams , Eugeniy Paltsev , Russell King , Niklas Cassel , Lorenzo Pieralisi , Joao Pinto , Jose Abreu , Luis Oliveira , Vitor Soares , Nelson Costa , Pedro Sousa List-ID: T24gTW9uLCBKYW4gMTQsIDIwMTkgYXQgMTE6Mzg6MDJBTSArMDAwMCwgR3VzdGF2byBQaW1lbnRl bCB3cm90ZToKPiBPbiAxMS8wMS8yMDE5IDE5OjQ3LCBBbmR5IFNoZXZjaGVua28gd3JvdGU6Cj4g PiBPbiBGcmksIEphbiAxMSwgMjAxOSBhdCAwNzozMzo0MVBNICswMTAwLCBHdXN0YXZvIFBpbWVu dGVsIHdyb3RlOgoKPiA+PiArc3RhdGljIGJvb2wgZGlzYWJsZV9tc2l4Owo+ID4+ICttb2R1bGVf cGFyYW0oZGlzYWJsZV9tc2l4LCBib29sLCAwNjQ0KTsKPiA+PiArTU9EVUxFX1BBUk1fREVTQyhk aXNhYmxlX21zaXgsICJEaXNhYmxlIE1TSS1YIGludGVycnVwdHMiKTsKPiA+IAo+ID4gV2h5PyEK PiA+IFdlIGFyZSBubyBhbGxvdyBuZXcgbW9kdWxlIHBhcmFtZXRlcnMgd2l0aG91dCB2ZXJ5IHN0 cm9uZyBhcmd1bWVudHMuCj4gCj4gU2luY2UgdGhpcyBpcyBhIHJlZmVyZW5jZSBkcml2ZXIgYW5k IG1pZ2h0IGJlIHVzZWQgdG8gdGVzdCBjdXN0b21pemVkIEhXCj4gc29sdXRpb25zLCBJIGFkZGVk IHRoaXMgcGFyYW1ldGVyIHRvIGFsbG93IHRoZSBwb3NzaWJpbGl0eSB0byB0ZXN0IHRoZSBzb2x1 dGlvbgo+IGZvcmNpbmcgdGhlIE1TSSBmZWF0dXJlIGJpbmRpbmcuIFRoaXMgaXMgcmVxdWlyZWQg c3BlY2lhbGx5IGlmIHdobyB3aWxsIHRlc3QKPiB0aGlzIHNvbHV0aW9uIGhhcyBhIFJvb3QgQ29t cGxleCB3aXRoIGJvdGggZmVhdHVyZXMgYXZhaWxhYmxlIChNU0kgYW5kIE1TSS1YKSwKPiBiZWNh dXNlIHRoZSBLZXJuZWwgd2lsbCBnaXZlIGFsd2F5cyBwcmVmZXJlbmNlIHRvIE1TSS1YIGJpbmRp bmcgKGFzc3VtaW5nIHRoYXQKPiB0aGUgRVAgaGFzIGFsc28gYm90aCBmZWF0dXJlcyBhdmFpbGFi bGUpLgoKWWVzLCB5b3UgbWF5IGRvIGl0IGZvciB0ZXN0aW5nIHB1cnBvc2VzLCBidXQgaXQgZG9l c24ndCBmaXQgdGhlIGtlcm5lbCBzdGFuZGFyZHMuCgo+ID4+ICsJaWYgKCFwZGF0YSkgewo+ID4+ ICsJCWRldl9lcnIoZGV2LCAiJXMgbWlzc2luZyBkYXRhIHN0cnVjdHVyZVxuIiwgcGNpX25hbWUo cGRldikpOwo+ID4+ICsJCXJldHVybiAtRUZBVUxUOwo+ID4+ICsJfQo+ID4gCj4gPiBVc2VsZXNz IGNoZWNrLgo+IAo+IFdoeT8gSXQncyBqdXN0IGEgcHJlY2F1dGlvbiwgaXNuJ3QgaXQgYSBnb29k IHByYWN0aWNlIGFsd2F5cyB0byB0aGluayBvZiB0aGUKPiB3b3JzdCBjYXNlPwoKWW91IGp1c3Qg Y2FuIHB1dCBhbiBpbXBsaWNpdCByZXF1aXJlbWVudCBvZiBwZGF0YSByYXRoZXIgdGhhbiBkb2lu ZyB0aGlzCnVzZWxlc3MgY2hlY2suIEkgZG9uJ3QgYmVsaWV2ZSBpdCB3b3VsZCBtYWtlIHNlbnNl IHRvIGhhdmUgTlVMTCBwZGF0YSBmb3IgdGhlCmRyaXZlciBzaW5jZSBpdCB3b3VsZG4ndCBiZSBm dW5jdGlvbmFsIGFueWhvdy4KCj4gPj4gKwkvKiBNYXBwaW5nIFBDSSBCQVIgcmVnaW9ucyAqLwo+ ID4+ICsJZXJyID0gcGNpbV9pb21hcF9yZWdpb25zKHBkZXYsIEJJVChwZGF0YS0+cmdfYmFyKSB8 Cj4gPj4gKwkJCQkgICAgICAgQklUKHBkYXRhLT5sbF9iYXIpIHwKPiA+PiArCQkJCSAgICAgICBC SVQocGRhdGEtPmR0X2JhciksCj4gPj4gKwkJCQkgcGNpX25hbWUocGRldikpOwo+ID4+ICsJaWYg KGVycikgewoKPiA+PiArCQlyZXR1cm4gZXJyOwo+ID4+ICsJfQoKPiA+PiArCWlmICghcGNpbV9p b21hcF90YWJsZShwZGV2KSkKPiA+PiArCQlyZXR1cm4gLUVBQ0NFUzsKPiA+IAo+ID4gTmV2ZXIg aGFwcGVuIGNvbmRpdGlvbi4gVGh1cyB1c2VsZXNzLgo+IAo+IHBjaW1faW9tYXBfdGFibGUoKSBj YW4gcmV0dXJuIE5VTEwgaW4gY2FzZSBvZiBhbGxvY2F0aW9uIGZhaWx1cmUuIEJlc2lkZXMgdGhh dCwKPiBpc24ndCBpdCBhIGdvb2QgcHJhY3RpY2UgYWx3YXlzIHRvIHRoaW5rIG9mIHRoZSB3b3Jz dCBjYXNlPwoKTm8sIGl0IGNhbid0IGluIHRoZSBjb25kaXRpb25zIHlvdXIgaGF2ZSBpbiB0aGUg Y29kZS4gU2VlIGFib3ZlIHRoZSBsaW5lcyBJIGxlZnQuCklmIHBjaW1faW9tYXBfcmVnaW9ucygp IHN1Y2Nlc3NmdWxseSBmaW5pc2hlZC4uLgoKPiA+PiArCWRldl9pbmZvKGRldiwgIkRlc2lnbldh cmUgZURNQSBQQ0llIGRyaXZlciBsb2FkZWQgY29tcGxldGVseVxuIik7Cj4gPiAKPiA+IFVzZWxl c3MuCj4gCj4gSXQncyBoZWxwZnVsIGZvciBicmluZyB1cCwgSSBjYW4gcGFzcyBpdCB0byBkYmcu CgpJdCBqdXN0IHNob3dzIHRoYXQgc29tZW9uZSBkaWRuJ3QgdXNlIGV4aXN0aW5nIHRvb2xzIGFu ZCBmZWF0dXJlcy4gVGhpcyBtZXNzYWdlIGFuZCBzaW1pbGFyIGFyZSB1c2VsZXNzLgpIaW50OiBp bml0Y2FsbF9kZWJ1Zy4KCj4gPj4gKwlkZXZfaW5mbyhkZXYsICJEZXNpZ25XYXJlIGVETUEgUENJ ZSBkcml2ZXIgdW5sb2FkZWQgY29tcGxldGVseVxuIik7Cj4gPiAKPiA+IERpdHRvLgo+IAo+IEl0 J3MgaGVscGZ1bCBmb3IgYnJpbmcgdXAsIEkgY2FuIHBhc3MgaXQgdG8gZGJnLgoKRGl0dG8uCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 46FCEC43387 for ; Tue, 15 Jan 2019 05:44:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1E29020656 for ; Tue, 15 Jan 2019 05:44:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727792AbfAOFoF (ORCPT ); Tue, 15 Jan 2019 00:44:05 -0500 Received: from mga06.intel.com ([134.134.136.31]:36166 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727211AbfAOFoF (ORCPT ); Tue, 15 Jan 2019 00:44:05 -0500 X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Jan 2019 21:44:04 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,480,1539673200"; d="scan'208";a="108303389" Received: from smile.fi.intel.com (HELO smile) ([10.237.72.86]) by orsmga006.jf.intel.com with ESMTP; 14 Jan 2019 21:44:00 -0800 Received: from andy by smile with local (Exim 4.92-RC4) (envelope-from ) id 1gjHW3-0000d6-07; Tue, 15 Jan 2019 07:43:59 +0200 Date: Tue, 15 Jan 2019 07:43:58 +0200 From: Andy Shevchenko To: Gustavo Pimentel Cc: "linux-pci@vger.kernel.org" , "dmaengine@vger.kernel.org" , Vinod Koul , Dan Williams , Eugeniy Paltsev , Russell King , Niklas Cassel , Lorenzo Pieralisi , Joao Pinto , Jose Abreu , Luis Oliveira , Vitor Soares , Nelson Costa , Pedro Sousa Subject: Re: [RFC v3 5/7] dmaengine: Add Synopsys eDMA IP PCIe glue-logic Message-ID: <20190115054358.GB9170@smile.fi.intel.com> References: <20190111194705.GU9170@smile.fi.intel.com> <916f319b-9e45-6d78-4ecc-850feda84bb3@synopsys.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <916f319b-9e45-6d78-4ecc-850feda84bb3@synopsys.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org 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. > >> + 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 useless check. I don't believe it would make sense to have NULL pdata for the driver since it wouldn't be functional anyhow. > >> + /* Mapping PCI BAR regions */ > >> + err = pcim_iomap_regions(pdev, BIT(pdata->rg_bar) | > >> + BIT(pdata->ll_bar) | > >> + BIT(pdata->dt_bar), > >> + pci_name(pdev)); > >> + if (err) { > >> + return err; > >> + } > >> + if (!pcim_iomap_table(pdev)) > >> + return -EACCES; > > > > Never happen condition. Thus useless. > > pcim_iomap_table() can return NULL in case of allocation failure. Besides that, > isn't it a good practice always to think of the worst case? No, it can't in the conditions your have in the code. See above the lines I left. If pcim_iomap_regions() successfully finished... > >> + dev_info(dev, "DesignWare eDMA PCIe driver loaded completely\n"); > > > > Useless. > > It's helpful for bring up, I can pass it to dbg. It just shows that someone didn't use existing tools and features. This message and similar are useless. Hint: initcall_debug. > >> + dev_info(dev, "DesignWare eDMA PCIe driver unloaded completely\n"); > > > > Ditto. > > It's helpful for bring up, I can pass it to dbg. Ditto. -- With Best Regards, Andy Shevchenko