Re: [PATCH] ata_piix: ignore all zero PCS value on ICH5's

[Jeff Garzik <jgarzik@pobox.com>]

Tejun Heo wrote:
> Some ICH5's report 0 for PCS even when they have active working ports.
> This patch makes piix_sata_probe() ignore 0 PCS value on ICH5's and
> probe all ports in such cases.  And while at it, remove bogus
> assignment to mask.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> --
> 
> Hi, Jean-Francois (sorry, cannot type that character) and 0602.
> 
> Can you guys please verify that this patch works?  This patch is
> supposed to do the same thing as my previous patch with
> override_PCS=1.
> 
> 
> diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
> index 887b2b9..482995b 100644
> --- a/drivers/scsi/ata_piix.c
> +++ b/drivers/scsi/ata_piix.c
> @@ -58,6 +58,7 @@ enum {
>  	ICH5_PCS		= 0x92,	/* port control and status */
>  	PIIX_SCC		= 0x0A, /* sub-class code register */
>  
> +	PIIX_FLAG_UNRELIABLE_PCS= (1 << 27), /* PCS bits are unreliable */
>  	PIIX_FLAG_AHCI		= (1 << 28), /* AHCI possible */
>  	PIIX_FLAG_CHECKINTR	= (1 << 29), /* make sure PCI INTx enabled */
>  	PIIX_FLAG_COMBINED	= (1 << 30), /* combined mode possible */
> @@ -223,7 +224,8 @@ static struct ata_port_info piix_port_in
>  	{
>  		.sht		= &piix_sht,
>  		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST |
> -				  PIIX_FLAG_COMBINED | PIIX_FLAG_CHECKINTR,
> +				  PIIX_FLAG_COMBINED | PIIX_FLAG_CHECKINTR |
> +				  PIIX_FLAG_UNRELIABLE_PCS,
>  		.pio_mask	= 0x1f,	/* pio0-4 */
>  		.mwdma_mask	= 0x07, /* mwdma0-2 */
>  		.udma_mask	= 0x7f,	/* udma0-6 */
> @@ -362,12 +364,15 @@ static int piix_sata_probe (struct ata_p
>  	int orig_mask, mask, i;
>  	u8 pcs;
>  
> -	mask = (PIIX_PORT_PRESENT << ap->hard_port_no) |
> -	       (PIIX_PORT_ENABLED << ap->hard_port_no);
> -
>  	pci_read_config_byte(pdev, ICH5_PCS, &pcs);
>  	orig_mask = (int) pcs & 0xff;
>  
> +	if (orig_mask == 0 && ap->flags & PIIX_FLAG_UNRELIABLE_PCS) {
> +		printk(KERN_INFO "ata%u: PIIX PCS reports 0x00, ignoring PCS\n",
> +		       ap->id);
> +		return 1;
> +	}
> +

I'm just not too thrilled about this.  I wonder if its an isolated 
incident related to power or something.  I would wait for more problem 
reports, before applying this.

	Jeff