Re: [PATCH 2.6.20-rc4 1/2] sata_promise: TX2plus PATA support

[Jeff Garzik]

Mikael Pettersson wrote:
> @@ -271,6 +272,11 @@ static int pdc_port_start(struct ata_por
>  	struct pdc_port_priv *pp;
>  	int rc;
>  
> +	/* fix up port flags and cable type for SATA+PATA chips */
> +	ap->flags |= hp->port_flags[ap->port_no];
> +	if (ap->flags & ATA_FLAG_SATA)
> +		ap->cbl = ATA_CBL_SATA;
> +
>  	rc = ata_port_start(ap);
>  	if (rc)
>  		return rc;
> @@ -377,7 +383,7 @@ static void pdc_pata_phy_reset(struct at
>  
>  static u32 pdc_sata_scr_read (struct ata_port *ap, unsigned int sc_reg)
>  {
> -	if (sc_reg > SCR_CONTROL)
> +	if (sc_reg > SCR_CONTROL || ap->cbl != ATA_CBL_SATA)
>  		return 0xffffffffU;
>  	return readl((void __iomem *) ap->ioaddr.scr_addr + (sc_reg * 4));
>  }
> @@ -386,7 +392,7 @@ static u32 pdc_sata_scr_read (struct ata
>  static void pdc_sata_scr_write (struct ata_port *ap, unsigned int sc_reg,
>  			       u32 val)
>  {
> -	if (sc_reg > SCR_CONTROL)
> +	if (sc_reg > SCR_CONTROL || ap->cbl != ATA_CBL_SATA)
>  		return;
>  	writel(val, (void __iomem *) ap->ioaddr.scr_addr + (sc_reg * 4));
>  }


It would be nice to see a [tested] follow-up patch that separates SATA 
and PATA into two separate sets of ata_port_operations hooks.  That 
should eliminate these 'ap->cbl' tests, and some other tests.

You should be able to set ap->ops in the same manner as you are setting 
the flags now.

	Jeff


-
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/