[PATCH 1/1] pata_via.c: Support VX855 and future chips whose IDE controller use 0x0571.

[PATCH 1/1] pata_via.c: Support VX855 and future chips whose IDE controller use 0x0571.

[JosephChan]

This patch based on 2.6.28-git10.
It supports VX855 and future chips whose IDE controller use 0x0571.


Signed-off-by: Joseph Chan <josephchan@via.com.tw>

--- a/include/linux/pci_ids.h	2009-01-09 23:28:18.000000000 +0800
+++ b/include/linux/pci_ids.h	2009-01-10 00:35:15.000000000 +0800
@@ -1370,6 +1370,7 @@
 #define PCI_DEVICE_ID_VIA_82C598_1	0x8598
 #define PCI_DEVICE_ID_VIA_838X_1	0xB188
 #define PCI_DEVICE_ID_VIA_83_87XX_1	0xB198
+#define PCI_DEVICE_ID_VIA_ANON		0xFFFF
 
 #define PCI_VENDOR_ID_SIEMENS           0x110A
 #define PCI_DEVICE_ID_SIEMENS_DSCC4     0x2102
--- a/drivers/ata/pata_via.c	2009-01-10 00:35:04.000000000 +0800
+++ b/drivers/ata/pata_via.c	2009-01-10 00:35:15.000000000 +0800
@@ -97,6 +97,8 @@
 	u8 rev_max;
 	u16 flags;
 } via_isa_bridges[] = {
+	{ "vtxxxx",	PCI_DEVICE_ID_VIA_ANON,    0x00, 0x2f, VIA_UDMA_133 |
+	VIA_BAD_AST },
 	{ "vx800",	PCI_DEVICE_ID_VIA_VX800,    0x00, 0x2f, VIA_UDMA_133 |
 	VIA_BAD_AST | VIA_SATA_PATA },
 	{ "vt8237s",	PCI_DEVICE_ID_VIA_8237S,    0x00, 0x2f, VIA_UDMA_133 | VIA_BAD_AST },
@@ -176,6 +178,14 @@
 	if ((config->flags & VIA_SATA_PATA) && ap->port_no == 0)
 		return ATA_CBL_SATA;
 
+	if (ap->port_no == 0 && pdev->device == 0xC409) {
+		pci_read_config_dword(pdev, 0x52, &ata66);
+		return (ata66 & 0x10)  ? ATA_CBL_PATA80 : ATA_CBL_PATA40;
+	} else if (ap->port_no == 1 && pdev->device == 0xC409) {
+		DPRINTK("C409 only has one pata channel\n");
+		return ATA_CBL_PATA_UNK;
+	}
+
 	/* Early chips are 40 wire */
 	if ((config->flags & VIA_UDMA) < VIA_UDMA_66)
 		return ATA_CBL_PATA40;
@@ -335,13 +345,13 @@
  */
 static void via_tf_load(struct ata_port *ap, const struct ata_taskfile *tf)
 {
-	struct ata_taskfile tmp_tf;
+	struct ata_ioports *ioaddr = &ap->ioaddr;
 
-	if (ap->ctl != ap->last_ctl && !(tf->flags & ATA_TFLAG_DEVICE)) {
-		tmp_tf = *tf;
-		tmp_tf.flags |= ATA_TFLAG_DEVICE;
-		tf = &tmp_tf;
+	if (tf->ctl != ap->last_ctl) {
+		iowrite8(tf->ctl, ioaddr->ctl_addr);
+		iowrite8(tf->device, ioaddr->device_addr);
 	}
+
 	ata_sff_tf_load(ap, tf);
 }
 
@@ -484,9 +494,9 @@
 
 	if (!config->id) {
 		printk(KERN_WARNING "via: Unknown VIA SouthBridge, disabling.\n");
-		return -ENODEV;
-	}
-	pci_dev_put(isa);
+		config = via_isa_bridges;
+	} else
+		pci_dev_put(isa);
 
 	if (!(config->flags & VIA_NO_ENABLES)) {
 		/* 0x40 low bits indicate enabled channels */
@@ -587,6 +597,7 @@
 	{ PCI_VDEVICE(VIA, 0x1571), },
 	{ PCI_VDEVICE(VIA, 0x3164), },
 	{ PCI_VDEVICE(VIA, 0x5324), },
+	{ PCI_VDEVICE(VIA, 0xC409), },
 
 	{ },
 };

Re: [PATCH 1/1] pata_via.c: Support VX855 and future chips whose IDE controller use 0x0571.

[Sergei Shtylyov]

Hello.

JosephChan@via.com.tw wrote:

> This patch based on 2.6.28-git10.
> It supports VX855 and future chips whose IDE controller use 0x0571.
>
> Signed-off-by: Joseph Chan <josephchan@via.com.tw>
>
> --- a/include/linux/pci_ids.h	2009-01-09 23:28:18.000000000 +0800
> +++ b/include/linux/pci_ids.h	2009-01-10 00:35:15.000000000 +0800
> @@ -1370,6 +1370,7 @@
>  #define PCI_DEVICE_ID_VIA_82C598_1	0x8598
>  #define PCI_DEVICE_ID_VIA_838X_1	0xB188
>  #define PCI_DEVICE_ID_VIA_83_87XX_1	0xB198
> +#define PCI_DEVICE_ID_VIA_ANON		0xFFFF
>  
>  #define PCI_VENDOR_ID_SIEMENS           0x110A
>  #define PCI_DEVICE_ID_SIEMENS_DSCC4     0x2102
> --- a/drivers/ata/pata_via.c	2009-01-10 00:35:04.000000000 +0800
> +++ b/drivers/ata/pata_via.c	2009-01-10 00:35:15.000000000 +0800
> @@ -97,6 +97,8 @@
>  	u8 rev_max;
>  	u16 flags;
>  } via_isa_bridges[] = {
> +	{ "vtxxxx",	PCI_DEVICE_ID_VIA_ANON,    0x00, 0x2f, VIA_UDMA_133 |
> +	VIA_BAD_AST },
>  	{ "vx800",	PCI_DEVICE_ID_VIA_VX800,    0x00, 0x2f, VIA_UDMA_133 |
>  	VIA_BAD_AST | VIA_SATA_PATA },
>  	{ "vt8237s",	PCI_DEVICE_ID_VIA_8237S,    0x00, 0x2f, VIA_UDMA_133 | VIA_BAD_AST },
> @@ -176,6 +178,14 @@
>  	if ((config->flags & VIA_SATA_PATA) && ap->port_no == 0)
>  		return ATA_CBL_SATA;
>  
> +	if (ap->port_no == 0 && pdev->device == 0xC409) {
> +		pci_read_config_dword(pdev, 0x52, &ata66);
> +		return (ata66 & 0x10)  ? ATA_CBL_PATA80 : ATA_CBL_PATA40;
> +	} else if (ap->port_no == 1 && pdev->device == 0xC409) {
> +		DPRINTK("C409 only has one pata channel\n");
> +		return ATA_CBL_PATA_UNK;
> +	}
> +
>   

   Why not:

	if (pdev->device == 0xC409)
		if (ap->port_no == 0) {
			pci_read_config_dword(pdev, 0x52, &ata66);
			return (ata66 & 0x10)  ? ATA_CBL_PATA80 : ATA_CBL_PATA40;
		} else if (ap->port_no == 1) {
			DPRINTK("C409 only has one pata channel\n");
			return ATA_CBL_PATA_UNK;
	}


> @@ -335,13 +345,13 @@
>   */
>  static void via_tf_load(struct ata_port *ap, const struct ata_taskfile *tf)
>  {
> -	struct ata_taskfile tmp_tf;
> +	struct ata_ioports *ioaddr = &ap->ioaddr;
>  
> -	if (ap->ctl != ap->last_ctl && !(tf->flags & ATA_TFLAG_DEVICE)) {
> -		tmp_tf = *tf;
> -		tmp_tf.flags |= ATA_TFLAG_DEVICE;
> -		tf = &tmp_tf;
> +	if (tf->ctl != ap->last_ctl) {
> +		iowrite8(tf->ctl, ioaddr->ctl_addr);
> +		iowrite8(tf->device, ioaddr->device_addr);
>  	}
> +
>  	ata_sff_tf_load(ap, tf);
>  }
>   

   This looks like an unrelated fix...

MBR, Sergei

Re: [PATCH 1/1] pata_via.c: Support VX855 and future chips whose IDE controller use 0x0571.

[Alan Cox]

>    Why not:
> 
> 	if (pdev->device == 0xC409)
> 		if (ap->port_no == 0) {
> 			pci_read_config_dword(pdev, 0x52, &ata66);

More to the point - why not just register it as a single port device ?

Re: [PATCH 1/1] pata_via.c: Support VX855 and future chips whose IDE controller use 0x0571.

[Tejun Heo]

Hello, Joseph.

JosephChan@via.com.tw wrote:
> This patch based on 2.6.28-git10.
> It supports VX855 and future chips whose IDE controller use 0x0571.
> 
> Signed-off-by: Joseph Chan <josephchan@via.com.tw>
> 
> --- a/drivers/ata/pata_via.c	2009-01-10 00:35:04.000000000 +0800
> +++ b/drivers/ata/pata_via.c	2009-01-10 00:35:15.000000000 +0800
> @@ -97,6 +97,8 @@
>  	u8 rev_max;
>  	u16 flags;
>  } via_isa_bridges[] = {
> +	{ "vtxxxx",	PCI_DEVICE_ID_VIA_ANON,    0x00, 0x2f, VIA_UDMA_133 |
> +	VIA_BAD_AST },
>  	{ "vx800",	PCI_DEVICE_ID_VIA_VX800,    0x00, 0x2f, VIA_UDMA_133 |
>  	VIA_BAD_AST | VIA_SATA_PATA },
>  	{ "vt8237s",	PCI_DEVICE_ID_VIA_8237S,    0x00, 0x2f, VIA_UDMA_133 | VIA_BAD_AST },
> @@ -176,6 +178,14 @@
>  	if ((config->flags & VIA_SATA_PATA) && ap->port_no == 0)
>  		return ATA_CBL_SATA;
>  
> +	if (ap->port_no == 0 && pdev->device == 0xC409) {
> +		pci_read_config_dword(pdev, 0x52, &ata66);
> +		return (ata66 & 0x10)  ? ATA_CBL_PATA80 : ATA_CBL_PATA40;
> +	} else if (ap->port_no == 1 && pdev->device == 0xC409) {
> +		DPRINTK("C409 only has one pata channel\n");
> +		return ATA_CBL_PATA_UNK;
> +	}

Eh.. can you add 0xC409 entry like other controllers and put necessary
restrictions there?

>  	/* Early chips are 40 wire */
>  	if ((config->flags & VIA_UDMA) < VIA_UDMA_66)
>  		return ATA_CBL_PATA40;
> @@ -335,13 +345,13 @@
>   */
>  static void via_tf_load(struct ata_port *ap, const struct ata_taskfile *tf)
>  {
> -	struct ata_taskfile tmp_tf;
> +	struct ata_ioports *ioaddr = &ap->ioaddr;
>  
> -	if (ap->ctl != ap->last_ctl && !(tf->flags & ATA_TFLAG_DEVICE)) {
> -		tmp_tf = *tf;
> -		tmp_tf.flags |= ATA_TFLAG_DEVICE;
> -		tf = &tmp_tf;
> +	if (tf->ctl != ap->last_ctl) {
> +		iowrite8(tf->ctl, ioaddr->ctl_addr);
> +		iowrite8(tf->device, ioaddr->device_addr);
>  	}
> +
>  	ata_sff_tf_load(ap, tf);

Can you please explain why this change is necessary?  And if it's
necessary, please put it in a separate patch.

Thanks.

-- 
tejun

RE: [PATCH 1/1] pata_via.c: Support VX855 and future chips whose IDE controller use 0x0571.

[JosephChan]

> >    Why not:
> > 
> > 	if (pdev->device == 0xC409)
> > 		if (ap->port_no == 0) {
> > 			pci_read_config_dword(pdev, 0x52, &ata66);
> 
> More to the point - why not just register it as a single port device ?
> 

Thanks Alan.
We're checking this now, and make sure there is a fixed register for future chips.

RE: [PATCH 1/1] pata_via.c: Support VX855 and future chips whose IDE controller use 0x0571.

[JosephChan]

>    Why not:
> 
> 	if (pdev->device == 0xC409)
> 		if (ap->port_no == 0) {
> 			pci_read_config_dword(pdev, 0x52, &ata66);
> 			return (ata66 & 0x10)  ? ATA_CBL_PATA80 
> : ATA_CBL_PATA40;
> 		} else if (ap->port_no == 1) {
> 			DPRINTK("C409 only has one pata channel\n");
> 			return ATA_CBL_PATA_UNK;
> 	}
> 
> 

Thx Sergei. We might refer to Alan's opinion for next patch.

RE: [PATCH 1/1] pata_via.c: Support VX855 and future chips whose IDE controller use 0x0571.

[JosephChan]

> > +	if (ap->port_no == 0 && pdev->device == 0xC409) {
> > +		pci_read_config_dword(pdev, 0x52, &ata66);
> > +		return (ata66 & 0x10)  ? ATA_CBL_PATA80 : 
> ATA_CBL_PATA40;
> > +	} else if (ap->port_no == 1 && pdev->device == 0xC409) {
> > +		DPRINTK("C409 only has one pata channel\n");
> > +		return ATA_CBL_PATA_UNK;
> > +	}
> 
> Eh.. can you add 0xC409 entry like other controllers and put 
> necessary restrictions there?
> 

It's a pity that we don't have corresponding registers for identifying the single
port controller. So, we just can do this by checking the device ID at present.


> >  static void via_tf_load(struct ata_port *ap, const struct 
> > ata_taskfile *tf)  {
> > -	struct ata_taskfile tmp_tf;
> > +	struct ata_ioports *ioaddr = &ap->ioaddr;
> >  
> > -	if (ap->ctl != ap->last_ctl && !(tf->flags & 
> ATA_TFLAG_DEVICE)) {
> > -		tmp_tf = *tf;
> > -		tmp_tf.flags |= ATA_TFLAG_DEVICE;
> > -		tf = &tmp_tf;
> > +	if (tf->ctl != ap->last_ctl) {
> > +		iowrite8(tf->ctl, ioaddr->ctl_addr);
> > +		iowrite8(tf->device, ioaddr->device_addr);
> >  	}
> > +
> >  	ata_sff_tf_load(ap, tf);
> 
> Can you please explain why this change is necessary?  And if 
> it's necessary, please put it in a separate patch.
> 

Yes,  I will separate it.

Re: [PATCH 1/1] pata_via.c: Support VX855 and future chips whose IDE controller use 0x0571.

[Tejun Heo]

JosephChan@via.com.tw wrote:
>>> +	if (ap->port_no == 0 && pdev->device == 0xC409) {
>>> +		pci_read_config_dword(pdev, 0x52, &ata66);
>>> +		return (ata66 & 0x10)  ? ATA_CBL_PATA80 : 
>> ATA_CBL_PATA40;
>>> +	} else if (ap->port_no == 1 && pdev->device == 0xC409) {
>>> +		DPRINTK("C409 only has one pata channel\n");
>>> +		return ATA_CBL_PATA_UNK;
>>> +	}
>> Eh.. can you add 0xC409 entry like other controllers and put 
>> necessary restrictions there?
>>
> 
> It's a pity that we don't have corresponding registers for
> identifying the single port controller. So, we just can do this by
> checking the device ID at present.

I feel a bit confused so please correct me if I'm babbling.  Can't you
add { PCI_VDEVICE(VIA, 0xC409), FLAGS_OR_IDENTIFIER } to pci_device_id
table and use different port_info on it?  If that's too different from
how other devices are beign identified, you can just add the
corresponding logic to init_one() and use, say, via_single_port_info
there instead of doing it deep in the reset sequence.

>>>  static void via_tf_load(struct ata_port *ap, const struct 
>>> ata_taskfile *tf)  {
>>> -	struct ata_taskfile tmp_tf;
>>> +	struct ata_ioports *ioaddr = &ap->ioaddr;
>>>  
>>> -	if (ap->ctl != ap->last_ctl && !(tf->flags & 
>> ATA_TFLAG_DEVICE)) {
>>> -		tmp_tf = *tf;
>>> -		tmp_tf.flags |= ATA_TFLAG_DEVICE;
>>> -		tf = &tmp_tf;
>>> +	if (tf->ctl != ap->last_ctl) {
>>> +		iowrite8(tf->ctl, ioaddr->ctl_addr);
>>> +		iowrite8(tf->device, ioaddr->device_addr);
>>>  	}
>>> +
>>>  	ata_sff_tf_load(ap, tf);
>> Can you please explain why this change is necessary?  And if 
>> it's necessary, please put it in a separate patch.
>>
> 
> Yes,  I will separate it.

Thanks.

-- 
tejun