diff mbox series

[v2,1/7] piix_ide_reset: Use pci_set_* functions instead of direct access

Message ID 20220707031140.158958-1-lkujaw@member.fsf.org (mailing list archive)
State New, archived
Headers show
Series [v2,1/7] piix_ide_reset: Use pci_set_* functions instead of direct access | expand

Commit Message

Lev Kujawski July 7, 2022, 3:11 a.m. UTC 
Eliminate the remaining TODOs in hw/ide/piix.c by:
* Using pci_set_{size} functions to write the PIIX PCI configuration
  space instead of manipulating it directly as an array; and
* Documenting the default register values by reference to the
  controlling specification.

Signed-off-by: Lev Kujawski <lkujaw@member.fsf.org>
---
 hw/ide/piix.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

Comments

Kevin Wolf Sept. 30, 2022, 3:17 p.m. UTC | #1 
Am 07.07.2022 um 05:11 hat Lev Kujawski geschrieben:
> Eliminate the remaining TODOs in hw/ide/piix.c by:
> * Using pci_set_{size} functions to write the PIIX PCI configuration
>   space instead of manipulating it directly as an array; and
> * Documenting the default register values by reference to the
>   controlling specification.
> 
> Signed-off-by: Lev Kujawski <lkujaw@member.fsf.org>

Thanks, dropped patches 5 and 6 because I see that you have posted a
newer version of them, and applied the rest to the block branch.

Kevin
diff mbox series

Patch

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 9a9b28078e..de1f4f0efb 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -21,6 +21,10 @@ 
  * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
+ *
+ * References:
+ *  [1] 82371FB (PIIX) AND 82371SB (PIIX3) PCI ISA IDE XCELERATOR,
+ *      290550-002, Intel Corporation, April 1997.
  */
 
 #include "qemu/osdep.h"
@@ -114,14 +118,11 @@  static void piix_ide_reset(DeviceState *dev)
         ide_bus_reset(&d->bus[i]);
     }
 
-    /* TODO: this is the default. do not override. */
-    pci_conf[PCI_COMMAND] = 0x00;
-    /* TODO: this is the default. do not override. */
-    pci_conf[PCI_COMMAND + 1] = 0x00;
-    /* TODO: use pci_set_word */
-    pci_conf[PCI_STATUS] = PCI_STATUS_FAST_BACK;
-    pci_conf[PCI_STATUS + 1] = PCI_STATUS_DEVSEL_MEDIUM >> 8;
-    pci_conf[0x20] = 0x01; /* BMIBA: 20-23h */
+    /* PCI command register default value (0000h) per [1, p.48].  */
+    pci_set_word(pci_conf + PCI_COMMAND, 0x0000);
+    pci_set_word(pci_conf + PCI_STATUS,
+                 PCI_STATUS_DEVSEL_MEDIUM | PCI_STATUS_FAST_BACK);
+    pci_set_byte(pci_conf + 0x20, 0x01);  /* BMIBA: 20-23h */
 }
 
 static int pci_piix_init_ports(PCIIDEState *d)