[loongson-PATCH-v1 24/27] fixup for FUJITSU disk

[loongson-PATCH-v1 24/27] fixup for FUJITSU disk

[wuzhangjin]

From: Wu Zhangjin <wuzhangjin@gmail.com>

This is originally from the to-mips branch from
http://dev.lemote.com/code/linux_loongson

Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
---
 drivers/ide/amd74xx.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/ide/amd74xx.c b/drivers/ide/amd74xx.c
index 77267c8..8f488b8 100644
--- a/drivers/ide/amd74xx.c
+++ b/drivers/ide/amd74xx.c
@@ -23,6 +23,11 @@
 
 #define DRV_NAME "amd74xx"
 
+static const char *am74xx_quirk_drives[] = {
+	"FUJITSU MHZ2160BH G2",
+	NULL
+};
+
 enum {
 	AMD_IDE_CONFIG		= 0x41,
 	AMD_CABLE_DETECT	= 0x42,
@@ -112,6 +117,19 @@ static void amd_set_pio_mode(ide_drive_t *drive, const u8 pio)
 	amd_set_drive(drive, XFER_PIO_0 + pio);
 }
 
+static void amd_quirkproc(ide_drive_t *drive)
+{
+	const char **list, *m = (char *)&drive->id[ATA_ID_PROD];
+
+	for (list = am74xx_quirk_drives; *list != NULL; list++)
+			if (strstr(m, *list) != NULL) {
+				drive->quirk_list = 2;
+				return;
+			}
+
+	drive->quirk_list = 0;
+}
+
 static void amd7409_cable_detect(struct pci_dev *dev)
 {
 	/* no host side cable detection */
@@ -194,6 +212,7 @@ static void __devinit init_hwif_amd74xx(ide_hwif_t *hwif)
 static const struct ide_port_ops amd_port_ops = {
 	.set_pio_mode		= amd_set_pio_mode,
 	.set_dma_mode		= amd_set_drive,
+	.quirkproc		= amd_quirkproc,
 	.cable_detect		= amd_cable_detect,
 };
 
-- 
1.6.2.1

Re: [loongson-PATCH-v1 24/27] fixup for FUJITSU disk

[Bartlomiej Zolnierkiewicz]

On Thursday 21 May 2009 00:12:46 wuzhangjin@gmail.com wrote:
> From: Wu Zhangjin <wuzhangjin@gmail.com>
> 
> This is originally from the to-mips branch from
> http://dev.lemote.com/code/linux_loongson

Sadly, the patch description lacks all the important information.

What is the original problem that this fixup tries to address?

Is it limited to amd74xx controllers?

[ We are generalizing quirk_drives handling currently... ]

> Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
> ---
>  drivers/ide/amd74xx.c |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/ide/amd74xx.c b/drivers/ide/amd74xx.c
> index 77267c8..8f488b8 100644
> --- a/drivers/ide/amd74xx.c
> +++ b/drivers/ide/amd74xx.c
> @@ -23,6 +23,11 @@
>  
>  #define DRV_NAME "amd74xx"
>  
> +static const char *am74xx_quirk_drives[] = {
> +	"FUJITSU MHZ2160BH G2",
> +	NULL
> +};
> +
>  enum {
>  	AMD_IDE_CONFIG		= 0x41,
>  	AMD_CABLE_DETECT	= 0x42,
> @@ -112,6 +117,19 @@ static void amd_set_pio_mode(ide_drive_t *drive, const u8 pio)
>  	amd_set_drive(drive, XFER_PIO_0 + pio);
>  }
>  
> +static void amd_quirkproc(ide_drive_t *drive)
> +{
> +	const char **list, *m = (char *)&drive->id[ATA_ID_PROD];
> +
> +	for (list = am74xx_quirk_drives; *list != NULL; list++)
> +			if (strstr(m, *list) != NULL) {
> +				drive->quirk_list = 2;
> +				return;
> +			}
> +
> +	drive->quirk_list = 0;
> +}
> +
>  static void amd7409_cable_detect(struct pci_dev *dev)
>  {
>  	/* no host side cable detection */
> @@ -194,6 +212,7 @@ static void __devinit init_hwif_amd74xx(ide_hwif_t *hwif)
>  static const struct ide_port_ops amd_port_ops = {
>  	.set_pio_mode		= amd_set_pio_mode,
>  	.set_dma_mode		= amd_set_drive,
> +	.quirkproc		= amd_quirkproc,
>  	.cable_detect		= amd_cable_detect,
>  };

Re: [loongson-PATCH-v1 24/27] fixup for FUJITSU disk

[yanh]

在 2009-05-22五的 20:32 +0200，Bartlomiej Zolnierkiewicz写道：
> On Thursday 21 May 2009 00:12:46 wuzhangjin@gmail.com wrote:
> > From: Wu Zhangjin <wuzhangjin@gmail.com>
> > 
> > This is originally from the to-mips branch from
> > http://dev.lemote.com/code/linux_loongson
> 
> Sadly, the patch description lacks all the important information.
> 
> What is the original problem that this fixup tries to address?
> 
> Is it limited to amd74xx controllers?

In loongson2f yeeloong machines, the ide controller is AMD cs5536, or
say  amd74xx, and the hard drives is Fujistu. 
While debuging the hard disk suspned and resume, the ide irq can not be
cleared. I guess this is a fake interrupt, hence the clear irq action
can not be finished. 
This patch is to fix this issue. Maybe other controller and drives also
have this issue, but I am not sure.

Thanks for your reply.  
> 
> [ We are generalizing quirk_drives handling currently... ]
> 
> > Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
> > ---
> >  drivers/ide/amd74xx.c |   19 +++++++++++++++++++
> >  1 files changed, 19 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/ide/amd74xx.c b/drivers/ide/amd74xx.c
> > index 77267c8..8f488b8 100644
> > --- a/drivers/ide/amd74xx.c
> > +++ b/drivers/ide/amd74xx.c
> > @@ -23,6 +23,11 @@
> >  
> >  #define DRV_NAME "amd74xx"
> >  
> > +static const char *am74xx_quirk_drives[] = {
> > +	"FUJITSU MHZ2160BH G2",
> > +	NULL
> > +};
> > +
> >  enum {
> >  	AMD_IDE_CONFIG		= 0x41,
> >  	AMD_CABLE_DETECT	= 0x42,
> > @@ -112,6 +117,19 @@ static void amd_set_pio_mode(ide_drive_t *drive, const u8 pio)
> >  	amd_set_drive(drive, XFER_PIO_0 + pio);
> >  }
> >  
> > +static void amd_quirkproc(ide_drive_t *drive)
> > +{
> > +	const char **list, *m = (char *)&drive->id[ATA_ID_PROD];
> > +
> > +	for (list = am74xx_quirk_drives; *list != NULL; list++)
> > +			if (strstr(m, *list) != NULL) {
> > +				drive->quirk_list = 2;
> > +				return;
> > +			}
> > +
> > +	drive->quirk_list = 0;
> > +}
> > +
> >  static void amd7409_cable_detect(struct pci_dev *dev)
> >  {
> >  	/* no host side cable detection */
> > @@ -194,6 +212,7 @@ static void __devinit init_hwif_amd74xx(ide_hwif_t *hwif)
> >  static const struct ide_port_ops amd_port_ops = {
> >  	.set_pio_mode		= amd_set_pio_mode,
> >  	.set_dma_mode		= amd_set_drive,
> > +	.quirkproc		= amd_quirkproc,
> >  	.cable_detect		= amd_cable_detect,
> >  };
> 

Re: [loongson-PATCH-v1 24/27] fixup for FUJITSU disk

[Bartlomiej Zolnierkiewicz]

On Saturday 23 May 2009 09:11:42 yanh wrote:
> 在 2009-05-22五的 20:32 +0200，Bartlomiej Zolnierkiewicz写道：
> > On Thursday 21 May 2009 00:12:46 wuzhangjin@gmail.com wrote:
> > > From: Wu Zhangjin <wuzhangjin@gmail.com>
> > > 
> > > This is originally from the to-mips branch from
> > > http://dev.lemote.com/code/linux_loongson
> > 
> > Sadly, the patch description lacks all the important information.
> > 
> > What is the original problem that this fixup tries to address?
> > 
> > Is it limited to amd74xx controllers?
> 
> In loongson2f yeeloong machines, the ide controller is AMD cs5536, or
> say  amd74xx, and the hard drives is Fujistu. 

Then it should use the new & shiny :) native cs5536 IDE host driver
instead of legacy support in amd74xx...

> While debuging the hard disk suspned and resume, the ide irq can not be
> cleared. I guess this is a fake interrupt, hence the clear irq action
> can not be finished. 

AFAICS the only change that the fixup would cause for suspend/resume paths
is the one in ide_config_drive_speed() which is called during resume to set
transfer mode on the drive:

       tp_ops->write_devctl(hwif, ATA_NIEN | ATA_DEVCTL_OBS);

        memset(&tf, 0, sizeof(tf));
        tf.feature = SETFEATURES_XFER;
        tf.nsect   = speed;

        tp_ops->tf_load(drive, &tf, IDE_VALID_FEATURE | IDE_VALID_NSECT);

        tp_ops->exec_command(hwif, ATA_CMD_SET_FEATURES);
--->
        if (drive->quirk_list == 2)
                tp_ops->write_devctl(hwif, ATA_DEVCTL_OBS);
--->
        error = __ide_wait_stat(drive, drive->ready_stat,
                                ATA_BUSY | ATA_DRQ | ATA_ERR,
                                WAIT_CMD, &stat);

Please tell me I if understand the issue correctly: if the above quirk is
not executed we end up with spurious IRQs, right?

> This patch is to fix this issue. Maybe other controller and drives also
> have this issue, but I am not sure.

Probably moving it to a generic quirk_drives list later will be useful...

Anyway this fixup needs to be ported to / verified with cs5536 first.

Thanks.
Bart

Re: [loongson-PATCH-v1 24/27] fixup for FUJITSU disk

[yanh]

在 2009-05-23六的 13:47 +0200，Bartlomiej Zolnierkiewicz写道：
> On Saturday 23 May 2009 09:11:42 yanh wrote:
> > 在 2009-05-22五的 20:32 +0200，Bartlomiej Zolnierkiewicz写道：
> > > On Thursday 21 May 2009 00:12:46 wuzhangjin@gmail.com wrote:
> > > > From: Wu Zhangjin <wuzhangjin@gmail.com>
> > > > 
> > > > This is originally from the to-mips branch from
> > > > http://dev.lemote.com/code/linux_loongson
> > > 
> > > Sadly, the patch description lacks all the important information.
> > > 
> > > What is the original problem that this fixup tries to address?
> > > 
> > > Is it limited to amd74xx controllers?
> > 
> > In loongson2f yeeloong machines, the ide controller is AMD cs5536, or
> > say  amd74xx, and the hard drives is Fujistu. 
> 
> Then it should use the new & shiny :) native cs5536 IDE host driver
> instead of legacy support in amd74xx...
> 
> > While debuging the hard disk suspned and resume, the ide irq can not be
> > cleared. I guess this is a fake interrupt, hence the clear irq action
> > can not be finished. 
> 
> AFAICS the only change that the fixup would cause for suspend/resume paths
> is the one in ide_config_drive_speed() which is called during resume to set
> transfer mode on the drive:
> 
>        tp_ops->write_devctl(hwif, ATA_NIEN | ATA_DEVCTL_OBS);
> 
>         memset(&tf, 0, sizeof(tf));
>         tf.feature = SETFEATURES_XFER;
>         tf.nsect   = speed;
> 
>         tp_ops->tf_load(drive, &tf, IDE_VALID_FEATURE | IDE_VALID_NSECT);
> 
>         tp_ops->exec_command(hwif, ATA_CMD_SET_FEATURES);
> --->
>         if (drive->quirk_list == 2)
>                 tp_ops->write_devctl(hwif, ATA_DEVCTL_OBS);

> --->
>         error = __ide_wait_stat(drive, drive->ready_stat,
>                                 ATA_BUSY | ATA_DRQ | ATA_ERR,
>                                 WAIT_CMD, &stat);
> 
> Please tell me I if understand the issue correctly: if the above quirk is
> not executed we end up with spurious IRQs, right?
right.
> 
> > This patch is to fix this issue. Maybe other controller and drives also
> > have this issue, but I am not sure.
> 
> Probably moving it to a generic quirk_drives list later will be useful...
> 
> Anyway this fixup needs to be ported to / verified with cs5536 first.
the cs5536 pata driver have some geode platform dependent codes. We can
just ignore it, but the performance is poor(using hdparm to test it),
which only get 22+ MB/s. we find it only use udma2. However if using
amd74xx driver, it can set udma5, and the speed can reach to 50+ MB/s. 

we will test whether this driver is working well without this patch.

Anyway, thanks your advice.
> 
> Thanks.
> Bart
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [loongson-PATCH-v1 24/27] fixup for FUJITSU disk

[胡洪兵]

yanh 写道:
> 在 2009-05-23六的 13:47 +0200，Bartlomiej Zolnierkiewicz写道：
>   
>> On Saturday 23 May 2009 09:11:42 yanh wrote:
>>     
>>> 在 2009-05-22五的 20:32 +0200，Bartlomiej Zolnierkiewicz写道：
>>>       
>>>> On Thursday 21 May 2009 00:12:46 wuzhangjin@gmail.com wrote:
>>>>         
>>>>> From: Wu Zhangjin <wuzhangjin@gmail.com>
>>>>>
>>>>> This is originally from the to-mips branch from
>>>>> http://dev.lemote.com/code/linux_loongson
>>>>>           
>>>> Sadly, the patch description lacks all the important information.
>>>>
>>>> What is the original problem that this fixup tries to address?
>>>>
>>>> Is it limited to amd74xx controllers?
>>>>         
>>> In loongson2f yeeloong machines, the ide controller is AMD cs5536, or
>>> say  amd74xx, and the hard drives is Fujistu. 
>>>       
>> Then it should use the new & shiny :) native cs5536 IDE host driver
>> instead of legacy support in amd74xx...
>>
>>     
>>> While debuging the hard disk suspned and resume, the ide irq can not be
>>> cleared. I guess this is a fake interrupt, hence the clear irq action
>>> can not be finished. 
>>>       
>> AFAICS the only change that the fixup would cause for suspend/resume paths
>> is the one in ide_config_drive_speed() which is called during resume to set
>> transfer mode on the drive:
>>
>>        tp_ops->write_devctl(hwif, ATA_NIEN | ATA_DEVCTL_OBS);
>>
>>         memset(&tf, 0, sizeof(tf));
>>         tf.feature = SETFEATURES_XFER;
>>         tf.nsect   = speed;
>>
>>         tp_ops->tf_load(drive, &tf, IDE_VALID_FEATURE | IDE_VALID_NSECT);
>>
>>         tp_ops->exec_command(hwif, ATA_CMD_SET_FEATURES);
>> --->
>>         if (drive->quirk_list == 2)
>>                 tp_ops->write_devctl(hwif, ATA_DEVCTL_OBS);
>>     
>
>   
>> --->
>>         error = __ide_wait_stat(drive, drive->ready_stat,
>>                                 ATA_BUSY | ATA_DRQ | ATA_ERR,
>>                                 WAIT_CMD, &stat);
>>
>> Please tell me I if understand the issue correctly: if the above quirk is
>> not executed we end up with spurious IRQs, right?
>>     
> right.
>   
>>> This patch is to fix this issue. Maybe other controller and drives also
>>> have this issue, but I am not sure.
>>>       
>> Probably moving it to a generic quirk_drives list later will be useful...
>>
>> Anyway this fixup needs to be ported to / verified with cs5536 first.
>>     
> the cs5536 pata driver have some geode platform dependent codes. We can
> just ignore it, but the performance is poor(using hdparm to test it),
> which only get 22+ MB/s. we find it only use udma2. However if using
> amd74xx driver, it can set udma5, and the speed can reach to 50+ MB/s. 
>
> we will test whether this driver is working well without this patch.
>
> Anyway, thanks your advice.
>   
Use the driver drivers/ata/pata_cs5536.c, unfortunately it also have the 
same issue.
>> Thanks.
>> Bart
>>
>>     
>
>
>
>   


-- 
---------------------------------------------------------
Hongbing,Hu (Software Department)
Tel:    0512-52308631
E-mail:	huhb@lemote.com
MSN:	[huhb04@gmail.com]
JiangSu Lemote Corp. Ltd.
MengLan, Yushan, Changshu, JiangSu Province, China
---------------------------------------------------------

Re: [loongson-PATCH-v1 24/27] fixup for FUJITSU disk

[Bartlomiej Zolnierkiewicz]

On Monday 25 May 2009 09:35:48 胡洪兵 wrote:
> yanh 写道:

[...]

> > the cs5536 pata driver have some geode platform dependent codes. We can
> > just ignore it, but the performance is poor(using hdparm to test it),
> > which only get 22+ MB/s. we find it only use udma2. However if using
> > amd74xx driver, it can set udma5, and the speed can reach to 50+ MB/s. 
> >
> > we will test whether this driver is working well without this patch.
> >
> > Anyway, thanks your advice.
> >   
> Use the driver drivers/ata/pata_cs5536.c, unfortunately it also have the 
> same issue.

Both drivers (pata_cs5536 & cs5536) should handle UDMA5.

Could you please explain the issue with platform specific code a bit more?

Is it related to a cable detection by any chance?

Thanks.
Bart
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [loongson-PATCH-v1 24/27] fixup for FUJITSU disk

[Sergei Shtylyov]

Bartlomiej Zolnierkiewicz wrote:

>>yanh 写道:

> [...]

>>>the cs5536 pata driver have some geode platform dependent codes. We can
>>>just ignore it, but the performance is poor(using hdparm to test it),
>>>which only get 22+ MB/s. we find it only use udma2. However if using
>>>amd74xx driver, it can set udma5, and the speed can reach to 50+ MB/s. 

>>>we will test whether this driver is working well without this patch.

>>>Anyway, thanks your advice.

>>Use the driver drivers/ata/pata_cs5536.c, unfortunately it also have the 
>>same issue.

> Both drivers (pata_cs5536 & cs5536) should handle UDMA5.

> Could you please explain the issue with platform specific code a bit more?

> Is it related to a cable detection by any chance?

    I guess it's rdmsr()/wrmsr()...

> Thanks.
> Bart

MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [loongson-PATCH-v1 24/27] fixup for FUJITSU disk

[Martin K. Petersen]

>>>>> "Sergei" == Sergei Shtylyov <sshtylyov@ru.mvista.com> writes:

>> Could you please explain the issue with platform specific code a bit
>> more?

>> Is it related to a cable detection by any chance?

Sergei>    I guess it's rdmsr()/wrmsr()...

Those are only used when the driver is explicitly loaded with
use_msr=1.  The default is to use PCI config space/VSA.

In any case I have yet to see a CS5536 system whose BIOS sets the cable
detection bit.  If that's the problem I guess we could add a quirk flag
to override the (lack of) BIOS setting.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [loongson-PATCH-v1 24/27] fixup for FUJITSU disk

[Sergei Shtylyov]

Hello.

Martin K. Petersen wrote:

>>>Could you please explain the issue with platform specific code a bit
>>>more?

>>>Is it related to a cable detection by any chance?

> Sergei>    I guess it's rdmsr()/wrmsr()...

> Those are only used when the driver is explicitly loaded with
> use_msr=1.  The default is to use PCI config space/VSA.

    But you need them defined for the driver to build...

MBR, Sergei

Re: [loongson-PATCH-v1 24/27] fixup for FUJITSU disk

[yanh]

在 2009-05-25一的 15:10 -0400，Martin K. Petersen写道：
> >>>>> "Sergei" == Sergei Shtylyov <sshtylyov@ru.mvista.com> writes:
> 
> >> Could you please explain the issue with platform specific code a bit
> >> more?
> 
> >> Is it related to a cable detection by any chance?
> 
> Sergei>    I guess it's rdmsr()/wrmsr()...
currently, we just commented out the rdmsr/wrmsr.
and we use pci VSA to access it. 
> 
> Those are only used when the driver is explicitly loaded with
> use_msr=1.  The default is to use PCI config space/VSA.
> 
> In any case I have yet to see a CS5536 system whose BIOS sets the cable
> detection bit.  If that's the problem I guess we could add a quirk flag
> to override the (lack of) BIOS setting.
> 
This may be the cause that udma5 cannot be set.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [loongson-PATCH-v1 24/27] fixup for FUJITSU disk

[Alan Cox]

> > In any case I have yet to see a CS5536 system whose BIOS sets the cable
> > detection bit.  If that's the problem I guess we could add a quirk flag
> > to override the (lack of) BIOS setting.
> > 
> This may be the cause that udma5 cannot be set.


If the system doesn't have working BIOS cable detect as per the
documentation for the chip then just return the "unknown" cable type and
libata will do device side cable detect.

Alan

Re: [loongson-PATCH-v1 24/27] fixup for FUJITSU disk

[yanh]

在 2009-05-26二的 08:56 +0100，Alan Cox写道：
> > > In any case I have yet to see a CS5536 system whose BIOS sets the cable
> > > detection bit.  If that's the problem I guess we could add a quirk flag
> > > to override the (lack of) BIOS setting.
> > > 
> > This may be the cause that udma5 cannot be set.
> 
> 
> If the system doesn't have working BIOS cable detect as per the
> documentation for the chip then just return the "unknown" cable type and
> libata will do device side cable detect.

After force cable type as 80 wire, udma5 can be set OK now. 
after some raw test, the legacy and native driver have no obvious
difference in performance.

Remain issue: this native driver still have suspend/resume issue.

 
> 
> Alan

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html