lkml.org 
[lkml]   [2007]   [Jan]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 5/5] fixing errors handling during pci_driver resume stage [serial]
From
Date
Russell King <rmk+lkml@arm.linux.org.uk> writes:

> On Tue, Jan 09, 2007 at 12:01:58PM +0300, Dmitriy Monakhov wrote:
>> serial pci drivers have to return correct error code during resume stage in
>> case of errors.
>
> Sigh. *hate* *hate* *hate*.
>
>> diff --git a/drivers/serial/8250_pci.c b/drivers/serial/8250_pci.c
>> index 52e2e64..e26e4a6 100644
>> --- a/drivers/serial/8250_pci.c
>> +++ b/drivers/serial/8250_pci.c
>> @@ -1805,6 +1805,7 @@ static int pciserial_suspend_one(struct
>> static int pciserial_resume_one(struct pci_dev *dev)
>> {
>> struct serial_private *priv = pci_get_drvdata(dev);
>> + int err;
>>
>> pci_set_power_state(dev, PCI_D0);
>> pci_restore_state(dev);
>> @@ -1813,7 +1814,12 @@ static int pciserial_resume_one(struct p
>> /*
>> * The device may have been disabled. Re-enable it.
>> */
>> - pci_enable_device(dev);
>> + err = pci_enable_device(dev);
>> + if (err) {
>> + dev_err(&dev->dev, "Cannot enable PCI device, "
>> + "aborting.\n");
>> + return err;
>> + }
>>
>> pciserial_resume_ports(priv);
>> }
>
> So if pci_enable_device() fails, what do we do with the still suspended
> serial port? Does it clean up that state? Probably not.
>
> Look, merely going around bunging this stupid "oh lets propagate the
> error" crap into the kernel doesn't actually fix _anything_. In fact
> it potentially _hides_ the warnings produced by __must_check which
> give a hint that _something_ needs to be done to _properly_ fix the
> problem.
>
> And by "properly", I mean not just merely propagating the error.
>
> In this particular case, the above may result in resources not being
> freed.
Yep 100% true. But the question is _HOW_? We want shutdown not enabled device.
Is it safe just call pciserial_remove_ports() for this device?
>
> --
> Russell King
> Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
> maintainer of:
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-01-09 16:27    [W:0.034 / U:1.288 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site