Re: HPA patches

2007-04-13 Thread Alan Cox
> 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

2007-04-13 Thread Alan Cox
 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

2007-04-12 Thread Kyle McMartin
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

2007-04-12 Thread Kyle McMartin
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

2007-03-28 Thread Kyle McMartin
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

2007-03-28 Thread Matthew Garrett
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

2007-03-28 Thread Alan Cox
> 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

2007-03-28 Thread Matthew Garrett
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

2007-03-28 Thread Alan Cox
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

2007-03-28 Thread Alan Cox
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

2007-03-28 Thread Matthew Garrett
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

2007-03-28 Thread Alan Cox
 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

2007-03-28 Thread Matthew Garrett
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

2007-03-28 Thread Kyle McMartin
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

2007-03-27 Thread Matthew Garrett
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

2007-03-27 Thread Matthew Garrett
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

2007-03-27 Thread Matthew Garrett
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

2007-03-27 Thread Matthew Garrett
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

2007-03-26 Thread Jeff Garzik

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

2007-03-26 Thread Kyle McMartin
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

2007-03-26 Thread Kyle McMartin
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

2007-03-26 Thread Jeff Garzik

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

2007-03-23 Thread Chuck Ebbert
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

2007-03-23 Thread Matthew Garrett
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

2007-03-23 Thread Alan Cox
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

2007-03-23 Thread Alan Cox
> > > 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

2007-03-23 Thread Randy Dunlap
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

2007-03-23 Thread David Miller
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

2007-03-23 Thread Alan Cox
> > +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

2007-03-23 Thread Randy Dunlap
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

2007-03-23 Thread Alan Cox
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

2007-03-23 Thread Alan Cox
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

2007-03-23 Thread Randy Dunlap
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

2007-03-23 Thread Alan Cox
  +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

2007-03-23 Thread David Miller
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

2007-03-23 Thread Randy Dunlap
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

2007-03-23 Thread Alan Cox
   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

2007-03-23 Thread Alan Cox
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

2007-03-23 Thread Matthew Garrett
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

2007-03-23 Thread Chuck Ebbert
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/