Re: What's needed for PMP support?
Heck, if .dev_select() took a *device* instead of a *port* as it's parameter, then I could probably manage it fine in there. dev_select gets called during probing before the relevant structures are neccessarily set up. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's needed for PMP support?
Tejun Heo wrote: Hello, Mark. Mark Lord wrote: Tejun, I've added PMP to sata_mv, and am now trying to get it to work with a Marvell PM attached. And the behaviour I see is very bizarre. After hard+soft resets, the PM signature is found, and libata interrogates the PM registers. It successfully reads register 0, and then register 1. But all subsequent registers read out (incorrectly) as zeros. Hmmm... I've traced the taskfiles in/out, and it all looks proper except for the actual data coming back from the PM. After some experimentation, I found that all of the PM registers were readable, if I simple inserted a sata_pmp_read(link, 0, junk) in front of each issue of sata_pmp_read(link, reg, r_val). Hmmm... Then the PM is recognized and all, but fails port enumerations probably due to either my hack or the same original bug (whatever that is?) I'm confused. That makes two of us. The PM itself works fine here on sata_sil24 and AHCI(jmicron), .. Well, last night I found someone else to try the code with a 0x1095:0x3726 (vendor/device?) Port-Multiplier. It *appears* to work fine with sata_mv, unlike the Marvell one. Hopefully the nice folks at Marvell will take a look (I've emailed them) and report back with some useful information eventually. No but I had my fair share of problems doing PMP support for both sil24 and ahci. Any chance you can get access to a bus analyzer? I had a very weird problem w/ ahci PMP support which I don't think I could fix if I didn't have access to a bus analyzer at the time. (I don't remember details now tho.) .. I figure they must have a few of those kicking around at Marvell.. Cheers -- Mark Lord Real-Time Remedies Inc. [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's needed for PMP support?
Tejun Heo wrote:
The following things are needed for a LLD to support PMP.
..
I think that's about it. Feel free to ask if something isn't clear.
..
I think we need better semantics around sata_scr_{read,write}(),
or more specifically
These need to be moved into ata_port_operations
so that LLDs can wrap them to properly manage
the host controller's global link-pmp value.
The problem I've been debugging here for the past 24hrs,
is that sata_mv sets the pmp number globally in hardware,
but then libata does a call to sata_scr_read()
which causes it to change. Without ever changing it back.
Subsequent accesses of shadow registers now point at
the pmp==15 instead of the original PM port.
Doh! No wonder device detection fails for me.
The LLD needs a way to properly manage the current pmp selection,
without having to clone all of the reset logic from libata-core.
I'd like to just re-use that code, but I cannot if it's going
to muck up the pmp selection.
I *think* ata_link_online() is my immediate problem.
It gets called from inside ata_std_softreset(),
and it invokes sata_scr_read().
This prevents me from re-using ata_std_softreset(),
and all of the non-exported functions that *it* calls.
There's very little that's special in the LLD for pmp support,
but the amount of code required seems huge, just to cope with
this simple problem caused.
Ugh.
If sata_scr_{read,write}() were in ata_port_operations,
then I could wrap them to save/restore the original pmp value.
But I'm not sure if that would result in a race against
other command-issue paths on the same port (?).
Tejun ???
For now, I'll try to hack it into sata_mv locally, somehow.
Suggestions?
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's needed for PMP support?
Mark Lord wrote:
Tejun Heo wrote:
The following things are needed for a LLD to support PMP.
..
I think that's about it. Feel free to ask if something isn't clear.
..
I think we need better semantics around sata_scr_{read,write}(),
or more specifically
These need to be moved into ata_port_operations
so that LLDs can wrap them to properly manage
the host controller's global link-pmp value.
..
Heck, if .dev_select() took a *device* instead of a *port*
as it's parameter, then I could probably manage it fine in there.
-ml
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's needed for PMP support?
Mark Lord wrote: Heh... I never thought a PMP aware controller would use TF SRST, so what you want to do is set pmp value in the register and calling ata_std_softreset(), right? I think the correct thing to do is to separate out SRST sequence proper from ata_std_softreset() into, say, ata_sff_SRST() and build custom softreset around it. After all, the problem here is the reset sequence not the SCR access. .. Actually, I believe the problem *is* the (pmp) SCR access. The same issue will return again when trying to support hotplug, for example. Can you elaborate a bit? Any SCR access will steal the active pmp on such hosts. I think we really do need to snoop those, somehow. Adding -sata_pmp_scr_read/write should do but I wanna avoid that if possible. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's needed for PMP support?
Mark Lord wrote:
Mark Lord wrote:
Tejun Heo wrote:
The following things are needed for a LLD to support PMP.
..
I think that's about it. Feel free to ask if something isn't clear.
..
I think we need better semantics around sata_scr_{read,write}(),
or more specifically
These need to be moved into ata_port_operations
so that LLDs can wrap them to properly manage
the host controller's global link-pmp value.
..
Heck, if .dev_select() took a *device* instead of a *port*
as it's parameter, then I could probably manage it fine in there.
Heh... I never thought a PMP aware controller would use TF SRST, so what
you want to do is set pmp value in the register and calling
ata_std_softreset(), right? I think the correct thing to do is to
separate out SRST sequence proper from ata_std_softreset() into, say,
ata_sff_SRST() and build custom softreset around it. After all, the
problem here is the reset sequence not the SCR access.
Thanks.
--
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's needed for PMP support?
Tejun Heo wrote:
Mark Lord wrote:
Heh... I never thought a PMP aware controller would use TF SRST, so what
you want to do is set pmp value in the register and calling
ata_std_softreset(), right? I think the correct thing to do is to
separate out SRST sequence proper from ata_std_softreset() into, say,
ata_sff_SRST() and build custom softreset around it. After all, the
problem here is the reset sequence not the SCR access.
..
Actually, I believe the problem *is* the (pmp) SCR access.
The same issue will return again when trying to support hotplug, for
example.
Can you elaborate a bit?
..
For example: a simple call to ata_link_online() is enough to mess it up.
Say, from hotplug polling.
Or an SEMB access (if not now, then someday when we implement it)
Any SCR access will steal the active pmp on such hosts.
I think we really do need to snoop those, somehow.
Adding -sata_pmp_scr_read/write should do but I wanna avoid that if
possible.
..
We already have .pmp_scr_{read,write} operations.
If NULL, then default to the built-ins that are there now.
I'll try it here first, though, and make sure it solves things.
Cheers
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's needed for PMP support?
Mark Lord wrote:
Tejun Heo wrote:
Mark Lord wrote:
Heh... I never thought a PMP aware controller would use TF SRST, so
what
you want to do is set pmp value in the register and calling
ata_std_softreset(), right? I think the correct thing to do is to
separate out SRST sequence proper from ata_std_softreset() into, say,
ata_sff_SRST() and build custom softreset around it. After all, the
problem here is the reset sequence not the SCR access.
..
Actually, I believe the problem *is* the (pmp) SCR access.
The same issue will return again when trying to support hotplug, for
example.
Can you elaborate a bit?
..
For example: a simple call to ata_link_online() is enough to mess it up.
Say, from hotplug polling.
Or an SEMB access (if not now, then someday when we implement it)
Any SCR access will steal the active pmp on such hosts.
I think we really do need to snoop those, somehow.
Adding -sata_pmp_scr_read/write should do but I wanna avoid that if
possible.
..
We already have .pmp_scr_{read,write} operations.
If NULL, then default to the built-ins that are there now.
..
Mmm.. lost some lines there, try again:
We already have .scr_{read,write} operations, and
what I think we need are .pmp_scr_{read,write} in addition.
If NULL, then default to the built-ins that are there now.
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's needed for PMP support?
Mark Lord wrote:
Mark Lord wrote:
..
We already have .pmp_scr_{read,write} operations.
If NULL, then default to the built-ins that are there now.
..
Mmm.. lost some lines there, try again:
We already have .scr_{read,write} operations, and
what I think we need are .pmp_scr_{read,write} in addition.
If NULL, then default to the built-ins that are there now.
Tejun Heo wrote:
Adding -sata_pmp_scr_read/write should do but I wanna avoid that if possible.
..
Well, what it really needed was .pmp_read() and .pmp_write().
I implemented that, and (finally!!) sata_mv works perfectly
with the Marvell port multiplier I have here.
All other attempts to do this entirely within sata_mv
just left me (and the hardware) dazed and confused.
But being able to wrap sata_pmp_read/write() with save/restore
logic fixed things enough to work on the first try.
Whew!
I'll post a summary patch once I get it generated here.
Lots of debug code to back out first.
Thanks Tejun, Saeed!
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's needed for PMP support?
Tejun Heo wrote: The following things are needed for a LLD to support PMP. .. I think that's about it. Feel free to ask if something isn't clear. .. Tejun, I've added PMP to sata_mv, and am now trying to get it to work with a Marvell PM attached. And the behaviour I see is very bizarre. After hard+soft resets, the PM signature is found, and libata interrogates the PM registers. It successfully reads register 0, and then register 1. But all subsequent registers read out (incorrectly) as zeros. I've traced the taskfiles in/out, and it all looks proper except for the actual data coming back from the PM. After some experimentation, I found that all of the PM registers were readable, if I simple inserted a sata_pmp_read(link, 0, junk) in front of each issue of sata_pmp_read(link, reg, r_val). Then the PM is recognized and all, but fails port enumerations probably due to either my hack or the same original bug (whatever that is?) I'm confused. The PM itself works fine here on sata_sil24 and AHCI(jmicron), so it's obviously the sata_mv driver or chipset that's being weird. Ever seen anything like this before? I'm trying to use stock libata functions for all of this where possible, so there's not really that much new/necessary code in sata_mv for this. I do force the PMP value for outgoing-FIS's (non standard register for it), but that's about all that's custom here. The driver uses ATA register mode emulation for all commands other than READ/WRITE disk stuff, including for the PM register accesses. Ever seen anything like this before? Cheers -- Mark Lord Real-Time Remedies Inc. [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
