* [PATCH 2.6.9-libata1-dev1] libata-scsi: hdparm support
@ 2004-10-22 23:20 Tobias Lorenz
2004-10-23 12:36 ` Bartlomiej Zolnierkiewicz
2004-10-30 12:43 ` Jeff Garzik
0 siblings, 2 replies; 9+ messages in thread
From: Tobias Lorenz @ 2004-10-22 23:20 UTC (permalink / raw)
To: linux-ide, jgarzik
Hi,
this patch adds support for transfering drive informations via the
hd_driveid structure to the hdparm utility. At the moment, only
cylinders, sectors, heads, model and firmware version are transfered.
Our intention was to display the supported and the used UDMA mode(s),
but we didn't find the correct structures yet to get these infos...
Signed-off-by: Tobias Lorenz <tobias.lorenz@gmx.net>
> --- a/drivers/scsi/libata-scsi.c 2004-10-20 22:07:50.000000000 +0200
> +++ b/drivers/scsi/libata-scsi.c 2004-10-20 19:25:15.000000000 +0200
> @@ -215,6 +215,17 @@
> struct ata_port *ap;
> struct ata_device *dev;
> int val = -EINVAL, rc = -EINVAL;
> + struct hd_driveid drv_id = {
> + .cyls = 0,
> + .sectors = 0,
> + .heads = 0,
> + .fw_rev = "",
> + .model = "",
> + .cur_cyls = 0,
> + .cur_heads = 0,
> + .cur_sectors = 0,
> + };
> + int geom[3];
>
> ap = (struct ata_port *) &scsidev->host->hostdata[0];
> if (!ap)
> @@ -249,7 +260,25 @@
> return -EACCES;
> return ata_task_ioctl(scsidev, arg);
>
> + case HDIO_GET_IDENTITY:
> + ata_std_bios_param(scsidev, NULL, dev->n_sectors, geom);
> + drv_id.cur_heads = drv_id.heads = geom[0];
> + drv_id.cur_sectors = drv_id.sectors = geom[1];
> + drv_id.cur_cyls = drv_id.cyls = geom[2];
> + strncpy((char *) &drv_id.model, scsidev->model, sizeof(drv_id.model));
> + strncpy((char *) &drv_id.fw_rev, scsidev->rev, sizeof(drv_id.fw_rev));
> + if(copy_to_user((char *) arg, (char *) &drv_id,
> + sizeof(drv_id)))
> + return(-EFAULT);
> + return 0;
> +
> default:
> rc = -EOPNOTSUPP;
> break;
> }
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 2.6.9-libata1-dev1] libata-scsi: hdparm support
2004-10-22 23:20 [PATCH 2.6.9-libata1-dev1] libata-scsi: hdparm support Tobias Lorenz
@ 2004-10-23 12:36 ` Bartlomiej Zolnierkiewicz
2004-10-23 21:59 ` Tobias Lorenz
2004-10-30 12:43 ` Jeff Garzik
1 sibling, 1 reply; 9+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2004-10-23 12:36 UTC (permalink / raw)
To: Tobias Lorenz; +Cc: linux-ide, jgarzik
Hi,
On Sat, 23 Oct 2004 01:20:53 +0200, Tobias Lorenz <tobias.lorenz@gmx.net> wrote:
> Hi,
>
> this patch adds support for transfering drive informations via the
> hd_driveid structure to the hdparm utility. At the moment, only
> cylinders, sectors, heads, model and firmware version are transfered.
>
> Our intention was to display the supported and the used UDMA mode(s),
> but we didn't find the correct structures yet to get these infos...
dev->id
HDIO_GET_IDENTITY (as used by 'hdparm -i') should return _original_
drive ID (dev->id should be returned) so currently used mode may not
be reported correctly.
'hdparm -I' should be already supported by current libata-dev tree.
Bartlomiej
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.9-libata1-dev1] libata-scsi: hdparm support
2004-10-23 12:36 ` Bartlomiej Zolnierkiewicz
@ 2004-10-23 21:59 ` Tobias Lorenz
0 siblings, 0 replies; 9+ messages in thread
From: Tobias Lorenz @ 2004-10-23 21:59 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, jgarzik
Hi,
Bartlomiej Zolnierkiewicz wrote:
> dev->id
>
> HDIO_GET_IDENTITY (as used by 'hdparm -i') should return _original_
> drive ID (dev->id should be returned) so currently used mode may not
> be reported correctly.
The used mode isn't reported yet. Only cylinders, sectors, heads, model
and firmware version are supported by now.
dev is of type ata_device, so dev->id is
u16 id[ATA_ID_WORDS]; /* IDENTIFY xxx DEVICE data */
Where in dev->id are the model and revision strings encoded ? Here is a comparison:
ata1: dev 0 cfg 49:2f00 82:74eb 83:4bea 84:4000 85:7469 86:0802 87:4003 88:203f
ata_dump_id: 49==0x2f00 53==0x0007 63==0x0007 64==0x0003 75==0x001f
ata_dump_id: 80==0x003c 81==0x0015 82==0x74eb 83==0x4bea 84==0x4000
ata_dump_id: 88==0x203f 93==0x600b
ata_scsiop_inq_std: ENTER
Vendor: ATA Model: IC35L060AVER07-0 Rev: 1.01
Type: Direct-Access ANSI SCSI revision: 05
> 'hdparm -I' should be already supported by current libata-dev tree.
hdparm -I needs implementations for the HDIO_GET_MULTCOUNT and
HDIO_GET_IDENTITY commands in ata_scsi_ioctl to report informations.
Both were not implemented.
Toby
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.9-libata1-dev1] libata-scsi: hdparm support
2004-10-22 23:20 [PATCH 2.6.9-libata1-dev1] libata-scsi: hdparm support Tobias Lorenz
2004-10-23 12:36 ` Bartlomiej Zolnierkiewicz
@ 2004-10-30 12:43 ` Jeff Garzik
2004-10-30 14:58 ` Tobias Lorenz
1 sibling, 1 reply; 9+ messages in thread
From: Jeff Garzik @ 2004-10-30 12:43 UTC (permalink / raw)
To: Tobias Lorenz; +Cc: linux-ide
Tobias Lorenz wrote:
> Hi,
>
> this patch adds support for transfering drive informations via the
> hd_driveid structure to the hdparm utility. At the moment, only
> cylinders, sectors, heads, model and firmware version are transfered.
>
> Our intention was to display the supported and the used UDMA mode(s),
> but we didn't find the correct structures yet to get these infos...
>
> Signed-off-by: Tobias Lorenz <tobias.lorenz@gmx.net>
>
>>--- a/drivers/scsi/libata-scsi.c 2004-10-20 22:07:50.000000000 +0200
>>+++ b/drivers/scsi/libata-scsi.c 2004-10-20 19:25:15.000000000 +0200
>>@@ -215,6 +215,17 @@
>> struct ata_port *ap;
>> struct ata_device *dev;
>> int val = -EINVAL, rc = -EINVAL;
>>+ struct hd_driveid drv_id = {
>>+ .cyls = 0,
>>+ .sectors = 0,
>>+ .heads = 0,
>>+ .fw_rev = "",
>>+ .model = "",
>>+ .cur_cyls = 0,
>>+ .cur_heads = 0,
>>+ .cur_sectors = 0,
>>+ };
>>+ int geom[3];
>>
>> ap = (struct ata_port *) &scsidev->host->hostdata[0];
>> if (!ap)
>>@@ -249,7 +260,25 @@
>> return -EACCES;
>> return ata_task_ioctl(scsidev, arg);
>>
>>+ case HDIO_GET_IDENTITY:
>>+ ata_std_bios_param(scsidev, NULL, dev->n_sectors, geom);
>>+ drv_id.cur_heads = drv_id.heads = geom[0];
>>+ drv_id.cur_sectors = drv_id.sectors = geom[1];
>>+ drv_id.cur_cyls = drv_id.cyls = geom[2];
>>+ strncpy((char *) &drv_id.model, scsidev->model, sizeof(drv_id.model));
>>+ strncpy((char *) &drv_id.fw_rev, scsidev->rev, sizeof(drv_id.fw_rev));
>>+ if(copy_to_user((char *) arg, (char *) &drv_id,
>>+ sizeof(drv_id)))
>>+ return(-EFAULT);
>>+ return 0;
>>+
>> default:
>> rc = -EOPNOTSUPP;
>> break;
This patch doesn't apply, since it is prefixed with "> " ...
Jeff
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 2.6.9-libata1-dev1] libata-scsi: hdparm support
2004-10-30 12:43 ` Jeff Garzik
@ 2004-10-30 14:58 ` Tobias Lorenz
2004-10-30 15:27 ` Jeff Garzik
0 siblings, 1 reply; 9+ messages in thread
From: Tobias Lorenz @ 2004-10-30 14:58 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linux-ide
Hi,
> This patch doesn't apply, since it is prefixed with "> " ...
You are right... here is a corrected version.
This patch adds support for transfering drive informations via the
hd_driveid structure to the hdparm utility. At the moment, only
cylinders, sectors, heads, model and firmware version are transfered.
Signed-off-by: Tobias Lorenz <tobias.lorenz@gmx.net>
--- a/drivers/scsi/libata-scsi.c 2004-10-30 16:31:01.000000000 +0200
+++ b/drivers/scsi/libata-scsi.c 2004-10-30 16:35:58.000000000 +0200
@@ -215,6 +215,17 @@
struct ata_port *ap;
struct ata_device *dev;
int val = -EINVAL, rc = -EINVAL;
+ struct hd_driveid drv_id = {
+ .cyls = 0,
+ .sectors = 0,
+ .heads = 0,
+ .fw_rev = "",
+ .model = "",
+ .cur_cyls = 0,
+ .cur_heads = 0,
+ .cur_sectors = 0,
+ };
+ int geom[3];
ap = (struct ata_port *) &scsidev->host->hostdata[0];
if (!ap)
@@ -249,6 +260,17 @@
return -EACCES;
return ata_task_ioctl(scsidev, arg);
+ case HDIO_GET_IDENTITY:
+ ata_std_bios_param(scsidev, NULL, dev->n_sectors, geom);
+ drv_id.cur_heads = drv_id.heads = geom[0];
+ drv_id.cur_sectors = drv_id.sectors = geom[1];
+ drv_id.cur_cyls = drv_id.cyls = geom[2];
+ strncpy((char *) &drv_id.model, scsidev->model, sizeof(drv_id.model));
+ strncpy((char *) &drv_id.fw_rev, scsidev->rev, sizeof(drv_id.fw_rev));
+ if(copy_to_user((char *) arg, (char *) &drv_id, sizeof(drv_id)))
+ return(-EFAULT);
+ return 0;
+
default:
rc = -EOPNOTSUPP;
break;
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 2.6.9-libata1-dev1] libata-scsi: hdparm support
2004-10-30 14:58 ` Tobias Lorenz
@ 2004-10-30 15:27 ` Jeff Garzik
2004-11-02 21:51 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 9+ messages in thread
From: Jeff Garzik @ 2004-10-30 15:27 UTC (permalink / raw)
To: Tobias Lorenz; +Cc: linux-ide, Bartlomiej Zolnierkiewicz
Tobias Lorenz wrote:
> Hi,
>
>
>>This patch doesn't apply, since it is prefixed with "> " ...
>
> You are right... here is a corrected version.
>
> This patch adds support for transfering drive informations via the
> hd_driveid structure to the hdparm utility. At the moment, only
> cylinders, sectors, heads, model and firmware version are transfered.
Applied to libata-dev... I'll need to ponder the correctness a bit
more, before pushing upstream.
Jeff
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.9-libata1-dev1] libata-scsi: hdparm support
2004-10-30 15:27 ` Jeff Garzik
@ 2004-11-02 21:51 ` Bartlomiej Zolnierkiewicz
2004-11-04 16:55 ` Tobias Lorenz
0 siblings, 1 reply; 9+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2004-11-02 21:51 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Tobias Lorenz, linux-ide
On Sat, 30 Oct 2004 11:27:02 -0400, Jeff Garzik <jgarzik@pobox.com> wrote:
> Tobias Lorenz wrote:
> > Hi,
> >
> >
> >>This patch doesn't apply, since it is prefixed with "> " ...
> >
> > You are right... here is a corrected version.
> >
> > This patch adds support for transfering drive informations via the
> > hd_driveid structure to the hdparm utility. At the moment, only
> > cylinders, sectors, heads, model and firmware version are transfered.
>
> Applied to libata-dev... I'll need to ponder the correctness a bit
> more, before pushing upstream.
Speaking about correctness...
HDIO_GET_IDENTIFY returns ID block in CPU order (not LE).
If you decide to add it sooner or later you will need something ala
ide_fix_driveid() from ide-iops.c (bloat).
Bartlomiej
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.9-libata1-dev1] libata-scsi: hdparm support
2004-11-02 21:51 ` Bartlomiej Zolnierkiewicz
@ 2004-11-04 16:55 ` Tobias Lorenz
2004-11-04 19:50 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 9+ messages in thread
From: Tobias Lorenz @ 2004-11-04 16:55 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, Jeff Garzik
Hi,
Bartlomiej Zolnierkiewicz wrote:
> Speaking about correctness...
>
> HDIO_GET_IDENTIFY returns ID block in CPU order (not LE).
>
> If you decide to add it sooner or later you will need something ala
> ide_fix_driveid() from ide-iops.c (bloat).
Okay, I am wondering, who defined, that hd_driveid has to be little
endian and not cpu order. Does that really has to be platform
independent ? But anyway...
There are two possible solutions:
The quick way is attached and just adds the missing conversions by
copy&paste from ide-iops:ide_fix_driveid.
The better and less redundant way would be to use the original function:
- by setting a Kconfig dependency on CONFIG_IDE.
- or by moving ide_fix_driveid to a seperate file (header file ?).
What do you mean ?
Toby
Signed-off-by: Tobias Lorenz <tobias.lorenz@gmx.net>
--- a/drivers/scsi/libata-scsi.c 2004-10-30 16:31:01.000000000 +0200
+++ b/drivers/scsi/libata-scsi.c 2004-11-04 17:39:58.000000000 +0100
@@ -31,6 +31,7 @@
#include <linux/libata.h>
#include <linux/hdreg.h>
#include <asm/uaccess.h>
+#include <asm/byteorder.h>
#include "libata.h"
@@ -215,6 +216,17 @@
struct ata_port *ap;
struct ata_device *dev;
int val = -EINVAL, rc = -EINVAL;
+ struct hd_driveid drv_id = {
+ .cyls = 0,
+ .sectors = 0,
+ .heads = 0,
+ .fw_rev = "",
+ .model = "",
+ .cur_cyls = 0,
+ .cur_heads = 0,
+ .cur_sectors = 0,
+ };
+ int geom[3];
ap = (struct ata_port *) &scsidev->host->hostdata[0];
if (!ap)
@@ -249,6 +261,40 @@
return -EACCES;
return ata_task_ioctl(scsidev, arg);
+ case HDIO_GET_IDENTITY:
+ ata_std_bios_param(scsidev, NULL, dev->n_sectors, geom);
+ drv_id.cur_heads = drv_id.heads = geom[0];
+ drv_id.cur_sectors = drv_id.sectors = geom[1];
+ drv_id.cur_cyls = drv_id.cyls = geom[2];
+ strncpy((char *) &drv_id.model, scsidev->model, sizeof(drv_id.model));
+ strncpy((char *) &drv_id.fw_rev, scsidev->rev, sizeof(drv_id.fw_rev));
+#ifndef __LITTLE_ENDIAN
+# ifdef __BIG_ENDIAN
+ {
+ int i;
+ u16 *stringcast;
+
+ drv_id.cyls = __le16_to_cpu(drv_id.cyls);
+ drv_id.heads = __le16_to_cpu(drv_id.heads);
+ drv_id.sectors = __le16_to_cpu(drv_id.sectors);
+ stringcast = (u16 *)&drv_id.fw_rev[0];
+ for (i = 0; i < (8/2); i++)
+ stringcast[i] = __le16_to_cpu(stringcast[i]);
+ stringcast = (u16 *)&drv_id.model[0];
+ for (i = 0; i < (40/2); i++)
+ stringcast[i] = __le16_to_cpu(stringcast[i]);
+ drv_id.cur_cyls = __le16_to_cpu(drv_id.cur_cyls);
+ drv_id.cur_heads = __le16_to_cpu(drv_id.cur_heads);
+ drv_id.cur_sectors = __le16_to_cpu(drv_id.cur_sectors);
+ }
+# else
+# error "Please fix <asm/byteorder.h>"
+# endif
+#endif
+ if(copy_to_user((char *) arg, (char *) &drv_id, sizeof(drv_id)))
+ return(-EFAULT);
+ return 0;
+
default:
rc = -EOPNOTSUPP;
break;
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 2.6.9-libata1-dev1] libata-scsi: hdparm support
2004-11-04 16:55 ` Tobias Lorenz
@ 2004-11-04 19:50 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 9+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2004-11-04 19:50 UTC (permalink / raw)
To: Tobias Lorenz; +Cc: linux-ide, Jeff Garzik
Hi!
On Thu, 04 Nov 2004 17:55:34 +0100, Tobias Lorenz <tobias.lorenz@gmx.net> wrote:
> Hi,
>
> Bartlomiej Zolnierkiewicz wrote:
> > Speaking about correctness...
> >
> > HDIO_GET_IDENTIFY returns ID block in CPU order (not LE).
> >
> > If you decide to add it sooner or later you will need something ala
> > ide_fix_driveid() from ide-iops.c (bloat).
>
> Okay, I am wondering, who defined, that hd_driveid has to be little
> endian and not cpu order. Does that really has to be platform
ATA spec ;-)
HDIO_DRIVE_CMD: LE order
HDIO_GET_IDENTIFY : CPU order
> independent ? But anyway...
HDIO_GET_IDENTIFY - yes
> There are two possible solutions:
>
> The quick way is attached and just adds the missing conversions by
> copy&paste from ide-iops:ide_fix_driveid.
This is not needed (yet)...
ata_std_bios_params() returns things in CPU order arleady,
model and fw_rev are also in CPU order (libata driver handles them).
> The better and less redundant way would be to use the original function:
> - by setting a Kconfig dependency on CONFIG_IDE.
> - or by moving ide_fix_driveid to a seperate file (header file ?).
There was recent discussion about this wrt to isd200.c driver.
IMHO the correct solution is to not need ide_fix_driveid() et all.
> What do you mean ?
My point is that once you decide to support HDIO_GET_IDENTIFY
in libata you will have to take care of this issue (in the future)...
The result will be the same crap as ide_fix_driveid() etc.
I think that is much easier to teach blktool about SG_IO ioctl
and do the stuff right than to try to be "smart" and support all
badly designed IDE driver ioctls used by hdparm...
Bartlomiej
> Toby
>
>
> Signed-off-by: Tobias Lorenz <tobias.lorenz@gmx.net>
>
> --- a/drivers/scsi/libata-scsi.c 2004-10-30 16:31:01.000000000 +0200
> +++ b/drivers/scsi/libata-scsi.c 2004-11-04 17:39:58.000000000 +0100
> @@ -31,6 +31,7 @@
> #include <linux/libata.h>
> #include <linux/hdreg.h>
> #include <asm/uaccess.h>
> +#include <asm/byteorder.h>
>
> #include "libata.h"
>
> @@ -215,6 +216,17 @@
>
>
> struct ata_port *ap;
> struct ata_device *dev;
> int val = -EINVAL, rc = -EINVAL;
> + struct hd_driveid drv_id = {
> + .cyls = 0,
> + .sectors = 0,
> + .heads = 0,
> + .fw_rev = "",
> + .model = "",
> + .cur_cyls = 0,
> + .cur_heads = 0,
> + .cur_sectors = 0,
> + };
> + int geom[3];
>
> ap = (struct ata_port *) &scsidev->host->hostdata[0];
> if (!ap)
> @@ -249,6 +261,40 @@
>
>
> return -EACCES;
> return ata_task_ioctl(scsidev, arg);
>
> + case HDIO_GET_IDENTITY:
> + ata_std_bios_param(scsidev, NULL, dev->n_sectors, geom);
> + drv_id.cur_heads = drv_id.heads = geom[0];
> + drv_id.cur_sectors = drv_id.sectors = geom[1];
> + drv_id.cur_cyls = drv_id.cyls = geom[2];
> + strncpy((char *) &drv_id.model, scsidev->model, sizeof(drv_id.model));
> + strncpy((char *) &drv_id.fw_rev, scsidev->rev, sizeof(drv_id.fw_rev));
> +#ifndef __LITTLE_ENDIAN
> +# ifdef __BIG_ENDIAN
> + {
> + int i;
> + u16 *stringcast;
> +
> + drv_id.cyls = __le16_to_cpu(drv_id.cyls);
> + drv_id.heads = __le16_to_cpu(drv_id.heads);
> + drv_id.sectors = __le16_to_cpu(drv_id.sectors);
> + stringcast = (u16 *)&drv_id.fw_rev[0];
> + for (i = 0; i < (8/2); i++)
> + stringcast[i] = __le16_to_cpu(stringcast[i]);
> + stringcast = (u16 *)&drv_id.model[0];
> + for (i = 0; i < (40/2); i++)
> + stringcast[i] = __le16_to_cpu(stringcast[i]);
> + drv_id.cur_cyls = __le16_to_cpu(drv_id.cur_cyls);
> + drv_id.cur_heads = __le16_to_cpu(drv_id.cur_heads);
> + drv_id.cur_sectors = __le16_to_cpu(drv_id.cur_sectors);
> + }
> +# else
> +# error "Please fix <asm/byteorder.h>"
> +# endif
> +#endif
>
>
> + if(copy_to_user((char *) arg, (char *) &drv_id, sizeof(drv_id)))
> + return(-EFAULT);
> + return 0;
> +
> default:
> rc = -EOPNOTSUPP;
> break;
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2004-11-04 19:50 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-10-22 23:20 [PATCH 2.6.9-libata1-dev1] libata-scsi: hdparm support Tobias Lorenz
2004-10-23 12:36 ` Bartlomiej Zolnierkiewicz
2004-10-23 21:59 ` Tobias Lorenz
2004-10-30 12:43 ` Jeff Garzik
2004-10-30 14:58 ` Tobias Lorenz
2004-10-30 15:27 ` Jeff Garzik
2004-11-02 21:51 ` Bartlomiej Zolnierkiewicz
2004-11-04 16:55 ` Tobias Lorenz
2004-11-04 19:50 ` Bartlomiej Zolnierkiewicz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).