Re: pata_it821x completely broken
From: Ondrej Zary
Date: Tue Jul 22 2008 - 14:00:21 EST
On Sunday 13 July 2008 13:47:12 Ondrej Zary wrote:
> On Saturday 12 July 2008 23:42:10 Ondrej Zary wrote:
> > On Friday 11 July 2008 22:14:09 Alan Cox wrote:
> > > > > commenting out the error check after ata_dev_init_params() call in
> > > > > ata_dev_read_id() function (libata-core.c), I got at least the
> > > > > device name. The capacity is 0 so it doesn't work, obviously:
> > >
> > > If you don't read the ID then it wouldn't.
> > >
> > > > I captured the IDENTIFY data from the virtual device. I'm not ATA
> > > > guru but looking at the data, there are zeros at many places where
> > > > something should be. That number starting at 0x78 looks like size of
> > > > the array in sectors (0x4C726C or 0x4C6C72 - the array is built from
> > > > 2.5GB and 6.4GB drives).
> > >
> > > The Ident data for the virtual device is fairly sparse but the specs
> > > don't require a lot of the field are filled in and only the LBA really
> > > matters.
> >
> > The problem is that ata_id_n_sectors() function:
> >
> > static u64 ata_id_n_sectors(const u16 *id)
> > {
> > if (ata_id_has_lba(id)) {
> > if (ata_id_has_lba48(id))
> > return ata_id_u64(id, 100);
> > else
> > return ata_id_u32(id, 60);
> > } else {
> > if (ata_id_current_chs_valid(id))
> > return ata_id_u32(id, 57);
> > else
> > return id[1] * id[3] * id[6];
> > }
> > }
> >
> > fails to retrieve the LBA48 value.
> >
> >
> > This is because the ata_id_has_lba() test
> >
> > #define ata_id_has_lba(id) ((id)[49] & (1 << 9))
> >
> > fails as the identify data contains only zeros at word 49 (byte 0x62).
> >
> > Another problem is that ata_id_has_lba48() would fail too - that will
> > break array over 2TB (if the controller BIOS and firmware can do it).
> >
> > Looks like this needs to force LBA48 with these virtual drives.
>
> The patch below fixes the IDENTIFY problem for me and makes the RAID array
> accessible. Is it OK or is there a better way to do it?
>
New patch below, this time with DMA forced on RAID volumes - as my firmware
does not identify the RAID arrays as DMA capable :(
diff -ur linux-2.6.26/drivers/ata/libata-core.c linux-2.6.26-pentium/drivers/ata/libata-core.c
--- linux-2.6.26/drivers/ata/libata-core.c 2008-07-13 23:51:29.000000000 +0200
+++ linux-2.6.26-pentium/drivers/ata/libata-core.c 2008-07-16 20:50:33.000000000 +0200
@@ -2030,7 +2030,8 @@
* Note that ATA4 says lba is mandatory so the second check
* shoud never trigger.
*/
- if (ata_id_major_version(id) < 4 || !ata_id_has_lba(id)) {
+ if ((ata_id_major_version(id) < 4 || !ata_id_has_lba(id)) &&
+ id[3] != 0 && id[6] != 0) {
err_mask = ata_dev_init_params(dev, id[3], id[6]);
if (err_mask) {
rc = -EIO;
@@ -2195,18 +2196,23 @@
"not be fully accessable.\n");
}
- dev->n_sectors = ata_id_n_sectors(id);
+ if (dev->horkage & ATA_HORKAGE_LBA48_FORCE)
+ dev->n_sectors = ata_id_u64(id, 100);
+ else
+ dev->n_sectors = ata_id_n_sectors(id);
if (dev->id[59] & 0x100)
dev->multi_count = dev->id[59] & 0xff;
- if (ata_id_has_lba(id)) {
+ if (ata_id_has_lba(id) ||
+ dev->horkage & ATA_HORKAGE_LBA48_FORCE) {
const char *lba_desc;
char ncq_desc[20];
lba_desc = "LBA";
dev->flags |= ATA_DFLAG_LBA;
- if (ata_id_has_lba48(id)) {
+ if (ata_id_has_lba48(id) ||
+ dev->horkage & ATA_HORKAGE_LBA48_FORCE) {
dev->flags |= ATA_DFLAG_LBA48;
lba_desc = "LBA48";
@@ -3946,6 +3952,9 @@
{ "TSSTcorp CDDVDW SH-S202N", "SB00", ATA_HORKAGE_IVB, },
{ "TSSTcorp CDDVDW SH-S202N", "SB01", ATA_HORKAGE_IVB, },
+ /* Has LBA48 but advertises neither LBA nor LBA48 */
+ { "Integrated Technology Express Inc", NULL, ATA_HORKAGE_LBA48_FORCE, },
+
/* End Marker */
{ }
};
diff -ur linux-2.6.26/drivers/ata/pata_it821x.c linux-2.6.26-pentium/drivers/ata/pata_it821x.c
--- linux-2.6.26/drivers/ata/pata_it821x.c 2008-07-13 23:51:29.000000000 +0200
+++ linux-2.6.26-pentium/drivers/ata/pata_it821x.c 2008-07-22 19:56:14.000000000 +0200
@@ -462,15 +462,19 @@
static int it821x_smart_set_mode(struct ata_link *link, struct ata_device **unused)
{
struct ata_device *dev;
+ unsigned char model_num[ATA_ID_PROD_LEN + 1];
ata_link_for_each_dev(dev, link) {
if (ata_dev_enabled(dev)) {
/* We don't really care */
dev->pio_mode = XFER_PIO_0;
dev->dma_mode = XFER_MW_DMA_0;
+ ata_id_c_string(dev->id, model_num, ATA_ID_PROD,
+ sizeof(model_num));
/* We do need the right mode information for DMA or PIO
and this comes from the current configuration flags */
- if (ata_id_has_dma(dev->id)) {
+ if (ata_id_has_dma(dev->id) ||
+ strstr(model_num, "Integrated Technology Express")) {
ata_dev_printk(dev, KERN_INFO, "configured for DMA\n");
dev->xfer_mode = XFER_MW_DMA_0;
dev->xfer_shift = ATA_SHIFT_MWDMA;
diff -ur linux-2.6.26/include/linux/libata.h linux-2.6.26-pentium/include/linux/libata.h
--- linux-2.6.26/include/linux/libata.h 2008-07-13 23:51:29.000000000 +0200
+++ linux-2.6.26-pentium/include/linux/libata.h 2008-07-16 20:50:33.000000000 +0200
@@ -353,6 +353,7 @@
ATA_HORKAGE_IPM = (1 << 7), /* Link PM problems */
ATA_HORKAGE_IVB = (1 << 8), /* cbl det validity bit bugs */
ATA_HORKAGE_STUCK_ERR = (1 << 9), /* stuck ERR on next PACKET */
+ ATA_HORKAGE_LBA48_FORCE = (1 << 10), /* Has hidden LBA48 */
/* DMA mask for user DMA control: User visible values; DO NOT
renumber */
--
Ondrej Zary
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/