Re: designware/imx6: question regarding MSI

Re: designware/imx6: question regarding MSI

[Jingoo Han]

On Thursday, January 23, 2014 5:39 AM, Jingoo Han wrote:
> 
> I’m currently playing with MSI support in pci-imx6.c using Harro Haan’s patch [*].
> 
> Are there any technical reasons why dw_pcie_msi_init cannot be called from dw_msi_setup_irq? Or rather,
> why it should not?

No, there is no special reason.
The patch (call dw_pcie_msi_init from dw_msi_setup_irq) would be
good. Thank you for your effort.

Mohit, Pratyush,
If you have different opinions, please let us know. :-)

Best regards,
Jingoo Han

> 
> I’m asking because I have a hotplug scenario where this little hack (call dw_pcie_msi_init from
> dw_msi_setup_irq) fixes a problem with MSI. I therefore need to understand whether it is a sensible
> hack or just crap, and more importantly; how it can be fixed properly.
> 
> Scenario:
> 
> (remove endpoints)
> # echo 1 > /sys/bus/pci/devices/0000:01:00.0/remove
> # echo 1 > /sys/bus/pci/devices/0000:01:00.1/remove
> 
> # take fpga in and out of reset (this will mess up the configuration space of the bridge)
> 
> (reset bridge)
> # echo 1 > /sys/bus/pci/devices/0000:00:00.0/reset
> 
> (rescan bus)
> # echo 1 > /sys/bus/pci/rescan
> 
> At this point everything works fine except MSI. With above mention hack it works like a charm.
> 
> I would be grateful if someone could shed light on this. I might have missed some important details.
> 
> 
> Best regards,
> Bjørn Erik Nilsen
> 
> [*]
> 
> From: Harro Haan <hrhaan@gmail.com>
> Date: Thu, 5 Dec 2013 14:06:37 +0100
> Subject: [PATCH 2/2] PCI: imx6: add support for MSI

Re: designware/imx6: question regarding MSI

[Pratyush Anand]

On Thu, Jan 23, 2014 at 01:52:19PM +0800, Jingoo Han wrote:
> On Thursday, January 23, 2014 5:39 AM, Jingoo Han wrote:
> > 
> > I’m currently playing with MSI support in pci-imx6.c using Harro Haan’s patch [*].
> > 
> > Are there any technical reasons why dw_pcie_msi_init cannot be called from dw_msi_setup_irq? Or rather,
> > why it should not?
> 
> No, there is no special reason.
> The patch (call dw_pcie_msi_init from dw_msi_setup_irq) would be
> good. Thank you for your effort.
> 
> Mohit, Pratyush,
> If you have different opinions, please let us know. :-)

dw_msi_setup_irq will be called for each function which needs msi
assignment. However, dw_pcie_msi_init should be called only once. So,
in my opinion calling dw_pcie_msi_init from dw_msi_setup_irq would not
be the best option.

@Bjørn, can you help us with tracing calling sequence of
dw_pcie_setup & dw_pcie_scan_bus. I think when you do "reset bridge",
it will call dw_pcie_setup and when you do "rescan bus", it will call
dw_pcie_scan_bus. If yes, then you can move dw_pcie_msi_init to
dw_pcie_setup.

Regards
Pratyush

> 
> Best regards,
> Jingoo Han
> 
> > 
> > I’m asking because I have a hotplug scenario where this little hack (call dw_pcie_msi_init from
> > dw_msi_setup_irq) fixes a problem with MSI. I therefore need to understand whether it is a sensible
> > hack or just crap, and more importantly; how it can be fixed properly.
> > 
> > Scenario:
> > 
> > (remove endpoints)
> > # echo 1 > /sys/bus/pci/devices/0000:01:00.0/remove
> > # echo 1 > /sys/bus/pci/devices/0000:01:00.1/remove
> > 
> > # take fpga in and out of reset (this will mess up the configuration space of the bridge)
> > 
> > (reset bridge)
> > # echo 1 > /sys/bus/pci/devices/0000:00:00.0/reset
> > 
> > (rescan bus)
> > # echo 1 > /sys/bus/pci/rescan
> > 
> > At this point everything works fine except MSI. With above mention hack it works like a charm.
> > 
> > I would be grateful if someone could shed light on this. I might have missed some important details.
> > 
> > 
> > Best regards,
> > Bjørn Erik Nilsen
> > 
> > [*]
> > 
> > From: Harro Haan <hrhaan@gmail.com>
> > Date: Thu, 5 Dec 2013 14:06:37 +0100
> > Subject: [PATCH 2/2] PCI: imx6: add support for MSI

Re: designware/imx6: question regarding MSI

[Harro Haan]

On 23 January 2014 16:20, Bjørn Erik Nilsen <ben@datarespons.no> wrote:
> Hi Pratyush,
>
> On Thu, 2014-01-23 at 07:51 +0100, Pratyush Anand wrote:
>> On Thu, Jan 23, 2014 at 01:52:19PM +0800, Jingoo Han wrote:
>> > On Thursday, January 23, 2014 5:39 AM, Jingoo Han wrote:
>> > >
>> > > I’m currently playing with MSI support in pci-imx6.c using Harro Haan’s patch [*].

The above patch mentioned will not become part of mainline Linux, but
I expect/hope a reworked version of "[PATCH] pci: imx: enable pcie msi
support" from Richard Zhu will become part of mainline, because this
patch supports MSIX as well. Reference to the patch with review
remarks:
http://www.spinics.net/lists/linux-ide/msg47074.html

@Richard Zhu: Is this a correct assumption?

>> > >
>> > > Are there any technical reasons why dw_pcie_msi_init cannot be called from dw_msi_setup_irq? Or rather,
>> > > why it should not?
>> >
>> > No, there is no special reason.
>> > The patch (call dw_pcie_msi_init from dw_msi_setup_irq) would be
>> > good. Thank you for your effort.
>> >
>> > Mohit, Pratyush,
>> > If you have different opinions, please let us know. :-)
>>
>> dw_msi_setup_irq will be called for each function which needs msi
>> assignment. However, dw_pcie_msi_init should be called only once. So,
>> in my opinion calling dw_pcie_msi_init from dw_msi_setup_irq would not
>> be the best option.
>>
>> @Bjørn, can you help us with tracing calling sequence of
>> dw_pcie_setup & dw_pcie_scan_bus. I think when you do "reset bridge",
>> it will call dw_pcie_setup and when you do "rescan bus", it will call
>> dw_pcie_scan_bus. If yes, then you can move dw_pcie_msi_init to
>> dw_pcie_setup.
>
>
> It looks like dw_pcie_setup and dw_pcie_scan_bus is only called when
> booting. Resetting the bridge or scanning the bus will not trigger any
> of these functions, unfortunately.
>
> I have looked for other hooks too, without luck.

@Bjørn: have you applied the following patch?
"[PATCH] pci: designware: fix missing msi irqs"
Reference to the patch:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-December/218629.html

Best regards,

Harro

>
>>
>> Regards
>> Pratyush
>>
>> >
>> > Best regards,
>> > Jingoo Han
>> >
>> > >
>> > > I’m asking because I have a hotplug scenario where this little hack (call dw_pcie_msi_init from
>> > > dw_msi_setup_irq) fixes a problem with MSI. I therefore need to understand whether it is a sensible
>> > > hack or just crap, and more importantly; how it can be fixed properly.
>> > >
>> > > Scenario:
>> > >
>> > > (remove endpoints)
>> > > # echo 1 > /sys/bus/pci/devices/0000:01:00.0/remove
>> > > # echo 1 > /sys/bus/pci/devices/0000:01:00.1/remove
>> > >
>> > > # take fpga in and out of reset (this will mess up the configuration space of the bridge)
>> > >
>> > > (reset bridge)
>> > > # echo 1 > /sys/bus/pci/devices/0000:00:00.0/reset
>> > >
>> > > (rescan bus)
>> > > # echo 1 > /sys/bus/pci/rescan
>> > >
>> > > At this point everything works fine except MSI. With above mention hack it works like a charm.
>> > >
>> > > I would be grateful if someone could shed light on this. I might have missed some important details.
>> > >
>> > >
>> > > Best regards,
>> > > Bjørn Erik Nilsen
>> > >
>> > > [*]
>> > >
>> > > From: Harro Haan <hrhaan@gmail.com>
>> > > Date: Thu, 5 Dec 2013 14:06:37 +0100
>> > > Subject: [PATCH 2/2] PCI: imx6: add support for MSI
>

Re: designware/imx6: question regarding MSI

[Pratyush Anand]

On Thu, Jan 23, 2014 at 11:20:55PM +0800, Bjørn Erik Nilsen wrote:
> Hi Pratyush,
> 
> On Thu, 2014-01-23 at 07:51 +0100, Pratyush Anand wrote:
> > On Thu, Jan 23, 2014 at 01:52:19PM +0800, Jingoo Han wrote:
> > > On Thursday, January 23, 2014 5:39 AM, Jingoo Han wrote:
> > > > 
> > > > I’m currently playing with MSI support in pci-imx6.c using Harro Haan’s patch [*].
> > > > 
> > > > Are there any technical reasons why dw_pcie_msi_init cannot be called from dw_msi_setup_irq? Or rather,
> > > > why it should not?
> > > 
> > > No, there is no special reason.
> > > The patch (call dw_pcie_msi_init from dw_msi_setup_irq) would be
> > > good. Thank you for your effort.
> > > 
> > > Mohit, Pratyush,
> > > If you have different opinions, please let us know. :-)
> > 
> > dw_msi_setup_irq will be called for each function which needs msi
> > assignment. However, dw_pcie_msi_init should be called only once. So,
> > in my opinion calling dw_pcie_msi_init from dw_msi_setup_irq would not
> > be the best option.
> > 
> > @Bjørn, can you help us with tracing calling sequence of
> > dw_pcie_setup & dw_pcie_scan_bus. I think when you do "reset bridge",
> > it will call dw_pcie_setup and when you do "rescan bus", it will call
> > dw_pcie_scan_bus. If yes, then you can move dw_pcie_msi_init to
> > dw_pcie_setup.
> 
> 
> It looks like dw_pcie_setup and dw_pcie_scan_bus is only called when
> booting. Resetting the bridge or scanning the bus will not trigger any
> of these functions, unfortunately.

Yes, I browsed pcie sysfs rescan and reset code and it does not go
into that path. Sorry for the wrong pointers. I should have looked
into code earlier :(

Coming to the issue, I am still not convinced that msi_init part should
go to msi_setup_irq. May be something is missing.
@Mohit, Jingoo: How does remove, reset and rescan behave with your
platform.

Regards
Pratyush

> 
> I have looked for other hooks too, without luck.
> 
> > 
> > Regards
> > Pratyush
> > 
> > > 
> > > Best regards,
> > > Jingoo Han
> > > 
> > > > 
> > > > I’m asking because I have a hotplug scenario where this little hack (call dw_pcie_msi_init from
> > > > dw_msi_setup_irq) fixes a problem with MSI. I therefore need to understand whether it is a sensible
> > > > hack or just crap, and more importantly; how it can be fixed properly.
> > > > 
> > > > Scenario:
> > > > 
> > > > (remove endpoints)
> > > > # echo 1 > /sys/bus/pci/devices/0000:01:00.0/remove
> > > > # echo 1 > /sys/bus/pci/devices/0000:01:00.1/remove
> > > > 
> > > > # take fpga in and out of reset (this will mess up the configuration space of the bridge)
> > > > 
> > > > (reset bridge)
> > > > # echo 1 > /sys/bus/pci/devices/0000:00:00.0/reset
> > > > 
> > > > (rescan bus)
> > > > # echo 1 > /sys/bus/pci/rescan
> > > > 
> > > > At this point everything works fine except MSI. With above mention hack it works like a charm.
> > > > 
> > > > I would be grateful if someone could shed light on this. I might have missed some important details.
> > > > 
> > > > 
> > > > Best regards,
> > > > Bjørn Erik Nilsen
> > > > 
> > > > [*]
> > > > 
> > > > From: Harro Haan <hrhaan@gmail.com>
> > > > Date: Thu, 5 Dec 2013 14:06:37 +0100
> > > > Subject: [PATCH 2/2] PCI: imx6: add support for MSI
> 

RE: designware/imx6: question regarding MSI

[Hong-Xing.Zhu]

SGk6DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQmrDuHJuIEVyaWsg
Tmlsc2VuIFttYWlsdG86YmVuQGRhdGFyZXNwb25zLm5vXQ0KPiBTZW50OiBGcmlkYXksIEphbnVh
cnkgMjQsIDIwMTQgMzo0NyBQTQ0KPiBUbzogSGFycm8gSGFhbg0KPiBDYzogUmljaGFyZCBaaHU7
IFpodSBSaWNoYXJkLVI2NTAzNzsgUHJhdHl1c2ggQW5hbmQ7IEppbmdvbyBIYW47IGxpbnV4LQ0K
PiBwY2lAdmdlci5rZXJuZWwub3JnOyBNb2hpdCBLVU1BUiBEQ0c7IE1hcmVrIFZhc3V0DQo+IFN1
YmplY3Q6IFJlOiBkZXNpZ253YXJlL2lteDY6IHF1ZXN0aW9uIHJlZ2FyZGluZyBNU0kNCj4gDQo+
IE9uIFRodSwgMjAxNC0wMS0yMyBhdCAxODozMiArMDEwMCwgSGFycm8gSGFhbiB3cm90ZToNCj4g
PiBPbiAyMyBKYW51YXJ5IDIwMTQgMTY6MjAsIEJqw7hybiBFcmlrIE5pbHNlbiA8YmVuQGRhdGFy
ZXNwb25zLm5vPiB3cm90ZToNCj4gPiA+IEhpIFByYXR5dXNoLA0KPiA+ID4NCj4gPiA+IE9uIFRo
dSwgMjAxNC0wMS0yMyBhdCAwNzo1MSArMDEwMCwgUHJhdHl1c2ggQW5hbmQgd3JvdGU6DQo+ID4g
Pj4gT24gVGh1LCBKYW4gMjMsIDIwMTQgYXQgMDE6NTI6MTlQTSArMDgwMCwgSmluZ29vIEhhbiB3
cm90ZToNCj4gPiA+PiA+IE9uIFRodXJzZGF5LCBKYW51YXJ5IDIzLCAyMDE0IDU6MzkgQU0sIEpp
bmdvbyBIYW4gd3JvdGU6DQo+ID4gPj4gPiA+DQo+ID4gPj4gPiA+IEnigJltIGN1cnJlbnRseSBw
bGF5aW5nIHdpdGggTVNJIHN1cHBvcnQgaW4gcGNpLWlteDYuYyB1c2luZyBIYXJybw0KPiBIYWFu
4oCZcyBwYXRjaCBbKl0uDQo+ID4NCj4gPiBUaGUgYWJvdmUgcGF0Y2ggbWVudGlvbmVkIHdpbGwg
bm90IGJlY29tZSBwYXJ0IG9mIG1haW5saW5lIExpbnV4LCBidXQNCj4gPiBJIGV4cGVjdC9ob3Bl
IGEgcmV3b3JrZWQgdmVyc2lvbiBvZiAiW1BBVENIXSBwY2k6IGlteDogZW5hYmxlIHBjaWUgbXNp
DQo+ID4gc3VwcG9ydCIgZnJvbSBSaWNoYXJkIFpodSB3aWxsIGJlY29tZSBwYXJ0IG9mIG1haW5s
aW5lLCBiZWNhdXNlIHRoaXMNCj4gPiBwYXRjaCBzdXBwb3J0cyBNU0lYIGFzIHdlbGwuIFJlZmVy
ZW5jZSB0byB0aGUgcGF0Y2ggd2l0aCByZXZpZXcNCj4gPiByZW1hcmtzOg0KPiA+IGh0dHA6Ly93
d3cuc3Bpbmljcy5uZXQvbGlzdHMvbGludXgtaWRlL21zZzQ3MDc0Lmh0bWwNCj4gDQo+IE9LLCBj
b29sISBUaGFuayB5b3UgZ3V5cyBmb3IgdGhlIGdvb2Qgd29yay4gSSB3aWxsIGdpdmUgaXQgYSBz
aG90IGxhdGVyLg0KPiANCj4gPiBAUmljaGFyZCBaaHU6IElzIHRoaXMgYSBjb3JyZWN0IGFzc3Vt
cHRpb24/DQo+ID4NCltSaWNoYXJkXSBZZXMsIGl0IGlzLiBJIHVzZSB0aGlzIHBhdGNoIHRvIGVu
YWJsZSBpbXg2IHBjaWUgbXNpIG5vdy4NClRoaXMgcGF0Y2ggc2hvdWxkIGJlIHJlLWNvbnNpZGVy
ZWQsIHJlLXN0cnVjdHVyZSB0b28uDQoNCj4gPiA+PiA+ID4NCj4gPiA+PiA+ID4gQXJlIHRoZXJl
IGFueSB0ZWNobmljYWwgcmVhc29ucyB3aHkgZHdfcGNpZV9tc2lfaW5pdCBjYW5ub3QgYmUNCj4g
PiA+PiA+ID4gY2FsbGVkIGZyb20gZHdfbXNpX3NldHVwX2lycT8gT3IgcmF0aGVyLCB3aHkgaXQg
c2hvdWxkIG5vdD8NCj4gPiA+PiA+DQo+ID4gPj4gPiBObywgdGhlcmUgaXMgbm8gc3BlY2lhbCBy
ZWFzb24uDQo+ID4gPj4gPiBUaGUgcGF0Y2ggKGNhbGwgZHdfcGNpZV9tc2lfaW5pdCBmcm9tIGR3
X21zaV9zZXR1cF9pcnEpIHdvdWxkIGJlDQo+ID4gPj4gPiBnb29kLiBUaGFuayB5b3UgZm9yIHlv
dXIgZWZmb3J0Lg0KPiA+ID4+ID4NCj4gPiA+PiA+IE1vaGl0LCBQcmF0eXVzaCwNCj4gPiA+PiA+
IElmIHlvdSBoYXZlIGRpZmZlcmVudCBvcGluaW9ucywgcGxlYXNlIGxldCB1cyBrbm93LiA6LSkN
Cj4gPiA+Pg0KPiA+ID4+IGR3X21zaV9zZXR1cF9pcnEgd2lsbCBiZSBjYWxsZWQgZm9yIGVhY2gg
ZnVuY3Rpb24gd2hpY2ggbmVlZHMgbXNpDQo+ID4gPj4gYXNzaWdubWVudC4gSG93ZXZlciwgZHdf
cGNpZV9tc2lfaW5pdCBzaG91bGQgYmUgY2FsbGVkIG9ubHkgb25jZS4NCj4gPiA+PiBTbywgaW4g
bXkgb3BpbmlvbiBjYWxsaW5nIGR3X3BjaWVfbXNpX2luaXQgZnJvbSBkd19tc2lfc2V0dXBfaXJx
DQo+ID4gPj4gd291bGQgbm90IGJlIHRoZSBiZXN0IG9wdGlvbi4NCj4gPiA+Pg0KPiA+ID4+IEBC
asO4cm4sIGNhbiB5b3UgaGVscCB1cyB3aXRoIHRyYWNpbmcgY2FsbGluZyBzZXF1ZW5jZSBvZg0K
PiA+ID4+IGR3X3BjaWVfc2V0dXAgJiBkd19wY2llX3NjYW5fYnVzLiBJIHRoaW5rIHdoZW4geW91
IGRvICJyZXNldA0KPiA+ID4+IGJyaWRnZSIsIGl0IHdpbGwgY2FsbCBkd19wY2llX3NldHVwIGFu
ZCB3aGVuIHlvdSBkbyAicmVzY2FuIGJ1cyIsDQo+ID4gPj4gaXQgd2lsbCBjYWxsIGR3X3BjaWVf
c2Nhbl9idXMuIElmIHllcywgdGhlbiB5b3UgY2FuIG1vdmUNCj4gPiA+PiBkd19wY2llX21zaV9p
bml0IHRvIGR3X3BjaWVfc2V0dXAuDQo+ID4gPg0KPiA+ID4NCj4gPiA+IEl0IGxvb2tzIGxpa2Ug
ZHdfcGNpZV9zZXR1cCBhbmQgZHdfcGNpZV9zY2FuX2J1cyBpcyBvbmx5IGNhbGxlZCB3aGVuDQo+
ID4gPiBib290aW5nLiBSZXNldHRpbmcgdGhlIGJyaWRnZSBvciBzY2FubmluZyB0aGUgYnVzIHdp
bGwgbm90IHRyaWdnZXINCj4gPiA+IGFueSBvZiB0aGVzZSBmdW5jdGlvbnMsIHVuZm9ydHVuYXRl
bHkuDQo+ID4gPg0KPiA+ID4gSSBoYXZlIGxvb2tlZCBmb3Igb3RoZXIgaG9va3MgdG9vLCB3aXRo
b3V0IGx1Y2suDQo+ID4NCj4gPiBAQmrDuHJuOiBoYXZlIHlvdSBhcHBsaWVkIHRoZSBmb2xsb3dp
bmcgcGF0Y2g/DQo+ID4gIltQQVRDSF0gcGNpOiBkZXNpZ253YXJlOiBmaXggbWlzc2luZyBtc2kg
aXJxcyINCj4gPiBSZWZlcmVuY2UgdG8gdGhlIHBhdGNoOg0KPiA+IGh0dHA6Ly9saXN0cy5pbmZy
YWRlYWQub3JnL3BpcGVybWFpbC9saW51eC1hcm0ta2VybmVsLzIwMTMtRGVjZW1iZXIvMjENCj4g
PiA4NjI5Lmh0bWwNCj4gDQo+IFllcywgSSBoYXZlIHRoYXQgb25lIGFwcGxpZWQgYXMgd2VsbC4g
DQoNCkJlc3QgUmVnYXJkcw0KUmljaGFyZCBaaHUNCg==

Re: designware/imx6: question regarding MSI

[Jingoo Han]

On Thursday, January 30, 2014 1:29 AM, Bjørn Erik Nilsen wrote:
> On Fri, 2014-01-24 at 07:48 +0100, Pratyush Anand wrote:
> > On Thu, Jan 23, 2014 at 11:20:55PM +0800, Bjørn Erik Nilsen wrote:
> > > On Thu, 2014-01-23 at 07:51 +0100, Pratyush Anand wrote:
> > > > On Thu, Jan 23, 2014 at 01:52:19PM +0800, Jingoo Han wrote:
> > > > > On Thursday, January 23, 2014 5:39 AM, Jingoo Han wrote:
> > > > > >
> > > > > > I’m currently playing with MSI support in pci-imx6.c using Harro Haan’s patch [*].
> > > > > >
> > > > > > Are there any technical reasons why dw_pcie_msi_init cannot be called
> > > > > > from dw_msi_setup_irq?
> Or rather,
> > > > > > why it should not?
> > > > >
> > > > > No, there is no special reason.
> > > > > The patch (call dw_pcie_msi_init from dw_msi_setup_irq) would be
> > > > > good. Thank you for your effort.
> > > > >
> > > > > Mohit, Pratyush,
> > > > > If you have different opinions, please let us know. :-)
> > > >
> > > > dw_msi_setup_irq will be called for each function which needs msi
> > > > assignment. However, dw_pcie_msi_init should be called only once. So,
> > > > in my opinion calling dw_pcie_msi_init from dw_msi_setup_irq would not
> > > > be the best option.
> > > >
> > > > @Bjørn, can you help us with tracing calling sequence of
> > > > dw_pcie_setup & dw_pcie_scan_bus. I think when you do "reset bridge",
> > > > it will call dw_pcie_setup and when you do "rescan bus", it will call
> > > > dw_pcie_scan_bus. If yes, then you can move dw_pcie_msi_init to
> > > > dw_pcie_setup.
> > >
> > >
> > > It looks like dw_pcie_setup and dw_pcie_scan_bus is only called when
> > > booting. Resetting the bridge or scanning the bus will not trigger any
> > > of these functions, unfortunately.
> >
> > Yes, I browsed pcie sysfs rescan and reset code and it does not go
> > into that path. Sorry for the wrong pointers. I should have looked
> > into code earlier :(
> >
> > Coming to the issue, I am still not convinced that msi_init part should
> > go to msi_setup_irq. May be something is missing.
> > @Mohit, Jingoo: How does remove, reset and rescan behave with your
> > platform.
> 
> Any comments?

Sorry for late response.
I tested 'remove, reset and rescan' on Exynos platform
as you did.

(remove endpoints)
# echo 1 > /sys/bus/pci/devices/0000:01:00.0/remove
# echo 1 > /sys/bus/pci/devices/0000:01:00.1/remove

(reset bridge)
# echo 1 > /sys/bus/pci/devices/0000:00:00.0/reset

(rescan bus)
# echo 1 > /sys/bus/pci/rescan

Without MSI (legacy INTx mode), it works properly. However,
with MSI, it makes the problem.

> 
> I can prepare a patch (which calls dw_pcie_msi_init from
> dw_msi_setup_irq), however if this is not the right solution then we
> need to figure out what a proper solution should look like.

If you send the patch, I will test it on Exynos platform.
Thank you for your effort. :-)

Best regards,
Jingoo Han

Re: designware/imx6: question regarding MSI

[Jingoo Han]

On Monday, February 03, 2014 6:29 PM, Jingoo Han wrote:
> On Thursday, January 30, 2014 1:29 AM, Bjørn Erik Nilsen wrote:
> > On Fri, 2014-01-24 at 07:48 +0100, Pratyush Anand wrote:
> > > On Thu, Jan 23, 2014 at 11:20:55PM +0800, Bjørn Erik Nilsen wrote:
> > > > On Thu, 2014-01-23 at 07:51 +0100, Pratyush Anand wrote:
> > > > > On Thu, Jan 23, 2014 at 01:52:19PM +0800, Jingoo Han wrote:
> > > > > > On Thursday, January 23, 2014 5:39 AM, Jingoo Han wrote:
> > > > > > >
> > > > > > > I’m currently playing with MSI support in pci-imx6.c using Harro Haan’s patch [*].
> > > > > > >
> > > > > > > Are there any technical reasons why dw_pcie_msi_init cannot be called
> > > > > > > from dw_msi_setup_irq?
> > Or rather,
> > > > > > > why it should not?
> > > > > >
> > > > > > No, there is no special reason.
> > > > > > The patch (call dw_pcie_msi_init from dw_msi_setup_irq) would be
> > > > > > good. Thank you for your effort.
> > > > > >
> > > > > > Mohit, Pratyush,
> > > > > > If you have different opinions, please let us know. :-)
> > > > >
> > > > > dw_msi_setup_irq will be called for each function which needs msi
> > > > > assignment. However, dw_pcie_msi_init should be called only once. So,
> > > > > in my opinion calling dw_pcie_msi_init from dw_msi_setup_irq would not
> > > > > be the best option.
> > > > >
> > > > > @Bjørn, can you help us with tracing calling sequence of
> > > > > dw_pcie_setup & dw_pcie_scan_bus. I think when you do "reset bridge",
> > > > > it will call dw_pcie_setup and when you do "rescan bus", it will call
> > > > > dw_pcie_scan_bus. If yes, then you can move dw_pcie_msi_init to
> > > > > dw_pcie_setup.
> > > >
> > > >
> > > > It looks like dw_pcie_setup and dw_pcie_scan_bus is only called when
> > > > booting. Resetting the bridge or scanning the bus will not trigger any
> > > > of these functions, unfortunately.
> > >
> > > Yes, I browsed pcie sysfs rescan and reset code and it does not go
> > > into that path. Sorry for the wrong pointers. I should have looked
> > > into code earlier :(
> > >
> > > Coming to the issue, I am still not convinced that msi_init part should
> > > go to msi_setup_irq. May be something is missing.
> > > @Mohit, Jingoo: How does remove, reset and rescan behave with your
> > > platform.
> >
> > Any comments?
> 
> Sorry for late response.
> I tested 'remove, reset and rescan' on Exynos platform
> as you did.
> 
> (remove endpoints)
> # echo 1 > /sys/bus/pci/devices/0000:01:00.0/remove
> # echo 1 > /sys/bus/pci/devices/0000:01:00.1/remove
> 
> (reset bridge)
> # echo 1 > /sys/bus/pci/devices/0000:00:00.0/reset
> 
> (rescan bus)
> # echo 1 > /sys/bus/pci/rescan
> 
> Without MSI (legacy INTx mode), it works properly. However,
> with MSI, it makes the problem.

Oh, there was my mistake. There is no problem on Exynos platform.
I tested two different LAN cards. With MSI, 'remove, reset and
rescan' works properly on Exynos platform using the current
pci/next tree. Sorry for the confusion.
Thank you.

Best regards,
Jingoo Han

> 
> >
> > I can prepare a patch (which calls dw_pcie_msi_init from
> > dw_msi_setup_irq), however if this is not the right solution then we
> > need to figure out what a proper solution should look like.
> 
> If you send the patch, I will test it on Exynos platform.
> Thank you for your effort. :-)
> 
> Best regards,
> Jingoo Han

Re: designware/imx6: question regarding MSI

[Jingoo Han]

On Tuesday, February 04, 2014 1:01 AM, Bjørn Erik Nilsen wrote:
> On Mon, 2014-02-03 at 10:55 +0100, Jingoo Han wrote:
> > On Monday, February 03, 2014 6:29 PM, Jingoo Han wrote:
> > > On Thursday, January 30, 2014 1:29 AM, Bjørn Erik Nilsen wrote:
> > > > On Fri, 2014-01-24 at 07:48 +0100, Pratyush Anand wrote:
> > > > >
> > > > > Yes, I browsed pcie sysfs rescan and reset code and it does not go
> > > > > into that path. Sorry for the wrong pointers. I should have looked
> > > > > into code earlier :(
> > > > >
> > > > > Coming to the issue, I am still not convinced that msi_init part should
> > > > > go to msi_setup_irq. May be something is missing.
> > > > > @Mohit, Jingoo: How does remove, reset and rescan behave with your
> > > > > platform.
> > > >
> > > > Any comments?
> > >
> > > Sorry for late response.
> > > I tested 'remove, reset and rescan' on Exynos platform
> > > as you did.
> > >
> > > (remove endpoints)
> > > # echo 1 > /sys/bus/pci/devices/0000:01:00.0/remove
> > > # echo 1 > /sys/bus/pci/devices/0000:01:00.1/remove
> > >
> > > (reset bridge)
> > > # echo 1 > /sys/bus/pci/devices/0000:00:00.0/reset
> > >
> > > (rescan bus)
> > > # echo 1 > /sys/bus/pci/rescan
> > >
> > > Without MSI (legacy INTx mode), it works properly. However,
> > > with MSI, it makes the problem.
> >
> > Oh, there was my mistake. There is no problem on Exynos platform.
> > I tested two different LAN cards. With MSI, 'remove, reset and
> > rescan' works properly on Exynos platform using the current
> > pci/next tree. Sorry for the confusion.
> > Thank you.
> 
> Right.
> 
> So if I understand you correctly it works fine with pci/next, however
> you were able to reproduce with an earlier kernel version? That is
> indeed very good news. I will try to pull in the latest and greatest
> from pci/next to verify this.

Hi Bjørn Erik Nilsen,

I tested an earlier kernel v3.13. As you know, this version have
Some problems about designware PCIe MSI handling.

I tested the latest v3.14-rc1 kernel (vanilla kernel). It works
properly. Currently, v3.14-rc1 kernel was merged to the current
pci/next kernel. So, you can test 'mainline kernel v3.14-rc1' or
'the latest pci/next kernel'. (the commit 7f2d86 "Merge branch
'pci/numa-cleanup' into next")

If you have some problems, please let us know. :-)
Thank you.

Best regards,
Jingoo Han

Re: designware/imx6: question regarding MSI

[Marek Vasut]

On Tuesday, February 04, 2014 at 05:40:07 AM, Jingoo Han wrote:

[...]

> Hi Bjørn Erik Nilsen,
> 
> I tested an earlier kernel v3.13. As you know, this version have
> Some problems about designware PCIe MSI handling.
> 
> I tested the latest v3.14-rc1 kernel (vanilla kernel). It works
> properly. Currently, v3.14-rc1 kernel was merged to the current
> pci/next kernel. So, you can test 'mainline kernel v3.14-rc1' or
> 'the latest pci/next kernel'. (the commit 7f2d86 "Merge branch
> 'pci/numa-cleanup' into next")
> 
> If you have some problems, please let us know. :-)
> Thank you.

I'm now also back in the game, so please keep me on the list.

Best regards,
Marek Vasut