From: Alan Douglas <adouglas@cadence.com>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>,
"bhelgaas@google.com" <bhelgaas@google.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"gustavo.pimentel@synopsys.com" <gustavo.pimentel@synopsys.com>,
"cyrille.pitchen@bootlin.com" <cyrille.pitchen@bootlin.com>,
Scott Telford <stelford@cadence.com>
Subject: RE: [PATCH v2 5/5] PCI: cadence: Add MSI-X capability to EP driver
Date: Mon, 17 Sep 2018 15:55:49 +0000 [thread overview]
Message-ID: <MWHPR07MB3502D6EF11136AF4D0EA03A9D81E0@MWHPR07MB3502.namprd07.prod.outlook.com> (raw)
In-Reply-To: <20180917152602.GI7239@e107981-ln.cambridge.arm.com>
SGkgTG9yZW56bywNCg0KT24gMTcgU2VwdGVtYmVyIDIwMTggMTY6MjYgTG9yZW56byBQaWVyYWxp
c2kgd3JvdGU6DQo+IE9uIFR1ZSwgU2VwIDA0LCAyMDE4IGF0IDA5OjMyOjU2QU0gKzAwMDAsIEFs
YW4gRG91Z2xhcyB3cm90ZToNCj4gPiBIaSwNCj4gPiBPbiAwNCBTZXB0ZW1iZXIgMjAxOCAwNTo0
NiwgS2lzaG9uIFZpamF5IEFicmFoYW0gSSB3cm90ZToNCj4gPiA+IE9uIFdlZG5lc2RheSAxNSBB
dWd1c3QgMjAxOCAwNzoxOSBQTSwgQWxhbiBEb3VnbGFzIHdyb3RlOg0KPiA+ID4gPiBBZGQgc2V0
X21zaXggYW5kIGdldF9tc2l4IGZ1bmN0aW9ucyB0byBkcml2ZXIsIGFuZCBoYW5kbGUNCj4gPiA+
ID4gUENJX0VQQ19JUlFfTVNJWCByZXF1ZXN0IGluIHJhaXNlX2lycS4gIEJBUjUgaXMgdXNlZCBm
b3INCj4gPiA+ID4gdGhlIE1TSS1YIHZlY3RvcnMuDQo+ID4gPiA+DQo+ID4gPiA+IFNpZ25lZC1v
ZmYtYnk6IEFsYW4gRG91Z2xhcyA8YWRvdWdsYXNAY2FkZW5jZS5jb20+DQo+ID4gPiA+IC0tLQ0K
PiA+ID4gPiAgIGRyaXZlcnMvcGNpL2NvbnRyb2xsZXIvcGNpZS1jYWRlbmNlLWVwLmMgfCAxMDcg
KysrKysrKysrKysrKysrKysrKysrKysrKysrKysrLQ0KPiA+ID4gPiAgIGRyaXZlcnMvcGNpL2Nv
bnRyb2xsZXIvcGNpZS1jYWRlbmNlLmggICAgfCAgIDEgKw0KPiA+ID4gPiAgIDIgZmlsZXMgY2hh
bmdlZCwgMTA3IGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24oLSkNCj4gPiA+ID4NCj4gPiA+ID4g
ZGlmZiAtLWdpdCBhL2RyaXZlcnMvcGNpL2NvbnRyb2xsZXIvcGNpZS1jYWRlbmNlLWVwLmMgYi9k
cml2ZXJzL3BjaS9jb250cm9sbGVyL3BjaWUtY2FkZW5jZS1lcC5jDQo+ID4gPiA+IGluZGV4IDEy
NDhkNzUuLjI1OWI5YTYgMTAwNjQ0DQo+ID4gPiA+IC0tLSBhL2RyaXZlcnMvcGNpL2NvbnRyb2xs
ZXIvcGNpZS1jYWRlbmNlLWVwLmMNCj4gPiA+ID4gKysrIGIvZHJpdmVycy9wY2kvY29udHJvbGxl
ci9wY2llLWNhZGVuY2UtZXAuYw0KPiA+ID4gPiBAQCAtMTYsNiArMTYsNyBAQA0KPiA+ID4gPiAg
ICNkZWZpbmUgQ0ROU19QQ0lFX0VQX01JTl9BUEVSVFVSRQkJMTI4CS8qIDEyOCBieXRlcyAqLw0K
PiA+ID4gPiAgICNkZWZpbmUgQ0ROU19QQ0lFX0VQX0lSUV9QQ0lfQUREUl9OT05FCQkweDENCj4g
PiA+ID4gICAjZGVmaW5lIENETlNfUENJRV9FUF9JUlFfUENJX0FERFJfTEVHQUNZCTB4Mw0KPiA+
ID4gPiArI2RlZmluZSBDRE5TX1BDSUVfRVBfTVNJWF9CQVIJCQkweDUNCj4gPiA+ID4NCj4gPiA+
ID4gICAvKioNCj4gPiA+ID4gICAgKiBzdHJ1Y3QgY2Ruc19wY2llX2VwIC0gcHJpdmF0ZSBkYXRh
IGZvciB0aGlzIFBDSWUgZW5kcG9pbnQgY29udHJvbGxlciBkcml2ZXINCj4gPiA+ID4gQEAgLTI1
NSw2ICsyNTYsNDMgQEAgc3RhdGljIGludCBjZG5zX3BjaWVfZXBfZ2V0X21zaShzdHJ1Y3QgcGNp
X2VwYyAqZXBjLCB1OCBmbikNCj4gPiA+ID4gICAJcmV0dXJuIG1tZTsNCj4gPiA+ID4gICB9DQo+
ID4gPiA+DQo+ID4gPiA+ICtzdGF0aWMgaW50IGNkbnNfcGNpZV9lcF9nZXRfbXNpeChzdHJ1Y3Qg
cGNpX2VwYyAqZXBjLCB1OCBmdW5jX25vKQ0KPiA+ID4gPiArew0KPiA+ID4gPiArCXN0cnVjdCBj
ZG5zX3BjaWVfZXAgKmVwID0gZXBjX2dldF9kcnZkYXRhKGVwYyk7DQo+ID4gPiA+ICsJc3RydWN0
IGNkbnNfcGNpZSAqcGNpZSA9ICZlcC0+cGNpZTsNCj4gPiA+ID4gKwl1MzIgY2FwID0gQ0ROU19Q
Q0lFX0VQX0ZVTkNfTVNJWF9DQVBfT0ZGU0VUOw0KPiA+ID4gPiArCXUzMiB2YWwsIHJlZzsNCj4g
PiA+ID4gKw0KPiA+ID4gPiArCXJlZyA9IGNhcCArIFBDSV9NU0lYX0ZMQUdTOw0KPiA+ID4gPiAr
CXZhbCA9IGNkbnNfcGNpZV9lcF9mbl9yZWFkdyhwY2llLCBmdW5jX25vLCByZWcpOw0KPiA+ID4g
PiArCWlmICghKHZhbCAmIFBDSV9NU0lYX0ZMQUdTX0VOQUJMRSkpDQo+ID4gPiA+ICsJCXJldHVy
biAtRUlOVkFMOw0KPiA+ID4gPiArDQo+ID4gPiA+ICsJdmFsICY9IFBDSV9NU0lYX0ZMQUdTX1FT
SVpFOw0KPiA+ID4gPiArDQo+ID4gPiA+ICsJcmV0dXJuIHZhbDsNCj4gPiA+ID4gK30NCj4gPiA+
ID4gKw0KPiA+ID4gPiArc3RhdGljIGludCBjZG5zX3BjaWVfZXBfc2V0X21zaXgoc3RydWN0IHBj
aV9lcGMgKmVwYywgdTggZm4sIHUxNiBpbnRlcnJ1cHRzKQ0KPiA+ID4gPiArew0KPiA+ID4gPiAr
CXN0cnVjdCBjZG5zX3BjaWVfZXAgKmVwID0gZXBjX2dldF9kcnZkYXRhKGVwYyk7DQo+ID4gPiA+
ICsJc3RydWN0IGNkbnNfcGNpZSAqcGNpZSA9ICZlcC0+cGNpZTsNCj4gPiA+ID4gKwl1MzIgY2Fw
ID0gQ0ROU19QQ0lFX0VQX0ZVTkNfTVNJWF9DQVBfT0ZGU0VUOw0KPiA+ID4gPiArCXUzMiB2YWws
IHJlZzsNCj4gPiA+ID4gKw0KPiA+ID4gPiArCXJlZyA9IGNhcCArIFBDSV9NU0lYX0ZMQUdTOw0K
PiA+ID4gPiArCXZhbCA9IGNkbnNfcGNpZV9lcF9mbl9yZWFkdyhwY2llLCBmbiwgcmVnKTsNCj4g
PiA+ID4gKwl2YWwgJj0gflBDSV9NU0lYX0ZMQUdTX1FTSVpFOw0KPiA+ID4gPiArCXZhbCB8PSBp
bnRlcnJ1cHRzOw0KPiA+ID4gPiArCWNkbnNfcGNpZV9lcF9mbl93cml0ZXcocGNpZSwgZm4sIHJl
ZywgdmFsKTsNCj4gPiA+ID4gKwkvKiBTZXQgTVNJWCBCQVIgYW5kIG9mZnNldCAqLw0KPiA+ID4g
PiArCWNkbnNfcGNpZV9lcF9mbl93cml0ZWwocGNpZSwgZm4sIDB4YjQsIENETlNfUENJRV9FUF9N
U0lYX0JBUik7DQo+ID4gPg0KPiA+ID4gUGxlYXNlIGFkZCBhIG1hY3JvIGZvciBNU0lYIHRhYmxl
IG9mZnNldC4NCj4gPiA+IEkgdGhpbmsgaXQgcmVsaWVzIG9uIGVuZHBvaW50IGZ1bmN0aW9uIGRy
aXZlciB0byBpbnZva2Ugc2V0X2JhciBmb3INCj4gPiA+IEJBUjU/IEl0J3MgcG9zc2libGUgYSBm
dW5jdGlvbiBkcml2ZXIgY2FuIGludm9rZSBzZXRfbXNpeCB3aXRob3V0IHNldF9iYXIuDQo+ID4g
Pg0KPiA+IEknbGwgYWRkIGEgbWFjcm8gZm9yIE1TSVggdGFibGUgb2Zmc2V0Lg0KPiA+IFllcywg
aXQgcmVsaWVzIG9uIHRoZSBCQVIgYmVpbmcgc2V0IHVwIGFscmVhZHkuICBJJ2xsIGFkZCBjb2Rl
IHRvIGNoZWNrIEJBUg0KPiA+IGV4aXN0cyBhbmQgaXMgbGFyZ2UgZW5vdWdoLCBhbmQgY3JlYXRl
IGl0IGlmIG5vdC4NCj4gPg0KPiA+ID4gPiArCS8qIFNldCBQQkEgQkFSIGFuZCBvZmZzZXQuICBC
QVIgbXVzdCBtYXRjaCBNU0lYIEJBUiAqLw0KPiA+ID4gPiArCWNkbnNfcGNpZV9lcF9mbl93cml0
ZWwocGNpZSwgZm4sIDB4YjgsIDB4MTAwMDAgfCBDRE5TX1BDSUVfRVBfTVNJWF9CQVIpOw0KPiA+
ID4NCj4gPiA+IEhlcmUgdG9vIGFkZCBhIG1hY3JvIGZvciBQQkEuDQo+ID4gPiBIb3cgZGlkIHlv
dSBnZXQgdGhlIDB4MTAwMDA/DQo+ID4gSSdsbCBhZGQgYSBtYWNybyBmb3IgUEJBLg0KPiA+IFRo
ZSAweDEwMDAwICgzMioyMDQ4KSBpcyB0byBhbGxvdyBzcGFjZSBpbiB0aGUgQkFSIGZvciAyMDQ4
IE1TSS1YIHZlY3RvcnMgYmVuZWF0aA0KPiA+IHRoZSBQQkEuIEFjdHVhbGx5LCBJIGNvdWxkIGNo
YW5nZSB0aGlzIHRvIDMyKmludGVycnVwdHMgc28gdGhhdCB0aGUgQkFSIHNpemUgY2FuIGJlDQo+
ID4gbWluaW1pemVkIGRlcGVuZGluZyBvbiB0aGUgbnVtYmVyIG9mIE1TSS1YIHByb2dyYW1tZWQu
DQo+ID4NCj4gPiBUaGFua3MgZm9yIHlvdXIgY29tbWVudHMsDQo+ID4gQWxhbg0KPiANCj4gSGkg
QWxhbiwNCj4gDQo+IHNob3VsZCBJIGV4cGVjdCBhIHYzIGZvciB0aGlzIHNlcmllcyB0byBhZGRy
ZXNzIEtpc2hvbidzIGNvbW1lbnRzID8NCj4gUGxlYXNlIGxldCBtZSBrbm93LCB0aGUgcHJldmlv
dXMgZml4ZXMgbG9vayBPSyB0byBtZS4NCj4gDQpJJ20gcHJlcGFyaW5nIGEgdjMgd2hpY2ggd2ls
bCBjaGVjayB0aGF0IHRoZSBCQVIgdG8gYmUgdXNlZCBmb3IgTVNJLVgNCmhhcyBiZWVuIHNldCB1
cCwgYW5kIHdpbGwgcmV0dXJuIC1FSU5WQUwgaWYgbm90LiAgSSBjb3VsZG4ndCBmaW5kIGENCnN1
aXRhYmxlIHdheSB0byBhbGxvY2F0ZSB0aGUgQkFSIGluc2lkZSB0aGUgc2V0X21zaXggZnVuY3Rp
b24uDQpJdCB3aWxsIGFsc28gYWRkcmVzcyBLaXNob24ncyBvdGhlciBjb21tZW50cy4NCg0KSSBl
eHBlY3QgdG8gc2VuZCBpdCB0b21vcnJvdywganVzdCBiYWNrIGZyb20gaG9saWRheSB0b2RheS4N
Cg0KUmVnYXJkcywNCkFsYW4NCiANCg==
WARNING: multiple messages have this Message-ID (diff)
From: Alan Douglas <adouglas@cadence.com>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>,
"bhelgaas@google.com" <bhelgaas@google.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"gustavo.pimentel@synopsys.com" <gustavo.pimentel@synopsys.com>,
"cyrille.pitchen@bootlin.com" <cyrille.pitchen@bootlin.com>,
Scott Telford <stelford@cadence.com>
Subject: RE: [PATCH v2 5/5] PCI: cadence: Add MSI-X capability to EP driver
Date: Mon, 17 Sep 2018 15:55:49 +0000 [thread overview]
Message-ID: <MWHPR07MB3502D6EF11136AF4D0EA03A9D81E0@MWHPR07MB3502.namprd07.prod.outlook.com> (raw)
Message-ID: <20180917155549.bXiCH7LBsFliephZ6uXBOL0xNE2SeTlXKM0N-2WX0zk@z> (raw)
In-Reply-To: <20180917152602.GI7239@e107981-ln.cambridge.arm.com>
Hi Lorenzo,
On 17 September 2018 16:26 Lorenzo Pieralisi wrote:
> On Tue, Sep 04, 2018 at 09:32:56AM +0000, Alan Douglas wrote:
> > Hi,
> > On 04 September 2018 05:46, Kishon Vijay Abraham I wrote:
> > > On Wednesday 15 August 2018 07:19 PM, Alan Douglas wrote:
> > > > Add set_msix and get_msix functions to driver, and handle
> > > > PCI_EPC_IRQ_MSIX request in raise_irq. BAR5 is used for
> > > > the MSI-X vectors.
> > > >
> > > > Signed-off-by: Alan Douglas <adouglas@cadence.com>
> > > > ---
> > > > drivers/pci/controller/pcie-cadence-ep.c | 107 ++++++++++++++++++++++++++++++-
> > > > drivers/pci/controller/pcie-cadence.h | 1 +
> > > > 2 files changed, 107 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/pci/controller/pcie-cadence-ep.c b/drivers/pci/controller/pcie-cadence-ep.c
> > > > index 1248d75..259b9a6 100644
> > > > --- a/drivers/pci/controller/pcie-cadence-ep.c
> > > > +++ b/drivers/pci/controller/pcie-cadence-ep.c
> > > > @@ -16,6 +16,7 @@
> > > > #define CDNS_PCIE_EP_MIN_APERTURE 128 /* 128 bytes */
> > > > #define CDNS_PCIE_EP_IRQ_PCI_ADDR_NONE 0x1
> > > > #define CDNS_PCIE_EP_IRQ_PCI_ADDR_LEGACY 0x3
> > > > +#define CDNS_PCIE_EP_MSIX_BAR 0x5
> > > >
> > > > /**
> > > > * struct cdns_pcie_ep - private data for this PCIe endpoint controller driver
> > > > @@ -255,6 +256,43 @@ static int cdns_pcie_ep_get_msi(struct pci_epc *epc, u8 fn)
> > > > return mme;
> > > > }
> > > >
> > > > +static int cdns_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no)
> > > > +{
> > > > + struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
> > > > + struct cdns_pcie *pcie = &ep->pcie;
> > > > + u32 cap = CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET;
> > > > + u32 val, reg;
> > > > +
> > > > + reg = cap + PCI_MSIX_FLAGS;
> > > > + val = cdns_pcie_ep_fn_readw(pcie, func_no, reg);
> > > > + if (!(val & PCI_MSIX_FLAGS_ENABLE))
> > > > + return -EINVAL;
> > > > +
> > > > + val &= PCI_MSIX_FLAGS_QSIZE;
> > > > +
> > > > + return val;
> > > > +}
> > > > +
> > > > +static int cdns_pcie_ep_set_msix(struct pci_epc *epc, u8 fn, u16 interrupts)
> > > > +{
> > > > + struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
> > > > + struct cdns_pcie *pcie = &ep->pcie;
> > > > + u32 cap = CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET;
> > > > + u32 val, reg;
> > > > +
> > > > + reg = cap + PCI_MSIX_FLAGS;
> > > > + val = cdns_pcie_ep_fn_readw(pcie, fn, reg);
> > > > + val &= ~PCI_MSIX_FLAGS_QSIZE;
> > > > + val |= interrupts;
> > > > + cdns_pcie_ep_fn_writew(pcie, fn, reg, val);
> > > > + /* Set MSIX BAR and offset */
> > > > + cdns_pcie_ep_fn_writel(pcie, fn, 0xb4, CDNS_PCIE_EP_MSIX_BAR);
> > >
> > > Please add a macro for MSIX table offset.
> > > I think it relies on endpoint function driver to invoke set_bar for
> > > BAR5? It's possible a function driver can invoke set_msix without set_bar.
> > >
> > I'll add a macro for MSIX table offset.
> > Yes, it relies on the BAR being set up already. I'll add code to check BAR
> > exists and is large enough, and create it if not.
> >
> > > > + /* Set PBA BAR and offset. BAR must match MSIX BAR */
> > > > + cdns_pcie_ep_fn_writel(pcie, fn, 0xb8, 0x10000 | CDNS_PCIE_EP_MSIX_BAR);
> > >
> > > Here too add a macro for PBA.
> > > How did you get the 0x10000?
> > I'll add a macro for PBA.
> > The 0x10000 (32*2048) is to allow space in the BAR for 2048 MSI-X vectors beneath
> > the PBA. Actually, I could change this to 32*interrupts so that the BAR size can be
> > minimized depending on the number of MSI-X programmed.
> >
> > Thanks for your comments,
> > Alan
>
> Hi Alan,
>
> should I expect a v3 for this series to address Kishon's comments ?
> Please let me know, the previous fixes look OK to me.
>
I'm preparing a v3 which will check that the BAR to be used for MSI-X
has been set up, and will return -EINVAL if not. I couldn't find a
suitable way to allocate the BAR inside the set_msix function.
It will also address Kishon's other comments.
I expect to send it tomorrow, just back from holiday today.
Regards,
Alan
next prev parent reply other threads:[~2018-09-17 22:32 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-15 13:49 [PATCH v2 5/5] PCI: cadence: Add MSI-X capability to EP driver Alan Douglas
2018-09-04 4:45 ` Kishon Vijay Abraham I
2018-09-04 9:32 ` Alan Douglas
2018-09-17 15:26 ` Lorenzo Pieralisi
2018-09-17 15:55 ` Alan Douglas [this message]
2018-09-17 15:55 ` Alan Douglas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MWHPR07MB3502D6EF11136AF4D0EA03A9D81E0@MWHPR07MB3502.namprd07.prod.outlook.com \
--to=adouglas@cadence.com \
--cc=bhelgaas@google.com \
--cc=cyrille.pitchen@bootlin.com \
--cc=gustavo.pimentel@synopsys.com \
--cc=kishon@ti.com \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=stelford@cadence.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).