ata: acpi/zpodd: some small fixes

PATCH In review Aaron Lu 2 participants 10 messages Mar 14, 2014 libata

lore

Share & tools

lore

Aaron Lu

author

2014-03-14 05:45 UTC #0 lore raw

cover ata: acpi/zpodd: some small fixes

Two small fix for libata-acpi and libata-zpodd and one simplification for libata-zpodd.

Aaron Lu (3):
libata: zpodd: should depend on PM_RUNTIME
libata: acpi: avoid passing NULL to ACPI evaluation method
libata: zpodd: eliminate odd_can_poweroff

 drivers/ata/Kconfig        |  2 +-
 drivers/ata/libata-acpi.c  |  1 +
 drivers/ata/libata-zpodd.c | 21 ++-------------------
 3 files changed, 4 insertions(+), 20 deletions(-)

Signature · 1 line

1.8.5.3

Aaron Lu

author

2014-03-14 05:45 UTC #1 lore raw

PATCH 1/3 libata: zpodd: should depend on PM_RUNTIME

ZPODD is built on top of runtime PM functionality, it doesn't make sense to have it in a kernel that doesn't have CONFIG_PM_RUNTIME set.

Signed-off-by Aaron Lu <[email protected]>

Cc [email protected]

---
 drivers/ata/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/ata/Kconfig +1 -1

index 93fc2f052645..e4e608c72e01 100644
@@ -60,7 +60,7 @@ config ATA_ACPI
 
 config SATA_ZPODD
 	bool "SATA Zero Power Optical Disc Drive (ZPODD) support"
-	depends on ATA_ACPI
+	depends on ATA_ACPI && PM_RUNTIME
 	default n
 	help
 	  This option adds support for SATA Zero Power Optical Disc

Signature · 1 line

1.8.5.3

Aaron Lu

author

2014-03-14 05:45 UTC #2 lore raw

PATCH 2/3 libata: acpi: avoid passing NULL to ACPI evaluation method

In reply to Aaron Lu

If ACPI handle for an ATA device is NULL, we shouldn't call ata_dev_get_GTF as that function will use handle to do some ACPI evaluation.

Signed-off-by Aaron Lu <[email protected]>

Cc [email protected]

---
 drivers/ata/libata-acpi.c | 1 +
 1 file changed, 1 insertion(+)

drivers/ata/libata-acpi.c +1 -0

index 9e69a5308693..b4f7cc2522d9 100644
@@ -835,6 +835,7 @@ void ata_acpi_on_resume(struct ata_port *ap)
 		ata_for_each_dev(dev, &ap->link, ALL) {
 			ata_acpi_clear_gtf(dev);
 			if (ata_dev_enabled(dev) &&
+			    ata_dev_acpi_handle(dev) &&
 			    ata_dev_get_GTF(dev, NULL) >= 0)
 				dev->flags |= ATA_DFLAG_ACPI_PENDING;
 		}

Signature · 1 line

1.8.5.3

Tejun Heo

2014-03-14 12:58 UTC #3 lore raw

Re: [PATCH 2/3] libata: acpi: avoid passing NULL to ACPI evaluation method

Quoted 3 lines · Aaron Lu

If ACPI handle for an ATA device is NULL, we shouldn't call ata_dev_get_GTF as that function will use handle to do some ACPI evaluation.

This caused a crash, right? Can you please include what can trigger such oops and how it looks like and reference to the original bug report. In general, when you're cc'ing stable, please try to provide as much information about the bug as possible so that downstreams have easier time evaluating the importance and risk of the change. The same goes for the previous patch.

Thanks.

Signature · 1 line

tejun

Aaron Lu

author

2014-03-14 14:25 UTC #4 lore raw

Re: [PATCH 2/3] libata: acpi: avoid passing NULL to ACPI evaluation method

Quoted 11 lines · Tejun Heo

Quoted 3 lines · Aaron Lu
If ACPI handle for an ATA device is NULL, we shouldn't call ata_dev_get_GTF as that function will use handle to do some ACPI evaluation.
This caused a crash, right? Can you please include what can trigger such oops and how it looks like and reference to the original bug report. In general, when you're cc'ing stable, please try to provide as much information about the bug as possible so that downstreams have easier time evaluating the importance and risk of the change. The same goes for the previous patch.

It shouldn't cause a crash, just some error messages from ACPICA I think. There isn't a bug for it right now, I have only some vague memory that I saw such error messages when dealing with some other bugs some time ago. So perhaps I shouldn't add the stable tag, sorry for that, this seems to belong to the 'this patch solves a potential problem' case...

For the 1/3 patch, no crash either. It's just that if CONFIG_PM_RUNTIME is not set, ZPODD will not function so building it doesn't make sense.

Thanks,
Aaron

Aaron Lu

author

2014-03-14 14:32 UTC #5 lore raw

Re: [PATCH 2/3] libata: acpi: avoid passing NULL to ACPI evaluation method

Quoted 13 lines · Aaron Lu

Quoted 11 lines · Tejun Heo
Quoted 3 lines · Aaron Lu
If ACPI handle for an ATA device is NULL, we shouldn't call ata_dev_get_GTF as that function will use handle to do some ACPI evaluation.
This caused a crash, right? Can you please include what can trigger such oops and how it looks like and reference to the original bug report. In general, when you're cc'ing stable, please try to provide as much information about the bug as possible so that downstreams have easier time evaluating the importance and risk of the change. The same goes for the previous patch.

Quoted 2 lines

For the 1/3 patch, no crash either. It's just that if CONFIG_PM_RUNTIME is not set, ZPODD will not function so building it doesn't make sense.

No bug report for this either, I realized this when I'm preparing a test build for !CONFIG_PM_RUNTIME and found that I can still select ZPODD.

Tejun Heo

2014-03-14 14:33 UTC #6 lore raw

Re: [PATCH 2/3] libata: acpi: avoid passing NULL to ACPI evaluation method

Quoted 14 lines · Aaron Lu

Quoted 13 lines · Aaron Lu
Quoted 11 lines · Tejun Heo
Quoted 3 lines · Aaron Lu
If ACPI handle for an ATA device is NULL, we shouldn't call ata_dev_get_GTF as that function will use handle to do some ACPI evaluation.
This caused a crash, right? Can you please include what can trigger such oops and how it looks like and reference to the original bug report. In general, when you're cc'ing stable, please try to provide as much information about the bug as possible so that downstreams have easier time evaluating the importance and risk of the change. The same goes for the previous patch.

Quoted 5 lines

Quoted 2 lines
For the 1/3 patch, no crash either. It's just that if CONFIG_PM_RUNTIME is not set, ZPODD will not function so building it doesn't make sense.
No bug report for this either, I realized this when I'm preparing a test build for !CONFIG_PM_RUNTIME and found that I can still select ZPODD.

Both aren't really -stable material then, right?

Signature · 1 line

tejun

Aaron Lu

author

2014-03-14 15:20 UTC #7 lore raw

Re: [PATCH 2/3] libata: acpi: avoid passing NULL to ACPI evaluation method

Quoted 15 lines · Tejun Heo

Quoted 14 lines · Aaron Lu
Quoted 13 lines · Aaron Lu
Quoted 11 lines · Tejun Heo
Quoted 3 lines · Aaron Lu
If ACPI handle for an ATA device is NULL, we shouldn't call ata_dev_get_GTF as that function will use handle to do some ACPI evaluation.
This caused a crash, right? Can you please include what can trigger such oops and how it looks like and reference to the original bug report. In general, when you're cc'ing stable, please try to provide as much information about the bug as possible so that downstreams have easier time evaluating the importance and risk of the change. The same goes for the previous patch.

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
For the 1/3 patch, no crash either. It's just that if CONFIG_PM_RUNTIME is not set, ZPODD will not function so building it doesn't make sense.
No bug report for this either, I realized this when I'm preparing a test build for !CONFIG_PM_RUNTIME and found that I can still select ZPODD.
Both aren't really -stable material then, right?

Yes.

Aaron Lu

author

2014-03-14 05:45 UTC #8 lore raw

PATCH 3/3 libata: zpodd: eliminate odd_can_poweroff

In reply to Aaron Lu

Now that we can directly get the ACPI device conterpart of the physical ATA transport device, the odd_can_poweroff can be eliminated.

Signed-off-by Aaron Lu <[email protected]>

---
 drivers/ata/libata-zpodd.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

drivers/ata/libata-zpodd.c +2 ~~-19~~

index 88949c6d55dd..f3a65a3140d3 100644
@@ -85,21 +85,6 @@ static enum odd_mech_type zpodd_get_mech_type(struct ata_device *dev)
 		return ODD_MECH_TYPE_UNSUPPORTED;
 }
 
-static bool odd_can_poweroff(struct ata_device *ata_dev)
-{
-	acpi_handle handle;
-	struct acpi_device *acpi_dev;
-
-	handle = ata_dev_acpi_handle(ata_dev);
-	if (!handle)
-		return false;
-
-	if (acpi_bus_get_device(handle, &acpi_dev))
-		return false;
-
-	return acpi_device_can_poweroff(acpi_dev);
-}
-
 /* Test if ODD is zero power ready by sense code */
 static bool zpready(struct ata_device *dev)
 {

@@ -267,13 +252,11 @@ static void ata_acpi_remove_pm_notifier(struct ata_device *dev)
 
 void zpodd_init(struct ata_device *dev)
 {
+	struct acpi_device *adev = ACPI_COMPANION(&dev->tdev);
 	enum odd_mech_type mech_type;
 	struct zpodd *zpodd;
 
-	if (dev->zpodd)
-		return;
-
-	if (!odd_can_poweroff(dev))
+	if (dev->zpodd || !adev || !acpi_device_can_poweroff(adev))
 		return;
 
 	mech_type = zpodd_get_mech_type(dev);

Signature · 1 line

1.8.5.3

Tejun Heo

2014-03-14 15:24 UTC #9 lore raw

In reply to Aaron Lu

Quoted 7 lines · Aaron Lu

Two small fix for libata-acpi and libata-zpodd and one simplification for libata-zpodd.
Aaron Lu (3):
libata: zpodd: should depend on PM_RUNTIME
libata: acpi: avoid passing NULL to ACPI evaluation method
libata: zpodd: eliminate odd_can_poweroff

Applied 1-3 to libata/for-3.15 w/ stable cc dropped.

Thanks.

Signature · 1 line

tejun