[PATCH 0/3] ata: acpi/zpodd: some small fixes

[PATCH 0/3] ata: acpi/zpodd: some small fixes

[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

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

-- 
1.8.5.3

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

[Aaron Lu]

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 <aaron.lu@intel.com>
Cc: stable@vger.kernel.org
---
 drivers/ata/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 93fc2f052645..e4e608c72e01 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -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
-- 
1.8.5.3

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

[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 <aaron.lu@intel.com>
Cc: stable@vger.kernel.org
---
 drivers/ata/libata-acpi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 9e69a5308693..b4f7cc2522d9 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -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;
 		}
-- 
1.8.5.3

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

[Tejun Heo]

On Fri, Mar 14, 2014 at 01:46:09PM +0800, Aaron Lu wrote:
> 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.

-- 
tejun

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

[Aaron Lu]

On 03/14/2014 08:58 PM, Tejun Heo wrote:
> On Fri, Mar 14, 2014 at 01:46:09PM +0800, Aaron Lu wrote:
>> 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

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

[Aaron Lu]

On 03/14/2014 10:25 PM, Aaron Lu wrote:
> On 03/14/2014 08:58 PM, Tejun Heo wrote:
>> On Fri, Mar 14, 2014 at 01:46:09PM +0800, Aaron Lu wrote:
>>> 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.
>
> 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.

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

[Tejun Heo]

On Fri, Mar 14, 2014 at 10:32:08PM +0800, Aaron Lu wrote:
> On 03/14/2014 10:25 PM, Aaron Lu wrote:
> > On 03/14/2014 08:58 PM, Tejun Heo wrote:
> >> On Fri, Mar 14, 2014 at 01:46:09PM +0800, Aaron Lu wrote:
> >>> 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.
> >
> > 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?

-- 
tejun

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

[Aaron Lu]

On 03/14/2014 10:33 PM, Tejun Heo wrote:
> On Fri, Mar 14, 2014 at 10:32:08PM +0800, Aaron Lu wrote:
>> On 03/14/2014 10:25 PM, Aaron Lu wrote:
>>> On 03/14/2014 08:58 PM, Tejun Heo wrote:
>>>> On Fri, Mar 14, 2014 at 01:46:09PM +0800, Aaron Lu wrote:
>>>>> 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.
>>>
>>> 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.

[PATCH 3/3] libata: zpodd: eliminate odd_can_poweroff

[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 <aaron.lu@intel.com>
---
 drivers/ata/libata-zpodd.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/drivers/ata/libata-zpodd.c b/drivers/ata/libata-zpodd.c
index 88949c6d55dd..f3a65a3140d3 100644
--- a/drivers/ata/libata-zpodd.c
+++ b/drivers/ata/libata-zpodd.c
@@ -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);
-- 
1.8.5.3

Re: [PATCH 0/3] ata: acpi/zpodd: some small fixes

[Tejun Heo]

On Fri, Mar 14, 2014 at 01:46:07PM +0800, Aaron Lu wrote:
> 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.

-- 
tejun