From: Mark Lord <liml@rtr.ca>
To: Tejun Heo <htejun@gmail.com>
Cc: IDE/ATA development list <linux-ide@vger.kernel.org>,
Saeed Bishara <saeed@marvell.com>
Subject: Re: new ata_port_operations for .pmp_{read,write} ?
Date: Fri, 22 Feb 2008 09:23:46 -0500 [thread overview]
Message-ID: <47BEDAF2.8000301@rtr.ca> (raw)
In-Reply-To: <47BE4DFE.2030407@rtr.ca>
Mark Lord wrote:
> Tejun,
>
> Here's a first cut at this for discussion.
>
> You may prefer different names for the invoking functions
> inside libata-pmp.c, rather than simply pmp_read() and pmp_write(),
> but I've been up too long and couldn't think of a better name.
>
> An alternative to all this, might be to expose the "select_pmp()"
> function shown in the sample code, and have libata-pmp.c call that,
> instead of having the new new .pmp_{read,write} functions.
>
> What do you think?
>
> * * *
>
> SATA host controllers which are not purely FIS-based require setup
> of a special register to select the active pmp for taskfile accesses.
>
> This can be done in the low-level driver for regular command issue
> on a link. But commands directed at a port-multiplier cause problems,
> because they leave the original link pmp de-selected afterwards.
>
> To fix this in a sane fashion, without tons of code duplication
> from libata into the low-level driver, it is necessary to be able
> to wrap the sata_pmp_read() and sata_pmp_write() functions.
>
> This patch provides two new struct ata_port_operations methods for this,
> and modifies the code in libata-pmp to use them if provided.
...
Note that, while this does work for sata_mv, I'm still thinking about it.
I'm not totally clear yet (more reading to do) as to how/when
the ATA shadow/taskfile registers get updated to reflect those
for the currently selected pmp..
It would seem that with other parts of libata-sff directly reading
from the ctl, status, and altstatus "registers" during polling,
command setup, and probing, that there might (?) be a loophole
somewhere in this strategy.
Is this scenario going to be possible: somebody calls sata_pmp_read()
as part of, say, hotplug polling, and after that operation completes
we then have code that calls ata_check_status() prior to the next
tf_load / command issue ? If so, they'll see the wrong cached
shadow status register.
And for that matter, is it possible for sata_pmp_read() to be called
while the link is active with another command ? Not today, it seems,
but what about when hotplug polling gets implemented ?
Mmm.. an amazing amount of complexity there for something that
ought to be very simple.
More reading to do, but comments from Tejun/others are welcomed.
Thanks.
next prev parent reply other threads:[~2008-02-22 14:23 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-11-06 21:56 only one drive in a port multiplier system is being recognized Greg Hennessy
2007-11-12 2:58 ` Tejun Heo
[not found] ` <4738827D.9060405@pobox.com>
2007-11-13 1:09 ` What's needed for PMP support? Tejun Heo
2008-02-20 19:03 ` Mark Lord
2008-02-21 3:39 ` Tejun Heo
2008-02-21 15:07 ` Mark Lord
2008-02-21 20:52 ` [PATCH] libata-pmp: clear hob for pmp register accesses Mark Lord
2008-02-21 21:51 ` saeed bishara
2008-02-22 1:40 ` Tejun Heo
2008-02-24 5:29 ` Jeff Garzik
2008-02-22 0:31 ` What's needed for PMP support? Mark Lord
2008-02-22 0:32 ` Mark Lord
2008-02-22 1:57 ` Tejun Heo
2008-02-22 2:04 ` Mark Lord
2008-02-22 2:12 ` Tejun Heo
2008-02-22 2:25 ` Mark Lord
2008-02-22 2:27 ` Mark Lord
2008-02-22 3:52 ` Mark Lord
2008-02-22 4:22 ` new ata_port_operations for .pmp_{read,write} ? Mark Lord
2008-02-22 14:23 ` Mark Lord [this message]
2008-02-22 14:28 ` Mark Lord
2008-02-23 0:38 ` Mark Lord
2008-02-23 2:49 ` Tejun Heo
2008-02-23 2:43 ` Tejun Heo
2008-02-23 2:59 ` Jeff Garzik
2008-02-23 5:15 ` Mark Lord
2008-02-24 7:03 ` Tejun Heo
2008-02-24 7:14 ` Jeff Garzik
2008-02-25 4:34 ` Mark Lord
2008-02-25 4:46 ` Jeff Garzik
2008-02-25 4:31 ` Mark Lord
2008-02-25 4:49 ` Mark Lord
2008-02-25 4:56 ` Jeff Garzik
2008-02-25 5:20 ` Tejun Heo
2008-02-25 16:55 ` Mark Lord
2008-02-25 23:44 ` Tejun Heo
2008-02-26 0:12 ` Mark Lord
2008-02-26 2:01 ` Tejun Heo
2008-02-22 9:57 ` What's needed for PMP support? Alan Cox
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=47BEDAF2.8000301@rtr.ca \
--to=liml@rtr.ca \
--cc=htejun@gmail.com \
--cc=linux-ide@vger.kernel.org \
--cc=saeed@marvell.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).