Hello,
On 05/12/2010 11:19 PM, Larry Baker wrote:
> 1. I needed to see more of the IDENTIFY DEVICE response, so I modified
> the probe printout to include words 47, 49, 59, and 80-88, like this:
>
> - "%s: cfg 49:%04x 82:%04x 83:%04x 84:%04x "
> - "85:%04x 86:%04x 87:%04x 88:%04x\n",
> + "%s: cfg 47:%04x 49:%04x 59:%04x "
> + "80-88:%04x %04x %04x %04x %04x %04x %04x %04x
> %04x\n",
> __func__,
> - id[49], id[82], id[83], id[84],
> - id[85], id[86], id[87], id[88]);
> + id[47], id[49], id[59], id[80], id[81], id[82],
> + id[83], id[84], id[85], id[86], id[87], id[88]);
I think it would be better to include a module parameter which makes
libata dump the whole thing. The message is only useful for
development and debugging anyway. I'll add such an option.
> 2. The device I was having trouble with claims to support multi-sector
> transfers, with a maximum Sector Count of 1. There's not much point
> then, is there? I suggest adding a test for max >= 2 in the conditional
> where dev->multi_count is assigned:
>
> /* get current R/W Multiple count setting */
> if ((dev->id[47] >> 8) == 0x80 && (dev->id[59] & 0x100)) {
> unsigned int max = dev->id[47] & 0xff;
> unsigned int cnt = dev->id[59] & 0xff;
> + /* no point unless max >= 2 */
> + if (max >= 2)
> /* only recognize/allow powers of two here */
> if (is_power_of_2(max) && is_power_of_2(cnt))
> if (cnt <= max)
> dev->multi_count = cnt;
> }
Yeap, this does make sense. Sergei, Alan, what do you guys think?
> 3. In the error recovery code (in libata-eh.c?), I suggest a way to
> recover for devices like the one I've got that improperly implement
> READ/WRITE MULTI commands:
>
> If the failed command is one of the READ/WRITE MULTI commands (entries
> 0-4 in ata_rw_cmds[]), and the device response is ABORT, use your
> HORKAGE mechanism to set a flag like ATA_HORKAGE_BROKEN_MULTI to skip
> setting dev->multi_count in libata-core.c (the code in 2., above), i.e.,
> disable multi-sector transfers, then reconfigure the device to disable
> multi-sector transfers (call ata_dev_configure(), I guess, so the new
> configuration gets printed out in dmesg) and retry the transfer. Any
> future calls to ata_dev_configure() for that device will not attempt to
> enable multi-sector transfers because the ATA_HORKAGE_BROKEN_MULTI is set:
>
> + /* don't try to enable R/W multi if it is broken */
> + if (!(dev->horkage & ATA_HORKAGE_BROKEN_MULTI))
So, turn off MULTI on ABORT.... Oh yeah, I can add that to the
existing speed down logic. Would you be available to test patches?
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
