Re: [PATCH] libata-pmp: clear hob for pmp register accesses

["saeed bishara" <saeed.bishara@gmail.com>]

Hi,
 I just what to make it clear that this is not a bug of the Marvell's
PM, the sata specification (I'm looking at version 2.5) defines the
READ/WRITE PM registers to be extended commands, specifically, the
features HOB of the READ PM register defined to be RegNum[15:8].

saeed

On Thu, Feb 21, 2008 at 10:52 PM, Mark Lord <liml@rtr.ca> wrote:
> Mark Lord wrote:
>  > 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.
>  ..
>
>  Saeed has confirmed this behaviour with a SATA analyzer.
>  The Marvell port-multiplier apparently likes to see clean HOB
>  information when accessing PMP registers.
>
>  Since sata_mv uses PIO shadow register access, this doesn't happen
>  automatically, as it might in a more purely FIS-based driver (eg. ahci).
>
>  One way to fix this is to flag these commands with ATA_TFLAG_LBA48,
>  forcing libata to write out the HOB fields with known (zero) values.
>
>  Signed-off-by: Saeed Bishara <saeed@marvell.com>
>  Acked-by: Mark Lord <mlord@pobox.com>
>  ---
>   drivers/ata/libata-pmp.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
>  diff --git a/drivers/ata/libata-pmp.c b/drivers/ata/libata-pmp.c
>  index caef2bb..d91f509 100644
>  --- a/drivers/ata/libata-pmp.c
>  +++ b/drivers/ata/libata-pmp.c
>  @@ -35,7 +35,7 @@ static unsigned int sata_pmp_read(struct ata_link *link, int reg, u32 *r_val)
>         ata_tf_init(pmp_dev, &tf);
>         tf.command = ATA_CMD_PMP_READ;
>         tf.protocol = ATA_PROT_NODATA;
>  -       tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
>  +       tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48;
>         tf.feature = reg;
>         tf.device = link->pmp;
>
>  @@ -71,7 +71,7 @@ static unsigned int sata_pmp_write(struct ata_link *link, int reg, u32 val)
>         ata_tf_init(pmp_dev, &tf);
>         tf.command = ATA_CMD_PMP_WRITE;
>         tf.protocol = ATA_PROT_NODATA;
>  -       tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
>  +       tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48;
>         tf.feature = reg;
>         tf.device = link->pmp;
>         tf.nsect = val & 0xff;
>  -
>  To unsubscribe from this list: send the line "unsubscribe linux-ide" in
>  the body of a message to majordomo@vger.kernel.org
>  More majordomo info at  http://vger.kernel.org/majordomo-info.html
>