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