| Message ID | 20130917201104.GA5209@obsidianresearch.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Sep 17, 2013 at 02:11:04PM -0600, Jason Gunthorpe wrote: > If the property was not specified then then the returned resource > had a resource_size(..) == 1, rather than 0. The PCI-E driver checks > for 0 so it blindly continues on with a corrupted resource. > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > --- > drivers/bus/mvebu-mbus.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) Applied to mvebu/drivers since you didn't mention a specific regression. If there is one, please let me know shortly and I'll try to queue it up for v3.12 instead. thx, Jason.
On Tue, Oct 01, 2013 at 12:44:09PM -0400, Jason Cooper wrote: > On Tue, Sep 17, 2013 at 02:11:04PM -0600, Jason Gunthorpe wrote: > > If the property was not specified then then the returned resource > > had a resource_size(..) == 1, rather than 0. The PCI-E driver checks > > for 0 so it blindly continues on with a corrupted resource. > > > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > drivers/bus/mvebu-mbus.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > Applied to mvebu/drivers since you didn't mention a specific regression. > If there is one, please let me know shortly and I'll try to queue it up > for v3.12 instead. If the DT is not formed exactly how the driver expects it goes sideways. The kernel DT's are all OK. Was the PCI driver new in 3.12? If so it should probably be fixed in 3.12 as well, since this sequence in the PCI driver: mvebu_mbus_get_pcie_io_aperture(&pcie->io); if (resource_size(&pcie->io) == 0) { dev_err(&pdev->dev, "invalid I/O aperture size\n"); return -EINVAL; } Doesn't work. This patch is a necessary precondition to applying: https://github.com/jgunthorpe/linux/commit/ef90b0bf7d8552dc7dfaad82d964446f6a9b6a3b PCI: mvebu - Support a bridge with no IO port window Regards, Jason
On Tue, Oct 01, 2013 at 10:50:45AM -0600, Jason Gunthorpe wrote: > On Tue, Oct 01, 2013 at 12:44:09PM -0400, Jason Cooper wrote: > > On Tue, Sep 17, 2013 at 02:11:04PM -0600, Jason Gunthorpe wrote: > > > If the property was not specified then then the returned resource > > > had a resource_size(..) == 1, rather than 0. The PCI-E driver checks > > > for 0 so it blindly continues on with a corrupted resource. > > > > > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > > drivers/bus/mvebu-mbus.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > Applied to mvebu/drivers since you didn't mention a specific regression. > > If there is one, please let me know shortly and I'll try to queue it up > > for v3.12 instead. > > If the DT is not formed exactly how the driver expects it goes > sideways. The kernel DT's are all OK. > > Was the PCI driver new in 3.12? > > If so it should probably be fixed in 3.12 as well, since this > sequence in the PCI driver: > > mvebu_mbus_get_pcie_io_aperture(&pcie->io); > if (resource_size(&pcie->io) == 0) { > dev_err(&pdev->dev, "invalid I/O aperture size\n"); > return -EINVAL; > } > > Doesn't work. Ok, I've moved it over the mvebu/fixes and amended the commit as follows: """ bus: mvebu-mbus: Fix optional pcie-mem/io-aperture properties If the property was not specified then the returned resource had a resource_size(..) == 1, rather than 0. The PCI-E driver checks for 0 so it blindly continues on with a corrupted resource. The regression was introduced into v3.12 by: 11be654 PCI: mvebu: Adapt to the new device tree layout """ > This patch is a necessary precondition to applying: > > https://github.com/jgunthorpe/linux/commit/ef90b0bf7d8552dc7dfaad82d964446f6a9b6a3b > PCI: mvebu - Support a bridge with no IO port window I don't see this one in my stack, have you submitted it yet? When you do, please add a note mentioning the dependency on this commit in mvebu/fixes. thx, Jason.
On Tue, Oct 01, 2013 at 01:09:18PM -0400, Jason Cooper wrote: > Ok, I've moved it over the mvebu/fixes and amended the commit as > follows: > > """ > bus: mvebu-mbus: Fix optional pcie-mem/io-aperture properties > > If the property was not specified then the returned resource had a > resource_size(..) == 1, rather than 0. The PCI-E driver checks for 0 so it > blindly continues on with a corrupted resource. > > The regression was introduced into v3.12 by: > > 11be654 PCI: mvebu: Adapt to the new device tree layout > > """ Reads fine to me, thanks > > This patch is a necessary precondition to applying: > > > > https://github.com/jgunthorpe/linux/commit/ef90b0bf7d8552dc7dfaad82d964446f6a9b6a3b > > PCI: mvebu - Support a bridge with no IO port window > > I don't see this one in my stack, have you submitted it yet? When you > do, please add a note mentioning the dependency on this commit in > mvebu/fixes. Yes, I posted it along with the other PCI patches: http://permalink.gmane.org/gmane.linux.kernel.pci/25498 Just so we are on the same page, these are the Kirkwood/mvebu patches I was hoping to progress for 3.13: https://github.com/jgunthorpe/linux/commit/9f20eec4696627afe87bf0f6a204909004062e8e PCI: mvebu: Dynamically detect if the PEX link is up to enable hot plug (Thomas soft ack'd this @ http://www.spinics.net/lists/linux-pci/msg25323.html) https://github.com/jgunthorpe/linux/commit/ef90b0bf7d8552dc7dfaad82d964446f6a9b6a3b PCI: mvebu - Support a bridge with no IO port window (no comments) And these which I think you have already taken: ARM: kirkwood - Remove kirkwood_setup_wins and rely on the DT binding ARM: kirkwood: Move the crypto node under the mbus node ARM: kirkwood: Move the nand node under the mbus node bus: mvebu-mbus: Fix optional pcie-mem/io-aperture properties Thanks, Jason
On Tue, Oct 01, 2013 at 11:22:59AM -0600, Jason Gunthorpe wrote: > On Tue, Oct 01, 2013 at 01:09:18PM -0400, Jason Cooper wrote: ... > > > This patch is a necessary precondition to applying: > > > > > > https://github.com/jgunthorpe/linux/commit/ef90b0bf7d8552dc7dfaad82d964446f6a9b6a3b > > > PCI: mvebu - Support a bridge with no IO port window > > > > I don't see this one in my stack, have you submitted it yet? When you > > do, please add a note mentioning the dependency on this commit in > > mvebu/fixes. > > Yes, I posted it along with the other PCI patches: > > http://permalink.gmane.org/gmane.linux.kernel.pci/25498 > > Just so we are on the same page, these are the Kirkwood/mvebu patches > I was hoping to progress for 3.13: > > https://github.com/jgunthorpe/linux/commit/9f20eec4696627afe87bf0f6a204909004062e8e > PCI: mvebu: Dynamically detect if the PEX link is up to enable hot plug > (Thomas soft ack'd this @ http://www.spinics.net/lists/linux-pci/msg25323.html) > > https://github.com/jgunthorpe/linux/commit/ef90b0bf7d8552dc7dfaad82d964446f6a9b6a3b > PCI: mvebu - Support a bridge with no IO port window > (no comments) Could you resend the above patches to me? thx, Jason.
diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c index 19ab6ff..8aa6cdd 100644 --- a/drivers/bus/mvebu-mbus.c +++ b/drivers/bus/mvebu-mbus.c @@ -861,11 +861,13 @@ static void __init mvebu_mbus_get_pcie_resources(struct device_node *np, int ret; /* - * These are optional, so we clear them and they'll - * be zero if they are missing from the DT. + * These are optional, so we make sure that resource_size(x) will + * return 0. */ memset(mem, 0, sizeof(struct resource)); + mem->end = -1; memset(io, 0, sizeof(struct resource)); + io->end = -1; ret = of_property_read_u32_array(np, "pcie-mem-aperture", reg, ARRAY_SIZE(reg)); if (!ret) {
If the property was not specified then then the returned resource had a resource_size(..) == 1, rather than 0. The PCI-E driver checks for 0 so it blindly continues on with a corrupted resource. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> --- drivers/bus/mvebu-mbus.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) - Revise the comment to clarify the code is setting resource_size(x) to 0 [Thomas] - Use -1 instead of -- for clarity