Re: PCIe IO space support on Tilera GX: Is there any one who canconfirm my modification to fix it is OK?

From: Chris Metcalf
Date: Fri Oct 26 2012 - 10:08:46 EST


On 10/26/2012 4:03 AM, Bjorn Helgaas wrote:
> [+cc Chris, also a few comments below]

Bjorn, thanks for looping me in.

> On Fri, Oct 26, 2012 at 12:59 AM, Cyberman Wu <cypher.w@xxxxxxxxx> wrote:
>> After we upgrade to MDE 4.1.0 from Tilera, we encounter a problem that
>> only on HighPoint 2680 card works, I've
>> tried to fix it, but since most time I'm working in user space, I'm
>> not sure my fix is enough. Their FAE said that
>> the guy who add PCIe I/O space support is on vacation and I can't get
>> help from him now, I hope maybe there
>> will have somebody can help.

I asked internally and I think the response provided by Tilera was more
along the lines of "it may take a bit longer to get you an answer". :-)

> Per http://lkml.indiana.edu/hypermail/linux/kernel/1205.1/01176.html,
> Chris considered adding I/O space support and decided against it at
> that time, partly because it would use up a TRIO PIO region.
>
> I don't know his current thoughts. Possibly it could be done under a
> config option or something.

In the end, we did code up I/O port support in the root complex driver,
since we had a customer with a device that needed it. We haven't yet
returned it to the community - getting the basic PCI root complex support
returned (and networking support) took way more time than I had anticipated
so I'm batching up the next round of stuff to return for later. But
"later" is probably going to come fairly soon since it would make sense to
target the 3.8 merge window.

>> It works now. But I really need some one to confirm whether my
>> modification is enough or not,
>> if there have other potential problems.

Cyberman: it seems like your bias hack is working for you. But, as Bjorn
says, this sounds like a driver bug. What happens if you just revert your
changes, but then in mvsas.c change the "if (!res_start || !res_len)" to
just say "if (!res_len)"? That seems like the true error test. If that
works, you should submit that change to the community.

Bjorn et al: does it seem reasonable to add a bias to the mappings so that
we never report a zero value as valid? This may be sufficiently defensive
programming that it's just the right thing to do regardless of whether
drivers are technically at fault or not. If so, what's a good bias? (I'm
inclined to think 64K rather than 4K.)

Thanks!

--
Chris Metcalf, Tilera Corp.
http://www.tilera.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/