linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] sata_promise: handle TX2plus PATA locally
@ 2007-01-07 18:32 Mikael Pettersson
  2007-01-08  1:38 ` Jeff Garzik
  0 siblings, 1 reply; 3+ messages in thread
From: Mikael Pettersson @ 2007-01-07 18:32 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: linux-ide

This is an RFC on a simpler and less intrusive way of setting
up port flags on the mixed SATA/PATA Promise TX2plus chips.

The #promise-sata-pata branch requires libata core to be updated
with a per-port flags array in the ata_probe_ent structure. I
guess you don't approve of that solution.

This alternative patch is based on the observation that ap->flags
isn't really used until after ->port_start() has been invoked.
So this patch places the "exceptional" per-port flags array in
the driver's private host structure, and uses it in ->port_start()
to finalise the flags.

This patch also eliminates #promise-sata-pata's redundant code
formatting changes, and eliminates the redundant port_flags[]
assignments for the SATA-only TX4 chips.

Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>

--- linux-2.6.20-rc4/drivers/ata/sata_promise.c.~1~	2007-01-07 18:17:02.000000000 +0100
+++ linux-2.6.20-rc4/drivers/ata/sata_promise.c	2007-01-07 18:25:33.000000000 +0100
@@ -92,6 +92,7 @@ struct pdc_port_priv {
 
 struct pdc_host_priv {
 	unsigned long		flags;
+	unsigned long		port_flags[ATA_MAX_PORTS];
 };
 
 static u32 pdc_sata_scr_read (struct ata_port *ap, unsigned int sc_reg);
@@ -183,7 +184,7 @@ static const struct ata_port_info pdc_po
 	/* board_2037x */
 	{
 		.sht		= &pdc_ata_sht,
-		.flags		= PDC_COMMON_FLAGS | ATA_FLAG_SATA,
+		.flags		= PDC_COMMON_FLAGS,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
@@ -213,7 +214,7 @@ static const struct ata_port_info pdc_po
 	/* board_2057x */
 	{
 		.sht		= &pdc_ata_sht,
-		.flags		= PDC_COMMON_FLAGS | ATA_FLAG_SATA,
+		.flags		= PDC_COMMON_FLAGS,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
@@ -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 heterogeneous chips like TX2plus */
+	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->flags & ATA_FLAG_SLAVE_POSS))
 		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->flags & ATA_FLAG_SLAVE_POSS))
 		return;
 	writel(val, (void __iomem *) ap->ioaddr.scr_addr + (sc_reg * 4));
 }
@@ -740,6 +746,7 @@ static int pdc_ata_init_one (struct pci_
 	unsigned int board_idx = (unsigned int) ent->driver_data;
 	int pci_dev_busy = 0;
 	int rc;
+	u8 tmp;
 
 	if (!printed_version++)
 		dev_printk(KERN_DEBUG, &pdev->dev, "version " DRV_VERSION "\n");
@@ -820,7 +827,17 @@ static int pdc_ata_init_one (struct pci_
 		hp->flags |= PDC_FLAG_GEN_II;
 		/* Fall through */
 	case board_2037x:
-		probe_ent->n_ports = 2;
+		/* TX2plus boards also have a PATA port */
+		tmp = readb(mmio_base + PDC_FLASH_CTL+1);
+		if (!(tmp & 0x80)) {
+			probe_ent->n_ports = 3;
+			pdc_ata_setup_port(&probe_ent->port[2], base + 0x300);
+			hp->port_flags[2] = ATA_FLAG_SLAVE_POSS;
+			printk(KERN_INFO DRV_NAME " PATA port found\n");
+		} else
+			probe_ent->n_ports = 2;
+		hp->port_flags[0] = ATA_FLAG_SATA;
+		hp->port_flags[1] = ATA_FLAG_SATA;
 		break;
 	case board_20619:
 		probe_ent->n_ports = 4;

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC] sata_promise: handle TX2plus PATA locally
  2007-01-07 18:32 [RFC] sata_promise: handle TX2plus PATA locally Mikael Pettersson
@ 2007-01-08  1:38 ` Jeff Garzik
  0 siblings, 0 replies; 3+ messages in thread
From: Jeff Garzik @ 2007-01-08  1:38 UTC (permalink / raw)
  To: Mikael Pettersson; +Cc: linux-ide, Alan Cox

Mikael Pettersson wrote:
> This is an RFC on a simpler and less intrusive way of setting
> up port flags on the mixed SATA/PATA Promise TX2plus chips.
> 
> The #promise-sata-pata branch requires libata core to be updated
> with a per-port flags array in the ata_probe_ent structure. I
> guess you don't approve of that solution.

Correct.

The long explanation:  struct ata_host/ata_port support different cable 
types and ata_port_operations sets.  Unfortunately, struct ata_probe_ent 
does a very poor job of making this capability available to drivers. 
The long term plan to support PATA+SATA combinations like sata_promise, 
sata_sis, sata_via etc. is to kill ata_probe_ent, and move to a 
commonly-understood probe+init model:  (1) allocate struct, (2) probe 
hardware and fill in struct, and (3) register struct with kernel 
subsystem.  Both net drivers and SCSI drivers use this model of API.


> This alternative patch is based on the observation that ap->flags
> isn't really used until after ->port_start() has been invoked.
> So this patch places the "exceptional" per-port flags array in
> the driver's private host structure, and uses it in ->port_start()
> to finalise the flags.

I agree with this approach, modulo a minor change...


> @@ -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->flags & ATA_FLAG_SLAVE_POSS))
>  		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->flags & ATA_FLAG_SLAVE_POSS))
>  		return;
>  	writel(val, (void __iomem *) ap->ioaddr.scr_addr + (sc_reg * 4));
>  }

Seems like a better test than ATA_FLAG_SLAVE_POSS would be to test the 
cable type.

	Jeff



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC] sata_promise: handle TX2plus PATA locally
@ 2007-01-08  8:06 Mikael Pettersson
  0 siblings, 0 replies; 3+ messages in thread
From: Mikael Pettersson @ 2007-01-08  8:06 UTC (permalink / raw)
  To: jeff; +Cc: alan, linux-ide

On Sun, 07 Jan 2007 20:38:56 -0500, Jeff Garzik wrote:
> > This alternative patch is based on the observation that ap->flags
> > isn't really used until after ->port_start() has been invoked.
> > So this patch places the "exceptional" per-port flags array in
> > the driver's private host structure, and uses it in ->port_start()
> > to finalise the flags.
> 
> I agree with this approach, modulo a minor change...

Excellent.

> > @@ -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->flags & ATA_FLAG_SLAVE_POSS))
> >  		return;
> >  	writel(val, (void __iomem *) ap->ioaddr.scr_addr + (sc_reg * 4));
> >  }
> 
> Seems like a better test than ATA_FLAG_SLAVE_POSS would be to test the 
> cable type.

Agreed. This test is what #promise-sata-pata does now, but
I'll change it inspect the cable type instead.

/Mikael

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2007-01-08  8:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-01-07 18:32 [RFC] sata_promise: handle TX2plus PATA locally Mikael Pettersson
2007-01-08  1:38 ` Jeff Garzik
  -- strict thread matches above, loose matches on Subject: below --
2007-01-08  8:06 Mikael Pettersson

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).