Re: HPA patches
> Pondering about this, it's ATA_LBA according to the docs, specifying > that the address is an LBA. This is true for some commands, but not all. It gets used for other stuff too. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
Pondering about this, it's ATA_LBA according to the docs, specifying that the address is an LBA. This is true for some commands, but not all. It gets used for other stuff too. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Fri, Mar 23, 2007 at 01:03:15PM -0700, Randy Dunlap wrote: > > It's 0x40. Its a "command dependant bit" - no useful name. > > dependent. OK, thanks. > Hi, Pondering about this, it's ATA_LBA according to the docs, specifying that the address is an LBA. Cheers, Kyle - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Fri, Mar 23, 2007 at 01:03:15PM -0700, Randy Dunlap wrote: It's 0x40. Its a command dependant bit - no useful name. dependent. OK, thanks. Hi, Pondering about this, it's ATA_LBA according to the docs, specifying that the address is an LBA. Cheers, Kyle - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Wed, Mar 28, 2007 at 10:54:31PM +0100, Alan Cox wrote: > > Hm. I tried adding it in the eh code after ata_set_mode() in > > ata_eh_recover(), which alters the problem slightly - hpa_sectors is > > still 0, so the taskfile call is still failing, but now the system just > > stops at around the time that anything attempts to access sda with no > > errors other than > > I wonder if the firmware is dying when we ask the disk to go zero sized > rather than erroring politely. I'm not sure hth HPA sectors can come back > as zero but we can be fairly sure 0 means "no HPA" in this case I guess ? > The command is "Read Native Max Sectors" which should be the full disk size as long as the command is supported, and the size returned by IDENTIFY would be smaller if HPA was in use. AIUI at least. Cheers, Kyle - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Wed, Mar 28, 2007 at 10:54:31PM +0100, Alan Cox wrote: > I wonder if the firmware is dying when we ask the disk to go zero sized > rather than erroring politely. I'm not sure hth HPA sectors can come back > as zero but we can be fairly sure 0 means "no HPA" in this case I guess ? No, it seems to be looking at 0 because ata_read_native_max_address_ext returns 0 in the error case - the error that ata_exec_internal generates seems to be AC_ERR_HSM. Since 0 isn't > the size reported, we'll never try to resize it anyway, judging by ata_hpa_resize - that is, it seems to be the ata_read_native_max_address_ext call that breaks it. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
> Hm. I tried adding it in the eh code after ata_set_mode() in > ata_eh_recover(), which alters the problem slightly - hpa_sectors is > still 0, so the taskfile call is still failing, but now the system just > stops at around the time that anything attempts to access sda with no > errors other than I wonder if the firmware is dying when we ask the disk to go zero sized rather than erroring politely. I'm not sure hth HPA sectors can come back as zero but we can be fairly sure 0 means "no HPA" in this case I guess ? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Wed, Mar 28, 2007 at 10:57:54AM +0100, Alan Cox wrote: > Ok thanks. This is interesting as the only thing new it is doing is > asking for the HPA size. Does I think explain the problem however: Can > you move the HPA setting call to after the mode has been set and see if > that makes the problem vanish ? Hm. I tried adding it in the eh code after ata_set_mode() in ata_eh_recover(), which alters the problem slightly - hpa_sectors is still 0, so the taskfile call is still failing, but now the system just stops at around the time that anything attempts to access sda with no errors other than sd: 2:0:1:0: timing out command, waited 180s sd: 2:0:1:0: SCSI error: return code = 0x0028 end_request: I/O error, dev sda, sector 0 Buffer I/O error on device sda, logical block 0 -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Wed, 28 Mar 2007 01:08:52 +0100 Matthew Garrett <[EMAIL PROTECTED]> wrote: > On Fri, Mar 23, 2007 at 07:13:21PM +, Alan Cox wrote: > > For reference this is what I am currently using with 2.6.21-rc4-mm1 and > > it is working for all my test cases so far: Its basically Kyle's patch > > with a libata switch to turn it on/off and some minor fixups from > > the original patch as posted > > Fails for me on a Macbook Pro with: Ok thanks. This is interesting as the only thing new it is doing is asking for the HPA size. Does I think explain the problem however: Can you move the HPA setting call to after the mode has been set and see if that makes the problem vanish ? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Wed, 28 Mar 2007 01:08:52 +0100 Matthew Garrett [EMAIL PROTECTED] wrote: On Fri, Mar 23, 2007 at 07:13:21PM +, Alan Cox wrote: For reference this is what I am currently using with 2.6.21-rc4-mm1 and it is working for all my test cases so far: Its basically Kyle's patch with a libata switch to turn it on/off and some minor fixups from the original patch as posted Fails for me on a Macbook Pro with: Ok thanks. This is interesting as the only thing new it is doing is asking for the HPA size. Does I think explain the problem however: Can you move the HPA setting call to after the mode has been set and see if that makes the problem vanish ? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Wed, Mar 28, 2007 at 10:57:54AM +0100, Alan Cox wrote: Ok thanks. This is interesting as the only thing new it is doing is asking for the HPA size. Does I think explain the problem however: Can you move the HPA setting call to after the mode has been set and see if that makes the problem vanish ? Hm. I tried adding it in the eh code after ata_set_mode() in ata_eh_recover(), which alters the problem slightly - hpa_sectors is still 0, so the taskfile call is still failing, but now the system just stops at around the time that anything attempts to access sda with no errors other than sd: 2:0:1:0: timing out command, waited 180s sd: 2:0:1:0: SCSI error: return code = 0x0028 end_request: I/O error, dev sda, sector 0 Buffer I/O error on device sda, logical block 0 -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
Hm. I tried adding it in the eh code after ata_set_mode() in ata_eh_recover(), which alters the problem slightly - hpa_sectors is still 0, so the taskfile call is still failing, but now the system just stops at around the time that anything attempts to access sda with no errors other than I wonder if the firmware is dying when we ask the disk to go zero sized rather than erroring politely. I'm not sure hth HPA sectors can come back as zero but we can be fairly sure 0 means no HPA in this case I guess ? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Wed, Mar 28, 2007 at 10:54:31PM +0100, Alan Cox wrote: I wonder if the firmware is dying when we ask the disk to go zero sized rather than erroring politely. I'm not sure hth HPA sectors can come back as zero but we can be fairly sure 0 means no HPA in this case I guess ? No, it seems to be looking at 0 because ata_read_native_max_address_ext returns 0 in the error case - the error that ata_exec_internal generates seems to be AC_ERR_HSM. Since 0 isn't the size reported, we'll never try to resize it anyway, judging by ata_hpa_resize - that is, it seems to be the ata_read_native_max_address_ext call that breaks it. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Wed, Mar 28, 2007 at 10:54:31PM +0100, Alan Cox wrote: Hm. I tried adding it in the eh code after ata_set_mode() in ata_eh_recover(), which alters the problem slightly - hpa_sectors is still 0, so the taskfile call is still failing, but now the system just stops at around the time that anything attempts to access sda with no errors other than I wonder if the firmware is dying when we ask the disk to go zero sized rather than erroring politely. I'm not sure hth HPA sectors can come back as zero but we can be fairly sure 0 means no HPA in this case I guess ? The command is Read Native Max Sectors which should be the full disk size as long as the command is supported, and the size returned by IDENTIFY would be smaller if HPA was in use. AIUI at least. Cheers, Kyle - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Wed, Mar 28, 2007 at 01:08:52AM +0100, Matthew Garrett wrote: > ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 0 ^ Does this just indicate the lack of an hpa? If so, the /* if no hpa, both should be equal */ comment seems to be wrong (or, alternatively, it's the ata_read_native_max_address_ext call that's failing and returning garbage? I'll look into that) -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Fri, Mar 23, 2007 at 07:13:21PM +, Alan Cox wrote: > For reference this is what I am currently using with 2.6.21-rc4-mm1 and > it is working for all my test cases so far: Its basically Kyle's patch > with a libata switch to turn it on/off and some minor fixups from > the original patch as posted Fails for me on a Macbook Pro with: ata1: PATA max UDMA/133 cmd 0x000101f0 ctl 0x000103f6 bmdma 0x000140b0 irq 14 ata2: PATA max UDMA/133 cmd 0x00010170 ctl 0x00010376 bmdma 0x000140b8 irq 15 scsi0: ata_piix ata1.00: ATAPI, max UDMA/66 ata1.00: configured for UDMA/66 scsi1 : ata_piix ATA: abornaml status 0x7f on port 0x00010177 scsi 0:0:0:0: CD-ROM MATSHITA DVD-R UJ-857D KCV9 PQ : 0 ANSI: 5 ata_piix :00:1f.2: MAP [ P0 p" XX XX ] ata_piix :00:1f.2: invalid MAP value 0 ata3: SATA max UDMA/133 cmd 0x000140c8 ctl 0x000140e6 bmdma 0x000140a0 irq 20 ata4: SATA max UDMA/133 cmd 0x000140c0 ctl 0x000140e2 bmdma 0x000140a8 irq 20 scsi2 : ata_piix ATA: abnormal status 0x7F on port 0x000140cf ATA: abnormal status 0x7F on port 0x000140cf ATA: abnormal status 0x7F on port 0x000140cf ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 0 ata3.01: ATA-8: FUJITSU MHW2120BH, 00810013, max UDMA/100 ata3.01: 234441648 sectors, multi 16: LBA48 NCQ (depth 0/32) ata3.01: failed to set xfermode (err_mask=0x40) ata3: failed to recover some devices, retrying in 5 secs ATA: abnormal status 0x7F on port 0x000140cf ATA: abnormal status 0x7F on port 0x000140cf ATA: abnormal status 0x7F on port 0x000140cf ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 0 ata3.01: failed to set xfermode (err_mask=0x40) ata3.01: limiting speed to UDMA/100:PIO3 ata3: failed to recover some devices, retrying in 5 secs ATA: abnormal status 0x7F on port 0x000140cf ATA: abnormal status 0x7F on port 0x000140cf ATA: abnormal status 0x7F on port 0x000140cf ata3.01: qc timeout (cmd 0x27) ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 0 ata3.01: failed to set xfermode (err_mask=0x40) ata3.01: disabled scsi3: ata_piix ATA: abnormal status 0x7F on port 0x000140c7 and I end up with no root filesystem. Reverting the patch leaves things working. This is the ubuntu tree - I can try libata-dev if you think there's likely to be any relevant difference. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Fri, Mar 23, 2007 at 07:13:21PM +, Alan Cox wrote: For reference this is what I am currently using with 2.6.21-rc4-mm1 and it is working for all my test cases so far: Its basically Kyle's patch with a libata switch to turn it on/off and some minor fixups from the original patch as posted Fails for me on a Macbook Pro with: ata1: PATA max UDMA/133 cmd 0x000101f0 ctl 0x000103f6 bmdma 0x000140b0 irq 14 ata2: PATA max UDMA/133 cmd 0x00010170 ctl 0x00010376 bmdma 0x000140b8 irq 15 scsi0: ata_piix ata1.00: ATAPI, max UDMA/66 ata1.00: configured for UDMA/66 scsi1 : ata_piix ATA: abornaml status 0x7f on port 0x00010177 scsi 0:0:0:0: CD-ROM MATSHITA DVD-R UJ-857D KCV9 PQ : 0 ANSI: 5 ata_piix :00:1f.2: MAP [ P0 p XX XX ] ata_piix :00:1f.2: invalid MAP value 0 ata3: SATA max UDMA/133 cmd 0x000140c8 ctl 0x000140e6 bmdma 0x000140a0 irq 20 ata4: SATA max UDMA/133 cmd 0x000140c0 ctl 0x000140e2 bmdma 0x000140a8 irq 20 scsi2 : ata_piix ATA: abnormal status 0x7F on port 0x000140cf ATA: abnormal status 0x7F on port 0x000140cf ATA: abnormal status 0x7F on port 0x000140cf ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 0 ata3.01: ATA-8: FUJITSU MHW2120BH, 00810013, max UDMA/100 ata3.01: 234441648 sectors, multi 16: LBA48 NCQ (depth 0/32) ata3.01: failed to set xfermode (err_mask=0x40) ata3: failed to recover some devices, retrying in 5 secs ATA: abnormal status 0x7F on port 0x000140cf ATA: abnormal status 0x7F on port 0x000140cf ATA: abnormal status 0x7F on port 0x000140cf ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 0 ata3.01: failed to set xfermode (err_mask=0x40) ata3.01: limiting speed to UDMA/100:PIO3 ata3: failed to recover some devices, retrying in 5 secs ATA: abnormal status 0x7F on port 0x000140cf ATA: abnormal status 0x7F on port 0x000140cf ATA: abnormal status 0x7F on port 0x000140cf ata3.01: qc timeout (cmd 0x27) ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 0 ata3.01: failed to set xfermode (err_mask=0x40) ata3.01: disabled scsi3: ata_piix ATA: abnormal status 0x7F on port 0x000140c7 and I end up with no root filesystem. Reverting the patch leaves things working. This is the ubuntu tree - I can try libata-dev if you think there's likely to be any relevant difference. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Wed, Mar 28, 2007 at 01:08:52AM +0100, Matthew Garrett wrote: ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 0 ^ Does this just indicate the lack of an hpa? If so, the /* if no hpa, both should be equal */ comment seems to be wrong (or, alternatively, it's the ata_read_native_max_address_ext call that's failing and returning garbage? I'll look into that) -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
Matthew Garrett wrote: On Fri, Mar 23, 2007 at 07:13:21PM +, Alan Cox wrote: +static int ata_ignore_hpa = 0; +module_param_named(ignore_hpa, ata_ignore_hpa, int, 0644); +MODULE_PARM_DESC(ignore_hpa, "Ignore HPA (0=off 1=on)"); I'm not sure I like the language here. "Ignore HPA" appears to mean "Explicitly disable the HPA", which I guess is one interpretation of "ignore" - however, naively I'd expect "Ignore HPA" to mean "Don't touch the HPA" with the result that it would remain inaccessible to userspace. "ignore" sounds more appropriate to me. We're not just making it inaccessible, we are actively ignoring all traces of its existence everywhere, when that setting is enabled. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Fri, Mar 23, 2007 at 07:13:21PM +, Alan Cox wrote: > For reference this is what I am currently using with 2.6.21-rc4-mm1 and > it is working for all my test cases so far: Its basically Kyle's patch > with a libata switch to turn it on/off and some minor fixups from > the original patch as posted > This looks good to me. Thanks for picking this up! Acked-By: Kyle McMartin <[EMAIL PROTECTED]> Cheers, Kyle M. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Fri, Mar 23, 2007 at 07:13:21PM +, Alan Cox wrote: For reference this is what I am currently using with 2.6.21-rc4-mm1 and it is working for all my test cases so far: Its basically Kyle's patch with a libata switch to turn it on/off and some minor fixups from the original patch as posted This looks good to me. Thanks for picking this up! Acked-By: Kyle McMartin [EMAIL PROTECTED] Cheers, Kyle M. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
Matthew Garrett wrote: On Fri, Mar 23, 2007 at 07:13:21PM +, Alan Cox wrote: +static int ata_ignore_hpa = 0; +module_param_named(ignore_hpa, ata_ignore_hpa, int, 0644); +MODULE_PARM_DESC(ignore_hpa, Ignore HPA (0=off 1=on)); I'm not sure I like the language here. Ignore HPA appears to mean Explicitly disable the HPA, which I guess is one interpretation of ignore - however, naively I'd expect Ignore HPA to mean Don't touch the HPA with the result that it would remain inaccessible to userspace. ignore sounds more appropriate to me. We're not just making it inaccessible, we are actively ignoring all traces of its existence everywhere, when that setting is enabled. Jeff - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
Alan Cox wrote: What is 0x40? can it be #defined (or enum-ed) instead of a magic value? please? (more of same below) >>> It's 0x40. Its a "command dependant bit" - no useful name. >> dependent. OK, thanks. > > IDE is a bit like that. I'm amazed some of the command flags arent in > latin. > Hmm, what's latin for "no useful name?" You could call it that. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Fri, Mar 23, 2007 at 07:13:21PM +, Alan Cox wrote: > +static int ata_ignore_hpa = 0; > +module_param_named(ignore_hpa, ata_ignore_hpa, int, 0644); > +MODULE_PARM_DESC(ignore_hpa, "Ignore HPA (0=off 1=on)"); I'm not sure I like the language here. "Ignore HPA" appears to mean "Explicitly disable the HPA", which I guess is one interpretation of "ignore" - however, naively I'd expect "Ignore HPA" to mean "Don't touch the HPA" with the result that it would remain inaccessible to userspace. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Fri, 23 Mar 2007 12:22:32 -0700 (PDT) David Miller <[EMAIL PROTECTED]> wrote: > From: Alan Cox <[EMAIL PROTECTED]> > Date: Fri, 23 Mar 2007 20:08:19 + > > > u64 is always unsigned long long (and its debug anyway) > > It's plain "unsigned long" on sparc64 and some other 64-bit platforms. I stand corrected. In which case Randy is right and I do need to go fix that formatting. Thanks Alan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
> > > What is 0x40? can it be #defined (or enum-ed) instead of a magic > > > value? please? (more of same below) > > > > It's 0x40. Its a "command dependant bit" - no useful name. > > dependent. OK, thanks. IDE is a bit like that. I'm amazed some of the command flags arent in latin. > Already corrected (printk types). And putting > hpa_sectors); > on a separate line doesn't hurt readability. Maybe Thanks for looking over the diff. Alan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Fri, 23 Mar 2007 20:08:19 + Alan Cox wrote: > > > +static int ata_ignore_hpa = 0; > > > > Don't init to 0. Not needed, bloats binary files. > > It'll be one for the final release 8) > > > > +module_param_named(ignore_hpa, ata_ignore_hpa, int, 0644); > > > +MODULE_PARM_DESC(ignore_hpa, "Ignore HPA (0=off 1=on)"); > > > > So 1 = on = ignore, right? > > Yes. > > > > + tf.command = ATA_CMD_READ_NATIVE_MAX_EXT; > > > + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR; > > > + tf.protocol |= ATA_PROT_NODATA; > > > + tf.device = 0x40; > > > > What is 0x40? can it be #defined (or enum-ed) instead of a magic > > value? please? (more of same below) > > It's 0x40. Its a "command dependant bit" - no useful name. dependent. OK, thanks. > > > + u64 sectors = dev->n_sectors; > > > + u64 hpa_sectors; > > > + > > > + if (ata_id_has_lba48(dev->id)) > > > + hpa_sectors = ata_read_native_max_address_ext(dev); > > > + else > > > + hpa_sectors = ata_read_native_max_address(dev); > > > + > > > + /* if no hpa, both should be equal */ > > > + ata_dev_printk(dev, KERN_INFO, "%s 1: sectors = %lld, hpa_sectors = > > > %lld\n", > > > + __FUNCTION__, sectors, hpa_sectors); > > > > (long long) or (unsigned long long) on sectors and hpa_sectors... > > u64 is always unsigned long long (and its debug anyway) > > > > > > + > > > + if (hpa_sectors > sectors) { > > > + ata_dev_printk(dev, KERN_INFO, > > > + "Host Protected Area detected:\n" > > > + "\tcurrent size: %lld sectors\n" > > > + "\tnative size: %lld sectors\n", > > > + sectors, hpa_sectors); > > > > printk format types ok? > > Yes > > > > + if (hpa_sectors) { > > > + ata_dev_printk(dev, KERN_INFO, > > > + "native size increased to %lld > > > sectors\n", hpa_sectors); > > > > Line lengths < 80 and printk format types? > > See above, and the 80 column fascists can suffer in the name of > readability. Already corrected (printk types). And putting hpa_sectors); on a separate line doesn't hurt readability. --- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
From: Alan Cox <[EMAIL PROTECTED]> Date: Fri, 23 Mar 2007 20:08:19 + > u64 is always unsigned long long (and its debug anyway) It's plain "unsigned long" on sparc64 and some other 64-bit platforms. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
> > +static int ata_ignore_hpa = 0; > > Don't init to 0. Not needed, bloats binary files. It'll be one for the final release 8) > > +module_param_named(ignore_hpa, ata_ignore_hpa, int, 0644); > > +MODULE_PARM_DESC(ignore_hpa, "Ignore HPA (0=off 1=on)"); > > So 1 = on = ignore, right? Yes. > > + tf.command = ATA_CMD_READ_NATIVE_MAX_EXT; > > + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR; > > + tf.protocol |= ATA_PROT_NODATA; > > + tf.device = 0x40; > > What is 0x40? can it be #defined (or enum-ed) instead of a magic > value? please? (more of same below) It's 0x40. Its a "command dependant bit" - no useful name. > > + u64 sectors = dev->n_sectors; > > + u64 hpa_sectors; > > + > > + if (ata_id_has_lba48(dev->id)) > > + hpa_sectors = ata_read_native_max_address_ext(dev); > > + else > > + hpa_sectors = ata_read_native_max_address(dev); > > + > > + /* if no hpa, both should be equal */ > > + ata_dev_printk(dev, KERN_INFO, "%s 1: sectors = %lld, hpa_sectors = > > %lld\n", > > + __FUNCTION__, sectors, hpa_sectors); > > (long long) or (unsigned long long) on sectors and hpa_sectors... u64 is always unsigned long long (and its debug anyway) > > > + > > + if (hpa_sectors > sectors) { > > + ata_dev_printk(dev, KERN_INFO, > > + "Host Protected Area detected:\n" > > + "\tcurrent size: %lld sectors\n" > > + "\tnative size: %lld sectors\n", > > + sectors, hpa_sectors); > > printk format types ok? Yes > > + if (hpa_sectors) { > > + ata_dev_printk(dev, KERN_INFO, > > + "native size increased to %lld > > sectors\n", hpa_sectors); > > Line lengths < 80 and printk format types? See above, and the 80 column fascists can suffer in the name of readability. Alan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Fri, 23 Mar 2007 19:13:21 + Alan Cox wrote: > For reference this is what I am currently using with 2.6.21-rc4-mm1 and > it is working for all my test cases so far: Its basically Kyle's patch > with a libata switch to turn it on/off and some minor fixups from > the original patch as posted > > diff -u --new-file --recursive --exclude-from /usr/src/exclude > linux.vanilla-2.6.21-rc4-mm1/drivers/ata/libata-core.c > linux-2.6.21-rc4-mm1/drivers/ata/libata-core.c > --- linux.vanilla-2.6.21-rc4-mm1/drivers/ata/libata-core.c2007-03-23 > 11:49:49.0 + > +++ linux-2.6.21-rc4-mm1/drivers/ata/libata-core.c2007-03-23 > 13:05:15.0 + > @@ -89,6 +89,10 @@ > module_param_named(fua, libata_fua, int, 0444); > MODULE_PARM_DESC(fua, "FUA support (0=off, 1=on)"); > > +static int ata_ignore_hpa = 0; Don't init to 0. Not needed, bloats binary files. > +module_param_named(ignore_hpa, ata_ignore_hpa, int, 0644); > +MODULE_PARM_DESC(ignore_hpa, "Ignore HPA (0=off 1=on)"); So 1 = on = ignore, right? > + > static int ata_probe_timeout = ATA_TMOUT_INTERNAL / HZ; > module_param(ata_probe_timeout, int, 0444); > MODULE_PARM_DESC(ata_probe_timeout, "Set ATA probing timeout (seconds)"); > @@ -808,6 +812,202 @@ > *p = '\0'; > } > > + > +/** > + * ata_read_native_max_address_ext - LBA48 native max query > + * @dev: Device to query > + * > + * Performa an LBA48 size query upon the device in question. Return the Perform > + * actual LBA48 size or zero if the command fails. > + */ > + > +static u64 ata_read_native_max_address_ext(struct ata_device *dev) > +{ > + unsigned int err; > + struct ata_taskfile tf; > + > + ata_tf_init(dev, ); > + > + tf.command = ATA_CMD_READ_NATIVE_MAX_EXT; > + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR; > + tf.protocol |= ATA_PROT_NODATA; > + tf.device = 0x40; What is 0x40? can it be #defined (or enum-ed) instead of a magic value? please? (more of same below) > + > + err = ata_exec_internal(dev, , NULL, DMA_NONE, NULL, 0); > + if (err) > + return 0; > + > + return ata_tf_to_lba48(); > +} > + > +/** > + * ata_read_native_max_address - LBA28 native max query > + * @dev: Device to query > + * > + * Performa an LBA28 size query upon the device in question. Return the Perform > + * actual LBA28 size or zero if the command fails. > + */ > + > +static u64 ata_read_native_max_address(struct ata_device *dev) > +{ > + unsigned int err; > + struct ata_taskfile tf; > + > + ata_tf_init(dev, ); > + > + tf.command = ATA_CMD_READ_NATIVE_MAX; > + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; > + tf.protocol |= ATA_PROT_NODATA; > + tf.device = 0x40; > + > + err = ata_exec_internal(dev, , NULL, DMA_NONE, NULL, 0); > + if (err) > + return 0; > + > + return ata_tf_to_lba(); > +} > + > +/** > + * ata_set_native_max_address_ext - LBA48 native max set > + * @dev: Device to query > + * > + * Perform an LBA48 size set max upon the device in question. Return the > + * actual LBA48 size or zero if the command fails. > + */ > + > +static u64 ata_set_native_max_address_ext(struct ata_device *dev, u64 > new_sectors) > +{ ... > +} > + > +/** > + * ata_set_native_max_address - LBA28 native max set > + * @dev: Device to query > + * > + * Perform an LBA28 size set max upon the device in question. Return the > + * actual LBA28 size or zero if the command fails. > + */ > + > +static u64 ata_set_native_max_address(struct ata_device *dev, u64 > new_sectors) > +{ ... > +} > + > +/** > + * ata_hpa_resize - Resize a device with an HPA set > + * @dev: Device to resize > + * > + * Read the size of an LBA28 or LBA48 disk with HPA features and resize > + * it if required to the full size of the media. The caller must check > + * the drive has the HPA feature set enabled. > + */ > + > +static u64 ata_hpa_resize(struct ata_device *dev) > +{ > + u64 sectors = dev->n_sectors; > + u64 hpa_sectors; > + > + if (ata_id_has_lba48(dev->id)) > + hpa_sectors = ata_read_native_max_address_ext(dev); > + else > + hpa_sectors = ata_read_native_max_address(dev); > + > + /* if no hpa, both should be equal */ > + ata_dev_printk(dev, KERN_INFO, "%s 1: sectors = %lld, hpa_sectors = > %lld\n", > + __FUNCTION__, sectors, hpa_sectors); (long long) or (unsigned long long) on sectors and hpa_sectors... > + > + if (hpa_sectors > sectors) { > + ata_dev_printk(dev, KERN_INFO, > + "Host Protected Area detected:\n" > + "\tcurrent size: %lld sectors\n" > + "\tnative size: %lld sectors\n", > + sectors, hpa_sectors); printk format types ok? > + > + if (ata_ignore_hpa) { > + if
HPA patches
For reference this is what I am currently using with 2.6.21-rc4-mm1 and it is working for all my test cases so far: Its basically Kyle's patch with a libata switch to turn it on/off and some minor fixups from the original patch as posted diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.21-rc4-mm1/drivers/ata/libata-core.c linux-2.6.21-rc4-mm1/drivers/ata/libata-core.c --- linux.vanilla-2.6.21-rc4-mm1/drivers/ata/libata-core.c 2007-03-23 11:49:49.0 + +++ linux-2.6.21-rc4-mm1/drivers/ata/libata-core.c 2007-03-23 13:05:15.0 + @@ -89,6 +89,10 @@ module_param_named(fua, libata_fua, int, 0444); MODULE_PARM_DESC(fua, "FUA support (0=off, 1=on)"); +static int ata_ignore_hpa = 0; +module_param_named(ignore_hpa, ata_ignore_hpa, int, 0644); +MODULE_PARM_DESC(ignore_hpa, "Ignore HPA (0=off 1=on)"); + static int ata_probe_timeout = ATA_TMOUT_INTERNAL / HZ; module_param(ata_probe_timeout, int, 0444); MODULE_PARM_DESC(ata_probe_timeout, "Set ATA probing timeout (seconds)"); @@ -808,6 +812,202 @@ *p = '\0'; } +static u64 ata_tf_to_lba48(struct ata_taskfile *tf) +{ + u64 sectors = 0; + + sectors |= ((u64)(tf->hob_lbah & 0xff)) << 40; + sectors |= ((u64)(tf->hob_lbam & 0xff)) << 32; + sectors |= (tf->hob_lbal & 0xff) << 24; + sectors |= (tf->lbah & 0xff) << 16; + sectors |= (tf->lbam & 0xff) << 8; + sectors |= (tf->lbal & 0xff); + + return ++sectors; +} + +static u64 ata_tf_to_lba(struct ata_taskfile *tf) +{ + u64 sectors = 0; + + sectors |= (tf->device & 0x0f) << 24; + sectors |= (tf->lbah & 0xff) << 16; + sectors |= (tf->lbam & 0xff) << 8; + sectors |= (tf->lbal & 0xff); + + return ++sectors; +} + +/** + * ata_read_native_max_address_ext - LBA48 native max query + * @dev: Device to query + * + * Performa an LBA48 size query upon the device in question. Return the + * actual LBA48 size or zero if the command fails. + */ + +static u64 ata_read_native_max_address_ext(struct ata_device *dev) +{ + unsigned int err; + struct ata_taskfile tf; + + ata_tf_init(dev, ); + + tf.command = ATA_CMD_READ_NATIVE_MAX_EXT; + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR; + tf.protocol |= ATA_PROT_NODATA; + tf.device = 0x40; + + err = ata_exec_internal(dev, , NULL, DMA_NONE, NULL, 0); + if (err) + return 0; + + return ata_tf_to_lba48(); +} + +/** + * ata_read_native_max_address - LBA28 native max query + * @dev: Device to query + * + * Performa an LBA28 size query upon the device in question. Return the + * actual LBA28 size or zero if the command fails. + */ + +static u64 ata_read_native_max_address(struct ata_device *dev) +{ + unsigned int err; + struct ata_taskfile tf; + + ata_tf_init(dev, ); + + tf.command = ATA_CMD_READ_NATIVE_MAX; + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; + tf.protocol |= ATA_PROT_NODATA; + tf.device = 0x40; + + err = ata_exec_internal(dev, , NULL, DMA_NONE, NULL, 0); + if (err) + return 0; + + return ata_tf_to_lba(); +} + +/** + * ata_set_native_max_address_ext - LBA48 native max set + * @dev: Device to query + * + * Perform an LBA48 size set max upon the device in question. Return the + * actual LBA48 size or zero if the command fails. + */ + +static u64 ata_set_native_max_address_ext(struct ata_device *dev, u64 new_sectors) +{ + unsigned int err; + struct ata_taskfile tf; + + new_sectors--; + + ata_tf_init(dev, ); + + tf.command = ATA_CMD_SET_MAX_EXT; + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR; + tf.protocol |= ATA_PROT_NODATA; + tf.device = 0x40; + + tf.lbal = (new_sectors >> 0) & 0xff; + tf.lbam = (new_sectors >> 8) & 0xff; + tf.lbah = (new_sectors >> 16) & 0xff; + + tf.hob_lbal = (new_sectors >> 24) & 0xff; + tf.hob_lbam = (new_sectors >> 32) & 0xff; + tf.hob_lbah = (new_sectors >> 40) & 0xff; + + err = ata_exec_internal(dev, , NULL, DMA_NONE, NULL, 0); + if (err) + return 0; + + return ata_tf_to_lba48(); +} + +/** + * ata_set_native_max_address - LBA28 native max set + * @dev: Device to query + * + * Perform an LBA28 size set max upon the device in question. Return the + * actual LBA28 size or zero if the command fails. + */ + +static u64 ata_set_native_max_address(struct ata_device *dev, u64 new_sectors) +{ + unsigned int err; + struct ata_taskfile tf; + + new_sectors--; + + ata_tf_init(dev, ); + + tf.command = ATA_CMD_SET_MAX; + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; + tf.protocol |= ATA_PROT_NODATA; + + tf.lbal = (new_sectors >> 0) & 0xff; + tf.lbam = (new_sectors >>
HPA patches
For reference this is what I am currently using with 2.6.21-rc4-mm1 and it is working for all my test cases so far: Its basically Kyle's patch with a libata switch to turn it on/off and some minor fixups from the original patch as posted diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.21-rc4-mm1/drivers/ata/libata-core.c linux-2.6.21-rc4-mm1/drivers/ata/libata-core.c --- linux.vanilla-2.6.21-rc4-mm1/drivers/ata/libata-core.c 2007-03-23 11:49:49.0 + +++ linux-2.6.21-rc4-mm1/drivers/ata/libata-core.c 2007-03-23 13:05:15.0 + @@ -89,6 +89,10 @@ module_param_named(fua, libata_fua, int, 0444); MODULE_PARM_DESC(fua, FUA support (0=off, 1=on)); +static int ata_ignore_hpa = 0; +module_param_named(ignore_hpa, ata_ignore_hpa, int, 0644); +MODULE_PARM_DESC(ignore_hpa, Ignore HPA (0=off 1=on)); + static int ata_probe_timeout = ATA_TMOUT_INTERNAL / HZ; module_param(ata_probe_timeout, int, 0444); MODULE_PARM_DESC(ata_probe_timeout, Set ATA probing timeout (seconds)); @@ -808,6 +812,202 @@ *p = '\0'; } +static u64 ata_tf_to_lba48(struct ata_taskfile *tf) +{ + u64 sectors = 0; + + sectors |= ((u64)(tf-hob_lbah 0xff)) 40; + sectors |= ((u64)(tf-hob_lbam 0xff)) 32; + sectors |= (tf-hob_lbal 0xff) 24; + sectors |= (tf-lbah 0xff) 16; + sectors |= (tf-lbam 0xff) 8; + sectors |= (tf-lbal 0xff); + + return ++sectors; +} + +static u64 ata_tf_to_lba(struct ata_taskfile *tf) +{ + u64 sectors = 0; + + sectors |= (tf-device 0x0f) 24; + sectors |= (tf-lbah 0xff) 16; + sectors |= (tf-lbam 0xff) 8; + sectors |= (tf-lbal 0xff); + + return ++sectors; +} + +/** + * ata_read_native_max_address_ext - LBA48 native max query + * @dev: Device to query + * + * Performa an LBA48 size query upon the device in question. Return the + * actual LBA48 size or zero if the command fails. + */ + +static u64 ata_read_native_max_address_ext(struct ata_device *dev) +{ + unsigned int err; + struct ata_taskfile tf; + + ata_tf_init(dev, tf); + + tf.command = ATA_CMD_READ_NATIVE_MAX_EXT; + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR; + tf.protocol |= ATA_PROT_NODATA; + tf.device = 0x40; + + err = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0); + if (err) + return 0; + + return ata_tf_to_lba48(tf); +} + +/** + * ata_read_native_max_address - LBA28 native max query + * @dev: Device to query + * + * Performa an LBA28 size query upon the device in question. Return the + * actual LBA28 size or zero if the command fails. + */ + +static u64 ata_read_native_max_address(struct ata_device *dev) +{ + unsigned int err; + struct ata_taskfile tf; + + ata_tf_init(dev, tf); + + tf.command = ATA_CMD_READ_NATIVE_MAX; + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; + tf.protocol |= ATA_PROT_NODATA; + tf.device = 0x40; + + err = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0); + if (err) + return 0; + + return ata_tf_to_lba(tf); +} + +/** + * ata_set_native_max_address_ext - LBA48 native max set + * @dev: Device to query + * + * Perform an LBA48 size set max upon the device in question. Return the + * actual LBA48 size or zero if the command fails. + */ + +static u64 ata_set_native_max_address_ext(struct ata_device *dev, u64 new_sectors) +{ + unsigned int err; + struct ata_taskfile tf; + + new_sectors--; + + ata_tf_init(dev, tf); + + tf.command = ATA_CMD_SET_MAX_EXT; + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR; + tf.protocol |= ATA_PROT_NODATA; + tf.device = 0x40; + + tf.lbal = (new_sectors 0) 0xff; + tf.lbam = (new_sectors 8) 0xff; + tf.lbah = (new_sectors 16) 0xff; + + tf.hob_lbal = (new_sectors 24) 0xff; + tf.hob_lbam = (new_sectors 32) 0xff; + tf.hob_lbah = (new_sectors 40) 0xff; + + err = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0); + if (err) + return 0; + + return ata_tf_to_lba48(tf); +} + +/** + * ata_set_native_max_address - LBA28 native max set + * @dev: Device to query + * + * Perform an LBA28 size set max upon the device in question. Return the + * actual LBA28 size or zero if the command fails. + */ + +static u64 ata_set_native_max_address(struct ata_device *dev, u64 new_sectors) +{ + unsigned int err; + struct ata_taskfile tf; + + new_sectors--; + + ata_tf_init(dev, tf); + + tf.command = ATA_CMD_SET_MAX; + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; + tf.protocol |= ATA_PROT_NODATA; + + tf.lbal = (new_sectors 0) 0xff; + tf.lbam = (new_sectors 8) 0xff; + tf.lbah = (new_sectors
Re: HPA patches
On Fri, 23 Mar 2007 19:13:21 + Alan Cox wrote: For reference this is what I am currently using with 2.6.21-rc4-mm1 and it is working for all my test cases so far: Its basically Kyle's patch with a libata switch to turn it on/off and some minor fixups from the original patch as posted diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.21-rc4-mm1/drivers/ata/libata-core.c linux-2.6.21-rc4-mm1/drivers/ata/libata-core.c --- linux.vanilla-2.6.21-rc4-mm1/drivers/ata/libata-core.c2007-03-23 11:49:49.0 + +++ linux-2.6.21-rc4-mm1/drivers/ata/libata-core.c2007-03-23 13:05:15.0 + @@ -89,6 +89,10 @@ module_param_named(fua, libata_fua, int, 0444); MODULE_PARM_DESC(fua, FUA support (0=off, 1=on)); +static int ata_ignore_hpa = 0; Don't init to 0. Not needed, bloats binary files. +module_param_named(ignore_hpa, ata_ignore_hpa, int, 0644); +MODULE_PARM_DESC(ignore_hpa, Ignore HPA (0=off 1=on)); So 1 = on = ignore, right? + static int ata_probe_timeout = ATA_TMOUT_INTERNAL / HZ; module_param(ata_probe_timeout, int, 0444); MODULE_PARM_DESC(ata_probe_timeout, Set ATA probing timeout (seconds)); @@ -808,6 +812,202 @@ *p = '\0'; } + +/** + * ata_read_native_max_address_ext - LBA48 native max query + * @dev: Device to query + * + * Performa an LBA48 size query upon the device in question. Return the Perform + * actual LBA48 size or zero if the command fails. + */ + +static u64 ata_read_native_max_address_ext(struct ata_device *dev) +{ + unsigned int err; + struct ata_taskfile tf; + + ata_tf_init(dev, tf); + + tf.command = ATA_CMD_READ_NATIVE_MAX_EXT; + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR; + tf.protocol |= ATA_PROT_NODATA; + tf.device = 0x40; What is 0x40? can it be #defined (or enum-ed) instead of a magic value? please? (more of same below) + + err = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0); + if (err) + return 0; + + return ata_tf_to_lba48(tf); +} + +/** + * ata_read_native_max_address - LBA28 native max query + * @dev: Device to query + * + * Performa an LBA28 size query upon the device in question. Return the Perform + * actual LBA28 size or zero if the command fails. + */ + +static u64 ata_read_native_max_address(struct ata_device *dev) +{ + unsigned int err; + struct ata_taskfile tf; + + ata_tf_init(dev, tf); + + tf.command = ATA_CMD_READ_NATIVE_MAX; + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; + tf.protocol |= ATA_PROT_NODATA; + tf.device = 0x40; + + err = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0); + if (err) + return 0; + + return ata_tf_to_lba(tf); +} + +/** + * ata_set_native_max_address_ext - LBA48 native max set + * @dev: Device to query + * + * Perform an LBA48 size set max upon the device in question. Return the + * actual LBA48 size or zero if the command fails. + */ + +static u64 ata_set_native_max_address_ext(struct ata_device *dev, u64 new_sectors) +{ ... +} + +/** + * ata_set_native_max_address - LBA28 native max set + * @dev: Device to query + * + * Perform an LBA28 size set max upon the device in question. Return the + * actual LBA28 size or zero if the command fails. + */ + +static u64 ata_set_native_max_address(struct ata_device *dev, u64 new_sectors) +{ ... +} + +/** + * ata_hpa_resize - Resize a device with an HPA set + * @dev: Device to resize + * + * Read the size of an LBA28 or LBA48 disk with HPA features and resize + * it if required to the full size of the media. The caller must check + * the drive has the HPA feature set enabled. + */ + +static u64 ata_hpa_resize(struct ata_device *dev) +{ + u64 sectors = dev-n_sectors; + u64 hpa_sectors; + + if (ata_id_has_lba48(dev-id)) + hpa_sectors = ata_read_native_max_address_ext(dev); + else + hpa_sectors = ata_read_native_max_address(dev); + + /* if no hpa, both should be equal */ + ata_dev_printk(dev, KERN_INFO, %s 1: sectors = %lld, hpa_sectors = %lld\n, + __FUNCTION__, sectors, hpa_sectors); (long long) or (unsigned long long) on sectors and hpa_sectors... + + if (hpa_sectors sectors) { + ata_dev_printk(dev, KERN_INFO, + Host Protected Area detected:\n + \tcurrent size: %lld sectors\n + \tnative size: %lld sectors\n, + sectors, hpa_sectors); printk format types ok? + + if (ata_ignore_hpa) { + if (ata_id_has_lba48(dev-id)) + hpa_sectors = ata_set_native_max_address_ext(dev, hpa_sectors); + else +
Re: HPA patches
+static int ata_ignore_hpa = 0; Don't init to 0. Not needed, bloats binary files. It'll be one for the final release 8) +module_param_named(ignore_hpa, ata_ignore_hpa, int, 0644); +MODULE_PARM_DESC(ignore_hpa, Ignore HPA (0=off 1=on)); So 1 = on = ignore, right? Yes. + tf.command = ATA_CMD_READ_NATIVE_MAX_EXT; + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR; + tf.protocol |= ATA_PROT_NODATA; + tf.device = 0x40; What is 0x40? can it be #defined (or enum-ed) instead of a magic value? please? (more of same below) It's 0x40. Its a command dependant bit - no useful name. + u64 sectors = dev-n_sectors; + u64 hpa_sectors; + + if (ata_id_has_lba48(dev-id)) + hpa_sectors = ata_read_native_max_address_ext(dev); + else + hpa_sectors = ata_read_native_max_address(dev); + + /* if no hpa, both should be equal */ + ata_dev_printk(dev, KERN_INFO, %s 1: sectors = %lld, hpa_sectors = %lld\n, + __FUNCTION__, sectors, hpa_sectors); (long long) or (unsigned long long) on sectors and hpa_sectors... u64 is always unsigned long long (and its debug anyway) + + if (hpa_sectors sectors) { + ata_dev_printk(dev, KERN_INFO, + Host Protected Area detected:\n + \tcurrent size: %lld sectors\n + \tnative size: %lld sectors\n, + sectors, hpa_sectors); printk format types ok? Yes + if (hpa_sectors) { + ata_dev_printk(dev, KERN_INFO, + native size increased to %lld sectors\n, hpa_sectors); Line lengths 80 and printk format types? See above, and the 80 column fascists can suffer in the name of readability. Alan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
From: Alan Cox [EMAIL PROTECTED] Date: Fri, 23 Mar 2007 20:08:19 + u64 is always unsigned long long (and its debug anyway) It's plain unsigned long on sparc64 and some other 64-bit platforms. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Fri, 23 Mar 2007 20:08:19 + Alan Cox wrote: +static int ata_ignore_hpa = 0; Don't init to 0. Not needed, bloats binary files. It'll be one for the final release 8) +module_param_named(ignore_hpa, ata_ignore_hpa, int, 0644); +MODULE_PARM_DESC(ignore_hpa, Ignore HPA (0=off 1=on)); So 1 = on = ignore, right? Yes. + tf.command = ATA_CMD_READ_NATIVE_MAX_EXT; + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR; + tf.protocol |= ATA_PROT_NODATA; + tf.device = 0x40; What is 0x40? can it be #defined (or enum-ed) instead of a magic value? please? (more of same below) It's 0x40. Its a command dependant bit - no useful name. dependent. OK, thanks. + u64 sectors = dev-n_sectors; + u64 hpa_sectors; + + if (ata_id_has_lba48(dev-id)) + hpa_sectors = ata_read_native_max_address_ext(dev); + else + hpa_sectors = ata_read_native_max_address(dev); + + /* if no hpa, both should be equal */ + ata_dev_printk(dev, KERN_INFO, %s 1: sectors = %lld, hpa_sectors = %lld\n, + __FUNCTION__, sectors, hpa_sectors); (long long) or (unsigned long long) on sectors and hpa_sectors... u64 is always unsigned long long (and its debug anyway) + + if (hpa_sectors sectors) { + ata_dev_printk(dev, KERN_INFO, + Host Protected Area detected:\n + \tcurrent size: %lld sectors\n + \tnative size: %lld sectors\n, + sectors, hpa_sectors); printk format types ok? Yes + if (hpa_sectors) { + ata_dev_printk(dev, KERN_INFO, + native size increased to %lld sectors\n, hpa_sectors); Line lengths 80 and printk format types? See above, and the 80 column fascists can suffer in the name of readability. Already corrected (printk types). And putting hpa_sectors); on a separate line doesn't hurt readability. --- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
What is 0x40? can it be #defined (or enum-ed) instead of a magic value? please? (more of same below) It's 0x40. Its a command dependant bit - no useful name. dependent. OK, thanks. IDE is a bit like that. I'm amazed some of the command flags arent in latin. Already corrected (printk types). And putting hpa_sectors); on a separate line doesn't hurt readability. Maybe Thanks for looking over the diff. Alan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Fri, 23 Mar 2007 12:22:32 -0700 (PDT) David Miller [EMAIL PROTECTED] wrote: From: Alan Cox [EMAIL PROTECTED] Date: Fri, 23 Mar 2007 20:08:19 + u64 is always unsigned long long (and its debug anyway) It's plain unsigned long on sparc64 and some other 64-bit platforms. I stand corrected. In which case Randy is right and I do need to go fix that formatting. Thanks Alan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
On Fri, Mar 23, 2007 at 07:13:21PM +, Alan Cox wrote: +static int ata_ignore_hpa = 0; +module_param_named(ignore_hpa, ata_ignore_hpa, int, 0644); +MODULE_PARM_DESC(ignore_hpa, Ignore HPA (0=off 1=on)); I'm not sure I like the language here. Ignore HPA appears to mean Explicitly disable the HPA, which I guess is one interpretation of ignore - however, naively I'd expect Ignore HPA to mean Don't touch the HPA with the result that it would remain inaccessible to userspace. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: HPA patches
Alan Cox wrote: What is 0x40? can it be #defined (or enum-ed) instead of a magic value? please? (more of same below) It's 0x40. Its a command dependant bit - no useful name. dependent. OK, thanks. IDE is a bit like that. I'm amazed some of the command flags arent in latin. Hmm, what's latin for no useful name? You could call it that. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/