Skip to contentSkip to conversation

ide: small ide_register_port() cleanup

PATCH In review Bartlomiej Zolnierkiewicz 3 participants 4 messages Nov 12, 2008 – Jan 19, 2009 libata

lore
Share & tools
lore
#0 lore raw

PATCH 3/6 ide: small ide_register_port() cleanup

---
 drivers/ide/ide-probe.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/ide/ide-probe.c +2 -7
Index: b/drivers/ide/ide-probe.c
===================================================================
@@ -640,14 +640,9 @@ static int ide_register_port(ide_hwif_t 
 	/* register with global device tree */
 	dev_set_name(&hwif->gendev, hwif->name);
 	hwif->gendev.driver_data = hwif;
-	if (hwif->gendev.parent == NULL) {
-		if (hwif->dev)
-			hwif->gendev.parent = hwif->dev;
-		else
-			/* Would like to do = &device_legacy */
-			hwif->gendev.parent = NULL;
-	}
+	hwif->gendev.parent = hwif->dev;
 	hwif->gendev.release = hwif_release_dev;
+
 	ret = device_register(&hwif->gendev);
 	if (ret < 0) {
 		printk(KERN_WARNING "IDE: %s: device_register error: %d\n",
#1 lore raw
Quoted 11 lines · Bartlomiej Zolnierkiewicz
---
 drivers/ide/ide-probe.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

Index: b/drivers/ide/ide-probe.c
===================================================================

drivers/ide/ide-probe.c +0 -0
@@ -640,14 +640,9 @@ static int ide_register_port(ide_hwif_t 
 	/* register with global device tree */

drivers/ide/ide-probe.c static int ide_register_port(ide_hwif_t · line 643

Quoted 10 lines
drivers/ide/ide-probe.c +1 -7
 	dev_set_name(&hwif->gendev, hwif->name);
 	hwif->gendev.driver_data = hwif;
-	if (hwif->gendev.parent == NULL) {
-		if (hwif->dev)
-			hwif->gendev.parent = hwif->dev;
-		else
-			/* Would like to do = &device_legacy */
-			hwif->gendev.parent = NULL;
-	}
+	hwif->gendev.parent = hwif->dev;

This breaks ide-pmac. It overwrites the parent that pmac_ide_macio_attach has set.

drivers/ide/ide-probe.c +2 -1
index 312127e..0db1ed9 100644
@@ -649,7 +649,8 @@ static int ide_register_port(ide_hwif_t *hwif)
 	/* register with global device tree */
 	dev_set_name(&hwif->gendev, hwif->name);
 	hwif->gendev.driver_data = hwif;
-	hwif->gendev.parent = hwif->dev;
+	if (hwif->gendev.parent == NULL)
+		hwif->gendev.parent = hwif->dev;
 	hwif->gendev.release = hwif_release_dev;
 
 	ret = device_register(&hwif->gendev);
Signature · 4 lines
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
#2 lore raw
Quoted 34 lines · Andreas Schwab
Quoted 11 lines · Bartlomiej Zolnierkiewicz
---
 drivers/ide/ide-probe.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

Index: b/drivers/ide/ide-probe.c
===================================================================

drivers/ide/ide-probe.c +0 -0
@@ -640,14 +640,9 @@ static int ide_register_port(ide_hwif_t 
 	/* register with global device tree */

drivers/ide/ide-probe.c static int ide_register_port(ide_hwif_t · line 643

Quoted 10 lines
drivers/ide/ide-probe.c +1 -7
 	dev_set_name(&hwif->gendev, hwif->name);
 	hwif->gendev.driver_data = hwif;
-	if (hwif->gendev.parent == NULL) {
-		if (hwif->dev)
-			hwif->gendev.parent = hwif->dev;
-		else
-			/* Would like to do = &device_legacy */
-			hwif->gendev.parent = NULL;
-	}
+	hwif->gendev.parent = hwif->dev;

This breaks ide-pmac. It overwrites the parent that pmac_ide_macio_attach has set.

drivers/ide/ide-probe.c +0 -0
index 312127e..0db1ed9 100644

drivers/ide/ide-probe.c static int ide_register_port(ide_hwif_t *hwif) · line 656

Quoted 9 lines
drivers/ide/ide-probe.c +2 -1
@@ -649,7 +649,8 @@ static int ide_register_port(ide_hwif_t *hwif)
 	/* register with global device tree */
 	dev_set_name(&hwif->gendev, hwif->name);
 	hwif->gendev.driver_data = hwif;
-	hwif->gendev.parent = hwif->dev;
+	if (hwif->gendev.parent == NULL)
+		hwif->gendev.parent = hwif->dev;
 	hwif->gendev.release = hwif_release_dev;
 
 	ret = device_register(&hwif->gendev);

Applied, thanks for fixing it.

Since it is the second time this ->parent special case bited us in the *ss:

pmac_ide_macio_attach():
...

	hw.dev = &mdev->bus->pdev->dev;
	hw.parent = &mdev->ofdev.dev;

...

I again wonder what is the reason for having different struct device-s used for DBDMA and for device tree on MACIO PMAC controllers. Maybe there is no real reason after all and we can use &mdev->ofdev.dev also for hwif->dev?

Thanks,
Bart

#3 lore raw
Quoted 5 lines

Since it is the second time this ->parent special case bited us in the *ss:

pmac_ide_macio_attach():
...

Quoted 7 lines
	hw.dev = &mdev->bus->pdev->dev;
	hw.parent = &mdev->ofdev.dev;

...

I again wonder what is the reason for having different struct device-s used for DBDMA and for device tree on MACIO PMAC controllers. Maybe there is no real reason after all and we can use &mdev->ofdev.dev also for hwif->dev?

Historically, dma_map_* wasn't supported on the macio devs, had to use the underlying pci dev.

In fact, it might even still be the case. I need to have a look at it after LCA, though with our current DMA ops, it should be easy to fix.

Cheers,
Ben.