Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#12295 closed enhancement (fixed)

Enhancement to the ahci driver

Reported by: Anarchos Owned by: axeld
Priority: low Milestone: Unscheduled
Component: Drivers/Disk/ATA Version: R1/Development
Keywords: ahci sata Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

I added some documentation, and made the code a bit clearer. Anyway i am not sure of the following points :

  • endianness issues and order of the fields in the scontrol struct.
  • "0x%04" B_PRIx16 format string.
  • "0x%02" B_PRIx8 format string.
  • coding style

Attachments (3)

ahci.patch (5.5 KB ) - added by Anarchos 4 years ago.
ahci_irq_storm.patch (6.1 KB ) - added by Anarchos 4 years ago.
Solving boot failures due to "Port Connect Change" IRQ storm.
0001-AHCI-Fix-boot-failures-due-to-Port-Connect-Change-IR.patch (5.1 KB ) - added by waddlesplash 4 years ago.
Cleaned-up Git-patch version of Anarchos' changes.

Download all attachments as: .zip

Change History (12)

by Anarchos, 4 years ago

Attachment: ahci.patch added

comment:1 by Anarchos, 4 years ago

Has a Patch: set

by Anarchos, 4 years ago

Attachment: ahci_irq_storm.patch added

Solving boot failures due to "Port Connect Change" IRQ storm.

comment:2 by Anarchos, 4 years ago

The ahci_irq_storm.patch is the same as the ahci.patch I added code and specification documentation for the dealing of the "Port Connect Change" IRQ storm. Same uncertainties remain as before (endianness, format string, coding/doc style)

comment:3 by Anarchos, 4 years ago

I think it could solve #9275, and other tickets related to boot failures with AMD IXP700 sata controller.

comment:4 by Anarchos, 4 years ago

By waddlesplah advice, i must add that this patch solved my boot problem.

comment:5 by waddlesplash, 4 years ago

From IRC:

1:12 PM <axeld> One thing that should be resolved is the TODO comments
1:13 PM <axeld> Another thing is that it should be enough to refer to the spec instead of quoting it
1:13 PM <axeld> Excessive comments usually aren't helpful in reading source code
1:14 PM <axeld> Anarchos: Other than that, I would have to actually read the specs to be able to comment more :-)

by waddlesplash, 4 years ago

Cleaned-up Git-patch version of Anarchos' changes.

comment:6 by pulkomandy, 4 years ago

The order of the fields in a bitfield is implementation defined, I don't think it is a good idea to map them to the hardware this way. This should use some endian-aware solution - we probably have macros borrowed from FreeBSD to manage this somewhere.

I can't comment on the functional changes, since I don't know the ATA spec much.

comment:7 by axeld, 4 years ago

Owner: changed from nobody to axeld
Status: newin-progress

comment:8 by axeld, 4 years ago

Resolution: fixed
Status: in-progressclosed

Applied in hrev49590. Unfortunately, the scontrol structure changed the access to a 32 bit field to 8 bits which broke some HW, and I didn't notice that.

Apart from that, I cleaned it up a little, and added a check for the ST flag in the command register as the specification suggests.

comment:9 by korli, 4 years ago

@Anarchos may I suggest you post an update to this article you wrote?

https://www.haiku-os.org/node/6004 ?

Note: See TracTickets for help on using tickets.