linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Unbreak build of PMP with ACPI disabled
@ 2007-07-13  6:56 Petr Vandrovec
  2007-07-13  7:09 ` Tejun Heo
  0 siblings, 1 reply; 11+ messages in thread
From: Petr Vandrovec @ 2007-07-13  6:56 UTC (permalink / raw)
  To: htejun; +Cc: linux-ide

[-- Attachment #1: Type: text/plain, Size: 2335 bytes --]

Hello,
  first let me thank you for your PMP code.  Works great.  Though I could not
built it with ACPI disable - it is possible that I did merge wrong (I'm using
your tree with current Linus's TOT), though I do not think.  Patch is at
the end.

If you are interested in success report, then my dmesg is attached.  SiI3132,
with Sans Digital MS4UM, with SiI3726 multiplier.  Multipliers ports 0-3 are
connected to hotpluggable slots, port 4 is not connected anywhere, and port
5 is connected to some well hidden 1.5Gbps device.

Only problem I've noticed is that smartctl on drives connected to sata_nv
needs '-T permissive', while if same drive is connected to sata_sil24's PMP,
it works without problem.  

Error SMART Status command failed
Please get assistance from http://smartmontools.sourceforge.net/
Values from ATA status return descriptor are:
 00     09 0c 00 00 00 00 00 00  00 00 00 00 00 50
A mandatory SMART command failed: exiting. To continue, add one or more '-T permissive' options.

							Thanks,
								Petr Vandrovec


From: Petr Vandrovec <petr@vandrovec.name>

Unbreak building port multiplier without ACPI

ata_acpi_associate_sata_port exists only if ACPI is enabled, so it is
wise to provide dummy implementation for disabled ACPI.  Otherwise build
breaks...

Signed-off-by: Petr Vandrovec <petr@vandrovec.name>

---
commit 15cf93ebda55fc16ab4ad3f52d9186c536505a62
tree ea4a2b38471cc5bba5c5f23743ef8f8c53ccb250
parent 61e519cb5dbfa3fa87f57e60204e86d92344dc32
author Petr Vandrovec <petr@vandrovec.name> Thu, 12 Jul 2007 23:29:36 -0700
committer Petr Vandrovec <petr@vandrovec.name> Thu, 12 Jul 2007 23:29:36 -0700

 drivers/ata/libata.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 9577db0..d568c53 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -107,6 +107,7 @@ extern int ata_acpi_on_suspend(struct ata_port *ap);
 extern void ata_acpi_on_resume(struct ata_port *ap);
 extern int ata_acpi_on_devcfg(struct ata_device *adev);
 #else
+static inline void ata_acpi_associate_sata_port(struct ata_port *ap) { }
 static inline void ata_acpi_associate(struct ata_host *host) { }
 static inline int ata_acpi_on_suspend(struct ata_port *ap) { return 0; }
 static inline void ata_acpi_on_resume(struct ata_port *ap) { }

[-- Attachment #2: dmesg.gz --]
[-- Type: application/octet-stream, Size: 9133 bytes --]

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

* Re: [PATCH] Unbreak build of PMP with ACPI disabled
  2007-07-13  6:56 [PATCH] Unbreak build of PMP with ACPI disabled Petr Vandrovec
@ 2007-07-13  7:09 ` Tejun Heo
  2007-07-13 23:30   ` Robert Hancock
  0 siblings, 1 reply; 11+ messages in thread
From: Tejun Heo @ 2007-07-13  7:09 UTC (permalink / raw)
  To: Petr Vandrovec; +Cc: linux-ide, Robert Hancock

Petr Vandrovec wrote:
> Hello,
>   first let me thank you for your PMP code.  Works great.  Though I could not
> built it with ACPI disable - it is possible that I did merge wrong (I'm using
> your tree with current Linus's TOT), though I do not think.  Patch is at
> the end.

It's my screw up.  I will integrate the patch in the next round
(scheduled for later today).

> If you are interested in success report, then my dmesg is attached.  SiI3132,
> with Sans Digital MS4UM, with SiI3726 multiplier.  Multipliers ports 0-3 are
> connected to hotpluggable slots, port 4 is not connected anywhere, and port
> 5 is connected to some well hidden 1.5Gbps device.
> 
> Only problem I've noticed is that smartctl on drives connected to sata_nv
> needs '-T permissive', while if same drive is connected to sata_sil24's PMP,
> it works without problem.  
> 
> Error SMART Status command failed
> Please get assistance from http://smartmontools.sourceforge.net/
> Values from ATA status return descriptor are:
>  00     09 0c 00 00 00 00 00 00  00 00 00 00 00 50
> A mandatory SMART command failed: exiting. To continue, add one or more '-T permissive' options.

Yeah, we seem to have some problem with passing result TF to userland in
sata_nv (cc'ed Robert).

Thanks.

-- 
tejun

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

* Re: [PATCH] Unbreak build of PMP with ACPI disabled
  2007-07-13  7:09 ` Tejun Heo
@ 2007-07-13 23:30   ` Robert Hancock
  2007-07-14  0:00     ` Ryan Power
  0 siblings, 1 reply; 11+ messages in thread
From: Robert Hancock @ 2007-07-13 23:30 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Petr Vandrovec, linux-ide

Tejun Heo wrote:
>> Only problem I've noticed is that smartctl on drives connected to sata_nv
>> needs '-T permissive', while if same drive is connected to sata_sil24's PMP,
>> it works without problem.  
>>
>> Error SMART Status command failed
>> Please get assistance from http://smartmontools.sourceforge.net/
>> Values from ATA status return descriptor are:
>>  00     09 0c 00 00 00 00 00 00  00 00 00 00 00 50
>> A mandatory SMART command failed: exiting. To continue, add one or more '-T permissive' options.
> 
> Yeah, we seem to have some problem with passing result TF to userland in
> sata_nv (cc'ed Robert).
> 
> Thanks.

I've seen that report, but it's a bit of a mystery to me. Apparently the 
problem still happens if ADMA is disabled, and in that mode we just use 
the standard ata_tf_read function, so it's not clear how it could not be 
working on that controller but work on others..

Also I haven't noticed any SMART problems on my machine with ADMA 
enabled.. maybe it's drive or motherboard dependent?

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/


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

* Re: [PATCH] Unbreak build of PMP with ACPI disabled
  2007-07-13 23:30   ` Robert Hancock
@ 2007-07-14  0:00     ` Ryan Power
  2007-07-14  2:31       ` Tejun Heo
  0 siblings, 1 reply; 11+ messages in thread
From: Ryan Power @ 2007-07-14  0:00 UTC (permalink / raw)
  To: Robert Hancock, Tejun Heo; +Cc: Petr Vandrovec, linux-ide

I'm also experiencing this problem on a with the sata_nv module.  I suspect 
that it may at least not be controller dependant, as it's also occurring on 
my silicon image controller (sata_sil, SiI3512).  It looks like something 
is zeroing out part of the return registers for the overall smart status.

Tested with Maxtor 6L300S0 and Western Digital WD3200JS drives on both the 
sil and nv controllers.  This was working in 2.6.21.6, but I haven't had a 
chance to try and isolate the problem any further.

Thanks.
-Ryan Power

At 05:30 PM 7/13/2007, Robert Hancock wrote:
>Tejun Heo wrote:
>>>Only problem I've noticed is that smartctl on drives connected to sata_nv
>>>needs '-T permissive', while if same drive is connected to sata_sil24's PMP,
>>>it works without problem.
>>>
>>>Error SMART Status command failed
>>>Please get assistance from http://smartmontools.sourceforge.net/
>>>Values from ATA status return descriptor are:
>>>  00     09 0c 00 00 00 00 00 00  00 00 00 00 00 50
>>>A mandatory SMART command failed: exiting. To continue, add one or more 
>>>'-T permissive' options.
>>Yeah, we seem to have some problem with passing result TF to userland in
>>sata_nv (cc'ed Robert).
>>Thanks.
>
>I've seen that report, but it's a bit of a mystery to me. Apparently the 
>problem still happens if ADMA is disabled, and in that mode we just use 
>the standard ata_tf_read function, so it's not clear how it could not be 
>working on that controller but work on others..
>
>Also I haven't noticed any SMART problems on my machine with ADMA 
>enabled.. maybe it's drive or motherboard dependent?


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

* Re: [PATCH] Unbreak build of PMP with ACPI disabled
  2007-07-14  0:00     ` Ryan Power
@ 2007-07-14  2:31       ` Tejun Heo
  2007-07-14 12:13         ` Petr Vandrovec
  0 siblings, 1 reply; 11+ messages in thread
From: Tejun Heo @ 2007-07-14  2:31 UTC (permalink / raw)
  To: Ryan Power; +Cc: Robert Hancock, Petr Vandrovec, linux-ide

Ryan Power wrote:
> I'm also experiencing this problem on a with the sata_nv module.  I
> suspect that it may at least not be controller dependant, as it's also
> occurring on my silicon image controller (sata_sil, SiI3512).  It looks
> like something is zeroing out part of the return registers for the
> overall smart status.
> 
> Tested with Maxtor 6L300S0 and Western Digital WD3200JS drives on both
> the sil and nv controllers.  This was working in 2.6.21.6, but I haven't
> had a chance to try and isolate the problem any further.

Hmmm... Incidentally, Jeff told me yesterday he was seeing the same
problem on sil.  Maybe it's something in the core.  I'll investigate.

Thanks.

-- 
tejun

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

* Re: [PATCH] Unbreak build of PMP with ACPI disabled
  2007-07-14  2:31       ` Tejun Heo
@ 2007-07-14 12:13         ` Petr Vandrovec
  2007-07-16  3:38           ` Ryan Power
  2007-07-16 10:32           ` Tejun Heo
  0 siblings, 2 replies; 11+ messages in thread
From: Petr Vandrovec @ 2007-07-14 12:13 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Ryan Power, Robert Hancock, linux-ide

[-- Attachment #1: Type: text/plain, Size: 2161 bytes --]

Tejun Heo wrote:
> Ryan Power wrote:
>> I'm also experiencing this problem on a with the sata_nv module.  I
>> suspect that it may at least not be controller dependant, as it's also
>> occurring on my silicon image controller (sata_sil, SiI3512).  It looks
>> like something is zeroing out part of the return registers for the
>> overall smart status.
>>
>> Tested with Maxtor 6L300S0 and Western Digital WD3200JS drives on both
>> the sil and nv controllers.  This was working in 2.6.21.6, but I haven't
>> had a chance to try and isolate the problem any further.
> 
> Hmmm... Incidentally, Jeff told me yesterday he was seeing the same
> problem on sil.  Maybe it's something in the core.  I'll investigate.

What about attached patch?  Seems to make my mcp61 happy... (it is sent 
from Mozilla, so patch below is most probably whitespace challenged... 
attachment should be fine)
							Petr


Fix reported task file values in sense data

ata_tf_read was setting HOB bit when lba48 command was submitted, but
was not clearing it before reading "normal" data.  Maybe it would be
better to just clear HOB bit immediately after reading upper halves
for lba48 command, but I just decided to clear HOB bit in each
ata_tf_read...

Signed-off-by: Petr Vandrovec <petr@vandrovec.name>

---
commit de1eff411670a3e5dfadcc281754cb26035779fa
tree 5fbcc8f667a34aa9688ce3aa19b9c92944a12862
parent c2e8a9b937eb84d1b712874f458790d2df25641a
author Petr Vandrovec <petr@vandrovec.name> Sat, 14 Jul 2007 05:11:03 -0700
committer root <root@gwy.hsd1.ca.comcast.net> Sat, 14 Jul 2007 05:11:03 
-0700

  drivers/ata/libata-sff.c |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index a74afea..507ab69 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -196,6 +196,7 @@ void ata_tf_read(struct ata_port *ap, struct 
ata_taskfile *t
f)
  {
         struct ata_ioports *ioaddr = &ap->ioaddr;

+       iowrite8(tf->ctl, ioaddr->ctl_addr);
         tf->command = ata_check_status(ap);
         tf->feature = ioread8(ioaddr->error_addr);
         tf->nsect = ioread8(ioaddr->nsect_addr);

[-- Attachment #2: unbreak-smart --]
[-- Type: text/plain, Size: 1225 bytes --]

Fix reported task file values in sense data

ata_tf_read was setting HOB bit when lba48 command was submitted, but
was not clearing it before reading "normal" data.  Maybe it would be
better to just clear HOB bit immediately after reading upper halves
for lba48 command, but I just decided to clear HOB bit in each
ata_tf_read...

Signed-off-by: Petr Vandrovec <petr@vandrovec.name>

---
commit de1eff411670a3e5dfadcc281754cb26035779fa
tree 5fbcc8f667a34aa9688ce3aa19b9c92944a12862
parent c2e8a9b937eb84d1b712874f458790d2df25641a
author Petr Vandrovec <petr@vandrovec.name> Sat, 14 Jul 2007 05:11:03 -0700
committer root <root@gwy.hsd1.ca.comcast.net> Sat, 14 Jul 2007 05:11:03 -0700

 drivers/ata/libata-sff.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index a74afea..507ab69 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -196,6 +196,7 @@ void ata_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
 {
 	struct ata_ioports *ioaddr = &ap->ioaddr;
 
+	iowrite8(tf->ctl, ioaddr->ctl_addr);
 	tf->command = ata_check_status(ap);
 	tf->feature = ioread8(ioaddr->error_addr);
 	tf->nsect = ioread8(ioaddr->nsect_addr);

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

* Re: [PATCH] Unbreak build of PMP with ACPI disabled
  2007-07-14 12:13         ` Petr Vandrovec
@ 2007-07-16  3:38           ` Ryan Power
  2007-07-16 10:32           ` Tejun Heo
  1 sibling, 0 replies; 11+ messages in thread
From: Ryan Power @ 2007-07-16  3:38 UTC (permalink / raw)
  To: Petr Vandrovec, Tejun Heo; +Cc: Robert Hancock, linux-ide

At 06:13 AM 7/14/2007, Petr Vandrovec wrote:
>Tejun Heo wrote:
>>Ryan Power wrote:
>>>I'm also experiencing this problem on a with the sata_nv module.  I
>>>suspect that it may at least not be controller dependant, as it's also
>>>occurring on my silicon image controller (sata_sil, SiI3512).  It looks
>>>like something is zeroing out part of the return registers for the
>>>overall smart status.
>>Hmmm... Incidentally, Jeff told me yesterday he was seeing the same
>>problem on sil.  Maybe it's something in the core.  I'll investigate.
>
>What about attached patch?  Seems to make my mcp61 happy... (it is sent 
>from Mozilla, so patch below is most probably whitespace challenged... 
>attachment should be fine)
>                                                         Petr

That patch fixes the problems I was seeing on both interfaces.  Everything 
looks be to working correctly again.

Thanks.
-Ryan 


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

* Re: [PATCH] Unbreak build of PMP with ACPI disabled
  2007-07-14 12:13         ` Petr Vandrovec
  2007-07-16  3:38           ` Ryan Power
@ 2007-07-16 10:32           ` Tejun Heo
  2007-07-17 10:54             ` [PATCH] Fix SMART reporting on 2.6.22 Petr Vandrovec
  1 sibling, 1 reply; 11+ messages in thread
From: Tejun Heo @ 2007-07-16 10:32 UTC (permalink / raw)
  To: Petr Vandrovec
  Cc: Ryan Power, Robert Hancock, linux-ide, Jeff Garzik, Albert Lee

[cc'ing Jeff and Albert]

Petr Vandrovec wrote:
> Fix reported task file values in sense data
> 
> ata_tf_read was setting HOB bit when lba48 command was submitted, but
> was not clearing it before reading "normal" data.  Maybe it would be
> better to just clear HOB bit immediately after reading upper halves
> for lba48 command, but I just decided to clear HOB bit in each
> ata_tf_read...
> 
> Signed-off-by: Petr Vandrovec <petr@vandrovec.name>

Acked-by: Tejun Heo <htejun@gmail.com>

Gee, thanks a lot for spotting this.  This is definitely for -stable.
Hmm... it's a separate issue and not your fault but ap->last_ctl caching
is broken in the function.  Albert is about to remove ap->last_ctl
caching but we still need to fix it for -stable.  Petr, are you
interested in submitting a separate patch for fixing ap->last_ctl
handling in the function?

Thanks.

-- 
tejun

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

* [PATCH] Fix SMART reporting on 2.6.22
  2007-07-16 10:32           ` Tejun Heo
@ 2007-07-17 10:54             ` Petr Vandrovec
  2007-07-18  3:08               ` Tejun Heo
  2007-07-20 11:44               ` Jeff Garzik
  0 siblings, 2 replies; 11+ messages in thread
From: Petr Vandrovec @ 2007-07-17 10:54 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Ryan Power, Robert Hancock, linux-ide, Jeff Garzik, Albert Lee,
	Andrew Morton

[-- Attachment #1: Type: text/plain, Size: 3076 bytes --]

On Mon, Jul 16, 2007 at 07:32:57PM +0900, Tejun Heo wrote:
> [cc'ing Jeff and Albert]
> 
> Petr Vandrovec wrote:
> > Fix reported task file values in sense data
> > 
> > ata_tf_read was setting HOB bit when lba48 command was submitted, but
> > was not clearing it before reading "normal" data.  Maybe it would be
> > better to just clear HOB bit immediately after reading upper halves
> > for lba48 command, but I just decided to clear HOB bit in each
> > ata_tf_read...
> > 
> > Signed-off-by: Petr Vandrovec <petr@vandrovec.name>
> 
> Acked-by: Tejun Heo <htejun@gmail.com>
> 
> Gee, thanks a lot for spotting this.  This is definitely for -stable.
> Hmm... it's a separate issue and not your fault but ap->last_ctl caching
> is broken in the function.  Albert is about to remove ap->last_ctl
> caching but we still need to fix it for -stable.  Petr, are you
> interested in submitting a separate patch for fixing ap->last_ctl
> handling in the function?

OK, that pushed me over edge so this replaces my previous patch.  Now
there is no ctl access when 24bit commands are issued back to back, and
ctl is touched (twice...) only for 48bit commands.  smartctl still works...
Any other email address I should CC?
					Thanks,
						Petr Vandrovec


Fix reported task file values in sense data

ata_tf_read was setting HOB bit when lba48 command was submitted, but
was not clearing it before reading "normal" data.  As it is only place
which sets HOB bit in control register, and register reads should not
be affected by other bits, let's just clear it when we are done with
reading upper bytes so non-48bit commands do not have to touch ctl
at all.

pata_scc suffered from same problem...

Signed-off-by: Petr Vandrovec <petr@vandrovec.name>

---
commit d09591edad8e75c9bc850d47b68a9a6f6f107998
tree 1bd5a79c9cade9b7ebf68b13bf24a879b969e4c8
parent a5fcaa210626a79465321e344c91a6a7dc3881fa
author Petr Vandrovec <petr@vandrovec.name> Tue, 17 Jul 2007 03:45:43 -0700
committer Petr Vandrovec <petr@vandrovec.name> Tue, 17 Jul 2007 03:45:43 -0700

 drivers/ata/libata-sff.c |    2 ++
 drivers/ata/pata_scc.c   |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index ca7d224..6a579a9 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -211,6 +211,8 @@ void ata_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
 		tf->hob_lbal = ioread8(ioaddr->lbal_addr);
 		tf->hob_lbam = ioread8(ioaddr->lbam_addr);
 		tf->hob_lbah = ioread8(ioaddr->lbah_addr);
+		iowrite8(tf->ctl, ioaddr->ctl_addr);
+		ap->last_ctl = tf->ctl;
 	}
 }
 
diff --git a/drivers/ata/pata_scc.c b/drivers/ata/pata_scc.c
index c55667e..60cce07 100644
--- a/drivers/ata/pata_scc.c
+++ b/drivers/ata/pata_scc.c
@@ -358,6 +358,8 @@ static void scc_tf_read (struct ata_port *ap, struct ata_taskfile *tf)
 		tf->hob_lbal = in_be32(ioaddr->lbal_addr);
 		tf->hob_lbam = in_be32(ioaddr->lbam_addr);
 		tf->hob_lbah = in_be32(ioaddr->lbah_addr);
+		out_be32(ioaddr->ctl_addr, tf->ctl);
+		ap->last_ctl = tf->ctl;
 	}
 }
 

[-- Attachment #2: q1 --]
[-- Type: text/plain, Size: 1827 bytes --]

Fix reported task file values in sense data

ata_tf_read was setting HOB bit when lba48 command was submitted, but
was not clearing it before reading "normal" data.  As it is only place
which sets HOB bit in control register, and register reads should not
be affected by other bits, let's just clear it when we are done with
reading upper bytes so non-48bit commands do not have to touch ctl
at all.

pata_scc suffered from same problem...

Signed-off-by: Petr Vandrovec <petr@vandrovec.name>

---
commit d09591edad8e75c9bc850d47b68a9a6f6f107998
tree 1bd5a79c9cade9b7ebf68b13bf24a879b969e4c8
parent a5fcaa210626a79465321e344c91a6a7dc3881fa
author Petr Vandrovec <petr@vandrovec.name> Tue, 17 Jul 2007 03:45:43 -0700
committer Petr Vandrovec <petr@vandrovec.name> Tue, 17 Jul 2007 03:45:43 -0700

 drivers/ata/libata-sff.c |    2 ++
 drivers/ata/pata_scc.c   |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index ca7d224..6a579a9 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -211,6 +211,8 @@ void ata_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
 		tf->hob_lbal = ioread8(ioaddr->lbal_addr);
 		tf->hob_lbam = ioread8(ioaddr->lbam_addr);
 		tf->hob_lbah = ioread8(ioaddr->lbah_addr);
+		iowrite8(tf->ctl, ioaddr->ctl_addr);
+		ap->last_ctl = tf->ctl;
 	}
 }
 
diff --git a/drivers/ata/pata_scc.c b/drivers/ata/pata_scc.c
index c55667e..60cce07 100644
--- a/drivers/ata/pata_scc.c
+++ b/drivers/ata/pata_scc.c
@@ -358,6 +358,8 @@ static void scc_tf_read (struct ata_port *ap, struct ata_taskfile *tf)
 		tf->hob_lbal = in_be32(ioaddr->lbal_addr);
 		tf->hob_lbam = in_be32(ioaddr->lbam_addr);
 		tf->hob_lbah = in_be32(ioaddr->lbah_addr);
+		out_be32(ioaddr->ctl_addr, tf->ctl);
+		ap->last_ctl = tf->ctl;
 	}
 }
 

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

* Re: [PATCH] Fix SMART reporting on 2.6.22
  2007-07-17 10:54             ` [PATCH] Fix SMART reporting on 2.6.22 Petr Vandrovec
@ 2007-07-18  3:08               ` Tejun Heo
  2007-07-20 11:44               ` Jeff Garzik
  1 sibling, 0 replies; 11+ messages in thread
From: Tejun Heo @ 2007-07-18  3:08 UTC (permalink / raw)
  To: Petr Vandrovec
  Cc: Ryan Power, Robert Hancock, linux-ide, Jeff Garzik, Albert Lee,
	Andrew Morton

> Fix reported task file values in sense data
> 
> ata_tf_read was setting HOB bit when lba48 command was submitted, but
> was not clearing it before reading "normal" data.  As it is only place
> which sets HOB bit in control register, and register reads should not
> be affected by other bits, let's just clear it when we are done with
> reading upper bytes so non-48bit commands do not have to touch ctl
> at all.
> 
> pata_scc suffered from same problem...
> 
> Signed-off-by: Petr Vandrovec <petr@vandrovec.name>

Acked-by: Tejun Heo <htejun@gmail.com>

-- 
tejun

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

* Re: [PATCH] Fix SMART reporting on 2.6.22
  2007-07-17 10:54             ` [PATCH] Fix SMART reporting on 2.6.22 Petr Vandrovec
  2007-07-18  3:08               ` Tejun Heo
@ 2007-07-20 11:44               ` Jeff Garzik
  1 sibling, 0 replies; 11+ messages in thread
From: Jeff Garzik @ 2007-07-20 11:44 UTC (permalink / raw)
  To: Petr Vandrovec
  Cc: Tejun Heo, Ryan Power, Robert Hancock, linux-ide, Albert Lee,
	Andrew Morton

Petr Vandrovec wrote:
> On Mon, Jul 16, 2007 at 07:32:57PM +0900, Tejun Heo wrote:
>> [cc'ing Jeff and Albert]
>>
>> Petr Vandrovec wrote:
>>> Fix reported task file values in sense data
>>>
>>> ata_tf_read was setting HOB bit when lba48 command was submitted, but
>>> was not clearing it before reading "normal" data.  Maybe it would be
>>> better to just clear HOB bit immediately after reading upper halves
>>> for lba48 command, but I just decided to clear HOB bit in each
>>> ata_tf_read...
>>>
>>> Signed-off-by: Petr Vandrovec <petr@vandrovec.name>
>> Acked-by: Tejun Heo <htejun@gmail.com>
>>
>> Gee, thanks a lot for spotting this.  This is definitely for -stable.
>> Hmm... it's a separate issue and not your fault but ap->last_ctl caching
>> is broken in the function.  Albert is about to remove ap->last_ctl
>> caching but we still need to fix it for -stable.  Petr, are you
>> interested in submitting a separate patch for fixing ap->last_ctl
>> handling in the function?
> 
> OK, that pushed me over edge so this replaces my previous patch.  Now
> there is no ctl access when 24bit commands are issued back to back, and
> ctl is touched (twice...) only for 48bit commands.  smartctl still works...
> Any other email address I should CC?
> 					Thanks,
> 						Petr Vandrovec
> 
> 
> Fix reported task file values in sense data
> 
> ata_tf_read was setting HOB bit when lba48 command was submitted, but
> was not clearing it before reading "normal" data.  As it is only place
> which sets HOB bit in control register, and register reads should not
> be affected by other bits, let's just clear it when we are done with
> reading upper bytes so non-48bit commands do not have to touch ctl
> at all.
> 
> pata_scc suffered from same problem...
> 
> Signed-off-by: Petr Vandrovec <petr@vandrovec.name>
> 
> ---
> commit d09591edad8e75c9bc850d47b68a9a6f6f107998
> tree 1bd5a79c9cade9b7ebf68b13bf24a879b969e4c8
> parent a5fcaa210626a79465321e344c91a6a7dc3881fa
> author Petr Vandrovec <petr@vandrovec.name> Tue, 17 Jul 2007 03:45:43 -0700
> committer Petr Vandrovec <petr@vandrovec.name> Tue, 17 Jul 2007 03:45:43 -0700
> 
>  drivers/ata/libata-sff.c |    2 ++
>  drivers/ata/pata_scc.c   |    2 ++
>  2 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
> index ca7d224..6a579a9 100644
> --- a/drivers/ata/libata-sff.c
> +++ b/drivers/ata/libata-sff.c
> @@ -211,6 +211,8 @@ void ata_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
>  		tf->hob_lbal = ioread8(ioaddr->lbal_addr);
>  		tf->hob_lbam = ioread8(ioaddr->lbam_addr);
>  		tf->hob_lbah = ioread8(ioaddr->lbah_addr);
> +		iowrite8(tf->ctl, ioaddr->ctl_addr);
> +		ap->last_ctl = tf->ctl;
>  	}
>  }
>  
> diff --git a/drivers/ata/pata_scc.c b/drivers/ata/pata_scc.c
> index c55667e..60cce07 100644
> --- a/drivers/ata/pata_scc.c
> +++ b/drivers/ata/pata_scc.c
> @@ -358,6 +358,8 @@ static void scc_tf_read (struct ata_port *ap, struct ata_taskfile *tf)
>  		tf->hob_lbal = in_be32(ioaddr->lbal_addr);
>  		tf->hob_lbam = in_be32(ioaddr->lbam_addr);
>  		tf->hob_lbah = in_be32(ioaddr->lbah_addr);
> +		out_be32(ioaddr->ctl_addr, tf->ctl);
> +		ap->last_ctl = tf->ctl;
>  	}
>  }
>  
> 
> 
> ------------------------------------------------------------------------
> 
> Fix reported task file values in sense data
> 
> ata_tf_read was setting HOB bit when lba48 command was submitted, but
> was not clearing it before reading "normal" data.  As it is only place
> which sets HOB bit in control register, and register reads should not
> be affected by other bits, let's just clear it when we are done with
> reading upper bytes so non-48bit commands do not have to touch ctl
> at all.
> 
> pata_scc suffered from same problem...
> 
> Signed-off-by: Petr Vandrovec <petr@vandrovec.name>
> 
> ---
> commit d09591edad8e75c9bc850d47b68a9a6f6f107998
> tree 1bd5a79c9cade9b7ebf68b13bf24a879b969e4c8
> parent a5fcaa210626a79465321e344c91a6a7dc3881fa
> author Petr Vandrovec <petr@vandrovec.name> Tue, 17 Jul 2007 03:45:43 -0700
> committer Petr Vandrovec <petr@vandrovec.name> Tue, 17 Jul 2007 03:45:43 -0700
> 
>  drivers/ata/libata-sff.c |    2 ++
>  drivers/ata/pata_scc.c   |    2 ++
>  2 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
> index ca7d224..6a579a9 100644
> --- a/drivers/ata/libata-sff.c
> +++ b/drivers/ata/libata-sff.c
> @@ -211,6 +211,8 @@ void ata_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
>  		tf->hob_lbal = ioread8(ioaddr->lbal_addr);
>  		tf->hob_lbam = ioread8(ioaddr->lbam_addr);
>  		tf->hob_lbah = ioread8(ioaddr->lbah_addr);
> +		iowrite8(tf->ctl, ioaddr->ctl_addr);
> +		ap->last_ctl = tf->ctl;
>  	}
>  }
>  
> diff --git a/drivers/ata/pata_scc.c b/drivers/ata/pata_scc.c
> index c55667e..60cce07 100644
> --- a/drivers/ata/pata_scc.c
> +++ b/drivers/ata/pata_scc.c
> @@ -358,6 +358,8 @@ static void scc_tf_read (struct ata_port *ap, struct ata_taskfile *tf)
>  		tf->hob_lbal = in_be32(ioaddr->lbal_addr);
>  		tf->hob_lbam = in_be32(ioaddr->lbam_addr);
>  		tf->hob_lbah = in_be32(ioaddr->lbah_addr);
> +		out_be32(ioaddr->ctl_addr, tf->ctl);
> +		ap->last_ctl = tf->ctl;

applied



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

end of thread, other threads:[~2007-07-20 11:45 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-13  6:56 [PATCH] Unbreak build of PMP with ACPI disabled Petr Vandrovec
2007-07-13  7:09 ` Tejun Heo
2007-07-13 23:30   ` Robert Hancock
2007-07-14  0:00     ` Ryan Power
2007-07-14  2:31       ` Tejun Heo
2007-07-14 12:13         ` Petr Vandrovec
2007-07-16  3:38           ` Ryan Power
2007-07-16 10:32           ` Tejun Heo
2007-07-17 10:54             ` [PATCH] Fix SMART reporting on 2.6.22 Petr Vandrovec
2007-07-18  3:08               ` Tejun Heo
2007-07-20 11:44               ` Jeff Garzik

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